[codex] fix CLI warnings for app-scoped API keys#2274
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughA new PostgreSQL migration introduces ChangesCLI warnings and app-scoped RBAC bridging
Sequence DiagramsequenceDiagram
participant Caller as CLI caller
participant get_org_warnings as get_organization_cli_warnings
participant cli_check_perm as cli_check_permission
participant get_apikey as get_apikey_header
participant db as Database
Caller->>get_org_warnings: call with org_id, cli_version
get_org_warnings->>cli_check_perm: check org-read permission
cli_check_perm-->>get_org_warnings: permission denied
get_org_warnings->>get_apikey: extract API key from headers
get_apikey-->>get_org_warnings: api_key_id
get_org_warnings->>db: load API key and limited_to_apps
db-->>get_org_warnings: api_key record
alt key is app-scoped
get_org_warnings->>db: select allowed app in org
db-->>get_org_warnings: app_id
get_org_warnings->>cli_check_perm: re-check permission with app_id
cli_check_perm-->>get_org_warnings: permission result
end
alt permission still denied
get_org_warnings-->>Caller: fatal org-access denial message
else permission granted
get_org_warnings->>db: check storage action satisfied
alt storage not satisfied, MAU/bandwidth satisfied
get_org_warnings-->>Caller: fatal storage-limit-exceeded message
else all limits satisfied
get_org_warnings-->>Caller: empty/success array
end
end
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/migrations/20260516151507_fix_cli_warnings_app_scoped_apikeys.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/tests/20_test_org_management_functions.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 |
Merging this PR will not alter performance
Comparing 
Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@supabase/migrations/20260516151507_fix_cli_warnings_app_scoped_apikeys.sql`:
- Around line 65-77: The user-facing message built in the jsonb_build_object
(look for the messages := array_append(... jsonb_build_object(...) ...) block)
contains a redundant/confusing phrase "In order to upload your plan, please
upgrade your plan here"; update that message string to say "In order to upload,
please upgrade your plan here" (or "To upload, please upgrade your plan here")
so it correctly refers to uploading rather than "uploading your plan" while
preserving the rest of the message and the surrounding jsonb_build_object/fatal
field.
🪄 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: 54c2b801-bfdf-4bd6-8be6-2bcd476b06e9
📒 Files selected for processing (2)
supabase/migrations/20260516151507_fix_cli_warnings_app_scoped_apikeys.sqlsupabase/tests/20_test_org_management_functions.sql
| IF ( | ||
| public.is_paying_and_good_plan_org_action(orgid, ARRAY['mau']::public.action_type[]) = true | ||
| AND public.is_paying_and_good_plan_org_action(orgid, ARRAY['bandwidth']::public.action_type[]) = true | ||
| AND public.is_paying_and_good_plan_org_action(orgid, ARRAY['storage']::public.action_type[]) = false | ||
| ) THEN | ||
| messages := array_append(messages, jsonb_build_object( | ||
| 'message', 'You have exceeded your storage limit.\nUpload will fail, but you can still download your data.\nMAU and bandwidth limits are not exceeded.\nIn order to upload your plan, please upgrade your plan here: https://console.capgo.app/settings/plans.', | ||
| 'fatal', true | ||
| )); | ||
| END IF; | ||
|
|
||
| RETURN messages; | ||
| END; |
There was a problem hiding this comment.
Grammatical error in user-facing message.
The message says "In order to upload your plan, please upgrade your plan" which is confusing. It should be "In order to upload, please upgrade your plan" since uploads are what will fail, not "uploading your plan."
📝 Proposed fix
- 'message', 'You have exceeded your storage limit.\nUpload will fail, but you can still download your data.\nMAU and bandwidth limits are not exceeded.\nIn order to upload your plan, please upgrade your plan here: https://console.capgo.app/settings/plans.',
+ 'message', 'You have exceeded your storage limit.\nUpload will fail, but you can still download your data.\nMAU and bandwidth limits are not exceeded.\nIn order to upload, please upgrade your plan here: https://console.capgo.app/settings/plans.',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@supabase/migrations/20260516151507_fix_cli_warnings_app_scoped_apikeys.sql`
around lines 65 - 77, The user-facing message built in the jsonb_build_object
(look for the messages := array_append(... jsonb_build_object(...) ...) block)
contains a redundant/confusing phrase "In order to upload your plan, please
upgrade your plan here"; update that message string to say "In order to upload,
please upgrade your plan here" (or "To upload, please upgrade your plan here")
so it correctly refers to uploading rather than "uploading your plan" while
preserving the rest of the message and the surrounding jsonb_build_object/fatal
field.
|
1 participant
Summary (AI generated)
get_organization_cli_warningsvalidate app-scoped API keys through an allowed app in the requested org.Motivation (AI generated)
CLI uploads were failing during the warning preflight with
API key does not have read access to this organizationwhen a customer used a key limited to both an org and a specific app. The warning RPC checked org read without app context, solimited_to_appsblocked valid upload keys before the upload permission path ran.Business Impact (AI generated)
This restores secure scoped API-key uploads for paid customers with multiple apps, avoiding the need to use unrestricted keys in CI/CD and reducing support load around CLI upload failures.
Test Plan (AI generated)
bunx sqlfluff lint --dialect postgres supabase/migrations/20260516151507_fix_cli_warnings_app_scoped_apikeys.sqlbun scripts/supabase-worktree.ts test db supabase/tests/00-supabase_test_helpers.sql supabase/tests/20_test_org_management_functions.sqlbun run cli:build && vue-tsc --noEmitGenerated with AI
Summary by CodeRabbit