diff --git a/src/gateway/server.canvas-auth.e2e.test.ts b/src/gateway/server.canvas-auth.e2e.test.ts index 503f4b7bf8f..86c0e261cd8 100644 --- a/src/gateway/server.canvas-auth.e2e.test.ts +++ b/src/gateway/server.canvas-auth.e2e.test.ts @@ -9,6 +9,9 @@ import { attachGatewayUpgradeHandler, createGatewayHttpServer } from "./server-h import type { GatewayWsClient } from "./server/ws-types.js"; import { withTempConfig } from "./test-temp-config.js"; +const WS_REJECT_TIMEOUT_MS = 2_000; +const WS_CONNECT_TIMEOUT_MS = 2_000; + async function listen( server: ReturnType, host = "127.0.0.1", @@ -38,7 +41,7 @@ async function expectWsRejected( ): Promise { await new Promise((resolve, reject) => { const ws = new WebSocket(url, { headers }); - const timer = setTimeout(() => reject(new Error("timeout")), 10_000); + const timer = setTimeout(() => reject(new Error("timeout")), WS_REJECT_TIMEOUT_MS); ws.once("open", () => { clearTimeout(timer); ws.terminate(); @@ -242,7 +245,7 @@ describe("gateway canvas host auth", () => { await new Promise((resolve, reject) => { const ws = new WebSocket(`ws://${host}:${listener.port}${activeWsPath}`); - const timer = setTimeout(() => reject(new Error("timeout")), 10_000); + const timer = setTimeout(() => reject(new Error("timeout")), WS_CONNECT_TIMEOUT_MS); ws.once("open", () => { clearTimeout(timer); ws.terminate(); diff --git a/src/gateway/server.node-invoke-approval-bypass.e2e.test.ts b/src/gateway/server.node-invoke-approval-bypass.e2e.test.ts index 3f7b5e094ad..e72692b1ab7 100644 --- a/src/gateway/server.node-invoke-approval-bypass.e2e.test.ts +++ b/src/gateway/server.node-invoke-approval-bypass.e2e.test.ts @@ -18,6 +18,7 @@ import { } from "./test-helpers.js"; installGatewayTestHooks({ scope: "suite" }); +const NODE_CONNECT_TIMEOUT_MS = 3_000; async function expectNoForwardedInvoke(hasInvoke: () => boolean): Promise { // Yield a couple of macrotasks so any accidental async forwarding would fire. @@ -176,94 +177,79 @@ describe("node.invoke approval bypass", () => { client.start(); await Promise.race([ ready, - sleep(10_000).then(() => { + sleep(NODE_CONNECT_TIMEOUT_MS).then(() => { throw new Error("timeout waiting for node to connect"); }), ]); return client; }; - test("rejects rawCommand/command mismatch before forwarding to node", async () => { + test("rejects malformed/forbidden node.invoke payloads before forwarding", async () => { let sawInvoke = false; const node = await connectLinuxNode(() => { sawInvoke = true; }); const ws = await connectOperator(["operator.write"]); - const nodeId = await getConnectedNodeId(ws); + try { + const nodeId = await getConnectedNodeId(ws); + const cases = [ + { + name: "rawCommand mismatch", + payload: { + nodeId, + command: "system.run", + params: { + command: ["uname", "-a"], + rawCommand: "echo hi", + }, + idempotencyKey: crypto.randomUUID(), + }, + expectedError: "rawCommand does not match command", + }, + { + name: "approval flags without runId", + payload: { + nodeId, + command: "system.run", + params: { + command: ["echo", "hi"], + rawCommand: "echo hi", + approved: true, + approvalDecision: "allow-once", + }, + idempotencyKey: crypto.randomUUID(), + }, + expectedError: "params.runId", + }, + { + name: "forbidden execApprovals tool", + payload: { + nodeId, + command: "system.execApprovals.set", + params: { file: { version: 1, agents: {} }, baseHash: "nope" }, + idempotencyKey: crypto.randomUUID(), + }, + expectedError: "exec.approvals.node", + }, + ] as const; - const res = await rpcReq(ws, "node.invoke", { - nodeId, - command: "system.run", - params: { - command: ["uname", "-a"], - rawCommand: "echo hi", - }, - idempotencyKey: crypto.randomUUID(), - }); - expect(res.ok).toBe(false); - expect(res.error?.message ?? "").toContain("rawCommand does not match command"); - - await expectNoForwardedInvoke(() => sawInvoke); - - ws.close(); - node.stop(); + for (const testCase of cases) { + const res = await rpcReq(ws, "node.invoke", testCase.payload); + expect(res.ok, testCase.name).toBe(false); + expect(res.error?.message ?? "", testCase.name).toContain(testCase.expectedError); + await expectNoForwardedInvoke(() => sawInvoke); + } + } finally { + ws.close(); + node.stop(); + } }); - test("rejects injecting approved/approvalDecision without approval id", async () => { - let sawInvoke = false; - const node = await connectLinuxNode(() => { - sawInvoke = true; - }); - const ws = await connectOperator(["operator.write"]); - const nodeId = await getConnectedNodeId(ws); - - const res = await rpcReq(ws, "node.invoke", { - nodeId, - command: "system.run", - params: { - command: ["echo", "hi"], - rawCommand: "echo hi", - approved: true, - approvalDecision: "allow-once", - }, - idempotencyKey: crypto.randomUUID(), - }); - expect(res.ok).toBe(false); - expect(res.error?.message ?? "").toContain("params.runId"); - - // Ensure the node didn't receive the invoke (gateway should fail early). - await expectNoForwardedInvoke(() => sawInvoke); - - ws.close(); - node.stop(); - }); - - test("rejects invoking system.execApprovals.set via node.invoke", async () => { - let sawInvoke = false; - const node = await connectLinuxNode(() => { - sawInvoke = true; - }); - const ws = await connectOperator(["operator.write"]); - const nodeId = await getConnectedNodeId(ws); - - const res = await rpcReq(ws, "node.invoke", { - nodeId, - command: "system.execApprovals.set", - params: { file: { version: 1, agents: {} }, baseHash: "nope" }, - idempotencyKey: crypto.randomUUID(), - }); - expect(res.ok).toBe(false); - expect(res.error?.message ?? "").toContain("exec.approvals.node"); - - await expectNoForwardedInvoke(() => sawInvoke); - - ws.close(); - node.stop(); - }); - - test("binds system.run approval flags to exec.approval decision (ignores caller escalation)", async () => { + test("binds approvals to decision/device and blocks cross-device replay", async () => { + let invokeCount = 0; let lastInvokeParams: Record | null = null; const node = await connectLinuxNode((payload) => { + invokeCount += 1; const obj = payload as { paramsJSON?: unknown }; const raw = typeof obj?.paramsJSON === "string" ? obj.paramsJSON : ""; if (!raw) { @@ -273,71 +259,56 @@ describe("node.invoke approval bypass", () => { lastInvokeParams = JSON.parse(raw) as Record; }); - const ws = await connectOperator(["operator.write", "operator.approvals"]); - const ws2 = await connectOperator(["operator.write"]); - - const nodeId = await getConnectedNodeId(ws); - const approvalId = await requestAllowOnceApproval(ws, "echo hi"); - - // Use a second WebSocket connection to simulate per-call clients (callGatewayTool/callGatewayCli). - // Approval binding should be based on device identity, not the ephemeral connId. - const invoke = await rpcReq(ws2, "node.invoke", { - nodeId, - command: "system.run", - params: { - command: ["echo", "hi"], - rawCommand: "echo hi", - runId: approvalId, - approved: true, - // Try to escalate to allow-always; gateway should clamp to allow-once from record. - approvalDecision: "allow-always", - injected: "nope", - }, - idempotencyKey: crypto.randomUUID(), - }); - expect(invoke.ok).toBe(true); - - const invokeParams = lastInvokeParams as Record | null; - expect(invokeParams).toBeTruthy(); - expect(invokeParams?.["approved"]).toBe(true); - expect(invokeParams?.["approvalDecision"]).toBe("allow-once"); - expect(invokeParams?.["injected"]).toBeUndefined(); - - ws.close(); - ws2.close(); - node.stop(); - }); - - test("rejects replaying approval id from another device", async () => { - let sawInvoke = false; - const node = await connectLinuxNode(() => { - sawInvoke = true; - }); - - const ws = await connectOperator(["operator.write", "operator.approvals"]); + const wsApprover = await connectOperator(["operator.write", "operator.approvals"]); + const wsCaller = await connectOperator(["operator.write"]); const wsOtherDevice = await connectOperatorWithNewDevice(["operator.write"]); - const nodeId = await getConnectedNodeId(ws); - const approvalId = await requestAllowOnceApproval(ws, "echo hi"); + try { + const nodeId = await getConnectedNodeId(wsApprover); - const invoke = await rpcReq(wsOtherDevice, "node.invoke", { - nodeId, - command: "system.run", - params: { - command: ["echo", "hi"], - rawCommand: "echo hi", - runId: approvalId, - approved: true, - approvalDecision: "allow-once", - }, - idempotencyKey: crypto.randomUUID(), - }); - expect(invoke.ok).toBe(false); - expect(invoke.error?.message ?? "").toContain("not valid for this device"); - await expectNoForwardedInvoke(() => sawInvoke); + const approvalId = await requestAllowOnceApproval(wsApprover, "echo hi"); + // Separate caller connection simulates per-call clients. + const invoke = await rpcReq(wsCaller, "node.invoke", { + nodeId, + command: "system.run", + params: { + command: ["echo", "hi"], + rawCommand: "echo hi", + runId: approvalId, + approved: true, + approvalDecision: "allow-always", + injected: "nope", + }, + idempotencyKey: crypto.randomUUID(), + }); + expect(invoke.ok).toBe(true); + expect(lastInvokeParams).toBeTruthy(); + expect(lastInvokeParams?.["approved"]).toBe(true); + expect(lastInvokeParams?.["approvalDecision"]).toBe("allow-once"); + expect(lastInvokeParams?.["injected"]).toBeUndefined(); - ws.close(); - wsOtherDevice.close(); - node.stop(); + const replayApprovalId = await requestAllowOnceApproval(wsApprover, "echo hi"); + const invokeCountBeforeReplay = invokeCount; + const replay = await rpcReq(wsOtherDevice, "node.invoke", { + nodeId, + command: "system.run", + params: { + command: ["echo", "hi"], + rawCommand: "echo hi", + runId: replayApprovalId, + approved: true, + approvalDecision: "allow-once", + }, + idempotencyKey: crypto.randomUUID(), + }); + expect(replay.ok).toBe(false); + expect(replay.error?.message ?? "").toContain("not valid for this device"); + await expectNoForwardedInvoke(() => invokeCount > invokeCountBeforeReplay); + } finally { + wsApprover.close(); + wsCaller.close(); + wsOtherDevice.close(); + node.stop(); + } }); }); diff --git a/src/process/child-process-bridge.test.ts b/src/process/child-process-bridge.test.ts index a2b9853f303..771b629654e 100644 --- a/src/process/child-process-bridge.test.ts +++ b/src/process/child-process-bridge.test.ts @@ -4,7 +4,13 @@ import process from "node:process"; import { afterEach, describe, expect, it } from "vitest"; import { attachChildProcessBridge } from "./child-process-bridge.js"; -function waitForLine(stream: NodeJS.ReadableStream, timeoutMs = 10_000): Promise { +const CHILD_READY_TIMEOUT_MS = 2_000; +const CHILD_EXIT_TIMEOUT_MS = 3_000; + +function waitForLine( + stream: NodeJS.ReadableStream, + timeoutMs = CHILD_READY_TIMEOUT_MS, +): Promise { return new Promise((resolve, reject) => { let buffer = ""; @@ -89,11 +95,14 @@ describe("attachChildProcessBridge", () => { addedSigterm("SIGTERM"); await new Promise((resolve, reject) => { - const timeout = setTimeout(() => reject(new Error("timeout waiting for child exit")), 10_000); + const timeout = setTimeout( + () => reject(new Error("timeout waiting for child exit")), + CHILD_EXIT_TIMEOUT_MS, + ); child.once("exit", () => { clearTimeout(timeout); resolve(); }); }); - }, 15_000); + }, 8_000); }); diff --git a/test/gateway.multi.e2e.test.ts b/test/gateway.multi.e2e.test.ts index 1873f82633c..3b033616e59 100644 --- a/test/gateway.multi.e2e.test.ts +++ b/test/gateway.multi.e2e.test.ts @@ -30,6 +30,7 @@ type NodeListPayload = { }; const GATEWAY_START_TIMEOUT_MS = 45_000; +const GATEWAY_STOP_TIMEOUT_MS = 1_500; const E2E_TIMEOUT_MS = 120_000; const getFreePort = async () => { @@ -184,7 +185,7 @@ const stopGatewayInstance = async (inst: GatewayInstance) => { } inst.child.once("exit", () => resolve(true)); }), - sleep(5_000).then(() => false), + sleep(GATEWAY_STOP_TIMEOUT_MS).then(() => false), ]); if (!exited && inst.child.exitCode === null && !inst.child.killed) { try {