diff --git a/assistant/openapi.yaml b/assistant/openapi.yaml index 89b71d6fe0d..bd3985cd492 100644 --- a/assistant/openapi.yaml +++ b/assistant/openapi.yaml @@ -6325,6 +6325,10 @@ paths: required: - accepted additionalProperties: false + "400": + description: x-vellum-client-id header is missing for a targeted host transfer request. + "403": + description: Submitting client does not match the targeted client for this transfer. requestBody: required: true content: @@ -11280,6 +11284,10 @@ paths: responses: "200": description: Successful response + "400": + description: x-vellum-client-id header is missing for a targeted transfer. + "403": + description: Submitting client does not match the targeted client for this transfer. parameters: - name: transferId in: path @@ -11295,6 +11303,10 @@ paths: responses: "200": description: Successful response + "400": + description: x-vellum-client-id header is missing for a targeted transfer. + "403": + description: Submitting client does not match the targeted client for this transfer. parameters: - name: transferId in: path diff --git a/assistant/src/__tests__/host-transfer-proxy-targeted.test.ts b/assistant/src/__tests__/host-transfer-proxy-targeted.test.ts new file mode 100644 index 00000000000..8ebbe5dc8b8 --- /dev/null +++ b/assistant/src/__tests__/host-transfer-proxy-targeted.test.ts @@ -0,0 +1,583 @@ +/** + * Tests for HostTransferProxy targetClientId behaviour (Phase 1). + * + * Covers: + * - requestToHost() explicit valid targetClientId → validates, broadcasts with targetClientId + * - requestToHost() auto-resolve when exactly one host_file-capable client → auto-resolves + * - requestToHost() unknown targetClientId → early error, no broadcast + * - requestToHost() incapable client → early error, no broadcast + * - requestToSandbox() explicit valid targetClientId → same 4 cases + * - Abort path sends host_transfer_cancel with targetClientId + * - cancel(requestId) reads targetClientId from pending interaction + * - dispose() reads targetClientId from pending interaction + * - getTargetClientIdForTransfer() returns correct value after requestToHost() + * - Timeout message includes client ID when resolvedTargetClientId is set + * - Regression: no-targetClientId requestToHost / requestToSandbox (smoke tests) + */ +import { afterEach, describe, expect, jest, mock, test } from "bun:test"; + +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, _conversationId?: string, options?: unknown) => { + sentMessages.push(msg); + sentMessageOptions.push(options); + }, + assistantEventHub: { + getMostRecentClientByCapability: (cap: string) => + cap === "host_file" && mockHasClient ? { id: "mock-client" } : null, + listClientsByCapability: (_cap: string) => mockCapableClients, + getClientById: (clientId: string) => mockClientRegistry.get(clientId), + }, +})); + +const pendingInteractionMap = new Map>(); +mock.module("../runtime/pending-interactions.js", () => ({ + register: (requestId: string, interaction: Record) => { + pendingInteractionMap.set(requestId, interaction); + }, + resolve: (requestId: string) => { + const interaction = pendingInteractionMap.get(requestId); + pendingInteractionMap.delete(requestId); + resolvedInteractionIds.push(requestId); + return interaction; + }, + get: (requestId: string) => pendingInteractionMap.get(requestId), + getByKind: (_kind: string) => + Array.from(pendingInteractionMap.entries()) + .filter(([, v]) => v.kind === _kind) + .map(([requestId, v]) => ({ requestId, ...v })), + getByConversation: () => [], + removeByConversation: () => {}, + clear: () => pendingInteractionMap.clear(), +})); + +const { HostTransferProxy } = await import("../daemon/host-transfer-proxy.js"); + +/** + * Poll until sentMessages reaches the expected length. + * Needed because requestToHost() does async readFile before broadcasting. + */ +async function waitForMessages( + msgs: unknown[], + expectedLength: number, + timeoutMs = 2000, +): Promise { + const start = Date.now(); + while (msgs.length < expectedLength) { + if (Date.now() - start > timeoutMs) { + throw new Error( + `Timed out waiting for ${expectedLength} message(s), got ${msgs.length}`, + ); + } + await new Promise((r) => setTimeout(r, 5)); + } +} + +describe("HostTransferProxy — targetClientId", () => { + let proxy: InstanceType; + + function setup() { + sentMessages.length = 0; + sentMessageOptions.length = 0; + resolvedInteractionIds.length = 0; + pendingInteractionMap.clear(); + mockHasClient = false; + mockCapableClients = []; + mockClientRegistry = new Map(); + HostTransferProxy.reset(); + proxy = new (HostTransferProxy as any)(); + } + + function setupSingleClient(clientId = "client-1") { + const entry = { clientId, capabilities: ["host_file"] }; + mockCapableClients = [entry]; + mockClientRegistry.set(clientId, entry); + } + + afterEach(() => { + proxy?.dispose(); + HostTransferProxy.reset(); + }); + + // ── requestToHost() — explicit valid targetClientId ─────────────────── + + describe("requestToHost() — explicit valid targetClientId", () => { + test("broadcasts with targetClientId in message body and options", async () => { + setup(); + setupSingleClient("client-mac"); + + // Create a fake file for requestToHost to read + const fileContent = "hello"; + const srcPath = `/tmp/htp-targeted-${Date.now()}.txt`; + await globalThis.Bun.write(srcPath, fileContent); + + const resultPromise = proxy.requestToHost({ + sourcePath: srcPath, + destPath: "/host/dest.txt", + overwrite: false, + conversationId: "conv-1", + targetClientId: "client-mac", + }); + + await waitForMessages(sentMessages, 1); + + const sent = sentMessages[0] as Record; + expect(sent.type).toBe("host_transfer_request"); + expect(sent.targetClientId).toBe("client-mac"); + + const opts = sentMessageOptions[0] as Record | undefined; + expect(opts?.targetClientId).toBe("client-mac"); + + const requestId = sent.requestId as string; + proxy.resolveTransferResult(requestId, { isError: false, bytesWritten: 5 }); + + const result = await resultPromise; + expect(result.isError).toBe(false); + }); + }); + + // ── requestToHost() — auto-resolve single capable client ───────────── + + describe("requestToHost() — auto-resolve when exactly one capable client", () => { + test("auto-resolves targetClientId to the single capable client", async () => { + setup(); + setupSingleClient("client-solo"); + + const srcPath = `/tmp/htp-targeted-solo-${Date.now()}.txt`; + await globalThis.Bun.write(srcPath, "content"); + + const resultPromise = proxy.requestToHost({ + sourcePath: srcPath, + destPath: "/host/dest.txt", + overwrite: false, + conversationId: "conv-2", + }); + + await waitForMessages(sentMessages, 1); + + const sent = sentMessages[0] as Record; + expect(sent.targetClientId).toBe("client-solo"); + + const opts = sentMessageOptions[0] as Record | undefined; + expect(opts?.targetClientId).toBe("client-solo"); + + const requestId = sent.requestId as string; + proxy.resolveTransferResult(requestId, { isError: false }); + const result = await resultPromise; + expect(result.isError).toBe(false); + }); + }); + + // ── requestToHost() — unknown targetClientId ───────────────────────── + + describe("requestToHost() — unknown targetClientId", () => { + test("returns error immediately without broadcasting", async () => { + setup(); + // No clients registered + + const result = await proxy.requestToHost({ + sourcePath: "/tmp/file.txt", + destPath: "/host/dest.txt", + overwrite: false, + conversationId: "conv-3", + targetClientId: "client-ghost", + }); + + expect(result.isError).toBe(true); + expect(result.content).toContain("client-ghost"); + expect(result.content).toContain("assistant clients list --capability host_file"); + expect(sentMessages).toHaveLength(0); + }); + }); + + // ── requestToHost() — incapable client ─────────────────────────────── + + describe("requestToHost() — client lacks host_file capability", () => { + test("returns error immediately without broadcasting", async () => { + setup(); + mockClientRegistry.set("client-no-file", { + clientId: "client-no-file", + capabilities: ["host_bash"], + }); + + const result = await proxy.requestToHost({ + sourcePath: "/tmp/file.txt", + destPath: "/host/dest.txt", + overwrite: false, + conversationId: "conv-4", + targetClientId: "client-no-file", + }); + + expect(result.isError).toBe(true); + expect(result.content).toContain("client-no-file"); + expect(result.content).toContain("does not support host_file"); + expect(sentMessages).toHaveLength(0); + }); + }); + + // ── requestToSandbox() — explicit valid targetClientId ──────────────── + + describe("requestToSandbox() — explicit valid targetClientId", () => { + test("broadcasts with targetClientId in message body and options", async () => { + setup(); + setupSingleClient("client-mac"); + + const resultPromise = proxy.requestToSandbox({ + sourcePath: "/host/source.txt", + destPath: "/sandbox/dest.txt", + conversationId: "conv-5", + targetClientId: "client-mac", + }); + + expect(sentMessages).toHaveLength(1); + const sent = sentMessages[0] as Record; + expect(sent.type).toBe("host_transfer_request"); + expect(sent.targetClientId).toBe("client-mac"); + + const opts = sentMessageOptions[0] as Record | undefined; + expect(opts?.targetClientId).toBe("client-mac"); + + // Cancel to resolve the promise + proxy.cancel(sent.requestId as string); + await resultPromise; + }); + }); + + // ── requestToSandbox() — auto-resolve single capable client ────────── + + describe("requestToSandbox() — auto-resolve when exactly one capable client", () => { + test("auto-resolves targetClientId", async () => { + setup(); + setupSingleClient("client-solo"); + + const resultPromise = proxy.requestToSandbox({ + sourcePath: "/host/source.txt", + destPath: "/sandbox/dest.txt", + conversationId: "conv-6", + }); + + expect(sentMessages).toHaveLength(1); + const sent = sentMessages[0] as Record; + expect(sent.targetClientId).toBe("client-solo"); + + proxy.cancel(sent.requestId as string); + await resultPromise; + }); + }); + + // ── requestToSandbox() — unknown targetClientId ────────────────────── + + describe("requestToSandbox() — unknown targetClientId", () => { + test("returns error immediately without broadcasting", async () => { + setup(); + + const result = await proxy.requestToSandbox({ + sourcePath: "/host/source.txt", + destPath: "/sandbox/dest.txt", + conversationId: "conv-7", + targetClientId: "client-ghost", + }); + + expect(result.isError).toBe(true); + expect(result.content).toContain("client-ghost"); + expect(sentMessages).toHaveLength(0); + }); + }); + + // ── requestToSandbox() — incapable client ──────────────────────────── + + describe("requestToSandbox() — client lacks host_file capability", () => { + test("returns error immediately without broadcasting", async () => { + setup(); + mockClientRegistry.set("client-no-file", { + clientId: "client-no-file", + capabilities: ["host_bash"], + }); + + const result = await proxy.requestToSandbox({ + sourcePath: "/host/source.txt", + destPath: "/sandbox/dest.txt", + conversationId: "conv-8", + targetClientId: "client-no-file", + }); + + expect(result.isError).toBe(true); + expect(result.content).toContain("does not support host_file"); + expect(sentMessages).toHaveLength(0); + }); + }); + + // ── Abort path includes targetClientId in cancel (requestToHost) ────── + + describe("abort path — requestToHost sends cancel with targetClientId", () => { + test("cancel broadcast includes targetClientId when request was targeted", async () => { + setup(); + setupSingleClient("client-abc"); + + const srcPath = `/tmp/htp-targeted-abort-${Date.now()}.txt`; + await globalThis.Bun.write(srcPath, "content"); + + const controller = new AbortController(); + const resultPromise = proxy.requestToHost( + { + sourcePath: srcPath, + destPath: "/host/dest.txt", + overwrite: false, + conversationId: "conv-9", + targetClientId: "client-abc", + }, + controller.signal, + ); + + await waitForMessages(sentMessages, 1); + + const sent = sentMessages[0] as Record; + expect(sent.targetClientId).toBe("client-abc"); + + controller.abort(); + const result = await resultPromise; + expect(result.isError).toBe(true); + + // Second message is the cancel + expect(sentMessages).toHaveLength(2); + const cancelMsg = sentMessages[1] as Record; + expect(cancelMsg.type).toBe("host_transfer_cancel"); + expect(cancelMsg.targetClientId).toBe("client-abc"); + + const cancelOpts = sentMessageOptions[1] as Record | undefined; + expect(cancelOpts?.targetClientId).toBe("client-abc"); + }); + }); + + // ── cancel(requestId) reads targetClientId from pending interaction ─── + + describe("cancel() reads targetClientId from pending interaction", () => { + test("cancel broadcast includes targetClientId", async () => { + setup(); + setupSingleClient("client-xyz"); + + const resultPromise = proxy.requestToSandbox({ + sourcePath: "/host/source.txt", + destPath: "/sandbox/dest.txt", + conversationId: "conv-10", + targetClientId: "client-xyz", + }); + + const sent = sentMessages[0] as Record; + const requestId = sent.requestId as string; + expect(sent.targetClientId).toBe("client-xyz"); + + proxy.cancel(requestId); + const result = await resultPromise; + expect(result.isError).toBe(true); + expect(result.content).toBe("Transfer cancelled"); + + // Cancel message + expect(sentMessages).toHaveLength(2); + const cancelMsg = sentMessages[1] as Record; + expect(cancelMsg.type).toBe("host_transfer_cancel"); + expect(cancelMsg.targetClientId).toBe("client-xyz"); + + const cancelOpts = sentMessageOptions[1] as Record | undefined; + expect(cancelOpts?.targetClientId).toBe("client-xyz"); + }); + }); + + // ── dispose() reads targetClientId from pending interaction ─────────── + + describe("dispose() reads targetClientId from pending interaction", () => { + test("dispose cancel broadcast includes targetClientId for targeted request", () => { + setup(); + setupSingleClient("client-dispose"); + + const p = proxy.requestToSandbox({ + sourcePath: "/host/source.txt", + destPath: "/sandbox/dest.txt", + conversationId: "conv-11", + targetClientId: "client-dispose", + }); + p.catch(() => {}); // expected rejection on dispose + + const sent = sentMessages[0] as Record; + expect(sent.targetClientId).toBe("client-dispose"); + const requestId = sent.requestId as string; + + proxy.dispose(); + + const cancelMessages = sentMessages + .slice(1) + .filter( + (m) => (m as Record).type === "host_transfer_cancel", + ) as Array>; + expect(cancelMessages).toHaveLength(1); + expect(cancelMessages[0].requestId).toBe(requestId); + expect(cancelMessages[0].targetClientId).toBe("client-dispose"); + }); + }); + + // ── getTargetClientIdForTransfer() ──────────────────────────────────── + + describe("getTargetClientIdForTransfer()", () => { + test("returns targetClientId after requestToHost()", async () => { + setup(); + setupSingleClient("client-peek"); + + const srcPath = `/tmp/htp-targeted-peek-${Date.now()}.txt`; + await globalThis.Bun.write(srcPath, "content"); + + const resultPromise = proxy.requestToHost({ + sourcePath: srcPath, + destPath: "/host/dest.txt", + overwrite: false, + conversationId: "conv-12", + targetClientId: "client-peek", + }); + + await waitForMessages(sentMessages, 1); + + const sent = sentMessages[0] as Record; + const transferId = sent.transferId as string; + + expect(proxy.getTargetClientIdForTransfer(transferId)).toBe("client-peek"); + + // Clean up + const requestId = sent.requestId as string; + proxy.resolveTransferResult(requestId, { isError: false }); + await resultPromise; + }); + + test("returns null for untargeted transfer", async () => { + setup(); + // No clients — no auto-resolve + + const srcPath = `/tmp/htp-targeted-null-${Date.now()}.txt`; + await globalThis.Bun.write(srcPath, "hello"); + + const resultPromise = proxy.requestToHost({ + sourcePath: srcPath, + destPath: "/host/dest.txt", + overwrite: false, + conversationId: "conv-13", + }); + + await waitForMessages(sentMessages, 1); + + const sent = sentMessages[0] as Record; + const transferId = sent.transferId as string; + + expect(proxy.getTargetClientIdForTransfer(transferId)).toBeNull(); + + const requestId = sent.requestId as string; + proxy.resolveTransferResult(requestId, { isError: false }); + await resultPromise; + }); + + test("returns null for unknown transferId", () => { + setup(); + expect(proxy.getTargetClientIdForTransfer("nonexistent-id")).toBeNull(); + }); + }); + + // ── Timeout message includes clientId ───────────────────────────────── + + describe("timeout message includes clientId when targeted", () => { + test("timeout resolve message mentions resolvedTargetClientId for requestToSandbox", async () => { + setup(); + setupSingleClient("client-timeout"); + + jest.useFakeTimers(); + try { + const resultPromise = proxy.requestToSandbox({ + sourcePath: "/host/source.txt", + destPath: "/sandbox/dest.txt", + conversationId: "conv-14", + targetClientId: "client-timeout", + }); + + const sent = sentMessages[0] as Record; + expect(sent.targetClientId).toBe("client-timeout"); + + // Advance past the 120s default timeout + jest.advanceTimersByTime(121 * 1000); + + const result = await resultPromise; + expect(result.isError).toBe(true); + expect(result.content).toContain("client-timeout"); + } finally { + jest.useRealTimers(); + } + }); + }); + + // ── Regression: no-targetClientId path is unbroken ─────────────────── + + describe("regression — untargeted requestToHost completes normally", () => { + test("no-targetClientId requestToHost resolves successfully", async () => { + setup(); + // Multiple clients so no auto-resolve + mockCapableClients = [ + { clientId: "client-a", capabilities: ["host_file"] }, + { clientId: "client-b", capabilities: ["host_file"] }, + ]; + + const srcPath = `/tmp/htp-regression-tohost-${Date.now()}.txt`; + await globalThis.Bun.write(srcPath, "regression content"); + + const resultPromise = proxy.requestToHost({ + sourcePath: srcPath, + destPath: "/host/dest.txt", + overwrite: false, + conversationId: "conv-reg-1", + }); + + await waitForMessages(sentMessages, 1); + + const sent = sentMessages[0] as Record; + expect(sent.type).toBe("host_transfer_request"); + expect(sent.targetClientId).toBeUndefined(); + + const opts = sentMessageOptions[0] as Record | undefined; + expect(opts?.targetClientId).toBeUndefined(); + + const requestId = sent.requestId as string; + proxy.resolveTransferResult(requestId, { isError: false, bytesWritten: 18 }); + + const result = await resultPromise; + expect(result.isError).toBe(false); + expect(result.content).toContain("successfully"); + }); + }); + + describe("regression — untargeted requestToSandbox completes normally", () => { + test("no-targetClientId requestToSandbox resolves via cancel", async () => { + setup(); + // Multiple clients so no auto-resolve + mockCapableClients = [ + { clientId: "client-a", capabilities: ["host_file"] }, + { clientId: "client-b", capabilities: ["host_file"] }, + ]; + + const resultPromise = proxy.requestToSandbox({ + sourcePath: "/host/source.txt", + destPath: "/sandbox/dest.txt", + conversationId: "conv-reg-2", + }); + + expect(sentMessages).toHaveLength(1); + const sent = sentMessages[0] as Record; + expect(sent.type).toBe("host_transfer_request"); + expect(sent.targetClientId).toBeUndefined(); + + proxy.cancel(sent.requestId as string); + const result = await resultPromise; + expect(result.isError).toBe(true); + expect(result.content).toBe("Transfer cancelled"); + }); + }); +}); diff --git a/assistant/src/__tests__/host-transfer-proxy.test.ts b/assistant/src/__tests__/host-transfer-proxy.test.ts index 886644c21e9..28188e3ce66 100644 --- a/assistant/src/__tests__/host-transfer-proxy.test.ts +++ b/assistant/src/__tests__/host-transfer-proxy.test.ts @@ -12,6 +12,9 @@ mock.module("../runtime/assistant-event-hub.js", () => ({ assistantEventHub: { getMostRecentClientByCapability: (cap: string) => cap === "host_file" && mockHasClient ? { id: "mock-client" } : null, + listClientsByCapability: (_cap: string) => + mockHasClient ? [{ clientId: "mock-client", capabilities: ["host_file"] }] : [], + getClientById: (_id: string) => null, }, })); diff --git a/assistant/src/__tests__/host-transfer-routes-targeted.test.ts b/assistant/src/__tests__/host-transfer-routes-targeted.test.ts new file mode 100644 index 00000000000..cd27f8df321 --- /dev/null +++ b/assistant/src/__tests__/host-transfer-routes-targeted.test.ts @@ -0,0 +1,447 @@ +/** + * Tests for the host-transfer route 403 guard introduced in Phase 3. + * + * Covers GET /transfers/:transferId/content, PUT /transfers/:transferId/content, + * and POST /host-transfer-result ownership checks. + * + * 1. Targeted + correct x-vellum-client-id header → success + * 2. Targeted + missing header → 400 BadRequestError + * 3. Targeted + wrong header → 403 ForbiddenError, operation NOT performed + * 4. Untargeted (no targetClientId, no header) → success (regression) + */ +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"; + +const pendingStore = new Map(); + +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); + return entry; + }, +})); + +// Per-test controls for the proxy stub +let stubTargetClientId: string | null = null; +const getTransferContentCalls: string[] = []; +const receiveTransferContentCalls: string[] = []; +const resolveTransferResultCalls: string[] = []; + +mock.module("../daemon/host-transfer-proxy.js", () => ({ + HostTransferProxy: { + get instance() { + return { + getRequestIdForTransfer(_transferId: string) { + return "req-1"; + }, + getTargetClientIdForTransfer(_transferId: string) { + return stubTargetClientId; + }, + getTransferContent(transferId: string) { + getTransferContentCalls.push(transferId); + return { buffer: Buffer.from("data"), sizeBytes: 4, sha256: "abc123" }; + }, + async receiveTransferContent(transferId: string, _data: Buffer, _sha256: string) { + receiveTransferContentCalls.push(transferId); + return { accepted: true }; + }, + resolveTransferResult(requestId: string, _result: unknown) { + resolveTransferResultCalls.push(requestId); + }, + }; + }, + }, +})); + +// ── Real imports (after mocks) ────────────────────────────────────────────── + +import { + BadRequestError, + ForbiddenError, +} from "../runtime/routes/errors.js"; +import { ROUTES } from "../runtime/routes/host-transfer-routes.js"; + +afterAll(() => { + mock.restore(); +}); + +const handleTransferContentGet = ROUTES.find( + (r) => r.endpoint === "transfers/:transferId/content" && r.method === "GET", +)!.handler; + +const handleTransferContentPut = ROUTES.find( + (r) => r.endpoint === "transfers/:transferId/content" && r.method === "PUT", +)!.handler; + +const handleTransferResult = ROUTES.find( + (r) => r.endpoint === "host-transfer-result", +)!.handler; + +// ── Helpers ───────────────────────────────────────────────────────────────── + +const TEST_TRANSFER_ID = "transfer-abc"; +const TEST_REQUEST_ID = "req-1"; + +function registerPending(overrides: Partial = {}): void { + pendingStore.set(TEST_REQUEST_ID, { + conversationId: "conv-1", + kind: "host_transfer", + ...overrides, + }); +} + +// ── Tests ──────────────────────────────────────────────────────────────────── + +describe("handleTransferContentGet — Phase 3 targetClientId guard", () => { + beforeEach(() => { + pendingStore.clear(); + stubTargetClientId = null; + getTransferContentCalls.length = 0; + }); + + // ── 1. Targeted + correct header → success ──────────────────────────────── + + describe("targeted + correct x-vellum-client-id header", () => { + test("returns Uint8Array and calls getTransferContent", async () => { + stubTargetClientId = "client-A"; + const result = await handleTransferContentGet({ + pathParams: { transferId: TEST_TRANSFER_ID }, + headers: { "x-vellum-client-id": "client-A" }, + }); + + expect(result).toBeInstanceOf(Uint8Array); + expect(getTransferContentCalls).toContain(TEST_TRANSFER_ID); + }); + + test("trims whitespace from header before comparing", async () => { + stubTargetClientId = "client-A"; + const result = await handleTransferContentGet({ + pathParams: { transferId: TEST_TRANSFER_ID }, + headers: { "x-vellum-client-id": " client-A " }, + }); + + expect(result).toBeInstanceOf(Uint8Array); + }); + }); + + // ── 2. Targeted + missing header → 400 ─────────────────────────────────── + + describe("targeted + missing x-vellum-client-id header", () => { + test("throws BadRequestError when header is absent", () => { + stubTargetClientId = "client-A"; + expect(() => + handleTransferContentGet({ + pathParams: { transferId: TEST_TRANSFER_ID }, + }), + ).toThrow(BadRequestError); + }); + + test("getTransferContent NOT called on 400", () => { + stubTargetClientId = "client-A"; + try { + handleTransferContentGet({ + pathParams: { transferId: TEST_TRANSFER_ID }, + }); + } catch { + // expected + } + expect(getTransferContentCalls).toHaveLength(0); + }); + }); + + // ── 3. Targeted + wrong header → 403 ───────────────────────────────────── + + describe("targeted + wrong x-vellum-client-id header", () => { + test("throws ForbiddenError when client ID does not match", () => { + stubTargetClientId = "client-A"; + expect(() => + handleTransferContentGet({ + pathParams: { transferId: TEST_TRANSFER_ID }, + headers: { "x-vellum-client-id": "client-B" }, + }), + ).toThrow(ForbiddenError); + }); + + test("getTransferContent NOT called on 403", () => { + stubTargetClientId = "client-A"; + try { + handleTransferContentGet({ + pathParams: { transferId: TEST_TRANSFER_ID }, + headers: { "x-vellum-client-id": "client-B" }, + }); + } catch { + // expected + } + expect(getTransferContentCalls).toHaveLength(0); + }); + }); + + // ── 4. Untargeted — regression ──────────────────────────────────────────── + + describe("untargeted request (no targetClientId)", () => { + test("returns Uint8Array without a header", async () => { + stubTargetClientId = null; + const result = await handleTransferContentGet({ + pathParams: { transferId: TEST_TRANSFER_ID }, + }); + + expect(result).toBeInstanceOf(Uint8Array); + expect(getTransferContentCalls).toContain(TEST_TRANSFER_ID); + }); + }); +}); + +describe("handleTransferContentPut — Phase 3 targetClientId guard", () => { + beforeEach(() => { + pendingStore.clear(); + stubTargetClientId = null; + receiveTransferContentCalls.length = 0; + }); + + // ── 1. Targeted + correct header → success ──────────────────────────────── + + describe("targeted + correct x-vellum-client-id header", () => { + test("returns { accepted: true } and calls receiveTransferContent", async () => { + stubTargetClientId = "client-A"; + const result = await handleTransferContentPut({ + pathParams: { transferId: TEST_TRANSFER_ID }, + headers: { "x-vellum-client-id": "client-A", "x-transfer-sha256": "abc" }, + rawBody: new Uint8Array(Buffer.from("data")), + }); + + expect(result).toEqual({ accepted: true }); + expect(receiveTransferContentCalls).toContain(TEST_TRANSFER_ID); + }); + + test("trims whitespace from header before comparing", async () => { + stubTargetClientId = "client-A"; + const result = await handleTransferContentPut({ + pathParams: { transferId: TEST_TRANSFER_ID }, + headers: { "x-vellum-client-id": " client-A ", "x-transfer-sha256": "abc" }, + rawBody: new Uint8Array(Buffer.from("data")), + }); + + expect(result).toEqual({ accepted: true }); + }); + }); + + // ── 2. Targeted + missing header → 400 ─────────────────────────────────── + + describe("targeted + missing x-vellum-client-id header", () => { + test("throws BadRequestError when header is absent", async () => { + stubTargetClientId = "client-A"; + await expect( + handleTransferContentPut({ + pathParams: { transferId: TEST_TRANSFER_ID }, + headers: { "x-transfer-sha256": "abc" }, + rawBody: new Uint8Array(Buffer.from("data")), + }), + ).rejects.toBeInstanceOf(BadRequestError); + }); + + test("receiveTransferContent NOT called on 400", async () => { + stubTargetClientId = "client-A"; + try { + await handleTransferContentPut({ + pathParams: { transferId: TEST_TRANSFER_ID }, + headers: { "x-transfer-sha256": "abc" }, + rawBody: new Uint8Array(Buffer.from("data")), + }); + } catch { + // expected + } + expect(receiveTransferContentCalls).toHaveLength(0); + }); + }); + + // ── 3. Targeted + wrong header → 403 ───────────────────────────────────── + + describe("targeted + wrong x-vellum-client-id header", () => { + test("throws ForbiddenError when client ID does not match", async () => { + stubTargetClientId = "client-A"; + await expect( + handleTransferContentPut({ + pathParams: { transferId: TEST_TRANSFER_ID }, + headers: { "x-vellum-client-id": "client-B", "x-transfer-sha256": "abc" }, + rawBody: new Uint8Array(Buffer.from("data")), + }), + ).rejects.toBeInstanceOf(ForbiddenError); + }); + + test("receiveTransferContent NOT called on 403", async () => { + stubTargetClientId = "client-A"; + try { + await handleTransferContentPut({ + pathParams: { transferId: TEST_TRANSFER_ID }, + headers: { "x-vellum-client-id": "client-B", "x-transfer-sha256": "abc" }, + rawBody: new Uint8Array(Buffer.from("data")), + }); + } catch { + // expected + } + expect(receiveTransferContentCalls).toHaveLength(0); + }); + }); + + // ── 4. Untargeted — regression ──────────────────────────────────────────── + + describe("untargeted request (no targetClientId)", () => { + test("returns { accepted: true } without a header", async () => { + stubTargetClientId = null; + const result = await handleTransferContentPut({ + pathParams: { transferId: TEST_TRANSFER_ID }, + headers: { "x-transfer-sha256": "abc" }, + rawBody: new Uint8Array(Buffer.from("data")), + }); + + expect(result).toEqual({ accepted: true }); + expect(receiveTransferContentCalls).toContain(TEST_TRANSFER_ID); + }); + }); +}); + +describe("handleTransferResult — Phase 3 targetClientId guard", () => { + beforeEach(() => { + pendingStore.clear(); + stubTargetClientId = null; + resolveTransferResultCalls.length = 0; + }); + + function registerHostTransferPending(targetClientId?: string): void { + registerPending({ targetClientId }); + } + + function resultBody(): Record { + return { requestId: TEST_REQUEST_ID }; + } + + // ── 1. Targeted + correct header → success ──────────────────────────────── + + describe("targeted + correct x-vellum-client-id header", () => { + test("returns { accepted: true } and calls resolveTransferResult", async () => { + registerHostTransferPending("client-A"); + const result = await handleTransferResult({ + body: resultBody(), + headers: { "x-vellum-client-id": "client-A" }, + }); + + expect(result).toEqual({ accepted: true }); + expect(resolveTransferResultCalls).toContain(TEST_REQUEST_ID); + }); + + test("trims whitespace from header before comparing", async () => { + registerHostTransferPending("client-A"); + const result = await handleTransferResult({ + body: resultBody(), + headers: { "x-vellum-client-id": " client-A " }, + }); + + expect(result).toEqual({ accepted: true }); + }); + }); + + // ── 2. Targeted + missing header → 400 ─────────────────────────────────── + + describe("targeted + missing x-vellum-client-id header", () => { + test("throws BadRequestError when header is absent", () => { + registerHostTransferPending("client-A"); + expect(() => + handleTransferResult({ body: resultBody() }), + ).toThrow(BadRequestError); + }); + + test("resolveTransferResult NOT called on 400", () => { + registerHostTransferPending("client-A"); + try { + handleTransferResult({ body: resultBody() }); + } catch { + // expected + } + expect(resolveTransferResultCalls).toHaveLength(0); + }); + + test("pending interaction still present after 400", () => { + registerHostTransferPending("client-A"); + try { + handleTransferResult({ body: resultBody() }); + } catch { + // expected + } + expect(pendingStore.has(TEST_REQUEST_ID)).toBe(true); + }); + }); + + // ── 3. Targeted + wrong header → 403 ───────────────────────────────────── + + describe("targeted + wrong x-vellum-client-id header", () => { + test("throws ForbiddenError when client ID does not match", () => { + registerHostTransferPending("client-A"); + expect(() => + handleTransferResult({ + body: resultBody(), + headers: { "x-vellum-client-id": "client-B" }, + }), + ).toThrow(ForbiddenError); + }); + + test("resolveTransferResult NOT called on 403", () => { + registerHostTransferPending("client-A"); + try { + handleTransferResult({ + body: resultBody(), + headers: { "x-vellum-client-id": "client-B" }, + }); + } catch { + // expected + } + expect(resolveTransferResultCalls).toHaveLength(0); + }); + + test("pending interaction still present after 403", () => { + registerHostTransferPending("client-A"); + try { + handleTransferResult({ + body: resultBody(), + headers: { "x-vellum-client-id": "client-B" }, + }); + } catch { + // expected + } + expect(pendingStore.has(TEST_REQUEST_ID)).toBe(true); + }); + }); + + // ── 4. Untargeted — regression ──────────────────────────────────────────── + + describe("untargeted request (no targetClientId)", () => { + test("accepts when no header is provided", async () => { + registerHostTransferPending(); + const result = await handleTransferResult({ + body: resultBody(), + }); + + expect(result).toEqual({ accepted: true }); + expect(resolveTransferResultCalls).toContain(TEST_REQUEST_ID); + }); + + test("accepts when header is present (header ignored for untargeted)", async () => { + registerHostTransferPending(); + const result = await handleTransferResult({ + body: resultBody(), + headers: { "x-vellum-client-id": "client-whatever" }, + }); + + expect(result).toEqual({ accepted: true }); + }); + }); +}); diff --git a/assistant/src/daemon/host-transfer-proxy.ts b/assistant/src/daemon/host-transfer-proxy.ts index 47579503ec9..373f1b624f0 100644 --- a/assistant/src/daemon/host-transfer-proxy.ts +++ b/assistant/src/daemon/host-transfer-proxy.ts @@ -30,6 +30,7 @@ interface TransferEntry { sizeBytes?: number; sha256?: string; fileBuffer?: Buffer; + targetClientId?: string; } /** @@ -106,6 +107,7 @@ export class HostTransferProxy { destPath: string; overwrite: boolean; conversationId: string; + targetClientId?: string; }, signal?: AbortSignal, ): Promise { @@ -113,6 +115,26 @@ export class HostTransferProxy { return Promise.resolve({ content: "Aborted", isError: true }); } + let resolvedTargetClientId: string | undefined = input.targetClientId; + if (resolvedTargetClientId != null) { + const client = assistantEventHub.getClientById(resolvedTargetClientId); + if (!client) { + return Promise.resolve({ + content: `No connected client with id '${resolvedTargetClientId}' supports host_file. Run \`assistant clients list --capability host_file\` to see available clients.`, + isError: true, + }); + } + if (!client.capabilities.includes("host_file")) { + return Promise.resolve({ + content: `Client '${resolvedTargetClientId}' does not support host_file. Run \`assistant clients list --capability host_file\` to see available clients.`, + isError: true, + }); + } + } else { + const capable = assistantEventHub.listClientsByCapability("host_file"); + if (capable.length === 1) resolvedTargetClientId = capable[0].clientId; + } + const requestId = uuid(); const transferId = uuid(); @@ -140,8 +162,9 @@ export class HostTransferProxy { "Host transfer proxy request timed out", ); resolve({ - content: - "Host transfer proxy timed out waiting for client response", + content: resolvedTargetClientId + ? `Host transfer proxy timed out waiting for response from client '${resolvedTargetClientId}'` + : "Host transfer proxy timed out waiting for client response", isError: true, }); }, timeoutMs); @@ -152,11 +175,18 @@ export class HostTransferProxy { this.transfers.delete(transferId); pendingInteractions.resolve(requestId); try { - broadcastMessage({ - type: "host_transfer_cancel", - requestId, - conversationId: input.conversationId, - }); + broadcastMessage( + { + type: "host_transfer_cancel", + requestId, + conversationId: input.conversationId, + ...(resolvedTargetClientId != null + ? { targetClientId: resolvedTargetClientId } + : {}), + }, + input.conversationId, + { targetClientId: resolvedTargetClientId }, + ); } catch { // Best-effort cancel notification } @@ -175,11 +205,13 @@ export class HostTransferProxy { sizeBytes, sha256, fileBuffer, + targetClientId: resolvedTargetClientId, }); pendingInteractions.register(requestId, { conversationId: input.conversationId, kind: "host_transfer", + targetClientId: resolvedTargetClientId, rpcResolve: resolve, rpcReject: reject, timer, @@ -188,17 +220,24 @@ export class HostTransferProxy { }); try { - broadcastMessage({ - type: "host_transfer_request", - requestId, - conversationId: input.conversationId, - direction: "to_host", - transferId, - destPath: input.destPath, - sizeBytes, - sha256, - overwrite: input.overwrite, - }); + broadcastMessage( + { + type: "host_transfer_request", + requestId, + conversationId: input.conversationId, + direction: "to_host", + transferId, + destPath: input.destPath, + sizeBytes, + sha256, + overwrite: input.overwrite, + ...(resolvedTargetClientId != null + ? { targetClientId: resolvedTargetClientId } + : {}), + }, + input.conversationId, + { targetClientId: resolvedTargetClientId }, + ); } catch (err) { this.transfers.delete(transferId); pendingInteractions.resolve(requestId); @@ -235,6 +274,7 @@ export class HostTransferProxy { destPath: string; overwrite?: boolean; conversationId: string; + targetClientId?: string; }, signal?: AbortSignal, ): Promise { @@ -242,6 +282,26 @@ export class HostTransferProxy { return Promise.resolve({ content: "Aborted", isError: true }); } + let resolvedTargetClientId: string | undefined = input.targetClientId; + if (resolvedTargetClientId != null) { + const client = assistantEventHub.getClientById(resolvedTargetClientId); + if (!client) { + return Promise.resolve({ + content: `No connected client with id '${resolvedTargetClientId}' supports host_file. Run \`assistant clients list --capability host_file\` to see available clients.`, + isError: true, + }); + } + if (!client.capabilities.includes("host_file")) { + return Promise.resolve({ + content: `Client '${resolvedTargetClientId}' does not support host_file. Run \`assistant clients list --capability host_file\` to see available clients.`, + isError: true, + }); + } + } else { + const capable = assistantEventHub.listClientsByCapability("host_file"); + if (capable.length === 1) resolvedTargetClientId = capable[0].clientId; + } + const requestId = uuid(); const transferId = uuid(); @@ -258,7 +318,9 @@ export class HostTransferProxy { "Host transfer proxy request timed out", ); resolve({ - content: "Host transfer proxy timed out waiting for client response", + content: resolvedTargetClientId + ? `Host transfer proxy timed out waiting for response from client '${resolvedTargetClientId}'` + : "Host transfer proxy timed out waiting for client response", isError: true, }); }, timeoutMs); @@ -269,11 +331,18 @@ export class HostTransferProxy { this.transfers.delete(transferId); pendingInteractions.resolve(requestId); try { - broadcastMessage({ - type: "host_transfer_cancel", - requestId, - conversationId: input.conversationId, - }); + broadcastMessage( + { + type: "host_transfer_cancel", + requestId, + conversationId: input.conversationId, + ...(resolvedTargetClientId != null + ? { targetClientId: resolvedTargetClientId } + : {}), + }, + input.conversationId, + { targetClientId: resolvedTargetClientId }, + ); } catch { // Best-effort cancel notification } @@ -290,11 +359,13 @@ export class HostTransferProxy { direction: "to_sandbox", filePath: input.destPath, overwrite: input.overwrite, + targetClientId: resolvedTargetClientId, }); pendingInteractions.register(requestId, { conversationId: input.conversationId, kind: "host_transfer", + targetClientId: resolvedTargetClientId, rpcResolve: resolve, rpcReject: reject, timer, @@ -303,14 +374,21 @@ export class HostTransferProxy { }); try { - broadcastMessage({ - type: "host_transfer_request", - requestId, - conversationId: input.conversationId, - direction: "to_sandbox", - transferId, - sourcePath: input.sourcePath, - }); + broadcastMessage( + { + type: "host_transfer_request", + requestId, + conversationId: input.conversationId, + direction: "to_sandbox", + transferId, + sourcePath: input.sourcePath, + ...(resolvedTargetClientId != null + ? { targetClientId: resolvedTargetClientId } + : {}), + }, + input.conversationId, + { targetClientId: resolvedTargetClientId }, + ); } catch (err) { this.transfers.delete(transferId); pendingInteractions.resolve(requestId); @@ -458,11 +536,18 @@ export class HostTransferProxy { if (transferId) this.transfers.delete(transferId); pendingInteractions.resolve(requestId); try { - broadcastMessage({ - type: "host_transfer_cancel", - requestId, - conversationId: interaction.conversationId, - }); + broadcastMessage( + { + type: "host_transfer_cancel", + requestId, + conversationId: interaction.conversationId, + ...(interaction.targetClientId != null + ? { targetClientId: interaction.targetClientId } + : {}), + }, + interaction.conversationId, + { targetClientId: interaction.targetClientId }, + ); } catch { // Best-effort cancel notification } @@ -483,17 +568,33 @@ export class HostTransferProxy { return entry?.requestId ?? null; } + /** + * Look up the targetClientId for a given transferId without consuming the entry. + * Routes call this to verify ownership without affecting the transfer state. + * Returns null when untargeted (no validation needed). + */ + getTargetClientIdForTransfer(transferId: string): string | null { + return this.transfers.get(transferId)?.targetClientId ?? null; + } + dispose(): void { for (const entry of pendingInteractions.getByKind("host_transfer")) { const transferId = entry.metadata?.transferId as string | undefined; if (transferId) this.transfers.delete(transferId); pendingInteractions.resolve(entry.requestId); try { - broadcastMessage({ - type: "host_transfer_cancel", - requestId: entry.requestId, - conversationId: entry.conversationId, - }); + broadcastMessage( + { + type: "host_transfer_cancel", + requestId: entry.requestId, + conversationId: entry.conversationId, + ...(entry.targetClientId != null + ? { targetClientId: entry.targetClientId } + : {}), + }, + entry.conversationId, + { targetClientId: entry.targetClientId }, + ); } catch { // Best-effort cancel notification } diff --git a/assistant/src/daemon/message-types/host-transfer.ts b/assistant/src/daemon/message-types/host-transfer.ts index d214118d4b8..cea77071dd6 100644 --- a/assistant/src/daemon/message-types/host-transfer.ts +++ b/assistant/src/daemon/message-types/host-transfer.ts @@ -8,6 +8,7 @@ export interface HostTransferToHostRequest { type: "host_transfer_request"; requestId: string; conversationId: string; + targetClientId?: string; direction: "to_host"; transferId: string; destPath: string; @@ -20,6 +21,7 @@ export interface HostTransferToSandboxRequest { type: "host_transfer_request"; requestId: string; conversationId: string; + targetClientId?: string; direction: "to_sandbox"; transferId: string; sourcePath: string; @@ -33,6 +35,7 @@ export interface HostTransferCancelRequest { type: "host_transfer_cancel"; requestId: string; conversationId: string; + targetClientId?: string; } // --- Domain-level union aliases (consumed by the barrel file) --- diff --git a/assistant/src/runtime/routes/host-transfer-routes.ts b/assistant/src/runtime/routes/host-transfer-routes.ts index 65243593e4f..dbbf22d7556 100644 --- a/assistant/src/runtime/routes/host-transfer-routes.ts +++ b/assistant/src/runtime/routes/host-transfer-routes.ts @@ -9,7 +9,7 @@ import { z } from "zod"; import { HostTransferProxy } from "../../daemon/host-transfer-proxy.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,6 +30,7 @@ function findProxyByTransferId(transferId: string) { function handleTransferContentGet({ pathParams = {}, + headers = {}, }: RouteHandlerArgs): Uint8Array { const transferId = pathParams.transferId; if (!transferId) { @@ -41,6 +42,13 @@ function handleTransferContentGet({ throw new NotFoundError("Unknown or consumed transfer"); } + const targetClientId = match.proxy.getTargetClientIdForTransfer(transferId); + if (targetClientId != null) { + const submittingClientId = (headers as Record)["x-vellum-client-id"]?.trim() || undefined; + if (!submittingClientId) throw new BadRequestError("x-vellum-client-id header required for targeted transfer"); + if (submittingClientId !== targetClientId) throw new ForbiddenError(`Client "${submittingClientId}" is not the owner of this transfer`); + } + const content = match.proxy.getTransferContent(transferId); if (!content) { throw new NotFoundError("Unknown or consumed transfer"); @@ -94,6 +102,13 @@ async function handleTransferContentPut({ throw new NotFoundError("Unknown or consumed transfer"); } + const targetClientId = match.proxy.getTargetClientIdForTransfer(transferId); + if (targetClientId != null) { + const submittingClientId = (headers as Record)["x-vellum-client-id"]?.trim() || undefined; + if (!submittingClientId) throw new BadRequestError("x-vellum-client-id header required for targeted transfer"); + if (submittingClientId !== targetClientId) throw new ForbiddenError(`Client "${submittingClientId}" is not the owner of this transfer`); + } + const data = rawBody ? Buffer.from(rawBody) : Buffer.alloc(0); const sha256 = headers["x-transfer-sha256"] ?? ""; @@ -114,7 +129,7 @@ async function handleTransferContentPut({ // POST /v1/host-transfer-result // --------------------------------------------------------------------------- -function handleTransferResult({ body }: RouteHandlerArgs) { +function handleTransferResult({ body, headers }: RouteHandlerArgs) { if (!body || typeof body !== "object") { throw new BadRequestError("Request body is required"); } @@ -141,6 +156,13 @@ function handleTransferResult({ body }: RouteHandlerArgs) { ); } + if (peeked.targetClientId != null) { + const rawClientId = (headers as Record)?.["x-vellum-client-id"]; + const submittingClientId = rawClientId?.trim() || undefined; + if (!submittingClientId) throw new BadRequestError("x-vellum-client-id header is missing for a targeted host transfer request."); + if (submittingClientId !== peeked.targetClientId) throw new ForbiddenError(`Client "${submittingClientId}" is not the target for this request (expected "${peeked.targetClientId}").`); + } + HostTransferProxy.instance.resolveTransferResult(requestId, { isError: isError ?? false, bytesWritten, @@ -166,6 +188,16 @@ export const ROUTES: RouteDefinition[] = [ "Serve raw file bytes for a to_host transfer. Single-use: returns 404 after first consumption.", tags: ["host-transfer"], responseHeaders: resolveTransferContentGetHeaders, + additionalResponses: { + "400": { + description: + "x-vellum-client-id header is missing for a targeted transfer.", + }, + "403": { + description: + "Submitting client does not match the targeted client for this transfer.", + }, + }, handler: handleTransferContentGet, }, { @@ -178,6 +210,16 @@ export const ROUTES: RouteDefinition[] = [ description: "Receive raw file bytes for a to_sandbox transfer. Verifies SHA-256 integrity via the X-Transfer-SHA256 header.", tags: ["host-transfer"], + additionalResponses: { + "400": { + description: + "x-vellum-client-id header is missing for a targeted transfer.", + }, + "403": { + description: + "Submitting client does not match the targeted client for this transfer.", + }, + }, handler: handleTransferContentPut, }, { @@ -198,6 +240,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 transfer request.", + }, + "403": { + description: + "Submitting client does not match the targeted client for this transfer.", + }, + }, handler: handleTransferResult, }, ]; diff --git a/assistant/src/tools/host-filesystem/transfer.ts b/assistant/src/tools/host-filesystem/transfer.ts index 71bbf20d980..2a5f0a5e343 100644 --- a/assistant/src/tools/host-filesystem/transfer.ts +++ b/assistant/src/tools/host-filesystem/transfer.ts @@ -2,16 +2,18 @@ import { constants } from "node:fs"; import { copyFile, lstat, mkdir, realpath } from "node:fs/promises"; import { dirname, isAbsolute } from "node:path"; +import { supportsHostProxy } from "../../channels/types.js"; import { HostTransferProxy } from "../../daemon/host-transfer-proxy.js"; import { RiskLevel } from "../../permissions/types.js"; import type { ToolDefinition } from "../../providers/types.js"; +import { assistantEventHub } from "../../runtime/assistant-event-hub.js"; import { sandboxPolicy } from "../shared/filesystem/path-policy.js"; import type { Tool, ToolContext, ToolExecutionResult } from "../types.js"; class HostFileTransferTool implements Tool { name = "host_file_transfer"; description = - "Copy a file between the assistant's workspace and the user's host machine. Set direction to 'to_host' to send a workspace file to the host, or 'to_sandbox' to pull a host file into the workspace."; + "Copy a file between the assistant's workspace and the user's host machine. Set direction to 'to_host' to send a workspace file to the host, or 'to_sandbox' to pull a host file into the workspace. When multiple clients support host_file, specify which one to use with target_client_id."; category = "host-filesystem"; defaultRiskLevel = RiskLevel.Medium; @@ -48,6 +50,11 @@ class HostFileTransferTool implements Tool { description: "Brief description of why the file is being transferred (for audit logging)", }, + target_client_id: { + type: "string", + description: + "ID of the specific client to transfer files to/from. Required when multiple clients support host_file; omit when only one is connected. Obtain IDs from `assistant clients list --capability host_file`.", + }, }, required: ["source_path", "dest_path", "direction"], }, @@ -85,6 +92,20 @@ class HostFileTransferTool implements Tool { const overwrite = input.overwrite === true; + const targetClientId = + typeof input.target_client_id === "string" && input.target_client_id !== "" + ? input.target_client_id + : undefined; + + if ( + targetClientId == null && + context.transportInterface != null && + !supportsHostProxy(context.transportInterface) && + assistantEventHub.listClientsByCapability("host_file").length > 1 + ) { + return { content: `Error: multiple clients support host_file. Specify which client to use with \`target_client_id\`. Run \`assistant clients list --capability host_file\` to see client IDs and labels.`, isError: true }; + } + // Validate that host-side paths are absolute. if (direction === "to_host" && !isAbsolute(destPath)) { return { @@ -134,6 +155,7 @@ class HostFileTransferTool implements Tool { destPath, overwrite, conversationId: context.conversationId, + targetClientId, }, context.signal, ); @@ -144,11 +166,19 @@ class HostFileTransferTool implements Tool { destPath: resolvedDestPath, overwrite, conversationId: context.conversationId, + targetClientId, }, context.signal, ); } + if (targetClientId != null) { + return { + content: `Error: target_client_id '${targetClientId}' was specified but no host client is available. Ensure the client is connected.`, + isError: true, + }; + } + // Local mode: direct filesystem copy. return this.executeLocal(resolvedSourcePath, resolvedDestPath, overwrite); } diff --git a/clients/macos/vellum-assistant/App/AppDelegate+ConnectionSetup.swift b/clients/macos/vellum-assistant/App/AppDelegate+ConnectionSetup.swift index 68559a8b6dd..4cf7d2d9312 100644 --- a/clients/macos/vellum-assistant/App/AppDelegate+ConnectionSetup.swift +++ b/clients/macos/vellum-assistant/App/AppDelegate+ConnectionSetup.swift @@ -446,6 +446,12 @@ extension AppDelegate { self.hostBrowserExecutor.cancel(msg.requestId) case .hostTransferRequest(let msg): + 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.executeHostTransferRequest(msg) case .hostTransferCancel(let msg): HostToolExecutor.cancelHostTransferRequest(msg.requestId) diff --git a/clients/shared/Network/EventStreamClient.swift b/clients/shared/Network/EventStreamClient.swift index 5e91507c6bc..40fa2c710a1 100644 --- a/clients/shared/Network/EventStreamClient.swift +++ b/clients/shared/Network/EventStreamClient.swift @@ -634,6 +634,7 @@ public final class EventStreamClient { log.warning("Ignoring host_browser_request for non-local conversation \(msg.conversationId, privacy: .public)") return true case .hostTransferRequest(let msg): + if msg.targetClientId != nil { return false } // pass through targeted requests if locallyOwnedConversationIds.contains(msg.conversationId) { return false } log.warning("Ignoring host_transfer_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 7cd4dfa9337..ea23e4ad80a 100644 --- a/clients/shared/Network/GatewayHTTPClient.swift +++ b/clients/shared/Network/GatewayHTTPClient.swift @@ -64,8 +64,8 @@ public enum GatewayHTTPClient { /// - quiet: When `true`, suppresses HTTP request/response logging for this request. /// - 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 get(path: String, params: [String: String]? = nil, timeout: TimeInterval = 30, quiet: Bool = false, unprefixed: Bool = false) async throws -> Response { - return try await executeWithRetry(path: path, params: params, method: "GET", timeout: timeout, quiet: quiet, unprefixed: unprefixed) + public static func get(path: String, params: [String: String]? = nil, timeout: TimeInterval = 30, quiet: Bool = false, unprefixed: Bool = false, extraHeaders: [String: String]? = nil) async throws -> Response { + return try await executeWithRetry(path: path, params: params, method: "GET", timeout: timeout, quiet: quiet, unprefixed: unprefixed, configure: extraHeaders.map { h in { req in for (k, v) in h { req.setValue(v, forHTTPHeaderField: k) } } }) } /// Performs an authenticated GET request and decodes the JSON response into the given type. diff --git a/clients/shared/Network/HostProxyClient.swift b/clients/shared/Network/HostProxyClient.swift index 0719a8bbff9..88b77c90762 100644 --- a/clients/shared/Network/HostProxyClient.swift +++ b/clients/shared/Network/HostProxyClient.swift @@ -136,6 +136,7 @@ public struct HostProxyClient: HostProxyClientProtocol { let response = try await GatewayHTTPClient.post( path: "host-transfer-result", body: body, + extraHeaders: ["X-Vellum-Client-Id": DeviceIdStore.getOrCreate()], timeout: timeout ) guard response.isSuccess else { @@ -153,7 +154,8 @@ public struct HostProxyClient: HostProxyClientProtocol { // Use a generous timeout — large files may take a while to download. let response = try await GatewayHTTPClient.get( path: "transfers/\(transferId)/content", - timeout: 300 + timeout: 300, + extraHeaders: ["X-Vellum-Client-Id": DeviceIdStore.getOrCreate()] ) guard response.isSuccess else { throw TransferError.pullFailed(statusCode: response.statusCode) @@ -169,7 +171,7 @@ public struct HostProxyClient: HostProxyClientProtocol { body: data, params: ["sourcePath": sourcePath], contentType: "application/octet-stream", - extraHeaders: ["X-Transfer-SHA256": sha256], + extraHeaders: ["X-Transfer-SHA256": sha256, "X-Vellum-Client-Id": DeviceIdStore.getOrCreate()], timeout: timeout ) guard response.isSuccess else { diff --git a/clients/shared/Network/MessageTypes.swift b/clients/shared/Network/MessageTypes.swift index eeac7d19e1b..a0e36835679 100644 --- a/clients/shared/Network/MessageTypes.swift +++ b/clients/shared/Network/MessageTypes.swift @@ -2044,11 +2044,15 @@ public struct HostTransferRequest: Decodable, Sendable { public let sizeBytes: Int? public let sha256: String? public let overwrite: Bool? + /// When set, this request is targeted at a specific client ID. Non-nil only for + /// cross-client proxy requests routed through HostTransferProxy. + public let targetClientId: String? private enum CodingKeys: String, CodingKey { case type, requestId, conversationId, direction case transferId, destPath, sourcePath, sizeBytes case sha256, overwrite + case targetClientId } }