Skip to content

fix(files): read remote-case previews, downloads and attachments over ssh - #421

Open
Randalix wants to merge 2 commits into
Ark0N:masterfrom
Randalix:fix/remote-file-access
Open

fix(files): read remote-case previews, downloads and attachments over ssh#421
Randalix wants to merge 2 commits into
Ark0N:masterfrom
Randalix:fix/remote-file-access

Conversation

@Randalix

Copy link
Copy Markdown

The bug

In a remote (SSH) case, opening a file that the agent produced never worked: the preview/download routes resolved the path against the local filesystem of whatever machine runs Codeman, while session.workingDir for a remote case is an absolute path on the remote host (Session.workingDir = RemoteCase.remotePath).

validateSessionFilePath() calls realpathSync on both the workspace and the candidate, so for a remote-only path it fails by construction and every route answered 404 File not found before a single byte was read — while the ssh-aware launch path right next to it (buildSshConnectionArgs, buildRemoteLaunchCommand) had no counterpart on the file side.

The same holds for a path outside the case directory, which is the half the frontend sends somewhere else entirely: _isExternalPreviewPath() routes every absolute path not under workingDir to POST /api/sessions/:id/attachments, whose registerExternalAttachment() did a local realpathSync too. Both halves are fixed here.

Fixes #415.

What changed

New src/remote-files.ts is the single remote-READ layer, built the same way as every other ssh line in the codebase — through buildSshConnectionArgs(), never a hand-built one (shellescape is now exported from remote-hosts.ts instead of being copied a third time):

  • remoteProbePaths(remote, paths) — ONE ssh round trip returning realpath + stat (size, mtime, kind) for the requested path and the workspace root. Resolving the root remotely is what keeps the boundary honest for a symlinked remotePath; the probe uses readlink -f where available and a POSIX cd/pwd -P fallback otherwise.
  • remoteCreateReadStream(remote, path, range) — streams the body (cat, or tail -c +N | head -c L for a Range, both constant-memory), nothing buffered in server RAM. Its close() is wired to the response's close event so an aborted download cannot leave an ssh process behind.
  • remoteReadFile() — bounded read for file-content.

Wired into: file-raw, file-content, file-preview, file-thumbnail; registerExternalAttachment() (+ the by-id raw/preview/thumbnail routes, the metadata poll, and the attachment history list); and the magic-link / Codex-artifact registration paths.

Guards keep their local strength — the same order, with symlinks resolved on the host that can resolve them:

  1. ownership (unchanged),
  2. lexical containment (a ../ escape is refused before any connection is opened),
  3. remote realpath of file and workspace root,
  4. containment of the resolved remote path against the resolved remote root (plus the sensitive-path blocklist on the routes that already apply it locally),
  5. size cap (CODEMAN_MAX_DOWNLOAD_BYTES) applied to the remote size, before the body is requested.

A browser-supplied ?path= is always interpolated as a single shellescape-quoted token; BatchMode=yes keeps a passphrase host from hanging. An unreachable host is a 502 with the remote reason — never the 404 that made an infrastructure problem look like a typo in the agent's output.

⚠️ There is deliberately no local fallback. A remote case reads the remote bytes or fails, even when a file with the same absolute name exists on the Codeman host (an sshfs mount of the same tree — the stop-gap workaround in the issue). Serving the local twin would silently hand back a different filesystem's bytes under a name the user believes is the remote file. Which host a record is read from follows the session, never the path string.

Deliberately NOT in this PR

Kept to the read path, so the diff stays reviewable:

  • Writesedit=1 / PUT now answer 400 "Editing is not supported for files in a remote (SSH) case" instead of the old misleading 404, and editable is always false; the viewer hides its Edit affordance. docs/file-viewer-edit-plan.md §6 already scoped SFTP out, and this PR makes the degradation honest rather than silent.
  • Office-document previews and generated thumbnails — both need the bytes on the server's disk (LibreOffice / first-page rendering). They answer 400 for remote instead of 404, and no remote file is ever copied onto the Codeman host (no temp spill).
  • The file-tree listing, the path picker, and tail-file.

Docker cases are untouched: their workspace is bind-mounted at the same absolute path, so local fs reads real bytes.

Tests

  • test/remote-files.test.ts (new) — command construction through the shared connection args (asserted by splitting the produced line into the argv ssh would actually receive), injection attempts (;, $(…), backticks, quotes, newlines) staying inside one quoted token, the probe parser (BSD/GNU stat, | in a filename, banners), and the probe script executed by a real /bin/sh against a temp dir, including a hostile filename that would touch a marker if the quoting were wrong.
  • test/routes/file-routes-remote.test.ts (new) — app.inject() with the ssh layer mocked: 200/206 (Range headers + args), 413 on the remote size, 404 without opening a connection, symlink escape, a symlinked workspace (must NOT be refused), an unreachable host → 502, directory → 400, the ssh child reaped on response end, the local path untouched (regression), and the full attachment path (register outside the case → by-id stream + Range, metadata size from the probe, 404/403/502, office+thumbnail 400, history list). Plus the same-path-on-both-hosts cases: remote bytes win, and a file that exists only locally is still a 404.
  • npm test (the CI gate): 6914 passed; typecheck, lint, format:check green. The only failures are 3 pre-existing git-clone/case-clone cases that also fail on master here (a git-version artefact: FAILED vs REF_NOT_FOUND), unrelated to this change.

