Fix public document access with a reversible corpus mode - #2369
Conversation
|
Warning Review limit reachedNext included review available in 31 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 95 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
Comment |
|
Updates to Preview Branch (codex/chat-document-access-mode-f5e4) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Rollback fails after owner deletion
- Private-mode restore now left-joins auth.users and writes null when the snapshotted owner was deleted, so rollback no longer hits the documents.owner_id foreign key.
You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 68b04de. Configure here.
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #13894 (success). That run's conclusion is an aggregate and did not exercise Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
There was a problem hiding this comment.
Stale comment
Left a non-blocking comment: Cursor Bugbot found 1 high-severity unresolved issue (rollback can fail after owner deletion), so this is not approved. Human review is needed; no additional reviewers were assigned because the only CODEOWNER is the PR author.
Sent by Cursor Approval Agent: Pull Request Router and Approver
Private-mode restore now left-joins auth.users before writing snapshot.owner_id, so a user deleted during a public window cannot abort the reversible switch.
There was a problem hiding this comment.
Stale comment
Left a non-blocking comment: Cursor Bugbot’s high-severity rollback finding is still unresolved on this head, so this is not approved. Human review is needed; no reviewers were assigned because the only CODEOWNER is the PR author.
Sent by Cursor Approval Agent: Pull Request Router and Approver
There was a problem hiding this comment.
Agentic security review of this head found one remaining HIGH issue on the deleted-owner private rollback path. Viewer/list authorization fails closed after stripping public_corpus, but live retrieval still treats documents.owner_id IS NULL as the public corpus.
Sent by Cursor Security Agent: Security Reviewer
| update public.documents d | ||
| set | ||
| owner_id = existing_owner.id, | ||
| metadata = case | ||
| -- Restoring a public marker without its former owner would turn an | ||
| -- owner-scoped row into a public row. Remove the marker instead. | ||
| when snapshot.owner_id is not null and existing_owner.id is null then |
There was a problem hiding this comment.
🔒 Agentic Security Review
Severity: HIGH
Private rollback for a deleted snapshotted owner sets owner_id to NULL and only removes metadata.public_corpus. That matches the document-viewer/list predicate (owner_id IS NULL AND public_corpus is true), so the row disappears from /documents, but retrieval still treats owner_id IS NULL as the public corpus and never consults public_corpus.
Impact: After an intended restore to private mode, formerly owner-scoped clinical text stays in anonymous/other-tenant RAG search. The document route looks correctly unavailable while /api/answer and scoped retrieval RPCs using the public sentinel keep matching those rows.
Reviewed by Cursor Security Reviewer for commit e78eb01. Configure here.




Summary
metadata.public_corpusstate before publishing.documentsrows so high-volume derived artifact ownership is left unchanged and rollback remains bounded.supabase/schema.sql, and add a focused migration contract test.Verification
npm run verify:pr-local— not run under the requested bare-PR publication route.npm run verify:ui— not run; no UI, routing, or styling code changed.npm run verify:release— not run; release confidence was not requested.npm run eval:retrieval:quality— not run; no retrieval, ranking, selection, chunking, or scoring implementation changed.npm run eval:rag -- --limit 15+npm run eval:quality -- --rag-only— not run; answer generation is unchanged.npm run check:production-readiness— not run under the requested bare-PR publication route.npm run check:deployment-readiness— not run; no application deployment change.npm run check:migration-role— passed.npm run check:function-grants— passed; all 36 SECURITY DEFINER functions remain revoked from PUBLIC/anon.PostgreSQL 17 full migration-chain replay — passed through the new access-mode migrations.
Local public -> private behavioral fixtures — passed, including exact owner/public-marker restoration and unchanged derived artifact ownership.
Live production verification — 2,851/2,851 documents explicitly public, 2,851 rollback snapshots, exact reported document route returned HTTP 200 without the unavailable state.
Blocked: focused Vitest wrapper could not acquire the repository heavy-test lease because another worktree owned it.
Blocked:
npm run drift:manifestwas attempted twice; the isolated scratch PostgreSQL container shut down during bootstrap before schema replay.supabase/drift-manifest.jsonis therefore not regenerated.Risk and rollout
select public.set_document_corpus_access_mode('private');restores the exact captured pre-change document owner/public-marker state.sjrfecxgysukkwxsowpy), and public mode is currently active for all 2,851 documents.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
--no-verifyafter it reported the two new schema tables as unindexed indocs/codebase-index.md; no unrelated documentation expansion was added.Note
High Risk
Corpus-wide publication mutates every document’s owner and public marker behind a trigger bypass; mis-invocation or partial rollback would have broad authorization impact, though the design is explicitly reversible via snapshots.
Overview
Adds a service-role-only operational switch so legacy ownerless documents without
metadata.public_corpus=truecan satisfy the stricter document viewer gate, with an explicit rollback path.New tables
document_corpus_access_stateanddocument_corpus_access_snapshotsdefault to private; callingset_document_corpus_access_mode('public')snapshots each document’sowner_idand exactpublic_corpusmetadata, then clears owners and setspublic_corpusondocumentswhile briefly bypassingdocuments_require_publication_approvalinside a serialized transaction.privaterestores from the snapshot rather than inventing new ownership. A follow-up migration drops updates to high-volume derived tables (labels, sections, embeddings, etc.) so only authoritativedocumentsrows change—faster toggles and a smaller rollback surface.Hardening keeps control tables service-role SELECT-only, adds
document_corpus_access_snapshots_document_id_idxfor cascade deletes, mirrors everything insupabase/schema.sql, and adds Vitest migration contract assertions (ACLs, snapshot semantics, no derived-table rewrites).Reviewed by Cursor Bugbot for commit 68b04de. Configure here.