fix: harden runtime config lease boundary

harness/tests/test_runtime_config_lease_io.py

@@ -0,0 +1,84 @@
+"""Focused unit coverage for the privileged runtime publication seam."""
+
+import os
+
+import pytest
+
+from tht import runtime_config_lease_io as lease_io
+
+
+def _manifest() -> dict:
+    return {
+        "version": 1, "workspace_id": "abc", "workspace_revision": "a" * 40,
+        "descriptor_git_blob": "b" * 40, "descriptor_sha256": "c" * 64,
+        "descriptor_dev": "1", "descriptor_ino": "2", "config_sha256": "d" * 64,
+        "config_dwh_binding": {
+            "workspace_id": "abc", "config_fingerprint": "e", "input_fingerprint": "f"
+        },
+        "config_dev": "1", "config_ino": "3", "config_size": "4",
+        "config_mode": "400", "config_uid": str(os.getuid()), "config_nlink": "1",
+        "directory_identities": [{"path": "/", "dev": "1", "ino": "1", "mode": "755", "uid": "0"}],
+    }
+
+
+def test_strict_manifest_rejects_unknown_or_missing_fields():
+    value = _manifest()
+    assert lease_io.strict_manifest(value) is value
+    with pytest.raises(RuntimeError):
+        lease_io.strict_manifest({**value, "unexpected": True})
+    missing = dict(value)
+    del missing["config_sha256"]
+    with pytest.raises(RuntimeError):
+        lease_io.strict_manifest(missing)
+
+
+def test_private_alias_is_darwin_only(monkeypatch):
+    monkeypatch.setattr(lease_io.sys, "platform", "linux")
+    assert lease_io._canonical_root("/tmp/runtime") == "/tmp/runtime"
+    assert lease_io._canonical_root("/var/lib/runtime") == "/var/lib/runtime"
+    monkeypatch.setattr(lease_io.sys, "platform", "darwin")
+    assert lease_io._canonical_root("/tmp/runtime") == "/private/tmp/runtime"
+    assert lease_io._canonical_root("/var/lib/runtime") == "/private/var/lib/runtime"
+
+
+def test_read_all_enforces_bound(tmp_path):
+    path = tmp_path / "large"
+    path.write_bytes(b"0123456789")
+    fd = os.open(path, os.O_RDONLY)
+    try:
+        with pytest.raises(RuntimeError, match="too large"):
+            lease_io.read_all(fd, limit=4)
+        with pytest.raises(RuntimeError, match="too large"):
+            lease_io.read_all(fd, limit=0)
+    finally:
+        os.close(fd)
+
+
+@pytest.mark.parametrize("failure_stage", ["config-parent", "manifest-parent"])
+def test_retry_reasserts_parent_durability_before_success(tmp_path, monkeypatch, failure_stage):
+    events: list[str] = []
+    failed = False
+
+    def fsync(fd: int, stage: str) -> None:
+        nonlocal failed
+        events.append(stage)
+        if stage == failure_stage and not failed:
+            failed = True
+            raise RuntimeError("injected fsync failure")
+
+    monkeypatch.setattr(lease_io, "publication_fsync", fsync)
+    inp = {
+        "data_root": str(tmp_path / "data"), "workspace_id": "abc",
+        "workspace_revision": "a" * 40, "config_hex": b"config".hex(),
+        "manifest_base": {
+            "workspace_id": "abc", "workspace_revision": "a" * 40,
+            "descriptor_git_blob": "b" * 40, "descriptor_sha256": "c" * 64,
+            "descriptor_dev": "1", "descriptor_ino": "2",
+            "config_dwh_binding": {"workspace_id": "abc", "config_fingerprint": "e", "input_fingerprint": "f"},
+        },
+    }
+    with pytest.raises(RuntimeError, match="injected"):
+        lease_io.publish(inp)
+    events.clear()
+    lease_io.publish(inp)
+    assert events.index("config-parent") < events.index("manifest-parent")

harness/tht/config.py

@@ -638,10 +638,11 @@ def _strict_runtime_manifest(raw: object) -> dict[str, object]:
 
 def _canonical_runtime_path(path: Path) -> Path:
     value = str(path)
