Skip to content

Fix public document access with a reversible corpus mode - #2369

Open
BigSimmo wants to merge 4 commits into
mainfrom
codex/chat-document-access-mode-f5e4
Open

Fix public document access with a reversible corpus mode#2369
BigSimmo wants to merge 4 commits into
mainfrom
codex/chat-document-access-mode-f5e4

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Aug 25, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add a service-role-only, reversible corpus visibility switch that snapshots each document's prior owner and metadata.public_corpus state before publishing.
  • Scope the effective switch to authoritative documents rows so high-volume derived artifact ownership is left unchanged and rollback remains bounded.
  • Harden the control tables to service-role read-only access, add the snapshot cascade index, reconcile 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:manifest was attempted twice; the isolated scratch PostgreSQL container shut down during bootstrap before schema replay. supabase/drift-manifest.json is therefore not regenerated.

Risk and rollout

  • Risk: This intentionally exposes every current document through the public document authorization boundary. It does not change answer generation or retrieval ranking code.
  • Rollback: service-role call select public.set_document_corpus_access_mode('private'); restores the exact captured pre-change document owner/public-marker state.
  • Provider or production effects: Explicitly authorized production migrations were applied to Clinical KB Database (sjrfecxgysukkwxsowpy), and public mode is currently active for all 2,851 documents.
  • RAG impact: no retrieval behaviour change — no retrieval RPC, ranking, chunking, scoring, or derived artifact ownership logic changed; only the document authorization marker/owner state was intentionally published.

Clinical Governance Preflight

  • Source-backed claims still require linked source verification before clinical use
  • No patient-identifiable document workflow was introduced or expanded without explicit governance approval — corpus publication was explicitly authorized, but corpus-level PHI classification was not independently re-audited in this task
  • Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy)
  • Service-role keys and private document access remain server-only
  • Demo/synthetic content remains clearly separated from real clinical sources
  • Source metadata, review status, and outdated/unknown-source behavior remain conservative
  • Deployment classification/TGA SaMD impact was checked when clinical decision-support behavior changed — not applicable; clinical decision-support behavior is unchanged

Notes

  • Published as a bare PR at the user's request. Local broad readiness gates and hosted CI observation were intentionally not added to this publication step.
  • The pre-commit documentation-index hook was bypassed with --no-verify after it reported the two new schema tables as unindexed in docs/codebase-index.md; no unrelated documentation expansion was added.
  • The push retained the ownership/auto-merge guard and used only the repository's scoped format, drift, and static overrides allowed for bare publication.

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=true can satisfy the stricter document viewer gate, with an explicit rollback path.

New tables document_corpus_access_state and document_corpus_access_snapshots default to private; calling set_document_corpus_access_mode('public') snapshots each document’s owner_id and exact public_corpus metadata, then clears owners and sets public_corpus on documents while briefly bypassing documents_require_publication_approval inside a serialized transaction. private restores 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 authoritative documents rows change—faster toggles and a smaller rollback surface.

Hardening keeps control tables service-role SELECT-only, adds document_corpus_access_snapshots_document_id_idx for cascade deletes, mirrors everything in supabase/schema.sql, and adds Vitest migration contract assertions (ACLs, snapshot semantics, no derived-table rewrites).

Reviewed by Cursor Bugbot for commit 68b04de. Configure here.

@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Next included review available in 31 minutes.

View limit details

Limit 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.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e6e58b02-9b36-4e21-81ea-84f508305477

📥 Commits

Reviewing files that changed from the base of the PR and between 069058a and e78eb01.

⛔ Files ignored due to path filters (1)
  • docs/codebase-index.md is excluded by !docs/**, !**/*.md
📒 Files selected for processing (7)
  • supabase/drift-manifest.json
  • supabase/migrations/20260825025032_reversible_document_corpus_access_mode.sql
  • supabase/migrations/20260825025717_scope_document_corpus_access_mode_to_documents.sql
  • supabase/migrations/20260825030411_harden_document_corpus_access_state.sql
  • supabase/migrations/20260826090000_fail_closed_deleted_document_owner_rollback.sql
  • supabase/schema.sql
  • tests/document-corpus-access-mode.test.ts

Comment @coderabbitai help to get the list of available commands.

@supabase

supabase Bot commented Aug 25, 2026

Copy link
Copy Markdown

Updates to Preview Branch (codex/chat-document-access-mode-f5e4) ↗︎

Deployments Status Updated
Database Tue, 25 Aug 2026 16:28:58 UTC
Services Tue, 25 Aug 2026 16:28:58 UTC
APIs Tue, 25 Aug 2026 16:28:58 UTC

Tasks are run on every commit but only new migration files are pushed.
Close and reopen this PR if you want to apply changes from existing seed or migration files.

Tasks Status Updated
Configurations Tue, 25 Aug 2026 16:29:00 UTC
Migrations Tue, 25 Aug 2026 16:29:02 UTC
Seeding Tue, 25 Aug 2026 16:29:04 UTC
Edge Functions Tue, 25 Aug 2026 16:29:08 UTC

View logs for this Workflow Run ↗︎.
Learn more about Supabase for Git ↗︎.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

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.

@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

CI triage

CI failed on this PR. Automated classification of the 2 failed job(s):

  • Migration replaynot baselined: this job did NOT run on the main comparison below (path-scoped skip), so that run says nothing about it either way. Treat the comparison as absent, not green, and inspect the failing step.
  • PR requiredneeds investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.

Compared with main CI run #13894 (success). That run's conclusion is an aggregate and did not exercise Migration replay.

Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Open in Web View Automation 

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.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved: Cursor Bugbot’s earlier high-severity rollback finding is resolved on this head, and no approval policy requires human review. No reviewers were assigned.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Open in Web View Automation 

Sent by Cursor Security Agent: Security Reviewer

Comment on lines +111 to +117
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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 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.

Fix in Cursor Fix in Web

Reviewed by Cursor Security Reviewer for commit e78eb01. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants