Skip to content

Commit b5a6fa3

Browse files
Exceptions are raised instead of returning None
1 parent 8ad9379 commit b5a6fa3

File tree

2 files changed

+40
-42
lines changed

2 files changed

+40
-42
lines changed

pycti/api/opencti_api_client.py

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -298,18 +298,6 @@ def _setup_proxy_certificates(self):
298298

299299
# Determine if HTTPS_CA_CERTIFICATES contains inline content or file path
300300
cert_content = self._get_certificate_content(https_ca_certificates)
301-
if not cert_content:
302-
self.app_logger.warning(
303-
"Invalid HTTPS_CA_CERTIFICATES: not a valid certificate or file path",
304-
{
305-
"value": (
306-
https_ca_certificates[:50] + "..."
307-
if len(https_ca_certificates) > 50
308-
else https_ca_certificates
309-
)
310-
},
311-
)
312-
return
313301

314302
# Write proxy certificate to temp file
315303
proxy_cert_file = os.path.join(cert_dir, "proxy-ca.crt")
@@ -373,8 +361,9 @@ def _get_certificate_content(self, https_ca_certificates):
373361
374362
:param https_ca_certificates: Content from HTTPS_CA_CERTIFICATES env var
375363
:type https_ca_certificates: str
376-
:return: Certificate content in PEM format or None if invalid
377-
:rtype: str or None
364+
:return: Certificate content in PEM format
365+
:rtype: str
366+
:raises ValueError: If the certificate content is invalid or cannot be read
378367
"""
379368
# Strip whitespace once at the beginning
380369
stripped_https_ca_certificates = https_ca_certificates.strip()
@@ -400,20 +389,21 @@ def _get_certificate_content(self, https_ca_certificates):
400389
)
401390
return cert_content
402391
else:
403-
self.app_logger.warning(
404-
"File at HTTPS_CA_CERTIFICATES path does not contain valid certificate",
405-
{"file_path": cert_file_path},
392+
raise ValueError(
393+
f"File at HTTPS_CA_CERTIFICATES path does not contain valid certificate: {cert_file_path}"
406394
)
407-
return None
395+
except ValueError:
396+
# Re-raise ValueError from certificate validation
397+
raise
408398
except Exception as e:
409-
self.app_logger.warning(
410-
"Failed to read certificate file",
411-
{"file_path": cert_file_path, "error": str(e)},
399+
raise ValueError(
400+
f"Failed to read certificate file at {cert_file_path}: {str(e)}"
412401
)
413-
return None
414402

415403
# Neither inline content nor valid file path
416-
return None
404+
raise ValueError(
405+
f"HTTPS_CA_CERTIFICATES is not a valid certificate or file path: {https_ca_certificates[:50]}..."
406+
)
417407

418408
def set_applicant_id_header(self, applicant_id):
419409
self.request_headers["opencti-applicant-id"] = applicant_id

tests/01-unit/api/test_opencti_api_client-proxy.py

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -75,13 +75,11 @@ def test_get_certificate_content_invalid_file_content(self, api_client):
7575
invalid_file_path = invalid_file.name
7676

7777
try:
78-
result = api_client._get_certificate_content(invalid_file_path)
79-
80-
assert result is None
81-
api_client.app_logger.warning.assert_called_with(
82-
"File at HTTPS_CA_CERTIFICATES path does not contain valid certificate",
83-
{"file_path": invalid_file_path},
84-
)
78+
with pytest.raises(
79+
ValueError,
80+
match="File at HTTPS_CA_CERTIFICATES path does not contain valid certificate",
81+
):
82+
api_client._get_certificate_content(invalid_file_path)
8583
finally:
8684
# Clean up
8785
os.unlink(invalid_file_path)
@@ -90,15 +88,19 @@ def test_get_certificate_content_nonexistent_file(self, api_client):
9088
"""Test _get_certificate_content with a nonexistent file path."""
9189
nonexistent_path = "/tmp/nonexistent_certificate.crt"
9290

93-
result = api_client._get_certificate_content(nonexistent_path)
94-
95-
assert result is None
91+
with pytest.raises(
92+
ValueError,
93+
match="HTTPS_CA_CERTIFICATES is not a valid certificate or file path",
94+
):
95+
api_client._get_certificate_content(nonexistent_path)
9696

9797
def test_get_certificate_content_invalid_content(self, api_client):
9898
"""Test _get_certificate_content with invalid content (not PEM, not file)."""
99-
result = api_client._get_certificate_content(self.INVALID_CONTENT)
100-
101-
assert result is None
99+
with pytest.raises(
100+
ValueError,
101+
match="HTTPS_CA_CERTIFICATES is not a valid certificate or file path",
102+
):
103+
api_client._get_certificate_content(self.INVALID_CONTENT)
102104

103105
def test_get_certificate_content_whitespace_handling(self, api_client):
104106
"""Test _get_certificate_content handles whitespace correctly."""
@@ -174,13 +176,19 @@ def test_setup_proxy_certificates_with_invalid_path(self, mock_mkdtemp, api_clie
174176

175177
mock_mkdtemp.return_value = "/tmp/test_certs"
176178

177-
# Mock _get_certificate_content to return None (invalid)
178-
with patch.object(api_client, "_get_certificate_content", return_value=None):
179-
api_client._setup_proxy_certificates()
179+
# Mock _get_certificate_content to raise ValueError (invalid)
180+
with patch.object(
181+
api_client,
182+
"_get_certificate_content",
183+
side_effect=ValueError("Invalid certificate"),
184+
):
185+
with pytest.raises(ValueError, match="Invalid certificate"):
186+
api_client._setup_proxy_certificates()
180187

181-
# Should log warning and return early
182-
api_client.app_logger.warning.assert_called()
183-
assert not hasattr(api_client, "ssl_verify") or api_client.ssl_verify is False
188+
# Should log error before raising
189+
api_client.app_logger.error.assert_called_with(
190+
"Failed to setup proxy certificates", {"error": "Invalid certificate"}
191+
)
184192

185193
# Cleanup
186194
opencti_api_client._PROXY_CERT_BUNDLE = None

0 commit comments

Comments
 (0)