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