From e83ff72b61c760f74bc1c596655b60c63153cf2c Mon Sep 17 00:00:00 2001
From: Aamer Akhter <aakhter@gmail.com>
Date: Sat, 13 Jun 2026 10:37:39 -0400
Subject: [PATCH] COD-107 fix: shellescape -J jumpHost + structural validator
 (close command-injection)

buildSshConnectionArgs interpolated jumpHost raw while its siblings
(identityFile/socksProxy/extraSshOptions) were shellescaped. The token array is
joined and run via execAsync (/bin/sh -c), so a jumpHost like "x; touch /tmp/pwned"
executed. The Zod denylist only blocked backtick/newline/$( and let ;|& and spaces
through.

- shellescape jumpHost in buildSshConnectionArgs (primary fix)
- replace jumpHost denylist with a structural allowlist: [user@]host[:port],
  comma-separated multi-hop, bracketed IPv6; no shell metachar can appear
- update/extend tests: escaped -J assertion + injection-safety case

Verified: remote-ssh-options (11) + case-routes (33) pass, tsc --noEmit clean,
regex accepts valid forms / rejects 8 injection payloads.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 src/remote-hosts.ts             |  4 ++--
 src/web/schemas.ts              | 12 +++++++++---
 test/remote-ssh-options.test.ts | 14 ++++++++++++--
 3 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/src/remote-hosts.ts b/src/remote-hosts.ts
index eb1b1c3d..f7525af9 100644
--- a/src/remote-hosts.ts
+++ b/src/remote-hosts.ts
@@ -105,7 +105,7 @@ function expandIdentityPath(identityFile: string): string {
  *   ssh -o BatchMode=yes
  *       [-p <port>]
  *       [-i <abs-identity>]              (~/$HOME expanded, then shellescaped)
- *       [-J <jumpHost>]
+ *       [-J <jumpHost>]                  (shellescaped, single token)
  *       [-o ProxyCommand=nc -X 5 -x <socks> %h %p]   (ONE shellescaped -o token)
  *       [-o <KEY=VALUE>] …               (each extra option, shellescaped)
  *
@@ -120,7 +120,7 @@ export function buildSshConnectionArgs(remote: RemoteSshOptions & Pick<RemoteHos
   const parts: string[] = ['ssh', '-o BatchMode=yes'];
   if (remote.port) parts.push(`-p ${remote.port}`);
   if (remote.identityFile) parts.push(`-i ${shellescape(expandIdentityPath(remote.identityFile))}`);
-  if (remote.jumpHost) parts.push(`-J ${remote.jumpHost}`);
+  if (remote.jumpHost) parts.push(`-J ${shellescape(remote.jumpHost)}`);
   if (remote.socksProxy) {
     parts.push(`-o ${shellescape(`ProxyCommand=nc -X 5 -x ${remote.socksProxy} %h %p`)}`);
   }
diff --git a/src/web/schemas.ts b/src/web/schemas.ts
index 97497c0e..a9c0e443 100644
--- a/src/web/schemas.ts
+++ b/src/web/schemas.ts
@@ -315,13 +315,19 @@ export const RemoteHostSchema = z.object({
     .string()
     .regex(/^[\w.-]+:\d{1,5}$/, 'SOCKS proxy must be host:port')
     .optional(),
-  // SSH jump host ([user@]host[:port]); reject shell metacharacters.
+  // SSH jump host: a comma-separated chain of [user@]host[:port] hops. Structural
+  // ALLOWLIST (not an open denylist) — only chars valid in user/host/port/IPv6,
+  // so no shell metacharacter (;, |, &, space, $, quotes, …) can appear. The value
+  // is also shellescaped at command-build time (buildSshConnectionArgs); this is the
+  // belt to that suspenders.
   jumpHost: z
     .string()
     .min(1)
     .max(255)
-    .regex(NO_SHELL_INJECTION, 'Invalid jump host')
-    .refine(noCommandSubstitution, 'Invalid jump host')
+    .regex(
+      /^(?:[A-Za-z0-9._-]+@)?[A-Za-z0-9.:\[\]-]+(?::\d{1,5})?(?:,(?:[A-Za-z0-9._-]+@)?[A-Za-z0-9.:\[\]-]+(?::\d{1,5})?)*$/,
+      'Jump host must be [user@]host[:port] (comma-separated for multiple hops)'
+    )
     .optional(),
   // Arbitrary extra -o KEY=VALUE options (escape hatch); each must be KEY=VALUE.
   extraSshOptions: z
diff --git a/test/remote-ssh-options.test.ts b/test/remote-ssh-options.test.ts
index dde21d04..d3bb7de7 100644
--- a/test/remote-ssh-options.test.ts
+++ b/test/remote-ssh-options.test.ts
@@ -87,9 +87,19 @@ describe('COD-107 buildSshConnectionArgs — shared ssh connection tokens', () =
     expect(idxProxy).toBeLessThan(idxExtra);
   });
 
-  it('supports an explicit -J jump host', () => {
+  it('supports an explicit -J jump host (shellescaped, like its siblings)', () => {
     const args = buildSshConnectionArgs({ ...baseRemote, jumpHost: 'bastion@10.0.0.1:22' });
-    expect(args.join(' ')).toContain('-J bastion@10.0.0.1:22');
+    expect(args.join(' ')).toContain("-J 'bastion@10.0.0.1:22'");
+  });
+
+  it('shellescapes a -J jump host containing shell metacharacters (no injection)', () => {
+    // Defense-in-depth: even if a metachar-laden value slipped past schema validation,
+    // it must stay a single shell token and never break out of the ssh command.
+    const args = buildSshConnectionArgs({ ...baseRemote, jumpHost: 'x; touch /tmp/pwned' });
+    const joined = args.join(' ');
+    // The whole value is wrapped in single quotes — the `;` cannot start a new command.
+    expect(joined).toContain("-J 'x; touch /tmp/pwned'");
+    expect(joined).not.toContain('-J x;');
   });
 
   it('expands a $HOME-prefixed identity path', () => {