Studio: stop seeded admin to cross-origin callers (#5739)

* Studio: stop leaking seeded admin pw to cross-origin callers The "/" SPA fallback serves index.html with an inline ``window.__UNSLOTH_BOOTSTRAP__`` script containing the seeded admin password while a password change is pending. Default web mode runs ``CORSMiddleware`` with ``allow_origins=["*"]`` + ``allow_credentials= True``, which reflects an attacker-controlled ``Origin`` back on every request and sets Access-Control-Allow-Credentials true. The combination let any cross-origin page ``fetch('/')`` with credentials and read the bootstrap admin password out of the HTML body. The API smoke ``CORS: GET / leaks bootstrap pw to cross-origin caller`` audit already tracked this (tests/studio/studio_api_smoke.py:224) but did not gate CI. Gate ``_inject_bootstrap`` on a same-origin check: legitimate top-level navigations omit ``Origin`` on most engines, so the absence of the header is treated as same-origin; when the header IS present and does not match ``request.url.scheme://request.url.netloc`` exactly, we now skip injecting the bootstrap tag. ``Vary: Origin`` is added so an intermediary cache cannot serve a same-origin response (with bootstrap) to a later cross-origin caller (and vice versa). Coverage: ``test_index_bootstrap_origin.py`` exercises the helper with missing / matching / evil / scheme-mismatch / port-mismatch origins. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: tighten comments on bootstrap cross-origin helper * Studio: canonicalise Origin before same-origin gate A plain string-compare between the Origin header and request.url.netloc misclassifies legitimate same-origin requests as cross-origin in three scenarios: - Browser strips the default port from Origin (https://example.com) but Starlette's netloc keeps it (example.com:443). Per RFC 6454 the default port is dropped on the wire, so the strings will not match even though the requests share an origin. - Host case differs (Origin: http://Example.com vs netloc: example.com). Per RFC 3986 host comparison is case-insensitive. - Scheme case differs (HTTP:// vs http://). Per RFC 3986 the scheme is also case-insensitive. These are usability degradations rather than security gaps (legitimate user denied the bootstrap injection, no attacker gain), but worth shipping so non-default Studio deployments keep the change-password auto-fill. Adds _canonical_origin(scheme, netloc) -> (scheme, host, port) and compares the canonical tuples. Default-port lookup covers http/https/ws/wss; userinfo (user:pass@) is stripped per RFC 3986 since Origin never carries credentials. Origin: "null" (sandboxed iframes, file:// pages) and unparseable values collapse to cross- origin so the bootstrap pw is never leaked through those paths either. Tests: 14 cases (was 5). Covers the original same/missing/evil/ scheme/port matrix plus default-port stripping in both directions, host + scheme case folding, Origin: null, garbage values, and userinfo-in-netloc. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix IPv6 netloc parsing for PR #5739 The canonical-origin helper used ``netloc.partition(":")`` which mis-parses bracketed IPv6 hosts (``[::1]:8902`` -> host=``[``, port-str=``:1]:8902``). The int() then raises and the canonicaliser returns None, so every IPv6 same-origin request is misclassified as cross-origin and Studio refuses to inject the bootstrap pw on a legitimate top-level nav when launched with ``unsloth studio -H ::1``. Bracket-aware split per RFC 3986 §3.2.2, plus extra regression tests for IPv6, opaque (data:/blob:/file:), comma-joined multi-Origin and localhost-vs-127 cases. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Guard urlparse ValueError in same-origin gate urlparse raises ValueError on malformed bracketed Origin values (unclosed [, invalid IPv6 hex, text after ]) and on a few NFKC edge cases since Py 3.8. Without a guard, a request carrying Origin: http://[malformed surfaced as HTTP 500 from the SPA handler rather than being treated as cross-origin per the docstring's safer-default rule. Wrap both urlparse calls in try/except ValueError and return False on parse failure. Also distinguish a missing Origin header (top-level same-document GET, treat as same-origin) from an explicit empty string (not a valid serialised origin per RFC 6454 §6.1, treat as cross-origin). Four new regression tests pinned down by the PR audit: malformed IPv6 bracket, invalid IPv6 hex, bracket with trailing garbage, and the empty Origin header. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Shorten origin-gate comments for PR #5739 --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
2026-05-25 23:36:51 -07:00 · 2026-05-25 23:36:51 -07:00 · 034ff512e7
commit 034ff512e7
parent b73480e554
3 changed files with 438 additions and 11 deletions
--- a/studio/backend/main.py
+++ b/studio/backend/main.py
@ -49,6 +49,8 @@ import shutil
 import warnings
 from contextlib import asynccontextmanager
 from importlib.metadata import PackageNotFoundError, version as package_version
+from typing import Optional
+from urllib.parse import urlparse


 _STUDIO_INSTALL_ID_RE = _re.compile(r"^[0-9a-f]{64}$")
@ -715,10 +717,8 @@ def _strip_crossorigin(html_bytes: bytes) -> bytes:

 def _inject_bootstrap(html_bytes: bytes, app: FastAPI):
    """Inject bootstrap credentials when password change is pending.
-
-    Returns ``(html_bytes, script_nonce_or_None)``. Callers must forward
-    the nonce via ``_CSP_SCRIPT_NONCE_HEADER`` so the inline script is
-    not blocked by CSP.
+    Returns ``(html_bytes, script_nonce_or_None)``; callers forward the
+    nonce via ``_CSP_SCRIPT_NONCE_HEADER`` so CSP allows the inline script.
    """
    import json as _json
    import secrets as _secrets
@ -743,6 +743,86 @@ def _inject_bootstrap(html_bytes: bytes, app: FastAPI):
    return html.encode("utf-8"), nonce


+_DEFAULT_PORTS = {"http": 80, "https": 443, "ws": 80, "wss": 443}
+
+
+def _canonical_origin(scheme: str, netloc: str) -> Optional[tuple[str, str, int]]:
+    """Canonicalise an Origin to ``(scheme, host, port)`` for equality.
+    Browsers strip default ports (RFC 6454 sec 6.1) and scheme/host are
+    case-insensitive (RFC 3986), so bare string compare misclassifies
+    same-origin requests as cross-origin. Returns ``None`` on unparseable
+    input so callers fall to the safer cross-origin default.
+    """
+    scheme = (scheme or "").strip().lower()
+    if not scheme or not netloc:
+        return None
+    # Strip userinfo (RFC 3986); Origin never carries credentials.
+    if "@" in netloc:
+        netloc = netloc.rsplit("@", 1)[1]
+    # IPv6 hosts use brackets (RFC 3986 sec 3.2.2): ``[::1]:8902``. Bare
+    # ``partition(":")`` mis-parses these and breaks ``unsloth studio -H ::1``.
+    if netloc.startswith("["):
+        close = netloc.find("]")
+        if close == -1:
+            return None
+        host = netloc[1:close]
+        rest = netloc[close + 1 :]
+        if rest.startswith(":"):
+            port_str = rest[1:]
+        elif rest == "":
+            port_str = ""
+        else:
+            return None
+    else:
+        host, _, port_str = netloc.partition(":")
+    host = host.strip().lower()
+    if not host:
+        return None
+    if port_str:
+        try:
+            port = int(port_str)
+        except ValueError:
+            return None
+    else:
+        port = _DEFAULT_PORTS.get(scheme, 0)
+    return (scheme, host, port)
+
+
+def _is_same_origin_request(request: Request) -> bool:
+    """True when Origin is missing or matches request's scheme://host:port.
+    Top-level same-document GETs omit Origin, so missing counts as same-origin.
+    Callers must also emit ``Vary: Origin``. Both sides are canonicalised via
+    :func:`_canonical_origin` so default-port stripping and scheme/host case
+    do not misclassify same-origin requests as cross-origin.
+    """
+    origin = request.headers.get("origin")
+    if origin is None:
+        # Missing header: top-level same-document GETs omit Origin.
+        return True
+    # Empty string is not a valid serialised origin (RFC 6454 sec 6.1).
+    if not origin:
+        return False
+    # "null" token (sandboxed iframes, file:// pages) is never same-origin.
+    if origin == "null":
+        return False
+    # ``urlparse`` raises ``ValueError`` on malformed IPv6 brackets; swallow
+    # so a garbage Origin doesn't 500 the SPA handler.
+    try:
+        parsed = urlparse(origin)
+    except ValueError:
+        return False
+    origin_canon = _canonical_origin(parsed.scheme, parsed.netloc)
+    if origin_canon is None:
+        return False
+    try:
+        self_canon = _canonical_origin(request.url.scheme, request.url.netloc)
+    except ValueError:
+        return False
+    if self_canon is None:
+        return False
+    return origin_canon == self_canon
+
+
 def setup_frontend(app: FastAPI, build_path: Path):
    """Mount frontend static files (optional)"""
    if not build_path.exists():
@ -753,11 +833,18 @@ def setup_frontend(app: FastAPI, build_path: Path):
    if assets_dir.exists():
        app.mount("/assets", StaticFiles(directory = assets_dir), name = "assets")

-    def _build_index_response() -> Response:
+    def _build_index_response(request: Request) -> Response:
        content = (build_path / "index.html").read_bytes()
        content = _strip_crossorigin(content)
-        content, nonce = _inject_bootstrap(content, app)
-        headers = {"Cache-Control": "no-cache, no-store, must-revalidate"}
+        # Bootstrap pw is same-origin only; Vary: Origin keeps caches honest.
+        if _is_same_origin_request(request):
+            content, nonce = _inject_bootstrap(content, app)
+        else:
+            nonce = None
+        headers = {
+            "Cache-Control": "no-cache, no-store, must-revalidate",
+            "Vary": "Origin",
+        }
        if nonce:
            headers[_CSP_SCRIPT_NONCE_HEADER] = nonce
        return Response(
@ -767,11 +854,11 @@ def setup_frontend(app: FastAPI, build_path: Path):
        )

    @app.get("/")
-    async def serve_root():
-        return _build_index_response()
+    async def serve_root(request: Request):
+        return _build_index_response(request)

    @app.get("/{full_path:path}")
-    async def serve_frontend(full_path: str):
+    async def serve_frontend(request: Request, full_path: str):
        if full_path in {"api", "v1"} or full_path.startswith(("api/", "v1/")):
            return {"error": "API endpoint not found"}

@ -785,6 +872,6 @@ def setup_frontend(app: FastAPI, build_path: Path):
            return FileResponse(file_path)

        # Serve index.html as bytes — avoids Content-Length mismatch
-        return _build_index_response()
+        return _build_index_response(request)

    return True
--- a/studio/backend/tests/test_index_bootstrap_origin.py
+++ b/studio/backend/tests/test_index_bootstrap_origin.py
@ -0,0 +1,144 @@
+# SPDX-License-Identifier: AGPL-3.0-only
+# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0
+
+"""Regression coverage for the bootstrap-pw cross-origin leak (PR 5739).
+``_is_same_origin_request`` gates ``_inject_bootstrap`` so the seeded
+admin password only ships to same-origin callers.
+"""
+
+import os
+from types import SimpleNamespace
+from unittest.mock import MagicMock
+
+import pytest
+
+
+def _build_request(host: str, origin: str | None, scheme: str = "http") -> MagicMock:
+    request = MagicMock()
+    request.url.scheme = scheme
+    request.url.netloc = host
+    request.headers = {"origin": origin} if origin is not None else {}
+    return request
+
+
+def test_is_same_origin_request_missing_origin_is_same_origin(monkeypatch):
+    from main import _is_same_origin_request
+
+    req = _build_request("127.0.0.1:8888", origin = None)
+    assert _is_same_origin_request(req) is True
+
+
+def test_is_same_origin_request_matching_origin_is_same_origin():
+    from main import _is_same_origin_request
+
+    req = _build_request("127.0.0.1:8888", origin = "http://127.0.0.1:8888")
+    assert _is_same_origin_request(req) is True
+
+
+def test_is_same_origin_request_evil_origin_is_cross_origin():
+    from main import _is_same_origin_request
+
+    req = _build_request("127.0.0.1:8888", origin = "https://evil.example")
+    assert _is_same_origin_request(req) is False
+
+
+def test_is_same_origin_request_scheme_mismatch_is_cross_origin():
+    # https origin against an http listener is not same-origin.
+    from main import _is_same_origin_request
+
+    req = _build_request("127.0.0.1:8888", origin = "https://127.0.0.1:8888")
+    assert _is_same_origin_request(req) is False
+
+
+def test_is_same_origin_request_port_mismatch_is_cross_origin():
+    # Same host different port is not same-origin per the web platform.
+    from main import _is_same_origin_request
+
+    req = _build_request("127.0.0.1:8888", origin = "http://127.0.0.1:5173")
+    assert _is_same_origin_request(req) is False
+
+
+# ── Canonicalisation: default-port stripping + case folding ─────────
+
+
+def test_is_same_origin_request_https_default_port_stripped_on_origin():
+    """RFC 6454 strips default ports on Origin; Starlette's netloc may still
+    carry ``:443``. Canonicalise both sides so this stays same-origin.
+    """
+    from main import _is_same_origin_request
+
+    req = _build_request(
+        "example.com:443", origin = "https://example.com", scheme = "https"
+    )
+    assert _is_same_origin_request(req) is True
+
+
+def test_is_same_origin_request_http_default_port_stripped_on_origin():
+    from main import _is_same_origin_request
+
+    req = _build_request("example.com:80", origin = "http://example.com")
+    assert _is_same_origin_request(req) is True
+
+
+def test_is_same_origin_request_default_port_present_on_origin():
+    """Mirror case: Origin carries the default port, netloc doesn't. Same-origin."""
+    from main import _is_same_origin_request
+
+    req = _build_request(
+        "example.com", origin = "https://example.com:443", scheme = "https"
+    )
+    assert _is_same_origin_request(req) is True
+
+
+def test_is_same_origin_request_host_case_insensitive():
+    """Host portion is case-insensitive per RFC 3986."""
+    from main import _is_same_origin_request
+
+    req = _build_request("example.com", origin = "http://EXAMPLE.com")
+    assert _is_same_origin_request(req) is True
+
+
+def test_is_same_origin_request_scheme_case_insensitive():
+    """Scheme portion is case-insensitive per RFC 3986."""
+    from main import _is_same_origin_request
+
+    req = _build_request("example.com", origin = "HTTP://example.com")
+    assert _is_same_origin_request(req) is True
+
+
+def test_is_same_origin_request_null_origin_is_cross_origin():
+    """Sandboxed iframes / file:// pages send ``Origin: null``; cross-origin."""
+    from main import _is_same_origin_request
+
+    req = _build_request("example.com", origin = "null")
+    assert _is_same_origin_request(req) is False
+
+
+def test_is_same_origin_request_unparseable_origin_is_cross_origin():
+    """Garbage values without a host fall to cross-origin; a malformed header
+    must not leak the bootstrap.
+    """
+    from main import _is_same_origin_request
+
+    req = _build_request("example.com", origin = "not-a-url")
+    assert _is_same_origin_request(req) is False
+
+
+def test_is_same_origin_request_userinfo_in_netloc_ignored():
+    """``user:pass@host:port`` netlocs (RFC 3986) must compare equal to the
+    credentials-less Origin.
+    """
+    from main import _is_same_origin_request
+
+    req = _build_request("user:pass@example.com:80", origin = "http://example.com")
+    assert _is_same_origin_request(req) is True
+
+
+def test_is_same_origin_request_explicit_non_default_port_still_mismatch():
+    """Canonicalisation does NOT collapse non-default ports to default."""
+    from main import _is_same_origin_request
+
+    req = _build_request(
+        "example.com", origin = "https://example.com:9999", scheme = "https"
+    )
+    assert _is_same_origin_request(req) is False
--- a/studio/backend/tests/test_index_bootstrap_origin_extra.py
+++ b/studio/backend/tests/test_index_bootstrap_origin_extra.py
@ -0,0 +1,196 @@
+# SPDX-License-Identifier: AGPL-3.0-only
+# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0
+
+"""Extra edge-case coverage for the bootstrap-pw cross-origin gate.
+Companion to ``test_index_bootstrap_origin.py``: IPv6 netlocs, opaque
+origins (``data:``, ``blob:``), comma-joined multi-Origin headers, and
+the ``localhost`` vs ``127.0.0.1`` distinct-origin rule.
+"""
+
+from unittest.mock import MagicMock
+
+
+def _build_request(host: str, origin, scheme: str = "http") -> MagicMock:
+    request = MagicMock()
+    request.url.scheme = scheme
+    request.url.netloc = host
+    request.headers = {"origin": origin} if origin is not None else {}
+    return request
+
+
+# ── IPv6 ────────────────────────────────────────────────────────────
+
+
+def test_is_same_origin_request_ipv6_loopback_same_origin():
+    """Studio supports ``-H ::1`` binds; netloc is ``[::1]:8902``. Bare
+    ``partition(":")`` mis-parses the bracketed form and would refuse the
+    bootstrap on legitimate same-origin nav.
+    """
+    from main import _is_same_origin_request
+
+    req = _build_request("[::1]:8902", origin = "http://[::1]:8902")
+    assert _is_same_origin_request(req) is True
+
+
+def test_is_same_origin_request_ipv6_full_address_same_origin():
+    from main import _is_same_origin_request
+
+    req = _build_request(
+        "[2001:db8::1]:8443",
+        origin = "https://[2001:db8::1]:8443",
+        scheme = "https",
+    )
+    assert _is_same_origin_request(req) is True
+
+
+def test_is_same_origin_request_ipv6_default_port_stripped():
+    """Browser drops :80 on ``http://[::1]``."""
+    from main import _is_same_origin_request
+
+    req = _build_request("[::1]:80", origin = "http://[::1]")
+    assert _is_same_origin_request(req) is True
+
+
+def test_is_same_origin_request_ipv6_case_insensitive():
+    """Hex digits in IPv6 are case-insensitive per RFC 5952."""
+    from main import _is_same_origin_request
+
+    req = _build_request(
+        "[2001:DB8::1]:8443",
+        origin = "https://[2001:db8::1]:8443",
+        scheme = "https",
+    )
+    assert _is_same_origin_request(req) is True
+
+
+def test_is_same_origin_request_ipv6_different_host_cross_origin():
+    from main import _is_same_origin_request
+
+    req = _build_request("[::1]:8902", origin = "http://[2001:db8::1]:8902")
+    assert _is_same_origin_request(req) is False
+
+
+def test_is_same_origin_request_ipv6_port_mismatch_cross_origin():
+    from main import _is_same_origin_request
+
+    req = _build_request("[::1]:8902", origin = "http://[::1]:9999")
+    assert _is_same_origin_request(req) is False
+
+
+def test_is_same_origin_request_ipv6_userinfo_stripped():
+    from main import _is_same_origin_request
+
+    req = _build_request("user:pass@[::1]:8902", origin = "http://[::1]:8902")
+    assert _is_same_origin_request(req) is True
+
+
+# ── Opaque origins (data:, blob:) ───────────────────────────────────
+
+
+def test_is_same_origin_request_data_url_origin_is_cross_origin():
+    """``data:`` URLs are opaque origins (HTML living standard); no host,
+    never same-origin.
+    """
+    from main import _is_same_origin_request
+
+    req = _build_request(
+        "127.0.0.1:8902", origin = "data:text/html,<script>alert(1)</script>"
+    )
+    assert _is_same_origin_request(req) is False
+
+
+def test_is_same_origin_request_blob_url_origin_is_cross_origin():
+    """``blob:`` URLs carry the inner origin only in non-canonical form; the
+    canonical comparison rejects them.
+    """
+    from main import _is_same_origin_request
+
+    req = _build_request("127.0.0.1:8902", origin = "blob:http://127.0.0.1:8902/uuid")
+    assert _is_same_origin_request(req) is False
+
+
+def test_is_same_origin_request_file_url_origin_is_cross_origin():
+    """``file://`` pages usually send ``Origin: null``; historical engines
+    sent ``Origin: file://``. Neither is same-origin vs an http listener.
+    """
+    from main import _is_same_origin_request
+
+    req = _build_request("127.0.0.1:8902", origin = "file://")
+    assert _is_same_origin_request(req) is False
+
+
+# ── Multi-Origin header (comma-joined by Starlette) ────────────────
+
+
+def test_is_same_origin_request_comma_joined_origins_cross_origin():
+    """Starlette concatenates repeated headers with ``, ``; the canonical
+    parser can't safely split this, so it falls to cross-origin.
+    """
+    from main import _is_same_origin_request
+
+    req = _build_request(
+        "127.0.0.1:8902",
+        origin = "http://127.0.0.1:8902, http://evil.example",
+    )
+    assert _is_same_origin_request(req) is False
+
+
+# ── localhost vs 127.0.0.1 (distinct origins per web platform) ──────
+
+
+def test_is_same_origin_request_localhost_vs_127_is_cross_origin():
+    """Browsers treat ``localhost`` and ``127.0.0.1`` as distinct origins;
+    the canonical comparison must not DNS-collapse them.
+    """
+    from main import _is_same_origin_request
+
+    req = _build_request("127.0.0.1:8902", origin = "http://localhost:8902")
+    assert _is_same_origin_request(req) is False
+
+
+def test_is_same_origin_request_127_vs_localhost_is_cross_origin():
+    from main import _is_same_origin_request
+
+    req = _build_request("localhost:8902", origin = "http://127.0.0.1:8902")
+    assert _is_same_origin_request(req) is False
+
+
+# ── urlparse ValueError robustness ─────────────────────────────────
+
+
+def test_is_same_origin_request_malformed_ipv6_bracket_is_cross_origin():
+    """``urlparse`` raises ``ValueError('Invalid IPv6 URL')`` on unclosed
+    brackets (CVE-2024-11168 hardening). The gate must swallow and fall to
+    cross-origin rather than 500 the SPA handler.
+    """
+    from main import _is_same_origin_request
+
+    req = _build_request("127.0.0.1:8902", origin = "http://[malformed")
+    assert _is_same_origin_request(req) is False
+
+
+def test_is_same_origin_request_invalid_ipv6_address_is_cross_origin():
+    """Bracketed but invalid IPv6 (e.g. ``[::g]``) also raises
+    ``ValueError`` inside ``urlparse``."""
+    from main import _is_same_origin_request
+
+    req = _build_request("127.0.0.1:8902", origin = "http://[::g]:8902")
+    assert _is_same_origin_request(req) is False
+
+
+def test_is_same_origin_request_bracket_with_trailing_garbage_is_cross_origin():
+    """Text after the closing bracket also raises inside ``urlparse``."""
+    from main import _is_same_origin_request
+
+    req = _build_request("127.0.0.1:8902", origin = "http://[2001:db8::1]extra:8902")
+    assert _is_same_origin_request(req) is False
+
+
+def test_is_same_origin_request_empty_origin_header_is_cross_origin():
+    """Explicit empty ``Origin:`` is not a valid serialised origin and must
+    not be conflated with a missing header; cross-origin, bootstrap withheld.
+    """
+    from main import _is_same_origin_request
+
+    req = _build_request("127.0.0.1:8902", origin = "")
+    assert _is_same_origin_request(req) is False