fix(sandbox): require auth token for browser bridge server#11027
fix(sandbox): require auth token for browser bridge server#11027coygeek wants to merge 1 commit intoopenclaw:mainfrom
Conversation
Generate a cryptographically random auth token when starting the sandbox browser bridge and pass it to startBrowserBridgeServer(). This ensures all bridge endpoints require Bearer token authentication, preventing cross-session local takeover of sandbox browser state. The token is registered in a URL-keyed registry so that fetchBrowserJson transparently attaches the Authorization header to all outgoing HTTP requests targeting the protected bridge. The token is also exposed via SandboxBrowserContext.bridgeAuthToken for the owning session. Co-Authored-By: Claude Opus 4.6 <[email protected]>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const mergedHeaders = { | ||
| ...authHeaders, | ||
| ...(init?.headers as Record<string, string> | undefined), | ||
| }; | ||
| return await fetchHttpJson<T>(url, { ...init, headers: mergedHeaders, timeoutMs }); | ||
| } |
There was a problem hiding this comment.
Bearer auth header lost
fetchBrowserJson() merges headers with { ...authHeaders, ...(init?.headers as Record<string, string> | undefined) }, but RequestInit.headers is not always a plain object (it can be Headers or [string, string][]). Spreading a Headers instance produces {} in practice, so callers that pass new Headers(...) will silently drop both their own headers and the injected Authorization header, causing sandbox bridge requests to fail with 401.
Consider normalizing via new Headers(init?.headers) and then set("Authorization", ...) before calling fetchHttpJson.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/browser/client-fetch.ts
Line: 91:96
Comment:
**Bearer auth header lost**
`fetchBrowserJson()` merges headers with `{ ...authHeaders, ...(init?.headers as Record<string, string> | undefined) }`, but `RequestInit.headers` is not always a plain object (it can be `Headers` or `[string, string][]`). Spreading a `Headers` instance produces `{}` in practice, so callers that pass `new Headers(...)` will silently drop both their own headers *and* the injected `Authorization` header, causing sandbox bridge requests to fail with 401.
Consider normalizing via `new Headers(init?.headers)` and then `set("Authorization", ...)` before calling `fetchHttpJson`.
How can I resolve this? If you propose a fix, please make it concise.
|
Heads up: the lint failure ( git fetch origin main && git rebase origin/main && git push --force-with-lease |
|
Update: the lint failure has been resolved by #11093 (merged), which excluded git fetch origin main && git rebase origin/main && git push --force-with-lease |
thewilloftheshadow
force-pushed
the
main
branch
from
bfc1ccb to
f92900f
Compare
|
Cleanup note for traceability:
Closing this PR to align with duplicate routing. If maintainers want this exact patch revived, I can reopen or submit a retargeted PR. |
2 participants
Fix Summary
Generate a cryptographically random auth token when starting the sandbox browser bridge and pass it to
startBrowserBridgeServer(). This ensures all bridge endpoints require Bearer token authentication, preventing cross-session local takeover of sandbox browser state.Previously, the sandbox startup path called
startBrowserBridgeServer()without anauthToken, leaving the bridge API completely unauthenticated to any local process.Issue Linkage
Fixes #11023
Security Snapshot
Implementation Details
Files Changed
src/agents/sandbox/browser-bridges.ts(+4/-1)src/agents/sandbox/browser.ts(+19/-7)src/agents/sandbox/types.ts(+1/-0)src/browser/client-fetch.ts(+38/-1)Technical Analysis
Generate a cryptographically random auth token when starting the sandbox browser bridge and pass it to
startBrowserBridgeServer(). This ensures all bridge endpoints require Bearer token authentication, preventing cross-session local takeover of sandbox browser state.Validation Evidence
pnpm buildRisk and Compatibility
non-breaking; compatibility impact was not explicitly documented in the original PR body.
AI-Assisted Disclosure
Greptile Overview
Greptile Summary
startBrowserBridgeServer()so the local bridge API enforces Bearer authentication.Authorization: Bearer <token>to bridge requests.Confidence Score: 4/5
fetchBrowserJson’s new header merge logic: it assumesinit.headersis a plain object, which is not guaranteed by Fetch and can cause Authorization injection to be dropped, leading to 401s at runtime depending on how callers construct headers.