fix(security): eliminate exception-detail leakage from 500 responses (CWE-209) by groupthinking · Pull Request #883 · groupthinking/EventRelay

groupthinking · 2026-07-19T05:06:28Z

Summary

Hardens the backend so that HTTP 500 responses never disclose internal exception
detail to callers (CWE-209 information disclosure), while preserving full
diagnostics server-side and keeping intentional 4xx client errors intact.

Changes

Static 500 bodies (no str(exc) / class name to the client):

backend/main.py — global Exception handler no longer returns str(exc) in
detail nor an error_type derived from exc.__class__.__name__; the full
exception (type, message, traceback) is logged via exc_info=True only.
backend/cloud_api_endpoints.py — process_video_cloud and
process_video_task_handler return a static "Internal server error" detail
instead of a dict echoing the exception message, video_url, and timestamp.
The persisted error_message written to Firestore state (surfaced to clients by
status/result endpoints) is also made generic.
backend/real_api_endpoints.py — process_video 500 path returns a static
string instead of a detail dict carrying the exception message and video_url.

Preserve intentional client errors (stop 4xx → 500 swallowing):

cloud_ai_routes.py (analyze_video_multi_provider) and
real_api_endpoints.py (batch_process_videos, search_youtube_videos) now
re-raise HTTPException before the broad except Exception, so a deliberate
400 (e.g. bad analysis_types) is no longer converted into a generic 500.

Diagnostics preserved: every touched handler logs with exc_info=True so the
full traceback remains available in server logs.

Tests

tests/unit/test_500_info_disclosure.py — expanded AST-based guard that fails if
any handler puts dynamic exception detail into a 500 body; includes synthetic
positive-control cases proving the guard actually detects leaks. Passing locally.
Updated test_backend_main.py, test_cloud_routes.py, test_real_api_endpoints.py
to assert the new static-body contract.

Verification

