fix(api): a malformed id is a 422 or 400, not a server error

.claude/rules/file-map.md

@@ -138,6 +138,7 @@ Full module index for LocalChat. **Keep this current** — update in the same co
 | `docs/MIGRATIONS.md` | Migration docs — how to apply, write, and roll back |
 | `tests/integration/test_object_authorization_over_the_wire.py` | P2-2b — a plain user holding workspace A addresses workspace B's objects over HTTP against a real Postgres, and every route must refuse. The route list is derived from `app.openapi()` (not `app.routes`, which this FastAPI version leaves unflattened as `_IncludedRouter` with `path=None`), an unclassified path parameter fails rather than silently dropping its routes, and the fixture proves its own objects are reachable by their owner — a 404 is a pass here, so an empty fixture would be a perfect green run testing nothing. Runs as an editor and as an owner of A (R-5), and probes ids named in a body or header beside a control with the caller's own |
 | `tests/unit/test_all_workspaces_has_a_reason.py` | R-5b (QP-9 review) — every function in `src/`, `mcp_servers/` and `plugins/` passing `ALL_WORKSPACES` (bare or as `scope.ALL_WORKSPACES`) has a recorded reason that holds for every caller; a new use without one fails, so does a reason whose use is gone, and so does importing it under another name. A comparison on the scope is not a use |
+| `tests/integration/test_malformed_ids_are_not_server_errors.py` | Every object route of the authorisation matrix, given `not-a-uuid` and a number past bigint, answers below 500: a malformed UUID path id is a 422 (`UuidId` in `src/models.py`), one well-formed but absent still reaches the database as a 404, and a malformed `X-Workspace-ID` is the guard's 400. Each used to be the route's own 500 |
 | `tests/integration/test_graph_expansion_scope.py` | R-12 (QP-9 review) — GraphRAG's 1-hop lookup returns a name that co-occurs in the caller's workspace, not one from another workspace's document or from a retired one |
 | `tests/integration/test_ontology_reads_live_documents.py` | A workspace's ontology counts only its live documents and live chunks: a retired document and a chunk retired by a re-ingest leave it |
 | `tests/integration/test_chunk_statistics_scope.py` | R-1 (QP-9 review) — document statistics count and sample only the caller's workspace's live chunks, with a second workspace and a retired document beside it |

CHANGELOG.md

