PR-5: Security trio — archive hardening, license-secret hygiene, bind warning + client token sync

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())

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:

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: <empty>")
+    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)

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:

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
 
 

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")