diff --git a/.github/workflows/exe-dev-image.yml b/.github/workflows/exe-dev-image.yml index a983e6cf6e0..b9711d85311 100644 --- a/.github/workflows/exe-dev-image.yml +++ b/.github/workflows/exe-dev-image.yml @@ -29,7 +29,7 @@ jobs: - run: pnpm install --frozen-lockfile - name: Build and publish immutable experimental image run: node cli/node_modules/tsx/dist/cli.mjs scripts/build-exe-dev-image.ts --push --metadata-file /tmp/exe-image.json - - uses: actions/upload-artifact@v4 + - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 with: name: exe-dev-image-digest path: /tmp/exe-image.json diff --git a/packages/plugins/sandbox-providers/exe-dev/src/plugin.ts b/packages/plugins/sandbox-providers/exe-dev/src/plugin.ts index dd052a71103..1a040a5ac15 100644 --- a/packages/plugins/sandbox-providers/exe-dev/src/plugin.ts +++ b/packages/plugins/sandbox-providers/exe-dev/src/plugin.ts @@ -64,7 +64,7 @@ let context: PluginContext | undefined; async function management(config: Config, command: string): Promise> { let output: string; - try { output = await ssh(config, "exe.dev", command); } + try { output = await ssh(config, "exe.dev", command, undefined, Boolean(config.registryAuth)); } catch (error) { throw new Error(config.registryAuth ? String((error as Error).message).split(config.registryAuth).join("[REDACTED]") : (error as Error).message); } const result = JSON.parse(output); if (result.error) throw new Error(`exe.dev: ${config.registryAuth ? String(result.error).split(config.registryAuth).join("[REDACTED]") : String(result.error)}`); diff --git a/packages/plugins/sandbox-providers/exe-dev/src/transport.test.ts b/packages/plugins/sandbox-providers/exe-dev/src/transport.test.ts new file mode 100644 index 00000000000..279bbf1defb --- /dev/null +++ b/packages/plugins/sandbox-providers/exe-dev/src/transport.test.ts @@ -0,0 +1,27 @@ +import { EventEmitter } from "node:events"; +import { PassThrough } from "node:stream"; +import { readFile, stat } from "node:fs/promises"; +import { afterEach, expect, it, vi } from "vitest"; +const mocks = vi.hoisted(() => ({ spawn: vi.fn() })); +vi.mock("node:child_process", () => ({ spawn: mocks.spawn })); +import { openSsh } from "./transport.js"; +const cleanups: (() => Promise)[] = []; +afterEach(async () => { await Promise.all(cleanups.splice(0).map((cleanup) => cleanup())); mocks.spawn.mockReset(); }); +it("keeps sensitive commands in a private file, escapes percent expansion, and removes it", async () => { + const child = Object.assign(new EventEmitter(), { stdin: new PassThrough(), stdout: new PassThrough(), stderr: new PassThrough() }); + mocks.spawn.mockReturnValue(child); + const command = "new --registry-auth='test:tok%en'"; + const opened = await openSsh({ sshPrivateKey: "test-key", strictHostKeyChecking: "accept-new", timeoutMs: 1000 }, "exe.dev", command, true); + cleanups.push(opened.cleanup); + const args = mocks.spawn.mock.calls[0][1] as string[]; + expect(args.join(" ")).not.toContain("tok%en"); + const file = args[args.indexOf("-F") + 1]; + expect((await stat(file)).mode & 0o777).toBe(0o600); + expect(await readFile(file, "utf8")).toBe("RemoteCommand new --registry-auth='test:tok%%en'\n"); + await opened.cleanup(); + await expect(stat(file)).rejects.toMatchObject({ code: "ENOENT" }); +}); +it("rejects newline injection into a sensitive SSH config before spawning", async () => { + await expect(openSsh({ strictHostKeyChecking: "accept-new", timeoutMs: 1000 }, "exe.dev", "new\nLocalCommand unsafe", true)).rejects.toThrow("single-line"); + expect(mocks.spawn).not.toHaveBeenCalled(); +}); diff --git a/packages/plugins/sandbox-providers/exe-dev/src/transport.ts b/packages/plugins/sandbox-providers/exe-dev/src/transport.ts index 44445554e10..bfcd1489312 100644 --- a/packages/plugins/sandbox-providers/exe-dev/src/transport.ts +++ b/packages/plugins/sandbox-providers/exe-dev/src/transport.ts @@ -13,7 +13,7 @@ export interface SshConfig { export const quote = (value: string): string => `'${value.replace(/'/g, `'"'"'`)}'`; -export async function openSsh(config: SshConfig, host: string, command: string): Promise<{ +export async function openSsh(config: SshConfig, host: string, command: string, sensitiveCommand = false): Promise<{ child: ChildProcessWithoutNullStreams; cleanup: () => Promise; }> { @@ -32,12 +32,21 @@ export async function openSsh(config: SshConfig, host: string, command: string): knownHosts = path.join(temporary, "known_hosts"); await writeFile(knownHosts, config.knownHosts + "\n", { mode: 0o600 }); } - const args = ["-F", "/dev/null", "-T", "-o", "BatchMode=yes", "-o", "ForwardAgent=no", + let configFile = "/dev/null"; + if (sensitiveCommand) { + if (/[\r\n\0]/.test(command)) throw new Error("Sensitive SSH commands must be single-line"); + configFile = path.join(temporary, "config"); + // RemoteCommand keeps registry credentials out of the process argv. OpenSSH + // expands percent tokens in this directive; escape every literal percent. + await writeFile(configFile, `RemoteCommand ${command.replaceAll("%", "%%")}\n`, { mode: 0o600 }); + } + const args = ["-F", configFile, "-T", "-o", "BatchMode=yes", "-o", "ForwardAgent=no", "-o", "ClearAllForwardings=yes", "-o", "ConnectTimeout=15", "-o", "ServerAliveInterval=15", "-o", "ServerAliveCountMax=3", "-o", `StrictHostKeyChecking=${config.knownHosts ? "yes" : config.strictHostKeyChecking}`, "-o", `UserKnownHostsFile=${knownHosts}`]; if (identity) args.push("-i", identity, "-o", "IdentitiesOnly=yes"); - args.push(host, command); + args.push(host); + if (!sensitiveCommand) args.push(command); const child = spawn("ssh", args, { stdio: "pipe" }); child.stdin.on("error", () => {}); const cleanup = async () => { await rm(temporary, { recursive: true, force: true }); }; @@ -49,8 +58,8 @@ export async function openSsh(config: SshConfig, host: string, command: string): } } -export async function ssh(config: SshConfig, host: string, command: string, stdin?: string): Promise { - const { child } = await openSsh(config, host, command); +export async function ssh(config: SshConfig, host: string, command: string, stdin?: string, sensitiveCommand = false): Promise { + const { child } = await openSsh(config, host, command, sensitiveCommand); return await new Promise((resolve, reject) => { let stdout = ""; let stderr = ""; let expired = false; const timer = setTimeout(() => { expired = true; child.kill("SIGKILL"); }, config.timeoutMs);