@@ -22,6 +22,12 @@ reasoning attached, in [docs/history/LESSONS_LEARNED.md](docs/history/LESSONS_LE
 
 ### Fixed
 
+- **A malformed id is the caller's mistake, not a server error.** A path id that is not a
+  UUID reached Postgres, which refused it, and the route answered 500 on about thirty
+  routes, from users and workspaces to conversations and connectors. Such an id is now a
+  422 before the route runs, as an integer id already was. The workspace guard answers
+  400 to a workspace named in a header, query or body that is not a UUID in canonical
+  form; Python reads `urn:uuid:` forms that Postgres refuses.
 - **A failing plugin no longer leaves tools behind, and a reloaded one can be unloaded.** A
   plugin that raised after registering a tool kept that tool. A reload recorded none of the
   plugin's tools, so unloading it afterwards left them registered. And a loader given an

src/models.py

@@ -1,9 +1,11 @@
 # Python 3.14 compatibility: Use string annotations to avoid __annotate__ issues
 from __future__ import annotations
 
-from typing import Literal
+from typing import Annotated, Literal
+from uuid import UUID
 
 from pydantic import (
+    AfterValidator,
     BaseModel,
     Field,
     StrictBool,
@@ -12,6 +14,15 @@
     model_validator,
 )
 
+
+def _canonical_uuid(value: str) -> str:
+    return str(UUID(value))
+
+
+#: A path id addressing a row keyed by UUID. Refused at the boundary with a 422, as an
+#: ``int`` id already is: unchecked, Postgres refused it and the route answered 500.
+UuidId = Annotated[str, AfterValidator(_canonical_uuid)]
+
 _MODEL_NAME_EMPTY = 'Model name cannot be empty'
 _MODEL_NAME_INVALID_CHARS = 'Model name contains invalid characters'
 from . import config

src/routes_fastapi/annotation_routes.py

@@ -7,6 +7,7 @@
 from fastapi import APIRouter, Depends, Request
 from fastapi.responses import JSONResponse
 
+from ..models import UuidId
 from ..security_fastapi import get_current_user_id
 from ..services.annotations import AnnotationsService
 from ..utils.logging_config import get_logger
@@ -70,7 +71,7 @@ def list_chunk_annotations(chunk_id: int, request: Request, annotations: Annotat
 
 
 @router.delete("/annotations/{annotation_id}")
-def delete_annotation(annotation_id: str, request: Request, annotations: Annotations) -> Any:
+def delete_annotation(annotation_id: UuidId, request: Request, annotations: Annotations) -> Any:
     denied = _deny(request, None, "editor")
     if denied:
         return denied

src/routes_fastapi/auth_routes.py

@@ -16,6 +16,7 @@
     GrantWorkspaceRequest,
     LoginRequest,
     UpdateUserRequest,
+    UuidId,
 )
 from ..security_fastapi import (
     SESSION_COOKIE,
@@ -205,7 +206,7 @@ def get_own_identity(request: Request, users: Users) -> Any:
 
 @router.get("/users/{user_id}/workspaces")
 def list_user_workspaces(
-    user_id: str,
+    user_id: UuidId,
     users: Users,
     _admin: Annotated[str, Depends(require_admin_dep)],
 ) -> Any:
@@ -219,7 +220,7 @@ def list_user_workspaces(
 
 @router.post("/users/{user_id}/workspaces")
 async def grant_user_workspace(
-    user_id: str,
+    user_id: UuidId,
     request: Request,
     users: Users,
     _admin: Annotated[str, Depends(require_admin_dep)],
@@ -240,8 +241,8 @@ async def grant_user_workspace(
 
 @router.delete("/users/{user_id}/workspaces/{workspace_id}")
 def revoke_user_workspace(
-    user_id: str,
-    workspace_id: str,
+    user_id: UuidId,
+    workspace_id: UuidId,
     users: Users,
     _admin: Annotated[str, Depends(require_admin_dep)],
 ) -> Any:
@@ -258,7 +259,7 @@ def revoke_user_workspace(
 
 
 @router.get("/users/{user_id}")
-def get_user(user_id: str, users: Users, _admin: Annotated[str, Depends(require_admin_dep)]) -> Any:
+def get_user(user_id: UuidId, users: Users, _admin: Annotated[str, Depends(require_admin_dep)]) -> Any:
     try:
         user = users.get_user(user_id)
         if user is None:
@@ -271,7 +272,7 @@ def get_user(user_id: str, users: Users, _admin: Annotated[str, Depends(require_
 
 @router.put("/users/{user_id}")
 async def update_user(
-    user_id: str, request: Request, users: Users, _admin: Annotated[str, Depends(require_admin_dep)]
+    user_id: UuidId, request: Request, users: Users, _admin: Annotated[str, Depends(require_admin_dep)]
 ) -> Any:
     data = await request.json() if await request.body() else {}
     try:
@@ -294,7 +295,7 @@ async def update_user(
 
 
 @router.delete("/users/{user_id}/purge")
-def purge_user(user_id: str, users: Users, admin_user_id: Annotated[str, Depends(require_admin_dep)]) -> Any:
+def purge_user(user_id: UuidId, users: Users, admin_user_id: Annotated[str, Depends(require_admin_dep)]) -> Any:
     try:
         if not users.purge_user(user_id):
             return JSONResponse(
@@ -316,7 +317,7 @@ def purge_user(user_id: str, users: Users, admin_user_id: Annotated[str, Depends
 
 
 @router.delete("/users/{user_id}")
-def delete_user(user_id: str, users: Users, admin_user_id: Annotated[str, Depends(require_admin_dep)]) -> Any:
+def delete_user(user_id: UuidId, users: Users, admin_user_id: Annotated[str, Depends(require_admin_dep)]) -> Any:
     try:
         if not users.retire_user(user_id, actor=admin_user_id):
             return JSONResponse({"success": False, "message": _NOT_FOUND}, status_code=404)

src/routes_fastapi/connector_routes.py

@@ -10,6 +10,7 @@
 from fastapi import APIRouter, Depends, HTTPException, Request
 from fastapi.responses import JSONResponse
 
+from ..models import UuidId
 from ..security_fastapi import get_current_user_id, require_admin_dep
 from ..services.connectors import ConnectorsService
 from ..services.errors import ServiceError
@@ -113,7 +114,7 @@ async def create_connector(request: Request, connectors: Connectors) -> Any:
 
 
 @router.get("/connectors/{connector_id}")
-def get_connector(connector_id: str, request: Request, connectors: Connectors) -> Any:
+def get_connector(connector_id: UuidId, request: Request, connectors: Connectors) -> Any:
     denied = _deny(request, None, "owner")
     if denied:
         return denied
@@ -128,7 +129,7 @@ def get_connector(connector_id: str, request: Request, connectors: Connectors) -
 
 
 @router.put("/connectors/{connector_id}")
-async def update_connector(connector_id: str, request: Request, connectors: Connectors) -> Any:
+async def update_connector(connector_id: UuidId, request: Request, connectors: Connectors) -> Any:
     denied = _deny(request, None, "owner")
     if denied:
         return denied
@@ -153,7 +154,7 @@ async def update_connector(connector_id: str, request: Request, connectors: Conn
 
 @router.delete("/connectors/{connector_id}/purge")
 def purge_connector(
-    connector_id: str,
+    connector_id: UuidId,
     connectors: Connectors,
     _admin: Annotated[str, Depends(require_admin_dep)],
 ) -> Any:
@@ -179,7 +180,7 @@ def purge_connector(
 
 
 @router.delete("/connectors/{connector_id}")
-def delete_connector(connector_id: str, request: Request, connectors: Connectors) -> Any:
+def delete_connector(connector_id: UuidId, request: Request, connectors: Connectors) -> Any:
     denied = _deny(request, None, "owner")
     if denied:
         return denied
@@ -193,7 +194,7 @@ def delete_connector(connector_id: str, request: Request, connectors: Connectors
 
 
 @router.post("/connectors/{connector_id}/sync")
-def trigger_sync(connector_id: str, request: Request, connectors: Connectors) -> Any:
+def trigger_sync(connector_id: UuidId, request: Request, connectors: Connectors) -> Any:
     denied = _deny(request, None, "owner")
     if denied:
         return denied
@@ -218,7 +219,7 @@ def _run() -> None:
 
 @router.get("/connectors/{connector_id}/history")
 def sync_history(
-    connector_id: str, request: Request, connectors: Connectors, limit: int = 20
+    connector_id: UuidId, request: Request, connectors: Connectors, limit: int = 20
 ) -> Any:
     denied = _deny(request, None, "owner")
     if denied:
@@ -235,7 +236,7 @@ def sync_history(
 
 
 @router.post("/connectors/{connector_id}/webhook")
-async def receive_webhook(connector_id: str, request: Request, connectors: Connectors) -> Any:
+async def receive_webhook(connector_id: UuidId, request: Request, connectors: Connectors) -> Any:
     # No user session here: this is a public receiver, and the connector id plus its
     # secret is the whole credential. ALL_WORKSPACES says that rather than relying on
     # an omitted argument to mean it.

src/routes_fastapi/gkb_routes.py

@@ -8,7 +8,7 @@
 from fastapi.responses import JSONResponse
 from pydantic import ValidationError
 
-from ..models import ContributionRequest
+from ..models import ContributionRequest, UuidId
 from ..security_fastapi import require_admin_dep
 from ..services.contributions import ContributionsService
 from ..services.errors import ServiceError
@@ -27,7 +27,7 @@
 
 @router.post("/workspaces/{workspace_id}/contribute")
 async def contribute(
-    workspace_id: str,
+    workspace_id: UuidId,
     request: Request,
     contributions: Contributions,
     admin_id: Annotated[str, Depends(require_admin_dep)],
@@ -54,7 +54,7 @@ async def contribute(
 
 
 @router.delete("/workspaces/{workspace_id}/contributions")
-def archive_contributions(workspace_id: str, request: Request, contributions: Contributions) -> Any:
+def archive_contributions(workspace_id: UuidId, request: Request, contributions: Contributions) -> Any:
     # Withdrawing only narrows what every workspace reads, so the owner may do it alone.
     denied = _deny(request, workspace_id, "owner")
     if denied:

src/routes_fastapi/longterm_memory_routes.py

@@ -8,6 +8,7 @@
 from fastapi.responses import JSONResponse
 
 from .. import config
+from ..models import UuidId
 from ..security_fastapi import get_current_user_id
 from ..services.memories import MemoriesService
 from ..utils.logging_config import get_logger
@@ -67,7 +68,7 @@ async def extract_memories(request: Request, memories_svc: Memories) -> Any:
 
 
 @router.delete("/{memory_id}")
-def delete_memory(memory_id: str, request: Request, memories_svc: Memories) -> Any:
+def delete_memory(memory_id: UuidId, request: Request, memories_svc: Memories) -> Any:
     denied = _deny(request, None, "editor")
     if denied:
         return denied

src/routes_fastapi/memory_routes.py

@@ -8,6 +8,7 @@
 from fastapi import APIRouter, Depends, Request
 from fastapi.responses import JSONResponse, Response
 
+from ..models import UuidId
 from ..security_fastapi import get_current_user_id, require_admin_dep
 from ..services.conversations import ConversationsService
 from ..services.errors import ServiceError
@@ -62,7 +63,7 @@ def delete_all_conversations(request: Request, conversations: Conversations) ->
 
 
 @router.get("/conversations/{conversation_id}")
-def get_conversation(conversation_id: str, request: Request, conversations: Conversations) -> Any:
+def get_conversation(conversation_id: UuidId, request: Request, conversations: Conversations) -> Any:
     denied = _deny(request, None, "viewer")
     if denied:
         return denied
@@ -93,7 +94,7 @@ def _build_binary_export(fmt: str, title: str, messages: list) -> bytes:
 
 @router.get("/conversations/{conversation_id}/export")
 def export_conversation(
-    conversation_id: str, request: Request, conversations: Conversations, format: str = "json"
+    conversation_id: UuidId, request: Request, conversations: Conversations, format: str = "json"
 ) -> Any:
     denied = _deny(request, None, "viewer")
     if denied:
@@ -143,7 +144,7 @@ def export_conversation(
 
 @router.get("/conversations/{conversation_id}/documents")
 def get_conversation_documents(
-    conversation_id: str, request: Request, conversations: Conversations
+    conversation_id: UuidId, request: Request, conversations: Conversations
 ) -> Any:
     denied = _deny(request, None, "viewer")
     if denied:
@@ -156,7 +157,7 @@ def get_conversation_documents(
 
 @router.put("/conversations/{conversation_id}/documents")
 async def set_conversation_documents(
-    conversation_id: str, request: Request, conversations: Conversations
+    conversation_id: UuidId, request: Request, conversations: Conversations
 ) -> Any:
     denied = _deny(request, None, "editor")
     if denied:
@@ -174,7 +175,7 @@ async def set_conversation_documents(
 
 @router.patch("/conversations/{conversation_id}")
 async def update_conversation(
-    conversation_id: str, request: Request, conversations: Conversations
+    conversation_id: UuidId, request: Request, conversations: Conversations
 ) -> Any:
     denied = _deny(request, None, "editor")
     if denied:
@@ -192,7 +193,7 @@ async def update_conversation(
 
 @router.delete("/conversations/{conversation_id}/purge")
 def purge_conversation(
-    conversation_id: str,
+    conversation_id: UuidId,
     conversations: Conversations,
     _admin: Annotated[str, Depends(require_admin_dep)],
 ) -> Any:
@@ -215,7 +216,7 @@ def purge_conversation(
 
 
 @router.delete("/conversations/{conversation_id}")
-def delete_conversation(conversation_id: str, request: Request, conversations: Conversations) -> Any:
+def delete_conversation(conversation_id: UuidId, request: Request, conversations: Conversations) -> Any:
     denied = _deny(request, None, "editor")
     if denied:
         return denied