fix(api): keep anon CLI app-list helper grants#1974
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughTemporarily grants the Changes
Sequence DiagramsequenceDiagram
participant Client as Anonymous Client
participant PostgREST as PostgREST
participant RLS as RLS Evaluator
participant Helpers as Helper Functions
participant DB as public.apps
Client->>PostgREST: GET /rest/v1/apps (capgkey header)
PostgREST->>RLS: Trigger row-level checks
RLS->>Helpers: get_apikey_header(capgkey)
Helpers-->>RLS: api_key
RLS->>Helpers: is_apikey_expired(api_key)
Helpers-->>RLS: expiry_status
RLS->>Helpers: get_identity_org_appid(api_key)
Helpers-->>RLS: identity/org/appid
RLS->>Helpers: check_min_rights(identity, appid)
Helpers-->>RLS: permission_ok
RLS->>DB: allow SELECT for matching rows
DB-->>PostgREST: rows
PostgREST-->>Client: 200 OK + apps
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 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)
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.1.0)supabase/tests/49_test_apikey_oracle_rpc_permissions.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/20260427175506_temporary_cli_apps_list_anon_helper_grants.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 |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
supabase/migrations/20260427175506_temporary_cli_apps_list_anon_helper_grants.sql (1)
1-8: Add a concrete sunset pointer for this temporary compatibility migration.The intent is clear, but include a tracking issue/PR reference (or target cleanup migration name) so these grants are reliably removed after CLI migration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/migrations/20260427175506_temporary_cli_apps_list_anon_helper_grants.sql` around lines 1 - 8, Add a concrete sunset pointer to this temporary compatibility migration by including a tracking reference (e.g., GitHub issue or PR URL) or the exact name of the follow-up cleanup migration that will remove these anonymous execute grants for the apps RLS helper functions; update the migration header/comments in 20260427175506_temporary_cli_apps_list_anon_helper_grants.sql to state that reference and an expected removal condition (e.g., "remove when PR `#1234` lands" or "cleanup migration: 20260601_remove_temp_cli_apps_anon_grants"), so reviewers can reliably find and remove these grants once the CLI migration is complete.supabase/tests/49_test_apikey_oracle_rpc_permissions.sql (1)
201-210: Add a negative-pathpublic.appscheck after setting an invalidcapgkey.You already validate invalid-key behavior for
storage.objects; mirroring that forpublic.appswould harden this regression test.Proposed test hardening diff
-SELECT plan(19); +SELECT plan(20); @@ DO $$ BEGIN PERFORM set_config('request.headers', '{"capgkey": "invalid-key"}', true); END $$; +SELECT + is( + ( + SELECT count(*) + FROM public.apps + WHERE app_id = 'com.demo.app' + ), + 0::bigint, + 'anon with invalid capgkey cannot read apps through helper identity' + ); + SELECT is( ( SELECT count(*) FROM storage.objects🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/tests/49_test_apikey_oracle_rpc_permissions.sql` around lines 201 - 210, Add a negative-path assertion that mirrors the invalid-key check for storage.objects: after you set an invalid capgkey, run the same SELECT count(*) FROM public.apps WHERE app_id = 'com.demo.app' and assert it does NOT return 1 (e.g. expect 0::bigint or a permission-denied outcome). Update the test near the existing is(..., 1::bigint, 'anon API-key apps query still works through RLS helper identity') block to include a new is(...) assertion for public.apps when capgkey is invalid (use the same message pattern like 'anon invalid API-key apps query blocked by RLS helper identity') so the test verifies both positive and negative behaviors for public.apps.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@supabase/migrations/20260427175506_temporary_cli_apps_list_anon_helper_grants.sql`:
- Around line 1-8: Add a concrete sunset pointer to this temporary compatibility
migration by including a tracking reference (e.g., GitHub issue or PR URL) or
the exact name of the follow-up cleanup migration that will remove these
anonymous execute grants for the apps RLS helper functions; update the migration
header/comments in 20260427175506_temporary_cli_apps_list_anon_helper_grants.sql
to state that reference and an expected removal condition (e.g., "remove when PR
`#1234` lands" or "cleanup migration: 20260601_remove_temp_cli_apps_anon_grants"),
so reviewers can reliably find and remove these grants once the CLI migration is
complete.
In `@supabase/tests/49_test_apikey_oracle_rpc_permissions.sql`:
- Around line 201-210: Add a negative-path assertion that mirrors the
invalid-key check for storage.objects: after you set an invalid capgkey, run the
same SELECT count(*) FROM public.apps WHERE app_id = 'com.demo.app' and assert
it does NOT return 1 (e.g. expect 0::bigint or a permission-denied outcome).
Update the test near the existing is(..., 1::bigint, 'anon API-key apps query
still works through RLS helper identity') block to include a new is(...)
assertion for public.apps when capgkey is invalid (use the same message pattern
like 'anon invalid API-key apps query blocked by RLS helper identity') so the
test verifies both positive and negative behaviors for public.apps.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 137bea97-58b2-40f2-a5d1-3e51b4200452
📒 Files selected for processing (2)
supabase/migrations/20260427175506_temporary_cli_apps_list_anon_helper_grants.sqlsupabase/tests/49_test_apikey_oracle_rpc_permissions.sql
|
1 participant
Summary (AI generated)
app listflowappsreadMotivation (AI generated)
The currently published CLI still authenticates through legacy anonymous PostgREST helper calls and then issues a direct
GET /rest/v1/appsrequest with thecapgkeyheader. When these helper grants drift or get tightened, CLI users regress from working API-key auth to opaqueApps not found/ auth failures.Business Impact (AI generated)
This restores short-term CLI reliability for existing customers without forcing an immediate CLI upgrade. It reduces support load around broken API-key workflows and buys time to ship the longer-term CLI-side migration to RBAC-aware wrappers.
Test Plan (AI generated)
bun run typecheckbun scripts/supabase-worktree.ts test dbbunx @capgo/cli app listsucceeds against a local stack after the legacy anon grants are presentGenerated with AI
Summary by CodeRabbit