diff --git a/client/src/utils/__tests__/artifacts.test.ts b/client/src/utils/__tests__/artifacts.test.ts index f8ad7a6a58..2f3c99bd4c 100644 --- a/client/src/utils/__tests__/artifacts.test.ts +++ b/client/src/utils/__tests__/artifacts.test.ts @@ -107,6 +107,11 @@ describe('detectArtifactTypeFromFile', () => { ['application/vnd.oasis.opendocument.spreadsheet', TOOL_ARTIFACT_TYPES.SPREADSHEET], ['text/csv', TOOL_ARTIFACT_TYPES.SPREADSHEET], ['application/csv', TOOL_ARTIFACT_TYPES.SPREADSHEET], + /* Legacy CSV MIME variant — backend's `CSV_MIME_PATTERN` accepts + * it, so the client must too or extensionless CSVs with this MIME + * would be skipped despite the backend producing valid HTML. + * Regression for Codex P3 review on PR #12934. */ + ['text/comma-separated-values', TOOL_ARTIFACT_TYPES.SPREADSHEET], [ 'application/vnd.openxmlformats-officedocument.presentationml.presentation', TOOL_ARTIFACT_TYPES.PRESENTATION, diff --git a/client/src/utils/artifacts.ts b/client/src/utils/artifacts.ts index 6d4eede87d..e6cf67310c 100644 --- a/client/src/utils/artifacts.ts +++ b/client/src/utils/artifacts.ts @@ -609,6 +609,13 @@ const MIME_TO_TOOL_ARTIFACT_TYPE: Record = { 'application/vnd.oasis.opendocument.spreadsheet': TOOL_ARTIFACT_TYPES.SPREADSHEET, 'text/csv': TOOL_ARTIFACT_TYPES.SPREADSHEET, 'application/csv': TOOL_ARTIFACT_TYPES.SPREADSHEET, + /* `text/comma-separated-values` is a legacy CSV MIME variant — rare + * in modern HTTP traffic but still emitted by some sandboxes. Kept + * in lock-step with the backend's `CSV_MIME_PATTERN` in + * `packages/api/src/files/documents/html.ts` so an extensionless CSV + * with this MIME doesn't slip through the client routing while the + * backend has already produced full HTML for it. */ + 'text/comma-separated-values': TOOL_ARTIFACT_TYPES.SPREADSHEET, 'application/vnd.openxmlformats-officedocument.presentationml.presentation': TOOL_ARTIFACT_TYPES.PRESENTATION, // Note: bare `text/plain` is NOT mapped here. The extension map handles diff --git a/packages/api/src/files/documents/html.spec.ts b/packages/api/src/files/documents/html.spec.ts index 70a97c1a77..e285dedcef 100644 --- a/packages/api/src/files/documents/html.spec.ts +++ b/packages/api/src/files/documents/html.spec.ts @@ -290,6 +290,38 @@ describe('Office HTML producers', () => { ])('extension wins over conflicting MIME: (%s, %s) → %s', (name, mime, expected) => { expect(officeHtmlBucket(name, mime)).toBe(expected); }); + + /* Regression for Codex P2 review on PR #12934. Real Content-Type + * headers carry parameters like `; charset=utf-8` and `; boundary`; + * the predicate must strip them before matching, otherwise the + * backend silently falls through to raw text while the client's + * `baseMime` strips the same parameters and routes the file to the + * spreadsheet bucket — yielding a broken preview. */ + it.each([ + ['data', 'text/csv; charset=utf-8', 'csv'], + ['data', 'text/csv;charset=utf-8', 'csv'], + ['data', 'TEXT/CSV; CHARSET=UTF-8', 'csv'], + ['data', 'application/csv; charset=ascii', 'csv'], + ['data', 'text/comma-separated-values', 'csv'], + ['data', 'text/comma-separated-values; charset=utf-8', 'csv'], + [ + 'workbook', + 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet; charset=binary', + 'spreadsheet', + ], + [ + 'report', + 'application/vnd.openxmlformats-officedocument.wordprocessingml.document; charset=binary', + 'docx', + ], + [ + 'deck', + 'application/vnd.openxmlformats-officedocument.presentationml.presentation; foo=bar', + 'pptx', + ], + ])('strips MIME parameters before matching: (%s, %s) → %s', (name, mime, expected) => { + expect(officeHtmlBucket(name, mime)).toBe(expected); + }); }); describe('zip-bomb defense (SEC review on PR #12934)', () => { diff --git a/packages/api/src/files/documents/html.ts b/packages/api/src/files/documents/html.ts index 62505fffa2..f8d8c2a7c0 100644 --- a/packages/api/src/files/documents/html.ts +++ b/packages/api/src/files/documents/html.ts @@ -675,6 +675,24 @@ function extensionOf(name: string): string { return name.slice(dot + 1).toLowerCase(); } +/** + * Strip MIME parameters (`; charset=utf-8`, `; boundary=...`) and + * lowercase the result so exact-match MIME comparisons survive servers + * and tool sandboxes that decorate Content-Type headers. Without this, + * an extensionless `text/csv; charset=utf-8` would slip past the bucket + * predicate (returning null) while the client's MIME router — which + * already strips parameters via `baseMime` — would route it to + * SPREADSHEET expecting an HTML body that the backend never produced. + * Mirrors `client/src/utils/artifacts.ts:baseMime`. + */ +function baseMime(mime: string): string { + if (!mime) { + return ''; + } + const semi = mime.indexOf(';'); + return (semi < 0 ? mime : mime.slice(0, semi)).trim().toLowerCase(); +} + /** * The four buckets the dispatcher can route to. Used by both * `bufferToOfficeHtml` (which actually renders) and `officeHtmlBucket` @@ -716,19 +734,23 @@ export function officeHtmlBucket(name: string, mimeType: string): OfficeHtmlBuck 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) { + /* MIME-only fallback. Strip parameters first — Content-Type headers + * routinely carry `; charset=utf-8` or `; boundary=...` decorations + * that would otherwise fail exact-match comparisons. Order within the + * fallback doesn't matter: there are no overlapping MIME patterns + * across buckets, and the extension table already took precedence + * above for any conflicting input. */ + const normalized = baseMime(mimeType); + if (normalized === DOCX_MIME) { return 'docx'; } - if (CSV_MIME_PATTERN.test(mimeType)) { + if (CSV_MIME_PATTERN.test(normalized)) { return 'csv'; } - if (excelMimeTypes.test(mimeType) || mimeType === ODS_MIME) { + if (excelMimeTypes.test(normalized) || normalized === ODS_MIME) { return 'spreadsheet'; } - if (mimeType === PPTX_MIME) { + if (normalized === PPTX_MIME) { return 'pptx'; } return null;