[BigSimmo]

Summary

Before: three database privacy rules (manual document tags, document sections and document memory cards) checked that a row was yours, but not that the document it hangs off was yours. In principle, a signed-in doctor could attach their own tag, section or memory card to another doctor's document.

After: those rules also require the parent document to be yours. The two-user privacy test's known-gap list is now empty, so the test fails if any of these rules loosens again.

Nothing could reach the gap before this change. Browser roles have no grant on these tables, and the server routes check document ownership before they write. This closes the gap at the database as well.

How: one new migration uses alter policy ... with check on four policies (labels owner manual insert, labels owner manual update, document sections owner all, document memory cards owner all). It adds an exists check against public.documents.owner_id. Only WITH CHECK changes, so reads and deletes are unchanged. The server's service role bypasses row-level security, so app behaviour is unchanged. supabase/schema.sql mirrors the change, and the drift manifest and migration seal are regenerated. The privacy test also probes moving a tag, section or memory card onto the other doctor's document.

#3556 (the two-user privacy test) has merged, and main is merged in here, so the diff is now only this change.

Areas touched: Data platform and access, Delivery and assurance, Knowledge and records, Source intake and indexing

Verification

• Checks run:
  • npm run test:sql after merging main: All 5 SQL behaviour tests passed.
  • The migration applied on top of the previous schema.sql, then the privacy test: TWO_USER_PRIVACY_SQL_PASS.
  • npm run check:migration-role: Hosted migration-role guard passed
  • npm run check:migration-immutability: Migration immutability guard passed: 310 shipped migrations unchanged.
  • npm run drift:manifest regenerated from the new schema.sql.

Risk and rollout

• Risk: low. This is a policy predicate change only, on tables browser roles cannot reach. Merging applies it to the live database within seconds.
• Rollback: a new migration restoring the previous WITH CHECK expressions.
• Provider or production effects: merging changes four row-level security policies on the live database. The owner gave a named yes for this specific change on 15 Oct 2026. Merge only inside an approved window.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KhogQUCHgdtFBHFtpjnd1n

---

Generated by Claude Code

Summary by CodeRabbit

• Security
  • Document labels, sections, and memory cards can now be added or updated only when they reference a document owned by the same authenticated account.
  • Attempts to move these records onto another account’s document are rejected, strengthening safeguards against cross-account access.

[supabase]

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy due to reaching the limit of concurrent preview branches.
Go to Project Integrations Settings ↗︎ if you wish to update this limit.

---

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: BigSimmo/PsychSift/.coderabbit.yaml
• Review profile: CHILL
• Plan: Team
• Run ID: e5703a2e-0001-4e5f-bb6a-e5cb9de5b2f7

📥 Commits

Reviewing files that changed from the base of the PR and between 0e04b21 and f0fc989.

📒 Files selected for processing: 5

• supabase/applied-migration-hashes.json
• supabase/drift-manifest.json
• supabase/migrations/20261015160000_document_child_policies_check_parent_owner.sql
• supabase/schema.sql
• tests/sql/two-user-privacy.sql

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

📝 Walkthrough

Write policies for document labels, sections, and memory cards now check ownership of the referenced document. The two-user SQL privacy test probes cross-owner reassignment. The migration and database manifests record the policy changes.

Changes: Database privacy and SQL testing

