fix: resolve CodeQL security alerts – log injection, info exposure, incomplete URL checks

Agent-Logs-Url: https://github.com/christianlouis/dmarq/sessions/093a5de2-644b-434d-8ce8-529710ecb40b Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
2026-03-29 21:14:02 +00:00
parent 0c05ded74f
commit dbbf539182
2 changed files with 39 additions and 12 deletions
@@ -25,6 +25,15 @@ router = APIRouter()
 logger = logging.getLogger(__name__)


+def _sanitize_log_value(value: object) -> str:
+    """Strip newline/carriage-return characters from a value before logging.
+
+    Prevents log-injection attacks where a user-provided string contains
+    embedded newlines that forge additional log lines.
+    """
+    return str(value).replace("\n", "\\n").replace("\r", "\\r")
+
+
 # ---------------------------------------------------------------------------
 # Pydantic schemas
 # ---------------------------------------------------------------------------
@@ -292,9 +301,14 @@ async def test_stored_mail_source(
                "timestamp": datetime.now().isoformat(),
            }
        except Exception as exc:  # pylint: disable=broad-exception-caught
+            logger.error(
+                "Gmail API test failed for source id=%d: %s",
+                source_id,
+                _sanitize_log_value(exc),
+            )
            return {
                "success": False,
-                "message": f"Gmail API test failed: {exc}",
+                "message": "Gmail API test failed. Check server logs for details.",
                "timestamp": datetime.now().isoformat(),
            }

@@ -459,7 +473,11 @@ async def gmail_oauth_callback(
            redirect_uri=redirect_uri,
        )
    except ValueError as exc:
-        logger.error("Gmail token exchange error for source id=%d: %s", source_id, exc)
+        logger.error(
+            "Gmail token exchange error for source id=%d: %s",
+            source_id,
+            _sanitize_log_value(exc),
+        )
        html = (
            "<html><body><p>Token exchange failed. "
            "Please close this window and try again.</p></body></html>"
@@ -488,7 +506,7 @@ async def gmail_oauth_callback(
    logger.info(
        "Gmail OAuth2 authorisation complete for source id=%d (account=%s)",
        source_id,
-        gmail_email or "unknown",
+        _sanitize_log_value(gmail_email or "unknown"),
    )

    html = (
@@ -560,7 +578,7 @@ async def gmail_oauth_callback_post(
    logger.info(
        "Gmail OAuth2 tokens saved for source id=%d (account=%s)",
        source_id,
-        gmail_email or "unknown",
+        _sanitize_log_value(gmail_email or "unknown"),
    )
    return _source_to_response(source)

@@ -617,17 +635,24 @@ async def gmail_fetch_reports(

    logger.info(
        "Gmail fetch for source id=%d: processed=%d reports_found=%d",
-        source_id,
-        results.get("processed", 0),
-        results.get("reports_found", 0),
+        int(source_id),
+        int(results.get("processed", 0)),
+        int(results.get("reports_found", 0)),
    )

+    for err in results.get("errors", []):
+        logger.warning(
+            "Gmail fetch warning for source id=%d: %s",
+            int(source_id),
+            _sanitize_log_value(err),
+        )
+
    return {
        "success": results.get("success", False),
        "processed": results.get("processed", 0),
        "reports_found": results.get("reports_found", 0),
        "new_domains": results.get("new_domains", []),
-        "errors": results.get("errors", []) or None,
+        "error_count": len(results.get("errors", [])),
        "timestamp": datetime.now().isoformat(),
    }

@@ -652,4 +677,4 @@ async def gmail_disconnect(
    source.gmail_email = None
    source.updated_at = datetime.utcnow()
    db.commit()
-    logger.info("Gmail tokens cleared for source id=%d", source_id)
+    logger.info("Gmail tokens cleared for source id=%d", int(source_id))
@@ -4,6 +4,7 @@ Tests for MailSource model and mail-sources API endpoints.

 import asyncio
 from unittest.mock import MagicMock, patch
+from urllib.parse import parse_qs, urlparse

 import pytest
 from fastapi.testclient import TestClient
@@ -558,8 +559,9 @@ class TestGmailAPIMailSource:
        assert resp.status_code == 200
        data = resp.json()
        assert "authorization_url" in data
-        assert "accounts.google.com" in data["authorization_url"]
-        assert "123-abc.apps.googleusercontent.com" in data["authorization_url"]
+        _parsed = urlparse(data["authorization_url"])
+        assert _parsed.hostname == "accounts.google.com"
+        assert parse_qs(_parsed.query).get("client_id") == ["123-abc.apps.googleusercontent.com"]
        assert "gmail.readonly" in data["authorization_url"]

    def test_gmail_disconnect_clears_tokens(self, authed_client: TestClient):
@@ -694,7 +696,7 @@ class TestGmailClientHelpers:
            redirect_uri="https://example.com/callback",
            state="42",
        )
-        assert "accounts.google.com" in url
+        assert urlparse(url).hostname == "accounts.google.com"
        assert "test-client-id" in url
        assert "gmail.readonly" in url
        assert "offline" in url