diff --git a/packages/api/src/files/documents/html.spec.ts b/packages/api/src/files/documents/html.spec.ts index fffa8aa011..bed14a2e57 100644 --- a/packages/api/src/files/documents/html.spec.ts +++ b/packages/api/src/files/documents/html.spec.ts @@ -262,6 +262,32 @@ describe('Office HTML producers', () => { ])('returns null for non-office (%s, %s)', (name, mime) => { expect(officeHtmlBucket(name, mime)).toBeNull(); }); + + /* Regression for Codex P2 review on PR #12934. A binary office file + * with a mismatched MIME (e.g. a tool sandbox sets `text/csv` on + * everything it ships) must NOT be re-routed to a different bucket + * just because the MIME matches a different bucket's pattern. The + * documented "extension wins" precedence is enforced by checking + * extensions exhaustively before any MIME pattern fires. */ + it.each([ + ['deck.pptx', 'text/csv', 'pptx'], + ['workbook.xlsx', 'text/csv', 'spreadsheet'], + [ + 'legacy.xls', + 'application/vnd.openxmlformats-officedocument.presentationml.presentation', + 'spreadsheet', + ], + ['sheet.ods', 'text/csv', 'spreadsheet'], + ['report.docx', 'text/csv', 'docx'], + ['report.docx', 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', 'docx'], + [ + 'data.csv', + 'application/vnd.openxmlformats-officedocument.wordprocessingml.document', + 'csv', + ], + ])('extension wins over conflicting MIME: (%s, %s) → %s', (name, mime, expected) => { + expect(officeHtmlBucket(name, mime)).toBe(expected); + }); }); describe('sanitizeOfficeHtml security', () => { diff --git a/packages/api/src/files/documents/html.ts b/packages/api/src/files/documents/html.ts index 13e5826401..d4df8e30fe 100644 --- a/packages/api/src/files/documents/html.ts +++ b/packages/api/src/files/documents/html.ts @@ -663,33 +663,47 @@ function extensionOf(name: string): string { */ export type OfficeHtmlBucket = 'docx' | 'spreadsheet' | 'csv' | 'pptx'; +const OFFICE_EXTENSIONS: Record = { + docx: 'docx', + csv: 'csv', + xlsx: 'spreadsheet', + xls: 'spreadsheet', + ods: 'spreadsheet', + pptx: '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`. MIME-only - * routing is the fallback for extensionless filenames where the upstream - * supplied a useful Content-Type header. + * **Extension always wins** over MIME for any known office extension. + * Without this precedence, a `deck.pptx` whose `Content-Type` was + * mislabeled `text/csv` (a known footgun when tool sandboxes ship + * generic MIMEs) would be routed to `csvToHtml` and try to parse a + * binary PPTX as CSV — yielding garbage at best, a parse exception at + * worst. MIME-only routing is the fallback for extensionless filenames + * (or extensions outside our known set) where the upstream supplied a + * useful Content-Type header. */ export function officeHtmlBucket(name: string, mimeType: string): OfficeHtmlBucket | null { const ext = extensionOf(name); - if (ext === 'docx' || mimeType === DOCX_MIME) { + const byExtension = OFFICE_EXTENSIONS[ext]; + if (byExtension) { + return byExtension; + } + /* MIME-only fallback. Order doesn't matter here — there are no + * overlapping MIME patterns across buckets, and the extension table + * has already taken precedence above for any conflicting input. */ + if (mimeType === DOCX_MIME) { return 'docx'; } - if (ext === 'csv' || CSV_MIME_PATTERN.test(mimeType)) { + if (CSV_MIME_PATTERN.test(mimeType)) { return 'csv'; } - if ( - ext === 'xlsx' || - ext === 'xls' || - ext === 'ods' || - excelMimeTypes.test(mimeType) || - mimeType === ODS_MIME - ) { + if (excelMimeTypes.test(mimeType) || mimeType === ODS_MIME) { return 'spreadsheet'; } - if (ext === 'pptx' || mimeType === PPTX_MIME) { + if (mimeType === PPTX_MIME) { return 'pptx'; } return null;