Skip to content

refactor(db): remove user team provision triggers#2421

Open
ben-fornefeld wants to merge 2 commits intoremove/user-team-provision-triggersfrom
remove/user-team-provision-triggers-migration
Open

refactor(db): remove user team provision triggers#2421
ben-fornefeld wants to merge 2 commits intoremove/user-team-provision-triggersfrom
remove/user-team-provision-triggers-migration

Conversation

@ben-fornefeld
Copy link
Copy Markdown
Member

@ben-fornefeld ben-fornefeld commented Apr 16, 2026

Summary

  • remove the legacy main DB triggers/functions that synced auth.users into public.users
  • remove the signup-time default-team trigger and its related helper functions, policies, and grants
  • stack this migration on top of #2413, which updates seeding/test helpers to stop relying on those triggers

Depends On

Validation

  • reviewed the migration against the existing trigger/policy chain in packages/db/migrations

@cursor
Copy link
Copy Markdown

cursor bot commented Apr 16, 2026

PR Summary

High Risk
High risk because it changes database-side user/team bootstrap behavior and revokes related permissions; any remaining code paths that relied on the triggers for public.users projection or default team creation may break or leave orphaned/missing rows.

Overview
This PR removes the legacy Postgres trigger chain that projected auth.users into public.users and auto-created a default team on signup, along with the associated functions, RLS policies, and trigger_user grants. Seed and integration/local-dev helpers are updated to explicitly upsert/delete the projected public.users row (and adjust cleanup queries) instead of relying on trigger side effects when inserting/deleting auth.users.

Reviewed by Cursor Bugbot for commit 85d6908. Bugbot is set up for automated code reviews on this repo. Configure here.

Copy link
Copy Markdown

@cursor cursor bot left a comment

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 and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 5f9fdf7. Configure here.

END
$post_user_signup$ SECURITY DEFINER SET search_path = public;

ALTER FUNCTION public.post_user_signup() OWNER TO trigger_user;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Down migration restores incomplete post_user_signup function

Medium Severity

The post_user_signup function restored in the Down migration is missing two INSERT statements that exist in the latest production version (defined in 20250106142106_remove_team_is_default.sql): INSERT INTO public.team_api_keys (team_id) and INSERT INTO public.access_tokens (user_id). If this migration is rolled back, new user signups would get a team created but no API key or access token, breaking their ability to use the service.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 5f9fdf7. Configure here.

Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

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

The seed script (packages/db/scripts/seed/postgres/seed-db.go lines 133-148) inserts into auth.users relying on the sync_inserts_to_public_users trigger to create a public.users row. After this migration removes that trigger, the subsequent INSERT INTO users_teams at line 160 will hit an FK violation (user_id references public.users which has no row), breaking all integration tests. Merging this migration alone without the seed follow-up will fail CI.

REVOKE DELETE ON public.users FROM trigger_user;

REVOKE SELECT, INSERT, TRIGGER ON public.teams FROM trigger_user;
REVOKE INSERT ON public.users_teams FROM trigger_user;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The trigger_user role itself is not dropped here. The 20231220094836 migration granted CREATE+USAGE ON SCHEMA public, USAGE ON SCHEMA extensions, and USAGE ON SCHEMA auth to trigger_user - none of which are revoked in this migration. After applying this Up, trigger_user retains schema-level access despite having no functions or triggers. If trigger_user is being fully retired, add REVOKE for the schema grants and DROP ROLE IF EXISTS trigger_user (plus restore in Down).

Comment on lines +28 to +35
REVOKE SELECT (id) ON public.users FROM trigger_user;
REVOKE UPDATE ON public.users FROM trigger_user;
REVOKE DELETE ON public.users FROM trigger_user;

REVOKE SELECT, INSERT, TRIGGER ON public.teams FROM trigger_user;
REVOKE INSERT ON public.users_teams FROM trigger_user;

-- +goose Down
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 The Up migration revokes trigger_user grants on public.users, public.users_teams, and public.teams, but omits revoking INSERT on public.team_api_keys and public.access_tokens (granted in migration 20231220094836), and does not drop the RLS policies 'Allow to create an access token to new user' and 'Allow to create a team api key to new user' on those same tables. After this PR, trigger_user retains INSERT privileges and active permissive RLS policies on two security-sensitive tables with no remaining code path exercising them — a missed least-privilege cleanup.

Extended reasoning...

What the bug is: The migration explicitly cleans up trigger_user grants and RLS policies on public.users, public.users_teams, and public.teams, but two additional grants and two RLS policies from migration 20231220094836_create_triggers_and_policies.sql are left behind: GRANT INSERT ON public.team_api_keys TO trigger_user, GRANT INSERT ON public.access_tokens TO trigger_user, the policy 'Allow to create an access token to new user' ON public.access_tokens, and the policy 'Allow to create a team api key to new user' ON public.team_api_keys.

Code path that triggered these grants: In 20231220094836_create_triggers_and_policies.sql (lines 17-18), trigger_user was granted INSERT on both tables so the post_user_signup() trigger function could insert default team API keys and access tokens on user signup. The function was later updated in 20240605070918_refactor_triggers_and_policies.sql to explicitly INSERT INTO both tables (lines 67-71), confirming ongoing use. This PR now drops post_user_signup() entirely — the last code path using these privileges.

Why existing code doesn't prevent it: The migration correctly removes the triggers and functions, but the permission cleanup section (lines 28–35 of the new file) only issues REVOKE for public.users, public.users_teams, and public.teams. There are zero REVOKE statements for public.team_api_keys and public.access_tokens across all migrations, and zero DROP POLICY statements targeting the two policies named above. The grants and policies survive the migration untouched.

Impact: After this migration, trigger_user retains INSERT privileges on public.team_api_keys and public.access_tokens — two tables that store credentials and authentication tokens — with no functional trigger or application code path ever invoking those privileges. The retained permissive RLS policies compound the risk: any future code (or SQL injection gaining trigger_user context) could INSERT into these tables and pass RLS unchallenged. This directly contradicts the principle of least privilege that this PR is trying to enforce.

Step-by-step proof:

  1. 20231220094836_create_triggers_and_policies.sql: Issues GRANT INSERT ON public.team_api_keys TO trigger_user and GRANT INSERT ON public.access_tokens TO trigger_user; creates policies 'Allow to create an access token to new user' and 'Allow to create a team api key to new user'.
  2. 20240605070918_refactor_triggers_and_policies.sql: Updates post_user_signup() to INSERT INTO both tables — grants remain in active use.
  3. 20260416120000_remove_user_team_provision_triggers.sql (this PR): Drops post_user_signup() and all three associated triggers. REVOKE block only covers public.users, public.users_teams, and public.teams.
  4. Post-migration state: trigger_user still holds INSERT on public.team_api_keys and public.access_tokens, and both permissive RLS policies remain active — orphaned with no legitimate caller.

How to fix: Add to the Up migration's REVOKE block:

REVOKE INSERT ON public.team_api_keys FROM trigger_user;
REVOKE INSERT ON public.access_tokens FROM trigger_user;
DROP POLICY IF EXISTS "Allow to create a team api key to new user" ON public.team_api_keys;
DROP POLICY IF EXISTS "Allow to create an access token to new user" ON public.access_tokens;

And add corresponding GRANT + CREATE POLICY statements to the Down section for symmetry.

On the duplicate refutation: A refutation claims this duplicates bug_005. While both bugs cover the missing REVOKEs, this bug additionally identifies the two orphaned RLS policies that were never dropped. The policies independently allow trigger_user to pass RLS checks on INSERT — a distinct and additive security surface beyond the raw grants alone. The combined presence of both grants and permissive policies makes this more than a simple REVOKE omission.

