From 06289b36da72a65f8d1cafd7cbab5f3d8654df98 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 29 Jan 2026 16:33:36 +0000 Subject: [PATCH] fix(security): harden SSH target handling (#4001) Thanks @YLChen-007. Co-authored-by: Edward-x --- CHANGELOG.md | 1 + src/commands/gateway-status.test.ts | 39 +++++++++++++++++++++++++++++ src/commands/gateway-status.ts | 4 ++- src/infra/ssh-config.test.ts | 2 ++ src/infra/ssh-config.ts | 3 ++- src/infra/ssh-tunnel.test.ts | 27 ++++++++++++++++++++ src/infra/ssh-tunnel.ts | 7 +++++- src/plugins/config-state.ts | 4 +-- 8 files changed, 82 insertions(+), 5 deletions(-) create mode 100644 src/infra/ssh-tunnel.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c5321870..1b13b7835 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ Docs: https://docs.molt.bot Status: beta. ### Changes +- Security: harden SSH tunnel target parsing to prevent option injection/DoS. (#4001) Thanks @YLChen-007. - Rebrand: rename the npm package/CLI to `moltbot`, add a `moltbot` compatibility shim, and move extensions to the `@moltbot/*` scope. - Commands: group /help and /commands output with Telegram paging. (#2504) Thanks @hougangdev. - macOS: limit project-local `node_modules/.bin` PATH preference to debug builds (reduce PATH hijacking risk). diff --git a/src/commands/gateway-status.test.ts b/src/commands/gateway-status.test.ts index 09547a83d..5e816f581 100644 --- a/src/commands/gateway-status.test.ts +++ b/src/commands/gateway-status.test.ts @@ -192,6 +192,45 @@ describe("gateway-status command", () => { expect(targets.some((t) => t.kind === "sshTunnel")).toBe(true); }); + it("skips invalid ssh-auto discovery targets", async () => { + const runtimeLogs: string[] = []; + const runtime = { + log: (msg: string) => runtimeLogs.push(msg), + error: (_msg: string) => {}, + exit: (code: number) => { + throw new Error(`__exit__:${code}`); + }, + }; + + const originalUser = process.env.USER; + try { + process.env.USER = "steipete"; + loadConfig.mockReturnValueOnce({ + gateway: { + mode: "remote", + remote: {}, + }, + }); + discoverGatewayBeacons.mockResolvedValueOnce([ + { tailnetDns: "-V" }, + { tailnetDns: "goodhost" }, + ]); + + startSshPortForward.mockClear(); + const { gatewayStatusCommand } = await import("./gateway-status.js"); + await gatewayStatusCommand( + { timeout: "1000", json: true, sshAuto: true }, + runtime as unknown as import("../runtime.js").RuntimeEnv, + ); + + expect(startSshPortForward).toHaveBeenCalledTimes(1); + const call = startSshPortForward.mock.calls[0]?.[0] as { target: string }; + expect(call.target).toBe("steipete@goodhost"); + } finally { + process.env.USER = originalUser; + } + }); + it("infers SSH target from gateway.remote.url and ssh config", async () => { const runtimeLogs: string[] = []; const runtime = { diff --git a/src/commands/gateway-status.ts b/src/commands/gateway-status.ts index 3a51a7886..a5a34d6e4 100644 --- a/src/commands/gateway-status.ts +++ b/src/commands/gateway-status.ts @@ -107,7 +107,9 @@ export async function gatewayStatusCommand( const base = user ? `${user}@${host.trim()}` : host.trim(); return sshPort !== 22 ? `${base}:${sshPort}` : base; }) - .filter((x): x is string => Boolean(x)); + .filter((candidate): candidate is string => + Boolean(candidate && parseSshTarget(candidate)), + ); if (candidates.length > 0) sshTarget = candidates[0] ?? null; } diff --git a/src/infra/ssh-config.test.ts b/src/infra/ssh-config.test.ts index 8f3248e0c..48a8bf310 100644 --- a/src/infra/ssh-config.test.ts +++ b/src/infra/ssh-config.test.ts @@ -54,6 +54,8 @@ describe("ssh-config", () => { expect(config?.host).toBe("peters-mac-studio-1.sheep-coho.ts.net"); expect(config?.port).toBe(2222); expect(config?.identityFiles).toEqual(["/tmp/id_ed25519"]); + const args = spawnMock.mock.calls[0]?.[1] as string[] | undefined; + expect(args?.slice(-2)).toEqual(["--", "me@alias"]); }); it("returns null when ssh -G fails", async () => { diff --git a/src/infra/ssh-config.ts b/src/infra/ssh-config.ts index 037405e8c..0b0e95015 100644 --- a/src/infra/ssh-config.ts +++ b/src/infra/ssh-config.ts @@ -58,7 +58,8 @@ export async function resolveSshConfig( args.push("-i", opts.identity.trim()); } const userHost = target.user ? `${target.user}@${target.host}` : target.host; - args.push(userHost); + // Use "--" so userHost can't be parsed as an ssh option. + args.push("--", userHost); return await new Promise((resolve) => { const child = spawn(sshPath, args, { diff --git a/src/infra/ssh-tunnel.test.ts b/src/infra/ssh-tunnel.test.ts new file mode 100644 index 000000000..d31f25d1a --- /dev/null +++ b/src/infra/ssh-tunnel.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from "vitest"; + +import { parseSshTarget } from "./ssh-tunnel.js"; + +describe("parseSshTarget", () => { + it("parses user@host:port targets", () => { + expect(parseSshTarget("me@example.com:2222")).toEqual({ + user: "me", + host: "example.com", + port: 2222, + }); + }); + + it("parses host-only targets with default port", () => { + expect(parseSshTarget("example.com")).toEqual({ + user: undefined, + host: "example.com", + port: 22, + }); + }); + + it("rejects hostnames that start with '-'", () => { + expect(parseSshTarget("-V")).toBeNull(); + expect(parseSshTarget("me@-badhost")).toBeNull(); + expect(parseSshTarget("-oProxyCommand=echo")).toBeNull(); + }); +}); diff --git a/src/infra/ssh-tunnel.ts b/src/infra/ssh-tunnel.ts index 8b3c7693b..399dc22e3 100644 --- a/src/infra/ssh-tunnel.ts +++ b/src/infra/ssh-tunnel.ts @@ -41,10 +41,14 @@ export function parseSshTarget(raw: string): SshParsedTarget | null { const portRaw = hostPart.slice(colonIdx + 1).trim(); const port = Number.parseInt(portRaw, 10); if (!host || !Number.isFinite(port) || port <= 0) return null; + // Security: Reject hostnames starting with '-' to prevent argument injection + if (host.startsWith("-")) return null; return { user: userPart, host, port }; } if (!hostPart) return null; + // Security: Reject hostnames starting with '-' to prevent argument injection + if (hostPart.startsWith("-")) return null; return { user: userPart, host: hostPart, port: 22 }; } @@ -134,7 +138,8 @@ export async function startSshPortForward(opts: { if (opts.identity?.trim()) { args.push("-i", opts.identity.trim()); } - args.push(userHost); + // Security: Use '--' to prevent userHost from being interpreted as an option + args.push("--", userHost); const stderr: string[] = []; const child = spawn("/usr/bin/ssh", args, { diff --git a/src/plugins/config-state.ts b/src/plugins/config-state.ts index 6b1a6c54a..5bfff7dbc 100644 --- a/src/plugins/config-state.ts +++ b/src/plugins/config-state.ts @@ -99,7 +99,7 @@ export function applyTestPluginDefaults( ...plugins, slots: { ...plugins?.slots, - memory: null, + memory: "none", }, }, }; @@ -112,7 +112,7 @@ export function applyTestPluginDefaults( enabled: false, slots: { ...plugins?.slots, - memory: null, + memory: "none", }, }, };