fix(preview): normalize entry names the way ExcelJS sees them, skip defined names, pin fflate 0.8.3

Admission checked ZIP entry names as stored, but JSZip (inside ExcelJS)
resolves `.`, `..` and empty segments on load, and ExcelJS strips one
leading `/` and matches worksheets with an unanchored pattern. Names like
`/xl/worksheets/sheet1.xml` or `xl/worksheets/sheet1.xml.x` skipped every
counter. Admission now computes the name ExcelJS will see for each entry,
refuses two entries that resolve to the same name, keys the rebuilt
archive on it, and picks the worksheet/styles counters from it.

ExcelJS's DefinedNames model setter expands every range into one object
per cell. The preview never shows defined names, so the worker stubs
`_definedNames.model` before load.

Pin fflate to 0.8.3 (GHSA-px8p-9vwx-vf98) and refresh
SPREADSHEET_ASSET_VERSION.
This commit is contained in:
Aamer Akhter
2026-10-03 08:21:54 -04:00
parent d65ee4f89d
commit 2d0ffb71aa
12 changed files with 195 additions and 18 deletions
+1 -1
View File
@@ -289,7 +289,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) **Attachments** (live external document references; all wiring in `file-routes.ts`): a **registry** maps a stable `attachmentId` to a realpath-resolved, extension-allowlisted absolute path, so browser requests never carry arbitrary absolute paths. ⚠️ The **magic-link scanner** (`codeman://attach?...` in terminal output) is **prompt-injectable**, so its scan path is force-confined to the session workspace; a hostile prompt could otherwise exfiltrate arbitrary host files over SSE. The security gate is an extension **allowlist**, not a blocklist. `document-conversion-limiter.ts` caps converter spawns globally: without it, N large docs detected at once fork N multi-minute processes, which is a resource-exhaustion vector. → [architecture-invariants#attachments](docs/architecture-invariants.md#attachments)
**File-path links (terminal + chat)**: a path an agent prints is clickable on BOTH surfaces and opens the file-preview overlay. ⚠️ ONE pattern (`FILE_PATH_LINK_PATTERN` / `absoluteFilePathPattern()` in constants.js) feeds the xterm link provider AND `_linkifyFilePaths()`, a fresh instance per call (`lastIndex`). The chat linkifier walks TEXT NODES with DOM APIs, never rebuilds sanitized markup as a string. ⚠️ An out-of-workspace path goes through the ATTACHMENT routes (`POST /api/sessions/:id/attachments` with `notify: false`), never by widening `file-content`/`file-raw` or `file-stream-manager`'s `tail -f` allowlist. ⚠️ `TEXT_ATTACHMENT_EXTENSIONS` IS `EDITABLE_EXTENSIONS` (never a second list), and widening READ must never widen RUN: `html`/`htm`/`svg` stay download-only, other text is inert `text/plain`+`nosniff`. Media extensions are single-sourced in `attachment-registry.ts`. ⚠️ **XLSX previews parse in the BROWSER**, never on the server: `spreadsheet-preview.js` fetches the raw route with `?preview=true` (413 above `MAX_XLSX_BROWSER_PREVIEW_BYTES`, 10 MB) and hands the bytes to `spreadsheet-preview-worker.js`, the only place the pinned `exceljs`/`fflate` vendor bundles load (never on page load). `admitXlsx()` caps the ZIP before ExcelJS runs, counting what ExcelJS will EXPAND as well as what it reads (a merge costs its area, a `<col>` past 16384 is refused, validations are never parsed), and ExcelJS parses only a STORE-only archive rebuilt from the entries admission inflated (`buildAdmittedArchive()`), never the fetched bytes; cell text goes through `textContent`, formulas are never evaluated. Bumping either package or editing the worker/core changes `SPREADSHEET_ASSET_VERSION`, which `npm run check:public-assets` pins. xls/ods stay download-only. → [architecture-invariants#file-path-links-terminal--response-viewer](docs/architecture-invariants.md#file-path-links-terminal--response-viewer) **File-path links (terminal + chat)**: a path an agent prints is clickable on BOTH surfaces and opens the file-preview overlay. ⚠️ ONE pattern (`FILE_PATH_LINK_PATTERN` / `absoluteFilePathPattern()` in constants.js) feeds the xterm link provider AND `_linkifyFilePaths()`, a fresh instance per call (`lastIndex`). The chat linkifier walks TEXT NODES with DOM APIs, never rebuilds sanitized markup as a string. ⚠️ An out-of-workspace path goes through the ATTACHMENT routes (`POST /api/sessions/:id/attachments` with `notify: false`), never by widening `file-content`/`file-raw` or `file-stream-manager`'s `tail -f` allowlist. ⚠️ `TEXT_ATTACHMENT_EXTENSIONS` IS `EDITABLE_EXTENSIONS` (never a second list), and widening READ must never widen RUN: `html`/`htm`/`svg` stay download-only, other text is inert `text/plain`+`nosniff`. Media extensions are single-sourced in `attachment-registry.ts`. ⚠️ **XLSX previews parse in the BROWSER**, never on the server: `spreadsheet-preview.js` fetches the raw route with `?preview=true` (413 above `MAX_XLSX_BROWSER_PREVIEW_BYTES`, 10 MB) and hands the bytes to `spreadsheet-preview-worker.js`, the only place the pinned `exceljs`/`fflate` vendor bundles load (never on page load). `admitXlsx()` caps the ZIP before ExcelJS runs, counting what ExcelJS will EXPAND as well as what it reads (a merge costs its area, a `<col>` past 16384 is refused, validations are never parsed via `ignoreNodes`, and defined names are never expanded: the worker stubs `_definedNames.model`). ⚠️ Admission keys every entry on the name ExcelJS will SEE (`excelJsEntryName()`: JSZip's `.`/`..`/empty-segment resolution, then one leading `/` stripped), refuses two entries that land on one name, and treats anything matching ExcelJS's UNANCHORED `xl/worksheets/sheet<N>.xml` as a worksheet, so `/xl/...` or `xl/./...` cannot skip a counter, and ExcelJS parses only a STORE-only archive rebuilt from the entries admission inflated (`buildAdmittedArchive()`), never the fetched bytes; cell text goes through `textContent`, formulas are never evaluated. Bumping either package or editing the worker/core changes `SPREADSHEET_ASSET_VERSION`, which `npm run check:public-assets` pins. xls/ods stay download-only. → [architecture-invariants#file-path-links-terminal--response-viewer](docs/architecture-invariants.md#file-path-links-terminal--response-viewer)
**Filesystem path picker** (Link Existing "Browse" + the mobile keyboard's `📁 Path` key): lazy one-directory browsing via `GET /api/filesystem/browse`, with `GET /api/filesystem/preview` for the tapped file. Inserts the path **without** Enter, so the prompt is never submitted; the sibling `⌫ All` key clears only the unsent prompt and must never send the agent's `/clear`. ⚠️ This is a **second file-serving surface and inherits neither the attachment confinement nor its ownership scoping** — it allowlists Home, `CASES_DIR`, `/mnt/d` and `CODEMAN_FILE_PICKER_ROOTS`, blocks sensitive trees, and rejects symlink escapes **after** `realpath`. ⚠️ The optional `sessionId` is an ownership boundary that must be `canAccessOwned`-checked by hand (it does not go through `findSessionOrFail`), and in multi-user mode a non-admin gets only their own `userSpacePath` as a root: per-user spaces live INSIDE `homedir()`, so a `Home` root exposes every other user's workspace. Previews go through the same global conversion limiter, and Markdown/TXT/JSON are served as inert `text/plain`. → [architecture-invariants#filesystem-path-picker](docs/architecture-invariants.md#filesystem-path-picker) **Filesystem path picker** (Link Existing "Browse" + the mobile keyboard's `📁 Path` key): lazy one-directory browsing via `GET /api/filesystem/browse`, with `GET /api/filesystem/preview` for the tapped file. Inserts the path **without** Enter, so the prompt is never submitted; the sibling `⌫ All` key clears only the unsent prompt and must never send the agent's `/clear`. ⚠️ This is a **second file-serving surface and inherits neither the attachment confinement nor its ownership scoping** — it allowlists Home, `CASES_DIR`, `/mnt/d` and `CODEMAN_FILE_PICKER_ROOTS`, blocks sensitive trees, and rejects symlink escapes **after** `realpath`. ⚠️ The optional `sessionId` is an ownership boundary that must be `canAccessOwned`-checked by hand (it does not go through `findSessionOrFail`), and in multi-user mode a non-admin gets only their own `userSpacePath` as a root: per-user spaces live INSIDE `homedir()`, so a `Home` root exposes every other user's workspace. Previews go through the same global conversion limiter, and Markdown/TXT/JSON are served as inert `text/plain`. → [architecture-invariants#filesystem-path-picker](docs/architecture-invariants.md#filesystem-path-picker)
+1 -1
View File
@@ -362,7 +362,7 @@ A file path an agent prints is a link on both surfaces it can appear on, and cli
⚠️ **The preview overlay must outrank the panel that launched it.** `.file-preview-overlay` sits at `z-index: 5100`, above the response viewer (5000) and its backdrop (4999); at its historical 2000 a path clicked in the chat opened the overlay *behind* the chat, which reads as a dead link. It stays below the toast/picker band (10000+) so a "Saved" toast still lands on top. ⚠️ **The preview overlay must outrank the panel that launched it.** `.file-preview-overlay` sits at `z-index: 5100`, above the response viewer (5000) and its backdrop (4999); at its historical 2000 a path clicked in the chat opened the overlay *behind* the chat, which reads as a dead link. It stays below the toast/picker band (10000+) so a "Saved" toast still lands on top.
**XLSX previews parse in the browser worker, and ExcelJS only ever sees what admission checked.** An `.xlsx` joins the raw routes like any other file (`?preview=true` caps it at 10 MB with a 413); the server never parses it. `spreadsheet-preview.js` hands the bytes to `spreadsheet-preview-worker.js`, the ONLY place the pinned `exceljs`/`fflate` vendor bundles load: fflate and `spreadsheet-xlsx-core.js` at worker start, ExcelJS only after `admitXlsx()` passes, and the page itself never loads either. `admitXlsx()` streams every entry through fflate and enforces the entry count, per-entry and total inflated bytes (64 MB), compression ratio, worksheet, cell, merge and style caps on the actual inflated output, refusing (never truncating) a workbook that trips one. ⚠️ Admission walks LOCAL headers while ExcelJS (JSZip) reads the CENTRAL directory, so overlapping entries (a stored entry hiding a whole `sheet1.xml`, a one-cell decoy later in the stream) once let a file be admitted as 1 cell and parsed as 300k. The worker therefore never hands ExcelJS the fetched bytes: it gets `buildAdmittedArchive()`, a STORE-only `fflate.zipSync(entries, { level: 0 })` of exactly the entries admission inflated, and a name streamed twice is refused. That costs one transient copy of the inflated entries (bounded by the same 64 MB cap) and is pinned by the overlapping-entry fixture in `test/spreadsheet-preview-worker.test.ts`. ⚠️ Admission also has to bound what ExcelJS EXPANDS, not just what it reads: ExcelJS 4.4.0 builds one object per covered cell of a `<mergeCell>`, per address of a `<dataValidation sqref>` and per index up to `<col max>`, so a 6.5 KB file admitted as one cell once cost a gigabyte. The counter therefore charges each merge its full AREA against the cell caps (an unparseable `ref` is refused), refuses a `<col>` whose `min`/`max` is past 16384, and the worker loads with `ignoreNodes: ['dataValidations']` (the preview never shows validations). The counter reads `<mergeCell>`/`<col>` attributes IN ORDER from the tag name with a sticky regex that consumes each quoted value whole (a raw `>` or the other quote character is legal inside a value, so a first-match search could be fed a fake `ref`/`max`), and refuses a tag whose attributes do not parse up to `>` or repeat a name. It counts every `<row>` (ExcelJS keeps one Row object per element, cells or not) against per-sheet and total row caps, and every `<xf>` in styles.xml with no list-tracking state (a `</cellXfs>` inside a comment used to desync it). It carries everything from the last `<` into the next inflate chunk, since `<` can never appear inside an attribute value, and a central-directory `compressedSize` that runs past the file is refused, since the ratio cap divides by it. Fixtures for each are in `test/spreadsheet-preview-worker.test.ts` and `test/spreadsheet-xlsx-core.test.ts`. The worker is a stable URL cache-busted by `SPREADSHEET_ASSET_VERSION` in `spreadsheet-preview.js`, the content hash of the worker, the core and both vendor bundles; `npm run check:public-assets` fails when it drifts, so editing any of those (or bumping either package) means updating the token, or a deploy pairs a new worker with a year-cached old core. **XLSX previews parse in the browser worker, and ExcelJS only ever sees what admission checked.** An `.xlsx` joins the raw routes like any other file (`?preview=true` caps it at 10 MB with a 413); the server never parses it. `spreadsheet-preview.js` hands the bytes to `spreadsheet-preview-worker.js`, the ONLY place the pinned `exceljs`/`fflate` vendor bundles load: fflate and `spreadsheet-xlsx-core.js` at worker start, ExcelJS only after `admitXlsx()` passes, and the page itself never loads either. `admitXlsx()` streams every entry through fflate and enforces the entry count, per-entry and total inflated bytes (64 MB), compression ratio, worksheet, cell, merge and style caps on the actual inflated output, refusing (never truncating) a workbook that trips one. ⚠️ Admission walks LOCAL headers while ExcelJS (JSZip) reads the CENTRAL directory, so overlapping entries (a stored entry hiding a whole `sheet1.xml`, a one-cell decoy later in the stream) once let a file be admitted as 1 cell and parsed as 300k. The worker therefore never hands ExcelJS the fetched bytes: it gets `buildAdmittedArchive()`, a STORE-only `fflate.zipSync(entries, { level: 0 })` of exactly the entries admission inflated, and a name streamed twice is refused. That costs one transient copy of the inflated entries (bounded by the same 64 MB cap) and is pinned by the overlapping-entry fixture in `test/spreadsheet-preview-worker.test.ts`. ⚠️ Admission also has to bound what ExcelJS EXPANDS, not just what it reads: ExcelJS 4.4.0 builds one object per covered cell of a `<mergeCell>`, per address of a `<dataValidation sqref>` and per index up to `<col max>`, so a 6.5 KB file admitted as one cell once cost a gigabyte. The counter therefore charges each merge its full AREA against the cell caps (an unparseable `ref` is refused), refuses a `<col>` whose `min`/`max` is past 16384, and the worker loads with `ignoreNodes: ['dataValidations']` (the preview never shows validations). Defined names expand the same way and live in `xl/workbook.xml`, which admission never scans: ExcelJS's `DefinedNames` model setter builds one object per cell of every range (a whole-sheet name exhausted a 4 GB heap), so right after `new ExcelJS.Workbook()` the worker replaces `_definedNames.model` with an `Object.defineProperty` stub whose getter returns `[]` and whose setter drops the value. Print areas and titles are split off in the workbook xform's reconcile, before that setter runs, so they are unaffected, and `defineProperty` throws if a future ExcelJS renames `_definedNames` rather than silently expanding again. ⚠️ Every name admission uses is the one ExcelJS will SEE, not the stored one: JSZip resolves each entry name on load (`utils.resolve`: `.` and empty middle segments dropped, `..` pops a segment) and ExcelJS then strips one leading `/` and matches worksheets with an UNANCHORED `xl/worksheets/sheet<N>.xml`. Checking the stored name once let `/xl/worksheets/sheet1.xml`, `xl/./...`, `xl//...`, `xl/xl/worksheets/sheet1.xml` and `xl/worksheets/sheet1.xml.x` skip every counter. `excelJsEntryName()` mirrors both steps; admission refuses two entries that resolve to one name, keys `inflatedEntries` (so the rebuilt archive) on the resolved name, picks the worksheet and styles counters from it, and counts any name matching ExcelJS's unanchored worksheet pattern as a worksheet. The local-versus-central consistency checks still compare the stored names. The counter reads `<mergeCell>`/`<col>` attributes IN ORDER from the tag name with a sticky regex that consumes each quoted value whole (a raw `>` or the other quote character is legal inside a value, so a first-match search could be fed a fake `ref`/`max`), and refuses a tag whose attributes do not parse up to `>` or repeat a name. It counts every `<row>` (ExcelJS keeps one Row object per element, cells or not) against per-sheet and total row caps, and every `<xf>` in styles.xml with no list-tracking state (a `</cellXfs>` inside a comment used to desync it). It carries everything from the last `<` into the next inflate chunk, since `<` can never appear inside an attribute value, and a central-directory `compressedSize` that runs past the file is refused, since the ratio cap divides by it. Fixtures for each are in `test/spreadsheet-preview-worker.test.ts` and `test/spreadsheet-xlsx-core.test.ts`. The worker is a stable URL cache-busted by `SPREADSHEET_ASSET_VERSION` in `spreadsheet-preview.js`, the content hash of the worker, the core and both vendor bundles; `npm run check:public-assets` fails when it drifts, so editing any of those (or bumping either package) means updating the token, or a deploy pairs a new worker with a year-cached old core.
### Filesystem path picker ### Filesystem path picker
+1
View File
@@ -17,6 +17,7 @@ It renders what it can:
| Markdown | Rendered by default: headings, tables, code blocks with copy buttons, images and links relative to the file (root-relative ones resolve from the workspace root, as on GitHub). Opened from an attachment card, where the file's folder is unknown, relative images show their alt text and relative links show as plain text. The MD pill in the header flips to source. | | Markdown | Rendered by default: headings, tables, code blocks with copy buttons, images and links relative to the file (root-relative ones resolve from the workspace root, as on GitHub). Opened from an attachment card, where the file's folder is unknown, relative images show their alt text and relative links show as plain text. The MD pill in the header flips to source. |
| Images | Inline. | | Images | Inline. |
| Audio and video | Inline with a working scrub bar, because range requests are supported. | | Audio and video | Inline with a working scrub bar, because range requests are supported. |
| Spreadsheets (`.xlsx`) | Read-only grid, parsed in your browser (never on the server), up to 10 MB. `.xls` and `.ods` are download only. |
| PDF and Office documents | Converted for preview when a converter is available. | | PDF and Office documents | Converted for preview when a converter is available. |
| Anything else | Download. | | Anything else | Download. |
+4 -4
View File
@@ -60,7 +60,7 @@
"esbuild": "^0.27.3", "esbuild": "^0.27.3",
"eslint": "^9.0.0", "eslint": "^9.0.0",
"exceljs": "4.4.0", "exceljs": "4.4.0",
"fflate": "0.8.2", "fflate": "0.8.3",
"pixelmatch": "^6.0.0", "pixelmatch": "^6.0.0",
"playwright": "^1.58.0", "playwright": "^1.58.0",
"pngjs": "^7.0.0", "pngjs": "^7.0.0",
@@ -7149,9 +7149,9 @@
} }
}, },
"node_modules/fflate": { "node_modules/fflate": {
"version": "0.8.2", "version": "0.8.3",
"resolved": "https://registry.npmjs.org/fflate/-/fflate-0.8.2.tgz", "resolved": "https://registry.npmjs.org/fflate/-/fflate-0.8.3.tgz",
"integrity": "sha512-cPJU47OaAoCbg0pBvzsgpTPhmhqI5eJjh/JIu8tPj5q+T7iLvW/JAYUqmE7KOB4R1ZyEhzBaIQpQpardBF5z8A==", "integrity": "sha512-tbZNuJrLwGUp3zshBtdy4W+ORxZuIh8a5ilyIEQDC5rY1f3U20JMry0Ll3WBzU58EZKsEuJFXhb5gwv8CsPvgA==",
"dev": true, "dev": true,
"license": "MIT" "license": "MIT"
}, },
+1 -1
View File
@@ -129,7 +129,7 @@
"esbuild": "^0.27.3", "esbuild": "^0.27.3",
"eslint": "^9.0.0", "eslint": "^9.0.0",
"exceljs": "4.4.0", "exceljs": "4.4.0",
"fflate": "0.8.2", "fflate": "0.8.3",
"pixelmatch": "^6.0.0", "pixelmatch": "^6.0.0",
"playwright": "^1.58.0", "playwright": "^1.58.0",
"pngjs": "^7.0.0", "pngjs": "^7.0.0",
@@ -125,6 +125,13 @@ async function loadWorkbook(bytes) {
const admitted = core.buildAdmittedArchive(admission, self.fflate); const admitted = core.buildAdmittedArchive(admission, self.fflate);
if (!self.ExcelJS) importScripts(`vendor/exceljs.min.js${spreadsheetAssetQuery}`); if (!self.ExcelJS) importScripts(`vendor/exceljs.min.js${spreadsheetAssetQuery}`);
const nextWorkbook = new self.ExcelJS.Workbook(); const nextWorkbook = new self.ExcelJS.Workbook();
// ExcelJS's DefinedNames model setter expands every range into one object per
// cell (a whole-sheet name exhausts the heap), and admission never scans
// xl/workbook.xml. The preview never shows defined names, so they are not
// stored at all; print areas and titles are split off before this setter runs.
// defineProperty throws if a future ExcelJS renames `_definedNames`, rather
// than silently expanding again.
Object.defineProperty(nextWorkbook._definedNames, 'model', { configurable: true, get: () => [], set: () => {} });
// ExcelJS expands every address of a `<dataValidation sqref>` into its own // ExcelJS expands every address of a `<dataValidation sqref>` into its own
// object (a whole-column dropdown is a million), and the preview never shows // object (a whole-column dropdown is a million), and the preview never shows
// validations, so they are not parsed at all. `maxRows` is a per-sheet // validations, so they are not parsed at all. `maxRows` is a per-sheet
+1 -1
View File
@@ -20,7 +20,7 @@
(function initSpreadsheetPreview(global) { (function initSpreadsheetPreview(global) {
'use strict'; 'use strict';
const SPREADSHEET_ASSET_VERSION = 'ac0c75e6303e'; const SPREADSHEET_ASSET_VERSION = '2bb0eff45e39';
const MAX_PREVIEW_BYTES = 10 * 1024 * 1024; const MAX_PREVIEW_BYTES = 10 * 1024 * 1024;
const DEFAULT_TIMEOUT_MS = 20000; const DEFAULT_TIMEOUT_MS = 20000;
const MAX_SCROLL_PX = 8000000; const MAX_SCROLL_PX = 8000000;
+54 -9
View File
@@ -9,7 +9,9 @@
* hands ExcelJS a STORE-only archive rebuilt from exactly those * hands ExcelJS a STORE-only archive rebuilt from exactly those
* (`buildAdmittedArchive()`), never the original bytes: admission follows local * (`buildAdmittedArchive()`), never the original bytes: admission follows local
* headers while ExcelJS (JSZip) follows the central directory, so overlapping * headers while ExcelJS (JSZip) follows the central directory, so overlapping
* entries could otherwise show each reader a different file. * entries could otherwise show each reader a different file. Entries are
* counted and rebuilt under the name ExcelJS will see (`excelJsEntryName()`),
* never the stored spelling, so `/xl/...` or `xl/./...` cannot skip a counter.
*/ */
(function initSpreadsheetXlsxCore(global) { (function initSpreadsheetXlsxCore(global) {
@@ -55,6 +57,36 @@
return new DataView(bytes.buffer, bytes.byteOffset, bytes.byteLength).getUint32(offset, true); return new DataView(bytes.buffer, bytes.byteOffset, bytes.byteLength).getUint32(offset, true);
} }
/**
* The name ExcelJS will give a ZIP entry. JSZip resolves every name on load
* (`utils.resolve` in jszip/lib/utils.js, called from lib/load.js): `.` and
* empty middle segments are dropped and `..` pops the previous segment. ExcelJS
* then strips ONE leading `/` (lib/xlsx/xlsx.js, the `load` loop). Admission
* counts, and the admitted archive is rebuilt, under this name, so the
* stored spelling of a name cannot steer an entry past the counters.
*/
function excelJsEntryName(name) {
const parts = String(name).split('/');
const resolved = [];
for (let index = 0; index < parts.length; index += 1) {
const part = parts[index];
// JSZip keeps an empty first or last segment (a leading or trailing `/`).
if (part === '.' || (part === '' && index !== 0 && index !== parts.length - 1)) continue;
if (part === '..') resolved.pop();
else resolved.push(part);
}
const joined = resolved.join('/');
return joined[0] === '/' ? joined.slice(1) : joined;
}
// ExcelJS's own worksheet test, copied verbatim (lib/xlsx/xlsx.js): it is
// UNANCHORED, so `xl/xl/worksheets/sheet1.xml` and `.../sheet1.xml.x` are
// worksheets too. The older anchored test stays as a conservative superset.
const EXCELJS_WORKSHEET = /xl\/worksheets\/sheet(\d+)[.]xml/;
function isWorksheetName(name) {
return EXCELJS_WORKSHEET.test(name) || /^xl\/worksheets\/[^/]+\.xml$/i.test(name);
}
function inspectZipDirectory(bytes, overrides) { function inspectZipDirectory(bytes, overrides) {
const limits = mergedLimits(overrides); const limits = mergedLimits(overrides);
if (bytes.length >= 4 && bytes[0] === 0xd0 && bytes[1] === 0xcf && bytes[2] === 0x11 && bytes[3] === 0xe0) { if (bytes.length >= 4 && bytes[0] === 0xd0 && bytes[1] === 0xcf && bytes[2] === 0x11 && bytes[3] === 0xe0) {
@@ -78,6 +110,7 @@
if (entryCount > limits.maxEntries) fail('entry-limit', `Workbook exceeds ${limits.maxEntries} ZIP entries`); if (entryCount > limits.maxEntries) fail('entry-limit', `Workbook exceeds ${limits.maxEntries} ZIP entries`);
if (directoryOffset + directorySize > eocd) fail('malformed', 'Malformed XLSX central directory bounds'); if (directoryOffset + directorySize > eocd) fail('malformed', 'Malformed XLSX central directory bounds');
const entries = []; const entries = [];
const excelJsNames = new Set();
let cursor = directoryOffset; let cursor = directoryOffset;
for (let i = 0; i < entryCount; i += 1) { for (let i = 0; i < entryCount; i += 1) {
if (cursor + 46 > eocd || u32(bytes, cursor) !== 0x02014b50) if (cursor + 46 > eocd || u32(bytes, cursor) !== 0x02014b50)
@@ -107,7 +140,13 @@
} }
const localName = new TextDecoder().decode(bytes.subarray(localHeaderOffset + 30, localNameEnd)); const localName = new TextDecoder().decode(bytes.subarray(localHeaderOffset + 30, localNameEnd));
if (localName !== name) fail('malformed', 'XLSX local and central directory names do not match'); if (localName !== name) fail('malformed', 'XLSX local and central directory names do not match');
entries.push({ name, compressedSize, declaredSize, localHeaderOffset }); // JSZip keeps only the last of two entries that resolve to one name, and
// the rebuilt archive can hold only one, so both are refused.
const excelJsName = excelJsEntryName(name);
if (excelJsName === '') fail('malformed', 'XLSX entry name resolves to nothing');
if (excelJsNames.has(excelJsName)) fail('malformed', 'Two XLSX entries resolve to the same name');
excelJsNames.add(excelJsName);
entries.push({ name, excelJsName, compressedSize, declaredSize, localHeaderOffset });
cursor = end; cursor = end;
} }
return { entries }; return { entries };
@@ -183,8 +222,9 @@
let sheetCells = 0; let sheetCells = 0;
let sheetMerges = 0; let sheetMerges = 0;
let sheetRows = 0; let sheetRows = 0;
const worksheet = /^xl\/worksheets\/[^/]+\.xml$/i.test(name); const excelJsName = excelJsEntryName(name);
const styles = name === 'xl/styles.xml'; const worksheet = isWorksheetName(excelJsName);
const styles = excelJsName === 'xl/styles.xml';
const addCells = (cells) => { const addCells = (cells) => {
sheetCells += cells; sheetCells += cells;
counts.cells += cells; counts.cells += cells;
@@ -255,13 +295,15 @@
streamedEntries.set(file.name, (streamedEntries.get(file.name) || 0) + 1); streamedEntries.set(file.name, (streamedEntries.get(file.name) || 0) + 1);
seenEntries += 1; seenEntries += 1;
if (seenEntries > limits.maxEntries) fail('entry-limit', 'Workbook exceeds the ZIP entries limit'); if (seenEntries > limits.maxEntries) fail('entry-limit', 'Workbook exceeds the ZIP entries limit');
if (/^xl\/worksheets\/[^/]+\.xml$/i.test(file.name)) { // Everything below keys on the name ExcelJS will see, never the stored one.
const name = directoryByName.get(file.name).excelJsName;
if (isWorksheetName(name)) {
counts.worksheets += 1; counts.worksheets += 1;
if (counts.worksheets > limits.maxWorksheets) fail('worksheet-limit', 'Workbook exceeds the worksheet limit'); if (counts.worksheets > limits.maxWorksheets) fail('worksheet-limit', 'Workbook exceeds the worksheet limit');
} }
const feature = featureForName(file.name); const feature = featureForName(name);
if (feature) features.add(feature); if (feature) features.add(feature);
const counter = createXmlCounter(file.name, counts, limits); const counter = createXmlCounter(name, counts, limits);
let entryInflated = 0; let entryInflated = 0;
const chunks = []; const chunks = [];
file.ondata = (error, chunk, final) => { file.ondata = (error, chunk, final) => {
@@ -284,7 +326,7 @@
offset += part.length; offset += part.length;
} }
chunks.length = 0; chunks.length = 0;
inflatedEntries[file.name] = data; inflatedEntries[name] = data;
} }
}; };
file.start(); file.start();
@@ -304,7 +346,9 @@
if (streamedEntries.get(name) !== count) fail('malformed', 'Central XLSX entry was not streamed for admission'); if (streamedEntries.get(name) !== count) fail('malformed', 'Central XLSX entry was not streamed for admission');
} }
for (const name of streamedEntries.keys()) { for (const name of streamedEntries.keys()) {
if (!(name in inflatedEntries)) fail('malformed', 'XLSX entry did not finish streaming for admission'); if (!(directoryByName.get(name).excelJsName in inflatedEntries)) {
fail('malformed', 'XLSX entry did not finish streaming for admission');
}
} }
return { counts, features: Array.from(features), inflatedBytes: totalInflated, entries: inflatedEntries }; return { counts, features: Array.from(features), inflatedBytes: totalInflated, entries: inflatedEntries };
} }
@@ -741,6 +785,7 @@
createXmlCounter, createXmlCounter,
admitXlsx, admitXlsx,
buildAdmittedArchive, buildAdmittedArchive,
excelJsEntryName,
parseCellRef, parseCellRef,
parseRange, parseRange,
deriveExtent, deriveExtent,
+4
View File
@@ -160,6 +160,10 @@ describe('dependency security policy', () => {
expectEveryLockedVersionAtLeast(lock, 'find-my-way', '9.7.0'); expectEveryLockedVersionAtLeast(lock, 'find-my-way', '9.7.0');
expectEveryLockedVersionAtLeast(lock, 'basic-ftp', '5.3.1'); expectEveryLockedVersionAtLeast(lock, 'basic-ftp', '5.3.1');
expectEveryLockedVersionAtLeast(lock, 'flatted', '3.4.2'); expectEveryLockedVersionAtLeast(lock, 'flatted', '3.4.2');
// GHSA-px8p-9vwx-vf98 (unbounded loop on a ZIP64 marker in a local header)
// covers <=0.8.2. The XLSX preview worker streams untrusted files through
// fflate's Unzip before any admission callback runs.
expectEveryLockedVersionAtLeast(lock, 'fflate', '0.8.3');
expectNoVulnerableBraceExpansion(lock); expectNoVulnerableBraceExpansion(lock);
expectNoVulnerableVite(lock); expectNoVulnerableVite(lock);
expectNoVulnerablePicomatch(lock); expectNoVulnerablePicomatch(lock);
+1 -1
View File
@@ -22,7 +22,7 @@ describe('spreadsheet preview assets', () => {
devDependencies?: Record<string, string>; devDependencies?: Record<string, string>;
}; };
expect(pkg.devDependencies?.exceljs).toBe('4.4.0'); expect(pkg.devDependencies?.exceljs).toBe('4.4.0');
expect(pkg.devDependencies?.fflate).toBe('0.8.2'); expect(pkg.devDependencies?.fflate).toBe('0.8.3');
// They are vendored into dist/ at build time; a runtime install never needs them. // They are vendored into dist/ at build time; a runtime install never needs them.
expect(pkg.dependencies?.exceljs).toBeUndefined(); expect(pkg.dependencies?.exceljs).toBeUndefined();
expect(pkg.dependencies?.fflate).toBeUndefined(); expect(pkg.dependencies?.fflate).toBeUndefined();
+71
View File
@@ -625,3 +625,74 @@ async function emptyRowsWorkbook(extraRows: string): Promise<ArrayBuffer> {
entries['xl/worksheets/sheet1.xml'] = fflate.strToU8(patched); entries['xl/worksheets/sheet1.xml'] = fflate.strToU8(patched);
return toArrayBuffer(fflate.zipSync(entries)); return toArrayBuffer(fflate.zipSync(entries));
} }
/**
* A one-cell workbook whose sheet carries a merge one cell over the per-sheet
* cap, stored under `entryName` instead of `xl/worksheets/sheet1.xml`. JSZip
* (inside ExcelJS) resolves `.`, `..` and empty segments, and ExcelJS strips one
* leading `/` and matches worksheets with an unanchored pattern, so each of
* these names still reaches ExcelJS as a worksheet.
*/
async function renamedOversizedSheet(entryName: string, relTarget?: string): Promise<ArrayBuffer> {
const workbook = new ExcelJS.Workbook();
workbook.addWorksheet('Data').getCell('A1').value = 'one';
const entries = fflate.unzipSync(new Uint8Array(await workbook.xlsx.writeBuffer()));
const sheet = fflate.strFromU8(entries['xl/worksheets/sheet1.xml']);
const patched = sheet.replace(
'</sheetData>',
'</sheetData><mergeCells count="1"><mergeCell ref="A1:A100001"/></mergeCells>'
);
expect(patched).not.toBe(sheet);
delete entries['xl/worksheets/sheet1.xml'];
entries[entryName] = fflate.strToU8(patched);
if (relTarget) {
const rels = fflate.strFromU8(entries['xl/_rels/workbook.xml.rels']);
const retargeted = rels.replace('Target="worksheets/sheet1.xml"', `Target="${relTarget}"`);
expect(retargeted).not.toBe(rels);
entries['xl/_rels/workbook.xml.rels'] = fflate.strToU8(retargeted);
}
return toArrayBuffer(fflate.zipSync(entries));
}
describe('spreadsheet preview worker: entry names as ExcelJS sees them', () => {
it.each([
['/xl/worksheets/sheet1.xml', undefined],
['xl/./worksheets/sheet1.xml', undefined],
['xl//worksheets/sheet1.xml', undefined],
['xl/xl/worksheets/sheet1.xml', 'xl/worksheets/sheet1.xml'],
['xl/worksheets/sheet1.xml.x', 'worksheets/sheet1.xml.x'],
] as const)(
'counts a sheet stored as %s and refuses it before ExcelJS loads',
async (entryName, relTarget) => {
const bytes = await renamedOversizedSheet(entryName, relTarget);
// The fixture is real: unguarded ExcelJS parses this entry as the sheet.
const direct = new ExcelJS.Workbook();
await direct.xlsx.load(bytes.slice(0));
expect(direct.worksheets[0]?.model.merges).toContain('A1:A100001');
const harness = createHarness();
await harness.send({ type: 'load', bytes });
expect(harness.messages.at(-1)).toMatchObject({ type: 'error', code: 'cell-limit' });
expect(harness.imports.some((url) => url.includes('exceljs'))).toBe(false);
},
30_000
);
});
describe('spreadsheet preview worker: defined names', () => {
// ExcelJS's DefinedNames model setter creates one object per cell of every
// range; a whole-sheet name exhausted a 4 GB heap. The preview never shows
// defined names, so they are not expanded at all.
it('loads a workbook with a defined name without expanding its range', async () => {
const workbook = new ExcelJS.Workbook();
workbook.addWorksheet('Data').getCell('A1').value = 'one';
workbook.definedNames.add('Data!$A$1:$J$10', 'Block');
const bytes = await writeWorkbook(workbook);
expect(fflate.strFromU8(fflate.unzipSync(new Uint8Array(bytes))['xl/workbook.xml'])).toContain('Block');
const harness = createHarness();
const metadata = await loadMetadata(harness, bytes);
expect(metadata.sheets[0]).toMatchObject({ name: 'Data', rows: 1, cols: 1 });
expect(harness.peek('Object.keys(workbook.definedNames.matrixMap).length')).toBe(0);
});
});
+49
View File
@@ -17,6 +17,7 @@ type Core = {
limits: Record<string, number> limits: Record<string, number>
): { push(chunk: Uint8Array, final: boolean): void }; ): { push(chunk: Uint8Array, final: boolean): void };
buildAdmittedArchive(admission: unknown, zip: typeof fflate): Uint8Array; buildAdmittedArchive(admission: unknown, zip: typeof fflate): Uint8Array;
excelJsEntryName(name: string): string;
parseCellRef(ref: string): { row: number; col: number } | null; parseCellRef(ref: string): { row: number; col: number } | null;
deriveExtent(cells: string[], merges: string[]): { rows: number; cols: number }; deriveExtent(cells: string[], merges: string[]): { rows: number; cols: number };
createSparseAxis(count: number, defaultSize: number, overrides: Array<[number, number]>): unknown; createSparseAxis(count: number, defaultSize: number, overrides: Array<[number, number]>): unknown;
@@ -152,6 +153,54 @@ describe('spreadsheet XLSX core', () => {
expect(() => core.admitXlsx(zip, duplicate)).toThrowError(/duplicate/i); expect(() => core.admitXlsx(zip, duplicate)).toThrowError(/duplicate/i);
}); });
it('names every entry the way JSZip and ExcelJS will see it', () => {
expect(core.excelJsEntryName('xl/worksheets/sheet1.xml')).toBe('xl/worksheets/sheet1.xml');
expect(core.excelJsEntryName('/xl/worksheets/sheet1.xml')).toBe('xl/worksheets/sheet1.xml');
expect(core.excelJsEntryName('xl/./worksheets/sheet1.xml')).toBe('xl/worksheets/sheet1.xml');
expect(core.excelJsEntryName('xl//worksheets/sheet1.xml')).toBe('xl/worksheets/sheet1.xml');
expect(core.excelJsEntryName('xl/foo/../worksheets/sheet1.xml')).toBe('xl/worksheets/sheet1.xml');
expect(core.excelJsEntryName('../xl/styles.xml')).toBe('xl/styles.xml');
expect(core.excelJsEntryName('//xl/styles.xml')).toBe('xl/styles.xml');
expect(core.excelJsEntryName('xl/media/')).toBe('xl/media/');
});
it('refuses two entries that resolve to the same name', () => {
const entries = fflate.unzipSync(workbookZip());
const big = '<worksheet><sheetData>' + '<c r="A1"/>'.repeat(4) + '</sheetData></worksheet>';
for (const twin of ['/xl/worksheets/sheet1.xml', 'xl/./worksheets/sheet1.xml', 'xl/x/../worksheets/sheet1.xml']) {
const zip = fflate.zipSync({ ...entries, [twin]: fflate.strToU8(big) });
expect(() => core.admitXlsx(zip, fflate), twin).toThrowError(/same name/i);
}
});
it('counts and rebuilds entries under the names ExcelJS will see', () => {
const cells = '<worksheet><sheetData>' + '<c r="A1"/>'.repeat(4) + '</sheetData></worksheet>';
const renamed = (name: string) => {
const entries = fflate.unzipSync(workbookZip(cells));
const sheet = entries['xl/worksheets/sheet1.xml'];
delete entries['xl/worksheets/sheet1.xml'];
return fflate.zipSync({ ...entries, [name]: sheet });
};
for (const name of ['/xl/worksheets/sheet1.xml', 'xl/./worksheets/sheet1.xml', 'a/xl/worksheets/sheet2.xml.x']) {
expect(() => core.admitXlsx(renamed(name), fflate, { maxCellsPerSheet: 3 }), name).toThrowError(/cells/i);
expect(() => core.admitXlsx(renamed(name), fflate, { maxWorksheets: 0 }), name).toThrowError(/worksheet/i);
}
const admission = core.admitXlsx(renamed('/xl/./worksheets/sheet1.xml'), fflate) as {
entries: Record<string, Uint8Array>;
};
expect(Object.keys(admission.entries)).toContain('xl/worksheets/sheet1.xml');
expect(Object.keys(fflate.unzipSync(core.buildAdmittedArchive(admission, fflate)))).toContain(
'xl/worksheets/sheet1.xml'
);
// `/xl/styles.xml` is ExcelJS's `xl/styles.xml`, so its <xf> tags count.
const styles = fflate.unzipSync(workbookZip());
const xf = styles['xl/styles.xml'];
delete styles['xl/styles.xml'];
const leading = fflate.zipSync({ ...styles, '/xl/styles.xml': xf });
expect(() => core.admitXlsx(leading, fflate, { maxStyles: 0 })).toThrowError(/styles limit/i);
});
it('charges a merged range its full area and refuses one that does not parse', () => { it('charges a merged range its full area and refuses one that does not parse', () => {
const merged = (ref: string) => const merged = (ref: string) =>
workbookZip( workbookZip(