test(artifact): cover the RECOTEM_ARTIFACT_ROOT write-time containment call site

CLAUDE.md

@@ -130,10 +130,22 @@ Binary container `magic | version | reserved | kid | hmac | header_json | payloa
 - HMAC scope: `kid_bytes || header_json || payload`. Tampering inside those
   bytes fails verify. The 4-byte `header_len` field is **not** covered — it
   only says where the header stops and the payload starts, and both halves
-  are authenticated as one run of bytes. Moving that boundary therefore
-  still passes `verify_hmac` (`recotem inspect` prints `HMAC: OK`) and is
-  caught one layer later by the header JSON parse or the deserializer,
-  reported as exit 5. It shifts a split point; it cannot inject a byte.
+  are authenticated as one run of bytes. It shifts a split point; it cannot
+  inject a byte. Every move of that boundary is refused, and the outcome is
+  always exit 5 with nothing deserialized, but the layer that refuses depends
+  on which way it moved, so do not expect `HMAC: OK` as the tell:
+  - **Shrunk** (`header_len` lowered, `0` included) — reaches `verify_hmac`
+    and passes it, because the authenticated run is unchanged. `recotem
+    inspect` prints `HMAC: OK`, then the header JSON parse fails on the
+    truncated slice. Serve fails the same way, at the `json.loads` in
+    `ModelRegistry._build_entry`, before `unpickle_payload`.
+  - **Enlarged** — refused earlier, inside `parse_header_from_bytes`, so
+    `verify_hmac` is never called and `HMAC: OK` is never printed: the
+    widened header slice swallows payload bytes and fails the UTF-8 decode,
+    or the file is shorter than the claimed header, or the value trips the
+    64 KiB `MAX_HEADER_LEN` cap.
+  The deserializer is never the layer that catches this — the header JSON
+  parse always fires first.
 - Header JSON carries `recipe_name`, `recipe_hash`, `best_class`, `best_params`,
   `best_score`, `metric`, `cutoff`, `tuning`, `data_stats`, `recotem_version`,
   `irspack_version`, `trained_at`. Inspectable without deserialisation via

src/recotem/_http_fetch.py

@@ -154,7 +154,8 @@ def _is_address_internal(addr: ipaddress._BaseAddress) -> bool:
     the IPv6-level properties for these addresses would produce false positives.
     Evaluating the unwrapped IPv4 directly avoids the ambiguity entirely.
     The bogon check also uses the unwrapped IPv4 for mapped addresses.
-    See MAJOR-3 in :doc:`/security`.
+    See https://recotem.org/2.2/docs/security.html ("IPv4-mapped IPv6 inputs are
+    explicitly unwrapped").
     """
     # Primary check for IPv4-mapped IPv6 addresses (``::ffff:a.b.c.d``):
     # unwrap to the embedded IPv4 and evaluate properties there, bypassing

tests/unit/test_artifact_io.py

@@ -391,17 +391,131 @@ def _evil_fsync(fd: int) -> None:
 
     monkeypatch.setattr(_os, "fsync", _evil_fsync)
 
-    with pytest.raises((ArtifactError, OSError)):
-        # _write_atomic must either detect the escape and raise ArtifactError,
-        # or raise OSError because the temp file's parent (inside_dir) no longer
-        # exists as a real directory after the symlink swap.
+    # ArtifactError specifically, not (ArtifactError, OSError).  The swap also
+    # destroys the directory holding the temp file, so ``os.replace`` would
+    # raise OSError on its own -- accepting either exception let the test pass
+    # whether or not the containment re-check ran at all, which is what it
+    # exists to prove.  ``_write_atomic`` calls the check *before*
+    # ``os.replace``, so on a tree that still has the guard the ArtifactError
+    # always wins the race to be raised.
+    with pytest.raises(ArtifactError, match="RECOTEM_ARTIFACT_ROOT"):
         _write_atomic(
             None,  # type: ignore[arg-type] — not used for local FS path
             dest,
             b"dummy artifact bytes",
             is_local=True,
         )
 
+    assert not list(outside_dir.iterdir()), (
+        "the containment re-check must refuse before os.replace, so no bytes "
+        f"may land outside the artifact root; found {list(outside_dir.iterdir())}"
+    )
+
+
+def test_write_artifact_refuses_symlinked_parent_out_of_artifact_root(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """``write_artifact`` itself must enforce RECOTEM_ARTIFACT_ROOT containment.
+
+    The recipe loader validates ``output.path`` at load time; the write-time
+    re-check in ``_write_atomic`` exists because a directory under the root can
+    become a symlink out of it between those two moments.  Every other test
+    around this guard calls ``_assert_output_root_containment`` directly, so
+    deleting its *call site* changed nothing they measure -- and the write then
+    completes, landing artifact bytes outside the root.
+
+    The symlink is in place before the call here (rather than swapped in at
+    fsync time as in the TOCTOU test above) so that the temp file remains
+    writable and renameable: the containment re-check is then the only thing
+    that can refuse, and a pass cannot be borrowed from an incidental OSError.
+    """
+    artifact_root = tmp_path / "root"
+    artifact_root.mkdir()
+    outside_dir = tmp_path / "outside"
+    outside_dir.mkdir()
+    (artifact_root / "models").symlink_to(outside_dir, target_is_directory=True)
+
+    monkeypatch.setenv("RECOTEM_ARTIFACT_ROOT", str(artifact_root))
+    dest = artifact_root / "models" / "model.recotem"
+
+    with pytest.raises(ArtifactError, match="RECOTEM_ARTIFACT_ROOT"):
+        write_artifact(
+            {"payload": "x"},
+            {"recipe_name": "probe"},
+            _make_keyring(),
+            str(dest),
+            versioning="always_overwrite",
+        )
+
+    assert list(outside_dir.iterdir()) == [], (
+        "artifact bytes escaped RECOTEM_ARTIFACT_ROOT through a symlinked "
+        f"parent: {list(outside_dir.iterdir())}"
+    )
+
+
+def test_write_artifact_refuses_dest_that_is_itself_a_symlink_out_of_root(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """The final path component is checked too, not only its parent directory.
+
+    ``_assert_output_root_containment`` resolves the parent *and* the
+    destination.  The parent-symlink case above cannot tell the two apart: the
+    temp file is created inside the same escaped directory, so a check pointed
+    at the temp path instead of at ``dest`` would refuse there as well.  Here
+    the directory is a genuine in-root directory -- only ``dest`` itself is a
+    symlink out -- so a check that does not resolve ``dest`` lets the write
+    through and the bytes follow the symlink.
+    """
+    artifact_root = tmp_path / "root"
+    (artifact_root / "models").mkdir(parents=True)
+    outside_dir = tmp_path / "outside"
+    outside_dir.mkdir()
+
+    dest = artifact_root / "models" / "model.recotem"
+    dest.symlink_to(outside_dir / "model.recotem")
+
+    monkeypatch.setenv("RECOTEM_ARTIFACT_ROOT", str(artifact_root))
+
+    with pytest.raises(ArtifactError, match="RECOTEM_ARTIFACT_ROOT"):
+        write_artifact(
+            {"payload": "x"},
+            {"recipe_name": "probe"},
+            _make_keyring(),
+            str(dest),
+            versioning="always_overwrite",
+        )
+
+    assert not (outside_dir / "model.recotem").exists(), (
+        "artifact bytes escaped RECOTEM_ARTIFACT_ROOT through a symlinked "
+        "destination file"
+    )
+
+
+def test_write_artifact_accepts_real_directory_inside_artifact_root(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Control for the test above: the same call succeeds without the symlink.
+
+    Without this, the sibling test would also pass against a ``write_artifact``
+    that refused every write.
+    """
+    artifact_root = tmp_path / "root"
+    (artifact_root / "models").mkdir(parents=True)
+
+    monkeypatch.setenv("RECOTEM_ARTIFACT_ROOT", str(artifact_root))
+    dest = artifact_root / "models" / "model.recotem"
+
+    written = write_artifact(
+        {"payload": "x"},
+        {"recipe_name": "probe"},
+        _make_keyring(),
+        str(dest),
+        versioning="always_overwrite",
+    )
+
+    assert Path(written) == dest
+    assert dest.exists()
+
 
 def test_assert_output_root_containment_no_op_without_env(tmp_path: Path) -> None:
     """_assert_output_root_containment is a no-op when RECOTEM_ARTIFACT_ROOT is unset."""