From 67c5136c7ea1c2a7c1088797d8db2eeeef1c05cf Mon Sep 17 00:00:00 2001 From: _Kerman Date: Mon, 7 Sep 2026 18:04:07 +0800 Subject: [PATCH] fix(api): refuse non-regular files before open and bound full streams - A media-named FIFO or device node is refused before the open would block on it (pre-open isFile check); POSIX FIFO regression test added. - The full-body stream is bounded to the stat'ed size so concurrent appends cannot exceed the declared Content-Length. - Error responses carry no body for HEAD requests. - Docs no longer overclaim the replacement race: the stat-identity comparison narrows (does not fully close) the replacement window, and the earlier realpath-to-stat window is acknowledged. Pairs re-recorded. --- ...ession-prose-local-media-display.i18n.yaml | 4 +- ...09-07-session-prose-local-media-display.md | 2 +- ...07-session-prose-local-media-display.zh.md | 2 +- .../api/session-controller/README.i18n.yaml | 4 +- packages/api/session-controller/README.md | 2 +- packages/api/session-controller/README.zh.md | 2 +- .../src/media-references.ts | 51 ++++++++++++------- .../tests/media-references.host.spec.ts | 24 +++++++++ 8 files changed, 66 insertions(+), 25 deletions(-) diff --git a/.agents/notes/implemented/feature/2026-09-07-session-prose-local-media-display.i18n.yaml b/.agents/notes/implemented/feature/2026-09-07-session-prose-local-media-display.i18n.yaml index 04132eddc5..b3ccacf68f 100644 --- a/.agents/notes/implemented/feature/2026-09-07-session-prose-local-media-display.i18n.yaml +++ b/.agents/notes/implemented/feature/2026-09-07-session-prose-local-media-display.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/feature/2026-09-07-session-prose-local-media-display.md -2026-09-07-session-prose-local-media-display.md: fb277ebde175fbf3b331c65d5413400287b2443c -2026-09-07-session-prose-local-media-display.zh.md: 6e68482db993d9667348560cd522e686ddb657d9 +2026-09-07-session-prose-local-media-display.md: bd898e6ced09dd55a6d6eeaee7e73b9187cded9d +2026-09-07-session-prose-local-media-display.zh.md: 80ebe968f86022f3261ac5d2929b71dc68dc7f67 diff --git a/.agents/notes/implemented/feature/2026-09-07-session-prose-local-media-display.md b/.agents/notes/implemented/feature/2026-09-07-session-prose-local-media-display.md index fb277ebde1..bd898e6ced 100644 --- a/.agents/notes/implemented/feature/2026-09-07-session-prose-local-media-display.md +++ b/.agents/notes/implemented/feature/2026-09-07-session-prose-local-media-display.md @@ -14,7 +14,7 @@ Local media paths in session prose render through one same-origin file route, wi - **Renderer seam (`ui-primitives`)**: `MarkdownText` gained a `MarkdownPathImages` vocabulary (`pathImages` prop) with the same settled-only gate as `fileMentions`: while a message streams, frozen cached blocks never bake in a vocabulary handler; the settled pass rewrites image destinations that fail the remote-URL allowlist, and a rewritten destination is emitted only when it is an absolute `http(s)`/`blob`/`data` URL. Without a vocabulary the renderer output is byte-identical to before. - **Chat wiring (`ui-chat`)**: `AssistantMarkdown` supplies a page-stable vocabulary (`localPathMediaUrl`, module-private in the same file) mapping absolute POSIX paths to same-origin `/api/file?path=…` GETs; non-HTTP transports (Electron `file://`) and relative/protocol-relative destinations stay inert. Windows-style host paths (`C:\...`) are deliberately inert on the client side and are recorded as a limitation in Consequences. -- **Host route (`session-controller`)**: `SessionMediaReferences`, a plugin contribution registered beside `SessionFileReferences`, mounts `GET|HEAD /api/file` on the shared authenticated `connection.fetch` channel (same trust fence and browser authentication as `/api` RPC). Per request it fail-closes: the path must be absolute, its `realpath` must lie inside a registered workspace root (path-component containment, including a filesystem-root workspace), the file must be regular, and its MIME type must belong to the served categories image/video/audio (resolved by the `mime-types` package, excluding `image/svg+xml`); media bytes are never sniffed on this route. Validation and reading bind to the same opened file: the pre-open and opened stat identities are compared, so a replacement or re-linking race is refused instead of followed. Responses stream with HTTP range support parsed by the `range-parser` package: single-range `bytes` requests answer 206, malformed/unknown-unit/multi-range headers are ignored for a full 200 body, HEAD never opens a file stream, and an aborted client destroys the stream. Responses carry `private, no-store` and `nosniff`. The contribution activates only where `connection` and `workspaceRegistry` are composed (pending-until-composed, like the package's other optional contributions). +- **Host route (`session-controller`)**: `SessionMediaReferences`, a plugin contribution registered beside `SessionFileReferences`, mounts `GET|HEAD /api/file` on the shared authenticated `connection.fetch` channel (same trust fence and browser authentication as `/api` RPC). Per request it fail-closes: the path must be absolute, its `realpath` must lie inside a registered workspace root (path-component containment, including a filesystem-root workspace), the file must be regular, and its MIME type must belong to the served categories image/video/audio (resolved by the `mime-types` package, excluding `image/svg+xml`); media bytes are never sniffed on this route. Regular-file checks run before opening (a named pipe or device node is refused instead of blocking the open); reading binds to the opened file, and a stat-identity comparison with the pre-open stat narrows, but does not fully close, a concurrent replacement window. Responses stream with HTTP range support parsed by the `range-parser` package: single-range `bytes` requests answer 206, malformed/unknown-unit/multi-range headers are ignored for a full 200 body, HEAD never opens a file stream, and an aborted client destroys the stream. Responses carry `private, no-store` and `nosniff`. The contribution activates only where `connection` and `workspaceRegistry` are composed (pending-until-composed, like the package's other optional contributions). The route is presentational and stateless: it never writes, follows no redirects, and returns 400/403/404/415/416 instead of approximating another file-serving behavior. diff --git a/.agents/notes/implemented/feature/2026-09-07-session-prose-local-media-display.zh.md b/.agents/notes/implemented/feature/2026-09-07-session-prose-local-media-display.zh.md index 6e68482db9..80ebe968f8 100644 --- a/.agents/notes/implemented/feature/2026-09-07-session-prose-local-media-display.zh.md +++ b/.agents/notes/implemented/feature/2026-09-07-session-prose-local-media-display.zh.md @@ -14,7 +14,7 @@ Assistant 正文有时用本地文件系统路径引用图片（markdown `![](/U - **渲染缝（`ui-primitives`）**：`MarkdownText` 新增 `MarkdownPathImages` 词表（`pathImages` prop），与 `fileMentions` 同用 settled-only 门：消息流式期间冻结的缓存块绝不烘入词表处理器；落定渲染把未通过远程 URL 白名单的图片目标重写为可展示 URL，且只有结果是绝对 `http(s)`/`blob`/`data` URL 时才发射。不提供词表时渲染输出与之前逐字节一致。 - **聊天接线（`ui-chat`）**：`AssistantMarkdown` 提供页面级稳定的词表（`localPathMediaUrl`，与组件同文件的模块私有函数），把绝对 POSIX 路径映射为同源 `GET /api/file?path=…`；非 HTTP 载体（Electron `file://`）与相对/协议相对目标保持惰性。Windows 风格 Host 路径（`C:\...`）在客户端侧刻意保持惰性，作为局限记录于 Consequences。 -- **Host 路由（`session-controller`）**：`SessionMediaReferences` 是与 `SessionFileReferences` 并列注册的插件贡献，把 `GET|HEAD /api/file` 挂到共享鉴权 `connection.fetch` 通道（与 `/api` RPC 同一 trust fence 与浏览器认证）。每次请求 fail-closed：路径必须绝对、其 `realpath` 必须落在已注册 workspace 根内（按路径组件判定包含，含文件系统根目录作为 workspace 的情况）、文件必须是常规文件、其 MIME 类型（由 `mime-types` 解析）必须属于受服务类别 image/video/audio（排除 `image/svg+xml`）；该路由从不嗅探媒体字节。校验与读取绑定同一个已打开文件：比较打开前后 stat 身份，替换/重链接竞态被拒绝而非跟随。响应由 `range-parser` 解析 Range 后流式输出：单段 `bytes` 请求回 206，畸形/未知单位/多段 Range 被忽略并回完整 200，HEAD 从不打开文件流，客户端中止即销毁流；响应携带 `private, no-store` 与 `nosniff`。该贡献只在 `connection` 与 `workspaceRegistry` 均被组合时激活（pending-until-composed，与包内其它可选贡献一致）。 +- **Host 路由（`session-controller`）**：`SessionMediaReferences` 是与 `SessionFileReferences` 并列注册的插件贡献，把 `GET|HEAD /api/file` 挂到共享鉴权 `connection.fetch` 通道（与 `/api` RPC 同一 trust fence 与浏览器认证）。每次请求 fail-closed：路径必须绝对、其 `realpath` 必须落在已注册 workspace 根内（按路径组件判定包含，含文件系统根目录作为 workspace 的情况）、文件必须是常规文件、其 MIME 类型（由 `mime-types` 解析）必须属于受服务类别 image/video/audio（排除 `image/svg+xml`）；该路由从不嗅探媒体字节。常规文件检查在打开前执行（命名管道与设备节点会被拒绝而不是阻塞打开）；读取绑定已打开文件，并与打开前的 stat 做身份比较，收窄（而非完全消除）并发替换窗口。响应由 `range-parser` 解析 Range 后流式输出：单段 `bytes` 请求回 206，畸形/未知单位/多段 Range 被忽略并回完整 200，HEAD 从不打开文件流，客户端中止即销毁流；响应携带 `private, no-store` 与 `nosniff`。该贡献只在 `connection` 与 `workspaceRegistry` 均被组合时激活（pending-until-composed，与包内其它可选贡献一致）。 该路由纯呈现且无状态：从不写入、不跟随重定向，失败返回 400/403/404/415/416，不近似其它文件服务行为。 diff --git a/packages/api/session-controller/README.i18n.yaml b/packages/api/session-controller/README.i18n.yaml index a66cbc1301..97a859db57 100644 --- a/packages/api/session-controller/README.i18n.yaml +++ b/packages/api/session-controller/README.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write packages/api/session-controller/README.md -README.md: c108687ac712dcc7603d0bfdcac11bdc71826357 -README.zh.md: 336734ebf4eb677f853a905443e035b8c8fb909a +README.md: c7ecb98b4f9a791c65b8915e57e710e4038a90de +README.zh.md: 74f70f167454a0ac76b18cc272c0133b427ca223 diff --git a/packages/api/session-controller/README.md b/packages/api/session-controller/README.md index c108687ac7..c7ecb98b4f 100644 --- a/packages/api/session-controller/README.md +++ b/packages/api/session-controller/README.md @@ -36,7 +36,7 @@ The Session object also carries local submission echoes: `session.beginSubmissio ## Session media references -`SessionMediaReferences` mounts `GET|HEAD /api/file?path=` on the authenticated `connection.fetch` channel when both `connection` and `workspaceRegistry` are composed, so conversation prose can display workspace-contained local media (images today; video/audio later) in the browser. Per request the route fail-closes: the path must be absolute, its `realpath` must lie inside a registered workspace root (path-component containment, including a filesystem-root workspace), the file must be regular, and its `mime-types` category must be image/video/audio (excluding `image/svg+xml`); media bytes are never sniffed. Validation and reading bind to the same opened file, so a replacement or re-linking race is refused. Single-range `bytes` requests stream a 206 slice parsed by `range-parser`; malformed, unknown-unit, and multi-range headers are ignored for a full 200 body; HEAD never opens a file stream; an aborted client destroys it. Responses carry `private, no-store` and `nosniff`; failures answer 400/403/404/415/416. The client-side rewrite vocabulary lives in `ui-chat` (`AssistantMarkdown`); this package owns only the serving contract. +`SessionMediaReferences` mounts `GET|HEAD /api/file?path=` on the authenticated `connection.fetch` channel when both `connection` and `workspaceRegistry` are composed, so conversation prose can display workspace-contained local media (images today; video/audio later) in the browser. Per request the route fail-closes: the path must be absolute, its `realpath` must lie inside a registered workspace root (path-component containment, including a filesystem-root workspace), the file must be regular, and its `mime-types` category must be image/video/audio (excluding `image/svg+xml`); media bytes are never sniffed. Regular-file checks run before opening, so named pipes and device nodes are refused instead of blocking the open; reading binds to the opened file, and a stat-identity comparison with the pre-open stat narrows (does not fully close) a concurrent replacement window. Single-range `bytes` requests stream a 206 slice parsed by `range-parser`; malformed, unknown-unit, and multi-range headers are ignored for a full 200 body; HEAD never opens a file stream; an aborted client destroys it. Responses carry `private, no-store` and `nosniff`; failures answer 400/403/404/415/416. The client-side rewrite vocabulary lives in `ui-chat` (`AssistantMarkdown`); this package owns only the serving contract. ----- diff --git a/packages/api/session-controller/README.zh.md b/packages/api/session-controller/README.zh.md index 336734ebf4..74f70f1674 100644 --- a/packages/api/session-controller/README.zh.md +++ b/packages/api/session-controller/README.zh.md @@ -36,7 +36,7 @@ Session 对象还承载本地提交回显：`session.beginSubmission` 在调用 ## 会话媒体引用 -当 `connection` 与 `workspaceRegistry` 均被组合时，`SessionMediaReferences` 在鉴权 `connection.fetch` 通道上挂载 `GET|HEAD /api/file?path=<绝对路径>`，使会话正文可以引用 workspace 内的本地媒体（图片今天可用；视频/音频后续同一条 URL）。每次请求 fail-closed：路径必须绝对、其 `realpath` 必须落在已注册 workspace 根内（按路径组件判定包含，含文件系统根目录作为 workspace）、文件必须是常规文件、其 `mime-types` 类别必须属于 image/video/audio（排除 `image/svg+xml`）；该路由从不嗅探媒体字节。校验与读取绑定同一个已打开文件，替换/重链接竞态会被拒绝。单段 `bytes` Range 请求由 `range-parser` 解析后流式返回 206 分片；畸形、未知单位与多段 Range 头被忽略并返回完整 200；HEAD 从不打开文件流；客户端中止即销毁流。响应携带 `private, no-store` 与 `nosniff`；失败返回 400/403/404/415/416。客户端侧的重写词表位于 `ui-chat`（`AssistantMarkdown`）；本包只拥有服务契约。 +当 `connection` 与 `workspaceRegistry` 均被组合时，`SessionMediaReferences` 在鉴权 `connection.fetch` 通道上挂载 `GET|HEAD /api/file?path=<绝对路径>`，使会话正文可以引用 workspace 内的本地媒体（图片今天可用；视频/音频后续同一条 URL）。每次请求 fail-closed：路径必须绝对、其 `realpath` 必须落在已注册 workspace 根内（按路径组件判定包含，含文件系统根目录作为 workspace）、文件必须是常规文件、其 `mime-types` 类别必须属于 image/video/audio（排除 `image/svg+xml`）；该路由从不嗅探媒体字节。常规文件检查在打开前执行，命名管道与设备节点会被拒绝而不是阻塞打开；读取绑定已打开文件，并与打开前的 stat 做身份比较，收窄（而非完全消除）并发替换窗口。单段 `bytes` Range 请求由 `range-parser` 解析后流式返回 206 分片；畸形、未知单位与多段 Range 头被忽略并返回完整 200；HEAD 从不打开文件流；客户端中止即销毁流。响应携带 `private, no-store` 与 `nosniff`；失败返回 400/403/404/415/416。客户端侧的重写词表位于 `ui-chat`（`AssistantMarkdown`）；本包只拥有服务契约。 ----- diff --git a/packages/api/session-controller/src/media-references.ts b/packages/api/session-controller/src/media-references.ts index 2da56f758f..136559a51a 100644 --- a/packages/api/session-controller/src/media-references.ts +++ b/packages/api/session-controller/src/media-references.ts @@ -12,10 +12,11 @@ * - its MIME type (resolved by `mime-types`) must belong to the served * categories image/video/audio, excluding `image/svg+xml`; media bytes are * never sniffed here, and a corrupt payload fails in the browser; - * - the file must exist and be a regular file; validation and reading bind to - * the same opened file (a replacement or re-linking race between the - * containment check and the read is detected by comparing the pre-open and - * opened stat identities and refused); + * - the file must exist and be a regular file (checked before opening, so a + * named pipe or device node is refused instead of blocking the open); + * reading binds to the opened file, and comparing its stat identity with + * the pre-open stat narrows (does not fully close) a concurrent + * replacement window; * - multi-range requests are answered with the full 200 body (no Range is * honored) rather than a mislabeled single-segment 206; * - responses are private, uncached, sniff-proof, and stream with HTTP range @@ -58,44 +59,57 @@ async function serveMediaReference( request: Request, roots: readonly { path: string }[], ): Promise { + // Error responses carry a text body for GET debugging; HEAD responses must + // never carry one. + const fail = (status: number, text: string): Response => + new Response(request.method === 'HEAD' ? null : text, { status }) const path = new URL(request.url).searchParams.get('path') - if (path === null || path.length === 0) return new Response('missing path', { status: 400 }) + if (path === null || path.length === 0) return fail(400, 'missing path') if (path.includes('\0') || !isAbsolute(path)) { - return new Response('absolute path required', { status: 400 }) + return fail(400, 'absolute path required') } let canonical: string try { canonical = await realpath(path) } catch { - return new Response('not found', { status: 404 }) + return fail(404, 'not found') } // Containment compares path components; a filesystem root already ends in // the separator and prefixes every absolute path. const contained = roots.some(({ path: root }) => canonical === root || canonical.startsWith(root.endsWith(sep) ? root : root + sep)) - if (!contained) return new Response('outside workspace roots', { status: 403 }) + if (!contained) return fail(403, 'outside workspace roots') const mediaType = mime.lookup(canonical) if (mediaType === false || mediaType === 'image/svg+xml' || !SERVED_MEDIA_TYPE.test(mediaType)) { - return new Response('not an allowlisted media type', { status: 415 }) + return fail(415, 'not an allowlisted media type') } - // The stat identity of the validated path is compared with the identity of - // the file actually opened, so a replacement or re-linking race between the - // containment check and the read is refused instead of followed. + // Open only after proving the validated path is a regular file: opening a + // FIFO or device node first would block or read unbounded data. let before - let handle try { before = await stat(canonical) + } catch { + /* v8 ignore next 1 -- the path cannot vanish between the realpath above and this stat except in a concurrent deletion race */ + return fail(404, 'not found') + } + if (!before.isFile()) return fail(403, 'not a regular file') + let handle + try { handle = await open(canonical, 'r') } catch { - return new Response('not found', { status: 404 }) + return fail(404, 'not found') } let streamed = false try { const after = await handle.stat() - if (!after.isFile()) return new Response('not a regular file', { status: 403 }) + /* v8 ignore next 1 -- only a concurrent replacement could make the opened file non-regular after the pre-open check */ + if (!after.isFile()) return fail(403, 'not a regular file') + // The opened handle's identity must match the validated stat; this only + // narrows a replacement race between the stat and the open, it does not + // eliminate the earlier realpath-to-stat window. /* v8 ignore next 2 -- the replacement race cannot be produced deterministically; this arm refuses it when it happens */ if (after.dev !== before.dev || after.ino !== before.ino) { - return new Response('file changed during validation', { status: 403 }) + return fail(403, 'file changed during validation') } const total = after.size const rangeHeader = request.headers.get('range')?.trim() ?? null @@ -129,7 +143,10 @@ async function serveMediaReference( return new Response(null, { status, headers }) } streamed = true - const source = addAbortSignal(request.signal, handle.createReadStream(slice)) + // Bound the full-body stream to the stat'ed size so concurrent appends + // cannot push the payload past the declared Content-Length. + const stream = handle.createReadStream(slice ?? { start: 0, end: total - 1 }) + const source = addAbortSignal(request.signal, stream) return new Response(Readable.toWeb(source) as ReadableStream, { status, headers }) } finally { if (!streamed) await handle.close() diff --git a/packages/api/session-controller/tests/media-references.host.spec.ts b/packages/api/session-controller/tests/media-references.host.spec.ts index b90f2e3e5e..da4a22d42d 100644 --- a/packages/api/session-controller/tests/media-references.host.spec.ts +++ b/packages/api/session-controller/tests/media-references.host.spec.ts @@ -212,6 +212,13 @@ describe('SessionMediaReferences /api/file', () => { expect(ranged.body).toBeNull() }) + it('answers HEAD errors without a body', async () => { + const route = await mount() + const response = await route.raw('http://127.0.0.1:3080/api/file', { method: 'HEAD' }) + expect(response.status).toBe(400) + expect(response.body).toBeNull() + }) + it('denies malformed, missing, empty, and uncontained requests', async () => { const route = await mount() expect((await route.raw('http://127.0.0.1:3080/api/file')).status).toBe(400) @@ -234,6 +241,23 @@ describe('SessionMediaReferences /api/file', () => { } }) + // A FIFO would block a plain open; the pre-open regular-file check refuses + // it first. POSIX-only because Windows has no named-pipe path construction + // here. + it.skipIf(process.platform === 'win32')( + 'refuses a FIFO named as media without blocking on the open', + async () => { + const route = await mount() + const pipe = join(root, 'stream.png') + const { execFile } = await import('node:child_process') + const { promisify } = await import('node:util') + await promisify(execFile)('mkfifo', [pipe]) + const response = await route.call(pipe) + expect(response.status).toBe(403) + expect(await response.text()).toBe('not a regular file') + }, + ) + it('answers 404 when an existing file cannot be opened for reading', async () => { // A permission-less file passes realpath/stat but fails the open; the // route treats the unreadable file like a missing one. chmod has no