-    if value == "/tmp" or value.startswith("/tmp/"):
-        return Path("/private" + value)
-    if value == "/var" or value.startswith("/var/"):
-        return Path("/private" + value)
+    if sys.platform == "darwin":
+        if value == "/tmp" or value.startswith("/tmp/"):
+            return Path("/private" + value)
+        if value == "/var" or value.startswith("/var/"):
+            return Path("/private" + value)
     return path
 
 

harness/tht/runtime_config_lease_io.py

@@ -12,6 +12,7 @@ import stat
 import subprocess
 import sys
 import tempfile
+import time
 from pathlib import Path
 
 
@@ -20,12 +21,13 @@ def fail(msg: str) -> None:
 
 
 def _canonical_root(root: str) -> str:
-    # Darwin exposes /tmp and /var as symlink aliases.  The trusted path boundary
-    # records the real OS-owned prefix so a manifest never contains a symlink.
-    if root == "/tmp" or root.startswith("/tmp/"):
-        return "/private" + root
-    if root == "/var" or root.startswith("/var/"):
-        return "/private" + root
+    # Darwin exposes /tmp and /var as symlink aliases. Linux does not: rewriting
+    # these paths there would redirect valid installations to a different root.
+    if sys.platform == "darwin":
+        if root == "/tmp" or root.startswith("/tmp/"):
+            return "/private" + root
+        if root == "/var" or root.startswith("/var/"):
+            return "/private" + root
     return root
 
 
@@ -180,7 +182,7 @@ def walk(root: str, comps: list[str], create: bool = True) -> int:
     # macOS exposes temporary directories through the conventional /var and
     # /tmp symlinks. Resolve only these OS-owned aliases; workspace-owned
     # ancestors remain component checked and are never realpath-followed.
-    if root == "/var" or root == "/tmp" or root.startswith(("/var/", "/tmp/")):
+    if sys.platform == "darwin" and (root == "/var" or root == "/tmp" or root.startswith(("/var/", "/tmp/"))):
         root = "/private" + root
     parts = [part for part in Path(root).parts if part not in ("", "/")]
     if any(part in (".", "..") or "/" in part for part in parts + comps):
@@ -249,7 +251,13 @@ def publication_fsync(fd: int, stage: str) -> None:
 
 
 def read_all(fd: int, limit: int = 16 * 1024 * 1024) -> bytes:
+    if limit < 0:
+        fail("runtime config file is too large")
     os.lseek(fd, 0, os.SEEK_SET)
+    if limit == 0:
+        if os.read(fd, 1):
+            fail("runtime config file is too large")
+        return b""
     chunks: list[bytes] = []
     total = 0
     while True:
@@ -258,8 +266,12 @@ def read_all(fd: int, limit: int = 16 * 1024 * 1024) -> bytes:
             return b"".join(chunks)
         chunks.append(chunk)
         total += len(chunk)
-        if total > limit:
-            fail("runtime config file is too large")
+        if total >= limit:
+            # The bounded read above cannot observe an additional byte when it
+            # lands exactly on the ceiling; probe once before accepting it.
+            if os.read(fd, 1):
+                fail("runtime config file is too large")
+            return b"".join(chunks)
 
 
 def strict_manifest(value: object) -> dict:
@@ -341,7 +353,17 @@ def publish(inp: dict) -> dict:
     ])
     # The retained preprocessing directory is the single cross-process lock seam.
     # No pathname lock file is created in the workspace layout.
-    fcntl.flock(prep, fcntl.LOCK_EX)
+    deadline = time.monotonic() + 2.0
+    while True:
+        try:
+            fcntl.flock(prep, fcntl.LOCK_EX | fcntl.LOCK_NB)
+            break
+        except BlockingIOError:
+            if time.monotonic() >= deadline:
+                # Never let a wedged publisher block its caller indefinitely. The
+                # Node boundary turns this stable conflict into a bounded failure.
+                fail("runtime config publication is busy")
+            time.sleep(0.01)
     try:
         name = f"{rev}.yaml"
         mname = f"{rev}.json"
@@ -397,6 +419,10 @@ def publish(inp: dict) -> dict:
             finally:
                 os.close(fd)
             publication_fsync(cfgdir, "config-parent")
+        # A prior invocation may have reported success after publishing the
+        # entry but before its parent fsync. Re-establish that durability
+        # boundary before making the manifest durable.
+        publication_fsync(cfgdir, "config-parent")
         # Identity is deliberately recorded after final no-replace publication.
         got = current(cfgdir, name, 0o400)
         assert got
