Skip to content

fix: load safeupdate but disable for all but Data API#2027

Open
encima wants to merge 6 commits intodevelopfrom
fix/enable-safeupdate
Open

fix: load safeupdate but disable for all but Data API#2027
encima wants to merge 6 commits intodevelopfrom
fix/enable-safeupdate

Conversation

@encima
Copy link
Member

@encima encima commented Jan 30, 2026

What kind of change does this PR introduce?

Bug fix

What is the current behavior?

pg-safeupdate is only usable by the authenticator role

What is the new behavior?

pg-safeupdate is enabled by default for the authenticated role - i.e. for the Data API
Can be enabled for the postgres user by running SET safeupdate.enabled=1;

Additional context

Fixes #1308 (comment)
PSQL-910

Summary by CodeRabbit

  • Chores

    • Added a database migration to install and configure a PostgreSQL safety extension (installed and configured but disabled by default).
  • Tests

    • Added test coverage to validate role setup, privilege elevation during maintenance, full-table cleanup, and schema removal behaviors.

✏️ Tip: You can customize this high-level summary in your review settings.

@encima encima requested review from a team as code owners January 30, 2026 12:01
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 30, 2026

📝 Walkthrough

Walkthrough

Adds an up-only migration that configures PostgreSQL to load the safeupdate extension for the authenticated and anon roles, loads the safeupdate extension, and sets safeupdate.enabled = 0. Adds a test SQL that grants privileges on schema v to the authenticated role, switches to that role to delete all rows from v.foo, resets the role, and drops schema v cascade. Adds a test bootstrap script to create authenticated and anon roles with NOLOGIN/NOINHERIT and grant USAGE on public to both.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

migrations, tests, extensions, nix

Suggested reviewers

  • samrose
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: loading safeupdate extension but disabling it for all roles except the Data API (authenticated role).
Description check ✅ Passed The description includes bug fix classification, current vs new behavior comparison, additional context with issue references, and clear objectives; it matches the spirit of the template requirements.
Linked Issues check ✅ Passed The PR addresses issue #1308 by enabling safeupdate for the authenticated role (Data API) through session_preload_libraries, resolving the constraint that prevented enabling it previously.
Out of Scope Changes check ✅ Passed All changes (migration script, test SQL, and test configuration) are directly related to implementing safeupdate support for the authenticated role as specified in the linked issue.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/enable-safeupdate

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.

❤️ Share

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

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.

@encima
Copy link
Member Author

encima commented Jan 30, 2026

Failing on 15 because the authenticated role does not exist...checking why

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

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.

@steve-chavez
Copy link
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.

@samrose
Copy link
Collaborator

samrose commented Feb 3, 2026

@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

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.

Cannot use safeupdate for the database nor the postgres user (session_preload_libraries)

3 participants