Layer / File(s)	Summary
Parent-document ownership policies supabase/migrations/*, supabase/schema.sql, supabase/drift-manifest.json, supabase/applied-migration-hashes.json	The migration and schema add parent-document ownership checks to write policies for labels, sections, and memory cards. The drift and migration-hash manifests record the updated database state.
Two-user privacy checks tests/sql/two-user-privacy.sql	The test removes the known parent-document gaps and probes whether users can move existing child rows to another user’s document.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix

Suggested reviewers: claude

Merge Risk: ⚪ Minimal · up to f0fc9

No actionable merge-blocking risk is established. The live policy change should still follow the approved deployment window and owner approval.

Pre-merge checks: 5

Passed checks: 5 passed

Check name	Status	Explanation
Docstring Coverage		No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Title check		The title clearly and concisely describes the main change: enforcing parent-document ownership for document tags, sections, and memory cards.
Description check		The description includes the required summary, verification results, risk, rollback, production effects, and affected areas. The optional Notes section is omitted, but the description is otherwise com…

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

[codeant-ai]

🏁 CodeAnt Quality Gate Results

Commit: f0fc9895
Scan Time: 2026-10-15 16:53:49 UTC

✅ Overall Status: PASSED

Quality Gate Details

Quality Gate	Status	Details
Secrets	✅ PASSED	0 secrets found
Duplicate Code	✅ PASSED	0.0% duplicated
SAST	✅ PASSED	No security issues
Bugs	✅ PASSED	Rating S: No bugs
IAC	✅ PASSED	Rating S: No issues

View Full Results

[gitar-bot]

Gitar has auto-approved this PR (configure)

[greptile-apps]

BigSimmo has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

[codeant-ai]

🤖 CodeAnt AI — Review Status

Status	Commit	Started (UTC)	Finished (UTC)
✅ Reviewed your PR	4091fd5	Oct 15, 2026 · 16:11	16:22

[codeant-ai]

---

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-15T16:15:36.194250Z	a7d1fc8	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[qodo-code-review]

PR Summary by Qodo

Require document ownership for child-row writes

🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

AI Description

• Require document ownership when writing manual labels, sections, and memory cards.
• Add two-user SQL privacy coverage and run database behaviour tests locally and in CI.
• Synchronize the schema, drift manifest, and migration seal with the new policies.

Diagram

graph TD
  Actor["Signed-in doctor"] --> Row{"Own child row?"} -->|yes| Docs["Parent document"] --> Parent{"Own document?"} -->|yes| Write["Write allowed"]
  Row -->|no| Deny["Write denied"]
  Parent -->|no| Deny

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Enforce ownership with composite foreign keys

• ➕ Would enforce parent-child owner consistency for writes that bypass RLS, including service-role writes.
• ➖ Requires broader schema changes and validation of existing data and service write paths.

2. Rely on server ownership checks

• ➕ Avoids a database policy migration.
• ➖ Leaves the database policy gap open if access paths or grants change.

Recommendation: Use the targeted WITH CHECK migration for this PR: it closes the identified policy gap without changing read/delete access or current service-role behaviour. A database-wide ownership constraint would be a separate, broader undertaking.

Files changed (12) +738 / -17

Bug fix (2) +53 / -4

20261015160000_document_child_policies_check_parent_owner.sqlRequire parent ownership in four write policies +37/-0

Require parent ownership in four write policies

• Tightens manual-label insert/update and section and memory-card policies to reject writes unless the referenced document belongs to the authenticated user. It changes only WITH CHECK expressions.

supabase/migrations/20261015160000_document_child_policies_check_parent_owner.sql

schema.sqlMirror document-child policy checks +16/-4

Mirror document-child policy checks

• Adds the same parent-document ownership checks to the schema definitions for the four affected policies.

supabase/schema.sql

Documentation (1) +1 / -0

data-platform.jsonAssign the SQL runner to Data platform +1/-0

Assign the SQL runner to Data platform

• Registers the new SQL behaviour runner in the Data platform system inventory.

docs/organisation/systems/data-platform.json

Other (9) +684 / -13

ci.ymlRun SQL behaviour tests after migration replay +16/-0

Run SQL behaviour tests after migration replay

• Adds a CI step that executes every tests/sql file against the replayed migration chain, failing the job on SQL errors.

.github/workflows/ci.yml

package.jsonExpose a local SQL test command +1/-0

Expose a local SQL test command

• Adds npm run test:sql as the entry point for the scratch-database behaviour tests.

package.json

ci-change-scope.mjsInclude SQL tests in database change scope +1/-0

Include SQL tests in database change scope

• Classifies changes under tests/sql as database-related for CI scope selection.

scripts/ci-change-scope.mjs

test-sql-behaviour.tsAdd disposable-database SQL test runner +226/-0

Add disposable-database SQL test runner

• Replays schema.sql in a digest-pinned Docker container and runs selected SQL behaviour tests. It supplies the missing bare-image auth.jwt() definition and removes only its worktree-owned container unless inspection is requested.

scripts/test-sql-behaviour.ts

applied-migration-hashes.jsonSeal the new migration +3/-2

Seal the new migration

• Records the new migration hash and refreshes the seal timestamp without changing earlier migration hashes.

supabase/applied-migration-hashes.json

drift-manifest.jsonRefresh expected policy snapshot +7/-7

Refresh expected policy snapshot

• Updates the schema digest and the four WITH CHECK expressions in the generated drift snapshot.

supabase/drift-manifest.json

enrichment-repair.sqlRepair processing-job test fixtures +14/-4

Repair processing-job test fixtures

• Supplies complete lease tuples for processing indexing and ingestion jobs so the fixtures satisfy current database constraints.

tests/sql/enrichment-repair.sql

roster-behaviour.sqlExplicitly roll back roster fixtures +4/-0

Explicitly roll back roster fixtures

• Ends the test with ROLLBACK so its synthetic data cannot persist when the file is run outside the transactional runners.

tests/sql/roster-behaviour.sql

two-user-privacy.sqlAdd two-user database privacy regression test +412/-0

Add two-user database privacy regression test

• Tests browser-role privilege denial, callable-function restrictions, and owner-policy isolation using two synthetic doctors and temporary grants. Cross-owner insert probes now require child rows to be rejected when only their parent document belongs to the other doctor.

tests/sql/two-user-privacy.sql

[claude]

Claude encountered an error after 2s —— View job

---

I'll analyze this and get back to you.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a7d1fc87aa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

No findings for this group

No findings for this group

No findings for this group

Resolved findings

1. Cross-document updates go untested ✓ Resolved

Description

check_isolation probes cross-document inserts and owner changes, but never updates an owned label,
section, or memory card so that only its document_id points to another doctor's document. If the
parent-ownership condition is removed from any of the three UPDATE policies, the owner-change probe
can still be rejected by the owner check while the INSERT probes continue to pass.

Code

tests/sql/two-user-privacy.sql[R369-374]

+        execute format('update %s set %I = %L::uuid where %I = %L::uuid', r.rel, r.owner_col, v_other, r.owner_col, v_me);
+        get diagnostics v_rows = row_count;
+        if v_rows > 0 then
+          raise exception 'PRIVACY LEAK: doctor % moved % row(s) into doctor %''s name in %', p_me, v_rows, p_other, r.rel;
+        end if;
+      exception when insufficient_privilege then null;

Relevance

●●● Strong

Accepted testing precedent favors targeted SQL regressions for unexercised policy paths and edge
cases.

PR-#3485
PR-#2369

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The migration adds a parent-document ownership condition to three policies governing updates, but
the test's UPDATE probes never assign document_id: they change an owner column or target a foreign
row. The test assigns a foreign document only in an INSERT probe, which does not exercise the
separate manual-label UPDATE policy or independently test UPDATE through the all-operations
policies.

tests/sql/two-user-privacy.sql[366-375]
supabase/migrations/20261015160000_document_child_policies_check_parent_owner.sql[21-36]
tests/sql/two-user-privacy.sql[299-307]
tests/sql/two-user-privacy.sql[324-335]
tests/sql/two-user-privacy.sql[366-376]
Skill: testing-review
Skill: access

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The two-user privacy test does not exercise the new parent-ownership condition when an existing label, section, or memory card is updated to reference another doctor's document.

## Fix Focus Areas
- tests/sql/two-user-privacy.sql[299-307]
- tests/sql/two-user-privacy.sql[366-376]
- supabase/migrations/20261015160000_document_child_policies_check_parent_owner.sql[21-36]

## Recommended Fix
For each affected child table, attempt to update an owned fixture row so that only its `document_id` points to the other doctor's existing document. Require a row-level-security refusal, make the probe fail if the update succeeds or reaches a constraint error, and keep the original fixture row unchanged.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

✅ Compliance rules (platform): 9 rules

✅ Skills: 7 invoked

  testing-review

  test

  access

  privacy

  security-review

  supabase

  review

✅ Cross-repo context — repo relationships

✅ REVIEW.md

Review mode: Auto: ⚖️ Balanced: Runtime timeout and interaction changes carry real, localized behavioral risk.

Tip of the day

💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit f0fc989

Results up to commit 4091fd5 🧠 Deep

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

No findings for this group

No findings for this group

No findings for this group

Resolved findings

1. Cross-document updates go untested ✓ Resolved

Description

check_isolation probes cross-document inserts and owner changes, but never updates an owned label,
section, or memory card so that only its document_id points to another doctor's document. If the
parent-ownership condition is removed from any of the three UPDATE policies, the owner-change probe
can still be rejected by the owner check while the INSERT probes continue to pass.

Code

tests/sql/two-user-privacy.sql[R369-374]

+        execute format('update %s set %I = %L::uuid where %I = %L::uuid', r.rel, r.owner_col, v_other, r.owner_col, v_me);
+        get diagnostics v_rows = row_count;
+        if v_rows > 0 then
+          raise exception 'PRIVACY LEAK: doctor % moved % row(s) into doctor %''s name in %', p_me, v_rows, p_other, r.rel;
+        end if;
+      exception when insufficient_privilege then null;

Relevance

●●● Strong

Accepted testing precedent favors targeted SQL regressions for unexercised policy paths and edge
cases.

PR-#3485
PR-#2369

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The migration adds a parent-document ownership condition to three policies governing updates, but
the test's UPDATE probes never assign document_id: they change an owner column or target a foreign
row. The test assigns a foreign document only in an INSERT probe, which does not exercise the
separate manual-label UPDATE policy or independently test UPDATE through the all-operations
policies.

tests/sql/two-user-privacy.sql[366-375]
supabase/migrations/20261015160000_document_child_policies_check_parent_owner.sql[21-36]
tests/sql/two-user-privacy.sql[299-307]
tests/sql/two-user-privacy.sql[324-335]
tests/sql/two-user-privacy.sql[366-376]
Skill: testing-review
Skill: access

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The two-user privacy test does not exercise the new parent-ownership condition when an existing label, section, or memory card is updated to reference another doctor's document.

## Fix Focus Areas
- tests/sql/two-user-privacy.sql[299-307]
- tests/sql/two-user-privacy.sql[366-376]
- supabase/migrations/20261015160000_document_child_policies_check_parent_owner.sql[21-36]

## Recommended Fix
For each affected child table, attempt to update an owned fixture row so that only its `document_id` points to the other doctor's existing document. Require a row-level-security refusal, make the probe fail if the update succeeds or reaches a constraint error, and keep the original fixture row unchanged.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

Qodo Fixer

✅ Committed (1) · ☑ Fixed (1)

Commits pushed directly to this PR — no separate fix PR opened.

Process — 1 fixed

• ☑ Fixed: Cross-document updates go untested

[gitar-bot]

Gitar has auto-approved this PR (configure)

[gitar-bot]

Gitar has auto-approved this PR (configure)

[codeant-ai]

CodeAnt Nitpicks

2 code suggestions

1. removeOwnedContainer can throw from finally, replacing the accumulated test-failure summary with a cleanup error when Docker removal fails.

Possible bug · scripts/test-sql-behaviour.ts:216

---

2. The UPDATE probe only changes owner_id; it never moves an owner's label, section, or card to the other document, so a missing parent-owner UPDATE check goes undetected.

Incomplete implementation · tests/sql/two-user-privacy.sql:369

[codeant-ai]

CodeAnt PR Risk: Low Risk

• The PR appears safe to merge; the parent-owner checks tighten writes without changing read or delete policies.
• The migration and schema snapshot show the same checks for labels, sections, and memory cards.
• CI runs the SQL behaviour tests against the replayed migration chain, including the two-user privacy test.

Assessed commit: 683e94e2e710

[qodo-code-review]

Code review by qodo was updated up to the latest commit 683e94e

The merge-base changed after approval.

[qodo-code-review]

No code changes since the last review — review skipped

[gitar-bot]

Gitar has auto-approved this PR (configure)

[gitar-bot]

Code Review ✅ Approved

🔴 High risk · Tightening RLS checks for child rows changes authorization; incorrect owner/document matching could reject legitimate tag, section, or memory-card writes.

Adds parent-document ownership checks to the WITH CHECK clauses of four row-level security policies (manual labels insert/update, document sections, document memory cards), so a doctor can no longer attach their own tag, section, or memory card to another doctor's document. Includes a two-user privacy SQL test wired into CI. No issues found.

Verification

Rules No rules evaluated

Functional validation Not enabled

Set up

Auto-approval Approved

_{For more verification details, comment gitar display:verbose.}

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing compact verification results.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

---

Important

Your trial ends in 6 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.

_{Was this helpful? React with 👍 / 👎 | Gitar}