From dbbf53918211f66b74b7e4ba28b4e7275bc0981d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 29 Mar 2026 21:14:02 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20resolve=20CodeQL=20security=20alerts=20?= =?UTF-8?q?=E2=80=93=20log=20injection,=20info=20exposure,=20incomplete=20?= =?UTF-8?q?URL=20checks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agent-Logs-Url: https://github.com/christianlouis/dmarq/sessions/093a5de2-644b-434d-8ce8-529710ecb40b Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- .../app/api/api_v1/endpoints/mail_sources.py | 43 +++++++++++++++---- backend/app/tests/test_mail_sources.py | 8 ++-- 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/backend/app/api/api_v1/endpoints/mail_sources.py b/backend/app/api/api_v1/endpoints/mail_sources.py index e79beab..6b6ce60 100644 --- a/backend/app/api/api_v1/endpoints/mail_sources.py +++ b/backend/app/api/api_v1/endpoints/mail_sources.py @@ -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 = ( "

Token exchange failed. " "Please close this window and try again.

" @@ -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)) diff --git a/backend/app/tests/test_mail_sources.py b/backend/app/tests/test_mail_sources.py index 32f736a..3f98f14 100644 --- a/backend/app/tests/test_mail_sources.py +++ b/backend/app/tests/test_mail_sources.py @@ -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