fix: revoke public execute from API key helper RPCs#1965
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR restricts API-key RPC access by revoking Changes
Sequence DiagramsequenceDiagram
actor Client
participant RPC as Public RPC<br/>(get_user_id)
participant Helper as capgo_private Helper<br/>(SECURITY DEFINER)
participant DB as Database
participant Storage as Storage Bucket<br/>(apps)
Client->>RPC: call get_user_id() or<br/>get_org_perm_for_apikey()
Note over RPC: Only AUTHENTICATED<br/>or SERVICE_ROLE allowed
Client->>Storage: DELETE/UPDATE/INSERT/SELECT<br/>request + capgkey header
Storage->>Helper: invoke matches_app_storage_apikey_owner()<br/>with (folder_user_id, app_id, key_modes)
Helper->>DB: extract capgkey from<br/>request.headers
Helper->>DB: lookup API key record<br/>by value hash
alt Key exists & valid
Helper->>DB: check key expiration<br/>& mode compatibility
Helper->>DB: verify app/folder<br/>ownership constraints
Helper-->>Storage: return TRUE
Storage-->>Client: grant access
else Invalid/expired/insufficient perms
Helper-->>Storage: return FALSE
Storage-->>Client: deny access
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![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 `@tests/cli-sdk-utils.ts`:
- Around line 770-829: The code creates the DB record via createBundleRecord
before calling uploadBufferToStorage, so if uploadBufferToStorage fails the
app_versions row (versionId) is left pointing to a missing blob; modify the flow
to ensure cleanup: after calling createBundleRecord(store the returned
versionId) wrap uploadBufferToStorage in a try/catch (or add cleanup in the
outer catch) and on any upload error delete the created DB row (use the same
identifier versionId and existing DB client pattern used elsewhere, e.g.
getSupabaseClient().from('app_versions').delete().eq('id', versionId) or a
deleteBundleRecord helper) before returning the failure result; ensure this runs
only when versionId was set so you don’t delete unrelated rows.
- Around line 493-509: The encrypted branch returns a sessionKey but omits the
required key_id, causing DB inserts to fail for encrypted bundles; update the
code that builds the return object (where sessionKey / session_key and
encryptedSessionKey are set) to also supply key_id (e.g., derive from
options.encryptionKey or accept an explicit options.encryptionKeyId) and include
it in the returned payload as key_id so the inserted app_versions row matches
the encrypted-bundle shape expected by the DB. Ensure the symbol names match the
DB field (key_id) and use the same source of truth as the encryption key
(options.encryptionKey or a new options.encryptionKeyId) when constructing the
returned object.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 61a21421-7993-4b50-98aa-d8ecf6c35a55
📒 Files selected for processing (7)
supabase/migrations/20260427105909_fix_apikey_helper_rpc_public_execute.sqlsupabase/tests/07_auth_functions.sqlsupabase/tests/19_test_identity_functions.sqlsupabase/tests/49_test_apikey_oracle_rpc_permissions.sqltests/cli-channel.test.tstests/cli-hashed-apikey.test.tstests/cli-sdk-utils.ts
|
2 participants
Summary (AI generated)
PUBLICexecute access frompublic.get_user_id(...)andpublic.get_org_perm_for_apikey(...)authenticatedandservice_rolesoanonno longer inherits access implicitlyMotivation (AI generated)
The previous hardening migrations revoked these helper RPCs from
anon, but did not revoke them fromPUBLIC. In PostgreSQL, new functions are executable byPUBLICby default, andanoninherits that privilege, so the original mitigation did not actually remove anonymous access. This change closes the advisory by fixing the real ACL root cause and adding a regression test for it.Compatibility note: the current public CLI still calls these RPCs through the public Supabase client. This PR intentionally closes the server-side oracle in Capgo first; CLI rollout needs to be coordinated before deployment.
Business Impact (AI generated)
This removes an unauthenticated API-key validity and permission oracle from the public Supabase surface, which reduces the blast radius of leaked keys and prevents external user/org/app enumeration. It improves customer trust and lowers the risk of follow-on abuse using exposed metadata.
Test Plan (AI generated)
bun run lint:sql -- supabase/migrations/20260427105909_fix_apikey_helper_rpc_public_execute.sql supabase/tests/49_test_apikey_oracle_rpc_permissions.sqlbunx supabase test db supabase/tests/10_utility_functions.sql supabase/tests/14_test_apikey.sql supabase/tests/49_test_apikey_oracle_rpc_permissions.sql --local --workdir /Users/martindonadieu/.codex/worktrees/635c/capgo/.context/supabase-worktrees/3eee24d7Generated with AI
Summary by CodeRabbit
Security
Tests