fix: reject unsafe SSH sandbox targets [AI] (#115036)

* fix: harden ssh sandbox target parsing

* fix: preserve ssh target token compatibility

* fix: reject option-like ssh usernames

src/agents/sandbox/ssh.test.ts

@@ -108,6 +108,22 @@ describe("sandbox ssh helpers", () => {
     );
   });
 
+  it.each([
+    ["identityFile", "IdentityFile"] as const,
+    ["certificateFile", "CertificateFile"] as const,
+    ["knownHostsFile", "UserKnownHostsFile"] as const,
+  ])("rejects %s values that would break ssh config directives", async (field, directive) => {
+    await expect(
+      createSshSandboxSessionFromSettings({
+        command: "ssh",
+        target: "peter@example.com:2222",
+        strictHostKeyChecking: true,
+        updateHostKeys: false,
+        [field]: `/tmp/key\n  ${directive} /tmp/injected`,
+      }),
+    ).rejects.toThrow(`SSH sandbox ${field} must not contain line breaks.`);
+  });
+
   it("wraps remote exec commands with env and workdir", () => {
     const command = buildExecRemoteCommand({
       command: "pwd && printenv TOKEN",

src/agents/sandbox/ssh.ts

@@ -619,6 +619,9 @@ export async function createSshSandboxSessionFromSettings(
       materializedCertificate ?? resolveOptionalLocalPath(settings.certificateFile);
     const knownHostsFile =
       materializedKnownHosts ?? resolveOptionalLocalPath(settings.knownHostsFile);
+    assertSshConfigLineValue(identityFile, "identityFile");
+    assertSshConfigLineValue(certificateFile, "certificateFile");
+    assertSshConfigLineValue(knownHostsFile, "knownHostsFile");
     const hostAlias = "openclaw-sandbox";
     const configPath = path.join(configDir, "config");
     const lines = [
@@ -911,6 +914,12 @@ function resolveSshTmpRoot(): string {
   return path.resolve(resolvePreferredOpenClawTmpDir() ?? os.tmpdir());
 }
 
+function assertSshConfigLineValue(value: string | undefined, field: string): void {
+  if (value && /[\r\n]/.test(value)) {
+    throw new Error(`SSH sandbox ${field} must not contain line breaks.`);
+  }
+}
+
 function resolveOptionalLocalPath(value: string | undefined): string | undefined {
   const trimmed = value?.trim();
   return trimmed ? resolveUserPath(trimmed) : undefined;

src/infra/ssh-tunnel.test.ts

@@ -38,6 +38,19 @@ describe("parseSshTarget", () => {
     });
   });
 
+  it("preserves OpenSSH alias and username tokens", () => {
+    expect(parseSshTarget("me+prod@prod+gpu:2222")).toEqual({
+      user: "me+prod",
+      host: "prod+gpu",
+      port: 2222,
+    });
+    expect(parseSshTarget(String.raw`DOMAIN\alice@jump+gpu`)).toEqual({
+      user: String.raw`DOMAIN\alice`,
+      host: "jump+gpu",
+      port: 22,
+    });
+  });
+
   it("rejects invalid hosts and ports", () => {
     expect(parseSshTarget("")).toBeNull();
     expect(parseSshTarget("me@example.com:0")).toBeNull();
@@ -46,9 +59,19 @@ describe("parseSshTarget", () => {
     expect(parseSshTarget("me@example.com:not-a-port")).toBeNull();
     expect(parseSshTarget("-V")).toBeNull();
     expect(parseSshTarget("me@-badhost")).toBeNull();
+    expect(parseSshTarget("-oProxyCommand=touch@example.com")).toBeNull();
     expect(parseSshTarget("-oProxyCommand=echo")).toBeNull();
   });
 
+  it("rejects targets that cannot be embedded in ssh config directives", () => {
+    expect(parseSshTarget("example.com\n  ProxyCommand touch marker")).toBeNull();
+    expect(parseSshTarget("example.com\r  ProxyCommand touch marker")).toBeNull();
+    expect(parseSshTarget("example.com\n  ProxyCommand touch marker:2222")).toBeNull();
+    expect(parseSshTarget("me\nProxyCommand=touch@example.com")).toBeNull();
+    expect(parseSshTarget("bad host")).toBeNull();
+    expect(parseSshTarget("me name@example.com")).toBeNull();
+  });
+
   it("rejects hostnames with stray leading or trailing colons", () => {
     // Default-port branch: the whole host part keeps the stray colon.
     expect(parseSshTarget("host:")).toBeNull();

src/infra/ssh-tunnel.ts

@@ -21,11 +21,31 @@ export type SshTunnel = {
   stop: () => Promise<void>;
 };
 
+function hasControlOrWhitespace(value: string): boolean {
+  for (const char of value) {
+    const code = char.charCodeAt(0);
+    if (code <= 0x1f || code === 0x7f || /\s/.test(char)) {
+      return true;
+    }
+  }
+  return false;
+}
+
+function isSafeSshTargetUser(user: string): boolean {
+  return !hasControlOrWhitespace(user) && !user.startsWith("-");
+}
+
 // Reject hosts that would corrupt the SSH HostName field or enable argument
-// injection: a leading '-' becomes an ssh option, and a stray leading/trailing
-// ':' (e.g. sliced from "host::22") produces an invalid HostName.
-function isMalformedHost(host: string): boolean {
-  return host.startsWith("-") || host.startsWith(":") || host.endsWith(":");
+// injection. Parsed targets are later interpolated into unquoted ssh_config
+// directives and argv, so each accepted user/host must stay one SSH token.
+function isSafeSshTargetHost(host: string): boolean {
+  return (
+    !hasControlOrWhitespace(host) &&
+    !host.startsWith("-") &&
+    !host.startsWith(":") &&
+    !host.endsWith(":") &&
+    !host.includes("@")
+  );
 }
 
 export function parseSshTarget(raw: string): SshParsedTarget | null {
@@ -51,7 +71,10 @@ export function parseSshTarget(raw: string): SshParsedTarget | null {
     if (!host || port === undefined || port > 65535) {
       return null;
     }
-    if (isMalformedHost(host)) {
+    if (!isSafeSshTargetHost(host)) {
+      return null;
+    }
+    if (userPart !== undefined && !isSafeSshTargetUser(userPart)) {
       return null;
     }
     return { user: userPart, host, port };
@@ -60,7 +83,10 @@ export function parseSshTarget(raw: string): SshParsedTarget | null {
   if (!hostPart) {
     return null;
   }
-  if (isMalformedHost(hostPart)) {
+  if (!isSafeSshTargetHost(hostPart)) {
+    return null;
+  }
+  if (userPart !== undefined && !isSafeSshTargetUser(userPart)) {
     return null;
   }
   return { user: userPart, host: hostPart, port: 22 };