diff --git a/src/app/api/eval-cases/route.ts b/src/app/api/eval-cases/route.ts index 7d7b0b7a3..92387fdb6 100644 --- a/src/app/api/eval-cases/route.ts +++ b/src/app/api/eval-cases/route.ts @@ -85,6 +85,36 @@ export async function POST(request: Request) { const sourceFiles = uniqueValues(parsed.data.sourceFiles); const rating = feedbackRating(parsed.data); const missReason = missReasonFor(parsed.data, rating); + // Caller-supplied expected_* FKs are UUID-validated and FK-checked for existence, + // but neither enforces ownership. Confirm they resolve to rows owned by the + // caller so eval data can't be attributed to another tenant's document/chunk. + let expectedDocumentId: string | null = null; + if (parsed.data.expectedDocumentId) { + const { data: ownedDoc } = await supabase + .from("documents") + .select("id") + .eq("id", parsed.data.expectedDocumentId) + .eq("owner_id", user.id) + .maybeSingle(); + if (ownedDoc) expectedDocumentId = parsed.data.expectedDocumentId; + } + let expectedChunkId: string | null = null; + if (parsed.data.expectedChunkId) { + const { data: chunkRow } = await supabase + .from("document_chunks") + .select("document_id") + .eq("id", parsed.data.expectedChunkId) + .maybeSingle(); + if (chunkRow?.document_id) { + const { data: ownedChunkDoc } = await supabase + .from("documents") + .select("id") + .eq("id", chunkRow.document_id) + .eq("owner_id", user.id) + .maybeSingle(); + if (ownedChunkDoc) expectedChunkId = parsed.data.expectedChunkId; + } + } const { data, error } = await supabase .from("rag_query_misses") .insert({ @@ -99,8 +129,8 @@ export async function POST(request: Request) { top_chunk_ids: sourceChunkIds, cited_chunk_ids: citedChunkIds, miss_reason: missReason, - expected_document_id: parsed.data.expectedDocumentId ?? null, - expected_chunk_id: parsed.data.expectedChunkId ?? citedChunkIds[0] ?? sourceChunkIds[0] ?? null, + expected_document_id: expectedDocumentId, + expected_chunk_id: expectedChunkId ?? citedChunkIds[0] ?? sourceChunkIds[0] ?? null, candidate_aliases: normalizedClinicalSearchTokens(parsed.data.query).slice(0, 12), promoted_eval_case: true, promoted_at: new Date().toISOString(), diff --git a/src/app/api/search/interaction/route.ts b/src/app/api/search/interaction/route.ts index 0e82a524f..35e0ba40a 100644 --- a/src/app/api/search/interaction/route.ts +++ b/src/app/api/search/interaction/route.ts @@ -28,27 +28,39 @@ export async function POST(request: Request) { // Carry the authenticated owner through so the miss row is attributable and // owner-cleanable instead of being orphaned with owner_id: null (RET-H4). const user = await serverAuth.requireAuthenticatedUser(request, supabase); + // Only attribute the clicked document/labels to the miss row when the document + // belongs to the caller. Otherwise a client could inject an arbitrary (or + // cross-owner) document_id + free-text label into the miss-promotion pipeline + // (scripts/promote-query-misses). The query telemetry is still recorded. + const { data: ownedDocument } = await supabase + .from("documents") + .select("id") + .eq("id", body.documentId) + .eq("owner_id", user.id) + .maybeSingle(); + const documentOwned = Boolean(ownedDocument); await supabase.from("rag_query_misses").insert({ owner_id: user.id, query: queryTextForStorage(body.query), normalized_query: normalizeQueryText(body.query), query_class: body.queryClass ?? null, - clicked_document_id: body.documentId, - clicked_chunk_id: body.chunkId ?? null, - top_files: body.fileName ? [body.fileName] : [], - top_chunk_ids: body.chunkId ? [body.chunkId] : [], + clicked_document_id: documentOwned ? body.documentId : null, + clicked_chunk_id: documentOwned ? (body.chunkId ?? null) : null, + top_files: documentOwned && body.fileName ? [body.fileName] : [], + top_chunk_ids: documentOwned && body.chunkId ? [body.chunkId] : [], miss_reason: "clicked_result", candidate_aliases: normalizedClinicalSearchTokens(body.query).slice(0, 10), - candidate_labels: body.title - ? [ - { - label: body.title, - label_type: "document_type", - document_id: body.documentId, - confidence: 0.6, - }, - ] - : [], + candidate_labels: + documentOwned && body.title + ? [ + { + label: body.title, + label_type: "document_type", + document_id: body.documentId, + confidence: 0.6, + }, + ] + : [], metadata: { interaction: "source_open", ...queryPrivacyMetadata(body.query), diff --git a/tests/eval-cases-route.test.ts b/tests/eval-cases-route.test.ts index 2719a0122..a0b7af9dd 100644 --- a/tests/eval-cases-route.test.ts +++ b/tests/eval-cases-route.test.ts @@ -158,4 +158,52 @@ describe("/api/eval-cases", () => { unverified_numeric_tokens: ["15"], }); }); + + it("drops a caller-supplied expected_document_id the caller does not own", async () => { + const unownedDocumentId = "bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb"; + const insert = vi.fn((payload: unknown) => ({ + select: vi.fn(() => ({ single: vi.fn(async () => ({ data: { id: "capture-1" }, error: null })) })), + payload, + })); + // documents ownership lookup resolves to no owned row → unowned. + const documentsLookup = { + select: vi.fn(() => ({ + eq: vi.fn(() => ({ + eq: vi.fn(() => ({ maybeSingle: vi.fn(async () => ({ data: null, error: null })) })), + })), + })), + }; + const client = { + from: vi.fn((table: string) => { + if (table === "documents") return documentsLookup; + if (table === "rag_query_misses") return { insert }; + throw new Error(`unexpected table ${table}`); + }), + }; + vi.doMock("@/lib/env", () => ({ isDemoMode: () => false, env: { RAG_PERSIST_RAW_QUERY_TEXT: false } })); + vi.doMock("@/lib/supabase/admin", () => ({ createAdminClient: () => client })); + vi.doMock("@/lib/supabase/auth", () => ({ + AuthenticationError: class AuthenticationError extends Error {}, + requireAuthenticatedUser: vi.fn(async () => ({ id: userId })), + unauthorizedResponse: () => Response.json({ error: "Authentication required." }, { status: 401 }), + })); + const { POST } = await import("../src/app/api/eval-cases/route"); + + const response = await POST( + request({ + query: "Which document covers clozapine titration?", + rating: "needs_fixing", + expectedDocumentId: unownedDocumentId, + sourceChunkIds: [validChunkId], + citedChunkIds: [validChunkId], + }), + ); + const payload = insert.mock.calls[0]?.[0] as Record; + + expect(response.status).toBe(201); + // Unowned expected_document_id is not attributed to this owner's eval row. + expect(payload.expected_document_id).toBeNull(); + // expected_chunk_id still falls back to the caller's own cited chunk. + expect(payload.expected_chunk_id).toBe(validChunkId); + }); });