Repository navigation
Add cmux.com team-vault APIs for local Subrouter egress - #9099
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change centralizes subrouter authentication and request context handling, adds account repair, credential lease, logout, and team endpoints, strengthens account and lease validation/parsing, updates dashboard permissions and CLI vault wrappers, and expands related route and authentication tests. ChangesSubrouter API and authorization
CLI authentication route wrappers
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant LeaseRoute
participant RequestContext
participant TenantDatabase
participant SubrouterClient
CLI->>LeaseRoute: POST lease request
LeaseRoute->>RequestContext: Resolve authenticated team context
RequestContext->>TenantDatabase: Find shared tenant
LeaseRoute->>SubrouterClient: Create credential lease
SubrouterClient-->>LeaseRoute: Return lease
LeaseRoute-->>CLI: Return teamId and lease JSON
🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2aa57dcdc5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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 `@web/app/api/subrouter/accounts/`[accountId]/repair/route.ts:
- Around line 19-23: Extract the duplicated trim-and-200-character validation
into a shared normalizeAccountId helper, then update the repair route and the
accounts route to use it. Preserve the existing invalid_request 400 response
when normalization returns null, and use the helper’s normalized value for valid
requests.
In `@web/app/api/subrouter/logout/route.ts`:
- Around line 18-23: Wrap the Stack Auth calls in the logout route—specifically
getStackServerApp().getUser and user.signOut—in try/catch handling, and return
the established subrouterErrorResponse for failures. Preserve the unauthorized
response when no user is found and the existing successful logout flow.
- Around line 10-20: Replace the inline authorization and refresh-token parsing
in the logout route with the shared token-parsing logic used by verifyRequest in
auth.ts, extracting or reusing a helper such as parseNativeStackTokens(request).
Preserve the existing unauthorized response for missing or invalid tokens and
pass the parsed accessToken and refreshToken to getStackServerApp().getUser.
In `@web/app/api/subrouter/teams/route.ts`:
- Around line 44-51: In the response-building logic, extract user.selectedTeamId
?? user.billingTeamId into a local constant before the teams.some check, then
reuse that constant for both the membership comparison and selectedTeamId value
while preserving the existing null fallback behavior.
In `@web/services/subrouter/boundedJson.ts`:
- Around line 21-35: Update the reader-processing try/catch in bounded JSON
parsing so the reader lock is always released via a finally block, including
read errors and overflow returns. Preserve the existing cancellation and
response statuses while ensuring cleanup runs on every exit path.
In `@web/services/subrouter/client.ts`:
- Around line 418-424: Update parseAccountHealth to treat null the same as
undefined by returning undefined before the record validation. Preserve the
existing validation and SubrouterClientError behavior for non-null values that
are not valid health records, allowing parseAccountList to continue when an
account’s health is explicitly absent.
In `@web/services/subrouter/routeHelpers.ts`:
- Around line 62-71: Update subrouterTeamAllowed so an unset or empty
SUBROUTER_ALLOWED_TEAM_IDS value denies all teams by default. Preserve matching
for explicitly listed team IDs, and support an explicit wildcard such as "*" to
intentionally allow all teams; do not retain the current implicit allow-all
behavior.
In `@web/services/vms/auth.ts`:
- Around line 196-198: Update the catch block in the permission-checking flow of
auth.ts to capture and log the caught error server-side before returning { use:
false, manageAccounts: false }. Preserve the existing fail-closed return
behavior and avoid exposing error details to clients.
- Around line 140-148: Update the permission configuration around
enforceSubrouterPermissions and subrouterPermissions so enforcement is enabled
by default, with only an explicit opt-out disabling it; ensure disabled or
invalid configuration fails closed rather than granting permissions. In the
enforced branch, run the two hasPermission checks concurrently with Promise.all
while preserving their existing permission results.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5b16c1cc-5016-4b68-a1be-a202dfbccce4
📒 Files selected for processing (16)
web/app/[locale]/dashboard/components/ai-account-forms.tsxweb/app/api/subrouter/accounts/[accountId]/repair/route.tsweb/app/api/subrouter/accounts/[accountId]/route.tsweb/app/api/subrouter/accounts/route.tsweb/app/api/subrouter/leases/[leaseId]/events/route.tsweb/app/api/subrouter/leases/route.tsweb/app/api/subrouter/logout/route.tsweb/app/api/subrouter/teams/route.tsweb/services/subrouter/accountInput.tsweb/services/subrouter/boundedJson.tsweb/services/subrouter/client.tsweb/services/subrouter/requestContext.tsweb/services/subrouter/routeHelpers.tsweb/services/vms/auth.tsweb/tests/subrouter-accounts-route.test.tsweb/tests/vm-route-auth.test.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@web/app/`[locale]/dashboard/subrouter/page.tsx:
- Around line 84-85: Update the teams filter in the dashboard page to retain
candidates where either use or manageAccounts is true, preserving accounts-only
managers for the permission-gated controls. Add coverage for a candidate with
manageAccounts true and use false.
In `@web/app/api/subrouter/teams/route.ts`:
- Around line 20-36: The teams response currently maps authorized teams and then
performs a second scan with teams.some. Replace this with a single iteration in
the route handler that builds teams while recording whether the preferred
selectedTeamId or billingTeamId was emitted, then return that recorded ID or
null without rescanning the collection.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b9e09133-3189-4b26-85d5-958196f7b2c1
📒 Files selected for processing (4)
web/app/[locale]/dashboard/subrouter/page.tsxweb/app/api/subrouter/logout/route.tsweb/app/api/subrouter/teams/route.tsweb/app/lib/stack.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/services/vms/auth.ts (1)
144-145: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick winPermission enforcement remains fail-open by default (unresolved from prior review).
enforceSubrouterPermissionsis still onlytruewhenSUBROUTER_ENFORCE_STACK_PERMISSIONS === "1", andsubrouterPermissions(Line 197) returns{ use: true, manageAccounts: true }whenever enforcement is off. Any deployment that omits this env var still grants every authenticated team membersubrouter:useandsubrouter:manage_accounts, which gate credential upload, repair, deletion, and lease issuance downstream.Invert the default so enforcement is the standard behavior and the bypass is an explicit opt-out.
🔒 Proposed fix: enforce by default
- const enforceSubrouterPermissions = - process.env.SUBROUTER_ENFORCE_STACK_PERMISSIONS === "1"; + // Opt-out only; an unset env var must not silently grant every permission. + const enforceSubrouterPermissions = + process.env.SUBROUTER_ENFORCE_STACK_PERMISSIONS !== "0";As per coding guidelines, "Do not add an unreliable fallback, guess, default, or 'best effort' branch when an incorrect value would be a correctness bug; fail closed instead."
[source_coding_guidelines,source_other]🤖 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 `@web/services/vms/auth.ts` around lines 144 - 145, Update the enforceSubrouterPermissions initialization in auth.ts so subrouter permission enforcement is enabled by default; only an explicit opt-out environment value should disable it. Preserve the existing enforcement and subrouterPermissions behavior once the flag is resolved, ensuring omitted or unrecognized values remain fail-closed.
♻️ Duplicate comments (2)
web/services/vms/auth.ts (1)
210-212: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winSilent catch still hides permission-service failures.
Failing closed is correct, but with no logging an outage in Stack's permission API is indistinguishable from a legitimate denial. This was flagged in a prior review and remains unaddressed. Log the error server-side before returning the closed result.
🤖 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 `@web/services/vms/auth.ts` around lines 210 - 212, Update the catch block in the permission-checking flow to capture and server-side log the caught permission-service error before returning { use: false, manageAccounts: false }. Preserve the existing fail-closed return behavior while ensuring the log includes the actual error details.web/services/subrouter/routeHelpers.ts (1)
167-176: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAllowlist still defaults to allow-all (unresolved from prior review).
Per the described behavior,
subrouterTeamAllowedstill allows all teams whenSUBROUTER_ALLOWED_TEAM_IDSis unset/empty. Combined withenforceSubrouterPermissionsalso defaulting off (web/services/vms/auth.tsLines 144-145), an unconfigured deployment has no effective authorization on credential upload, deletion, repair, or lease issuance.Treat an unset/empty list as "no teams allowed" and require an explicit wildcard (e.g.
*) to intentionally open access.🤖 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 `@web/services/subrouter/routeHelpers.ts` around lines 167 - 176, Update subrouterTeamAllowed so an unset or empty SUBROUTER_ALLOWED_TEAM_IDS value denies every team instead of allowing all; only return true for all teams when the parsed allowlist explicitly contains the wildcard entry “*”, while preserving explicit team-ID matching.
🤖 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 `@web/tests/subrouter-accounts-route.test.ts`:
- Around line 775-815: Replace the real setTimeout delay in the “resolves one
permission snapshot per scope with bounded concurrency” test with manually
controlled promise resolvers: collect each listPermissions call’s resolver, wait
until enough calls are in flight to prove overlap, then release them so all
requests complete. Keep the existing concurrency assertions and permission-call
behavior without relying on wall-clock timing.
---
Outside diff comments:
In `@web/services/vms/auth.ts`:
- Around line 144-145: Update the enforceSubrouterPermissions initialization in
auth.ts so subrouter permission enforcement is enabled by default; only an
explicit opt-out environment value should disable it. Preserve the existing
enforcement and subrouterPermissions behavior once the flag is resolved,
ensuring omitted or unrecognized values remain fail-closed.
---
Duplicate comments:
In `@web/services/subrouter/routeHelpers.ts`:
- Around line 167-176: Update subrouterTeamAllowed so an unset or empty
SUBROUTER_ALLOWED_TEAM_IDS value denies every team instead of allowing all; only
return true for all teams when the parsed allowlist explicitly contains the
wildcard entry “*”, while preserving explicit team-ID matching.
In `@web/services/vms/auth.ts`:
- Around line 210-212: Update the catch block in the permission-checking flow to
capture and server-side log the caught permission-service error before returning
{ use: false, manageAccounts: false }. Preserve the existing fail-closed return
behavior while ensuring the log includes the actual error details.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f7ed2f3a-ef75-42fc-b6d9-bdd77fc844fe
📒 Files selected for processing (5)
web/app/api/subrouter/leases/route.tsweb/services/subrouter/client.tsweb/services/subrouter/routeHelpers.tsweb/services/vms/auth.tsweb/tests/subrouter-accounts-route.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cac38df8e6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
3 tasks done
This was referenced Jul 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the authenticated cmux.com control plane for shared Subrouter credentials. Every macOS or Linux client runs a loopback proxy and sends provider traffic from that machine. cmux.com selects the team account and brokers an access-only five-minute lease. Provider refresh tokens remain in the central Worker.
sr loginand team membership.subrouter:useleases credentials;subrouter:manage_accountsuploads, repairs, and deletes them.SUBROUTER_ALLOWED_TEAM_IDS.Depends on manaflow-ai/subrouter#99.
Deployment and canary
dpl_D3q6spAKA8vnL3Wsr3JkdR4ThceDsubrouter.cmux.dev, version467267f5-fb1c-4d25-bacc-730a566a42ffsubrouter-staging.cmux.dev, versionde30e48e-aa7d-4bb1-8704-a27dcd9d2e71sr loginselected the Stack team,sr doctorpassed, andsr codex execreturned exactlyOKthrough the local daemon. The central account remainedauth_valid: truewith no refresh failure.Verification
bun run typecheckbun test tests/vault-route-helpers.test.ts tests/subrouter-accounts-route.test.ts tests/vm-route-auth.test.ts(82 pass)Summary by CodeRabbit