diff --git a/assistant/openapi.yaml b/assistant/openapi.yaml index cc106c19ee1..b39a9ba488d 100644 --- a/assistant/openapi.yaml +++ b/assistant/openapi.yaml @@ -5960,6 +5960,10 @@ paths: required: - accepted additionalProperties: false + "400": + description: x-vellum-client-id header is missing for a targeted host bash request. + "403": + description: Submitting client does not match the targeted client for this request. requestBody: required: true content: diff --git a/assistant/src/__tests__/assistant-event-hub-machine-name.test.ts b/assistant/src/__tests__/assistant-event-hub-machine-name.test.ts new file mode 100644 index 00000000000..3c35b4bba11 --- /dev/null +++ b/assistant/src/__tests__/assistant-event-hub-machine-name.test.ts @@ -0,0 +1,146 @@ +/** + * Tests for machineName field in AssistantEventHub client registration. + * + * Validates: + * - subscribing with machineName set results in listClients() returning the name + * - subscribing without machineName results in listClients() returning undefined + */ +import { describe, expect, mock, test } from "bun:test"; + +mock.module("../util/logger.js", () => ({ + getLogger: () => + new Proxy({} as Record, { + get: () => () => {}, + }), +})); + +mock.module("../config/loader.js", () => ({ + getConfig: () => ({ + ui: {}, + model: "test", + provider: "test", + memory: { enabled: false }, + rateLimit: { maxRequestsPerMinute: 0 }, + secretDetection: { enabled: false }, + }), +})); + +import { initializeDb } from "../memory/db-init.js"; +import { AssistantEventHub } from "../runtime/assistant-event-hub.js"; +import { handleSubscribeAssistantEvents } from "../runtime/routes/events-routes.js"; + +initializeDb(); + +describe("AssistantEventHub — machineName", () => { + test("subscribing with machineName returns it from listClients()", () => { + const ac = new AbortController(); + const hub = new AssistantEventHub(); + + handleSubscribeAssistantEvents( + { + headers: { + "x-vellum-client-id": "client-with-name-001", + "x-vellum-interface-id": "macos", + "x-vellum-machine-name": "alice-mbp.local", + }, + abortSignal: ac.signal, + }, + { hub }, + ); + + const clients = hub.listClients(); + const entry = clients.find((c) => c.clientId === "client-with-name-001"); + expect(entry).toBeDefined(); + expect(entry?.machineName).toBe("alice-mbp.local"); + + ac.abort(); + }); + + test("subscribing without machineName returns undefined from listClients()", () => { + const ac = new AbortController(); + const hub = new AssistantEventHub(); + + handleSubscribeAssistantEvents( + { + headers: { + "x-vellum-client-id": "client-without-name-001", + "x-vellum-interface-id": "macos", + }, + abortSignal: ac.signal, + }, + { hub }, + ); + + const clients = hub.listClients(); + const entry = clients.find( + (c) => c.clientId === "client-without-name-001", + ); + expect(entry).toBeDefined(); + expect(entry?.machineName).toBeUndefined(); + + ac.abort(); + }); + + test("machineName is trimmed when set", () => { + const ac = new AbortController(); + const hub = new AssistantEventHub(); + + handleSubscribeAssistantEvents( + { + headers: { + "x-vellum-client-id": "client-with-trimmed-name-001", + "x-vellum-interface-id": "macos", + "x-vellum-machine-name": " bob-mbp.local ", + }, + abortSignal: ac.signal, + }, + { hub }, + ); + + const clients = hub.listClients(); + const entry = clients.find( + (c) => c.clientId === "client-with-trimmed-name-001", + ); + expect(entry).toBeDefined(); + expect(entry?.machineName).toBe("bob-mbp.local"); + + ac.abort(); + }); + + test("direct hub subscribe with machineName returns it from listClients()", () => { + const hub = new AssistantEventHub(); + + hub.subscribe({ + type: "client", + clientId: "direct-client-001", + interfaceId: "macos", + capabilities: ["host_bash"], + machineName: "charlie-mbp.local", + callback: () => {}, + }); + + const clients = hub.listClients(); + const entry = clients.find((c) => c.clientId === "direct-client-001"); + expect(entry).toBeDefined(); + expect(entry?.machineName).toBe("charlie-mbp.local"); + }); + + test("direct hub subscribe without machineName returns undefined from listClients()", () => { + const hub = new AssistantEventHub(); + + hub.subscribe({ + type: "client", + clientId: "direct-client-no-name-001", + interfaceId: "macos", + capabilities: ["host_bash"], + callback: () => {}, + }); + + const clients = hub.listClients(); + const entry = clients.find( + (c) => c.clientId === "direct-client-no-name-001", + ); + expect(entry).toBeDefined(); + expect(entry?.machineName).toBeUndefined(); + }); +}); diff --git a/assistant/src/__tests__/assistant-event-hub-targeted.test.ts b/assistant/src/__tests__/assistant-event-hub-targeted.test.ts new file mode 100644 index 00000000000..1d02ea47c23 --- /dev/null +++ b/assistant/src/__tests__/assistant-event-hub-targeted.test.ts @@ -0,0 +1,257 @@ +/** + * Tests for targeted delivery in AssistantEventHub. + * + * Validates: + * - hub.publish(event, { targetClientId }) delivers only to the named client, + * even when that subscriber's filter.conversationId doesn't match. + * - hub.publish(event, { targetClientId }) does NOT deliver to other clients. + * - hub.publish(event, { targetClientId, targetCapability }) skips subscribers + * that don't have the required capability. + * - hub.publish(event, { targetCapability }) (untargeted) still applies + * conversation scoping normally. + * - getClientById() returns the correct entry or undefined. + */ +import { describe, expect, test } from "bun:test"; + +import type { AssistantEvent } from "../runtime/assistant-event.js"; +import { AssistantEventHub } from "../runtime/assistant-event-hub.js"; + +function makeEvent(overrides: Partial = {}): AssistantEvent { + return { + id: "evt_test", + conversationId: "sess_web", + emittedAt: "2026-05-03T00:00:00.000Z", + message: { + type: "assistant_text_delta", + conversationId: "sess_web", + text: "hi", + }, + ...overrides, + }; +} + +// ── Targeted delivery ───────────────────────────────────────────────────────── + +describe("AssistantEventHub — targeted delivery (targetClientId)", () => { + test("delivers only to the named client, bypassing conversation filter", async () => { + const hub = new AssistantEventHub(); + const receivedA: AssistantEvent[] = []; + const receivedB: AssistantEvent[] = []; + + // client-a is subscribed to "sess_macos" — different from the event's "sess_web" + hub.subscribe({ + type: "client", + clientId: "client-a", + interfaceId: "macos", + capabilities: ["host_bash"], + filter: { conversationId: "sess_macos" }, + callback: (e) => { + receivedA.push(e); + }, + }); + + // client-b is subscribed to "sess_web" — same as the event's conversationId + hub.subscribe({ + type: "client", + clientId: "client-b", + interfaceId: "macos", + capabilities: ["host_bash"], + filter: { conversationId: "sess_web" }, + callback: (e) => { + receivedB.push(e); + }, + }); + + // Target client-a specifically — should bypass its conversation filter + await hub.publish(makeEvent({ conversationId: "sess_web" }), { + targetClientId: "client-a", + }); + + // client-a receives it despite mismatched conversationId + expect(receivedA).toHaveLength(1); + // client-b does NOT receive it even though its conversationId matches + expect(receivedB).toHaveLength(0); + }); + + test("does not deliver to a client with a different clientId", async () => { + const hub = new AssistantEventHub(); + const receivedA: AssistantEvent[] = []; + const receivedB: AssistantEvent[] = []; + + hub.subscribe({ + type: "client", + clientId: "client-a", + interfaceId: "macos", + capabilities: ["host_bash"], + callback: (e) => { + receivedA.push(e); + }, + }); + + hub.subscribe({ + type: "client", + clientId: "client-b", + interfaceId: "macos", + capabilities: ["host_bash"], + callback: (e) => { + receivedB.push(e); + }, + }); + + await hub.publish(makeEvent(), { targetClientId: "client-a" }); + + expect(receivedA).toHaveLength(1); + expect(receivedB).toHaveLength(0); + }); + + test("targeted delivery with wrong capability does not deliver", async () => { + const hub = new AssistantEventHub(); + const receivedA: AssistantEvent[] = []; + + // client-a only has host_file capability, NOT host_bash + hub.subscribe({ + type: "client", + clientId: "client-a", + interfaceId: "macos", + capabilities: ["host_file"], + callback: (e) => { + receivedA.push(e); + }, + }); + + await hub.publish(makeEvent(), { + targetClientId: "client-a", + targetCapability: "host_bash", + }); + + // client-a is the target but lacks the required capability — not delivered + expect(receivedA).toHaveLength(0); + }); + + test("targeted delivery with matching capability delivers", async () => { + const hub = new AssistantEventHub(); + const receivedA: AssistantEvent[] = []; + + hub.subscribe({ + type: "client", + clientId: "client-a", + interfaceId: "macos", + capabilities: ["host_bash"], + callback: (e) => { + receivedA.push(e); + }, + }); + + await hub.publish(makeEvent(), { + targetClientId: "client-a", + targetCapability: "host_bash", + }); + + expect(receivedA).toHaveLength(1); + }); + + test("process-type subscriber is never matched by targetClientId", async () => { + const hub = new AssistantEventHub(); + const received: AssistantEvent[] = []; + + hub.subscribe({ + type: "process", + callback: (e) => { + received.push(e); + }, + }); + + await hub.publish(makeEvent(), { targetClientId: "some-client" }); + + // Process subscribers have no clientId — they should never receive targeted events + expect(received).toHaveLength(0); + }); +}); + +// ── Untargeted delivery unchanged ───────────────────────────────────────────── + +describe("AssistantEventHub — untargeted capability targeting is unchanged", () => { + test("targetCapability without targetClientId still applies conversation scoping", async () => { + const hub = new AssistantEventHub(); + const receivedA: AssistantEvent[] = []; + const receivedB: AssistantEvent[] = []; + + hub.subscribe({ + type: "client", + clientId: "client-a", + interfaceId: "macos", + capabilities: ["host_bash"], + filter: { conversationId: "sess_A" }, + callback: (e) => { + receivedA.push(e); + }, + }); + + hub.subscribe({ + type: "client", + clientId: "client-b", + interfaceId: "macos", + capabilities: ["host_bash"], + filter: { conversationId: "sess_B" }, + callback: (e) => { + receivedB.push(e); + }, + }); + + await hub.publish(makeEvent({ conversationId: "sess_A" }), { + targetCapability: "host_bash", + }); + + expect(receivedA).toHaveLength(1); + expect(receivedB).toHaveLength(0); + }); +}); + +// ── getClientById ───────────────────────────────────────────────────────────── + +describe("AssistantEventHub — getClientById()", () => { + test("returns the client entry for the given clientId", () => { + const hub = new AssistantEventHub(); + + hub.subscribe({ + type: "client", + clientId: "client-x", + interfaceId: "macos", + capabilities: ["host_bash"], + callback: () => {}, + }); + + const entry = hub.getClientById("client-x"); + expect(entry).toBeDefined(); + expect(entry?.clientId).toBe("client-x"); + }); + + test("returns undefined when no client has the given clientId", () => { + const hub = new AssistantEventHub(); + + hub.subscribe({ + type: "client", + clientId: "client-x", + interfaceId: "macos", + capabilities: ["host_bash"], + callback: () => {}, + }); + + expect(hub.getClientById("client-y")).toBeUndefined(); + }); + + test("returns undefined after the subscriber is disposed", () => { + const hub = new AssistantEventHub(); + + const sub = hub.subscribe({ + type: "client", + clientId: "client-x", + interfaceId: "macos", + capabilities: ["host_bash"], + callback: () => {}, + }); + + sub.dispose(); + expect(hub.getClientById("client-x")).toBeUndefined(); + }); +}); diff --git a/assistant/src/__tests__/host-bash-proxy.test.ts b/assistant/src/__tests__/host-bash-proxy.test.ts index d4e6df87954..db3ea134e24 100644 --- a/assistant/src/__tests__/host-bash-proxy.test.ts +++ b/assistant/src/__tests__/host-bash-proxy.test.ts @@ -15,14 +15,22 @@ mock.module("../config/loader.js", () => ({ })); const sentMessages: unknown[] = []; +const sentMessageOptions: unknown[] = []; const resolvedInteractionIds: string[] = []; let mockHasClient = false; +let mockCapableClients: Array<{ clientId: string; capabilities: string[] }> = []; +let mockClientRegistry: Map = new Map(); mock.module("../runtime/assistant-event-hub.js", () => ({ - broadcastMessage: (msg: unknown) => sentMessages.push(msg), + broadcastMessage: (msg: unknown, _conversationId?: string, options?: unknown) => { + sentMessages.push(msg); + sentMessageOptions.push(options); + }, assistantEventHub: { getMostRecentClientByCapability: (cap: string) => cap === "host_bash" && mockHasClient ? { id: "mock-client" } : null, + listClientsByCapability: (_cap: string) => mockCapableClients, + getClientById: (clientId: string) => mockClientRegistry.get(clientId), }, })); @@ -44,11 +52,30 @@ describe("HostBashProxy", () => { function setup() { sentMessages.length = 0; + sentMessageOptions.length = 0; resolvedInteractionIds.length = 0; mockHasClient = false; + mockCapableClients = []; + mockClientRegistry = new Map(); proxy = new (HostBashProxy as any)(); } + function setupSingleClient(clientId = "client-1") { + const entry = { clientId, capabilities: ["host_bash"] }; + mockCapableClients = [entry]; + mockClientRegistry.set(clientId, entry); + } + + function setupMultipleClients(clientIds: string[]) { + mockCapableClients = clientIds.map((id) => ({ + clientId: id, + capabilities: ["host_bash"], + })); + for (const entry of mockCapableClients) { + mockClientRegistry.set(entry.clientId, entry); + } + } + afterEach(() => { proxy?.dispose(); HostBashProxy.reset(); @@ -527,4 +554,184 @@ describe("HostBashProxy", () => { expect(resolvedInteractionIds).toEqual([]); }); }); + + describe("target client routing", () => { + test("auto-resolves when exactly one capable client is connected", async () => { + setup(); + setupSingleClient("client-abc"); + + const resultPromise = proxy.request( + { command: "echo hello" }, + "session-1", + ); + + expect(sentMessages).toHaveLength(1); + const sent = sentMessages[0] as Record; + expect(sent.targetClientId).toBe("client-abc"); + + // Options passed to broadcastMessage should also have targetClientId + const opts = sentMessageOptions[0] as Record | undefined; + expect(opts?.targetClientId).toBe("client-abc"); + + const requestId = sent.requestId as string; + proxy.resolve(requestId, { + stdout: "hello\n", + stderr: "", + exitCode: 0, + timedOut: false, + }); + + const result = await resultPromise; + expect(result.isError).toBe(false); + }); + + test("uses explicit targetClientId when it is valid", async () => { + setup(); + setupSingleClient("client-abc"); + // Also register a second client so we're sure explicit targeting works + const entry2 = { clientId: "client-xyz", capabilities: ["host_bash"] }; + mockCapableClients.push(entry2); + mockClientRegistry.set("client-xyz", entry2); + + const resultPromise = proxy.request( + { command: "echo hello", targetClientId: "client-abc" }, + "session-1", + ); + + expect(sentMessages).toHaveLength(1); + const sent = sentMessages[0] as Record; + expect(sent.targetClientId).toBe("client-abc"); + + const opts = sentMessageOptions[0] as Record | undefined; + expect(opts?.targetClientId).toBe("client-abc"); + + const requestId = sent.requestId as string; + proxy.resolve(requestId, { + stdout: "ok\n", + stderr: "", + exitCode: 0, + timedOut: false, + }); + + const result = await resultPromise; + expect(result.isError).toBe(false); + }); + + test("returns error for explicit targetClientId that is not connected", async () => { + setup(); + setupSingleClient("client-abc"); + + const result = await proxy.request( + { command: "echo hello", targetClientId: "client-unknown" }, + "session-1", + ); + + // Should return error without broadcasting + expect(result.isError).toBe(true); + expect(result.content).toContain("client-unknown"); + expect(result.content).toContain("assistant clients list --capability host_bash"); + expect(sentMessages).toHaveLength(0); + }); + + test("returns error for explicit targetClientId that is connected but lacks host_bash", async () => { + setup(); + // Register a client without host_bash capability + mockClientRegistry.set("client-no-bash", { + clientId: "client-no-bash", + capabilities: [], + }); + + const result = await proxy.request( + { command: "echo hello", targetClientId: "client-no-bash" }, + "session-1", + ); + + expect(result.isError).toBe(true); + expect(result.content).toContain("client-no-bash"); + expect(result.content).toContain("does not support host_bash"); + expect(sentMessages).toHaveLength(0); + }); + + test("falls through to untargeted broadcast when multiple capable clients are connected and no targetClientId", async () => { + setup(); + setupMultipleClients(["client-1", "client-2", "client-3"]); + + const resultPromise = proxy.request( + { command: "echo hello" }, + "session-1", + ); + + // Should broadcast without an early error return + expect(sentMessages).toHaveLength(1); + const sent = sentMessages[0] as Record; + expect(sent.type).toBe("host_bash_request"); + // No target client resolved — untargeted broadcast + expect(sent.targetClientId).toBeUndefined(); + + const opts = sentMessageOptions[0] as Record | undefined; + expect(opts?.targetClientId).toBeUndefined(); + + // Manually resolve to clean up + const requestId = sent.requestId as string; + proxy.resolve(requestId, { + stdout: "hello\n", + stderr: "", + exitCode: 0, + timedOut: false, + }); + + const result = await resultPromise; + expect(result.isError).toBe(false); + }); + + test("falls through to broadcast when zero capable clients (existing timeout path)", async () => { + setup(); + // mockCapableClients is empty (default), so capableClients.length === 0 + + const resultPromise = proxy.request( + { command: "echo hello" }, + "session-1", + ); + + // Should still broadcast (no early return) + expect(sentMessages).toHaveLength(1); + const sent = sentMessages[0] as Record; + expect(sent.type).toBe("host_bash_request"); + // targetClientId is undefined when no clients present + expect(sent.targetClientId).toBeUndefined(); + + // Manually resolve to clean up + const requestId = sent.requestId as string; + proxy.resolve(requestId, { + stdout: "", + stderr: "", + exitCode: 0, + timedOut: false, + }); + + await resultPromise; + }); + + test("includes targetClientId in timeout error message when client was resolved", async () => { + setup(); + setupSingleClient("client-mac"); + + jest.useFakeTimers(); + try { + const resultPromise = proxy.request( + { command: "echo slow", timeout_seconds: 30 }, + "session-1", + ); + + // Proxy timeout = 33s; advance past it + jest.advanceTimersByTime(34 * 1000); + + const result = await resultPromise; + expect(result.isError).toBe(true); + expect(result.content).toContain("client-mac"); + } finally { + jest.useRealTimers(); + } + }); + }); }); diff --git a/assistant/src/__tests__/host-bash-routes.test.ts b/assistant/src/__tests__/host-bash-routes.test.ts new file mode 100644 index 00000000000..4927876443e --- /dev/null +++ b/assistant/src/__tests__/host-bash-routes.test.ts @@ -0,0 +1,291 @@ +/** + * Unit tests for the /v1/host-bash-result route handler. + * + * Covers the client-identity validation introduced by the targeted-host-proxy + * plan: when a pending interaction has a `targetClientId`, the submitting + * client must supply a matching `x-vellum-client-id` header or be rejected + * with 400 (missing) or 403 (mismatch). + */ +import { afterAll, beforeEach, describe, expect, mock, test } from "bun:test"; + +// ── Module mocks ───────────────────────────────────────────────────── + +mock.module("../config/env.js", () => ({ + isHttpAuthDisabled: () => true, + hasUngatedHttpAuthDisabled: () => false, +})); + +import type { PendingInteraction } from "../runtime/pending-interactions.js"; + +// Stored pending interactions keyed by requestId. +const pendingStore = new Map(); +const resolvedIds: string[] = []; + +mock.module("../runtime/pending-interactions.js", () => ({ + get: (requestId: string) => pendingStore.get(requestId), + resolve: (requestId: string) => { + const entry = pendingStore.get(requestId); + if (entry) { + pendingStore.delete(requestId); + resolvedIds.push(requestId); + } + return entry; + }, +})); + +interface ResolveCall { + requestId: string; + result: { stdout: string; stderr: string; exitCode: number | null; timedOut: boolean }; +} + +const resolveSpy: ResolveCall[] = []; + +mock.module("../daemon/host-bash-proxy.js", () => ({ + HostBashProxy: { + get instance() { + return { + resolve( + requestId: string, + result: { stdout: string; stderr: string; exitCode: number | null; timedOut: boolean }, + ) { + resolveSpy.push({ requestId, result }); + }, + }; + }, + }, +})); + +// ── Real imports (after mocks) ─────────────────────────────────────── + +import { + BadRequestError, + ConflictError, + ForbiddenError, + NotFoundError, +} from "../runtime/routes/errors.js"; +import { ROUTES } from "../runtime/routes/host-bash-routes.js"; + +afterAll(() => { + mock.restore(); +}); + +const handleHostBashResult = ROUTES.find( + (r) => r.endpoint === "host-bash-result", +)!.handler; + +// ── Helpers ────────────────────────────────────────────────────────── + +function registerPending( + requestId: string, + overrides: Partial = {}, +): void { + pendingStore.set(requestId, { + conversationId: "conv-1", + kind: "host_bash", + ...overrides, + }); +} + +function bashBody(requestId: string): Record { + return { + requestId, + stdout: "hello\n", + stderr: "", + exitCode: 0, + timedOut: false, + }; +} + +// ── Tests ──────────────────────────────────────────────────────────── + +describe("handleHostBashResult", () => { + beforeEach(() => { + pendingStore.clear(); + resolvedIds.length = 0; + resolveSpy.length = 0; + }); + + // ── Happy paths ──────────────────────────────────────────────────── + + describe("untargeted request (no targetClientId)", () => { + test("accepts when header is present", async () => { + const requestId = "req-untargeted-with-header"; + registerPending(requestId); + + const result = await handleHostBashResult({ + body: bashBody(requestId), + headers: { "x-vellum-client-id": "client-abc" }, + }); + + expect(result).toEqual({ accepted: true }); + expect(resolveSpy).toHaveLength(1); + expect(resolvedIds).toContain(requestId); + }); + + test("accepts when header is absent", async () => { + const requestId = "req-untargeted-no-header"; + registerPending(requestId); + + const result = await handleHostBashResult({ + body: bashBody(requestId), + }); + + expect(result).toEqual({ accepted: true }); + expect(resolveSpy).toHaveLength(1); + expect(resolvedIds).toContain(requestId); + }); + }); + + describe("targeted request (targetClientId set)", () => { + test("accepts when x-vellum-client-id matches targetClientId", async () => { + const requestId = "req-targeted-match"; + registerPending(requestId, { targetClientId: "client-abc" }); + + const result = await handleHostBashResult({ + body: bashBody(requestId), + headers: { "x-vellum-client-id": "client-abc" }, + }); + + expect(result).toEqual({ accepted: true }); + expect(resolveSpy).toHaveLength(1); + expect(resolveSpy[0].requestId).toBe(requestId); + expect(resolvedIds).toContain(requestId); + }); + + test("trims whitespace from x-vellum-client-id before comparing", async () => { + const requestId = "req-targeted-trim"; + registerPending(requestId, { targetClientId: "client-abc" }); + + const result = await handleHostBashResult({ + body: bashBody(requestId), + headers: { "x-vellum-client-id": " client-abc " }, + }); + + expect(result).toEqual({ accepted: true }); + }); + }); + + // ── Error: missing header on targeted request ────────────────────── + + describe("targeted request — missing x-vellum-client-id header", () => { + test("throws BadRequestError (400) when header is absent", () => { + const requestId = "req-targeted-no-header"; + registerPending(requestId, { targetClientId: "client-abc" }); + + expect(() => + handleHostBashResult({ body: bashBody(requestId) }), + ).toThrow(BadRequestError); + }); + + test("throws BadRequestError (400) when header is empty string", () => { + const requestId = "req-targeted-empty-header"; + registerPending(requestId, { targetClientId: "client-abc" }); + + expect(() => + handleHostBashResult({ + body: bashBody(requestId), + headers: { "x-vellum-client-id": " " }, + }), + ).toThrow(BadRequestError); + }); + + test("interaction is NOT resolved on 400 (still pending)", () => { + const requestId = "req-targeted-no-header-stays"; + registerPending(requestId, { targetClientId: "client-abc" }); + + try { + handleHostBashResult({ body: bashBody(requestId) }); + } catch { + // expected + } + + expect(resolvedIds).not.toContain(requestId); + expect(pendingStore.has(requestId)).toBe(true); + }); + }); + + // ── Error: wrong client ──────────────────────────────────────────── + + describe("targeted request — mismatched x-vellum-client-id", () => { + test("throws ForbiddenError (403) when client ID does not match", () => { + const requestId = "req-targeted-mismatch"; + registerPending(requestId, { targetClientId: "client-abc" }); + + expect(() => + handleHostBashResult({ + body: bashBody(requestId), + headers: { "x-vellum-client-id": "client-xyz" }, + }), + ).toThrow(ForbiddenError); + }); + + test("ForbiddenError message names both the submitting and expected client", () => { + const requestId = "req-targeted-mismatch-msg"; + registerPending(requestId, { targetClientId: "client-abc" }); + + let caught: unknown; + try { + handleHostBashResult({ + body: bashBody(requestId), + headers: { "x-vellum-client-id": "client-xyz" }, + }); + } catch (e) { + caught = e; + } + + expect(caught).toBeInstanceOf(ForbiddenError); + const msg = (caught as ForbiddenError).message; + expect(msg).toContain("client-xyz"); + expect(msg).toContain("client-abc"); + }); + + test("interaction is NOT resolved on 403 (still pending)", () => { + const requestId = "req-targeted-mismatch-stays"; + registerPending(requestId, { targetClientId: "client-abc" }); + + try { + handleHostBashResult({ + body: bashBody(requestId), + headers: { "x-vellum-client-id": "client-xyz" }, + }); + } catch { + // expected + } + + expect(resolvedIds).not.toContain(requestId); + expect(pendingStore.has(requestId)).toBe(true); + }); + }); + + // ── Other existing validations (regression) ──────────────────────── + + test("throws BadRequestError when body is missing", () => { + expect(() => handleHostBashResult({})).toThrow(BadRequestError); + }); + + test("throws BadRequestError when requestId is missing", () => { + expect(() => + handleHostBashResult({ body: { stdout: "x" } }), + ).toThrow(BadRequestError); + }); + + test("throws NotFoundError for unknown requestId", () => { + expect(() => + handleHostBashResult({ + body: bashBody("unknown-req-id"), + }), + ).toThrow(NotFoundError); + }); + + test("throws ConflictError when pending interaction is not host_bash kind", () => { + const requestId = "req-wrong-kind"; + pendingStore.set(requestId, { + conversationId: "conv-1", + kind: "confirmation", + }); + + expect(() => + handleHostBashResult({ body: bashBody(requestId) }), + ).toThrow(ConflictError); + }); +}); diff --git a/assistant/src/__tests__/host-shell-tool.test.ts b/assistant/src/__tests__/host-shell-tool.test.ts index 8fc6e0b9828..320312b9889 100644 --- a/assistant/src/__tests__/host-shell-tool.test.ts +++ b/assistant/src/__tests__/host-shell-tool.test.ts @@ -56,12 +56,7 @@ mock.module("../util/logger.js", () => ({ // Mock the host-bash-proxy singleton so proxy delegation tests can control it. let mockProxyAvailable = false; let mockProxyRequestFn: ( - input: { - command: string; - working_dir?: string; - timeout_seconds?: number; - env?: Record; - }, + input: { command: string; working_dir?: string; timeout_seconds?: number; env?: Record; targetClientId?: string }, conversationId: string, signal?: AbortSignal, ) => Promise = () => @@ -281,12 +276,13 @@ describe("host_bash — regression: no proxied-mode additions", () => { expect(schemaProps).not.toHaveProperty("credential_ids"); }); - test("schema only contains the expected properties (command, working_dir, timeout_seconds, activity, background)", () => { + test("schema only contains the expected properties (command, working_dir, timeout_seconds, activity, background, target_client_id)", () => { const propertyNames = Object.keys(schemaProps).sort(); expect(propertyNames).toEqual([ "activity", "background", "command", + "target_client_id", "timeout_seconds", "working_dir", ]); @@ -732,6 +728,7 @@ describe("host_bash — proxy delegation", () => { working_dir?: string; timeout_seconds?: number; env?: Record; + targetClientId?: string; }; conversationId: string; }> = []; @@ -833,6 +830,22 @@ describe("host_bash — proxy delegation", () => { expect(spawnCalls.length).toBe(1); }); + test("returns error when explicit targetClientId is set but proxy is unavailable (client disconnected)", async () => { + // mockProxyAvailable defaults to false — simulates client disconnecting + // after tool definitions were built (targetClientId already resolved). + spawnCalls.length = 0; + const result = await hostShellTool.execute( + { command: "echo should-not-run", target_client_id: "client-mac-abc123" }, + { ...makeContext(), transportInterface: "web" }, + ); + + // Must error, NOT fall through to local spawn + expect(result.isError).toBe(true); + expect(result.content).toContain("client-mac-abc123"); + expect(result.content).toContain("no longer connected"); + expect(spawnCalls.length).toBe(0); + }); + test("falls back to local execution when no proxy is set", async () => { const dir = mkdtempSync(join(tmpdir(), "host-shell-no-proxy-")); testDirs.push(dir); diff --git a/assistant/src/cli/commands/clients.ts b/assistant/src/cli/commands/clients.ts index b738965abef..1e70326c19d 100644 --- a/assistant/src/cli/commands/clients.ts +++ b/assistant/src/cli/commands/clients.ts @@ -9,6 +9,7 @@ interface ClientEntryJSON { clientId: string; interfaceId: string; capabilities: string[]; + machineName?: string; connectedAt: string; lastActiveAt: string; } @@ -104,6 +105,7 @@ Examples: "CLIENT ID", "INTERFACE", "CAPABILITIES", + "LABEL", "CONNECTED", "LAST ACTIVE", ]; @@ -111,6 +113,7 @@ Examples: e.clientId, e.interfaceId, e.capabilities.length > 0 ? e.capabilities.join(", ") : "—", + e.machineName ?? "—", formatRelativeTime(e.connectedAt), formatRelativeTime(e.lastActiveAt), ]); diff --git a/assistant/src/daemon/__tests__/conversation-tool-setup.test.ts b/assistant/src/daemon/__tests__/conversation-tool-setup.test.ts index 2c2aae2f0b8..36f7409d26f 100644 --- a/assistant/src/daemon/__tests__/conversation-tool-setup.test.ts +++ b/assistant/src/daemon/__tests__/conversation-tool-setup.test.ts @@ -18,17 +18,46 @@ * first and is authoritative for structural support, so host_bash and * host_file_* are filtered out for chrome-extension regardless of the * hasNoClient flag. + * + * Cross-client exception (Phase 1): host_bash is allowed for non-host-proxy + * interfaces (e.g. "web") when at least one host_bash-capable client is + * connected via the event hub. host_file_* and host_browser remain filtered + * regardless (Phase 2). */ -import { describe, expect, test } from "bun:test"; +import { beforeEach, describe, expect, mock, test } from "bun:test"; + +// ── Module-level mocks ───────────────────────────────────────────── + +// Control how many host_bash-capable clients the hub reports. +let mockHostBashClientCount = 0; + +mock.module("../../runtime/assistant-event-hub.js", () => ({ + assistantEventHub: { + listClientsByCapability: (cap: string) => { + if (cap === "host_bash") { + return Array.from({ length: mockHostBashClientCount }, (_, i) => ({ + clientId: `mock-client-${i}`, + capabilities: ["host_bash"], + })); + } + return []; + }, + }, + broadcastMessage: () => {}, +})); -import type { SkillProjectionCache } from "../conversation-skill-tools.js"; -import { +// Dynamic imports after mock.module calls so the stubs take effect +// before the modules under test are loaded. +const { HOST_TOOL_NAMES, HOST_TOOL_TO_CAPABILITY, isToolActiveForContext, - type SkillProjectionContext, -} from "../conversation-tool-setup.js"; +} = await import("../conversation-tool-setup.js"); +type SkillProjectionContext = + import("../conversation-tool-setup.js").SkillProjectionContext; +type SkillProjectionCache = + import("../conversation-skill-tools.js").SkillProjectionCache; function makeCtx( overrides: Partial = {}, @@ -42,6 +71,10 @@ function makeCtx( }; } +beforeEach(() => { + mockHostBashClientCount = 0; +}); + describe("isToolActiveForContext — host tool capability gating", () => { // macOS transport: SSE-based interactive approval required. test("host_bash is active for macOS with a connected client", () => { @@ -176,6 +209,94 @@ describe("isToolActiveForContext — host tool capability gating", () => { }); }); +describe("isToolActiveForContext — cross-client exception (Phase 1: host_bash)", () => { + test("host_bash is active for web transport when a host_bash-capable client is connected", () => { + // Cross-client path: a web turn should see host_bash when a macOS client + // with host_bash capability is connected via the event hub. + mockHostBashClientCount = 1; + expect( + isToolActiveForContext( + "host_bash", + makeCtx({ hasNoClient: false, transportInterface: "web" }), + ), + ).toBe(true); + }); + + test("host_bash is NOT active for web transport when no capable client is connected", () => { + // No cross-client fallback: hub has no host_bash-capable subscribers. + mockHostBashClientCount = 0; + expect( + isToolActiveForContext( + "host_bash", + makeCtx({ hasNoClient: false, transportInterface: "web" }), + ), + ).toBe(false); + }); + + test("host_file_read is NOT active for web transport even when a capable client is connected (Phase 2 gate)", () => { + // The cross-client exception is scoped to host_bash only. + // host_file_* remain filtered for non-host-proxy interfaces regardless + // of connected clients until Phase 2 lands. + mockHostBashClientCount = 1; + expect( + isToolActiveForContext( + "host_file_read", + makeCtx({ hasNoClient: false, transportInterface: "web" }), + ), + ).toBe(false); + }); + + test("host_bash for macos transport is unaffected by the cross-client exception", () => { + // macos natively supports host_bash via host proxy — the supportsHostProxy + // check passes, so the cross-client branch is never reached. + mockHostBashClientCount = 0; + expect( + isToolActiveForContext( + "host_bash", + makeCtx({ hasNoClient: false, transportInterface: "macos" }), + ), + ).toBe(true); + }); + + test("host_bash for macos with no client is still denied (security invariant unaffected)", () => { + // Even with a capable client in the hub, the macos SSE path takes + // precedence — it passes the supportsHostProxy check, bypasses the + // cross-client branch, and reaches the hasNoClient gate. + mockHostBashClientCount = 1; + expect( + isToolActiveForContext( + "host_bash", + makeCtx({ hasNoClient: true, transportInterface: "macos" }), + ), + ).toBe(false); + }); + + test("host_bash is NOT active for chrome-extension even when a capable client is connected", () => { + // Security boundary: chrome-extension only gets host_browser. The + // cross-client exception explicitly excludes chrome-extension transport + // regardless of how many host_bash-capable clients are in the hub. + mockHostBashClientCount = 1; + expect( + isToolActiveForContext( + "host_bash", + makeCtx({ hasNoClient: false, transportInterface: "chrome-extension" }), + ), + ).toBe(false); + }); + + test("host_bash is NOT active for web transport when hasNoClient is true (no approval UI)", () => { + // hasNoClient gate: no interactive approval UI available for this turn. + // Cross-client exception must not bypass this gate. + mockHostBashClientCount = 1; + expect( + isToolActiveForContext( + "host_bash", + makeCtx({ hasNoClient: true, 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 b1a1d425b29..85eb177707d 100644 --- a/assistant/src/daemon/conversation-tool-setup.ts +++ b/assistant/src/daemon/conversation-tool-setup.ts @@ -18,6 +18,7 @@ import type { PermissionPrompter } from "../permissions/prompter.js"; import type { SecretPrompter } from "../permissions/secret-prompter.js"; import type { Message, ToolDefinition } from "../providers/types.js"; import type { TrustClass } from "../runtime/actor-trust-resolver.js"; +import { assistantEventHub } from "../runtime/assistant-event-hub.js"; import { coreAppProxyTools } from "../tools/apps/definitions.js"; import { registerConversationSender } from "../tools/browser/browser-screencast.js"; import type { ToolExecutor } from "../tools/executor.js"; @@ -384,6 +385,19 @@ export function isToolActiveForContext( // Per-capability check is authoritative for structural support: if the // transport cannot service this capability, the tool is filtered out. if (transport && capability && !supportsHostProxy(transport, capability)) { + // Cross-client exception: allow host_bash for non-host-proxy interfaces when + // at least one capable client is connected via the event hub. + // Only applies to host_bash (not host_file, host_cu, host_browser — Phase 2). + // Excludes chrome-extension (security boundary: extension only gets host_browser) + // and hasNoClient turns (no interactive approval UI available). + if ( + capability === "host_bash" && + transport !== "chrome-extension" && + !ctx.hasNoClient && + assistantEventHub.listClientsByCapability("host_bash").length > 0 + ) { + return true; + } return false; } diff --git a/assistant/src/daemon/host-bash-proxy.ts b/assistant/src/daemon/host-bash-proxy.ts index 1efa1ceb753..f58c3b5b1a8 100644 --- a/assistant/src/daemon/host-bash-proxy.ts +++ b/assistant/src/daemon/host-bash-proxy.ts @@ -21,6 +21,7 @@ interface PendingRequest { conversationId: string; /** Detach the abort listener from the caller's signal. No-op when no signal was passed. */ detachAbort: () => void; + targetClientId?: string; } export class HostBashProxy { @@ -69,6 +70,7 @@ export class HostBashProxy { working_dir?: string; timeout_seconds?: number; env?: Record; + targetClientId?: string; }, conversationId: string, signal?: AbortSignal, @@ -78,6 +80,28 @@ export class HostBashProxy { return Promise.resolve(result); } + const capableClients = assistantEventHub.listClientsByCapability("host_bash"); + + let resolvedTargetClientId: string | undefined; + + if (input.targetClientId) { + const target = assistantEventHub.getClientById(input.targetClientId); + if (!target || !target.capabilities.includes("host_bash")) { + return Promise.resolve({ + content: `Error: client "${input.targetClientId}" is not connected or does not support host_bash. Run \`assistant clients list --capability host_bash\` to see available clients.`, + isError: true, + }); + } + resolvedTargetClientId = input.targetClientId; + } else if (capableClients.length === 1) { + // Auto-resolve when exactly one capable client is connected. + resolvedTargetClientId = capableClients[0].clientId; + } + // capableClients.length === 0 or > 1 without explicit target: resolvedTargetClientId + // stays undefined and falls through to untargeted broadcast — the existing timeout/error + // path handles the zero-client case, and multi-client ambiguity is enforced at the tool + // executor layer (not here) once target_client_id is exposed in the tool schema. + const requestId = uuid(); return new Promise((resolve, reject) => { @@ -98,10 +122,13 @@ export class HostBashProxy { { requestId, command: input.command }, "Host bash proxy request timed out", ); + const timeoutMessage = resolvedTargetClientId + ? `Host bash proxy timed out waiting for response from client ${resolvedTargetClientId}` + : "Host bash proxy timed out waiting for client response"; resolve( formatShellOutput( "", - "Host bash proxy timed out waiting for client response", + timeoutMessage, null, true, timeoutSec, @@ -117,11 +144,16 @@ export class HostBashProxy { detachAbort(); pendingInteractions.resolve(requestId); try { - broadcastMessage({ - type: "host_bash_cancel", - requestId, + broadcastMessage( + { + type: "host_bash_cancel", + requestId, + conversationId, + targetClientId: resolvedTargetClientId, + }, conversationId, - }); + { targetClientId: resolvedTargetClientId }, + ); } catch { // Best-effort cancel notification — connection may already be closed. } @@ -139,20 +171,26 @@ export class HostBashProxy { timeoutSec, conversationId, detachAbort, + targetClientId: resolvedTargetClientId, }); try { - broadcastMessage({ - type: "host_bash_request", - requestId, + broadcastMessage( + { + type: "host_bash_request", + requestId, + conversationId, + command: input.command, + working_dir: input.working_dir, + timeout_seconds: input.timeout_seconds, + targetClientId: resolvedTargetClientId, + ...(input.env && Object.keys(input.env).length > 0 + ? { env: input.env } + : {}), + }, conversationId, - command: input.command, - working_dir: input.working_dir, - timeout_seconds: input.timeout_seconds, - ...(input.env && Object.keys(input.env).length > 0 - ? { env: input.env } - : {}), - }); + { targetClientId: resolvedTargetClientId }, + ); } catch (err) { clearTimeout(timer); this.pending.delete(requestId); @@ -204,11 +242,16 @@ export class HostBashProxy { entry.detachAbort(); pendingInteractions.resolve(requestId); try { - broadcastMessage({ - type: "host_bash_cancel", - requestId, - conversationId: entry.conversationId, - }); + broadcastMessage( + { + type: "host_bash_cancel", + requestId, + conversationId: entry.conversationId, + targetClientId: entry.targetClientId, + }, + entry.conversationId, + { targetClientId: entry.targetClientId }, + ); } catch { // Best-effort cancel notification — connection may already be closed. } diff --git a/assistant/src/daemon/message-types/host-bash.ts b/assistant/src/daemon/message-types/host-bash.ts index b72526e7fba..6a956f50908 100644 --- a/assistant/src/daemon/message-types/host-bash.ts +++ b/assistant/src/daemon/message-types/host-bash.ts @@ -13,12 +13,16 @@ export interface HostBashRequest { timeout_seconds?: number; /** Extra environment variables to inject into the subprocess (e.g. VELLUM_UNTRUSTED_SHELL). */ env?: Record; + /** When set, route this request only to the client with this ID. */ + targetClientId?: string; } export interface HostBashCancelRequest { type: "host_bash_cancel"; requestId: string; conversationId: string; + /** When set, route this cancel only to the client that owns the request. */ + targetClientId?: string; } // --- Domain-level union aliases (consumed by the barrel file) --- diff --git a/assistant/src/runtime/assistant-event-hub.ts b/assistant/src/runtime/assistant-event-hub.ts index 6cbe9537015..737b3b156d5 100644 --- a/assistant/src/runtime/assistant-event-hub.ts +++ b/assistant/src/runtime/assistant-event-hub.ts @@ -90,6 +90,7 @@ export interface ClientEntry extends BaseSubscriberEntry { clientId: string; interfaceId: InterfaceId; capabilities: HostProxyCapability[]; + machineName?: string; } export interface ProcessEntry extends BaseSubscriberEntry { @@ -245,7 +246,12 @@ export class AssistantEventHub { * Publish an event to all matching subscribers. * * Matching rules: - * - if `filter.conversationId` is set, `event.conversationId` must equal it + * - if `targetClientId` is set, deliver only to the subscriber with that + * clientId, bypassing the conversation-id filter entirely (the web-origin + * event's conversationId differs from the macOS client's subscribed + * conversation). + * - if `filter.conversationId` is set (and `targetClientId` is not), the + * `event.conversationId` must equal it * - if `targetCapability` is set, only subscribers whose capabilities include * it receive the event; untargeted events go to all * @@ -254,7 +260,7 @@ export class AssistantEventHub { */ async publish( event: AssistantEvent, - options?: { targetCapability?: HostProxyCapability }, + options?: { targetCapability?: HostProxyCapability; targetClientId?: string }, ): Promise { if (event.conversationId) { try { @@ -265,29 +271,40 @@ export class AssistantEventHub { } const targetCapability = options?.targetCapability; + const targetClientId = options?.targetClientId; const snapshot = Array.from(this.subscribers); const errors: unknown[] = []; for (const entry of snapshot) { if (!entry.active) continue; - // Conversation scoping: scoped events skip subscribers filtering on a - // different conversation. - if ( - event.conversationId != null && - entry.filter.conversationId != null && - entry.filter.conversationId !== event.conversationId - ) - continue; - - // Capability targeting: targeted events only go to subscribers that - // declare the required capability. - if (targetCapability != null) { + if (targetClientId != null) { + // Targeted: bypass conversation filter, deliver only to the named client. + if (entry.type !== "client" || entry.clientId !== targetClientId) + continue; if ( - entry.type !== "client" || + targetCapability != null && !entry.capabilities.includes(targetCapability) ) continue; + } else { + // Untargeted: existing conversation-scoped + capability logic. + if ( + event.conversationId != null && + entry.filter.conversationId != null && + entry.filter.conversationId !== event.conversationId + ) + continue; + + // Capability targeting: targeted events only go to subscribers that + // declare the required capability. + if (targetCapability != null) { + if ( + entry.type !== "client" || + !entry.capabilities.includes(targetCapability) + ) + continue; + } } try { @@ -305,6 +322,18 @@ export class AssistantEventHub { } } + /** + * Return the active client subscriber with the given clientId, or + * `undefined` if no such subscriber exists. + */ + getClientById(clientId: string): ClientEntry | undefined { + for (const entry of this.subscribers) { + if (entry.active && entry.type === "client" && entry.clientId === clientId) + return entry; + } + return undefined; + } + /** * Returns true when at least one active subscriber would receive the given * event based on the same conversation matching rules as publish(). @@ -470,13 +499,15 @@ let _hubChain = Promise.resolve(); export function broadcastMessage( msg: ServerMessage, conversationId?: string, + options?: { targetClientId?: string }, ): void { const resolvedConversationId = conversationId ?? extractConversationId(msg); + const targetClientId = options?.targetClientId; // Register pending interactions so approval/host prompts are tracked // regardless of which path triggered the broadcast. if (resolvedConversationId) { - registerPendingInteraction(msg, resolvedConversationId); + registerPendingInteraction(msg, resolvedConversationId, targetClientId); } // Emit feed events for confirmation requests (tool approval prompts). @@ -492,13 +523,12 @@ export function broadcastMessage( : resolvedConversationId; const event = buildAssistantEvent(msg, scopedConversationId); const targetCapability = capabilityForMessageType(msg.type); + const publishOptions = + targetCapability != null || targetClientId != null + ? { targetCapability, targetClientId } + : undefined; _hubChain = _hubChain - .then(() => - assistantEventHub.publish( - event, - targetCapability ? { targetCapability } : undefined, - ), - ) + .then(() => assistantEventHub.publish(event, publishOptions)) .then(() => { // When a conversation title changes, also broadcast an unscoped // `conversation_list_invalidated` so every connected client's sidebar @@ -549,10 +579,14 @@ function resolveCanonicalRequestSourceType( * Heavy dependencies (conversation-store, canonical-guardian-store, etc.) are * imported lazily so that loading this module during tests doesn't trigger * config/data-dir side effects. + * + * @param targetClientId - When set, the host_bash request should be routed to + * this specific client. May be undefined for macos-origin turns. */ function registerPendingInteraction( msg: ServerMessage, conversationId: string, + targetClientId?: string, ): void { if (msg.type === "confirmation_request") { pendingInteractions.register(msg.requestId, { @@ -582,6 +616,7 @@ function registerPendingInteraction( pendingInteractions.register(msg.requestId, { conversationId, kind: "host_bash", + targetClientId, // NEW — may be undefined for macos-origin turns }); } else if (msg.type === "host_browser_request") { pendingInteractions.register(msg.requestId, { diff --git a/assistant/src/runtime/pending-interactions.ts b/assistant/src/runtime/pending-interactions.ts index af0f50c60bf..3ec53fd688c 100644 --- a/assistant/src/runtime/pending-interactions.ts +++ b/assistant/src/runtime/pending-interactions.ts @@ -52,6 +52,8 @@ export interface PendingInteraction { confirmationDetails?: ConfirmationDetails; /** For ACP permissions: resolves directly without a Conversation object. */ directResolve?: (decision: UserDecision) => void; + /** When set, the host_bash request should be routed to this specific client. */ + targetClientId?: string; } const pending = new Map(); diff --git a/assistant/src/runtime/routes/client-routes.ts b/assistant/src/runtime/routes/client-routes.ts index edefc56414d..f1288b4f910 100644 --- a/assistant/src/runtime/routes/client-routes.ts +++ b/assistant/src/runtime/routes/client-routes.ts @@ -48,6 +48,7 @@ export const ROUTES: RouteDefinition[] = [ clientId: c.clientId, interfaceId: c.interfaceId, capabilities: c.capabilities, + machineName: c.machineName, connectedAt: c.connectedAt, lastActiveAt: c.lastActiveAt, }), diff --git a/assistant/src/runtime/routes/events-routes.ts b/assistant/src/runtime/routes/events-routes.ts index 13ab4a0fa2d..41eff27b321 100644 --- a/assistant/src/runtime/routes/events-routes.ts +++ b/assistant/src/runtime/routes/events-routes.ts @@ -77,6 +77,7 @@ export function handleSubscribeAssistantEvents( // ── Client identity from headers ────────────────────────────────────── const rawClientId = headers?.["x-vellum-client-id"]; const rawInterfaceId = headers?.["x-vellum-interface-id"]; + const rawMachineName = headers?.["x-vellum-machine-name"]; const clientId = rawClientId?.trim() || null; const interfaceId = clientId ? parseInterfaceId(rawInterfaceId?.trim()) @@ -166,6 +167,7 @@ export function handleSubscribeAssistantEvents( capabilities: ALL_CAPABILITIES.filter((cap) => supportsHostProxy(interfaceId, cap), ), + machineName: rawMachineName?.trim() || undefined, }) : hub.subscribe({ ...subscriberBase, diff --git a/assistant/src/runtime/routes/host-bash-routes.ts b/assistant/src/runtime/routes/host-bash-routes.ts index 19a4fd48a41..f1eab46844b 100644 --- a/assistant/src/runtime/routes/host-bash-routes.ts +++ b/assistant/src/runtime/routes/host-bash-routes.ts @@ -11,6 +11,7 @@ import * as pendingInteractions from "../pending-interactions.js"; import { BadRequestError, ConflictError, + ForbiddenError, NotFoundError, } from "./errors.js"; import type { RouteDefinition, RouteHandlerArgs } from "./types.js"; @@ -19,7 +20,7 @@ import type { RouteDefinition, RouteHandlerArgs } from "./types.js"; // POST /v1/host-bash-result // --------------------------------------------------------------------------- -function handleHostBashResult({ body }: RouteHandlerArgs) { +function handleHostBashResult({ body, headers }: RouteHandlerArgs) { if (!body || typeof body !== "object") { throw new BadRequestError("Request body is required"); } @@ -36,6 +37,8 @@ function handleHostBashResult({ body }: RouteHandlerArgs) { throw new BadRequestError("requestId is required"); } + const submittingClientId = headers?.["x-vellum-client-id"]?.trim() || undefined; + const peeked = pendingInteractions.get(requestId); if (!peeked) { throw new NotFoundError( @@ -49,6 +52,20 @@ function handleHostBashResult({ body }: RouteHandlerArgs) { ); } + const { targetClientId } = peeked; + if (targetClientId) { + if (!submittingClientId) { + throw new BadRequestError( + "x-vellum-client-id header is required for targeted host bash requests", + ); + } + if (submittingClientId !== targetClientId) { + throw new ForbiddenError( + `Client "${submittingClientId}" is not the target for this request (expected "${targetClientId}"). The targeted client must submit the result.`, + ); + } + } + pendingInteractions.resolve(requestId); HostBashProxy.instance.resolve(requestId, { @@ -84,6 +101,16 @@ export const ROUTES: RouteDefinition[] = [ responseBody: z.object({ accepted: z.boolean(), }), + additionalResponses: { + "400": { + description: + "x-vellum-client-id header is missing for a targeted host bash request.", + }, + "403": { + description: + "Submitting client does not match the targeted client for this request.", + }, + }, handler: handleHostBashResult, }, ]; diff --git a/assistant/src/tools/host-terminal/host-shell.ts b/assistant/src/tools/host-terminal/host-shell.ts index 63e3e7a80f4..2a12be61784 100644 --- a/assistant/src/tools/host-terminal/host-shell.ts +++ b/assistant/src/tools/host-terminal/host-shell.ts @@ -18,6 +18,7 @@ import { existsSync } from "node:fs"; import { homedir } from "node:os"; import { isAbsolute } from "node:path"; +import { supportsHostProxy } from "../../channels/types.js"; import { getConfig } from "../../config/loader.js"; import { isCesShellLockdownEnabled } from "../../credential-execution/feature-gates.js"; import { HostBashProxy } from "../../daemon/host-bash-proxy.js"; @@ -25,6 +26,7 @@ import { RiskLevel } from "../../permissions/types.js"; import type { ToolDefinition } from "../../providers/types.js"; import { isUntrustedTrustClass } from "../../runtime/actor-trust-resolver.js"; import { wakeAgentForOpportunity } from "../../runtime/agent-wake.js"; +import { assistantEventHub } from "../../runtime/assistant-event-hub.js"; import { redactSecrets } from "../../security/secret-scanner.js"; import { getLogger } from "../../util/logger.js"; import { @@ -131,6 +133,11 @@ class HostShellTool implements Tool { description: "Run the command in the background on the host machine. The tool returns immediately with a background tool ID. When the process exits, its output is delivered to the conversation as a wake.", }, + target_client_id: { + type: "string", + description: + "ID of the specific client to execute this command on. Required when multiple clients support host_bash; omit when only one client is connected. Obtain IDs from `assistant clients list --capability host_bash`.", + }, }, required: ["command", "activity"], }, @@ -173,6 +180,11 @@ class HostShellTool implements Tool { } const background = input.background === true; + const targetClientId = + typeof input.target_client_id === "string" && input.target_client_id !== "" + ? input.target_client_id + : undefined; + const config = getConfig(); const { shellDefaultTimeoutSec, shellMaxTimeoutSec } = config.timeouts; @@ -190,6 +202,50 @@ class HostShellTool implements Tool { isCesShellLockdownEnabled(config) && isUntrustedTrustClass(context.trustClass); + // Guard: non-host-proxy interfaces need an explicit target when multiple + // capable clients are connected to avoid ambiguous untargeted broadcasts. + const transportInterface = context.transportInterface; + if ( + targetClientId == null && + transportInterface != null && + !supportsHostProxy(transportInterface) && + assistantEventHub.listClientsByCapability("host_bash").length > 1 + ) { + return { + content: `Error: multiple clients support host_bash. Specify which client to use with \`target_client_id\`. Run \`assistant clients list --capability host_bash\` to see client IDs and labels.`, + isError: true, + }; + } + + // Guard: non-host-proxy interfaces with no capable clients connected. + if ( + targetClientId == null && + transportInterface != null && + !supportsHostProxy(transportInterface) && + !HostBashProxy.instance.isAvailable() + ) { + return { + content: + "Error: no client with host_bash capability is connected. Connect a macOS client to use host_bash from a non-desktop interface.", + isError: true, + }; + } + + // Guard: explicit targetClientId provided but proxy is unavailable (client + // disconnected between tool-definition and tool-execution). Without this + // guard both targetClientId != null guards above are bypassed, and the + // code falls through to local daemon execution — silently running commands + // inside the Docker container instead of on the intended host machine. + if ( + targetClientId != null && + !HostBashProxy.instance.isAvailable() + ) { + return { + content: `Error: target client "${targetClientId}" is no longer connected. The specified client may have disconnected since the tool was called. Run \`assistant clients list --capability host_bash\` to see currently connected clients.`, + isError: true, + }; + } + // Proxy to connected client for execution on the user's machine // when a capable client is available (managed/cloud-hosted mode). if (HostBashProxy.instance.isAvailable()) { @@ -227,6 +283,7 @@ class HostShellTool implements Tool { working_dir: rawWorkingDir as string | undefined, timeout_seconds: normalizedTimeout, env: proxyEnv, + targetClientId, }, context.conversationId, abortController.signal, @@ -273,6 +330,7 @@ class HostShellTool implements Tool { working_dir: rawWorkingDir as string | undefined, timeout_seconds: normalizedTimeout, env: proxyEnv, + targetClientId, }, context.conversationId, context.signal, diff --git a/clients/macos/vellum-assistant/App/AppDelegate+ConnectionSetup.swift b/clients/macos/vellum-assistant/App/AppDelegate+ConnectionSetup.swift index 0b9c00cc162..e297909fbed 100644 --- a/clients/macos/vellum-assistant/App/AppDelegate+ConnectionSetup.swift +++ b/clients/macos/vellum-assistant/App/AppDelegate+ConnectionSetup.swift @@ -365,6 +365,19 @@ extension AppDelegate { self.featureFlagStore.reloadFromGateway() // Host tool execution — run locally and post results back case .hostBashRequest(let msg): + // Accept if the request is explicitly targeted at this client, OR if + // the request is untargeted and the conversation is locally owned. + // Do NOT accept if targetClientId is set to a different client, even + // if this conversation is in the local list (all clients sync the same + // conversation list, so isLocalConversation alone is not sufficient). + let localClientId = DeviceIdStore.getOrCreate() + let isLocalConversation = self.mainWindow?.conversationManager + .conversations.contains(where: { $0.conversationId == msg.conversationId }) ?? false + let isTargeted = msg.targetClientId == localClientId + let isUntargetedLocal = msg.targetClientId == nil && isLocalConversation + guard isTargeted || isUntargetedLocal else { + break + } HostToolExecutor.executeHostBashRequest(msg) case .hostFileRequest(let msg): HostToolExecutor.executeHostFileRequest(msg) diff --git a/clients/shared/Network/EventStreamClient.swift b/clients/shared/Network/EventStreamClient.swift index 2930c103fe5..e1d30af9ff8 100644 --- a/clients/shared/Network/EventStreamClient.swift +++ b/clients/shared/Network/EventStreamClient.swift @@ -607,6 +607,9 @@ public final class EventStreamClient { private func shouldIgnoreHostToolRequest(_ message: ServerMessage) -> Bool { switch message { case .hostBashRequest(let msg): + // Targeted cross-client requests carry a non-local conversationId by design. + // Pass them through so AppDelegate+ConnectionSetup can perform the targetClientId check. + if msg.targetClientId != nil { return false } if locallyOwnedConversationIds.contains(msg.conversationId) { return false } log.warning("Ignoring host_bash_request for non-local conversation \(msg.conversationId, privacy: .public)") return true diff --git a/clients/shared/Network/GatewayHTTPClient.swift b/clients/shared/Network/GatewayHTTPClient.swift index 34ec7ab2b13..7cd4dfa9337 100644 --- a/clients/shared/Network/GatewayHTTPClient.swift +++ b/clients/shared/Network/GatewayHTTPClient.swift @@ -106,15 +106,22 @@ public enum GatewayHTTPClient { /// - body: Optional HTTP body data. /// - params: Optional query parameters. Keys and values are percent-encoded /// using a restricted character set that escapes `&`, `=`, `+`, and `#`. + /// - contentType: Optional Content-Type override. Defaults to `application/json`. + /// - extraHeaders: Optional additional headers to include in the request. /// - timeout: Request timeout in seconds. Defaults to 30. /// - Returns: A `Response` with the raw data and HTTP status code. /// - Throws: `ClientError` if the request cannot be constructed, or network errors from `URLSession`. - public static func post(path: String, body: Data? = nil, params: [String: String]? = nil, contentType: String? = nil, timeout: TimeInterval = 30, unprefixed: Bool = false) async throws -> Response { + public static func post(path: String, body: Data? = nil, params: [String: String]? = nil, contentType: String? = nil, extraHeaders: [String: String]? = nil, timeout: TimeInterval = 30, unprefixed: Bool = false) async throws -> Response { return try await executeWithRetry(path: path, params: params, method: "POST", timeout: timeout, unprefixed: unprefixed) { request in request.httpBody = body if let contentType { request.setValue(contentType, forHTTPHeaderField: "Content-Type") } + if let extraHeaders { + for (k, v) in extraHeaders { + request.setValue(v, forHTTPHeaderField: k) + } + } } } @@ -488,6 +495,7 @@ public enum GatewayHTTPClient { request.setValue(sseAcceptHeader, forHTTPHeaderField: "Accept") request.setValue(DeviceIdStore.getOrCreate(), forHTTPHeaderField: "X-Vellum-Client-Id") request.setValue(clientInterfaceId, forHTTPHeaderField: "X-Vellum-Interface-Id") + request.setValue(ProcessInfo.processInfo.hostName, forHTTPHeaderField: "X-Vellum-Machine-Name") logOutgoing(request, quiet: false) let (bytes, response) = try await session.bytes(for: request) if let http = response as? HTTPURLResponse { diff --git a/clients/shared/Network/HostProxyClient.swift b/clients/shared/Network/HostProxyClient.swift index b233a31c213..5b6b7a25d0a 100644 --- a/clients/shared/Network/HostProxyClient.swift +++ b/clients/shared/Network/HostProxyClient.swift @@ -25,6 +25,7 @@ public struct HostProxyClient: HostProxyClientProtocol { let response = try await GatewayHTTPClient.post( path: "host-bash-result", body: body, + extraHeaders: ["X-Vellum-Client-Id": DeviceIdStore.getOrCreate()], timeout: 30 ) guard response.isSuccess else { diff --git a/clients/shared/Network/MessageTypes.swift b/clients/shared/Network/MessageTypes.swift index 5536a746087..002e725ce30 100644 --- a/clients/shared/Network/MessageTypes.swift +++ b/clients/shared/Network/MessageTypes.swift @@ -1546,6 +1546,9 @@ public struct HostBashRequest: Decodable, Sendable { public let timeoutSeconds: Double? /// Extra environment variables to inject into the subprocess (e.g. VELLUM_UNTRUSTED_SHELL). public let env: [String: String]? + /// When set, this request is targeted at a specific client ID. Non-nil only for + /// cross-client proxy requests routed through HostBashProxy. + public let targetClientId: String? private enum CodingKeys: String, CodingKey { case type @@ -1555,6 +1558,7 @@ public struct HostBashRequest: Decodable, Sendable { case workingDir = "working_dir" case timeoutSeconds = "timeout_seconds" case env + case targetClientId } }