From 46122247e4417465eb87a64fa1a9145f15cb89dd Mon Sep 17 00:00:00 2001 From: timkjr Date: Tue, 27 Oct 2026 09:25:47 -0500 Subject: [PATCH 1/2] fix: read a remote session's @fix_plan.md on its host, refuse remote Ralph writes A remote-SSH session's workingDir is a path on the remote host, but the Ralph file routes used local fs on it: fix-plan/read imported a same-named LOCAL @fix_plan.md (or 404'd), and fix-plan/write and ralph-prompt/write wrote their files into the local twin of the remote folder. Same root cause as the Ralph @fix_plan.md watcher fix. - fix-plan/read reads the file on the remote host through src/remote-files.ts (the File Viewer's ssh path): one probe of the file and the workspace, the File Viewer's confinement (a file resolving outside the workspace is treated as missing), then a capped read (1 MB). A missing remote file is the same NOT_FOUND as a missing local one; a non-file, an oversized file or an unreachable host is a failed read, never a local fallback. - fix-plan/write and ralph-prompt/write refuse a remote session (INVALID_INPUT): remote writes are out of scope by design, as for the File Viewer's remote edit. The Ralph panel already shows the error. --- src/web/routes/ralph-routes.ts | 54 ++++++- test/routes/ralph-routes-remote.test.ts | 184 ++++++++++++++++++++++++ 2 files changed, 235 insertions(+), 3 deletions(-) create mode 100644 test/routes/ralph-routes-remote.test.ts diff --git a/src/web/routes/ralph-routes.ts b/src/web/routes/ralph-routes.ts index 03456872..2a22a90b 100644 --- a/src/web/routes/ralph-routes.ts +++ b/src/web/routes/ralph-routes.ts @@ -5,7 +5,7 @@ */ import { FastifyInstance } from 'fastify'; -import { join, resolve, relative, isAbsolute } from 'node:path'; +import { join, resolve, relative, isAbsolute, sep } from 'node:path'; import { existsSync, mkdirSync, writeFileSync } from 'node:fs'; import fs from 'node:fs/promises'; import { ApiErrorCode, createErrorResponse, getErrorMessage, type ApiResponse } from '../../types.js'; @@ -29,8 +29,13 @@ import { writeHooksConfig, stripCaseEnvKeys } from '../../hooks-config.js'; import { generateClaudeMd } from '../../templates/claude-md.js'; import { buildRalphLoopPrompt } from '../../prompts/index.js'; import { getLifecycleLog } from '../../session-lifecycle-log.js'; +import { remoteProbePaths, remoteReadFile, RemoteFileAccessError } from '../../remote-files.js'; +import type { SessionRemote } from '../../types/session.js'; import type { SessionPort, EventPort, RespawnPort, ConfigPort, InfraPort } from '../ports/index.js'; +/** Cap on a remote `@fix_plan.md` read over ssh (a task list, not a document). */ +const MAX_REMOTE_FIX_PLAN_BYTES = 1024 * 1024; + export function registerRalphRoutes( app: FastifyInstance, ctx: SessionPort & EventPort & RespawnPort & ConfigPort & InfraPort @@ -191,6 +196,15 @@ export function registerRalphRoutes( app.post('/api/sessions/:id/fix-plan/write', async (req) => { const { id } = req.params as { id: string }; const session = findSessionOrFail(ctx, id, req); + // A remote session's workingDir is a path on the remote host. Writing it with local fs would land + // the file in a same-named local folder (or fail), and remote writes are out of scope by design + // (src/remote-files.ts is read-only), so it is refused, like the File Viewer's remote edit. + if (session.remote) { + return createErrorResponse( + ApiErrorCode.INVALID_INPUT, + 'Writing @fix_plan.md is not supported for a remote (SSH) session' + ); + } const workingDir = session.workingDir; if (!workingDir) { @@ -227,7 +241,14 @@ export function registerRalphRoutes( const filePath = join(workingDir, '@fix_plan.md'); try { - const content = await fs.readFile(filePath, 'utf-8'); + // A remote session's workingDir is a path on the remote host: read the file there, over the + // same ssh path the File Viewer uses, never a same-named local file. + const content = session.remote + ? await readRemoteFixPlan(session.remote, workingDir, filePath) + : await fs.readFile(filePath, 'utf-8'); + if (content === null) { + return createErrorResponse(ApiErrorCode.NOT_FOUND, '@fix_plan.md not found in working directory'); + } const importedCount = session.ralphTracker.importFixPlanMarkdown(content); ctx.persistSessionState(session); @@ -243,7 +264,8 @@ export function registerRalphRoutes( if ((error as NodeJS.ErrnoException).code === 'ENOENT') { return createErrorResponse(ApiErrorCode.NOT_FOUND, '@fix_plan.md not found in working directory'); } - return createErrorResponse(ApiErrorCode.OPERATION_FAILED, `Failed to read file: ${error}`); + const reason = error instanceof RemoteFileAccessError ? error.message : String(error); + return createErrorResponse(ApiErrorCode.OPERATION_FAILED, `Failed to read file: ${reason}`); } }); @@ -257,6 +279,13 @@ export function registerRalphRoutes( const { id } = req.params as { id: string }; const { content } = parseBody(RalphPromptWriteSchema, req.body, 'Invalid request body'); const session = findSessionOrFail(ctx, id, req); + // Same rule as fix-plan/write: never write a remote session's file on the local disk. + if (session.remote) { + return createErrorResponse( + ApiErrorCode.INVALID_INPUT, + 'Writing @ralph_prompt.md is not supported for a remote (SSH) session' + ); + } const workingDir = session.workingDir; if (!workingDir) { @@ -511,3 +540,22 @@ export function registerRalphRoutes( }; }); } + +/** + * Read a remote session's `@fix_plan.md` on its host. `null` = no such file, or one that resolves outside + * the workspace (the caller's 404, as for a local ENOENT); anything that is not a regular file, a file over the cap, or an ssh failure throws + * `RemoteFileAccessError`, which the route reports as a failed read. + */ +async function readRemoteFixPlan(remote: SessionRemote, workingDir: string, filePath: string): Promise { + // The file and the workspace in one round trip, so a symlinked @fix_plan.md that resolves outside the + // workspace is refused like any missing file (the File Viewer's confinement, file-routes.ts). + const [probe, rootProbe] = await remoteProbePaths(remote, [filePath, workingDir]); + if (!probe) return null; + const rel = relative(rootProbe?.realPath ?? resolve(workingDir), probe.realPath); + if (rel === '' || isAbsolute(rel) || rel === '..' || rel.startsWith(`..${sep}`)) return null; + if (probe.kind !== 'file') throw new RemoteFileAccessError('@fix_plan.md is not a regular file'); + if (probe.size > MAX_REMOTE_FIX_PLAN_BYTES) { + throw new RemoteFileAccessError(`@fix_plan.md is larger than ${MAX_REMOTE_FIX_PLAN_BYTES / 1024 / 1024}MB`); + } + return (await remoteReadFile(remote, probe.realPath, MAX_REMOTE_FIX_PLAN_BYTES)).toString('utf-8'); +} diff --git a/test/routes/ralph-routes-remote.test.ts b/test/routes/ralph-routes-remote.test.ts new file mode 100644 index 00000000..91ff2735 --- /dev/null +++ b/test/routes/ralph-routes-remote.test.ts @@ -0,0 +1,184 @@ +/** + * @fileoverview Ralph file routes for a REMOTE (SSH) session. + * + * A remote session's `workingDir` is a path on the remote host. `fix-plan/read` + * used to read `@fix_plan.md` with local `fs`, and `fix-plan/write` and + * `ralph-prompt/write` wrote their files with it, so with a same-named local folder + * (an sshfs mount, a shared home layout) they read and wrote the LOCAL twin. Now: + * - `fix-plan/read` reads the file on the remote host through `src/remote-files.ts` + * (the File Viewer's ssh path), and a missing remote file is the same 404 as a + * missing local one; + * - both writes are refused (remote writes are out of scope, like the File Viewer's + * remote edit). + * + * The workspace path EXISTS locally here, holding a different `@fix_plan.md`, so a + * route that fell back to local `fs` would visibly import or overwrite the wrong file. + * The ssh layer is mocked (no connection is opened); the pure builders and + * `RemoteFileAccessError` stay real. + * + * Port: N/A (app.inject doesn't open ports) + */ + +import { mkdirSync, readFileSync, writeFileSync, existsSync } from 'node:fs'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { createRouteTestHarness, type RouteTestHarness } from './_route-test-utils.js'; +import { safeRmHomeTree } from '../mocks/test-helpers.js'; +import { registerRalphRoutes } from '../../src/web/routes/ralph-routes.js'; +import type { RemoteProbe } from '../../src/remote-files.js'; +import type { SessionRemote } from '../../src/types/session.js'; + +vi.mock('../../src/remote-files.js', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, remoteProbePaths: vi.fn(), remoteReadFile: vi.fn() }; +}); + +import { remoteProbePaths, remoteReadFile, RemoteFileAccessError } from '../../src/remote-files.js'; + +const mockedProbePaths = vi.mocked(remoteProbePaths); +const mockedReadFile = vi.mocked(remoteReadFile); + +// The local twin of the remote workspace: same absolute path, different file. +const TWIN = join(homedir(), '.codeman-test-ralph-remote-twin'); +const LOCAL_PLAN = '# Fix Plan\n\n- [ ] LOCAL task\n'; +const REMOTE_PLAN = '# Fix Plan\n\n- [ ] REMOTE task\n'; +const remote: SessionRemote = { + hostId: 'host-1', + label: 'testhost', + host: '192.0.2.10', + username: 'j', + remotePath: TWIN, +}; + +function fileProbe(size = REMOTE_PLAN.length): RemoteProbe { + return { realPath: join(TWIN, '@fix_plan.md'), kind: 'file', size, mtimeMs: 1_700_000_000_000 }; +} + +const rootProbe: RemoteProbe = { realPath: TWIN, kind: 'directory', size: 0, mtimeMs: 0 }; + +describe('ralph file routes for a remote (SSH) session', () => { + let harness: RouteTestHarness; + let importFixPlanMarkdown: ReturnType; + + beforeAll(() => { + mkdirSync(TWIN, { recursive: true }); + }); + + afterAll(() => { + safeRmHomeTree(TWIN); + }); + + beforeEach(async () => { + writeFileSync(join(TWIN, '@fix_plan.md'), LOCAL_PLAN); + harness = await createRouteTestHarness(registerRalphRoutes); + importFixPlanMarkdown = vi.fn(() => 1); + const session = harness.ctx._session as unknown as Record; + session.workingDir = TWIN; + session.remote = { ...remote }; + session.ralphTracker = { + generateFixPlanMarkdown: vi.fn(() => '# Fix Plan\n\n- [ ] generated\n'), + importFixPlanMarkdown, + todos: [], + }; + mockedProbePaths.mockResolvedValue([fileProbe(), rootProbe]); + mockedReadFile.mockResolvedValue(Buffer.from(REMOTE_PLAN)); + }); + + afterEach(async () => { + await harness.app.close(); + vi.clearAllMocks(); + }); + + const post = (route: string, payload?: unknown) => + harness.app.inject({ method: 'POST', url: `/api/sessions/${harness.ctx._sessionId}/${route}`, payload }); + + describe('POST fix-plan/read', () => { + it('imports the file from the remote host, not the local twin', async () => { + const res = await post('fix-plan/read'); + expect(JSON.parse(res.body)).toMatchObject({ success: true, data: { importedCount: 1 } }); + expect(mockedProbePaths).toHaveBeenCalledWith(remote, [join(TWIN, '@fix_plan.md'), TWIN]); + expect(mockedReadFile).toHaveBeenCalledWith(remote, join(TWIN, '@fix_plan.md'), expect.any(Number)); + expect(importFixPlanMarkdown).toHaveBeenCalledWith(REMOTE_PLAN); + expect(harness.ctx.persistSessionState).toHaveBeenCalled(); + }); + + it('answers the same not-found as a local read when the remote file is missing', async () => { + mockedProbePaths.mockResolvedValue([null, rootProbe]); + const res = await post('fix-plan/read'); + expect(JSON.parse(res.body)).toMatchObject({ success: false, errorCode: 'NOT_FOUND' }); + expect(mockedReadFile).not.toHaveBeenCalled(); + expect(importFixPlanMarkdown).not.toHaveBeenCalled(); + }); + + it('treats a @fix_plan.md that resolves outside the workspace as missing', async () => { + // e.g. a symlink to ~/.ssh/id_rsa on the remote host: the File Viewer's confinement applies. + mockedProbePaths.mockResolvedValue([{ ...fileProbe(), realPath: '/home/j/.ssh/id_rsa' }, rootProbe]); + const res = await post('fix-plan/read'); + expect(JSON.parse(res.body)).toMatchObject({ success: false, errorCode: 'NOT_FOUND' }); + expect(mockedReadFile).not.toHaveBeenCalled(); + expect(importFixPlanMarkdown).not.toHaveBeenCalled(); + }); + + it('refuses a remote path that is not a regular file, or one over the cap, without reading it', async () => { + for (const probe of [ + { ...fileProbe(), kind: 'directory' as const }, + { ...fileProbe(), kind: 'other' as const }, + fileProbe(64 * 1024 * 1024), + ]) { + mockedProbePaths.mockResolvedValue([probe, rootProbe]); + const res = await post('fix-plan/read'); + expect(JSON.parse(res.body)).toMatchObject({ success: false, errorCode: 'OPERATION_FAILED' }); + } + expect(mockedReadFile).not.toHaveBeenCalled(); + expect(importFixPlanMarkdown).not.toHaveBeenCalled(); + }); + + it('reports an unreachable host as a failed read, never a local fallback', async () => { + mockedProbePaths.mockRejectedValue(new RemoteFileAccessError('remote host testhost unreachable: timeout')); + const res = await post('fix-plan/read'); + const body = JSON.parse(res.body); + expect(body).toMatchObject({ success: false, errorCode: 'OPERATION_FAILED' }); + expect(body.error).toContain('unreachable'); + expect(body.error).not.toContain('RemoteFileAccessError'); + expect(importFixPlanMarkdown).not.toHaveBeenCalled(); + }); + }); + + describe('writes', () => { + it('refuses fix-plan/write and leaves the local twin alone', async () => { + const res = await post('fix-plan/write'); + expect(JSON.parse(res.body)).toMatchObject({ success: false, errorCode: 'INVALID_INPUT' }); + expect(readFileSync(join(TWIN, '@fix_plan.md'), 'utf-8')).toBe(LOCAL_PLAN); + }); + + it('refuses ralph-prompt/write and creates nothing locally', async () => { + const res = await post('ralph-prompt/write', { content: 'do the thing' }); + expect(JSON.parse(res.body)).toMatchObject({ success: false, errorCode: 'INVALID_INPUT' }); + expect(existsSync(join(TWIN, '@ralph_prompt.md'))).toBe(false); + }); + }); + + describe('a local session is unchanged', () => { + beforeEach(() => { + delete (harness.ctx._session as unknown as Record).remote; + }); + + it('reads the local file without touching ssh', async () => { + const res = await post('fix-plan/read'); + expect(JSON.parse(res.body)).toMatchObject({ success: true }); + expect(importFixPlanMarkdown).toHaveBeenCalledWith(LOCAL_PLAN); + expect(mockedProbePaths).not.toHaveBeenCalled(); + expect(mockedReadFile).not.toHaveBeenCalled(); + }); + + it('writes the local files', async () => { + expect(JSON.parse((await post('fix-plan/write')).body)).toMatchObject({ success: true }); + expect(readFileSync(join(TWIN, '@fix_plan.md'), 'utf-8')).toContain('generated'); + expect(JSON.parse((await post('ralph-prompt/write', { content: 'do the thing' })).body)).toMatchObject({ + success: true, + }); + expect(readFileSync(join(TWIN, '@ralph_prompt.md'), 'utf-8')).toBe('do the thing'); + }); + }); +}); From 79a7f494b1bb21b04b20e336e2da183105d0bdf7 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Wed, 28 Oct 2026 09:09:12 +0100 Subject: [PATCH 2/2] fix: apply the review items for #728 at merge Export isPathWithinRoot from file-routes.ts and use it for the remote @fix_plan.md confinement, so the two checks cannot drift; rewrap the readRemoteFixPlan JSDoc and say NOT_FOUND; list fix-plan/read and the two write refusals in docs/remote-sessions.md and architecture-invariants. Co-Authored-By: Claude Opus 5.5 --- docs/architecture-invariants.md | 2 +- docs/remote-sessions.md | 2 ++ src/web/routes/file-routes.ts | 6 +++++- src/web/routes/ralph-routes.ts | 11 ++++++----- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index d4484540..40342901 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -87,7 +87,7 @@ File reads in a remote case are the second ssh surface (see Remote SSH cases bel **Remote host wake-on-LAN from user input**: an optional `RemoteHost.wakeMac` (magic packet built and broadcast by Codeman) or `RemoteHost.wakeCommand` (a single executable path, run WITHOUT a shell, and the explicit override) lets the input route — and an explicit `POST /api/sessions/:id/wake` — wake a SLEEPING host instead of writing into a stalled ssh pane; `tmux send-keys` succeeds against a stalled pane, so the bytes used to vanish silently. The wake flow lives in `src/remote-wake.ts` and is reachable **only** from an EXPLICIT user request: `POST /api/sessions/:id/input`, that explicit wake route, and the create/attach path (`POST /api/quick-start` for a remote case, `POST /api/sessions` with `attachRemoteSession`, via `ensureHostAwake`), because "the user pressed Run on a sleeping host" is the same kind of request and the tmux probe would otherwise fail with a misleading "needs tmux installed". Everything TIMER-driven must never wake a host: the COD-108 auto-reconnect watcher, `Server.handleRemoteSessionDropped` and boot recovery have no access to the registry, or a host would be re-woken seconds after each suspend and could never stay asleep (asserted by wiring guards in `test/remote-wake.test.ts`, not just documented — including that `ensureHostAwake` is called from the HTTP route only, since `cron-service.ts` builds sessions through the shared service with nobody waiting on the answer). `GET /api/sessions/:id/reachability` only ASKS — it never wakes — and feeds the amber "host unreachable" banner (`host-wake-ui.js`) whose action is either Wake or, with no target configured, "Configure WoL" → `#wakeConfigModal` (saved via `PUT /api/remote-hosts/:id`). Detection is a throttled bare TCP probe (no ssh, no `ServerAliveInterval` — keepalives would move bytes into an idle connection every interval; and a host behind a jump host/SOCKS proxy is reachability-UNKNOWN, never "asleep": `isProbeable()` keeps the registry from buffering, gating or bannering on a probe that cannot reach it), input is buffered and flushed in order after `reattachRemote()` (the send-and-wait path blocks instead, as does the create path, with a shorter request budget), and the wake fields are re-read from `remote-hosts.json` on recovery AND (throttled, cached) live for a running session, because the persisted `remote` snapshot would never see a field added later (`rehydrateRemoteHostFields` + `RemoteWakeDeps.resolveRemote`). Design + invariants: `docs/remote-sessions.md` §Wake-on-LAN from user input. -**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 ` that creates a durable REMOTE tmux session on a **dedicated socket** `-L codeman-remote` with name `codeman-ssh-` — 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 || claude --resume ` 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: ` 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 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 ` that creates a durable REMOTE tmux session on a **dedicated socket** `-L codeman-remote` with name `codeman-ssh-` — 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 || claude --resume ` 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: ` 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`. The Ralph file routes follow the same rules (`ralph-routes.ts`): `POST .../fix-plan/read` reads `@fix_plan.md` on the remote host through `remote-files.ts` (one probe of the file and the workspace, confined with the shared `isPathWithinRoot`, capped at 1 MB; missing or outside the workspace is `NOT_FOUND`, anything else `OPERATION_FAILED`, never a local fallback), and `fix-plan/write` and `ralph-prompt/write` refuse a remote session (`INVALID_INPUT`) before touching the local disk. Tests: `test/remote-files.test.ts`, `test/routes/file-routes-remote.test.ts`, `test/routes/ralph-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. 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. diff --git a/docs/remote-sessions.md b/docs/remote-sessions.md index 2d80397b..6c74ef0c 100644 --- a/docs/remote-sessions.md +++ b/docs/remote-sessions.md @@ -348,6 +348,8 @@ and it follows the same rule as the launch path: every ssh command line comes fr | `POST /api/sessions/:id/folders` | Lists a folder (absolute or `~/`) on the **remote** host in one ssh round trip (`remoteListFolder`): resolved with `cd -P`, missing → `404`, a file → `400 Not a folder` | | `GET /api/sessions/:id/folders/:folderId/files/:name/raw` | Streams one file of a granted remote folder over ssh after the same re-probe + guard as an attachment; `preview` (office) and `thumbnail` answer `400` | | `GET /api/sessions/:id/attachments/:attachmentId`, `GET …/attachments` (history) | Size/mtime/existence resolved over ssh, so a remote entry is not reported `missing`; the history list resolves EVERY entry in one batched probe, never one connection per entry | +| `POST /api/sessions/:id/fix-plan/read` | Reads `@fix_plan.md` on the **remote** host (one probe of the file and the workspace, the same confinement, a 1 MB cap); missing or resolving outside the workspace → `NOT_FOUND`, not a regular file, over the cap or an ssh failure → `OPERATION_FAILED`; never a local fallback | +| `POST /api/sessions/:id/fix-plan/write`, `POST /api/sessions/:id/ralph-prompt/write` | `400` (`INVALID_INPUT`) before any disk is touched: remote writes are out of scope, and a local write would land in a same-named local folder | ⚠️ The attachment route is the one a clicked path takes when it is **outside** the case directory (a remote `/tmp` scratchpad capture, a screenshot elsewhere in the home dir): diff --git a/src/web/routes/file-routes.ts b/src/web/routes/file-routes.ts index 395e6af2..995a15c0 100644 --- a/src/web/routes/file-routes.ts +++ b/src/web/routes/file-routes.ts @@ -767,7 +767,11 @@ const FILESYSTEM_IMAGE_PREVIEW_EXTENSIONS = new Set(['png', 'jpg', 'jpeg', 'gif' const FILESYSTEM_TEXT_PREVIEW_EXTENSIONS = new Set(['md', 'txt', 'json']); const FILESYSTEM_DOCUMENT_PREVIEW_EXTENSIONS = new Set(['pdf', 'docx', 'pptx']); -function isPathWithinRoot(root: string, candidate: string): boolean { +/** + * Whether `candidate` is `root` itself or lies beneath it (both already resolved). The one lexical + * containment check for workspace confinement; ralph-routes.ts uses it for a remote `@fix_plan.md`. + */ +export function isPathWithinRoot(root: string, candidate: string): boolean { const rel = relative(root, candidate); return rel === '' || (!isAbsolute(rel) && rel !== '..' && !rel.startsWith(`..${sep}`)); } diff --git a/src/web/routes/ralph-routes.ts b/src/web/routes/ralph-routes.ts index 2a22a90b..5aec61d5 100644 --- a/src/web/routes/ralph-routes.ts +++ b/src/web/routes/ralph-routes.ts @@ -5,7 +5,7 @@ */ import { FastifyInstance } from 'fastify'; -import { join, resolve, relative, isAbsolute, sep } from 'node:path'; +import { join, resolve, relative, isAbsolute } from 'node:path'; import { existsSync, mkdirSync, writeFileSync } from 'node:fs'; import fs from 'node:fs/promises'; import { ApiErrorCode, createErrorResponse, getErrorMessage, type ApiResponse } from '../../types.js'; @@ -31,6 +31,7 @@ import { buildRalphLoopPrompt } from '../../prompts/index.js'; import { getLifecycleLog } from '../../session-lifecycle-log.js'; import { remoteProbePaths, remoteReadFile, RemoteFileAccessError } from '../../remote-files.js'; import type { SessionRemote } from '../../types/session.js'; +import { isPathWithinRoot } from './file-routes.js'; import type { SessionPort, EventPort, RespawnPort, ConfigPort, InfraPort } from '../ports/index.js'; /** Cap on a remote `@fix_plan.md` read over ssh (a task list, not a document). */ @@ -543,16 +544,16 @@ export function registerRalphRoutes( /** * Read a remote session's `@fix_plan.md` on its host. `null` = no such file, or one that resolves outside - * the workspace (the caller's 404, as for a local ENOENT); anything that is not a regular file, a file over the cap, or an ssh failure throws - * `RemoteFileAccessError`, which the route reports as a failed read. + * the workspace (the caller's `NOT_FOUND`, as for a local ENOENT); anything that is not a regular file, a + * file over the cap, or an ssh failure throws `RemoteFileAccessError`, which the route reports as a failed + * read. */ async function readRemoteFixPlan(remote: SessionRemote, workingDir: string, filePath: string): Promise { // The file and the workspace in one round trip, so a symlinked @fix_plan.md that resolves outside the // workspace is refused like any missing file (the File Viewer's confinement, file-routes.ts). const [probe, rootProbe] = await remoteProbePaths(remote, [filePath, workingDir]); if (!probe) return null; - const rel = relative(rootProbe?.realPath ?? resolve(workingDir), probe.realPath); - if (rel === '' || isAbsolute(rel) || rel === '..' || rel.startsWith(`..${sep}`)) return null; + if (!isPathWithinRoot(rootProbe?.realPath ?? resolve(workingDir), probe.realPath)) return null; if (probe.kind !== 'file') throw new RemoteFileAccessError('@fix_plan.md is not a regular file'); if (probe.size > MAX_REMOTE_FIX_PLAN_BYTES) { throw new RemoteFileAccessError(`@fix_plan.md is larger than ${MAX_REMOTE_FIX_PLAN_BYTES / 1024 / 1024}MB`);