diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index d3898fabc..a4ef58ce3 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -555,3 +555,4 @@ Use this ledger to prevent repeated branch and PR reviews when the reviewed HEAD | 2026-07-15 | PR #677 / claude/cleanup-branches-worktrees-mxov4x | a93db73a29ca6a19619a6592e2e30ca9ea2f8218 | active PR review and remediation | All three actionable review findings are fixed: every branch namespace is parsed from the ledger table, only an exact completed cleanup review at the current HEAD suppresses repeat work, pending deletions remain actionable, and GitHub provider provenance is accurate. No additional high-confidence defect remains in the changed scope. | GitHub unresolved-thread inventory (0 remaining); hosted required CI green; exact-head focused Vitest `tests/repo-hygiene.test.ts` (9/9); `node scripts/sweep-branch-ledger.mjs --no-fetch --json`; `git diff --check`. No Supabase, OpenAI, or other live-service checks. | | 2026-07-15 | PR #656 / claude/specifiers-v2-design-r55baf | 58ce935758c31672a0a051c5b3e6b888a7d8d153 | full DSM/ICD specifier catalogue and clinical-content gate review | No remaining high-confidence code defect after the review sequence corrected source provenance, verified-content wording, search ranking/deduplication, empty-state behavior, and neutral mixed-source labelling. The 494 unverified definitions remain withheld from display and ranking. Residual risk is the PR-declared qualified-clinician and TGA classification review before broader clinical deployment. | GitHub review-thread inventory (0 unresolved); exact-head hosted required CI, build, critical UI, UI regression, coverage, static, and security checks green. No live Supabase/OpenAI or provider-backed clinical workflow run. | | 2026-07-15 | PR #679 / claude/therapy-compass-pages-rz0m5l | f5ca25f8b6b14f41e63f708933fe4bb311994795 | Therapy Compass production promotion and review-followup | All five review findings are fixed on the reviewed head: run-enabled shared-composer links stay on the Therapy Compass route, production data loads outside `/mockups`, deep links seed the in-tool search, and same-route query changes remount the provider. No additional high-confidence defect remains in the changed scope. | GitHub review-thread inventory; exact-head hosted build, critical UI smoke, UI regression, unit coverage, static, security, and image checks green; focused Vitest 28/28; TypeScript; `git diff --check`. No live Supabase/OpenAI checks run. | +| 2026-07-15 | PR #680 / claude/rag-scalability-review-x0s55l | d842f3fd + reviewed follow-up fixes | privacy, public-catalog throttling, ingestion-recovery, and merge-readiness review | Audit remediation wave 1 plus review follow-ups. Confirmed and fixed: mixed-owner authenticated document lists returned owner-internal nested label/summary fields for public documents (now fetched with ownership-specific projections plus response redaction); the `[id]` detail route likewise now redacts summary provenance (`source_chunk_ids`/`source_image_ids`/`model`) for non-owners; anonymous catalog rate limiting now covers the `[slug]` DETAIL routes as well as the list routes (removed the `shouldResolvePublicCatalogAccess` bypass and its dead helpers); and ingestion recovery keeps an open pending/freshly-processing job while superseding only failed/stale siblings, with both recovery scripts (`recover-ingestion-queue.ts`, `reindex.ts`) passing all queried statuses to the planner so a failed+fresh-processing document cannot collide on `ingestion_jobs_one_open_per_document_uidx`. Residual (low, deferred): base document metadata is still returned on non-owned public list rows. | GitHub review-thread and exact-head inspection; Next.js route-handler guide; `npm run verify:cheap` (lint, TypeScript, full Vitest) green after merging origin/main; focused route + ingestion-recovery Vitest; Prettier; `git diff --check`. No live Supabase/OpenAI/provider checks run. | diff --git a/scripts/recover-ingestion-queue.ts b/scripts/recover-ingestion-queue.ts index ba5984063..c74817fa4 100644 --- a/scripts/recover-ingestion-queue.ts +++ b/scripts/recover-ingestion-queue.ts @@ -82,7 +82,7 @@ function parseArgs(argv: string[]) { async function main() { const [ { env, requireServerEnv }, - { buildIngestionRecoveryPlan }, + { buildIngestionRecoveryPlan, INGESTION_RECOVERY_JOB_STATUSES }, { createAdminClient }, { assertSupabaseHealthy, probeSupabaseHealth }, ] = await Promise.all([ @@ -105,7 +105,7 @@ async function main() { const { data, error } = await supabase .from("ingestion_jobs") .select("id,document_id,status,locked_at,documents(status,page_count,chunk_count)") - .in("status", ["pending", "processing", "failed"]) + .in("status", [...INGESTION_RECOVERY_JOB_STATUSES]) .order("created_at", { ascending: true }); if (error) throw supabaseStageError("load open ingestion jobs", error); diff --git a/scripts/reindex.ts b/scripts/reindex.ts index 88bf700eb..cfeb48006 100644 --- a/scripts/reindex.ts +++ b/scripts/reindex.ts @@ -73,7 +73,7 @@ async function safeCount(label: string, countPromise: PromiseLike) async function main() { const [ { env, requireServerEnv }, - { buildIngestionRecoveryPlan, isFreshProcessingJob }, + { buildIngestionRecoveryPlan, INGESTION_RECOVERY_JOB_STATUSES, isFreshProcessingJob }, { hasIncompleteDocumentsWithoutOpenJobs, isReindexQueueClear }, { createAdminClient }, { assertSupabaseHealthy, probeSupabaseHealth }, @@ -229,7 +229,7 @@ async function main() { const { data, error } = await supabase .from("ingestion_jobs") .select("id,document_id,status,locked_at,documents(status,page_count,chunk_count)") - .in("status", ["processing", "failed"]) + .in("status", [...INGESTION_RECOVERY_JOB_STATUSES]) .order("created_at", { ascending: true }); if (error) { @@ -246,26 +246,17 @@ async function main() { locked_at: string | null; documents: RecoveryDocument | RecoveryDocument[] | null; }; - const staleAfterCutoff = new Date(staleCutoff); - const jobs = (data ?? []) - .map((job: RawJobRow) => ({ - ...job, - documents: Array.isArray(job.documents) - ? (job.documents[0] as RecoveryDocument | undefined) - : (job.documents as RecoveryDocument | undefined), - })) - .filter((job) => { - if (job.status === "failed") { - return true; - } - if (job.status !== "processing") { - return false; - } - if (job.locked_at === null) { - return true; - } - return new Date(job.locked_at) <= staleAfterCutoff; - }); + // Pass every queried job (pending/processing/failed) to the planner and let its + // active-vs-recoverable classification decide. Filtering out fresh `processing` jobs here + // hid legitimate in-flight owners from the planner: a document with a failed row plus a + // fresh processing retry would then requeue the failed row and collide with the still-open + // processing sibling on ingestion_jobs_one_open_per_document_uidx (Audit I2/E2 follow-up). + const jobs = (data ?? []).map((job: RawJobRow) => ({ + ...job, + documents: Array.isArray(job.documents) + ? (job.documents[0] as RecoveryDocument | undefined) + : (job.documents as RecoveryDocument | undefined), + })); hasActiveProcessingJobs = (data ?? []) .map((job: RawJobRow) => ({ ...job, diff --git a/src/app/api/differentials/[slug]/route.ts b/src/app/api/differentials/[slug]/route.ts index 16d51546b..e61e7fcf1 100644 --- a/src/app/api/differentials/[slug]/route.ts +++ b/src/app/api/differentials/[slug]/route.ts @@ -19,7 +19,7 @@ import { getDifferentialDetailContext, getDifferentialRecord, getPresentationWor import { isDemoMode, isLocalNoAuthMode } from "@/lib/env"; import { jsonError } from "@/lib/http"; import { safeErrorLogDetails } from "@/lib/privacy"; -import { publicAccessContext, shouldResolvePublicCatalogAccess } from "@/lib/public-api-access"; +import { publicAccessContext } from "@/lib/public-api-access"; import { createAdminClient } from "@/lib/supabase/admin"; import { AuthenticationError, unauthorizedResponse } from "@/lib/supabase/auth"; import { parseRequestQuery } from "@/lib/validation/query"; @@ -69,28 +69,9 @@ export async function GET(request: Request, context: { params: Promise<{ slug: s }); } - if (!shouldResolvePublicCatalogAccess(request)) { - const snapshot = loadDifferentialSnapshot(); - const governance = deriveGovernanceFromSnapshot(snapshot); - if (kind === "presentation") { - const workflow = getPresentationWorkflow(normalizedSlug); - if (!workflow) return notFoundResponse(normalizedSlug); - return differentialResponse({ - workflow, - governance: { sourceStatus: governance.source_status, validationStatus: governance.validation_status }, - publicAccess: true, - }); - } - const record = getDifferentialRecord(normalizedSlug); - if (!record) return notFoundResponse(normalizedSlug); - return differentialResponse({ - record, - detailContext: getDifferentialDetailContext(record), - governance: { sourceStatus: governance.source_status, validationStatus: governance.validation_status }, - publicAccess: true, - }); - } - + // Anonymous callers still resolve access + rate limit before we serve the seed detail: + // publicAccessContext skips the Supabase auth round-trip when there's no session cookie/bearer, + // but every caller must pass the registry limiter (M4/C1 — no anonymous bypass). const supabase = createAdminClient(); const access = await publicAccessContext(request, supabase); diff --git a/src/app/api/differentials/presentations/[slug]/route.ts b/src/app/api/differentials/presentations/[slug]/route.ts index ff5be6a18..afc883800 100644 --- a/src/app/api/differentials/presentations/[slug]/route.ts +++ b/src/app/api/differentials/presentations/[slug]/route.ts @@ -18,7 +18,7 @@ import { getDifferentialRecord, getPresentationWorkflow } from "@/lib/differenti import { isDemoMode, isLocalNoAuthMode } from "@/lib/env"; import { jsonError } from "@/lib/http"; import { safeErrorLogDetails } from "@/lib/privacy"; -import { publicAccessContext, shouldResolvePublicCatalogAccess } from "@/lib/public-api-access"; +import { publicAccessContext } from "@/lib/public-api-access"; import { createAdminClient } from "@/lib/supabase/admin"; import { AuthenticationError, unauthorizedResponse } from "@/lib/supabase/auth"; @@ -58,24 +58,9 @@ export async function GET(request: Request, context: { params: Promise<{ slug: s }); } - if (!shouldResolvePublicCatalogAccess(request)) { - const snapshot = loadDifferentialSnapshot(); - const workflow = getPresentationWorkflow(normalizedSlug); - if (!workflow) return notFoundResponse(normalizedSlug); - const governance = deriveGovernanceFromSnapshot(snapshot); - const candidates = workflow.candidates.flatMap((candidate) => { - const record = getDifferentialRecord(candidate.slug); - if (!record) return []; - return [{ ...candidate, record }]; - }); - return differentialResponse({ - workflow, - candidates, - governance: { sourceStatus: governance.source_status, validationStatus: governance.validation_status }, - publicAccess: true, - }); - } - + // Anonymous callers still resolve access + rate limit before we serve the seed detail: + // publicAccessContext skips the Supabase auth round-trip when there's no session cookie/bearer, + // but every caller must pass the registry limiter (M4/C1 — no anonymous bypass). const supabase = createAdminClient(); const access = await publicAccessContext(request, supabase); diff --git a/src/app/api/differentials/route.ts b/src/app/api/differentials/route.ts index 28361fec2..256d6a187 100644 --- a/src/app/api/differentials/route.ts +++ b/src/app/api/differentials/route.ts @@ -23,7 +23,7 @@ import { } from "@/lib/differentials"; import { isDemoMode, isLocalNoAuthMode } from "@/lib/env"; import { jsonError } from "@/lib/http"; -import { publicAccessContext, shouldResolvePublicCatalogAccess } from "@/lib/public-api-access"; +import { publicAccessContext } from "@/lib/public-api-access"; import { createAdminClient } from "@/lib/supabase/admin"; import { AuthenticationError, unauthorizedResponse } from "@/lib/supabase/auth"; import { parseRequestQuery, queryInteger } from "@/lib/validation/query"; @@ -88,13 +88,9 @@ export async function GET(request: Request) { }); } - if (!shouldResolvePublicCatalogAccess(request)) { - return differentialResponse({ - ...publicDifferentialPayload(kind, q, limit), - publicAccess: true, - }); - } - + // Anonymous callers still resolve access + rate limit: publicAccessContext skips the + // Supabase auth round-trip for requests with no session cookie/bearer, but every caller + // (authenticated or not) must pass the registry limiter before we serve the full catalog. const supabase = createAdminClient(); const access = await publicAccessContext(request, supabase); diff --git a/src/app/api/documents/[id]/route.ts b/src/app/api/documents/[id]/route.ts index 35bc57b16..5246c653c 100644 --- a/src/app/api/documents/[id]/route.ts +++ b/src/app/api/documents/[id]/route.ts @@ -10,7 +10,7 @@ import { invalidateRagCachesForDocumentMutation } from "@/lib/rag"; import { committedIndexGeneration, isCommittedGenerationMetadata } from "@/lib/reindex-pipeline"; import { createAdminClient } from "@/lib/supabase/admin"; import { AuthenticationError, requireAuthenticatedUser, unauthorizedResponse } from "@/lib/supabase/auth"; -import { enforceDocumentReadRateLimit, withOwnerReadScope } from "@/lib/public-api-access"; +import { callerOwnsDocumentRow, enforceDocumentReadRateLimit, withOwnerReadScope } from "@/lib/public-api-access"; import { writeAuditLog } from "@/lib/audit"; import { parseJsonBody } from "@/lib/validation/body"; import { parseRouteParams } from "@/lib/validation/params"; @@ -296,6 +296,13 @@ export async function GET(request: Request, { params }: { params: Promise<{ id: if (error) throw new Error(error.message); if (!document) return NextResponse.json({ error: "Document not found." }, { status: 404 }); + // withOwnerReadScope also returns PUBLIC (owner_id IS NULL) documents to an authenticated + // caller. The owner-only projection (raw metadata, storage_path/content_hash, index health, + // image storage paths) must be gated on OWNERSHIP, not merely on being authenticated, so an + // authed non-owner viewing a shared public document gets the same redacted view as an + // anonymous caller (S1/D1). + const isOwner = callerOwnsDocumentRow(document, access.ownerId); + const chunkId = detailQuery.chunk ?? null; const requestedPage = Math.min(detailQuery.page, Math.max(1, document.page_count ?? 1)); const pageLimit = detailQuery.pageLimit; @@ -390,21 +397,25 @@ export async function GET(request: Request, { params }: { params: Promise<{ id: "import_batch_id", "error_message", "metadata", + // Summary provenance: only present on document_summaries rows (fetched with select("*")). + // A non-owner viewing a public document's summary must not see the owner's chunk/image + // source IDs or the generation model, matching the list route's PUBLIC_SUMMARY projection. + "source_chunk_ids", + "source_image_ids", + "model", ]); return Object.fromEntries(Object.entries(row).filter(([key]) => !internalKeys.has(key))); }; const publicRows = >(rows: T[]) => - access.authenticated ? rows : rows.map(omitPublicInternalFields); - const responseDocument = access.authenticated - ? document - : omitPublicInternalFields(document as Record); + isOwner ? rows : rows.map(omitPublicInternalFields); + const responseDocument = isOwner ? document : omitPublicInternalFields(document as Record); return NextResponse.json({ document: { ...responseDocument, labels: publicRows((labelsResult.data ?? []) as Record[]), summary: - access.authenticated || !summaryResult.data + isOwner || !summaryResult.data ? (summaryResult.data ?? null) : omitPublicInternalFields(summaryResult.data as Record), }, @@ -430,7 +441,7 @@ export async function GET(request: Request, { params }: { params: Promise<{ id: hasAfter: Boolean(document.chunk_count && chunkRangeEnd + 1 < document.chunk_count), selectedChunkId: selectedChunk?.id ?? null, }, - ...(access.authenticated + ...(isOwner ? { indexHealth: { extractionQuality: safeMetadata(document.metadata).extraction_quality ?? null, diff --git a/src/app/api/documents/bulk/route.ts b/src/app/api/documents/bulk/route.ts index dcece4aa6..0d04d269e 100644 --- a/src/app/api/documents/bulk/route.ts +++ b/src/app/api/documents/bulk/route.ts @@ -3,6 +3,8 @@ import { z } from "zod"; import { normalizeDocumentLabelForStorage } from "@/lib/document-tags"; import { isDemoMode } from "@/lib/env"; import { jsonError, PublicApiError } from "@/lib/http"; +import { logger } from "@/lib/logger"; +import { safeErrorLogDetails } from "@/lib/privacy"; import { invalidateRagCachesForOwner } from "@/lib/rag"; import { createAdminClient } from "@/lib/supabase/admin"; import type { Json, TablesUpdate } from "@/lib/supabase/database.types"; @@ -207,10 +209,17 @@ export async function POST(request: Request) { if (updateError) throw new Error(updateError.message); results.push({ documentId: document.id, updated: true }); } catch (error) { + // Never surface the raw DB error text (constraint names, column details) to the client; + // log the sanitized detail server-side and return a generic per-document failure (S10/D5). + logger.error("Bulk document edit failed for a document", { + ownerId: user.id, + documentId: document.id, + ...safeErrorLogDetails(error), + }); results.push({ documentId: document.id, updated: false, - error: error instanceof Error ? error.message : "Bulk edit failed.", + error: "Bulk edit failed for this document.", }); } } diff --git a/src/app/api/documents/route.ts b/src/app/api/documents/route.ts index 27c38c423..9a4e24ccb 100644 --- a/src/app/api/documents/route.ts +++ b/src/app/api/documents/route.ts @@ -6,7 +6,12 @@ import { isDemoMode } from "@/lib/env"; import { jsonError } from "@/lib/http"; import { createAdminClient } from "@/lib/supabase/admin"; import { AuthenticationError, unauthorizedResponse } from "@/lib/supabase/auth"; -import { enforceDocumentReadRateLimit, withOwnerReadScope } from "@/lib/public-api-access"; +import { + callerOwnsDocumentRow, + enforceDocumentReadRateLimit, + redactNonOwnedDocumentFields, + withOwnerReadScope, +} from "@/lib/public-api-access"; import { parseRequestQuery, queryBoolean, queryInteger } from "@/lib/validation/query"; export const runtime = "nodejs"; @@ -92,10 +97,18 @@ const VALID_STATUSES = new Set(["queued", "processing", "indexed", "failed"]); const ACTIVE_DOCUMENT_STATUSES = new Set(["queued", "processing"]); const ACTIVE_INDEXING_POLL_MS = 5_000; -type DocumentListRow = Record & { id: string; status?: string | null }; +type DocumentListRow = Record & { id: string; owner_id?: unknown; status?: string | null }; type LabelListRow = Record & { document_id: string }; type SummaryListRow = Record & { document_id: string }; +function projectPublicFields>(row: T, columns: string): Partial { + const projected: Record = {}; + for (const field of columns.split(",")) { + if (Object.hasOwn(row, field)) projected[field] = row[field]; + } + return projected as Partial; +} + const documentListQuerySchema = z.object({ limit: queryInteger({ fallback: 100, min: 1, max: 200 }), offset: queryInteger({ fallback: 0, min: 0, max: 1_000_000 }), @@ -181,7 +194,17 @@ export async function GET(request: Request) { const { data, error, count } = await query; if (error) throw new Error(error.message); - const documents = (data ?? []) as unknown as DocumentListRow[]; + // An authenticated caller reads PUBLIC (owner_id IS NULL) documents alongside their own via + // withOwnerReadScope. Redact operator-internal storage fields on the rows they do not own so a + // shared public document never exposes its owner's storage_path/content_hash/etc. (S1/D1). + const rawDocuments = (data ?? []) as unknown as DocumentListRow[]; + const ownedDocumentIds = new Set( + rawDocuments.filter((document) => callerOwnsDocumentRow(document, access.ownerId)).map((document) => document.id), + ); + const publicDocumentIds = rawDocuments + .filter((document) => !ownedDocumentIds.has(document.id)) + .map((document) => document.id); + const documents = rawDocuments.map((document) => redactNonOwnedDocumentFields(document, access.ownerId)); const documentIds = documents.map((document) => document.id); const indexing = indexingState(documents); @@ -197,24 +220,49 @@ export async function GET(request: Request) { return documentsResponse({ documents, pagination }, indexing); } - const labelColumns = access.authenticated ? LABEL_LIST_COLUMNS : PUBLIC_LABEL_LIST_COLUMNS; - const summaryColumns = access.authenticated ? SUMMARY_LIST_COLUMNS : PUBLIC_SUMMARY_LIST_COLUMNS; - const [labelsResult, summariesResult] = await Promise.all([ - supabase.from("document_labels").select(labelColumns).in("document_id", documentIds), - supabase.from("document_summaries").select(summaryColumns).in("document_id", documentIds), + const ownedIds = [...ownedDocumentIds]; + const emptyResult = () => Promise.resolve({ data: [], error: null }); + const [ownedLabelsResult, publicLabelsResult, ownedSummariesResult, publicSummariesResult] = await Promise.all([ + ownedIds.length + ? supabase.from("document_labels").select(LABEL_LIST_COLUMNS).in("document_id", ownedIds) + : emptyResult(), + publicDocumentIds.length + ? supabase.from("document_labels").select(PUBLIC_LABEL_LIST_COLUMNS).in("document_id", publicDocumentIds) + : emptyResult(), + ownedIds.length + ? supabase.from("document_summaries").select(SUMMARY_LIST_COLUMNS).in("document_id", ownedIds) + : emptyResult(), + publicDocumentIds.length + ? supabase.from("document_summaries").select(PUBLIC_SUMMARY_LIST_COLUMNS).in("document_id", publicDocumentIds) + : emptyResult(), ]); - if (labelsResult.error) throw new Error(labelsResult.error.message); - if (summariesResult.error) throw new Error(summariesResult.error.message); + for (const result of [ownedLabelsResult, publicLabelsResult, ownedSummariesResult, publicSummariesResult]) { + if (result.error) throw new Error(result.error.message); + } const labelsByDocument = new Map(); - for (const label of (labelsResult.data ?? []) as unknown as LabelListRow[]) { + const labelRows = [ + ...(ownedLabelsResult.data ?? []), + ...(publicLabelsResult.data ?? []), + ] as unknown as LabelListRow[]; + for (const label of labelRows) { const existing = labelsByDocument.get(label.document_id) ?? []; - existing.push(label); + existing.push( + ownedDocumentIds.has(label.document_id) ? label : projectPublicFields(label, PUBLIC_LABEL_LIST_COLUMNS), + ); labelsByDocument.set(label.document_id, existing); } const summariesByDocument = new Map( - ((summariesResult.data ?? []) as unknown as SummaryListRow[]).map((summary) => [summary.document_id, summary]), + [...(ownedSummariesResult.data ?? []), ...(publicSummariesResult.data ?? [])].map((value) => { + const summary = value as unknown as SummaryListRow; + return [ + summary.document_id, + ownedDocumentIds.has(summary.document_id) + ? summary + : projectPublicFields(summary, PUBLIC_SUMMARY_LIST_COLUMNS), + ]; + }), ); return documentsResponse( diff --git a/src/app/api/jobs/route.ts b/src/app/api/jobs/route.ts index a1f39092f..2601c061d 100644 --- a/src/app/api/jobs/route.ts +++ b/src/app/api/jobs/route.ts @@ -87,12 +87,9 @@ export async function GET(request: Request) { if (error instanceof AuthenticationError) { return unauthorizedResponse(); } - if (error instanceof Error && error.message.includes("Missing server environment")) { - return jobsResponse(demoJobs, { - demoMode: true, - error: "Server environment is not configured; demo jobs are being served.", - }); - } + // A partially-configured production (isDemoMode() false but createAdminClient throws + // "Missing server environment") must surface as a real error rather than silently serving + // unauthenticated demo jobs, which masked the misconfiguration and returned fake data (S11/H6). return jsonError(error); } } diff --git a/src/app/api/medications/[slug]/route.ts b/src/app/api/medications/[slug]/route.ts index 9dc043993..48de02a16 100644 --- a/src/app/api/medications/[slug]/route.ts +++ b/src/app/api/medications/[slug]/route.ts @@ -17,7 +17,7 @@ import { rowToMedicationRecord, type MedicationRecordRow, } from "@/lib/medication-records"; -import { publicAccessContext, shouldResolvePublicCatalogAccess } from "@/lib/public-api-access"; +import { publicAccessContext } from "@/lib/public-api-access"; import { createAdminClient } from "@/lib/supabase/admin"; import { AuthenticationError, unauthorizedResponse } from "@/lib/supabase/auth"; @@ -61,15 +61,9 @@ export async function GET(request: Request, context: { params: Promise<{ slug: s }); } - if (!shouldResolvePublicCatalogAccess(request)) { - const payload = publicMedicationDetailPayload(normalizedSlug); - if (!payload) return notFoundResponse(normalizedSlug); - return medicationResponse({ - ...payload, - publicAccess: true, - }); - } - + // Anonymous callers still resolve access + rate limit before we serve the seed detail: + // publicAccessContext skips the Supabase auth round-trip when there's no session cookie/bearer, + // but every caller must pass the registry limiter (M4/C1 — no anonymous bypass). const supabase = createAdminClient(); const access = await publicAccessContext(request, supabase); diff --git a/src/app/api/medications/route.ts b/src/app/api/medications/route.ts index cb996d8fa..adb6e9743 100644 --- a/src/app/api/medications/route.ts +++ b/src/app/api/medications/route.ts @@ -21,7 +21,7 @@ import { type MedicationRecord, type MedicationSearchMatch, } from "@/lib/medications"; -import { publicAccessContext, shouldResolvePublicCatalogAccess } from "@/lib/public-api-access"; +import { publicAccessContext } from "@/lib/public-api-access"; import { createAdminClient } from "@/lib/supabase/admin"; import { AuthenticationError, unauthorizedResponse } from "@/lib/supabase/auth"; import { parseRequestQuery, queryInteger } from "@/lib/validation/query"; @@ -105,13 +105,9 @@ export async function GET(request: Request) { }); } - if (!shouldResolvePublicCatalogAccess(request)) { - return medicationResponse({ - ...publicMedicationPayload(q, limit, fields), - publicAccess: true, - }); - } - + // Anonymous callers still resolve access + rate limit: publicAccessContext skips the + // Supabase auth round-trip for requests with no session cookie/bearer, but every caller + // (authenticated or not) must pass the registry limiter before we serve the full catalog. const supabase = createAdminClient(); const access = await publicAccessContext(request, supabase); diff --git a/src/app/api/registry/records/[slug]/route.ts b/src/app/api/registry/records/[slug]/route.ts index 10978d62c..0734098cd 100644 --- a/src/app/api/registry/records/[slug]/route.ts +++ b/src/app/api/registry/records/[slug]/route.ts @@ -9,7 +9,7 @@ import { import { isDemoMode, isLocalNoAuthMode } from "@/lib/env"; import { jsonError } from "@/lib/http"; import { safeErrorLogDetails } from "@/lib/privacy"; -import { publicAccessContext, shouldResolvePublicCatalogAccess } from "@/lib/public-api-access"; +import { publicAccessContext } from "@/lib/public-api-access"; import { getFormRecord } from "@/lib/forms"; import { deriveGovernanceColumns, @@ -67,15 +67,9 @@ export async function GET(request: Request, context: { params: Promise<{ slug: s }); } - if (!shouldResolvePublicCatalogAccess(request)) { - const payload = publicRegistryDetailPayload(kind, normalizedSlug); - if (!payload) return notFoundResponse(normalizedSlug); - return registryResponse({ - ...payload, - publicAccess: true, - }); - } - + // Anonymous callers still resolve access + rate limit before we serve the seed detail: + // publicAccessContext skips the Supabase auth round-trip when there's no session cookie/bearer, + // but every caller must pass the registry limiter (M4/C1 — no anonymous bypass). const supabase = createAdminClient(); const access = await publicAccessContext(request, supabase); diff --git a/src/app/api/registry/records/route.ts b/src/app/api/registry/records/route.ts index 310c3c055..552d0b42b 100644 --- a/src/app/api/registry/records/route.ts +++ b/src/app/api/registry/records/route.ts @@ -8,7 +8,7 @@ import { } from "@/lib/api-rate-limit"; import { isDemoMode, isLocalNoAuthMode } from "@/lib/env"; import { jsonError } from "@/lib/http"; -import { publicAccessContext, shouldResolvePublicCatalogAccess } from "@/lib/public-api-access"; +import { publicAccessContext } from "@/lib/public-api-access"; import { rankFormRecords, formRecords } from "@/lib/forms"; import { deriveGovernanceColumns, @@ -81,13 +81,9 @@ export async function GET(request: Request) { }); } - if (!shouldResolvePublicCatalogAccess(request)) { - return registryResponse({ - ...publicRegistryPayload(kind, q, limit), - publicAccess: true, - }); - } - + // Anonymous callers still resolve access + rate limit: publicAccessContext skips the + // Supabase auth round-trip for requests with no session cookie/bearer, but every caller + // (authenticated or not) must pass the registry limiter before we serve the full catalog. const supabase = createAdminClient(); const access = await publicAccessContext(request, supabase); diff --git a/src/components/ui-primitives.tsx b/src/components/ui-primitives.tsx index 1fc77e3ba..fa1652d10 100644 --- a/src/components/ui-primitives.tsx +++ b/src/components/ui-primitives.tsx @@ -84,7 +84,7 @@ export const sourceCapsuleHit = export const sourceCapsule = "source-capsule-face inline-flex items-center gap-1.5 rounded-full border bg-[color-mix(in_srgb,var(--clinical-accent-soft)_55%,var(--surface))] px-2.5 py-1 text-2xs font-medium text-[color:var(--clinical-accent)]"; export const sourceCapsuleCountBadge = - "nums inline-flex h-4 min-w-4 shrink-0 items-center justify-center rounded-full bg-[color:var(--surface-raised)] px-1 text-[10px] font-semibold leading-none text-[color:var(--clinical-accent)] shadow-[var(--shadow-inset)]"; + "nums inline-flex h-4 min-w-4 shrink-0 items-center justify-center rounded-full bg-[color:var(--surface-raised)] px-1 text-3xs font-semibold leading-none text-[color:var(--clinical-accent)] shadow-[var(--shadow-inset)]"; export const evidenceRow = "flex min-h-12 w-full items-center justify-between gap-3 rounded-lg border border-[color:var(--border)] bg-[color:var(--surface)] px-3 py-2 text-left shadow-[var(--shadow-inset)] transition hover:border-[color:var(--border-strong)] hover:bg-[color:var(--surface-subtle)] focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[color:var(--focus)]"; export const clinicalNotesRow = diff --git a/src/lib/api-rate-limit.ts b/src/lib/api-rate-limit.ts index 06ecfe7f0..04b7472f2 100644 --- a/src/lib/api-rate-limit.ts +++ b/src/lib/api-rate-limit.ts @@ -68,6 +68,11 @@ const anonymousApiRateLimitDefaults: Partial; diff --git a/src/lib/ingestion-recovery.ts b/src/lib/ingestion-recovery.ts index 13eb99142..5969c617e 100644 --- a/src/lib/ingestion-recovery.ts +++ b/src/lib/ingestion-recovery.ts @@ -14,6 +14,8 @@ export type IngestionRecoveryAction = | { action: "supersede"; jobId: string; documentId: string } | { action: "retry"; jobId: string; documentId: string; resetDocument: boolean }; +export const INGESTION_RECOVERY_JOB_STATUSES = ["pending", "processing", "failed"] as const; + function parseLockedAt(value: string | null | undefined) { if (!value) return null; const time = Date.parse(value); @@ -50,39 +52,65 @@ export function buildIngestionRecoveryPlan(args: { const resetDocuments = new Set(); const actions: IngestionRecoveryAction[] = []; + const jobsByDocument = new Map(); for (const job of args.jobs) { - const documentStatus = job.documents?.status ?? null; - const chunkCount = Number(job.documents?.chunk_count ?? 0); - const isIndexedDocument = documentStatus === "indexed" && chunkCount > 0; - const isRecoverableStatus = - job.status === "failed" || - job.status === "pending" || - isRecoverableProcessingJob(job, now, args.staleAfterMinutes); - - if (isIndexedDocument && job.status !== "completed") { - // Audit R22: a `pending` job on an already-indexed document is a - // legitimately-queued reindex, not an abandoned leftover. Superseding it - // silently cancels the reindex ("completed / superseded by successful - // index"); routing it through the retry branch below would reset the live - // index (R19). Leave it untouched for the worker's atomic reindex path, - // which keeps the old generation live until the new commit swaps it. - if (job.status === "pending") { - continue; + const siblings = jobsByDocument.get(job.document_id) ?? []; + siblings.push(job); + jobsByDocument.set(job.document_id, siblings); + } + + for (const [documentId, jobs] of jobsByDocument) { + const document = jobs[0]?.documents; + const isIndexedDocument = document?.status === "indexed" && Number(document.chunk_count ?? 0) > 0; + const activeJob = + jobs.find((job) => job.status === "pending") ?? + jobs.find((job) => isFreshProcessingJob(job, now, args.staleAfterMinutes)); + + if (activeJob) { + // A pending or freshly processing job is already the legitimate queue owner. Keep it intact + // and close only failed/stale siblings; retrying an older sibling would either collide with + // the open-job unique index or silently supersede valid work depending on fetch order. + for (const job of jobs) { + if (job.id === activeJob.id || job.status === "completed") continue; + actions.push({ action: "supersede", jobId: job.id, documentId }); } - actions.push({ action: "supersede", jobId: job.id, documentId: job.document_id }); continue; } - if (isRecoverableStatus) { - resetDocuments.add(job.document_id); - actions.push({ action: "retry", jobId: job.id, documentId: job.document_id, resetDocument: true }); + const recoverableJobs = jobs.filter( + (job) => job.status === "failed" || isRecoverableProcessingJob(job, now, args.staleAfterMinutes), + ); + + if (isIndexedDocument) { + for (const job of recoverableJobs) { + actions.push({ action: "supersede", jobId: job.id, documentId }); + } + continue; } + + const retryJob = recoverableJobs.find((job) => job.status === "processing") ?? recoverableJobs[0]; + if (!retryJob) continue; + + for (const job of recoverableJobs) { + if (job.id !== retryJob.id) actions.push({ action: "supersede", jobId: job.id, documentId }); + } + resetDocuments.add(documentId); + actions.push({ action: "retry", jobId: retryJob.id, documentId, resetDocument: true }); } + // Apply supersedes before retries. A retry flips a row to `pending`; if a redundant sibling for + // the same document is still open when that happens, the two rows collide on the partial unique + // index. Both consumers apply `actions` in array order, so closing the siblings first here keeps + // recovery crash-safe regardless of the order jobs were fetched in. (Audit I2/E2) + const orderedActions = [ + ...actions.filter((action) => action.action === "supersede"), + ...actions.filter((action) => action.action === "retry"), + ]; + return { - actions, + actions: orderedActions, resetDocumentIds: Array.from(resetDocuments), - supersedeCount: actions.filter((action) => action.action === "supersede").length, - retryCount: actions.filter((action) => action.action === "retry").length, + supersedeCount: orderedActions.filter((action) => action.action === "supersede").length, + retryCount: orderedActions.filter((action) => action.action === "retry").length, }; } diff --git a/src/lib/public-api-access.ts b/src/lib/public-api-access.ts index b17007cad..101faae41 100644 --- a/src/lib/public-api-access.ts +++ b/src/lib/public-api-access.ts @@ -55,26 +55,6 @@ export function anonymousApiSubjectKey(request: Request) { return `anon:${createHash("sha256").update(source).digest("hex").slice(0, 32)}`; } -export function hasSessionCookieSignal(request: Request) { - const cookieHeader = request.headers.get("cookie") ?? ""; - return cookieHeader.includes("sb-"); -} - -export function hasBearerAuthAttempt(request: Request) { - const authorization = request.headers.get("authorization") ?? ""; - return /^Bearer\s+\S+/i.test(authorization); -} - -/** True when the request may carry a durable Supabase session (cookie), not a bare bearer attempt. */ -export function hasPublicApiAuthSignal(request: Request) { - return hasSessionCookieSignal(request); -} - -/** Anonymous callers with no cookie or bearer skip auth resolution and rate limits on curated public catalogs. */ -export function shouldResolvePublicCatalogAccess(request: Request) { - return hasSessionCookieSignal(request) || hasBearerAuthAttempt(request); -} - type OwnerScopedQuery = { eq(column: string, value: unknown): T; is(column: string, value: null): T; @@ -96,6 +76,45 @@ export function withOwnerReadScope>(query: T, owne return query.is("owner_id", null); } +// Owner-internal document columns that must never be exposed on a row the caller does not own. +// `withOwnerReadScope` lets an authenticated caller read PUBLIC documents (owner_id IS NULL) that +// belong to nobody; those rows must not leak the storage location, dedup hash, import provenance, +// raw stage error, or free-form `metadata` of the operator who ingested them. `metadata` is +// arbitrary and can carry owner-internal provenance (e.g. the bulk-edit author's user id, prior +// titles, indexing internals), so — matching the anonymous list projection and the `[id]` detail +// route — it is stripped for non-owners rather than surfaced as governance data. +const NON_OWNER_INTERNAL_DOCUMENT_FIELDS = [ + "storage_path", + "content_hash", + "source_path", + "import_batch_id", + "error_message", + "metadata", +] as const; + +/** True when `viewerOwnerId` is set and owns the row (i.e. the caller's own document). */ +export function callerOwnsDocumentRow(row: { owner_id?: unknown }, viewerOwnerId: string | undefined): boolean { + return Boolean(viewerOwnerId) && row.owner_id === viewerOwnerId; +} + +/** + * Strip operator-internal storage fields from a document row unless the caller owns it. + * + * No-op for owned rows and for rows already selected without those columns (e.g. the anonymous + * public projection), so it is safe to map over every returned row regardless of caller identity. + */ +export function redactNonOwnedDocumentFields>( + row: T, + viewerOwnerId: string | undefined, +): T { + if (callerOwnsDocumentRow(row, viewerOwnerId)) return row; + const redacted = { ...row }; + for (const field of NON_OWNER_INTERNAL_DOCUMENT_FIELDS) { + delete redacted[field]; + } + return redacted; +} + export async function publicAccessContext(request: Request, supabase: AdminClient) { const user = await getOptionalAuthenticatedUser(request, supabase); if (user) { diff --git a/tests/document-mutation-routes.test.ts b/tests/document-mutation-routes.test.ts index 8cf92b404..d3af3afd6 100644 --- a/tests/document-mutation-routes.test.ts +++ b/tests/document-mutation-routes.test.ts @@ -197,6 +197,45 @@ describe("/api/documents/bulk", () => { ); expect(invalidateRagCachesForOwner).toHaveBeenCalledWith(ownerId); }); + + it("does not leak the raw database error in a per-document bulk failure", async () => { + const rawDbError = 'duplicate key value violates unique constraint "documents_secret_internal_idx"'; + const supabase = createSupabaseMock((call) => { + if (call.table === "documents" && call.operation === "select") { + return { data: [{ id: documentId, title: "Old title", metadata: {} }], error: null }; + } + if (call.table === "documents" && call.operation === "update") { + return { data: null, error: { message: rawDbError } }; + } + return { data: null, error: null }; + }); + mockRouteRuntime(supabase.client); + const { POST } = await import("../src/app/api/documents/bulk/route"); + + const response = await POST( + new Request("http://localhost/api/documents/bulk", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ documentIds: [documentId], metadata: { publisher: "WA Health" } }), + }), + ); + const payload = (await response.json()) as { + ok: boolean; + updatedCount: number; + results: Array<{ documentId: string; updated: boolean; error?: string }>; + }; + + expect(response.status).toBe(200); + expect(payload).toMatchObject({ ok: false, updatedCount: 0 }); + expect(payload.results[0]).toMatchObject({ + documentId, + updated: false, + error: "Bulk edit failed for this document.", + }); + // The raw DB constraint text must never reach the client. + expect(JSON.stringify(payload)).not.toContain("documents_secret_internal_idx"); + expect(JSON.stringify(payload)).not.toContain("unique constraint"); + }); }); describe("/api/documents/[id]/table-facts", () => { diff --git a/tests/ingestion-recovery.test.ts b/tests/ingestion-recovery.test.ts index cd2976ed6..24f7c7b8a 100644 --- a/tests/ingestion-recovery.test.ts +++ b/tests/ingestion-recovery.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { buildIngestionRecoveryPlan, + INGESTION_RECOVERY_JOB_STATUSES, isFreshProcessingJob, isRecoverableProcessingJob, isStaleProcessingJob, @@ -9,6 +10,10 @@ import { describe("ingestion queue recovery planning", () => { const now = new Date("2026-06-15T00:00:00.000Z"); + it("loads pending jobs alongside failed and processing siblings", () => { + expect(INGESTION_RECOVERY_JOB_STATUSES).toEqual(["pending", "processing", "failed"]); + }); + it("selects stale processing and failed jobs for retry", () => { const plan = buildIngestionRecoveryPlan({ now, @@ -92,6 +97,60 @@ describe("ingestion queue recovery planning", () => { expect(plan.retryCount).toBe(0); }); + it("preserves a pending job and supersedes its failed sibling regardless of fetch order (I2)", () => { + const plan = buildIngestionRecoveryPlan({ + now, + staleAfterMinutes: 45, + jobs: [ + // The older `failed` row is iterated first; recovery must still prefer the legitimate + // open `pending` sibling rather than replacing it based on fetch order. + { + id: "failed-first", + document_id: "doc-double", + status: "failed", + documents: { status: "failed", chunk_count: 0 }, + }, + { + id: "pending-second", + document_id: "doc-double", + status: "pending", + documents: { status: "queued", chunk_count: 0 }, + }, + ], + }); + + expect(plan.retryCount).toBe(0); + expect(plan.supersedeCount).toBe(1); + expect(plan.resetDocumentIds).toEqual([]); + expect(plan.actions).toEqual([{ action: "supersede", jobId: "failed-first", documentId: "doc-double" }]); + }); + + it("preserves a fresh processing job and supersedes its failed sibling", () => { + const plan = buildIngestionRecoveryPlan({ + now, + staleAfterMinutes: 45, + jobs: [ + { + id: "failed-first", + document_id: "doc-active", + status: "failed", + documents: { status: "processing", chunk_count: 0 }, + }, + { + id: "processing-second", + document_id: "doc-active", + status: "processing", + locked_at: "2026-06-14T23:30:00.000Z", + documents: { status: "processing", chunk_count: 0 }, + }, + ], + }); + + expect(plan.retryCount).toBe(0); + expect(plan.resetDocumentIds).toEqual([]); + expect(plan.actions).toEqual([{ action: "supersede", jobId: "failed-first", documentId: "doc-active" }]); + }); + it("does not reclaim fresh processing jobs", () => { expect( isStaleProcessingJob( diff --git a/tests/medications-route.test.ts b/tests/medications-route.test.ts index 14ee3b5bd..2b30fb779 100644 --- a/tests/medications-route.test.ts +++ b/tests/medications-route.test.ts @@ -227,6 +227,21 @@ describe("medications API", () => { expect(payload.publicAccess).toBe(true); expect(payload.records.some((record) => record.slug === "acamprosate")).toBe(true); expect(payload.matches?.[0]?.medication.slug).toBe("acamprosate"); + // The catalog is served from seed data (no table read) and no auth round-trip is needed, + // but anonymous list requests must still pass the registry limiter (M4/C1). + expect(client.from).not.toHaveBeenCalled(); + expect(client.rpc).toHaveBeenCalled(); + expect(client.auth.getUser).not.toHaveBeenCalled(); + }); + + it("rate-limits anonymous list requests (429) without falling back to unlimited access", async () => { + const client = createSupabaseMock(() => ok([]), { limited: true }); + mockRuntime(client); + const { GET } = await import("../src/app/api/medications/route"); + + const response = await GET(request("/api/medications")); + + expect(response.status).toBe(429); expect(client.from).not.toHaveBeenCalled(); expect(client.auth.getUser).not.toHaveBeenCalled(); }); @@ -266,6 +281,22 @@ describe("medications API", () => { expect(payload.publicAccess).toBe(true); expect(payload.record.slug).toBe("acamprosate"); expect(payload.record.name).toBe("Acamprosate"); + // The seed detail is served without a table read, but anonymous detail requests must still + // pass the registry limiter (finding C — no anonymous bypass). + expect(client.from).not.toHaveBeenCalled(); + expect(client.rpc).toHaveBeenCalled(); + }); + + it("rate-limits anonymous detail requests (429) instead of skipping the limiter", async () => { + const client = createSupabaseMock(() => ok([]), { limited: true }); + mockRuntime(client); + const { GET } = await import("../src/app/api/medications/[slug]/route"); + + const response = await GET(request("/api/medications/acamprosate"), { + params: Promise.resolve({ slug: "acamprosate" }), + }); + + expect(response.status).toBe(429); expect(client.from).not.toHaveBeenCalled(); }); diff --git a/tests/private-access-routes.test.ts b/tests/private-access-routes.test.ts index f58484873..ba2aaf4a0 100644 --- a/tests/private-access-routes.test.ts +++ b/tests/private-access-routes.test.ts @@ -527,6 +527,120 @@ describe("private document API access", () => { expect(client.calls[0].range).toEqual({ from: 0, to: 99 }); }); + it("redacts owner-internal storage fields on public rows in an authenticated document list", async () => { + const owned = { + id: documentId, + owner_id: userId, + title: "Owned document", + status: "indexed", + storage_path: `${userId}/documents/owned.pdf`, + content_hash: "sha256:owned", + metadata: { extraction_quality: "good" }, + }; + const shared = { + id: otherDocumentId, + owner_id: null, + title: "Public guideline", + status: "indexed", + storage_path: "someone-else/documents/shared.pdf", + content_hash: "sha256:shared", + source_path: "/import/shared.pdf", + import_batch_id: "batch-1", + error_message: "internal stage error", + metadata: { extraction_quality: "partial" }, + }; + const client = createSupabaseMock((call) => (call.table === "documents" ? ok([owned, shared]) : ok([]))); + mockRuntime(client); + const { GET } = await import("../src/app/api/documents/route"); + + const response = await GET(authenticatedRequest("/api/documents?includeMeta=false")); + const body = await payload(response); + const [ownedRow, sharedRow] = body.documents as Array>; + + expect(response.status).toBe(200); + // The caller's own row keeps its storage internals. + expect(ownedRow).toMatchObject({ + id: documentId, + storage_path: `${userId}/documents/owned.pdf`, + content_hash: "sha256:owned", + }); + // The shared public row keeps its public fields but not the owner's storage internals or + // free-form metadata (which can carry owner-internal provenance). + expect(sharedRow).toMatchObject({ id: otherDocumentId, title: "Public guideline" }); + expect(sharedRow).not.toHaveProperty("metadata"); + expect(sharedRow).not.toHaveProperty("storage_path"); + expect(sharedRow).not.toHaveProperty("content_hash"); + expect(sharedRow).not.toHaveProperty("source_path"); + expect(sharedRow).not.toHaveProperty("import_batch_id"); + expect(sharedRow).not.toHaveProperty("error_message"); + }); + + it("redacts nested metadata for non-owned public documents in an authenticated list", async () => { + const owned = { id: documentId, owner_id: userId, title: "Owned document", status: "indexed" }; + const shared = { id: otherDocumentId, owner_id: null, title: "Public guideline", status: "indexed" }; + const client = createSupabaseMock((call) => { + if (call.table === "documents") return ok([owned, shared]); + const requestedIds = call.inFilters.find((filter) => filter.column === "document_id")?.values ?? []; + if (call.table === "document_labels") { + const rows = [ + { id: "owned-label", document_id: documentId, label: "Owned", metadata: { private: true } }, + { id: "shared-label", document_id: otherDocumentId, label: "Shared", metadata: { private: true } }, + ]; + return ok(rows.filter((row) => requestedIds.includes(row.document_id))); + } + if (call.table === "document_summaries") { + const rows = [ + { + id: "owned-summary", + document_id: documentId, + summary: "Owned summary", + source_chunk_ids: ["owned-chunk"], + metadata: { private: true }, + }, + { + id: "shared-summary", + document_id: otherDocumentId, + summary: "Shared summary", + source_chunk_ids: ["shared-chunk"], + metadata: { private: true }, + }, + ]; + return ok(rows.filter((row) => requestedIds.includes(row.document_id))); + } + return ok([]); + }); + mockRuntime(client); + const { GET } = await import("../src/app/api/documents/route"); + + const response = await GET(authenticatedRequest("/api/documents?includeMeta=true")); + const body = await payload(response); + const [ownedRow, sharedRow] = body.documents as Array>; + + expect(response.status).toBe(200); + const labelCalls = client.calls.filter((call) => call.table === "document_labels"); + const summaryCalls = client.calls.filter((call) => call.table === "document_summaries"); + expect(labelCalls).toHaveLength(2); + expect(summaryCalls).toHaveLength(2); + expect(labelCalls.map((call) => call.selected)).toEqual( + expect.arrayContaining([expect.stringContaining("metadata"), expect.not.stringContaining("metadata")]), + ); + expect(summaryCalls.map((call) => call.selected)).toEqual( + expect.arrayContaining([expect.stringContaining("source_chunk_ids"), expect.not.stringContaining("metadata")]), + ); + expect((ownedRow.labels as Array>)[0]).toHaveProperty("metadata"); + expect(ownedRow.summary).toHaveProperty("source_chunk_ids"); + expect((sharedRow.labels as Array>)[0]).toEqual({ + id: "shared-label", + document_id: otherDocumentId, + label: "Shared", + }); + expect(sharedRow.summary).toEqual({ + id: "shared-summary", + document_id: otherDocumentId, + summary: "Shared summary", + }); + }); + it("accepts legacy Supabase auth cookies for private document access", async () => { const documents = [{ id: documentId, owner_id: userId, title: "Owned document" }]; const client = createSupabaseMock((call) => (call.table === "documents" ? ok(documents) : ok([]))); @@ -557,7 +671,7 @@ describe("private document API access", () => { expect(client.calls[0].orFilters).toContain(`owner_id.eq.${userId},owner_id.is.null`); }); - it("allows authenticated users to read public document detail", async () => { + it("redacts owner-internal fields when an authenticated user reads a public document they do not own", async () => { const client = createSupabaseMock((call) => { if (call.table === "documents" && matchesOwnerReadScope(call, userId)) { return ok({ @@ -568,7 +682,26 @@ describe("private document API access", () => { file_type: "application/pdf", page_count: 2, chunk_count: 1, - metadata: { index_generation_id: "generation-a" }, + storage_path: "someone-else/documents/guideline.pdf", + content_hash: "sha256:leaky-hash", + source_path: "/import/guideline.pdf", + import_batch_id: "batch-99", + error_message: "internal stage error", + metadata: { index_generation_id: "generation-a", extraction_quality: "good" }, + }); + } + if (call.table === "document_summaries") { + return ok({ + id: "summary-1", + document_id: documentId, + owner_id: null, + summary: "Public summary text.", + clinical_specifics: null, + source_chunk_ids: ["chunk-a", "chunk-b"], + source_image_ids: ["image-a"], + model: "gpt-internal", + metadata: { internal_note: "owner only" }, + generated_at: "2026-01-01T00:00:00.000Z", }); } if (call.table === "document_pages") return ok([]); @@ -584,10 +717,71 @@ describe("private document API access", () => { params: Promise.resolve({ id: documentId }), }); const body = await payload(response); + const document = body.document as Record; expect(response.status).toBe(200); - expect(body.document).toMatchObject({ id: documentId, title: "Public guideline", owner_id: null }); + // The caller can still read the shared public document... + expect(document).toMatchObject({ id: documentId, title: "Public guideline" }); expect(client.calls[0].orFilters).toContain(`owner_id.eq.${userId},owner_id.is.null`); + // ...but not the owner's storage location, dedup hash, import provenance, raw error, metadata, + // or index-health diagnostics — an authed non-owner gets the same redacted view as anonymous. + expect(document).not.toHaveProperty("storage_path"); + expect(document).not.toHaveProperty("content_hash"); + expect(document).not.toHaveProperty("source_path"); + expect(document).not.toHaveProperty("import_batch_id"); + expect(document).not.toHaveProperty("error_message"); + expect(document).not.toHaveProperty("owner_id"); + expect(document).not.toHaveProperty("metadata"); + expect(body).not.toHaveProperty("indexHealth"); + // The public document's generated summary is readable, but its owner-internal provenance + // (chunk/image source IDs, generation model, owner_id, metadata) must be redacted. + const summary = document.summary as Record | null; + expect(summary).toMatchObject({ summary: "Public summary text." }); + expect(summary).not.toHaveProperty("source_chunk_ids"); + expect(summary).not.toHaveProperty("source_image_ids"); + expect(summary).not.toHaveProperty("model"); + expect(summary).not.toHaveProperty("owner_id"); + expect(summary).not.toHaveProperty("metadata"); + }); + + it("returns full owner-internal fields when the authenticated caller owns the document", async () => { + const client = createSupabaseMock((call) => { + if (call.table === "documents" && matchesOwnerReadScope(call, userId)) { + return ok({ + id: documentId, + owner_id: userId, + title: "Owned guideline", + file_name: "guideline.pdf", + file_type: "application/pdf", + page_count: 2, + chunk_count: 1, + storage_path: `${userId}/documents/guideline.pdf`, + content_hash: "sha256:owned-hash", + metadata: { index_generation_id: "generation-a", extraction_quality: "good" }, + }); + } + if (["document_pages", "document_images", "document_chunks", "document_table_facts"].includes(call.table)) { + return ok([]); + } + return ok([]); + }); + mockRuntime(client); + const { GET } = await import("../src/app/api/documents/[id]/route"); + + const response = await GET(authenticatedRequest(`/api/documents/${documentId}`), { + params: Promise.resolve({ id: documentId }), + }); + const body = await payload(response); + const document = body.document as Record; + + expect(response.status).toBe(200); + expect(document).toMatchObject({ + id: documentId, + owner_id: userId, + storage_path: `${userId}/documents/guideline.pdf`, + content_hash: "sha256:owned-hash", + }); + expect(body).toHaveProperty("indexHealth"); }); it("allows anonymous users to read public document detail", async () => { diff --git a/tests/registry-records-route.test.ts b/tests/registry-records-route.test.ts index ab95c3680..9373179c4 100644 --- a/tests/registry-records-route.test.ts +++ b/tests/registry-records-route.test.ts @@ -213,8 +213,22 @@ describe("registry records API", () => { expect(payload.publicAccess).toBe(true); expect(payload.records.some((record) => record.slug === "13yarn")).toBe(true); expect(payload.matches?.[0]?.record.slug).toBe("13yarn"); + // The full catalog is served from seed data (no table read) and no auth round-trip is + // needed, but anonymous list requests must still pass the registry limiter (M4/C1). + expect(client.from).not.toHaveBeenCalled(); + expect(client.rpc).toHaveBeenCalled(); + expect(client.auth.getUser).not.toHaveBeenCalled(); + }); + + it("rate-limits anonymous list requests (429) without falling back to unlimited access", async () => { + const client = createSupabaseMock(() => ok([]), { limited: true }); + mockRuntime(client); + const { GET } = await import("../src/app/api/registry/records/route"); + + const response = await GET(request("/api/registry/records?kind=service")); + + expect(response.status).toBe(429); expect(client.from).not.toHaveBeenCalled(); - expect(client.rpc).not.toHaveBeenCalled(); expect(client.auth.getUser).not.toHaveBeenCalled(); }); @@ -323,8 +337,24 @@ describe("registry records API", () => { expect(payload.publicAccess).toBe(true); expect(payload.record.slug).toBe("13yarn"); expect(payload.linkedDocuments).toEqual([]); + // The seed detail is served without a table read or auth round-trip, but anonymous detail + // requests must still pass the registry limiter (finding C — no anonymous bypass). + expect(client.from).not.toHaveBeenCalled(); + expect(client.rpc).toHaveBeenCalled(); + expect(client.auth.getUser).not.toHaveBeenCalled(); + }); + + it("rate-limits anonymous detail requests (429) instead of skipping the limiter", async () => { + const client = createSupabaseMock(() => ok([]), { limited: true }); + mockRuntime(client); + const { GET } = await import("../src/app/api/registry/records/[slug]/route"); + + const response = await GET(request("/api/registry/records/13YARN?kind=service"), { + params: Promise.resolve({ slug: "13YARN" }), + }); + + expect(response.status).toBe(429); expect(client.from).not.toHaveBeenCalled(); - expect(client.rpc).not.toHaveBeenCalled(); expect(client.auth.getUser).not.toHaveBeenCalled(); }); diff --git a/tests/worker-visual-capture.test.ts b/tests/worker-visual-capture.test.ts index 91147f06d..d74793a19 100644 --- a/tests/worker-visual-capture.test.ts +++ b/tests/worker-visual-capture.test.ts @@ -40,8 +40,11 @@ describe("worker visual capture hardening", () => { expect(workerSource).toContain("p_job_id: args.jobId"); expect(workerSource).toContain("p_worker_id: workerId"); expect(workerSource).not.toContain("await replacePageRows(args.documentId, args.pages)"); - expect(workerSource).toContain("await deleteStaleIndexGenerationRows(args.documentId, args.indexGenerationId)"); - expect(workerSource).toContain("async function deleteStaleIndexGenerationRows"); + // The commit RPC prunes stale/legacy generation rows and upserts index quality atomically + // server-side; the worker must surface any RPC failure (fail closed) rather than run a + // client-side fallback that could partially write after an error. + expect(workerSource).toContain('if (error) throw supabaseStageError("commit_document_index_generation", error)'); + expect(workerSource).not.toContain("deleteStaleIndexGenerationRows"); expect(workerSource).toContain("`${imagePrefix}/${indexGenerationId}/image-${index + 1}${ext}`"); expect(workerSource).toContain('indexing_v3_agent_repair_reason: "core_index_committed"'); expect(workerSource).toContain("indexing_v3_agent_repair_reason: null"); diff --git a/worker/main.ts b/worker/main.ts index 837800fa5..f0027f40a 100644 --- a/worker/main.ts +++ b/worker/main.ts @@ -89,23 +89,6 @@ type OptionalIndexWriteIssue = { code?: string | null; }; -type GenerationTableResult = { - data: unknown[] | null; - error: { message?: string; code?: string; details?: string; hint?: string } | null; -}; - -type GenerationTableFilter = PromiseLike & { - eq: (column: string, value: string) => GenerationTableFilter; - neq: (column: string, value: string) => PromiseLike; - is: (column: string, value: null) => PromiseLike; - limit: (count: number) => GenerationTableFilter; -}; - -type GenerationTableQuery = { - select: (columns: string) => GenerationTableFilter; - delete: () => GenerationTableFilter; -}; - function supabaseStageError( stage: string, error: { message?: string; code?: string; details?: string; hint?: string }, @@ -541,10 +524,10 @@ async function commitDocumentIndexGeneration(args: { })), p_quality: sanitizeJsonbRecord(args.quality), }); - if (!error) return; - throw supabaseStageError("commit_document_index_generation", error); - await upsertIndexQuality(args.quality); - await deleteStaleIndexGenerationRows(args.documentId, args.indexGenerationId); + // The RPC upserts index quality and prunes stale/legacy generation rows atomically server-side + // (20260702000000_commit_generation_preserve_legacy_artifacts, lease-fenced by + // 20260713062125_fence_index_generation_commit), so the worker only needs to surface failures. + if (error) throw supabaseStageError("commit_document_index_generation", error); } function buildDocumentPageRows(documentId: string, extracted: ExtractedDocument) { @@ -557,65 +540,6 @@ function buildDocumentPageRows(documentId: string, extracted: ExtractedDocument) })); } -// Audit M13 (mirrors 20260702000000_commit_generation_preserve_legacy_artifacts): -// this client-side fallback (used only when the commit RPC is missing) must -// apply the same preservation rule as the RPC — legacy generationless rows are -// purged only when this generation wrote replacement rows into the same -// table, so a transient artifact-write failure cannot destroy the -// previously-good legacy artifacts. Rows tagged with a DIFFERENT generation -// are always removed. Note: chunk-anchored artifacts (table facts, embedding -// fields, index units) cascade with their legacy chunks regardless — the -// guarantee fully protects images, memory cards, and sections. -async function deleteStaleIndexGenerationRows(documentId: string, indexGenerationId: string) { - // The generated Supabase client only types known table literals. This cleanup - // path selects among a small runtime-known set of tables, so use a minimal - // adapter at the dynamic boundary instead of widening the whole admin client. - const fromGenerationTable = supabase.from.bind(supabase) as unknown as (table: string) => GenerationTableQuery; - const hasReplacementRows = async (table: string, direct: boolean) => { - let query = fromGenerationTable(table).select("id").eq("document_id", documentId).limit(1); - query = direct - ? query.eq("index_generation_id", indexGenerationId) - : query.eq("metadata->>index_generation_id", indexGenerationId); - const { data, error } = await query; - if (error) throw supabaseStageError(`check replacement ${table}`, error); - return (data ?? []).length > 0; - }; - const deleteDirectGenerationRows = async (table: string) => { - const stale = await fromGenerationTable(table) - .delete() - .eq("document_id", documentId) - .neq("index_generation_id", indexGenerationId); - if (stale.error) throw supabaseStageError(`delete stale ${table}`, stale.error); - if (!(await hasReplacementRows(table, true))) return; - const missing = await fromGenerationTable(table) - .delete() - .eq("document_id", documentId) - .is("index_generation_id", null); - if (missing.error) throw supabaseStageError(`delete generationless ${table}`, missing.error); - }; - const deleteMetadataGenerationRows = async (table: string) => { - const stale = await fromGenerationTable(table) - .delete() - .eq("document_id", documentId) - .neq("metadata->>index_generation_id", indexGenerationId); - if (stale.error) throw supabaseStageError(`delete stale ${table}`, stale.error); - if (!(await hasReplacementRows(table, false))) return; - const missing = await fromGenerationTable(table) - .delete() - .eq("document_id", documentId) - .is("metadata->>index_generation_id", null); - if (missing.error) throw supabaseStageError(`delete generationless ${table}`, missing.error); - }; - - await deleteDirectGenerationRows("document_chunks"); - await deleteMetadataGenerationRows("document_images"); - await deleteMetadataGenerationRows("document_table_facts"); - await deleteMetadataGenerationRows("document_embedding_fields"); - await deleteMetadataGenerationRows("document_index_units"); - await deleteMetadataGenerationRows("document_memory_cards"); - await deleteMetadataGenerationRows("document_sections"); -} - async function upsertIndexQuality(quality: ReturnType) { const { error } = await supabase .from("document_index_quality")