diff --git a/CLAUDE.md b/CLAUDE.md index 88e2e751..a698230e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -395,7 +395,7 @@ Frontend JS modules have `@fileoverview` with `@dependency`/`@loadorder` tags. L **SSE staleness watchdog** (`computeSseStale()` in constants.js, `_checkSseStale()` + a 5s interval in app.js): an `EventSource` can stop delivering without erroring, so the client forces a reconnect when nothing arrives. ⚠️ The server keepalive must stay the named `sse:heartbeat` event (`cleanupDeadClients()`, sse-stream-manager.ts), never an SSE comment, which `EventSource` cannot observe; its no-op client listener must stay registered. ⚠️ Judge staleness only while `connected` and online (the loop breaker). ⚠️ The liveness stamp lives inside `addListener`. ⚠️ Clear the interval only at the top of `connectSSE()`, or intervals stack. → [architecture-invariants#sse-staleness-watchdog](docs/architecture-invariants.md#sse-staleness-watchdog) -**Z-index layers** (keep new overlays consistent with this stack): local echo overlay (7; with local echo on it also draws the iOS IME composition preview, as an underlined tail after its pending text via `setComposition`), iOS IME composition preview span when local echo is off (6 inside `.xterm-helpers`, whose own z-index 5 is its EFFECTIVE layer, so it sits UNDER the overlay and must never be used while the overlay shows text), terminal touch-selection bar (900, below floating agent windows), subagent windows + split picker menu + Tiles count menu (1000), plan agents (1100), mobile/tablet fixed header (1200), modals on ≤768px (1300, must beat the fixed header), log viewers (2000), connection-loss overlay (2500), image popups (3000), response viewer (5000, backdrop 4999), file-preview overlay (5100, must outrank the response viewer that launches it), toasts/path picker (10000+), custom-model center-status banner (10001; its `[hidden]` must re-assert `display: none` or `dismiss()` leaves an invisible click-blocker), custom-model swap-confirm/context-warning modals (10010). → [architecture-invariants#z-index-layers](docs/architecture-invariants.md#z-index-layers) +**Z-index layers** (keep new overlays consistent with this stack): local echo overlay (7; with local echo on it also draws the iOS IME composition preview, as an underlined tail after its pending text via `setComposition`), iOS IME composition preview span when local echo is off (6 inside `.xterm-helpers`, whose own z-index 5 is its EFFECTIVE layer, so it sits UNDER the overlay and must never be used while the overlay shows text), terminal touch-selection bar (900, below floating agent windows), subagent windows + split picker menu + Tiles count menu (1000), plan agents (1100), mobile/tablet fixed header (1200), modals on ≤768px (1300, must beat the fixed header), log viewers (2000), connection-loss overlay (2500), image popups (3000), response viewer (5000, backdrop 4999), folder grid (5090, above the response viewer it opens from, below the file preview its tiles open), file-preview overlay (5100, must outrank the response viewer that launches it), toasts/path picker (10000+), custom-model center-status banner (10001; its `[hidden]` must re-assert `display: none` or `dismiss()` leaves an invisible click-blocker), custom-model swap-confirm/context-warning modals (10010). → [architecture-invariants#z-index-layers](docs/architecture-invariants.md#z-index-layers) **Respawn presets**: `solo-work` (3s/60min), `subagent-workflow` (45s/240min), `team-lead` (90s/480min), `ralph-todo` (8s/480min), `overnight-autonomous` (10s/480min). diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 0b0cb216..cfcedd4a 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -375,7 +375,7 @@ A file path an agent prints is a link on both surfaces it can appear on, and cli **Path forms beyond the rooted one.** `findFilePathLinks(text)` (constants.js) is the single entry point both consumers call; it returns the rooted matches first, then `~/` and relative paths (`RELATIVE_FILE_PATH_LINK_PATTERN`, same extension list, pinned equal by `test/link-provider-regex.test.ts`), then folder paths, each only where it overlaps neither an earlier match nor an `http(s)` URL. A relative match must start at a boundary (line start, whitespace, quote, bracket, `=`, `,`, `>`), so it can never begin inside a word, an absolute path or a URL; brackets, `=` and `,` end it (`(shot.png)`, `OUT=shots/a.png`). `resolveLinkedFilePath()` resolves a relative path against the session's `workingDir` (`.`/`..` collapsed, so `../x.png` lands outside and routes like any external path) and passes absolute and `~/` paths through. ⚠️ `~` is the home of the SESSION'S host, which only the server knows: `~/` paths always take the attachment route, where `registerExternalAttachment` expands them (`homedir()` locally, `$HOME` inside the remote probe script), and the guard runs on the expanded, realpath-resolved path as before. ⚠️ Programs hard-wrap their own output short of the terminal edge (Claude Code breaks tool output at 59-118 columns), so xterm's `isWrapped` never fires. `terminalPathLine()` (link provider only; touch selection keeps `terminalLogicalLine`) joins the next row when the previous row ends in a path token with a `/` and no extension and the join reaches an extension; a row that starts a new rooted or `~/` path is never joined, a 3+ row chain is kept only if a later row completes the path, and the broken row must be at least as long as the continuing one (keeps `cd /mnt/foo` + a longer next line apart). Tests: `test/file-path-link-forms.test.ts`. -**Folder grid: a folder is granted, then served by bare file name.** A folder path (rooted or `~/` whose last segment has no extension, relative only with a trailing `/`; `FOLDER_PATH_LINK_PATTERN` + `folderLinkPath()`) opens `openFolderGrid()` (panels-ui.js), as does the preview's "Show folder" button. `POST /api/sessions/:id/folders {path}` (`src/folder-listing.ts`) resolves the folder (locally `realpath`; remotely one ssh round trip via `remoteListFolder`, through `runWithRemoteSshLimit`), runs the attachment guard on it (blocklist, `confineToWorkspace`), lists non-hidden subfolders and files of a supported attachment type (each file also blocklist-checked, newest first, capped at `MAX_FOLDER_ENTRIES`), and GRANTS it an id. Files are then fetched as `GET …/folders/:folderId/files/:name/(raw|preview|thumbnail)`: `:name` must be a bare file name of a supported type (no `/`, `\`, `.`, `..`, NUL), and the joined path goes through the SAME `resolveServableAttachmentPath` as an attachment by id (symlink re-resolution, remote re-probe, guard) before `serveServableRaw` (the attachment raw route's body, shared) streams it. So browser requests still never carry a path. ⚠️ Grants live in their own registry (`folderGrants`, 32 per session, cleared with the session's attachments), NOT the attachment registry: that one is capped at 200 per session, and paging a folder of 300 renders would evict the session's real attachments. ⚠️ The remote listing script tells "missing" (`E|n`, 404) from "is a file" (`E|f`, 400 `Not a folder`), because the client falls back to the file preview on `Not a folder`; a remote extensionless file otherwise answered "Folder not found". ⚠️ `.folder-grid-overlay` sits at `z-index: 5090`, one step BELOW the preview (5100) and above the response viewer (5000): a tile's preview opens on top and closing it lands back on the grid. ⚠️ ←/→/Esc are handled in a document CAPTURE listener with `stopPropagation`, because the terminal keeps keyboard focus under the overlays and an arrow that reached xterm would be typed into the session; inputs other than xterm's helper textarea keep their keys. Tests: `test/routes/folder-grid-routes.test.ts` (incl. the listing script on a real `sh`), `test/folder-grid-ui.test.ts`. +**Folder grid: a folder is granted, then served by bare file name.** A folder path (rooted or `~/` whose last segment has no extension, relative only with a trailing `/`; `FOLDER_PATH_LINK_PATTERN` + `folderLinkPath()`) opens `openFolderGrid()` (panels-ui.js), as does the preview's "Show folder" button. `POST /api/sessions/:id/folders {path}` (`src/folder-listing.ts`) resolves the folder (locally a bounded `probePathKind(…, { pastCap: true })` first, then async `fs.realpath`; remotely one ssh round trip via `remoteListFolder`, through `runWithRemoteSshLimit`), runs the attachment guard on it (blocklist, `confineToWorkspace`, and in multi-user mode `folderAllowedForUser`: a non-admin only inside their own `userSpacePath` or the session's `workingDir`, as the filesystem path picker), locally BEFORE reading it, lists non-hidden subfolders and files of a supported attachment type (each file also blocklist-checked, newest first, capped at `MAX_FOLDER_ENTRIES`), and GRANTS it an id. Files are then fetched as `GET …/folders/:folderId/files/:name/(raw|preview|thumbnail)`: `:name` must be a bare file name of a supported type (no `/`, `\`, NUL, no leading `.`, since the listing hides those), the local file is probed bounded (`unknown` → 503, never 404), and the joined path goes through the SAME `resolveServableAttachmentPath` as an attachment by id (symlink re-resolution, remote re-probe, guard), then `folderAllowedForUser` once more on the resolved file, before `serveServableRaw` (the attachment raw route's body, shared) streams it. So browser requests still never carry a path. ⚠️ Grants live in their own registry (`folderGrants`, 32 per session, cleared with the session's attachments), NOT the attachment registry: that one is capped at 200 per session, and paging a folder of 300 renders would evict the session's real attachments. ⚠️ A local folder is never touched unbounded (#516 rule): a folder on a network mount that stopped answering answers 503 with `describeUnknownPath()`, not 404 and not a frozen server. ⚠️ The remote listing script tells "missing" (`E|n`, 404) from "is a file" (`E|f`, 400 `Not a folder`), because the client falls back to the file preview on `Not a folder`; a remote extensionless file otherwise answered "Folder not found". ⚠️ `.folder-grid-overlay` sits at `z-index: 5090`, one step BELOW the preview (5100) and above the response viewer (5000): a tile's preview opens on top and closing it lands back on the grid. ⚠️ ←/→/Esc are handled in a document CAPTURE listener with `stopPropagation`, because the terminal keeps keyboard focus under the overlays and an arrow that reached xterm would be typed into the session; inputs other than xterm's helper textarea keep their keys. Tests: `test/routes/folder-grid-routes.test.ts` (incl. the listing script on a real `sh`), `test/folder-grid-ui.test.ts`. **Text reuses the edit-mode allowlist; markup stays download-only.** `TEXT_ATTACHMENT_EXTENSIONS` IS `EDITABLE_EXTENSIONS` (`config/file-editing.ts`) rather than a second curated list that would drift from it: if the viewer would open a file for editing inside the workspace, the same file outside it can be read. The justification for widening is that the agent in the session can already `cat` any of these and the picker already previews them, so the suffix was never the confidentiality gate; the path guard is (sensitive-file blocklist, `/root` and `/etc` trees, realpath first). ⚠️ Two consequences had to be handled at the same time: `~/.codeman*/state.json` joined `isSensitivePath` (it persists `SessionState.envOverrides`, and the env allowlist admits key-shaped names like `GEMINI_API_KEY`, so it can hold a live credential), and `html`/`htm` joined `svg` in `serveRawFile`'s **download-only** branch so that widening what can be READ never widens what can RUN on our own origin. Text with no dedicated MIME entry goes out as inert `text/plain; charset=utf-8` + `nosniff`, matching the picker. The by-id text preview is bounded like the workspace one: a `Range` request for the first 512KB (a real partial read, not a discarded 50MB download) plus a 500-line cap, with the footer saying so. @@ -1063,7 +1063,7 @@ Tests: `test/mobile-prompt-composer.test.ts` (in the CI gate, deliberately not u ### Z-index layers -**Z-index layers**: subagent windows (1000), split picker menu (1000, `.split-picker-menu`), Tiles count menu (1000, `.tile-count-menu`), the closing grid's still copy (`.tile-grid-ghosts`, z-index 3 inside `.main`, over the single view coming back; hidden under `.main.webview-active`), plan agents (1100), mobile/tablet fixed header (1200, `mobile.css`), modals on ≤768px (1300 — must beat the fixed header or the modal close button is buried), log viewers (2000), connection-loss overlay (2500, above the fixed header and modals), image popups (3000), response viewer (5000, backdrop 4999), file-preview overlay (5100 — must outrank the response viewer, which can launch it; at its old 2000 a path clicked in the chat opened BEHIND the chat), toasts/path picker (10000+, deliberately above the preview), the custom-model center-status banner (10001, `.center-status-banner` — `[hidden]` must re-assert `display: none` over its own `display: flex`, same trap as `.home-sessions[hidden]`, or `dismiss()` leaves an invisible click-blocker dead centre on screen), the swap-confirm and context-warning modals (10010, `#customModelSwapConfirmModal`/`#customModelContextWarningModal` — must clear both the plain `.modal` z-index of 1000 and the center-status banner it can appear over), terminal touch-selection bar (900 — above terminal content and the local-echo overlay, deliberately BELOW floating agent windows so it can never cover their controls), local echo overlay (7), iOS IME composition preview (EFFECTIVE layer depends on its home: with local echo on it is part of the local echo overlay at 7, drawn by `setComposition()` as an underlined tail after the pending text; with local echo off it is a span at z-index 6 inside `.xterm-helpers`, but `.xterm-helpers` is its own z-index 5 stacking context, so the span's effective layer is 5. That is below the overlay's 7, which is why the span cannot be used while the overlay holds text: typed text never reaches the PTY before Enter, the PTY cursor that places the span stays at the prompt start, and the overlay's opaque line div covers it). +**Z-index layers**: subagent windows (1000), split picker menu (1000, `.split-picker-menu`), Tiles count menu (1000, `.tile-count-menu`), the closing grid's still copy (`.tile-grid-ghosts`, z-index 3 inside `.main`, over the single view coming back; hidden under `.main.webview-active`), plan agents (1100), mobile/tablet fixed header (1200, `mobile.css`), modals on ≤768px (1300 — must beat the fixed header or the modal close button is buried), log viewers (2000), connection-loss overlay (2500, above the fixed header and modals), image popups (3000), response viewer (5000, backdrop 4999), folder grid (5090, `#folderGridOverlay` — above the response viewer a folder link opens it from, below the file preview its tiles open, so a tile's preview and its ←/→ paging sit on top of the grid), file-preview overlay (5100 — must outrank the response viewer, which can launch it; at its old 2000 a path clicked in the chat opened BEHIND the chat), toasts/path picker (10000+, deliberately above the preview), the custom-model center-status banner (10001, `.center-status-banner` — `[hidden]` must re-assert `display: none` over its own `display: flex`, same trap as `.home-sessions[hidden]`, or `dismiss()` leaves an invisible click-blocker dead centre on screen), the swap-confirm and context-warning modals (10010, `#customModelSwapConfirmModal`/`#customModelContextWarningModal` — must clear both the plain `.modal` z-index of 1000 and the center-status banner it can appear over), terminal touch-selection bar (900 — above terminal content and the local-echo overlay, deliberately BELOW floating agent windows so it can never cover their controls), local echo overlay (7), iOS IME composition preview (EFFECTIVE layer depends on its home: with local echo on it is part of the local echo overlay at 7, drawn by `setComposition()` as an underlined tail after the pending text; with local echo off it is a span at z-index 6 inside `.xterm-helpers`, but `.xterm-helpers` is its own z-index 5 stacking context, so the span's effective layer is 5. That is below the overlay's 7, which is why the span cannot be used while the overlay holds text: typed text never reaches the PTY before Enter, the PTY cursor that places the span stays at the prompt start, and the overlay's opaque line div covers it). ## Security layers diff --git a/docs/security-architecture.md b/docs/security-architecture.md index 8a9f39ed..d0992447 100644 --- a/docs/security-architecture.md +++ b/docs/security-architecture.md @@ -382,6 +382,11 @@ left out). The grant is an id for a resolved folder; files are served by `folderId` + a bare file name only (no `/`, `..` or NUL, supported type), and each request re‑runs `resolveServableAttachmentPath` on the joined path, so a file that was swapped for a symlink to a blocked target after listing is still refused. +In multi‑user mode a regular user is held to the filesystem path picker's rule +(`folderAllowedForUser`): the folder, and again each served file after symlink +resolution, must sit inside their own `userSpacePath` or the session's +`workingDir`, else 403. User spaces live inside `homedir()`, so without it one user +could list and read another's space. ### SSE log‑tail route — intentional extra read roots diff --git a/src/folder-listing.ts b/src/folder-listing.ts index a9cd0b20..36d10d12 100644 --- a/src/folder-listing.ts +++ b/src/folder-listing.ts @@ -18,7 +18,6 @@ */ import { randomUUID } from 'node:crypto'; -import { realpathSync } from 'node:fs'; import fs from 'node:fs/promises'; import { homedir } from 'node:os'; import { extname, isAbsolute, join, posix } from 'node:path'; @@ -28,9 +27,11 @@ import { supportedAttachmentExtensions, } from './attachment-registry.js'; import { isBlockedAttachmentPath, isUnderTree, loadAttachmentGuardConfig } from './config/attachment-guard.js'; +import { isMultiUserMode, userSpacePath } from './config/multiuser.js'; import { remoteListFolder, RemoteFileAccessError, type RemoteFolderEntry } from './remote-files.js'; -import type { AttachmentDetectedType } from './types.js'; +import type { AttachmentDetectedType, AuthUser } from './types.js'; import type { SessionRemote } from './types/session.js'; +import { describeUnknownPath, probePathKind } from './utils/bounded-path-probe.js'; import { validateSessionFilePath } from './web/route-helpers.js'; /** Most entries (files + subfolders) one listing returns. */ @@ -111,11 +112,12 @@ export const folderGrants = new FolderGrants(); /** * A file name inside a granted folder, or null when it is not one: no path - * separator, not `.`/`..`, no NUL, and a type the preview supports. + * separator, not hidden (which also rules out `.`/`..`; the listing leaves hidden + * entries out, so the routes do too), no NUL, and a type the preview supports. */ export function validFolderFileName(name: unknown): string | null { if (typeof name !== 'string' || !name || name.length > 255) return null; - if (name === '.' || name === '..' || name.includes('/') || name.includes('\\') || name.includes('\0')) return null; + if (name.startsWith('.') || name.includes('/') || name.includes('\\') || name.includes('\0')) return null; const extension = extname(name).toLowerCase().replace(/^\./, ''); return isSupportedAttachmentExtension(extension) ? name : null; } @@ -128,13 +130,43 @@ export function folderFilePath(dir: string, name: string): string { interface ListFolderOptions { sessionWorkingDir?: string; remote?: SessionRemote; + /** The requesting identity; in multi-user mode a regular user is held to {@link folderAllowedForUser}. */ + user?: AuthUser; +} + +/** + * Multi-user rule for folder grants, the same one the filesystem path picker follows: + * a regular user reaches only their own space (`userSpacePath`) or the session's + * `workingDir`. User spaces live inside `homedir()`, so without it a user could list + * and read another user's space. Always true in single-user mode and for admins; a + * remote session's folders are on its host, where only the `workingDir` applies. + * + * @param path Already resolved (realpath on the session's host). + */ +export function folderAllowedForUser( + path: string, + user: AuthUser | undefined, + scope: { workingDir?: string; remote?: SessionRemote } +): boolean { + if (!isMultiUserMode()) return true; + if (!user) return false; + if (user.role === 'admin') return true; + if (scope.remote) return !!scope.workingDir && isUnderTree(path, scope.workingDir); + return [userSpacePath(user.username), scope.workingDir].some( + (root) => !!root && !!validateSessionFilePath(root, path) + ); } /** * Resolve, guard, list and grant a folder for `sessionId`. * + * A local folder is probed bounded first (`probePathKind`): one on a network mount + * that stopped answering must not freeze the server, and "did not answer" is never + * reported as "not found". The guard runs on the resolved folder before it is read. + * * @param requestedPath Absolute, or `~/…` (the home of the session's host). - * @throws FolderListingError 400 not absolute / not a folder, 403 blocked, 404 missing, 502 remote unreachable. + * @throws FolderListingError 400 not absolute / not a folder, 403 blocked or outside the + * user's space, 404 missing, 502 remote unreachable, 503 local folder not answering. */ export async function listFolder( sessionId: string, @@ -146,6 +178,22 @@ export async function listFolder( throw new FolderListingError('Folder path must be absolute'); } + const guard = await loadAttachmentGuardConfig(); + const workingDir = options.sessionWorkingDir; + const assertAllowed = (dir: string): void => { + const confined = !guard.confineToWorkspace + ? true + : options.remote + ? !!workingDir && isUnderTree(dir, workingDir) + : !!workingDir && !!validateSessionFilePath(workingDir, dir); + if (!confined || isBlockedAttachmentPath(dir, guard.blockedTrees)) { + throw new FolderListingError('Access to this folder is blocked', 403); + } + if (!folderAllowedForUser(dir, options.user, { workingDir, remote: options.remote })) { + throw new FolderListingError('Folder is outside your space', 403); + } + }; + let dir: string; let entries: RemoteFolderEntry[]; let truncated = false; @@ -160,29 +208,26 @@ export async function listFolder( if (!listing) throw new FolderListingError('Folder not found', 404); if (listing === 'not-a-folder') throw new FolderListingError('Not a folder'); ({ dir, entries, truncated } = listing); + // One ssh call lists and resolves together, so the guard can only run after it; + // nothing of a refused listing leaves this function. + assertAllowed(dir); } else { const expanded = homeRelative ? join(homedir(), requestedPath.slice(2)) : requestedPath; + const kind = await probePathKind(expanded, { pastCap: true }); + if (kind === 'unknown') { + throw new FolderListingError(describeUnknownPath('Folder', expanded, { pastCap: true }), 503); + } + if (kind === 'absent') throw new FolderListingError('Folder not found', 404); + if (kind !== 'directory') throw new FolderListingError('Not a folder'); try { - dir = realpathSync(expanded); + dir = await fs.realpath(expanded); } catch { throw new FolderListingError('Folder not found', 404); } - const stat = await fs.stat(dir); - if (!stat.isDirectory()) throw new FolderListingError('Not a folder'); + assertAllowed(dir); ({ entries, truncated } = await listLocalFolder(dir)); } - const guard = await loadAttachmentGuardConfig(); - const workingDir = options.sessionWorkingDir; - const confined = !guard.confineToWorkspace - ? true - : options.remote - ? !!workingDir && isUnderTree(dir, workingDir) - : !!workingDir && !!validateSessionFilePath(workingDir, dir); - if (!confined || isBlockedAttachmentPath(dir, guard.blockedTrees)) { - throw new FolderListingError('Access to this folder is blocked', 403); - } - const files: FolderFileEntry[] = []; const folders: string[] = []; for (const entry of entries) { diff --git a/src/remote-files.ts b/src/remote-files.ts index ba5e20e6..1188e7d2 100644 --- a/src/remote-files.ts +++ b/src/remote-files.ts @@ -293,7 +293,7 @@ export interface RemoteFolderListing { /** * List one remote folder in a SINGLE ssh round trip: its canonical path, then every * non-hidden subfolder and every regular file (symlinks followed) whose name ends in - * one of `extensions` (either case), up to `maxEntries`. + * one of `extensions` (any case), up to `maxEntries`. * * Same output discipline as {@link buildRemoteProbeCommand}: a lone NUL first (so rc * chatter before it is discarded), then NUL-terminated records — `E|n` (no such @@ -303,9 +303,9 @@ export interface RemoteFolderListing { * hold. A leading `~/` is the remote user's `$HOME`, as in the probe. */ export function buildRemoteFolderListCommand(dir: string, extensions: readonly string[], maxEntries: number): string { - const patterns = extensions - .flatMap((ext) => [ext.toLowerCase(), ext.toUpperCase()]) - .map((ext) => `*.${ext}`) + // `[pP][nN][gG]`: any case, as the local listing compares lowercased (`shot.Png`). + const patterns = [...new Set(extensions.map((ext) => ext.toLowerCase()))] + .map((ext) => `*.${ext.replace(/[a-z]/g, (c) => `[${c}${c.toUpperCase()}]`)}`) .join('|'); return [ `d=${shellescape(dir)}`, diff --git a/src/web/public/constants.js b/src/web/public/constants.js index 3a8f6750..837dd75b 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -2054,13 +2054,16 @@ function findFilePathLinks(text) { * trailing slash (`builds/captures/`). Same boundary in front as the relative * file pattern (group 1), the candidate itself is group 2 and goes through * {@link folderLinkPath}, which decides whether it reads as a folder at all. - * File links are found first and win any overlap. + * File links are found first and win any overlap. A folder path ends at `:`, + * which is also a boundary in front: `PATH=/a/bin:/b/bin` is two candidates, and + * the `://` lookahead stops at the next colon instead of rescanning the token from + * every one (quadratic on `a:a:a:…//`). * * ⚠ Like its siblings, never share an instance (`lastIndex`): use * {@link findFilePathLinks}. */ const FOLDER_PATH_LINK_PATTERN = - /(^|[\s"'`(\[{<>=,:])((?:\/(?:home|Users|tmp|var|private|opt|mnt|srv|media|data|workspace)\/|~\/|(?![^\s"'`<>|;&()[\]{}=,\n\x00-\x1f]*:\/\/)[^\s"'`<>|;&()[\]{}=,\/~\n\x00-\x1f])[^\s"'`<>|;&()[\]{}=,\n\x00-\x1f]*)/g; + /(^|[\s"'`(\[{<>=,:])((?:\/(?:home|Users|tmp|var|private|opt|mnt|srv|media|data|workspace)\/|~\/|(?![^\s"'`<>|;&()[\]{}=,:\n\x00-\x1f]*:\/\/)[^\s"'`<>|;&()[\]{}=,:\/~\n\x00-\x1f])[^\s"'`<>|;&()[\]{}=,:\n\x00-\x1f]*)/g; /** * A {@link FOLDER_PATH_LINK_PATTERN} candidate as a folder link, or null. diff --git a/src/web/public/panels-ui.js b/src/web/public/panels-ui.js index 5033affd..f2f8c709 100644 --- a/src/web/public/panels-ui.js +++ b/src/web/public/panels-ui.js @@ -4107,6 +4107,12 @@ Object.assign(CodemanApp.prototype, { if (!overlay || !bodyEl) return; + // Only the newest load may write: paging ←/→ through a folder starts the next + // load while the previous one still awaits its fetch, and its late answer would + // put the previous file's text under the next file's title. + const load = (this._filePreviewLoad = (this._filePreviewLoad || 0) + 1); + const stale = () => load !== this._filePreviewLoad; + // Edit mode: reset any prior editor state whenever a preview (re)loads. this._resetFilePreviewEdit(); // Stop whatever the previous preview was playing. Overwriting innerHTML @@ -4138,6 +4144,7 @@ Object.assign(CodemanApp.prototype, { let externalSize = 0; if (!attachmentId && !folderFile && this._isExternalPreviewPath(filePath, sessionId)) { const external = await this._registerExternalPreview(filePath, sessionId); + if (stale()) return; attachmentId = external.attachmentId || null; externalError = external.error || ''; externalSize = external.size || 0; @@ -4211,6 +4218,7 @@ Object.assign(CodemanApp.prototype, { const res = await fetch(`${base}/raw`, { headers: { Range: `bytes=0-${TEXT_PREVIEW_MAX_BYTES - 1}` } }); if (!res.ok) throw new Error('Failed to load attachment'); const text = await res.text(); + if (stale()) return; const clippedByBytes = res.status === 206 && text.length >= TEXT_PREVIEW_MAX_BYTES; const lines = text.split('\n'); // Markdown keeps every line the Range read returned: the byte bound is @@ -4229,6 +4237,7 @@ Object.assign(CodemanApp.prototype, { footerEl.textContent = `${footerEl.textContent} (${note})`; } } catch (err) { + if (stale()) return; bodyEl.innerHTML = `
Error: ${escapeHtml(err.message)}
`; } } @@ -4261,11 +4270,14 @@ Object.assign(CodemanApp.prototype, { try { const res = await fetch(`/api/sessions/${sessionId}/file-raw?path=${encodeURIComponent(filePath)}`); if (!res.ok) throw new Error('Failed to load image'); - const blobUrl = URL.createObjectURL(new Blob([await res.text()], { type: 'image/svg+xml' })); + const svgText = await res.text(); + if (stale()) return; + const blobUrl = URL.createObjectURL(new Blob([svgText], { type: 'image/svg+xml' })); bodyEl.innerHTML = `${escapeHtml(filePath)}`; const img = bodyEl.querySelector('img'); if (img) img.onload = () => URL.revokeObjectURL(blobUrl); } catch (err) { + if (stale()) return; bodyEl.innerHTML = `
Error: ${escapeHtml(err.message)}
`; } return; @@ -4281,6 +4293,7 @@ Object.assign(CodemanApp.prototype, { if (!res.ok) throw new Error('Failed to load file'); const result = await res.json(); + if (stale()) return; if (!result.success) throw new Error(result.error || 'Failed to load file'); const data = result.data; @@ -4320,6 +4333,7 @@ Object.assign(CodemanApp.prototype, { } } } catch (err) { + if (stale()) return; console.error('Failed to preview file:', err); bodyEl.innerHTML = `
Error: ${escapeHtml(err.message)}
`; } @@ -4341,6 +4355,8 @@ Object.assign(CodemanApp.prototype, { const body = this.$('folderGridBody'); if (!overlay || !body) return; this._bindFolderGridKeys(); + // Two quick opens (a chip, then its parent): only the newer one may render. + const load = (this._folderGridLoad = (this._folderGridLoad || 0) + 1); let result = null; let status = 0; try { @@ -4352,9 +4368,11 @@ Object.assign(CodemanApp.prototype, { status = res.status; result = await res.json().catch(() => null); } catch (err) { + if (load !== this._folderGridLoad) return; this.showToast(`Cannot open folder: ${err.message || err}`, 'error'); return; } + if (load !== this._folderGridLoad) return; if (!result?.success || !result.data) { const reason = result?.error || `HTTP ${status}`; if (status === 400 && /not a folder/i.test(reason)) { @@ -4371,6 +4389,8 @@ Object.assign(CodemanApp.prototype, { }, closeFolderGrid() { + // A listing still on its way must not reopen the grid that was just closed. + this._folderGridLoad = (this._folderGridLoad || 0) + 1; this.$('folderGridOverlay')?.classList.remove('visible'); const body = this.$('folderGridBody'); // Drop the tiles: lazy s still loading would keep fetching otherwise. diff --git a/src/web/routes/file-routes.ts b/src/web/routes/file-routes.ts index 153ad376..576775c2 100644 --- a/src/web/routes/file-routes.ts +++ b/src/web/routes/file-routes.ts @@ -23,6 +23,7 @@ import type { } from '../../types.js'; import { ApiErrorCode, createErrorResponse, getErrorMessage } from '../../types.js'; import { compileFileQuery } from '../../utils/file-query.js'; +import { describeUnknownPath, probePathKind } from '../../utils/bounded-path-probe.js'; import { fileStreamManager } from '../../file-stream-manager.js'; import { AUDIO_ATTACHMENT_EXTENSIONS, @@ -41,6 +42,7 @@ import { FolderListingError, folderFilePath, folderGrants, + folderAllowedForUser, listFolder, validFolderFileName, } from '../../folder-listing.js'; @@ -2387,14 +2389,18 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even return; } try { - const listing = await listFolder(id, body.path, { sessionWorkingDir: scope.workingDir, remote: scope.remote }); + const listing = await listFolder(id, body.path, { + sessionWorkingDir: scope.workingDir, + remote: scope.remote, + user: getAuthUser(req), + }); return { success: true, data: listing }; } catch (err) { if (err instanceof FolderListingError) { const code = err.statusCode === 404 ? ApiErrorCode.NOT_FOUND - : err.statusCode === 502 + : err.statusCode >= 500 ? ApiErrorCode.OPERATION_FAILED : ApiErrorCode.INVALID_INPUT; reply.code(err.statusCode).send(createErrorResponse(code, err.message)); @@ -2410,7 +2416,9 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even * One file of a granted folder, guard-checked like an attachment: the name is a * bare file name of a supported type, and the joined path goes through the same * {@link resolveServableAttachmentPath} (blocklist, workspace confinement, symlink - * re-resolution, remote re-probe) an attachment by id does. + * re-resolution, remote re-probe) an attachment by id does. The multi-user rule + * ({@link folderAllowedForUser}) runs again on the resolved file: a symlink inside + * the granted folder must not lead into another user's space. */ async function resolveFolderFile( req: FastifyRequest, @@ -2431,9 +2439,17 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even } const filePath = folderFilePath(grant.dir, fileName); if (!scope.remote) { - try { - if (!(await fs.stat(filePath)).isFile()) throw new Error('not a file'); - } catch { + // Bounded, like the folder itself: the grant may point at a mount that has stopped answering since. + const kind = await probePathKind(filePath, { pastCap: true }); + if (kind === 'unknown') { + reply + .code(503) + .send( + createErrorResponse(ApiErrorCode.OPERATION_FAILED, describeUnknownPath('File', filePath, { pastCap: true })) + ); + return null; + } + if (kind !== 'file') { reply.code(404).send(createErrorResponse(ApiErrorCode.NOT_FOUND, 'File not found')); return null; } @@ -2453,6 +2469,10 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even }; const servable = await resolveServableAttachmentPath(reply, record, scope); if (!servable) return null; + if (!folderAllowedForUser(servable.path, getAuthUser(req), scope)) { + reply.code(403).send(createErrorResponse(ApiErrorCode.INVALID_INPUT, 'File is outside your space')); + return null; + } return { scope, servable, name: fileName, extension }; } diff --git a/test/folder-grid-ui.test.ts b/test/folder-grid-ui.test.ts index 95803303..4479955a 100644 --- a/test/folder-grid-ui.test.ts +++ b/test/folder-grid-ui.test.ts @@ -55,6 +55,14 @@ describe('folder links (findFilePathLinks)', () => { expect(folders('/etc/nginx is not a link root')).toEqual([]); }); + it('ends a folder path at a colon (PATH lists), and stays linear on colon runs', () => { + expect(folders('export PATH=/home/u/bin:/usr/bin')).toEqual(['/home/u/bin']); + expect(folders('dirs /mnt/a/frames:/mnt/b/frames')).toEqual(['/mnt/a/frames', '/mnt/b/frames']); + const start = Date.now(); + findFilePathLinks('a:'.repeat(16000) + '//'); + expect(Date.now() - start).toBeLessThan(100); + }); + it('keeps the file link where both could match, and orders by position', () => { const found = findFilePathLinks('dir /mnt/a/frames then /mnt/a/frames/x.png'); expect(found.map((f) => [f.path, !!f.folder])).toEqual([ @@ -324,6 +332,65 @@ describe('folder preview paging', () => { }); }); +describe('stale loads', () => { + const TEXT_LISTING = { + ...LISTING, + path: '/mnt/raid1/logs', + files: [ + { name: 'a.log', extension: 'log', attachmentType: 'text', size: 1, mtimeMs: 2 }, + { name: 'b.log', extension: 'log', attachmentType: 'text', size: 1, mtimeMs: 1 }, + ], + }; + const text = (body: string) => ({ ok: true, status: 200, text: async () => body }); + const flush = () => new Promise((r) => setTimeout(r, 0)); + + it("paging past a text file that is still loading never shows its text under the next file's title", async () => { + const pending: Record void> = {}; + const { app, elements } = loadApp((url) => + url.endsWith('/folders') ? ok(TEXT_LISTING) : new Promise((r) => (pending[url] = r)) + ); + await app.openFolderGrid('/mnt/raid1/logs', 's1'); + app._renderFilePreviewText = vi.fn(); + elements.folderGridBody.children[0].onclick?.(); + elements.filePreviewOverlay.classList.add('visible'); + app.stepFolderPreview(1); + await flush(); + + pending['/api/sessions/s1/folders/fld_1/files/b.log/raw'](text('B')); + await flush(); + pending['/api/sessions/s1/folders/fld_1/files/a.log/raw'](text('A')); + await flush(); + + expect(elements.filePreviewTitle.textContent).toBe('/mnt/raid1/logs/b.log'); + expect(app.filePreviewContent).toBe('B'); + expect(app.filePreviewText.filePath).toBe('/mnt/raid1/logs/b.log'); + expect(app._renderFilePreviewText).toHaveBeenCalledTimes(1); + }); + + it('two quick folder opens render only the newer one, and a closed grid stays closed', async () => { + const pending: Record void> = {}; + const { app, elements } = loadApp((_url, init) => { + const path = JSON.parse(init?.body || '{}').path; + return new Promise((r) => (pending[path] = r)); + }); + const first = app.openFolderGrid('/mnt/raid1/styleframes', 's1'); + const second = app.openFolderGrid('/mnt/raid1/logs', 's1'); + pending['/mnt/raid1/logs'](ok(TEXT_LISTING)); + await second; + pending['/mnt/raid1/styleframes'](ok(LISTING)); + await first; + expect(app.folderGrid.path).toBe('/mnt/raid1/logs'); + expect(elements.folderGridTitle.textContent).toBe('/mnt/raid1/logs'); + + const late = app.openFolderGrid('/mnt/raid1/styleframes', 's1'); + app.closeFolderGrid(); + pending['/mnt/raid1/styleframes'](ok(LISTING)); + await late; + expect(app.folderGrid).toBeNull(); + expect(elements.folderGridOverlay.classList.contains('visible')).toBe(false); + }); +}); + describe('"Show folder" in the preview', () => { it('knows the folder of absolute, relative and ~/ paths, not of a bare attachment card', () => { const { app } = loadApp(() => ok(LISTING)); diff --git a/test/link-provider-regex.test.ts b/test/link-provider-regex.test.ts index 8375d50a..0568e482 100644 --- a/test/link-provider-regex.test.ts +++ b/test/link-provider-regex.test.ts @@ -56,6 +56,8 @@ const KILLER_LINES = [ // long URL-ish and path-ish soup for the other patterns 'https://example.com/' + 'a/'.repeat(1500) + ' ' + '/home/x/'.repeat(400) + '.'.repeat(2000), 'Bash(' + 'x'.repeat(4000), + // a colon run ending in `//`: a folder body that kept `:` rescanned it for `://` from every colon + 'a:'.repeat(16000) + '//', ]; describe('terminal link-provider regexes (shipped source)', () => { diff --git a/test/routes/file-routes.test.ts b/test/routes/file-routes.test.ts index 6e278048..f56054a2 100644 --- a/test/routes/file-routes.test.ts +++ b/test/routes/file-routes.test.ts @@ -18,6 +18,7 @@ vi.mock('node:fs/promises', () => ({ readdir: vi.fn(async () => []), readFile: vi.fn(async () => 'file content'), stat: vi.fn(async () => ({ size: 100, isFile: () => true, isDirectory: () => true })), + realpath: vi.fn(async (p: string) => p), }, })); @@ -442,6 +443,86 @@ describe('file-routes', () => { } }); + // The folder grid is a third file-serving surface (folder-listing.ts): the + // same rule as the picker, on the resolved folder AND on each served file. + describe('folder grid', () => { + const ALICE = `${SPACES}/alice/cases`; + const BOB = `${SPACES}/bob/renders`; + + const owned = async (role: 'admin' | 'user', username: string) => { + const scoped = await harnessAs(role, username); + scoped.ctx._session.owner = username; + return scoped; + }; + const grant = (scoped: RouteTestHarness, path: string) => + scoped.app.inject({ + method: 'POST', + url: `/api/sessions/${scoped.ctx._sessionId}/folders`, + payload: { path }, + }); + + beforeEach(() => { + mockedStat.mockImplementation(async (p) => { + const file = /\.\w+$/.test(String(p)); + return { size: 100, mtimeMs: 1, isFile: () => file, isDirectory: () => !file } as never; + }); + }); + + it("refuses another user's space and never reads it", async () => { + const scoped = await owned('user', 'bob'); + try { + mockedReaddir.mockClear(); + const res = await grant(scoped, ALICE); + expect(res.statusCode).toBe(403); + expect(JSON.parse(res.body).error).toBe('Folder is outside your space'); + expect(mockedReaddir).not.toHaveBeenCalled(); + } finally { + await scoped.app.close(); + } + }); + + it('lists a folder in their own space', async () => { + const scoped = await owned('user', 'bob'); + try { + mockedReaddir.mockResolvedValueOnce([] as never); + const res = await grant(scoped, BOB); + expect(res.statusCode).toBe(200); + expect(JSON.parse(res.body).data.path).toBe(BOB); + } finally { + await scoped.app.close(); + } + }); + + it("refuses a file of their own folder that resolves into another user's space", async () => { + const scoped = await owned('user', 'bob'); + try { + mockedReaddir.mockResolvedValueOnce([] as never); + const { folderId } = JSON.parse((await grant(scoped, BOB)).body).data; + mockedRealpathSync.mockImplementation(((p: string) => + p === `${BOB}/plan.png` ? `${ALICE}/secret-project/plan.png` : p) as never); + const res = await scoped.app.inject({ + method: 'GET', + url: `/api/sessions/${scoped.ctx._sessionId}/folders/${folderId}/files/plan.png/raw`, + }); + expect(res.statusCode).toBe(403); + expect(JSON.parse(res.body).error).toBe('File is outside your space'); + expect(mockedCreateReadStream).not.toHaveBeenCalled(); + } finally { + await scoped.app.close(); + } + }); + + it('lets a multi-user admin list any folder', async () => { + const scoped = await owned('admin', 'root'); + try { + mockedReaddir.mockResolvedValueOnce([] as never); + expect((await grant(scoped, ALICE)).statusCode).toBe(200); + } finally { + await scoped.app.close(); + } + }); + }); + it('keeps the host-wide roots for a multi-user admin', async () => { const scoped = await harnessAs('admin', 'root'); try { diff --git a/test/routes/folder-grid-routes.test.ts b/test/routes/folder-grid-routes.test.ts index db360270..a678656f 100644 --- a/test/routes/folder-grid-routes.test.ts +++ b/test/routes/folder-grid-routes.test.ts @@ -11,6 +11,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { Readable } from 'node:stream'; import { execFileSync } from 'node:child_process'; import { mkdirSync, mkdtempSync, rmSync, symlinkSync, utimesSync, writeFileSync } from 'node:fs'; +import fs from 'node:fs/promises'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; import { createRouteTestHarness, type RouteTestHarness } from './_route-test-utils.js'; @@ -28,6 +29,12 @@ vi.mock('../../src/remote-files.js', async (importOriginal) => { }; }); +vi.mock('../../src/utils/bounded-path-probe.js', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, probePathKind: vi.fn(actual.probePathKind) }; +}); + +import { probePathKind } from '../../src/utils/bounded-path-probe.js'; import { buildRemoteFolderListCommand, parseRemoteFolderListOutput, @@ -36,6 +43,7 @@ import { remoteProbePaths, } from '../../src/remote-files.js'; +const mockedProbePathKind = vi.mocked(probePathKind); const mockedListFolder = vi.mocked(remoteListFolder); const mockedProbePaths = vi.mocked(remoteProbePaths); const mockedCreateReadStream = vi.mocked(remoteCreateReadStream); @@ -115,6 +123,48 @@ describe('folder grid routes — local folder', () => { expect((await grant(dir)).statusCode).toBe(200); }); + it('never reads a refused folder: the guard runs on the resolved folder first', async () => { + const readdir = vi.spyOn(fs, 'readdir'); + try { + process.env.CODEMAN_ATTACHMENT_BLOCKED_PATHS = dir; + expect((await grant(dir)).statusCode).toBe(403); + expect(readdir).not.toHaveBeenCalled(); + } finally { + readdir.mockRestore(); + } + }); + + it('answers a folder or file that does not respond with 503, never 404, and does not read it', async () => { + const readdir = vi.spyOn(fs, 'readdir'); + try { + mockedProbePathKind.mockResolvedValueOnce('unknown'); + const res = await grant(dir); + expect(res.statusCode).toBe(503); + expect(JSON.parse(res.body).error).toContain(`Folder is not responding or not readable: ${dir}`); + expect(readdir).not.toHaveBeenCalled(); + } finally { + readdir.mockRestore(); + } + + const { folderId } = JSON.parse((await grant(dir)).body).data; + mockedProbePathKind.mockResolvedValueOnce('unknown'); + const file = await harness.app.inject({ + method: 'GET', + url: `/api/sessions/${sessionId}/folders/${folderId}/files/old.png/raw`, + }); + expect(file.statusCode).toBe(503); + expect(JSON.parse(file.body).error).toContain('File is not responding or not readable'); + }); + + it('does not serve a hidden file by name, as the listing hides it', async () => { + const { folderId } = JSON.parse((await grant(dir)).body).data; + const res = await harness.app.inject({ + method: 'GET', + url: `/api/sessions/${sessionId}/folders/${folderId}/files/.hidden.png/raw`, + }); + expect(res.statusCode).toBe(400); + }); + it('serves a granted file by name, with ranges', async () => { const { folderId } = JSON.parse((await grant(dir)).body).data; const res = await harness.app.inject({ @@ -306,6 +356,7 @@ describe('remote folder listing script (real sh)', () => { mkdirSync(dir); touch(join(dir, "it's a | frame.PNG"), '12345', 1_700_000_000); touch(join(dir, 'doc.pdf'), 'pdf', 1_700_000_100); + touch(join(dir, 'shot.Png'), 'x', 1_700_000_100); touch(join(dir, 'skip.txt'), 'x', 1_700_000_100); touch(join(dir, '.hidden.png'), 'x', 1_700_000_100); mkdirSync(join(dir, 'sub dir')); @@ -315,7 +366,8 @@ describe('remote folder listing script (real sh)', () => { expect(listing?.dir).toBe(dir); expect(listing?.truncated).toBe(false); const byName = Object.fromEntries((listing?.entries ?? []).map((e) => [e.name, e])); - expect(Object.keys(byName).sort()).toEqual(['doc.pdf', "it's a | frame.PNG", 'sub dir']); + // Any case, as the local listing: `shot.Png` too. + expect(Object.keys(byName).sort()).toEqual(['doc.pdf', "it's a | frame.PNG", 'shot.Png', 'sub dir']); expect(byName["it's a | frame.PNG"]).toMatchObject({ kind: 'file', size: 5, mtimeMs: 1_700_000_000_000 }); expect(byName['sub dir']).toMatchObject({ kind: 'directory' }); }); @@ -333,7 +385,7 @@ describe('remote folder listing script (real sh)', () => { describe('validFolderFileName', () => { it('accepts bare supported names only', () => { expect(validFolderFileName('a b.png')).toBe('a b.png'); - for (const bad of ['', '.', '..', 'a/b.png', 'a\\b.png', 'x.xyz', 'a\0.png', 42]) { + for (const bad of ['', '.', '..', '.hidden.png', 'a/b.png', 'a\\b.png', 'x.xyz', 'a\0.png', 42]) { expect(validFolderFileName(bad)).toBeNull(); } });