Skip to content

fix(cli): validate app-scoped plans with app context - #2825

Merged
WcaleNieWolny merged 4 commits into
mainfrom
wolny/fix-app-scoped-plan-check
Aug 1, 2026
Merged

fix(cli): validate app-scoped plans with app context#2825
WcaleNieWolny merged 4 commits into
mainfrom
wolny/fix-app-scoped-plan-check

Conversation

@WcaleNieWolny

@WcaleNieWolny WcaleNieWolny commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Summary (AI generated)

  • Replace the legacy API-key plan check with the app-aware is_allowed_action_org_action RPC when an app ID is available.
  • Remove the unused raw API-key argument from the CLI plan-validation helpers and their callers.
  • Preserve the general plan-health gate by checking every metered action, and surface RPC errors separately from genuine plan-upgrade failures.
  • Retain compatibility with older servers by falling back to org-scoped validation only when PostgREST reports the app-aware overload as unavailable.
  • Add regression coverage for app-scoped checks, legacy fallback, and RPC failures.

Motivation (AI generated)

App-scoped API keys can have sufficient permissions for an app without having organization-level org.read. The legacy is_allowed_action(apikey, appid) path eventually evaluated the plan through an organization-only permission check, returned false, and caused commands such as channel set to incorrectly report You need to upgrade your plan for healthy paid organizations.

Business Impact (AI generated)

This restores channel and bundle operations for customers using least-privilege app-scoped API keys, prevents healthy paid accounts from being blocked by a misleading billing error, and keeps plan enforcement intact for all metered resources.

Test Plan (AI generated)

  • bun run cli:check
  • Run the new app-aware plan-validation regression tests, including legacy fallback and RPC error handling.
  • Verify git diff --check is clean.
  • Confirm all GitHub Actions checks pass.

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The CLI replaces API-key-based plan checks with organization and app-scoped validation. Bundle and channel callers use revised signatures. New tests verify RPC results and errors through the aggregate test script.

Changes

Plan validation

Layer / File(s) Summary
App-scoped validation core
cli/src/utils.ts
isAllowedPlanActions calls the organization/app-scoped RPC. checkPlanValid and checkPlanValidUpload no longer accept an API key.
Validation caller integration
cli/src/bundle/unlink.ts, cli/src/bundle/upload.ts, cli/src/channel/set.ts
Bundle and channel operations use the revised validation arguments.
Validation test coverage
cli/test/test-plan-validation.mjs, cli/package.json
The standalone test verifies RPC arguments, boolean results, and error propagation. The aggregate test script runs the new test command.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CLIOperation
  participant PlanValidation
  participant Supabase
  CLIOperation->>PlanValidation: validate organization/app action
  PlanValidation->>Supabase: call is_allowed_action_org_action
  Supabase-->>PlanValidation: return approval or error
  PlanValidation-->>CLIOperation: return boolean or validation error
Loading

Possibly related PRs

Suggested labels: codex

Suggested reviewers: riderx

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly describes the main change: app-scoped plan validation in the CLI.
Description check ✅ Passed The description includes the change summary and test plan, but it omits the template's Screenshots and Checklist sections.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands.

@codspeed-hq

codspeed-hq Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 43 untouched benchmarks
⏩ 2 skipped benchmarks1


Comparing wolny/fix-app-scoped-plan-check (4d2ca6f) with main (10c8a55)

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 6 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@WcaleNieWolny

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot added the codex label Aug 1, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 6 files

Confidence score: 4/5

  • In cli/src/utils.ts (checkPlanValid/isAllowedActionOrg), the org-scoped path still ignores RPC error and returns !!data, which can silently deny valid org-scoped legacy users and create inconsistent behavior versus the new app-aware helper — make the org branch check and surface RPC errors the same way.
  • In cli/src/utils.ts, the new helper duplicates the is_allowed_action_org_action call pattern already in checkPlanValidUpload (including the cast and error handling), which raises drift risk when one path is updated and the other is not — consolidate to a shared helper or parameterize actions to keep behavior aligned.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="cli/src/utils.ts">

<violation number="1" location="cli/src/utils.ts:855">
P3: This new helper duplicates the `is_allowed_action_org_action` call shape, `as never` cast, and `if (error) throw` handling already present in `checkPlanValidUpload` just below it. The two differ only in the `actions` list. A shared helper parameterized by the action list would keep the RPC contract in one place and make the different action sets (all metered vs storage-only) easier to compare.</violation>

<violation number="2" location="cli/src/utils.ts:856">
P2: The new app-aware helper surfaces RPC errors, but the org-scope branch it sits next to in `checkPlanValid` still swallows them. `isAllowedActionOrg` returns `!!data` without checking `error`, so an org-scoped/legacy key hitting a transient or RBAC error gets told to "upgrade your plan" — the same misleading billing error this change is meant to eliminate. Consider giving the org-scope path the same error propagation so both branches behave consistently, or document why org-scope is intentionally lenient.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread cli/src/utils.ts Outdated
Comment thread cli/src/utils.ts Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@cli/src/utils.ts`:
- Around line 850-858: Update isAllowedPlanActions to fall back to the existing
organization-level validation when the app-aware is_allowed_action_org_action
RPC is unavailable on the server, while preserving app-aware behavior when
supported. Detect only the missing-function/unsupported-RPC case for fallback,
and propagate all other RPC errors unchanged.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: cd8f121b-7154-4183-adb3-e1eec3cd244f

📥 Commits

Reviewing files that changed from the base of the PR and between deeae10 and 375e896.

📒 Files selected for processing (2)
  • cli/src/utils.ts
  • cli/test/test-plan-validation.mjs
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • Cap-go/capacitor-updater (manual)

Comment thread cli/src/utils.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 6 files

Confidence score: 2/5

  • In cli/src/channel/set.ts, the app-aware plan RPC now requires app.read, so channel-scoped keys with only channel_admin can no longer update channels; this is a concrete least-privilege regression that can block legitimate admin workflows — adjust the plan gate to accept channel-level permissions (or use a channel-scoped check) for this path.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="cli/src/channel/set.ts">

<violation number="1" location="cli/src/channel/set.ts:242">
P1: Channel-scoped API keys with `channel_admin` access can no longer update a channel: this app-aware plan RPC requires `app.read`, which that least-privilege role intentionally does not grant. The plan gate needs a permission context that accepts the already-authorized channel scope (or the RPC/helper must be adjusted) without broadening the key to app-wide read access.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread cli/src/channel/set.ts
@sonarqubecloud

sonarqubecloud Bot commented Aug 1, 2026

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 6 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@WcaleNieWolny
WcaleNieWolny merged commit 5b33f83 into main Aug 1, 2026
64 checks passed
@WcaleNieWolny
WcaleNieWolny deleted the wolny/fix-app-scoped-plan-check branch August 1, 2026 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant