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

src/youtube_extension/backend/cloud_ai_routes.py

@@ -228,7 +228,7 @@ async def get_provider_status():
             )
 
     except Exception as e:
-        logger.error(f"Failed to get provider status: {e}")
+        logger.error(f"Failed to get provider status: {e}", exc_info=True)
         raise HTTPException(status_code=500, detail="Internal server error")
 
 
@@ -262,12 +262,12 @@ async def analyze_video(request: VideoAnalysisRequest):
         logger.warning(f"Rate limit exceeded: {e}")
         raise HTTPException(status_code=429, detail=f"Rate limit exceeded: {str(e)}")
     except ConfigurationError as e:
-        logger.error(f"Configuration error: {e}")
+        logger.error(f"Configuration error: {e}", exc_info=True)
         raise HTTPException(status_code=500, detail="Internal server error")
     except HTTPException:
         raise
     except Exception as e:
-        logger.error(f"Unexpected error during video analysis: {e}")
+        logger.error(f"Unexpected error during video analysis: {e}", exc_info=True)
         raise HTTPException(status_code=500, detail="Internal server error")
 
 
@@ -300,7 +300,7 @@ async def analyze_batch_videos(request: BatchAnalysisRequest, background_tasks:
     except HTTPException:
         raise
     except Exception as e:
-        logger.error(f"Failed to start batch analysis: {e}")
+        logger.error(f"Failed to start batch analysis: {e}", exc_info=True)
         raise HTTPException(status_code=500, detail="Internal server error")
 
 
@@ -323,8 +323,12 @@ async def analyze_video_multi_provider(request: VideoAnalysisRequest):
 
             return formatted_results
 
+    except HTTPException:
+        # Preserve client-facing status codes (e.g. 400 from parse_analysis_types);
+        # do not let the broad handler below convert them into a generic 500.
+        raise
     except Exception as e:
-        logger.error(f"Multi-provider analysis failed: {e}")
+        logger.error(f"Multi-provider analysis failed: {e}", exc_info=True)
         raise HTTPException(status_code=500, detail="Internal server error")
 
 

src/youtube_extension/backend/cloud_api_endpoints.py

@@ -142,18 +142,10 @@ async def process_video_cloud(
             )
 
     except Exception as e:
-        error_msg = f"Cloud processing failed: {str(e)}"
-        logger.error(error_msg)
+        logger.error(f"Cloud processing failed: {e}", exc_info=True)
 
-        raise HTTPException(
-            status_code=500,
-            detail={
-                "error": "cloud_processing_failed",
-                "message": error_msg,
-                "video_url": request.video_url,
-                "timestamp": datetime.now(timezone.utc).isoformat()
-            }
-        )
+        # detail must be a static string — the exception is logged above only.
+        raise HTTPException(status_code=500, detail="Internal server error")
 
 @router.post("/api/v3/process-video-task")
 async def process_video_task_handler(
@@ -216,20 +208,23 @@ async def process_video_task_handler(
 
     except Exception as e:
         error_msg = f"Task processing failed: {str(e)}"
-        logger.error(error_msg)
+        logger.error(error_msg, exc_info=True)
 
         # Update state with error
         try:
             firestore_service = await get_firestore_service()
+            # error_message is returned to clients by the status/result endpoints,
+            # so it must stay generic — the full error_msg is in the logs above only.
             await firestore_service.update_state(
                 payload.video_id,
                 status='failed',
-                error_message=error_msg
+                error_message="Internal server error"
             )
         except Exception as state_error:
             logger.error(f"Failed to update error state: {state_error}")
 
-        raise HTTPException(status_code=500, detail=error_msg)
+        # detail must be a static string — error_msg (with the exception) is logged above only.
+        raise HTTPException(status_code=500, detail="Internal server error")
 
 @router.post("/api/v3/batch-process")
 async def batch_process_videos_cloud(request: BatchCloudProcessingRequest):

src/youtube_extension/backend/main.py

@@ -444,21 +444,24 @@ async def value_error_handler(request, exc):
 
 @app.exception_handler(Exception)
 async def global_exception_handler(request, exc):
-    """Global exception handler with enhanced error details"""
+    """Global handler for unhandled exceptions.
+
+    The full exception — type, message, and traceback — is logged server-side
+    only. The client receives a static body: neither the exception message
+    (``str(exc)``) nor its class name may be disclosed, as both leak internal
+    state to the caller (CWE-209 information disclosure).
+    """
     logger.error(f"Unhandled exception: {exc}", exc_info=True)
 
     error_detail = {
         "error": "Internal server error",
-        "detail": str(exc),
+        "detail": "Internal server error",
         "timestamp": datetime.now().isoformat(),
         "path": str(request.url) if hasattr(request, "url") else "unknown",
         "version": "2.0.0",
         "architecture": "service-oriented",
     }
 
-    if hasattr(exc, "__class__"):
-        error_detail["error_type"] = exc.__class__.__name__
-
     return JSONResponse(status_code=500, content=error_detail)
 
 

src/youtube_extension/backend/real_api_endpoints.py

@@ -118,18 +118,10 @@ async def process_video_real_api(request: VideoProcessingRequest, background_tas
             return response
 
         except Exception as e:
-            error_msg = f"Real API processing failed: {str(e)}"
-            logger.error(error_msg)
+            logger.error(f"Real API processing failed: {e}", exc_info=True)
 
-            raise HTTPException(
-                status_code=500,
-                detail={
-                    "error": "video_processing_failed",
-                    "message": error_msg,
-                    "video_url": request.video_url,
-                    "timestamp": datetime.now(timezone.utc).isoformat()
-                }
-            )
+            # detail must be a static string — the exception is logged above only.
+            raise HTTPException(status_code=500, detail="Internal server error")
 
     @app.post("/api/v2/validate-video")
     async def validate_video_url(request: VideoValidationRequest):
@@ -177,6 +169,9 @@ async def batch_process_videos(request: BatchProcessingRequest):
 
             return result
 
+        except HTTPException:
+            # Preserve intentional client errors (e.g. the 400 above).
+            raise
         except Exception as e:
             logger.error(f"Unhandled error in batch_process_videos: {e}", exc_info=True)
             raise HTTPException(
@@ -435,6 +430,9 @@ async def search_youtube_videos(
                 "timestamp": datetime.now(timezone.utc).isoformat()
             }
 
+        except HTTPException:
+            # Preserve intentional client errors (e.g. the 400 above).
+            raise
         except Exception as e:
             logger.error(f"Unhandled error in search_youtube_videos: {e}", exc_info=True)
             raise HTTPException(

tests/unit/test_500_info_disclosure.py

@@ -28,16 +28,17 @@
 # tolerant of the call spanning multiple lines.
 _HTTP_EXC = re.compile(r"HTTPException\((?P<args>.*?)\)", re.DOTALL)
 
-# A `detail=` argument whose value is derived from a variable/exception:
-#   detail=str(e)              detail=str(exc)
-#   detail=f"... {e} ..."      detail=f"... {str(e)} ..."
-_DYNAMIC_DETAIL = re.compile(
-    r"""detail\s*=\s*(?:
-        str\(                         # detail=str(...)
-      | f["'][^"']*\{                 # detail=f"...{...}..."
-    )""",
-    re.VERBOSE,
-)
+# A 500 `detail=` is safe only when it is an inline *static string literal*
+# (``detail="Internal server error"``). Anything else can carry internal state to
+# the client and is flagged:
+#   detail=str(e)          detail=f"... {e} ..."      (inline dynamic string)
+#   detail=error_msg       (a variable — may hold f"...{e}...")
+#   detail={...}           (a dict whose values embed the exception)
+# The value after ``detail=`` is dynamic unless its first non-space character
+# opens a plain string literal (``"`` or ``'``). A leading ``{`` (dict) or any
+# identifier char — ``f`` of an f-string, ``s`` of ``str(``, or a bare variable
+# name — means it is not a static literal.
+_DYNAMIC_DETAIL = re.compile(r"""detail\s*=\s*(?:\{|[A-Za-z_])""")
 
 
 def _backend_python_files() -> list[Path]:
@@ -59,6 +60,62 @@ def _iter_500_dynamic_detail(text: str):
             yield line_no, " ".join(args.split())[:120]
 
 
+# FastAPI exception handlers are a second 500 sink the HTTPException scan above
+# does not model: they build a response body directly (dict / JSONResponse) rather
+# than raising. A handler must not place the exception message (`str(exc)`) or its
+# class name (`exc.__class__.__name__`) into that body — both leak internal state.
+# Logging the exception server-side is fine; those tokens appear only in response
+# construction, never in a `logger.`/`log`/`raise`/comment line, so we exclude
+# those lines to avoid false positives.
+_HANDLER_DECORATOR = re.compile(r"^\s*@\w+\.exception_handler\(", re.MULTILINE)
+_HANDLER_DISCLOSURE = re.compile(r"str\(\s*(?:exc|e)\s*\)|__class__\.__name__")
+# Triple-quoted docstrings, so prose that *mentions* str(exc) is not mistaken for code.
+_TRIPLE_STR = re.compile(r'"""(?:.|\n)*?"""|\'\'\'(?:.|\n)*?\'\'\'')
+
+
+def _strip_docstrings(body: str) -> str:
+    """Blank triple-quoted blocks, preserving line count so line numbers stay aligned."""
+    return _TRIPLE_STR.sub(lambda m: "\n" * m.group(0).count("\n"), body)
+
+
+def _exception_handler_bodies(text: str):
+    """Yield (start_line, body_text) for each @<app>.exception_handler function."""
+    lines = text.splitlines(keepends=True)
+    for m in _HANDLER_DECORATOR.finditer(text):
+        start_line = text.count("\n", 0, m.start())
+        # Find the `def`/`async def` line that the decorator applies to.
+        i = start_line
+        while i < len(lines) and not lines[i].lstrip().startswith(("def ", "async def ")):
+            i += 1
+        if i >= len(lines):
+            continue
+        def_indent = len(lines[i]) - len(lines[i].lstrip())
+        j = i + 1
+        body: list[str] = []
+        while j < len(lines):
+            line = lines[j]
+            stripped = line.strip()
+            if stripped and (len(line) - len(line.lstrip())) <= def_indent:
+                break  # dedented back to <= the def's level: end of function
+            body.append(line)
+            j += 1
+        yield i + 1, "".join(body)
+
+
+def _iter_handler_disclosures(text: str):
+    """Yield (line_no, snippet) for exception-handler bodies that leak exc into the response."""
+    for start_line, body in _exception_handler_bodies(text):
+        if "status_code=500" not in re.sub(r"\s+", "", body):
+            continue
+        for offset, line in enumerate(_strip_docstrings(body).splitlines()):
+            code = line.split("#", 1)[0]  # ignore inline comments
+            bare = line.strip()
+            if bare.startswith(("logger", "log", "self.logger", "raise")):
+                continue
+            if _HANDLER_DISCLOSURE.search(code):
+                yield start_line + offset, bare[:120]
+
+
 def test_no_dynamic_detail_in_500_responses() -> None:
     offenders: list[str] = []
     for path in _backend_python_files():
@@ -75,16 +132,75 @@ def test_no_dynamic_detail_in_500_responses() -> None:
     )
 
 
+def test_no_disclosure_in_500_exception_handlers() -> None:
+    offenders: list[str] = []
+    for path in _backend_python_files():
+        text = path.read_text(encoding="utf-8")
+        for line_no, snippet in _iter_handler_disclosures(text):
+            rel = path.relative_to(_BACKEND.parents[2])
+            offenders.append(f"{rel}:{line_no}: {snippet}")
+
+    assert not offenders, (
+        "A FastAPI exception handler that returns HTTP 500 must not place the "
+        "exception message (`str(exc)`) or its class name (`__class__.__name__`) "
+        "into the response body — log it server-side instead. Offending sites:\n  "
+        + "\n  ".join(offenders)
+    )
+
+
+def test_guard_detects_a_synthetic_handler_leak() -> None:
+    """The exception-handler scanner flags exc message/class-name disclosure in a 500 body."""
+    leaky = (
+        "@app.exception_handler(Exception)\n"
+        "async def h(request, exc):\n"
+        '    logger.error(f"boom: {exc}", exc_info=True)\n'
+        '    body = {"detail": str(exc), "error_type": exc.__class__.__name__}\n'
+        "    return JSONResponse(status_code=500, content=body)\n"
+    )
+    assert list(_iter_handler_disclosures(leaky)), "handler scanner missed a real leak"
+
+    safe = (
+        "@app.exception_handler(Exception)\n"
+        "async def h(request, exc):\n"
+        '    logger.error(f"boom: {exc}", exc_info=True)\n'
+        '    body = {"detail": "Internal server error"}\n'
+        "    return JSONResponse(status_code=500, content=body)\n"
+    )
+    assert not list(
+        _iter_handler_disclosures(safe)
+    ), "handler scanner false-positived a sanitized 500 handler"
+
+    # A 4xx handler may echo the exception (client-supplied validation input).
+    client_err = (
+        "@app.exception_handler(ValueError)\n"
+        "async def h(request, exc):\n"
+        '    return JSONResponse(status_code=400, content={"detail": str(exc)})\n'
+    )
+    assert not list(
+        _iter_handler_disclosures(client_err)
+    ), "handler scanner must ignore 4xx handlers"
+
+
 def test_guard_detects_a_synthetic_leak() -> None:
-    """Sanity check: the scanner actually flags a dynamic 500 detail."""
-    leaky = "raise HTTPException(status_code=500, detail=str(e))"
+    """Sanity check: the scanner flags every dynamic-detail shape, not just str(e)."""
+    # Each of these leaks internal state and must be flagged.
+    leaks = [
+        "raise HTTPException(status_code=500, detail=str(e))",
+        'raise HTTPException(status_code=500, detail=f"failed: {e}")',
+        "raise HTTPException(status_code=500, detail=error_msg)",  # bare variable
+        'raise HTTPException(status_code=500, detail={"message": error_msg})',  # dict
+    ]
+    for leak in leaks:
+        assert list(_iter_500_dynamic_detail(leak)), f"scanner missed a real 500 leak: {leak}"
+
+    # A static string literal is the only safe form.
     safe = 'raise HTTPException(status_code=500, detail="Internal server error")'
-    client_err = "raise HTTPException(status_code=400, detail=str(exc))"
-
-    assert list(_iter_500_dynamic_detail(leaky)), "scanner missed a real 500 leak"
     assert not list(
         _iter_500_dynamic_detail(safe)
     ), "scanner false-positived a static detail"
+
+    # 4xx responses echo client-supplied input and are intentionally out of scope.
+    client_err = "raise HTTPException(status_code=400, detail=str(exc))"
     assert not list(
         _iter_500_dynamic_detail(client_err)
     ), "scanner must ignore 4xx responses"

tests/unit/test_backend_main.py

@@ -590,14 +590,19 @@ async def test_global_exception_handler_returns_500(self):
         response = await main_module.global_exception_handler(mock_req, RuntimeError("crash"))
         assert response.status_code == 500
 
-    async def test_global_exception_handler_includes_error_type(self):
+    async def test_global_exception_handler_omits_internal_details(self):
+        """The 500 body must not leak the exception class name or message (CWE-209)."""
         import json as _json
 
         mock_req = MagicMock()
         mock_req.url = "http://test/api"
         response = await main_module.global_exception_handler(mock_req, RuntimeError("crash"))
         body = _json.loads(response.body)
-        assert body["error_type"] == "RuntimeError"
+        # The exception type must not be disclosed to the client...
+        assert "error_type" not in body
+        # ...nor may the exception message appear anywhere in the response.
+        assert "crash" not in _json.dumps(body)
+        assert "RuntimeError" not in _json.dumps(body)
 
     async def test_global_exception_handler_includes_version(self):
         import json as _json

tests/unit/test_cloud_routes.py

@@ -485,6 +485,16 @@ def test_analyze_video_multi_provider_error(self):
             })
         assert response.status_code == 500
 
+    def test_analyze_video_multi_provider_invalid_analysis_type(self):
+        """A bad analysis type is a client error: the 400 from parse_analysis_types
+        must be preserved, not swallowed into a 500 by the broad exception handler."""
+        client = TestClient(_make_cloud_ai_app())
+        response = client.post("/api/v1/cloud-ai/analyze/multi-provider", json={
+            "video_url": "https://www.youtube.com/watch?v=auJzb1D-fag",
+            "analysis_types": ["invalid_type"],
+        })
+        assert response.status_code == 400
+
 
 # ===========================================================================
 # Tests for cloud_api_endpoints.py