fix(files): read remote-case previews, downloads and attachments over ssh - #421
fix(files): read remote-case previews, downloads and attachments over ssh#421Randalix wants to merge 2 commits into
Conversation
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.
|
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 Blocker: a symlink can escape the workspace on a host without 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 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
The PR description, the changeset, 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 (
No bound on ssh spawns from magic links (
Smaller things I can take at merge time if you would rather not touch them:
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. |
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.workingDirfor a remote case is an absolute path on the remote host (Session.workingDir = RemoteCase.remotePath).validateSessionFilePath()callsrealpathSyncon both the workspace and the candidate, so for a remote-only path it fails by construction and every route answered404 File not foundbefore 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 underworkingDirtoPOST /api/sessions/:id/attachments, whoseregisterExternalAttachment()did a localrealpathSynctoo. Both halves are fixed here.Fixes #415.
What changed
New
src/remote-files.tsis the single remote-READ layer, built the same way as every other ssh line in the codebase — throughbuildSshConnectionArgs(), never a hand-built one (shellescapeis now exported fromremote-hosts.tsinstead of being copied a third time):remoteProbePaths(remote, paths)— ONE ssh round trip returningrealpath+stat(size, mtime, kind) for the requested path and the workspace root. Resolving the root remotely is what keeps the boundary honest for a symlinkedremotePath; the probe usesreadlink -fwhere available and a POSIXcd/pwd -Pfallback otherwise.remoteCreateReadStream(remote, path, range)— streams the body (cat, ortail -c +N | head -c Lfor aRange, both constant-memory), nothing buffered in server RAM. Itsclose()is wired to the response'scloseevent so an aborted download cannot leave ansshprocess behind.remoteReadFile()— bounded read forfile-content.Wired into:
file-raw,file-content,file-preview,file-thumbnail;registerExternalAttachment()(+ the by-idraw/preview/thumbnailroutes, 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:
../escape is refused before any connection is opened),CODEMAN_MAX_DOWNLOAD_BYTES) applied to the remote size, before the body is requested.A browser-supplied
?path=is always interpolated as a singleshellescape-quoted token;BatchMode=yeskeeps 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.sshfsmount 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:
edit=1/PUTnow answer400 "Editing is not supported for files in a remote (SSH) case"instead of the old misleading 404, andeditableis alwaysfalse; 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.400for remote instead of 404, and no remote file is ever copied onto the Codeman host (no temp spill).tail-file.Docker cases are untouched: their workspace is bind-mounted at the same absolute path, so local
fsreads 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/GNUstat,|in a filename, banners), and the probe script executed by a real/bin/shagainst a temp dir, including a hostile filename that wouldtoucha 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:checkgreen. The only failures are 3 pre-existinggit-clone/case-clonecases that also fail onmasterhere (a git-version artefact:FAILEDvsREF_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 whosesha256matched the remote file exactly,Range: bytes=10-19→206 bytes 10-19/73,file-contentreturned the remote text,../../../etc/passwd→ 404,edit=1→ 400; and for the outside-the-case path:POST /attachments→ 200 with the remote size, by-idraw→ 200 with the matching sha256, Range → 206, metadata → remote size, thumbnail → 400, missing file → 404,/etc/shadowon 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.aicodeman: patch.