From 4dd75fff29c3ea9b6e01da1464bc2697fdc6b8ef Mon Sep 17 00:00:00 2001 From: Codex Date: Wed, 3 Jun 2026 07:05:53 +0000 Subject: [PATCH] fix: reject shell strings in trans argv --- scripts/src/help.ts | 1 + scripts/src/ssh.ts | 20 +++++++++++++++++++- scripts/ssh-argv-guidance-contract-test.ts | 14 +++++++++++++- 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/scripts/src/help.ts b/scripts/src/help.ts index ef089d85..fd826166 100644 --- a/scripts/src/help.ts +++ b/scripts/src/help.ts @@ -191,6 +191,7 @@ export function sshHelp(): unknown { notes: [ "trans --help and trans --help print this JSON help and never open an interactive session; the underlying ssh subcommand keeps the same help behavior.", "For non-interactive remote commands, prefer argv for a single process and script/stdin for shell logic.", + "`argv` executes direct argv tokens only: `trans argv ls -la` is valid, but `trans argv 'ls -la'` is rejected because the single string would be treated as an executable path; use `script -- 'ls -la'` for one-line shell logic.", "For one-line remote shell logic without a heredoc, use `script -- ''`; outer shell operators written outside trans, such as `trans G14:/repo sed ... && sed ...`, are parsed by the local shell before UniDesk starts and therefore cannot be redirected by the CLI. The explicit `shell ''` operation remains available for the same sh -c path.", "When a one-line shell command is easier to type through the script path, `script -- ''` runs that single string through the remote shell without waiting for stdin. When `script --` is followed by multiple tokens, it stays a direct argv form for commands such as `trans D601:/work script -- sed -n '1,20p' file`.", "script and shell helper modes inject a tiny POSIX-compatible printf wrapper before user shell text, so portable printf headings such as `printf \"--- section ---\\n\"` work consistently under dash/sh and bash. Direct argv commands are unchanged.", diff --git a/scripts/src/ssh.ts b/scripts/src/ssh.ts index b185276b..9d11f84f 100644 --- a/scripts/src/ssh.ts +++ b/scripts/src/ssh.ts @@ -870,6 +870,7 @@ export function parseSshArgs(args: string[]): ParsedSshArgs { if (subcommand === "argv" || subcommand === "exec") { const toolArgs = args.slice(1); if (toolArgs.length === 0) throw new Error(`ssh ${subcommand} requires a command`); + validateDirectArgvCommand(subcommand, toolArgs); return { remoteCommand: shellArgv(toolArgs), requiresStdin: false, invocationKind: "argv" }; } if (subcommand === "find") { @@ -910,6 +911,20 @@ export function parseSshArgs(args: string[]): ParsedSshArgs { }; } +function validateDirectArgvCommand(commandName: string, toolArgs: string[]): void { + if (toolArgs.length !== 1) return; + const command = toolArgs[0] ?? ""; + if (!looksLikeShellCommandString(command)) return; + throw new Error( + `ssh ${commandName} received one shell-like command string; ${commandName} executes a single process and treats that string as the executable path. ` + + `Use \`trans script -- ${shellQuote(command)}\` for one-line shell logic, or split argv tokens, for example \`trans ${commandName} ls -la\`.` + ); +} + +function looksLikeShellCommandString(value: string): boolean { + return /\s/u.test(value) || /&&|\|\||[;|<>]/u.test(value); +} + export function parseSshInvocation(target: string, args: string[]): ParsedSshInvocation { const route = parseSshRoute(target); if (route.plane === "k3s") { @@ -1401,7 +1416,10 @@ function parseK3sTargetOperation(route: ParsedSshRoute, args: string[]): ParsedS return { remoteCommand: buildK3sExecCommand([...targetArgs, "--", parsed.shell, "-c", shellScriptWithCompatibility(parsed.command)]), requiresStdin: false, invocationKind: "helper" }; } if (operation === "logs") return { remoteCommand: buildK3sLogsCommand([...targetArgs, ...operationArgs]), requiresStdin: false, invocationKind: "helper" }; - if (operation === "argv") return { remoteCommand: buildK3sExecCommand([...targetArgs, ...k3sRouteCommandArgs(operationArgs)]), requiresStdin: false, invocationKind: "argv" }; + if (operation === "argv") { + validateDirectArgvCommand(operation, operationArgs); + return { remoteCommand: buildK3sExecCommand([...targetArgs, ...k3sRouteCommandArgs(operationArgs)]), requiresStdin: false, invocationKind: "argv" }; + } if (operation === "get" || operation === "describe") { return { remoteCommand: buildK3sTargetObjectCommand(operation, route, operationArgs), requiresStdin: false, invocationKind: "helper" }; } diff --git a/scripts/ssh-argv-guidance-contract-test.ts b/scripts/ssh-argv-guidance-contract-test.ts index 722713de..3b35f554 100644 --- a/scripts/ssh-argv-guidance-contract-test.ts +++ b/scripts/ssh-argv-guidance-contract-test.ts @@ -382,6 +382,13 @@ export async function runSshArgvGuidanceContract(): Promise { assertCondition(argv.remoteCommand === "'true'", "argv command must shell-quote each token", argv); assertCondition(argv.requiresStdin === false, "argv command must not require stdin", argv); assertCondition(sshFailureHint("D601", argv, 255, "kex_exchange_identification: Connection closed by remote host") === null, "argv failures must not produce ssh-like friction hint", argv); + assertThrows( + () => parseSshInvocation("D601:/tmp", ["argv", "pwd && ls -la"]), + /one shell-like command string.*script --/u, + "argv must reject a single shell command string before the remote host treats it as an executable path", + ); + const argvExplicitShell = parseSshInvocation("D601:/tmp", ["argv", "sh", "-c", "pwd && ls -la"]); + assertCondition(argvExplicitShell.parsed.remoteCommand === "'sh' '-c' 'pwd && ls -la'", "argv must still allow explicit sh -c as multi-token direct argv", argvExplicitShell); const shortcut = parseSshArgs(["pwd"]); assertCondition(shortcut.invocationKind === "argv", "safe command shortcuts must use argv quoting", shortcut); @@ -559,6 +566,11 @@ export async function runSshArgvGuidanceContract(): Promise { const routeTargetArgv = parseSshInvocation("D601:k3s:hwlab-dev:hwlab-cloud-api", ["argv", "sh", "-c", "printf ok"]); assertCondition(routeTargetArgv.parsed.invocationKind === "argv", "k3s target argv operation must stay explicit argv", routeTargetArgv); assertCondition(routeTargetArgv.parsed.remoteCommand === "'env' 'KUBECONFIG=/etc/rancher/k3s/k3s.yaml' 'kubectl' 'exec' '-n' 'hwlab-dev' 'deployment/hwlab-cloud-api' '--' 'sh' '-c' 'printf ok'", "D601:k3s:: argv must exec the argv payload instead of treating argv as a pod command", routeTargetArgv); + assertThrows( + () => parseSshInvocation("D601:k3s:hwlab-dev:hwlab-cloud-api", ["argv", "pwd && ls -la"]), + /one shell-like command string.*script --/u, + "k3s workload argv must reject a single shell command string before kubectl exec treats it as an executable path", + ); const routeTargetShell = parseSshInvocation("D601:k3s:hwlab-dev:hwlab-cloud-api/app", ["shell", "pwd && ls"]); assertCondition(routeTargetShell.parsed.remoteCommand === shellArgv(["env", "KUBECONFIG=/etc/rancher/k3s/k3s.yaml", "kubectl", "exec", "-n", "hwlab-dev", "deployment/hwlab-cloud-api", "--", "sh", "-c", 'cd "$1" || exit; shift; exec "$@"', "unidesk-cwd", "/app", "sh", "-c", `${sshShellScriptPrelude()}\npwd && ls`]), "D601:k3s::/ shell must run shell logic after cd inside the pod", routeTargetShell); @@ -1075,7 +1087,7 @@ export async function runSshArgvGuidanceContract(): Promise { assertThrows( () => parseSshInvocation("D601:k3s:kubectl", ["get", "pods"]), - /route must locate a target only.*trans D601:k3s kubectl/u, + /route must locate a target only.*ssh D601:k3s kubectl/u, "operation names must not be accepted as k3s route segments", ); assertThrows(