Merge pull request #652: fix(uploads): say a refused text upload is not UTF-8 instead of a type mismatch

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

CLAUDE.md

@@ -215,7 +215,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph
 
 **Attachments** (live external document references; all wiring in `file-routes.ts`): a **registry** maps a stable `attachmentId` to a realpath-resolved, extension-allowlisted absolute path, so browser requests never carry arbitrary absolute paths. ⚠️ The **magic-link scanner** (`codeman://attach?...` in terminal output) is **prompt-injectable**, so its scan path is force-confined to the session workspace; a hostile prompt could otherwise exfiltrate arbitrary host files over SSE. The security gate is an extension **allowlist**, not a blocklist. `document-conversion-limiter.ts` caps converter spawns globally: without it, N large docs detected at once fork N multi-minute processes, which is a resource-exhaustion vector. → [architecture-invariants#attachments](docs/architecture-invariants.md#attachments)
 
-**Prompt uploads** (`POST /api/sessions/:id/paste-image`: clipboard paste, drop, the Compose Attach key and the phone toolbar's Attach button; client wiring in `image-input.js` and `keyboard-accessory.js`): a pick is written to `<workspace>/.codeman-uploads/` and its path lands on the prompt as unsent text. The folder is IN the workspace because that is the only path identical for a local and a container agent, FLAT because `<workspace>/.codeman/` would BE the data dir when the workspace is the home directory, and self-ignoring through a `.gitignore` of `*` written ONCE with `wx` (a file already there is the user's and is never overwritten; the user's own ignore file is never touched). ⚠️ The directory names live ONLY in `paste-image-gc.ts` (`UPLOAD_DIR_NAMES`): the pre-move `.claude-images/` receives nothing but stays readable for one release, so the GC sweep and the delete cleanup (through `uploadDirs()`) and the image watcher's ignore filter (reading the names) cover BOTH; retire the legacy entry there, never by hand elsewhere. ⚠️ `uploadDirs()` is async and probes the workspace BOUNDED first (`probePathKind()`, the #516 rule: an `unknown` path is skipped, never touched; the sweep keeps the stall cap and the delete passes `pastCap`), then returns only REAL directories (lstat, so a planted symlink is never one), none that is or contains `getDataDir()` (one strictly inside it only holds uploads and is listed), and none for a remote session, since both consumers delete; the check is made at list time, and the same-user race after it is accepted. Images are buffered in memory under `MAX_PASTE_IMAGE_BYTES` (50MB) after a client-side canvas re-encode; a DOCUMENT (pdf, docx, pptx, xlsx, zip and the text family txt/md/csv/tsv/json/log/yaml/yml/xml; the curated `ALLOWED_DOCUMENT_EXTS`, never `SUPPORTED_ATTACHMENT_EXTENSIONS` or `EDITABLE_EXTENSIONS`, which carry sh/py/js/html) is admitted by its name's extension alone, buffered like an image, judged on its bytes by the same fail-closed `imageMagicMatchesExt` (`%PDF-`, `PK\x03\x04` for the OOXML trio and zip, UTF-8 without a NUL byte for text: a SIGNATURE or ENCODING check, never a structure or content check, so generic zip bytes pass as `.docx` and a script renamed `.txt` is text; the sh/py/js/html exclusion is a SCOPE choice by extension, keeping the server from storing what it would serve or run, not a content filter: the agent can already write any of those) and saved as `paste-<ts>-<hex>-<stem><ext>` with a sanitized stem (`[A-Za-z0-9._-]`, 40 chars, a wholly non-ASCII name leaves no stem; the `paste-` prefix is what the GC matches). A VIDEO is never buffered: `?kind=video` streams it to disk under `MAX_PASTE_VIDEO_BYTES` (1GB, env `CODEMAN_MAX_PASTE_VIDEO_BYTES`), judges the container on the first 12 bytes as they arrive (ISO BMFF / QuickTime atom, EBML for webm, Ogg for ogv; a mislabeled body is drained unwritten from there, never parked on disk first) and unlinks on any failure, a dead transfer included. ⚠️ The flag is the ONLY way onto that path and admits video types alone (the registry's `VIDEO_ATTACHMENT_EXTENSIONS`, single-sourced: mp4/webm/mov/m4v/ogv, so an uploaded video is one the file viewer can also play; 3gp/mkv are out, since a phone's camera roll produces neither and neither previews), so an image can never ride the larger cap through memory; without it the route is byte-identical to the image-only version. Busboy takes its size limit before the part is seen, which is why the choice rides the URL and not the file. ⚠️ A video draws on its OWN per-(IP, session) rate bucket (`consumeVideoToken`, 3/min) beside the image bucket (`consumePasteToken`, 30/min): at 1GB each, 30/min would be 30GB of disk a minute, so the lower rate bounds it to 3GB; the image path keeps the 30/min bucket and is unchanged without the flag. A remote (SSH) session is refused (400) before any disk touch: its `workingDir` is the remote path, so the file would land on THIS host where the agent cannot read it (Docker is fine, the workspace is bind-mounted at the same absolute path). ⚠️ A live session's workspace can still sit on a network mount that stopped answering, so the route probes it BOUNDED (`probePathKind(session.workingDir, { pastCap: true })`, the #516 rule) before the first disk touch and refuses an `unknown` mount through `describeUnknownPath()` (OPERATION_FAILED), the way session create does, so a mount already unreachable cannot freeze a libuv worker on every upload (a mount that dies after the probe is the same residual race create accepts). ⚠️ Dispatch mirrors the client: an `image/*` MIME takes the IMAGE path whatever the name says (a GIF named `animation.txt` stays a GIF), a document extension takes the document branch, and anything else takes the image path exactly as before, where the HEIC sniff still recovers a HEIC under a generic name; a foreign extension with no image MIME gets its `Unsupported file type` 400 only AFTER that sLine truncated
+**Prompt uploads** (`POST /api/sessions/:id/paste-image`: clipboard paste, drop, the Compose Attach key and the phone toolbar's Attach button; client wiring in `image-input.js` and `keyboard-accessory.js`): a pick is written to `<workspace>/.codeman-uploads/` and its path lands on the prompt as unsent text. The folder is IN the workspace because that is the only path identical for a local and a container agent, FLAT because `<workspace>/.codeman/` would BE the data dir when the workspace is the home directory, and self-ignoring through a `.gitignore` of `*` written ONCE with `wx` (a file already there is the user's and is never overwritten; the user's own ignore file is never touched). ⚠️ The directory names live ONLY in `paste-image-gc.ts` (`UPLOAD_DIR_NAMES`): the pre-move `.claude-images/` receives nothing but stays readable for one release, so the GC sweep and the delete cleanup (through `uploadDirs()`) and the image watcher's ignore filter (reading the names) cover BOTH; retire the legacy entry there, never by hand elsewhere. ⚠️ `uploadDirs()` is async and probes the workspace BOUNDED first (`probePathKind()`, the #516 rule: an `unknown` path is skipped, never touched; the sweep keeps the stall cap and the delete passes `pastCap`), then returns only REAL directories (lstat, so a planted symlink is never one), none that is or contains `getDataDir()` (one strictly inside it only holds uploads and is listed), and none for a remote session, since both consumers delete; the check is made at list time, and the same-user race after it is accepted. Images are buffered in memory under `MAX_PASTE_IMAGE_BYTES` (50MB) after a client-side canvas re-encode; a DOCUMENT (pdf, docx, pptx, xlsx, zip and the text family txt/md/csv/tsv/json/log/yaml/yml/xml; the curated `ALLOWED_DOCUMENT_EXTS`, never `SUPPORTED_ATTACHMENT_EXTENSIONS` or `EDITABLE_EXTENSIONS`, which carry sh/py/js/html) is admitted by its name's extension alone, buffered like an image, judged on its bytes by the same fail-closed `imageMagicMatchesExt` (`%PDF-`, `PK\x03\x04` for the OOXML trio and zip, UTF-8 without a NUL byte for text: a SIGNATURE or ENCODING check, never a structure or content check, so generic zip bytes pass as `.docx` and a script renamed `.txt` is text; the sh/py/js/html exclusion is a SCOPE choice by extension, keeping the server from storing what it would serve or run, not a content filter: the agent can already write any of those) and saved as `paste-<ts>-<hex>-<stem><ext>` with a sanitized stem (`[A-Za-z0-9._-]`, 40 chars, a wholly non-ASCII name leaves no stem; the `paste-` prefix is what the GC matches). A VIDEO is never buffered: `?kind=video` streams it to disk under `MAX_PASTE_VIDEO_BYTES` (1GB, env `CODEMAN_MAX_PASTE_VIDEO_BYTES`), judges the container on the first 12 bytes as they arrive (ISO BMFF / QuickTime atom, EBML for webm, Ogg for ogv; a mislabeled body is drained unwritten from there, never parked on disk first) and unlinks on any failure, a dead transfer included. ⚠️ The flag is the ONLY way onto that path and admits video types alone (the registry's `VIDEO_ATTACHMENT_EXTENSIONS`, single-sourced: mp4/webm/mov/m4v/ogv, so an uploaded video is one the file viewer can also play; 3gp/mkv are out, since a phone's camera roll produces neither and neither previews), so an image can never ride the larger cap through memory; without it the route is byte-identical to the image-only version. Busboy takes its size limit before the part is seen, which is why the choice rides the URL and not the file. ⚠️ A video draws on its OWN per-(IP, session) rate bucket (`consumeVideoToken`, 3/min) beside the image bucket (`consumePasteToken`, 30/min): at 1GB each, 30/min would be 30GB of disk a minute, so the lower rate bounds it to 3GB; the image path keeps the 30/min bucket and is unchanged without the flag. A remote (SSH) session is refused (400) before any disk touch: its `workingDir` is the remote path, so the file would land on THIS host where the agent cannot read it (Docker is fine, the workspace is bind-mounted at the same absolute path). ⚠️ A live session's workspace can still sit on a network mount that stopped answering, so the route probes it BOUNDED (`probePathKind(session.workingDir, { pastCap: true })`, the #516 rule) before the first disk touch and refuses an `unknown` mount through `describeUnknownPath()` (OPERATION_FAILED), the way session create does, so a mount already unreachable cannot freeze a libuv worker on every upload (a mount that dies after the probe is the same residual race create accepts). ⚠️ Dispatch mirrors the client: an `image/*` MIME takes the IMAGE path whatever the name says (a GIF named `animation.txt` stays a GIF), a document extension takes the document branch, and anything else takes the image path exactly as before, where the HEIC sniff still recovers a HEIC under a generic name; a foreign extension with no image MIME gets its `Unsupported file type` 400 only AFTER that sLine truncated
 
 **File-path links (terminal + chat)**: a path an agent prints is clickable on BOTH surfaces and opens the file-preview overlay. ⚠️ ONE pattern (`FILE_PATH_LINK_PATTERN` / `absoluteFilePathPattern()` in constants.js) feeds the xterm link provider AND `_linkifyFilePaths()`, a fresh instance per call (`lastIndex`). Both call `findFilePathLinks()`, which adds `~/`, relative and folder paths without overlapping a rooted match or a URL; a relative path resolves against `workingDir` (`resolveLinkedFilePath`), `~/` only server-side on the session's host (attachment route), and `terminalPathLine()` joins a path a program hard-wrapped short of the edge. ⚠️ A folder link opens the **folder grid**: `POST /api/sessions/:id/folders` guards + GRANTS the folder (`folder-listing.ts`, own registry, never the 200-capped attachment registry), files come by bare name via `…/folders/:folderId/files/:name/*` through `resolveServableAttachmentPath` plus the same extension/regular-file gate on the symlink's resolved target, so no browser request carries a path; `~/` is refused in a Docker case (the container's home is not this host's). The chat linkifier walks TEXT NODES with DOM APIs, never rebuilds sanitized markup as a string. ⚠️ An out-of-workspace path goes through the ATTACHMENT routes (`POST /api/sessions/:id/attachments` with `notify: false`), never by widening `file-content`/`file-raw` or `file-stream-manager`'s `tail -f` allowlist. ⚠️ `TEXT_ATTACHMENT_EXTENSIONS` IS `EDITABLE_EXTENSIONS` (never a second list), and widening READ must never widen RUN: `html`/`htm`/`svg` stay download-only, other text is inert `text/plain`+`nosniff`. ⚠️ The ONE place HTML renders is `/html-view/<cap>/...` (`html-view-routes.ts`, `html-view-capabilities.ts`): `POST /api/sessions/:id/html-view` mints a capability for the file's own directory, every response carries `Content-Security-Policy: sandbox` WITHOUT `allow-same-origin` (opaque origin even as a top-level tab, so the page can never read Codeman or drive its API) and with NO fetch directive (open network, owner decision 2026-10-13: never restrict it or add `allow-same-origin` without the owner), and only non-dot, asset-allowlisted files under that directory are served (GET/HEAD only; capabilities revoked with the web-tab ones on logout). That tree is served recursively, so minting is refused for a hidden directory or a broad root (`isBroadHtmlViewRoot`); the preview renders by default and the **Page** pill (or a refused mint) falls through to the source view and its Edit button. Never serve HTML same-origin anywhere else. Tests: `test/routes/html-view-routes.test.ts`. Media extensions are single-sourced in `attachment-registry.ts`. ⚠️ **XLSX previews parse in the BROWSER**, never on the server; the admission limits that keep a hostile workbook from freezing the worker live in `src/web/CLAUDE.md` (Spreadsheet preview). → [architecture-invariants#file-path-links-terminal--response-viewer](docs/architecture-invariants.md#file-path-links-terminal--response-viewer)
 

docs/api-reference.md

@@ -626,7 +626,9 @@ or `.csv` is unreliable): the office formats `.pdf`, `.docx`, `.pptx`, `.xlsx`,
 `.zip` and the text family `.txt`, `.md`, `.csv`, `.tsv`, `.json`, `.log`,
 `.yaml`, `.yml`, `.xml`. A document is buffered under the same 50 MB image cap,
 its bytes are checked against the extension (a magic-number signature, or UTF-8
-with no NUL byte for text) and answered 415 on a mismatch, and it is saved under
+with no NUL byte for text) and answered 415 on a mismatch (for text, an `error`
+saying the file is not UTF-8 text, or UTF-16 text when it opens with a UTF-16
+byte order mark, and to save it as UTF-8), and it is saved under
 a sanitized stem of its name, so `Q3 report.pdf` is findable as
 `paste-<ms>-<hex>-Q3-report.pdf`. A named type that is neither an image nor a
 document (a script, an HTML page, a `.sh`) is refused 400 `Unsupported file

docs/wiki/Working-With-Files.md

@@ -184,13 +184,15 @@ through server memory. Documents are the office formats (pdf, docx, pptx, xlsx,
 text family (txt, md, csv, tsv, json, log, yaml, yml, xml). Each is checked on the way in,
 against its file signature for the binary formats and against its encoding for text (UTF-8,
 no NUL bytes), so a script renamed to `.pdf` is refused while a `.txt` can hold any text at
-all. Files with script or page extensions (sh, py, js, html) are not accepted, since a prompt
-upload is something for the agent to read, not something for Codeman to store and serve. A
-document keeps a trimmed copy of its original name, letters, digits, dot, dash and underscore
-with anything else turned into a dash, so `Q3 report.pdf` is findable as
-`paste-<time>-<id>-Q3-report.pdf`; a name made entirely of other characters saves without
-one. Uploads are refused for remote (SSH) sessions, whose working directory is on the other
-host.
+all. A text file in another encoding is refused with a message saying it is not UTF-8 (or that
+it is UTF-16, which Excel's "Unicode Text" and Notepad's "Unicode" produce); save it as UTF-8
+(in Excel, "CSV UTF-8") and attach it again. Files with script or page extensions (sh, py,
+js, html) are not accepted, since a prompt upload is something for the agent to read, not
+something for Codeman to store and serve. A document keeps a trimmed copy of its original
+name, letters, digits, dot, dash and underscore with anything else turned into a dash, so
+`Q3 report.pdf` is findable as `paste-<time>-<id>-Q3-report.pdf`; a name made entirely of
+other characters saves without one. Uploads are refused for remote (SSH) sessions, whose
+working directory is on the other host.
 
 Uploads land in `.codeman-uploads/` inside the case, next to the code the agent is working
 on, because that is the one path a local agent and a Docker case (whose workspace is mounted

src/web/routes/session-routes.ts

@@ -5683,14 +5683,24 @@ export function registerSessionRoutes(
       if (!imageMagicMatchesExt(imageBytes, origExt)) {
         // The first bytes of a text file are user content, so for text the log
         // names the failure and never the bytes; a signature is safe to print.
-        const detail = TEXT_UPLOAD_EXTS.has(origExt)
-          ? `reason=${imageBytes.includes(0) ? 'nul-byte' : 'not-utf8'}`
+        // A text refusal names the encoding, since an Excel or Notepad export
+        // (UTF-16 with a BOM, or Windows-1252) is not a corrupt file.
+        const isText = TEXT_UPLOAD_EXTS.has(origExt);
+        const bom = imageBytes.length >= 2 ? imageBytes.readUInt16BE(0) : 0;
+        const utf16 = bom === 0xfffe || bom === 0xfeff;
+        const detail = isText
+          ? `reason=${utf16 ? 'utf16-bom' : imageBytes.includes(0) ? 'nul-byte' : 'not-utf8'}`
           : `magic=${imageBytes.subarray(0, 12).toString('hex')}`;
         console.warn(
           `[paste-image] document mismatch: filename=${JSON.stringify(part.filename)} mime=${JSON.stringify(part.mimetype)} declaredExt=${origExt} size=${imageBytes.length} ${detail}`
         );
         reply.code(415);
-        return createErrorResponse(ApiErrorCode.INVALID_INPUT, `File bytes do not match declared type ${origExt}`);
+        return createErrorResponse(
+          ApiErrorCode.INVALID_INPUT,
+          isText
+            ? `This ${origExt} file is ${utf16 ? 'UTF-16 text, not UTF-8' : 'not UTF-8 text'}; save it as UTF-8 and attach it again`
+            : `File bytes do not match declared type ${origExt}`
+        );
       }
       const saved = await writeBufferedUpload(
         session.workingDir,

test/routes/session-routes.test.ts

@@ -17,7 +17,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
 import Fastify, { type FastifyInstance } from 'fastify';
 import fastifyCookie from '@fastify/cookie';
 import fastifyMultipart from '@fastify/multipart';
-import { dirname, join } from 'node:path';
+import { dirname, extname, join } from 'node:path';
 import { existsSync, mkdirSync, rmSync, writeFileSync } from 'node:fs';
 import { registryFilePath, reloadCliRegistry } from '../../src/config/cli-registry/registry.js';
 import fs, { mkdtemp, rm, mkdir, writeFile, readFile, readdir, symlink } from 'node:fs/promises';
@@ -858,12 +858,39 @@ describe('session-routes', () => {
       for (const [name, bytes] of [
         ['fake.pdf', png], // PNG bytes under a PDF name
         ['fake.docx', pdf], // a PDF under an OOXML name
-        ['binary.txt', Buffer.from('hello\u0000world')], // a NUL byte is never text
-        ['latin1.csv', Buffer.from([0x61, 0x2c, 0xe9, 0x0a])], // not UTF-8
       ] as const) {
         const res = await upload(name, 'application/octet-stream', bytes);
         expect(res.statusCode, name).toBe(415);
-        expect(JSON.parse(res.body).error, name).toMatch(/do not match declared type/);
+        expect(JSON.parse(res.body).error, name).toBe(`File bytes do not match declared type ${extname(name)}`);
+      }
+      expect(await readdir(workDir)).toEqual([]);
+      await rm(workDir, { recursive: true });
+    });
+
+    it('refuses non-UTF-8 text by naming the encoding, UTF-16 when a BOM says so', async () => {
+      const workDir = await mkdtemp(join(tmpdir(), 'codeman-doc-'));
+      harness.ctx._session.workingDir = workDir;
+      const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+      const notUtf8 = (ext: string) => `This ${ext} file is not UTF-8 text; save it as UTF-8 and attach it again`;
+      const utf16 = (ext: string) => `This ${ext} file is UTF-16 text, not UTF-8; save it as UTF-8 and attach it again`;
+      try {
+        for (const [name, bytes, error, reason] of [
+          // Excel "Unicode Text" / Notepad "Unicode": UTF-16LE with its BOM.
+          ['export.txt', Buffer.from('\ufeffa\tb\n', 'utf16le'), utf16('.txt'), 'utf16-bom'],
+          ['export.csv', Buffer.from([0xfe, 0xff, 0x00, 0x61, 0x00, 0x0a]), utf16('.csv'), 'utf16-bom'], // UTF-16BE
+          ['binary.txt', Buffer.from('hello\u0000world'), notUtf8('.txt'), 'nul-byte'], // NUL, no BOM
+          ['latin1.csv', Buffer.from('a,René\n', 'latin1'), notUtf8('.csv'), 'not-utf8'], // Excel's default CSV
+        ] as const) {
+          warn.mockClear();
+          const res = await upload(name, 'application/octet-stream', bytes);
+          expect(res.statusCode, name).toBe(415);
+          expect(JSON.parse(res.body).error, name).toBe(error);
+          expect(String(warn.mock.calls[0]?.[0]), name).toMatch(
+            new RegExp(`declaredExt=\\S+ size=\\d+ reason=${reason}$`)
+          );
+        }
+      } finally {
+        warn.mockRestore();
       }
       expect(await readdir(workDir)).toEqual([]);
       await rm(workDir, { recursive: true });