Commit b485768
authored
fix(dashboard): pin MIME types for the vendored static assets (#3193)
## Description
The dashboard's vendored scripts can be served as `text/plain`, and the
proxy's own
`X-Content-Type-Options: nosniff` then stops the browser executing them
— the dashboard
loads unstyled and dataless.
`StaticFiles` types every response from `mimetypes.guess_type`, and
Python seeds that
database from the host: the Windows registry (`HKCR\<ext>\Content Type`)
and, elsewhere,
files like `/etc/mime.types`. headroom never calls `mimetypes.add_type`
anywhere, so it
inherits whatever the host says. On a host that maps `.js` to
`text/plain` — a stale
registry entry, or a minimal container image with no mime database at
all — the three
vendored assets go out as plain text.
Neither half is wrong on its own. `nosniff` at `_apply_security_headers`
is correct and
should stay; the mislabel is the bug. Together they break the dashboard
completely.
Closes #3179
## Type of Change
- [x] Bug fix (non-breaking change that fixes an issue)
## Changes Made
- Added `register_static_mime_types()` and the `_STATIC_MIME_TYPES`
table to `headroom/dashboard/__init__.py`, next to the `STATIC_DIR` it
describes.
- `create_app` calls it immediately before mounting `/dashboard/static`,
so the served type no longer depends on the host mime database.
- Registered `.js`/`.mjs` as `text/javascript`, `.css` as `text/css`,
and `.json`/`.map` as `application/json`.
- Added `tests/test_dashboard_static_mime_types.py` (11 tests) covering
a deliberately broken host database, each registered extension,
idempotency, and a guard that fails if a future vendored asset arrives
with an unregistered extension.
### Design notes
`mimetypes.add_type` is strict by default, so these registrations
replace a bad host
entry rather than losing to it. They are the current IANA/WHATWG values,
so this only
ever repairs a host database — it never invents a mapping.
Registration runs from `create_app` rather than at module import.
Mutating the
process-wide table is right for the proxy that serves these files, but
it should not be
a side effect of `import headroom` for someone using the library.
Two deliberate departures from the fix sketched in the issue:
`text/javascript` rather
than `application/javascript` (the current registration, and what Python
3.12+ returns
natively, so the fix converges with the stdlib instead of diverging from
it — both
execute in every browser), and `.map` as `application/json` rather than
`application/javascript`, since a source map is a JSON document.
## Testing
- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [x] Type checking passes (`mypy headroom`)
- [x] New tests added for new functionality
- [x] Manual testing performed
### Test Output
```text
$ python -m pytest tests/test_dashboard_static_mime_types.py -q
11 passed, 1 warning in 0.94s
# against the unpatched tree the same file cannot even import:
ERROR tests/test_dashboard_static_mime_types.py
ImportError: cannot import name 'register_static_mime_types' from 'headroom.dashboard'
$ python -m pytest tests/*dashboard* -q --continue-on-collection-errors
2 failed, 18 passed, 5 skipped, 2 errors in 11.75s
# baseline on the same tree with the fix stashed:
2 failed, 7 passed, 5 skipped, 2 errors in 6.98s
# identical failures/errors either way (they need the Rust _core extension, which is
# not built on this machine); the fix adds the 11 passing tests and breaks nothing.
$ python -m ruff check headroom/dashboard/__init__.py headroom/proxy/server.py tests/test_dashboard_static_mime_types.py
All checks passed!
$ python -m ruff format --check ...
3 files already formatted
$ python -m mypy headroom/dashboard/__init__.py headroom/proxy/server.py
Success: no issues found in 2 source files
```
## Real Behavior Proof
- Environment: Windows 11 Home 26200, Python 3.11.9, clone of
`upstream/main` at `202c189`. This machine's registry happens to have no
`.js` Content Type value, so the reporter's broken host was reproduced
by `mimetypes.add_type("text/plain", ".js")` — precisely the state
Python's `mimetypes` loads from a registry that does have it.
- Exact command / steps: mounted the real `headroom/dashboard/static`
directory through Starlette `StaticFiles` exactly as `create_app`
constructs it, then fetched all three assets over `TestClient` twice in
one process — first with no registration (today's behaviour), then after
calling `register_static_mime_types()` (the new behaviour).
- Observed result: before the fix all three assets are served
`text/plain; charset=utf-8`, which is what `nosniff` blocks and what the
reporter's console errors show; after the fix all three are
`text/javascript; charset=utf-8`. 3/3 blocked before, 3/3 executable
after. Full output below.
- Not tested: a real browser against a real Windows host carrying the
bad registry entry; and the `create_app` wiring itself, because the
proxy module will not import on this machine (the Rust `_core` extension
is unbuilt and there is no toolchain here) — that one line is covered by
CI rather than locally.
```text
using package: ...\headroom\headroom\dashboard\__init__.py
host mimetypes: .js -> text/plain
BEFORE (create_app does not register anything):
alpine.min.js 200 text/plain; charset=utf-8
htmx.min.js 200 text/plain; charset=utf-8
tailwind.min.js 200 text/plain; charset=utf-8
after register_static_mime_types(): .js -> text/javascript
AFTER (create_app calls register_static_mime_types before mounting):
alpine.min.js 200 text/javascript; charset=utf-8
htmx.min.js 200 text/javascript; charset=utf-8
tailwind.min.js 200 text/javascript; charset=utf-8
blocked before: 3/3 executable after: 3/3
```
## Runtime Rollout Safety
- Rollout-managed feature(s): none — an unconditional correctness fix,
not a rollout-channel feature.
- Minimum rollout channel: n/a — applies on every channel.
- Stable/default behavior changed: yes, deliberately — dashboard assets
are now served with a correct `Content-Type` on hosts whose mime
database was wrong. On a host that was already correct, the served
headers are unchanged.
- Kill switch / disable path: none needed; behaviour is inert where the
host database is already right. Reverting the commit restores the
previous behaviour.
- Unsafe override required: no.
- Qualification impact: none — no effect on compression, proxying, or
provider behavior. Only the `/dashboard/static` mount is touched.
- Rollback path: revert the commit; no persisted state, no migration, no
config.
## Review Readiness
- [x] I have performed a self-review
- [x] This PR is ready for human review
## Checklist
- [x] My code follows the project's style guidelines
- [x] I have performed a self-review of my code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] New and existing unit tests pass locally with my changes
- [x] I did **not** edit `CHANGELOG.md` — it is generated by
release-please from my Conventional Commit PR title (a CI guard enforces
this)
## Additional Notes
**Alternatives considered.** Subclassing `StaticFiles` to force a
`Content-Type` per
extension avoids touching the global table at all and would be scoped to
the dashboard
mount, but it means overriding Starlette internals for no gain in
correctness. Relaxing
`nosniff` on the static mount would also make the dashboard work, but it
trades a
security header away to paper over a labelling bug. Serving each asset
from an explicit
route with a hardcoded `media_type` works too, but replaces
`StaticFiles` wholesale.
**Scope.** Only `.js` is served from `STATIC_DIR` today; `.mjs`, `.css`,
`.json` and
`.map` are registered because they would fail in exactly the same way
the moment one is
vendored. `test_every_vendored_asset_extension_is_registered` fails if
an asset appears
with an extension the table does not cover, so the list cannot silently
fall behind.
Happy to trim it to `.js` alone if you would rather keep the surface
minimal.1 parent 9c30b62 commit b485768
3 files changed
Lines changed: 158 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
| 3 | + | |
3 | 4 | | |
4 | 5 | | |
5 | 6 | | |
| |||
10 | 11 | | |
11 | 12 | | |
12 | 13 | | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
13 | 52 | | |
14 | 53 | | |
15 | 54 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3642 | 3642 | | |
3643 | 3643 | | |
3644 | 3644 | | |
3645 | | - | |
| 3645 | + | |
| 3646 | + | |
| 3647 | + | |
| 3648 | + | |
| 3649 | + | |
| 3650 | + | |
3646 | 3651 | | |
3647 | 3652 | | |
3648 | 3653 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
0 commit comments