From 892c9096ecbaaceea3afa58c14315c19931d0ac5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Utku=20=C5=9EAH=C4=B0N?= Date: Fri, 4 Sep 2026 18:02:52 +0300 Subject: [PATCH 1/3] feat: harden archive extraction with rollback-capable library install Validate every zip/tar member (OS-independent traversal, link and device rejection) via shared core/archive_safety; ingest and library install use it. _install_package stages, backups, and swaps so a failed replacement restores the working install. Install ValueError still aborts sync. --- core/archive_safety.py | 63 +++++++++ core/reference_library.py | 42 ++++-- tests/test_archive_extraction_safety.py | 168 ++++++++++++++++++++++++ tools/library_ingest/common.py | 6 +- 4 files changed, 265 insertions(+), 14 deletions(-) create mode 100644 core/archive_safety.py create mode 100644 tests/test_archive_extraction_safety.py diff --git a/core/archive_safety.py b/core/archive_safety.py new file mode 100644 index 00000000..78a269dd --- /dev/null +++ b/core/archive_safety.py @@ -0,0 +1,63 @@ +"""Safe archive extraction for remotely fetched library packages. + +Both the ingest tooling (``tools/library_ingest/common.py``) and the runtime +library installer (``core/reference_library.py``) unpack archives downloaded +from remote feed sources. Plain ``extractall`` trusts member names, which +allows ZipSlip/tar-slip writes outside the target directory plus link and +device members. The helpers here validate every member before anything is +written. + +Validation is manual (not ``tarfile`` ``filter="data"``) because the +``filter`` keyword exists only on Python 3.12+ while this project supports +``>=3.11``; manual checks behave identically on both CI legs. +""" + +from __future__ import annotations + +import tarfile +import zipfile +from pathlib import Path, PurePosixPath, PureWindowsPath + + +def _reject_unsafe_name(member: str) -> None: + """Raise ValueError unless ``member`` is a portable relative archive path.""" + if not member: + raise ValueError("Archive member has unsafe path: ") + if "\x00" in member: + raise ValueError(f"Archive member has unsafe path: {member!r}") + if PurePosixPath(member).is_absolute(): + raise ValueError(f"Archive member has unsafe path: {member}") + windows = PureWindowsPath(member) + if windows.drive or windows.root: + raise ValueError(f"Archive member has unsafe path: {member}") + if any(part == ".." for part in windows.parts): + raise ValueError(f"Archive member has unsafe path: {member}") + + +def _resolve_within(target_dir: Path, member: str) -> Path: + """Resolve ``member`` under ``target_dir``; raise ValueError on escape.""" + root = target_dir.resolve() + resolved = (root / member).resolve() + if not resolved.is_relative_to(root): + raise ValueError(f"Archive member escapes target directory: {member}") + return resolved + + +def safe_extract_zip(archive: zipfile.ZipFile, target_dir: Path) -> None: + """Extract ``archive`` into ``target_dir`` after validating every member.""" + for info in archive.infolist(): + if (info.external_attr >> 16) & 0o170000 == 0o120000: + raise ValueError(f"Archive member is a link: {info.filename}") + _reject_unsafe_name(info.filename) + _resolve_within(target_dir, info.filename) + archive.extractall(target_dir) + + +def safe_extract_tar(archive: tarfile.TarFile, target_dir: Path) -> None: + """Extract ``archive`` into ``target_dir`` after validating every member.""" + for member in archive.getmembers(): + _reject_unsafe_name(member.name) + _resolve_within(target_dir, member.name) + if not (member.isdir() or member.isreg()): + raise ValueError(f"Archive member has unsafe type: {member.name}") + archive.extractall(target_dir) diff --git a/core/reference_library.py b/core/reference_library.py index e56e631a..85eae50d 100644 --- a/core/reference_library.py +++ b/core/reference_library.py @@ -20,6 +20,7 @@ import numpy as np from core.path_env import library_filesystem_env_looks_like_windows_leak +from core.archive_safety import safe_extract_zip from utils.license_manager import encode_license_key, get_storage_dir @@ -782,21 +783,38 @@ def record_cloud_lookup( self.save_sync_state(state) def _install_package(self, package: LibraryPackage, raw: bytes) -> tuple[Path, Path]: - archive_path = self._packages_root() / package.archive_name - archive_path.parent.mkdir(parents=True, exist_ok=True) - archive_path.write_bytes(raw) - + # Staged, rollback-capable install: validate and extract the replacement + # fully before touching the active install. A hostile or corrupt + # replacement raises before the working package is disturbed, and a + # failed promotion restores the previous install. Install-level + # ValueError propagates out of sync(), like hash mismatches do. extract_dir = self._installed_root() / package.package_id / package.version - if extract_dir.exists(): - shutil.rmtree(extract_dir) extract_dir.parent.mkdir(parents=True, exist_ok=True) - - with tempfile.TemporaryDirectory(prefix="ta_lib_", dir=str(self.root)) as tmp_dir: - temp_extract = Path(tmp_dir) / "extract" - temp_extract.mkdir(parents=True, exist_ok=True) + with tempfile.TemporaryDirectory(prefix="ta_lib_install_", dir=str(self.root)) as tmp_dir: + staged = Path(tmp_dir) / "staged" + staged.mkdir(parents=True, exist_ok=True) with zipfile.ZipFile(io.BytesIO(raw), "r") as archive: - archive.extractall(temp_extract) - shutil.move(str(temp_extract), str(extract_dir)) + safe_extract_zip(archive, staged) + archive_path = self._packages_root() / package.archive_name + archive_path.parent.mkdir(parents=True, exist_ok=True) + archive_path.write_bytes(raw) + backup = extract_dir.with_name(extract_dir.name + ".bak") + if extract_dir.exists(): + if backup.exists(): + shutil.rmtree(backup) + os.replace(extract_dir, backup) + elif backup.exists(): + # A previous swap died between backup and promotion; the backup + # holds the last working install, so restore it first. + os.replace(backup, extract_dir) + try: + os.replace(staged, extract_dir) + except BaseException: + if backup.exists(): + os.replace(backup, extract_dir) + raise + if backup.exists(): + shutil.rmtree(backup, ignore_errors=True) return archive_path, extract_dir def count_installed_candidates(self, analysis_type: str) -> int: diff --git a/tests/test_archive_extraction_safety.py b/tests/test_archive_extraction_safety.py new file mode 100644 index 00000000..2a4b8fca --- /dev/null +++ b/tests/test_archive_extraction_safety.py @@ -0,0 +1,168 @@ +"""Track A (PR-5): hostile archives are rejected; installs are failure-atomic.""" + +from __future__ import annotations + +import io +import os +import tarfile +import zipfile +from pathlib import Path + +import pytest + +from core.archive_safety import safe_extract_tar, safe_extract_zip +from core.reference_library import LibraryPackage, ReferenceLibraryManager +from tools.library_ingest.common import _extract_archive + +HOSTILE_NAMES = [ + "../evil.txt", + "a/../../evil.txt", + "/abs.txt", + "\\rooted\\evil.txt", + "C:evil.txt", + "C:/evil.txt", + "C:\\evil.txt", + "\\\\server\\share\\evil.txt", +] + + +def _zip_bytes(names: list[str], *, symlink: str | None = None) -> bytes: + buffer = io.BytesIO() + with zipfile.ZipFile(buffer, "w") as archive: + for name in names: + archive.writestr(name, b"evil") + if symlink is not None: + info = zipfile.ZipInfo(symlink) + info.create_system = 3 + info.external_attr = (0o120777 << 16) + archive.writestr(info, "target.txt") + return buffer.getvalue() + + +def _tar_bytes(names: list[str], *, link: tuple[str, str] | None = None, fifo: str | None = None) -> bytes: + buffer = io.BytesIO() + with tarfile.open(fileobj=buffer, mode="w") as archive: + for name in names: + payload = b"evil" + info = tarfile.TarInfo(name) + info.size = len(payload) + archive.addfile(info, io.BytesIO(payload)) + if link is not None: + kind, linkname = link + info = tarfile.TarInfo("link.txt") + info.type = tarfile.SYMTYPE if kind == "sym" else tarfile.LNKTYPE + info.linkname = linkname + archive.addfile(info) + if fifo is not None: + info = tarfile.TarInfo(fifo) + info.type = tarfile.FIFOTYPE + archive.addfile(info) + return buffer.getvalue() + + +def _write_archive(tmp_path: Path, filename: str, raw: bytes) -> Path: + path = tmp_path / filename + path.write_bytes(raw) + return path + + +@pytest.mark.parametrize("name", HOSTILE_NAMES) +def test_hostile_zip_member_rejected_without_escape(tmp_path: Path, name: str) -> None: + archive_path = _write_archive(tmp_path, "payload.zip", _zip_bytes([name])) + target = tmp_path / "extracted" + with pytest.raises(ValueError): + _extract_archive(archive_path, target) + assert list(tmp_path.rglob("*evil*")) == [] + + +@pytest.mark.parametrize("name", HOSTILE_NAMES) +def test_hostile_tar_member_rejected_without_escape(tmp_path: Path, name: str) -> None: + archive_path = _write_archive(tmp_path, "payload.tar", _tar_bytes([name])) + target = tmp_path / "extracted" + with pytest.raises(ValueError): + _extract_archive(archive_path, target) + assert list(tmp_path.rglob("*evil*")) == [] + + +def test_zip_symlink_member_rejected(tmp_path: Path) -> None: + archive_path = _write_archive(tmp_path, "payload.zip", _zip_bytes(["ok.txt"], symlink="link.txt")) + with pytest.raises(ValueError, match="link"): + _extract_archive(archive_path, tmp_path / "extracted") + assert list(tmp_path.rglob("link*")) == [] + + +@pytest.mark.parametrize("kind", ["sym", "hard"]) +def test_tar_link_members_rejected(tmp_path: Path, kind: str) -> None: + archive_path = _write_archive(tmp_path, "payload.tar", _tar_bytes(["ok.txt"], link=(kind, "ok.txt"))) + with pytest.raises(ValueError, match="unsafe type"): + _extract_archive(archive_path, tmp_path / "extracted") + + +def test_tar_fifo_member_rejected(tmp_path: Path) -> None: + archive_path = _write_archive(tmp_path, "payload.tar", _tar_bytes(["ok.txt"], fifo="pipe")) + with pytest.raises(ValueError, match="unsafe type"): + _extract_archive(archive_path, tmp_path / "extracted") + + +def test_benign_archives_roundtrip(tmp_path: Path) -> None: + zip_raw = _zip_bytes(["folder/", "folder/nested.txt", "top.txt"]) + zip_target = tmp_path / "zip_out" + with zipfile.ZipFile(io.BytesIO(zip_raw), "r") as archive: + safe_extract_zip(archive, zip_target) + assert (zip_target / "folder" / "nested.txt").read_bytes() == b"evil" + assert (zip_target / "top.txt").read_bytes() == b"evil" + + tar_raw = _tar_bytes(["folder/nested.txt", "top.txt"]) + tar_target = tmp_path / "tar_out" + with tarfile.open(fileobj=io.BytesIO(tar_raw), mode="r") as archive: + safe_extract_tar(archive, tar_target) + assert (tar_target / "folder" / "nested.txt").read_bytes() == b"evil" + assert (tar_target / "top.txt").read_bytes() == b"evil" + + +def _test_package() -> LibraryPackage: + return LibraryPackage( + package_id="testpkg", + analysis_type="XRD", + provider="test", + version="v1", + archive_name="testpkg.zip", + sha256="0" * 64, + entry_count=0, + ) + + +def test_hostile_replacement_keeps_working_install(tmp_path: Path) -> None: + manager = ReferenceLibraryManager(root=tmp_path / "lib", feed_source="") + package = _test_package() + manager._install_package(package, _zip_bytes(["sentinel.txt"])) + sentinel = manager._installed_root() / "testpkg" / "v1" / "sentinel.txt" + assert sentinel.read_bytes() == b"evil" + + with pytest.raises(ValueError): + manager._install_package(package, _zip_bytes(["../evil.txt"])) + assert sentinel.read_bytes() == b"evil" + assert list(tmp_path.rglob("*evil*")) == [] + + +def test_promotion_failure_restores_previous_install(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + manager = ReferenceLibraryManager(root=tmp_path / "lib", feed_source="") + package = _test_package() + manager._install_package(package, _zip_bytes(["sentinel.txt"])) + extract_dir = manager._installed_root() / "testpkg" / "v1" + assert (extract_dir / "sentinel.txt").read_bytes() == b"evil" + + real_replace = os.replace + + def _flaky_replace(src: Path, dst: Path, *args: object, **kwargs: object) -> None: + if Path(dst) == extract_dir and Path(src).name == "staged": + raise RuntimeError("injected promotion failure") + real_replace(src, dst, *args, **kwargs) + + monkeypatch.setattr(os, "replace", _flaky_replace) + with pytest.raises(RuntimeError, match="injected promotion failure"): + manager._install_package(package, _zip_bytes(["replacement.txt"])) + + assert (extract_dir / "sentinel.txt").read_bytes() == b"evil" + assert not (extract_dir / "replacement.txt").exists() + assert not extract_dir.with_name("v1.bak").exists() diff --git a/tools/library_ingest/common.py b/tools/library_ingest/common.py index 8fd94cff..edb77fe8 100644 --- a/tools/library_ingest/common.py +++ b/tools/library_ingest/common.py @@ -22,6 +22,8 @@ from .schema import PackageSpec +from core.archive_safety import safe_extract_tar, safe_extract_zip + BUILD_ROOT = Path("build") / "reference_library_ingest" JOB_STATE_ROOT = Path("build") / "reference_library_jobs" BUILDER_VERSION = "b1" @@ -255,11 +257,11 @@ def _extract_archive(archive_path: Path, target_dir: Path) -> Path: lower = archive_path.name.lower() if lower.endswith(".zip"): with zipfile.ZipFile(archive_path, "r") as archive: - archive.extractall(target_dir) + safe_extract_zip(archive, target_dir) return target_dir if lower.endswith((".tar.gz", ".tgz", ".tar")): with tarfile.open(archive_path, "r:*") as archive: - archive.extractall(target_dir) + safe_extract_tar(archive, target_dir) return target_dir raise ValueError(f"Unsupported archive format: {archive_path}") From 13c9ad72015599faf747c33d35fe6e724ea34ca0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Utku=20=C5=9EAH=C4=B0N?= Date: Fri, 4 Sep 2026 18:04:52 +0300 Subject: [PATCH 2/3] chore: centralize license-secret resolution with honest forgeability docs Extract the existing explicit/env/legacy/default precedence into _resolve_license_secret; no behavior change. Docstrings state the repo default is publicly forgeable demo-only, commercial builds must inject MATERIALSCOPE_LICENSE_SECRET, rotation requires re-issuance, and real distributable licensing wants asymmetric signing (deferred). --- tests/test_license_manager.py | 58 +++++++++++++++++++++++++++++++++++ utils/license_manager.py | 38 ++++++++++++++++++----- 2 files changed, 88 insertions(+), 8 deletions(-) diff --git a/tests/test_license_manager.py b/tests/test_license_manager.py index ff37f1f5..b8d3a407 100644 --- a/tests/test_license_manager.py +++ b/tests/test_license_manager.py @@ -68,3 +68,61 @@ def test_commercial_mode_requires_license_for_write_access(tmp_path, monkeypatch assert state["status"] == "unlicensed" assert license_allows_write(state) is False + + +def _professional_payload(**overrides): + fields = { + "customer_name": "Ada Lovelace", + "company_name": "Acme Lab", + "sku": "PROFESSIONAL", + "seat_count": 1, + "issued_at": datetime(2026, 3, 7, tzinfo=UTC), + "expires_at": datetime(2027, 3, 7, tzinfo=UTC), + "allowed_major_version": 2, + } + fields.update(overrides) + return create_signed_license(**fields) + + +def _clear_secret_env(monkeypatch): + monkeypatch.delenv("MATERIALSCOPE_LICENSE_SECRET", raising=False) + monkeypatch.delenv("THERMOANALYZER_LICENSE_SECRET", raising=False) + + +def test_env_secret_sign_verify_roundtrip(monkeypatch): + from utils.license_manager import validate_license_payload + + _clear_secret_env(monkeypatch) + monkeypatch.setenv("MATERIALSCOPE_LICENSE_SECRET", "test-secret-a") + state = validate_license_payload(_professional_payload()) + assert state["status"] == "activated" + + +def test_rotated_env_secret_invalidates_old_payload(monkeypatch): + from utils.license_manager import validate_license_payload + + _clear_secret_env(monkeypatch) + monkeypatch.setenv("MATERIALSCOPE_LICENSE_SECRET", "test-secret-a") + payload = _professional_payload() + monkeypatch.setenv("MATERIALSCOPE_LICENSE_SECRET", "test-secret-b") + state = validate_license_payload(payload) + assert state["status"] == "unlicensed" + assert "signature is invalid" in state["message"] + + +def test_default_secret_roundtrip_preserved(monkeypatch): + from utils.license_manager import validate_license_payload + + _clear_secret_env(monkeypatch) + state = validate_license_payload(_professional_payload()) + assert state["status"] == "activated" + + +def test_explicit_secret_beats_env(monkeypatch): + from utils.license_manager import validate_license_payload + + _clear_secret_env(monkeypatch) + monkeypatch.setenv("MATERIALSCOPE_LICENSE_SECRET", "test-secret-a") + payload = _professional_payload(secret="explicit-secret") + assert validate_license_payload(payload, secret="explicit-secret")["status"] == "activated" + assert validate_license_payload(payload)["status"] == "unlicensed" diff --git a/utils/license_manager.py b/utils/license_manager.py index bf0e395c..dd7eec2a 100644 --- a/utils/license_manager.py +++ b/utils/license_manager.py @@ -1,4 +1,11 @@ -"""Offline license helpers for MaterialScope.""" +"""Offline license helpers for MaterialScope. + +Licensing here is HMAC with a client-held shared secret: enough for local +demo/development gating, not for distributable commercial licensing, since +anyone holding the secret can forge keys. The correct long-term fix is +asymmetric signing (Ed25519/RSA) with only a public verification key in the +application; that migration is explicitly out of scope for this change. +""" from __future__ import annotations @@ -33,10 +40,30 @@ "signature", } -# Demo-only signing secret. Commercial builds should override this via env var. +# Demo/development-only signing secret. This value is public in the repository, +# so anyone with the source can forge keys that validate. Commercial +# deployments MUST provide MATERIALSCOPE_LICENSE_SECRET externally. Changing +# the secret invalidates existing HMAC-signed licenses (re-issue required). DEFAULT_LICENSE_SECRET = "materialscope-professional-demo-secret" +def _resolve_license_secret(explicit: str | None = None) -> str: + """Return the HMAC secret, preserving the long-standing precedence. + + Order: explicit argument, then MATERIALSCOPE_LICENSE_SECRET, then the + legacy THERMOANALYZER_LICENSE_SECRET, then the public demo default. + Sign and verify share this single resolution point, so a payload created + under one environment validates under the same environment. Rotating the + secret invalidates previously signed licenses (re-issue required). + """ + return ( + explicit + or os.getenv("MATERIALSCOPE_LICENSE_SECRET") + or os.getenv("THERMOANALYZER_LICENSE_SECRET") + or DEFAULT_LICENSE_SECRET + ) + + def commercial_mode_enabled() -> bool: """Return whether license enforcement is enabled for this runtime.""" raw = str(os.getenv(COMMERCIAL_MODE_ENV, "") or os.getenv(COMMERCIAL_MODE_ENV_LEGACY, "")).strip().lower() @@ -116,12 +143,7 @@ def create_trial_payload( def sign_license_payload(payload: dict[str, Any], secret: str | None = None) -> str: """Return HMAC signature for a license payload.""" - secret_bytes = ( - secret - or os.getenv("MATERIALSCOPE_LICENSE_SECRET") - or os.getenv("THERMOANALYZER_LICENSE_SECRET") - or DEFAULT_LICENSE_SECRET - ).encode("utf-8") + secret_bytes = _resolve_license_secret(secret).encode("utf-8") message = json.dumps(_canonical_payload(payload), sort_keys=True, separators=(",", ":"), ensure_ascii=False).encode("utf-8") return hmac.new(secret_bytes, message, hashlib.sha256).hexdigest() From b3cc97c9147f817132ac64b2e9d1511da03552a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Utku=20=C5=9EAH=C4=B0N?= Date: Fri, 4 Sep 2026 18:08:26 +0300 Subject: [PATCH 3/3] feat: warn on unauthenticated non-loopback bind and sync bundled client token Explicit --token/api_token stays the sole server-auth source and now overwrites a stale MATERIALSCOPE_API_TOKEN so the co-located Dash UI keeps working; client resolves the token per call. No enforcement; /health stays open. --- backend/app.py | 25 +++++++++++++++++++ backend/main.py | 5 +++- dash_app/api_client.py | 19 +++++++++++--- dash_app/server.py | 13 ++++++++++ tests/test_container_bind_warning.py | 37 ++++++++++++++++++++++++++++ tests/test_dash_fastapi_backend.py | 35 ++++++++++++++++++++++++++ 6 files changed, 129 insertions(+), 5 deletions(-) create mode 100644 tests/test_container_bind_warning.py diff --git a/backend/app.py b/backend/app.py index 9cbf58d5..7e2204b4 100644 --- a/backend/app.py +++ b/backend/app.py @@ -6,6 +6,7 @@ import binascii import io from datetime import datetime +import ipaddress from pathlib import Path from collections.abc import Callable, Mapping from typing import Any @@ -138,6 +139,30 @@ def _decode_base64_field(payload: str, *, field_name: str) -> bytes: raise HTTPException(status_code=400, detail=f"{field_name} is not valid base64: {exc}") from exc +def non_loopback_bind_warning(*, host: str | None, api_token: str | None) -> str | None: + """Return a startup warning when an unauthenticated bind may be remote. + + Local-first behavior is intentionally unchanged: the service stays open by + default because Docker requires a non-loopback bind and an auto-generated + token would lock out the bundled Dash UI and break existing setups. Callers + print a non-None result at startup. ``/health`` stays unauthenticated. + """ + if api_token: + return None + label = str(host or "") + try: + if ipaddress.ip_address(label.strip()).is_loopback: + return None + except ValueError: + if label.strip().lower() == "localhost": + return None + return ( + f"MaterialScope is listening on {label} without an API token; it may be " + "reachable from other hosts depending on the deployment network. " + "Restart with --token to require X-TA-Token." + ) + + def _model_payload(model: Any) -> dict[str, Any]: if hasattr(model, "model_dump"): return dict(model.model_dump()) diff --git a/backend/main.py b/backend/main.py index c71b1b41..7b04d57c 100644 --- a/backend/main.py +++ b/backend/main.py @@ -9,7 +9,7 @@ import uvicorn from dotenv import load_dotenv -from backend.app import create_app +from backend.app import create_app, non_loopback_bind_warning REPO_ROOT = Path(__file__).resolve().parents[1] REPO_DOTENV_PATH = REPO_ROOT / ".env" @@ -42,6 +42,9 @@ def main() -> None: except OSError as exc: raise SystemExit(f"MaterialScope backend failed preflight bind on {bind_url}: {exc}") from exc app = create_app(api_token=args.token or None) + bind_warning = non_loopback_bind_warning(host=args.host, api_token=args.token or None) + if bind_warning: + print(bind_warning, flush=True) @app.on_event("startup") async def _log_bound_address() -> None: diff --git a/dash_app/api_client.py b/dash_app/api_client.py index 6e99a5ff..587538f6 100644 --- a/dash_app/api_client.py +++ b/dash_app/api_client.py @@ -14,15 +14,26 @@ import httpx _BASE_URL = os.environ.get("MATERIALSCOPE_API_URL", "http://127.0.0.1:8050") -_TOKEN = os.environ.get("MATERIALSCOPE_API_TOKEN", "") + + +def _client_token() -> str: + """Return the outbound API token, resolved per call. + + Resolved late (not at import) so a combined server started with an + explicit ``--token`` can synchronize this process' outbound token before + the first request without depending on Dash import order. + """ + return os.environ.get("MATERIALSCOPE_API_TOKEN", "") + _TIMEOUT = 60.0 def _headers() -> dict[str, str]: h: dict[str, str] = {"Accept": "application/json"} - if _TOKEN: - h["X-MaterialScope-Token"] = _TOKEN - h["X-TA-Token"] = _TOKEN + token = _client_token() + if token: + h["X-MaterialScope-Token"] = token + h["X-TA-Token"] = token return h diff --git a/dash_app/server.py b/dash_app/server.py index 6d266de9..9942f514 100644 --- a/dash_app/server.py +++ b/dash_app/server.py @@ -80,6 +80,12 @@ def create_combined_app(*, api_token: str | None = None): its routes directly on the existing FastAPI instance, so no WSGI bridge is involved anywhere in the request path. """ + if api_token: + # Explicit server token is authoritative: synchronize the co-located + # Dash client's outbound token to the same value (explicit wins over + # any stale MATERIALSCOPE_API_TOKEN). Server auth itself is still + # controlled only by api_token; the env var is never read as auth. + os.environ["MATERIALSCOPE_API_TOKEN"] = api_token from backend.app import create_app as create_backend from dash_app.app import create_dash_app @@ -107,9 +113,16 @@ def main() -> None: for line in apply_combined_dash_server_library_env(listen_host=args.host, listen_port=args.port): print(line, flush=True) os.environ.setdefault("MATERIALSCOPE_API_URL", f"http://127.0.0.1:{args.port}") + if args.token: + os.environ["MATERIALSCOPE_API_TOKEN"] = args.token app = create_combined_app(api_token=args.token or None) print(f"MaterialScope (Dash) starting on http://{args.host}:{args.port}", flush=True) + from backend.app import non_loopback_bind_warning + + warning = non_loopback_bind_warning(host=args.host, api_token=args.token or None) + if warning: + print(warning, flush=True) uvicorn.run(app, host=args.host, port=args.port, log_level="info", http="h11") diff --git a/tests/test_container_bind_warning.py b/tests/test_container_bind_warning.py new file mode 100644 index 00000000..df689678 --- /dev/null +++ b/tests/test_container_bind_warning.py @@ -0,0 +1,37 @@ +"""Track C (PR-5): bind-warning helper cases.""" + +from __future__ import annotations + +import pytest + +from backend.app import non_loopback_bind_warning + +EXPECTED = ( + "MaterialScope is listening on 0.0.0.0 without an API token; it may be " + "reachable from other hosts depending on the deployment network. " + "Restart with --token to require X-TA-Token." +) + + +@pytest.mark.parametrize("host", ["127.0.0.1", "::1", "localhost"]) +def test_loopback_binds_stay_quiet(host: str) -> None: + assert non_loopback_bind_warning(host=host, api_token=None) is None + + +def test_non_loopback_bind_without_token_warns() -> None: + assert non_loopback_bind_warning(host="0.0.0.0", api_token=None) == EXPECTED + + +def test_token_silences_warning() -> None: + assert non_loopback_bind_warning(host="0.0.0.0", api_token="secret") is None + + +@pytest.mark.parametrize("host", ["", "example.com"]) +def test_unparseable_or_remote_host_warns(host: str) -> None: + warning = non_loopback_bind_warning(host=host, api_token=None) + assert warning is not None + assert "without an API token" in warning + + +def test_none_host_warns() -> None: + assert non_loopback_bind_warning(host=None, api_token=None) is not None diff --git a/tests/test_dash_fastapi_backend.py b/tests/test_dash_fastapi_backend.py index 5bbf0514..c6bd1bf9 100644 --- a/tests/test_dash_fastapi_backend.py +++ b/tests/test_dash_fastapi_backend.py @@ -200,3 +200,38 @@ async def send(message): assert response.get("complete") body = jsonlib.loads(response["body"]) assert body["response"]["proxy-probe-output"]["children"] == "echo:absolute-form" + + +def test_explicit_token_wins_over_stale_client_env(monkeypatch): + """Explicit api_token synchronizes the bundled client; backend needs it.""" + import os + + from dash_app import api_client + + monkeypatch.setenv("MATERIALSCOPE_API_TOKEN", "old") + app = create_combined_app(api_token="new") + client = TestClient(app) + + assert os.environ["MATERIALSCOPE_API_TOKEN"] == "new" + assert api_client._headers()["X-TA-Token"] == "new" + + assert client.get("/health").status_code == 200 + assert client.post("/workspace/new").status_code == 401 + assert client.post("/workspace/new", headers={"X-TA-Token": "wrong"}).status_code == 401 + authed = client.post("/workspace/new", headers={"X-TA-Token": "new"}) + assert authed.status_code == 200 + assert authed.json()["project_id"] + assert client.get("/").status_code == 200 + + +def test_no_explicit_token_leaves_client_env_alone(monkeypatch): + """Without api_token the open-by-default contract is unchanged.""" + import os + + monkeypatch.setenv("MATERIALSCOPE_API_TOKEN", "env-only") + app = create_combined_app() + client = TestClient(app) + + assert os.environ["MATERIALSCOPE_API_TOKEN"] == "env-only" + assert client.get("/health").status_code == 200 + assert client.post("/workspace/new").status_code == 200