From 16c9c9d5358d012eb8365e2afa0e6da6580e4971 Mon Sep 17 00:00:00 2001 From: Luis Novo Date: Tue, 14 Jul 2026 12:22:19 -0300 Subject: [PATCH] fix(sources): cap error text surfaced to clients (#1140) * fix(sources): cap error text surfaced to clients Source processing status (get_source_status) and sync-processing failures returned the raw command/result error_message to clients unbounded, which could leak arbitrary internal exception text and didn't match the error capping applied elsewhere in the API. Add a None-safe _truncate_error helper (200-char cap, ellipsis when cut) and apply it on both paths. Adds focused unit tests for the helper. Closes #1136 * test: narrow Optional return before assertions to satisfy mypy --- api/routers/sources.py | 17 ++++++++-- tests/test_error_message_sanitization.py | 41 ++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/api/routers/sources.py b/api/routers/sources.py index c7ff97cd..a7abb149 100644 --- a/api/routers/sources.py +++ b/api/routers/sources.py @@ -71,6 +71,19 @@ async def _assert_file_supported(file_path: str) -> None: detail = f"{detail} (detected type: {support.identified_type})" raise UnsupportedTypeException(detail) + +def _truncate_error(msg: Optional[str], limit: int = 200) -> Optional[str]: + """Cap error text surfaced to clients. + + Command/processing failures can carry arbitrary internal exception text; + return at most ``limit`` characters so a raw traceback message can't leak + to the API response. ``None`` passes through unchanged. + """ + if not msg: + return msg + return msg if len(msg) <= limit else msg[:limit] + "…" + + SOURCE_SORT_FIELDS = { "created": "created", "updated": "updated", @@ -326,7 +339,7 @@ async def get_sources( processing_info = { "started_at": execution_metadata.get("started_at"), "completed_at": execution_metadata.get("completed_at"), - "error": command.get("error_message"), + "error": _truncate_error(command.get("error_message")), } elif command: # Command exists but FETCH failed to resolve it (broken reference) @@ -603,7 +616,7 @@ async def _create_source_sync_path( pass raise HTTPException( status_code=500, - detail=f"Processing failed: {result.error_message}", + detail=f"Processing failed: {_truncate_error(result.error_message)}", ) # Get the processed source diff --git a/tests/test_error_message_sanitization.py b/tests/test_error_message_sanitization.py index 6973e4e8..d3882cc6 100644 --- a/tests/test_error_message_sanitization.py +++ b/tests/test_error_message_sanitization.py @@ -180,3 +180,44 @@ class TestPodcastServiceDoesNotLeakExceptionText: assert exc_info.value.status_code == 500 assert SECRET not in exc_info.value.detail assert exc_info.value.detail == "Failed to submit podcast generation job" + + +class TestTruncateErrorHelper: + """`_truncate_error` caps client-facing error text surfaced by the source + status and sync-processing paths (#1136).""" + + def test_none_passes_through(self): + from api.routers.sources import _truncate_error + + assert _truncate_error(None) is None + + def test_empty_string_passes_through(self): + from api.routers.sources import _truncate_error + + assert _truncate_error("") == "" + + def test_short_message_unchanged(self): + from api.routers.sources import _truncate_error + + assert _truncate_error("boom") == "boom" + + def test_message_at_limit_unchanged(self): + from api.routers.sources import _truncate_error + + msg = "x" * 200 + assert _truncate_error(msg) == msg + + def test_long_message_truncated_with_ellipsis(self): + from api.routers.sources import _truncate_error + + result = _truncate_error(SECRET + "x" * 500) + assert result is not None + # capped at limit + the single-character ellipsis + assert len(result) == 201 + assert result.endswith("…") + assert result.startswith(SECRET[:50]) + + def test_custom_limit(self): + from api.routers.sources import _truncate_error + + assert _truncate_error("abcdefghij", limit=4) == "abcd…"