diff --git a/src/youtube_extension/backend/api/advanced_video_routes.py b/src/youtube_extension/backend/api/advanced_video_routes.py index 1d66b0bee..69f153e08 100644 --- a/src/youtube_extension/backend/api/advanced_video_routes.py +++ b/src/youtube_extension/backend/api/advanced_video_routes.py @@ -143,7 +143,7 @@ async def analyze_segment(request: TemporalSegmentRequest): } except Exception as e: logger.error(f"Segment analysis failed: {e}", exc_info=True) - raise HTTPException(500, str(e)) + raise HTTPException(status_code=500, detail="Internal server error") @router.post("/temporal/events") @@ -211,7 +211,7 @@ async def extract_temporal_events(request: TemporalEventsRequest): } except Exception as e: logger.error(f"Event extraction failed: {e}", exc_info=True) - raise HTTPException(500, str(e)) + raise HTTPException(status_code=500, detail="Internal server error") @router.post("/temporal/question") @@ -244,7 +244,7 @@ async def answer_temporal_question(request: TemporalQuestionRequest): } except Exception as e: logger.error(f"Temporal question failed: {e}", exc_info=True) - raise HTTPException(500, str(e)) + raise HTTPException(status_code=500, detail="Internal server error") @router.post("/temporal/timeline") @@ -285,7 +285,7 @@ async def create_timeline(request: TimelineRequest): raise except Exception as e: logger.error(f"Timeline creation failed: {e}", exc_info=True) - raise HTTPException(500, str(e)) + raise HTTPException(status_code=500, detail="Internal server error") @router.post("/temporal/compare-segments") @@ -319,7 +319,7 @@ async def compare_segments(request: SegmentComparisonRequest): } except Exception as e: logger.error(f"Segment comparison failed: {e}", exc_info=True) - raise HTTPException(500, str(e)) + raise HTTPException(status_code=500, detail="Internal server error") @router.post("/temporal/tutorial-steps") @@ -347,7 +347,7 @@ async def extract_tutorial_steps(request: TutorialStepsRequest): } except Exception as e: logger.error(f"Tutorial extraction failed: {e}", exc_info=True) - raise HTTPException(500, str(e)) + raise HTTPException(status_code=500, detail="Internal server error") # ============ Structured Output Endpoint ============ @@ -435,7 +435,7 @@ async def analyze_with_schema(request: StructuredAnalysisRequest): } except Exception as e: logger.error(f"Structured analysis failed: {e}", exc_info=True) - raise HTTPException(500, str(e)) + raise HTTPException(status_code=500, detail="Internal server error") # ============ CloudEvents Publishing Endpoint ============ @@ -488,4 +488,4 @@ async def publish_video_event( } except Exception as e: logger.error(f"Event publishing failed: {e}", exc_info=True) - raise HTTPException(500, str(e)) + raise HTTPException(status_code=500, detail="Internal server error") diff --git a/src/youtube_extension/backend/cloud_ai_routes.py b/src/youtube_extension/backend/cloud_ai_routes.py index a1242a418..6008cdf10 100644 --- a/src/youtube_extension/backend/cloud_ai_routes.py +++ b/src/youtube_extension/backend/cloud_ai_routes.py @@ -228,8 +228,8 @@ async def get_provider_status(): ) except Exception as e: - logger.error(f"Failed to get provider status: {e}") - raise HTTPException(status_code=500, detail=f"Failed to get provider status: {str(e)}") + logger.error(f"Failed to get provider status: {e}", exc_info=True) + raise HTTPException(status_code=500, detail="Internal server error") @router.post("/analyze/video", response_model=VideoAnalysisResponse) @@ -262,13 +262,13 @@ 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}") - raise HTTPException(status_code=500, detail=f"Configuration error: {str(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}") - raise HTTPException(status_code=500, detail=f"Analysis failed: {str(e)}") + logger.error(f"Unexpected error during video analysis: {e}", exc_info=True) + raise HTTPException(status_code=500, detail="Internal server error") @router.post("/analyze/batch") @@ -300,8 +300,8 @@ 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}") - raise HTTPException(status_code=500, detail=f"Batch analysis failed: {str(e)}") + logger.error(f"Failed to start batch analysis: {e}", exc_info=True) + raise HTTPException(status_code=500, detail="Internal server error") @router.post("/analyze/multi-provider", response_model=list[VideoAnalysisResponse]) @@ -323,9 +323,13 @@ 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}") - raise HTTPException(status_code=500, detail=f"Multi-provider analysis failed: {str(e)}") + logger.error(f"Multi-provider analysis failed: {e}", exc_info=True) + raise HTTPException(status_code=500, detail="Internal server error") @router.get("/analysis-types") diff --git a/src/youtube_extension/backend/cloud_api_endpoints.py b/src/youtube_extension/backend/cloud_api_endpoints.py index e38655f99..fc7dedee0 100644 --- a/src/youtube_extension/backend/cloud_api_endpoints.py +++ b/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): @@ -260,9 +255,10 @@ async def batch_process_videos_cloud(request: BatchCloudProcessingRequest): except HTTPException: raise except Exception as e: + logger.error(f"Unhandled error in batch_process_videos_cloud: {e}", exc_info=True) raise HTTPException( status_code=500, - detail=f"Batch processing failed: {str(e)}" + detail="Internal server error" ) @router.get("/api/v3/videos/{video_id}/status", response_model=VideoStatusResponse) @@ -293,9 +289,10 @@ async def get_video_status(video_id: str): except HTTPException: raise except Exception as e: + logger.error(f"Unhandled error in get_video_status: {e}", exc_info=True) raise HTTPException( status_code=500, - detail=f"Error retrieving status: {str(e)}" + detail="Internal server error" ) @router.get("/api/v3/videos/{video_id}/result") @@ -330,9 +327,10 @@ async def get_video_result(video_id: str): except HTTPException: raise except Exception as e: + logger.error(f"Unhandled error in get_video_result: {e}", exc_info=True) raise HTTPException( status_code=500, - detail=f"Error retrieving result: {str(e)}" + detail="Internal server error" ) @router.get("/api/v3/queue/stats") diff --git a/src/youtube_extension/backend/main.py b/src/youtube_extension/backend/main.py index ff1234740..c7dabe921 100644 --- a/src/youtube_extension/backend/main.py +++ b/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) diff --git a/src/youtube_extension/backend/real_api_endpoints.py b/src/youtube_extension/backend/real_api_endpoints.py index 03fd9e4e6..65c8b9fe6 100644 --- a/src/youtube_extension/backend/real_api_endpoints.py +++ b/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): @@ -150,9 +142,10 @@ async def validate_video_url(request: VideoValidationRequest): } except Exception as e: + logger.error(f"Unhandled error in validate_video_url: {e}", exc_info=True) raise HTTPException( status_code=500, - detail=f"Validation failed: {str(e)}" + detail="Internal server error" ) @app.post("/api/v2/batch-process") @@ -176,10 +169,14 @@ 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( status_code=500, - detail=f"Batch processing failed: {str(e)}" + detail="Internal server error" ) @app.get("/api/v2/videos/list") @@ -252,9 +249,10 @@ async def get_video_analysis(video_id: str): except HTTPException: raise except Exception as e: + logger.error(f"Unhandled error in get_video_analysis: {e}", exc_info=True) raise HTTPException( status_code=500, - detail=f"Error retrieving video analysis: {str(e)}" + detail="Internal server error" ) @app.get("/api/v2/cost-dashboard") @@ -384,9 +382,10 @@ async def clear_processing_cache(): } except Exception as e: + logger.error(f"Unhandled error in clear_processing_cache: {e}", exc_info=True) raise HTTPException( status_code=500, - detail=f"Failed to clear cache: {str(e)}" + detail="Internal server error" ) @app.post("/api/v2/search-videos") @@ -431,10 +430,14 @@ 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( status_code=500, - detail=f"Search failed: {str(e)}" + detail="Internal server error" ) logger.info("🚀 Real API endpoints setup complete") diff --git a/tests/unit/test_500_info_disclosure.py b/tests/unit/test_500_info_disclosure.py new file mode 100644 index 000000000..f552d790a --- /dev/null +++ b/tests/unit/test_500_info_disclosure.py @@ -0,0 +1,270 @@ +"""Regression guard against information disclosure in HTTP 500 responses. + +Context: several FastAPI handlers historically raised +``HTTPException(status_code=500, detail=str(e))`` (or an f-string embedding the +exception), leaking internal exception text — stack-adjacent messages, backend +API errors, database errors — to clients. See PR #801, which sanitized most but +not all handlers. + +This test encodes the invariant directly on the source: a 500 response must use +a *static* ``detail`` string, never one derived from the caught exception. It is +hermetic (pure source scan, no app import / no pydantic) so it runs anywhere and +catches new leaks in any backend route, not just the ones fixed today. + +It deliberately does not constrain 4xx responses: those echo client-supplied +validation errors, which are not internal-disclosure vectors. +""" + +from __future__ import annotations + +import ast +import re +from pathlib import Path + +import pytest + +_BACKEND = Path(__file__).resolve().parents[2] / "src" / "youtube_extension" / "backend" + + +def _backend_python_files() -> list[Path]: + return sorted(_BACKEND.rglob("*.py")) + + +# --- 500 `detail=` disclosure scan (AST-based) -------------------------------- +# +# A 500 response is safe only when its ``detail`` is a *static* string literal +# (``detail="Internal server error"``) — or is omitted entirely. Anything derived +# from the caught exception can carry internal state to the client and is flagged. +# A regex over ``detail=...`` misses two live shapes, so the scan parses the AST +# and inspects every ``HTTPException(...)`` call, positional and keyword alike: +# +# HTTPException(500, str(e)) # positional status *and* detail +# HTTPException(status_code=500, +# detail="failed: " + str(e)) # concat that *starts* with a literal +# +# Both the status code and the detail may be passed positionally +# (``HTTPException(status, detail, headers)``) or by keyword; either form counts. + + +def _is_static_str(node: ast.AST | None) -> bool: + """True iff ``node`` is a string literal, or a ``+`` concatenation of only + string literals. A concat with any non-literal operand (``"x " + str(e)``) + is dynamic and therefore not static.""" + if isinstance(node, ast.Constant) and isinstance(node.value, str): + return True + if isinstance(node, ast.BinOp) and isinstance(node.op, ast.Add): + return _is_static_str(node.left) and _is_static_str(node.right) + return False + + +def _is_500(node: ast.AST | None) -> bool: + """True for a literal ``500`` or a ``status.HTTP_500_*`` attribute.""" + if isinstance(node, ast.Constant) and node.value == 500: + return True + if isinstance(node, ast.Attribute) and "500" in node.attr: + return True + return False + + +def _http_exception_calls(tree: ast.AST): + for node in ast.walk(tree): + if isinstance(node, ast.Call): + func = node.func + name = func.attr if isinstance(func, ast.Attribute) else getattr(func, "id", None) + if name == "HTTPException": + yield node + + +def _status_and_detail(call: ast.Call): + """Resolve (status_node, detail_node, detail_present) from positional and + keyword arguments. Signature is ``HTTPException(status_code, detail, ...)``.""" + status = call.args[0] if len(call.args) >= 1 else None + detail = call.args[1] if len(call.args) >= 2 else None + detail_present = len(call.args) >= 2 + for kw in call.keywords: + if kw.arg == "status_code": + status = kw.value + elif kw.arg == "detail": + detail = kw.value + detail_present = True + return status, detail, detail_present + + +def _iter_500_dynamic_detail(text: str): + """Yield (line_no, snippet) for each 500 HTTPException with a dynamic detail.""" + tree = ast.parse(text) + for call in _http_exception_calls(tree): + status, detail, detail_present = _status_and_detail(call) + if not _is_500(status): + continue + if not detail_present: + # A 500 with no detail falls back to FastAPI's generic phrase — safe. + continue + if not _is_static_str(detail): + snippet = ast.get_source_segment(text, call) or ast.dump(detail) + yield call.lineno, " ".join(snippet.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 @.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(): + text = path.read_text(encoding="utf-8") + for line_no, snippet in _iter_500_dynamic_detail(text): + rel = path.relative_to(_BACKEND.parents[2]) + offenders.append(f"{rel}:{line_no}: HTTPException({snippet})") + + assert not offenders, ( + "HTTP 500 responses must use a static `detail` string (e.g. " + '"Internal server error") and never leak the caught exception. ' + "Log the full error server-side instead. Offending sites:\n " + + "\n ".join(offenders) + ) + + +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 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 + "raise HTTPException(500, str(e))", # positional status *and* detail + "raise HTTPException(500, error_msg)", # positional bare variable + 'raise HTTPException(500, detail=f"{e}")', # positional status, kw detail + 'raise HTTPException(status_code=500, detail="failed: " + str(e))', # concat w/ leading literal + ] + for leak in leaks: + assert list(_iter_500_dynamic_detail(leak)), f"scanner missed a real 500 leak: {leak}" + + # Static string literals — including a concatenation of only literals — are safe, + # as is a 500 with no detail at all (FastAPI supplies a generic phrase). + safe = [ + 'raise HTTPException(status_code=500, detail="Internal server error")', + 'raise HTTPException(500, "Internal server error")', # positional static + 'raise HTTPException(status_code=500, detail="Internal " + "server error")', # literal concat + "raise HTTPException(status_code=500)", # no detail + ] + for ok in safe: + assert not list( + _iter_500_dynamic_detail(ok) + ), f"scanner false-positived a safe 500 detail: {ok}" + + # 4xx responses echo client-supplied input and are intentionally out of scope. + client_errs = [ + "raise HTTPException(status_code=400, detail=str(exc))", + "raise HTTPException(404, str(exc))", # positional 4xx + ] + for ce in client_errs: + assert not list( + _iter_500_dynamic_detail(ce) + ), f"scanner must ignore 4xx responses: {ce}" + + +if __name__ == "__main__": # pragma: no cover + raise SystemExit(pytest.main([__file__, "-v"])) diff --git a/tests/unit/test_backend_main.py b/tests/unit/test_backend_main.py index a73dcf792..f89a9df2b 100644 --- a/tests/unit/test_backend_main.py +++ b/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 diff --git a/tests/unit/test_cloud_routes.py b/tests/unit/test_cloud_routes.py index 4c3b29839..e9b30eb08 100644 --- a/tests/unit/test_cloud_routes.py +++ b/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 @@ -642,6 +652,15 @@ def test_process_video_task_exception(self): ) assert response.status_code == 500 + # Regression lock for the persisted-error disclosure sink: the failed + # state must be recorded with a generic message. The status/result + # endpoints return `error_message` verbatim to clients, so the original + # exception text ("processing failed") must never reach Firestore. + mock_firestore.update_state.assert_awaited_once_with( + "auJzb1D-fag", status="failed", error_message="Internal server error" + ) + assert "processing failed" not in str(mock_firestore.update_state.call_args) + # -------- POST /api/v3/batch-process -------- def test_batch_process_success(self): @@ -1316,7 +1335,7 @@ def test_generate_dashboard_url_service_error(self): } ) assert response.status_code == 500 - assert "Internal server error" in response.json()["detail"] + assert response.json()["detail"] == "Internal server error" def test_generate_dashboard_url_missing_fields(self): """Missing required fields return 422.""" diff --git a/tests/unit/test_real_api_endpoints.py b/tests/unit/test_real_api_endpoints.py index 4eb7109c6..49aa6cf01 100644 --- a/tests/unit/test_real_api_endpoints.py +++ b/tests/unit/test_real_api_endpoints.py @@ -344,14 +344,19 @@ def test_returns_500_when_processor_raises(self, client, mock_processor): ) assert response.status_code == 500 - def test_error_response_includes_video_url(self, client, mock_processor): + def test_error_response_is_sanitized(self, client, mock_processor): + # A 500 must not leak internal state (CWE-209): the response body must be a + # static message, never the caught exception or the caller-supplied video_url. mock_processor.process_video = AsyncMock(side_effect=RuntimeError("crash")) response = client.post( "/api/v2/process-video", json={"video_url": "https://youtube.com/watch?v=auJzb1D-fag"}, ) - detail = response.json()["detail"] - assert "auJzb1D-fag" in str(detail) + assert response.status_code == 500 + detail = str(response.json()["detail"]) + assert detail == "Internal server error" + assert "crash" not in detail + assert "auJzb1D-fag" not in detail def test_missing_video_url_returns_422(self, client): response = client.post("/api/v2/process-video", json={}) @@ -437,15 +442,15 @@ def test_valid_batch_returns_200(self, client): assert response.status_code == 200 def test_batch_with_more_than_20_videos_returns_error(self, client): - """Source code raises HTTPException(400) inside a try block that - re-wraps it as 500. Test matches actual behaviour.""" + """Batching more than 20 videos is rejected with HTTP 400. The handler + re-raises the HTTPException(400) via `except HTTPException: raise` instead + of masking it as a generic 500 (regression lock for that re-raise).""" urls = [f"https://youtube.com/watch?v=vid{i:05d}" for i in range(21)] response = client.post( "/api/v2/batch-process", json={"video_urls": urls, "max_concurrent": 3}, ) - # The HTTPException(400) is caught by the outer except -> HTTP 500 - assert response.status_code in (400, 500) + assert response.status_code == 400 def test_batch_response_contains_results(self, client): response = client.post( @@ -797,10 +802,11 @@ def test_result_video_url_format(self, client): assert "youtube.com/watch?v=" in result["video_url"] def test_max_results_above_50_returns_error(self, client): - """Source code raises HTTPException(400) inside a try block that - catches Exception -> results in HTTP 500.""" + """max_results above 50 is rejected with HTTP 400. The handler re-raises + the HTTPException(400) via `except HTTPException: raise` rather than + swallowing it into a generic 500 (regression lock for that re-raise).""" response = client.post("/api/v2/search-videos?query=python&max_results=51") - assert response.status_code in (400, 500) + assert response.status_code == 400 def test_default_order_is_relevance(self, client, mock_youtube): client.post("/api/v2/search-videos?query=test")