fix(security): harden SSH target handling (#4001)

steipete · YLChen-007 · steipete · commit 06289b36da72 · 2026-01-29T16:33:36.000Z
Thanks @YLChen-007. Co-authored-by: Edward-x <YLChen-007@users.noreply.github.com>
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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<SshResolvedConfig | null>((resolve) => {
     const child = spawn(sshPath, args, {
diff --git a/src/infra/ssh-tunnel.test.ts 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
@@ -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
@@ -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",
       },
     },
   };

Original file line number	Diff line number	Diff line change
`@@ -58,7 +58,8 @@ export async function resolveSshConfig(`
`58`	`58`	`args.push("-i", opts.identity.trim());`
`59`	`59`	`}`
`60`	`60`	const userHost = target.user ? `${target.user}@${target.host}` : target.host;
`61`		`- args.push(userHost);`
	`61`	`+ // Use "--" so userHost can't be parsed as an ssh option.`
	`62`	`+ args.push("--", userHost);`
`62`	`63`
`63`	`64`	`return await new Promise<SshResolvedConfig \| null>((resolve) => {`
`64`	`65`	`const child = spawn(sshPath, args, {`