@@ -462,9 +488,12 @@ def publish(inp: dict) -> dict:
                 except FileNotFoundError:
                     pass
             publication_fsync(mandir, "manifest-parent")
+        # As with the config directory, retries must repair a boundary that
+        # failed after the no-replace publication on an earlier invocation.
+        publication_fsync(mandir, "manifest-parent")
         return {
-            "path": f"{root}/sessions/{wid}/preprocessing/runtime-config/{name}",
-            "manifestPath": f"{root}/sessions/{wid}/preprocessing/runtime-config-manifests/{mname}",
+            "path": f"{canonical}/sessions/{wid}/preprocessing/runtime-config/{name}",
+            "manifestPath": f"{canonical}/sessions/{wid}/preprocessing/runtime-config-manifests/{mname}",
             "manifest": mb.decode(),
             "manifest_sha256": hashlib.sha256(mb).hexdigest(),
             "dev": s.st_dev,
@@ -570,6 +599,20 @@ def verified_snapshot(inp: dict) -> dict:
     git_env.update({"GIT_NO_REPLACE_OBJECTS": "1", "GIT_CONFIG_NOSYSTEM": "1",
                     "GIT_CONFIG_GLOBAL": os.devnull, "GIT_CONFIG_SYSTEM": os.devnull})
     try:
+        # A 40-hex object name is not necessarily a commit (trees and blobs are
+        # valid Git objects and also accept the <object>:path syntax). Require
+        # the raw object itself to be a commit, with replacement/config controls
+        # disabled, before reading any descriptor bytes.
+        object_type = subprocess.check_output(
+            ["git", "--no-replace-objects", "-C", repo, "cat-file", "-t", rev],
+            stderr=subprocess.DEVNULL, text=True, timeout=5, env=git_env,
+        ).strip()
+        resolved_commit = subprocess.check_output(
+            ["git", "--no-replace-objects", "-C", repo, "rev-parse", f"{rev}^{{commit}}"],
+            stderr=subprocess.DEVNULL, text=True, timeout=5, env=git_env,
+        ).strip()
+        if object_type != "commit" or resolved_commit != rev:
+            fail("workspace Git revision is not an exact commit")
         blob = subprocess.check_output(
             ["git", "--no-replace-objects", "-C", repo, "rev-parse", f"{rev}:workspaces/{wid}.yaml"],
             stderr=subprocess.DEVNULL, text=True, timeout=5, env=git_env,
@@ -600,6 +643,11 @@ def verified_snapshot(inp: dict) -> dict:
 
 
 def binding(inp: dict) -> dict:
+    # Runtime handoff variables are capabilities, never helper input. Remove
+    # inherited values before load_config can inspect its environment.
+    for key in tuple(os.environ):
+        if key.startswith(("THT_RUNTIME_CONFIG_", "THT_CONFIG_")):
+            os.environ.pop(key, None)
     try:
         raw = bytes.fromhex(inp["config_hex"])
     except (TypeError, ValueError):
@@ -626,15 +674,22 @@ def binding(inp: dict) -> dict:
 def main() -> None:
     try:
         inp = json.load(sys.stdin)
+        if not isinstance(inp, dict) or inp.get("protocol_version") != 1:
+            fail("unsupported runtime config protocol")
         action = inp.get("action")
+        request_keys = {
+            "publish": {"protocol_version", "action", "data_root", "workspace_id", "workspace_revision", "config_hex", "manifest_base"},
+            "verified-snapshot": {"protocol_version", "action", "snapshots_root", "repository_root", "workspace_revision", "workspace_id"},
+            "binding": {"protocol_version", "action", "config_hex"},
+        }
+        if action not in request_keys or set(inp) != request_keys[action]:
+            fail("invalid runtime config request")
         if action == "publish":
             result = publish(inp)
         elif action == "verified-snapshot":
             result = verified_snapshot(inp)
-        elif action == "binding":
-            result = binding(inp)
         else:
-            fail("unsupported runtime config action")
+            result = binding(inp)
         print(json.dumps(result))
     except Exception as e:  # noqa: BLE001
         print(json.dumps({"error": str(e)}))