Conversation
📝 WalkthroughWalkthroughAdds two SQL migrations that drop and recreate row-level security policies for Changes
Sequence DiagramsequenceDiagram
participant Client
participant RLS as RLS Policy Engine
participant Key as get_apikey_header()
participant Org as get_identity_org_allowed()
participant Rights as check_min_rights()
participant User as auth.uid() (User Session)
Client->>RLS: Request read/write on webhooks/deliveries (may include API key)
RLS->>Key: Is API key header present?
alt API key present
Key-->>RLS: API key info
RLS->>Org: validate org scope for key
Org-->>RLS: org allowed / not allowed
alt org allowed
RLS->>Rights: verify required rights (read/write/admin)
Rights-->>RLS: rights granted / denied
RLS-->>Client: allow or deny operation
else org not allowed
RLS-->>Client: deny
end
else No API key
Key-->>RLS: no API key
RLS->>User: evaluate user session (auth.uid())
User-->>RLS: user id / not authenticated
RLS->>Rights: verify rights for user (via check_min_rights)
Rights-->>RLS: allow or deny
RLS-->>Client: allow or deny operation
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 SQLFluff (4.0.4)supabase/migrations/20260308235013_webhook-org-scope-api-key-org-check.sqlUser Error: No dialect was specified. You must configure a dialect or specify one on the command line using --dialect after the command. Available dialects: supabase/migrations/20260308180224_webhook-api-key-org-scope-hardening.sqlUser Error: No dialect was specified. You must configure a dialect or specify one on the command line using --dialect after the command. Available dialects: Comment |
riderx
changed the title
riderx
force-pushed
the
riderx/fix-webhook-org-scope
branch
from
3c11624 to
a1be230
Compare
riderx
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@supabase/migrations/20260308091000_restrict_webhook_key_modes.sql`:
- Around line 138-153: The UPDATE policy "Allow admin to update
webhook_deliveries" on public.webhook_deliveries is missing a WITH CHECK clause
so new values (e.g., org_id) aren't validated; add a WITH CHECK that mirrors the
USING condition but evaluates the proposed row (use NEW.org_id or equivalent)
and calls public.check_min_rights(...) with
public.get_identity_org_allowed('{all,write,upload}'::public.key_mode[],
NEW.org_id) so updates are authorized against the target org; reference the
existing webhooks UPDATE policy for the exact predicate structure and use the
same functions public.check_min_rights and public.get_identity_org_allowed.
- Around line 1-11: The migration 20260308091000_restrict_webhook_key_modes.sql
references the function public.get_identity_org_allowed() which doesn't exist
until migration 20260309091500_enforce_api_key_app_scoped_org_access.sql,
causing the migration to fail; fix by ensuring the function exists before this
policy migration — either rename 20260308091000 to a timestamp after
20260309091500 so it runs later, or merge the two migrations into one file with
the CREATE FUNCTION public.get_identity_org_allowed(...) placed before the ALTER
POLICY/GRANT statements that reference it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 992b48c6-5fb1-44ad-9ea8-5c4b6c78ed88
📒 Files selected for processing (2)
supabase/migrations/20260308091000_restrict_webhook_key_modes.sqlsupabase/migrations/20260309091500_enforce_api_key_app_scoped_org_access.sql
supabase/migrations/20260308091000_restrict_webhook_key_modes.sql
Outdated
Show resolved
Hide resolved
supabase/migrations/20260308091000_restrict_webhook_key_modes.sql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@supabase/migrations/20260308180224_webhook-api-key-org-scope-hardening.sql`:
- Around line 23-24: The SELECT allowlist for API keys mistakenly includes the
"read" mode; update the calls to public.get_identity_org_allowed that pass
'{read,upload,write,all}'::public.key_mode[] (seen around the
webhook/webhook_deliveries allowlist) to remove "read" so they pass
'{upload,write,all}'::public.key_mode[] instead; apply the same change to the
other occurrence referenced (lines near the second call to
public.get_identity_org_allowed).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dba9de6b-5484-47ab-b7d8-79a06c9d080b
📒 Files selected for processing (1)
supabase/migrations/20260308180224_webhook-api-key-org-scope-hardening.sql
supabase/migrations/20260308180224_webhook-api-key-org-scope-hardening.sql
Outdated
Show resolved
Hide resolved
riderx
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@supabase/migrations/20260308180224_webhook-api-key-org-scope-hardening.sql`:
- Around line 9-17: This migration recreates the webhook SELECT policy but
forgot to remove the earlier overlapping policy "Allow org members to select
webhooks", causing a follow-up cleanup migration; add a DROP POLICY IF EXISTS
"Allow org members to select webhooks" ON public.webhooks; at the top of this
migration alongside the other DROP POLICY lines, then ensure you have only one
SELECT policy for public.webhooks by merging any org-member condition into the
single CREATE POLICY "Allow admin to select webhooks" (use OR in the policy
USING expression if both admin and org-member checks are needed); finally delete
the follow-up migration file named
20260308235013_webhook-org-scope-api-key-org-check.sql so the cleanup is folded
into this migration.
- Around line 14-17: Several webhook RLS CREATE POLICY statements (e.g., "Allow
admin to select webhooks" and the other webhook admin policies) currently grant
TO authenticated, anon; remove the anon role from each of these policies so they
read TO authenticated only. Locate the nine CREATE POLICY statements that target
the public.webhooks table (all webhook admin policies across the two migration
files) and update their TO clauses from "TO authenticated, anon" to "TO
authenticated", keeping the rest of each policy unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d5c0b099-b22c-4f50-aecf-98ab680b2aec
📒 Files selected for processing (2)
supabase/migrations/20260308180224_webhook-api-key-org-scope-hardening.sqlsupabase/migrations/20260308235013_webhook-org-scope-api-key-org-check.sql
supabase/migrations/20260308180224_webhook-api-key-org-scope-hardening.sql
Outdated
Show resolved
Hide resolved
supabase/migrations/20260308180224_webhook-api-key-org-scope-hardening.sql
Outdated
Show resolved
Hide resolved
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f84ba4c73a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
supabase/migrations/20260308180224_webhook-api-key-org-scope-hardening.sql
Outdated
Show resolved
Hide resolved
supabase/migrations/20260308180224_webhook-api-key-org-scope-hardening.sql
Outdated
Show resolved
Hide resolved
|
PR #1753: I checked the webhook policy review comment. The overlapping-policy cleanup was valid on the original branch, but the 'remove anon' part is not safe because these policies rely on anon for API-key-authenticated RLS access. The logic here has also been superseded by later webhook migrations, so I’m not applying a patch on this PR. |
|
1 participant
Summary (AI generated)
20260308235013_webhook-org-scope-api-key-org-check.sqlto enforce API-key-first authorization on webhook and webhook_delivery RLS paths, including org-scoped checks and safe update validation.tests/hashed-apikey-rls.test.tsfor webhook read precedence and unauthorizedwebhook_deliveries.org_idupdates when both user auth and an out-of-scope key are present.Motivation (AI generated)
capgkeyheader is supplied.Business Impact (AI generated)
Test Plan (AI generated)
bun lint:backendbun test tests/hashed-apikey-rls.test.ts(targeted regression)Summary by CodeRabbit