Skip to content

Commit 16c9c9d

Browse files
authored
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
1 parent f7d9f0f commit 16c9c9d

2 files changed

Lines changed: 56 additions & 2 deletions

File tree

api/routers/sources.py

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,19 @@ async def _assert_file_supported(file_path: str) -> None:
7171
detail = f"{detail} (detected type: {support.identified_type})"
7272
raise UnsupportedTypeException(detail)
7373

74+
75+
def _truncate_error(msg: Optional[str], limit: int = 200) -> Optional[str]:
76+
"""Cap error text surfaced to clients.
77+
78+
Command/processing failures can carry arbitrary internal exception text;
79+
return at most ``limit`` characters so a raw traceback message can't leak
80+
to the API response. ``None`` passes through unchanged.
81+
"""
82+
if not msg:
83+
return msg
84+
return msg if len(msg) <= limit else msg[:limit] + "…"
85+
86+
7487
SOURCE_SORT_FIELDS = {
7588
"created": "created",
7689
"updated": "updated",
@@ -326,7 +339,7 @@ async def get_sources(
326339
processing_info = {
327340
"started_at": execution_metadata.get("started_at"),
328341
"completed_at": execution_metadata.get("completed_at"),
329-
"error": command.get("error_message"),
342+
"error": _truncate_error(command.get("error_message")),
330343
}
331344
elif command:
332345
# Command exists but FETCH failed to resolve it (broken reference)
@@ -603,7 +616,7 @@ async def _create_source_sync_path(
603616
pass
604617
raise HTTPException(
605618
status_code=500,
606-
detail=f"Processing failed: {result.error_message}",
619+
detail=f"Processing failed: {_truncate_error(result.error_message)}",
607620
)
608621

609622
# Get the processed source

tests/test_error_message_sanitization.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,3 +180,44 @@ async def test_submit_generation_job_failure_returns_generic_message(self):
180180
assert exc_info.value.status_code == 500
181181
assert SECRET not in exc_info.value.detail
182182
assert exc_info.value.detail == "Failed to submit podcast generation job"
183+
184+
185+
class TestTruncateErrorHelper:
186+
"""`_truncate_error` caps client-facing error text surfaced by the source
187+
status and sync-processing paths (#1136)."""
188+
189+
def test_none_passes_through(self):
190+
from api.routers.sources import _truncate_error
191+
192+
assert _truncate_error(None) is None
193+
194+
def test_empty_string_passes_through(self):
195+
from api.routers.sources import _truncate_error
196+
197+
assert _truncate_error("") == ""
198+
199+
def test_short_message_unchanged(self):
200+
from api.routers.sources import _truncate_error
201+
202+
assert _truncate_error("boom") == "boom"
203+
204+
def test_message_at_limit_unchanged(self):
205+
from api.routers.sources import _truncate_error
206+
207+
msg = "x" * 200
208+
assert _truncate_error(msg) == msg
209+
210+
def test_long_message_truncated_with_ellipsis(self):
211+
from api.routers.sources import _truncate_error
212+
213+
result = _truncate_error(SECRET + "x" * 500)
214+
assert result is not None
215+
# capped at limit + the single-character ellipsis
216+
assert len(result) == 201
217+
assert result.endswith("…")
218+
assert result.startswith(SECRET[:50])
219+
220+
def test_custom_limit(self):
221+
from api.routers.sources import _truncate_error
222+
223+
assert _truncate_error("abcdefghij", limit=4) == "abcd…"

0 commit comments

Comments
 (0)