diff --git a/packages/api/src/files/code/extract.spec.ts b/packages/api/src/files/code/extract.spec.ts index dc705cd85d..56d7a16097 100644 --- a/packages/api/src/files/code/extract.spec.ts +++ b/packages/api/src/files/code/extract.spec.ts @@ -20,14 +20,23 @@ jest.mock('~/files/documents/crud', () => ({ * assertions (parseDocument receives the canonical MIME) keep exercising * `extractDocument`. Tests that need real HTML output drive `bufferToOfficeHtml` * directly via its own spec file (`html.spec.ts`); a separate `office-html` - * describe block below exercises the integration with this mock relaxed. */ + * describe block below exercises the integration with this mock relaxed. + * + * `officeHtmlBucket` is the gate predicate the upstream uses to decide + * whether to call the dispatcher at all. We pass through to the real + * implementation so the gate routes the same files in tests as in prod. */ const mockOfficeHtml = jest.fn( async (_buffer: Buffer, _name: string, _mime: string) => null as string | null, ); -jest.mock('~/files/documents/html', () => ({ - bufferToOfficeHtml: (buffer: Buffer, name: string, mime: string) => - mockOfficeHtml(buffer, name, mime), -})); +jest.mock('~/files/documents/html', () => { + const actual = + jest.requireActual('~/files/documents/html'); + return { + bufferToOfficeHtml: (buffer: Buffer, name: string, mime: string) => + mockOfficeHtml(buffer, name, mime), + officeHtmlBucket: actual.officeHtmlBucket, + }; +}); describe('extractCodeArtifactText', () => { describe('utf8-text', () => { @@ -298,5 +307,44 @@ describe('extractCodeArtifactText', () => { expect(mockOfficeHtml).not.toHaveBeenCalled(); expect(text).toBe('hello world\n'); }); + + it('routes extensionless CSV-by-MIME (utf8-text category) through the office HTML path', async () => { + /* Regression for the Codex review on PR #12934. A tool emitting + * `data` with `text/csv` classifies as `utf8-text` (csv has no + * extension here, MIME is text/* which the classifier treats as + * utf8-text). The previous gate skipped the office-render branch + * because the extension wasn't in OFFICE_HTML_EXTENSIONS — so the + * raw CSV text shipped to the client, which routes by MIME to the + * SPREADSHEET bucket and expects a full HTML document. The fix + * shares the dispatcher's MIME-aware predicate so the gate fires + * here too. */ + mockOfficeHtml.mockResolvedValueOnce('
1
'); + const text = await extractCodeArtifactText( + Buffer.from('a,b\n1,2', 'utf-8'), + 'data', + 'text/csv', + 'utf8-text', + ); + expect(mockOfficeHtml).toHaveBeenCalledWith(expect.any(Buffer), 'data', 'text/csv'); + expect(text).toContain(''); + }); + + it.each([ + ['workbook', 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet'], + ['workbook', 'application/vnd.ms-excel'], + ['workbook', 'application/vnd.oasis.opendocument.spreadsheet'], + ['report', 'application/vnd.openxmlformats-officedocument.wordprocessingml.document'], + ['deck', 'application/vnd.openxmlformats-officedocument.presentationml.presentation'], + ])('routes extensionless office files by MIME alone (%s, %s)', async (name, mime) => { + mockOfficeHtml.mockResolvedValueOnce('x'); + const category = mime.includes('presentation') + ? 'pptx' + : mime.startsWith('text/') + ? 'utf8-text' + : 'document'; + const text = await extractCodeArtifactText(Buffer.from('PK'), name, mime, category); + expect(mockOfficeHtml).toHaveBeenCalledWith(expect.any(Buffer), name, mime); + expect(text).toContain(''); + }); }); }); diff --git a/packages/api/src/files/code/extract.ts b/packages/api/src/files/code/extract.ts index 7c7a8c2e6a..cc349204eb 100644 --- a/packages/api/src/files/code/extract.ts +++ b/packages/api/src/files/code/extract.ts @@ -5,7 +5,7 @@ import { randomUUID } from 'crypto'; import { logger } from '@librechat/data-schemas'; import type { CodeArtifactCategory } from './classify'; import { parseDocument } from '~/files/documents/crud'; -import { bufferToOfficeHtml } from '~/files/documents/html'; +import { bufferToOfficeHtml, officeHtmlBucket } from '~/files/documents/html'; import { isBinaryBuffer } from '~/skills/binary'; import { withTimeout } from '~/utils/promise'; @@ -17,20 +17,16 @@ const TRUNCATION_MARKER = '\n\n…[truncated]'; const TRUNCATION_MARKER_BYTES = Buffer.byteLength(TRUNCATION_MARKER, 'utf-8'); /** - * Extensions whose buffers should be rendered as HTML (rich preview) rather - * than plain text. CSV is included here even though it lives in the - * `utf8-text` category — when an office HTML producer is available we want - * the styled-table preview, not raw comma-separated text. + * Decide whether a buffer is a candidate for rich HTML preview. Wraps the + * shared `officeHtmlBucket` predicate from `~/files/documents/html` so the + * gate here stays in lock-step with what the dispatcher will actually + * route — including extensionless office files identified solely by MIME + * (e.g. a tool emitting `data` with `text/csv`, which would otherwise + * classify as `utf8-text`, skip the office gate, and ship raw CSV text + * to the client's SPREADSHEET bucket that expects full HTML). */ -const OFFICE_HTML_EXTENSIONS = new Set(['docx', 'xlsx', 'xls', 'ods', 'pptx', 'csv']); - -const extensionOf = (name: string): string => { - const dot = name.lastIndexOf('.'); - if (dot < 0 || dot === name.length - 1) { - return ''; - } - return name.slice(dot + 1).toLowerCase(); -}; +const hasOfficeHtmlPath = (name: string, mimeType: string): boolean => + officeHtmlBucket(name, mimeType) !== null; /** * Truncate UTF-8 content to fit within MAX_TEXT_CACHE_BYTES. Walks back to a @@ -181,14 +177,14 @@ export async function extractCodeArtifactText( return null; } try { - /* Office HTML preview takes precedence for any file whose extension marks - * it as a rich-preview type. We check extension first because content - * sniffing routinely labels these formats as `application/zip` or - * `application/octet-stream`. The HTML producer's own dispatcher will - * also fall back to MIME for extensionless inputs. */ - const ext = extensionOf(name); - const officeByExt = OFFICE_HTML_EXTENSIONS.has(ext); - if (officeByExt || category === 'document' || category === 'pptx') { + /* Office HTML preview takes precedence for any file the dispatcher + * recognizes — by extension OR by MIME. The MIME fallback matters for + * extensionless tool outputs (e.g. `data` with `text/csv`); without + * it we'd ship raw CSV text to the client's SPREADSHEET bucket which + * expects a full HTML document. The `document`/`pptx` category checks + * keep the path open for sniffed `application/zip`/`octet-stream` MIMEs + * where neither the extension nor the MIME alone is dispositive. */ + if (hasOfficeHtmlPath(name, mimeType) || category === 'document' || category === 'pptx') { const html = await renderOfficeHtml(buffer, name, mimeType); if (html != null) { return html; diff --git a/packages/api/src/files/documents/html.spec.ts b/packages/api/src/files/documents/html.spec.ts index 5989952f5f..fffa8aa011 100644 --- a/packages/api/src/files/documents/html.spec.ts +++ b/packages/api/src/files/documents/html.spec.ts @@ -5,6 +5,7 @@ import { bufferToOfficeHtml, csvToHtml, excelSheetToHtml, + officeHtmlBucket, pptxToSlideListHtml, sanitizeOfficeHtml, wordDocToHtml, @@ -219,6 +220,50 @@ describe('Office HTML producers', () => { }); }); + describe('officeHtmlBucket predicate', () => { + /* The shared predicate is the single source of truth for "should the + * office HTML pipeline handle this file?". The upstream gate in + * `extract.ts` calls it directly; the dispatcher above delegates to + * it. Tests here lock in MIME-only routing for extensionless inputs + * (the case Codex flagged on PR #12934). */ + it.each([ + ['report.docx', 'application/zip', 'docx'], + ['report.docx', '', 'docx'], + ['noext', 'application/vnd.openxmlformats-officedocument.wordprocessingml.document', 'docx'], + ['data.csv', 'application/octet-stream', 'csv'], + ['data', 'text/csv', 'csv'], + ['data', 'application/csv', 'csv'], + ['workbook.xlsx', '', 'spreadsheet'], + ['legacy.xls', '', 'spreadsheet'], + ['sheet.ods', '', 'spreadsheet'], + ['noext', 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', 'spreadsheet'], + ['noext', 'application/vnd.ms-excel', 'spreadsheet'], + ['noext', 'application/vnd.oasis.opendocument.spreadsheet', 'spreadsheet'], + ['deck.pptx', '', 'pptx'], + [ + 'noext', + 'application/vnd.openxmlformats-officedocument.presentationml.presentation', + 'pptx', + ], + ])('classifies (%s, %s) as %s', (name, mime, expected) => { + expect(officeHtmlBucket(name, mime)).toBe(expected); + }); + + it.each([ + ['note.txt', 'text/plain'], + ['code.py', 'text/x-python'], + ['data.json', 'application/json'], + ['photo.jpg', 'image/jpeg'], + ['doc.pdf', 'application/pdf'], + ['archive.zip', 'application/zip'], + ['notes.odt', 'application/vnd.oasis.opendocument.text'], + ['noext', 'text/plain'], + ['noext', ''], + ])('returns null for non-office (%s, %s)', (name, mime) => { + expect(officeHtmlBucket(name, mime)).toBeNull(); + }); + }); + describe('sanitizeOfficeHtml security', () => { test('strips after

'); diff --git a/packages/api/src/files/documents/html.ts b/packages/api/src/files/documents/html.ts index b16137ea99..13e5826401 100644 --- a/packages/api/src/files/documents/html.ts +++ b/packages/api/src/files/documents/html.ts @@ -651,24 +651,34 @@ function extensionOf(name: string): string { } /** - * Route an office-format buffer to the matching HTML producer. Returns `null` - * if the file isn't a recognized office type — caller should fall through to - * its existing text/binary handling. + * The four buckets the dispatcher can route to. Used by both + * `bufferToOfficeHtml` (which actually renders) and `officeHtmlBucket` + * (which the upstream gate in `extract.ts` calls to decide whether to + * invoke the dispatcher at all). Sharing one source of truth prevents the + * gate from going out of sync with the dispatcher — a previous version + * had a narrower extension-only gate that missed extensionless office + * files identified solely by MIME (e.g. a tool emitting `data` with + * `text/csv`), which then routed to the SPREADSHEET bucket on the + * client expecting full HTML and got raw text instead. + */ +export type OfficeHtmlBucket = 'docx' | 'spreadsheet' | 'csv' | 'pptx'; + +/** + * Classify a file by extension OR MIME into the bucket the office HTML + * dispatcher would route it to, or `null` if no producer applies. * * Extension wins over MIME because content sniffing routinely labels these - * formats as `application/zip` or `application/octet-stream`. + * formats as `application/zip` or `application/octet-stream`. MIME-only + * routing is the fallback for extensionless filenames where the upstream + * supplied a useful Content-Type header. */ -export async function bufferToOfficeHtml( - buffer: Buffer, - name: string, - mimeType: string, -): Promise { +export function officeHtmlBucket(name: string, mimeType: string): OfficeHtmlBucket | null { const ext = extensionOf(name); if (ext === 'docx' || mimeType === DOCX_MIME) { - return wordDocToHtml(buffer); + return 'docx'; } if (ext === 'csv' || CSV_MIME_PATTERN.test(mimeType)) { - return csvToHtml(buffer); + return 'csv'; } if ( ext === 'xlsx' || @@ -677,10 +687,35 @@ export async function bufferToOfficeHtml( excelMimeTypes.test(mimeType) || mimeType === ODS_MIME ) { - return excelSheetToHtml(buffer); + return 'spreadsheet'; } if (ext === 'pptx' || mimeType === PPTX_MIME) { - return pptxToSlideListHtml(buffer); + return 'pptx'; } return null; } + +/** + * Route an office-format buffer to the matching HTML producer. Returns `null` + * if the file isn't a recognized office type — caller should fall through to + * its existing text/binary handling. + */ +export async function bufferToOfficeHtml( + buffer: Buffer, + name: string, + mimeType: string, +): Promise { + const bucket = officeHtmlBucket(name, mimeType); + switch (bucket) { + case 'docx': + return wordDocToHtml(buffer); + case 'csv': + return csvToHtml(buffer); + case 'spreadsheet': + return excelSheetToHtml(buffer); + case 'pptx': + return pptxToSlideListHtml(buffer); + default: + return null; + } +}