test_500_info_disclosure.py: 4 passed locally.
All changed files compile (py_compile).
Zero new lint introduced (ruff finding count on the four files is unchanged vs.
merge-base, and all present rule classes are already in CI's ignore list).

🤖 Generated with Claude Code

Generated by Claude Code

The source-scan guard added for the 500 info-disclosure work only matched inline detail=str(e) / detail=f"...{e}...". It silently passed while three handlers still leaked internal state through detail shapes it did not model: - cloud_api_endpoints.py process_video_cloud -> detail={... "message": error_msg ...} - cloud_api_endpoints.py process_video_task -> detail=error_msg (bare variable) - real_api_endpoints.py process_video_real_api -> detail={... "message": error_msg ...} Fix all three to a static "Internal server error" (the exception is logged server-side via logger.error(..., exc_info=True)), and rewrite the guard to flag any 500 detail= that is not an inline static string literal — a leading '{' (dict) or identifier char (f-string, str(), or a bare variable) now fails the scan. Self-checks extended to cover the dict and variable shapes. test_error_response_includes_video_url asserted the old leaking body; it is replaced by test_error_response_is_sanitized, which asserts the 500 body is exactly "Internal server error" and contains neither the exception nor the caller-supplied video_url. Strict superset of the #807 approach; the router.py/reporting_routes.py sinks #804 targeted are already clean on main and covered by the tree-wide guard.

…leak, 400→500 swallow Round of fixes from the CodeRabbit full review and the Vercel VADE bot on #815: - main.py global_exception_handler leaked str(exc) and the exception class name (error_type) in its 500 JSONResponse body (VADE finding). Return a static body; the full exception is already logged with exc_info=True. - cloud_api_endpoints.process_video_task_handler persisted error_msg (containing str(e)) as the Firestore 'error_message', which get_video_status / get_video_result return verbatim to clients — exfiltrating the exception despite the sanitized 500. Persist a generic message; keep the detail in logs only. - real_api_endpoints batch_process_videos and search_youtube_videos caught their own HTTPException(400) validation errors in the broad 'except Exception' and turned them into 500s. Re-raise HTTPException first to preserve the 400. - cloud_ai_routes: add exc_info=True to the five 500-handler logger.error calls so the traceback is retained server-side. - Guard: exception handlers are a second 500 sink the HTTPException scan didn't model. Add test_no_disclosure_in_500_exception_handlers — it flags any @app.exception_handler that returns 500 while placing str(exc)/str(e) or __class__.__name__ in the response body (docstrings/comments/log lines excluded).

…ler test - analyze_video_multi_provider caught HTTPException(400) from parse_analysis_types in its broad 'except Exception' and re-raised it as a generic 500, masking a client error (flagged in review). Add 'except HTTPException: raise' to mirror analyze_video / analyze_batch_videos, plus a regression test asserting an invalid analysis type returns 400. - Update the stale global-exception-handler test: it asserted the 500 body leaks 'error_type' (the exception class name), which the info-disclosure hardening deliberately removes (CWE-209). Assert the secure contract instead: neither the exception type nor its message appears in the response. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014KW6wTcE4r8Uk11QpXBhVq

coderabbitai · 2026-07-19T05:06:34Z

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (1)

[‘architecture-gap’, ‘bug’, ‘ci-cd’, ‘ci/cd’, ‘copilot-rabbit’, ‘documentation’, ‘duplicate’, ‘enhancement’, ‘frontend’, ‘github_actions’, ‘good first issue’, ‘help wanted’, ‘high-priority’, ‘invalid’, ‘javascript’, ‘ml-model’, ‘needs-triage’, ‘pipeline-critical’, ‘placeholder-code’, ‘priority:high’, ‘python’, ‘python:uv’, ‘question’, ‘styling’, ‘tests’, ‘v0’]

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: cbe316fc-a577-4a2a-87d9-62fbfc6f347a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch claude/determined-maxwell-uybuwc

✨ Simplify code

Create PR with simplified code
Commit simplified code in branch claude/determined-maxwell-uybuwc

_{Comment @coderabbitai help to get the list of available commands.}

vercel · 2026-07-19T05:06:36Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
v0-uvai	Ready	Preview, Comment, Open in v0	Jul 19, 2026 5:07am

github-actions · 2026-07-19T05:06:52Z

Agent Completion Truth Gate: BLOCKED

Reasons: invalid_payload

Machine-readable verdict

{
  "details": {
    "invalid_fields": [
      "issue.number",
      "policy.agent_login",
      "policy.run_id"
    ]
  },
  "reasons": [
    "invalid_payload"
  ],
  "verdict": "blocked"
}

Workflow evidence

groupthinking · 2026-07-19T05:11:25Z

Closing as redundant / superseded — no action needed.

This PR's head commit (53fa3fc) is identical to #818, which was closed unmerged. On investigation, the CWE-209 HTTP 500 info-disclosure hardening this branch implements is already present on main in a more complete form:

main.py global exception handler already returns a static body and logs via exc_info=True, and additionally strips the query string from the log line (uses request.url.path + method) to avoid persisting ?token=…-style secrets to Cloud Run logs — a refinement this branch lacks.
cloud_api_endpoints.py / real_api_endpoints.py already use static detail strings with the real error logged server-side only.
The except HTTPException: raise 4xx-preservation guards are already present in every relevant handler on main (plus a 503 path this branch doesn't have).

This branch is ~150 commits behind main and adds nothing main doesn't already have. Closing it rather than merging stale, conflicting duplicate work.

Generated by Claude Code

claude added 3 commits July 17, 2026 02:18

groupthinking closed this Jul 19, 2026

Navigation Menu

fix(security): eliminate exception-detail leakage from 500 responses (CWE-209) - #883

fix(security): eliminate exception-detail leakage from 500 responses (CWE-209)#883
groupthinking wants to merge 3 commits into
mainfrom
claude/determined-maxwell-uybuwc

groupthinking commented Jul 19, 2026

Uh oh!

coderabbitai Bot commented Jul 19, 2026

Review skipped

Uh oh!

vercel Bot commented Jul 19, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 19, 2026 •

edited

Loading

Uh oh!

groupthinking commented Jul 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

groupthinking commented Jul 19, 2026

Summary

Changes

Tests

Verification

Uh oh!

coderabbitai Bot commented Jul 19, 2026

Review skipped

Uh oh!

vercel Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Agent Completion Truth Gate: BLOCKED

Uh oh!

groupthinking commented Jul 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vercel Bot commented Jul 19, 2026 •

edited

Loading

github-actions Bot commented Jul 19, 2026 •

edited

Loading