Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
163 changes: 160 additions & 3 deletions assistant/src/__tests__/host-browser-proxy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -394,9 +394,9 @@ describe("HostBrowserProxy", () => {
// ---------------------------------------------------------------------------
// 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:
// When the caller does not supply `targetClientId`, the proxy auto-resolves
// using `resolveTargetClient(sourceActorPrincipalId)` which filters clients
// to the same actor before applying the interface-preference order:
//
// 1. `resolveTargetClient(sourceActorPrincipalId)` filters candidate
// clients to those owned by the caller's actor before applying the
Expand Down Expand Up @@ -586,4 +586,161 @@ describe("HostBrowserProxy", () => {
expect(getPublishedMessages()).toHaveLength(0);
});
});

// ---------------------------------------------------------------------------
// Explicit targetClientId routing
//
// When `targetClientId` is supplied, the proxy skips the interface-preference
// sort and routes directly to the named client (subject to the same-actor
// enforcement that runs on all host-proxy requests).
// ---------------------------------------------------------------------------

describe("explicit targetClientId routing", () => {
test("routes to the named client and persists targetClientId in pending state", 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"],
},
];

// Explicitly target the macOS client even though chrome-extension
// would win under normal interface-preference ordering.
const resultPromise = proxy.request(
{ cdpMethod: "Page.navigate", cdpParams: { url: "https://a.test" } },
"session-1",
undefined,
"user-1",
"macos-client",
);

expect(getPublishedMessages()).toHaveLength(1);
const sent = getPublishedMessages()[0] as Record<string, unknown>;
const requestId = sent.requestId as string;

const pending = pendingInteractions.get(requestId);
expect(pending?.targetClientId).toBe("macos-client");

proxy.resolveResult(requestId, { content: "ok", isError: false });
const result = await resultPromise;
expect(result.isError).toBe(false);
});

test("rejects when targetClientId does not match any connected client", 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",
"nonexistent-client",
);

await expect(resultPromise).rejects.toThrow(
"no active extension connection",
);
expect(getPublishedMessages()).toHaveLength(0);
});

test("rejects when targetClientId points to a client without host_browser capability", async () => {
// The client exists but is not in the host_browser roster, so
// listClientsByCapability("host_browser") does not return it.
mockClients = [
{
clientId: "ext-client",
interfaceId: "chrome-extension",
actorPrincipalId: "user-1",
capabilities: ["host_bash"],
},
];

const resultPromise = proxy.request(
{ cdpMethod: "Page.navigate", cdpParams: { url: "https://a.test" } },
"session-1",
undefined,
"user-1",
"ext-client",
);

await expect(resultPromise).rejects.toThrow(
"no active extension connection",
);
expect(getPublishedMessages()).toHaveLength(0);
});

test("same-actor check rejects targetClientId that belongs to a different actor", async () => {
mockClients = [
{
clientId: "other-user-ext",
interfaceId: "chrome-extension",
actorPrincipalId: "user-2",
capabilities: ["host_browser"],
},
];

// actor user-1 explicitly targets user-2's client — same-actor gate
// should fire and return an isError result (not reject the promise).
const result = await proxy.request(
{ cdpMethod: "Page.navigate", cdpParams: { url: "https://a.test" } },
"session-1",
undefined,
"user-1",
"other-user-ext",
);

expect(result.isError).toBe(true);
expect(result.content).toContain("Submitting actor does not match");
expect(getPublishedMessages()).toHaveLength(0);
});

test("no targetClientId falls back to interface-preference order (regression guard)", 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"],
},
];

// No targetClientId — should auto-resolve to chrome-extension (higher priority).
const resultPromise = proxy.request(
{ cdpMethod: "Page.navigate", cdpParams: { url: "https://a.test" } },
"session-1",
undefined,
"user-1",
// targetClientId omitted
);

const requestId = (getPublishedMessages()[0] as Record<string, unknown>)
.requestId as string;
const pending = pendingInteractions.get(requestId);
expect(pending?.targetClientId).toBe("ext-client");

proxy.resolveResult(requestId, { content: "ok", isError: false });
await resultPromise;
});
});
});
25 changes: 19 additions & 6 deletions assistant/src/cli/commands/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,22 +164,31 @@ function buildSubcommand(parent: Command, meta: BrowserOperationMeta): void {
session?: string;
json?: boolean;
browserMode?: string;
targetClientId?: string;
};
const sessionId = parentOpts.session ?? "default";
const jsonMode = parentOpts.json ?? false;
const conversationId = resolveContextConversationId();

