Repository navigation
Fix CodeRouter trusted tenant exchange - #9607
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 CLI config now advertises ChangesHosted exchange flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant RequestContext
participant TenantExchangePOST
participant HostedStackEndpoint
RequestContext->>TenantExchangePOST: resolve context and authorize use-or-manage
TenantExchangePOST->>HostedStackEndpoint: POST tenant capabilities and identity
HostedStackEndpoint-->>TenantExchangePOST: return status and response body
TenantExchangePOST-->>RequestContext: return JSON response or propagated error
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (23 passed)
✨ Finishing Touches📝 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 |
![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 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/exchange/route.ts`:
- Around line 15-47: Add a behavior-level regression test for the exchange route
before changing implementation, with the initial commit containing only the
failing test; then place the route fix in a second commit. The test should
verify the authorization and control headers, capability derivation from
resolved team permissions, no-store fetch behavior, and mapping of upstream
failures. Anchor the test around the route handler and its existing upstream
fetch path.
- Around line 34-48: Update the upstream request in the exchange route around
fetch and response-body reading to use SUBROUTER_STACK_AUTH_TIMEOUT_MS with a
cancellation-aware AbortSignal covering both operations. Detect an abort caused
by this timeout and return the existing safe exchange error, while preserving
current handling for non-timeout failures.
🪄 Autofix
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: cdc19a1a-2dd2-4e18-8c69-8a600017e442
📒 Files selected for processing (1)
web/app/api/subrouter/exchange/route.ts
| // Keep the trusted exchange request in this route. Older cached server | ||
| // bundles delegated to a legacy client which omitted both the control | ||
| // credential and scoped capabilities. | ||
| const controlToken = | ||
| process.env.SUBROUTER_STACK_TENANT_DELETE_TOKEN?.trim(); | ||
| const hostedUrl = process.env.SUBROUTER_HOSTED_URL?.trim().replace( | ||
| /\/+$/, | ||
| "", | ||
| ); | ||
| if (!controlToken || !hostedUrl) { | ||
| return Response.json( | ||
| { error: "service_unavailable" }, | ||
| { status: 503 }, | ||
| ); | ||
| } | ||
| const capabilities = [ | ||
| ...(resolved.value.team.manageAccounts ? ["manage_accounts"] : []), | ||
| ...(resolved.value.team.use ? ["use"] : []), | ||
| ]; | ||
| const upstream = await fetch(`${hostedUrl}/_subrouter/auth/stack`, { | ||
| method: "POST", | ||
| headers: { | ||
| authorization: `Bearer ${resolved.value.accessToken}`, | ||
| "content-type": "application/json", | ||
| "x-subrouter-stack-control-token": controlToken, | ||
| }, | ||
| body: JSON.stringify({ | ||
| capabilities, | ||
| teamId: resolved.value.team.teamId, | ||
| teamName: resolved.value.team.teamName, | ||
| }), | ||
| cache: "no-store", | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the hosted-exchange regression test before merging.
The supplied PR contains no behavior-level regression test for the production login failure. Add a first commit that fails when the route omits the control header or scoped capabilities. Add the route fix in the second commit. Cover the authorization header, control header, capability derivation, no-store behavior, and upstream failure mapping.
As per coding guidelines, “Regression fixes should use two commits: the first adds only the failing test, and the second adds the fix.”
🤖 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/app/api/subrouter/exchange/route.ts` around lines 15 - 47, Add a
behavior-level regression test for the exchange route before changing
implementation, with the initial commit containing only the failing test; then
place the route fix in a second commit. The test should verify the authorization
and control headers, capability derivation from resolved team permissions,
no-store fetch behavior, and mapping of upstream failures. Anchor the test
around the route handler and its existing upstream fetch path.
Source: Coding guidelines
| const upstream = await fetch(`${hostedUrl}/_subrouter/auth/stack`, { | ||
| method: "POST", | ||
| headers: { | ||
| authorization: `Bearer ${resolved.value.accessToken}`, | ||
| "content-type": "application/json", | ||
| "x-subrouter-stack-control-token": controlToken, | ||
| }, | ||
| body: JSON.stringify({ | ||
| capabilities, | ||
| teamId: resolved.value.team.teamId, | ||
| teamName: resolved.value.team.teamName, | ||
| }), | ||
| cache: "no-store", | ||
| }); | ||
| const body = await upstream.text(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## locate target"
fd -a 'route.ts$' . | sed 's#^\./##' | rg 'web/app/api/subrouter/exchange/route.ts|route\.ts$' | sort
echo "## target file excerpt"
cat -n web/app/api/subrouter/exchange/route.ts | sed -n '1,120p'
echo "## timeout/abort patterns in web route API"
rg -n "fetch\\(|AbortSignal|timeout|AbortController|signal|\\{[\\s\\S]*fetch\\(" web/app/api -g 'route.ts' -g '*.ts' | head -200
echo "## candidate timeout helpers"
rg -n "TIMEOUT|TIME|timeout|AbortSignal|AbortController|setTimeout\\(" web app packages lib server 2>/dev/null | head -300 || trueRepository: manaflow-ai/cmux
Length of output: 38212
Apply the existing subrouter auth timeout to this upstream request.
web/app/api/subrouter/exchange/route.ts:34 calls fetch(...) without a cancellation-aware deadline, and web/app/api/subrouter/exchange/route.ts:48 can also wait indefinitely while reading the hosted response body. Use SUBROUTER_STACK_AUTH_TIMEOUT_MS with a bounded AbortSignal and return the safe exchange error when it is aborted.
🤖 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/app/api/subrouter/exchange/route.ts` around lines 34 - 48, Update the
upstream request in the exchange route around fetch and response-body reading to
use SUBROUTER_STACK_AUTH_TIMEOUT_MS with a cancellation-aware AbortSignal
covering both operations. Detect an abort caused by this timeout and return the
existing safe exchange error, while preserving current handling for non-timeout
failures.
Source: Coding guidelines
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/exchange-v2/route.ts`:
- Around line 17-25: Update the hostedUrl initialization in the exchange route
to fall back to defaultHostedSubrouterURL() when SUBROUTER_HOSTED_URL is unset,
then apply the same trimming and trailing-slash normalization used by the CLI
configuration route. Keep the existing controlToken/hostedUrl availability check
and 503 response unchanged.
- Around line 7-59: Add behavior-level regression coverage for the CLI login
flow before the implementation change, using the relevant v2 exchange route and
CLI login symbols. The test must verify that the CLI receives the v2 URL and
that the exchange request sends the scoped capabilities plus the control-token
header; place only this failing test in the first commit, with the
implementation fix committed separately afterward.
- Around line 31-45: The hosted Stack exchange request in the route handler must
use a cancellation-aware deadline instead of allowing fetch and upstream.text()
to run indefinitely. Update the flow around resolveSubrouterRequestContext and
the outbound _subrouter/auth/stack fetch to pass the request signal or an
equivalent bounded signal through the fetch and response-reading path, and add
coverage verifying an aborted hosted request is cancelled.
🪄 Autofix
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: 1aab6048-aa0a-4a0b-8242-494e08242fe9
📒 Files selected for processing (2)
web/app/api/cli/config/route.tsweb/app/api/subrouter/exchange-v2/route.ts
| export async function POST(request: Request): Promise<Response> { | ||
| const resolved = await resolveSubrouterRequestContext(request, { | ||
| permission: "use-or-manage", | ||
| allowCookie: false, | ||
| }); | ||
| if (!resolved.ok) return resolved.response; | ||
|
|
||
| try { | ||
| const controlToken = | ||
| process.env.SUBROUTER_STACK_TENANT_DELETE_TOKEN?.trim(); | ||
| const hostedUrl = process.env.SUBROUTER_HOSTED_URL?.trim().replace( | ||
| /\/+$/, | ||
| "", | ||
| ); | ||
| if (!controlToken || !hostedUrl) { | ||
| return Response.json( | ||
| { error: "service_unavailable" }, | ||
| { status: 503 }, | ||
| ); | ||
| } | ||
| const capabilities = [ | ||
| ...(resolved.value.team.manageAccounts ? ["manage_accounts"] : []), | ||
| ...(resolved.value.team.use ? ["use"] : []), | ||
| ]; | ||
| const upstream = await fetch(`${hostedUrl}/_subrouter/auth/stack`, { | ||
| method: "POST", | ||
| headers: { | ||
| authorization: `Bearer ${resolved.value.accessToken}`, | ||
| "content-type": "application/json", | ||
| "x-subrouter-stack-control-token": controlToken, | ||
| }, | ||
| body: JSON.stringify({ | ||
| capabilities, | ||
| teamId: resolved.value.team.teamId, | ||
| teamName: resolved.value.team.teamName, | ||
| }), | ||
| cache: "no-store", | ||
| }); | ||
| const body = await upstream.text(); | ||
| if (!upstream.ok) { | ||
| return new Response(body, { | ||
| status: upstream.status, | ||
| headers: { "content-type": "text/plain; charset=utf-8" }, | ||
| }); | ||
| } | ||
| const tenant: unknown = JSON.parse(body); | ||
| return Response.json(tenant, { | ||
| headers: { "cache-control": "no-store" }, | ||
| }); | ||
| } catch (error) { | ||
| return subrouterErrorResponse(error); | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add behavior-level regression coverage before the implementation commit.
This PR fixes a CLI login regression. Add a failing test that verifies the CLI receives the v2 URL and that the v2 exchange request includes the scoped capabilities and control header. Put that test in a test-only commit before the implementation commit. As per coding guidelines, “Regression fixes should use two commits: the first adds only the failing test, and the second adds the fix.”
🤖 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/app/api/subrouter/exchange-v2/route.ts` around lines 7 - 59, Add
behavior-level regression coverage for the CLI login flow before the
implementation change, using the relevant v2 exchange route and CLI login
symbols. The test must verify that the CLI receives the v2 URL and that the
exchange request sends the scoped capabilities plus the control-token header;
place only this failing test in the first commit, with the implementation fix
committed separately afterward.
Source: Coding guidelines
| const hostedUrl = process.env.SUBROUTER_HOSTED_URL?.trim().replace( | ||
| /\/+$/, | ||
| "", | ||
| ); | ||
| if (!controlToken || !hostedUrl) { | ||
| return Response.json( | ||
| { error: "service_unavailable" }, | ||
| { status: 503 }, | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use the same hosted URL fallback as the CLI configuration route.
When SUBROUTER_STACK_TENANT_DELETE_TOKEN is set but SUBROUTER_HOSTED_URL is unset, web/app/api/cli/config/route.ts returns defaultHostedSubrouterURL(). This route returns 503 at Line 21 instead. The CLI can complete discovery but cannot call its advertised exchange endpoint.
Use the same default URL and normalization path in both routes.
🤖 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/app/api/subrouter/exchange-v2/route.ts` around lines 17 - 25, Update the
hostedUrl initialization in the exchange route to fall back to
defaultHostedSubrouterURL() when SUBROUTER_HOSTED_URL is unset, then apply the
same trimming and trailing-slash normalization used by the CLI configuration
route. Keep the existing controlToken/hostedUrl availability check and 503
response unchanged.
| const upstream = await fetch(`${hostedUrl}/_subrouter/auth/stack`, { | ||
| method: "POST", | ||
| headers: { | ||
| authorization: `Bearer ${resolved.value.accessToken}`, | ||
| "content-type": "application/json", | ||
| "x-subrouter-stack-control-token": controlToken, | ||
| }, | ||
| body: JSON.stringify({ | ||
| capabilities, | ||
| teamId: resolved.value.team.teamId, | ||
| teamName: resolved.value.team.teamName, | ||
| }), | ||
| cache: "no-store", | ||
| }); | ||
| const body = await upstream.text(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate target and related files =="
fd -a 'route\.ts$' . | rg 'web/app/api/subrouter/exchange-v2/route\.ts|subrouter|exchange' || true
echo "== target excerpt =="
if [ -f web/app/api/subrouter/exchange-v2/route.ts ]; then
wc -l web/app/api/subrouter/exchange-v2/route.ts
cat -n web/app/api/subrouter/exchange-v2/route.ts
fi
echo "== reference resolveSubrouterRequestContext usages =="
rg -n "resolveSubrouterRequestContext|subrouter.*auth/stack|_subrouter/auth/stack|setTimeout|AbortSignal|with.*abort|cancellation|deadline" web/app web/lib packages apps . --glob '!node_modules' --glob '!dist' --glob '!build' | head -200Repository: manaflow-ai/cmux
Length of output: 22236
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('web/app/api/subrouter/exchange-v2/route.ts')
text = p.read_text() if p.exists() else ''
checks = {
'contains_fetch_call': 'fetch(`${hostedUrl}/_subrouter/auth/stack`' in text,
'contains_upstream_text': 'const body = await upstream.text()' in text,
'contains_no_abort_signal': '"signal"' not in text or text.find('"signal"') > text.find('fetch(') and 'AbortSignal' not in text,
'contains_abort_controller': 'AbortController' in text,
'imports_resolveSubrouterRequestContext': 'resolveSubrouterRequestContext' in text,
}
for k, v in checks.items():
print(f'{k}={v}')
PYRepository: manaflow-ai/cmux
Length of output: 316
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate subrouter requestContext files =="
fd -a 'requestContext\.(ts|tsx|js|jsx)$' web | sort
echo "== inspect requestContext =="
for f in $(fd -a 'requestContext\.(ts|tsx|js|jsx)$' web | sort); do
echo "--- $f ---"
wc -l "$f"
cat -n "$f"
done
echo "== inspect tenant-exchange route =="
f="web/app/api/subrouter/tenant-exchange/route.ts"
if [ -f "$f" ]; then
wc -l "$f"
cat -n "$f"
fi
echo "== inspect subrouter route tests under web =="
fd -a -e ts -e tsx 'subrouter.*test|test.*subrouter' web | sort
for f in $(fd -a -e ts -e tsx 'subrouter.*test|test.*subrouter' web | sort); do
echo "--- $f ---"
wc -l "$f"
sed -n '1,240p' "$f"
doneRepository: manaflow-ai/cmux
Length of output: 43186
Bound the hosted Stack exchange request with a cancellation-aware deadline.
resolveSubrouterRequestContext finishes with an unbounded signal after this call exits, then web/app/api/subrouter/tenant-exchange/route.ts calls fetch(...) and reads upstream.text() without a signal. A stalled _subrouter/auth/stack request can hold the tenant-exchange request until the platform timeout. Pass the request signal or another cancellation-aware boundary into the outbound call, and add coverage for an aborted hosted request.
🤖 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/app/api/subrouter/exchange-v2/route.ts` around lines 31 - 45, The hosted
Stack exchange request in the route handler must use a cancellation-aware
deadline instead of allowing fetch and upstream.text() to run indefinitely.
Update the flow around resolveSubrouterRequestContext and the outbound
_subrouter/auth/stack fetch to pass the request signal or an equivalent bounded
signal through the fetch and response-reading path, and add coverage verifying
an aborted hosted request is cancelled.
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/api/subrouter/tenant-exchange/route.ts`:
- Around line 43-50: Update the error-response paths in the tenant exchange
handler, including the !upstream.ok branch and catch path, to set Cache-Control:
no-store whenever returning unsubstituted upstream or subrouter response
content. Preserve the existing status, body, and content-type behavior, and
apply the same header to any equivalent error exits in the route.
- Around line 31-44: Update the hosted tenant exchange fetch in the route
handler using resolveSubrouterRequestContext to include an AbortSignal with a
reasonable timeout, ensuring stalled requests are cancelled independently of the
completed context resolution. Add coverage verifying the timeout/cancellation
response path and preserve existing successful exchange behavior.
🪄 Autofix
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: 0927f206-4d23-4efc-b9c1-917e7dbe2a4c
📒 Files selected for processing (2)
web/app/api/cli/config/route.tsweb/app/api/subrouter/tenant-exchange/route.ts
| const upstream = await fetch(`${hostedUrl}/_subrouter/auth/stack`, { | ||
| method: "POST", | ||
| headers: { | ||
| authorization: `Bearer ${resolved.value.accessToken}`, | ||
| "content-type": "application/json", | ||
| "x-subrouter-stack-control-token": controlToken, | ||
| }, | ||
| body: JSON.stringify({ | ||
| capabilities, | ||
| teamId: resolved.value.team.teamId, | ||
| teamName: resolved.value.team.teamName, | ||
| }), | ||
| cache: "no-store", | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file list =="
git ls-files | rg 'web/app/api/subrouter/tenant-exchange/route\.ts|permission|authorization|deadline|abort|fetch' | head -200
echo "== target file =="
cat -n web/app/api/subrouter/tenant-exchange/route.ts | sed -n '1,120p'
echo "== search relevant terms =="
rg -n "resolve|resolved\.value|accessToken|authorization|deadline|AbortSignal|fetch\(" web/app/api/web app web | rg "tenant-exchange|subrouter|auth" -n | head -250Repository: manaflow-ai/cmux
Length of output: 29951
🌐 Web query:
next.js NextRequest context deadline authorization header
💡 Result:
In Next.js, NextRequest extends the standard Web Request API, meaning it adheres to standard request handling patterns rather than having a built-in context or deadline object [1][2]. Handling Authorization Headers To access an Authorization header in Next.js middleware, use the standard Headers API via request.headers.get('authorization') [3]. Because Next.js middleware operates in the Edge runtime, you must use this standard Web API rather than legacy Node.js request properties [4][3]. To pass authorization or other context downstream (e.g., to API routes or Server Components), you must explicitly set them in the outgoing request headers using NextResponse.next [5][6]: // middleware.ts example import { NextResponse, NextRequest } from 'next/server'; export function middleware(request: NextRequest) { const requestHeaders = new Headers(request.headers); // Set context/headers for downstream routes requestHeaders.set('x-user-id', 'extracted-user-id'); return NextResponse.next({ request: { headers: requestHeaders, }, }); } Context and Deadlines Next.js does not provide a native "context" object or request "deadline" (timeout) mechanism within the NextRequest API [1][2]. 1. Context: There is no built-in mechanism to pass state between middleware and the application other than by modifying request headers [7]. Developers often use headers to pass verified user identities or other context metadata downstream [7][4]. 2. Deadlines: Next.js does not expose a request-level deadline property. If you need to enforce timeouts (e.g., for database queries or API calls performed within middleware), you must implement them manually using standard JavaScript approaches like AbortController [7]. Best Practices Avoid setting large headers, as this can lead to 431 Request Header Fields Too Large errors [5]. Additionally, verify authentication inside sensitive functions (like Server Functions) rather than relying solely on middleware-based authorization, as middleware can be bypassed if routes are not correctly matched [8].
Citations:
- 1: https://nextjs.org/docs/app/api-reference/functions/next-request
- 2: https://nextjs.org/docs/14/app/api-reference/functions/next-request
- 3: Getting token in middleware doesn't work with authorization header request nextauthjs/next-auth#4042
- 4: https://dev.to/jtorchia/nextjs-16-middleware-authorization-patterns-that-scale-and-the-ones-that-cause-race-conditions-4pfk
- 5: https://nextjs.org/docs/14/app/building-your-application/routing/middleware
- 6: Modifying headers in middleware from server-side request vercel/next.js#67540
- 7: https://juanchi.dev/en/blog/nextjs-16-middleware-authorization-patterns-race-conditions
- 8: https://nextjs.org/docs/app/api-reference/file-conventions/proxy
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tracked tests around hosted subrouter =="
git ls-files 'web/tests/*subrouter*.test.*' | sort
for f in $(git ls-files 'web/tests/*subrouter*.test.*'); do
echo "--- $f wc"
wc -l "$f"
done
echo "== target tests references route =="
for f in $(git ls-files 'web/tests/*subrouter*.test.*'); do
echo "--- $f route.ts refs"
rg -n "tenant-exchange|exchangeTeam|_subrouter/auth/stack|signal:|timeout|stubFetch|AbortSignal" "$f" || true
done
echo "== requestContext =="
cat -n web/services/subrouter/requestContext.ts | sed -n '1,220p'
echo "== subrouter routeHelpers =="
cat -n web/services/subrouter/routeHelpers.ts | sed -n '1,180p'
echo "== existing bounded fetch patterns =="
cat -n web/app/lib/agent-page-canonical-fetch.ts | sed -n '1,120p'
cat -n web/scripts/subrouter/migrate-legacy-tenants.ts | sed -n '370,400p'Repository: manaflow-ai/cmux
Length of output: 17024
Bound the hosted tenant exchange request with a cancellation-aware signal.
resolveSubrouterRequestContext() returns a value, so the stack authorization deadline has ended before this route makes the external fetch. The call also has no signal, so a stalled hosted service can hold route capacity. Use an AbortSignal with a reasonable timeout for this hosted request, and add test coverage for the timeout response path.
🤖 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/app/api/subrouter/tenant-exchange/route.ts` around lines 31 - 44, Update
the hosted tenant exchange fetch in the route handler using
resolveSubrouterRequestContext to include an AbortSignal with a reasonable
timeout, ensuring stalled requests are cancelled independently of the completed
context resolution. Add coverage verifying the timeout/cancellation response
path and preserve existing successful exchange behavior.
| cache: "no-store", | ||
| }); | ||
| const body = await upstream.text(); | ||
| if (!upstream.ok) { | ||
| return new Response(body, { | ||
| status: upstream.status, | ||
| headers: { "content-type": "text/plain; charset=utf-8" }, | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate file =="
wc -l web/app/api/subrouter/tenant-exchange/route.ts
cat -n web/app/api/subrouter/tenant-exchange/route.ts
echo "== route helpers and cache header helpers =="
rg -n "Cache-Control|cache-control|jsonResponse|serviceUnavailableResponse|unauthorized" web/services/subrouter -S
cat -n web/services/subrouter/routeHelpers.ts | sed -n '1,240p'
echo "== subrouter auth helpers for cache headers =="
rg -n "Cache-Control|cache-control|unauthorized|forbidden|serviceUnavailable" web/services/subrouter web/app/api/subrouter -SRepository: manaflow-ai/cmux
Length of output: 13353
Sensitive Data Exposure (CWE-525): Use of Web Browser Cache Containing Sensitive Information
Reachability: External
Reachability path
● Entry
web/app/api/cli/config/route.ts:6
runtime
│
▼
● Sink
web/app/api/subrouter/tenant-exchange/route.ts
Set Cache-Control: no-store on every response that includes unsubstituted upstream body content.
cache: "no-store" only affects the outbound fetch. This branch returns upstream.text() without a cache restriction, and the catch path also returns subrouter response content without cache-control: no-store. Add that header to these error responses and any equivalent subrouter error exits that include upstream body content.
🤖 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/app/api/subrouter/tenant-exchange/route.ts` around lines 43 - 50, Update
the error-response paths in the tenant exchange handler, including the
!upstream.ok branch and catch path, to set Cache-Control: no-store whenever
returning unsubstituted upstream or subrouter response content. Preserve the
existing status, body, and content-type behavior, and apply the same header to
any equivalent error exits in the route.
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 (2)
web/app/api/subrouter/tenant-exchange/route.ts (2)
45-50: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winInformation Disclosure (CWE-209): Generation of Error Message Containing Sensitive Information
Reachability: External
Do not return the upstream error body verbatim.
route.ts:45-50copiesupstream.text()directly into the API response. The hosted endpoint may include provider names, internal identifiers, headers, or raw diagnostics. Map non-2xx responses to a stable safe error code and keep detailed diagnostics in redacted server logs.🤖 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/app/api/subrouter/tenant-exchange/route.ts` around lines 45 - 50, Update the non-2xx handling in the tenant exchange route to avoid returning the raw upstream body; map failures to a stable, safe error code in the client response while preserving the upstream status as appropriate. Log the detailed upstream diagnostics only on the server after redaction, and keep successful response handling unchanged.Source: Coding guidelines
31-44: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSensitive Data Exposure (CWE-200): Exposure of Sensitive Information to an Unauthorized Actor
Reachability: Internal
Disable redirects for the credentialed exchange.
fetchfollows redirects by default. This request carries access tokens and thex-coderouter-controlheader; ifSUBROUTER_HOSTED_URLor an intermediary redirects, credentials may reach an unintended target. Useredirect: "error"or handle redirects manually with explicit host/header validation before retrying.🤖 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/app/api/subrouter/tenant-exchange/route.ts` around lines 31 - 44, Update the credentialed fetch in the tenant exchange flow to disable automatic redirects by setting its redirect policy to error. Preserve the existing request URL, headers, body, and no-store caching behavior.Source: MCP tools
🤖 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 `@web/app/api/subrouter/tenant-exchange/route.ts`:
- Around line 45-50: Update the non-2xx handling in the tenant exchange route to
avoid returning the raw upstream body; map failures to a stable, safe error code
in the client response while preserving the upstream status as appropriate. Log
the detailed upstream diagnostics only on the server after redaction, and keep
successful response handling unchanged.
- Around line 31-44: Update the credentialed fetch in the tenant exchange flow
to disable automatic redirects by setting its redirect policy to error. Preserve
the existing request URL, headers, body, and no-store caching behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 94aaad5e-8eea-467c-9bdb-8764b6a62e64
📒 Files selected for processing (1)
web/app/api/subrouter/tenant-exchange/route.ts
0eecd5a (#9607) switched SUBROUTER_STACK_TENANT_DELETE_TOKEN to the validated env object, but t3-env freezes values at first import, so tenant-control configuration became unobservable after boot and the unconfigured paths broke: the exchange route returns 200 instead of 503 and account deletion fires hosted tenant deletes for accounts that never enabled Subrouter. web tests have been red on main since (CI paused). env.ts still validates presence on Vercel non-preview deployments. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 participant
Moves the trusted hosted-tenant exchange request into the API route so the deployed function always sends the scoped capabilities and dedicated control credential. This fixes production CLI login failures caused by a stale bundled hosted client omitting the control header.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Moves the trusted hosted-tenant exchange into API routes and switches the CLI to a semantic endpoint. Always forwards scoped capabilities and the stack control token header to fix login failures from stale clients.
POST /api/subrouter/tenant-exchangethat posts to/_subrouter/auth/stackwithAuthorization,x-subrouter-stack-control-token, capability flags, and team info; uses no-store caching, propagates upstream errors, and returns parsed tenant JSON.exchangeUrlto/api/subrouter/tenant-exchangeandcache-controltono-storeon the config response.env(SUBROUTER_HOSTED_URL,SUBROUTER_STACK_TENANT_DELETE_TOKEN); return 503 if missing.Written for commit 935611b. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes