Fix CodeQL clear-text logging alerts and improve startup test coverage
Agent-Logs-Url: https://github.com/christianlouis/dmarq/sessions/3d5dbbdc-99d5-4dc3-8ca9-a763ba917ae4 Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
This commit is contained in:
+1
-1
@@ -14,7 +14,7 @@ SECRET_KEY="CHANGE_THIS_TO_A_RANDOM_SECRET_IN_PRODUCTION"
|
|||||||
# Admin API Key for the X-API-Key header (optional)
|
# Admin API Key for the X-API-Key header (optional)
|
||||||
# If set, this key is used instead of generating a random one at startup.
|
# If set, this key is used instead of generating a random one at startup.
|
||||||
# Generate with: openssl rand -hex 32
|
# Generate with: openssl rand -hex 32
|
||||||
# If not set, a random key is generated each restart (logged with last 8 chars).
|
# If not set, a random key is generated each restart (key length logged only).
|
||||||
# ADMIN_API_KEY="your_admin_api_key_here"
|
# ADMIN_API_KEY="your_admin_api_key_here"
|
||||||
|
|
||||||
# Environment (development/production)
|
# Environment (development/production)
|
||||||
|
|||||||
@@ -76,7 +76,7 @@ def add_api_key(api_key: str) -> bool:
|
|||||||
if api_key in _api_keys:
|
if api_key in _api_keys:
|
||||||
return False
|
return False
|
||||||
_api_keys.add(api_key)
|
_api_keys.add(api_key)
|
||||||
logger.info("API key added (ends with: ...%s)", api_key[-8:]) # lgtm[py/clear-text-logging-sensitive-data]
|
logger.info("API key added (length: %d chars)", len(api_key))
|
||||||
return True
|
return True
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
+4
-7
@@ -299,24 +299,21 @@ def create_app() -> FastAPI:
|
|||||||
if settings.ADMIN_API_KEY:
|
if settings.ADMIN_API_KEY:
|
||||||
api_key = settings.ADMIN_API_KEY
|
api_key = settings.ADMIN_API_KEY
|
||||||
add_api_key(api_key)
|
add_api_key(api_key)
|
||||||
key_suffix = api_key[-8:] if len(api_key) >= 8 else api_key
|
|
||||||
logger.info(
|
logger.info(
|
||||||
"Admin API key loaded from ADMIN_API_KEY environment variable "
|
"Admin API key loaded from ADMIN_API_KEY environment variable "
|
||||||
"(ends with: ...%s).",
|
"(length: %d chars).",
|
||||||
key_suffix, # lgtm[py/clear-text-logging-sensitive-data]
|
len(api_key),
|
||||||
)
|
)
|
||||||
else:
|
else:
|
||||||
api_key = generate_api_key()
|
api_key = generate_api_key()
|
||||||
add_api_key(api_key)
|
add_api_key(api_key)
|
||||||
# Security: Log only last 8 characters for reference
|
|
||||||
logger.warning(
|
logger.warning(
|
||||||
"%s\nIMPORTANT: Admin API Key Generated\n"
|
"%s\nIMPORTANT: Admin API Key Generated\n"
|
||||||
"API Key (last 8 chars): ...%s\n"
|
"Key length: %d chars. Full key stored securely in memory.\n"
|
||||||
"Full key stored securely in memory.\n"
|
|
||||||
"Set ADMIN_API_KEY in your environment to use a fixed key across restarts.\n"
|
"Set ADMIN_API_KEY in your environment to use a fixed key across restarts.\n"
|
||||||
"Use this key in the X-API-Key header for admin endpoints.\n%s",
|
"Use this key in the X-API-Key header for admin endpoints.\n%s",
|
||||||
"=" * 80,
|
"=" * 80,
|
||||||
api_key[-8:], # lgtm[py/clear-text-logging-sensitive-data]
|
len(api_key),
|
||||||
"=" * 80,
|
"=" * 80,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|||||||
@@ -5,9 +5,11 @@ Covers API key management, domain validation, file upload limits, and XML parsin
|
|||||||
"""
|
"""
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
from fastapi.testclient import TestClient
|
||||||
|
|
||||||
import app.services.dmarc_parser as parser_module
|
import app.services.dmarc_parser as parser_module
|
||||||
from app.core.security import add_api_key, generate_api_key, verify_api_key
|
from app.core.security import add_api_key, generate_api_key, verify_api_key
|
||||||
|
from app.main import create_app
|
||||||
from app.services.dmarc_parser import DMARCParser
|
from app.services.dmarc_parser import DMARCParser
|
||||||
from app.utils.domain_validator import validate_domain, validate_domain_config
|
from app.utils.domain_validator import validate_domain, validate_domain_config
|
||||||
|
|
||||||
@@ -151,3 +153,42 @@ class TestXMLParsingSecurity:
|
|||||||
assert "root:" not in org_name and "/bin" not in org_name
|
assert "root:" not in org_name and "/bin" not in org_name
|
||||||
except Exception: # pylint: disable=broad-exception-caught
|
except Exception: # pylint: disable=broad-exception-caught
|
||||||
pass # Expected – defusedxml blocks DTD processing
|
pass # Expected – defusedxml blocks DTD processing
|
||||||
|
|
||||||
|
|
||||||
|
class TestAdminApiKeyStartup:
|
||||||
|
"""Test admin API key loading during the application startup event."""
|
||||||
|
|
||||||
|
def test_startup_uses_env_api_key(self, monkeypatch):
|
||||||
|
"""When ADMIN_API_KEY is configured, startup should register it directly."""
|
||||||
|
import app.core.security as sec_module
|
||||||
|
import app.main as main_module
|
||||||
|
|
||||||
|
test_key = "a" * 64
|
||||||
|
monkeypatch.setattr(main_module.settings, "ADMIN_API_KEY", test_key)
|
||||||
|
|
||||||
|
saved_keys = set(sec_module._api_keys)
|
||||||
|
sec_module._api_keys.clear()
|
||||||
|
try:
|
||||||
|
application = create_app()
|
||||||
|
with TestClient(application):
|
||||||
|
assert sec_module.verify_api_key(test_key)
|
||||||
|
finally:
|
||||||
|
sec_module._api_keys.clear()
|
||||||
|
sec_module._api_keys.update(saved_keys)
|
||||||
|
|
||||||
|
def test_startup_generates_key_when_no_env(self, monkeypatch):
|
||||||
|
"""When ADMIN_API_KEY is not set, startup should generate a random key."""
|
||||||
|
import app.core.security as sec_module
|
||||||
|
import app.main as main_module
|
||||||
|
|
||||||
|
monkeypatch.setattr(main_module.settings, "ADMIN_API_KEY", None)
|
||||||
|
|
||||||
|
saved_keys = set(sec_module._api_keys)
|
||||||
|
sec_module._api_keys.clear()
|
||||||
|
try:
|
||||||
|
application = create_app()
|
||||||
|
with TestClient(application):
|
||||||
|
assert len(sec_module._api_keys) == 1
|
||||||
|
finally:
|
||||||
|
sec_module._api_keys.clear()
|
||||||
|
sec_module._api_keys.update(saved_keys)
|
||||||
|
|||||||
Reference in New Issue
Block a user