Merge pull request #421 from Randalix/fix/remote-file-access

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

src/attachment-registry.ts

@@ -10,10 +10,12 @@ import { randomUUID } from 'node:crypto';
 import { realpathSync } from 'node:fs';
 import fs from 'node:fs/promises';
 import { basename, extname, isAbsolute } from 'node:path';
-import { isBlockedAttachmentPath, loadAttachmentGuardConfig } from './config/attachment-guard.js';
+import { isBlockedAttachmentPath, isUnderTree, loadAttachmentGuardConfig } from './config/attachment-guard.js';
 import { EDITABLE_EXTENSIONS } from './config/file-editing.js';
 import { validateSessionFilePath } from './web/route-helpers.js';
+import { remoteProbePaths, RemoteFileAccessError } from './remote-files.js';
 import type { AttachmentDetectedEvent, AttachmentDetectedType } from './types.js';
+import type { SessionRemote } from './types/session.js';
 
 /**
  * Playable media extensions, single-sourced here because the WORKSPACE preview
@@ -215,6 +217,92 @@ export interface RegisterExternalAttachmentOptions {
    * `codeman attach` CLI (which POSTs directly when a session id is known).
    */
   forceWorkspaceConfinement?: boolean;
+  /**
+   * Remote (SSH) case: the path exists on the REMOTE host, so it is resolved and
+   * stat'ed there (`remoteProbePaths`) instead of with local `realpathSync`/`fs.stat`,
+   * which cannot see it at all (#415). A file outside the case directory is
+   * unreachable exactly like a file inside it.
+   *
+   * `sessionWorkingDir` must then be the REMOTE path too, and the workspace
+   * confinement check (when active) compares against the remotely canonicalized root,
+   * so a symlinked `remotePath` does not refuse every registration.
+   */
+  remote?: SessionRemote;
+}
+
+/**
+ * A path an attachment request resolved to, on whichever host it lives — the local
+ * filesystem or the remote host of a remote-SSH case. The rest of
+ * {@link registerExternalAttachment} (guards, extension allowlist, registry) is then
+ * host-agnostic: it only ever sees canonical absolute paths and numbers.
+ */
+interface ResolvedAttachmentFile {
+  resolvedPath: string;
+  size: number;
+  mtimeMs: number;
+  isFile: boolean;
+  extension: string;
+  /** Remote only: the workspace root, with symlinks resolved on the remote host. */
+  workspaceRoot?: string;
+}
+
+/** `extension` the way the attachment registry defines it (no dot, lowercased). */
+function attachmentExtensionOf(path: string): string {
+  return extname(path).toLowerCase().replace(/^\./, '');
+}
+
+/** Local resolution: the historical realpath + stat. */
+async function resolveLocalAttachment(requestedPath: string): Promise<ResolvedAttachmentFile> {
+  let resolvedPath: string;
+  try {
+    resolvedPath = realpathSync(requestedPath);
+  } catch {
+    throw new AttachmentRegistrationError('Attachment file not found', 404);
+  }
+  const stat = await fs.stat(resolvedPath);
+  return {
+    resolvedPath,
+    size: stat.size,
+    mtimeMs: stat.mtimeMs ?? 0,
+    isFile: typeof stat.isFile === 'function' ? stat.isFile() : true,
+    extension: attachmentExtensionOf(resolvedPath),
+  };
+}
+
+/**
+ * Remote resolution for a remote-SSH case: ONE ssh round trip returns the
+ * symlink-resolved path, the size/mtime and the kind, for the file AND (when a
+ * workspace is known) its root, which the confinement check compares against.
+ */
+async function resolveRemoteAttachment(
+  requestedPath: string,
+  remote: SessionRemote,
+  sessionWorkingDir?: string
+): Promise<ResolvedAttachmentFile> {
+  const paths = sessionWorkingDir ? [requestedPath, sessionWorkingDir] : [requestedPath];
+  let probes;
+  try {
+    probes = await remoteProbePaths(remote, paths);
+  } catch (err) {
+    throw new AttachmentRegistrationError(
+      err instanceof RemoteFileAccessError ? err.message : 'remote host unreachable',
+      502
+    );
+  }
+
+  const [probe, rootProbe] = probes;
+  if (!probe) {
+    throw new AttachmentRegistrationError('Attachment file not found', 404);
+  }
+
+  return {
+    resolvedPath: probe.realPath,
+    size: probe.size,
+    mtimeMs: probe.mtimeMs,
+    isFile: probe.kind === 'file',
+    extension: attachmentExtensionOf(probe.realPath),
+    workspaceRoot: rootProbe?.realPath,
+  };
 }
 
 export async function registerExternalAttachment(
@@ -226,12 +314,9 @@ export async function registerExternalAttachment(
     throw new AttachmentRegistrationError('Attachment path must be an absolute local path');
   }
 
-  let resolvedPath: string;
-  try {
-    resolvedPath = realpathSync(requestedPath);
-  } catch {
-    throw new AttachmentRegistrationError('Attachment file not found', 404);
-  }
+  const resolved = await (options.remote
+    ? resolveRemoteAttachment(requestedPath, options.remote, options.sessionWorkingDir)
+    : resolveLocalAttachment(requestedPath));
 
   // COD-53: enforce the active attachment-guard policy on the symlink-resolved
   // path before doing anything else.
@@ -243,7 +328,10 @@ export async function registerExternalAttachment(
     // the caller forces it for this registration (the magic-link scanner — see
     // forceWorkspaceConfinement). Strictly more restrictive than the blocklist.
     const workingDir = options.sessionWorkingDir;
-    if (!workingDir || !validateSessionFilePath(workingDir, resolvedPath)) {
+    const confined = options.remote
+      ? !!workingDir && isUnderTree(resolved.resolvedPath, resolved.workspaceRoot ?? workingDir)
+      : !!workingDir && !!validateSessionFilePath(workingDir, resolved.resolvedPath);
+    if (!confined) {
       throw new AttachmentRegistrationError('Access to this file is blocked', 403);
     }
   }
@@ -253,20 +341,25 @@ export async function registerExternalAttachment(
   // operator-configured extra trees. Symlinks are already resolved above.
   // Cross-workspace attachment of non-blocked files stays allowed, so
   // codeman-publish and the ~/.codeman review loop keep working.
-  if (isBlockedAttachmentPath(resolvedPath, guard.blockedTrees)) {
+  //
+  // The list is a pattern list over ABSOLUTE paths, so it is host-agnostic and holds
+  // for a remote path exactly as it does for a local one.
+  if (isBlockedAttachmentPath(resolved.resolvedPath, guard.blockedTrees)) {
     throw new AttachmentRegistrationError('Access to this file is blocked', 403);
   }
 
-  const extension = extname(resolvedPath).toLowerCase().replace(/^\./, '');
+  const resolvedPath = resolved.resolvedPath;
+  const extension = resolved.extension;
   if (!isSupportedAttachmentExtension(extension)) {
     throw new AttachmentRegistrationError('Unsupported attachment type');
   }
 
-  const stat = await fs.stat(resolvedPath);
-  if (typeof stat.isFile === 'function' && !stat.isFile()) {
+  if (!resolved.isFile) {
     throw new AttachmentRegistrationError('Attachment path is not a file');
   }
 
+  const stat = { size: resolved.size, mtimeMs: resolved.mtimeMs };
+
   const existing = attachmentRegistry.findByFilePath(sessionId, resolvedPath);
   if (existing) {
     existing.size = stat.size;

src/generated-artifact-attachments.ts

@@ -13,11 +13,14 @@ import { realpathSync } from 'node:fs';
 import { homedir } from 'node:os';
 import { join, normalize, sep } from 'node:path';
 import { registerExternalAttachment, type AttachmentRegistrationResult } from './attachment-registry.js';
+import type { SessionRemote } from './types/session.js';
 
 export interface GeneratedArtifactRegistrationOptions {
   sessionId: string;
   filePath: string;
   sessionWorkingDir: string;
+  /** Remote (SSH) case: the path lives on the remote host (see attachment-registry). */
+  remote?: SessionRemote;
 }
 
 export async function registerGeneratedArtifactAttachment(
@@ -26,19 +29,30 @@ export async function registerGeneratedArtifactAttachment(
   // Decide trust on the symlink-resolved path. If it can't be resolved, fall
   // back to the strict force-confined policy (registration will 404 a missing
   // file anyway).
-  let forceWorkspaceConfinement = true;
-  try {
-    const resolvedPath = realpathSync(options.filePath);
-    forceWorkspaceConfinement = !isAllowedGeneratedArtifactPath(resolvedPath, options.sessionWorkingDir);
-  } catch {
-    // Keep force confinement.
-  }
+  //
+  // A remote case keeps that strict policy unconditionally: the well-known Codex
+  // artifact directories are anchored at THIS host's home, which says nothing about
+  // a remote home, so only a file inside the remote workspace is trusted here.
+  const resolvedPath = options.remote ? undefined : tryRealpath(options.filePath);
+  const forceWorkspaceConfinement = !resolvedPath
+    ? true
+    : !isAllowedGeneratedArtifactPath(resolvedPath, options.sessionWorkingDir);
   return registerExternalAttachment(options.sessionId, options.filePath, {
     sessionWorkingDir: options.sessionWorkingDir,
     forceWorkspaceConfinement,
+    remote: options.remote,
   });
 }
 
+/** `realpathSync` without the throw — undefined when the path does not resolve. */
+function tryRealpath(path: string): string | undefined {
+  try {
+    return realpathSync(path);
+  } catch {
+    return undefined;
+  }
+}
+
 /** Well-known Codex generated-artifact directories, anchored at the user's home. */
 function codexGeneratedDirs(): string[] {
   const home = homedir();

src/remote-files.ts

@@ -0,0 +1,296 @@
+/**
+ * @fileoverview Remote (SSH) file access for remote-SSH cases.
+ *
+ * A remote case's `workingDir` is an absolute path on ANOTHER host
+ * (`Session.workingDir = RemoteCase.remotePath`, see docs/remote-sessions.md). Every
+ * file route used to read it with local `fs`, which cannot work: the local
+ * `realpathSync` in `validateSessionFilePath` fails first, so the request died as a
+ * 404 "File not found" before a byte was read (#415). This module is the ONE place
+ * that reads remote bytes, mirroring how `remote-hosts.ts` is the one place that
+ * builds an ssh command line.
+ *
+ * Connection options come from `buildSshConnectionArgs()` — never a hand-built ssh
+ * line (the COD-107 discipline in docs/remote-sessions.md) — so a proxied,
+ * custom-port or jump-hosted case reaches its files with exactly the credentials the
+ * launch used, and `BatchMode=yes` means a host that needs a passphrase fails fast
+ * instead of hanging on a prompt nothing can answer.
+ *
+ * ⚠️ The path is the injection surface: it arrives from the browser (`?path=`). It is
+ * always interpolated as a single `shellescape`d token, and the whole remote command
+ * is itself shellescaped into the ssh line, so the local shell and the remote shell
+ * each see one opaque argument. Never build a command here by concatenating a raw
+ * path into the string.
+ *
+ * Read-only by design: previews, text reads and streaming. Writing to a remote file
+ * is deliberately NOT implemented (docs/file-viewer-edit-plan.md §6), nor are the
+ * office-conversion/thumbnail paths that would need the bytes on the server's disk.
+ */
+
+import { exec, spawn } from 'node:child_process';
+import { promisify } from 'node:util';
+import type { Readable } from 'node:stream';
+import type { SessionRemote } from './types/session.js';
+import { buildSshConnectionArgs, remoteSshTarget, shellescape } from './remote-hosts.js';
+
+const execAsync = promisify(exec);
+
+/**
+ * Bound on the probe (realpath + stat) round trip. The connect itself is already
+ * bounded by `buildSshConnectionArgs`'s default `-o ConnectTimeout=10`; this covers
+ * a host that accepts the TCP connection and then never answers.
+ */
+const REMOTE_PROBE_TIMEOUT_MS = 20_000;
+
+/** Bound on a buffered remote read (`cat`), on top of the caller's own size cap. */
+const REMOTE_READ_TIMEOUT_MS = 30_000;
+
+/** Slack over the caller's byte cap so a file exactly at the limit still fits. */
+const READ_BUFFER_SLACK_BYTES = 64 * 1024;
+
+/** Marker a probe prints when the path does not exist on the remote host. */
+const NOT_FOUND_MARKER = 'n';
+
+/** What a remote path turned out to be. `other` = symlink/socket/fifo/device. */
+export type RemotePathKind = 'file' | 'directory' | 'other';
+
+export interface RemoteProbe {
+  /** The path with symlinks resolved on the REMOTE host. */
+  realPath: string;
+  kind: RemotePathKind;
+  /** Size in bytes (0 for anything that is not a regular file). */
+  size: number;
+  /** mtime in ms since epoch (0 when the remote `stat` reported none). */
+  mtimeMs: number;
+}
+
+/**
+ * A remote file access failed for a reason that is NOT "the file is missing" —
+ * unreachable host, timeout, ssh error, unexpected probe output. Callers map this to
+ * a 5xx with the remote reason in the message; a missing file is reported separately
+ * as `null`/404 so the two cannot be confused.
+ */
+export class RemoteFileAccessError extends Error {
+  constructor(message: string) {
+    super(message);
+    this.name = 'RemoteFileAccessError';
+  }
+}
+
+/**
+ * Wrap a remote shell command in the shared, shellescaped ssh line.
+ *
+ * The single entry point for "run this on the remote host": connection args (port,
+ * identity, jump host, SOCKS ProxyCommand, extra `-o`) all come from
+ * `buildSshConnectionArgs`, and the command is ONE shellescaped token, so a path with
+ * spaces, quotes or `$(…)` cannot escape into the ssh command line.
+ */
+export function buildRemoteFileCommand(remote: SessionRemote, shellCommand: string): string {
+  return [...buildSshConnectionArgs(remote), remoteSshTarget(remote), shellescape(shellCommand)].join(' ');
+}
+
+/**
+ * `realpath + stat + existence` for one or more paths, in a SINGLE ssh round trip.
+ *
+ * One call instead of three matters: without a shared connection (no ControlMaster)
+ * every extra `ssh` is a fresh handshake, and the file routes need the path AND the
+ * workspace root canonicalized to compare them.
+ *
+ * Each path emits exactly one line — `n` when it does not exist, otherwise
+ * `kind|size|mtime|realPath` with `realPath` LAST so a path containing `|` still
+ * parses (the earlier fields are fixed and the remainder is the path).
+ *
+ * Symlink resolution is portable on purpose: `readlink -f` where available (Linux,
+ * macOS >= 12.3), else the POSIX `cd`/`pwd -P` fallback, which resolves the DIRECTORY
+ * chain. Resolution is required here rather than optional: `isSensitivePath()`
+ * demands an already-realpath'd input, so a remote read must not be able to reach a
+ * blocked target through a symlink any more than a local one can.
+ */
+export function buildRemoteProbeCommand(paths: readonly string[]): string {
+  const probes = paths.map((path) => `probe ${shellescape(path)}`).join('\n');
+  return [
+    'probe() {',
+    '  p=$1',
+    '  r=$(readlink -f "$p" 2>/dev/null) || r=$(cd "$(dirname "$p")" 2>/dev/null && printf %s/%s "$(pwd -P)" "$(basename "$p")")',
+    '  [ -n "$r" ] || r=$p',
+    `  if [ ! -e "$p" ]; then printf '%s\\n' ${NOT_FOUND_MARKER}; return; fi`,
+    '  if [ -d "$r" ]; then t=d; elif [ -f "$r" ]; then t=f; else t=o; fi',
+    '  s=0',
+    '  if [ "$t" = f ]; then s=$(wc -c < "$r" 2>/dev/null | tr -d " "); [ -n "$s" ] || s=0; fi',
+    '  m=$(stat -c %Y "$r" 2>/dev/null || stat -f %m "$r" 2>/dev/null || printf 0)',
+    `  printf '%s|%s|%s|%s\\n' "$t" "$s" "$m" "$r"`,
+    '}',
+    probes,
+  ].join('\n');
+}
+
+/** Parse one probe line. `null` for the not-found marker or anything malformed. */
+export function parseRemoteProbeLine(line: string): RemoteProbe | null {
+  const trimmed = line.replace(/\r$/, '');
+  if (!trimmed || trimmed === NOT_FOUND_MARKER) return null;
+
+  const parts = trimmed.split('|');
+  if (parts.length < 4) return null;
+
+  const [kindRaw, sizeRaw, mtimeRaw] = parts;
+  const kind: RemotePathKind | null =
+    kindRaw === 'f' ? 'file' : kindRaw === 'd' ? 'directory' : kindRaw === 'o' ? 'other' : null;
+  if (!kind) return null;
+
+  const realPath = parts.slice(3).join('|');
+  if (!realPath) return null;
+
+  const size = Number.parseInt(sizeRaw, 10);
+  const mtimeSeconds = Number.parseInt(mtimeRaw, 10);
+  return {
+    realPath,
+    kind,
+    size: Number.isFinite(size) && size > 0 ? size : 0,
+    mtimeMs: Number.isFinite(mtimeSeconds) && mtimeSeconds > 0 ? mtimeSeconds * 1000 : 0,
+  };
+}
+
+/**
+ * Parse the output of {@link buildRemoteProbeCommand} into one entry per requested
+ * path, in order. Throws when the output cannot be one line per path — that means the
+ * transport or the remote shell did something unexpected, and silently treating it as
+ * "not found" would turn an infrastructure failure into a wrong 404.
+ *
+ * The LAST `paths.length` lines are used so a login banner or an eager rc-file `echo`
+ * on the remote host cannot shift the alignment.
+ */
+export function parseRemoteProbeLines(stdout: string, paths: readonly string[]): Array<RemoteProbe | null> {
+  const lines = stdout.split('\n').filter((line) => line !== '');
+  if (lines.length < paths.length) {
+    throw new RemoteFileAccessError('remote host returned no usable file information');
+  }
+  return lines.slice(-paths.length).map((line) => parseRemoteProbeLine(line));
+}
+
+/** Probe one or more remote paths. Entry is `null` for a path that does not exist. */
+export async function remoteProbePaths(
+  remote: SessionRemote,
+  paths: readonly string[]
+): Promise<Array<RemoteProbe | null>> {
+  const command = buildRemoteFileCommand(remote, buildRemoteProbeCommand(paths));
+  let stdout: string;
+  try {
+    const result = await execAsync(command, { timeout: REMOTE_PROBE_TIMEOUT_MS, maxBuffer: 64 * 1024 });
+    stdout = result.stdout;
+  } catch (err) {
+    throw new RemoteFileAccessError(
+      `remote host ${remote.label || remote.host} unreachable: ${describeExecError(err)}`
+    );
+  }
+  return parseRemoteProbeLines(stdout, paths);
+}
+
+/** Read a whole remote file into memory, capped by `maxBytes`. */
+export async function remoteReadFile(remote: SessionRemote, remotePath: string, maxBytes: number): Promise<Buffer> {
+  const command = buildRemoteFileCommand(remote, `cat ${shellescape(remotePath)}`);
+  try {
+    const result = await execAsync(command, {
+      timeout: REMOTE_READ_TIMEOUT_MS,
+      maxBuffer: maxBytes + READ_BUFFER_SLACK_BYTES,
+      encoding: 'buffer',
+    });
+    return Buffer.isBuffer(result.stdout) ? result.stdout : Buffer.from(result.stdout);
+  } catch (err) {
+    throw new RemoteFileAccessError(`failed to read remote file: ${describeExecError(err)}`);
+  }
+}
+
+/**
+ * Command that writes a remote file's bytes to stdout.
+ *
+ * ⚠️ Range reads use `tail -c +N | head -c L` (both POSIX, constant memory) because
+ * the alternative — `dd bs=1` — issues one read syscall per byte and would make video
+ * seeking unusable. The trade-off is that a `tail` failure (the file vanished
+ * mid-request) reports `head`'s exit status, i.e. a short body on an already-sent
+ * 206; the client retries. The uncompressed path (`cat`) reports its own failure
+ * correctly, so the streaming error path is still covered by the normal case.
+ */
+export function buildRemoteReadCommand(remotePath: string, range?: { start: number; end: number }): string {
+  const quoted = shellescape(remotePath);
+  if (!range) return `cat ${quoted}`;
+  const length = range.end - range.start + 1;
+  return `tail -c +${range.start + 1} ${quoted} | head -c ${length}`;
+}
+
+export interface RemoteFileStream {
+  /** The remote file's bytes, streamed from the ssh child's stdout. */
+  stream: Readable;
+  /**
+   * Abort the transfer and reap the ssh child. The caller MUST call this when the
+   * HTTP request ends — especially on a client disconnect — or the `ssh` process
+   * keeps running (and holding a connection open) after nobody is reading it.
+   */
+  close(): void;
+}
+
+/**
+ * Stream a remote file (optionally a byte range) as a Node Readable.
+ *
+ * Nothing is buffered in server memory: the bytes go from `ssh`'s stdout straight to
+ * the HTTP response, which is what makes a multi-GB remote video cost one pipe.
+ */
+export function remoteCreateReadStream(
+  remote: SessionRemote,
+  remotePath: string,
+  range?: { start: number; end: number }
+): RemoteFileStream {
+  const command = buildRemoteFileCommand(remote, buildRemoteReadCommand(remotePath, range));
+  const child = spawn(command, { shell: true, stdio: ['ignore', 'pipe', 'pipe'] });
+
+  let stderr = '';
+  child.stderr?.on('data', (chunk: Buffer) => {
+    if (stderr.length < 2000) stderr += chunk.toString();
+  });
+
+  const stream = child.stdout;
+  let ended = false;
+  stream.on('end', () => {
+    ended = true;
+  });
+  stream.on('error', () => {
+    ended = true;
+  });
+
+  child.on('error', (err: Error) => {
+    stream.destroy(err);
+  });
+  child.on('close', (code: number | null) => {
+    // Only a truncated transfer is an error. A non-zero exit AFTER the body finished
+    // (e.g. a signal delivered as the last byte was flushed) must not destroy an
+    // already-complete response, or the browser reports a broken body for a file it
+    // received in full.
+    if (ended || code === 0 || code === null) return;
+    const detail = stderr.trim().split('\n')[0];
+    stream.destroy(new RemoteFileAccessError(`remote read failed (ssh exit ${code})${detail ? `: ${detail}` : ''}`));
+  });
+
+  return {
+    stream,
+    close(): void {
+      if (!stream.destroyed) stream.destroy();
+      child.kill('SIGTERM');
+    },
+  };
+}
+
+/** First useful line of an exec/stderr error, for a user-facing message. */
+function describeExecError(err: unknown): string {
+  if (typeof err === 'object' && err !== null) {
+    const record = err as { stderr?: unknown; message?: unknown; code?: unknown; killed?: unknown };
+    const stderr =
+      typeof record.stderr === 'string' ? record.stderr : Buffer.isBuffer(record.stderr) ? String(record.stderr) : '';
+    const line = stderr
+      .split('\n')
+      .map((entry) => entry.trim())
+      .find((entry) => entry.length > 0);
+    if (line) return line;
+    if (record.killed) return 'timed out';
+    if (typeof record.message === 'string' && record.message.length > 0) return record.message;
+    if (typeof record.code === 'string' || typeof record.code === 'number') return `ssh exit ${record.code}`;
+  }
+  return 'unknown error';
+}

src/remote-hosts.ts

@@ -147,8 +147,14 @@ export function remoteSshTarget(host: Pick<RemoteHost, 'username' | 'host'>): st
  * POSIX single-quote shell-escaping (end-quote, escaped-quote, restart-quote).
  * Mirrors the helper in tmux-manager.ts so a value with spaces/metachars stays a
  * single shell token. Used here for identity paths and `-o KEY=VALUE` options.
+ *
+ * EXPORTED for `remote-files.ts` (#415, remote file access): that module wraps a
+ * remote shell command in the ssh line built by `buildSshConnectionArgs()`, so it
+ * needs the same escaping discipline for the remote command itself and for every
+ * path interpolated into it. A third private copy of this function is exactly how
+ * two escaping implementations drift apart.
  */
-function shellescape(str: string): string {
+export function shellescape(str: string): string {
   return "'" + str.replace(/'/g, "'\\''") + "'";
 }
 

src/web/route-helpers.ts

@@ -88,11 +88,43 @@ export function validateSessionFilePath(
   } catch {
     return null;
   }
-  const relativePath = relative(resolvedWorkingDir, resolvedPath);
+  return confineToRoot(resolvedWorkingDir, resolvedPath);
+}
+
+/**
+ * The lexical half of {@link validateSessionFilePath}: same containment rule, but
+ * WITHOUT touching the filesystem.
+ *
+ * Needed for remote-SSH cases (`src/remote-files.ts`), where `workingDir` is an
+ * absolute path on the REMOTE host and any local `realpathSync` fails by
+ * construction — which is how every file-raw/file-content request in a remote case
+ * used to end up as a 404 before a single byte was read. The caller follows this
+ * pre-check with a remote realpath + the same containment rule, so escapes are
+ * refused exactly as they are locally; what changes is only WHICH filesystem
+ * resolves the symlinks.
+ *
+ * A lexical check alone would follow nothing, so it must never be the last word for
+ * a path that can contain a symlink — it is the cheap reject in front of the real
+ * (local or remote) resolution, not a replacement for it.
+ */
+export function validateSessionFilePathLexical(
+  sessionWorkingDir: string,
+  filePath: string
+): { resolvedPath: string; relativePath: string } | null {
+  return confineToRoot(resolve(sessionWorkingDir), resolve(sessionWorkingDir, filePath));
+}
+
+/**
+ * Shared containment rule: `candidate` must sit inside `root` (both already
+ * canonical for their filesystem). `relative()` is the whole test — a `..` or an
+ * absolute result means the candidate escaped.
+ */
+function confineToRoot(root: string, candidate: string): { resolvedPath: string; relativePath: string } | null {
+  const relativePath = relative(root, candidate);
   if (relativePath.startsWith('..') || isAbsolute(relativePath)) {
     return null;
   }
-  return { resolvedPath, relativePath };
+  return { resolvedPath: candidate, relativePath };
 }
 
 // Maximum hook data size (prevents oversized SSE broadcasts)

src/web/server.ts

@@ -1719,10 +1719,12 @@ export class WebServer extends EventEmitter {
             sessionId,
             filePath,
             sessionWorkingDir: session.workingDir,
+            remote: session.remote,
           })
         : await registerExternalAttachment(sessionId, filePath, {
             sessionWorkingDir: session.workingDir,
             forceWorkspaceConfinement: true,
+            remote: session.remote,
           });
     const record = attachmentRegistry.get(sessionId, event.attachmentId);
     if (record) {