diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index c15093b..2234923 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -48,6 +48,10 @@ export type { GoalRunOptions, SessionConfig, SessionRunOptions } from "./session // (stripConversationMarkers) is exported from the markers module via the omnimessage barrel. export { sanitizeTitle } from "./internal/session-title.js"; export type { SessionTitleResult } from "./internal/session-title.js"; +// Session assembly likewise stays internal; only the attachment-line placement rule is +// re-exported, because the server appends `[attached file: …]` lines for the composer's +// uploads and both producers must place them identically (see the markers module). +export { appendAttachmentLines } from "./internal/session-support.js"; export { Agent, createAgent } from "./agent.js"; export type { CreateAgentOptions, CreateSessionOptions, ResumeSessionOptions } from "./agent.js"; diff --git a/packages/core/src/internal/session-support.ts b/packages/core/src/internal/session-support.ts index 4fd29f8..4b6b62b 100644 --- a/packages/core/src/internal/session-support.ts +++ b/packages/core/src/internal/session-support.ts @@ -12,7 +12,7 @@ import { formatLocalDate } from "./dates.js"; import { sessionShell } from "../environment/tools/command/shell.js"; import type { SessionEnvironmentValues } from "../state/agent-state.js"; import { workspacesDir } from "../state/index.js"; -import { userText } from "../omnimessage/index.js"; +import { attachedImageLine, isWholeOriginBlock, userText } from "../omnimessage/index.js"; import type { OmniMessage } from "../omnimessage/index.js"; /** Session runtime environment fields: the placeholder substitution values for `assembleSystemPrompt`; producer and consumer share the same type. */ @@ -135,7 +135,7 @@ export async function imagesToScratchpadPaths( if (!isImage(msg)) continue; const url = (msg.payload as { image_url?: string }).image_url ?? ""; if (/^https?:\/\//i.test(url)) { - lines.push(`[attached image: ${url}]`); + lines.push(attachedImageLine(url)); continue; } const match = /^data:([^;,]+);base64,(.+)$/s.exec(url); @@ -158,18 +158,44 @@ export async function imagesToScratchpadPaths( if ((err as NodeJS.ErrnoException).code !== "EEXIST") throw err; } } - lines.push(`[attached image: ${file}]`); + lines.push(attachedImageLine(file)); } - // Concatenation: the path lines are appended after the last user text message; if the input is images only, add a plain path-only text message. - const rest = input.filter((m) => !isImage(m)); + return appendAttachmentLines( + input.filter((m) => !isImage(m)), + lines, + ); +} + +/** + * Concatenation rule shared by every attachment-line producer (see the markers module's + * attachment-lines.ts): the lines are appended as one block after the **last user text + * message**; if the input carries no such message (attachments only), they become a plain + * line-only text message of their own. + * + * "User text" here excludes a message that is entirely a whole-message origin block — + * `[handoff_from]` / `[model_switch_from]` (isWholeOriginBlock). Those parsers only recognize + * the block when it IS the whole message, so appending to one would turn a one-line banner + * back into a raw marker in a user bubble. The Web composer reaches exactly that shape when a + * message carries attachments, no text, and a staged handoff: its only text message is the + * origin block. Such input therefore falls through to the line-only message, leaving the block + * intact. Prefix blocks (`[use_skills]`, `[scheduled_task]`) are parsed at index 0 and keep + * their own body, so they still take the lines as usual. + * + * Exported from the package barrel because the server writes `[attached file: …]` lines for + * the composer's uploads and must place them exactly the same way — one rule, so a message + * carrying both kinds of attachment still reads as a single trailing block. + * Returns `input` unchanged when there are no lines to append. + */ +export function appendAttachmentLines(input: OmniMessage[], lines: string[]): OmniMessage[] { + if (lines.length === 0) return input; const suffix = lines.join("\n"); - const lastTextIdx = rest.findLastIndex((m) => { - const p = m.payload as { type?: string; role?: string }; - return p.type === "text" && p.role === "user"; + const lastTextIdx = input.findLastIndex((m) => { + const p = m.payload as { type?: string; role?: string; text?: string }; + return p.type === "text" && p.role === "user" && !isWholeOriginBlock(p.text ?? ""); }); - if (lastTextIdx === -1) return [...rest, userText(suffix)]; - return rest.map((m, i) => { + if (lastTextIdx === -1) return [...input, userText(suffix)]; + return input.map((m, i) => { if (i !== lastTextIdx) return m; const p = m.payload as { type: string; role: string; text: string }; return { ...m, payload: { ...p, text: `${p.text}\n\n${suffix}` } } as OmniMessage; diff --git a/packages/core/src/omnimessage/markers/attachment-lines.ts b/packages/core/src/omnimessage/markers/attachment-lines.ts new file mode 100644 index 0000000..683a9fc --- /dev/null +++ b/packages/core/src/omnimessage/markers/attachment-lines.ts @@ -0,0 +1,51 @@ +/** + * `[attached image: …]` / `[attached file: …]` — the one-line markers that hand a file on + * disk to the model. + * + * Unlike this module's other markers these are not `[tag]…[/tag]` blocks and they are + * **appended after** the user's own text rather than prefixed to it. That placement is + * load-bearing: the origin blocks and `[use_skills]` are all parsed at index 0 of the + * message, so a second leading block would break that chain — an attachment is a trailing + * footnote to the message, not a new frame around it. + * + * Two producers write them, for the same reason (the bytes cannot travel in the message + * itself, so the message carries their path instead): + * - `[attached image: ]` — core, when the session model has no image input: the + * input images are written to the session scratchpad and the model reads them by path via + * describe_image (see internal/session-support.ts); + * - `[attached file: ]` — the server, for the composer's file attachments: the upload + * lands in the session scratchpad and the model opens it with its ordinary file tools + * (see services/task-attachments.ts). + * + * One consumer reads them: the Web App's message renderer, which pulls the lines back out of + * the body text and shows an image / an attachment banner instead (see lib/attachments.ts). + * Producer and parser share the spellings below so the two can never drift apart. + */ + +/** Line prefix of an image attachment; exported so a parser can cheaply pre-test a whole body text. */ +export const ATTACHED_IMAGE_PREFIX = "[attached image: "; +/** Line prefix of a file attachment (same purpose as ATTACHED_IMAGE_PREFIX). */ +export const ATTACHED_FILE_PREFIX = "[attached file: "; + +/** The line naming one input image by absolute path (image-less model) or by http(s) URL (referenced as-is). */ +export function attachedImageLine(target: string): string { + return `${ATTACHED_IMAGE_PREFIX}${target}]`; +} + +/** The line naming one uploaded file by absolute path. */ +export function attachedFileLine(filePath: string): string { + return `${ATTACHED_FILE_PREFIX}${filePath}]`; +} + +const ATTACHED_IMAGE_RE = /^\[attached image: (.+)\]$/; +const ATTACHED_FILE_RE = /^\[attached file: (.+)\]$/; + +/** Inverse of `attachedImageLine`: the target of a whole line that is one image attachment, else null. */ +export function matchAttachedImageLine(line: string): string | null { + return ATTACHED_IMAGE_RE.exec(line)?.[1] ?? null; +} + +/** Inverse of `attachedFileLine`: the path of a whole line that is one file attachment, else null. */ +export function matchAttachedFileLine(line: string): string | null { + return ATTACHED_FILE_RE.exec(line)?.[1] ?? null; +} diff --git a/packages/core/src/omnimessage/markers/index.ts b/packages/core/src/omnimessage/markers/index.ts index 5befd31..73eccd8 100644 --- a/packages/core/src/omnimessage/markers/index.ts +++ b/packages/core/src/omnimessage/markers/index.ts @@ -15,6 +15,11 @@ * - **goal** (`goal-block.ts`): `[goal]`, the goal-mode round protocol block prefixed to * each round's input by the Session's goal loop (line-anchored close — see the module). * + * `attachment-lines.ts` is the one exception to the block form: `[attached image: …]` / + * `[attached file: …]` are single lines **appended after** a user message, naming a file the + * model must open by path. They live here for the same reason the blocks do — core, the + * server and the Web renderer all have to spell them identically. + * * `block.ts` owns the spelling itself — the canonical square form for producers and the * dual-form (square + legacy angle) matching every parser applies, because markers persist in * Traces and in each agent's stored compaction prompt. `tags.ts` owns the tag list. @@ -24,6 +29,7 @@ */ export * from "./block.js"; export * from "./tags.js"; +export * from "./attachment-lines.js"; export * from "./engine-blocks.js"; export * from "./origin-blocks.js"; export * from "./steering.js"; diff --git a/packages/core/src/omnimessage/markers/origin-blocks.ts b/packages/core/src/omnimessage/markers/origin-blocks.ts index 1f5a96f..fff5c4a 100644 --- a/packages/core/src/omnimessage/markers/origin-blocks.ts +++ b/packages/core/src/omnimessage/markers/origin-blocks.ts @@ -281,3 +281,25 @@ export function parseModelSwitchMessage(text: string): ModelSwitchOrigin | null } return origin.sessionId ? origin : null; } + +// --------------------------------------------------------------------------- +// Shared predicate over the whole-message origin blocks +// --------------------------------------------------------------------------- + +/** + * True when `text` is **entirely** one origin block whose parser demands a whole-message match: + * `[handoff_from]` and `[model_switch_from]`, both of which compare the match length against the + * trimmed message. Anything appended after such a block — not just prefixed to it — makes it + * unparseable, and the raw marker then renders verbatim in a user bubble. + * + * Exported for the producers that append to an existing message (see core's + * `appendAttachmentLines`): a message of this shape is a machine-written frame, not user text, + * and must be left alone. Deliberately implemented by running the parsers rather than + * re-testing their patterns, so the predicate cannot drift from what they accept. + * + * `[use_skills]` and `[scheduled_task]` are **not** included: they are prefix blocks followed by + * the message's own body and are parsed at index 0 only, so appending after that body is safe. + */ +export function isWholeOriginBlock(text: string): boolean { + return parseHandoffMessage(text) !== null || parseModelSwitchMessage(text) !== null; +} diff --git a/packages/core/test/input-images.test.ts b/packages/core/test/input-images.test.ts index 2220104..0f13d53 100644 --- a/packages/core/test/input-images.test.ts +++ b/packages/core/test/input-images.test.ts @@ -3,13 +3,25 @@ * images -- data URL images are written to the session scratchpad and their paths appended to the * user text; http(s) URLs are referenced as-is; image messages are removed from the input; * image-free input is returned unchanged; images that fail to parse are replaced with an explanatory line. + * + * Plus appendAttachmentLines, the placement rule both attachment-line producers share (the + * images above and the server's `[attached file: …]` uploads). */ import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { mkdtemp, readFile, readdir, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; -import { imagesToScratchpadPaths } from "../src/internal/session-support.js"; -import { imageUrlMessage, userText } from "../src/omnimessage/index.js"; +import { appendAttachmentLines, imagesToScratchpadPaths } from "../src/internal/session-support.js"; +import { + buildHandoffMessage, + buildModelSwitchMessage, + buildScheduledMessage, + buildSkillsMessage, + imageUrlMessage, + parseHandoffMessage, + parseModelSwitchMessage, + userText, +} from "../src/omnimessage/index.js"; import type { TextPayload } from "../src/omnimessage/index.js"; const PNG_1X1 = Buffer.from( @@ -80,3 +92,71 @@ describe("imagesToScratchpadPaths", () => { expect(p.text).toContain("could not be saved"); }); }); + +/** Text of the message at `index` of an appendAttachmentLines result. */ +const textAt = (out: ReturnType, index: number): string => + (out[index]!.payload as TextPayload).text; + +describe("appendAttachmentLines placement", () => { + const LINE = "[attached file: /d/scratchpad/s1/report.pdf]"; + + it("appends to the last user text message", () => { + const out = appendAttachmentLines([userText("first"), userText("second")], [LINE]); + expect(out).toHaveLength(2); + expect(textAt(out, 0)).toBe("first"); + expect(textAt(out, 1)).toBe(`second\n\n${LINE}`); + }); + + it("no user text at all: the lines become a message of their own", () => { + const out = appendAttachmentLines([], [LINE]); + expect(out).toHaveLength(1); + expect(textAt(out, 0)).toBe(LINE); + }); + + it("a whole-message origin block is left alone; the lines get their own message", () => { + // The composer's files-only-plus-staged-handoff shape: the only text message is the origin + // block, and both of these parsers require the block to be the WHOLE message — appending to + // it would render the raw marker in a user bubble instead of a one-line banner. + for (const block of [ + buildHandoffMessage({ agentId: "alpha", agentName: "Alpha", sessionId: "s0" }), + buildModelSwitchMessage({ sessionId: "s0", prevModelId: "m0" }), + ]) { + const out = appendAttachmentLines([userText(block)], [LINE]); + expect(out).toHaveLength(2); + expect(textAt(out, 0)).toBe(block); + expect(textAt(out, 1)).toBe(LINE); + } + // …and the blocks still parse afterwards, which is the property that actually matters. + const handoff = appendAttachmentLines( + [userText(buildHandoffMessage({ agentId: "alpha", sessionId: "s0" }))], + [LINE], + ); + expect(parseHandoffMessage(textAt(handoff, 0))?.agentId).toBe("alpha"); + const switched = appendAttachmentLines( + [userText(buildModelSwitchMessage({ sessionId: "s0" }))], + [LINE], + ); + expect(parseModelSwitchMessage(textAt(switched, 0))?.sessionId).toBe("s0"); + }); + + it("prefix blocks still take the lines after their body ([use_skills] / [scheduled_task])", () => { + // The other direction: these two are parsed at index 0 and keep the body that follows, so a + // trailing line is an ordinary footnote to the message and must NOT start a new one. + for (const message of [ + buildSkillsMessage(["web-design"], "fix the layout"), + buildScheduledMessage("nightly", "2026-07-29T02:00:00Z", "run the report"), + ]) { + const out = appendAttachmentLines([userText(message)], [LINE]); + expect(out).toHaveLength(1); + expect(textAt(out, 0)).toBe(`${message}\n\n${LINE}`); + } + }); + + it("an earlier ordinary message wins over a trailing origin block", () => { + const block = buildHandoffMessage({ agentId: "alpha" }); + const out = appendAttachmentLines([userText("hello"), userText(block)], [LINE]); + expect(out).toHaveLength(2); + expect(textAt(out, 0)).toBe(`hello\n\n${LINE}`); + expect(textAt(out, 1)).toBe(block); + }); +}); diff --git a/packages/docs/content/server-api.en.md b/packages/docs/content/server-api.en.md index ee5f9fd..908e2c5 100644 --- a/packages/docs/content/server-api.en.md +++ b/packages/docs/content/server-api.en.md @@ -10,7 +10,7 @@ The PenguinHarness server exposes a same-origin HTTP API used by the bundled Web - Stack: Hono + @hono/node-server, requires Node >= 24; - Storage: SQLite (built-in `node:sqlite`, WAL mode) holds only indexes and aggregates — users, auth sessions, Project authorization, Agent / Session indexes, usage, UI preferences, error records, and Schedule state; all Agent, Trace, and Workspace data stays as files under `~/.penguin/data`, shared with the CLI / SDK — see the [Configuration Reference](/configuration); - Binding: defaults to `127.0.0.1:7364`, adjustable via the `PORT` / `HOST` environment variables; -- Request bodies: writes accept JSON only (Content-Type check, one of the CSRF defenses), capped at 20MB; +- Request bodies: writes accept JSON only (Content-Type check, one of the CSRF defenses), capped at 20MB — counted as the body is read, so a request that declares no length (chunked) is capped just the same; - Errors share a single shape: ```text @@ -161,7 +161,7 @@ The paths below omit the `/api/sessions/:sessionId` prefix. For the storage mode | DELETE | / | Delete the Session (along with its Traces and scratch files) | | GET | /messages | Full OmniMessage history; while a Task runs the response also carries `live` (the in-progress stream tail, see below) | | GET | /stream | SSE event stream (next section) | -| POST | /tasks | Start a Task: `{input: TaskInputPart[], thinkingLevel?, queueIfBusy?}` → 202. With `queueIfBusy`, a busy session holds the input as a follow-up (`queued: true`) and auto-starts it as an ordinary next task once idle; `task_state` events report the queued count | +| POST | /tasks | Start a Task: `{input: TaskInputPart[], thinkingLevel?, queueIfBusy?}` → 202. With `queueIfBusy`, a busy session holds the input as a follow-up (`queued: true`) and auto-starts it as an ordinary next task once idle; `task_state` events report the queued count. `file` input parts are written to the Session scratchpad and handed to the model as `[attached file: ]` lines (see the request body below) | | POST | /steer | Mid-run steering: `{text}` queues a message for the running Task (delivered between turns as a standalone `[user_steering]` user message) → 202; 409 `not_running` when no Task is in progress | | POST | /approvals/:toolCallId | Approval decision: `{decision}` is `allow` or `deny` → 204 | | POST | /abort | Interrupt the current Task: 202 when triggered, 204 when idle | @@ -175,7 +175,7 @@ The paths below omit the `/api/sessions/:sessionId` prefix. For the storage mode | GET | /traces | List this Session's Trace files | | GET | /traces/:index | Read Trace events (paginated) | | GET | /traces/:index/analysis | Trace performance analysis | -| GET | /scratchpad/:fileName | Read a session scratch file (e.g. input images) | +| GET | /scratchpad/:fileName | Read a session scratch file (e.g. input images, file attachments) | General conventions: Sessions the user cannot access always return 404 — their existence is never leaked; only one Task or compaction runs per Session at a time, and conflicts return 409 (`task_in_progress` / `compacting`). @@ -209,6 +209,8 @@ Workspace files may be Agent-generated, so `GET /files/content` treats them as u | `preview=1` | the real type (`text/html`, `image/svg+xml`, …) | `inline` | `sandbox allow-scripts allow-popups allow-modals allow-forms`, sent only for `.html` / `.htm` / `.svg` | | `download=1` | the real type | `attachment` | — | +`GET /scratchpad/:fileName` serves the same kind of untrusted bytes (uploads and Agent-written temp files) and is locked down the same way, without the flags: `nosniff` always, a fixed allowlist of five inert image types (`.png` / `.jpg` / `.jpeg` / `.gif` / `.webp`) served inline for the conversation's `` tags, and everything else `application/octet-stream` with `Content-Disposition: attachment` — so nothing that isn't one of those images can render as a document on the App's origin. + The filename always rides along as `filename*=UTF-8''` with percent-encoding. `preview=1` is where the preview redirect falls back when no separate preview origin is available: the document keeps its real type and does render and run, but the sandbox deliberately omits `allow-same-origin`, so it lands in an opaque origin and can reach neither this origin's cookies nor the API. That isolation is also why `localStorage`, `document.cookie` and third-party embeds do not work there. ### Preview on a separate origin @@ -241,7 +243,15 @@ interface TaskCreateRequest { } type TaskInputPart = | { type: "text"; text: string } - | { type: "image_url"; imageUrl: string }; // pasted images arrive as data URLs + | { type: "image_url"; imageUrl: string } // pasted images arrive as data URLs + // File attachment: base64 data: URL, ≤10MB each (413 file_too_large beyond that), at most 20 + // per request and 12MB of decoded bytes in total (413 too_many_files / payload_too_large; + // all three are checked before anything is written). The server writes it into the Session + // scratchpad and appends an `[attached file: ]` line to the message text — the model + // opens the file by path. `fileName` carries no path separators; on disk it keeps its own + // words (`报告 2026.pdf` → `报告-2026.pdf`: non-ASCII survives, shell-hostile ASCII becomes + // `-`), so a name is readable in the message and safe to paste into a command. + | { type: "file"; fileName: string; dataUrl: string }; // POST /api/sessions/:sessionId/approvals/:toolCallId interface ApprovalDecisionRequest { diff --git a/packages/docs/content/server-api.zh.md b/packages/docs/content/server-api.zh.md index 49813ca..7ff12a2 100644 --- a/packages/docs/content/server-api.zh.md +++ b/packages/docs/content/server-api.zh.md @@ -10,7 +10,7 @@ PenguinHarness Server 提供一套同源 HTTP API，自带的 Web App 与其他 - 技术栈：Hono + @hono/node-server，要求 Node >= 24； - 存储：SQLite（内置 `node:sqlite`，WAL 模式）仅存放索引与聚合数据——用户、登录会话、Project 授权、Agent / Session 索引、用量、UI 偏好、错误记录与 Schedule 状态；Agent、Trace 与 Workspace 数据全部以文件形式存放在 `~/.penguin/data` 下，与 CLI / SDK 共享，见[配置参考](/configuration)； - 监听：默认 `127.0.0.1:7364`，可用环境变量 `PORT` / `HOST` 调整； -- 请求体：写请求仅接受 JSON（Content-Type 校验，CSRF 防线之一），上限 20MB； +- 请求体：写请求仅接受 JSON（Content-Type 校验，CSRF 防线之一），上限 20MB —— 按读取到的字节数统计，未声明长度（分块传输）的请求同样受限； - 错误响应统一为： ```text @@ -161,7 +161,7 @@ Trace 下载对任意成员开放；导入仅限 owner（同 Agent 快照导入 | DELETE | / | 删除 Session（连同 Trace 与暂存文件） | | GET | /messages | 完整 OmniMessage 历史；Task 运行期间响应额外携带 `live`（进行中的流式尾部，见下） | | GET | /stream | SSE 事件流（见下节） | -| POST | /tasks | 发起 Task：`{input: TaskInputPart[], thinkingLevel?, queueIfBusy?}` → 202。带 `queueIfBusy` 时，运行中的 Session 会把输入暂存为跟进消息（`queued: true`），空闲后按序自动作为普通 Task 发出；`task_state` 事件携带排队数 | +| POST | /tasks | 发起 Task：`{input: TaskInputPart[], thinkingLevel?, queueIfBusy?}` → 202。带 `queueIfBusy` 时，运行中的 Session 会把输入暂存为跟进消息（`queued: true`），空闲后按序自动作为普通 Task 发出；`task_state` 事件携带排队数。`file` 输入部分会写入该 Session 的 scratchpad，并以 `[attached file: ]` 行交给模型（见下方请求体） | | POST | /steer | 运行中插话：`{text}` 为运行中的 Task 排队一条消息（作为独立的 `[user_steering]` 用户消息随下一轮送达）→ 202；无 Task 运行返回 409 `not_running` | | POST | /approvals/:toolCallId | 审批决定：`{decision}` 取 `allow` 或 `deny` → 204 | | POST | /abort | 中断当前 Task：已触发返回 202，无任务返回 204 | @@ -175,7 +175,7 @@ Trace 下载对任意成员开放；导入仅限 owner（同 Agent 快照导入 | GET | /traces | 本 Session 的 Trace 文件列表 | | GET | /traces/:index | 读取 Trace 事件（分页） | | GET | /traces/:index/analysis | Trace 性能分析结果 | -| GET | /scratchpad/:fileName | 读取会话暂存文件（如输入图片） | +| GET | /scratchpad/:fileName | 读取会话暂存文件（如输入图片、文件附件） | 通用约定：无权访问的 Session 一律返回 404，不泄露其存在性；每个 Session 同时只允许一个 Task 或压缩在运行，冲突时返回 409（`task_in_progress` / `compacting`）。 @@ -208,6 +208,8 @@ Workspace 文件可能由 Agent 生成，`GET /files/content` 一律按不可信 | `preview=1` | 真实类型（`text/html`、`image/svg+xml` 等） | `inline` | `sandbox allow-scripts allow-popups allow-modals allow-forms`，仅对 `.html` / `.htm` / `.svg` 下发 | | `download=1` | 真实类型 | `attachment` | 无 | +`GET /scratchpad/:fileName` 提供的同样是不可信字节（用户上传与 Agent 写下的临时文件），防护口径一致，只是没有那两个开关：始终带 `nosniff`；仅五种可安全内联的图片类型（`.png` / `.jpg` / `.jpeg` / `.gif` / `.webp`）按真实类型内联，供对话里的 `` 使用；其余一律 `application/octet-stream` 并带 `Content-Disposition: attachment` —— 非图片内容无法在 App 所在源上作为文档渲染。 + 文件名始终以 `filename*=UTF-8''` 形式携带（百分号编码）。`preview=1` 是预览跳转在没有独立预览源时的回退目标：文档保留真实类型，可以正常渲染并执行脚本，但沙箱刻意不含 `allow-same-origin`，因此它落在一个不透明源里，既拿不到本源的 Cookie，也调不动 API。这份隔离也正是那里 `localStorage`、`document.cookie` 与第三方 embed 全都不可用的原因。 ### 独立源预览 @@ -239,7 +241,14 @@ interface TaskCreateRequest { } type TaskInputPart = | { type: "text"; text: string } - | { type: "image_url"; imageUrl: string }; // 粘贴图片以 data URL 上送 + | { type: "image_url"; imageUrl: string } // 粘贴图片以 data URL 上送 + // 文件附件：base64 data: URL，单个 ≤10MB（超出返回 413 file_too_large），单次请求最多 20 个、 + // 解码后合计 ≤12MB（超出返回 413 too_many_files / payload_too_large；三项校验都在落盘前完成）。 + // 服务端将其写入该 Session 的 scratchpad，并在消息文本末尾追加一行 + // `[attached file: ]`——模型按路径读取该文件。`fileName` 不得含路径分隔符；落盘时保留 + // 原有词形（`报告 2026.pdf` → `报告-2026.pdf`：非 ASCII 字符原样保留，对 shell 不友好的 + // ASCII 字符替换为 `-`），既便于在消息中辨认，也可安全地拼进命令。 + | { type: "file"; fileName: string; dataUrl: string }; // POST /api/sessions/:sessionId/approvals/:toolCallId interface ApprovalDecisionRequest { diff --git a/packages/docs/content/web-app.en.md b/packages/docs/content/web-app.en.md index 74c8c24..6047436 100644 --- a/packages/docs/content/web-app.en.md +++ b/packages/docs/content/web-app.en.md @@ -47,6 +47,7 @@ There are four approval modes: `allow-all`, `deny-all`, `read-only` (only read-o ### Input and Shortcuts - Enter sends, Shift+Enter inserts a newline, and images can be pasted; +- The "+" menu holds the input add-ons: **image upload**, **file attachment** and goal mode. An attachment can be any type (up to 20 at a time, ≤ 10MB each and 12MB in total; an oversize pick is refused before it is read, so nothing is uploaded to earn the rejection); selected files show as removable chips above the text body in the order they were picked, and a message with attachments and no text is sendable. On send the files are written into the Session's scratchpad — deleted with the Session — and the message gains an `[attached file: ]` line per file, which the conversation renders as an "Attached files" notice: the bytes never enter the conversation, the model opens each file by path with its ordinary file tools; - Typing `/` opens the slash menu: trigger context compaction (`/compact`), hand the conversation over to another Agent (`/agent`), switch the model (`/model`) — both switch commands appear in an active session only, since a draft has nothing to switch and picks its Agent and model up front — or toggle installed Skills — chosen Skills are sent along with the message in a `[use_skills]` block; - While a Task is running the input stays live and the toolbar keeps a single action button: an empty composer shows **Stop**, and typing turns it into **Send**, whose behavior follows the **mid-run send mode** from the toolbar's More-settings popover (a compact extensible settings panel, also available in draft state; the choice is remembered): **Steer** (default) delivers the text mid-run as a `[user_steering]` user message with the next turn, **Queue** holds the whole message server-side as a follow-up and auto-sends it as an ordinary new message when the run finishes (an "N queued" hint shows near the input until then; the queue survives page reloads); - `/agent` and `/model` stage their pick instead of acting on it: the chosen Agent or model becomes a chip above the text body and nothing is sent yet, so you keep typing — Enter/Send is what hands the conversation over (a new chat for that Agent) or forks it onto the chosen model, carrying the text along; with an empty composer a default message is filled in, and the chip's × cancels. Both chips are cached with the draft, so they survive a reload or a trip to another conversation together with the text. A model fork additionally waits for the Session to be idle — it continues from the Session's Trace, which a running turn or a compaction is still writing — and a line above the composer says so while it waits; diff --git a/packages/docs/content/web-app.zh.md b/packages/docs/content/web-app.zh.md index 6ed93e6..4434621 100644 --- a/packages/docs/content/web-app.zh.md +++ b/packages/docs/content/web-app.zh.md @@ -47,6 +47,7 @@ penguin web ### 输入与快捷操作 - Enter 发送，Shift+Enter 换行，支持粘贴图片； +- 「+」菜单收纳输入附加项：**上传图片**、**上传文件**与目标模式。附件不限类型（一次最多 20 个，单个 ≤ 10MB、合计 ≤ 12MB；超限的文件在读取前即被拒绝，不会先上传再报错），已选文件按选择顺序以可移除的小卡片显示在文本框上方；只带附件、没有正文也可发送。发送时文件写入该 Session 的 scratchpad（随 Session 一并删除），消息里每个文件追加一行 `[attached file: ]`，对话中渲染为一条「附加文件」提示：文件内容不进入对话，模型用普通文件工具按路径读取； - 输入 `/` 打开快捷菜单：触发上下文压缩（`/compact`）、把会话交接给其他 Agent（`/agent`）、切换模型（`/model`）——两个切换命令都只在进行中的会话里提供，草稿没有可切换的对话，Agent 与模型本就在草稿页选定——或勾选已安装的 Skill——所选 Skill 会以 `[use_skills]` 块随消息发送； - Task 运行期间输入框保持可用，工具条只保留一个操作按钮：输入框为空时是**停止**，一旦输入内容即变为**发送**，其行为遵循工具条「更多设置」弹出分组中的**运行中发送方式**（一个可扩展的设置面板，草稿态同样可设，选择会被记忆）：**插话**（默认）把文字以 `[user_steering]` 用户消息随下一轮送达运行中的 Agent；**排队** 把整条消息暂存在服务端，本轮结束后自动作为普通新消息发出（期间在输入框附近显示「N 条已排队」提示；队列存放在服务端，刷新页面不丢失）； - `/agent` 与 `/model` 都是暂存而非立即生效：选中的 Agent 或模型只在文本区上方留下一枚 chip，此时不发送任何内容，可以继续输入——按 Enter / 点发送才真正交接（为该 Agent 新开一个对话）或换用所选模型继续本对话，输入的文字随之带走；正文为空时自动填入默认消息，点 chip 上的 × 即可取消。两枚 chip 都随草稿缓存，刷新页面或切到别的会话再回来时与文字一同恢复；其中切换模型还需等待会话空闲——新会话要从本会话的 Trace 接续，而运行中的一轮或压缩仍在写入——等待期间输入框上方会给出说明； diff --git a/packages/server/src/api/types.ts b/packages/server/src/api/types.ts index 0d4bf84..3f184eb 100644 --- a/packages/server/src/api/types.ts +++ b/packages/server/src/api/types.ts @@ -604,11 +604,21 @@ export interface MessagesResponse { // --------------------------------------------------------------------------- /** - * A single Prompt's input parts: text or image (data: / http(s) URL). + * A single Prompt's input parts: text, image (data: / http(s) URL), or an uploaded file. * Docs: /docs/server-api § "Session-Level Endpoints". */ export type TaskInputPart = - { type: "text"; text: string } | { type: "image_url"; imageUrl: string }; + | { type: "text"; text: string } + | { type: "image_url"; imageUrl: string } + /** + * File attachment (the composer's "+" menu): `dataUrl` is a base64 `data:` URL of the + * file's bytes, capped at 10MB each (413 `file_too_large` beyond that; the request as a + * whole still has to fit the global 20MB body limit). The server writes it into the + * Session scratchpad under a sanitized name and appends an `[attached file: ]` line + * to the message text — the bytes never enter the conversation, the model opens the file + * by path. `fileName` is the original name (no path separators, no `..`). + */ + | { type: "file"; fileName: string; dataUrl: string }; export interface TaskCreateRequest { input: TaskInputPart[]; diff --git a/packages/server/src/app.ts b/packages/server/src/app.ts index f688481..72a6ca4 100644 --- a/packages/server/src/app.ts +++ b/packages/server/src/app.ts @@ -11,6 +11,7 @@ import fsp from "node:fs/promises"; import path from "node:path"; import { Hono } from "hono"; import type { Context } from "hono"; +import { bodyLimit } from "hono/body-limit"; import type { DatabaseSync } from "node:sqlite"; import type { ServerConfig } from "./config.js"; import { openDatabase } from "./db/database.js"; @@ -332,13 +333,23 @@ export function createApp(deps: AppDeps): Hono { } // API common defenses: request body size cap (20MB) and write-request Content-Type (one of the CSRF MVP defenses). - app.use("/api/*", async (c, next) => { - const contentLength = Number(c.req.header("content-length") ?? 0); - if (contentLength > MAX_BODY_BYTES) { - throw new HttpError(413, "payload_too_large", "Request body exceeds the 20MB limit."); - } - await next(); - }); + // + // The cap has to be measured, not read: a chunked request carries no `content-length` at all, + // so a header check alone passes a body of any size — the sinks behind it (task input images, + // file attachments, Trace import) then decode whatever arrives. hono's bodyLimit keeps the + // header fast path when the length is declared and otherwise counts bytes off the stream, + // aborting the moment the total crosses the cap. + app.use( + "/api/*", + bodyLimit({ + maxSize: MAX_BODY_BYTES, + // Its default is a bare text/plain 413; throw the App's own error instead so the response + // stays the documented `payload_too_large` body that every client already handles. + onError: () => { + throw new HttpError(413, "payload_too_large", "Request body exceeds the 20MB limit."); + }, + }), + ); app.use("/api/*", jsonOnlyWrites); // Public routes (no login required). diff --git a/packages/server/src/http/routes/sessions.ts b/packages/server/src/http/routes/sessions.ts index 0c0c11a..5e776fa 100644 --- a/packages/server/src/http/routes/sessions.ts +++ b/packages/server/src/http/routes/sessions.ts @@ -46,6 +46,13 @@ import { } from "../validate.js"; import type { AppDeps } from "../../app.js"; import { MAX_UPLOAD_BYTES } from "../../services/workspace-files-service.js"; +import { + assertAttachmentBudget, + attachFilesToInput, + parseAttachmentPart, + removeAttachments, +} from "../../services/task-attachments.js"; +import type { TaskAttachment } from "../../services/task-attachments.js"; /** Max title length for manual renames: looser than the auto-generated 30-char limit, to accommodate users' own organizing conventions. */ const SESSION_TITLE_MAX = 120; @@ -72,13 +79,48 @@ const SESSION_CATEGORIES: readonly SessionCategory[] = [ "archived", ]; -/** Validate Prompt input parts: text or image (data: / http(s) URL). */ -function parseTaskInput(body: Record): OmniMessage[] { +/** + * Resolve a scratchpad file name to an absolute path inside `dir`, or null when it could point + * anywhere else (the caller turns that into the same 404 a missing file gets, so a probe learns + * nothing either way). + * + * A character whitelist is deliberately NOT the guard: an attachment keeps the name the user + * gave it, `报告.pdf` included, so the check is structural instead — no separators, no control + * characters, not a relative marker — and then *confirmed* by resolving the path and requiring + * its parent to be this session's directory exactly. That last step is what actually contains + * the read: it also rejects the shapes a character class misses, such as a Windows + * drive-relative `C:evil.png`. + */ +function resolveScratchpadFile(dir: string, fileName: string): string | null { + if (!fileName || fileName === "." || fileName === "..") return null; + for (const ch of fileName) { + const code = ch.codePointAt(0)!; + if (code < 0x20 || code === 0x7f || ch === "/" || ch === "\\") return null; + } + const resolved = path.resolve(dir, fileName); + return path.dirname(resolved) === path.resolve(dir) ? resolved : null; +} + +/** + * A validated Prompt: the message parts that go straight into the run, plus the file + * attachments, which still have to be written to disk (see attachFilesToInput). Kept apart + * because validation stays synchronous and side-effect free — nothing touches the filesystem + * until the request is known to be good, and goal mode can reject files before any bytes land. + */ +interface ParsedTaskInput { + messages: OmniMessage[]; + attachments: TaskAttachment[]; +} + +/** Validate Prompt input parts: text, image (data: / http(s) URL), or an uploaded file. */ +function parseTaskInput(body: Record): ParsedTaskInput { const input = body.input; if (!Array.isArray(input) || input.length === 0) { throw badRequest("input must be an array with at least one item."); } - return input.map((item, i) => { + const messages: OmniMessage[] = []; + const attachments: TaskAttachment[] = []; + input.forEach((item, i) => { if (item === null || typeof item !== "object" || Array.isArray(item)) { throw badRequest(`input[${i}] must be an object.`); } @@ -87,7 +129,8 @@ function parseTaskInput(body: Record): OmniMessage[] { if (typeof part.text !== "string" || part.text.length === 0) { throw badRequest(`input[${i}].text must be a non-empty string.`); } - return userText(part.text); + messages.push(userText(part.text)); + return; } if (part.type === "image_url") { const url = part.imageUrl; @@ -97,10 +140,21 @@ function parseTaskInput(body: Record): OmniMessage[] { ) { throw badRequest(`input[${i}].imageUrl only supports data: or http(s) URLs.`); } - return imageUrlMessage(url); + messages.push(imageUrlMessage(url)); + return; } - throw badRequest(`input[${i}].type must be one of text / image_url.`); + if (part.type === "file") { + // Not an OmniMessage of its own: the file becomes an `[attached file: …]` line on the + // text message once written to the scratchpad, so it carries no payload into the run. + attachments.push(parseAttachmentPart(part, i)); + // Per-request count / total-bytes caps, re-checked on every part so a hostile `input` + // is cut off at the item that crosses the line (see assertAttachmentBudget). + assertAttachmentBudget(attachments); + return; + } + throw badRequest(`input[${i}].type must be one of text / image_url / file.`); }); + return { messages, attachments }; } /** @@ -323,29 +377,33 @@ export function sessionsRoutes(deps: AppDeps): Hono { return c.body(null, 204); }); - // Session scratchpad files (e.g. input images saved to disk for image-unsupported - // models): read by filename, so the conversation UI can render a message's - // "[attached image: ]" attachment line back into an image. Restricted to this - // session's own scratchpad directory (the filename must not contain a path - // separator, blocking traversal); filenames include a timestamp and are globally - // unique, so the response is marked immutable and long-cacheable. + // Session scratchpad files (input images saved to disk for image-unsupported models, the + // composer's file attachments, model-generated temp files): read by filename, so the + // conversation UI can render a message's "[attached image: ]" attachment line back + // into an image. Restricted to this session's own scratchpad directory (see + // resolveScratchpadFile); a name is never reused for different bytes — uploads take a random + // suffix on collision — so the response is marked immutable and long-cacheable. app.get("/:sessionId/scratchpad/:fileName", async (c) => { const row = resolveSession(c); const fileName = c.req.param("fileName") ?? ""; - if (!/^[A-Za-z0-9._-]+$/.test(fileName) || fileName.includes("..")) { - throw new HttpError(404, "file_not_found", "File does not exist."); - } - const filePath = path.join( - scratchpadDir(deps.config.root, row.projectId, row.agentId), - row.sessionId, + const filePath = resolveScratchpadFile( + path.join(scratchpadDir(deps.config.root, row.projectId, row.agentId), row.sessionId), fileName, ); + if (!filePath) throw new HttpError(404, "file_not_found", "File does not exist."); let bytes: Buffer; try { bytes = await fs.readFile(filePath); } catch { throw new HttpError(404, "file_not_found", "File does not exist."); } + // SECURITY BOUNDARY — do not extend casually. This map is an allowlist of types that are + // safe to hand a browser inline from the App's own origin, and it is the only reason the + // bytes below (arbitrary user uploads and Agent-written temp files) cannot become stored + // XSS. Every image type here is inert when rendered. Adding `.svg`, `.html`, `.pdf` or + // anything else that a browser parses as a document would look like a one-line convenience + // and would immediately be same-origin script execution — such a type needs the treatment + // the Workspace read gives it (plain-text downgrade or a sandbox CSP), not a map entry. const MIME_BY_EXT: Record = { ".png": "image/png", ".jpg": "image/jpeg", @@ -353,9 +411,23 @@ export function sessionsRoutes(deps: AppDeps): Hono { ".gif": "image/gif", ".webp": "image/webp", }; - const mime = MIME_BY_EXT[path.extname(fileName).toLowerCase()] ?? "application/octet-stream"; + const mime = MIME_BY_EXT[path.extname(fileName).toLowerCase()]; return c.body(new Uint8Array(bytes), 200, { - "content-type": mime, + "content-type": mime ?? "application/octet-stream", + // nosniff: the composer's file attachments land in this same directory, so the bytes + // here are arbitrary user content served from the App's own origin — without it a + // browser could sniff an `application/octet-stream` upload back into HTML and run it + // same-origin (the same defense workspace file reads apply). + "x-content-type-options": "nosniff", + // Second, independent layer for everything that fell off the allowlist: the only reason + // this endpoint is fetched inline is the conversation's tags, so anything that is + // not one of those images is served as a download and never renders as a document — + // nosniff alone would be the whole defense otherwise. + ...(mime === undefined + ? { + "content-disposition": `attachment; filename*=UTF-8''${encodeURIComponent(fileName)}`, + } + : {}), "cache-control": "private, max-age=31536000, immutable", }); }); @@ -420,33 +492,63 @@ export function sessionsRoutes(deps: AppDeps): Hono { const thinkingLevel = optionalEnum(body, "thinkingLevel", THINKING_LEVELS); if (goal) { // Goal mode: the input must be plain non-empty text (its marker-stripped text becomes - // the objective, re-injected every round — images have no place in the protocol). - const input = parseTaskInput(body); - const text = input + // the objective, re-injected every round — images and file attachments have no place in + // the protocol; rejected before any upload is written to disk). + const { messages, attachments } = parseTaskInput(body); + const text = messages .filter((m) => (m.payload as { type?: string }).type === "text") .map((m) => (m.payload as { text: string }).text) .join("\n") .trim(); - if (!text || input.some((m) => (m.payload as { type?: string }).type !== "text")) { + if ( + !text || + attachments.length > 0 || + messages.some((m) => (m.payload as { type?: string }).type !== "text") + ) { throw badRequest("goal mode requires text-only input (the objective)."); } const { sessionId } = await deps.manager.startGoal(row.sessionId, { - input, + input: messages, budget: goal.budget, ...(thinkingLevel !== undefined ? { thinkingLevel } : {}), }); return c.json({ sessionId } satisfies TaskCreateResponse, 202); } - const input = parseTaskInput(body); + const parsed = parseTaskInput(body); // Follow-up queue: with queueIfBusy, a busy session enqueues the input instead of 409 // (auto-starts as an ordinary next task once idle; the response says which happened). const queueIfBusy = body.queueIfBusy === true; - // 202: the Task executes on the server, decoupled from the SSE connection; sessionId is the current actual id (the new id after self-heal). - const { sessionId, queued } = await deps.manager.startTask(row.sessionId, input, { - ...(thinkingLevel !== undefined ? { thinkingLevel } : {}), - queueIfBusy, - }); - return c.json({ sessionId, queued } satisfies TaskCreateResponse, 202); + // Advisory pre-check, so the overwhelmingly common rejection — sending while a Task is + // running, without queueIfBusy — never writes bytes it would then have to take back. The + // authoritative check still runs under the Session lock inside startTask; this one is + // lock-free and may pass on a race, which the cleanup below covers. + deps.manager.assertCanAcceptTask(row.sessionId, { queueIfBusy }); + // File attachments land in this Session's scratchpad (deleted along with the Session) and + // are handed to the model as `[attached file: ]` lines on the message text. Written + // even when the task ends up queued as a follow-up: the queued input must be complete, and + // the queue is drained by this same Session. A Trace-less Session that self-heals into a + // new id below keeps its files under the id they were written with — the paths in the + // message stay valid; only the delete-with-the-Session cleanup misses them in that case. + const { input, written } = await attachFilesToInput( + parsed.messages, + parsed.attachments, + scratchpadDir(deps.config.root, row.projectId, row.agentId), + row.sessionId, + ); + try { + // 202: the Task executes on the server, decoupled from the SSE connection; sessionId is the current actual id (the new id after self-heal). + const { sessionId, queued } = await deps.manager.startTask(row.sessionId, input, { + ...(thinkingLevel !== undefined ? { thinkingLevel } : {}), + queueIfBusy, + }); + return c.json({ sessionId, queued } satisfies TaskCreateResponse, 202); + } catch (err) { + // The Task never started, so nothing references these files and nothing will ever clean + // them up — and the Web keeps the chips on failure, so the user's retry would otherwise + // land a second copy of every one of them. + await removeAttachments(written); + throw err; + } }); // Mid-run steering: queue a user message for the running Task; core delivers it between diff --git a/packages/server/src/runtime/session-manager.ts b/packages/server/src/runtime/session-manager.ts index b12e89d..19efea6 100644 --- a/packages/server/src/runtime/session-manager.ts +++ b/packages/server/src/runtime/session-manager.ts @@ -442,6 +442,24 @@ export class SessionManager { // —— Task / compaction drive —— + /** + * Cheap, lock-free rehearsal of the 409/503 conditions startTask checks, throwing exactly the + * same HttpErrors. **Advisory only**: it neither takes the Session lock nor loads an entry, so + * a session that isn't in the active table reads as acceptable and a status change racing this + * call is not caught — the authoritative check is still the one inside startTask. + * + * It exists so a caller that has irreversible work to do first (POST /tasks writes the + * message's file attachments to disk) can find out about the ordinary "a Task is already + * running" rejection before doing it, instead of undoing it afterwards. + */ + assertCanAcceptTask(sessionId: string, opts?: { queueIfBusy?: boolean }): void { + this.assertOpen(); + this.assertAgentNotDeleting(sessionId); + this.assertSessionNotDeleting(sessionId); + const entry = this.entries.get(sessionId); + if (entry && !opts?.queueIfBusy) this.assertIdle(entry); + } + /** * Start a Task: get-or-load → 409 * mutual-exclusion check → publish the input messages first → drive run in the diff --git a/packages/server/src/services/task-attachments.ts b/packages/server/src/services/task-attachments.ts new file mode 100644 index 0000000..bb096e0 --- /dev/null +++ b/packages/server/src/services/task-attachments.ts @@ -0,0 +1,297 @@ +/** + * Composer file attachments — the `{type:"file"}` variant of TaskInputPart. + * + * The browser has no Session while a chat is still a draft, so there is no upload endpoint to + * call before sending: the file rides the task request itself as a base64 `data:` URL, exactly + * like a pasted image does. On the way in it is written to the **Session scratchpad** + * (`/scratchpad//`, deleted with the Session, so cleanup is free) and the + * message text gains one `[attached file: ]` line per file — the bytes never + * enter the conversation, the model opens the file by path with its ordinary file tools. + * + * The line format and its placement are not defined here: they are shared with core's + * `[attached image: …]` producer and the Web renderer that parses both + * (`@prismshadow/penguin-core/markers` → attachment-lines.ts, plus `appendAttachmentLines`), + * so the two conventions cannot drift apart. + */ +import fs from "node:fs/promises"; +import path from "node:path"; +import { randomBytes } from "node:crypto"; +import { appendAttachmentLines, attachedFileLine } from "@prismshadow/penguin-core"; +import type { OmniMessage } from "@prismshadow/penguin-core"; +import { HttpError } from "../http/errors.js"; +import { badRequest } from "../http/validate.js"; + +/** + * Per-file cap. Deliberately below the workspace upload's 14MB: several attachments can ride + * one task request, and the whole body still has to fit the global 20MB limit (base64 inflates + * by 4/3), so a smaller per-file ceiling keeps "one big file" working while leaving room for + * "a handful of ordinary ones". Oversize is a 413, matching the workspace upload route. + */ +export const MAX_ATTACHMENT_BYTES = 10 * 1024 * 1024; + +/** + * Per-request caps, checked while the parts are validated — before a single byte reaches the + * disk. They are deliberately NOT delegated to the global body cap: this module has to hold on + * its own, so that a change to that middleware (or a caller that never goes through it) cannot + * silently unbound it. Without them a legal 20MB body fits ~350k minimal `file` parts, which is + * 350k sequential writes into one directory and 350k marker lines on one message. + * + * 20 files is far past any plausible composer use — the chip row stops being usable long before + * that — while still allowing "drop a folder of small files in". 12MB of decoded bytes keeps one + * full-size 10MB attachment usable next to a couple of ordinary ones; base64 inflates by 4/3, so + * 12MB decoded is ~16MB of body and this cap, not the 20MB body cap, is the one a caller + * actually reaches. + */ +export const MAX_ATTACHMENT_COUNT = 20; +export const MAX_TOTAL_ATTACHMENT_BYTES = 12 * 1024 * 1024; + +/** + * Longest stem kept on disk, measured in **UTF-8 bytes**: filesystems cap a name near 255 + * bytes, and a CJK character costs three of them — a character count would let a Chinese name + * blow the real limit while an English one stayed far under it. + */ +const MAX_STEM_BYTES = 80; + +/** Windows reserves these device names with or without an extension (`con`, `con.txt`), case-insensitively. */ +const WINDOWS_RESERVED = /^(con|prn|aux|nul|com[1-9]|lpt[1-9])$/i; + +/** Format and control characters (Unicode category C) — invisible, and the vector behind right-to-left file-name spoofing. */ +const INVISIBLE_CHAR = /\p{C}/u; + +/** + * True when a character must not reach a file name. ASCII keeps the long-standing whitelist: + * a space or a shell metacharacter inside a path the model is about to paste into a command is + * a footgun, so anything outside `[A-Za-z0-9._-]` still becomes `-` down there. Above ASCII the + * rule inverts — the character is kept as typed, so `报告.pdf` reaches the model as `报告.pdf` + * instead of collapsing to an anonymous `file.pdf` (CJK, accents and emoji are all harmless to + * a shell). The exception is Unicode category C: invisible controls, and the bidi overrides + * that let a name render as something it is not. + */ +function unsafeNameChar(ch: string): boolean { + if (/[A-Za-z0-9._-]/.test(ch)) return false; + return ch.codePointAt(0)! < 0x80 || INVISIBLE_CHAR.test(ch); +} + +/** Replace every unsafe character with `-`, iterating code points so a surrogate pair survives intact. */ +function sanitizeSegment(value: string): string { + return Array.from(value, (ch) => (unsafeNameChar(ch) ? "-" : ch)).join(""); +} + +/** Truncate to a UTF-8 byte budget on character boundaries (iterating a string yields whole code points, so a surrogate pair is never split). */ +function truncateBytes(value: string, maxBytes: number): string { + if (Buffer.byteLength(value) <= maxBytes) return value; + let out = ""; + for (const ch of value) { + if (Buffer.byteLength(out) + Buffer.byteLength(ch) > maxBytes) break; + out += ch; + } + return out; +} + +/** The 6-hex space makes a second collision negligible; the cap only guards against a filesystem stuck on EEXIST. */ +const MAX_NAME_ATTEMPTS = 16; + +/** One validated attachment, bytes already decoded (they are held in memory only until the write below). */ +export interface TaskAttachment { + /** Original file name as submitted (validated: non-empty, no path separators, no `..`). */ + fileName: string; + bytes: Buffer; +} + +/** + * Validate one `{type:"file"}` input part. Shape problems are 400s in the same style as the + * neighbouring text/image checks; only the size cap answers 413 (`file_too_large`, the code + * the Web App already has copy for). `index` is the part's position in `input`, so the message + * points at the offending item like the other input errors do. + */ +export function parseAttachmentPart(part: Record, index: number): TaskAttachment { + const fileName = part.fileName; + // Path separators and `..` are rejected rather than sanitized away: the name is the user's, + // and a name that looks like a path means the caller is confused about the contract (the + // write below composes the path itself, and sanitization happens there). + if ( + typeof fileName !== "string" || + fileName.length === 0 || + fileName.includes("/") || + fileName.includes("\\") || + fileName.includes("..") || + fileName.includes("\0") + ) { + throw badRequest( + `input[${index}].fileName must be a non-empty file name without path separators or "..".`, + ); + } + const dataUrl = part.dataUrl; + // `[^,]*` for the media type, not `[^;,]*`: a browser may hand out parameters + // (`data:text/plain;charset=utf-8;base64,…`), and only the `;base64,` marker separates the + // type from the payload. The payload's character class is the actual check that it IS + // base64 (whitespace tolerated — line-wrapped encoders decode fine). + const match = + typeof dataUrl === "string" ? /^data:[^,]*;base64,([A-Za-z0-9+/=\s]+)$/.exec(dataUrl) : null; + if (!match) { + throw badRequest(`input[${index}].dataUrl must be a base64 data: URL of the file's bytes.`); + } + const bytes = Buffer.from(match[1]!, "base64"); + if (bytes.length === 0) { + throw badRequest(`input[${index}].dataUrl decodes to an empty file.`); + } + if (bytes.length > MAX_ATTACHMENT_BYTES) { + throw new HttpError( + 413, + "file_too_large", + `Attached file exceeds the ${MAX_ATTACHMENT_BYTES / (1024 * 1024)}MB limit.`, + ); + } + return { fileName, bytes }; +} + +/** + * Enforce the per-request caps against everything accepted so far. Called after **each** `file` + * part rather than once at the end, so an oversized `input` stops at the part that crosses the + * line instead of base64-decoding the whole array first. Both answer 413: the count reuses a + * dedicated `too_many_files` code, the aggregate the `payload_too_large` the body cap already + * uses — from the caller's side it is the same "this request is too big" outcome. + */ +export function assertAttachmentBudget(attachments: TaskAttachment[]): void { + if (attachments.length > MAX_ATTACHMENT_COUNT) { + throw new HttpError( + 413, + "too_many_files", + `A message may carry at most ${MAX_ATTACHMENT_COUNT} attached files.`, + ); + } + let total = 0; + for (const a of attachments) total += a.bytes.length; + if (total > MAX_TOTAL_ATTACHMENT_BYTES) { + throw new HttpError( + 413, + "payload_too_large", + `Attached files exceed the ${MAX_TOTAL_ATTACHMENT_BYTES / (1024 * 1024)}MB total limit for one message.`, + ); + } +} + +/** + * Map a submitted name onto a name that is safe on disk **and** still recognizably the user's + * own: `报告 2026.pdf` becomes `报告-2026.pdf` (see unsafeNameChar — the words survive, only the + * shell-hostile ASCII is replaced), so the model reads a meaningful path and a person looking + * at the message recognizes what they attached. + * + * The rest is Windows-shaped hygiene: trailing dots and spaces are dropped (Windows silently + * strips them, so `a.` and `a` would be the same file), a reserved device name is prefixed + * (`con.txt` → `_con.txt`), the stem is capped by UTF-8 bytes, and a stem that sanitizes away + * entirely falls back to `file` rather than producing a bare extension. + */ +function scratchpadName(fileName: string): string { + const ext = sanitizeSegment(path.extname(fileName)); + const rawStem = fileName.slice(0, fileName.length - path.extname(fileName).length); + // Trim after truncating: a cut can expose a trailing dot or space that was mid-name before. + const stem = truncateBytes(sanitizeSegment(rawStem), MAX_STEM_BYTES).replace(/[. ]+$/, ""); + // Nothing but replacement dashes carries no more information than an empty stem did. + if (!stem || /^-+$/.test(stem)) return `file${ext}`; + return WINDOWS_RESERVED.test(stem) ? `_${stem}${ext}` : `${stem}${ext}`; +} + +/** + * Write one attachment into `dir` and return its absolute path. The plain sanitized name is + * tried first (the model — and the user reading the message — sees `report.pdf`, not an opaque + * id); "wx" makes the create exclusive, so a second upload of the same name lands next to the + * first as `report-3f9a1c.pdf` instead of overwriting it (same convention as core's image + * uploads). + * + * "wx" is O_CREAT|O_EXCL, which also refuses to follow a symlink at the final component: a link + * planted at `report.pdf` fails with EEXIST and the retry allocates a suffixed name instead of + * writing through it. The containment check for the *directory* is separate — see openScratchpadDir. + */ +async function writeAttachment(dir: string, attachment: TaskAttachment): Promise { + const base = scratchpadName(attachment.fileName); + const ext = path.extname(base); + const stem = base.slice(0, base.length - ext.length); + for (let attempt = 0; attempt < MAX_NAME_ATTEMPTS; attempt++) { + const name = attempt === 0 ? base : `${stem}-${randomBytes(3).toString("hex")}${ext}`; + const file = path.join(dir, name); + try { + await fs.writeFile(file, attachment.bytes, { flag: "wx" }); + return file; + } catch (err) { + if ((err as NodeJS.ErrnoException).code !== "EEXIST") throw err; + } + } + throw new Error( + `failed to allocate a unique attachment file name under ${dir} after ${MAX_NAME_ATTEMPTS} attempts`, + ); +} + +/** + * Create (or reuse) this Session's scratchpad directory and hand back the path to write into. + * + * `fs.mkdir(…, {recursive:true})` succeeds silently when the directory is already a **symlink** + * to somewhere else, and nothing downstream would notice — so the result is realpath'd and + * required to still sit inside the Agent's scratchpad root, the same containment rule the + * Workspace upload path applies (workspace-files-service.resolveWriteParent). Only the Agent + * process can plant such a link and it runs as this server's uid, so this is consistency rather + * than a privilege boundary; it costs one resolution per message that carries attachments. + * + * The check is on the canonical path but the write stays on the logical one: the path travels + * into the message text, and the read/delete endpoints address a Session by its logical + * directory, so canonicalizing here would only make those disagree on hosts where the data root + * itself sits behind a link (macOS `/var`, a Windows 8.3 temp path). + */ +async function openScratchpadDir(root: string, sessionId: string): Promise { + const dir = path.join(root, sessionId); + await fs.mkdir(dir, { recursive: true }); + const canonicalRoot = await fs.realpath(root); + const rel = path.relative(canonicalRoot, await fs.realpath(dir)); + if (rel !== sessionId) { + throw new Error( + `session scratchpad ${dir} resolves outside the agent scratchpad root; refusing to write attachments`, + ); + } + return dir; +} + +/** Best-effort undo of a batch of writes; errors are swallowed because every caller is already on an error path (a failed cleanup must not replace the original failure). */ +export async function removeAttachments(files: string[]): Promise { + await Promise.all(files.map((f) => fs.rm(f, { force: true }).catch(() => {}))); +} + +/** Result of a write batch: the Prompt to run, plus the paths written so the caller can undo them if the Task never starts. */ +export interface AttachedFiles { + input: OmniMessage[]; + written: string[]; +} + +/** + * Land every attachment in the Session scratchpad under `root` and return the Prompt with one + * `[attached file: ]` line appended per file. Placement follows core's shared rule + * (after the last user text message; attachments-only input becomes a line-only text + * message), so a Prompt carrying both images and files still ends in a single trailing block. + * Returns `messages` untouched when there is nothing to attach — no directory is created. + * + * All-or-nothing: a failure part-way through the batch removes what it already wrote, so a 500 + * never leaves files on disk that no message refers to. The caller owns the other half of that + * guarantee — if starting the Task fails afterwards it must call removeAttachments(written), + * otherwise the user's retry would land a second copy of every file. + */ +export async function attachFilesToInput( + messages: OmniMessage[], + attachments: TaskAttachment[], + root: string, + sessionId: string, +): Promise { + if (attachments.length === 0) return { input: messages, written: [] }; + const dir = await openScratchpadDir(root, sessionId); + const written: string[] = []; + try { + // Sequential on purpose: the exclusive-create retry above resolves collisions against files + // that already exist, and writing the batch one at a time keeps two same-named uploads in + // the same message from racing each other for the plain name. + for (const attachment of attachments) { + written.push(await writeAttachment(dir, attachment)); + } + } catch (err) { + await removeAttachments(written); + throw err; + } + return { input: appendAttachmentLines(messages, written.map(attachedFileLine)), written }; +} diff --git a/packages/server/test/body-limit.test.ts b/packages/server/test/body-limit.test.ts new file mode 100644 index 0000000..ef1caad --- /dev/null +++ b/packages/server/test/body-limit.test.ts @@ -0,0 +1,142 @@ +/** + * Global request body cap (`/api/*`, 20MB). + * + * The cap used to read `content-length` only, which a chunked request simply does not carry — + * `Number(undefined ?? 0)` is 0, so a body of any size passed straight through to the sinks + * behind it (task input images, file attachments, Trace import). These tests post a body with + * **no declared length** and require the same 413 `payload_too_large` a declared one gets, plus + * an under-cap streamed body still arriving intact (the cap has to re-feed what it counted). + */ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { assistantText } from "@prismshadow/penguin-core"; +import type { OmniMessage } from "@prismshadow/penguin-core"; +import type { SessionRow } from "../src/db/repos/sessions.js"; +import type { RuntimeSession } from "../src/runtime/session-manager.js"; +import { apiClient, createTestApp, provisionUser, waitFor } from "./helpers.js"; +import type { TestApp } from "./helpers.js"; + +const SID = "session-2026-07-29-13-00-00-aabb0004"; +const PROJECT_ID = "streamer-default_project"; +const MB = 1024 * 1024; + +/** + * A `{"input":[{"type":"text","text":"aaa…"}]}` body delivered as a stream with no + * `content-length`, `fill` bytes of filler inside the text. Valid JSON on purpose: if the cap + * ever stops working the request is a plain 202, exactly the shape the review reproduced — + * not a 400 that would pass a "was rejected" assertion for the wrong reason. Chunks are + * produced on demand, so the cap aborting mid-body costs only what it actually read. + */ +function streamedTaskBody(fill: number): ReadableStream { + const enc = new TextEncoder(); + const chunk = enc.encode("a".repeat(64 * 1024)); + let sent = 0; + let tailWritten = false; + return new ReadableStream({ + start(controller) { + controller.enqueue(enc.encode('{"input":[{"type":"text","text":"')); + }, + pull(controller) { + if (sent >= fill) { + if (tailWritten) { + controller.close(); + return; + } + tailWritten = true; + controller.enqueue(enc.encode('"}]}')); + return; + } + const size = Math.min(chunk.length, fill - sent); + sent += size; + controller.enqueue(size === chunk.length ? chunk : chunk.subarray(0, size)); + }, + }); +} + +describe("request body cap", () => { + let t: TestApp; + let api: ReturnType; + let cookie: string; + let runs: OmniMessage[][]; + + const postStream = (fill: number) => + t.app.request(`/api/sessions/${SID}/tasks`, { + method: "POST", + headers: { cookie, "content-type": "application/json" }, + body: streamedTaskBody(fill), + // Required by fetch for a streaming request body; it is also what keeps the request + // free of a content-length header, which is the case under test. + duplex: "half", + } as RequestInit); + + beforeEach(async () => { + t = await createTestApp(); + ({ cookie } = await provisionUser(t.app, "streamer")); + api = apiClient(t.app, cookie); + const row: SessionRow = { + sessionId: SID, + projectId: PROJECT_ID, + agentId: "default_agent", + provider: "custom", + modelId: "m1", + workspace: "/tmp/w", + approvalMode: "allow-all", + title: null, + createdAt: new Date().toISOString(), + }; + t.deps.sessionsRepo.insert(row); + runs = []; + const session: RuntimeSession = { + sessionId: SID, + toolPermission: () => "rw", + generateTitle: async () => ({ title: null, usage: null }), + compactability: () => "ok" as const, + steer: () => false, + skipReconnectWait: () => false, + async *run(input: OmniMessage[]) { + runs.push(input); + yield assistantText("done"); + }, + async *compact() {}, + }; + t.deps.manager.adopt(row, session); + }); + afterEach(async () => { + await t.cleanup(); + }); + + it("a body with no declared length is still capped", async () => { + const res = await postStream(24 * MB); + expect(res.status).toBe(413); + expect(((await res.json()) as { error: { code: string } }).error.code).toBe( + "payload_too_large", + ); + expect(runs).toHaveLength(0); + }); + + it("a declared over-cap content-length short-circuits before the body is read", async () => { + // The header fast path, which the streaming case above deliberately cannot reach: the + // length is declared and the (tiny, valid) body is never looked at. + const res = await t.app.request(`/api/sessions/${SID}/tasks`, { + method: "POST", + headers: { + cookie, + "content-type": "application/json", + "content-length": String(21 * MB), + }, + body: JSON.stringify({ input: [{ type: "text", text: "small" }] }), + }); + expect(res.status).toBe(413); + expect(((await res.json()) as { error: { code: string } }).error.code).toBe( + "payload_too_large", + ); + expect(runs).toHaveLength(0); + }); + + it("an under-cap streamed body is passed through intact", async () => { + const res = await postStream(MB); + expect(res.status).toBe(202); + await waitFor(() => runs.length === 1); + const text = (runs[0]![0]!.payload as { text: string }).text; + expect(text.length).toBe(MB); + }); +}); diff --git a/packages/server/test/session-delete-scratchpad.test.ts b/packages/server/test/session-delete-scratchpad.test.ts index fc36957..cd29945 100644 --- a/packages/server/test/session-delete-scratchpad.test.ts +++ b/packages/server/test/session-delete-scratchpad.test.ts @@ -88,6 +88,16 @@ describe("session deletion cleans up the scratchpad", () => { expect(res.headers.get("content-type")).toBe("image/png"); expect(Buffer.from(await res.arrayBuffer())).toEqual(png); + // A non-ASCII attachment name round-trips: the composer percent-encodes it into the URL, + // and the route contains the read by resolving the path rather than by whitelisting + // characters — so `报告.pdf` stays fetchable instead of having to be renamed on upload. + await fs.writeFile(path.join(dir, "报告.pdf"), "cjk"); + const cjk = await owner.get( + `/api/sessions/${session.sessionId}/scratchpad/${encodeURIComponent("报告.pdf")}`, + ); + expect(cjk.status).toBe(200); + expect(await cjk.text()).toBe("cjk"); + // Missing files and filenames with path separators/traversal both 404 (no existence leak). expect((await owner.get(`/api/sessions/${session.sessionId}/scratchpad/nope.png`)).status).toBe( 404, @@ -95,5 +105,10 @@ describe("session deletion cleans up the scratchpad", () => { expect( (await owner.get(`/api/sessions/${session.sessionId}/scratchpad/..%2Fsecret.png`)).status, ).toBe(404); + // Backslash separators and a bare relative marker are rejected the same way. + expect( + (await owner.get(`/api/sessions/${session.sessionId}/scratchpad/..%5Csecret.png`)).status, + ).toBe(404); + expect((await owner.get(`/api/sessions/${session.sessionId}/scratchpad/..`)).status).toBe(404); }); }); diff --git a/packages/server/test/task-attachments.test.ts b/packages/server/test/task-attachments.test.ts new file mode 100644 index 0000000..a17d029 --- /dev/null +++ b/packages/server/test/task-attachments.test.ts @@ -0,0 +1,456 @@ +/** + * Integration tests for composer file attachments (POST /api/sessions/:id/tasks with a + * `{type:"file"}` input part): + * - the bytes land in the Session scratchpad and the Prompt gains an + * `[attached file: ]` line, so the model reaches the file by path; + * - a files-only Prompt still reaches the model (the lines become the message), including + * when the only text message is a `[handoff_from]` origin block that must stay parseable; + * - two uploads of the same name coexist instead of overwriting each other; + * - malformed parts are 400s, an oversize file / too many files / too many bytes are 413s, + * and goal mode rejects attachments before anything is written; + * - nothing survives a request that does not end up starting a Task, and a scratchpad + * directory that resolves outside the Agent's scratchpad root is refused outright. + */ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { + assistantText, + buildHandoffMessage, + parseHandoffMessage, + scratchpadDir, +} from "@prismshadow/penguin-core"; +import type { OmniMessage } from "@prismshadow/penguin-core"; +import type { SessionRow } from "../src/db/repos/sessions.js"; +import type { RuntimeSession } from "../src/runtime/session-manager.js"; +import { + MAX_ATTACHMENT_BYTES, + MAX_ATTACHMENT_COUNT, + MAX_TOTAL_ATTACHMENT_BYTES, +} from "../src/services/task-attachments.js"; +import { apiClient, createTestApp, provisionUser, waitFor } from "./helpers.js"; +import type { TestApp } from "./helpers.js"; + +const SID = "session-2026-07-29-10-00-00-aabb0001"; +const PROJECT_ID = "attacher-default_project"; + +/** Fake Session that records each run's input and finishes immediately (no LLM, no approvals). */ +function recordingFakeSession(sessionId: string, runs: OmniMessage[][]): RuntimeSession { + return { + sessionId, + toolPermission: () => "rw", + generateTitle: async () => ({ title: null, usage: null }), + compactability: () => "ok" as const, + steer: () => false, + skipReconnectWait: () => false, + async *run(input: OmniMessage[]) { + runs.push(input); + yield assistantText("done"); + }, + async *compact() {}, + }; +} + +/** Fake Session whose run parks until `until` resolves, so the Session stays busy while the test posts. */ +function parkingFakeSession(sessionId: string, until: Promise): RuntimeSession { + return { + ...recordingFakeSession(sessionId, []), + async *run() { + await until; + yield assistantText("done"); + }, + }; +} + +/** Base64 data URL of some bytes, the shape the composer submits. */ +function dataUrl(content: string, mime = "application/octet-stream"): string { + return `data:${mime};base64,${Buffer.from(content).toString("base64")}`; +} + +/** All text of a recorded Prompt, joined the way the model would read it. */ +function promptText(input: OmniMessage[]): string { + return input + .map((m) => (m.payload as { text?: string }).text ?? "") + .filter(Boolean) + .join("\n"); +} + +describe("task input file attachments", () => { + let t: TestApp; + let api: ReturnType; + let runs: OmniMessage[][]; + let dir: string; + let row: SessionRow; + + beforeEach(async () => { + t = await createTestApp(); + const { cookie } = await provisionUser(t.app, "attacher"); + api = apiClient(t.app, cookie); + row = { + sessionId: SID, + projectId: PROJECT_ID, + agentId: "default_agent", + provider: "custom", + modelId: "m1", + workspace: "/tmp/w", + approvalMode: "allow-all", + title: null, + createdAt: new Date().toISOString(), + }; + t.deps.sessionsRepo.insert(row); + runs = []; + t.deps.manager.adopt(row, recordingFakeSession(SID, runs)); + dir = path.join(scratchpadDir(t.root, PROJECT_ID, "default_agent"), SID); + }); + afterEach(async () => { + await t.cleanup(); + }); + + it("writes the file into the session scratchpad and appends the marker line to the text", async () => { + const res = await api.post(`/api/sessions/${SID}/tasks`, { + input: [ + { type: "text", text: "look at this" }, + { type: "file", fileName: "report.pdf", dataUrl: dataUrl("PDF-BYTES") }, + ], + }); + expect(res.status).toBe(202); + await waitFor(() => runs.length === 1); + + const text = promptText(runs[0]!); + const marker = /\[attached file: (.+)\]/.exec(text); + expect(marker).not.toBeNull(); + const filePath = marker![1]!; + expect(filePath).toBe(path.join(dir, "report.pdf")); + expect(await fs.readFile(filePath, "utf8")).toBe("PDF-BYTES"); + // The line trails the user's own text — it must not replace or reframe the message. + expect(text.startsWith("look at this")).toBe(true); + // One text message carries both (no extra message per file). + expect(runs[0]!).toHaveLength(1); + }); + + it("files-only input becomes a message of attachment lines; same names do not overwrite", async () => { + const res = await api.post(`/api/sessions/${SID}/tasks`, { + input: [ + { type: "file", fileName: "notes.txt", dataUrl: dataUrl("first") }, + { type: "file", fileName: "notes.txt", dataUrl: dataUrl("second") }, + ], + }); + expect(res.status).toBe(202); + await waitFor(() => runs.length === 1); + + const paths = [...promptText(runs[0]!).matchAll(/\[attached file: (.+)\]/g)].map((m) => m[1]!); + expect(paths).toHaveLength(2); + expect(paths[0]).toBe(path.join(dir, "notes.txt")); + // The second upload gets a random suffix rather than clobbering the first. + expect(paths[1]).not.toBe(paths[0]); + expect(path.basename(paths[1]!)).toMatch(/^notes-[0-9a-f]{6}\.txt$/); + expect(await fs.readFile(paths[0]!, "utf8")).toBe("first"); + expect(await fs.readFile(paths[1]!, "utf8")).toBe("second"); + }); + + it("unsafe characters in the name are sanitized, keeping the extension", async () => { + const res = await api.post(`/api/sessions/${SID}/tasks`, { + input: [{ type: "file", fileName: "my report (final).csv", dataUrl: dataUrl("a,b") }], + }); + expect(res.status).toBe(202); + await waitFor(() => runs.length === 1); + const filePath = /\[attached file: (.+)\]/.exec(promptText(runs[0]!))![1]!; + expect(path.basename(filePath)).toBe("my-report--final-.csv"); + expect(await fs.readFile(filePath, "utf8")).toBe("a,b"); + }); + + it("keeps a non-ASCII name instead of flattening it, and caps the stem by UTF-8 bytes", async () => { + // A CJK character costs three bytes: 40 of them are 120 bytes, well past the 80-byte cap, + // so the name is cut on a character boundary rather than mid-character (a split would leave + // an invalid sequence on disk and an unopenable path in the message). + const long = "报".repeat(40); + const res = await api.post(`/api/sessions/${SID}/tasks`, { + input: [ + { type: "file", fileName: "报告 2026.pdf", dataUrl: dataUrl("cjk") }, + { type: "file", fileName: `${long}.txt`, dataUrl: dataUrl("long") }, + ], + }); + expect(res.status).toBe(202); + await waitFor(() => runs.length === 1); + + const paths = [...promptText(runs[0]!).matchAll(/\[attached file: (.+)\]/g)].map((m) => m[1]!); + // The words survive; only the space (shell-hostile, and ASCII) is replaced. + expect(path.basename(paths[0]!)).toBe("报告-2026.pdf"); + expect(await fs.readFile(paths[0]!, "utf8")).toBe("cjk"); + const capped = path.basename(paths[1]!); + expect(capped).toBe(`${"报".repeat(26)}.txt`); + expect(Buffer.byteLength(capped.slice(0, capped.length - 4))).toBeLessThanOrEqual(80); + expect(await fs.readFile(paths[1]!, "utf8")).toBe("long"); + }); + + it("prefixes a Windows device name and falls back when the stem sanitizes away", async () => { + const res = await api.post(`/api/sessions/${SID}/tasks`, { + input: [ + { type: "file", fileName: "con.txt", dataUrl: dataUrl("device") }, + // Zero-width joiner only (spelled by code point — an invisible character in the source + // would read as an empty string): category C, so nothing is left to name the file with. + { + type: "file", + fileName: `${String.fromCodePoint(0x200d)}.bin`, + dataUrl: dataUrl("invisible"), + }, + ], + }); + expect(res.status).toBe(202); + await waitFor(() => runs.length === 1); + + const paths = [...promptText(runs[0]!).matchAll(/\[attached file: (.+)\]/g)].map((m) => m[1]!); + expect(path.basename(paths[0]!)).toBe("_con.txt"); + expect(path.basename(paths[1]!)).toBe("file.bin"); + }); + + it("accepts a data URL whose media type carries parameters", async () => { + const res = await api.post(`/api/sessions/${SID}/tasks`, { + input: [ + { + type: "file", + fileName: "notes.txt", + dataUrl: dataUrl("hello", "text/plain;charset=utf-8"), + }, + ], + }); + expect(res.status).toBe(202); + await waitFor(() => runs.length === 1); + const filePath = /\[attached file: (.+)\]/.exec(promptText(runs[0]!))![1]!; + expect(await fs.readFile(filePath, "utf8")).toBe("hello"); + }); + + it("malformed parts are 400s and write nothing", async () => { + const bad = [ + { type: "file", dataUrl: dataUrl("x") }, // no fileName + { type: "file", fileName: "", dataUrl: dataUrl("x") }, + { type: "file", fileName: "../escape.txt", dataUrl: dataUrl("x") }, + { type: "file", fileName: "sub/dir.txt", dataUrl: dataUrl("x") }, + { type: "file", fileName: "a.txt", dataUrl: "https://example.com/a.txt" }, + { type: "file", fileName: "a.txt", dataUrl: "data:text/plain,not-base64" }, + { type: "file", fileName: "a.txt" }, // no dataUrl + { type: "blob", fileName: "a.txt", dataUrl: dataUrl("x") }, // unknown part type + ]; + for (const part of bad) { + const res = await api.post(`/api/sessions/${SID}/tasks`, { input: [part] }); + expect(res.status, JSON.stringify(part)).toBe(400); + } + await expect(fs.access(dir)).rejects.toThrow(); + expect(runs).toHaveLength(0); + }); + + it("a file over the per-file cap is a 413", async () => { + const res = await api.post(`/api/sessions/${SID}/tasks`, { + input: [ + { + type: "file", + fileName: "big.bin", + dataUrl: `data:application/octet-stream;base64,${Buffer.alloc( + MAX_ATTACHMENT_BYTES + 1, + ).toString("base64")}`, + }, + ], + }); + expect(res.status).toBe(413); + expect(((await res.json()) as { error: { code: string } }).error.code).toBe("file_too_large"); + await expect(fs.access(dir)).rejects.toThrow(); + }); + + it("goal mode rejects attachments before anything is written", async () => { + const res = await api.post(`/api/sessions/${SID}/tasks`, { + input: [ + { type: "text", text: "ship the report" }, + { type: "file", fileName: "spec.md", dataUrl: dataUrl("# spec") }, + ], + goal: {}, + }); + expect(res.status).toBe(400); + await expect(fs.access(dir)).rejects.toThrow(); + }); + + it("files with only a handoff origin block: the block stays parseable, the lines get their own message", async () => { + // The composer's "attachments, no text, staged /agent handoff" shape. `[handoff_from]` only + // parses when the block is the WHOLE message, so appending the marker line to it would put + // the raw block in a user bubble instead of a one-line banner. + const block = buildHandoffMessage({ agentId: "alpha", agentName: "Alpha", sessionId: "s0" }); + const res = await api.post(`/api/sessions/${SID}/tasks`, { + input: [ + { type: "text", text: block }, + { type: "file", fileName: "notes.txt", dataUrl: dataUrl("hi") }, + ], + }); + expect(res.status).toBe(202); + await waitFor(() => runs.length === 1); + + const texts = runs[0]!.map((m) => (m.payload as { text?: string }).text ?? ""); + expect(texts).toHaveLength(2); + expect(texts[0]).toBe(block); + expect(parseHandoffMessage(texts[0]!)?.agentId).toBe("alpha"); + expect(texts[1]).toBe(`[attached file: ${path.join(dir, "notes.txt")}]`); + }); + + it("more than the per-request file count is a 413 and writes nothing", async () => { + const res = await api.post(`/api/sessions/${SID}/tasks`, { + input: Array.from({ length: MAX_ATTACHMENT_COUNT + 1 }, (_, i) => ({ + type: "file", + fileName: `f${i}.txt`, + dataUrl: dataUrl("x"), + })), + }); + expect(res.status).toBe(413); + expect(((await res.json()) as { error: { code: string } }).error.code).toBe("too_many_files"); + await expect(fs.access(dir)).rejects.toThrow(); + expect(runs).toHaveLength(0); + }); + + it("more than the per-request total size is a 413 and writes nothing", async () => { + // Two files, each individually legal, that together cross the aggregate cap: the per-file + // check alone would let this through and land both on disk. + const half = Buffer.alloc(Math.floor(MAX_TOTAL_ATTACHMENT_BYTES / 2) + 1).toString("base64"); + const res = await api.post(`/api/sessions/${SID}/tasks`, { + input: [ + { + type: "file", + fileName: "a.bin", + dataUrl: `data:application/octet-stream;base64,${half}`, + }, + { + type: "file", + fileName: "b.bin", + dataUrl: `data:application/octet-stream;base64,${half}`, + }, + ], + }); + expect(res.status).toBe(413); + expect(((await res.json()) as { error: { code: string } }).error.code).toBe( + "payload_too_large", + ); + await expect(fs.access(dir)).rejects.toThrow(); + expect(runs).toHaveLength(0); + }); + + it("a busy session without queueIfBusy 409s before the upload is written", async () => { + // Without the pre-check the files land first and the 409 comes after, so the user's retry + // (the Web keeps the chips on failure) would deposit a second copy of every one of them. + let release = () => {}; + const parked = new Promise((resolve) => { + release = resolve; + }); + t.deps.manager.adopt(row, parkingFakeSession(SID, parked)); + await api.post(`/api/sessions/${SID}/tasks`, { input: [{ type: "text", text: "busy" }] }); + await waitFor(() => t.deps.manager.statusOf(SID) === "running"); + + const res = await api.post(`/api/sessions/${SID}/tasks`, { + input: [{ type: "file", fileName: "late.txt", dataUrl: dataUrl("bytes") }], + }); + expect(res.status).toBe(409); + expect(((await res.json()) as { error: { code: string } }).error.code).toBe("task_in_progress"); + await expect(fs.access(dir)).rejects.toThrow(); + release(); + await waitFor(() => t.deps.manager.statusOf(SID) === "idle"); + }); +}); + +describe("task attachments are removed when the Task never starts", () => { + const FAIL_SID = "session-2026-07-29-11-00-00-aabb0002"; + const PID = "failer-default_project"; + let t: TestApp; + let api: ReturnType; + let dir: string; + + beforeEach(async () => { + // Never adopted into the active table, so startTask goes through the loader — which throws + // here. That is the window the route's cleanup exists for: the pre-check cannot see a + // session it hasn't loaded, so the files are already on disk when the failure happens. + t = await createTestApp({ + loader: { + load: async () => { + throw new Error("loader unavailable"); + }, + }, + }); + const { cookie } = await provisionUser(t.app, "failer"); + api = apiClient(t.app, cookie); + t.deps.sessionsRepo.insert({ + sessionId: FAIL_SID, + projectId: PID, + agentId: "default_agent", + provider: "custom", + modelId: "m1", + workspace: "/tmp/w", + approvalMode: "allow-all", + title: null, + createdAt: new Date().toISOString(), + }); + dir = path.join(scratchpadDir(t.root, PID, "default_agent"), FAIL_SID); + }); + afterEach(async () => { + await t.cleanup(); + }); + + it("a failure after the write leaves no orphaned bytes behind", async () => { + const res = await api.post(`/api/sessions/${FAIL_SID}/tasks`, { + input: [ + { type: "file", fileName: "a.txt", dataUrl: dataUrl("first") }, + { type: "file", fileName: "b.txt", dataUrl: dataUrl("second") }, + ], + }); + expect(res.status).toBe(500); + // The directory may remain (it is the Session's own and is deleted with it); the point is + // that a retry cannot find a stale `a-.txt` next to its own upload. + expect(await fs.readdir(dir).catch(() => [])).toEqual([]); + }); +}); + +describe("scratchpad directory containment", () => { + const LINK_SID = "session-2026-07-29-12-00-00-aabb0003"; + const PID = "linker-default_project"; + let t: TestApp; + let api: ReturnType; + let outside: string; + + beforeEach(async () => { + t = await createTestApp(); + const { cookie } = await provisionUser(t.app, "linker"); + api = apiClient(t.app, cookie); + t.deps.sessionsRepo.insert({ + sessionId: LINK_SID, + projectId: PID, + agentId: "default_agent", + provider: "custom", + modelId: "m1", + workspace: "/tmp/w", + approvalMode: "allow-all", + title: null, + createdAt: new Date().toISOString(), + }); + t.deps.manager.adopt( + t.deps.sessionsRepo.findById(LINK_SID)!, + recordingFakeSession(LINK_SID, []), + ); + outside = await fs.mkdtemp(path.join(os.tmpdir(), "penguin-outside-")); + }); + afterEach(async () => { + await fs.rm(outside, { recursive: true, force: true }); + await t.cleanup(); + }); + + // Symlink creation needs a privilege or developer mode on Windows; the containment rule + // itself is platform-independent. + it.skipIf(process.platform === "win32")( + "a session directory symlinked out of the scratchpad root is refused, not written through", + async () => { + const root = scratchpadDir(t.root, PID, "default_agent"); + await fs.mkdir(root, { recursive: true }); + // `fs.mkdir(dir, {recursive:true})` succeeds silently on an existing symlink-to-directory, + // so without the realpath check the upload would land in `outside`. + await fs.symlink(outside, path.join(root, LINK_SID), "dir"); + const res = await api.post(`/api/sessions/${LINK_SID}/tasks`, { + input: [{ type: "file", fileName: "escape.txt", dataUrl: dataUrl("bytes") }], + }); + expect(res.status).toBe(500); + expect(await fs.readdir(outside)).toEqual([]); + }, + ); +}); diff --git a/packages/web/src/features/chat/attached-files-banner.tsx b/packages/web/src/features/chat/attached-files-banner.tsx new file mode 100644 index 0000000..909df1b --- /dev/null +++ b/packages/web/src/features/chat/attached-files-banner.tsx @@ -0,0 +1,31 @@ +/** + * Attachment notice for a message's uploaded files: the `[attached file: ]` lines the + * server appends aren't shown verbatim, they collapse into a single line reading + * "Attached files: a.pdf, b.csv" (paperclip icon + static text, no navigation — the files live + * in the session scratchpad and the model opens them by path); the body text around them is + * rendered as usual by the caller. Same shape as SkillsBanner, so the two notices a message + * can carry read as one family. + */ +import { S } from "../../lib/strings"; +import { attachmentFileName } from "../../lib/attachments"; +import { GlyphIcon } from "../../components/ui/glyph-icon"; + +/** Paperclip glyph (24×24 line path), shared with the composer's file-attachment entry. */ +export const PAPERCLIP_ICON = + "M21.4 11.05l-9.19 9.19a6 6 0 0 1-8.49-8.49l9.2-9.19a4 4 0 0 1 5.65 5.66l-9.19 9.19a2 2 0 0 1-2.83-2.83l8.49-8.48"; + +export function AttachedFilesBanner({ files }: { files: string[] }) { + const label = S.chat.attachedFilesBanner(files.map(attachmentFileName)); + return ( + // max-w-full + truncate, the composer chip's rule (truncate max-w-56) applied to a notice + // that has no fixed width of its own: several long names would otherwise wrap the banner + // into a paragraph-tall block above the message. The full list stays reachable as a title. +

+ + {label} +

+ ); +} diff --git a/packages/web/src/features/chat/chat-input.tsx b/packages/web/src/features/chat/chat-input.tsx index b750f62..d4c44f8 100644 --- a/packages/web/src/features/chat/chat-input.tsx +++ b/packages/web/src/features/chat/chat-input.tsx @@ -34,6 +34,12 @@ * the model already in use clears the staging, and both are exclusive with goal mode); a chip is * removed via backspace at the start of the text or its x button, and both are cached with the * draft so they survive a session switch or reload along with the text they belong to; + * The "+" menu carries the input add-ons: image upload, file attachment (any type, several at a + * time — they ride the task request as base64 data URLs, and the server writes them into the + * session scratchpad and appends an `[attached file: ]` line to the message, so the model + * opens them by path), and goal mode; selected files show as removable chips above the text + * body, next to the image thumbnails, and — like images — an attachments-only message is + * sendable with no text at all. * The bottom toolbar provides a searchable multi-select skills dropdown (styled like the model * selector: a top search box filtering by name and localized description, plus a checklist; * clicking a row toggles its selection without closing the menu; the button = book icon + label + @@ -68,7 +74,7 @@ import type { TaskInputPart, } from "@prismshadow/penguin-server/api"; import { S } from "../../lib/strings"; -import { humanizeTokens } from "../../lib/format"; +import { formatBytes, humanizeTokens } from "../../lib/format"; import { resolveContextWindow } from "../../lib/context"; import { useLocale } from "../../state/locale"; import { agentDisplayName } from "../../state/project"; @@ -76,6 +82,7 @@ import { AgentAvatar } from "../../components/ui/agent-avatar"; import { Dropdown } from "../../components/ui/dropdown"; import { GlyphIcon } from "../../components/ui/glyph-icon"; import { noAutofill } from "../../components/ui/input"; +import { toastError } from "../../components/ui/toast"; import { SkillIcon } from "../skills/skill-icon-view"; import { ZoomableImage } from "../../components/ui/image-zoom"; import { ProviderLogo } from "../../components/ui/provider-logo"; @@ -97,6 +104,7 @@ import { skillSlashItems, } from "./skill-use"; import { GOAL_ICON, UNLIMITED_BUDGET, parseBudgetInput } from "./goal-use"; +import { PAPERCLIP_ICON } from "./attached-files-banner"; const APPROVAL_MODES: ApprovalMode[] = ["always-ask", "read-only", "allow-all", "deny-all"]; @@ -1078,6 +1086,46 @@ function ContextGauge({ ); } +/** + * One file attachment staged in the composer. `dataUrl` is the base64 `data:` URL sent as the + * task input's `file` part; `name` / `size` only feed the chip (the server decides the name the + * file actually gets on disk). + */ +interface Attachment { + name: string; + size: number; + dataUrl: string; +} + +/** Mirrors the server's per-file attachment cap (services/task-attachments.ts), so an oversize pick is refused here instead of costing an upload and a 413. */ +const MAX_ATTACHMENT_BYTES = 10 * 1024 * 1024; + +/** Reads one file as a base64 data URL; resolves to null on a read error rather than rejecting, so one unreadable file cannot drop the rest of the batch. */ +function readDataUrl(file: File): Promise { + return new Promise((resolve) => { + const reader = new FileReader(); + reader.onload = () => resolve(typeof reader.result === "string" ? reader.result : null); + reader.onerror = () => resolve(null); + reader.readAsDataURL(file); + }); +} + +/** + * Appends the draft's attachments to a task input — images first (in pick order), then files. + * One place, because every send path submits the same draft: the normal send, the follow-up + * queue, the @ handoff and the `/model` switch. + */ +function appendAttachmentParts( + input: TaskInputPart[], + images: string[], + attachments: Attachment[], +): void { + for (const url of images) input.push({ type: "image_url", imageUrl: url }); + for (const file of attachments) { + input.push({ type: "file", fileName: file.name, dataUrl: file.dataUrl }); + } +} + export function ChatInput({ status, onSend, @@ -1286,6 +1334,10 @@ export function ChatInput({ const textRef = useRef(text); textRef.current = text; const [images, setImages] = useState([]); + // File attachments picked from the "+" menu (any type): held as base64 data URLs, exactly + // like images — a draft has no Session yet, so there is nothing to upload them to ahead of + // time; they travel with the task request and the server files them into the scratchpad. + const [attachments, setAttachments] = useState([]); const [busy, setBusy] = useState(false); const [slashIndex, setSlashIndex] = useState(0); // Slash token start where Escape closed the menu: it stays shut for that one token. @@ -1347,13 +1399,14 @@ export function ChatInput({ canSwitchModel: onSwitchModel !== undefined, sessionBusy: running || compacting, }); - // Sending is also allowed with only a staged switch chip (/agent or /model) or skills selected - // and no text: a handoff's first message may be just a [handoff_from] source block, and the - // empty-text fallbacks fill in the rest (S.chat.skillsAutoMessage with skills selected, - // S.chat.modelSwitchAutoMessage for a staged model switch — see sendNormal). Goal mode instead - // requires a text objective and a parseable budget — and an open editor showing an invalid - // draft disables Send outright: combined with the editor refusing to close over an invalid - // draft (below), no click sequence can fire a goal with a stale committed budget. + // Sending is also allowed with no text at all: attachments (images or files), a staged switch + // chip (/agent or /model) and selected skills each carry a message on their own — a handoff's + // first message may be just a [handoff_from] source block, and the empty-text fallbacks fill in + // the rest (S.chat.skillsAutoMessage with skills selected, S.chat.modelSwitchAutoMessage for a + // staged model switch — see sendNormal). Goal mode instead requires a text objective and a + // parseable budget — and an open editor showing an invalid draft disables Send outright: + // combined with the editor refusing to close over an invalid draft (below), no click sequence + // can fire a goal with a stale committed budget. const canSend = !running && !compacting && @@ -1362,10 +1415,12 @@ export function ChatInput({ (goalOn ? text.trim().length > 0 && images.length === 0 && + attachments.length === 0 && goalBudget !== null && !(goalBudgetOpen && goalBudgetDraftInvalid) : text.trim().length > 0 || images.length > 0 || + attachments.length > 0 || target !== null || pendingModel !== null || selectedSkills.length > 0); @@ -1411,9 +1466,9 @@ export function ChatInput({ }, [goalBudgetDraft]); /** - * Engage/exit goal mode; engaging clears any staged switch chip and the images (genuinely - * exclusive: a handoff or a model switch opens another session, and the server rejects - * non-text goal input). Selected skills stay — they ride the round-1 message as a + * Engage/exit goal mode; engaging clears any staged switch chip and every attachment + * (genuinely exclusive: a handoff or a model switch opens another session, and the server + * rejects non-text goal input). Selected skills stay — they ride the round-1 message as a * [use_skills] block, like a normal send. */ const toggleGoal = useCallback( @@ -1427,19 +1482,20 @@ export function ChatInput({ onHandoffTargetChange?.(null); setPendingModel(null); onPendingModelChange?.(null); - // Images can't ride a goal (the server rejects non-text goal input): clear any already - // attached, or canSend would stay silently false with the objective looking ready. + // Attachments can't ride a goal (the server rejects non-text goal input): clear any + // already attached, or canSend would stay silently false with the objective looking ready. setImages([]); + setAttachments([]); } }, [onHandoffTargetChange, onPendingModelChange], ); // Mid-run steering: while running, Enter/send queues plain text for the running agent - // (delivered between turns as a [user_steering] user message). Text only — images / skills / - // a staged switch stay in the draft for a later normal send (a staged /agent or /model chip - // also blocks steering: the text belongs to the conversation the switch is about to open, not - // to the agent running here). + // (delivered between turns as a [user_steering] user message). Text only — attachments / + // skills / a staged switch stay in the draft for a later normal send (a staged /agent or + // /model chip also blocks steering: the text belongs to the conversation the switch is about + // to open, not to the agent running here). // `!goalOn`: with the goal chip engaged the text is an OBJECTIVE — steering it into a run // that happens to be active (e.g. a schedule fired) would silently repurpose it. const canSteer = @@ -1463,8 +1519,8 @@ export function ChatInput({ localStorage.setItem(STEER_MODE_KEY, mode); }; const followUpMode = steerMode === "followup" && onQueueFollowUp !== undefined; - // A follow-up is a full normal message: the whole draft (text / images / skills / a staged - // switch) is eligible, same content rule as canSend. + // A follow-up is a full normal message: the whole draft (text / attachments / skills / a + // staged switch) is eligible, same content rule as canSend. // `stagedRoute !== "blocked"`: a staged model fork is never eligible mid-run — the follow-up // path composes the whole draft and then hands it to onSwitchModel rather than to the queue, // so without this gate Enter would fork off a Trace that is still being written. @@ -1477,6 +1533,7 @@ export function ChatInput({ stagedRoute !== "blocked" && (text.trim().length > 0 || images.length > 0 || + attachments.length > 0 || target !== null || pendingModel !== null || selectedSkills.length > 0); @@ -1489,6 +1546,7 @@ export function ChatInput({ running && text.trim().length === 0 && images.length === 0 && + attachments.length === 0 && target === null && pendingModel === null && selectedSkills.length === 0; @@ -1823,11 +1881,11 @@ export function ChatInput({ /** * The full normal send path (task / handoff / model switch), also the follow-up queue path * and the fallback target when a steer hits the completion race: assembles the [use_skills] - * block, the images and the staged switch from the whole draft; `post` decides where a - * message that switches nothing goes (default: onSend; follow-up mode: onQueueFollowUp). - * Deliberately not gated on `running` — the caller decides (send() gates the normal path; - * the steering fallback calls this directly after the server said 409 not_running, when the - * local `status` may still lag behind). + * block, the attachments (images and files) and the staged switch from the whole draft; `post` + * decides where a message that switches nothing goes (default: onSend; follow-up mode: + * onQueueFollowUp). Deliberately not gated on `running` — the caller decides (send() gates the + * normal path; the steering fallback calls this directly after the server said 409 + * not_running, when the local `status` may still lag behind). */ // `post` accepts onSend's goal parameter so onSend can be its default; the follow-up queue // (fewer params) is assignable too. Non-goal calls always pass null. @@ -1840,6 +1898,8 @@ export function ChatInput({ // [use_skills] block, exactly like a normal send — the server strips leading marker blocks // when recording the objective, and rounds after the first re-inject the objective alone. if (goalOn) { + // Objective only: attachments were already cleared when goal mode engaged (and blocked + // from being added since), so there is nothing to carry here. setBusy(true); try { const ok = await onSend([{ type: "text", text: buildSkillsMessage(selectedSkills, t) }], { @@ -1882,7 +1942,7 @@ export function ChatInput({ const body = buildSkillsMessage(selectedSkills, bodyText); const input: TaskInputPart[] = []; if (body) input.push({ type: "text", text: body }); - for (const url of images) input.push({ type: "image_url", imageUrl: url }); + appendAttachmentParts(input, images, attachments); setBusy(true); try { const ok = target @@ -1893,10 +1953,11 @@ export function ChatInput({ input, ) : await post(input, null); - // Only clear the draft after a successful send: on failure (network / conflict / server error) keep the user's input and images. + // Only clear the draft after a successful send: on failure (network / conflict / server error) keep the user's input and attachments. if (ok) { setText(""); setImages([]); + setAttachments([]); setTarget(null); setPendingModel(null); setSelectedSkills([]); @@ -2023,12 +2084,51 @@ export function ChatInput({ if (e.target.files) addFiles(e.target.files); e.target.value = ""; }; + + /** + * File attachments (any type, no `accept` filter): read as base64 data URLs, the same + * transport images use — a draft has no Session to upload to yet. The name and size come + * from the File itself and only feed the chip; the server decides the on-disk name. + * + * Oversize files are rejected from `File.size` before anything is read, the same way trace + * import does it (traces-page.tsx): base64-encoding a rejected file in the tab first would + * cost the user a freeze and a 33%-larger upload to earn the same 413. + * + * The whole batch is read before any of it is staged, so the chips — and therefore the + * `[attached file: …]` lines the message ends up with — follow the order the files were + * picked in, not the order the reads happened to finish in. + */ + const addAttachments = (files: Iterable) => { + if (goalOn) return; // goal input is text-only, same rule as images + const picked: File[] = []; + for (const file of files) { + if (file.size > MAX_ATTACHMENT_BYTES) { + toastError(S.chat.attachmentTooLarge(file.name)); + continue; + } + picked.push(file); + } + if (picked.length === 0) return; + void Promise.all(picked.map(readDataUrl)).then((urls) => { + const staged = picked.flatMap((file, i) => + urls[i] ? [{ name: file.name, size: file.size, dataUrl: urls[i]! }] : [], + ); + if (staged.length > 0) setAttachments((prev) => [...prev, ...staged]); + }); + }; + + const onPickAttachments = (e: ChangeEvent) => { + if (e.target.files) addAttachments(e.target.files); + e.target.value = ""; + }; /** * The image picker moved into the "+" menu, so the file input can no longer be a `