🛡️ fix: Enforce extension-wins precedence in officeHtmlBucket

Codex review on #12934 caught that the predicate's if-chain interleaved
extension and MIME checks for each bucket — e.g. CSV's branch was
`ext === 'csv' || CSV_MIME_PATTERN.test(mimeType)`. A `deck.pptx`
shipped with `text/csv` (sandboxed tools sometimes ship generic MIMEs)
matched the CSV branch BEFORE the PPTX extension branch was reached,
so a binary PPTX would have been handed to `csvToHtml` to parse as
text — yielding garbage or a parse exception.

Restructure to a strict two-pass dispatch: an exhaustive extension
table first (one lookup, all known extensions), then MIME-only
fallback for extensionless / unknown-ext inputs. The doc comment's
"extension wins" claim is now actually enforced by the implementation.

Add 7 regression cases covering the conflicting-MIME footgun for each
bucket: deck.pptx + text/csv → pptx; workbook.xlsx + text/csv →
spreadsheet; legacy.xls + pptx-MIME → spreadsheet; report.docx +
text/csv → docx; data.csv + docx-MIME → csv; etc.
This commit is contained in:
Danny Avila 2026-05-04 06:49:49 +09:00
parent f87a644eaf
commit b41055b5ba
2 changed files with 54 additions and 14 deletions

View file

@ -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', () => {

View file

@ -663,33 +663,47 @@ function extensionOf(name: string): string {
*/
export type OfficeHtmlBucket = 'docx' | 'spreadsheet' | 'csv' | 'pptx';
const OFFICE_EXTENSIONS: Record<string, OfficeHtmlBucket> = {
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;