fix(media): case-sensitive Content-Type checks bypass header extension and image-header safety rules#111184
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 20, 2026, 12:47 AM ET / 04:47 UTC. Summary PR surface: Source -1, Tests 0. Total -1 across 2 files. Reproducibility: yes. Current main's two trim-only helper calls leave Application/Zip and IMAGE/PNG unnormalized; the focused mixed-case cases in src/media/store.test.ts provide a high-confidence reproduction path without needing configuration or external services. Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Next step before merge
Security Review detailsBest possible solution: Merge this narrow normalization fix after the current exact-head checks finish, keeping media-store header decisions on the established media-core MIME normalization contract. Do we have a high-confidence way to reproduce the issue? Yes. Current main's two trim-only helper calls leave Application/Zip and IMAGE/PNG unnormalized; the focused mixed-case cases in src/media/store.test.ts provide a high-confidence reproduction path without needing configuration or external services. Is this the best way to solve the issue? Yes. Reusing media-core's existing normalizeMimeType at the two raw-header decision points is the narrowest maintainable fix and avoids a competing MIME-normalization rule in the media store. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against ce9e39319640. Label changesLabel justifications:
Evidence reviewedPR surface: Source -1, Tests 0. Total -1 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
Co-authored-by: 潘晓波0668000512 <[email protected]>
|
Merged via squash.
|
…n and image-header safety rules (openclaw#111184) * fix(media): normalize Content-Type casing in header extension and image-header checks * test(media): consolidate MIME case coverage Co-authored-by: 潘晓波0668000512 <[email protected]> --------- Co-authored-by: Peter Steinberger <[email protected]>
What Problem This Solves
Fixes an issue where mixed-case HTTP Content-Type headers such as
IMAGE/PNGorApplication/Zipbypass the media store's header-extension safety rules. RFC 9110 defines Content-Type as case-insensitive, but the store's helper functions usednormalizeOptionalString(trim-only) instead ofnormalizeMimeType(lowercase), so case-mismatched values skipped the exclusion gates and produced inconsistent extensions.Two concrete divergences from pinned behavior:
isImageHeaderMime(IMAGE/PNG)returnsfalse— the image-header detection fails and a trusted header.pngextension is kept for a generic ZIP container, whereas the lowercase equivalent is pinned to.zipby an existing test.extensionForAuthoritativeHeaderMime(Application/Zip)returns.zipinstead of skipping — the ZIP exclusion at line 253 misses because the string comparison is case-sensitive.Why This Change Was Made
Both
extensionForAuthoritativeHeaderMimeandisImageHeaderMimenow usenormalizeMimeTypeinstead ofnormalizeOptionalString(contentType?.split(";")[0]).normalizeMimeTypelowercases, strips parameters, and folds APNG to PNG — exactly the same normalizer used bydetectMime,extensionForMime, and the rest of the media-core MIME pipeline. The fix adds one import and changes two call sites.User Impact
Mixed-case Content-Type headers (rare but valid per RFC 9110) produce the same safe extensions and header decisions as their lowercase equivalents. No config, schema, storage format, or dependency changes.
Evidence
Live behavior proof (before/after)
Before (current main) — mixed-case
Application/Zipbypasses the ZIP exclusion and produces the wrong extension:After (this patch) — both mixed-case tests pass identically to their lowercase counterparts:
Regression coverage
src/media/store.test.ts(2 new):IMAGE/PNG+ ZIP bytes → correctly rejected as image header →.zipextension;Application/Zip+ ZIP bytes +.docxfilename → correctly excluded as ZIP header →.docxextension.Application/Ziptest fails (ZIP exclusion missed →.zipextension returned instead of.docx). The 48 existing tests pass unchanged.node scripts/run-vitest.mjs src/media/store.test.ts→ 48 passed (+ 2 new).oxlintandoxfmtpass on the two changed files.Sibling-surface context
The two helpers changed are the only call sites in
src/media/store.tsthat parse Content-Type without lowercasing. The three callers —saveMediaBuffer,saveMediaStream, andsaveMediaSource— all pass the raw content type through these helpers; no other file reads Content-Type in this module. The broader MIME pipeline inpackages/media-corealready lowercases vianormalizeMimeTypeat entry (thedetectMimeflow called at lines 495-498 callsnormalizeMimeTypeitself).