From d60348e77b8a4468808795ded1a41ce31b106e1f Mon Sep 17 00:00:00 2001
From: Chris Coutinho <chris@coutinho.io>
Date: Sat, 9 May 2026 16:54:06 +0200
Subject: [PATCH] fix(api): validate doc_id at chunk-context handler boundary
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 nextcloud_mcp_server/api/visualization.py | 14 ++++++++++++++
 nextcloud_mcp_server/auth/viz_routes.py   | 14 ++++++++++++++
 tests/unit/vector/test_qdrant_client.py   |  7 +++++--
 3 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/nextcloud_mcp_server/api/visualization.py b/nextcloud_mcp_server/api/visualization.py
index cec34e92..62d8e3f3 100644
--- a/nextcloud_mcp_server/api/visualization.py
+++ b/nextcloud_mcp_server/api/visualization.py
@@ -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(
diff --git a/nextcloud_mcp_server/auth/viz_routes.py b/nextcloud_mcp_server/auth/viz_routes.py
index c189ac0a..5c2eee3d 100644
--- a/nextcloud_mcp_server/auth/viz_routes.py
+++ b/nextcloud_mcp_server/auth/viz_routes.py
@@ -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,
diff --git a/tests/unit/vector/test_qdrant_client.py b/tests/unit/vector/test_qdrant_client.py
index 4743648a..32a498a2 100644
--- a/tests/unit/vector/test_qdrant_client.py
+++ b/tests/unit/vector/test_qdrant_client.py
@@ -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