mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-14 18:39:33 +01:00
fix: apply the review items for #691 at merge
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>
This commit is contained in:
1 parent
5b9445de9e
commit
1444c41250
2 files changed
+11
-1
No files matched your search
@@ -89,7 +89,7 @@ File reads in a remote case are the second ssh surface (see Remote SSH cases bel
|
||||
|
||||
**Remote SSH cases** (COD-94/#145): cases can point at a **remote host** (`~/.codeman/remote-hosts.json` + `remote-cases.json` via `src/remote-hosts.ts`; CRUD under `/api/cases` — cases route file). A remote session launches a LOCAL tmux pane running `ssh <host>` that creates a durable REMOTE tmux session on a **dedicated socket** `-L codeman-remote` with name `codeman-ssh-<id>` — deliberately failing the remote Codeman's `SAFE_MUX_NAME_PATTERN` so a Codeman instance on the target host never adopts it; no `-g` global tmux options are set remotely. `remotePath`/`identityFile` are schema-guarded against shell injection (backticks/`$` rejected — same approach as `extraSshOptions`); remote tmux availability is probed via `checkRemoteTmuxAvailable()` in quick-start (ssh args carry `-o ConnectTimeout=10`). Remote claude defaults to an idempotent `claude --session-id <id> || claude --resume <id>` pair under a login shell, so a respawn or reattach continues the SAME conversation rather than starting a fresh one (remote omp gets the same treatment via `--continue`; ⚠️ because the claude arm is an `a || b` pair under `-c`, that pane's PID is the login shell, not the agent); per-host `commands.*` override. Session kill best-effort kills the remote tmux too. `SessionState.remote`/`MuxSession.remote` round-trip through recovery (`restoreMuxSessions` passes `remote` back into the Session constructor). ⚠️ Run flows must route remote cases through `POST /api/quick-start` (which resolves the remote case and skips LOCAL CLI availability gates) — `POST /api/sessions` stat-validates `workingDir` locally and has no `caseName`. `envOverrides`/`effort`/`modelOverride`/`codexConfig`/`geminiConfig` are rejected for remote quick-starts (not silently dropped). UI: Create Case modal → Remote tab. Tests: `test/remote-hosts.test.ts`, `test/remote-ssh-options.test.ts`. ⚠️ **Reading a file in a remote case goes over ssh too** (#415): `src/remote-files.ts` is the single remote-READ layer (`buildRemoteFileCommand` = `buildSshConnectionArgs` + one shellescaped remote command; `remoteProbePaths` returns remote realpath + stat; `remoteCreateReadStream` streams a `Range` via `tail -c +N | head -c L` and its `close()` must be wired to the response's `close` or the ssh child outlives an aborted download). The guard order matches the local path exactly (`validateSessionFilePathLexical` → remote realpath of BOTH file and workspace root → containment → sensitive-path → size cap on the REMOTE size), a request path arrives from the browser and is only ever interpolated as a `shellescape`d token, and an unreachable host answers **502**, never a 404. ⚠️ The probe's symlink resolution FAILS CLOSED: `readlink -f` where it exists, otherwise a `cd -P`/`pwd -P` directory walk plus a bounded plain-`readlink` loop over the last component, and anything it cannot fully resolve is reported unresolvable (404), never as the unresolved string — the first version resolved the directory chain only, so on a host without `readlink -f` a `ws/notes.txt -> ~/.ssh/id_rsa` link passed containment under its own path while `cat` served the key. Records are NUL-separated and index-keyed so a newline in a filename cannot shift the mapping. ⚠️ ssh children are BOUNDED: probes and buffered reads go through `src/remote-ssh-limiter.ts` (a `document-conversion-limiter`-shaped semaphore, default 4), the attachment-history list probes its whole history in ONE batched call (`probeRemoteAttachmentHistory`, threaded into `registerExternalAttachment({remoteProbes})`), and probes chunk at 40 paths — a prompt-injected agent printing `codeman://attach` links in a remote session used to fork one `ssh` per link. `describeExecError` never returns Node's `Command failed: <ssh line>` message (identity path + probe script in a 502 body). The `PUT /file-content` guard sits AHEAD of `validateSessionFilePath`, which resolves LOCALLY, or a same-named local directory (an sshfs mount) takes the write. Under `VITEST` the three IO functions refuse rather than connect. This covers the ATTACHMENT routes too, which is the half a clicked path needs when the file is OUTSIDE the case directory (`_isExternalPreviewPath` sends it to `POST …/attachments`): registration, by-id `raw`, metadata and the history list all resolve over ssh (`registerExternalAttachment({remote})`, `resolveServableRemoteAttachment`), and what decides the host is the SESSION, never the path string — the same absolute path means a different file on each host. Deliberately NOT supported over ssh: writes (`edit=1`/`PUT` answer 400, `editable` is always false), office previews/thumbnails, the file tree/picker, `tail-file`. Tests: `test/remote-files.test.ts`, `test/routes/file-routes-remote.test.ts`.
|
||||
|
||||
**Remote tmux PATH**: every SSH command that invokes or locates tmux uses `withRemoteTmuxPath()` (`remote-hosts.ts`) before the complete remote script is shell-escaped: link preflight, launch/attach, alive probes, discovery, history/screen capture, and cleanup. Non-interactive SSH shells can omit Homebrew and per-user binaries even when the user's interactive shell finds them. The wrapper double-quotes the exported PATH value (including inherited PATH) so homes with spaces work under dash; it leaves the existing SSH options, socket/target and inner CLI login-shell handling intact. Keep new remote tmux operations on this same path. `test/remote-tmux-path.test.ts` exercises all builders under real sh/dash with a sparse PATH and a home containing spaces, using offline SSH/tmux stubs. Local and Docker tmux commands keep their own setup.
|
||||
**Remote tmux PATH**: every SSH command that invokes or locates tmux uses `withRemoteTmuxPath()` (`remote-hosts.ts`) before the complete remote script is shell-escaped: link preflight, launch/attach, alive probes, discovery, history/screen capture, and cleanup. Non-interactive SSH shells can omit Homebrew and per-user binaries even when the user's interactive shell finds them. The wrapper double-quotes the exported PATH value (including inherited PATH) so homes with spaces work under dash; it leaves the existing SSH options, socket/target and inner CLI login-shell handling intact. tmux gives a new pane the PATH of the client that created it (measured on tmux 3.4), so every new remote pane, agent or shell, now starts on the widened PATH before its login shell runs. Keep new remote tmux operations on this same path. `test/remote-tmux-path.test.ts` exercises all builders under real sh/dash with a sparse PATH and a home containing spaces, using offline SSH/tmux stubs. Local and Docker tmux commands keep their own setup.
|
||||
|
||||
Further wake-on-LAN detail: the wiring guards in `test/remote-wake.test.ts` are two; the second also pins that `server.ts` holds the registry for its LIFETIME only (`drop` on cleanup, `stop` on shutdown) and never calls a waking method. The create wake is wired in the route rather than the shared session service for exactly the no-timer-wakes reason. No `ServerAliveInterval`, because keepalives move bytes into an idle connection every interval and that is what a byte-threshold idle detector must not read as activity. For a host behind `jumpHost`/`socksProxy`/a `ProxyCommand` option, the probe connects to `host:port`, which such a host does not answer even while ssh works; the registry never gates create/attach on it (`'unprobeable'`), and `/reachability` answers `reachable: null, probeable: false`. The banner keys on a PROVEN `false`, and the banner's 30 s poller runs only for a host with a wake target (a timer connecting to a host Codeman cannot wake is the same timer-driven traffic the keepalive rule forbids).
|
||||
|
||||
|
||||
Reference in new issue
Block a user