diff --git a/packages/cli/src/__tests__/digitalocean-token.test.ts b/packages/cli/src/__tests__/digitalocean-token.test.ts index e0d329129..d8b746d4e 100644 --- a/packages/cli/src/__tests__/digitalocean-token.test.ts +++ b/packages/cli/src/__tests__/digitalocean-token.test.ts @@ -88,34 +88,37 @@ describe("doApi 401 OAuth recovery", () => { it("attempts OAuth recovery on 401 before throwing", async () => { state.token = "expired-token"; - let callCount = 0; + let apiCalls = 0; + let oauthChecks = 0; globalThis.fetch = mock((url: string | URL | Request) => { - callCount++; const urlStr = String(url); - // First call: the actual API call returning 401 - if (callCount === 1) { + // OAuth connectivity check — fail it so tryDoOAuth returns null quickly + if (urlStr.includes("cloud.digitalocean.com")) { + oauthChecks++; + return Promise.reject(new Error("network unavailable")); + } + // API calls to DigitalOcean API — always return 401 + if (urlStr.includes("api.digitalocean.com")) { + apiCalls++; return Promise.resolve( new Response("Unauthorized", { status: 401, }), ); } - // Second call: OAuth connectivity check — fail it so tryDoOAuth returns null quickly - // (avoids starting a real Bun.serve OAuth server) - if (urlStr.includes("cloud.digitalocean.com")) { - return Promise.reject(new Error("network unavailable")); - } + // Fallback for any other calls (e.g. from prior test leaks) return Promise.resolve( - new Response("Unauthorized", { - status: 401, + new Response("{}", { + status: 200, }), ); }); // OAuth recovery fails (connectivity check fails), so doApi throws the 401 await expect(doApi("GET", "/account", undefined, 1)).rejects.toThrow("DigitalOcean API error 401"); - // Verify recovery was attempted: 1 API call + 1 connectivity check = 2 - expect(callCount).toBe(2); + // Verify recovery was attempted: at least 1 API call + 1 connectivity check + expect(apiCalls).toBeGreaterThanOrEqual(1); + expect(oauthChecks).toBeGreaterThanOrEqual(1); }); it("succeeds after OAuth recovery provides a new token", async () => { diff --git a/packages/cli/src/__tests__/hetzner-cov.test.ts b/packages/cli/src/__tests__/hetzner-cov.test.ts index ed1c050f1..87aac3d5d 100644 --- a/packages/cli/src/__tests__/hetzner-cov.test.ts +++ b/packages/cli/src/__tests__/hetzner-cov.test.ts @@ -585,11 +585,13 @@ describe("hetzner/createServer", () => { }, }, }; + let createAttempts = 0; let callCount = 0; - global.fetch = mock(() => { + global.fetch = mock((url: string | URL | Request) => { callCount++; - if (callCount <= 1) { - // Token validation + const urlStr = String(url); + // Token validation — GET /servers?per_page=1 + if (urlStr.includes("/servers") && urlStr.includes("per_page=1") && !urlStr.includes("per_page=50")) { return Promise.resolve( new Response( JSON.stringify({ @@ -598,8 +600,8 @@ describe("hetzner/createServer", () => { ), ); } - if (callCount <= 2) { - // SSH keys + // SSH keys + if (urlStr.includes("/ssh_keys")) { return Promise.resolve( new Response( JSON.stringify({ @@ -608,24 +610,28 @@ describe("hetzner/createServer", () => { ), ); } - if (callCount <= 3) { - // First create attempt — resource_limit_exceeded (HTTP 403) - return Promise.resolve( - new Response( - JSON.stringify({ - error: { - code: "resource_limit_exceeded", - message: "primary_ip_limit", + // Create server (POST /servers) — first attempt fails, retry succeeds + if (urlStr.endsWith("/servers")) { + createAttempts++; + if (createAttempts <= 1) { + return Promise.resolve( + new Response( + JSON.stringify({ + error: { + code: "resource_limit_exceeded", + message: "primary_ip_limit", + }, + }), + { + status: 403, }, - }), - { - status: 403, - }, - ), - ); + ), + ); + } + return Promise.resolve(new Response(JSON.stringify(serverResp))); } - if (callCount <= 4) { - // List primary IPs for cleanup + // List primary IPs for cleanup + if (urlStr.includes("/primary_ips") && !urlStr.includes("/primary_ips/")) { return Promise.resolve( new Response( JSON.stringify({ @@ -645,22 +651,22 @@ describe("hetzner/createServer", () => { ), ); } - if (callCount <= 5) { - // Delete orphaned IP 100 + // Delete orphaned IP + if (urlStr.includes("/primary_ips/")) { return Promise.resolve( new Response("", { status: 204, }), ); } - // Retry create — success - return Promise.resolve(new Response(JSON.stringify(serverResp))); + // Fallback + return Promise.resolve(new Response(JSON.stringify({}))); }); const { ensureHcloudToken, createServer } = await import("../hetzner/hetzner"); await ensureHcloudToken(); const conn = await createServer("test-retry", "cx23", "fsn1"); expect(conn.ip).toBe("10.0.0.5"); - // Should have called: token(1), ssh_keys(2), create-fail(3), list-ips(4), delete-ip(5), create-ok(6) + // Should have called: token, ssh_keys, create-fail, list-ips, delete-ip, create-ok (at least 6) expect(callCount).toBeGreaterThanOrEqual(6); }); diff --git a/packages/cli/src/__tests__/spawn-md-injection.test.ts b/packages/cli/src/__tests__/spawn-md-injection.test.ts new file mode 100644 index 000000000..606bce50c --- /dev/null +++ b/packages/cli/src/__tests__/spawn-md-injection.test.ts @@ -0,0 +1,156 @@ +/** + * spawn-md-injection.test.ts — Verifies that applySpawnMdSetup base64-encodes + * API key values to prevent shell injection via /etc/spawn/secrets. + * + * Regression test for #3361. + */ + +import type { CloudRunner } from "../shared/agent-setup.js"; + +import { afterEach, beforeEach, describe, expect, it, mock, spyOn } from "bun:test"; + +describe("applySpawnMdSetup api_key injection safety", () => { + let capturedCommands: string[] = []; + let mockRunner: CloudRunner; + let stderrSpy: ReturnType; + let savedIsTTY: boolean | undefined; + let stdinOnSpy: ReturnType; + let stdinResumeSpy: ReturnType; + let stdinPauseSpy: ReturnType; + let stdinSetRawModeSpy: ReturnType; + let stdinRemoveListenerSpy: ReturnType; + + // A value designed to break out of shell quoting contexts + const MALICIOUS_KEY = '"; rm -rf /; echo "pwned\n$HOME\n$(whoami)'; + + beforeEach(() => { + capturedCommands = []; + mockRunner = { + runServer: mock(async (cmd: string) => { + capturedCommands.push(cmd); + }), + uploadFile: mock(async () => {}), + downloadFile: mock(async () => {}), + }; + + // Suppress stderr output from promptSecret / logInfo + stderrSpy = spyOn(process.stderr, "write").mockReturnValue(true); + + // Make stdin appear as a TTY so promptSecret doesn't bail with "" + savedIsTTY = process.stdin.isTTY; + Object.defineProperty(process.stdin, "isTTY", { + value: true, + configurable: true, + }); + + // Mock setRawMode — define it first if missing (non-TTY stdin lacks it) + if (typeof process.stdin.setRawMode !== "function") { + Object.defineProperty(process.stdin, "setRawMode", { + value: () => process.stdin, + configurable: true, + writable: true, + }); + } + stdinSetRawModeSpy = spyOn(process.stdin, "setRawMode").mockReturnValue(process.stdin); + stdinResumeSpy = spyOn(process.stdin, "resume").mockReturnValue(process.stdin); + stdinPauseSpy = spyOn(process.stdin, "pause").mockReturnValue(process.stdin); + stdinRemoveListenerSpy = spyOn(process.stdin, "removeListener").mockReturnValue(process.stdin); + + // When promptSecret calls process.stdin.on("data", handler), immediately + // feed the malicious value followed by a newline to resolve the promise. + stdinOnSpy = spyOn(process.stdin, "on").mockImplementation( + (event: string, handler: (...args: unknown[]) => void) => { + if (event === "data") { + queueMicrotask(() => { + handler(Buffer.from(MALICIOUS_KEY)); + handler(Buffer.from("\n")); + }); + } + return process.stdin; + }, + ); + }); + + afterEach(() => { + stderrSpy.mockRestore(); + Object.defineProperty(process.stdin, "isTTY", { + value: savedIsTTY, + configurable: true, + }); + stdinOnSpy.mockRestore(); + stdinSetRawModeSpy.mockRestore(); + stdinResumeSpy.mockRestore(); + stdinPauseSpy.mockRestore(); + stdinRemoveListenerSpy.mockRestore(); + }); + + it("stores api_key values as base64, never raw shell-interpolatable strings", async () => { + const { applySpawnMdSetup } = await import("../shared/spawn-md.js"); + + await applySpawnMdSetup( + mockRunner, + { + setup: [ + { + type: "api_key", + name: "MY_SECRET_KEY", + }, + ], + }, + "test-agent", + ); + + // The first runServer call writes to /etc/spawn/secrets + expect(capturedCommands.length).toBeGreaterThanOrEqual(1); + const secretsCmd = capturedCommands[0]; + + // The raw malicious value must NOT appear anywhere in the command + expect(secretsCmd).not.toContain("rm -rf"); + expect(secretsCmd).not.toContain("$(whoami)"); + expect(secretsCmd).not.toContain("$HOME"); + // No shell-executable export statement + expect(secretsCmd).not.toContain("export "); + + // The command should use base64 encoding — verify the value is valid base64 + const expectedB64 = Buffer.from(MALICIOUS_KEY).toString("base64"); + expect(secretsCmd).toContain(expectedB64); + + // The format should be printf '%s=%s\n' 'NAME' 'B64VALUE' + expect(secretsCmd).toContain("printf '%s=%s\\n'"); + expect(secretsCmd).toContain("'MY_SECRET_KEY'"); + + // The second command should install the base64 loader, not a direct source + expect(capturedCommands.length).toBeGreaterThanOrEqual(2); + const loaderCmd = capturedCommands[1]; + expect(loaderCmd).toContain("while IFS"); + expect(loaderCmd).toContain("base64 -d"); + // Must NOT contain the old vulnerable pattern + expect(loaderCmd).not.toContain("source /etc/spawn/secrets"); + }); + + it("sanitizes step name to alphanumeric + underscore only", async () => { + const { applySpawnMdSetup } = await import("../shared/spawn-md.js"); + + await applySpawnMdSetup( + mockRunner, + { + setup: [ + { + type: "api_key", + name: "MY;DROP$(evil)KEY", + }, + ], + }, + "test-agent", + ); + + expect(capturedCommands.length).toBeGreaterThanOrEqual(1); + const secretsCmd = capturedCommands[0]; + + // The escaped name should only contain [A-Za-z0-9_] + expect(secretsCmd).toContain("'MYDROPevilKEY'"); + // The dangerous characters from the name must not appear unescaped + expect(secretsCmd).not.toContain(";DROP"); + expect(secretsCmd).not.toContain("$(evil)"); + }); +}); diff --git a/packages/cli/src/shared/spawn-md.ts b/packages/cli/src/shared/spawn-md.ts index 6dfac623e..09d052102 100644 --- a/packages/cli/src/shared/spawn-md.ts +++ b/packages/cli/src/shared/spawn-md.ts @@ -390,11 +390,15 @@ async function applySetupStep(runner: CloudRunner, step: SetupStep): Promise> /etc/spawn/secrets && chmod 600 /etc/spawn/secrets`, + `mkdir -p /etc/spawn && printf '%s=%s\\n' '${escapedName}' '${b64Val}' >> /etc/spawn/secrets && chmod 600 /etc/spawn/secrets`, ); + // Install a loader that decodes base64 at source-time instead of shell-sourcing + const loaderSnippet = + 'while IFS="=" read -r k v; do [ -n "$k" ] && export "$k=$(printf "%s" "$v" | base64 -d)"; done < /etc/spawn/secrets'; await runner.runServer( - `grep -q '/etc/spawn/secrets' ~/.bashrc 2>/dev/null || echo 'source /etc/spawn/secrets 2>/dev/null' >> ~/.bashrc`, + `grep -q 'while IFS.*secrets' ~/.bashrc 2>/dev/null || { sed -i '/source.*\\/etc\\/spawn\\/secrets/d' ~/.bashrc 2>/dev/null; echo '${loaderSnippet}' >> ~/.bashrc; }`, ); logInfo(` ${step.name} saved`); } else {