fix(cli): support v2 RBAC API keys for setVersionInChannel#2081
fix(cli): support v2 RBAC API keys for setVersionInChannel#2081WcaleNieWolny merged 1 commit intomainfrom
Conversation
The setChannel gate used `is_allowed_capgkey`, which evaluates `api_key.mode = ANY(keymode)`. v2 RBAC keys have `mode IS NULL` (since migration 20260505163356), so the comparison returns NULL and v2 keys are always denied at this gate, even when their role bindings grant write access. Switch to `hasCliPermission` with `app.create_channel` — the same permission `get_org_perm_for_apikey_v2` uses to detect write tier. The underlying `rbac_check_permission_direct` handles both the v2 RBAC path (bindings) and the v1 legacy fallback (mode-based keys), so read/upload/write/all keys keep their existing behavior. Side benefit: the gate is now app-scoped, so v1 keys with `limited_to_apps` excluding the target app get the friendly warning instead of a confusing RLS failure on `updateOrCreateChannel`.
📝 WalkthroughWalkthroughThe PR updates ChangesPermission Check Utility Migration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cli/src/bundle/upload.ts (1)
695-695:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReplace legacy “use the all key” wording with capability-based guidance
Line 695 still tells users to “use the
all” key, which is outdated after moving to RBAC capability checks and can mislead v2-key users.💡 Suggested message update
- uploadFail(`Cannot set channel, the upload key is not allowed to do that, use the "all" for this. ${formatError(dbError3)}`) + uploadFail(`Cannot set channel: missing permission to manage channels for this app. ${formatError(dbError3)}`)🤖 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 `@cli/src/bundle/upload.ts` at line 695, The error message passed to uploadFail still suggests using the legacy "all" key; update the message in the uploadFail(...) call so it refers to RBAC capabilities instead (e.g., instruct the user to ensure their API key has the required channel:set or equivalent capability) and include formatError(dbError3) as before; modify the string in the uploadFail invocation that contains formatError(dbError3) (located where uploadFail is called with formatError(dbError3)) to replace "use the \"all\" for this" with capability-based guidance mentioning the specific capability to enable.
🧹 Nitpick comments (1)
cli/src/bundle/upload.ts (1)
23-23: ⚡ Quick winUse
~/alias instead of relativesrcimport on this changed lineLine 23 still imports from
'../utils'. Please switch this changed import to the~/alias for consistency with repo rules.As per coding guidelines, "use path alias
~/to referencesrc/directory".🤖 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 `@cli/src/bundle/upload.ts` at line 23, The import on the changed line still uses a relative path ('../utils')—replace it with the project path alias (use '~/utils') so the file imports baseKeyV2, BROTLI_MIN_UPDATER_VERSION_V5, BROTLI_MIN_UPDATER_VERSION_V6, BROTLI_MIN_UPDATER_VERSION_V7, canPromptInteractively, checkChecksum, checkCompatibilityCloud, checkPlanValidUpload, checkRemoteCliMessages, createSupabaseClient, deletedFailedVersion, findRoot, findSavedKey, formatError, getAppId, getBundleVersion, getCompatibilityDetails, getConfig, getInstalledVersion, getLocalConfig, getLocalDependencies, getOrganizationId, getPMAndCommand, getRemoteFileConfig, hasCliPermission, hasOrganizationPerm, isCompatible, isDeprecatedPluginVersion, OrganizationPerm, regexSemver, resolveUserIdFromApiKey, sendEvent, updateConfigUpdater, updateOrCreateChannel, updateOrCreateVersion, UPLOAD_TIMEOUT, uploadTUS, uploadUrl, zipFile from '~/utils' instead of '../utils'.
🤖 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.
Outside diff comments:
In `@cli/src/bundle/upload.ts`:
- Line 695: The error message passed to uploadFail still suggests using the
legacy "all" key; update the message in the uploadFail(...) call so it refers to
RBAC capabilities instead (e.g., instruct the user to ensure their API key has
the required channel:set or equivalent capability) and include
formatError(dbError3) as before; modify the string in the uploadFail invocation
that contains formatError(dbError3) (located where uploadFail is called with
formatError(dbError3)) to replace "use the \"all\" for this" with
capability-based guidance mentioning the specific capability to enable.
---
Nitpick comments:
In `@cli/src/bundle/upload.ts`:
- Line 23: The import on the changed line still uses a relative path
('../utils')—replace it with the project path alias (use '~/utils') so the file
imports baseKeyV2, BROTLI_MIN_UPDATER_VERSION_V5, BROTLI_MIN_UPDATER_VERSION_V6,
BROTLI_MIN_UPDATER_VERSION_V7, canPromptInteractively, checkChecksum,
checkCompatibilityCloud, checkPlanValidUpload, checkRemoteCliMessages,
createSupabaseClient, deletedFailedVersion, findRoot, findSavedKey, formatError,
getAppId, getBundleVersion, getCompatibilityDetails, getConfig,
getInstalledVersion, getLocalConfig, getLocalDependencies, getOrganizationId,
getPMAndCommand, getRemoteFileConfig, hasCliPermission, hasOrganizationPerm,
isCompatible, isDeprecatedPluginVersion, OrganizationPerm, regexSemver,
resolveUserIdFromApiKey, sendEvent, updateConfigUpdater, updateOrCreateChannel,
updateOrCreateVersion, UPLOAD_TIMEOUT, uploadTUS, uploadUrl, zipFile from
'~/utils' instead of '../utils'.
|



Summary
is_allowed_capgkey({ apikey, keymode: ['write','all'] })gate insetVersionInChannelwithhasCliPermission(supabase, apikey, 'app.create_channel', { appId: appid }).mode IS NULLsince migration20260505163356_apikey_nullable_mode_with_bindings) were silently denied here becausemode = ANY(keymode)evaluates toNULLfor v2 keys.cli_check_permission(called viahasCliPermission) goes throughrbac_check_permission_direct, which natively handles both v1 (legacy mode fallback) and v2 (RBAC bindings).app.create_channelis the canonical "write tier" probe —get_org_perm_for_apikey_v2already uses it to detectperm_write.Behavior matrix
app_developer(or higher)app_uploader/app_readerallmodewritemodeuploadmodereadmodelimited_to_appsexcluding targetWhy this approach
The CLI already has
hasCliPermission(seecli/src/utils.ts:1630) and usescli_check_permissionfor every other RBAC-aware gate (org.create_app,app.build_native, …). This bringssetVersionInChannelin line with that pattern instead of being the only call site still using legacyis_allowed_capgkey.No SQL migration. Single CLI file changed (2 insertions, 4 deletions).
Test plan
app_developerrole on Updater Example →npx @capgo/cli@next bundle uploadsets the channelapp_uploaderrole → bundle uploads, warning shown (no regression)allmode key → channel still set (no regression)writemode key → channel still set (no regression)uploadmode key → warning still shown (no regression)limited_to_appsexcluding the target app → warning shown at gate instead of RLS failure on channel upsertRelated
is_allowed_capgkeyreturningfalsefor v2 keys withorg_member+app_developerbindings even though they should be allowed to set channel versions.Summary by CodeRabbit