diff --git a/packages/api/src/files/code/extract.spec.ts b/packages/api/src/files/code/extract.spec.ts index 500971d9f1..1450d95641 100644 --- a/packages/api/src/files/code/extract.spec.ts +++ b/packages/api/src/files/code/extract.spec.ts @@ -3,13 +3,17 @@ import * as path from 'path'; import { extractCodeArtifactText, MAX_TEXT_CACHE_BYTES, MAX_TEXT_EXTRACT_BYTES } from './extract'; const docxText = '__DOCX_PARSED__'; +/* parseDocument throws on any originalname containing this token, so + * tests can force a failure with whatever extension/MIME they need + * (the legacy docx-named constant is preserved for backward-compat + * with tests that still reference it). */ const docxFailureName = 'force-docx-failure.docx'; const parseDocumentCalls: Array<{ path: string; originalname: string }> = []; jest.mock('~/files/documents/crud', () => ({ parseDocument: jest.fn(async ({ file }: { file: { path: string; originalname: string } }) => { parseDocumentCalls.push({ path: file.path, originalname: file.originalname }); - if (file.originalname === docxFailureName) { + if (file.originalname.includes('force-docx-failure')) { throw new Error('parse failed'); } return { text: docxText, filename: file.originalname, bytes: docxText.length }; @@ -92,73 +96,78 @@ describe('extractCodeArtifactText', () => { }); describe('document', () => { + /* These tests exercise the legacy `extractDocument` path, which now + * only fires for `category === 'document'` files that the office HTML + * dispatcher does NOT claim — i.e. PDF and ODT. All ZIP-backed office + * formats (DOCX/XLSX/XLS/ODS) bypass `extractDocument` entirely + * because they're HTML-or-null per the SEC fix in PR #12934 + * (Codex P1 review: text-fallback under `index.html` was XSS). */ beforeEach(() => { parseDocumentCalls.length = 0; }); - it('routes through parseDocument and returns its text', async () => { - const buffer = Buffer.from('PKfake-docx'); + it('routes through parseDocument and returns its text (ODT)', async () => { + const buffer = Buffer.from('PKfake-odt'); const text = await extractCodeArtifactText( buffer, - 'report.docx', - 'application/vnd.openxmlformats-officedocument.wordprocessingml.document', + 'notes.odt', + 'application/vnd.oasis.opendocument.text', 'document', ); expect(text).toBe(docxText); }); it('returns null when parseDocument throws', async () => { - const buffer = Buffer.from('PKfake-docx'); + const buffer = Buffer.from('PKfake-odt'); const text = await extractCodeArtifactText( buffer, - docxFailureName, - 'application/vnd.openxmlformats-officedocument.wordprocessingml.document', + docxFailureName.replace('.docx', '.odt'), + 'application/vnd.oasis.opendocument.text', 'document', ); expect(text).toBeNull(); }); - it('rewrites a generic sniffed MIME to the canonical document MIME by extension', async () => { + it('rewrites a generic sniffed MIME to the canonical document MIME by extension (ODT)', async () => { // Code-output buffers for office docs are commonly sniffed as // application/zip — without canonicalization, parseDocument would // reject these and inline previews would silently disappear. - const buffer = Buffer.from('PKfake-docx'); - await extractCodeArtifactText(buffer, 'report.docx', 'application/zip', 'document'); - expect(parseDocumentCalls[0]?.originalname).toBe('report.docx'); + const buffer = Buffer.from('PKfake-odt'); + await extractCodeArtifactText(buffer, 'notes.odt', 'application/zip', 'document'); + expect(parseDocumentCalls[0]?.originalname).toBe('notes.odt'); }); it.each([ - [ - 'report.docx', - 'application/zip', - 'application/vnd.openxmlformats-officedocument.wordprocessingml.document', - ], - [ - 'data.xlsx', - 'application/octet-stream', - 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', - ], - ['legacy.xls', 'application/octet-stream', 'application/vnd.ms-excel'], - ['sheet.ods', 'application/zip', 'application/vnd.oasis.opendocument.spreadsheet'], ['notes.odt', 'application/zip', 'application/vnd.oasis.opendocument.text'], - ])('passes canonical mimetype for %s when sniff returns %s', async (name, sniffed, _canon) => { - const parseDocumentMock = ( - jest.requireMock('~/files/documents/crud') as { - parseDocument: jest.Mock; - } - ).parseDocument; - parseDocumentMock.mockClear(); - await extractCodeArtifactText(Buffer.from('PK'), name, sniffed, 'document'); - const call = parseDocumentMock.mock.calls[0]?.[0]; - expect(call?.file?.mimetype).toBe(_canon); - }); + ['report.pdf', 'application/octet-stream', 'application/pdf'], + ])( + 'passes canonical mimetype for %s when sniff returns %s (legacy parseDocument path)', + async (name, sniffed, _canon) => { + const parseDocumentMock = ( + jest.requireMock('~/files/documents/crud') as { + parseDocument: jest.Mock; + } + ).parseDocument; + parseDocumentMock.mockClear(); + await extractCodeArtifactText(Buffer.from('PK'), name, sniffed, 'document'); + const call = parseDocumentMock.mock.calls[0]?.[0]; + /* PDF doesn't have an entry in `documentMimeFromExtension` so the + * sniffed MIME passes through unchanged. ODT does — gets + * canonicalized to ODT_MIME. */ + expect(call?.file?.mimetype).toBe(_canon === 'application/pdf' ? sniffed : _canon); + }, + ); it('writes the temp file inside os.tmpdir() regardless of artifact name', async () => { - const buffer = Buffer.from('PKfake-docx'); + /* Path traversal defense — even a malicious filename like + * `../../../etc/passwd.odt` must end up inside os.tmpdir() with a + * sanitized basename. Uses ODT since the office HTML path now + * short-circuits all ZIP-backed office formats. */ + const buffer = Buffer.from('PKfake-odt'); await extractCodeArtifactText( buffer, - '../../../etc/passwd.docx', - 'application/vnd.openxmlformats-officedocument.wordprocessingml.document', + '../../../etc/passwd.odt', + 'application/vnd.oasis.opendocument.text', 'document', ); const call = parseDocumentCalls[0]; @@ -166,7 +175,24 @@ describe('extractCodeArtifactText', () => { const tmpRoot = path.resolve(os.tmpdir()); expect(path.resolve(call.path).startsWith(tmpRoot)).toBe(true); expect(call.path).not.toContain('..'); - expect(call.originalname).toBe('passwd.docx'); + expect(call.originalname).toBe('passwd.odt'); + }); + + it('does NOT call parseDocument for office HTML types (DOCX/XLSX/XLS/ODS)', async () => { + /* Lock in the SEC contract: the four office HTML buckets are + * HTML-or-null and never fall back to `extractDocument`. */ + mockOfficeHtml.mockResolvedValue(null); + const cases: Array<[string, string]> = [ + ['report.docx', 'application/vnd.openxmlformats-officedocument.wordprocessingml.document'], + ['data.xlsx', 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet'], + ['legacy.xls', 'application/vnd.ms-excel'], + ['sheet.ods', 'application/vnd.oasis.opendocument.spreadsheet'], + ]; + for (const [name, mime] of cases) { + const text = await extractCodeArtifactText(Buffer.from('PK'), name, mime, 'document'); + expect(text).toBeNull(); + } + expect(parseDocumentCalls.length).toBe(0); }); }); @@ -233,7 +259,13 @@ describe('extractCodeArtifactText', () => { expect(text).toContain('