From 6dcadca45b88fec574c53085e74cb20263f57340 Mon Sep 17 00:00:00 2001 From: credence-the-bot <196986132+credence-the-bot@users.noreply.github.com> Date: Wed, 6 May 2026 13:51:52 +0000 Subject: [PATCH 1/3] feat(daemon): expose host_browser to LLM cross-client on web/iOS turns 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. --- .../__tests__/conversation-tool-setup.test.ts | 113 +++++++++++++++++- .../src/daemon/conversation-tool-setup.ts | 34 ++++-- 2 files changed, 130 insertions(+), 17 deletions(-) diff --git a/assistant/src/daemon/__tests__/conversation-tool-setup.test.ts b/assistant/src/daemon/__tests__/conversation-tool-setup.test.ts index 80fde45e68a..9bf650d0f38 100644 --- a/assistant/src/daemon/__tests__/conversation-tool-setup.test.ts +++ b/assistant/src/daemon/__tests__/conversation-tool-setup.test.ts @@ -20,10 +20,11 @@ * hasNoClient flag. * * Cross-client exception: tools whose capabilities are in - * CROSS_CLIENT_EXPOSED_CAPABILITIES (host_bash, host_file) are allowed for - * non-host-proxy interfaces (e.g. "web") when at least one capable client - * is connected via the event hub. host_browser is excluded (chrome-extension - * is its own executor; web turns have no CDP target model). + * CROSS_CLIENT_EXPOSED_CAPABILITIES (host_bash, host_file, host_browser) + * are allowed for non-host-proxy interactive interfaces ("web", "ios") + * when at least one capable client is connected via the event hub. + * chrome-extension is excluded as a security boundary, regardless of + * whether the capability is technically supported elsewhere. */ import { beforeEach, describe, expect, mock, test } from "bun:test"; @@ -358,6 +359,110 @@ describe("isToolActiveForContext — cross-client exposure for host_file_*", () }); }); +describe("isToolActiveForContext — cross-client exposure for host_browser", () => { + // host_browser cross-client routing was shipped in PR #27489 (host- + // browser-via-macos-host-proxy); LLM-exposure for non-host-proxy + // transports is added by including "host_browser" in + // CROSS_CLIENT_EXPOSED_CAPABILITIES. Web and iOS turns can now drive a + // connected macOS or chrome-extension client via the event hub. + test("host_browser is exposed for web transport when a host_browser client is connected", () => { + mockClientCountByCapability.set("host_browser", 1); + expect( + isToolActiveForContext( + "host_browser", + makeCtx({ hasNoClient: false, transportInterface: "web" }), + ), + ).toBe(true); + }); + + test("host_browser is exposed for ios transport when a host_browser client is connected", () => { + // INTERACTIVE_INTERFACES = {macos, ios, web}; ios goes through the same + // cross-client branch as web because supportsHostProxy("ios", *) is + // false. This pins the parity guarantee. + mockClientCountByCapability.set("host_browser", 1); + expect( + isToolActiveForContext( + "host_browser", + makeCtx({ hasNoClient: false, transportInterface: "ios" }), + ), + ).toBe(true); + }); + + test("host_browser is NOT exposed for web when no host_browser client is connected", () => { + mockClientCountByCapability.set("host_browser", 0); + expect( + isToolActiveForContext( + "host_browser", + makeCtx({ hasNoClient: false, transportInterface: "web" }), + ), + ).toBe(false); + }); + + test("host_browser is NOT exposed for ios when no host_browser client is connected", () => { + mockClientCountByCapability.set("host_browser", 0); + expect( + isToolActiveForContext( + "host_browser", + makeCtx({ hasNoClient: false, transportInterface: "ios" }), + ), + ).toBe(false); + }); + + test("host_browser is NOT exposed when hasNoClient is true (no approval UI)", () => { + // hasNoClient gate: cross-client exception must not bypass this. + mockClientCountByCapability.set("host_browser", 1); + expect( + isToolActiveForContext( + "host_browser", + makeCtx({ hasNoClient: true, transportInterface: "web" }), + ), + ).toBe(false); + }); + + test("host_browser for macos transport is unaffected by the cross-client exception", () => { + // macos natively supports host_browser via host proxy — the + // supportsHostProxy check passes, so the cross-client branch is never + // reached. + mockClientCountByCapability.set("host_browser", 0); + expect( + isToolActiveForContext( + "host_browser", + makeCtx({ hasNoClient: false, transportInterface: "macos" }), + ), + ).toBe(true); + }); + + test("host_browser for chrome-extension transport is unaffected by the cross-client exception", () => { + // chrome-extension natively supports host_browser via its own + // executor (supportsHostProxy("chrome-extension", "host_browser") + // returns true), so the cross-client branch is never reached. The + // hasNoClient gate is also bypassed for chrome-extension transports + // because the extension provides its own approval UI. + mockClientCountByCapability.set("host_browser", 0); + expect( + isToolActiveForContext( + "host_browser", + makeCtx({ hasNoClient: true, transportInterface: "chrome-extension" }), + ), + ).toBe(true); + }); + + test("listClientsByCapability is queried with host_browser, not host_bash or host_file (per-capability invariant)", () => { + // Defense against any future regression that hardcodes a different + // capability in the cross-client check. Only host_browser-capable + // clients should satisfy host_browser exposure. + mockClientCountByCapability.set("host_bash", 1); + mockClientCountByCapability.set("host_file", 1); + mockClientCountByCapability.set("host_browser", 0); + expect( + isToolActiveForContext( + "host_browser", + makeCtx({ hasNoClient: false, transportInterface: "web" }), + ), + ).toBe(false); + }); +}); + describe("HOST_TOOL_NAMES derivation", () => { test("HOST_TOOL_NAMES is derived from HOST_TOOL_TO_CAPABILITY", () => { // Sanity check: every tool in the names set has a capability mapping. diff --git a/assistant/src/daemon/conversation-tool-setup.ts b/assistant/src/daemon/conversation-tool-setup.ts index 3c2d723a1b3..c6039fa87f5 100644 --- a/assistant/src/daemon/conversation-tool-setup.ts +++ b/assistant/src/daemon/conversation-tool-setup.ts @@ -357,7 +357,7 @@ export const HOST_TOOL_TO_CAPABILITY = new Map([ export const HOST_TOOL_NAMES = new Set(HOST_TOOL_TO_CAPABILITY.keys()); /** * Capabilities eligible for cross-client exposure on non-host-proxy - * transports (e.g. web, ios routing to a connected macOS client). + * transports (e.g. web, ios routing to a connected capable client). * Adding a capability here exposes ALL tools that map to it (per * HOST_TOOL_TO_CAPABILITY) on non-host-proxy transports — the daemon then * routes the actual invocation to the connected capable client via the @@ -366,17 +366,25 @@ export const HOST_TOOL_NAMES = new Set(HOST_TOOL_TO_CAPABILITY.keys()); * Inclusions: * - host_bash (Phase 1, PR #29322) * - host_file (Phases 2 & 3, PRs #29398 + #29440) + * - host_browser (executor parity shipped in PR #27489 host-browser-via- + * macos-host-proxy, exposed cross-client here) * * Exclusions: - * - host_browser: chrome-extension is its own executor; web turns don't - * have a CDP target model. Re-evaluate when host browser via macOS - * host proxy ships (PR #27489). * - host_app_control, host_cu: not in HOST_TOOL_TO_CAPABILITY - * (skill-routed). + * (skill-routed; cross-client preactivation is a separate workstream). + * + * 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. */ const CROSS_CLIENT_EXPOSED_CAPABILITIES = new Set([ "host_bash", "host_file", + "host_browser", ]); const CLIENT_CAPABILITY_TOOL_NAMES = new Set(["app_open"]); const PLATFORM_TOOL_NAMES = new Set(["request_system_permission"]); @@ -413,15 +421,15 @@ export function isToolActiveForContext( // transport cannot service this capability, the tool is filtered out. if (transport && capability && !supportsHostProxy(transport, capability)) { // Cross-client exception: allow host tools whose capabilities have - // cross-client routing infrastructure (Phases 1–3) to be exposed for - // non-host-proxy transports (e.g. "web", "ios") when at least one - // capable client is connected via the event hub. Members of - // CROSS_CLIENT_EXPOSED_CAPABILITIES (host_bash, host_file) qualify; - // host_browser is intentionally excluded (chrome-extension is its - // own executor and web turns don't have a CDP target model). + // cross-client routing infrastructure (Phases 1–3 plus host_browser + // via PR #27489) to be exposed for non-host-proxy transports (e.g. + // "web", "ios") when at least one capable client is connected via + // the event hub. Members of CROSS_CLIENT_EXPOSED_CAPABILITIES + // (host_bash, host_file, host_browser) qualify. // chrome-extension transport is excluded as a security boundary - // (extension only gets host_browser); hasNoClient turns are excluded - // (no interactive approval UI available). + // (extension only gets host_browser via its own executor path); + // hasNoClient turns are excluded (no interactive approval UI + // available). if ( capability && CROSS_CLIENT_EXPOSED_CAPABILITIES.has(capability) && From ab370e77b5164b2d7326fce7a0f5a1e08d70db51 Mon Sep 17 00:00:00 2001 From: credence-the-bot Date: Wed, 6 May 2026 20:34:11 +0000 Subject: [PATCH 2/3] fix(host-browser): bind host_browser proxy + result route to caller actor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../fixtures/mock-chrome-extension.ts | 5 + .../src/__tests__/host-browser-proxy.test.ts | 217 ++++++++++++ .../src/__tests__/host-browser-routes.test.ts | 318 ++++++++++++++++++ assistant/src/daemon/host-browser-proxy.ts | 87 ++++- assistant/src/runtime/auth/same-actor.ts | 1 + .../src/runtime/routes/host-browser-routes.ts | 113 ++++++- .../cdp-client/__tests__/factory.test.ts | 44 +++ .../cdp-client/extension-cdp-client.ts | 16 +- .../src/tools/browser/cdp-client/factory.ts | 8 +- clients/chrome-extension/background/worker.ts | 10 +- clients/shared/Network/HostProxyClient.swift | 5 + 11 files changed, 805 insertions(+), 19 deletions(-) diff --git a/assistant/src/__tests__/fixtures/mock-chrome-extension.ts b/assistant/src/__tests__/fixtures/mock-chrome-extension.ts index 23e88434ec4..81f9f0e8b97 100644 --- a/assistant/src/__tests__/fixtures/mock-chrome-extension.ts +++ b/assistant/src/__tests__/fixtures/mock-chrome-extension.ts @@ -224,6 +224,11 @@ export function createMockChromeExtension( headers: { "Content-Type": "application/json", Authorization: `Bearer ${options.token}`, + // Same-actor binding: identifies this fixture to the daemon's + // result-route check. The daemon will reject targeted host_browser + // results without this header (or where it doesn't match the + // captured target client). + "X-Vellum-Client-Id": clientId, }, body: JSON.stringify(body), }); diff --git a/assistant/src/__tests__/host-browser-proxy.test.ts b/assistant/src/__tests__/host-browser-proxy.test.ts index 10f1310c783..ef223e63f3f 100644 --- a/assistant/src/__tests__/host-browser-proxy.test.ts +++ b/assistant/src/__tests__/host-browser-proxy.test.ts @@ -13,6 +13,22 @@ mock.module("../util/logger.js", () => ({ let publishedEvents: unknown[] = []; let mockHasConnection = true; +/** + * Per-test client roster used by the same-actor binding tests below. + * + * When non-empty, `listClientsByCapability` and `getActorPrincipalIdForClient` + * read from this list. The legacy `mockHasConnection` boolean continues to + * drive `getPreferredClientByCapability`, which is the fallback path used + * when the caller has no `sourceActorPrincipalId` (legacy/internal flows). + */ +type MockClient = { + clientId: string; + interfaceId: "chrome-extension" | "macos"; + actorPrincipalId?: string; + capabilities: string[]; +}; +let mockClients: MockClient[] = []; + mock.module("../runtime/assistant-event-hub.js", () => ({ assistantEventHub: { publish: async (event: unknown, _options?: unknown) => { @@ -27,6 +43,10 @@ mock.module("../runtime/assistant-event-hub.js", () => ({ capabilities: ["host_browser"], } : undefined, + listClientsByCapability: (cap: string) => + mockClients.filter((c) => c.capabilities.includes(cap)), + getActorPrincipalIdForClient: (clientId: string) => + mockClients.find((c) => c.clientId === clientId)?.actorPrincipalId, }, broadcastMessage: (msg: unknown) => { publishedEvents.push(msg); @@ -53,6 +73,7 @@ describe("HostBrowserProxy", () => { pendingInteractions.clear(); publishedEvents = []; mockHasConnection = true; + mockClients = []; proxy = HostBrowserProxy.instance; }); @@ -369,4 +390,200 @@ describe("HostBrowserProxy", () => { expect(spy.removeCalls).toEqual(["abort"]); }); }); + + // --------------------------------------------------------------------------- + // Same-actor binding (cross-user enforcement) + // + // The host-browser proxy is auto-resolved (the LLM-facing tool does not + // expose `target_client_id`), so same-user enforcement runs on the + // resolved candidate before we register the pending interaction: + // + // 1. `resolveTargetClient(sourceActorPrincipalId)` filters candidate + // clients to those owned by the caller's actor before applying the + // chrome-extension-first interface preference. When the caller has + // no actor (legacy/internal flow), it falls back to the legacy + // `getPreferredClientByCapability` path used by other tests above. + // 2. The proxy persists `targetClientId` and `targetActorPrincipalId` + // on the pending interaction so the result-route's same-actor check + // has authoritative bindings to compare against (mirrors host-cu). + // + // These tests focus on (1) and (2). Result-side guard coverage lives in + // `host-browser-routes.test.ts` (HTTP 400/403 against the same bindings). + // --------------------------------------------------------------------------- + + describe("same-actor binding", () => { + test("persists targetClientId + targetActorPrincipalId when caller actor matches", async () => { + mockClients = [ + { + clientId: "ext-client", + interfaceId: "chrome-extension", + actorPrincipalId: "user-1", + capabilities: ["host_browser"], + }, + ]; + + const resultPromise = proxy.request( + { cdpMethod: "Page.navigate", cdpParams: { url: "https://a.test" } }, + "session-1", + undefined, + "user-1", + ); + + expect(getPublishedMessages()).toHaveLength(1); + const sent = getPublishedMessages()[0] as Record; + const requestId = sent.requestId as string; + + const pending = pendingInteractions.get(requestId); + expect(pending).toBeDefined(); + expect(pending?.targetClientId).toBe("ext-client"); + expect(pending?.targetActorPrincipalId).toBe("user-1"); + + proxy.resolveResult(requestId, { content: "ok", isError: false }); + const result = await resultPromise; + expect(result.isError).toBe(false); + }); + + test("rejects when only different-actor clients are connected", async () => { + mockClients = [ + { + clientId: "other-user-ext", + interfaceId: "chrome-extension", + actorPrincipalId: "user-2", + capabilities: ["host_browser"], + }, + ]; + + const resultPromise = proxy.request( + { cdpMethod: "Page.navigate", cdpParams: { url: "https://a.test" } }, + "session-1", + undefined, + "user-1", + ); + + // Auto-resolution filters out the cross-user candidate, so the + // proxy falls into the existing "no active extension connection" + // rejection — we never broadcast to a different actor's client. + await expect(resultPromise).rejects.toThrow( + "no active extension connection", + ); + expect(getPublishedMessages()).toHaveLength(0); + }); + + test("prefers chrome-extension over macos among same-actor clients", async () => { + mockClients = [ + { + clientId: "macos-client", + interfaceId: "macos", + actorPrincipalId: "user-1", + capabilities: ["host_browser"], + }, + { + clientId: "ext-client", + interfaceId: "chrome-extension", + actorPrincipalId: "user-1", + capabilities: ["host_browser"], + }, + ]; + + const resultPromise = proxy.request( + { cdpMethod: "Page.navigate", cdpParams: { url: "https://a.test" } }, + "session-1", + undefined, + "user-1", + ); + + expect(getPublishedMessages()).toHaveLength(1); + const sent = getPublishedMessages()[0] as Record; + const requestId = sent.requestId as string; + + const pending = pendingInteractions.get(requestId); + expect(pending?.targetClientId).toBe("ext-client"); + + proxy.resolveResult(requestId, { content: "ok", isError: false }); + await resultPromise; + }); + + test("falls back to macOS bridge when no chrome-extension is connected for the caller's actor", async () => { + mockClients = [ + { + clientId: "macos-client", + interfaceId: "macos", + actorPrincipalId: "user-1", + capabilities: ["host_browser"], + }, + ]; + + const resultPromise = proxy.request( + { cdpMethod: "Page.navigate", cdpParams: { url: "https://a.test" } }, + "session-1", + undefined, + "user-1", + ); + + expect(getPublishedMessages()).toHaveLength(1); + const requestId = (getPublishedMessages()[0] as Record) + .requestId as string; + + const pending = pendingInteractions.get(requestId); + expect(pending?.targetClientId).toBe("macos-client"); + expect(pending?.targetActorPrincipalId).toBe("user-1"); + + proxy.resolveResult(requestId, { content: "ok", isError: false }); + await resultPromise; + }); + + test("legacy callers without a sourceActorPrincipalId use the unfiltered fallback path", async () => { + // No mockClients populated — but mockHasConnection is true, so the + // legacy `getPreferredClientByCapability` branch returns its default + // `test-client` candidate. The pending interaction binds to that + // client without an actor, preserving prior single-user behavior. + mockHasConnection = true; + mockClients = []; + + const resultPromise = proxy.request( + { cdpMethod: "Page.navigate", cdpParams: { url: "https://a.test" } }, + "session-1", + // signal omitted + // sourceActorPrincipalId omitted — legacy path + ); + + expect(getPublishedMessages()).toHaveLength(1); + const requestId = (getPublishedMessages()[0] as Record) + .requestId as string; + + const pending = pendingInteractions.get(requestId); + expect(pending?.targetClientId).toBe("test-client"); + expect(pending?.targetActorPrincipalId).toBeUndefined(); + + proxy.resolveResult(requestId, { content: "ok", isError: false }); + await resultPromise; + }); + + test("rejects when caller has actor but no host_browser-capable client is connected for that actor", async () => { + // Same-user filter returns empty even though listClientsByCapability + // would return a non-empty list (because that list is for a + // different actor). The legacy fallback IS NOT consulted in this + // branch — we don't want to silently broadcast to anyone. + mockClients = [ + { + clientId: "other-user-ext", + interfaceId: "chrome-extension", + actorPrincipalId: "user-99", + capabilities: ["host_browser"], + }, + ]; + + const resultPromise = proxy.request( + { cdpMethod: "Page.navigate", cdpParams: { url: "https://a.test" } }, + "session-1", + undefined, + "user-1", + ); + + await expect(resultPromise).rejects.toThrow( + "no active extension connection", + ); + expect(getPublishedMessages()).toHaveLength(0); + }); + }); }); diff --git a/assistant/src/__tests__/host-browser-routes.test.ts b/assistant/src/__tests__/host-browser-routes.test.ts index a0a1c96cdca..d09942cc677 100644 --- a/assistant/src/__tests__/host-browser-routes.test.ts +++ b/assistant/src/__tests__/host-browser-routes.test.ts @@ -14,6 +14,12 @@ mock.module("../config/env.js", () => ({ hasUngatedHttpAuthDisabled: () => false, })); +// local-actor-identity reads the contacts table; stub it to a passthrough so +// the resolver returns whatever header value the test provides. +mock.module("../runtime/local-actor-identity.js", () => ({ + resolveActorPrincipalIdForLocalGuardian: (raw: string | undefined) => raw, +})); + interface ResolveCall { requestId: string; response: { content: string; isError: boolean }; @@ -44,6 +50,7 @@ const pendingInteractions = await import("../runtime/pending-interactions.js"); import { BadRequestError, ConflictError, + ForbiddenError, NotFoundError, } from "../runtime/routes/errors.js"; import { ROUTES } from "../runtime/routes/host-browser-routes.js"; @@ -137,3 +144,314 @@ describe("handleHostBrowserResult", () => { expect(resolveSpy[0].response).toEqual({ content: "", isError: false }); }); }); + +// ── Same-actor guard ─────────────────────────────────────────────────── + +describe("handleHostBrowserResult — same-actor guard", () => { + beforeEach(() => { + pendingInteractions.clear(); + resolveSpy.length = 0; + }); + + // ── Targeted + correct headers → 200 ───────────────────────────────── + + describe("targeted + correct x-vellum-client-id + actor-principal", () => { + test("returns { accepted: true } and resolves the interaction", async () => { + const requestId = "browser-req-targeted-match"; + pendingInteractions.register(requestId, { + conversationId: "conv-1", + kind: "host_browser", + targetClientId: "client-A", + targetActorPrincipalId: "user-1", + }); + + const result = await handleHostBrowserResult({ + body: { requestId, content: "ok", isError: false }, + headers: { + "x-vellum-client-id": "client-A", + "x-vellum-actor-principal-id": "user-1", + }, + }); + + expect(result).toEqual({ accepted: true }); + expect(resolveSpy).toHaveLength(1); + expect(resolveSpy[0].requestId).toBe(requestId); + }); + + test("trims whitespace from x-vellum-client-id before comparing", async () => { + const requestId = "browser-req-targeted-trim"; + pendingInteractions.register(requestId, { + conversationId: "conv-1", + kind: "host_browser", + targetClientId: "client-A", + targetActorPrincipalId: "user-1", + }); + + const result = await handleHostBrowserResult({ + body: { requestId, content: "ok", isError: false }, + headers: { + "x-vellum-client-id": " client-A ", + "x-vellum-actor-principal-id": "user-1", + }, + }); + + expect(result).toEqual({ accepted: true }); + }); + }); + + // ── Targeted + missing x-vellum-client-id → 400 ────────────────────── + + describe("targeted + missing x-vellum-client-id", () => { + test("throws BadRequestError when header is absent", () => { + const requestId = "browser-req-targeted-no-header"; + pendingInteractions.register(requestId, { + conversationId: "conv-1", + kind: "host_browser", + targetClientId: "client-A", + targetActorPrincipalId: "user-1", + }); + + expect(() => + handleHostBrowserResult({ + body: { requestId, content: "ok", isError: false }, + }), + ).toThrow(BadRequestError); + }); + + test("throws BadRequestError when header is whitespace only", () => { + const requestId = "browser-req-targeted-empty-header"; + pendingInteractions.register(requestId, { + conversationId: "conv-1", + kind: "host_browser", + targetClientId: "client-A", + targetActorPrincipalId: "user-1", + }); + + expect(() => + handleHostBrowserResult({ + body: { requestId, content: "ok", isError: false }, + headers: { "x-vellum-client-id": " " }, + }), + ).toThrow(BadRequestError); + }); + + test("interaction is NOT consumed on 400 (still pending)", () => { + const requestId = "browser-req-targeted-no-header-stays"; + pendingInteractions.register(requestId, { + conversationId: "conv-1", + kind: "host_browser", + targetClientId: "client-A", + targetActorPrincipalId: "user-1", + }); + + try { + handleHostBrowserResult({ + body: { requestId, content: "ok", isError: false }, + }); + } catch { + // expected + } + + expect(pendingInteractions.get(requestId)).toBeDefined(); + expect(resolveSpy).toHaveLength(0); + }); + }); + + // ── Targeted + wrong x-vellum-client-id → 403 ──────────────────────── + + describe("targeted + wrong x-vellum-client-id", () => { + test("throws ForbiddenError when client id does not match target", () => { + const requestId = "browser-req-targeted-client-mismatch"; + pendingInteractions.register(requestId, { + conversationId: "conv-1", + kind: "host_browser", + targetClientId: "client-A", + targetActorPrincipalId: "user-1", + }); + + expect(() => + handleHostBrowserResult({ + body: { requestId, content: "ok", isError: false }, + headers: { + "x-vellum-client-id": "client-B", + "x-vellum-actor-principal-id": "user-1", + }, + }), + ).toThrow(ForbiddenError); + }); + + test("ForbiddenError message names both submitting and expected client", () => { + const requestId = "browser-req-targeted-mismatch-msg"; + pendingInteractions.register(requestId, { + conversationId: "conv-1", + kind: "host_browser", + targetClientId: "client-A", + targetActorPrincipalId: "user-1", + }); + + let caught: unknown; + try { + handleHostBrowserResult({ + body: { requestId, content: "ok", isError: false }, + headers: { + "x-vellum-client-id": "client-B", + "x-vellum-actor-principal-id": "user-1", + }, + }); + } catch (e) { + caught = e; + } + + expect(caught).toBeInstanceOf(ForbiddenError); + const msg = (caught as ForbiddenError).message; + expect(msg).toContain("client-A"); + expect(msg).toContain("client-B"); + }); + + test("interaction is NOT consumed on 403 (still pending)", () => { + const requestId = "browser-req-targeted-mismatch-stays"; + pendingInteractions.register(requestId, { + conversationId: "conv-1", + kind: "host_browser", + targetClientId: "client-A", + targetActorPrincipalId: "user-1", + }); + + try { + handleHostBrowserResult({ + body: { requestId, content: "ok", isError: false }, + headers: { + "x-vellum-client-id": "client-B", + "x-vellum-actor-principal-id": "user-1", + }, + }); + } catch { + // expected + } + + expect(pendingInteractions.get(requestId)).toBeDefined(); + expect(resolveSpy).toHaveLength(0); + }); + }); + + // ── Targeted + actor-principal binding (defense-in-depth) ──────────── + + describe("targeted + actor-principal binding", () => { + test("throws ForbiddenError when submitting actor does not match target's actor", () => { + const requestId = "browser-req-actor-mismatch"; + pendingInteractions.register(requestId, { + conversationId: "conv-1", + kind: "host_browser", + targetClientId: "client-A", + targetActorPrincipalId: "user-1", + }); + + expect(() => + handleHostBrowserResult({ + body: { requestId, content: "ok", isError: false }, + headers: { + "x-vellum-client-id": "client-A", + "x-vellum-actor-principal-id": "user-2", + }, + }), + ).toThrow(ForbiddenError); + }); + + test("throws ForbiddenError when submitting actor header is missing", () => { + const requestId = "browser-req-actor-missing"; + pendingInteractions.register(requestId, { + conversationId: "conv-1", + kind: "host_browser", + targetClientId: "client-A", + targetActorPrincipalId: "user-1", + }); + + expect(() => + handleHostBrowserResult({ + body: { requestId, content: "ok", isError: false }, + headers: { "x-vellum-client-id": "client-A" }, + }), + ).toThrow(ForbiddenError); + }); + + test("throws ForbiddenError when target has no captured actor principal", () => { + const requestId = "browser-req-actor-no-target"; + pendingInteractions.register(requestId, { + conversationId: "conv-1", + kind: "host_browser", + targetClientId: "client-A", + // targetActorPrincipalId intentionally omitted + }); + + expect(() => + handleHostBrowserResult({ + body: { requestId, content: "ok", isError: false }, + headers: { + "x-vellum-client-id": "client-A", + "x-vellum-actor-principal-id": "user-1", + }, + }), + ).toThrow(ForbiddenError); + }); + + test("interaction NOT consumed on actor-mismatch 403", () => { + const requestId = "browser-req-actor-mismatch-stays"; + pendingInteractions.register(requestId, { + conversationId: "conv-1", + kind: "host_browser", + targetClientId: "client-A", + targetActorPrincipalId: "user-1", + }); + + try { + handleHostBrowserResult({ + body: { requestId, content: "ok", isError: false }, + headers: { + "x-vellum-client-id": "client-A", + "x-vellum-actor-principal-id": "user-2", + }, + }); + } catch { + // expected + } + + expect(pendingInteractions.get(requestId)).toBeDefined(); + expect(resolveSpy).toHaveLength(0); + }); + }); + + // ── Untargeted (no targetClientId) — regression ─────────────────────── + + describe("untargeted (no targetClientId)", () => { + test("accepts when no headers are provided", async () => { + const requestId = "browser-req-untargeted-no-header"; + pendingInteractions.register(requestId, { + conversationId: "conv-1", + kind: "host_browser", + }); + + const result = await handleHostBrowserResult({ + body: { requestId, content: "ok", isError: false }, + }); + + expect(result).toEqual({ accepted: true }); + expect(resolveSpy).toHaveLength(1); + }); + + test("accepts when header is present (header ignored when untargeted)", async () => { + const requestId = "browser-req-untargeted-with-header"; + pendingInteractions.register(requestId, { + conversationId: "conv-1", + kind: "host_browser", + }); + + const result = await handleHostBrowserResult({ + body: { requestId, content: "ok", isError: false }, + headers: { "x-vellum-client-id": "anything" }, + }); + + expect(result).toEqual({ accepted: true }); + expect(resolveSpy).toHaveLength(1); + }); + }); +}); diff --git a/assistant/src/daemon/host-browser-proxy.ts b/assistant/src/daemon/host-browser-proxy.ts index f5b156dd75d..4a6c398c374 100644 --- a/assistant/src/daemon/host-browser-proxy.ts +++ b/assistant/src/daemon/host-browser-proxy.ts @@ -5,6 +5,7 @@ import { assistantEventHub, broadcastMessage, } from "../runtime/assistant-event-hub.js"; +import { enforceSameActorOrErrorResult } from "../runtime/auth/same-actor.js"; import * as pendingInteractions from "../runtime/pending-interactions.js"; import type { ToolExecutionResult } from "../tools/types.js"; import { AssistantError, ErrorCode } from "../util/errors.js"; @@ -30,6 +31,44 @@ const HOST_BROWSER_INTERFACE_PREFERENCE: InterfaceId[] = [ "macos", ]; +/** + * Pick the host_browser-capable client to dispatch to. + * + * When a `sourceActorPrincipalId` is supplied, candidate clients are + * filtered down to those owned by the same actor before applying the + * interface-preference order. Returns `undefined` when no same-actor + * client is connected; the caller surfaces this as the existing + * "no active extension connection" rejection. + * + * When `sourceActorPrincipalId` is undefined (legacy callers without a + * resolved actor identity), falls through to the prior behavior so the + * registry singleton continues to work as before. + */ +function resolveTargetClient(sourceActorPrincipalId: string | undefined) { + if (sourceActorPrincipalId == null) { + return assistantEventHub.getPreferredClientByCapability( + "host_browser", + HOST_BROWSER_INTERFACE_PREFERENCE, + ); + } + + const sameActorClients = assistantEventHub + .listClientsByCapability("host_browser") + .filter((c) => c.actorPrincipalId === sourceActorPrincipalId); + if (sameActorClients.length === 0) return undefined; + + // Stable sort by interface preference; lastActiveAt is the implicit + // tiebreaker because listClientsByCapability already returns clients + // in lastActiveAt-desc order. + return [...sameActorClients].sort((a, b) => { + const ai = HOST_BROWSER_INTERFACE_PREFERENCE.indexOf(a.interfaceId); + const bi = HOST_BROWSER_INTERFACE_PREFERENCE.indexOf(b.interfaceId); + const ea = ai === -1 ? HOST_BROWSER_INTERFACE_PREFERENCE.length : ai; + const eb = bi === -1 ? HOST_BROWSER_INTERFACE_PREFERENCE.length : bi; + return ea - eb; + })[0]; +} + export class HostBrowserProxy { private static _instance: HostBrowserProxy | null = null; @@ -84,15 +123,57 @@ export class HostBrowserProxy { return assistantEventHub.listClientsByInterface("chrome-extension").length > 0; } + /** + * Send a host_browser request to the connected extension/macOS bridge. + * + * When `sourceActorPrincipalId` is supplied, the proxy refuses to dispatch + * to a client owned by a different actor — same-user enforcement is the + * authoritative gate against routing one actor's CDP request onto another + * actor's connected extension. The auto-resolved target's `clientId` and + * `actorPrincipalId` are then persisted alongside the pending interaction + * so that the result-route's same-actor check can verify the submitting + * client at result time. + * + * When `sourceActorPrincipalId` is undefined (legacy/internal flows with + * no resolved actor identity), falls back to interface-preference + * resolution without an actor filter, preserving prior behavior. + */ request( input: HostBrowserInput, conversationId: string, signal?: AbortSignal, + sourceActorPrincipalId?: string, ): Promise { if (signal?.aborted) { return Promise.resolve({ content: "Aborted", isError: true }); } + // Resolve the target client up front so we can persist the actor binding + // alongside the pending interaction registration. Same shape as + // host-cu-proxy: the result-route same-actor check compares the + // submitting client's actor against this captured value. + const preferredClient = resolveTargetClient(sourceActorPrincipalId); + + // Same-user enforcement: when the caller's actor is known, refuse to + // dispatch to a client owned by a different actor. This covers the + // cross-client exposure case where a web/iOS turn for actor A would + // otherwise auto-resolve to actor B's connected extension. + if ( + sourceActorPrincipalId != null && + preferredClient != null && + preferredClient.actorPrincipalId !== sourceActorPrincipalId + ) { + const rejection = enforceSameActorOrErrorResult({ + hub: assistantEventHub, + sourceActorPrincipalId, + targetClientId: preferredClient.clientId, + op: "host_browser", + }); + if (rejection) return Promise.resolve(rejection); + } + + const targetClientId = preferredClient?.clientId; + const targetActorPrincipalId = preferredClient?.actorPrincipalId; const requestId = uuid(); return new Promise((resolve, reject) => { @@ -135,6 +216,8 @@ export class HostBrowserProxy { pendingInteractions.register(requestId, { conversationId, kind: "host_browser", + targetClientId, + targetActorPrincipalId, rpcResolve: resolve as (v: unknown) => void, rpcReject: reject, timer, @@ -142,10 +225,6 @@ export class HostBrowserProxy { }); try { - const preferredClient = assistantEventHub.getPreferredClientByCapability( - "host_browser", - HOST_BROWSER_INTERFACE_PREFERENCE, - ); if (!preferredClient) { pendingInteractions.resolve(requestId); reject( diff --git a/assistant/src/runtime/auth/same-actor.ts b/assistant/src/runtime/auth/same-actor.ts index ad1b9cd5bb9..b069521bd0a 100644 --- a/assistant/src/runtime/auth/same-actor.ts +++ b/assistant/src/runtime/auth/same-actor.ts @@ -45,6 +45,7 @@ export type SameActorOp = | "host_bash" | "host_file" | "host_cu" + | "host_browser" | "host_transfer"; /** diff --git a/assistant/src/runtime/routes/host-browser-routes.ts b/assistant/src/runtime/routes/host-browser-routes.ts index d3a8aeb0d35..fef3353b1bc 100644 --- a/assistant/src/runtime/routes/host-browser-routes.ts +++ b/assistant/src/runtime/routes/host-browser-routes.ts @@ -11,8 +11,18 @@ import { publishCdpEvent, } from "../../browser-session/events.js"; import { HostBrowserProxy } from "../../daemon/host-browser-proxy.js"; +import { + enforceSameActorOrThrow, + SAME_ACTOR_FORBIDDEN_DESCRIPTION, +} from "../auth/same-actor.js"; +import { resolveActorPrincipalIdForLocalGuardian } from "../local-actor-identity.js"; import * as pendingInteractions from "../pending-interactions.js"; -import { BadRequestError, ConflictError, NotFoundError } from "./errors.js"; +import { + BadRequestError, + ConflictError, + ForbiddenError, + NotFoundError, +} from "./errors.js"; import type { RouteDefinition, RouteHandlerArgs } from "./types.js"; /** @@ -30,8 +40,8 @@ export type HostBrowserResultResolution = | { ok: true } | { ok: false; - code: "BAD_REQUEST" | "NOT_FOUND" | "CONFLICT"; - status: 400 | 404 | 409; + code: "BAD_REQUEST" | "FORBIDDEN" | "NOT_FOUND" | "CONFLICT"; + status: 400 | 403 | 404 | 409; message: string; }; @@ -40,22 +50,33 @@ export type HostBrowserResultResolution = * the pending interaction by requestId, validates its kind is * `host_browser`, and forwards the response to the owning conversation. * + * Same-actor binding: when the pending interaction has a + * `targetClientId` (set by the proxy at request time when an actor is + * known), the submitting client must (a) identify itself via + * `x-vellum-client-id` matching the captured target, and (b) the + * submitting actor's principal must match the actor captured for that + * client at registration time. Mirrors the host-cu / host-bash result + * routes. + * * NOTE: The WebSocket `host_browser_result` frame path does NOT go * through this function — it is handled by `HostBrowserProxy.resolveResult` * directly, which only consults `pendingInteractions` and does not * currently perform a kind check. That asymmetry is pre-existing; if * the WS path is ever opened to less-trusted clients, it should adopt - * the same kind-check guard added here. + * the same kind-check + actor-binding guards added here. * * This function does NOT perform auth — callers are expected to have * already authenticated the caller (the HTTP route uses * `requireBoundGuardian`). */ -export function resolveHostBrowserResultByRequestId(frame: { - requestId?: unknown; - content?: unknown; - isError?: unknown; -}): HostBrowserResultResolution { +export function resolveHostBrowserResultByRequestId( + frame: { + requestId?: unknown; + content?: unknown; + isError?: unknown; + }, + headers?: Record, +): HostBrowserResultResolution { const { requestId, content, isError } = frame; if (!requestId || typeof requestId !== "string") { @@ -86,6 +107,55 @@ export function resolveHostBrowserResultByRequestId(frame: { }; } + // Validate submitting client matches the targeted client (if any). + if (peeked.targetClientId != null) { + const headerMap = headers ?? {}; + const submittingClientId = + headerMap["x-vellum-client-id"]?.trim() || undefined; + if (!submittingClientId) { + return { + ok: false, + code: "BAD_REQUEST", + status: 400, + message: + "x-vellum-client-id header is missing for a targeted host browser request.", + }; + } + if (submittingClientId !== peeked.targetClientId) { + return { + ok: false, + code: "FORBIDDEN", + status: 403, + message: `Client "${submittingClientId}" is not the target for this request (expected "${peeked.targetClientId}"). The targeted client must submit the result.`, + }; + } + + // Defense-in-depth: require the submitting actor's principal id to match + // the actor principal id captured when the target client opened its SSE + // stream. This prevents a different authenticated user with knowledge of + // both the requestId and target clientId from submitting a result on + // behalf of the targeted client. + const submittingActorPrincipalId = resolveActorPrincipalIdForLocalGuardian( + headerMap["x-vellum-actor-principal-id"]?.trim() || undefined, + ); + try { + enforceSameActorOrThrow({ + sourceActorPrincipalId: submittingActorPrincipalId, + targetActorPrincipalId: peeked.targetActorPrincipalId, + targetClientId: peeked.targetClientId, + op: "host_browser", + }); + } catch (err) { + // enforceSameActorOrThrow throws ForbiddenError on rejection. + return { + ok: false, + code: "FORBIDDEN", + status: 403, + message: err instanceof Error ? err.message : "Same-actor check failed", + }; + } + } + const normalizedContent = typeof content === "string" ? content : ""; const normalizedIsError = typeof isError === "boolean" ? isError : false; @@ -188,13 +258,18 @@ export function resolveHostBrowserSessionInvalidated(frame: { // POST /v1/host-browser-result // --------------------------------------------------------------------------- -function handleHostBrowserResult({ body }: RouteHandlerArgs) { +function handleHostBrowserResult({ body, headers }: RouteHandlerArgs) { if (!body || typeof body !== "object") { throw new BadRequestError("Request body is required"); } - const resolution = resolveHostBrowserResultByRequestId(body); + const resolution = resolveHostBrowserResultByRequestId( + body, + headers as Record | undefined, + ); if (!resolution.ok) { + if (resolution.code === "FORBIDDEN") + throw new ForbiddenError(resolution.message); if (resolution.code === "NOT_FOUND") throw new NotFoundError(resolution.message); if (resolution.code === "CONFLICT") @@ -260,6 +335,22 @@ export const ROUTES: RouteDefinition[] = [ responseBody: z.object({ accepted: z.boolean(), }), + additionalResponses: { + "400": { + description: + "x-vellum-client-id header is missing for a targeted host browser request.", + }, + "403": { + description: SAME_ACTOR_FORBIDDEN_DESCRIPTION, + }, + "404": { + description: "No pending browser request for the given requestId.", + }, + "409": { + description: + "Pending interaction kind is not host_browser (mismatched proxy ID space).", + }, + }, handler: handleHostBrowserResult, }, { diff --git a/assistant/src/tools/browser/cdp-client/__tests__/factory.test.ts b/assistant/src/tools/browser/cdp-client/__tests__/factory.test.ts index c6fc7439cf6..38f2566199c 100644 --- a/assistant/src/tools/browser/cdp-client/__tests__/factory.test.ts +++ b/assistant/src/tools/browser/cdp-client/__tests__/factory.test.ts @@ -78,13 +78,26 @@ let desktopAutoConfig = { enabled: true, cooldownMs: 30_000 }; const logWarnCalls: Array<{ args: unknown[] }> = []; const logDebugCalls: Array<{ args: unknown[] }> = []; +// Spread-real-module pattern: bun's `mock.module` is process-global, so a +// factory that returns ONLY `{ createExtensionCdpClient }` would clobber +// the `ExtensionCdpClient` class export for every later test file that +// imports from this module path (e.g. extension-cdp-client.test.ts). We +// snapshot the real exports first and override only the symbols this +// suite stubs out. +import * as realCdpInspectClient from "../cdp-inspect-client.js"; +import * as realExtensionCdpClient from "../extension-cdp-client.js"; +import * as realLocalCdpClient from "../local-cdp-client.js"; + mock.module("../extension-cdp-client.js", () => ({ + ...realExtensionCdpClient, createExtensionCdpClient: createExtensionCdpClientMock, })); mock.module("../local-cdp-client.js", () => ({ + ...realLocalCdpClient, createLocalCdpClient: createLocalCdpClientMock, })); mock.module("../cdp-inspect-client.js", () => ({ + ...realCdpInspectClient, createCdpInspectClient: createCdpInspectClientMock, })); mock.module("../../../../config/loader.js", () => ({ @@ -235,9 +248,15 @@ describe("getCdpClient", () => { ); expect(result).toEqual({ ok: true, via: "extension" }); expect(createExtensionCdpClientMock).toHaveBeenCalledTimes(1); + // Call signature includes optional cdpSessionId and sourceActorPrincipalId + // (both undefined here — no pinned session id, no actor binding in this + // legacy ctx). The `sourceActorPrincipalId` param exists so the proxy + // can refuse cross-user dispatch under cross-client exposure. expect(createExtensionCdpClientMock).toHaveBeenCalledWith( fakeProxy, "test-convo", + undefined, + undefined, ); expect(createLocalCdpClientMock).not.toHaveBeenCalled(); expect(createCdpInspectClientMock).not.toHaveBeenCalled(); @@ -658,6 +677,31 @@ describe("getCdpClient", () => { await client.send("Page.navigate"); expect(createCdpInspectClientMock).toHaveBeenCalledTimes(1); }); + + test("threads sourceActorPrincipalId from ToolContext into createExtensionCdpClient", async () => { + // The proxy uses sourceActorPrincipalId to refuse cross-user dispatch + // when host_browser is exposed cross-client (web/iOS turn → connected + // extension/macOS bridge). The factory must thread the value from the + // ToolContext through to ExtensionCdpClient on every candidate-list path + // so the actor identity reaches the proxy at request time. + const fakeProxy = makeAvailableProxy(); + mockSingletonProxy = fakeProxy; + const ctx = makeContext({ + conversationId: "actor-bound", + sourceActorPrincipalId: "user-actor-1", + }); + + const client = getCdpClient(ctx); + await client.send("Page.navigate"); + + expect(createExtensionCdpClientMock).toHaveBeenCalledTimes(1); + expect(createExtensionCdpClientMock).toHaveBeenCalledWith( + fakeProxy, + "actor-bound", + undefined, + "user-actor-1", + ); + }); }); // ── buildCandidateList tests ───────────────────────────────────────────── diff --git a/assistant/src/tools/browser/cdp-client/extension-cdp-client.ts b/assistant/src/tools/browser/cdp-client/extension-cdp-client.ts index 320bba4fffe..f4af7873788 100644 --- a/assistant/src/tools/browser/cdp-client/extension-cdp-client.ts +++ b/assistant/src/tools/browser/cdp-client/extension-cdp-client.ts @@ -44,6 +44,13 @@ export class ExtensionCdpClient implements ScopedCdpClient { private readonly proxy: HostBrowserProxy, public readonly conversationId: string, private readonly cdpSessionId?: string, + /** + * Caller's actor principal id. When provided, the proxy will refuse to + * dispatch this CDP command to a host_browser-capable client owned by a + * different actor — closing the cross-client exposure path's same-user + * boundary. + */ + private readonly sourceActorPrincipalId?: string, ) {} async send( @@ -74,6 +81,7 @@ export class ExtensionCdpClient implements ScopedCdpClient { }, this.conversationId, signal, + this.sourceActorPrincipalId, ); } catch (err) { throw new CdpError( @@ -194,6 +202,12 @@ export function createExtensionCdpClient( proxy: HostBrowserProxy, conversationId: string, cdpSessionId?: string, + sourceActorPrincipalId?: string, ): ExtensionCdpClient { - return new ExtensionCdpClient(proxy, conversationId, cdpSessionId); + return new ExtensionCdpClient( + proxy, + conversationId, + cdpSessionId, + sourceActorPrincipalId, + ); } diff --git a/assistant/src/tools/browser/cdp-client/factory.ts b/assistant/src/tools/browser/cdp-client/factory.ts index e2b3b7410ee..32b661cf8c0 100644 --- a/assistant/src/tools/browser/cdp-client/factory.ts +++ b/assistant/src/tools/browser/cdp-client/factory.ts @@ -183,7 +183,7 @@ export function buildPinnedCandidateList( context: ToolContext, mode: Exclude, ): BackendCandidate[] { - const { conversationId } = context; + const { conversationId, sourceActorPrincipalId } = context; switch (mode) { case "extension": { @@ -213,6 +213,8 @@ export function buildPinnedCandidateList( const client = createExtensionCdpClient( hostBrowserProxy, conversationId, + undefined, + sourceActorPrincipalId, ); const backend = createExtensionBackend({ isAvailable: () => true, @@ -287,7 +289,7 @@ export function buildPinnedCandidateList( * Exported for testing. */ export function buildCandidateList(context: ToolContext): BackendCandidate[] { - const { conversationId } = context; + const { conversationId, sourceActorPrincipalId } = context; const candidates: BackendCandidate[] = []; const hostBrowserProxy = HostBrowserProxy.instance; @@ -300,6 +302,8 @@ export function buildCandidateList(context: ToolContext): BackendCandidate[] { const client = createExtensionCdpClient( hostBrowserProxy, conversationId, + undefined, + sourceActorPrincipalId, ); const backend = createExtensionBackend({ isAvailable: () => true, diff --git a/clients/chrome-extension/background/worker.ts b/clients/chrome-extension/background/worker.ts index 7b38032f709..5ba89b21d3c 100644 --- a/clients/chrome-extension/background/worker.ts +++ b/clients/chrome-extension/background/worker.ts @@ -42,6 +42,7 @@ import { import { SseConnection, type SseMode } from "./sse-connection.js"; import { fetchAssistants } from "./cloud-api.js"; import { appendEvent, clearEventLog, getEventLog, getOperations, getOperationById, recordRequest, recordResponse } from "./event-log.js"; +import { getClientId } from "./client-identity.js"; import { startCloudLogin, getStoredSession, @@ -383,6 +384,10 @@ async function dispatchHostBrowserResult( const headers: Record = { "content-type": "application/json", + // Identifies this extension to the daemon's actor-binding check. + // The daemon validates this matches the client recorded at request + // time before resolving the pending host_browser interaction. + "X-Vellum-Client-Id": await getClientId(), }; if (mode.kind === "vellum-cloud") { if (mode.token) { @@ -416,7 +421,10 @@ async function dispatchHostBrowserResult( if (userMode !== "cloud") { const gatewayUrl = await getStoredGatewayUrl(); try { - const fallbackHeaders: Record = { "content-type": "application/json" }; + const fallbackHeaders: Record = { + "content-type": "application/json", + "X-Vellum-Client-Id": await getClientId(), + }; if (selfHostedPairToken) { fallbackHeaders["authorization"] = `Bearer ${selfHostedPairToken}`; } diff --git a/clients/shared/Network/HostProxyClient.swift b/clients/shared/Network/HostProxyClient.swift index 88b77c90762..542e201c016 100644 --- a/clients/shared/Network/HostProxyClient.swift +++ b/clients/shared/Network/HostProxyClient.swift @@ -112,9 +112,14 @@ public struct HostProxyClient: HostProxyClientProtocol { public func postBrowserResult(_ result: HostBrowserResultPayload) async -> Bool { do { let body = try JSONEncoder().encode(result) + // Attach X-Vellum-Client-Id so the daemon can verify the submitting + // client matches the targeted client recorded at request time. + // Without this header the daemon will reject targeted host_browser + // results with 400. Mirrors postBashResult / postCuResult / etc. let response = try await GatewayHTTPClient.post( path: "host-browser-result", body: body, + extraHeaders: ["X-Vellum-Client-Id": DeviceIdStore.getOrCreate()], timeout: 30 ) guard response.isSuccess else { From b94debab64c2b905ac9b4bc7e7a30ebc391cfb13 Mon Sep 17 00:00:00 2001 From: credence-the-bot Date: Wed, 6 May 2026 20:36:41 +0000 Subject: [PATCH 3/3] chore(openapi): regenerate openapi.yaml for host-browser-result actor-binding responses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- assistant/openapi.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/assistant/openapi.yaml b/assistant/openapi.yaml index d3ca5e046d3..187b98475d9 100644 --- a/assistant/openapi.yaml +++ b/assistant/openapi.yaml @@ -6498,6 +6498,16 @@ paths: required: - accepted additionalProperties: false + "400": + description: x-vellum-client-id header is missing for a targeted host browser request. + "403": + description: + Submitting client does not match the targeted client, or the submitting actor's principal does not match + the target client's actor. + "404": + description: No pending browser request for the given requestId. + "409": + description: Pending interaction kind is not host_browser (mismatched proxy ID space). requestBody: required: true content: