fix(backend): relax CORS regex and add debugging logs#26
fix(backend): relax CORS regex and add debugging logs#26
Conversation
- Update cloud_origin_regex in backend/main.py to allow `/?` at the end - Add Origin header logging in log_errors_middleware for better production debugging - Add test case for trailing slash origin in tests/test_cors_config.py - Update docs/IMPLEMENTATION_PLAN.md with status Co-authored-by: KnellBalm <90038472+KnellBalm@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Reviewer's GuideRelaxes the Cloud Run CORS origin regex to accept an optional trailing slash, adds targeted Origin-header logging for run.app traffic to aid CORS debugging, and extends CORS test coverage and documentation to cover the new behavior. Sequence diagram for CORS request handling with new Origin loggingsequenceDiagram
actor Browser
participant Frontend_run_app
participant CloudRun_Ingress
participant Backend_FastAPI
participant log_errors_middleware
participant CORS_Middleware
participant App_Handlers
Browser->>Frontend_run_app: Fetch API request with Origin https://query-craft-frontendXYZ.run.app/
Frontend_run_app->>CloudRun_Ingress: Forward request
CloudRun_Ingress->>Backend_FastAPI: HTTP request with Origin header
Backend_FastAPI->>log_errors_middleware: Process incoming request
log_errors_middleware->>log_errors_middleware: Read Origin header
alt Origin header contains run.app
log_errors_middleware->>Logging_backend_middleware_cors_debug: info Incoming Request Origin: {origin}
else Origin header missing_or_non_run_app
log_errors_middleware->>log_errors_middleware: Skip Origin logging
end
log_errors_middleware->>CORS_Middleware: Call next middleware
alt Origin matches cloud_origin_regex https://query-craft-frontend.*\.run\.app/?
CORS_Middleware->>App_Handlers: Allow request and forward to route handler
App_Handlers-->>CORS_Middleware: Response
CORS_Middleware-->>Browser: Response with CORS headers
else Origin does not match regex
CORS_Middleware-->>Browser: CORS error response
end
Flow diagram for Cloud Run frontend to backend CORS and logging pathflowchart LR
Browser[Browser] --> Frontend_run_app[Frontend Cloud Run service]
Frontend_run_app --> CloudRun_Ingress[Cloud Run ingress]
CloudRun_Ingress --> Backend_FastAPI[Backend FastAPI service]
Backend_FastAPI --> log_errors_middleware[log_errors_middleware]
log_errors_middleware --> CORS_Middleware[CORS middleware]
CORS_Middleware -->|Origin_matches_cloud_origin_regex_https_query_craft_frontend_run_app_optional_slash| App_Handlers[Application route handlers]
CORS_Middleware -->|Origin_does_not_match_regex| CORS_Error_Response[CORS error response]
log_errors_middleware -->|Origin_header_contains_run_app| Logging_backend_middleware_cors_debug[Logger backend_middleware_cors_debug]
log_errors_middleware -->|Origin_missing_or_not_run_app| CORS_Middleware
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The relaxed
cloud_origin_regex(https://query-craft-frontend.*\.run\.app/?) would be safer and more precise if it were anchored at the end (e.g.r"https://query-craft-frontend.*\.run\.app/?$") to avoid accidentally matching origins that only contain.run.appas a substring in a longer host. - Consider moving the
get_logger("backend.middleware.cors_debug")call outsidelog_errors_middleware(e.g. module-level) so the logger isn’t re-fetched on every request, reducing per-request overhead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The relaxed `cloud_origin_regex` (`https://query-craft-frontend.*\.run\.app/?`) would be safer and more precise if it were anchored at the end (e.g. `r"https://query-craft-frontend.*\.run\.app/?$"`) to avoid accidentally matching origins that only contain `.run.app` as a substring in a longer host.
- Consider moving the `get_logger("backend.middleware.cors_debug")` call outside `log_errors_middleware` (e.g. module-level) so the logger isn’t re-fetched on every request, reducing per-request overhead.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
- Update cloud_origin_regex to allow trailing slash - Add Origin logging in log_errors_middleware - Refactor tests/test_cors_config.py to use mock.patch.dict for environment isolation to fix CI failures in test_integration.py Co-authored-by: KnellBalm <90038472+KnellBalm@users.noreply.github.com>
This PR addresses a CORS issue reported in production where the frontend origin was being blocked.
Changes:
cloud_origin_regexinbackend/main.pyhas been updated tor"https://query-craft-frontend.*\.run\.app/?"to allow for an optional trailing slash. This ensures that even if the browser or a proxy appends a slash to the Origin header (an edge case), it will still be matched correctly.log_errors_middlewareto log theOriginheader for requests from*.run.appdomains. This will provide visibility into exactly what Origin header is reaching the backend in production, facilitating faster debugging if the issue persists.test_cors_origin_with_trailing_slashtotests/test_cors_config.pyto verify the regex fix. Existing tests were also run and passed.This change is safe as it only relaxes the regex slightly to cover valid variations of the frontend URL and adds harmless logging.
PR created automatically by Jules for task 7501668948331363069 started by @KnellBalm
Summary by Sourcery
Relax CORS origin matching and add CORS-focused diagnostics for Cloud Run traffic.
Bug Fixes:
Enhancements:
Documentation:
Tests: