Skip to content

Carry the production function revokes in the Supabase migrations #397

Description

@HMarzban

Summary

Production has the correct EXECUTE revokes on sensitive SECURITY DEFINER functions. The repo migrations do not. A fresh deploy from packages/supabase/migrations/ (a self-hoster, a new staging project, or a production rebuild) would expose these functions to anon or to any signed-in user.

This issue merges three review items: get_inactive_users, the push queue functions, and admin_get_document_member_counts.

  • Severity: Medium. Production is safe today (see below). Any new deploy from the repo is not.
  • Area: Supabase migrations
  • Source: security review of 2026-10-06, findings C1, H3, M10, L6, L7

Production check (2026-10-06, read-only)

has_function_privilege on production returns false for both anon and authenticated on every function below. So production is not exploitable today.

The gap in the repo

The broad revoke lives only in packages/supabase/scripts/29-lint-hardening.sql §5–§8. That file runs on a local seed only; packages/supabase/CLAUDE.md says "no migration carries it". Hosted Supabase grants EXECUTE on new public functions to anon and authenticated by default. So a migration that revokes only from public, anon leaves authenticated open.

Function Where it is defined What the repo revokes Risk on a fresh deploy
purge_document_footprint migrations/20260715082400_fix_purge_document_views_keying.sql:9-55 public, anon only Any signed-in user deletes any document's chat, chat media and view stats
consume_push_queue, ack_push_message migrations/20260519200000_scripts_functions_triggers_parity.sql:577-624 nothing Anyone reads pending push payloads, or acks them so no push is sent
get_inactive_users scripts/28-ghost-accounts-audit.sql:18-50 only (no migration) nothing Anyone reads the email of every never-active user
get_user_deletion_impact, get_ghost_summary_public scripts/28-ghost-accounts-audit.sql:58,88 only nothing Admin data open to anyone
admin_get_document_member_counts migrations/20260519200000_...sql:4478-4500 anon only, not public; body skips its check when auth.uid() is null Anon reads member counts
create_direct_message_channel migrations/20260519200000_...sql:5504-5603 none for authenticated Any user adds any user to a DM (notification spam)
enqueue_document_view, update_view_duration migrations/20260610135024_revoke_view_tracking_rpcs_from_browser.sql anon, authenticated but not public Anon inflates view counts
process_document_views_queue migrations/20260726181000_*.sql:32 nothing Anon runs a cron job
get_*_media_storage_* (3) migrations/20260623120000_chat_media_attachments.sql:1108,1150,1184 public only Anon reads per-document storage stats

Script 28 is not in any migration, but the production admin server calls two of its functions (apps/hocuspocus.server/src/api/services/adminGhostAccounts.service.ts:182,253). So it was applied to production by hand.

Fix plan

  1. Add one migration that, for each function above, runs revoke all on function ... from public, anon, authenticated; and grant execute on function ... to service_role;.
  2. Move the three script-28 functions into that migration with create or replace, so production and the repo match.
  3. Port the programmatic sweep from scripts/29-lint-hardening.sql §5–§8 into a migration. The same rule must then apply to every future DEFINER function, not only this list.
  4. Add a CI check after db reset: query every prosecdef function in public and fail when anon or authenticated holds EXECUTE on a function that is not in the user_facing_names allowlist.
  5. Mirror the change in packages/supabase/scripts/. Then run bun run --filter @docs.plus/supabase_back types.

Before you start, check every caller. Each function above is called only with the service role (hocuspocus worker or admin service). Confirm with grep -rn "<function name>" apps/.

Acceptance criteria

  • On a fresh db reset from migrations only (skip 29-lint-hardening.sql), every function in the table returns false for anon and authenticated in has_function_privilege.
  • Production still returns false for all of them after the migration.
  • The push worker, the email worker, the admin ghost-account pages and the document purge still work.
  • The new CI check fails when a test DEFINER function is added without a revoke.

Verify

select p.oid::regprocedure,
       has_function_privilege('anon', p.oid, 'EXECUTE') as anon,
       has_function_privilege('authenticated', p.oid, 'EXECUTE') as authed
from pg_proc p
where p.pronamespace = 'public'::regnamespace and p.prosecdef
  and p.prorettype <> 'trigger'::regtype
order by 2 desc, 3 desc, 1;

Every row with true must be a browser RPC that checks auth.uid() in its body.


Generated by Claude Code

Activity

  1. added theissue type on Oct 6, 2026
  2. HMarzban commented on Oct 6, 2026

    @HMarzban
    CollaboratorAuthor

    Fixed in 335138d on claude/youthful-lovelace-3nvsc6: explicit revokes from public, anon, authenticated for every listed function, and the three script-28 functions moved into migration 20261006130000_close_client_write_and_grant_gaps.sql. Their return tables were checked against production and match exactly, so create or replace is safe there.

    Deliberately not done:

    • Port the §5–§8 sweep. A blanket revoke on production would hit every DEFINER function outside a hard-coded allowlist. packages/supabase/CLAUDE.md records that this sweep already broke get_document_member_previews and get_document_members locally. The explicit list is safer.
    • CI grant check. CI runs no Supabase stack, so it needs new infrastructure. Run the Verify SQL above by hand after deploy instead.

    Also removed create_direct_message_channel from user_facing_names in 29-lint-hardening.sql, so a local reset matches production. Its webapp wrapper apps/webapp/src/api/rpc/createDirectMessageChannel.ts is now unused; that is a separate cleanup.


    Generated by Claude Code

  3. changed the title [-][Security] Migrations do not carry the function revokes that production has[/-] [+]Carry the production function revokes in the Supabase migrations[/+] on Oct 6, 2026
  4. added
    SecuritySecurity, access control, and data exposure
    and removed
    bugSomething isn't working
    on Oct 6, 2026
  5. HMarzban commented on Oct 9, 2026

    @HMarzban
    CollaboratorAuthor

    Reopened: the board automation closed this issue before the push. The code is on main through merge 083f37d84 (migration 20261006130000_close_client_write_and_grant_gaps.sql). It stays open until that migration is applied on prod. Fix-plan steps 3 and 4 were cut by the implementing agent, and that cut still needs a maintainer ruling.

  6. HMarzban commented on Oct 9, 2026

    @HMarzban
    CollaboratorAuthor

    Decision, 2026-10-09 (HoE, delegated by the maintainer for today's delivery)

    The scope cut is accepted. Fix-plan steps 3 (port the 29-lint-hardening.sql §5–§8 sweep into a migration) and 4 (a CI grant check) are not done. Reasons:

    • A blanket revoke sweep on prod is risky. It can remove a grant that a live client path needs, and nothing in CI would catch it.
    • CI runs no Supabase stack, so a grant check there has nothing to check against.

    What shipped instead:

    • Migration 20261006130000 revokes the explicit list of 14 functions. It was applied to prod on 2026-10-09.
    • packages/supabase/CLAUDE.md now has a rule: every new service-role SECURITY DEFINER function revokes its own EXECUTE in its own migration.

    The remaining gaps are tracked in #469 (the internal schema helpers) and #470 (anon default table privileges).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    DevOpsSecuritySecurity, access control, and data exposure

    Type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions