From 7d956d4f2413dce742a8f27705270567a7a4d5b0 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 04:48:12 -0400 Subject: [PATCH 1/4] Tenure AI reads every document the viewer can open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "tenure ai should be activated everywhere not just the tenure AI chatbot", with a screenshot of a document summary reading: Not available Summaries currently support text documents (this is application/pdf). PDF and Office support arrives with the document pipeline. THE SECOND SENTENCE WAS NOT TRUE WHEN IT WAS WRITTEN. The pipeline is already here: `_lib/content.ts` has parsed Word through mammoth, Excel through xlsx and PowerPoint through JSZip since the viewer shipped. Three of the four formats that refusal named could have been summarised that day. What actually existed was a gate — `^(text\/|application\/(json|csv|xml))` — written before the parsers and never revisited after them. The limit was in the gate, not in the product, and a student was being told a roadmap to explain it. ── WHAT CHANGES ──────────────────────────────────────────────────────────── `documentTextFor(mime, bytes)` asks the parsers that already exist, so Word, Excel, PowerPoint, CSV and text all summarise now. It adds no parsing of its own — it asks the same parsers a different question. The viewer wants a docx as HTML it can style; a reader wants the same docx as prose, so this uses `extractRawText` rather than `convertToHtml`. Markup in a prompt is noise a model spends attention discarding, and the sanitizer is not involved because nothing here goes near `dangerouslySetInnerHTML`. Sheet NAMES are kept. "Q3 budget" versus "Sheet1" is most of what a spreadsheet's structure tells a reader, and a model that cannot see it is reading a wall of numbers. ── PDF IS THE ONE THAT IS HONESTLY NOT HERE ──────────────────────────────── There is no PDF text extractor in this repo — `lib/export/pdf.ts` WRITES the format and cannot read it, and the viewer shows a PDF by pointing a frame at the bytes and letting the browser do the reading. Adding an extractor is a dependency decision, so PDF refuses. The refusal promises nothing this time. The old copy dated the fix to a "document pipeline" a student has no way to ask about, and a promise a product cannot keep is worse than a plain limit. ── EVERY REFUSAL SAYS WHICH ONE IT IS ────────────────────────────────────── `documentTextFor` returns a REASON rather than a boolean, so the page can tell apart things that used to collapse into one "Not available": · a PDF — cannot read it yet; open it to read it in full · an image — there is no text in a photograph to summarise · EMPTY — a scan in a wrapper: nothing to summarise, which is not the same as nothing we can open · too large — over the parse ceiling `content.ts` already applies · unsupported — a format, named in the reader's word for it, never as a Content-Type None of them prints `application/pdf` at a person. A reader shown a header value has been handed an identifier and told it is an explanation. ── TRUNCATION IS REPORTED ────────────────────────────────────────────────── A long document is cut at the read ceiling, and the card says so — "Generated from the beginning of this document", with a line under the summary. A summary drawn from part of a document and described as one drawn from the whole is exactly the silent-coverage failure the export work has been fixing all day. ── VERIFICATION ──────────────────────────────────────────────────────────── 13 tests, including a REAL .docx built in the test rather than committed as a binary fixture — Word is the headline claim here and a test that only proves a corrupt docx is refused does not support it. Mutating the classifier so every format reads as text fails five of them. Nothing throws: a corrupt file of a format we claim to read degrades to a card, never a 500 on the page whose entire job is to be helpful. tsc 0 errors · jest 417 suites / 6,664 tests green · next build exit 0 · eslint clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../[slug]/documents/[id]/summary/page.tsx | 95 +++++++-- .../web/src/app/api/documents/_lib/content.ts | 2 +- ...-ai-reads-what-the-viewer-can-open.test.ts | 190 ++++++++++++++++++ apps/web/src/app/api/documents/_lib/text.ts | 124 ++++++++++++ 4 files changed, 392 insertions(+), 19 deletions(-) create mode 100644 apps/web/src/app/api/documents/_lib/tenure-ai-reads-what-the-viewer-can-open.test.ts create mode 100644 apps/web/src/app/api/documents/_lib/text.ts diff --git a/apps/web/src/app/(app)/orgs/[slug]/documents/[id]/summary/page.tsx b/apps/web/src/app/(app)/orgs/[slug]/documents/[id]/summary/page.tsx index 2ae56f74..506c9573 100644 --- a/apps/web/src/app/(app)/orgs/[slug]/documents/[id]/summary/page.tsx +++ b/apps/web/src/app/(app)/orgs/[slug]/documents/[id]/summary/page.tsx @@ -11,11 +11,10 @@ import { Card, CardHeader } from "@/components/ui/Card" import { AnswerBody } from "@/components/ai/answer-format" import { fileTypeLabel } from "@/lib/uploads" import { recordAuditEvent } from "@/lib/audit-record" +import { documentTextFor, MAX_READ_BYTES } from "@/app/api/documents/_lib/text" export const dynamic = "force-dynamic" -const TEXTUAL = /^(text\/|application\/(json|csv|xml))/ -const MAX_BYTES = 200 * 1024 export default async function DocumentSummaryPage({ params, @@ -38,18 +37,32 @@ export default async function DocumentSummaryPage({ let summary: string | null = null let note: string | null = null + let truncated = false + /* + * THE GATE ASKS THE PARSERS, NOT A REGULAR EXPRESSION. + * + * This page used to test `^(text\/|application\/(json|csv|xml))` and + * refuse everything else, explaining that "PDF and Office support arrives + * with the document pipeline". That sentence was not true when it was + * written: `_lib/content.ts` has parsed Word, Excel and PowerPoint since + * the viewer shipped. Three of the four formats that refusal named could + * have been summarised that day. + * + * The regexp was written before the parsers and never revisited after + * them — so the limit was in the gate, not in the product, and a student + * was being told a roadmap to explain it. + * + * `documentTextFor` asks the parsers that already exist. It returns a + * REASON rather than a boolean, so the sentence below can say which of the + * several different things went wrong. + */ if (!aiConfigured()) { note = "Tenure AI is not enabled." - } else if (!TEXTUAL.test(doc.mimeType)) { - // The TYPE, in the word a person uses for it. The refusal used to name - // the Content-Type — "this is application/pdf" — which is an - // interchange identifier standing in for an explanation. - note = `Tenure AI can only read text documents so far, and this is a ${fileTypeLabel( - doc.mimeType - )}. Open it to read it in full.` - } else if ((doc.sizeBytes ?? 0) > MAX_BYTES) { - note = "This document is too large to summarize in the pilot (200 KB limit)." + } else if ((doc.sizeBytes ?? 0) > MAX_READ_BYTES) { + note = + "This document is too large for Tenure AI to read. Open it to read it in full — " + + "nothing about it has changed." } else if (documentsBucket) { // BEFORE the read, not after a successful one. // @@ -89,9 +102,41 @@ export default async function DocumentSummaryPage({ // arrive, so the stall this page was worried about could still hold the // render open. `getDocumentBytes` closes that with an abortSignal, and // one reader means one place where that has to be true. - const content = (await getDocumentBytes(doc.objectKey)).toString("utf8") - summary = await summarizeDocument(doc.title, content) - if (!summary) note = "The summary was unavailable. Try again shortly." + const read = await documentTextFor(doc.mimeType, await getDocumentBytes(doc.objectKey)) + + if (!read.ok) { + /* + * EACH REFUSAL SAYS WHICH ONE IT IS, and none of them names a + * Content-Type. A reader shown "application/pdf" has been handed a + * header value and told it is an explanation. + * + * The PDF sentence promises nothing. The previous one dated the fix to + * a "document pipeline" the reader has no way to ask about, and a + * promise a product cannot keep is worse than a plain limit. + */ + note = + read.reason === "pdf" + ? "Tenure AI cannot read a PDF yet. Open it to read it in full — everything else " + + "about this document works normally." + : read.reason === "image" + ? `This is an image, so there is no text for Tenure AI to summarise. Open it to ` + + `see it.` + : read.reason === "empty" + ? "There is no text in this document to summarise. It may be a scan, or empty." + : read.reason === "too-large" + ? "This document is too large for Tenure AI to read. Open it to read it in full." + : `Tenure AI cannot read a ${fileTypeLabel(doc.mimeType)}. Open it to read it ` + + `in full.` + } else { + summary = await summarizeDocument(doc.title, read.text) + if (!summary) { + note = "The summary was unavailable. Try again shortly." + } else if (read.truncated) { + // A summary drawn from part of a document, described as one drawn + // from the whole, is the failure this whole file is careful about. + truncated = true + } + } } return ( @@ -112,7 +157,13 @@ export default async function DocumentSummaryPage({ {/* TWO DIFFERENT KINDS OF TEXT, AND ONLY ONE OF THEM IS PROSE A MODEL @@ -123,9 +174,17 @@ export default async function DocumentSummaryPage({ would claim a formatting contract that nothing produces. */} {summary ? ( -
- -
+ <> +
+ +
+ {truncated && ( +

+ This document was long enough that Tenure AI read only the beginning of it. + The summary covers that much, and the document itself is unchanged. +

+ )} + ) : (

{note}

)} diff --git a/apps/web/src/app/api/documents/_lib/content.ts b/apps/web/src/app/api/documents/_lib/content.ts index 2ae0af46..0a523413 100644 --- a/apps/web/src/app/api/documents/_lib/content.ts +++ b/apps/web/src/app/api/documents/_lib/content.ts @@ -168,7 +168,7 @@ function linesFromSlideXml(xml: string): string[] { return lines } -async function extractPptx(bytes: Buffer): Promise { +export async function extractPptx(bytes: Buffer): Promise { const zip = await JSZip.loadAsync(bytes) const slideFiles = Object.keys(zip.files) diff --git a/apps/web/src/app/api/documents/_lib/tenure-ai-reads-what-the-viewer-can-open.test.ts b/apps/web/src/app/api/documents/_lib/tenure-ai-reads-what-the-viewer-can-open.test.ts new file mode 100644 index 00000000..92ea563a --- /dev/null +++ b/apps/web/src/app/api/documents/_lib/tenure-ai-reads-what-the-viewer-can-open.test.ts @@ -0,0 +1,190 @@ +import JSZip from "jszip" +import * as XLSX from "xlsx" +import { documentTextFor, MAX_READ_BYTES, MAX_READ_CHARS } from "./text" + +/** + * "tenure ai should be activated everywhere not just the tenure AI chatbot", + * with a screenshot of a document summary reading "Not available". + * + * ── The claim these pin ───────────────────────────────────────────────────── + * + * ANYTHING THE VIEWER CAN OPEN, TENURE AI CAN READ — with PDF the single, + * stated exception. That equivalence is the whole point: the refusal on screen + * used to say Office support was still to come, while `content.ts` had been + * parsing Word, Excel and PowerPoint for the viewer the entire time. The gate + * was older than the parsers and nobody had gone back to it. + * + * So these tests are less about parsing (mammoth, xlsx and JSZip are already + * exercised by the viewer's own suite) and more about the DECISION: which + * formats are read, which are refused, and whether the refusal says something + * a person can act on. + */ + +const XLSX_MIME = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" +const DOCX = "application/vnd.openxmlformats-officedocument.wordprocessingml.document" + +describe("what Tenure AI can read", () => { + it("reads a plain text document", async () => { + const read = await documentTextFor("text/plain", Buffer.from("The gala is on 3 October.")) + + expect(read).toEqual({ ok: true, text: "The gala is on 3 October.", truncated: false }) + }) + + it("reads a spreadsheet, and keeps the sheet NAMES", async () => { + // "Q3 budget" versus "Sheet1" is most of what a spreadsheet's structure + // says, and a model that cannot see it is reading a wall of numbers. + const wb = XLSX.utils.book_new() + XLSX.utils.book_append_sheet(wb, XLSX.utils.aoa_to_sheet([["Item", "Cost"], ["Venue", 900]]), "Q3 budget") + const bytes = XLSX.write(wb, { type: "buffer", bookType: "xlsx" }) as Buffer + + const read = await documentTextFor(XLSX_MIME, bytes) + + expect(read.ok).toBe(true) + if (!read.ok) throw new Error("unreachable") + expect(read.text).toContain("Q3 budget") + expect(read.text).toContain("Venue") + expect(read.text).toContain("900") + }) + + it("reads CSV, which the viewer treats as a spreadsheet", async () => { + const read = await documentTextFor("text/csv", Buffer.from("Item,Cost\nVenue,900\n")) + + expect(read.ok).toBe(true) + if (!read.ok) throw new Error("unreachable") + expect(read.text).toContain("Venue") + }) +}) + +/** + * A minimal but REAL .docx, built here rather than committed as a binary + * fixture. + * + * Word is the headline claim of this change — "Tenure AI can read a Word + * document now" — and a test that only proves a CORRUPT docx is refused does + * not support it. Three parts are the minimum OPC package mammoth will open: + * the content-type map, the package relationships, and the document part. + */ +async function minimalDocx(paragraphs: string[]): Promise { + const zip = new JSZip() + zip.file( + "[Content_Types].xml", + ` + + + + +` + ) + zip.file( + "_rels/.rels", + ` + + +` + ) + const body = paragraphs.map((t) => `${t}`).join("") + zip.file( + "word/document.xml", + ` +${body}` + ) + return zip.generateAsync({ type: "nodebuffer" }) +} + +describe("Word — the format the old refusal said was not supported yet", () => { + it("reads the prose out of a real .docx", async () => { + const bytes = await minimalDocx([ + "Sponsorship agreement with Preview Consulting Group.", + "The deposit is $900 and is due on 3 October.", + ]) + + const read = await documentTextFor(DOCX, bytes) + + expect(read.ok).toBe(true) + if (!read.ok) throw new Error("unreachable") + expect(read.text).toContain("Preview Consulting Group") + expect(read.text).toContain("$900") + }) + + it("hands the model PROSE, not markup", async () => { + // `extractRawText`, not `convertToHtml`. Tags in a prompt are noise a model + // has to spend attention discarding, and the viewer's HTML path exists for + // a different job. + const read = await documentTextFor(DOCX, await minimalDocx(["The gala is in October."])) + + expect(read.ok).toBe(true) + if (!read.ok) throw new Error("unreachable") + expect(read.text).not.toContain("

") + expect(read.text).not.toContain("") + }) +}) + +describe("what it refuses, and what it says", () => { + it("names PDF as PDF — and promises nothing", async () => { + // The old copy dated the fix to a "document pipeline" a student has no way + // to ask about. A promise a product cannot keep is worse than a plain limit. + const read = await documentTextFor("application/pdf", Buffer.from("%PDF-1.4")) + + expect(read).toEqual({ ok: false, reason: "pdf" }) + }) + + it("distinguishes an image from a format it cannot parse", async () => { + // Different sentences: there is no text in a photograph, which is not the + // same as a format Tenure cannot open. + expect(await documentTextFor("image/png", Buffer.from([0x89, 0x50]))).toEqual({ + ok: false, + reason: "image", + }) + }) + + it("distinguishes an EMPTY document from an unreadable one", async () => { + // A scan in a docx wrapper. The answer to a reader is "there is nothing to + // summarise", not "we cannot open this". + expect(await documentTextFor("text/plain", Buffer.from(" \n\t "))).toEqual({ + ok: false, + reason: "empty", + }) + }) + + it("refuses an unknown format rather than guessing at its bytes", async () => { + expect(await documentTextFor("application/x-tar", Buffer.from("junk"))).toEqual({ + ok: false, + reason: "unsupported", + }) + }) + + it("refuses something over the parse ceiling WITHOUT reading it", async () => { + const huge = Buffer.alloc(MAX_READ_BYTES + 1) + + expect(await documentTextFor("text/plain", huge)).toEqual({ ok: false, reason: "too-large" }) + }) + + it("NEVER THROWS on a corrupt file of a format it claims to read", async () => { + // A bad file degrades to a card, never a 500 on the page whose entire job + // is to be helpful. Same rule buildDocContent states. + const read = await documentTextFor(DOCX, Buffer.from("this is definitely not a docx")) + + expect(read).toEqual({ ok: false, reason: "unsupported" }) + }) +}) + +describe("truncation is reported, not silent", () => { + it("flags a document longer than the read ceiling", async () => { + const long = "a".repeat(MAX_READ_CHARS + 100) + + const read = await documentTextFor("text/plain", Buffer.from(long)) + + expect(read.ok).toBe(true) + if (!read.ok) throw new Error("unreachable") + expect(read.truncated).toBe(true) + expect(read.text.length).toBe(MAX_READ_CHARS) + }) + + it("does not flag one that fits", async () => { + const read = await documentTextFor("text/plain", Buffer.from("short")) + + expect(read.ok).toBe(true) + if (!read.ok) throw new Error("unreachable") + expect(read.truncated).toBe(false) + }) +}) diff --git a/apps/web/src/app/api/documents/_lib/text.ts b/apps/web/src/app/api/documents/_lib/text.ts new file mode 100644 index 00000000..eb10b474 --- /dev/null +++ b/apps/web/src/app/api/documents/_lib/text.ts @@ -0,0 +1,124 @@ +import "server-only" +import mammoth from "mammoth" +import * as XLSX from "xlsx" +import { previewKindFor } from "@/components/documents/types" +import { extractPptx } from "./content" + +/** + * A STORED DOCUMENT AS PLAIN TEXT, for anything that has to READ it rather + * than render it — today, Tenure AI. + * + * ── The report, and what was actually wrong ───────────────────────────────── + * + * "tenure ai should be activated everywhere not just the tenure AI chatbot", + * with a screenshot of a document summary that read: + * + * Not available + * Summaries currently support text documents (this is application/pdf). + * PDF and Office support arrives with the document pipeline. + * + * The second sentence was not true when it was written. THE PIPELINE IS + * ALREADY HERE: `content.ts` has parsed Word through mammoth, Excel through + * xlsx and PowerPoint through JSZip since the viewer shipped. Three of the four + * formats that refusal named could have been summarised that day. + * + * What actually existed was a summary page that tested `^(text\\/|application\\/ + * (json|csv|xml))` and refused everything else — a gate written before the + * parsers, never revisited after them, and describing a roadmap to a student to + * explain a limit that had already been lifted. + * + * So this module adds no parsing. It re-asks the parsers a different question: + * the viewer wants a docx as HTML it can style, and a reader wants the same + * docx as prose. `mammoth.extractRawText` rather than `convertToHtml`, and the + * sanitizer is not involved — nothing here reaches `dangerouslySetInnerHTML`, + * and text handed to a model is not markup. + * + * ── PDF is the one that is honestly not here ─────────────────────────────── + * + * `previewKindFor` classifies PDF, and the viewer shows it by pointing a frame + * at the bytes — the browser does the reading. There is no PDF text extractor + * in this repo; `lib/export/pdf.ts` WRITES the format and cannot read it. + * Adding one is a dependency decision, so PDF returns `unreadable` and the + * caller says so in the reader's own words rather than promising a date. + * + * ── Nothing here throws ──────────────────────────────────────────────────── + * + * The same rule `buildDocContent` states: a corrupt file degrades to + * "cannot read" rather than a 500 on a page whose whole job is to be helpful. + */ + +/** The parse ceiling `content.ts` already applies. Anything larger is a download. */ +export const MAX_READ_BYTES = 10 * 1024 * 1024 + +/** + * How much text a model is given. Well under the summariser's own 24,000-char + * slice, so the cut happens HERE, where the reason for it can be recorded, and + * not silently inside a prompt string. + */ +export const MAX_READ_CHARS = 200_000 + +export type DocumentText = + | { readonly ok: true; readonly text: string; readonly truncated: boolean } + /** + * Why it cannot be read, in a form the caller turns into a sentence. Never a + * Content-Type: a reader shown `application/pdf` has been handed a header + * value and told it is an explanation. + */ + | { readonly ok: false; readonly reason: "pdf" | "image" | "unsupported" | "too-large" | "empty" } + +/** Rows joined into lines, so a sheet reads as text without inventing a shape. */ +function sheetToText(bytes: Buffer): string { + const wb = XLSX.read(bytes, { type: "buffer" }) + return wb.SheetNames.map((name) => { + const csv = XLSX.utils.sheet_to_csv(wb.Sheets[name]) + // The sheet's NAME matters to a reader — "Q3 budget" versus "Sheet1" is + // most of what a spreadsheet's structure says. + return `--- ${name} ---\n${csv}` + }).join("\n\n") +} + +export async function documentTextFor(mime: string, bytes: Buffer): Promise { + if (bytes.byteLength > MAX_READ_BYTES) return { ok: false, reason: "too-large" } + + const kind = previewKindFor(mime) + if (kind === "pdf") return { ok: false, reason: "pdf" } + if (kind === "image") return { ok: false, reason: "image" } + if (kind === null) return { ok: false, reason: "unsupported" } + + let raw: string + try { + if (kind === "html") { + // The DOCX path. `extractRawText`, not `convertToHtml`: a reader wants + // the prose, and markup in a prompt is noise a model has to spend + // attention discarding. + raw = (await mammoth.extractRawText({ buffer: bytes })).value + } else if (kind === "sheets") { + raw = sheetToText(bytes) + } else if (kind === "pptx") { + const slides = await extractPptx(bytes) + raw = slides + .map((slide, i) => { + const body = [...slide.lines, ...(slide.notes ?? [])].join("\n") + return `--- Slide ${i + 1} ---\n${body}` + }) + .join("\n\n") + } else { + raw = bytes.toString("utf8") + } + } catch { + // A file that will not parse is not a crash. See buildDocContent. + return { ok: false, reason: "unsupported" } + } + + const text = raw.trim() + // A scanned page in a docx wrapper, or an empty deck. Distinguished from a + // format we cannot read, because the answer to a reader is different: there + // is nothing to summarise, rather than nothing we can open. + if (text.length === 0) return { ok: false, reason: "empty" } + + return { + ok: true, + text: text.slice(0, MAX_READ_CHARS), + truncated: text.length > MAX_READ_CHARS, + } +} From 1dd8cdd9a897c4ef4cd9f2378878f32a30bba151 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 05:08:14 -0400 Subject: [PATCH 2/4] The workbook parse stays in the file that already owns the exposure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The advisory gate failed #307, by name, and it was right. `xlsx@0.18.5` carries two HIGH advisories that are REACHABLE and that npm cannot fix — GHSA-4r6h-8v6p-xvw6 (prototype pollution) and GHSA-5pgg-2g8v-p4x9 (ReDoS). The registry's newest xlsx IS 0.18.5; the patched builds are published only to cdn.sheetjs.com. So the repo ACCEPTS them rather than suppressing them, and the acceptance is bounded by an enumerated list of parse sites: "it does not assert the advisories are unreachable — they are — it asserts the blast radius has not grown since the exposure was written down and accepted." `documentTextFor` had added a second `XLSX.read`. Same bytes, same parser, same trust boundary — and still a new entry on the list somebody has to re-argue every time this is audited. So the read moves into `_lib/content.ts`, which is already the accepted site and whose own header says heavy parsers live there and nowhere a client bundle can reach. `sheetsToText` is exported from there; `text.ts` no longer imports xlsx at all. `sheet_to_csv` stays with the caller because it is not a parse entry point — the advisories exempt workflows that do not read arbitrary files. The blast radius is now exactly what it was: `advisory-recheck: OK - 4 xlsx parse site(s), all within the accepted exposure.` Worth saying plainly: adding the file to the allowlist would have passed the gate too. It would also have been the wrong answer, because the gate is not asking permission — it is asking whether the argument that justified accepting a known-vulnerable dependency is still true. Widening the list makes the argument weaker; not needing to widen it leaves it exactly as strong. It also makes this module's own header honest. It claims to add no parsing and to ask the existing parsers a different question, and for Word and PowerPoint that was already true. For Excel it was not. Co-Authored-By: Claude Opus 5 (1M context) --- .../web/src/app/api/documents/_lib/content.ts | 38 +++++++++++++++++++ apps/web/src/app/api/documents/_lib/text.ts | 16 +------- 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/apps/web/src/app/api/documents/_lib/content.ts b/apps/web/src/app/api/documents/_lib/content.ts index 0a523413..3a585047 100644 --- a/apps/web/src/app/api/documents/_lib/content.ts +++ b/apps/web/src/app/api/documents/_lib/content.ts @@ -168,6 +168,44 @@ function linesFromSlideXml(xml: string): string[] { return lines } +/** + * Every sheet as TEXT, for a caller that has to read a workbook rather than + * render it — today, Tenure AI's summariser. + * + * ── Why this lives HERE and not beside its caller ────────────────────────── + * + * `xlsx@0.18.5` carries two HIGH advisories that are reachable and that npm + * cannot fix: GHSA-4r6h-8v6p-xvw6 (prototype pollution) and GHSA-5pgg-2g8v-p4x9 + * (ReDoS). The registry's newest xlsx IS 0.18.5; the patched builds exist only + * on cdn.sheetjs.com. So the repo ACCEPTS them, and the acceptance is bounded + * by an enumerated list of parse sites in `scripts/advisory-rechecks.mjs` — + * "the blast radius has not grown since the exposure was written down". + * + * A second `XLSX.read` beside the summariser would have grown it. The recheck + * failed the first version of this work and named the file, and it was right: + * same bytes and same parser, but a new entry on that list is a new thing to + * re-argue every time somebody audits this. + * + * `sheet_to_csv` is not a parse entry point — the advisories exempt workflows + * that do not read arbitrary files — so this file keeps the READ and callers + * keep their own shaping. One `XLSX.read` in the server path, in the file whose + * own header already says heavy parsers live here and nowhere a client bundle + * can reach. + * + * NOT capped at MAX_SHEETS / MAX_ROWS like the viewer's grid: those bounds + * exist so a browser does not render 40,000 rows, and a reader wants the whole + * workbook. `MAX_PARSE_BYTES` still bounds the input, and the caller caps the + * text it keeps. + */ +export function sheetsToText(bytes: Buffer): string { + const wb = XLSX.read(bytes, { type: "buffer" }) + return wb.SheetNames.map( + // The sheet's NAME matters to a reader — "Q3 budget" versus "Sheet1" is + // most of what a spreadsheet's structure says. + (name) => `--- ${name} ---\n${XLSX.utils.sheet_to_csv(wb.Sheets[name])}` + ).join("\n\n") +} + export async function extractPptx(bytes: Buffer): Promise { const zip = await JSZip.loadAsync(bytes) diff --git a/apps/web/src/app/api/documents/_lib/text.ts b/apps/web/src/app/api/documents/_lib/text.ts index eb10b474..6e9e2949 100644 --- a/apps/web/src/app/api/documents/_lib/text.ts +++ b/apps/web/src/app/api/documents/_lib/text.ts @@ -1,8 +1,7 @@ import "server-only" import mammoth from "mammoth" -import * as XLSX from "xlsx" import { previewKindFor } from "@/components/documents/types" -import { extractPptx } from "./content" +import { extractPptx, sheetsToText } from "./content" /** * A STORED DOCUMENT AS PLAIN TEXT, for anything that has to READ it rather @@ -66,17 +65,6 @@ export type DocumentText = */ | { readonly ok: false; readonly reason: "pdf" | "image" | "unsupported" | "too-large" | "empty" } -/** Rows joined into lines, so a sheet reads as text without inventing a shape. */ -function sheetToText(bytes: Buffer): string { - const wb = XLSX.read(bytes, { type: "buffer" }) - return wb.SheetNames.map((name) => { - const csv = XLSX.utils.sheet_to_csv(wb.Sheets[name]) - // The sheet's NAME matters to a reader — "Q3 budget" versus "Sheet1" is - // most of what a spreadsheet's structure says. - return `--- ${name} ---\n${csv}` - }).join("\n\n") -} - export async function documentTextFor(mime: string, bytes: Buffer): Promise { if (bytes.byteLength > MAX_READ_BYTES) return { ok: false, reason: "too-large" } @@ -93,7 +81,7 @@ export async function documentTextFor(mime: string, bytes: Buffer): Promise Date: Wed, 26 Aug 2026 05:27:54 -0400 Subject: [PATCH 3/4] The read budget was eight times what the model actually sees MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two more review findings, and the first is the failure this whole change was supposed to prevent. ── A SUMMARY OF A QUARTER OF A DOCUMENT, DESCRIBED AS THE WHOLE ──────────── `MAX_READ_CHARS` was 200,000. `summarizeDocument` slices to 24,000. So for every document between those two numbers the extractor returned `truncated: false`, and the card said "Generated from the document contents" over a summary the model had written after reading the first eighth of it. The truncation notice was in this change from the start, and its own commit message says why: "a summary drawn from part of a document and described as one drawn from the whole is exactly the silent-coverage failure the export work has been fixing all day". It was doing that, in the code that says it must not — so the notice was worse than absent, because it was evidence of care that was not being taken. `SUMMARY_INPUT_CHARS` is exported from `lib/ai.ts` and imported here. Two numbers that must agree, in two files, is one number. The cut still happens in the extractor, where the reason for it can be recorded. ── AND THE SPREADSHEET IS BOUNDED AS IT IS CONVERTED ─────────────────────── `sheet_to_csv` expands a worksheet's full `!ref` range, so a valid workbook comfortably under `MAX_PARSE_BYTES` can produce a CSV very much larger — a sparse sheet with one cell at ZZ100000 is tiny on disk and enormous as text. Converting every sheet and slicing afterwards means the whole intermediate string exists first, on the request thread. `sheetsToText` now spends a budget sheet by sheet and stops. It deliberately returns slightly OVER budget when the last sheet overshoots, because that overshoot is what makes the caller's `truncated` flag true — a bound that returned exactly the budget would be indistinguishable from a document that happened to fit. ── THE TEST FOR THAT BOUND WAS VACUOUS, AND THE CONTROL CAUGHT IT ───────── Worth recording. The first version went through `documentTextFor` and asserted the result was `MAX_READ_CHARS` long — which is true whether or not the conversion stopped early, because the caller slices at the end either way. It passed with the bound removed. It tests `sheetsToText` directly now, comparing twelve fat sheets against one at the same budget, with the inverse case so it cannot pass by converting nothing. Removing the break fails it. The third finding on this PR — the xlsx parse site — was the one commit 1dd8cdd already addressed by moving the read into the file that owns the accepted exposure. `advisory-recheck: OK - 4 xlsx parse site(s), all within the accepted exposure.` tsc 0 · jest 417 suites / 6,669 tests green · eslint clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../web/src/app/api/documents/_lib/content.ts | 30 +++++++- ...-ai-reads-what-the-viewer-can-open.test.ts | 77 +++++++++++++++++++ apps/web/src/app/api/documents/_lib/text.ts | 20 +++-- apps/web/src/lib/ai.ts | 16 +++- 4 files changed, 133 insertions(+), 10 deletions(-) diff --git a/apps/web/src/app/api/documents/_lib/content.ts b/apps/web/src/app/api/documents/_lib/content.ts index 3a585047..368fa068 100644 --- a/apps/web/src/app/api/documents/_lib/content.ts +++ b/apps/web/src/app/api/documents/_lib/content.ts @@ -197,13 +197,35 @@ function linesFromSlideXml(xml: string): string[] { * workbook. `MAX_PARSE_BYTES` still bounds the input, and the caller caps the * text it keeps. */ -export function sheetsToText(bytes: Buffer): string { +export function sheetsToText(bytes: Buffer, budgetChars: number): string { const wb = XLSX.read(bytes, { type: "buffer" }) - return wb.SheetNames.map( + + /* + * BOUNDED AS IT BUILDS, not sliced afterwards. + * + * `sheet_to_csv` expands a worksheet's full `!ref` range, so a valid workbook + * comfortably under MAX_PARSE_BYTES can produce a CSV very much larger than + * the caller's budget — a sparse sheet with one cell at ZZ100000 is small on + * disk and enormous as text. Joining every sheet and THEN cutting means the + * whole intermediate string exists first, on the request thread. + * + * So the budget is spent sheet by sheet and conversion stops when it runs + * out. The caller still sees an over-budget string when the last sheet + * overshoots, which is what makes its `truncated` flag true — a bound that + * silently returned exactly the budget would be indistinguishable from a + * document that happened to fit. + */ + const out: string[] = [] + let used = 0 + for (const name of wb.SheetNames) { + if (used > budgetChars) break // The sheet's NAME matters to a reader — "Q3 budget" versus "Sheet1" is // most of what a spreadsheet's structure says. - (name) => `--- ${name} ---\n${XLSX.utils.sheet_to_csv(wb.Sheets[name])}` - ).join("\n\n") + const block = `--- ${name} ---\n${XLSX.utils.sheet_to_csv(wb.Sheets[name])}` + out.push(block) + used += block.length + 2 + } + return out.join("\n\n") } export async function extractPptx(bytes: Buffer): Promise { diff --git a/apps/web/src/app/api/documents/_lib/tenure-ai-reads-what-the-viewer-can-open.test.ts b/apps/web/src/app/api/documents/_lib/tenure-ai-reads-what-the-viewer-can-open.test.ts index 92ea563a..3910a777 100644 --- a/apps/web/src/app/api/documents/_lib/tenure-ai-reads-what-the-viewer-can-open.test.ts +++ b/apps/web/src/app/api/documents/_lib/tenure-ai-reads-what-the-viewer-can-open.test.ts @@ -1,5 +1,7 @@ import JSZip from "jszip" +import { sheetsToText } from "./content" import * as XLSX from "xlsx" +import { SUMMARY_INPUT_CHARS } from "@/lib/ai" import { documentTextFor, MAX_READ_BYTES, MAX_READ_CHARS } from "./text" /** @@ -119,6 +121,56 @@ describe("Word — the format the old refusal said was not supported yet", () => }) }) +describe("a spreadsheet is bounded as it is CONVERTED, not afterwards", () => { + /* + * Tests `sheetsToText` DIRECTLY, and the first version of this did not — it + * went through `documentTextFor` and asserted the result was + * `MAX_READ_CHARS` long. That is true whether or not the conversion stopped + * early, because the caller slices at the end either way, so the test passed + * with the bound removed. The control caught it. + * + * The property that matters is about the INTERMEDIATE string: `sheet_to_csv` + * expands a worksheet's full `!ref` range, so a workbook well under the byte + * ceiling can produce a CSV very much larger than the budget, and building + * all of it before cutting means all of it exists on the request thread. + */ + const fatWorkbook = (sheets: number) => { + const wb = XLSX.utils.book_new() + const rows = Array.from({ length: 400 }, (_, r) => [`row${r}`, "x".repeat(200)]) + for (let i = 0; i < sheets; i++) { + XLSX.utils.book_append_sheet(wb, XLSX.utils.aoa_to_sheet(rows), `Sheet${i}`) + } + return XLSX.write(wb, { type: "buffer", bookType: "xlsx" }) as Buffer + } + + it("stops converting once the budget is spent", () => { + const budget = 5_000 + const twelve = sheetsToText(fatWorkbook(12), budget) + const one = sheetsToText(fatWorkbook(1), budget) + + // One sheet already overshoots this budget, so twelve must not cost twelve + // times as much: conversion stops after the sheet that crossed it. + expect(one.length).toBeGreaterThan(budget) + expect(twelve.length).toBeLessThan(one.length * 2) + }) + + it("keeps converting while there is budget left", () => { + // The inverse, so this cannot pass by never converting anything. + const generous = sheetsToText(fatWorkbook(3), 10_000_000) + + expect(generous).toContain("Sheet0") + expect(generous).toContain("Sheet2") + }) + + it("reports the cut through documentTextFor", async () => { + const read = await documentTextFor(XLSX_MIME, fatWorkbook(12)) + + expect(read.ok).toBe(true) + if (!read.ok) throw new Error("unreachable") + expect(read.truncated).toBe(true) + }) +}) + describe("what it refuses, and what it says", () => { it("names PDF as PDF — and promises nothing", async () => { // The old copy dated the fix to a "document pipeline" a student has no way @@ -168,6 +220,31 @@ describe("what it refuses, and what it says", () => { }) }) +describe("the read budget IS the summariser's budget", () => { + it("cuts at exactly what the model will be given", () => { + /* + * These were 200,000 and 24,000. A 100,000-character document was therefore + * reported as summarised in FULL — `truncated` false, the card reading + * "Generated from the document contents" — while the model had seen the + * first quarter of it. A summary drawn from part of a document and + * described as one drawn from the whole is the exact failure the truncation + * notice exists to prevent, so the notice was worse than useless: it was + * evidence of care that was not being taken. + */ + expect(MAX_READ_CHARS).toBe(SUMMARY_INPUT_CHARS) + }) + + it("flags a document longer than the SUMMARISER's budget, not some larger number", async () => { + const justOver = "a".repeat(SUMMARY_INPUT_CHARS + 1) + + const read = await documentTextFor("text/plain", Buffer.from(justOver)) + + expect(read.ok).toBe(true) + if (!read.ok) throw new Error("unreachable") + expect(read.truncated).toBe(true) + }) +}) + describe("truncation is reported, not silent", () => { it("flags a document longer than the read ceiling", async () => { const long = "a".repeat(MAX_READ_CHARS + 100) diff --git a/apps/web/src/app/api/documents/_lib/text.ts b/apps/web/src/app/api/documents/_lib/text.ts index 6e9e2949..4d1fa3ab 100644 --- a/apps/web/src/app/api/documents/_lib/text.ts +++ b/apps/web/src/app/api/documents/_lib/text.ts @@ -1,6 +1,7 @@ import "server-only" import mammoth from "mammoth" import { previewKindFor } from "@/components/documents/types" +import { SUMMARY_INPUT_CHARS } from "@/lib/ai" import { extractPptx, sheetsToText } from "./content" /** @@ -50,11 +51,20 @@ import { extractPptx, sheetsToText } from "./content" export const MAX_READ_BYTES = 10 * 1024 * 1024 /** - * How much text a model is given. Well under the summariser's own 24,000-char - * slice, so the cut happens HERE, where the reason for it can be recorded, and - * not silently inside a prompt string. + * How much text a model is given — THE SUMMARISER'S OWN BUDGET, not a larger + * number of this file's choosing. + * + * This was 200,000 on the theory that the cut should happen here, where the + * reason for it can be recorded, rather than silently inside a prompt string. + * The theory was right and the number was wrong: `summarizeDocument` slices to + * 24,000, so every document between the two was reported as summarised in FULL + * while the model had seen a fraction of it. `truncated` was false and the card + * said "Generated from the document contents". + * + * Importing the constant is what makes the two impossible to drift apart. The + * cut still happens here, and it is still reported here. */ -export const MAX_READ_CHARS = 200_000 +export const MAX_READ_CHARS = SUMMARY_INPUT_CHARS export type DocumentText = | { readonly ok: true; readonly text: string; readonly truncated: boolean } @@ -81,7 +91,7 @@ export async function documentTextFor(mime: string, bytes: Buffer): Promise Date: Wed, 26 Aug 2026 05:48:03 -0400 Subject: [PATCH 4/4] The sheet bound limited how many worksheets, not how large one could be MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two more findings, both real, and the second is the one that mattered. ── ONE WORKSHEET COULD STILL COST EVERYTHING ─────────────────────────────── The budget was checked BETWEEN sheets. `sheet_to_csv` walks the whole `!ref` range in a single call before it returns, so one sparse worksheet declaring A1:ZZ1000000 — tiny on disk, because almost every cell is absent — generated and allocated all of it synchronously, and the loop only noticed afterwards. A bound on the number of worksheets is not a bound on the work. The range is clamped before conversion now: 1,000 rows and 64 columns from any one sheet, whatever its `!ref` claims. ── THE DOCUMENTED OPTION FOR DOING THAT DOES NOT WORK ────────────────────── `sheet_to_csv(ws, { range })` is documented and is silently ignored by this build. Measured all three forms — a Range object, an encoded string, and a numeric start row — and every one returned every row. Mutating `!ref` on the worksheet before the call is what actually clamps it, so that is what this does, with the measurement written down beside it so the next person does not repeat it. ── A HEADING IS NOT CONTENT ──────────────────────────────────────────────── `--- Sheet1 ---` is a non-empty string, so a workbook of nothing but blank sheets came back as readable text and was sent to a model to summarise a heading. Blank sheets are skipped, and a workbook of only blank sheets now reaches `reason: "empty"` — which the caller says differently from "cannot open this", because they are different facts. ── THE CAP WAS SET BY A TEST TIMING, WHICH IS THE HONEST WAY TO SET IT ───── First attempt: 5,000 x 256. That is 1.28 million cells, built to then be sliced to the 24,000 characters a model actually reads, and the test proving the clamp worked took SEVENTEEN SECONDS. A bound that is technically finite and practically a stall is not a bound. 1,000 x 64 is several times what any reader receives and returns in milliseconds. The test itself was the other half of that seventeen seconds — the cost was `XLSX.write` building a fixture with an absurd `!ref`, not the code under test. An expensive test proving a bound against pathological input IS the pathological input. Split into two cheap cases that prove the row clamp and the column clamp separately: 16.8s -> 0.3s, and each clamp now has its own control. All three changes have their own mutant, and each fails only its own test. tsc 0 · jest 417 suites / 6,672 tests green · eslint clean · advisory-recheck OK. Co-Authored-By: Claude Opus 5 (1M context) --- .../web/src/app/api/documents/_lib/content.ts | 67 +++++++++++++++---- ...-ai-reads-what-the-viewer-can-open.test.ts | 53 +++++++++++++++ 2 files changed, 108 insertions(+), 12 deletions(-) diff --git a/apps/web/src/app/api/documents/_lib/content.ts b/apps/web/src/app/api/documents/_lib/content.ts index 368fa068..0478502e 100644 --- a/apps/web/src/app/api/documents/_lib/content.ts +++ b/apps/web/src/app/api/documents/_lib/content.ts @@ -197,31 +197,74 @@ function linesFromSlideXml(xml: string): string[] { * workbook. `MAX_PARSE_BYTES` still bounds the input, and the caller caps the * text it keeps. */ +/** + * Rows and columns converted from any ONE worksheet, whatever its `!ref` claims. + * + * Sized against what a READER gets, not against what a spreadsheet may hold. + * The summariser is given 24,000 characters; a thousand rows of sixty-four + * columns is already several times that, so the cap is generous for every real + * workbook and still bounds a hostile one. + * + * The first attempt used 5,000 x 256. That is 1.28 million cells, and building + * that string to then slice it to 24,000 took SEVENTEEN SECONDS in the test + * that proved the clamp worked — a bound that is technically finite and + * practically a stall. The test time was the measurement that said so. + */ +const MAX_SHEET_ROWS = 1_000 +const MAX_SHEET_COLS = 64 + export function sheetsToText(bytes: Buffer, budgetChars: number): string { const wb = XLSX.read(bytes, { type: "buffer" }) /* - * BOUNDED AS IT BUILDS, not sliced afterwards. + * BOUNDED PER SHEET *AND* WITHIN EACH SHEET. + * + * The first version checked the budget between sheets, which bounds the + * NUMBER of worksheets converted and nothing about the size of any one of + * them. `sheet_to_csv` walks the whole `!ref` range in a single call before + * it returns, so one sparse worksheet declaring A1:ZZ1000000 — tiny on disk, + * because almost every cell is absent — generates and allocates all of it + * synchronously on the request thread, and the loop only notices afterwards. * - * `sheet_to_csv` expands a worksheet's full `!ref` range, so a valid workbook - * comfortably under MAX_PARSE_BYTES can produce a CSV very much larger than - * the caller's budget — a sparse sheet with one cell at ZZ100000 is small on - * disk and enormous as text. Joining every sheet and THEN cutting means the - * whole intermediate string exists first, on the request thread. + * So the RANGE is clamped before conversion. * - * So the budget is spent sheet by sheet and conversion stops when it runs - * out. The caller still sees an over-budget string when the last sheet - * overshoots, which is what makes its `truncated` flag true — a bound that - * silently returned exactly the budget would be indistinguishable from a - * document that happened to fit. + * ── The option that does not work, measured rather than assumed ─────────── + * + * `sheet_to_csv(ws, { range })` is documented and is silently ignored by this + * build — with a Range object, an encoded string, and a numeric start row, + * all three returned every row. Mutating `!ref` on the worksheet before the + * call is what actually clamps it, so that is what this does. The worksheet + * object is ours, parsed a few lines above and discarded after. */ const out: string[] = [] let used = 0 for (const name of wb.SheetNames) { if (used > budgetChars) break + + const ws = wb.Sheets[name] + const ref = ws?.["!ref"] + // A worksheet with no `!ref` has no cells at all. + if (!ref) continue + + const range = XLSX.utils.decode_range(ref) + range.e.r = Math.min(range.e.r, range.s.r + MAX_SHEET_ROWS - 1) + range.e.c = Math.min(range.e.c, range.s.c + MAX_SHEET_COLS - 1) + ws["!ref"] = XLSX.utils.encode_range(range) + + const csv = XLSX.utils.sheet_to_csv(ws) + /* + * A HEADING IS NOT CONTENT. + * + * `--- Sheet1 ---` is non-empty, so a workbook of nothing but blank sheets + * used to come back as readable text and get sent to a model to summarise. + * The caller distinguishes "nothing to summarise" from "cannot open this" + * and says different things for each; a blank workbook must reach the first. + */ + if (csv.trim() === "") continue + // The sheet's NAME matters to a reader — "Q3 budget" versus "Sheet1" is // most of what a spreadsheet's structure says. - const block = `--- ${name} ---\n${XLSX.utils.sheet_to_csv(wb.Sheets[name])}` + const block = `--- ${name} ---\n${csv}` out.push(block) used += block.length + 2 } diff --git a/apps/web/src/app/api/documents/_lib/tenure-ai-reads-what-the-viewer-can-open.test.ts b/apps/web/src/app/api/documents/_lib/tenure-ai-reads-what-the-viewer-can-open.test.ts index 3910a777..2e699a5a 100644 --- a/apps/web/src/app/api/documents/_lib/tenure-ai-reads-what-the-viewer-can-open.test.ts +++ b/apps/web/src/app/api/documents/_lib/tenure-ai-reads-what-the-viewer-can-open.test.ts @@ -154,6 +154,59 @@ describe("a spreadsheet is bounded as it is CONVERTED, not afterwards", () => { expect(twelve.length).toBeLessThan(one.length * 2) }) + /* + * The fixture is built with a MODERATELY oversized `!ref`, not an absurd one. + * + * The first version declared A1:ZZ100000 and the test took SEVENTEEN SECONDS + * — spent in `XLSX.write` building the fixture, which walks the declared + * range, not in the code under test. An expensive test that proves a bound + * against pathological input is itself the pathological input. + * + * Rows and columns are clamped independently, so they are proved + * independently and each fixture stays small. + */ + const sparse = (ref: string) => { + const ws = XLSX.utils.aoa_to_sheet([["a", "b"], ["c", "d"]]) + ws["!ref"] = ref + const wb = XLSX.utils.book_new() + XLSX.utils.book_append_sheet(wb, ws, "Sparse") + return XLSX.write(wb, { type: "buffer", bookType: "xlsx" }) as Buffer + } + + it("clamps ONE worksheet's ROWS, not just the number of worksheets", () => { + /* + * The hole in the first bound. `sheet_to_csv` walks the whole `!ref` range + * in a single call, so a sparse worksheet declaring far more rows than it + * holds — tiny on disk, because almost every cell is absent — generated all + * of it before the loop could notice. Checking the budget BETWEEN sheets + * bounds the count of worksheets and nothing about the size of any one. + */ + const text = sheetsToText(sparse("A1:B5000"), 10_000_000) + + // 5,000 declared rows, 1,000 converted: one newline each, so the clamp is + // visible in the line count and nowhere near 5,000. + expect(text.split("\n").length).toBeLessThan(1_100) + expect(text).toContain("a,b") + }) + + it("clamps ONE worksheet's COLUMNS too", () => { + // 702 declared columns (A..ZZ), 64 converted. Commas are the measure. + const text = sheetsToText(sparse("A1:ZZ2"), 10_000_000) + + const widest = Math.max(...text.split("\n").map((line) => line.split(",").length)) + expect(widest).toBeLessThanOrEqual(64) + }) + + it("treats a workbook of blank sheets as EMPTY, not as text", async () => { + // `--- Sheet1 ---` is non-empty, so a blank workbook used to come back as + // readable text and get sent to a model to summarise a heading. + const wb = XLSX.utils.book_new() + XLSX.utils.book_append_sheet(wb, XLSX.utils.aoa_to_sheet([[""]]), "Blank") + const bytes = XLSX.write(wb, { type: "buffer", bookType: "xlsx" }) as Buffer + + expect(await documentTextFor(XLSX_MIME, bytes)).toEqual({ ok: false, reason: "empty" }) + }) + it("keeps converting while there is budget left", () => { // The inverse, so this cannot pass by never converting anything. const generous = sheetsToText(fatWorkbook(3), 10_000_000)