diff --git a/docs-site/src/content/docs/guides/remote-link.md b/docs-site/src/content/docs/guides/remote-link.md index 17bf414b3cb..9cb6bf7d023 100644 --- a/docs-site/src/content/docs/guides/remote-link.md +++ b/docs-site/src/content/docs/guides/remote-link.md @@ -64,7 +64,7 @@ To disconnect a Child-initiated link, run `ocx disconnect` on the Child. It disc ## Troubleshooting -When a step fails, the dashboard shows the reason and, when SSH reported one, the last line of its error output under the message. +When a step fails, the dashboard shows the reason and, when SSH reported one, a short sanitized hint from its last non-empty error line under the message. Remote-shell errors can appear there even when the remote shell emits non-UTF-8 text; OpenCodex removes terminal controls, link keys and URL queries and limits the hint's length. - **Could not connect to the SSH host**: the host must accept your SSH key without a password prompt; `ssh -o BatchMode=yes true` must succeed from a terminal. A `ProxyCommand` helper such as `cloudflared` must be installed in `/opt/homebrew/bin`, `/usr/local/bin`, `~/.bun/bin`, `~/.local/bin` or another directory on the PATH OpenCodex runs with. - **ocx was not found on the remote computer**: OpenCodex looks for `ocx` on the PATH of a non-interactive SSH session first, then in `~/.bun/bin`, `~/.local/bin`, `/opt/homebrew/bin` and `/usr/local/bin`. If it is installed elsewhere, add that directory to PATH in a file the remote shell reads for non-interactive sessions, such as `~/.zshenv` for zsh. diff --git a/src/link/ssh-argv.ts b/src/link/ssh-argv.ts index e9a82bc827c..5b779432556 100644 --- a/src/link/ssh-argv.ts +++ b/src/link/ssh-argv.ts @@ -114,7 +114,7 @@ export function buildTunnelArgv(options: TunnelArgvOptions): string[] { export interface ExecArgvOptions { alias: string; - /** Remote argv. Each element is quoted for the remote POSIX shell. */ + /** Remote argv. The command must be sh; its arguments are quoted for the remote shell. */ argv: readonly string[]; knownHostsFile: string; } @@ -177,10 +177,13 @@ export function buildResolveArgv(alias: string): string[] { return ["ssh", "-G", "--", assertSshAlias(alias)]; } -/** Quote argv for a POSIX remote shell: every element single-quoted, embedded quotes escaped. */ +/** Emit only sh in command position; single-quote every argument for the remote shell. */ export function quoteRemote(argv: readonly string[]): string { - return argv.map(arg => { + if (argv[0] !== "sh") throw new LinkSshArgumentError("remote command must be sh"); + return argv.map((arg, index) => { if (arg.includes("\0")) throw new LinkSshArgumentError("remote argument contains NUL"); + // PowerShell treats a leading quoted word as an expression, not a command invocation. + if (index === 0) return "sh"; return `'${arg.replaceAll("'", `'"'"'`)}'`; }).join(" "); } diff --git a/src/link/ssh-runner.ts b/src/link/ssh-runner.ts index 995de65ee99..4aa58ed4cd2 100644 --- a/src/link/ssh-runner.ts +++ b/src/link/ssh-runner.ts @@ -93,7 +93,7 @@ export class SshRunnerError extends Error { } } -async function readOutput(stream: ReadableStream, maxBytes: number, kill?: () => void): Promise { +async function readOutput(stream: ReadableStream, maxBytes: number, kill?: () => void, fatalUtf8 = true): Promise { const reader = stream.getReader(); const chunks: Uint8Array[] = []; let total = 0; @@ -118,7 +118,7 @@ async function readOutput(stream: ReadableStream, maxBytes: number, offset += chunk.byteLength; } try { - return new TextDecoder("utf-8", { fatal: true }).decode(bytes); + return new TextDecoder("utf-8", { fatal: fatalUtf8 }).decode(bytes); } catch (error) { throw new SshRunnerError("decode", "ssh output was not valid UTF-8", { cause: error }); } @@ -150,7 +150,7 @@ export function createSshRunner(deps: { spawn?: typeof Bun.spawn; timeoutMs?: nu throw new SshRunnerError("spawn", `could not spawn ${argv[0] ?? "ssh"}`, { cause: error }); } const stdout = readOutput(outputStream(child.stdout), DEFAULT_OUTPUT_BYTES, () => defaultKill(child, "SIGTERM")); - const stderr = readOutput(outputStream(child.stderr), DEFAULT_OUTPUT_BYTES, () => defaultKill(child, "SIGTERM")); + const stderr = readOutput(outputStream(child.stderr), DEFAULT_OUTPUT_BYTES, () => defaultKill(child, "SIGTERM"), false); void stdout.catch(() => {}); void stderr.catch(() => {}); return { @@ -181,8 +181,8 @@ export function createSshRunner(deps: { spawn?: typeof Bun.spawn; timeoutMs?: nu throw new SshRunnerError("spawn", `could not spawn ${argv[0] ?? "ssh"}`, { cause: error }); } - const stdout = readOutput(outputStream(child.stdout), Math.min(maxOutputBytes, DEFAULT_OUTPUT_BYTES), () => defaultKill(child, "SIGTERM")); - const stderr = readOutput(outputStream(child.stderr), Math.min(maxOutputBytes, DEFAULT_OUTPUT_BYTES), () => defaultKill(child, "SIGTERM")); + const stdout = readOutput(outputStream(child.stdout), Math.min(maxOutputBytes, DEFAULT_OUTPUT_BYTES), () => defaultKill(child, "SIGTERM")); + const stderr = readOutput(outputStream(child.stderr), Math.min(maxOutputBytes, DEFAULT_OUTPUT_BYTES), () => defaultKill(child, "SIGTERM"), false); if (options.stdin !== undefined) { try { const input = child.stdin; diff --git a/structure/remote-link.md b/structure/remote-link.md index c2bc2f3b03b..cfa1e878c26 100644 --- a/structure/remote-link.md +++ b/structure/remote-link.md @@ -4,7 +4,7 @@ `src/link/ssh-argv.ts` builds every OpenSSH argument vector. All commands run with BatchMode and trust only the link known_hosts file, keyed by `HostKeyAlias=`: the global file is disabled with `GlobalKnownHostsFile=none`, and `KnownHostsCommand=none`, `VerifyHostKeyDNS=no` and `CheckHostIP=no` shut out every other source of host-key trust. Command-line options take precedence over `~/.ssh/config`, so a user config cannot re-enable them. Tunnel and exec commands use `StrictHostKeyChecking=yes`; only the probe uses `accept-new`, against an empty temporary file, so an offered key can be shown before it is trusted. The known_hosts path must be absolute and free of ssh expansion syntax. Forwards bind 127.0.0.1 on both ends, and aliases that could be parsed as options are refused. -Every remote `ocx` call goes through `remoteOcxArgv`, which runs `sh -c` with a PATH prelude that appends `$HOME/.bun/bin`, `$HOME/.local/bin`, `/opt/homebrew/bin` and `/usr/local/bin` after the remote PATH and then `exec ocx "$@"`. A non-interactive ssh session reads no interactive profile, so without the fallbacks an ocx installed by Bun or Homebrew is not found; because they come last, an ocx the remote PATH already resolves keeps winning. `quoteRemote` single-quotes the script, so the login shell passes it through and `$HOME` and `$PATH` expand in the remote `sh`; the arguments reach ocx without another round of parsing. A remote exit status of 127 means ocx was not found and maps to `remote_ocx_missing`. +Every remote `ocx` call goes through `remoteOcxArgv`, which runs `sh -c` with a PATH prelude that appends `$HOME/.bun/bin`, `$HOME/.local/bin`, `/opt/homebrew/bin` and `/usr/local/bin` after the remote PATH and then `exec ocx "$@"`. A non-interactive ssh session reads no interactive profile, so without the fallbacks an ocx installed by Bun or Homebrew is not found; because they come last, an ocx the remote PATH already resolves keeps winning. `quoteRemote` accepts only `sh` in command position and emits it bare, so a PowerShell SSH default shell parses a command invocation; any other command name gets `LinkSshArgumentError`. It single-quotes every argument, including the script, so `$HOME` and `$PATH` expand only in the invoked `sh` and the arguments reach ocx without another round of parsing. A remote exit status of 127 means ocx was not found and maps to `remote_ocx_missing`. `src/link/ssh-config.ts` lists host candidates from `~/.ssh/config`. Arguments are split with the rules of OpenSSH's `argv_split`. Pattern hosts, `Match` blocks and aliases that fail the alias check produce no candidates, and only top-level `Include` directives are followed, because an include inside a `Host` or `Match` block is conditional. A candidate is an offer, not trust. @@ -38,7 +38,7 @@ A successful join restarts this proxy (a 503 drain of up to a minute while runni A connected Child answers `GET` and `HEAD /api/link/status` on its own listener for a GUI session: `src/client/link-status.ts` projects the client sidecar and the tunnel supervisor into the K16 document with `role: "child"`, the listener off, no links and the child row, plus `joinAvailable: false`. A Home-initiated Child has no sidecar and reports `child: null`. In link mode `/api/machine/status` advertises the machine origin as the shared plane, because the tunnel's hub-link ingress serves no `/api/*` and no session bootstrap. -`confirm-host` requires the remote `ocx --version` to print `opencodex ..` of at least 2.66.0, the first release with `ocx link`. The version is parsed to a bounded semver shape: each number has at most nine digits, an optional pre-release and build of at most 64 identifier characters each follow, and the token must end there. An older version answers `409 remote_ocx_outdated`, output that does not start with such a line (a usage banner, or a version with anything else attached) answers `502 remote_ocx_unrecognized`, and exit 127 answers `502 remote_ocx_missing`. Every refusal restores the link known_hosts file and keeps the pending probe, so a retry within the probe TTL needs no new probe. Link error bodies may carry `error.hint`, one line from one of three sources: the last non-empty ssh stderr line with terminal escapes removed, the ssh runner's own spawn, timeout or output-limit failure, or, for `remote_ocx_outdated`, `opencodex ` built only from the bounded version match. Every hint then has control and bidi characters removed, OpenCodex secrets and URL queries redacted, and is capped at 160 code points, cut between code points so a surrogate pair is never split. Hints never come from stdin and are never logged. +`confirm-host` requires the remote `ocx --version` to print `opencodex ..` of at least 2.66.0, the first release with `ocx link`. The version is parsed to a bounded semver shape: each number has at most nine digits, an optional pre-release and build of at most 64 identifier characters each follow, and the token must end there. An older version answers `409 remote_ocx_outdated`, output that does not start with such a line (a usage banner, or a version with anything else attached) answers `502 remote_ocx_unrecognized`, and exit 127 answers `502 remote_ocx_missing`. Every refusal restores the link known_hosts file and keeps the pending probe, so a retry within the probe TTL needs no new probe. Link error bodies may carry `error.hint`, one line from one of three sources: the last non-empty ssh stderr line with terminal escapes removed, the ssh runner's own spawn, timeout or output-limit failure, or, for `remote_ocx_outdated`, `opencodex ` built only from the bounded version match. Stderr bytes are capped before UTF-8 replacement decoding; structured stdout stays strict UTF-8. Every hint then has control and bidi characters removed, OpenCodex secrets and URL queries redacted, and is capped at 160 code points, cut between code points so a surrogate pair is never split. Hints never come from stdin and are never logged. ## Tunnels, management and CLI diff --git a/tests/clients/link-ssh-argv.test.ts b/tests/clients/link-ssh-argv.test.ts index 8d17fe617da..1cb14640700 100644 --- a/tests/clients/link-ssh-argv.test.ts +++ b/tests/clients/link-ssh-argv.test.ts @@ -64,11 +64,11 @@ test("tunnel argv uses a loopback forward and the confirmed host-key policy", () } }); -test("exec argv quotes the remote command and clears forwarding", () => { +test("exec argv emits only sh bare, quotes arguments, and clears forwarding", () => { const knownHostsFile = tempPath("exec"); const argv = buildExecArgv({ alias: "beta.example.test", - argv: ["printf", "it's ready"], + argv: ["sh", "it's ready"], knownHostsFile, }); expect(argv).toContain("-T"); @@ -76,7 +76,7 @@ test("exec argv quotes the remote command and clears forwarding", () => { expectCommonTrustOptions(argv, "yes", knownHostsFile); expect(argv).toContain("ClearAllForwardings=yes"); expect(argv.slice(-3, -1)).toEqual(["--", "beta.example.test"]); - expect(argv[argv.length - 1]).toBe(`'printf' 'it'"'"'s ready'`); + expect(argv[argv.length - 1]).toBe(`sh 'it'"'"'s ready'`); }); test("probe argv uses accept-new only with its temporary known_hosts file", () => { @@ -130,16 +130,36 @@ test("known_hosts option paths are absolute and safely quoted", () => { } }); -test("quoteRemote escapes single quotes and rejects NUL", () => { - expect(quoteRemote(["it's"])).toBe(`'it'"'"'s'`); - expect(() => quoteRemote(["bad\0argument"])).toThrow(LinkSshArgumentError); +test("quoteRemote allows only sh in command position and rejects NUL", () => { + expect(quoteRemote(["sh", "it's", "-c"])).toBe(`sh 'it'"'"'s' '-c'`); + for (const command of ["", "printf", "1", ".", "-x", "sh;echo bad", "sh\n", "sh\0bad"]) { + expect(() => quoteRemote([command, "safe"])).toThrow(LinkSshArgumentError); + } + expect(() => quoteRemote(["sh", "bad\0argument"])).toThrow(LinkSshArgumentError); }); test("remote ocx argv runs ocx through a single-quoted sh PATH prelude", () => { expect(REMOTE_OCX_SCRIPT).toBe('PATH="$PATH:$HOME/.bun/bin:$HOME/.local/bin:/opt/homebrew/bin:/usr/local/bin"; exec ocx "$@"'); expect(remoteOcxArgv(["link", "port"])).toEqual(["sh", "-c", REMOTE_OCX_SCRIPT, "ocx", "link", "port"]); const argv = buildExecArgv({ alias: "delta.example.test", argv: remoteOcxArgv(["link", "issue", "--alias", "it's x", "--json"]), knownHostsFile: tempPath("remote-ocx") }); - expect(argv.at(-1)).toBe(`'sh' '-c' 'PATH="$PATH:$HOME/.bun/bin:$HOME/.local/bin:/opt/homebrew/bin:/usr/local/bin"; exec ocx "$@"' 'ocx' 'link' 'issue' '--alias' 'it'"'"'s x' '--json'`); + expect(argv.at(-1)).toBe(`sh '-c' 'PATH="$PATH:$HOME/.bun/bin:$HOME/.local/bin:/opt/homebrew/bin:/usr/local/bin"; exec ocx "$@"' 'ocx' 'link' 'issue' '--alias' 'it'"'"'s x' '--json'`); +}); + +test.skipIf(process.platform !== "win32")("PowerShell parses the remote command as sh invocation", () => { + const remote = quoteRemote(remoteOcxArgv(["link", "port"])); + const script = [ + "$tokens = $null; $errors = $null", + "$ast = [System.Management.Automation.Language.Parser]::ParseInput($env:OCX_REMOTE_COMMAND, [ref]$tokens, [ref]$errors)", + "if ($errors.Count -ne 0) { Write-Error ($errors | Out-String); exit 1 }", + "$commands = @($ast.FindAll({ param($node) $node -is [System.Management.Automation.Language.CommandAst] }, $true))", + "if ($commands.Count -ne 1 -or $commands[0].GetCommandName() -cne 'sh') { Write-Error 'remote command did not dispatch sh'; exit 1 }", + "Write-Output $commands[0].GetCommandName()", + ].join("; "); + const result = Bun.spawnSync(["powershell.exe", "-NoProfile", "-NonInteractive", "-Command", script], { + env: { ...process.env, OCX_REMOTE_COMMAND: remote }, + }); + expect(result.exitCode).toBe(0); + expect(result.stdout.toString().trim()).toBe("sh"); }); function fakeOcx(dir: string, label: string): void { @@ -170,6 +190,19 @@ test.skipIf(process.platform === "win32")("an ocx the remote PATH already resolv expect(result.stdout.toString().split("\n")[0]).toBe("first"); }); +test.skipIf(process.platform === "win32")("the constructed remote command preserves POSIX argument bytes", () => { + const home = mkdtempSync(join(tmpdir(), "ocx-link-remote-bytes-")); + roots.push(home); + const bin = join(home, ".bun", "bin"); + mkdirSync(bin, { recursive: true }); + writeFileSync(join(bin, "ocx"), "#!/bin/sh\nprintf '%s\\0' \"$@\"\n", { mode: 0o755 }); + const args = ["it's x", "two\nlines", "火🔥", "x;$(echo no)", ""]; + const remote = quoteRemote(remoteOcxArgv(args)); + const result = Bun.spawnSync(["/bin/sh", "-c", remote], { env: { HOME: home, PATH: "/usr/bin:/bin" } }); + expect(result.exitCode).toBe(0); + expect(result.stdout).toEqual(new TextEncoder().encode(args.join("\0") + "\0")); +}); + test("ssh PATH appends helper directories once and leaves Windows untouched", () => { expect(linkSshPath({ PATH: "/usr/bin:/bin:/usr/sbin:/sbin", HOME: "/Users/test" }, "darwin")) .toBe("/usr/bin:/bin:/usr/sbin:/sbin:/opt/homebrew/bin:/usr/local/bin:/Users/test/.bun/bin:/Users/test/.local/bin"); @@ -190,6 +223,47 @@ function fakeSpawn(captured: Array>): typeof Bun.spawn { }) as unknown as typeof Bun.spawn; } +function byteSpawn(stdoutBytes: Uint8Array, stderrBytes: Uint8Array, capturedStdin: Uint8Array[] = []): typeof Bun.spawn { + const stream = (bytes: Uint8Array) => new ReadableStream({ start(controller) { controller.enqueue(bytes); controller.close(); } }); + return ((_argv: string[], _options: Record) => ({ + pid: 7, + stdout: stream(stdoutBytes), + stderr: stream(stderrBytes), + stdin: { async write(value: string | Uint8Array) { capturedStdin.push(typeof value === "string" ? new TextEncoder().encode(value) : value); }, async end() {} }, + exited: Promise.resolve(1), + kill() {}, + })) as unknown as typeof Bun.spawn; +} + +test("runner decodes capped non-UTF-8 stderr for a bounded redacted hint and keeps key stdin separate", async () => { + const secret = `ocx_data_${"a".repeat(40)}`; + const stderr = new Uint8Array([ + ...new TextEncoder().encode(`noise\nssh: ${secret} https://example.test/path?key=hidden `), + 0xa1, 0xad, + ...new TextEncoder().encode("\n"), + ]); + const stdin: Uint8Array[] = []; + const runner = createSshRunner({ spawn: byteSpawn(new TextEncoder().encode(""), stderr, stdin) }); + const result = await runner.run(["ssh"], { stdin: secret }); + expect(result.stderr).toContain("\ufffd"); + const hint = sshFailureHint(result.stderr); + expect(hint).toContain("ssh: ocx_data_[redacted] https://example.test/path"); + expect(hint).not.toContain(secret); + expect(hint).not.toContain("key=hidden"); + expect(Array.from(hint ?? "").length).toBeLessThanOrEqual(160); + expect(stdin).toEqual([new TextEncoder().encode(secret)]); +}); + +test("runner still rejects invalid UTF-8 stdout", async () => { + const runner = createSshRunner({ spawn: byteSpawn(new Uint8Array([0xa1, 0xad]), new TextEncoder().encode("diagnostic")) }); + await expect(runner.run(["ssh"])).rejects.toMatchObject({ code: "decode" }); +}); + +test("runner enforces stderr byte limit before replacement decoding", async () => { + const runner = createSshRunner({ spawn: byteSpawn(new Uint8Array(), new Uint8Array([0xa1, 0xad])) }); + await expect(runner.run(["ssh"], { maxOutputBytes: 1 })).rejects.toMatchObject({ code: "output_limit" }); +}); + test("the runner spawns commands and tunnels with the augmented environment", async () => { const captured: Array> = []; const runner = createSshRunner({ spawn: fakeSpawn(captured), env: () => linkSshSpawnEnv({ PATH: "/usr/bin:/bin", HOME: "/h" }, "darwin") });