From 4f9e3b4b4bfcac260befe6a6766b4f4e58b876f2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 29 Mar 2026 23:43:14 +0000 Subject: [PATCH] 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> --- .env.example | 2 +- backend/app/core/security.py | 2 +- backend/app/main.py | 11 +++----- backend/app/tests/test_security.py | 41 ++++++++++++++++++++++++++++++ 4 files changed, 47 insertions(+), 9 deletions(-) diff --git a/.env.example b/.env.example index 13ff4e5..53086e4 100644 --- a/.env.example +++ b/.env.example @@ -14,7 +14,7 @@ SECRET_KEY="CHANGE_THIS_TO_A_RANDOM_SECRET_IN_PRODUCTION" # Admin API Key for the X-API-Key header (optional) # If set, this key is used instead of generating a random one at startup. # 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" # Environment (development/production) diff --git a/backend/app/core/security.py b/backend/app/core/security.py index 43bb8e5..c016dc3 100644 --- a/backend/app/core/security.py +++ b/backend/app/core/security.py @@ -76,7 +76,7 @@ def add_api_key(api_key: str) -> bool: if api_key in _api_keys: return False _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 diff --git a/backend/app/main.py b/backend/app/main.py index f6483d9..c0f7d9c 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -299,24 +299,21 @@ def create_app() -> FastAPI: if settings.ADMIN_API_KEY: api_key = settings.ADMIN_API_KEY add_api_key(api_key) - key_suffix = api_key[-8:] if len(api_key) >= 8 else api_key logger.info( "Admin API key loaded from ADMIN_API_KEY environment variable " - "(ends with: ...%s).", - key_suffix, # lgtm[py/clear-text-logging-sensitive-data] + "(length: %d chars).", + len(api_key), ) else: api_key = generate_api_key() add_api_key(api_key) - # Security: Log only last 8 characters for reference logger.warning( "%s\nIMPORTANT: Admin API Key Generated\n" - "API Key (last 8 chars): ...%s\n" - "Full key stored securely in memory.\n" + "Key length: %d chars. Full key stored securely in memory.\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", "=" * 80, - api_key[-8:], # lgtm[py/clear-text-logging-sensitive-data] + len(api_key), "=" * 80, ) diff --git a/backend/app/tests/test_security.py b/backend/app/tests/test_security.py index f83b477..ff4da99 100644 --- a/backend/app/tests/test_security.py +++ b/backend/app/tests/test_security.py @@ -5,9 +5,11 @@ Covers API key management, domain validation, file upload limits, and XML parsin """ import pytest +from fastapi.testclient import TestClient import app.services.dmarc_parser as parser_module 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.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 except Exception: # pylint: disable=broad-exception-caught 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)