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.
This commit is contained in:
Ayush Kumar Jha
@@ -1,5 +1,6 @@
|
||||
"""Headroom Dashboard - Real-time proxy monitoring UI."""
|
||||
|
||||
import mimetypes
|
||||
from pathlib import Path
|
||||
|
||||
DASHBOARD_DIR = Path(__file__).parent
|
||||
@@ -10,6 +11,44 @@ TEMPLATES_DIR = DASHBOARD_DIR / "templates"
|
||||
STATIC_DIR = DASHBOARD_DIR / "static"
|
||||
|
||||
|
||||
#: Correct types for the asset kinds the dashboard mount can serve. Values are
|
||||
#: the current IANA/WHATWG registrations, so this only ever repairs a host
|
||||
#: database — it never invents a mapping of our own.
|
||||
_STATIC_MIME_TYPES: tuple[tuple[str, str], ...] = (
|
||||
("text/javascript", ".js"),
|
||||
("text/javascript", ".mjs"),
|
||||
("text/css", ".css"),
|
||||
("application/json", ".json"),
|
||||
# Source maps are JSON documents even though they accompany .js assets.
|
||||
("application/json", ".map"),
|
||||
)
|
||||
|
||||
|
||||
def register_static_mime_types() -> None:
|
||||
r"""Pin the MIME types used for the vendored dashboard assets.
|
||||
|
||||
``StaticFiles`` derives ``Content-Type`` from :func:`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``. A host that maps ``.js`` to ``text/plain`` — a stale
|
||||
registry entry, or a minimal container image carrying no mime database at
|
||||
all — makes the proxy serve ``alpine.min.js`` and its siblings as plain
|
||||
text. The proxy also sends ``X-Content-Type-Options: nosniff`` on every
|
||||
response, so the browser refuses to execute a script labelled that way and
|
||||
the dashboard loads unstyled and dataless (#3179).
|
||||
|
||||
Registering the standard mappings makes the served type independent of the
|
||||
host database. :func:`mimetypes.add_type` is strict by default, so these
|
||||
replace a bad host entry rather than losing to it.
|
||||
|
||||
Called from ``create_app`` rather than at import time: correcting the
|
||||
process-wide table is right for the proxy that serves these files, but it
|
||||
is not a side effect ``import headroom`` should have on a library consumer.
|
||||
"""
|
||||
for mime_type, extension in _STATIC_MIME_TYPES:
|
||||
mimetypes.add_type(mime_type, extension)
|
||||
|
||||
|
||||
def get_dashboard_html() -> str:
|
||||
"""Load the dashboard HTML template."""
|
||||
template_path = TEMPLATES_DIR / "dashboard.html"
|
||||
|
||||
@@ -3642,7 +3642,12 @@ def create_app(config: ProxyConfig | None = None) -> FastAPI:
|
||||
# register_provider_routes' catch-all so it is not tunneled upstream.
|
||||
from starlette.staticfiles import StaticFiles
|
||||
|
||||
from headroom.dashboard import STATIC_DIR
|
||||
from headroom.dashboard import STATIC_DIR, register_static_mime_types
|
||||
|
||||
# A host whose mime database maps .js to text/plain (stale Windows registry
|
||||
# entry, minimal container image) would otherwise have these served as plain
|
||||
# text, which the nosniff header above then blocks in the browser (#3179).
|
||||
register_static_mime_types()
|
||||
|
||||
# check_dir=False keeps a missing assets directory from aborting proxy
|
||||
# startup: the dashboard JS 404s, but proxying itself still works.
|
||||
|
||||
@@ -0,0 +1,113 @@
|
||||
"""Dashboard assets must be served as JavaScript even where the host disagrees.
|
||||
|
||||
``StaticFiles`` types every response from :func:`mimetypes.guess_type`, and
|
||||
Python seeds that database from the host — the Windows registry, or files like
|
||||
``/etc/mime.types``. Paired with the proxy's unconditional
|
||||
``X-Content-Type-Options: nosniff``, a host that calls ``.js`` ``text/plain``
|
||||
stops the browser executing the dashboard entirely (#3179).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import mimetypes
|
||||
|
||||
import pytest
|
||||
|
||||
pytest.importorskip("starlette")
|
||||
|
||||
from starlette.applications import Starlette # noqa: E402
|
||||
from starlette.staticfiles import StaticFiles # noqa: E402
|
||||
from starlette.testclient import TestClient # noqa: E402
|
||||
|
||||
from headroom.dashboard import ( # noqa: E402
|
||||
_STATIC_MIME_TYPES,
|
||||
STATIC_DIR,
|
||||
register_static_mime_types,
|
||||
)
|
||||
|
||||
ASSETS = ["tailwind.min.js", "htmx.min.js", "alpine.min.js"]
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _restore_mime_db():
|
||||
"""Rebuild the process-wide table afterwards — ``add_type`` mutates it."""
|
||||
yield
|
||||
mimetypes.init()
|
||||
|
||||
|
||||
def _break_host_db(*extensions: str) -> None:
|
||||
"""Simulate a host whose mime database calls these extensions plain text."""
|
||||
for extension in extensions or (".js",):
|
||||
mimetypes.add_type("text/plain", extension)
|
||||
|
||||
|
||||
def _static_client() -> TestClient:
|
||||
"""Mount the real vendored assets exactly as ``create_app`` does."""
|
||||
app = Starlette()
|
||||
app.mount("/dashboard/static", StaticFiles(directory=STATIC_DIR, check_dir=False))
|
||||
return TestClient(app)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("asset", ASSETS)
|
||||
def test_asset_is_javascript_on_a_broken_host(asset: str) -> None:
|
||||
_break_host_db()
|
||||
register_static_mime_types()
|
||||
|
||||
with _static_client() as client:
|
||||
resp = client.get(f"/dashboard/static/{asset}")
|
||||
|
||||
assert resp.status_code == 200, resp.text
|
||||
# Under nosniff the browser executes nothing that is not a JavaScript type.
|
||||
assert "javascript" in resp.headers["content-type"]
|
||||
|
||||
|
||||
def test_registration_overrides_a_bad_host_mapping() -> None:
|
||||
"""``add_type`` is strict by default, so it must win against the host."""
|
||||
_break_host_db()
|
||||
assert mimetypes.guess_type("alpine.min.js")[0] == "text/plain"
|
||||
|
||||
register_static_mime_types()
|
||||
|
||||
assert mimetypes.guess_type("alpine.min.js")[0] == "text/javascript"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("filename", "expected"),
|
||||
[
|
||||
("app.js", "text/javascript"),
|
||||
("module.mjs", "text/javascript"),
|
||||
("styles.css", "text/css"),
|
||||
("data.json", "application/json"),
|
||||
# A source map is a JSON document, despite shipping beside .js assets.
|
||||
("bundle.js.map", "application/json"),
|
||||
],
|
||||
)
|
||||
def test_each_registered_extension_resolves(filename: str, expected: str) -> None:
|
||||
_break_host_db(*(extension for _type, extension in _STATIC_MIME_TYPES))
|
||||
|
||||
register_static_mime_types()
|
||||
|
||||
assert mimetypes.guess_type(filename)[0] == expected
|
||||
|
||||
|
||||
def test_registration_is_idempotent() -> None:
|
||||
"""``create_app`` may run more than once in a process (tests, embedding)."""
|
||||
_break_host_db()
|
||||
register_static_mime_types()
|
||||
register_static_mime_types()
|
||||
|
||||
assert mimetypes.guess_type("alpine.min.js")[0] == "text/javascript"
|
||||
|
||||
|
||||
def test_every_vendored_asset_extension_is_registered() -> None:
|
||||
"""A newly vendored asset kind must not silently inherit the host database."""
|
||||
registered = {extension for _type, extension in _STATIC_MIME_TYPES}
|
||||
unregistered = sorted(
|
||||
{
|
||||
path.suffix
|
||||
for path in STATIC_DIR.iterdir()
|
||||
if path.is_file() and path.suffix not in registered
|
||||
}
|
||||
)
|
||||
|
||||
assert unregistered == []
|
||||
Reference in New Issue
Block a user