Comment on lines +28 to +35
REVOKE SELECT (id) ON public.users FROM trigger_user;
REVOKE UPDATE ON public.users FROM trigger_user;
REVOKE DELETE ON public.users FROM trigger_user;

REVOKE SELECT, INSERT, TRIGGER ON public.teams FROM trigger_user;
REVOKE INSERT ON public.users_teams FROM trigger_user;

-- +goose Down
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 The migration revokes all table-level grants from trigger_user but omits the schema-level grants originally issued in 20231220094836_create_triggers_and_policies.sql. After this PR runs, trigger_user still holds CREATE, USAGE on the public schema and USAGE on the extensions and auth schemas — allowing it to create new objects in public despite its entire purpose being retired. Add REVOKE CREATE, USAGE ON SCHEMA public FROM trigger_user; REVOKE USAGE ON SCHEMA extensions FROM trigger_user; REVOKE USAGE ON SCHEMA auth FROM trigger_user; to complete the cleanup.

Extended reasoning...

What the bug is and how it manifests

Migration 20231220094836_create_triggers_and_policies.sql granted trigger_user three schema-level privileges: GRANT CREATE, USAGE ON SCHEMA public TO trigger_user, GRANT USAGE ON SCHEMA extensions TO trigger_user, and GRANT USAGE ON SCHEMA auth TO trigger_user. These were necessary for the role to own and execute trigger functions in those schemas. This PR removes every artifact of that trigger infrastructure (triggers, functions, policies, and table-level grants) but does not revoke these schema-level grants.

The specific code path

Lines 28–35 of the new migration revoke INSERT, SELECT(id), UPDATE, DELETE on public.users, and SELECT, INSERT, TRIGGER on public.teams, and INSERT on public.users_teams. There is no corresponding REVOKE ... ON SCHEMA ... statement anywhere in the Up section or anywhere else in the migration history (a grep for REVOKE.*SCHEMA across all migration files returns no results).

Why existing code doesn't prevent it

PostgreSQL schema-level grants are entirely independent of object-level grants and are not automatically cascaded or cleaned up when object-level grants are revoked. There is no mechanism that automatically removes CREATE ON SCHEMA public just because a role's table grants are revoked. The omission must be corrected explicitly.

Impact

After this migration, trigger_user retains CREATE on the public schema, meaning the role could still create new tables, functions, sequences, or other objects there. It also retains USAGE on extensions and auth schemas. For a role whose sole purpose was owning sync/signup triggers — a purpose this very PR eliminates — these are unnecessary residual privileges. If role credentials were ever compromised or the role were reused, the CREATE privilege represents meaningful attack surface.

How to fix it

Add the following three REVOKE statements to the Up section of the migration (and the corresponding GRANT statements to the Down section, mirroring how the table grants are restored there):

REVOKE CREATE, USAGE ON SCHEMA public FROM trigger_user;
REVOKE USAGE ON SCHEMA extensions FROM trigger_user;
REVOKE USAGE ON SCHEMA auth FROM trigger_user;

Step-by-step proof

  1. Search migration history: 20231220094836_create_triggers_and_policies.sql contains GRANT CREATE, USAGE ON SCHEMA public TO trigger_user, GRANT USAGE ON SCHEMA extensions TO trigger_user, GRANT USAGE ON SCHEMA auth TO trigger_user.
  2. Search all subsequent migrations for REVOKE.*SCHEMA: zero matches found.
  3. The new migration (lines 28–35) revokes only table-level grants — no schema revocations.
  4. After running goose up, SELECT has_schema_privilege('trigger_user', 'public', 'CREATE') still returns true.
  5. trigger_user can now execute CREATE TABLE public.orphan_table (...) successfully, even though every trigger it ever owned has been dropped. The role is orphaned-but-privileged.

@ben-fornefeld ben-fornefeld changed the base branch from main to remove/user-team-provision-triggers April 16, 2026 23:13
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