From fe77b5238e1295700c962abb8714f87d674ea9cc Mon Sep 17 00:00:00 2001 From: stefaan van hoof Date: Wed, 7 Oct 2026 16:26:39 +0200 Subject: [PATCH 1/2] feat(file-preview): render local HTML files as sandboxed pages Clicking an agent's `.html` path showed source text or downloaded it, so a generated report could only be viewed by asking the agent to start a web server. Render it instead, without serving HTML same-origin: - POST /api/sessions/:id/html-view mints a capability for the file's own directory (workspace path, or outside it under the attachment guard). - GET /html-view/:cap/* serves the page and its relative assets with `Content-Security-Policy: sandbox` WITHOUT allow-same-origin, so the page runs in an opaque origin (also as a top-level tab) and cannot read the Codeman document, its cookies or its API. - Served files: non-dot, asset-allowlisted, realpath-confined to that directory, blocklist applied; GET only. Capabilities are memory-only with a rolling TTL and are revoked with the web-tab ones on logout. - Frontend: the file preview renders HTML in a sandboxed iframe (popout opens the same URL); terminal `.html` links go to the preview, not the log viewer. Co-Authored-By: Claude Opus 5.5 (1M context) --- CLAUDE.md | 2 +- src/html-view-capabilities.ts | 114 +++++++++++++++++ src/web/middleware/auth.ts | 20 +++ src/web/public/panels-ui.js | 24 ++++ src/web/public/terminal-ui.js | 8 +- src/web/routes/admin-routes.ts | 3 + src/web/routes/html-view-routes.ts | 179 +++++++++++++++++++++++++++ src/web/routes/index.ts | 1 + src/web/routes/session-routes.ts | 2 + src/web/server.ts | 2 + test/routes/html-view-routes.test.ts | 109 ++++++++++++++++ 11 files changed, 462 insertions(+), 2 deletions(-) create mode 100644 src/html-view-capabilities.ts create mode 100644 src/web/routes/html-view-routes.ts create mode 100644 test/routes/html-view-routes.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index 1ab43603..0dff1564 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -292,7 +292,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Attachments** (live external document references; all wiring in `file-routes.ts`): a **registry** maps a stable `attachmentId` to a realpath-resolved, extension-allowlisted absolute path, so browser requests never carry arbitrary absolute paths. ⚠️ The **magic-link scanner** (`codeman://attach?...` in terminal output) is **prompt-injectable**, so its scan path is force-confined to the session workspace; a hostile prompt could otherwise exfiltrate arbitrary host files over SSE. The security gate is an extension **allowlist**, not a blocklist. `document-conversion-limiter.ts` caps converter spawns globally: without it, N large docs detected at once fork N multi-minute processes, which is a resource-exhaustion vector. → [architecture-invariants#attachments](docs/architecture-invariants.md#attachments) -**File-path links (terminal + chat)**: a path an agent prints is clickable on BOTH surfaces and opens the file-preview overlay. ⚠️ ONE pattern (`FILE_PATH_LINK_PATTERN` / `absoluteFilePathPattern()` in constants.js) feeds the xterm link provider AND `_linkifyFilePaths()`, a fresh instance per call (`lastIndex`). The chat linkifier walks TEXT NODES with DOM APIs, never rebuilds sanitized markup as a string. ⚠️ An out-of-workspace path goes through the ATTACHMENT routes (`POST /api/sessions/:id/attachments` with `notify: false`), never by widening `file-content`/`file-raw` or `file-stream-manager`'s `tail -f` allowlist. ⚠️ `TEXT_ATTACHMENT_EXTENSIONS` IS `EDITABLE_EXTENSIONS` (never a second list), and widening READ must never widen RUN: `html`/`htm`/`svg` stay download-only, other text is inert `text/plain`+`nosniff`. Media extensions are single-sourced in `attachment-registry.ts`. → [architecture-invariants#file-path-links-terminal--response-viewer](docs/architecture-invariants.md#file-path-links-terminal--response-viewer) +**File-path links (terminal + chat)**: a path an agent prints is clickable on BOTH surfaces and opens the file-preview overlay. ⚠️ ONE pattern (`FILE_PATH_LINK_PATTERN` / `absoluteFilePathPattern()` in constants.js) feeds the xterm link provider AND `_linkifyFilePaths()`, a fresh instance per call (`lastIndex`). The chat linkifier walks TEXT NODES with DOM APIs, never rebuilds sanitized markup as a string. ⚠️ An out-of-workspace path goes through the ATTACHMENT routes (`POST /api/sessions/:id/attachments` with `notify: false`), never by widening `file-content`/`file-raw` or `file-stream-manager`'s `tail -f` allowlist. ⚠️ `TEXT_ATTACHMENT_EXTENSIONS` IS `EDITABLE_EXTENSIONS` (never a second list), and widening READ must never widen RUN: `html`/`htm`/`svg` stay download-only, other text is inert `text/plain`+`nosniff`. ⚠️ The ONE place HTML renders is `/html-view//...` (`html-view-routes.ts`, `html-view-capabilities.ts`): `POST /api/sessions/:id/html-view` mints a capability for the file's own directory, every response carries `Content-Security-Policy: sandbox` WITHOUT `allow-same-origin` (opaque origin even as a top-level tab, so the page can never read Codeman or drive its API), and only non-dot, asset-allowlisted files under that directory are served (GET only; capabilities revoked with the web-tab ones on logout). Never serve HTML same-origin anywhere else. Tests: `test/routes/html-view-routes.test.ts`. Media extensions are single-sourced in `attachment-registry.ts`. → [architecture-invariants#file-path-links-terminal--response-viewer](docs/architecture-invariants.md#file-path-links-terminal--response-viewer) **Filesystem path picker** (Link Existing "Browse" + the mobile keyboard's `📁 Path` key): lazy one-directory browsing via `GET /api/filesystem/browse`, with `GET /api/filesystem/preview` for the tapped file. Inserts the path **without** Enter, so the prompt is never submitted; the sibling `⌫ All` key clears only the unsent prompt and must never send the agent's `/clear`. ⚠️ This is a **second file-serving surface and inherits neither the attachment confinement nor its ownership scoping** — it allowlists Home, `CASES_DIR`, `/mnt/d` and `CODEMAN_FILE_PICKER_ROOTS`, blocks sensitive trees, and rejects symlink escapes **after** `realpath`. ⚠️ The optional `sessionId` is an ownership boundary that must be `canAccessOwned`-checked by hand (it does not go through `findSessionOrFail`), and in multi-user mode a non-admin gets only their own `userSpacePath` as a root: per-user spaces live INSIDE `homedir()`, so a `Home` root exposes every other user's workspace. Previews go through the same global conversion limiter, and Markdown/TXT/JSON are served as inert `text/plain`. → [architecture-invariants#filesystem-path-picker](docs/architecture-invariants.md#filesystem-path-picker) diff --git a/src/html-view-capabilities.ts b/src/html-view-capabilities.ts new file mode 100644 index 00000000..9d62ebb4 --- /dev/null +++ b/src/html-view-capabilities.ts @@ -0,0 +1,114 @@ +/** + * @fileoverview Capability tokens for rendering a local HTML file. + * + * `file-raw` never serves HTML as HTML: a page rendered from Codeman's own origin + * could read the Codeman document and drive the agent-spawning API. So an HTML + * file is rendered from `/html-view/:cap/*` instead, with a `Content-Security- + * Policy: sandbox` header that puts the document in an OPAQUE origin no matter how + * it is opened (iframe or top-level tab). That opaque origin is also why the route + * cannot authenticate on the session cookie: its subresource requests are + * cross-site, so the `SameSite=lax` cookie is never attached (the same reason the + * web-tab proxy uses capabilities, see webview-capabilities.ts). + * + * A capability is minted by an authenticated `POST /api/sessions/:id/html-view` and + * grants exactly one thing: GET access to the files in ONE directory (the HTML + * file's own), so a page's relative `style.css` / `chart.js` / images resolve. + * + * - 192 bits of `randomBytes` entropy, base64url. + * - Held in memory only. A restart invalidates every outstanding capability. + * - Rolling TTL, revoked on logout, admin logout and user deletion (`revokeOwner`). + */ + +import { randomBytes } from 'node:crypto'; +import { StaleExpirationMap } from './utils/index.js'; +import { MAX_WEBVIEW_CAPABILITIES, WEBVIEW_CAPABILITY_TTL_MS } from './config/webview-limits.js'; + +/** URL prefix the rendered files are served under. */ +export const HTML_VIEW_PREFIX = '/html-view'; + +export interface HtmlViewCapabilityRecord { + /** Realpath of the directory the capability serves. */ + rootDir: string; + /** Username that minted it (multi-user); undefined in single-user mode. */ + owner?: string; + createdAt: number; +} + +export class HtmlViewCapabilityStore { + private readonly capabilities: StaleExpirationMap; + /** Reverse index so re-opening a page reuses its capability instead of leaking one per click. */ + private readonly byRoot = new Map(); + + constructor(ttlMs: number = WEBVIEW_CAPABILITY_TTL_MS) { + this.capabilities = new StaleExpirationMap({ + ttlMs, + refreshOnGet: true, + onExpire: (token, record) => { + const key = rootKey(record.rootDir, record.owner); + if (this.byRoot.get(key) === token) this.byRoot.delete(key); + }, + }); + } + + /** Mint (or reuse) a capability for a directory. Returns the token. */ + mint(rootDir: string, owner?: string): string { + const key = rootKey(rootDir, owner); + const existing = this.byRoot.get(key); + if (existing && this.capabilities.get(existing)) return existing; + if (existing) this.byRoot.delete(key); + + if (this.capabilities.size >= MAX_WEBVIEW_CAPABILITIES) this.capabilities.cleanup(); + + const token = randomBytes(24).toString('base64url'); + this.capabilities.set(token, { rootDir, owner, createdAt: Date.now() }); + this.byRoot.set(key, token); + return token; + } + + /** Resolve a capability, refreshing its TTL. Returns undefined when unknown or expired. */ + resolve(token: string): HtmlViewCapabilityRecord | undefined { + if (!token) return undefined; + return this.capabilities.get(token); + } + + /** Revoke every capability bound to an identity (logout, admin logout, user deletion). */ + revokeOwner(owner: string | undefined): number { + let revoked = 0; + for (const [key, token] of [...this.byRoot]) { + const record = this.capabilities.peek(token); + if (!record || record.owner === owner) { + this.capabilities.delete(token); + this.byRoot.delete(key); + if (record) revoked++; + } + } + return revoked; + } + + get size(): number { + return this.capabilities.size; + } + + dispose(): void { + this.capabilities.dispose(); + this.byRoot.clear(); + } +} + +function rootKey(rootDir: string, owner: string | undefined): string { + return `${owner ?? ''}\0${rootDir}`; +} + +/** The capability token in an `/html-view//...` path, or null. */ +export function capabilityFromHtmlViewPath(pathname: string): string | null { + if (typeof pathname !== 'string') return null; + const prefix = `${HTML_VIEW_PREFIX}/`; + if (!pathname.startsWith(prefix)) return null; + const rest = pathname.slice(prefix.length); + const slash = rest.indexOf('/'); + const cap = slash === -1 ? rest : rest.slice(0, slash); + return /^[A-Za-z0-9_-]{16,128}$/.test(cap) ? cap : null; +} + +/** Process-wide store, shared by the routes and the auth middleware (see webviewCapabilities). */ +export const htmlViewCapabilities = new HtmlViewCapabilityStore(); diff --git a/src/web/middleware/auth.ts b/src/web/middleware/auth.ts index 9dc78bcd..0816c2ab 100644 --- a/src/web/middleware/auth.ts +++ b/src/web/middleware/auth.ts @@ -23,6 +23,7 @@ import { getHookSecret, HOOK_SECRET_HEADER } from '../../config/hook-secret.js'; import { isMultiUserMode } from '../../config/multiuser.js'; import { findUser, setPassword, touchLastLogin, verifyPassword } from '../../user-store.js'; import { webviewCapabilities } from '../../webview-capabilities.js'; +import { capabilityFromHtmlViewPath, htmlViewCapabilities } from '../../html-view-capabilities.js'; import { capabilityFromProxyPath, capabilityFromReferer, @@ -149,6 +150,18 @@ function isPasswordChangeExempt(req: FastifyRequest): boolean { * match the prefix at all. The Host allowlist is NOT bypassed, so DNS-rebinding * protection still applies to these requests. */ +/** + * Whether a request is a GET for a rendered HTML file (`/html-view//...`) with + * a live capability. Same reasoning as the web-tab proxy: the page is sandboxed into + * an opaque origin, so its asset requests carry no session cookie. Read-only by + * construction (safe methods only), and the Host allowlist still applies. + */ +function hasValidHtmlViewCapability(req: FastifyRequest): boolean { + if (req.method !== 'GET' && req.method !== 'HEAD') return false; + const cap = capabilityFromHtmlViewPath((req.url ?? '').split('?')[0]); + return !!cap && htmlViewCapabilities.resolve(cap) !== undefined; +} + function hasValidWebviewCapability(req: FastifyRequest, basePath = ''): boolean { // req.url is already base-stripped by the server's rewriteUrl, so the path form // needs no base; the Referer form below is browser-supplied and does. @@ -368,6 +381,11 @@ export function registerAuthMiddleware(app: FastifyInstance, https: boolean, bas done(); return; } + // Rendered HTML file, likewise authenticated by its capability. + if (hasValidHtmlViewCapability(req)) { + done(); + return; + } // A web-tab frame that lost its prefix: hand it back to its tab, no credentials involved. if (serveLostWebviewFrame(req, reply)) return; @@ -507,6 +525,8 @@ function registerMultiUserAuthHook( // ownership against the identity BOUND TO THE CAPABILITY, which is stricter // than re-deriving it from a request that carries no credentials. if (hasValidWebviewCapability(req, basePath)) return; + // Rendered HTML file, likewise authenticated by its capability. + if (hasValidHtmlViewCapability(req)) return; // A web-tab frame that lost its prefix: hand it back to its tab, no credentials involved. if (serveLostWebviewFrame(req, reply)) return; diff --git a/src/web/public/panels-ui.js b/src/web/public/panels-ui.js index 49a582ee..f878d918 100644 --- a/src/web/public/panels-ui.js +++ b/src/web/public/panels-ui.js @@ -4116,6 +4116,30 @@ Object.assign(CodemanApp.prototype, { const ext = (filePath.split('.').pop() || '').toLowerCase(); + // HTML renders as a page: the server mints a sandboxed /html-view URL for the + // file's directory (html-view-routes.ts), so relative CSS, scripts and images + // resolve, and the page runs in an opaque origin that cannot reach Codeman. + // This branch comes first because the routes below serve HTML download-only. + if (!attachmentId && (ext === 'html' || ext === 'htm')) { + footerEl.textContent = 'HTML'; + try { + const res = await fetch(`/api/sessions/${sessionId}/html-view`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ path: filePath }), + }); + const result = await res.json().catch(() => ({})); + if (!res.ok || !result.success) throw new Error(result.error || 'Failed to open page'); + const pageUrl = CodemanBase.url(result.data.url); + this.filePreviewDetachUrl = pageUrl; + if (detachBtn) detachBtn.hidden = false; + bodyEl.innerHTML = ``; + } catch (err) { + bodyEl.innerHTML = `
Error: ${escapeHtml(err.message)}
`; + } + return; + } + // Out-of-workspace path: mint an attachment id up front. Every branch below // talks to a workspace-confined route, so without this the image/PDF ones // render a broken frame and the text one reports a bare "File not found" diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index a03bc4d1..f33cee22 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -1978,7 +1978,13 @@ Object.assign(CodemanApp.prototype, { // path clicked in the response viewer previewed fine. The preview // reads those through the guarded attachment routes, so external // paths route there and the two surfaces agree. - if (previewsInFileViewer(text) || self._isExternalPreviewPath(text, self.activeSessionId)) { + // An HTML file renders as a page in the preview, never as source + // in the log viewer. + if ( + previewsInFileViewer(text) || + /\.html?$/i.test(text) || + self._isExternalPreviewPath(text, self.activeSessionId) + ) { self.openFilePreview(text, self.activeSessionId); return; } diff --git a/src/web/routes/admin-routes.ts b/src/web/routes/admin-routes.ts index 8f9294c9..5f0705d4 100644 --- a/src/web/routes/admin-routes.ts +++ b/src/web/routes/admin-routes.ts @@ -36,6 +36,7 @@ import { } from '../../user-store.js'; import { getAuthUser, requireAdmin, revokeUserSessions } from '../route-helpers.js'; import { webviewCapabilities } from '../../webview-capabilities.js'; +import { htmlViewCapabilities } from '../../html-view-capabilities.js'; import { appendAdminAudit } from '../admin-audit.js'; import { SseEvent } from '../sse-events.js'; import type { AuthPort } from '../ports/auth-port.js'; @@ -183,6 +184,7 @@ export function registerAdminRoutes(app: FastifyInstance, ctx: SessionPort & Aut // Web-tab proxy capabilities are a second credential the cookie purge does not // touch; a forced logout that left them alive would not be a logout. const revokedWebviews = webviewCapabilities.revokeOwner(normalizeUsername(username)); + htmlViewCapabilities.revokeOwner(normalizeUsername(username)); audit(req, 'user.logout', username, { revoked, revokedWebviews }); return { success: true, data: { revoked } }; }); @@ -206,6 +208,7 @@ export function registerAdminRoutes(app: FastifyInstance, ctx: SessionPort & Aut } revokeUserSessions(ctx.authSessions, username); webviewCapabilities.revokeOwner(normalizeUsername(username)); + htmlViewCapabilities.revokeOwner(normalizeUsername(username)); if (deleteSpace) await deleteUserSpace(username); audit(req, 'user.delete', username, { deleteSpace, killedSessions: owned.length }); ctx.broadcast(SseEvent.AdminUsersChanged, {}); diff --git a/src/web/routes/html-view-routes.ts b/src/web/routes/html-view-routes.ts new file mode 100644 index 00000000..6731641e --- /dev/null +++ b/src/web/routes/html-view-routes.ts @@ -0,0 +1,179 @@ +/** + * @fileoverview Render a local HTML file (an agent's report, a generated page) as a + * page instead of as source text. + * + * Routes: + * - `POST /api/sessions/:id/html-view` (authenticated) — `{ path }` of an `.html` file; + * returns `{ url }`, an `/html-view//` URL for that file. + * - `GET /html-view/:cap/*` (capability) — the HTML file and its sibling assets. + * + * ⚠️ Every response carries `Content-Security-Policy: sandbox ...` WITHOUT + * `allow-same-origin`, so the page runs in an opaque origin whether it is framed or + * opened as a top-level tab: it can run its own scripts but cannot read the Codeman + * document, its cookies or its API. That is what makes rendering safe at all; see + * html-view-capabilities.ts for why the route authenticates on a capability. + * + * Path policy: the HTML file itself is admitted like the file preview admits a file + * (inside the session workspace, or outside it under the attachment guard), and the + * capability then serves ONLY the files in that file's own directory tree, filtered + * by the same blocklist plus an asset-extension allowlist and a no-dotfile rule, so + * a page cannot use its own directory to pull a `.env` out through the browser. + */ + +import type { FastifyInstance } from 'fastify'; +import { createReadStream, realpathSync } from 'node:fs'; +import fs from 'node:fs/promises'; +import { basename, dirname, extname, isAbsolute, resolve, sep } from 'node:path'; +import { ApiErrorCode, createErrorResponse } from '../../types.js'; +import { isBlockedAttachmentPath, isUnderTree, loadAttachmentGuardConfig } from '../../config/attachment-guard.js'; +import { HTML_VIEW_PREFIX, htmlViewCapabilities } from '../../html-view-capabilities.js'; +import { findSessionOrFail, ownerFor, validateSessionFilePath } from '../route-helpers.js'; +import { exceedsDownloadLimit } from '../../config/buffer-limits.js'; +import type { SessionPort } from '../ports/index.js'; + +/** Extensions that open as a rendered page. */ +export const HTML_VIEW_EXTENSIONS: ReadonlySet = new Set(['html', 'htm']); + +/** What a rendered page may load from its own directory, with the type it is served as. */ +const ASSET_TYPES: Record = { + html: 'text/html; charset=utf-8', + htm: 'text/html; charset=utf-8', + css: 'text/css; charset=utf-8', + js: 'text/javascript; charset=utf-8', + mjs: 'text/javascript; charset=utf-8', + json: 'application/json; charset=utf-8', + map: 'application/json; charset=utf-8', + txt: 'text/plain; charset=utf-8', + csv: 'text/csv; charset=utf-8', + md: 'text/plain; charset=utf-8', + svg: 'image/svg+xml', + png: 'image/png', + jpg: 'image/jpeg', + jpeg: 'image/jpeg', + gif: 'image/gif', + webp: 'image/webp', + avif: 'image/avif', + ico: 'image/x-icon', + bmp: 'image/bmp', + woff: 'font/woff', + woff2: 'font/woff2', + ttf: 'font/ttf', + otf: 'font/otf', + wasm: 'application/wasm', + mp4: 'video/mp4', + webm: 'video/webm', + mp3: 'audio/mpeg', + wav: 'audio/wav', + pdf: 'application/pdf', +}; + +/** + * The sandbox every response is served under. No `allow-same-origin`: that one + * token is the difference between "a page" and "a page that can drive Codeman". + */ +const SANDBOX_CSP = 'sandbox allow-scripts allow-forms allow-popups allow-modals allow-downloads'; + +function extensionOf(path: string): string { + return extname(path).toLowerCase().replace(/^\./, ''); +} + +export function registerHtmlViewRoutes(app: FastifyInstance, ctx: SessionPort): void { + app.post('/api/sessions/:id/html-view', async (req, reply) => { + const { id } = req.params as { id: string }; + const session = findSessionOrFail(ctx, id, req); + const body = (req.body || {}) as { path?: string }; + + if (!body.path || typeof body.path !== 'string') { + reply.code(400).send(createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Missing path parameter')); + return; + } + if (session.remote) { + reply + .code(400) + .send(createErrorResponse(ApiErrorCode.INVALID_INPUT, 'HTML pages in a remote (SSH) case cannot be rendered')); + return; + } + if (!HTML_VIEW_EXTENSIONS.has(extensionOf(body.path))) { + reply.code(400).send(createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Not an HTML file')); + return; + } + + // Inside the workspace: the same check every file route uses. Outside it: the + // attachment guard, exactly as an external file preview is admitted. + let resolvedPath: string; + const inWorkspace = validateSessionFilePath(session.workingDir, body.path); + if (inWorkspace) { + resolvedPath = inWorkspace.resolvedPath; + } else { + if (!isAbsolute(body.path)) { + reply.code(404).send(createErrorResponse(ApiErrorCode.NOT_FOUND, 'File not found')); + return; + } + try { + resolvedPath = realpathSync(body.path); + } catch { + reply.code(404).send(createErrorResponse(ApiErrorCode.NOT_FOUND, 'File not found')); + return; + } + const guard = await loadAttachmentGuardConfig(); + if (guard.confineToWorkspace || isBlockedAttachmentPath(resolvedPath, guard.blockedTrees)) { + reply.code(403).send(createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Access to this file is blocked')); + return; + } + } + + const stat = await fs.stat(resolvedPath).catch(() => null); + if (!stat?.isFile()) { + reply.code(404).send(createErrorResponse(ApiErrorCode.NOT_FOUND, 'File not found')); + return; + } + + const cap = htmlViewCapabilities.mint(dirname(resolvedPath), ownerFor(req)); + return { + success: true, + data: { url: `${HTML_VIEW_PREFIX}/${cap}/${encodeURIComponent(basename(resolvedPath))}` }, + }; + }); + + app.get(`${HTML_VIEW_PREFIX}/:cap/*`, async (req, reply) => { + const { cap, '*': rest } = req.params as { cap: string; '*': string }; + const record = htmlViewCapabilities.resolve(cap); + const notFound = () => reply.code(404).header('Content-Type', 'text/plain; charset=utf-8').send('Not found'); + if (!record) return notFound(); + + // Fastify has already percent-decoded the wildcard. Hidden files and + // directories are never served, whatever their extension. + const segments = (rest || '').split('/').filter(Boolean); + if (segments.length === 0 || segments.some((s) => s.startsWith('.'))) return notFound(); + + const ext = extensionOf(segments[segments.length - 1]); + const contentType = ASSET_TYPES[ext]; + if (!contentType) return notFound(); + + let resolvedPath: string; + try { + resolvedPath = realpathSync(resolve(record.rootDir, segments.join(sep))); + } catch { + return notFound(); + } + // Symlinks are resolved above, so this is the real containment check. + if (!isUnderTree(resolvedPath, record.rootDir)) return notFound(); + const guard = await loadAttachmentGuardConfig(); + if (isBlockedAttachmentPath(resolvedPath, guard.blockedTrees)) return notFound(); + + const stat = await fs.stat(resolvedPath).catch(() => null); + if (!stat?.isFile() || exceedsDownloadLimit(stat.size)) return notFound(); + + reply.header('Content-Security-Policy', SANDBOX_CSP); + reply.header('Content-Type', contentType); + reply.header('Content-Length', String(stat.size)); + reply.header('Cache-Control', 'no-store'); + reply.header('Referrer-Policy', 'no-referrer'); + // The sandboxed page is opaque-origin, so even its fetch() of a sibling + // `data.json` is CORS-checked with `Origin: null`. `*` without credentials is + // enough, and widens nothing: these bytes are already readable by anyone + // holding the capability URL. + reply.header('Access-Control-Allow-Origin', '*'); + return reply.send(createReadStream(resolvedPath)); + }); +} diff --git a/src/web/routes/index.ts b/src/web/routes/index.ts index a8886041..7f3304fc 100644 --- a/src/web/routes/index.ts +++ b/src/web/routes/index.ts @@ -6,6 +6,7 @@ export { registerPushRoutes } from './push-routes.js'; export { registerTeamRoutes } from './team-routes.js'; export { registerMuxRoutes } from './mux-routes.js'; export { registerFileRoutes } from './file-routes.js'; +export { registerHtmlViewRoutes } from './html-view-routes.js'; export { registerScheduledRoutes } from './scheduled-routes.js'; export { registerCronRoutes } from './cron-routes.js'; export { registerSystemRoutes } from './system-routes.js'; diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index bb4bdbe0..7cd4b7b5 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -40,6 +40,7 @@ import { import type { PaneCaptureOptions } from '../../mux-interface.js'; import { SseEvent } from '../sse-events.js'; import { webviewCapabilities } from '../../webview-capabilities.js'; +import { htmlViewCapabilities } from '../../html-view-capabilities.js'; import { CreateSessionSchema, SessionNameSchema, @@ -849,6 +850,7 @@ export function registerSessionRoutes( // logout has to retire them too or every dashboard URL opened during this // login keeps relaying without one (WebviewCapabilityStore.revokeOwner). webviewCapabilities.revokeOwner(ownerFor(req)); + htmlViewCapabilities.revokeOwner(ownerFor(req)); reply.clearCookie(AUTH_COOKIE_NAME, { path: '/' }); return {}; }); diff --git a/src/web/server.ts b/src/web/server.ts index b9f40556..1f64eec8 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -178,6 +178,7 @@ import { registerTeamRoutes, registerMuxRoutes, registerFileRoutes, + registerHtmlViewRoutes, registerScheduledRoutes, registerHookEventRoutes, registerApprovalRoutes, @@ -1116,6 +1117,7 @@ export class WebServer extends EventEmitter { registerTeamRoutes(this.app, ctx); registerMuxRoutes(this.app, ctx); registerFileRoutes(this.app, ctx); + registerHtmlViewRoutes(this.app, ctx); registerScheduledRoutes(this.app, ctx); registerHookEventRoutes(this.app, ctx); registerApprovalRoutes(this.app, ctx); diff --git a/test/routes/html-view-routes.test.ts b/test/routes/html-view-routes.test.ts new file mode 100644 index 00000000..311dbff2 --- /dev/null +++ b/test/routes/html-view-routes.test.ts @@ -0,0 +1,109 @@ +/** + * @fileoverview Tests for html-view-routes: rendering a local HTML file as a page. + * + * Real files in a temp dir (no fs mocks): the point of the route is its path + * policy, and a mocked realpath would make every containment check pass. + * Port: N/A (app.inject doesn't open ports) + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { mkdtempSync, mkdirSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { createRouteTestHarness, type RouteTestHarness } from './_route-test-utils.js'; +import { registerHtmlViewRoutes } from '../../src/web/routes/html-view-routes.js'; +import { capabilityFromHtmlViewPath, htmlViewCapabilities } from '../../src/html-view-capabilities.js'; + +describe('html-view routes', () => { + let harness: RouteTestHarness; + let sid: string; + let root: string; + let workspace: string; + + beforeEach(async () => { + root = mkdtempSync(join(tmpdir(), 'html-view-')); + workspace = join(root, 'ws'); + mkdirSync(join(workspace, 'assets'), { recursive: true }); + writeFileSync(join(workspace, 'index.html'), '

hi

'); + writeFileSync(join(workspace, 'assets', 'app.css'), 'h1{}'); + writeFileSync(join(workspace, 'data.json'), '{}'); + writeFileSync(join(workspace, '.env'), 'SECRET=1'); + writeFileSync(join(workspace, 'notes.sh'), 'echo'); + writeFileSync(join(root, 'outside.txt'), 'outside'); + symlinkSync(join(root, 'outside.txt'), join(workspace, 'escape.txt')); + + harness = await createRouteTestHarness(registerHtmlViewRoutes); + sid = harness.ctx._sessionId as string; + (harness.ctx.sessions.get(sid) as { workingDir: string }).workingDir = workspace; + }); + + afterEach(async () => { + await harness.app.close(); + htmlViewCapabilities.revokeOwner(undefined); + rmSync(root, { recursive: true, force: true }); + }); + + async function open(path: string) { + return harness.app.inject({ method: 'POST', url: `/api/sessions/${sid}/html-view`, payload: { path } }); + } + + async function openUrl(): Promise { + const res = await open(join(workspace, 'index.html')); + expect(res.statusCode).toBe(200); + return JSON.parse(res.body).data.url as string; + } + + it('mints a capability URL for an HTML file in the workspace', async () => { + const url = await openUrl(); + expect(url).toMatch(/^\/html-view\/[A-Za-z0-9_-]{16,}\/index\.html$/); + expect(capabilityFromHtmlViewPath(url)).not.toBeNull(); + }); + + it('accepts a workspace-relative path', async () => { + const res = await open('index.html'); + expect(res.statusCode).toBe(200); + }); + + it('refuses non-HTML files and missing files', async () => { + expect((await open(join(workspace, 'data.json'))).statusCode).toBe(400); + expect((await open(join(workspace, 'missing.html'))).statusCode).toBe(404); + }); + + it('serves the page sandboxed, never same-origin', async () => { + const url = await openUrl(); + const res = await harness.app.inject({ method: 'GET', url }); + expect(res.statusCode).toBe(200); + expect(res.headers['content-type']).toContain('text/html'); + const csp = String(res.headers['content-security-policy']); + expect(csp).toMatch(/^sandbox /); + expect(csp).not.toContain('allow-same-origin'); + expect(res.body).toBe('

hi

'); + }); + + it('serves sibling assets, including subdirectories', async () => { + const base = (await openUrl()).replace(/index\.html$/, ''); + expect((await harness.app.inject({ method: 'GET', url: `${base}assets/app.css` })).statusCode).toBe(200); + expect((await harness.app.inject({ method: 'GET', url: `${base}data.json` })).statusCode).toBe(200); + }); + + it('never serves dotfiles, non-asset types, escapes or symlinks out of the directory', async () => { + const base = (await openUrl()).replace(/index\.html$/, ''); + for (const p of ['.env', 'assets/../.env', '%2e%2e/outside.txt', '..%2foutside.txt', 'notes.sh', 'escape.txt']) { + const res = await harness.app.inject({ method: 'GET', url: `${base}${p}` }); + expect(res.statusCode, p).toBe(404); + } + }); + + it('refuses an unknown or revoked capability', async () => { + const url = await openUrl(); + expect( + (await harness.app.inject({ method: 'GET', url: '/html-view/AAAAAAAAAAAAAAAAAAAAAAAA/index.html' })).statusCode + ).toBe(404); + htmlViewCapabilities.revokeOwner(undefined); + expect((await harness.app.inject({ method: 'GET', url })).statusCode).toBe(404); + }); + + it('reuses one capability per directory', async () => { + expect(await openUrl()).toBe(await openUrl()); + }); +}); From 91bdaee72425a0612871609eb0919248ad1d3673 Mon Sep 17 00:00:00 2001 From: stefaan van hoof Date: Thu, 8 Oct 2026 09:21:55 +0200 Subject: [PATCH 2/2] =?UTF-8?q?fix(html-view):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20scope,=20source=20view,=20tests,=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Refuse to mint for an HTML file in a hidden directory or directly in a broad root (/, home, tmpdir, cases and user-space roots): the capability serves the directory tree recursively to the page's own scripts. - Keep HTML's source view and editor: a Page pill (per-device htmlRendered, like MD) toggles to the file-content text path, and a refused mint (remote case, broad root, hidden dir) falls through to it automatically. - Tests: html-view auth exemption edges, revocation on logout / admin logout / user deletion, out-of-workspace mint (blocked tree, confinement), hidden and broad roots. - Docs: Working-With-Files, architecture-invariants, security-architecture, the file-routes comment and CLAUDE.md. Co-Authored-By: Claude Opus 5.5 (1M context) --- CLAUDE.md | 2 +- docs/architecture-invariants.md | 4 +- docs/security-architecture.md | 1 + docs/wiki/Working-With-Files.md | 12 +++- src/web/public/index.html | 1 + src/web/public/panels-ui.js | 70 ++++++++++++++++------ src/web/routes/file-routes.ts | 4 +- src/web/routes/html-view-routes.ts | 54 ++++++++++++++++- test/routes/html-view-routes.test.ts | 70 +++++++++++++++++++++- test/webview-auth-exemption.test.ts | 45 ++++++++++++++ test/webview-capability-revocation.test.ts | 39 ++++++++++++ 11 files changed, 275 insertions(+), 27 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 17913d77..ab547ddb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -293,7 +293,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Attachments** (live external document references; all wiring in `file-routes.ts`): a **registry** maps a stable `attachmentId` to a realpath-resolved, extension-allowlisted absolute path, so browser requests never carry arbitrary absolute paths. ⚠️ The **magic-link scanner** (`codeman://attach?...` in terminal output) is **prompt-injectable**, so its scan path is force-confined to the session workspace; a hostile prompt could otherwise exfiltrate arbitrary host files over SSE. The security gate is an extension **allowlist**, not a blocklist. `document-conversion-limiter.ts` caps converter spawns globally: without it, N large docs detected at once fork N multi-minute processes, which is a resource-exhaustion vector. → [architecture-invariants#attachments](docs/architecture-invariants.md#attachments) -**File-path links (terminal + chat)**: a path an agent prints is clickable on BOTH surfaces and opens the file-preview overlay. ⚠️ ONE pattern (`FILE_PATH_LINK_PATTERN` / `absoluteFilePathPattern()` in constants.js) feeds the xterm link provider AND `_linkifyFilePaths()`, a fresh instance per call (`lastIndex`). The chat linkifier walks TEXT NODES with DOM APIs, never rebuilds sanitized markup as a string. ⚠️ An out-of-workspace path goes through the ATTACHMENT routes (`POST /api/sessions/:id/attachments` with `notify: false`), never by widening `file-content`/`file-raw` or `file-stream-manager`'s `tail -f` allowlist. ⚠️ `TEXT_ATTACHMENT_EXTENSIONS` IS `EDITABLE_EXTENSIONS` (never a second list), and widening READ must never widen RUN: `html`/`htm`/`svg` stay download-only, other text is inert `text/plain`+`nosniff`. ⚠️ The ONE place HTML renders is `/html-view//...` (`html-view-routes.ts`, `html-view-capabilities.ts`): `POST /api/sessions/:id/html-view` mints a capability for the file's own directory, every response carries `Content-Security-Policy: sandbox` WITHOUT `allow-same-origin` (opaque origin even as a top-level tab, so the page can never read Codeman or drive its API), and only non-dot, asset-allowlisted files under that directory are served (GET only; capabilities revoked with the web-tab ones on logout). Never serve HTML same-origin anywhere else. Tests: `test/routes/html-view-routes.test.ts`. Media extensions are single-sourced in `attachment-registry.ts`. → [architecture-invariants#file-path-links-terminal--response-viewer](docs/architecture-invariants.md#file-path-links-terminal--response-viewer) +**File-path links (terminal + chat)**: a path an agent prints is clickable on BOTH surfaces and opens the file-preview overlay. ⚠️ ONE pattern (`FILE_PATH_LINK_PATTERN` / `absoluteFilePathPattern()` in constants.js) feeds the xterm link provider AND `_linkifyFilePaths()`, a fresh instance per call (`lastIndex`). The chat linkifier walks TEXT NODES with DOM APIs, never rebuilds sanitized markup as a string. ⚠️ An out-of-workspace path goes through the ATTACHMENT routes (`POST /api/sessions/:id/attachments` with `notify: false`), never by widening `file-content`/`file-raw` or `file-stream-manager`'s `tail -f` allowlist. ⚠️ `TEXT_ATTACHMENT_EXTENSIONS` IS `EDITABLE_EXTENSIONS` (never a second list), and widening READ must never widen RUN: `html`/`htm`/`svg` stay download-only, other text is inert `text/plain`+`nosniff`. ⚠️ The ONE place HTML renders is `/html-view//...` (`html-view-routes.ts`, `html-view-capabilities.ts`): `POST /api/sessions/:id/html-view` mints a capability for the file's own directory, every response carries `Content-Security-Policy: sandbox` WITHOUT `allow-same-origin` (opaque origin even as a top-level tab, so the page can never read Codeman or drive its API), and only non-dot, asset-allowlisted files under that directory are served (GET/HEAD only; capabilities revoked with the web-tab ones on logout). That tree is served recursively, so minting is refused for a hidden directory or a broad root (`isBroadHtmlViewRoot`); the preview renders by default and the **Page** pill (or a refused mint) falls through to the source view and its Edit button. Never serve HTML same-origin anywhere else. Tests: `test/routes/html-view-routes.test.ts`. Media extensions are single-sourced in `attachment-registry.ts`. → [architecture-invariants#file-path-links-terminal--response-viewer](docs/architecture-invariants.md#file-path-links-terminal--response-viewer) **Filesystem path picker** (Link Existing "Browse" + the mobile keyboard's `📁 Path` key): lazy one-directory browsing via `GET /api/filesystem/browse`, with `GET /api/filesystem/preview` for the tapped file. Inserts the path **without** Enter, so the prompt is never submitted; the sibling `⌫ All` key clears only the unsent prompt and must never send the agent's `/clear`. ⚠️ This is a **second file-serving surface and inherits neither the attachment confinement nor its ownership scoping** — it allowlists Home, `CASES_DIR`, `/mnt/d` and `CODEMAN_FILE_PICKER_ROOTS`, blocks sensitive trees, and rejects symlink escapes **after** `realpath`. ⚠️ The optional `sessionId` is an ownership boundary that must be `canAccessOwned`-checked by hand (it does not go through `findSessionOrFail`), and in multi-user mode a non-admin gets only their own `userSpacePath` as a root: per-user spaces live INSIDE `homedir()`, so a `Home` root exposes every other user's workspace. Previews go through the same global conversion limiter, and Markdown/TXT/JSON are served as inert `text/plain`. → [architecture-invariants#filesystem-path-picker](docs/architecture-invariants.md#filesystem-path-picker) diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index cd49d869..7c521ba8 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -365,9 +365,9 @@ A file path an agent prints is a link on both surfaces it can appear on, and cli **Out-of-workspace paths go through the attachment routes, not the file routes.** `file-content`/`file-raw` resolve against `workingDir` and 404 anything that escapes it, which is correct and unchanged — but the paths agents most often print (a `/tmp` capture, Claude's own scratchpad, another checkout) are exactly that, so clicking one used to report "File not found" for a file sitting on disk. `openFilePreview()` now detects the case (`_isExternalPreviewPath`, a string compare for ROUTING only; the real decision stays server-side) and registers the path via `POST /api/sessions/:id/attachments` first, rendering by id. ⚠️ That registration passes `notify: false`, which suppresses ONLY the `attachment:detected` broadcast — the guard, the registry entry and the by-id routes are identical either way. Without it every click also popped an attachment card announcing the file already filling the screen. ⚠️ The click is an explicit user action on the **explicit, Origin-guarded** registration route, which is why it may cross the workspace boundary at all; the passive magic-link scanner stays force-confined. A type outside `SUPPORTED_ATTACHMENT_EXTENSIONS` (`.svg`, `.bmp`) is refused with a message naming what IS previewable, rather than the registry's own policy term. -⚠️ **The terminal routes an out-of-workspace path to the preview, not the log viewer.** The log viewer spawns `tail -f` and allows only the workspace, `/var/log` and `~/logs`, so an external `.log`/`.json`/code path answered `Path must be within working directory or allowed log directories` while the SAME path clicked in the response viewer previewed fine. `activate()` now checks `_isExternalPreviewPath` alongside `previewsInFileViewer`. In-workspace text keeps the tail viewer, which is the point of it (live follow); nothing widened `file-stream-manager`'s allowlist, so no `tail -f` is spawned on an arbitrary host path. +⚠️ **The terminal routes an out-of-workspace path to the preview, not the log viewer.** The log viewer spawns `tail -f` and allows only the workspace, `/var/log` and `~/logs`, so an external `.log`/`.json`/code path answered `Path must be within working directory or allowed log directories` while the SAME path clicked in the response viewer previewed fine. `activate()` now checks `_isExternalPreviewPath` alongside `previewsInFileViewer`. In-workspace text keeps the tail viewer, which is the point of it (live follow), except `.html`/`.htm`, which always go to the preview so they render as a page (see below); nothing widened `file-stream-manager`'s allowlist, so no `tail -f` is spawned on an arbitrary host path. -**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. +**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 one place HTML renders is `/html-view//...`** (`html-view-routes.ts`, `html-view-capabilities.ts`), never these routes: `POST /api/sessions/:id/html-view` mints a capability for the HTML file's own directory, and every response carries `Content-Security-Policy: sandbox` WITHOUT `allow-same-origin`, so the page is opaque-origin even as a top-level tab and cannot read Codeman or drive its API. The directory is served recursively to the page's scripts (non-dot, asset-allowlisted, realpath-confined, blocklist applied, GET/HEAD only), which is why minting is refused for an HTML file in a hidden directory or directly in a broad root (`/`, home, the temp dir, the cases and user-space roots; `isBroadHtmlViewRoot`). The preview renders it by default; the **Page** pill (`htmlRendered`, per-device like `mdRendered`) and any refused mint fall through to the source view with its Edit button. Tests: `test/routes/html-view-routes.test.ts`, the html-view block in `test/webview-auth-exemption.test.ts`, `test/webview-capability-revocation.test.ts`. 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. **Media is single-sourced across the two preview paths.** `VIDEO_ATTACHMENT_EXTENSIONS` / `AUDIO_ATTACHMENT_EXTENSIONS` live in `attachment-registry.ts` and are imported by `file-content`'s media classification, so a clip plays identically whether it is in the workspace or reached by id from outside it. They diverged first: the workspace path had its own inline sets and the registry allowlist had no media at all, so a video an agent wrote to `/tmp` was refused as an unsupported type while the same file inside the repo played. ⚠️ Three things have to line up for a player rather than a dead frame: the extension in the allowlist, a **real MIME entry** in `MIME_TYPES` (a `