fix: align final production migration history#565
Conversation
|
Updates to Preview Branch (codex/production-migration-history-final) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughSupabase migrations now restrict retrieval and ingestion RPC execution to ChangesRPC privilege reconciliation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/supabase-schema.test.ts`:
- Around line 970-985: Strengthen the ACL migration test in the existing test
case by asserting the exact from public, anon, authenticated and to service_role
clauses for all four functions, preserving each function signature. Extend the
duplicate-migration assertions to verify it contains no set search_path, revoke
execute, or grant execute statements while retaining its neutralized marker and
false no-op assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c3b1135f-80a4-46e6-b452-e0391114e591
📒 Files selected for processing (5)
supabase/drift-manifest.jsonsupabase/migrations/20260708150150_harden_retrieval_public_execute.sqlsupabase/migrations/20260709062443_reconcile_ingestion_rpc_privileges_production.sqlsupabase/migrations/20260709150000_reconcile_ingestion_rpc_privileges.sqltests/supabase-schema.test.ts
| it("codifies production ACL migration versions and neutralizes the later duplicate", () => { | ||
| expect(retrievalPublicExecuteMigration).toContain( | ||
| "revoke execute on function public.retrieval_owner_matches(uuid, uuid)", | ||
| ); | ||
| expect(retrievalPublicExecuteMigration).toContain( | ||
| "revoke execute on function public.search_document_chunks(uuid, text, integer, uuid)", | ||
| ); | ||
| expect(ingestionRpcPrivilegesMigration).toContain( | ||
| "revoke execute on function public.complete_ingestion_job(uuid, uuid, uuid, text, text)", | ||
| ); | ||
| expect(ingestionRpcPrivilegesMigration).toContain( | ||
| "revoke execute on function public.fail_or_retry_ingestion_job(uuid, uuid, uuid, boolean, text, text, text, timestamp with time zone, text)", | ||
| ); | ||
| expect(ingestionRpcPrivilegesDuplicateMigration).toContain("NEUTRALIZED 2026-07-13"); | ||
| expect(ingestionRpcPrivilegesDuplicateMigration).toContain("select 1 where false;"); | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Assert the complete ACL and no-op contracts.
These assertions would remain green if service_role grants were removed, the revoked roles changed, or the neutralized migration retained active ACL SQL. Assert both the exact from public, anon, authenticated and to service_role clauses for all four functions, and assert the duplicate contains no set search_path, revoke execute, or grant execute statements.
Suggested assertions
+ for (const [migration, signature] of [
+ [retrievalPublicExecuteMigration, "public.retrieval_owner_matches(uuid, uuid)"],
+ [retrievalPublicExecuteMigration, "public.search_document_chunks(uuid, text, integer, uuid)"],
+ [ingestionRpcPrivilegesMigration, "public.complete_ingestion_job(uuid, uuid, uuid, text, text)"],
+ [ingestionRpcPrivilegesMigration, "public.fail_or_retry_ingestion_job(uuid, uuid, uuid, boolean, text, text, text, timestamp with time zone, text)"],
+ ] as const) {
+ expect(migration).toContain(
+ `revoke execute on function ${signature} from public, anon, authenticated;`,
+ );
+ expect(migration).toContain(
+ `grant execute on function ${signature} to service_role;`,
+ );
+ }
+ expect(ingestionRpcPrivilegesDuplicateMigration).not.toMatch(
+ /set search_path|revoke execute on function|grant execute on function/,
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it("codifies production ACL migration versions and neutralizes the later duplicate", () => { | |
| expect(retrievalPublicExecuteMigration).toContain( | |
| "revoke execute on function public.retrieval_owner_matches(uuid, uuid)", | |
| ); | |
| expect(retrievalPublicExecuteMigration).toContain( | |
| "revoke execute on function public.search_document_chunks(uuid, text, integer, uuid)", | |
| ); | |
| expect(ingestionRpcPrivilegesMigration).toContain( | |
| "revoke execute on function public.complete_ingestion_job(uuid, uuid, uuid, text, text)", | |
| ); | |
| expect(ingestionRpcPrivilegesMigration).toContain( | |
| "revoke execute on function public.fail_or_retry_ingestion_job(uuid, uuid, uuid, boolean, text, text, text, timestamp with time zone, text)", | |
| ); | |
| expect(ingestionRpcPrivilegesDuplicateMigration).toContain("NEUTRALIZED 2026-07-13"); | |
| expect(ingestionRpcPrivilegesDuplicateMigration).toContain("select 1 where false;"); | |
| }); | |
| it("codifies production ACL migration versions and neutralizes the later duplicate", () => { | |
| expect(retrievalPublicExecuteMigration).toContain( | |
| "revoke execute on function public.retrieval_owner_matches(uuid, uuid)", | |
| ); | |
| expect(retrievalPublicExecuteMigration).toContain( | |
| "revoke execute on function public.search_document_chunks(uuid, text, integer, uuid)", | |
| ); | |
| expect(ingestionRpcPrivilegesMigration).toContain( | |
| "revoke execute on function public.complete_ingestion_job(uuid, uuid, uuid, text, text)", | |
| ); | |
| expect(ingestionRpcPrivilegesMigration).toContain( | |
| "revoke execute on function public.fail_or_retry_ingestion_job(uuid, uuid, uuid, boolean, text, text, text, timestamp with time zone, text)", | |
| ); | |
| for (const [migration, signature] of [ | |
| [retrievalPublicExecuteMigration, "public.retrieval_owner_matches(uuid, uuid)"], | |
| [retrievalPublicExecuteMigration, "public.search_document_chunks(uuid, text, integer, uuid)"], | |
| [ingestionRpcPrivilegesMigration, "public.complete_ingestion_job(uuid, uuid, uuid, text, text)"], | |
| [ingestionRpcPrivilegesMigration, "public.fail_or_retry_ingestion_job(uuid, uuid, uuid, boolean, text, text, text, timestamp with time zone, text)"], | |
| ] as const) { | |
| expect(migration).toContain( | |
| `revoke execute on function ${signature} from public, anon, authenticated;`, | |
| ); | |
| expect(migration).toContain( | |
| `grant execute on function ${signature} to service_role;`, | |
| ); | |
| } | |
| expect(ingestionRpcPrivilegesDuplicateMigration).toContain("NEUTRALIZED 2026-07-13"); | |
| expect(ingestionRpcPrivilegesDuplicateMigration).toContain("select 1 where false;"); | |
| expect(ingestionRpcPrivilegesDuplicateMigration).not.toMatch( | |
| /set search_path|revoke execute on function|grant execute on function/, | |
| ); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/supabase-schema.test.ts` around lines 970 - 985, Strengthen the ACL
migration test in the existing test case by asserting the exact from public,
anon, authenticated and to service_role clauses for all four functions,
preserving each function signature. Extend the duplicate-migration assertions to
verify it contains no set search_path, revoke execute, or grant execute
statements while retaining its neutralized marker and false no-op assertion.
Summary
Why
A linked production migration-list preflight found
20260708150150and20260709062443as remote-only after PR #552 merged. Their recorded statements were read-only inspected and match the repository's canonical service-role-only ACL posture. Adding the real production versions removes the final migration-history divergence before the reviewed RAG remediation migrations are applied.Verification
npx vitest run tests/supabase-schema.test.ts— 46/46 passednpm run drift:manifest— scratch PostgreSQL schema replay passed; manifest regeneratednpx supabase migration list --linked— these two versions now align; only documented no-op/guard and unapplied remediation migrations remain local-onlygit diff --check— passedSafety
No live schema mutation is performed by this PR. The new files codify ACL statements already recorded on production; the later duplicate is a documented no-op.