Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 32 additions & 2 deletions src/app/api/eval-cases/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Comment on lines +101 to +105
.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({
Expand All @@ -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(),
Expand Down
40 changes: 26 additions & 14 deletions src/app/api/search/interaction/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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] : [],
Comment on lines +47 to +50
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),
Expand Down
48 changes: 48 additions & 0 deletions tests/eval-cases-route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown>;

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);
});
});
Loading