fix(api): validate doc_id at chunk-context handler boundary

Add an .isdigit() guard at the top of both chunk-context handlers so a
non-numeric doc_id fails fast with a clear 400 ("doc_id must be numeric,
got 'abc'") rather than silently bottoming out as a 404 from deep inside
get_chunk_with_context. The earlier int(doc_id) coercion was removed when
doc_id became a pure pass-through to Qdrant's keyword payload index, which
also dropped this boundary validation.

Also align test_backfill_emits_progress_log_every_20_batches' scroll stub
with real Qdrant: next_offset is now "next-1" (str) instead of 1 (int),
matching the sibling test_backfill_rewrites_int_doc_ids_to_str. Pure
stub-fidelity fix; production code already treats next_offset as opaque.

Addresses both 🟡 Important items from PR #773 review round 10.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Chris Coutinho
2026-05-09 16:54:06 +02:00
co-authored by Claude Opus 4.7
parent fec1596784
commit d60348e77b
3 changed files with 33 additions and 2 deletions
+14
View File
@@ -498,6 +498,20 @@ async def get_chunk_context(request: Request) -> JSONResponse:
assert doc_id is not None
assert doc_type is not None
# Validate doc_id at the handler boundary: a malformed doc_id would
# otherwise pass through to get_chunk_with_context and bottom out as a
# 404 from deep inside, not a clear 400. Nextcloud IDs are unsigned
# ints from MySQL auto_increment; doc_id stays a str downstream
# (Qdrant payload index is keyword-typed).
if not doc_id.isdigit():
return JSONResponse(
{
"success": False,
"error": f"doc_id must be numeric, got {doc_id!r}",
},
status_code=400,
)
# Parse and validate integer parameters with bounds checking
try:
context_chars = _parse_int_param(
+14
View File
@@ -560,6 +560,20 @@ async def chunk_context_endpoint(request: Request) -> JSONResponse:
assert start_str is not None
assert end_str is not None
# Validate doc_id at the handler boundary: a malformed doc_id would
# otherwise pass through to get_chunk_with_context and bottom out as a
# 404 from deep inside, not a clear 400. Nextcloud IDs are unsigned
# ints from MySQL auto_increment; doc_id stays a str downstream
# (Qdrant payload index is keyword-typed).
if not doc_id.isdigit():
return JSONResponse(
{
"success": False,
"error": f"doc_id must be numeric, got {doc_id!r}",
},
status_code=400,
)
context_chars = _parse_int_param(
request.query_params.get("context"),
500,
+5 -2
View File
@@ -577,8 +577,11 @@ async def test_backfill_emits_progress_log_every_20_batches(mocker, caplog):
# set_payload calls happen — the test focuses on the progress log
# cadence, not the rewrite path.
str_point = SimpleNamespace(id=1, payload={"doc_id": "abc"})
batches: list[tuple[list[SimpleNamespace], int | None]] = [
([str_point], 1) for _ in range(21)
# Real Qdrant returns next_offset as a UUID string (or None to terminate).
# Match that shape so the stub remains accurate if scroll's return type is
# ever tightened — and aligns with test_backfill_rewrites_int_doc_ids_to_str.
batches: list[tuple[list[SimpleNamespace], str | None]] = [
([str_point], "next-1") for _ in range(21)
] + [([], None)]
client.scroll.side_effect = batches