From 21256202ba1e702d7f965f444d275ce23cd09d87 Mon Sep 17 00:00:00 2001 From: Randalix Date: Tue, 3 Nov 2026 15:20:22 +0100 Subject: [PATCH] fix(remote): hand remote/docker launch commands to bash verbatim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit respawn-pane runs `bash -c ""` through the local sh, so every `$` in the remote command is expanded on the Codeman host. withRemoteTmuxPath (`export PATH="$HOME/.local/bin:…:$PATH"; tmux …`, 5b9445de) therefore sends the Codeman host's home and PATH to the remote host: a remote user with a different home (tmux in ~/.nix-profile/bin) gets "tmux: command not found" and every new remote pane exits 127. `${SHELL:-/bin/sh}` in the remote launch was the local shell for the same reason. Remote and docker commands are already quoted for the far end, so they now go to `bash -c` single-quoted (bashCArg); local commands keep the double-quoted form. Two tests run the captured respawn-pane line (create and respawn) through a real sh with a foreign HOME/PATH and check that ssh gets `$HOME` and `$PATH` unexpanded. Co-Authored-By: Claude Opus 5.5 (cherry picked from commit 2818e9455489b9acd65841258df4cd035609954c) --- src/tmux-manager.ts | 18 ++++++++-- test/tmux-manager.test.ts | 74 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 2 deletions(-) diff --git a/src/tmux-manager.ts b/src/tmux-manager.ts index fc944d14..f596f9b2 100644 --- a/src/tmux-manager.ts +++ b/src/tmux-manager.ts @@ -2661,7 +2661,7 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { // pane launch in /tmp, then cd inside bash against the current mount table. const launchCmd = remote || docker ? fullCmd : `cd ${JSON.stringify(workingDir)} && ${fullCmd}`; execSync( - `${this.tmux()} respawn-pane -k -c ${TMUX_LAUNCH_CWD} -t "${muxName}" bash -c ${JSON.stringify(launchCmd)}`, + `${this.tmux()} respawn-pane -k -c ${TMUX_LAUNCH_CWD} -t "${muxName}" bash -c ${bashCArg(launchCmd, !!(remote || docker))}`, { timeout: EXEC_TIMEOUT_MS, stdio: 'ignore', @@ -2914,7 +2914,7 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { // -c /tmp + cd bounce — see createSession() for rationale (stale FUSE state). const launchCmd = remote || docker ? fullCmd : `cd ${JSON.stringify(workingDir)} && ${fullCmd}`; await execAsync( - `${this.tmux()} respawn-pane -k -c ${TMUX_LAUNCH_CWD} -t "${muxName}" bash -c ${JSON.stringify(launchCmd)}`, + `${this.tmux()} respawn-pane -k -c ${TMUX_LAUNCH_CWD} -t "${muxName}" bash -c ${bashCArg(launchCmd, !!(remote || docker))}`, { timeout: EXEC_TIMEOUT_MS, } @@ -4663,6 +4663,20 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { } } +/** + * The `bash -c` argument of a pane launch (`respawn-pane … bash -c `, run through the + * local sh). A remote or docker command is already quoted for the shell at the far end and + * must reach bash verbatim, so it goes in single quotes. Double quotes (JSON.stringify) let + * the local sh expand every `$` first: the PATH bootstrap of `withRemoteTmuxPath` + * (`$HOME/.local/bin:…:$PATH`) arrived on the remote host as this host's home and PATH, so + * a remote user with another home found no tmux and every new remote pane exited 127. + * `${SHELL:-/bin/sh}` in the remote launch was the local shell for the same reason. + * Local commands keep their double-quoted form. + */ +function bashCArg(launchCmd: string, verbatim: boolean): string { + return verbatim ? shellescape(launchCmd) : JSON.stringify(launchCmd); +} + /** * Shell-escape a string for use as a single argument. * Wraps in single quotes, escaping any embedded single quotes. diff --git a/test/tmux-manager.test.ts b/test/tmux-manager.test.ts index ebd5f927..32cf39b4 100644 --- a/test/tmux-manager.test.ts +++ b/test/tmux-manager.test.ts @@ -28,6 +28,20 @@ import type { MuxSession } from '../src/mux-interface.js'; // ============================================================================ // Mock child_process +/** + * Runs a captured `… respawn-pane … bash -c ` line through a real local sh (with a + * foreign HOME/PATH) and returns the string bash would get, i.e. what ssh sends on. + */ +async function bashArgAfterLocalShell(respawnCmd: string): Promise { + const real = await vi.importActual('node:child_process'); + const at = respawnCmd.indexOf(' bash -c '); + expect(at).toBeGreaterThan(0); + return real.execFileSync('/bin/sh', ['-c', `printf '%s' ${respawnCmd.slice(at + ' bash -c '.length)}`], { + env: { HOME: '/local-home-xyz', PATH: '/local-path-xyz:/usr/bin:/bin' }, + encoding: 'utf8', + }); +} + vi.mock('node:child_process', async () => { const actual = await vi.importActual('node:child_process'); return { @@ -865,6 +879,66 @@ describe('TmuxManager (unit)', () => { } }); + it('hands a remote launch to ssh with $HOME and $PATH unexpanded', async () => { + // respawn-pane runs `bash -c ` through the local sh. In double quotes that sh + // expanded withRemoteTmuxPath's `$HOME/…:$PATH` to the Codeman host's values, so a + // remote user with another home found no tmux and the pane exited 127. + const NonTestTmuxManager = await importWithTmuxCommandsEnabled(); + const nonTestManager = new NonTestTmuxManager(); + + try { + await nonTestManager.createSession({ + sessionId: 'abc12345-1234-5678-90ab-cdef12345679', + workingDir: '/w', + mode: 'shell', + remote: { hostId: 'gpu-box', label: 'GPU Box', host: '10.0.0.42', username: 'ubuntu', remotePath: '/w' }, + }); + const launch = [...mockedExecSync.mock.calls, ...vi.mocked(exec).mock.calls] + .map(([cmd]) => cmd) + .find((cmd) => typeof cmd === 'string' && cmd.includes(' respawn-pane ')); + const sent = await bashArgAfterLocalShell(launch as string); + expect(sent).toContain('ssh '); + expect(sent).toContain('export PATH="$HOME/'); + expect(sent).toContain(':$PATH"; tmux '); + expect(sent).not.toContain('local-home-xyz'); + expect(sent).not.toContain('local-path-xyz'); + } finally { + nonTestManager.destroy(); + } + }); + + it('respawns a remote pane with $HOME unexpanded as well', async () => { + const NonTestTmuxManager = await importWithTmuxCommandsEnabled(); + const nonTestManager = new NonTestTmuxManager(); + nonTestManager.registerSession({ + sessionId: 'respawnremote2', + muxName: 'codeman-abcd8888', + pid: 1000, + createdAt: Date.now(), + workingDir: '/w', + mode: 'shell', + attached: false, + }); + + try { + await nonTestManager.respawnPane({ + sessionId: 'respawnremote2', + workingDir: '/w', + mode: 'shell', + remote: { hostId: 'gpu-box', label: 'GPU Box', host: '10.0.0.42', username: 'ubuntu', remotePath: '/w' }, + }); + const { exec: currentExec } = await import('node:child_process'); + const respawnCall = vi + .mocked(currentExec) + .mock.calls.find(([cmd]) => typeof cmd === 'string' && cmd.includes(' respawn-pane ')); + const sent = await bashArgAfterLocalShell(respawnCall?.[0] as string); + expect(sent).toContain('export PATH="$HOME/'); + expect(sent).not.toContain('local-home-xyz'); + } finally { + nonTestManager.destroy(); + } + }); + it('changes the global history default on tmux versions that cannot resize panes', async () => { const NonTestTmuxManager = await importWithTmuxCommandsEnabled(); const nonTestManager = new NonTestTmuxManager();