// Map Commander camelCase options back to snake_case input keys,
// filtering out parent-level options (session, json, browserMode)
// and screenshot ergonomics (output).
// filtering out parent-level options (session, json, browserMode,
// targetClientId) and screenshot ergonomics (output).
const input: Record<string, unknown> = {};
const excludeKeys = new Set(["session", "json", "output", "browserMode"]);

// Inject parent-level --browser-mode into the operation input so
// the backend receives the mode override for backend pinning.
const excludeKeys = new Set([
"session",
"json",
"output",
"browserMode",
"targetClientId",
]);

// Inject parent-level flags into the operation input.
if (parentOpts.browserMode) {
input.browser_mode = parentOpts.browserMode;
}
if (parentOpts.targetClientId) {
input.target_client_id = parentOpts.targetClientId;
}

for (const [key, value] of Object.entries(opts)) {
if (excludeKeys.has(key)) continue;
Expand Down Expand Up @@ -362,6 +371,10 @@ export function registerBrowserCommand(program: Command): void {
"--browser-mode <mode>",
"Browser backend to use. Overrides automatic selection.",
).choices([...BROWSER_MODES]),
)
.option(
"--target-client-id <id>",
"Route browser operations to a specific client. Obtain IDs from `assistant clients list --capability host_browser`.",
);

browser.addHelpText(
Expand Down
45 changes: 32 additions & 13 deletions assistant/src/daemon/host-browser-proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,30 @@ const HOST_BROWSER_INTERFACE_PREFERENCE: InterfaceId[] = [
/**
* 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 `targetClientId` is supplied, the client with that id is looked
* up directly in the `host_browser`-capable roster. The same-actor check
* in `request()` still runs on the returned client when
* `sourceActorPrincipalId` is present.
*
* 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.
* When `sourceActorPrincipalId` is supplied (and no explicit target),
* 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 neither is supplied (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) {
function resolveTargetClient(
sourceActorPrincipalId: string | undefined,
targetClientId?: string,
) {
if (targetClientId != null) {
const clients = assistantEventHub.listClientsByCapability("host_browser");
return clients.find((c) => c.clientId === targetClientId);
}

if (sourceActorPrincipalId == null) {
return assistantEventHub.getPreferredClientByCapability(
"host_browser",
Expand Down Expand Up @@ -126,10 +139,15 @@ export class HostBrowserProxy {
/**
* Send a host_browser request to the connected extension/macOS bridge.
*
* When `targetClientId` is supplied, the proxy dispatches to that specific
* client (subject to the `host_browser` capability check and the same-actor
* gate below). This mirrors the `target_client_id` pattern on `host_bash`,
* `host_file_*`, and `host_cu`.
*
* 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
* actor's connected extension. The 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.
Expand All @@ -143,6 +161,7 @@ export class HostBrowserProxy {
conversationId: string,
signal?: AbortSignal,
sourceActorPrincipalId?: string,
targetClientId?: string,
): Promise<ToolExecutionResult> {
if (signal?.aborted) {
return Promise.resolve({ content: "Aborted", isError: true });
Expand All @@ -152,7 +171,7 @@ export class HostBrowserProxy {
// 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);
const preferredClient = resolveTargetClient(sourceActorPrincipalId, targetClientId);

// Same-user enforcement: when the caller's actor is known, refuse to
// dispatch to a client owned by a different actor. This covers the
Expand All @@ -172,7 +191,7 @@ export class HostBrowserProxy {
if (rejection) return Promise.resolve(rejection);
}

const targetClientId = preferredClient?.clientId;
const resolvedClientId = preferredClient?.clientId;
const targetActorPrincipalId = preferredClient?.actorPrincipalId;
const requestId = uuid();

Expand Down Expand Up @@ -216,7 +235,7 @@ export class HostBrowserProxy {
pendingInteractions.register(requestId, {
conversationId,
kind: "host_browser",
targetClientId,
targetClientId: resolvedClientId,
targetActorPrincipalId,
rpcResolve: resolve as (v: unknown) => void,
rpcReject: reject,
Expand Down
Loading
Loading