Verified live against a real remote case

Albus (Linux, Codeman) → a second host over Tailscale: file-raw (relative and absolute remote path) returned bytes whose sha256 matched the remote file exactly, Range: bytes=10-19206 bytes 10-19/73, file-content returned the remote text, ../../../etc/passwd → 404, edit=1 → 400; and for the outside-the-case path: POST /attachments → 200 with the remote size, by-id raw → 200 with the matching sha256, Range → 206, metadata → remote size, thumbnail → 400, missing file → 404, /etc/shadow on the remote → 403.

Docs

  • docs/remote-sessions.md — new "File access over SSH" section: the layer, the guard order, the no-local-fallback rule, and what is not available.
  • docs/file-viewer-edit-plan.md §6 — corrected (reads work now; writes still don't).
  • docs/architecture-invariants.md + CLAUDE.md — the remote paragraph now names the file path as the second ssh surface.
  • Changeset: aicodeman: patch.

A remote case's workingDir is an absolute path on the remote host, but the
file read routes resolved it with local `fs`: `validateSessionFilePath`'s
realpathSync fails for a path that does not exist on the Codeman host, so
every preview of an agent-written file answered "File not found" (Ark0N#415).

Add src/remote-files.ts as the single remote-read layer, built on the same
buildSshConnectionArgs() the launch uses:

- remoteProbePaths(): ONE round trip returning realpath + stat for the
  requested path AND the workspace root, so containment is checked against a
  remotely canonicalized root (a symlinked remotePath is ordinary).
- remoteCreateReadStream(): streams the body (cat, or tail -c +N | head -c L
  for a Range) with nothing buffered in memory, and reaps the ssh child when
  the response ends so an aborted download cannot orphan it.
- remoteReadFile(): bounded read for file-content.

file-raw, file-content, file-preview and file-thumbnail now share one local/
remote target resolution. Guards keep their local strength: lexical pre-check,
remote realpath, workspace containment, sensitive-path blocklist, and the size
cap applied to the remote size before any bytes are read. An unreachable host
answers 502 with the remote reason instead of a misleading 404. Nothing is ever
copied to the Codeman host and there is NO local fallback (an sshfs mount of
the same tree must not shadow the remote bytes).

Deliberately unchanged: writes (edit=1 / PUT now answer 400 explicitly while
the viewer hides its Edit affordance), office previews, thumbnails, file tree,
picker, external attachment registration and tail-file stay local-only.
…side the case

A clicked path that points OUTSIDE the case directory goes through the attachment
routes (the frontend's `_isExternalPreviewPath` sends every absolute path not under
`workingDir` to `POST /attachments`), and those had the same local-`fs` assumption
as file-raw: `realpathSync`/`fs.stat` on a path that only exists on the remote host,
so the file never opened — the case the Ark0N#415 report was actually about.

- `registerExternalAttachment()` accepts `remote` and resolves through
  `remoteProbePaths` (canonical path, size/mtime, kind, plus the workspace root for
  the confinement check). Everything around it — blocklist, extension allowlist,
  workspace confinement, registry/dedupe — is now shared by both branches, so the
  remote path cannot drift from the local one.
- The by-id routes (`raw`, `preview`, `thumbnail`), the metadata poll and the
  attachment history list resolve over ssh too. `raw` streams with the same
  Range contract as file-raw; `preview` (office) and `thumbnail` answer 400 for a
  remote record; an unreachable host answers 502, a vanished file 404.
- Which host a record is read from follows the SESSION, never the path string: the
  same absolute path is a different file on each host, and a remote session never
  falls back to a local file with that name.
- Codex generated artifacts keep force-workspace confinement for a remote case: the
  well-known artifact directories are anchored at THIS host's home, so only a file
  inside the remote workspace is trusted.

Still local-only by design: writes, office conversion, thumbnails, the file
tree/picker and tail-file.
@Ark0N

Ark0N commented Sep 14, 2026

Copy link
Copy Markdown
Owner

Thanks for this, and for the depth of it: you added the whole missing ssh read side for remote cases (previews, downloads, text reads and the out-of-workspace attachment click path), reused buildSshConnectionArgs rather than hand-building a second ssh line, kept the 200/206/416 range contract, and wrote both a real-shell test for the probe script and a full route suite. Typecheck, lint, format, frontend syntax and the full npm test gate are all green here (6920 passed, 0 failed). A few things to fix before I merge.

Blocker: a symlink can escape the workspace on a host without readlink -f (src/remote-files.ts:113)

The fallback branch resolves the directory chain only, as your own comment at line 102 says, so the final component is left unresolved and that string is returned as realPath. Every caller then treats it as canonical: isPathWithinRoot in resolveFileTarget, isUnderTree in registerExternalAttachment, isBlockedAttachmentPath, and finally remoteCreateReadStream, which cats it. I reproduced it by running the line-113 expression against a real symlink:

ws/notes.txt -> secret/id_rsa
with readlink -f     : resolved=/.../secret/id_rsa      -> containment refuses
fallback forced      : f|13|...|/.../ws/notes.txt       -> containment passes, cat serves the target

Note the size (13) is the target's, so the probe is following the link while reporting the link's path. On a remote case on macOS 11, a prompt-injected agent can do ln -s ~/.ssh/id_rsa ./x.txt and print codeman://attach?path=<workspace>/x.txt; the force-confined magic-link scanner is satisfied and the key comes back as an attachment card. Please either resolve the last component too (a bounded loop over plain readlink "$p", which systems lacking -f do have) or fail closed: when readlink -f was unavailable and [ -L "$p" ], emit a marker that parseRemoteProbeLine turns into null so the route answers 404. A guard that silently stops resolving symlinks is worse than one that refuses, because the docs in this same commit tell the next person it is enforced.

PUT /api/sessions/:id/file-content never got the remote guard (src/web/routes/file-routes.ts:1819)

The PR description, the changeset, docs/file-viewer-edit-plan.md section 6, docs/architecture-invariants.md and the commit message all say PUT answers 400 for a remote case. Only the edit=1 half landed; the PUT handler still calls validateSessionFilePath against the local filesystem. I checked with a throwaway route test: a remote-only path answers 404, and when a directory with the same absolute path exists on the Codeman host the PUT returns 200 and overwrites the local file. That is the read-remote/write-local split your own no-local-fallback rule is written to prevent, and the same-path collision is common (/srv/case, /opt/app, a same-named home), especially on installs that set up an sshfs mount to work around #415. Three lines at the top of the handler plus a test:

if (session.remote) {
  throwFileEditError(400, ApiErrorCode.INVALID_INPUT, 'Editing is not supported for files in a remote (SSH) case');
}

The attachment history opens one ssh connection per entry (src/web/routes/file-routes.ts:2053)

GET /api/sessions/:id/attachments maps the history through Promise.all, and for a remote case each entry probes separately (detected entries at line 1037, external ones via registerExternalAttachment). ATTACHMENT_HISTORY_LIMIT is 100, and panels-ui.js:4403 re-runs the route on every attachment:detected event while the drawer is open, which is exactly when an agent is writing files. OpenSSH's default MaxStartups 10:30:100 will drop most of a burst that size. remoteProbePaths already takes an array of paths, so collecting the history's paths into one probe (or a few chunks) is the natural fix.

No bound on ssh spawns from magic links (src/attachment-registry.ts:318)

registerExternalAttachment probes before any confinement check, and the magic-link listener calls it fire-and-forget once per distinct codeman://attach?path= in a PTY chunk (src/web/session-listener-wiring.ts:450). Terminal output in a remote session is written by a process on the remote host, so an injected agent can make the server fork hundreds of ssh children, each holding a 20s probe timeout. This is the same shape as the vector src/document-conversion-limiter.ts exists to prevent, and that module is a good template: a small global semaphore around the remote calls would cap this and the history fan-out at once.

Smaller things I can take at merge time if you would rather not touch them:

  • src/web/routes/file-routes.ts:1120: an unreachable host makes external history entries missing: true, while the detected branch at line 1037 deliberately keeps missing: false for the same event. Branching on the error's status code lines the two up.
  • src/remote-files.ts:170: no VITEST guard, unlike checkRemoteTmuxAvailable and friends in remote-hosts.ts. Nothing hits it today because your route tests mock the module, but the next remote-session test that touches a file route will open a real connection from CI.
  • src/attachment-registry.ts:347: isSensitivePath's home-anchored members (~/.claude.json, ~/.claude/settings.json, ~/.claude/settings.local.json, which hold ANTHROPIC_API_KEY by schema) resolve against this host's home, so they do not match a remote path under a different remote home. Everything else in that list is depth-anchored and does apply. Worth a line in the doc if not a code change.
  • src/remote-files.ts:161: a remote filename containing a newline shifts the one-line-per-path alignment. Everything I traced fails closed, but an index prefix or a NUL separator would close it while the format is new.
  • src/remote-files.ts:205: head -c is not actually POSIX (only -n is), though every shell you will meet has it.
  • src/remote-files.ts:118: wc -c for size next to stat -c %Y || stat -f %m for mtime; stat -c %s || stat -f %z would match.
  • src/web/routes/file-routes.ts:139: the close cleanup is attached after open() has already spawned ssh, so a client that aborted during the probe leaves that child unreaped. A reply.raw.destroyed check before attaching covers it.
  • src/remote-files.ts:281: describeExecError can fall through to Node's Command failed: <full ssh line>, which then goes out in the 502 body including the identity path and the probe script.

Happy to merge once the symlink resolution and the PUT guard are in. The rest of this is exactly the shape I would have asked for: one module owning the ssh reads, the same connection args as the launch path, the guards resolved on the host that can resolve them, and documentation that says what is deliberately not supported.

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.

Remote (SSH) case: file preview / file-raw resolves against the local filesystem, not the remote host

2 participants