diff --git a/assistant/src/__tests__/host-cu-routes-targeted.test.ts b/assistant/src/__tests__/host-cu-routes-targeted.test.ts new file mode 100644 index 00000000000..d15f7bb72e6 --- /dev/null +++ b/assistant/src/__tests__/host-cu-routes-targeted.test.ts @@ -0,0 +1,276 @@ +/** + * Tests for the host-cu-result route 403 guard introduced in Phase 2. + * + * Covers: + * 1. Targeted + correct x-vellum-client-id header → 200 accepted + * 2. Targeted + missing header → 400 BadRequestError + * 3. Targeted + wrong header → 403 ForbiddenError, interaction NOT consumed + * 4. Untargeted (no targetClientId, no header) → 200 accepted (regression) + * + * Resolution goes through conversation.hostCuProxy?.resolve(...). The + * conversation store is mocked to return a controlled conversation object. + * + * Note: host-cu-routes.ts has a deep import chain (conversation-store → + * conversation.ts → ces-client → service-contracts) that requires mocking + * before the module loads. We use dynamic imports to ensure all mocks are + * registered before the route module is evaluated. + */ +import { afterAll, beforeEach, describe, expect, mock, test } from "bun:test"; + +// ── Module mocks ───────────────────────────────────────────────────────────── +// Must be registered before the host-cu-routes module is loaded. + +mock.module("../config/env.js", () => ({ + isHttpAuthDisabled: () => true, + hasUngatedHttpAuthDisabled: () => false, +})); + +import type { PendingInteraction } from "../runtime/pending-interactions.js"; + +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 CuResolveCall { + requestId: string; + payload: Record; +} + +const cuResolveSpy: CuResolveCall[] = []; + +// Controlled conversation map: conversationId → conversation object +const conversationStore = new Map void } }>(); + +mock.module("../daemon/conversation-store.js", () => ({ + findConversation: (conversationId: string) => conversationStore.get(conversationId), +})); + +// ── Real imports (after mocks) ────────────────────────────────────────────── +// Use dynamic import to ensure the mocks above are applied before loading. + +import { + BadRequestError, + ForbiddenError, +} from "../runtime/routes/errors.js"; + +const { ROUTES } = await import("../runtime/routes/host-cu-routes.js"); + +afterAll(() => { + mock.restore(); +}); + +const handleHostCuResult = ROUTES.find( + (r: { endpoint: string }) => r.endpoint === "host-cu-result", +)!.handler; + +// ── Helpers ────────────────────────────────────────────────────────────────── + +function registerPending( + requestId: string, + overrides: Partial = {}, +): void { + const entry: PendingInteraction = { + conversationId: "conv-cu-1", + kind: "host_cu", + ...overrides, + }; + pendingStore.set(requestId, entry); +} + +function registerConversation(conversationId = "conv-cu-1"): void { + conversationStore.set(conversationId, { + hostCuProxy: { + resolve(requestId: unknown, payload: unknown) { + cuResolveSpy.push({ + requestId: requestId as string, + payload: payload as Record, + }); + }, + }, + }); +} + +function cuBody(requestId: string): Record { + return { + requestId, + axTree: "Button [1]", + executionResult: "Clicked", + }; +} + +// ── Tests ───────────────────────────────────────────────────────────────────── + +describe("handleHostCuResult — Phase 2 targetClientId guard", () => { + beforeEach(() => { + pendingStore.clear(); + conversationStore.clear(); + resolvedIds.length = 0; + cuResolveSpy.length = 0; + // Default: register a conversation with a hostCuProxy + registerConversation("conv-cu-1"); + }); + + // ── 1. Targeted + correct header → 200 ──────────────────────────────────── + + describe("targeted + correct x-vellum-client-id header", () => { + test("returns { accepted: true } and resolves the interaction", async () => { + const requestId = "req-cu-targeted-match"; + registerPending(requestId, { targetClientId: "client-A" }); + + const result = await handleHostCuResult({ + body: cuBody(requestId), + headers: { "x-vellum-client-id": "client-A" }, + }); + + expect(result).toEqual({ accepted: true }); + expect(cuResolveSpy).toHaveLength(1); + expect(cuResolveSpy[0].requestId).toBe(requestId); + expect(resolvedIds).toContain(requestId); + }); + + test("trims whitespace from header before comparing", async () => { + const requestId = "req-cu-targeted-trim"; + registerPending(requestId, { targetClientId: "client-A" }); + + const result = await handleHostCuResult({ + body: cuBody(requestId), + 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 (400) when header is absent", () => { + const requestId = "req-cu-targeted-no-header"; + registerPending(requestId, { targetClientId: "client-A" }); + + expect(() => + handleHostCuResult({ body: cuBody(requestId) }), + ).toThrow(BadRequestError); + }); + + test("throws BadRequestError (400) when header is empty string", () => { + const requestId = "req-cu-targeted-empty-header"; + registerPending(requestId, { targetClientId: "client-A" }); + + expect(() => + handleHostCuResult({ + body: cuBody(requestId), + headers: { "x-vellum-client-id": " " }, + }), + ).toThrow(BadRequestError); + }); + + test("interaction is NOT resolved on 400 (still pending)", () => { + const requestId = "req-cu-targeted-no-header-stays"; + registerPending(requestId, { targetClientId: "client-A" }); + + try { + handleHostCuResult({ body: cuBody(requestId) }); + } catch { + // expected + } + + expect(resolvedIds).not.toContain(requestId); + expect(pendingStore.has(requestId)).toBe(true); + }); + }); + + // ── 3. Targeted + wrong header → 403 ────────────────────────────────────── + + describe("targeted + wrong x-vellum-client-id header", () => { + test("throws ForbiddenError (403) when client ID does not match", () => { + const requestId = "req-cu-targeted-mismatch"; + registerPending(requestId, { targetClientId: "client-A" }); + + expect(() => + handleHostCuResult({ + body: cuBody(requestId), + headers: { "x-vellum-client-id": "client-B" }, + }), + ).toThrow(ForbiddenError); + }); + + test("ForbiddenError message names both submitting and expected client", () => { + const requestId = "req-cu-targeted-mismatch-msg"; + registerPending(requestId, { targetClientId: "client-A" }); + + let caught: unknown; + try { + handleHostCuResult({ + body: cuBody(requestId), + headers: { "x-vellum-client-id": "client-B" }, + }); + } catch (e) { + caught = e; + } + + expect(caught).toBeInstanceOf(ForbiddenError); + const msg = (caught as ForbiddenError).message; + expect(msg).toContain("client-B"); + expect(msg).toContain("client-A"); + }); + + test("interaction is NOT consumed on 403 (pendingInteractions.get still returns it)", () => { + const requestId = "req-cu-targeted-mismatch-stays"; + registerPending(requestId, { targetClientId: "client-A" }); + + try { + handleHostCuResult({ + body: cuBody(requestId), + headers: { "x-vellum-client-id": "client-B" }, + }); + } catch { + // expected + } + + expect(resolvedIds).not.toContain(requestId); + expect(pendingStore.has(requestId)).toBe(true); + }); + }); + + // ── 4. Untargeted — regression ──────────────────────────────────────────── + + describe("untargeted request (no targetClientId)", () => { + test("accepts when no header is provided", async () => { + const requestId = "req-cu-untargeted-no-header"; + registerPending(requestId); + + const result = await handleHostCuResult({ + body: cuBody(requestId), + }); + + expect(result).toEqual({ accepted: true }); + expect(cuResolveSpy).toHaveLength(1); + expect(resolvedIds).toContain(requestId); + }); + + test("accepts when header is present (header ignored for untargeted)", async () => { + const requestId = "req-cu-untargeted-with-header"; + registerPending(requestId); + + const result = await handleHostCuResult({ + body: cuBody(requestId), + headers: { "x-vellum-client-id": "client-whatever" }, + }); + + expect(result).toEqual({ accepted: true }); + expect(cuResolveSpy).toHaveLength(1); + }); + }); +}); diff --git a/assistant/src/__tests__/host-file-edit-tool.test.ts b/assistant/src/__tests__/host-file-edit-tool.test.ts index bd0b031ddc1..58cb2fcdb12 100644 --- a/assistant/src/__tests__/host-file-edit-tool.test.ts +++ b/assistant/src/__tests__/host-file-edit-tool.test.ts @@ -1,7 +1,29 @@ import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { afterEach, describe, expect, test } from "bun:test"; +import { afterEach, describe, expect, mock, test } from "bun:test"; + +import type { HostFileInput } from "../daemon/host-file-proxy.js"; +import type { ToolExecutionResult } from "../tools/types.js"; + +// Mock HostFileProxy singleton so proxy delegation tests can control it. +let mockFileProxyAvailable = false; +let mockFileProxyRequestFn: ( + input: HostFileInput, + conversationId: string, + signal?: AbortSignal, +) => Promise = () => Promise.resolve({ content: "", isError: false }); + +mock.module("../daemon/host-file-proxy.js", () => ({ + HostFileProxy: { + get instance() { + return { + isAvailable: () => mockFileProxyAvailable, + request: mockFileProxyRequestFn, + }; + }, + }, +})); import { hostFileEditTool } from "../tools/host-filesystem/edit.js"; import type { ToolContext } from "../tools/types.js"; @@ -20,6 +42,8 @@ afterEach(() => { for (const dir of testDirs.splice(0)) { rmSync(dir, { recursive: true, force: true }); } + mockFileProxyAvailable = false; + mockFileProxyRequestFn = () => Promise.resolve({ content: "", isError: false }); }); describe("host_file_edit tool", () => { @@ -268,4 +292,26 @@ describe("host_file_edit tool", () => { result.content.includes("Successfully edited"), ).toBe(true); }); + + test("passes target_client_id to HostFileProxy.instance.request", async () => { + const capturedInputs: HostFileInput[] = []; + mockFileProxyAvailable = true; + mockFileProxyRequestFn = async (input) => { + capturedInputs.push(input); + return { content: "proxied edit", isError: false }; + }; + + await hostFileEditTool.execute( + { + path: "/host/file.txt", + old_string: "old", + new_string: "new", + target_client_id: "client-x", + }, + makeContext(), + ); + + expect(capturedInputs).toHaveLength(1); + expect(capturedInputs[0].targetClientId).toBe("client-x"); + }); }); diff --git a/assistant/src/__tests__/host-file-proxy-targeted.test.ts b/assistant/src/__tests__/host-file-proxy-targeted.test.ts new file mode 100644 index 00000000000..d498a49c944 --- /dev/null +++ b/assistant/src/__tests__/host-file-proxy-targeted.test.ts @@ -0,0 +1,330 @@ +/** + * Tests for HostFileProxy Phase 2 targetClientId behaviour. + * + * Covers: + * - Explicit targetClientId validation (valid, unknown, incapable) + * - Auto-resolve when exactly one host_file-capable client is connected + * - Untargeted broadcast when multiple capable clients are connected + * - targetClientId propagated into cancel messages (abort + dispose) + * - Timeout message includes clientId when resolvedTargetClientId is set + */ +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), + }, +})); + +mock.module("../runtime/pending-interactions.js", () => ({ + resolve: (requestId: string) => { + resolvedInteractionIds.push(requestId); + return undefined; + }, + get: () => undefined, + getByKind: () => [], + getByConversation: () => [], + removeByConversation: () => {}, +})); + +const { HostFileProxy } = await import("../daemon/host-file-proxy.js"); + +describe("HostFileProxy — targetClientId (Phase 2)", () => { + let proxy: InstanceType; + + function setup() { + sentMessages.length = 0; + sentMessageOptions.length = 0; + resolvedInteractionIds.length = 0; + mockHasClient = false; + mockCapableClients = []; + mockClientRegistry = new Map(); + proxy = new (HostFileProxy as any)(); + } + + function setupSingleClient(clientId = "client-1") { + const entry = { clientId, capabilities: ["host_file"] }; + mockCapableClients = [entry]; + mockClientRegistry.set(clientId, entry); + } + + function setupMultipleClients(clientIds: string[]) { + mockCapableClients = clientIds.map((id) => ({ + clientId: id, + capabilities: ["host_file"], + })); + for (const entry of mockCapableClients) { + mockClientRegistry.set(entry.clientId, entry); + } + } + + afterEach(() => { + proxy?.dispose(); + HostFileProxy.reset(); + }); + + // ── Explicit targetClientId — valid ────────────────────────────────── + + describe("explicit targetClientId — valid client with host_file", () => { + test("resolves to that client and broadcasts with targetClientId option", async () => { + setup(); + setupSingleClient("client-mac"); + // Also add a second client so explicit targeting is meaningful + const entry2 = { clientId: "client-linux", capabilities: ["host_file"] }; + mockCapableClients.push(entry2); + mockClientRegistry.set("client-linux", entry2); + + const resultPromise = proxy.request( + { + operation: "read", + path: "/home/user/notes.txt", + targetClientId: "client-mac", + }, + "session-1", + ); + + expect(sentMessages).toHaveLength(1); + const sent = sentMessages[0] as Record; + expect(sent.type).toBe("host_file_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.resolve(requestId, { content: "file contents", isError: false }); + + const result = await resultPromise; + expect(result.isError).toBe(false); + }); + }); + + // ── Explicit targetClientId — unknown client ───────────────────────── + + describe("explicit targetClientId — unknown client", () => { + test("returns error result immediately without broadcasting", async () => { + setup(); + setupSingleClient("client-mac"); + + const result = await proxy.request( + { + operation: "read", + path: "/tmp/file.txt", + targetClientId: "client-unknown", + }, + "session-1", + ); + + expect(result.isError).toBe(true); + expect(result.content).toContain("client-unknown"); + expect(result.content).toContain("assistant clients list --capability host_file"); + // No pending entry should have been created + expect(sentMessages).toHaveLength(0); + }); + + test("does not create a pending entry for unknown client", async () => { + setup(); + + const result = await proxy.request( + { + operation: "write", + path: "/tmp/out.txt", + content: "data", + targetClientId: "client-ghost", + }, + "session-1", + ); + + expect(result.isError).toBe(true); + expect(sentMessages).toHaveLength(0); + }); + }); + + // ── Explicit targetClientId — incapable client ─────────────────────── + + describe("explicit targetClientId — connected but lacks host_file", () => { + test("returns error result immediately without broadcasting", async () => { + setup(); + // Register a client that exists but does not have host_file + mockClientRegistry.set("client-no-file", { + clientId: "client-no-file", + capabilities: ["host_bash"], + }); + + const result = await proxy.request( + { + operation: "read", + path: "/tmp/test.txt", + targetClientId: "client-no-file", + }, + "session-1", + ); + + 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); + }); + }); + + // ── Auto-resolve single capable client ─────────────────────────────── + + describe("auto-resolve single capable client", () => { + test("resolves target when exactly one host_file-capable client is connected", async () => { + setup(); + setupSingleClient("client-solo"); + + const resultPromise = proxy.request( + { operation: "read", path: "/tmp/file.txt" }, + "session-1", + ); + + expect(sentMessages).toHaveLength(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.resolve(requestId, { content: "ok", isError: false }); + + const result = await resultPromise; + expect(result.isError).toBe(false); + }); + }); + + // ── No target — multiple capable clients ───────────────────────────── + + describe("no explicit target — multiple capable clients", () => { + test("broadcasts without targetClientId (untargeted)", async () => { + setup(); + setupMultipleClients(["client-1", "client-2"]); + + const resultPromise = proxy.request( + { operation: "read", path: "/tmp/file.txt" }, + "session-1", + ); + + expect(sentMessages).toHaveLength(1); + const sent = sentMessages[0] as Record; + expect(sent.type).toBe("host_file_request"); + expect(sent.targetClientId).toBeUndefined(); + + const opts = sentMessageOptions[0] as Record | undefined; + expect(opts?.targetClientId).toBeUndefined(); + + const requestId = sent.requestId as string; + proxy.resolve(requestId, { content: "ok", isError: false }); + + const result = await resultPromise; + expect(result.isError).toBe(false); + }); + }); + + // ── targetClientId in cancel (abort signal) ────────────────────────── + + describe("targetClientId in cancel — abort signal", () => { + test("cancel broadcast includes targetClientId when request was targeted", async () => { + setup(); + setupSingleClient("client-abc"); + + const controller = new AbortController(); + const resultPromise = proxy.request( + { operation: "read", path: "/tmp/file.txt" }, + "session-1", + controller.signal, + ); + + const sent = sentMessages[0] as Record; + const requestId = sent.requestId as string; + expect(sent.targetClientId).toBe("client-abc"); + + controller.abort(); + await resultPromise; + + // Second message is the cancel + expect(sentMessages).toHaveLength(2); + const cancelMsg = sentMessages[1] as Record; + expect(cancelMsg.type).toBe("host_file_cancel"); + expect(cancelMsg.requestId).toBe(requestId); + expect(cancelMsg.targetClientId).toBe("client-abc"); + + const cancelOpts = sentMessageOptions[1] as Record | undefined; + expect(cancelOpts?.targetClientId).toBe("client-abc"); + }); + }); + + // ── targetClientId in cancel (dispose) ─────────────────────────────── + + describe("targetClientId in cancel — dispose", () => { + test("dispose cancel broadcast includes targetClientId for targeted request", () => { + setup(); + setupSingleClient("client-xyz"); + + const p = proxy.request( + { operation: "read", path: "/tmp/file.txt" }, + "session-1", + ); + p.catch(() => {}); // expected rejection on dispose + + const sent = sentMessages[0] as Record; + expect(sent.targetClientId).toBe("client-xyz"); + const requestId = sent.requestId as string; + + proxy.dispose(); + + const cancelMessages = sentMessages + .slice(1) + .filter( + (m) => (m as Record).type === "host_file_cancel", + ) as Array>; + expect(cancelMessages).toHaveLength(1); + expect(cancelMessages[0].requestId).toBe(requestId); + expect(cancelMessages[0].targetClientId).toBe("client-xyz"); + }); + }); + + // ── Timeout message includes clientId ──────────────────────────────── + + describe("timeout message includes clientId", () => { + test("timeout resolve message mentions resolvedTargetClientId", async () => { + setup(); + setupSingleClient("client-mac"); + + jest.useFakeTimers(); + try { + const resultPromise = proxy.request( + { operation: "read", path: "/tmp/slow.txt" }, + "session-1", + ); + + const sent = sentMessages[0] as Record; + expect(sent.targetClientId).toBe("client-mac"); + + // Advance past the 30s internal timeout + jest.advanceTimersByTime(31 * 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-file-read-tool.test.ts b/assistant/src/__tests__/host-file-read-tool.test.ts index ba26705479e..17f4726ecd9 100644 --- a/assistant/src/__tests__/host-file-read-tool.test.ts +++ b/assistant/src/__tests__/host-file-read-tool.test.ts @@ -304,4 +304,21 @@ describe("host_file_read image support", () => { expect(result.content).toContain("2 second line"); expect((result as any).contentBlocks).toBeUndefined(); }); + + test("passes target_client_id to HostFileProxy.instance.request", async () => { + const capturedInputs: HostFileInput[] = []; + mockFileProxyAvailable = true; + mockFileProxyRequestFn = async (input) => { + capturedInputs.push(input); + return { content: "proxied", isError: false }; + }; + + await hostFileReadTool.execute( + { path: "/host/notes.txt", target_client_id: "client-x" }, + makeContext(), + ); + + expect(capturedInputs).toHaveLength(1); + expect(capturedInputs[0].targetClientId).toBe("client-x"); + }); }); diff --git a/assistant/src/__tests__/host-file-routes-targeted.test.ts b/assistant/src/__tests__/host-file-routes-targeted.test.ts new file mode 100644 index 00000000000..e1252ec3718 --- /dev/null +++ b/assistant/src/__tests__/host-file-routes-targeted.test.ts @@ -0,0 +1,256 @@ +/** + * Tests for the host-file-result route 403 guard introduced in Phase 2. + * + * Covers: + * 1. Targeted + correct x-vellum-client-id header → 200 accepted + * 2. Targeted + missing header → 400 BadRequestError + * 3. Targeted + wrong header → 403 ForbiddenError, interaction NOT consumed + * 4. Untargeted (no targetClientId, no header) → 200 accepted (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(); +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 FileResolveCall { + requestId: string; + result: { content: string; isError: boolean; imageData?: string }; +} + +const resolveSpy: FileResolveCall[] = []; + +mock.module("../daemon/host-file-proxy.js", () => ({ + HostFileProxy: { + get instance() { + return { + resolve( + requestId: string, + result: { content: string; isError: boolean; imageData?: string }, + ) { + resolveSpy.push({ requestId, result }); + }, + }; + }, + }, +})); + +// ── Real imports (after mocks) ────────────────────────────────────────────── + +import { + BadRequestError, + ForbiddenError, +} from "../runtime/routes/errors.js"; +import { ROUTES } from "../runtime/routes/host-file-routes.js"; + +afterAll(() => { + mock.restore(); +}); + +const handleHostFileResult = ROUTES.find( + (r) => r.endpoint === "host-file-result", +)!.handler; + +// ── Helpers ───────────────────────────────────────────────────────────────── + +function registerPending( + requestId: string, + overrides: Partial = {}, +): void { + pendingStore.set(requestId, { + conversationId: "conv-1", + kind: "host_file", + ...overrides, + }); +} + +function fileBody(requestId: string): Record { + return { + requestId, + content: "file content", + isError: false, + }; +} + +// ── Tests ──────────────────────────────────────────────────────────────────── + +describe("handleHostFileResult — Phase 2 targetClientId guard", () => { + beforeEach(() => { + pendingStore.clear(); + resolvedIds.length = 0; + resolveSpy.length = 0; + }); + + // ── 1. Targeted + correct header → 200 ──────────────────────────────────── + + describe("targeted + correct x-vellum-client-id header", () => { + test("returns { accepted: true } and resolves the interaction", async () => { + const requestId = "req-file-targeted-match"; + registerPending(requestId, { targetClientId: "client-A" }); + + const result = await handleHostFileResult({ + body: fileBody(requestId), + headers: { "x-vellum-client-id": "client-A" }, + }); + + expect(result).toEqual({ accepted: true }); + expect(resolveSpy).toHaveLength(1); + expect(resolveSpy[0].requestId).toBe(requestId); + expect(resolvedIds).toContain(requestId); + }); + + test("trims whitespace from header before comparing", async () => { + const requestId = "req-file-targeted-trim"; + registerPending(requestId, { targetClientId: "client-A" }); + + const result = await handleHostFileResult({ + body: fileBody(requestId), + 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 (400) when header is absent", () => { + const requestId = "req-file-targeted-no-header"; + registerPending(requestId, { targetClientId: "client-A" }); + + expect(() => + handleHostFileResult({ body: fileBody(requestId) }), + ).toThrow(BadRequestError); + }); + + test("throws BadRequestError (400) when header is empty string", () => { + const requestId = "req-file-targeted-empty-header"; + registerPending(requestId, { targetClientId: "client-A" }); + + expect(() => + handleHostFileResult({ + body: fileBody(requestId), + headers: { "x-vellum-client-id": " " }, + }), + ).toThrow(BadRequestError); + }); + + test("interaction is NOT resolved on 400 (still pending)", () => { + const requestId = "req-file-targeted-no-header-stays"; + registerPending(requestId, { targetClientId: "client-A" }); + + try { + handleHostFileResult({ body: fileBody(requestId) }); + } catch { + // expected + } + + expect(resolvedIds).not.toContain(requestId); + expect(pendingStore.has(requestId)).toBe(true); + }); + }); + + // ── 3. Targeted + wrong header → 403 ────────────────────────────────────── + + describe("targeted + wrong x-vellum-client-id header", () => { + test("throws ForbiddenError (403) when client ID does not match", () => { + const requestId = "req-file-targeted-mismatch"; + registerPending(requestId, { targetClientId: "client-A" }); + + expect(() => + handleHostFileResult({ + body: fileBody(requestId), + headers: { "x-vellum-client-id": "client-B" }, + }), + ).toThrow(ForbiddenError); + }); + + test("ForbiddenError message names both submitting and expected client", () => { + const requestId = "req-file-targeted-mismatch-msg"; + registerPending(requestId, { targetClientId: "client-A" }); + + let caught: unknown; + try { + handleHostFileResult({ + body: fileBody(requestId), + headers: { "x-vellum-client-id": "client-B" }, + }); + } catch (e) { + caught = e; + } + + expect(caught).toBeInstanceOf(ForbiddenError); + const msg = (caught as ForbiddenError).message; + expect(msg).toContain("client-B"); + expect(msg).toContain("client-A"); + }); + + test("interaction is NOT consumed on 403 (pendingInteractions.get still returns it)", () => { + const requestId = "req-file-targeted-mismatch-stays"; + registerPending(requestId, { targetClientId: "client-A" }); + + try { + handleHostFileResult({ + body: fileBody(requestId), + headers: { "x-vellum-client-id": "client-B" }, + }); + } catch { + // expected + } + + expect(resolvedIds).not.toContain(requestId); + expect(pendingStore.has(requestId)).toBe(true); + }); + }); + + // ── 4. Untargeted — regression ──────────────────────────────────────────── + + describe("untargeted request (no targetClientId)", () => { + test("accepts when no header is provided", async () => { + const requestId = "req-file-untargeted-no-header"; + registerPending(requestId); + + const result = await handleHostFileResult({ + body: fileBody(requestId), + }); + + expect(result).toEqual({ accepted: true }); + expect(resolveSpy).toHaveLength(1); + expect(resolvedIds).toContain(requestId); + }); + + test("accepts when header is present (header ignored for untargeted)", async () => { + const requestId = "req-file-untargeted-with-header"; + registerPending(requestId); + + const result = await handleHostFileResult({ + body: fileBody(requestId), + headers: { "x-vellum-client-id": "client-whatever" }, + }); + + expect(result).toEqual({ accepted: true }); + expect(resolveSpy).toHaveLength(1); + }); + }); +}); diff --git a/assistant/src/__tests__/host-file-write-tool.test.ts b/assistant/src/__tests__/host-file-write-tool.test.ts index 5de7233e2c8..25879f0ffef 100644 --- a/assistant/src/__tests__/host-file-write-tool.test.ts +++ b/assistant/src/__tests__/host-file-write-tool.test.ts @@ -1,7 +1,29 @@ import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { afterEach, describe, expect, test } from "bun:test"; +import { afterEach, describe, expect, mock, test } from "bun:test"; + +import type { HostFileInput } from "../daemon/host-file-proxy.js"; +import type { ToolExecutionResult } from "../tools/types.js"; + +// Mock HostFileProxy singleton so proxy delegation tests can control it. +let mockFileProxyAvailable = false; +let mockFileProxyRequestFn: ( + input: HostFileInput, + conversationId: string, + signal?: AbortSignal, +) => Promise = () => Promise.resolve({ content: "", isError: false }); + +mock.module("../daemon/host-file-proxy.js", () => ({ + HostFileProxy: { + get instance() { + return { + isAvailable: () => mockFileProxyAvailable, + request: mockFileProxyRequestFn, + }; + }, + }, +})); import { hostFileWriteTool } from "../tools/host-filesystem/write.js"; import type { ToolContext } from "../tools/types.js"; @@ -20,6 +42,8 @@ afterEach(() => { for (const dir of testDirs.splice(0)) { rmSync(dir, { recursive: true, force: true }); } + mockFileProxyAvailable = false; + mockFileProxyRequestFn = () => Promise.resolve({ content: "", isError: false }); }); describe("host_file_write tool", () => { @@ -168,4 +192,21 @@ describe("host_file_write tool", () => { expect(existsSync(filePath)).toBe(true); expect(readFileSync(filePath, "utf-8")).toBe("deep"); }); + + test("passes target_client_id to HostFileProxy.instance.request", async () => { + const capturedInputs: HostFileInput[] = []; + mockFileProxyAvailable = true; + mockFileProxyRequestFn = async (input) => { + capturedInputs.push(input); + return { content: "proxied write", isError: false }; + }; + + await hostFileWriteTool.execute( + { path: "/host/output.txt", content: "hello", target_client_id: "client-x" }, + makeContext(), + ); + + expect(capturedInputs).toHaveLength(1); + expect(capturedInputs[0].targetClientId).toBe("client-x"); + }); });