feat(daemon): expose host_browser to LLM cross-client on web/iOS turns#29829
Conversation
Adds "host_browser" to CROSS_CLIENT_EXPOSED_CAPABILITIES so the LLM- exposure carve-out in isToolActiveForContext lets web and iOS turns see the tool when at least one host_browser-capable client (macOS or chrome-extension) is connected via the assistant event hub. Routing for host_browser shipped in PR #27489 (host-browser-via-macos- host-proxy), but the LLM-exposure gate in conversation-tool-setup.ts was still hardcoded to deny non-host-proxy transports. With this change the user can drive their host browser from any interactive interface, matching the parity already extended to host_bash + host_file in PR #29632. Tests: 8 new cases mirror the host_file_* block (web exposed, ios exposed, web denied without client, ios denied without client, hasNoClient gate honored, macos and chrome-extension transports unaffected, per- capability invariant guard against hardcoded-capability regressions). Total: 44 pass / 0 fail. Defense-in-depth follow-up (deferred): host_browser does not yet adopt the same-actor guard that host_bash / host_file / host_cu / host_transfer apply on their result routes. The chrome-extension client today does not send `x-vellum-client-id` on its result POST; enforcing the guard requires a coordinated extension change. The current `requireGuardian` gate on `/v1/host-browser-result` is the active same-binding boundary.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6dcadca45b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const CROSS_CLIENT_EXPOSED_CAPABILITIES = new Set<HostProxyCapability>([ | ||
| "host_bash", | ||
| "host_file", | ||
| "host_browser", |
There was a problem hiding this comment.
Gate host_browser cross-client exposure by actor identity
Adding "host_browser" to the cross-client exposure set allows web/iOS turns to surface this tool whenever any host-browser-capable client is connected, but the host-browser execution path still lacks the same actor-binding protections used by other host proxies (HostBrowserProxy.request does not persist targetClientId/targetActorPrincipalId, and /v1/host-browser-result does not validate x-vellum-client-id). In a multi-actor daemon, this can route one actor’s browser request onto another actor’s connected extension/macOS client, which is a cross-user host-action boundary violation introduced by this exposure change.
Useful? React with 👍 / 👎.
| * Defense-in-depth follow-up (deferred): host_browser does not yet adopt | ||
| * the same-actor guard that host_bash / host_file / host_cu / host_transfer | ||
| * apply to their result routes. The chrome-extension client today does not | ||
| * send `x-vellum-client-id` on its result POST, so enforcing the guard | ||
| * requires a coordinated extension change. The current `requireGuardian` | ||
| * gate on `/v1/host-browser-result` is the same-binding boundary. Tracking | ||
| * issue: TBD. |
There was a problem hiding this comment.
🚩 Deferred same-actor guard gap is explicitly acknowledged
The comment block at assistant/src/daemon/conversation-tool-setup.ts:376-382 explicitly documents that host_browser does not yet have the same-actor guard that host_bash/host_file/host_cu/host_transfer apply to their result routes. The current security boundary is requireGuardian: true on /v1/host-browser-result (visible at assistant/src/runtime/routes/host-browser-routes.ts:251). This is a known gap with a tracking issue placeholder (TBD). Since the chrome-extension client doesn't send x-vellum-client-id, enforcing the guard requires a coordinated extension update. This is a reasonable deferral but worth tracking.
Was this helpful? React with 👍 or 👎 to provide feedback.
…ctor
The original cross-client exposure change opened host_browser to web/iOS
turns whenever any host-browser-capable client is connected, but did not
extend the same-user actor binding that already protects host_bash,
host_file, and host_cu. Codex flagged this on the parent commit as a
multi-actor cross-user host-action boundary violation; Devin tracked the
same gap as a deferral.
This commit closes the gap by mirroring the host-cu pattern at every
layer:
Daemon proxy (host-browser-proxy.ts):
- resolveTargetClient(sourceActorPrincipalId) filters host_browser
candidate clients by actorPrincipalId before applying the
chrome-extension-first interface preference. When no actor is
supplied (legacy/internal flows), falls back to the prior
getPreferredClientByCapability path.
- request() accepts optional sourceActorPrincipalId, refuses to
dispatch to a different-actor client via enforceSameActorOrErrorResult,
persists targetClientId/targetActorPrincipalId on the pending
interaction, and broadcasts with targetClientId so the client-id
binding rides on the SSE envelope.
Result route (host-browser-routes.ts):
- resolveHostBrowserResultByRequestId now reads x-vellum-client-id
and x-vellum-actor-principal-id headers, validates the submitter
matches peeked.targetClientId (400 missing / 403 mismatch), and
runs enforceSameActorOrThrow against the persisted
targetActorPrincipalId.
- On a 403 the pending interaction is preserved so the legitimate
client can still submit. Untargeted requests pass through unchanged.
- OpenAPI gains additionalResponses for 400/403/404/409.
CDP client plumbing (extension-cdp-client.ts, factory.ts):
- ExtensionCdpClient takes optional sourceActorPrincipalId and
forwards it on every proxy.request call.
- buildCandidateList and buildPinnedCandidateList read
sourceActorPrincipalId from ToolContext (already populated from
conversation.trustContext.guardianPrincipalId) and thread it
through createExtensionCdpClient.
Client-side header emission:
- clients/shared/Network/HostProxyClient.swift (macOS): postBrowserResult
now sends X-Vellum-Client-Id, mirroring postCuResult / postTransferResult.
- clients/chrome-extension/background/worker.ts: both the SSE-path POST
and the self-hosted fallback POST add X-Vellum-Client-Id from
getClientId(). The Chrome-extension WS host_browser_result code path
is untouched and remains dead code on the server side.
SameActorOp union gains "host_browser" so the structured-warn audit log
entry uses the same shape as the other host-proxy capabilities.
Test fixtures:
- assistant/src/__tests__/fixtures/mock-chrome-extension.ts now sends
X-Vellum-Client-Id on its result POST so e2e tests do not 400.
Tests added:
- assistant/src/__tests__/host-browser-proxy.test.ts: 6 new tests in
a "same-actor binding" describe — persists target binding when
actor matches, rejects when only different-actor clients are
connected, prefers chrome-extension over macos among same-actor
clients, falls back to macOS bridge for same actor, legacy path
still works, rejects when the only candidates are different-actor.
- assistant/src/__tests__/host-browser-routes.test.ts: new
"handleHostBrowserResult — same-actor guard" describe block covering
targeted-correct-headers, whitespace trim, missing/wrong client-id,
actor-principal mismatch, and untargeted regressions.
- assistant/src/tools/browser/cdp-client/__tests__/factory.test.ts:
asserts the factory threads sourceActorPrincipalId from ToolContext
into createExtensionCdpClient on the extension code path.
- factory.test.ts also adopts the spread-real-module mock pattern so
its createExtensionCdpClient stub does not clobber ExtensionCdpClient
for downstream test files (defensive, since CI test.sh already runs
each file in its own bun process).
Backwards compatibility:
- Chrome-extension WS host_browser_result code path is dead on the
server side; only the HTTP POST path is exercised by both clients.
- Untargeted requests with no targetClientId continue to work, so
pre-update Chrome extensions (which now will start sending the
header) do not regress.
- Legacy callers without sourceActorPrincipalId hit the
getPreferredClientByCapability fallback unchanged.
Refs: Codex P1 / Devin deferral on PR #29829.
|
Bundled the same-actor binding fix into this PR per Noa's greenlight, addressing Codex's P1 boundary-violation finding and Devin's matching deferral. What changed (commit
Why bundle vs follow-up PR:
Tests added
Local verification (per-file via
cc @chatgpt-codex-connector @devin-ai-integration — the actor-binding gap you flagged is closed. Ready for re-review. |
…-binding responses Adds 400/403/404/409 entries for /v1/host-browser-result. No spec authoring changes — `bun run generate:openapi` output for the additionalResponses introduced in the previous commit.
|
Codex Review: Didn't find any major issues. Another round soon, please! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@devin-ai-integration please re-review — bundled the same-actor binding fix at commit |
#30163) * refactor(host-browser): remove dead WS resolveResult path The chrome extension migrated to HTTP POST in PR #29829. There is no /v1/browser-relay WebSocket endpoint in the current runtime — no WS handler calls HostBrowserProxy.resolveResult. Removing the public method eliminates the foot-gun of a future WS handler bypassing the kind-check and same-actor guard that the HTTP route enforces. The route now inlines pendingInteractions.resolve() + rpcResolve directly. Tests are updated to use a local resolveResult helper that mirrors the same call sequence. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(host-proxy-preactivation): add cross-client drain-path coverage PR #30154 enabled web/iOS turns to drive host_app_control and host_cu on a connected macOS client (cross-client routing). The existing drain-path tests cover macOS-native + chrome-extension + slack; this adds the missing cross-client cases: - web source + listClientsByCapability returns 1 macOS client → both app-control and computer-use skills re-added - web source + listClientsByCapability returns [] → neither re-added Introduces a per-test mockCapabilityClients variable (with afterEach reset) so hub state doesn't bleed across tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: credence-the-bot[bot] <test@test.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
What
Adds
"host_browser"toCROSS_CLIENT_EXPOSED_CAPABILITIESso the LLM-exposure carve-out inisToolActiveForContextlets web and iOS turns see the tool when at least onehost_browser-capable client (macOS or chrome-extension) is connected via the assistant event hub.Why
Routing for
host_browsershipped in PR #27489 (host-browser-via-macos-host-proxy), but the LLM-exposure gate inconversation-tool-setup.tswas still hardcoded to deny non-host-proxy transports — even though the underlyingHostBrowserProxyhas worked cross-client since the macOS bridge merged. That left a dead carve-out comment ("Re-evaluate when host browser via macOS host proxy ships (PR #27489)") and asymmetric capability surface vs. host_bash and host_file_*, which got the same treatment in PR #29632 last week.With this change, a web or iOS user can drive their host browser end-to-end, matching the parity already extended to
host_bash+host_file.How
const CROSS_CLIENT_EXPOSED_CAPABILITIES = new Set<HostProxyCapability>([ "host_bash", "host_file", + "host_browser", ]);Plus comment cleanup in two places (the doc block above the Set, and the inline note inside
isToolActiveForContext) so the rationale stays accurate.Tests
8 new cases under a new
cross-client exposure for host_browserdescribe block, mirroring the existing host_file_* pattern:hasNoClient: true(no approval UI)ESLint clean. Pre-existing
@vellumai/ipc-server-utilstypecheck errors from PR #29825 are unrelated and exist on clean main.Defense-in-depth follow-up (deferred)
host_browserdoes not yet adopt the same-actor guard thathost_bash/host_file/host_cu/host_transferapply on their result routes. Two specific gaps:HostBrowserProxy.request()doesn't storetargetClientId/targetActorPrincipalIdin the pending request state./v1/host-browser-resultroute does not validatex-vellum-client-id(host_cu does — seehost-cu-routes.ts:77-103).The chrome-extension client today does not send
x-vellum-client-idon its result POST, so enforcing the guard requires a coordinated extension change. The currentrequireGuardiangate on/v1/host-browser-resultis the active same-binding boundary. Tracking issue: TBD.Related