Skip to content

fix(remote): use a shared quoted PATH for all remote tmux operations - #691

Closed
johoja12 wants to merge 1 commit into
Ark0N:masterfrom
johoja12:fix/15-upstream-remote-tmux-path
Closed

johoja12 wants to merge 1 commit into
Ark0N:masterfrom
johoja12:fix/15-upstream-remote-tmux-path

Conversation

@johoja12

Copy link
Copy Markdown
Contributor

Remote hosts with tmux installed in Homebrew or a per-user bin directory can fail Codeman's non-interactive SSH checks even though tmux works in their interactive shell. Fixing only history capture leaves those hosts blocked at linking or launch.

Add one withRemoteTmuxPath() wrapper and apply it to every remote tmux entry point: availability checks, launch, attach, alive probes, discovery, history/screen capture, and cleanup. The entire exported PATH value is double-quoted, including inherited PATH, so home directories and PATH entries containing spaces work under dash. Existing SSH options, PTY flags, sockets, session targeting, CLI login-shell handling, and capture/backoff behavior are preserved.

This is the separate PATH follow-up requested in #645. Tracks johoja12/Codeman#15.

Validation:

  • New real-shell regression: seven operations under both /bin/sh and /bin/dash, with sparse PATH, a remote home and inherited PATH entry containing spaces, and offline SSH/tmux stubs. All 14 cases fail before the fix and pass afterward. Argv checks cover port, identity, jump host, proxy, custom SSH options and quoted session names.
  • 514 focused/unit tests passed across remote commands, history, reconnect/wake, tmux manager and routes.
  • Typecheck, lint, formatting, lockfile, frontend syntax, public assets and browser-exclusion checks passed.
  • Isolated realistic Claude/Codex/shell smoke: 108 passed, zero failures, five documented skips (unconfigured external remote host and four established attachment-history baseline skips). All fixture sessions and temporary servers were cleaned up.

@Ark0N

Ark0N commented Oct 23, 2026

Copy link
Copy Markdown
Owner

Thank you @johoja12 for following up on #645 so quickly. This PR sends every remote tmux call (link preflight, launch, attach, alive probe, discovery, history capture and kill) through one withRemoteTmuxPath() wrapper with a double-quoted PATH, which is exactly the shape I asked for.

I checked it against master: the new test/remote-tmux-path.test.ts fails all 14 cases on master's source and passes here, all seven remote tmux call sites are covered (the CLI version probe and the remote file reads never call tmux, so leaving them alone is right), and CI is green.

Three small things, which I will apply on top when I merge:

  1. Pin the quoted string (test/remote-tmux-path.test.ts:71). On dash 0.5.11 and later (CI's dash is 0.5.12), plus bash and busybox, an unquoted export PATH=$HOME/... also passes the home-with-spaces test, because those shells do not field-split export arguments. The three tests that used to pin the literal (test/remote-auto-reconnect.test.ts:235, test/remote-ssh-options.test.ts:175 and :203, test/remote-discover-attach.test.ts:106) now compute their expectation from withRemoteTmuxPath() itself. So nothing fails if the quotes are dropped. I will add one literal assertion:
    expect(withRemoteTmuxPath('tmux -V')).toBe('export PATH="$HOME/.opencode/bin:$HOME/.local/bin:$HOME/bin:/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:$PATH"; tmux -V').
  2. One sentence in docs/architecture-invariants.md:92: tmux gives a new pane the creating client's PATH (I measured this on tmux 3.4), so every new remote pane, agent or shell, now starts on the widened PATH before its login shell runs. The paragraph should say so next to "leaves the inner CLI login-shell handling intact".
  3. Commit title: (#15) refers to your fork's issue, but in this repository [Security] HIGH: Symlink path traversal in file browser endpoints #15 is an unrelated closed security issue, and GitHub will link it there. I will reword it to johoja12/Codeman#15.

No need for another round. Thanks again for the careful regression test.

Ark0N pushed a commit that referenced this pull request Oct 24, 2026
test: pin the double-quoted PATH literal of withRemoteTmuxPath() (dash 0.5.11+, bash and busybox pass the shell cases unquoted)
docs: note in architecture-invariants that a new remote pane inherits the widened PATH from the creating tmux client

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Ark0N pushed a commit that referenced this pull request Oct 24, 2026
@Ark0N

Ark0N commented Oct 24, 2026

Copy link
Copy Markdown
Owner

This landed in 1.45.0, thank you! It's the follow-up promised in #645.

GitHub won't mark this PR as merged, because I reworded the commit subject before merging: (#15) became (johoja12/Codeman#15), since #15 in this repo is an unrelated issue and GitHub would have linked it. The commit on master (5b9445d) has the same tree and author as your 47dc43e. At merge I added a test pinning the double-quoted PATH literal of withRemoteTmuxPath(), and a note in architecture-invariants that a new remote pane inherits the widened PATH from the tmux client that creates it. Closing it by hand.

@Ark0N Ark0N closed this Oct 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants