mirror of
https://github.com/lfnovo/open-notebook.git
synced 2026-10-19 09:13:16 +00:00
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
This commit is contained in:
parent
f7d9f0fb1e
commit
16c9c9d535
2 changed files with 56 additions and 2 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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…"
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue