feat: make TEXT_MIME_ALLOWLIST configurable via env var - #2511
feat: make TEXT_MIME_ALLOWLIST configurable via env var#2511revanth-045 wants to merge 3 commits into
Conversation
…WLIST Allows self-hosted deployments to extend the default text MIME allowlist via an env var instead of forking the code. The variable takes a comma-separated list of MIME types or patterns (e.g. "text/x-typescript") which is appended to the built-in defaults, so existing behaviour is unchanged when it is unset. Closes huggingface#2194 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3cbd3ea75d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!value) return []; | ||
| return value | ||
| .split(",") | ||
| .map((entry) => entry.trim()) |
There was a problem hiding this comment.
When an operator supplies a MIME type with uppercase characters, such as Application/X-Yaml, server processing accepts it because prepareFiles lowercases each allowlist entry, but mimeMatchesAllowlist compares the configured entry and browser-provided file.type case-sensitively. Consequently, drag-and-drop, paste, and programmatic attachments are rejected even though the server considers the type allowed; normalize entries to lowercase while parsing them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in dd608e4.
Entries are now lowercased in parseExtraMimeTypes, so Application/X-Yaml normalises to application/x-yaml and the client matcher agrees with the server. Added a test covering it.
Worth noting the underlying inconsistency is wider than this PR: mimeMatchesAllowlist lowercases neither side, while prepareFiles and UploadedFile's matchesAllowed both lowercase both sides. Normalising at parse time keeps this PR scoped, but making mimeMatchesAllowlist case-insensitive would be a sensible follow-up.
…he server `prepareFiles` lowercases each allowlist entry before comparing, but `mimeMatchesAllowlist` — used by drag-and-drop, paste and programmatic attachments — compares case-sensitively. An operator writing `Application/X-Yaml` would have the file accepted server-side and rejected in the browser. MIME types are case-insensitive, so normalise entries at parse time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Addressed the Codex review in dd608e4 — configured MIME types are lowercased at parse time so the case-sensitive client matcher agrees with the server, with a test for it.
|
|
Checking in — this has been open a few weeks with CI green and the Codex feedback addressed (dd608e4). Happy to make any further changes if there's a preferred direction; let me know if anything's blocking a look. |
1 participant
TEXT_MIME_ALLOWLISTinsrc/lib/constants/mime.tsis hardcoded, so self-hosting this means forking the repo just to accept one extra file type — dropping in a.pyor.yamlfile currently fails with "Some file type not supported".This adds an optional
PUBLIC_TEXT_MIME_ALLOWLISTenv var: a comma-separated list of MIME types or patterns that gets appended to the existing defaults (kept asTEXT_MIME_ALLOWLIST_DEFAULT).It ships empty, and when it's unset the allowlist is exactly what it is today, so nothing changes for existing deployments. Documented in
.env.Happy to flip it to replace-instead-of-append, or rename the variable, if you'd rather have it work differently — the issue left that open.
This should be complementary to #2189 rather than overlapping with it: that one lets an individual model declare the mimetypes it accepts, whereas this is a deployment-wide default for what counts as a text file in the first place.
Closes #2194
Checks
npm run check,npm run lintandnpm run buildall pass. Addedsrc/lib/constants/mime.spec.tscovering the unset/empty fallback, appending, whitespace handling and wildcard patterns.One known limitation
.tsuploads work when the browser reports a sensible MIME type, but some OS/browser combinations report.tsasvideo/mp2tinstead, so those won't be picked up as text even with the type allowlisted. That's a pre-existing MIME-sniffing quirk rather than something this PR introduces — fixing it properly would need content sniffing (one of the alternatives floated in #2194), which felt like a separate change.