fix: load safeupdate but disable for all but Data API#2027
Open
fix: load safeupdate but disable for all but Data API#2027
Conversation
Contributor
📝 WalkthroughWalkthroughAdds an up-only migration that configures PostgreSQL to load the Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@nix/tests/sql/pg-safeupdate.sql`:
- Around line 15-18: The test grants schema privileges but not table privileges,
so the authenticated role lacks DELETE on v.foo; add an explicit table-level
grant (e.g., grant DELETE or GRANT ALL ON TABLE v.foo TO authenticated) before
switching role or running the DELETE so the test exercises safeupdate behavior
rather than permission errors; update the statements around GRANT ALL ON SCHEMA
v, set role authenticated, and the DELETE from v.foo accordingly.
🧹 Nitpick comments (1)
nix/tests/sql/pg-safeupdate.sql (1)
21-23: Minor: trailing blank lines.There are extra trailing blank lines at the end of the file that can be removed for cleanliness.
Member
Author
|
Failing on 15 because the |
steve-chavez
approved these changes
Jan 30, 2026
migrations/db/migrations/20260130074514_load_disable_pg_safeupdate.sql
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@migrations/db/migrations/20260130074514_load_disable_pg_safeupdate.sql`:
- Around line 2-3: The migration runs ALTER ROLE for the roles 'authenticated'
and 'anon' which will fail if those roles don't exist; update the migration to
guard those statements by checking for role existence first (e.g., query
pg_roles or use a DO block that checks IF EXISTS and only executes ALTER ROLE
when the role is present) so the ALTER ROLE session_preload_libraries =
'safeupdate' commands for roles 'authenticated' and 'anon' are applied
conditionally and the migration becomes idempotent.
Member
|
@encima I think we should also announce this in https://github.com/orgs/supabase/discussions/categories/changelog, it's a "breaking change" in some cases I assume. |
Collaborator
|
@encima @steve-chavez if we do that we need to make it approx 3 week ahead of time change announcement per our breaking change process thanks for catching that @steve-chavez |
encima
force-pushed
the
fix/enable-safeupdate
branch
from
b647c84 to
44c79c2
Compare
encima
force-pushed
the
fix/enable-safeupdate
branch
from
44c79c2 to
87bb02c
Compare
This comment has been minimized.
This comment has been minimized.
PostgreSQL Extension Dependency Analysis: PR #2027
SummaryNo extensions had dependencies with MAJOR version updates. Full Analysis ResultsPostgreSQL 15 Extension DependenciesPostgreSQL 17 Extension DependenciesOrioleDB 17 Extension Dependencies |
PostgreSQL Package Dependency Analysis: PR #2027
SummaryNo packages had MAJOR version updates. Full Analysis ResultsPostgreSQL 15 Dependency ChangesExtracting PostgreSQL 15 dependencies...
Runtime Closure Size
Raw Dependency ClosurePostgreSQL 17 Dependency ChangesExtracting PostgreSQL 17 dependencies...
Runtime Closure Size
Raw Dependency Closure |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to make pg-safeupdate available to Supabase Data API roles by preloading the safeupdate library for authenticated (and anon) and extending the Nix-based test suite to validate the behavior.
Changes:
- Adds a DB migration to set
session_preload_librariesforauthenticated/anonand loads thesafeupdatelibrary. - Extends the SQL regression test to verify
DELETEwithoutWHEREis rejected underauthenticated. - Adds a dedicated NixOS multi-version extension test for
safeupdate(PG 15 → 17 → OrioleDB 17).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
migrations/db/migrations/20260130074514_load_disable_pg_safeupdate.sql |
Configures role-level preloading for safeupdate and attempts to set safeupdate.enabled. |
nix/tests/sql/pg-safeupdate.sql |
Adds coverage for DELETE protection when running as authenticated. |
nix/tests/expected/pg-safeupdate.out |
Updates expected regression output to include the new DELETE failure. |
nix/tests/expected/roles.out |
Updates expected role configuration output for authenticated. |
nix/ext/tests/pg_safeupdate.nix |
Introduces a NixOS test to validate extension install/upgrade across PG major versions and OrioleDB. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
migrations/db/migrations/20260130074514_load_disable_pg_safeupdate.sql
Outdated
Show resolved
Hide resolved
migrations/db/migrations/20260130074514_load_disable_pg_safeupdate.sql
Outdated
Show resolved
Hide resolved
encima
force-pushed
the
fix/enable-safeupdate
branch
from
b732731 to
8d3dd2f
Compare
…r postgres by default but allow postgres role to load it
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What kind of change does this PR introduce?
Bug fix
What is the current behavior?
pg-safeupdate is only usable by the
authenticatorroleWhat is the new behavior?
pg-safeupdate is enabled by default for the
authenticatedrole - i.e. for the Data APICan be enabled for the
postgresuser by runningSET safeupdate.enabled=1;Additional context
Fixes #1308 (comment)
PSQL-910
Summary by CodeRabbit
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.