From 010bd2b6c8d223fdd5ba8a8d8c84a6268758e286 Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Mon, 3 Aug 2026 18:01:21 -0400 Subject: [PATCH 01/18] feat(gate): observe what a run does to the disk, and say so as it happens `contends` is a list an author typed, and `can_overlap` compares two such lists to each other. Nothing in that loop had ever looked at a disk, so the invariant actually enforced was "the declarations agree with each other" -- and a step that did not mention what it touched satisfied every check by saying nothing. The writer was usually not the step at all, but a unit test three subprocesses below it. That gap cost two release runs. `capsem-admin` staged profile payloads with `fs::hard_link`, putting 48 checked-in `config/` files inside published release output sharing an inode, while a container held the same tree bind-mounted. The symptom was an intermittent `Permission denied` on a file that was `0644` before and `0644` after -- so no before/after comparison could ever have found it. Two sources, because neither alone is enough. `interception.Instrument` proxies the mutating `os`/`shutil` primitives: the caller is known exactly through a `ContextVar` the scheduler sets, and `chmod` is observed with the mode before *and* after, which no watcher can recover. A `watchdog` observer covers what subprocesses do, which no in-process proxy can see. Reported three ways, and the first two were what was missing: stderr the minute it happens, a fault log beside the run -- fsynced per line so a `kill -9` still leaves it, and rotated before writing so the cap is exact rather than `keep + 1` lines of lie -- and the run log. Rules: a hardlink between checked-in source and build output (decidable from one `stat`, no concurrency needed); a mode that returns to one it already had; source writable beyond its owner; an artifact that ends the run empty; identical bytes under two names; and two concurrent steps touching one path neither declared. Existing links are surveyed at start, because a link made by a previous run raises no event in this one and the defect is in the state. `capsem_core::auditfs` is the Rust half: one audited `stage()` that copies when the source is checked in, still hardlinks build output, and fails *closed* when it cannot classify -- the first version failed open on a relative path and 192 files were still linked on the next build. Rust cannot be monkeypatched, so `tests/test_rust_filesystem_chokepoint.py` is the equivalent, scoped to `hard_link` for the reason written into the file. Proven by planting the defect and watching a real `capsem-gate doctor` name it on stderr, in `errors.log`, and in `run.jsonl` -- not by the 22 tests, which are the ratchet. Staging produced 192 hardlinks into published output before `auditfs` and 0 after, artifacts byte-identical with distinct inodes. 1950 gate tests, 909 contract tests, capsem-core 1614. --- CHANGELOG.md | 14 + config/gate.toml | 31 ++ crates/capsem-admin/src/main.rs | 30 +- crates/capsem-core/src/auditfs.rs | 115 ++++++ crates/capsem-core/src/auditfs/tests.rs | 146 +++++++ crates/capsem-core/src/lib.rs | 1 + pyproject.toml | 1 + src/capsem/gate/command.py | 4 +- src/capsem/gate/context.py | 22 + src/capsem/gate/faultlog.py | 68 ++++ src/capsem/gate/faults.py | 134 ++++++ src/capsem/gate/harnessschema.py | 7 + src/capsem/gate/interception.py | 117 ++++++ src/capsem/gate/observation.py | 249 ++++++++++++ src/capsem/gate/observing.py | 69 ++++ src/capsem/gate/planrunner.py | 5 + tests/test_gate_observation.py | 382 ++++++++++++++++++ .../test_gate_primitives_are_the_only_way.py | 16 + tests/test_rust_filesystem_chokepoint.py | 75 ++++ uv.lock | 29 ++ 20 files changed, 1497 insertions(+), 18 deletions(-) create mode 100644 crates/capsem-core/src/auditfs.rs create mode 100644 crates/capsem-core/src/auditfs/tests.rs create mode 100644 src/capsem/gate/faultlog.py create mode 100644 src/capsem/gate/faults.py create mode 100644 src/capsem/gate/interception.py create mode 100644 src/capsem/gate/observation.py create mode 100644 src/capsem/gate/observing.py create mode 100644 tests/test_gate_observation.py create mode 100644 tests/test_rust_filesystem_chokepoint.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 5dbf89c16..d80d3674c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- A gate now reports what it did to the filesystem, as it does it. `contends` + is a list an author typed and the overlap check compares two such lists to + each other -- nothing in that loop had ever looked at a disk, so a step that + did not mention what it touched satisfied every check by saying nothing, and + the writer was frequently a unit test three subprocesses down. Two sources + feed it now: the in-process primitives are proxied, so the caller and the + state *before* the call are both known exactly, and a `watchdog` observer + covers what subprocesses do. Faults land on stderr the minute they occur, in + a size-capped log beside the run that survives a `kill -9`, and in the run + log. It names hardlinks between checked-in source and build output, modes + that change and change back, source writable beyond its owner, artifacts that + end a run empty, identical bytes under two names, and two concurrent steps + touching one path neither declared. + - `release-profile` reached the step before publishing and refused, because `tested-head` was empty: four contracts that run a release plan to read back its argv had overwritten the running gate's record of the revision under diff --git a/config/gate.toml b/config/gate.toml index 59fd0ff4e..e8cd797b2 100644 --- a/config/gate.toml +++ b/config/gate.toml @@ -371,6 +371,11 @@ profiles_glob = "config/profiles/*/profile.toml" # evidence. Spelling the three here as well is how two lists come to disagree. evidence_artifacts = ["obom.cdx.json", "software-inventory.json"] failure_tail_lines = 200 +# What the run watches for filesystem faults, and where it writes them the +# moment they happen. Source trees only: `target/` is where a gate is supposed +# to write, and watching it would drown the report -- except that build output +# is exactly where a hardlink to checked-in source lands, so the staged +# release channel is watched too. shell_proof_timeout_seconds = 300 # `cargo run` rather than a built binary: the asset gate runs before anything @@ -559,6 +564,15 @@ direct_machine_access = [ "runhistory.py", "disk.py", "workspace.py", + # Observation has to touch the machine: it exists to report what the + # machine did. `faults.py` stats and hashes what changed; `faultlog.py` + # writes and fsyncs the report so a killed run still leaves one. + "faults.py", + "faultlog.py", + # And `interception.py` *is* the primitives, proxied -- the whole point is + # that nothing reaches `os` without passing through it. + "interception.py", + "observation.py", ] # Only the plan schedules concurrent work. A module that reaches for its own @@ -930,6 +944,8 @@ blocks_clippy = "frontend" # `tests/test_gate_*.py` to appear here, since by construction those need # neither. source_contract = [ + "tests/test_gate_observation.py", + "tests/test_rust_filesystem_chokepoint.py", "tests/test_agent_skill_index.py", "tests/test_authoritative_values_are_not_restated.py", "tests/test_build_assets_profile.py", @@ -1192,6 +1208,21 @@ events = "run.jsonl" event_schema = "capsem.gate.runlog.v1" step_log_dir = "steps" summary = "summary.txt" +# What each run watches for filesystem faults, and where it writes them the +# moment they happen -- not at the end, and not only into `run.jsonl`, because +# a fault nobody sees until someone reads a file is a fault nobody acts on. +# Source trees, because a gate must not edit what it is qualifying; plus the +# staged release channel, because a hardlink *to* checked-in source lands in +# build output and leaves the source directory untouched. +observed_roots = ["config", "crates", "scripts", "guest", "src", "target/web-parity"] +error_log = "errors.log" +# Bounded, because a run that trips one rule per file trips it thousands of +# times and an unbounded fault log on a machine that gates daily is a +# disk-full outage wearing a helpful name. Total stays under +# max_bytes * (keep + 1); the newest faults -- the ones describing the failure +# being looked at -- are the ones kept. +error_log_max_bytes = 4194304 +error_log_keep = 3 latest_link = "latest" # Allocation, rotation and repointing `latest` are serialized on this, briefly. # Not the machine lock: that is held for a whole gate, and opening a run log diff --git a/crates/capsem-admin/src/main.rs b/crates/capsem-admin/src/main.rs index f4c8d9546..68c27edc8 100644 --- a/crates/capsem-admin/src/main.rs +++ b/crates/capsem-admin/src/main.rs @@ -5864,24 +5864,20 @@ fn copy_file_with_digest(source: &Path, destination: &Path) -> Result<(u64, serd file_digest(destination) } +/// Stage a file into release output. +/// +/// Delegates, because the decision is not "link if you can". Linking a +/// checked-in file into published output makes them one file: this put 48 +/// `config/` seeds inside the release channel sharing an inode, where a chmod +/// on the artifact rewrote tracked source and no content digest noticed. See +/// `capsem_core::auditfs`. fn hardlink_or_copy(source: &Path, destination: &Path) -> Result<()> { - if destination.exists() { - fs::remove_file(destination) - .with_context(|| format!("replace {}", destination.display()))?; - } - match fs::hard_link(source, destination) { - Ok(()) => Ok(()), - Err(link_error) => { - fs::copy(source, destination).with_context(|| { - format!( - "copy {} -> {} after hardlink failed: {link_error}", - source.display(), - destination.display() - ) - })?; - Ok(()) - } - } + capsem_core::auditfs::stage(source, destination, &repo_root()) +} + +/// The checkout this admin invocation is staging from. +fn repo_root() -> PathBuf { + std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")) } fn validate_asset_digest( diff --git a/crates/capsem-core/src/auditfs.rs b/crates/capsem-core/src/auditfs.rs new file mode 100644 index 000000000..0ee22cfc3 --- /dev/null +++ b/crates/capsem-core/src/auditfs.rs @@ -0,0 +1,115 @@ +//! The one place a hardlink is made, because a hardlink is not a copy. +//! +//! `fs::hard_link` is the only operation that makes two paths *the same file*. +//! Everything else -- a write, a rename, a chmod -- affects one name. That +//! distinction stopped being academic when `capsem-admin` staged profile +//! payloads with it: 48 checked-in `config/` files ended up inside the +//! published release channel sharing an inode, so a `chmod` on an artifact +//! rewrote tracked source, and no content digest could notice because the +//! bytes never changed. +//! +//! Linking build output to build output is still right, and still fast: asset +//! staging moves multi-gigabyte images and copying them to satisfy a rule +//! aimed at small checked-in seeds would trade a defect for an hour of I/O. +//! So the rule is about *what* is being linked, not about linking. +//! +//! Rust cannot be monkeypatched the way the gate proxies Python's primitives, +//! so this module plus `tests/test_rust_filesystem_chokepoint.py` is the +//! equivalent: one audited call site, and a test that fails when a crate +//! reaches around it. + +use std::fs; +use std::os::unix::fs::MetadataExt; +use std::path::Path; + +use anyhow::{Context, Result}; + +/// Place `source` at `destination`, linking only when that cannot couple +/// build output to the checked-in tree. +/// +/// `root` is the checkout. A source outside it, or under a build-output +/// directory inside it, is disposable and gets a hardlink. A tracked file gets +/// a copy: the artifact must be able to have its own permissions, its own +/// lifetime, and its own inode. +pub fn stage(source: &Path, destination: &Path, root: &Path) -> Result<()> { + if let Some(parent) = destination.parent() { + fs::create_dir_all(parent) + .with_context(|| format!("create parent for {}", destination.display()))?; + } + if destination.exists() { + fs::remove_file(destination) + .with_context(|| format!("replace {}", destination.display()))?; + } + + if is_build_output(source, root) { + match fs::hard_link(source, destination) { + Ok(()) => return Ok(()), + // `EXDEV` and friends: staging onto another filesystem is normal, + // and is the reason this fell back to copying in the first place. + Err(_) => return copy(source, destination), + } + } + copy(source, destination) +} + +fn copy(source: &Path, destination: &Path) -> Result<()> { + fs::copy(source, destination) + .with_context(|| format!("copy {} -> {}", source.display(), destination.display()))?; + Ok(()) +} + +/// Whether this path is something the build produced rather than something a +/// human checked in. +/// +/// **Only a confident yes permits a link.** The first version answered "yes" +/// for anything it could not place, and a relative path -- which is what the +/// release scripts pass -- could not be placed, so 192 checked-in files were +/// still linked into the published channel after the fix. Failing open on a +/// question about publication integrity is the same mistake as not asking it. +/// +/// A needless copy costs I/O. A needless link costs a published artifact that +/// shares an inode with tracked source, which is what this exists to prevent. +/// +/// Path-based rather than asking git: this runs per staged file during a +/// release, and a subprocess each time would cost more than the copy it is +/// deciding about. +fn is_build_output(source: &Path, root: &Path) -> bool { + // Resolve both sides before comparing: a relative source, a symlinked + // checkout, and `/var` vs `/private/var` on macOS all defeat a plain + // `strip_prefix`, and each of them defeating it means "link the source + // tree into the release". + let (Ok(source), Ok(root)) = (absolute(source), absolute(root)) else { + return false; + }; + let Ok(relative) = source.strip_prefix(&root) else { + return false; + }; + matches!( + relative.components().next().and_then(|c| c.as_os_str().to_str()), + Some("target") | Some("packages") | Some("assets") + ) +} + +fn absolute(path: &Path) -> std::io::Result { + // `canonicalize` needs the path to exist, which a destination's parent may + // not; every caller here passes an existing source, and the fallback keeps + // the answer conservative rather than wrong. + path.canonicalize().or_else(|error| { + if path.is_absolute() { + Ok(path.to_path_buf()) + } else { + std::env::current_dir().map(|cwd| cwd.join(path)).map_err(|_| error) + } + }) +} + +/// How many names this file has. Exposed so callers and tests can assert the +/// property rather than infer it. +pub fn links(path: &Path) -> Result { + Ok(fs::metadata(path) + .with_context(|| format!("stat {}", path.display()))? + .nlink()) +} + +#[cfg(test)] +mod tests; diff --git a/crates/capsem-core/src/auditfs/tests.rs b/crates/capsem-core/src/auditfs/tests.rs new file mode 100644 index 000000000..ea68a60ca --- /dev/null +++ b/crates/capsem-core/src/auditfs/tests.rs @@ -0,0 +1,146 @@ +//! Linking is the only operation that makes two paths the same file. + +use super::*; +use std::fs; + +fn checkout(root: &std::path::Path) -> std::path::PathBuf { + let source = root.join("config/profiles/code"); + fs::create_dir_all(&source).unwrap(); + let seed = source.join("root.manifest.json"); + fs::write(&seed, b"{}").unwrap(); + seed +} + +#[test] +fn staging_checked_in_source_into_output_copies_instead_of_linking() { + // The defect, in one assertion. `capsem-admin` hardlinked profile seeds + // into the published release channel, so 48 tracked files sat inside + // build output sharing an inode. A `chmod` on the artifact rewrote the + // source file, and no content digest could notice: the bytes never moved. + let temp = tempfile::tempdir().unwrap(); + let root = temp.path(); + let seed = checkout(root); + let published = root.join("target/release-channel/root.manifest.json"); + + stage(&seed, &published, root).unwrap(); + + assert_eq!(fs::read(&published).unwrap(), b"{}"); + assert_ne!( + fs::metadata(&seed).unwrap().ino(), + fs::metadata(&published).unwrap().ino(), + "the published artifact is the same file as checked-in source" + ); + assert_eq!( + fs::metadata(&seed).unwrap().nlink(), + 1, + "the source file gained a link" + ); +} + +#[test] +fn a_chmod_on_the_published_artifact_cannot_reach_the_source() { + // Why the inode matters, stated as the consequence rather than the + // mechanism. This is the failure a reader has to be able to picture. + use std::os::unix::fs::PermissionsExt; + + let temp = tempfile::tempdir().unwrap(); + let root = temp.path(); + let seed = checkout(root); + let published = root.join("target/release-channel/root.manifest.json"); + stage(&seed, &published, root).unwrap(); + + fs::set_permissions(&published, fs::Permissions::from_mode(0o000)).unwrap(); + + let source_mode = fs::metadata(&seed).unwrap().permissions().mode() & 0o777; + assert_ne!(source_mode, 0o000, "chmod on the artifact reached the source"); + assert!(fs::read(&seed).is_ok(), "checked-in source became unreadable"); +} + +#[test] +fn staging_build_output_into_build_output_still_hardlinks() { + // The optimization is real and worth keeping: asset staging moves + // multi-gigabyte images, and copying them because of a rule aimed at + // small checked-in seeds would trade one defect for an hour of I/O. + let temp = tempfile::tempdir().unwrap(); + let root = temp.path(); + let built = root.join("target/assets/rootfs.erofs"); + fs::create_dir_all(built.parent().unwrap()).unwrap(); + fs::write(&built, b"image").unwrap(); + let staged = root.join("target/release-channel/rootfs.erofs"); + + stage(&built, &staged, root).unwrap(); + + assert_eq!( + fs::metadata(&built).unwrap().ino(), + fs::metadata(&staged).unwrap().ino(), + "build output should still be linked, not copied" + ); +} + +#[test] +fn a_cross_device_link_falls_back_to_copying() { + // `EXDEV` is the documented reason `hardlink_or_copy` existed at all, and + // dropping it would break staging onto a different filesystem. + let temp = tempfile::tempdir().unwrap(); + let root = temp.path(); + let built = root.join("target/assets/x"); + fs::create_dir_all(built.parent().unwrap()).unwrap(); + fs::write(&built, b"bytes").unwrap(); + + // A destination whose parent does not exist yet exercises the same path + // the release staging takes. + let staged = root.join("target/deep/nested/x"); + stage(&built, &staged, root).unwrap(); + assert_eq!(fs::read(&staged).unwrap(), b"bytes"); +} + +#[test] +fn a_relative_source_path_is_not_assumed_to_be_build_output() { + // The first version of `stage` answered "outside the checkout" for + // anything `strip_prefix` could not resolve, and then hardlinked it. A + // relative path -- which is what the release scripts actually pass -- took + // that branch, so the fix shipped and 192 checked-in files were still + // linked into the published channel on the very next build. + // + // Not knowing must mean copying. A needless copy costs I/O; a needless + // link costs a published artifact that shares an inode with tracked + // source, which is the defect this module exists for. + let temp = tempfile::tempdir().unwrap(); + let root = temp.path(); + let seed = checkout(root); + let published = root.join("target/release-channel/root.manifest.json"); + + let previous = std::env::current_dir().unwrap(); + std::env::set_current_dir(root).unwrap(); + let result = stage( + std::path::Path::new("config/profiles/code/root.manifest.json"), + &published, + root, + ); + std::env::set_current_dir(previous).unwrap(); + result.unwrap(); + + assert_ne!( + fs::metadata(&seed).unwrap().ino(), + fs::metadata(&published).unwrap().ino(), + "a relative checked-in path was treated as build output and linked" + ); +} + +#[test] +fn a_source_that_cannot_be_classified_is_copied() { + // The rule stated directly, so it cannot drift back to failing open. + let temp = tempfile::tempdir().unwrap(); + let root = temp.path(); + let elsewhere = tempfile::tempdir().unwrap(); + let source = elsewhere.path().join("unknown.bin"); + fs::write(&source, b"bytes").unwrap(); + let published = root.join("target/out.bin"); + + stage(&source, &published, root).unwrap(); + + assert_ne!( + fs::metadata(&source).unwrap().ino(), + fs::metadata(&published).unwrap().ino(), + ); +} diff --git a/crates/capsem-core/src/lib.rs b/crates/capsem-core/src/lib.rs index e35dbd5aa..5cb24df92 100644 --- a/crates/capsem-core/src/lib.rs +++ b/crates/capsem-core/src/lib.rs @@ -1,4 +1,5 @@ pub mod asset_manager; +pub mod auditfs; pub mod auto_snapshot; pub mod credential_broker; pub mod fs_monitor; diff --git a/pyproject.toml b/pyproject.toml index 9adc30abf..b26e56afc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -127,5 +127,6 @@ dev = [ "pyyaml>=6.0.3", "ruff>=0.15.16", "ty>=0.0.46", + "watchdog>=6.0.0", "websockets>=16.0", ] diff --git a/src/capsem/gate/command.py b/src/capsem/gate/command.py index 760d31aaf..45c885285 100644 --- a/src/capsem/gate/command.py +++ b/src/capsem/gate/command.py @@ -26,6 +26,7 @@ from .funnel import GuardedRunner from .lifecycle import Resource, environment_of, held from .locks import ExclusiveLock +from .observing import observing from .plan import Plan from .planseal import sealed from .proc import Runner @@ -193,7 +194,7 @@ def execute(self) -> None: if replacement is not None: raise SystemExit(self._runner.run(replacement, check=False)) - with self._recording() as log: + with self._recording() as log, observing(self._config, log, plan) as watch: # Every invocation from here is recorded, and none may start a # second gate. Neither is a call site's responsibility. runner = GuardedRunner( @@ -208,6 +209,7 @@ def execute(self) -> None: self._config, journal=log, env=environment_of(acquired), + watch=watch, ) ) # Outside the run log's own context, so `run.end` is on disk before diff --git a/src/capsem/gate/context.py b/src/capsem/gate/context.py index 8a3b12c5b..9ff75f2fc 100644 --- a/src/capsem/gate/context.py +++ b/src/capsem/gate/context.py @@ -29,6 +29,21 @@ from .execution import Step +class StepObserver(Protocol): + """What the scheduler tells the run's filesystem observer. + + A protocol rather than the concrete `Watch`, so nothing below the harness + depends on how observation is implemented -- the same reason `Journal` is + one. + """ + + def entered(self, label: str) -> None: + """This step's thread is now running.""" + + def left(self, label: str) -> None: + """It is not.""" + + class Journal(Protocol): """What a run is recorded into, as the rest of the package sees it. @@ -186,6 +201,13 @@ class Context: config: GateConfig journal: Journal = field(default_factory=NullJournal) + watch: StepObserver | None = None + """The run's filesystem observer, when one is running. + + Carried here so the scheduler can tell it which steps are in flight; an + action never touches it. + """ + observing: bool = False """This plan is being read, not run, so nothing may touch the machine. diff --git a/src/capsem/gate/faultlog.py b/src/capsem/gate/faultlog.py new file mode 100644 index 000000000..17d9ddbf9 --- /dev/null +++ b/src/capsem/gate/faultlog.py @@ -0,0 +1,68 @@ +"""Where faults go the moment they are found, and how they stop growing. + +Two properties, and the second is why this is not one `open()` at a call site. + +**It survives the run.** Line-buffered and fsynced per fault, because the run +being described may be killed -- and a report that only exists after a clean +exit is missing exactly when it is most wanted. + +**It is bounded.** A run that trips one rule per file trips it thousands of +times, and an unbounded fault log on a machine that runs the gate daily is a +disk-full outage wearing a helpful name. Rotation is by size, oldest dropped +first, so the newest faults -- the ones describing the failure you are looking +at -- are the ones kept. +""" + +from __future__ import annotations + +import os +import time +from pathlib import Path + +from .faults import Fault + + +class FaultLog: + """An append-only, size-capped record of what a run did wrong.""" + + def __init__(self, path: Path, *, max_bytes: int, keep: int) -> None: + self.path = path + self._max_bytes = max_bytes + self._keep = keep + path.parent.mkdir(parents=True, exist_ok=True) + self._handle = path.open("a", buffering=1, encoding="utf-8") + + def __call__(self, fault: Fault) -> None: + stamp = time.strftime("%H:%M:%S") + line = f"{stamp} {fault.render()}\n" + # Rotate *before* writing, not after exceeding. Rotating afterwards + # leaves every generation one line over the cap, so the budget a disk + # policy is written against is a lie by `keep + 1` lines. + if self._handle.tell() + len(line.encode("utf-8")) > self._max_bytes: + self._rotate() + self._handle.write(line) + # Per fault, not per close: the point is that a `kill -9` still leaves + # the line describing what went wrong just before it. + os.fsync(self._handle.fileno()) + + def _rotate(self) -> None: + """Shift the numbered generations down and start a new file. + + `keep` bounds total consumption at `max_bytes * (keep + 1)` exactly, + which is the number a disk budget can be written against. + """ + self._handle.close() + oldest = self.path.with_suffix(f"{self.path.suffix}.{self._keep}") + oldest.unlink(missing_ok=True) + for generation in range(self._keep - 1, 0, -1): + source = self.path.with_suffix(f"{self.path.suffix}.{generation}") + if source.exists(): + source.replace(self.path.with_suffix(f"{self.path.suffix}.{generation + 1}")) + if self._keep > 0: + self.path.replace(self.path.with_suffix(f"{self.path.suffix}.1")) + else: + self.path.unlink(missing_ok=True) + self._handle = self.path.open("a", buffering=1, encoding="utf-8") + + def close(self) -> None: + self._handle.close() diff --git a/src/capsem/gate/faults.py b/src/capsem/gate/faults.py new file mode 100644 index 000000000..ef6cbfa50 --- /dev/null +++ b/src/capsem/gate/faults.py @@ -0,0 +1,134 @@ +"""What a filesystem fault is made of, and how to find it out cheaply. + +Separated from the watcher because the rules are worth testing against a list +of facts, without a disk, a scheduler, or a sixty-minute gate behind them. +""" + +from __future__ import annotations + +import hashlib +import stat +import subprocess +from dataclasses import dataclass +from pathlib import Path + +#: Directories under the checkout a run may write. Everything else is input: +#: the gate reads it, and changing it mid-run means the thing being qualified +#: is not the thing that was measured. +BUILD_OUTPUT = frozenset({"target", ".git", "node_modules", ".venv"}) + +#: Hash files up to this size. Digests answer "are these the same bytes under +#: two names", which matters for seeds and manifests; a multi-gigabyte rootfs +#: is answered by inode and size at a fraction of the cost. +DIGEST_LIMIT = 1 << 20 + + +@dataclass(frozen=True, slots=True) +class Facts: + """Everything one `stat` already paid for, plus a digest when it is cheap. + + Typed rather than a dict: these reach `Event` directly, and a loose + `dict[str, int | str | None]` made `ty` unable to tell a mode from a + digest -- which is the sort of thing that reads fine and stores a hash in + a permission field. + """ + + mode: int | None = None + size: int | None = None + inode: int | None = None + links: int | None = None + digest: str | None = None + + +@dataclass(frozen=True, slots=True) +class Event: + """One observed change and who caused it.""" + + at: float + kind: str + path: Path + steps: tuple[str, ...] + facts: Facts = Facts() + + @property + def mode(self) -> int | None: + return self.facts.mode + + @property + def inode(self) -> int | None: + return self.facts.inode + + @property + def links(self) -> int | None: + return self.facts.links + + @property + def digest(self) -> str | None: + return self.facts.digest + + +@dataclass(frozen=True, slots=True) +class Fault: + """A rule broken, in the terms someone can act on.""" + + path: Path + steps: tuple[str, ...] + reason: str + detail: str + + def render(self) -> str: + who = ", ".join(self.steps) or "no step in flight" + return f"[{self.reason}] {self.path}: {self.detail} (steps: {who})" + + +def facts_of(path: Path) -> Facts: + """One `stat`, and a digest only when the file is small enough to be worth it.""" + try: + info = path.stat() + except OSError: + return Facts() + digest = None + if stat.S_ISREG(info.st_mode) and 0 < info.st_size <= DIGEST_LIMIT: + try: + digest = hashlib.blake2b(path.read_bytes(), digest_size=16).hexdigest() + except OSError: + digest = None + return Facts( + mode=stat.S_IMODE(info.st_mode), + size=info.st_size, + inode=info.st_ino, + links=info.st_nlink, + digest=digest, + ) + + +def source_inodes(source_root: Path) -> dict[int, Path]: + """Every checked-in file, keyed by inode. + + A hardlink into build output creates a directory entry in `target/` and + leaves the source directory untouched, so watching the source tree cannot + see it happen: the only trace is that the new file's inode is one of + these. `git ls-files` rather than a walk -- it is the authority on what is + tracked, it does not descend into build output, and it costs milliseconds. + """ + try: + listing = subprocess.run( + ["git", "ls-files", "-z"], + cwd=source_root, + capture_output=True, + text=True, + check=True, + ).stdout + except (OSError, subprocess.CalledProcessError): + return {} + + inodes: dict[int, Path] = {} + for name in listing.split("\0"): + if not name: + continue + path = source_root / name + try: + inodes[path.stat().st_ino] = path + except OSError: + continue + return inodes diff --git a/src/capsem/gate/harnessschema.py b/src/capsem/gate/harnessschema.py index 57dfceb88..302192043 100644 --- a/src/capsem/gate/harnessschema.py +++ b/src/capsem/gate/harnessschema.py @@ -179,6 +179,13 @@ class RunLogConfig(Strict): step_log_dir: str summary: str latest_link: str + #: Trees each run watches for filesystem faults, relative to the checkout. + observed_roots: tuple[str, ...] + #: Where those faults are written the instant they are found, per run. + error_log: str + #: Size cap and generations kept, so faults cannot fill the disk. + error_log_max_bytes: int + error_log_keep: int history_lock: str active_marker: str keep_runs: PositiveInt diff --git a/src/capsem/gate/interception.py b/src/capsem/gate/interception.py new file mode 100644 index 000000000..5f8a06a37 --- /dev/null +++ b/src/capsem/gate/interception.py @@ -0,0 +1,117 @@ +"""Proxy the primitives that change the disk, so nothing can avoid being seen. + +An external watcher answers "something happened here, shortly before I +looked". That leaves three gaps this closes: the state at the moment of the +call is already gone by the time a notification arrives, the caller is +unknown, and anything the platform coalesces is simply missing. + +Wrapping the call has none of those. `os.chmod` is observed with the mode +before *and* after; `os.link` is observed knowing both paths; and a primitive +added tomorrow that nobody remembers to instrument is caught by +`test_every_mutating_primitive_the_stdlib_offers_is_intercepted`, which reads +this list against the standard library rather than trusting it. + +In-process only, by construction: a subprocess makes its own syscalls, and +those stay the external watcher's job. +""" + +from __future__ import annotations + +import contextvars +import functools +import os +import shutil +import stat +from collections.abc import Callable +from pathlib import Path +from typing import Protocol + +#: The step whose thread is executing. A `ContextVar` rather than "everything +#: in flight", because the scheduler runs steps in worker threads and the +#: caller is then known exactly instead of narrowed to a set. +CURRENT_STEP: contextvars.ContextVar[str | None] = contextvars.ContextVar( + "capsem_gate_step", default=None +) + + +class Observer(Protocol): + """What interception needs from whatever is judging.""" + + def observed(self, kind: str, path: Path, *, before: int | None = None) -> None: ... + + +class Instrument: + """Patch the mutating primitives for the duration of a run.""" + + #: (module, attribute, kind). Data, so a test can compare it against what + #: the standard library actually offers instead of a reviewer noticing. + TARGETS: tuple[tuple[object, str, str], ...] = ( + (os, "link", "link"), + (os, "symlink", "symlink"), + (os, "unlink", "unlink"), + (os, "remove", "unlink"), + (os, "rmdir", "unlink"), + (os, "chmod", "chmod"), + (os, "rename", "rename"), + (os, "replace", "rename"), + (os, "truncate", "write"), + (shutil, "copy", "copy"), + (shutil, "copy2", "copy"), + (shutil, "copyfile", "copy"), + (shutil, "copytree", "copy"), + (shutil, "rmtree", "unlink"), + (shutil, "move", "rename"), + ) + + #: Calls whose *destination* is the interesting path: `link(src, dst)` + #: creates `dst`, and `dst` is what now shares an inode it should not. + DESTINATION_IS_SECOND = frozenset({"link", "copy", "rename"}) + + def __init__(self, observer: Observer) -> None: + self._observer = observer + self._saved: list[tuple[object, str, object]] = [] + + def __enter__(self) -> Instrument: + for module, name, kind in self.TARGETS: + original = getattr(module, name) + self._saved.append((module, name, original)) + setattr(module, name, self._wrap(original, kind)) + return self + + def __exit__(self, *_: object) -> None: + for module, name, original in reversed(self._saved): + setattr(module, name, original) + self._saved.clear() + + def _wrap(self, original: Callable[..., object], kind: str) -> Callable[..., object]: + observer = self._observer + + @functools.wraps(original) + def proxy(*args: object, **kwargs: object) -> object: + subject = _subject(kind, args) + before = _mode_of(subject) if kind == "chmod" else None + result = original(*args, **kwargs) + if subject is not None: + observer.observed(kind, Path(str(subject)), before=before) + return result + + return proxy + + +def _subject(kind: str, args: tuple[object, ...]) -> object | None: + if kind in Instrument.DESTINATION_IS_SECOND and len(args) > 1: + return args[1] + return args[0] if args else None + + +def _mode_of(path: object) -> int | None: + """The mode as it is *right now*, which after the call is unrecoverable.""" + # `str` only. Every proxied caller passes a path, and accepting the whole + # `PathLike` union leaves `PathLike[object]` for the checker to reject -- + # a file descriptor has no mode to read here anyway. + if not isinstance(path, str | os.PathLike): + return None + try: + return stat.S_IMODE(os.stat(str(path)).st_mode) + except (OSError, ValueError): + return None diff --git a/src/capsem/gate/observation.py b/src/capsem/gate/observation.py new file mode 100644 index 000000000..ff379d4bd --- /dev/null +++ b/src/capsem/gate/observation.py @@ -0,0 +1,249 @@ +"""What the run did to the filesystem, judged as it happens. + +`contends` is a list an author typed and `can_overlap` compares two such lists +to each other. Neither has ever looked at a disk, so the invariant actually +enforced is "the declarations agree with each other" -- and a step that does +not mention what it touches satisfies every check by saying nothing. The +writer is frequently not the step at all, but a unit test three subprocesses +below it. + +Collecting and judging are one pass on purpose. A separate analyzer is a thing +that runs later, which in practice means a thing that runs never: a rule +evaluated after a sixty-minute gate is a rule nobody acts on until someone +thinks to read a file. + +Two sources feed it. `interception.Instrument` wraps the in-process primitives +and is exact -- the caller, and the state before the call. A `watchdog` +observer covers what subprocesses do, which no in-process proxy can see, and +is best-effort: it is notified and then stats, so a change reverted inside +that window is already gone when it looks. +""" + +from __future__ import annotations + +import stat +import threading +import time +from collections.abc import Callable, Iterable, Mapping +from pathlib import Path +from types import TracebackType +from typing import TYPE_CHECKING + +from .faults import Event, Fault, facts_of, source_inodes +from .interception import CURRENT_STEP + +if TYPE_CHECKING: # pragma: no cover - imported for typing only + from watchdog.observers.api import BaseObserver + + +class Watch: + """Observes the paths a run must not disturb and judges each change live.""" + + def __init__( + self, + roots: Iterable[Path], + *, + source_root: Path, + declared: Mapping[str, frozenset[str]] | None = None, + on_fault: Callable[[Fault], None] | None = None, + ) -> None: + self._roots = [root for root in roots if root.exists()] + self._source_root = source_root.resolve() + self._source_inodes = source_inodes(self._source_root) + self._declared = dict(declared or {}) + self._on_fault = on_fault + self.faults: list[Fault] = [] + self.events: list[Event] = [] + + self._live: set[str] = set() + self._lock = threading.Lock() + self._observer: BaseObserver | None = None + self._modes: dict[Path, list[int]] = {} + self._digests: dict[str, Path] = {} + self._reported: set[tuple[Path, str]] = set() + + # -- step attribution --------------------------------------------------- + + def entered(self, label: str) -> None: + with self._lock: + self._live.add(label) + + def left(self, label: str) -> None: + with self._lock: + self._live.discard(label) + + # -- lifecycle ---------------------------------------------------------- + + def __enter__(self) -> Watch: + from watchdog.events import FileSystemEventHandler + from watchdog.observers import Observer + + watch = self + + class _Handler(FileSystemEventHandler): + def on_any_event(self, event) -> None: + if event.is_directory or event.event_type == "opened": + return + watch.observed(event.event_type, Path(str(event.src_path))) + + observer = Observer() + for root in self._roots: + observer.schedule(_Handler(), str(root), recursive=True) + observer.start() + self._observer = observer + self.survey() + return self + + def __exit__( + self, + kind: type[BaseException] | None, + error: BaseException | None, + traceback: TracebackType | None, + ) -> None: + observer = self._observer + if observer is None: + return + observer.stop() + # Bounded: a hung observer thread must not outlive the run it watched. + observer.join(timeout=5) + self._observer = None + + # -- observation -------------------------------------------------------- + + def observed(self, kind: str, path: Path, *, before: int | None = None) -> None: + """One change, from either source. + + `before` is the mode a moment ago, which only an intercepted caller + can supply: after the call it is simply the current mode, and the + transition is what makes a concurrent reader fail. + """ + step = CURRENT_STEP.get() + if step is not None: + steps: tuple[str, ...] = (step,) + else: + with self._lock: + steps = tuple(sorted(self._live)) + event = Event(at=time.time(), kind=kind, path=path, steps=steps, facts=facts_of(path)) + self.events.append(event) + if before is not None: + history = self._modes.setdefault(path, []) + if not history: + history.append(before) + self._judge(event) + + def _fault(self, event: Event, reason: str, detail: str) -> None: + key = (event.path, reason) + if key in self._reported: + return + self._reported.add(key) + fault = Fault(path=event.path, steps=event.steps, reason=reason, detail=detail) + self.faults.append(fault) + if self._on_fault is not None: + self._on_fault(fault) + + def _judge(self, event: Event) -> None: + source = self.is_source(event.path) + + if source and event.kind != "deleted": + self._fault( + event, "source-tree", f"{event.kind} during the run; the gate qualifies this tree" + ) + + if not source and event.inode is not None and event.links and event.links > 1: + origin = self._source_inodes.get(event.inode) + if origin is not None: + self._fault( + event, + "hardlinked-source", + f"shares inode {event.inode} (nlink={event.links}) with checked-in " + f"{origin.relative_to(self._source_root)}, so a chmod here rewrites " + "tracked source and no content digest will notice", + ) + + if event.mode is not None: + history = self._modes.setdefault(event.path, []) + # Returning to any mode already seen -- not merely the one before + # last -- is the flip-flop: 0644 -> 0000 -> 0644 is identical at + # both ends and unreadable in the middle. + if history and event.mode != history[-1] and event.mode in history[:-1]: + self._fault( + event, + "mode-flip-flop", + f"{history[-1]:04o} -> {event.mode:04o}, back to a mode it already had; " + "a concurrent reader sees the middle state and fails intermittently", + ) + if not history or history[-1] != event.mode: + history.append(event.mode) + if source and event.mode & (stat.S_IWGRP | stat.S_IWOTH): + self._fault( + event, "over-permission", f"mode {event.mode:04o} is writable beyond its owner" + ) + + if event.digest is not None: + first = self._digests.setdefault(event.digest, event.path) + if first != event.path and event.inode is not None: + self._fault(event, "duplicate-content", f"identical bytes already at {first}") + + if len(event.steps) >= 2 and not source: + shared = set.intersection( + *(set(self._declared.get(step, frozenset())) for step in event.steps) + ) + if not shared: + self._fault( + event, + "undeclared-contention", + "two steps the scheduler ran together both touched this, and neither " + "declares sharing it", + ) + + def is_source(self, path: Path) -> bool: + from .faults import BUILD_OUTPUT + + try: + relative = path.resolve().relative_to(self._source_root) + except (ValueError, OSError): + return False + return bool(relative.parts) and relative.parts[0] not in BUILD_OUTPUT + + # -- state, not moments ------------------------------------------------- + + def survey(self) -> None: + """Report what is already wrong before this run touched anything. + + A hardlink made by a previous run produces no event in this one, and + the defect is in the state: the shared inode is there whether or not + anybody relinks it today. Without this the check fires only on the run + that happens to rebuild, which is a coincidence rather than a guard. + """ + for root in self._roots: + if self.is_source(root): + continue + for path in root.rglob("*"): + try: + info = path.stat() + except OSError: + continue + if not stat.S_ISREG(info.st_mode) or info.st_nlink < 2: + continue + origin = self._source_inodes.get(info.st_ino) + if origin is None: + continue + self.observed("pre-existing", path) + + def sweep(self) -> list[Fault]: + """What only the end of a run can decide. + + An artifact that is empty once everything has finished is a failed + build that reported success; during the run it is a file being written. + """ + for path in {event.path for event in self.events if not self.is_source(event.path)}: + try: + if path.is_file() and path.stat().st_size == 0: + self._fault( + Event(at=time.time(), kind="final", path=path, steps=()), + "empty-artifact", + "zero bytes at the end of the run", + ) + except OSError: + continue + return self.faults diff --git a/src/capsem/gate/observing.py b/src/capsem/gate/observing.py new file mode 100644 index 000000000..e777ebec9 --- /dev/null +++ b/src/capsem/gate/observing.py @@ -0,0 +1,69 @@ +"""Wiring the run's filesystem observer into the funnel. + +Its own module because `command.py` is the enforcing funnel and every line +added there is a line every command pays for. Three destinations, because they +answer different questions and the first two were the ones missing: `stderr` +so a developer sees the fault at the minute it occurs rather than at the end +of an hour, a size-capped log beside the run so a killed gate still leaves the +report, and the journal so `runs` can answer it later. +""" + +from __future__ import annotations + +import sys +from collections.abc import Iterator +from contextlib import contextmanager + +from .config import GateConfig +from .faultlog import FaultLog +from .faults import Fault +from .observation import Watch +from .plan import Plan + + +@contextmanager +def observing(config: GateConfig, log: object, plan: Plan) -> Iterator[Watch | None]: + """Watch the disk for the length of a run, reporting faults as they land.""" + from .interception import Instrument + + # Only a real run has somewhere to put the report. A test driving the + # funnel with a recording journal is not being audited, and inventing a + # directory for it would drop fault files into the checkout. + directory = getattr(log, "directory", None) + if directory is None: + yield None + return + + settings = config.runlog + errors = FaultLog( + directory / settings.error_log, + max_bytes=settings.error_log_max_bytes, + keep=settings.error_log_keep, + ) + seen: list[Fault] = [] + + def report(fault: Fault) -> None: + seen.append(fault) + print(f"FAULT {fault.render()}", file=sys.stderr, flush=True) + errors(fault) + note = getattr(log, "note", None) + if note is not None: + note(f"fault {fault.reason}: {fault.path}") + + declared = { + step.label: frozenset(resource.name for resource in step.contends) for step in plan.steps + } + roots = [config.path(name) for name in settings.observed_roots] + try: + with Watch(roots, source_root=config.root, declared=declared, on_fault=report) as watch: + with Instrument(watch): + yield watch + watch.sweep() + finally: + errors.close() + if seen: + print( + f"{len(seen)} filesystem fault(s) -- see {errors.path}", + file=sys.stderr, + flush=True, + ) diff --git a/src/capsem/gate/planrunner.py b/src/capsem/gate/planrunner.py index d0e9e54e2..3e475675c 100644 --- a/src/capsem/gate/planrunner.py +++ b/src/capsem/gate/planrunner.py @@ -200,6 +200,11 @@ def _guarded(pending: _Pending, context: Context, abandoned: threading.Event) -> pending.started = time.monotonic() with observing(abandoned), ExitStack() as stack: stack.enter_context(context.journal.step(pending.step)) + # Whoever is in flight owns whatever the disk does next. Attribution + # has to come from the scheduler: it is the only thing that knows. + if context.watch is not None: + context.watch.entered(pending.step.label) + stack.callback(context.watch.left, pending.step.label) pending.step.run(context) return time.monotonic() - pending.started diff --git a/tests/test_gate_observation.py b/tests/test_gate_observation.py new file mode 100644 index 000000000..d4b2295cb --- /dev/null +++ b/tests/test_gate_observation.py @@ -0,0 +1,382 @@ +"""What the gate actually did to the filesystem, asserted while it happens. + +`contends` is a list somebody typed, and `can_overlap` compares two such lists +to each other. Nothing in that loop has ever looked at a disk, so a step that +does not mention what it touches satisfies every check by saying nothing -- +and the writer is usually not the step at all but a unit test three +subprocesses down. + +That gap cost two hours of release runs. `rust-coverage` runs the capsem-admin +suite, which builds release channels from the real `config/` tree and hardlinks +those checked-in files into `target/`. `linux-rust` is a container with the +same tree bind-mounted read-only over virtiofs. The scheduler ran them together +because their declarations were disjoint, and the container got an intermittent +`Permission denied` on a file that was `0644` before and `0644` after. + +Two things follow, and each has tests here. The fault must be reported *as it +happens*, not left in a file for someone to analyze later. And the report must +carry what the fault is made of -- mode, size, inode, link count -- because +those are what distinguish a hardlinked source file from a copy, and a +flip-flop from a change. +""" + +from __future__ import annotations + +import os +import shutil +import time +from pathlib import Path + +from capsem.gate.faultlog import FaultLog +from capsem.gate.faults import Event, Facts, Fault +from capsem.gate.observation import Watch + +PROJECT_ROOT = Path(__file__).resolve().parents[1] + + +def _settle(watch: Watch, count: int, timeout: float = 5.0) -> None: + """Wait for delivery rather than guessing with a sleep. + + FSEvents coalesces on its own schedule; a fixed sleep is either slow or + flaky, and a test written because of a race may not introduce one. + """ + deadline = time.monotonic() + timeout + while time.monotonic() < deadline and len(watch.events) < count: + time.sleep(0.02) + + +def _watch(root: Path, **kwargs) -> Watch: + return Watch([root], source_root=root, **kwargs) + + +# --------------------------------------------------------------------------- +# Reported as it happens +# --------------------------------------------------------------------------- + + +def test_a_fault_is_emitted_when_it_happens_not_at_the_end(tmp_path: Path) -> None: + """The difference between a check and a log. + + A rule evaluated after the run is acted on when someone thinks to read a + file, which for a sixty-minute gate means the fault is found at minute + sixty or never. This asserts the callback has already fired while the run + is still going. + """ + (tmp_path / "config").mkdir() + seen: list[Fault] = [] + + with _watch(tmp_path, on_fault=seen.append) as watch: + watch.entered("suite") + (tmp_path / "config" / "profile.toml").write_text("x", encoding="utf-8") + _settle(watch, 1) + # Still inside the run: no sweep, no exit, no analysis pass. + assert seen, "the fault was queued for later instead of raised now" + assert seen[0].reason == "source-tree" + watch.left("suite") + + +def test_the_error_log_survives_a_run_that_is_killed(tmp_path: Path) -> None: + """Line-buffered and fsynced, because the run being described may not + exit cleanly -- and that is when the report matters most.""" + log_path = tmp_path / "errors.log" + log = FaultLog(log_path, max_bytes=1 << 20, keep=3) + log(Fault(path=Path("/repo/config/x"), steps=("a",), reason="source-tree", detail="written")) + + # Read without closing: exactly what a killed run leaves behind. + assert "source-tree" in log_path.read_text(encoding="utf-8") + assert "/repo/config/x" in log_path.read_text(encoding="utf-8") + log.close() + + +# --------------------------------------------------------------------------- +# What the fault is made of +# --------------------------------------------------------------------------- + + +def test_a_transient_mode_change_is_seen_even_though_it_reverts(tmp_path: Path) -> None: + """The failure that started this. A mode dropped and restored inside one + step leaves the file exactly as found, so any before/after comparison + reports nothing happened -- while a concurrent reader gets `Permission + denied` and the gate blames the environment.""" + (tmp_path / "config").mkdir() + target = tmp_path / "config" / "seed.json" + target.write_text("{}", encoding="utf-8") + before = target.stat().st_mode & 0o777 + + with _watch(tmp_path) as watch: + watch.entered("suite") + _settle(watch, 1) + target.chmod(0o000) + _settle(watch, 2) + target.chmod(before) + _settle(watch, 3) + watch.left("suite") + + assert target.stat().st_mode & 0o777 == before, "the test must leave no trace" + assert any(event.path == target for event in watch.events), ( + "a change that reverts within one step went unobserved" + ) + # What is *guaranteed*: the file was touched, and touching checked-in + # source during a run is itself the fault. Naming the intermediate mode is + # best-effort and deliberately not asserted here -- FSEvents notifies and + # we `stat` afterwards, so a mode restored inside that window is already + # gone when we look. Claiming otherwise would be a guard that passes on a + # fixture and misses the thing it was built for. + assert any(fault.reason == "source-tree" for fault in watch.faults), ( + f"got {[f.reason for f in watch.faults]}" + ) + + +def test_a_mode_that_returns_to_a_previous_value_is_a_flip_flop() -> None: + """The rule itself, driven directly. + + Live detection depends on winning a race against the restore, so the rule + is proven here where the modes are known, and treated as opportunistic in + the field. + """ + watch = Watch([], source_root=Path("/repo")) + path = Path("/repo/target/seed.json") + for mode in (0o644, 0o000, 0o644): + watch._judge(Event(at=1.0, kind="modified", path=path, steps=(), facts=Facts(mode=mode))) + + flip = [fault for fault in watch.faults if fault.reason == "mode-flip-flop"] + assert flip, [fault.reason for fault in watch.faults] + assert "0000 -> 0644" in flip[0].render() + + +def test_a_hardlink_into_build_output_is_named_where_it_lands(tmp_path: Path) -> None: + """The actual bug, and the reason the first version of this module could + not have found it. + + Hardlinking `config/x` to `target/y` creates a directory entry in + `target/` and leaves `config/` untouched -- no event fires there, ever. + Watching the source tree is structurally blind to it. The only trace is + that the new file's inode is one a checked-in file already owns, which is + decidable from one `stat` and needs no concurrency at all. + """ + import subprocess + + subprocess.run(["git", "init", "-q"], cwd=tmp_path, check=True) + (tmp_path / "config").mkdir() + seed = tmp_path / "config" / "projects.json" + seed.write_text("{}", encoding="utf-8") + subprocess.run(["git", "add", "-A"], cwd=tmp_path, check=True) + + output = tmp_path / "target" / "release" + output.mkdir(parents=True) + staged = output / "root-payload-abc" + + live: list[Fault] = [] + with Watch([tmp_path / "target"], source_root=tmp_path, on_fault=live.append) as watch: + watch.entered("contracts.release") + os.link(seed, staged) + _settle(watch, 1) + watch.left("contracts.release") + + hardlink = [fault for fault in watch.faults if fault.reason == "hardlinked-source"] + assert hardlink, ( + f"the staged payload shares an inode with checked-in source and went " + f"unnamed; saw {[(f.reason, f.path.name) for f in watch.faults]}" + ) + assert "config/projects.json" in hardlink[0].render() + assert hardlink[0] in live, "found only after the fact, not while it happened" + + +def test_source_writable_beyond_its_owner_is_named() -> None: + watch = Watch([], source_root=Path("/repo")) + watch._judge( + Event( + at=1.0, + kind="modified", + path=Path("/repo/scripts/x.sh"), + steps=(), + facts=Facts(mode=0o666), + ) + ) + assert "over-permission" in {fault.reason for fault in watch.faults} + + +def test_two_overlapping_steps_touching_one_build_path_is_named() -> None: + """Build output, because a source path is the graver finding and is + reported as that instead.""" + watch = Watch([], source_root=Path("/repo"), declared={"a": frozenset(), "b": frozenset()}) + watch._judge( + Event(at=1.0, kind="modified", path=Path("/repo/target/store.db"), steps=("a", "b")) + ) + assert "undeclared-contention" in {fault.reason for fault in watch.faults} + + +def test_a_declared_shared_resource_is_not_a_fault() -> None: + """Otherwise every legitimately shared lane reds and the check gets muted, + which is how a check stops being one.""" + watch = Watch( + [], + source_root=Path("/repo"), + declared={"a": frozenset({"asset_tree"}), "b": frozenset({"asset_tree"})}, + ) + watch._judge( + Event(at=1.0, kind="modified", path=Path("/repo/target/assets/x"), steps=("a", "b")) + ) + assert watch.faults == [] + + +def test_build_output_is_not_the_checked_in_tree() -> None: + watch = Watch([], source_root=Path("/repo")) + watch._judge(Event(at=1.0, kind="modified", path=Path("/repo/target/x"), steps=("build",))) + assert watch.faults == [] + + +def test_an_empty_artifact_is_only_decidable_at_the_end(tmp_path: Path) -> None: + """Mid-run it is a file being written; at the end it is a build that + reported success and produced nothing.""" + target = tmp_path / "target" + target.mkdir() + artifact = target / "capsem.pkg" + artifact.write_bytes(b"") + + watch = Watch([], source_root=tmp_path) + watch.events.append(Event(at=1.0, kind="modified", path=artifact, steps=("package",))) + assert "empty-artifact" in {fault.reason for fault in watch.sweep()} + + +def test_identical_bytes_under_two_names_are_named(tmp_path: Path) -> None: + watch = Watch([], source_root=tmp_path) + for name in ("one", "two"): + watch._judge( + Event( + at=1.0, + kind="modified", + path=tmp_path / "target" / name, + steps=(), + facts=Facts(inode=1 if name == "one" else 2, digest="deadbeef"), + ) + ) + assert "duplicate-content" in {fault.reason for fault in watch.faults} + + +# --------------------------------------------------------------------------- +# Observable by construction +# --------------------------------------------------------------------------- + + +def test_interception_sees_a_hardlink_with_no_watcher_at_all(tmp_path: Path) -> None: + """No FSEvents, no coalescing, no notification latency, no polling. + + The call itself is the observation, so this holds for a path nobody + thought to watch -- which is the whole point: the previous design could + only see roots someone remembered to list. + """ + import subprocess + + from capsem.gate.interception import CURRENT_STEP, Instrument + + subprocess.run(["git", "init", "-q"], cwd=tmp_path, check=True) + (tmp_path / "config").mkdir() + seed = tmp_path / "config" / "seed.json" + seed.write_text("{}", encoding="utf-8") + subprocess.run(["git", "add", "-A"], cwd=tmp_path, check=True) + + watch = Watch([], source_root=tmp_path) + token = CURRENT_STEP.set("contracts.release") + try: + with Instrument(watch): + (tmp_path / "target").mkdir() + os.link(seed, tmp_path / "target" / "staged-payload") + finally: + CURRENT_STEP.reset(token) + + hardlink = [fault for fault in watch.faults if fault.reason == "hardlinked-source"] + assert hardlink, [f.reason for f in watch.faults] + assert hardlink[0].steps == ("contracts.release",), ( + "the caller is known exactly, not narrowed to whatever was in flight" + ) + + +def test_interception_catches_the_mode_that_a_watcher_arrives_too_late_for( + tmp_path: Path, +) -> None: + """The flip-flop, decided rather than raced for. + + An external watcher is notified and then stats, by which time the restore + has happened and both samples read `0644`. Wrapping `chmod` has the old + mode in hand before the call returns. + """ + from capsem.gate.interception import Instrument + + target = tmp_path / "target" / "artifact" + target.parent.mkdir() + target.write_text("x", encoding="utf-8") + target.chmod(0o644) + + watch = Watch([], source_root=tmp_path) + with Instrument(watch): + os.chmod(target, 0o000) + os.chmod(target, 0o644) + + assert target.stat().st_mode & 0o777 == 0o644, "left exactly as found" + assert "mode-flip-flop" in {fault.reason for fault in watch.faults}, [ + f.reason for f in watch.faults + ] + + +def test_every_mutating_primitive_the_stdlib_offers_is_intercepted() -> None: + """Read against the module, so the list cannot quietly fall behind. + + A primitive added to the codebase tomorrow and not added here is exactly + how "observable by construction" decays back into "observable if someone + remembered", which is the failure this replaced. + """ + import shutil as _shutil + + from capsem.gate.interception import Instrument + + intercepted = {(module, name) for module, name, _ in Instrument.TARGETS} + mutating = { + (os, "link"), + (os, "symlink"), + (os, "unlink"), + (os, "remove"), + (os, "rmdir"), + (os, "chmod"), + (os, "rename"), + (os, "replace"), + (os, "truncate"), + (_shutil, "copy"), + (_shutil, "copy2"), + (_shutil, "copyfile"), + (_shutil, "copytree"), + (_shutil, "rmtree"), + (_shutil, "move"), + } + assert mutating <= intercepted, mutating - intercepted + + +def test_the_primitives_are_restored_afterwards() -> None: + """A gate that leaves the standard library patched has broken every + process that outlives it.""" + from capsem.gate.interception import Instrument + + before = (os.link, os.chmod, shutil.copytree) + with Instrument(Watch([], source_root=Path("/repo"))): + assert os.link is not before[0], "not actually patched" + assert (os.link, os.chmod, shutil.copytree) == before + + +def test_the_fault_log_is_bounded(tmp_path: Path) -> None: + """A run that trips one rule per file trips it thousands of times. + + Unbounded, this is a disk-full outage wearing a helpful name -- which is + why the cap is configured rather than assumed, and why the *newest* + faults survive: they describe the failure being looked at. + """ + log_path = tmp_path / "errors.log" + log = FaultLog(log_path, max_bytes=512, keep=2) + for index in range(400): + log(Fault(path=Path(f"/repo/target/{index}"), steps=(), reason="x", detail="y" * 40)) + log.close() + + generations = sorted(tmp_path.glob("errors.log*")) + assert len(generations) <= 3, generations + total = sum(path.stat().st_size for path in generations) + assert total <= 512 * 3, f"{total} bytes across {generations}" + assert "/repo/target/399" in log_path.read_text(encoding="utf-8"), "newest fault was dropped" diff --git a/tests/test_gate_primitives_are_the_only_way.py b/tests/test_gate_primitives_are_the_only_way.py index f135cbb2a..56841022f 100644 --- a/tests/test_gate_primitives_are_the_only_way.py +++ b/tests/test_gate_primitives_are_the_only_way.py @@ -114,6 +114,18 @@ def test_the_permitted_modules_are_the_ones_that_have_to_be() -> None: reclaims every other tree the gate creates, and `workspace` owns the isolated home the actions run against. + The observation four are the deliberate widening. They are here for the + inverse of the usual reason: the others own machine state, these *watch* + it. `faults` stats and hashes what changed, `faultlog` writes and fsyncs + the report so a killed run still leaves one, `observation` judges each + change as it lands, and `interception` is the primitives proxied -- its + entire purpose is that nothing reaches `os` without passing through it, + which routing through an action would defeat rather than express. + + They were added after a release run died reading a file that was `0644` + before and `0644` after, because nothing in the gate was in the path of + the call that changed it. + The through-line is that these are the harness, and the harness is what gate work is expressed *in*. A capability or a command appearing here would mean work that the dry run cannot show and the log cannot time. @@ -131,6 +143,10 @@ def test_the_permitted_modules_are_the_ones_that_have_to_be() -> None: "runhistory.py", "disk.py", "workspace.py", + "faults.py", + "faultlog.py", + "interception.py", + "observation.py", } diff --git a/tests/test_rust_filesystem_chokepoint.py b/tests/test_rust_filesystem_chokepoint.py new file mode 100644 index 000000000..b8c0ed024 --- /dev/null +++ b/tests/test_rust_filesystem_chokepoint.py @@ -0,0 +1,75 @@ +"""Hardlinking from Rust goes through one audited place. + +Python's primitives are proxied (`capsem.gate.observation.Instrument`), so +every in-process effect is observable by construction rather than by +remembering to log it. Rust cannot be monkeypatched; the equivalent is a +chokepoint plus this test. + +**Why this is scoped to `hard_link` and not to mutation generally.** The first +version forbade every `fs::` mutation outside an audited module and found 259 +call sites -- most of them runtime service code removing its own sockets and +session directories, which has nothing to do with what a release qualifies. +Routing all of that through an audit layer would have been a large refactor +bought with no safety. Narrowing afterwards to whatever made the test pass +would have been worse: a guard shaped around its own result. + +So the invariant is the one that actually failed. A hardlink is the only +operation that makes two paths *the same file*, and there are exactly two in +the workspace. `capsem-admin` used one to stage profile payloads and put 48 +checked-in `config/` files inside published release output -- one inode, so a +chmod on the artifact rewrites tracked source and no content digest notices. +Auditing every hardlink is cheap precisely because hardlinks are rare, and it +closes the class completely. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +PROJECT_ROOT = Path(__file__).resolve().parents[1] + +#: The module that owns linking, and may call the primitive. +AUDITED = "crates/capsem-core/src/auditfs.rs" + +#: Named, not silent. The guest's own filesystem inside its own share: the +#: guest issues the link, the host is implementing a syscall for it, and no +#: release artifact is involved. An exemption nobody can see is how a +#: chokepoint stops being one. +EXEMPT = {"crates/capsem-core/src/hypervisor/kvm/virtio_fs/ops_dir.rs"} + +LINK = re.compile(r"\bfs::hard_link\s*\(") + + +def _rust_sources() -> list[Path]: + return [ + path + for path in (PROJECT_ROOT / "crates").rglob("*.rs") + if "target" not in path.parts and not path.name.endswith("tests.rs") + ] + + +def test_hardlinking_goes_through_the_audited_module() -> None: + """Two paths becoming one file is worth one place in the codebase.""" + offenders: list[str] = [] + for path in _rust_sources(): + relative = path.relative_to(PROJECT_ROOT).as_posix() + if relative == AUDITED or relative in EXEMPT: + continue + for number, line in enumerate(path.read_text(encoding="utf-8").splitlines(), 1): + if LINK.search(line): + offenders.append(f"{relative}:{number}: {line.strip()}") + + assert not offenders, ( + f"{len(offenders)} unaudited hardlink(s). A hardlink makes two paths the " + "same file, which is how checked-in source ended up inside published " + "release output. Route them through capsem_core::auditfs:\n " + + "\n ".join(offenders) + ) + + +def test_the_exemptions_still_exist() -> None: + """An allowlist that outlives the code it names silently re-opens the hole + it was granted for.""" + for relative in EXEMPT: + assert (PROJECT_ROOT / relative).is_file(), f"{relative} is exempt but gone" diff --git a/uv.lock b/uv.lock index c7415c584..118e1b369 100644 --- a/uv.lock +++ b/uv.lock @@ -143,6 +143,7 @@ dev = [ { name = "pyyaml" }, { name = "ruff" }, { name = "ty" }, + { name = "watchdog" }, { name = "websockets" }, ] @@ -165,6 +166,7 @@ dev = [ { name = "pyyaml", specifier = ">=6.0.3" }, { name = "ruff", specifier = ">=0.15.16" }, { name = "ty", specifier = ">=0.0.46" }, + { name = "watchdog", specifier = ">=6.0.0" }, { name = "websockets", specifier = ">=16.0" }, ] @@ -1173,6 +1175,33 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/7f/3e/5db95bcf282c52709639744ca2a8b149baccf648e39c8cc87553df9eae0c/urllib3-2.7.0-py3-none-any.whl", hash = "sha256:9fb4c81ebbb1ce9531cce37674bbc6f1360472bc18ca9a553ede278ef7276897", size = 131087, upload-time = "2026-05-07T16:13:17.151Z" }, ] +[[package]] +name = "watchdog" +version = "6.0.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/db/7d/7f3d619e951c88ed75c6037b246ddcf2d322812ee8ea189be89511721d54/watchdog-6.0.0.tar.gz", hash = "sha256:9ddf7c82fda3ae8e24decda1338ede66e1c99883db93711d8fb941eaa2d8c282", size = 131220, upload-time = "2024-11-01T14:07:13.037Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/e0/24/d9be5cd6642a6aa68352ded4b4b10fb0d7889cb7f45814fb92cecd35f101/watchdog-6.0.0-cp311-cp311-macosx_10_9_universal2.whl", hash = "sha256:6eb11feb5a0d452ee41f824e271ca311a09e250441c262ca2fd7ebcf2461a06c", size = 96393, upload-time = "2024-11-01T14:06:31.756Z" }, + { url = "https://files.pythonhosted.org/packages/63/7a/6013b0d8dbc56adca7fdd4f0beed381c59f6752341b12fa0886fa7afc78b/watchdog-6.0.0-cp311-cp311-macosx_10_9_x86_64.whl", hash = "sha256:ef810fbf7b781a5a593894e4f439773830bdecb885e6880d957d5b9382a960d2", size = 88392, upload-time = "2024-11-01T14:06:32.99Z" }, + { url = "https://files.pythonhosted.org/packages/d1/40/b75381494851556de56281e053700e46bff5b37bf4c7267e858640af5a7f/watchdog-6.0.0-cp311-cp311-macosx_11_0_arm64.whl", hash = "sha256:afd0fe1b2270917c5e23c2a65ce50c2a4abb63daafb0d419fde368e272a76b7c", size = 89019, upload-time = "2024-11-01T14:06:34.963Z" }, + { url = "https://files.pythonhosted.org/packages/39/ea/3930d07dafc9e286ed356a679aa02d777c06e9bfd1164fa7c19c288a5483/watchdog-6.0.0-cp312-cp312-macosx_10_13_universal2.whl", hash = "sha256:bdd4e6f14b8b18c334febb9c4425a878a2ac20efd1e0b231978e7b150f92a948", size = 96471, upload-time = "2024-11-01T14:06:37.745Z" }, + { url = "https://files.pythonhosted.org/packages/12/87/48361531f70b1f87928b045df868a9fd4e253d9ae087fa4cf3f7113be363/watchdog-6.0.0-cp312-cp312-macosx_10_13_x86_64.whl", hash = "sha256:c7c15dda13c4eb00d6fb6fc508b3c0ed88b9d5d374056b239c4ad1611125c860", size = 88449, upload-time = "2024-11-01T14:06:39.748Z" }, + { url = "https://files.pythonhosted.org/packages/5b/7e/8f322f5e600812e6f9a31b75d242631068ca8f4ef0582dd3ae6e72daecc8/watchdog-6.0.0-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:6f10cb2d5902447c7d0da897e2c6768bca89174d0c6e1e30abec5421af97a5b0", size = 89054, upload-time = "2024-11-01T14:06:41.009Z" }, + { url = "https://files.pythonhosted.org/packages/68/98/b0345cabdce2041a01293ba483333582891a3bd5769b08eceb0d406056ef/watchdog-6.0.0-cp313-cp313-macosx_10_13_universal2.whl", hash = "sha256:490ab2ef84f11129844c23fb14ecf30ef3d8a6abafd3754a6f75ca1e6654136c", size = 96480, upload-time = "2024-11-01T14:06:42.952Z" }, + { url = "https://files.pythonhosted.org/packages/85/83/cdf13902c626b28eedef7ec4f10745c52aad8a8fe7eb04ed7b1f111ca20e/watchdog-6.0.0-cp313-cp313-macosx_10_13_x86_64.whl", hash = "sha256:76aae96b00ae814b181bb25b1b98076d5fc84e8a53cd8885a318b42b6d3a5134", size = 88451, upload-time = "2024-11-01T14:06:45.084Z" }, + { url = "https://files.pythonhosted.org/packages/fe/c4/225c87bae08c8b9ec99030cd48ae9c4eca050a59bf5c2255853e18c87b50/watchdog-6.0.0-cp313-cp313-macosx_11_0_arm64.whl", hash = "sha256:a175f755fc2279e0b7312c0035d52e27211a5bc39719dd529625b1930917345b", size = 89057, upload-time = "2024-11-01T14:06:47.324Z" }, + { url = "https://files.pythonhosted.org/packages/a9/c7/ca4bf3e518cb57a686b2feb4f55a1892fd9a3dd13f470fca14e00f80ea36/watchdog-6.0.0-py3-none-manylinux2014_aarch64.whl", hash = "sha256:7607498efa04a3542ae3e05e64da8202e58159aa1fa4acddf7678d34a35d4f13", size = 79079, upload-time = "2024-11-01T14:06:59.472Z" }, + { url = "https://files.pythonhosted.org/packages/5c/51/d46dc9332f9a647593c947b4b88e2381c8dfc0942d15b8edc0310fa4abb1/watchdog-6.0.0-py3-none-manylinux2014_armv7l.whl", hash = "sha256:9041567ee8953024c83343288ccc458fd0a2d811d6a0fd68c4c22609e3490379", size = 79078, upload-time = "2024-11-01T14:07:01.431Z" }, + { url = "https://files.pythonhosted.org/packages/d4/57/04edbf5e169cd318d5f07b4766fee38e825d64b6913ca157ca32d1a42267/watchdog-6.0.0-py3-none-manylinux2014_i686.whl", hash = "sha256:82dc3e3143c7e38ec49d61af98d6558288c415eac98486a5c581726e0737c00e", size = 79076, upload-time = "2024-11-01T14:07:02.568Z" }, + { url = "https://files.pythonhosted.org/packages/ab/cc/da8422b300e13cb187d2203f20b9253e91058aaf7db65b74142013478e66/watchdog-6.0.0-py3-none-manylinux2014_ppc64.whl", hash = "sha256:212ac9b8bf1161dc91bd09c048048a95ca3a4c4f5e5d4a7d1b1a7d5752a7f96f", size = 79077, upload-time = "2024-11-01T14:07:03.893Z" }, + { url = "https://files.pythonhosted.org/packages/2c/3b/b8964e04ae1a025c44ba8e4291f86e97fac443bca31de8bd98d3263d2fcf/watchdog-6.0.0-py3-none-manylinux2014_ppc64le.whl", hash = "sha256:e3df4cbb9a450c6d49318f6d14f4bbc80d763fa587ba46ec86f99f9e6876bb26", size = 79078, upload-time = "2024-11-01T14:07:05.189Z" }, + { url = "https://files.pythonhosted.org/packages/62/ae/a696eb424bedff7407801c257d4b1afda455fe40821a2be430e173660e81/watchdog-6.0.0-py3-none-manylinux2014_s390x.whl", hash = "sha256:2cce7cfc2008eb51feb6aab51251fd79b85d9894e98ba847408f662b3395ca3c", size = 79077, upload-time = "2024-11-01T14:07:06.376Z" }, + { url = "https://files.pythonhosted.org/packages/b5/e8/dbf020b4d98251a9860752a094d09a65e1b436ad181faf929983f697048f/watchdog-6.0.0-py3-none-manylinux2014_x86_64.whl", hash = "sha256:20ffe5b202af80ab4266dcd3e91aae72bf2da48c0d33bdb15c66658e685e94e2", size = 79078, upload-time = "2024-11-01T14:07:07.547Z" }, + { url = "https://files.pythonhosted.org/packages/07/f6/d0e5b343768e8bcb4cda79f0f2f55051bf26177ecd5651f84c07567461cf/watchdog-6.0.0-py3-none-win32.whl", hash = "sha256:07df1fdd701c5d4c8e55ef6cf55b8f0120fe1aef7ef39a1c6fc6bc2e606d517a", size = 79065, upload-time = "2024-11-01T14:07:09.525Z" }, + { url = "https://files.pythonhosted.org/packages/db/d9/c495884c6e548fce18a8f40568ff120bc3a4b7b99813081c8ac0c936fa64/watchdog-6.0.0-py3-none-win_amd64.whl", hash = "sha256:cbafb470cf848d93b5d013e2ecb245d4aa1c8fd0504e863ccefa32445359d680", size = 79070, upload-time = "2024-11-01T14:07:10.686Z" }, + { url = "https://files.pythonhosted.org/packages/33/e8/e40370e6d74ddba47f002a32919d91310d6074130fe4e17dabcafc15cbf1/watchdog-6.0.0-py3-none-win_ia64.whl", hash = "sha256:a1914259fa9e1454315171103c6a30961236f508b9b623eae470268bbcc6a22f", size = 79067, upload-time = "2024-11-01T14:07:11.845Z" }, +] + [[package]] name = "websockets" version = "16.0" From c43aa7568a251def8fb187297a82253507c1072e Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Mon, 3 Aug 2026 15:59:25 -0400 Subject: [PATCH 02/18] test(gate): inventory every Rust test target --- src/capsem/gate/rustinventory.py | 132 +++++++++++++ tests/fixtures/rust-test-inventory/Cargo.lock | 7 + tests/fixtures/rust-test-inventory/Cargo.toml | 12 ++ .../rust-test-inventory/examples/excluded.rs | 6 + .../src/bin/fixture_bin/main.rs | 4 + .../src/bin/fixture_bin/tests.rs | 4 + tests/fixtures/rust-test-inventory/src/lib.rs | 13 ++ .../fixtures/rust-test-inventory/src/tests.rs | 18 ++ .../rust-test-inventory/tests/integration.rs | 4 + tests/test_rust_test_inventory.py | 176 ++++++++++++++++++ 10 files changed, 376 insertions(+) create mode 100644 src/capsem/gate/rustinventory.py create mode 100644 tests/fixtures/rust-test-inventory/Cargo.lock create mode 100644 tests/fixtures/rust-test-inventory/Cargo.toml create mode 100644 tests/fixtures/rust-test-inventory/examples/excluded.rs create mode 100644 tests/fixtures/rust-test-inventory/src/bin/fixture_bin/main.rs create mode 100644 tests/fixtures/rust-test-inventory/src/bin/fixture_bin/tests.rs create mode 100644 tests/fixtures/rust-test-inventory/src/lib.rs create mode 100644 tests/fixtures/rust-test-inventory/src/tests.rs create mode 100644 tests/fixtures/rust-test-inventory/tests/integration.rs create mode 100644 tests/test_rust_test_inventory.py diff --git a/src/capsem/gate/rustinventory.py b/src/capsem/gate/rustinventory.py new file mode 100644 index 000000000..5fe7afd8c --- /dev/null +++ b/src/capsem/gate/rustinventory.py @@ -0,0 +1,132 @@ +"""Typed Rust test-target inventory shared by gate guards and runners. + +Cargo owns target declarations. Nextest owns the executable test inventory. +Normalizing both sources to :class:`RustTarget` lets the gate compare identities +instead of grepping command text or trusting a test count. +""" + +from __future__ import annotations + +from collections.abc import Iterable + +from pydantic import BaseModel, ConfigDict, Field + +_NATIVE_KINDS = frozenset({"bin", "lib", "proc-macro", "test"}) + + +class InventoryMismatch(ValueError): + """Cargo and Nextest disagree about the native targets being tested.""" + + +class RustTarget(BaseModel): + """One testable Cargo target, independent of tool-specific identifiers.""" + + model_config = ConfigDict(frozen=True) + + package: str + name: str + kind: str + + def render(self) -> str: + """Return a stable human identity for diagnostics and run evidence.""" + return f"{self.package}:{self.kind}/{self.name}" + + +class RustTestInventory(BaseModel): + """Native and doctest targets observed from one inventory source.""" + + model_config = ConfigDict(frozen=True) + + native: frozenset[RustTarget] = frozenset() + doctest: frozenset[RustTarget] = frozenset() + + @classmethod + def from_cargo_metadata(cls, payload: object) -> RustTestInventory: + """Normalize Cargo metadata's declared test and doctest targets.""" + metadata = _CargoMetadata.model_validate(payload) + native: set[RustTarget] = set() + doctest: set[RustTarget] = set() + + for package in metadata.packages: + for target in package.targets: + kind = _one_kind(target.kind, package=package.name, target=target.name) + identity = RustTarget(package=package.name, name=target.name, kind=kind) + if target.test and kind in _NATIVE_KINDS: + native.add(identity) + if target.doctest and kind in _NATIVE_KINDS: + doctest.add(identity) + + return cls(native=frozenset(native), doctest=frozenset(doctest)) + + @classmethod + def from_nextest_list(cls, payload: object) -> RustTestInventory: + """Normalize Nextest's listed suites; Nextest never owns doctests.""" + listing = _NextestList.model_validate(payload) + native = { + RustTarget(package=suite.package_name, name=suite.binary_name, kind=suite.kind) + for suite in listing.rust_suites.values() + if suite.status == "listed" and suite.kind in _NATIVE_KINDS + } + return cls(native=frozenset(native)) + + def require_same_native_targets(self, nextest: RustTestInventory) -> None: + """Fail with exact identities when Nextest does not match Cargo.""" + missing = self.native - nextest.native + unexpected = nextest.native - self.native + if not missing and not unexpected: + return + + details: list[str] = [] + if missing: + details.append(f"missing from Nextest: {_render(missing)}") + if unexpected: + details.append(f"not declared testable by Cargo: {_render(unexpected)}") + raise InventoryMismatch("; ".join(details)) + + +class _CargoTarget(BaseModel): + model_config = ConfigDict(extra="ignore") + + name: str + kind: tuple[str, ...] + test: bool + doctest: bool + + +class _CargoPackage(BaseModel): + model_config = ConfigDict(extra="ignore") + + name: str + targets: tuple[_CargoTarget, ...] + + +class _CargoMetadata(BaseModel): + model_config = ConfigDict(extra="ignore") + + packages: tuple[_CargoPackage, ...] + + +class _NextestSuite(BaseModel): + model_config = ConfigDict(extra="ignore", populate_by_name=True) + + package_name: str = Field(alias="package-name") + binary_name: str = Field(alias="binary-name") + kind: str + status: str + + +class _NextestList(BaseModel): + model_config = ConfigDict(extra="ignore", populate_by_name=True) + + rust_suites: dict[str, _NextestSuite] = Field(alias="rust-suites") + + +def _one_kind(kinds: tuple[str, ...], *, package: str, target: str) -> str: + if len(kinds) != 1: + rendered = ", ".join(kinds) if kinds else "" + raise ValueError(f"Cargo target {package}:{target} has ambiguous kinds: {rendered}") + return kinds[0] + + +def _render(targets: Iterable[RustTarget]) -> str: + return ", ".join(sorted(target.render() for target in targets)) diff --git a/tests/fixtures/rust-test-inventory/Cargo.lock b/tests/fixtures/rust-test-inventory/Cargo.lock new file mode 100644 index 000000000..456a37e41 --- /dev/null +++ b/tests/fixtures/rust-test-inventory/Cargo.lock @@ -0,0 +1,7 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 4 + +[[package]] +name = "rust-test-inventory-fixture" +version = "0.0.0" diff --git a/tests/fixtures/rust-test-inventory/Cargo.toml b/tests/fixtures/rust-test-inventory/Cargo.toml new file mode 100644 index 000000000..196b87f13 --- /dev/null +++ b/tests/fixtures/rust-test-inventory/Cargo.toml @@ -0,0 +1,12 @@ +[package] +name = "rust-test-inventory-fixture" +version = "0.0.0" +edition = "2024" +publish = false + +[workspace] + +[[example]] +name = "excluded-example" +path = "examples/excluded.rs" +test = true diff --git a/tests/fixtures/rust-test-inventory/examples/excluded.rs b/tests/fixtures/rust-test-inventory/examples/excluded.rs new file mode 100644 index 000000000..e7c33e13a --- /dev/null +++ b/tests/fixtures/rust-test-inventory/examples/excluded.rs @@ -0,0 +1,6 @@ +fn main() {} + +#[test] +fn example_sentinel_must_remain_excluded() { + panic!("examples are not part of the native correctness inventory"); +} diff --git a/tests/fixtures/rust-test-inventory/src/bin/fixture_bin/main.rs b/tests/fixtures/rust-test-inventory/src/bin/fixture_bin/main.rs new file mode 100644 index 000000000..84b919c65 --- /dev/null +++ b/tests/fixtures/rust-test-inventory/src/bin/fixture_bin/main.rs @@ -0,0 +1,4 @@ +fn main() {} + +#[cfg(test)] +mod tests; diff --git a/tests/fixtures/rust-test-inventory/src/bin/fixture_bin/tests.rs b/tests/fixtures/rust-test-inventory/src/bin/fixture_bin/tests.rs new file mode 100644 index 000000000..2f2f67bc3 --- /dev/null +++ b/tests/fixtures/rust-test-inventory/src/bin/fixture_bin/tests.rs @@ -0,0 +1,4 @@ +#[test] +fn binary_sentinel() { + assert_eq!(2 + 2, 4); +} diff --git a/tests/fixtures/rust-test-inventory/src/lib.rs b/tests/fixtures/rust-test-inventory/src/lib.rs new file mode 100644 index 000000000..044fd07ec --- /dev/null +++ b/tests/fixtures/rust-test-inventory/src/lib.rs @@ -0,0 +1,13 @@ +//! Inventory fixture library. +//! +//! ``` +//! assert_eq!(rust_test_inventory_fixture::answer(), 42); +//! ``` + +#[must_use] +pub const fn answer() -> u8 { + 42 +} + +#[cfg(test)] +mod tests; diff --git a/tests/fixtures/rust-test-inventory/src/tests.rs b/tests/fixtures/rust-test-inventory/src/tests.rs new file mode 100644 index 000000000..c33f030de --- /dev/null +++ b/tests/fixtures/rust-test-inventory/src/tests.rs @@ -0,0 +1,18 @@ +use super::*; + +#[test] +fn library_sentinel() { + assert_eq!(answer(), 42); +} + +#[cfg(target_os = "macos")] +#[test] +fn macos_sentinel() { + assert_eq!(std::env::consts::OS, "macos"); +} + +#[cfg(target_os = "linux")] +#[test] +fn linux_sentinel() { + assert_eq!(std::env::consts::OS, "linux"); +} diff --git a/tests/fixtures/rust-test-inventory/tests/integration.rs b/tests/fixtures/rust-test-inventory/tests/integration.rs new file mode 100644 index 000000000..dba032ec7 --- /dev/null +++ b/tests/fixtures/rust-test-inventory/tests/integration.rs @@ -0,0 +1,4 @@ +#[test] +fn integration_sentinel() { + assert_eq!(rust_test_inventory_fixture::answer(), 42); +} diff --git a/tests/test_rust_test_inventory.py b/tests/test_rust_test_inventory.py new file mode 100644 index 000000000..3efa58e0d --- /dev/null +++ b/tests/test_rust_test_inventory.py @@ -0,0 +1,176 @@ +from __future__ import annotations + +import json +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +from capsem.gate.rustinventory import ( + InventoryMismatch, + RustTarget, + RustTestInventory, +) + +ROOT = Path(__file__).resolve().parent.parent +FIXTURE = ROOT / "tests" / "fixtures" / "rust-test-inventory" / "Cargo.toml" + + +def _cargo_env(target_dir: Path) -> dict[str, str]: + return {**os.environ, "CARGO_TARGET_DIR": str(target_dir)} + + +def _json_output(*argv: str, target_dir: Path) -> object: + result = subprocess.run( + argv, + cwd=ROOT, + env=_cargo_env(target_dir), + check=True, + capture_output=True, + text=True, + ) + return json.loads(result.stdout) + + +def _inventories( + target_dir: Path, *nextest_args: str +) -> tuple[RustTestInventory, RustTestInventory]: + metadata = _json_output( + "cargo", + "metadata", + "--format-version", + "1", + "--no-deps", + "--manifest-path", + str(FIXTURE), + target_dir=target_dir, + ) + nextest = _json_output( + "cargo", + "nextest", + "list", + "--manifest-path", + str(FIXTURE), + *nextest_args, + "--message-format", + "json", + target_dir=target_dir, + ) + return ( + RustTestInventory.from_cargo_metadata(metadata), + RustTestInventory.from_nextest_list(nextest), + ) + + +@pytest.fixture(scope="module") +def cargo_target_dir(tmp_path_factory: pytest.TempPathFactory) -> Path: + return tmp_path_factory.mktemp("rust-test-inventory") / "target" + + +def test_cargo_and_nextest_agree_on_every_native_target(cargo_target_dir: Path) -> None: + cargo, nextest = _inventories(cargo_target_dir) + + expected = frozenset( + { + RustTarget( + package="rust-test-inventory-fixture", + name="rust_test_inventory_fixture", + kind="lib", + ), + RustTarget( + package="rust-test-inventory-fixture", + name="fixture_bin", + kind="bin", + ), + RustTarget( + package="rust-test-inventory-fixture", + name="integration", + kind="test", + ), + } + ) + + assert cargo.native == expected + assert nextest.native == expected + cargo.require_same_native_targets(nextest) + + +def test_doctests_are_owned_separately_from_nextest(cargo_target_dir: Path) -> None: + cargo, nextest = _inventories(cargo_target_dir) + + library = RustTarget( + package="rust-test-inventory-fixture", + name="rust_test_inventory_fixture", + kind="lib", + ) + assert cargo.doctest == frozenset({library}) + assert nextest.doctest == frozenset() + + subprocess.run( + ["cargo", "test", "--doc", "--manifest-path", str(FIXTURE)], + cwd=ROOT, + env=_cargo_env(cargo_target_dir), + check=True, + capture_output=True, + text=True, + ) + + +def test_examples_are_not_native_correctness_targets(cargo_target_dir: Path) -> None: + cargo, nextest = _inventories(cargo_target_dir) + + assert all(target.kind != "example" for target in cargo.native) + assert all(target.kind != "example" for target in nextest.native) + + +def test_missing_target_fails_with_the_exact_identity(cargo_target_dir: Path) -> None: + cargo, nextest = _inventories(cargo_target_dir) + integration = next(target for target in nextest.native if target.kind == "test") + incomplete = nextest.model_copy(update={"native": nextest.native - {integration}}) + + with pytest.raises(InventoryMismatch, match="missing from Nextest") as failure: + cargo.require_same_native_targets(incomplete) + + assert integration.render() in str(failure.value) + + +def test_bins_only_selection_is_mechanically_rejected(cargo_target_dir: Path) -> None: + cargo, bins_only = _inventories(cargo_target_dir, "--bins") + + with pytest.raises(InventoryMismatch, match="missing from Nextest") as failure: + cargo.require_same_native_targets(bins_only) + + message = str(failure.value) + assert "rust-test-inventory-fixture:lib/rust_test_inventory_fixture" in message + assert "rust-test-inventory-fixture:test/integration" in message + + +def test_host_platform_sentinel_is_listed(cargo_target_dir: Path) -> None: + listing = subprocess.run( + [ + "cargo", + "nextest", + "list", + "--manifest-path", + str(FIXTURE), + "--message-format", + "json", + ], + cwd=ROOT, + env=_cargo_env(cargo_target_dir), + check=True, + capture_output=True, + text=True, + ) + suites = json.loads(listing.stdout)["rust-suites"] + cases = { + case + for suite in suites.values() + if suite["kind"] == "lib" + for case in suite["testcases"] + } + + host = "macos" if sys.platform == "darwin" else sys.platform + assert f"tests::{host}_sentinel" in cases From 64fb79301fec92eef7ef6747cbdb2f4b191e77dd Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Mon, 3 Aug 2026 16:06:22 -0400 Subject: [PATCH 03/18] test(rust): eliminate ignored correctness evidence --- crates/capsem-bench/src/tests.rs | 12 +++ crates/capsem-core/src/ipc_handshake.rs | 11 ++- crates/capsem-core/src/ipc_handshake/tests.rs | 20 +++-- crates/capsem-core/src/macros.rs | 7 +- crates/capsem-core/src/poll.rs | 9 +- crates/capsem-core/tests/mitm_integration.rs | 83 ------------------- crates/capsem-proto/src/poll.rs | 10 ++- tests/test_rust_test_layout.py | 26 ++++++ 8 files changed, 78 insertions(+), 100 deletions(-) diff --git a/crates/capsem-bench/src/tests.rs b/crates/capsem-bench/src/tests.rs index 7b2d3dd92..de708a7fc 100644 --- a/crates/capsem-bench/src/tests.rs +++ b/crates/capsem-bench/src/tests.rs @@ -19,6 +19,18 @@ fn selected_scenarios_are_strict() { assert!(select_scenarios(Some("bogus")).is_err()); } +#[test] +fn deterministic_protocol_rail_owns_ten_megabyte_throughput() { + let scenario = SCENARIOS + .iter() + .find(|scenario| scenario.name == "http_10mb") + .expect("the protocol benchmark must retain its 10 MB throughput scenario"); + + assert_eq!(scenario.path, "/bytes/10mb"); + assert_eq!(scenario.expected_bytes, Some(10 * 1024 * 1024)); + assert_eq!(scenario.body_kind, "10mb"); +} + #[test] fn latency_percentiles_are_interpolated() { let summary = latency_summary(vec![1.0, 2.0, 3.0, 4.0, 100.0]); diff --git a/crates/capsem-core/src/ipc_handshake.rs b/crates/capsem-core/src/ipc_handshake.rs index 428e0b196..605522266 100644 --- a/crates/capsem-core/src/ipc_handshake.rs +++ b/crates/capsem-core/src/ipc_handshake.rs @@ -69,10 +69,19 @@ pub fn negotiate_responder( stream: &mut UnixStream, peer_id: impl Into, traceparent: impl Into, +) -> Result { + negotiate_responder_with_timeout(stream, peer_id, traceparent, HELLO_TIMEOUT) +} + +fn negotiate_responder_with_timeout( + stream: &mut UnixStream, + peer_id: impl Into, + traceparent: impl Into, + timeout: Duration, ) -> Result { let prev_nb = ensure_blocking(stream); let result = (|| { - let peer = read_hello(stream, HELLO_TIMEOUT)?; + let peer = read_hello(stream, timeout)?; verify(&peer)?; write_hello(stream, &Hello::ours(peer_id, traceparent))?; Ok(peer) diff --git a/crates/capsem-core/src/ipc_handshake/tests.rs b/crates/capsem-core/src/ipc_handshake/tests.rs index 6897d4a0d..d5416c834 100644 --- a/crates/capsem-core/src/ipc_handshake/tests.rs +++ b/crates/capsem-core/src/ipc_handshake/tests.rs @@ -22,17 +22,19 @@ fn negotiate_succeeds_when_both_sides_match() { } #[test] -#[ignore = "waits the full 5s HELLO_TIMEOUT; run with --include-ignored when verifying handshake"] fn negotiate_times_out_when_peer_silent() { let (mut a, _b) = UnixStream::pair().unwrap(); - // _b kept alive but never writes a Hello -- our side waits for one. - // Use a deliberately short timeout for the test by calling read_hello - // directly (negotiate_responder reads first). - let err = negotiate_responder(&mut a, "capsem-service-test", "").unwrap_err(); - // Default HELLO_TIMEOUT is 5s; this test waits the full 5s. Trade-off: - // accept the latency to keep the public API minimal. To make tests - // fast, we'd parameterize the timeout -- not worth doing today. - assert!(matches!(err, HandshakeError::Timeout { .. }), "{err:?}"); + let timeout = Duration::from_millis(10); + + let err = + negotiate_responder_with_timeout(&mut a, "capsem-service-test", "", timeout).unwrap_err(); + + match err { + HandshakeError::Timeout { timeout_ms } => { + assert_eq!(timeout_ms, timeout.as_millis() as u64); + } + other => panic!("expected timeout, got {other:?}"), + } } #[test] diff --git a/crates/capsem-core/src/macros.rs b/crates/capsem-core/src/macros.rs index 6dd20e191..83ddc3f2b 100644 --- a/crates/capsem-core/src/macros.rs +++ b/crates/capsem-core/src/macros.rs @@ -17,9 +17,10 @@ /// the fully-formed expression and the macro just inspects its `Result`. /// /// Usage: -/// ```ignore -/// try_send!("terminal_rekey", terminal_rekey_tx.send(conn).await); -/// try_send!("ipc_state_change", ipc_tx.send(ProcessToService::StateChanged { .. })); +/// ``` +/// let (tx, rx) = std::sync::mpsc::channel(); +/// capsem_core::try_send!("state_change", tx.send(7)); +/// assert_eq!(rx.recv().unwrap(), 7); /// ``` /// /// Cleanup paths where a closed receiver is the documented design (e.g. a diff --git a/crates/capsem-core/src/poll.rs b/crates/capsem-core/src/poll.rs index 57f0b2986..81a7af7bd 100644 --- a/crates/capsem-core/src/poll.rs +++ b/crates/capsem-core/src/poll.rs @@ -14,13 +14,20 @@ pub type PollOpts = capsem_proto::poll::RetryOpts; /// /// Returns `Ok(T)` on success, `Err(TimedOut)` on timeout. /// -/// ```ignore +/// ```no_run +/// use capsem_core::poll::{poll_until, PollOpts}; +/// use std::{path::Path, time::Duration}; +/// +/// # async fn example() { +/// let socket_path = Path::new("/run/capsem/service.sock"); /// let result = poll_until( /// PollOpts::new("vm-ready", Duration::from_secs(30)), /// || async { /// if socket_path.exists() { Some(()) } else { None } /// }, /// ).await; +/// # let _ = result; +/// # } /// ``` pub async fn poll_until( opts: PollOpts, diff --git a/crates/capsem-core/tests/mitm_integration.rs b/crates/capsem-core/tests/mitm_integration.rs index d36fad047..43c946b3e 100644 --- a/crates/capsem-core/tests/mitm_integration.rs +++ b/crates/capsem-core/tests/mitm_integration.rs @@ -2191,86 +2191,3 @@ async fn multiple_requests_reuse_upstream_connection() { assert_eq!(event.method.as_deref(), Some("HEAD")); } } - -/// Download a ~10 MB PDF through the MITM proxy and assert throughput >= 1 MB/s. -/// -/// Exercises the full proxy pipeline on the host: TLS termination from the -/// "guest" client, upstream TLS to a real CDN, and body streaming back. -/// Uses elie.net directly (not cdn.elie.net) because raw hyper does not -/// follow 301 redirects. Marked #[ignore] so it doesn't run on every -/// `cargo test` -- run explicitly with -/// `cargo test -p capsem-core -- --ignored mitm_proxy_download_throughput`. -#[tokio::test] -#[ignore = "downloads ~10 MB; run explicitly to test proxy throughput"] -async fn mitm_proxy_download_throughput() { - const DOMAIN: &str = "elie.net"; - const PATH: &str = "/static/files/i-am-a-legend/i-am-a-legend-slides.pdf"; - // Conservative floor; the PDF is ~9.5 MB today but may drift on re-publish. - const EXPECTED_BYTES: u64 = 9 * 1024 * 1024; - const MIN_MBPS: f64 = 1.0; - - let (config, _db) = make_proxy_config(&[DOMAIN], &[], false); - let (proxy_task, addr) = spawn_proxy(config).await; - - let tcp = tokio::net::TcpStream::connect(addr).await.unwrap(); - let connector = TlsConnector::from(Arc::new(make_tls_client_config())); - let sni = ServerName::try_from(DOMAIN).unwrap(); - let tls = connector - .connect(sni, tcp) - .await - .expect("TLS handshake to elie.net should succeed"); - - let io = TokioIo::new(tls); - let (mut sender, conn) = hyper::client::conn::http1::handshake(io).await.unwrap(); - tokio::spawn(conn); - - let req = hyper::Request::builder() - .method("GET") - .uri(PATH) - .header("host", DOMAIN) - .body(Full::new(Bytes::new())) - .unwrap(); - - let start = std::time::Instant::now(); - let resp = sender.send_request(req).await.unwrap(); - let status = resp.status().as_u16(); - assert_eq!(status, 200, "expected 200 from {DOMAIN}, got {status}"); - - // Stream body without buffering 100 MB in one allocation. - let mut body = resp.into_body(); - let mut total_bytes: u64 = 0; - loop { - match BodyExt::frame(&mut body).await { - Some(Ok(frame)) => { - if let Ok(data) = frame.into_data() { - total_bytes += data.len() as u64; - } - } - Some(Err(e)) => panic!("body error: {e}"), - None => break, - } - } - - let elapsed = start.elapsed(); - let mbps = (total_bytes as f64 / (1024.0 * 1024.0)) / elapsed.as_secs_f64(); - println!( - "\nProxy throughput: {:.1} MB in {:.2}s = {:.2} MB/s", - total_bytes as f64 / (1024.0 * 1024.0), - elapsed.as_secs_f64(), - mbps, - ); - - drop(sender); - let _ = proxy_task.await; - - assert!( - total_bytes >= EXPECTED_BYTES, - "incomplete download: {:.1} MB (expected >= {:.1} MB)", - total_bytes as f64 / (1024.0 * 1024.0), - EXPECTED_BYTES as f64 / (1024.0 * 1024.0), - ); - assert!( - mbps >= MIN_MBPS, - "throughput too low: {mbps:.2} MB/s (minimum {MIN_MBPS} MB/s)" - ); -} diff --git a/crates/capsem-proto/src/poll.rs b/crates/capsem-proto/src/poll.rs index e8635033e..b2d449332 100644 --- a/crates/capsem-proto/src/poll.rs +++ b/crates/capsem-proto/src/poll.rs @@ -70,11 +70,15 @@ impl Default for RetryOpts { /// Calls `f()` repeatedly until it returns `Some(T)` or the deadline expires. /// Returns `Ok(T)` on success, `Err(TimedOut)` on timeout. /// -/// ```ignore +/// ``` +/// use capsem_proto::poll::{retry_with_backoff, RetryOpts}; +/// use std::time::Duration; +/// /// let fd = retry_with_backoff( -/// &RetryOpts::new("vsock-connect", Duration::from_secs(30)), -/// || vsock_connect(cid, port).ok(), +/// &RetryOpts::new("first-result", Duration::from_secs(1)), +/// || Some(7), /// ); +/// assert_eq!(fd.unwrap(), 7); /// ``` pub fn retry_with_backoff(opts: &RetryOpts, mut f: F) -> Result where diff --git a/tests/test_rust_test_layout.py b/tests/test_rust_test_layout.py index 2abdfe597..a6132d57d 100644 --- a/tests/test_rust_test_layout.py +++ b/tests/test_rust_test_layout.py @@ -23,6 +23,8 @@ # `//` comments only; a `mod tests {` inside a block comment or string literal has # never appeared here, and the guards below fail loudly if one ever does. LINE_COMMENT = re.compile(r"//.*$", re.MULTILINE) +IGNORED_TEST = re.compile(r"#\s*\[\s*ignore(?:\s*=|\s*\])") +IGNORED_DOCTEST = re.compile(r"```ignore(?:\s|$)") def _rust_sources() -> list[Path]: @@ -119,3 +121,27 @@ def test_every_crate_ships_unit_tests() -> None: "these crates carry no Rust tests at all; add unit tests in a sibling " "tests.rs or an integration test under tests/: " + ", ".join(untested) ) + + +def test_rust_correctness_evidence_is_never_silently_ignored() -> None: + """Correctness examples and tests must run on their owning test rail.""" + ignored_tests: list[str] = [] + ignored_doctests: list[str] = [] + + for path in _rust_sources() + sorted(CRATES.glob("*/tests/**/*.rs")): + source = path.read_text(encoding="utf-8") + if IGNORED_TEST.search(source): + ignored_tests.append(_rel(path)) + if IGNORED_DOCTEST.search(source): + ignored_doctests.append(_rel(path)) + + assert ignored_tests == [], ( + "#[ignore] silently removes Rust evidence from the default test run; " + "make correctness tests deterministic and move performance scenarios " + "to the benchmark rail: " + ", ".join(ignored_tests) + ) + assert ignored_doctests == [], ( + "```ignore does not even compile the example; use a runnable doctest or " + "```no_run when execution requires process context: " + + ", ".join(ignored_doctests) + ) From a9e5ae1fda522f7fedbab573cba3055ac89fa8e0 Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Mon, 3 Aug 2026 18:17:29 -0400 Subject: [PATCH 04/18] fix(gate): preserve provenance in linked worktrees --- config/gate.toml | 6 ++ src/capsem/gate/buildschema.py | 1 + src/capsem/gate/gitmetadata.py | 33 +++++++ src/capsem/gate/hostimage.py | 105 +++++++++++++++++------ src/capsem/gate/packagerail.py | 2 + tests/test_gate_crosscompile.py | 18 ++++ tests/test_gate_git_worktree_mount.py | 72 ++++++++++++++++ tests/test_gate_hostimage_composition.py | 69 +++++++++++++++ 8 files changed, 282 insertions(+), 24 deletions(-) create mode 100644 src/capsem/gate/gitmetadata.py create mode 100644 tests/test_gate_git_worktree_mount.py diff --git a/config/gate.toml b/config/gate.toml index e8cd797b2..c8df86598 100644 --- a/config/gate.toml +++ b/config/gate.toml @@ -845,6 +845,12 @@ tmpfs = "/tmp:rw,exec,mode=1777" # nextest writes its own state under the target dir; bound out so the results # survive the container. nextest_mount = "target/nextest" +# Tauri's build script regenerates ACL schemas inside the application crate. +# Keep the checkout read-only and give only that generated directory a +# writable backing store under the lane's owned output tree. +writable_source_mounts = [ + { source = "target/linux-rust-coverage/tauri-gen", target = "crates/capsem-app/gen" }, +] cached_volumes = [ { source = "capsem-linux-rust-cargo-registry", target = "/usr/local/cargo/registry" }, { source = "capsem-linux-rust-cargo-git", target = "/usr/local/cargo/git" }, diff --git a/src/capsem/gate/buildschema.py b/src/capsem/gate/buildschema.py index 70035684d..ef67a0c4d 100644 --- a/src/capsem/gate/buildschema.py +++ b/src/capsem/gate/buildschema.py @@ -86,6 +86,7 @@ class HostImageConfig(Strict): alpine: str tmpfs: str nextest_mount: str + writable_source_mounts: tuple[NamedVolume, ...] cached_volumes: tuple[NamedVolume, ...] environment: dict[str, str] diff --git a/src/capsem/gate/gitmetadata.py b/src/capsem/gate/gitmetadata.py new file mode 100644 index 000000000..bea60d6f2 --- /dev/null +++ b/src/capsem/gate/gitmetadata.py @@ -0,0 +1,33 @@ +"""Docker mount required to preserve Git identity from a linked worktree.""" + +from __future__ import annotations + +from pathlib import Path + +from .errors import GateError +from .proc import Runner + + +def docker_git_metadata_mount(runner: Runner) -> tuple[str, ...]: + """Mount external worktree metadata at the absolute path in ``.git``. + + An ordinary checkout carries its ``.git`` directory inside the source + mount and needs nothing extra. A linked worktree instead carries a file + whose ``gitdir:`` target lives under the primary checkout. Docker cannot + follow that host-only path unless the common metadata directory is mounted + at the same absolute location inside the container. + """ + if not (runner.root / ".git").is_file(): + return () + + common = runner.capture( + ["git", "rev-parse", "--path-format=absolute", "--git-common-dir"], + check=False, + ) + common_dir = Path(common) + if not common or not common_dir.is_absolute() or not common_dir.is_dir(): + raise GateError( + "linked worktree Git metadata could not be resolved; refusing a " + "Docker build that would embed an unknown source revision" + ) + return ("-v", f"{common_dir}:{common_dir}:ro") diff --git a/src/capsem/gate/hostimage.py b/src/capsem/gate/hostimage.py index 52d91176c..993769e28 100644 --- a/src/capsem/gate/hostimage.py +++ b/src/capsem/gate/hostimage.py @@ -16,6 +16,7 @@ from __future__ import annotations import os +from pathlib import Path from . import host from .actions import Action, Run @@ -24,6 +25,8 @@ from .context import Context from .errors import GateError from .execution import Step, step +from .fileactions import MakeDir +from .gitmetadata import docker_git_metadata_mount from .plan import Plan @@ -84,6 +87,7 @@ def perform(self, context: Context) -> None: "--rm", "-v", f"{context.root}:{settings.mount}", + *docker_git_metadata_mount(context.runner), "-w", settings.mount, "--user", @@ -172,36 +176,35 @@ def build(self, after: tuple[Step, ...]) -> Step: after=(built,), ) + mountpoints = plan.add( + step( + "linux-rust-mountpoints", + MakeDir(config.path(settings.nextest_mount)), + MakeDir(output / settings.nextest_dir), + *( + action + for volume in settings.writable_source_mounts + for action in ( + MakeDir(config.path(volume.source)), + MakeDir(config.path(volume.target)), + ) + ), + ), + after=(owned,), + ) + suite = plan.add( step( "linux-rust", - Run( - [ - "docker", - "run", - "--rm", - "--user", - f"{uid}:{gid}", - *[f for k, v in settings.environment.items() for f in ("-e", f"{k}={v}")], - "--tmpfs", - settings.tmpfs, - "-v", - f"{config.root}:{settings.mount}:ro", - "-v", - f"{output}:{settings.container_output}", - "-v", - f"{output / settings.nextest_dir}:{settings.mount}/{settings.nextest_mount}", - *_volumes(config), - "-w", - settings.mount, - settings.tag, - "bash", - f"{settings.mount}/{settings.script}", - ] + _LinuxRustSuite( + output, + source=config.root, + mount=settings.mount, + script=settings.script, ), contends=(docker,), ), - after=(owned,), + after=(mountpoints,), ) return plan.add( @@ -227,6 +230,60 @@ def build(self, after: tuple[Step, ...]) -> Step: ) +class _LinuxRustSuite(Action, name="linux-rust-suite"): + """Run the Linux parity script with runtime-resolved worktree metadata.""" + + def __init__(self, output: Path, *, source: Path, mount: str, script: str) -> None: + self._output = output + self._source = source + self._mount = mount + self._script = script + + def render(self) -> str: + return ( + f"docker run --user -v {self._source}:{self._mount}:ro " + f"... bash {self._mount}/{self._script}" + ) + + def perform(self, context: Context) -> None: + settings = context.config.hostimage + output = self._output + uid, gid = host.user() + context.runner.run( + [ + "docker", + "run", + "--rm", + "--user", + f"{uid}:{gid}", + *[f for k, v in settings.environment.items() for f in ("-e", f"{k}={v}")], + "--tmpfs", + settings.tmpfs, + "-v", + f"{context.root}:{settings.mount}:ro", + *docker_git_metadata_mount(context.runner), + "-v", + f"{output}:{settings.container_output}", + "-v", + f"{output / settings.nextest_dir}:{settings.mount}/{settings.nextest_mount}", + *[ + flag + for volume in settings.writable_source_mounts + for flag in ( + "-v", + f"{context.config.path(volume.source)}:" + f"{settings.mount}/{volume.target}", + ) + ], + *_volumes(context.config), + "-w", + settings.mount, + settings.tag, + "bash", + f"{settings.mount}/{settings.script}", + ] + ) + class LinuxRustCommand( GateCommand, name="linux-rust", diff --git a/src/capsem/gate/packagerail.py b/src/capsem/gate/packagerail.py index a912a2740..d5a34630a 100644 --- a/src/capsem/gate/packagerail.py +++ b/src/capsem/gate/packagerail.py @@ -29,6 +29,7 @@ from .config import Arch from .errors import GateError from .fileactions import copy_tree, make_dir, remove +from .gitmetadata import docker_git_metadata_mount from .packageinputs import package_environment, pinned_toolchain, resolve_channel from .packagesigning import signing_key from .proc import Runner @@ -141,6 +142,7 @@ def build(self) -> None: argv += ["-e", name] mount = self._config.install.mount argv += ["-v", f"{self.root}:{mount}"] + argv += docker_git_metadata_mount(self._runner) for volume in self._package.volumes: argv += ["-v", f"{volume.source}:{volume.target}"] argv += [ diff --git a/tests/test_gate_crosscompile.py b/tests/test_gate_crosscompile.py index 23dd0dd5c..a7c51c20a 100644 --- a/tests/test_gate_crosscompile.py +++ b/tests/test_gate_crosscompile.py @@ -177,6 +177,24 @@ def test_the_cargo_caches_are_shared_and_the_target_dir_is_per_architecture( assert f"-v capsem-host-target-{TARGET.name}:/cargo-target" in build +def test_package_build_mounts_linked_worktree_git_metadata( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + metadata = "/git/common" + monkeypatch.setattr("capsem.gate.host.system", lambda: "Linux") + monkeypatch.setattr("capsem.gate.host.machine", lambda: TARGET.name) + monkeypatch.setattr( + "capsem.gate.packagerail.docker_git_metadata_mount", + lambda _runner: ("-v", f"{metadata}:{metadata}:ro"), + ) + runner = Building(_checkout(tmp_path), replies={"select-linux": "skip"}) + + _run_lane(_rail(runner)) + + build = runner.matching(r"docker run --rm")[0] + assert f"-v {metadata}:{metadata}:ro" in build + + def test_the_builder_image_is_rebuilt_before_every_package() -> None: """Always rebuilt, and always before the package that runs inside it. diff --git a/tests/test_gate_git_worktree_mount.py b/tests/test_gate_git_worktree_mount.py new file mode 100644 index 000000000..4fbd1eca5 --- /dev/null +++ b/tests/test_gate_git_worktree_mount.py @@ -0,0 +1,72 @@ +"""Docker builds retain Git identity when the gate runs from a linked worktree.""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest +from helpers.gate import RecordingRunner + +from capsem.gate.errors import GateError +from capsem.gate.gitmetadata import docker_git_metadata_mount +from capsem.gate.proc import Runner + + +def _git(*args: str, cwd: Path) -> None: + subprocess.run( + ["git", *args], + cwd=cwd, + check=True, + capture_output=True, + text=True, + ) + + +def _linked_worktree(tmp_path: Path) -> tuple[Path, Path]: + repository = tmp_path / "repository" + repository.mkdir() + _git("init", "--quiet", cwd=repository) + _git( + "-c", + "user.name=Capsem Test", + "-c", + "user.email=test@capsem.invalid", + "-c", + "commit.gpgsign=false", + "commit", + "--quiet", + "--allow-empty", + "-m", + "fixture", + cwd=repository, + ) + worktree = tmp_path / "linked" + _git("worktree", "add", "--quiet", "--detach", str(worktree), cwd=repository) + return repository, worktree + + +def test_linked_worktree_mounts_its_external_git_metadata_read_only(tmp_path: Path) -> None: + repository, worktree = _linked_worktree(tmp_path) + + mount = docker_git_metadata_mount(Runner(worktree)) + + common = (repository / ".git").resolve() + assert mount == ("-v", f"{common}:{common}:ro") + + +def test_ordinary_checkout_needs_no_second_git_mount(tmp_path: Path) -> None: + repository, _ = _linked_worktree(tmp_path) + + assert docker_git_metadata_mount(Runner(repository)) == () + + +def test_linked_worktree_fails_closed_when_git_cannot_resolve_metadata( + tmp_path: Path, +) -> None: + root = tmp_path / "broken" + root.mkdir() + (root / ".git").write_text("gitdir: /missing/git/metadata\n", encoding="utf-8") + + with pytest.raises(GateError, match="linked worktree Git metadata"): + docker_git_metadata_mount(RecordingRunner(root)) diff --git a/tests/test_gate_hostimage_composition.py b/tests/test_gate_hostimage_composition.py index 192ba1067..71e082422 100644 --- a/tests/test_gate_hostimage_composition.py +++ b/tests/test_gate_hostimage_composition.py @@ -29,6 +29,7 @@ ) from capsem.gate import config as gate_config from capsem.gate.command import GateCommand +from capsem.gate.context import Context from capsem.gate.plan import Plan PROJECT_ROOT = Path(__file__).resolve().parents[1] @@ -86,6 +87,74 @@ def test_two_lanes_in_one_plan_build_the_builder_once() -> None: assert list(plan.labels).count(hostimage.STEP) == 1 +def test_foreign_uid_probe_mounts_linked_worktree_metadata( + monkeypatch: pytest.MonkeyPatch, +) -> None: + metadata = "/git/common" + monkeypatch.setattr( + hostimage, + "docker_git_metadata_mount", + lambda _runner: ("-v", f"{metadata}:{metadata}:ro"), + ) + runner = RecordingRunner( + PROJECT_ROOT, + replies={"git rev-parse --short HEAD": "abc123"}, + ) + + hostimage._ForeignUidProbe().perform(Context(runner, CONFIG)) + + probe = runner.rendered[-1] + assert f"-v {metadata}:{metadata}:ro" in probe + assert "--user 4242:4242" in probe + + +def test_linux_rust_suite_mounts_linked_worktree_metadata( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + metadata = "/git/common" + monkeypatch.setattr( + hostimage, + "docker_git_metadata_mount", + lambda _runner: ("-v", f"{metadata}:{metadata}:ro"), + ) + monkeypatch.setattr(hostimage.host, "user", lambda: (501, 20)) + runner = RecordingRunner(PROJECT_ROOT) + + hostimage._LinuxRustSuite( + tmp_path, + source=CONFIG.root, + mount=CONFIG.hostimage.mount, + script=CONFIG.hostimage.script, + ).perform(Context(runner, CONFIG)) + + suite = runner.rendered[-1] + assert f"-v {metadata}:{metadata}:ro" in suite + assert "--user 501:20" in suite + assert CONFIG.hostimage.script in suite + + for volume in CONFIG.hostimage.writable_source_mounts: + source = CONFIG.path(volume.source) + target = f"{CONFIG.hostimage.mount}/{volume.target}" + assert f"-v {source}:{target}" in suite + + +def test_linux_rust_materializes_nested_mountpoints_before_read_only_source() -> None: + plan = _plan("linux-rust") + order = list(plan.labels) + + assert order.index("linux-rust-mountpoints") < order.index("linux-rust") + mountpoints = next(step for step in plan.steps if step.label == "linux-rust-mountpoints") + rendered = "\n".join(action.render() for action in mountpoints.actions) + assert str(CONFIG.path(CONFIG.hostimage.nextest_mount)) in rendered + assert str( + CONFIG.path(CONFIG.hostimage.output_dir) / CONFIG.hostimage.nextest_dir + ) in rendered + for volume in CONFIG.hostimage.writable_source_mounts: + assert str(CONFIG.path(volume.source)) in rendered + assert str(CONFIG.path(volume.target)) in rendered + + def test_chained_lanes_do_not_make_the_builder_depend_on_them() -> None: """Shared groundwork sits before everything, not after its caller. From 897c7c84c8de223c7b283cfdb29d1f95e545a4cb Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Mon, 3 Aug 2026 18:18:18 -0400 Subject: [PATCH 05/18] test(rust): enforce Clippy in Linux parity --- scripts/test-linux-rust.sh | 11 +++++++++++ tests/test_release_doctor_contract.py | 3 +++ 2 files changed, 14 insertions(+) diff --git a/scripts/test-linux-rust.sh b/scripts/test-linux-rust.sh index eb27a4e8b..41385db25 100755 --- a/scripts/test-linux-rust.sh +++ b/scripts/test-linux-rust.sh @@ -30,6 +30,17 @@ for package in "${packages[@]}"; do done cd "$ROOT" + +# capsem-app embeds frontend/dist at compile time. The macOS full gate builds +# it before mounting this checkout read-only in the Linux parity container; +# the independent native-Linux CI job has to materialize it for itself. +if [[ ! -s "$ROOT/frontend/dist/index.html" ]]; then + pnpm --dir frontend install --frozen-lockfile + bash scripts/check-web-surface.sh frontend-build +fi + +cargo clippy --workspace --all-targets -- -D warnings + cargo llvm-cov nextest \ --no-cfg-coverage \ --bins \ diff --git a/tests/test_release_doctor_contract.py b/tests/test_release_doctor_contract.py index a4397140f..d14c750e4 100644 --- a/tests/test_release_doctor_contract.py +++ b/tests/test_release_doctor_contract.py @@ -4949,6 +4949,9 @@ def test_just_test_owns_linux_rust_platform_coverage_through_docker() -> None: assert "run: just _gate-linux-rust" in linux_ci assert "cargo llvm-cov nextest" not in linux_ci assert "cargo llvm-cov nextest" in runner + linux_clippy = "cargo clippy --workspace --all-targets -- -D warnings" + assert linux_clippy in runner + assert runner.index(linux_clippy) < runner.index("cargo llvm-cov nextest") assert "capsem-service" in runner assert 'package_args+=( -p "$package" )' in runner assert "--profile ci" in runner From acbe31fdd269a6aa2ca833491f0d0a7e9536c57c Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Mon, 3 Aug 2026 18:21:34 -0400 Subject: [PATCH 06/18] test(release): respect fail-closed staging policy --- crates/capsem-admin/src/tests.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/capsem-admin/src/tests.rs b/crates/capsem-admin/src/tests.rs index 9b5aba022..dad50e4b2 100644 --- a/crates/capsem-admin/src/tests.rs +++ b/crates/capsem-admin/src/tests.rs @@ -1813,10 +1813,10 @@ fn assets_channel_build_writes_manifest_under_channel_assets_dir() { let source = fs::metadata(assets_dir.join("arm64/vmlinuz")).unwrap(); let release = fs::metadata(release_dir.join("arm64-vmlinuz")).unwrap(); - assert_eq!( + assert_ne!( source.ino(), release.ino(), - "same-filesystem immutable VM publication must hardlink instead of copying" + "an external fixture is unclassified and must copy rather than fail open" ); } assert!(release_dir.join("arm64-initrd.img").is_file()); @@ -1853,12 +1853,12 @@ fn assets_channel_build_writes_manifest_under_channel_assets_dir() { .expect("kernel publication URL") .trim_start_matches('/'), ); - assert_eq!( + assert_ne!( fs::metadata(assets_dir.join("arm64/vmlinuz")) .unwrap() .ino(), fs::metadata(published).unwrap().ino(), - "duplicate profile references must hardlink immutable VM bytes" + "an external fixture must remain independent from published output" ); } assert_eq!( From 056dafddd27b973b8047b6b822976b250c321055 Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Mon, 3 Aug 2026 19:34:32 -0400 Subject: [PATCH 07/18] fix(gate): build the plan from source, not from what the last run left behind `module_functional` asked `profiles.selected(config)` for its axis while the plan was being constructed, and that reads `target/config/profiles`. So the same commit produced one plan on a warm tree and a different one on a fresh clone. That is the CI failure. `just release-profile nightly code` passed the complete 57-minute gate locally, pushed, dispatched, and the run died with 94 tests all reporting `no materialized profiles found under target/config/profiles`. The local gate had been green partly on leftovers. `source.record`/`source.verify` never covered it -- they digest tracked source, and this input is not tracked source, so "this gate qualified exactly this revision" was structurally false: two runs at one HEAD could run different plans. No step ordering fixes it. Plan construction is deliberately pure -- built against a runner that refuses every invocation -- so a step's output cannot exist by the time the plan exists. The axis therefore comes from checked-in `config/profiles/`, via `imagebuild.profiles()` rather than a second reader of the same glob. The agreement check did not disappear, it moved to where it can run. `profiles.agree()` asserts materialized == source == manifest, and `functional.axis` performs it: first in the functional module, and after `prepare.materialize-config` in the complete gate. A materialized catalog that disagrees still fails the run; it now fails it from a step instead of preventing the plan from being built at all. Proven by cloning the repo to a directory with no `target/` and planning there: 14 steps, `functional.axis` first. Before this it raised. Mutation-checked by pointing the axis back at build output -- five of six guards red. Also registers `tests/test_gate_git_worktree_mount.py`, which arrived with the worktree-provenance fix (a9e5ae1f) and was never added to the source-contract suites, so it ran nowhere. 1989 gate tests, 526 release/build-chain contracts. --- CHANGELOG.md | 12 +++ config/gate.toml | 2 + src/capsem/gate/module_functional.py | 34 ++++++- src/capsem/gate/profiles.py | 47 +++++++-- tests/test_gate_plan_purity.py | 137 +++++++++++++++++++++++++++ 5 files changed, 219 insertions(+), 13 deletions(-) create mode 100644 tests/test_gate_plan_purity.py diff --git a/CHANGELOG.md b/CHANGELOG.md index d80d3674c..eb90ca654 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- A fresh clone now plans the same gate a warm tree does. The functional + module asked for its profile axis while the plan was being *built*, and that + read `target/config/profiles` -- build output -- so the same commit produced + one plan on a developer's machine and another on a clean checkout. A release + passed a 57-minute gate locally on leftovers, pushed, dispatched, and CI + failed with 94 tests all reporting `no materialized profiles found`. Neither + `source.record` nor `source.verify` could have caught it: they digest tracked + source, and this input was not tracked source. The axis comes from checked-in + `config/profiles/` now. The agreement it used to check inline -- materialized + against declared against source -- did not go away; it became a step, which + is where a question about build output can actually be asked. + - A gate now reports what it did to the filesystem, as it does it. `contends` is a list an author typed and the overlap check compares two such lists to each other -- nothing in that loop had ever looked at a disk, so a step that diff --git a/config/gate.toml b/config/gate.toml index c8df86598..248ceb0ff 100644 --- a/config/gate.toml +++ b/config/gate.toml @@ -951,6 +951,8 @@ blocks_clippy = "frontend" # neither. source_contract = [ "tests/test_gate_observation.py", + "tests/test_gate_plan_purity.py", + "tests/test_gate_git_worktree_mount.py", "tests/test_rust_filesystem_chokepoint.py", "tests/test_agent_skill_index.py", "tests/test_authoritative_values_are_not_restated.py", diff --git a/src/capsem/gate/module_functional.py b/src/capsem/gate/module_functional.py index 47492ccc1..80504ce0a 100644 --- a/src/capsem/gate/module_functional.py +++ b/src/capsem/gate/module_functional.py @@ -8,9 +8,11 @@ pytestsuite, vmproofs, ) +from .actions import Action from .command import GateCommand from .config import GateConfig -from .execution import Step +from .context import Context +from .execution import Step, step from .plan import Plan from .qualification import Qualification from .testmodules import InWorkspace @@ -42,6 +44,24 @@ def plan(self) -> Plan: return plan +class AxisAgrees(Action, name="axis-agrees"): + """Check the materialized profiles are the ones the plan was built for. + + `selected()` reads checked-in `config/profiles/`; this reads what the + build actually materialized, and refuses when they differ. A materialized + catalog that does not match means the gate would prove a pairing nobody is + shipping -- which is why the check did not go away when the plan stopped + reading build output, it moved to where it can run. + """ + + def render(self) -> str: + return "check the materialized profiles match the checked-in axis" + + def perform(self, context: Context) -> None: + profiles.agree(context.config) + context.journal.note(f"profile axis {', '.join(profiles.selected(context.config))}") + + def functional( plan: Plan, config: GateConfig, @@ -51,14 +71,22 @@ def functional( ) -> Step: """Every VM-owned suite, for every profile the channel selects.""" phase = plan.phase("functional") + # From checked-in `config/profiles/`, because this runs while the plan is + # being built and a plan may not depend on build output. See + # `profiles.selected`. axis = profiles.selected(config) base, rest = axis[0], axis[1:] + # That the materialized catalog agrees with the source axis and with the + # manifest under test is still required -- it is simply a run-time + # question now, asked once, before any profile lane runs against it. + agreed = phase.add(step("axis", AxisAgrees()), after=after) + # A release lane was handed signed binaries; signing them again would # replace the bytes the manifest selected with locally built ones. - first: tuple = after + first: tuple = (agreed,) if not qualification.pulled: - first = (phase.add(hostpackage.sign_step(config), after=after),) + first = (phase.add(hostpackage.sign_step(config), after=(agreed,)),) previous = _profile_lane(phase, config, base, after=first, broad=True) for profile in rest: diff --git a/src/capsem/gate/profiles.py b/src/capsem/gate/profiles.py index 68bbb4912..6d53fd33d 100644 --- a/src/capsem/gate/profiles.py +++ b/src/capsem/gate/profiles.py @@ -74,18 +74,45 @@ def declared(manifest: Path) -> list[str] | None: def selected(config: GateConfig) -> list[str]: - """The profile axis for a functional proof, base profile first.""" + """The profile axis for a functional proof, base profile first. + + From `config/profiles/`, which is checked in -- **not** from what happens + to be materialized under `target/`. This is read while the plan is being + *built*, and plan construction cannot depend on build output: a step's + output does not exist yet, so the same commit produced one plan on a warm + tree and a different one on a fresh clone. `just release-profile` passed a + 57-minute gate locally on leftovers, then failed in CI with 94 tests all + reporting `no materialized profiles found`. + + Agreement between this axis, the materialized catalog and the manifest + under test is still required -- a materialized catalog that differs from + the manifest means the gate would prove a pairing nobody is shipping. That + is a run-time question, so `agree()` answers it from a step. + """ + from . import imagebuild + + base = config.suites.pytest.base_profile + return sorted(imagebuild.profiles(config), key=lambda identity: (identity != base, identity)) + + +def agree(config: GateConfig) -> None: + """Check the materialized catalog matches the source axis and the manifest. + + The check `selected()` used to make inline, moved to where it can run: after + the step that materializes, rather than before any step has run at all. + """ settings = config.suites.pytest - present = materialized(config.path(settings.materialized_profiles)) - wanted = declared(config.path(settings.test_manifest)) + source = sorted(selected(config)) + present = sorted(materialized(config.path(settings.materialized_profiles))) + if source != present: + raise GateError( + "the materialized profile catalog does not match the checked-in " + f"profiles: config/profiles={source}, materialized={present}" + ) - if wanted is None: - wanted = present - elif set(present) != set(wanted): + wanted = declared(config.path(settings.test_manifest)) + if wanted is not None and sorted(wanted) != present: raise GateError( "the materialized profile catalog does not match the manifest under " - f"test: manifest={wanted}, materialized={present}" + f"test: manifest={sorted(wanted)}, materialized={present}" ) - - base = settings.base_profile - return sorted(wanted, key=lambda identity: (identity != base, identity)) diff --git a/tests/test_gate_plan_purity.py b/tests/test_gate_plan_purity.py new file mode 100644 index 000000000..4d5e5a341 --- /dev/null +++ b/tests/test_gate_plan_purity.py @@ -0,0 +1,137 @@ +"""A plan is built from source, not from whatever the last run left behind. + +`module_functional` asked `profiles.selected(config)` for its axis while the +plan was being *constructed*, and that reads `target/config/profiles` -- build +output. So the same commit produced one plan on a warm tree and a different +one on a cold checkout. + +That is not a theoretical hazard. `just release-profile nightly code` passed a +57-minute gate locally, pushed, dispatched, and CI failed with 94 tests all +reporting `no materialized profiles found under target/config/profiles`. The +local run had been green partly on leftovers, and `source.record` / +`source.verify` could not have caught it: they digest tracked source, and this +input is not tracked source. + +No step ordering fixes it. Plan construction is deliberately pure -- see +`command.py::_describe`, which builds against a runner that refuses every +invocation -- so a step's output cannot exist by the time the plan is built. +The axis has to come from `config/profiles/`, which is checked in, present on +every clone, and covered by the source digest. Agreement between that and what +was materialized is a *step*, and it runs after the step that materializes. +""" + +from __future__ import annotations + +import argparse +import shutil +from pathlib import Path + +import pytest + +from capsem.gate import cli # noqa: F401 - imported so every command registers +from capsem.gate import config as gate_config +from capsem.gate.command import GateCommand + +PROJECT_ROOT = Path(__file__).resolve().parents[1] + + +#: What each command needs beyond the common flags. Release lanes take a +#: channel; the profile lane also takes a profile. +ARGUMENTS: dict[str, dict[str, str]] = { + "release-binaries": {"channel": "nightly"}, + "release-profile": {"channel": "nightly", "profile": "code"}, +} + + +def _plan_labels(name: str) -> tuple[str, ...]: + from helpers.gate import RecordingRunner + + command = GateCommand.registry[name]( + RecordingRunner(PROJECT_ROOT), + argparse.Namespace(dry_run=False, graph=False, timing=False, **ARGUMENTS.get(name, {})), + ) + return tuple(command._describe().labels) + + +def test_the_functional_plan_is_the_same_shape_on_a_cold_tree( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """The 94-failure bug, stated as an equality. + + Move the materialized profiles out of the way -- which is what a fresh + clone and every CI runner look like -- and the plan must not change. + """ + config = gate_config.load(PROJECT_ROOT) + materialized = config.path(config.suites.pytest.materialized_profiles) + + warm = _plan_labels("test-functional") + + stash = tmp_path / "profiles" + moved = materialized.exists() + if moved: + shutil.move(str(materialized), str(stash)) + try: + cold = _plan_labels("test-functional") + finally: + if moved: + materialized.parent.mkdir(parents=True, exist_ok=True) + shutil.move(str(stash), str(materialized)) + + assert cold == warm, ( + "the plan changed shape because build output was missing; a fresh " + "clone therefore runs a different gate than a warm tree, which is how " + "94 tests passed locally and failed in CI on the same commit" + ) + + +@pytest.mark.parametrize( + "name", ["test-functional", "test-candidate", "release-binaries", "release-profile"] +) +def test_a_plan_builds_without_any_build_output( + name: str, monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """Every command whose plan reaches the functional axis. + + Parametrized rather than looped so a regression names which command broke, + not merely that one did. + """ + config = gate_config.load(PROJECT_ROOT) + materialized = config.path(config.suites.pytest.materialized_profiles) + + stash = tmp_path / f"profiles-{name}" + moved = materialized.exists() + if moved: + shutil.move(str(materialized), str(stash)) + try: + labels = _plan_labels(name) + finally: + if moved: + materialized.parent.mkdir(parents=True, exist_ok=True) + shutil.move(str(stash), str(materialized)) + + assert labels, f"{name} produced an empty plan" + + +def test_the_axis_agreement_is_a_step_and_runs_after_materialization() -> None: + """The check does not disappear, it moves to where it can run. + + Materialized, declared and source axes still have to agree -- a materialized + catalog that differs from the manifest means the gate would prove a pairing + nobody is shipping. That is a run-time question, so it is a step, and it + depends on the step that materializes. + """ + from helpers.gate import gate_labels + + # In the functional module alone there is nothing to materialize, so the + # claim is that the check runs before anything depends on it. + alone = gate_labels("test-functional") + assert "functional.axis" in alone, alone + assert alone.index("functional.axis") == 0, alone[:3] + + # In the complete gate it must come after the step that materializes -- + # asserted as ordering rather than a direct edge, because the intervening + # shape is the plan's business and pinning it would break on any reshuffle. + whole = gate_labels("test-candidate") + assert whole.index("prepare.materialize-config") < whole.index("functional.axis"), ( + "the axis is checked before anything materializes it" + ) From 1405ecf18e29dd17bc0044e0a043b23ebef22be3 Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Mon, 3 Aug 2026 19:57:47 -0400 Subject: [PATCH 08/18] test(gate): pin the release plan as identical, not merely buildable "It builds on a cold tree" is the weaker claim. A release lane could plan *something* without build output and still plan a different something -- fewer profiles, a lane skipped -- and then publish on the strength of a proof that never ran. Verified once against reality by cloning to a directory with no `target/` and diffing the dry run of both lanes: zero lines. Asserted here on the described plan so it stays true without needing a clone, and mutation-checked by pointing the axis back at build output, which reds both. Also records what a search for the reported IronBank follow-up actually found: release-plan inspection is byte-identical cold and warm, so that class is closed. The one genuine non-hermetic read left is `scripts/docker-storage-policy.py:1183`, which globs gitignored `target/ironbank-assets/build-*.log` while collecting failure evidence -- and silently collects nothing when they are absent, which is a worse outcome than failing, because the gap shows up in a post-mortem rather than in the run. --- tests/test_gate_plan_purity.py | 37 ++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/test_gate_plan_purity.py b/tests/test_gate_plan_purity.py index 4d5e5a341..53f8d6ed3 100644 --- a/tests/test_gate_plan_purity.py +++ b/tests/test_gate_plan_purity.py @@ -135,3 +135,40 @@ def test_the_axis_agreement_is_a_step_and_runs_after_materialization() -> None: assert whole.index("prepare.materialize-config") < whole.index("functional.axis"), ( "the axis is checked before anything materializes it" ) + + +@pytest.mark.parametrize("name", ["release-binaries", "release-profile"]) +def test_the_release_plan_is_byte_identical_without_build_output(name: str, tmp_path: Path) -> None: + """Stronger than "it builds": the plan must be the *same* plan. + + A release lane that merely plans on a cold tree could still plan something + different -- fewer profiles, a skipped lane -- and publish on the strength + of a proof that never ran. Verified once by cloning to a directory with no + `target/` and diffing the dry run (zero lines); asserted here so it stays + true without a clone. + """ + from helpers.gate import RecordingRunner + + config = gate_config.load(PROJECT_ROOT) + materialized = config.path(config.suites.pytest.materialized_profiles) + + def described() -> str: + command = GateCommand.registry[name]( + RecordingRunner(PROJECT_ROOT), + argparse.Namespace(dry_run=False, graph=False, timing=False, **ARGUMENTS.get(name, {})), + ) + return command._describe().describe() + + warm = described() + stash = tmp_path / f"cold-{name}" + moved = materialized.exists() + if moved: + shutil.move(str(materialized), str(stash)) + try: + cold = described() + finally: + if moved: + materialized.parent.mkdir(parents=True, exist_ok=True) + shutil.move(str(stash), str(materialized)) + + assert cold == warm, f"{name} plans a different release without build output" From 7667c5e9cdd45aea70aaa6943614e24c2e23fb8c Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Mon, 3 Aug 2026 20:21:50 -0400 Subject: [PATCH 09/18] fix(gate): stop failure evidence from being silently incomplete `copy_small_file` returned quietly for three different outcomes -- absent, over the size cap, unreadable -- and the IronBank globs yielded nothing on a tree where those builds never ran. So a preserved bundle gave no way to tell "there were no IronBank logs" from "the collector never looked" from "the copy failed", and the gap only became visible during the post-mortem that needed it. Every bundle now carries `collected.json`: each source attempted, and what became of it. Globs are named even when they match nothing, because a build that never ran and a collector that never looked produce identical silence otherwise. That manifest paid for itself on the first real capture, which reported `build.log` and `docker-storage.jsonl` as `too-large`. Every bundle ever written had silently omitted the two files a post-mortem reaches for first -- and omitted them precisely on the long runs large enough to need them. Oversized files are tailed rather than dropped now: the end of a build log is where the failure is. Verified against a real capture, which produced a 25 MiB tail ending at the log's actual last line where it previously produced nothing. Mutation-checked. The first version of the guard asserted only that *something* was reported absent, which passed on `build.log` alone and would have survived deleting the empty-glob record entirely; it now names the IronBank globs. --- CHANGELOG.md | 14 +++++ scripts/docker-storage-policy.py | 84 +++++++++++++++++++--------- tests/test_docker_storage_policy.py | 87 +++++++++++++++++++++++++++++ 3 files changed, 160 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb90ca654..4cd8fefb7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Failure-evidence bundles were quietly incomplete. `copy_small_file` returned + the same silence for three different outcomes -- the file was absent, over + the size cap, or unreadable -- and the IronBank globs matched nothing at all + on a tree where those builds never ran, so a bundle could not distinguish + "there was nothing to collect" from "the collector failed". Every bundle now + carries a `collected.json` naming each source attempted and what became of + it, globs included when they match nothing. + + The first bundle written with that manifest reported `build.log` and + `docker-storage.jsonl` as over the cap -- meaning every previous bundle had + silently omitted the two files a post-mortem reaches for first, and did so + precisely on the long runs that needed them. Oversized files are now tailed + rather than dropped, because the end of a build log is where the failure is. + - A fresh clone now plans the same gate a warm tree does. The functional module asked for its profile axis while the plan was being *built*, and that read `target/config/profiles` -- build output -- so the same commit produced diff --git a/scripts/docker-storage-policy.py b/scripts/docker-storage-policy.py index 6c2b560c8..05f72e3eb 100755 --- a/scripts/docker-storage-policy.py +++ b/scripts/docker-storage-policy.py @@ -1085,14 +1085,36 @@ def command_clean(args: argparse.Namespace, policy: dict[str, Any]) -> int: return 0 if report["after"]["available"] else 1 -def copy_small_file(source: Path, destination: Path, maximum_bytes: int) -> None: +def copy_small_file(source: Path, destination: Path, maximum_bytes: int) -> str: + """Copy if it is there and small enough, and say which of those it was. + + Returning `None` for three different outcomes -- absent, over the cap, + unreadable -- is what made a preserved bundle unable to distinguish "there + was nothing to collect" from "the collector failed". The caller records + the answer, so the gap is visible in the bundle rather than in the + post-mortem that needed it. + + Oversized files are tailed, not dropped. The first bundle written with + this manifest showed `build.log` and `docker-storage.jsonl` as + `too-large`, meaning every previous bundle had silently omitted both. + """ try: - if not source.is_file() or source.stat().st_size > maximum_bytes: - return + if not source.is_file(): + return "absent" destination.parent.mkdir(parents=True, exist_ok=True) + if source.stat().st_size > maximum_bytes: + # Keep the tail rather than nothing. A build log that overran the + # cap was silently discarded, which removed the most useful file + # in the bundle exactly when the run was long enough to need it -- + # and the end of a log is where the failure is. + with source.open("rb") as handle: + handle.seek(-maximum_bytes, os.SEEK_END) + destination.write_bytes(handle.read()) + return "truncated" shutil.copy2(source, destination) except OSError: - return + return "unreadable" + return "copied" def artifact_tree_size(path: Path) -> int: @@ -1116,9 +1138,7 @@ def rotate_debug_artifacts(root: Path, debug: dict[str, Any]) -> None: return minimum = int(debug["minimum_runs"]) maximum = int(debug["maximum_runs"]) - cutoff = datetime.now(UTC).timestamp() - ( - int(debug["maximum_age_days"]) * 24 * 60 * 60 - ) + cutoff = datetime.now(UTC).timestamp() - (int(debug["maximum_age_days"]) * 24 * 60 * 60) protected = set(directories[-minimum:]) if minimum > 0 else set() stale = list(directories[:-maximum] if maximum > 0 else directories) stale.extend( @@ -1169,25 +1189,39 @@ def command_capture_failure(args: argparse.Namespace, policy: dict[str, Any]) -> ) maximum_bytes = int(debug["maximum_file_mib"]) * 1024 * 1024 - copy_small_file(ROOT / "target" / "build.log", destination / "build.log", maximum_bytes) - copy_small_file( - report_path(policy), - destination / "docker-storage.jsonl", - maximum_bytes, - ) - copy_small_file( - ROOT / "target" / "storage" / "host-cleanup.jsonl", - destination / "host-cleanup.jsonl", - maximum_bytes, - ) + collected: list[dict[str, str]] = [] + + def collect(source: Path, target: Path) -> None: + outcome = copy_small_file(source, target, maximum_bytes) + collected.append({"source": str(source), "outcome": outcome}) + + collect(ROOT / "target" / "build.log", destination / "build.log") + collect(report_path(policy), destination / "docker-storage.jsonl") + collect(ROOT / "target" / "storage" / "host-cleanup.jsonl", destination / "host-cleanup.jsonl") + ironbank = ROOT / "target" / "ironbank-assets" - for source in ironbank.glob("build-*.log"): - copy_small_file(source, destination / "ironbank" / source.name, maximum_bytes) - for source in ironbank.glob("*/run-failure/**/*"): - if source.name in set(debug["skip_names"]): - continue - relative = source.relative_to(ironbank) - copy_small_file(source, destination / "ironbank" / relative, maximum_bytes) + # Named even when the glob is empty. A build that never ran and a + # collector that never looked produce the same silence otherwise, and the + # difference is the whole point of preserving evidence. + build_logs = sorted(ironbank.glob("build-*.log")) + if not build_logs: + collected.append({"source": str(ironbank / "build-*.log"), "outcome": "absent"}) + for source in build_logs: + collect(source, destination / "ironbank" / source.name) + + failures = [ + source + for source in sorted(ironbank.glob("*/run-failure/**/*")) + if source.name not in set(debug["skip_names"]) + ] + if not failures: + collected.append({"source": str(ironbank / "*/run-failure"), "outcome": "absent"}) + for source in failures: + collect(source, destination / "ironbank" / source.relative_to(ironbank)) + + (destination / "collected.json").write_text( + json.dumps({"files": collected}, indent=2, sort_keys=True) + "\n" + ) rotate_debug_artifacts(root, debug) print(f"ARTIFACT: preserved release-gate storage evidence at {destination}") diff --git a/tests/test_docker_storage_policy.py b/tests/test_docker_storage_policy.py index b2eef0948..e6c910f35 100644 --- a/tests/test_docker_storage_policy.py +++ b/tests/test_docker_storage_policy.py @@ -372,3 +372,90 @@ def test_bootstrap_and_doctor_share_the_recommended_disk_policy() -> None: assert "minimum_docker_disk_gib" in doctor assert "Colima Docker disk:" in doctor assert "--disk ${recommended_disk_gib}" in doctor + + +def test_the_evidence_bundle_says_what_it_could_not_collect(tmp_path: Path) -> None: + """Silence is the one answer a post-mortem cannot use. + + `copy_small_file` returns quietly for three different outcomes -- the file + was absent, it was over the size cap, or it could not be read -- and the + IronBank globs yield nothing at all on a tree where those builds never + ran. So a preserved bundle gave no way to tell "there were no IronBank + logs" from "the collector never looked" from "the copy failed", and the + gap only became visible during the post-mortem that needed it. + + The bundle now carries a manifest of every source attempted and what + happened to it. + """ + policy_text = POLICY_PATH.read_text().replace( + 'root = "test-artifacts"', f'root = "{tmp_path.as_posix()}"' + ) + policy_path = tmp_path / "policy.toml" + policy_path.write_text(policy_text) + + subprocess.run( + [ + sys.executable, + str(POLICY_SCRIPT), + "--policy", + str(policy_path), + "capture-failure", + "--rail", + "assets", + "--label", + "gap", + "--offline", + ], + cwd=ROOT, + check=True, + capture_output=True, + text=True, + ) + capture_dir = next(tmp_path.glob("*-storage-gap")) + + collected = json.loads((capture_dir / "collected.json").read_text()) + by_source = {entry["source"]: entry for entry in collected["files"]} + + # Every optional source is accounted for by name, present or not. + assert any("ironbank" in source for source in by_source), sorted(by_source) + assert {entry["outcome"] for entry in collected["files"]} <= { + "copied", + "absent", + "truncated", + "unreadable", + }, collected["files"] + # Specifically the globs. Asserting merely that *something* was absent + # passes on `build.log` alone, which is not the gap being closed -- a glob + # that matches nothing is the case that produced no record at all. + ironbank_absent = { + entry["source"] + for entry in collected["files"] + if entry["outcome"] == "absent" and "ironbank" in entry["source"] + } + assert any(source.endswith("build-*.log") for source in ironbank_absent), ( + f"an empty IronBank build-log glob left no record: {sorted(ironbank_absent)}" + ) + assert any(source.endswith("run-failure") for source in ironbank_absent), ( + f"an empty IronBank run-failure glob left no record: {sorted(ironbank_absent)}" + ) + + +def test_an_oversized_log_is_tailed_rather_than_dropped(tmp_path: Path) -> None: + """Because the end of a build log is where the failure is. + + The first bundle written with a collection manifest reported both + `build.log` and `docker-storage.jsonl` as over the cap -- so every bundle + before it had silently omitted the two files a post-mortem reaches for + first, and precisely on the long runs that needed them most. + """ + module = load_policy_module() + source = tmp_path / "build.log" + source.write_bytes(b"discard\n" * 1000 + b"THE ACTUAL FAILURE\n") + destination = tmp_path / "out" / "build.log" + + outcome = module.copy_small_file(source, destination, 256) + + assert outcome == "truncated", outcome + kept = destination.read_bytes() + assert len(kept) <= 256 + assert b"THE ACTUAL FAILURE" in kept, "the tail -- the part that matters -- was lost" From b403b3cf24f5deaacb7ad9cbf9d9fb6d6728a3e8 Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Mon, 3 Aug 2026 20:57:25 -0400 Subject: [PATCH 10/18] feat(gate): make the container boundary a decision instead of an omission Two flags, both previously invisible. `--network` was never passed anywhere, so every container had outbound access because nobody had said otherwise, and several lanes fetch dependencies mid-run. It is now a required keyword with no default: a call site states what it needs, and the two privileged install containers state `bridge` because `dpkg -i || apt-get install -f` and a `pnpm install` genuinely need it until those inputs are baked into the image. `-v :/src` was the other. `Mount` refuses a source inside the checkout now, because that flag let `rust-coverage` churn hardlinks on the host while `linux-rust` read the same inodes through virtiofs -- a race no `contends` declaration could constrain, since the two steps share nothing except a filesystem nobody wrote down. It killed a release run with a `Permission denied` on a file that was `0644` before and `0644` after. Adds what copying instead of mounting needs: `build`, `create`, `start`, `copy_out`, `run_once`, `image_exists`. `--rm` and `docker cp` are mutually exclusive -- a removed container has nothing left to copy from -- which is why extraction is create/start/cp rather than a flag on `run_once`. Two checkout mounts remain and are not hidden. `Mount.unmigrated` is deliberately ugly and greppable, and a test pins the exact set at `{debproof.py: 1, installcontainer.py: 1}`. The alternative was disabling the guard globally during the migration, which is how a temporary exemption becomes the behaviour. A ratchet also pins the nine remaining hand-built docker argv sites by module, so a tenth fails rather than passing unnoticed. Named volumes are exempt by Docker's own rule -- a source with no separator is a volume, not a path -- because resolving one relative to the cwd put it inside the checkout and refused every legitimate cache. 1995 gate tests, 526 release/build-chain contracts. --- CHANGELOG.md | 12 +++ config/gate.toml | 6 ++ src/capsem/gate/debproof.py | 3 +- src/capsem/gate/docker.py | 108 ++++++++++++++++++- src/capsem/gate/installcontainer.py | 3 +- src/capsem/gate/productschema.py | 3 + tests/test_gate_docker.py | 3 +- tests/test_gate_docker_boundary.py | 162 ++++++++++++++++++++++++++++ 8 files changed, 295 insertions(+), 5 deletions(-) create mode 100644 tests/test_gate_docker_boundary.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 4cd8fefb7..940436d8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Containers now declare their network, and a mount of the working tree is + refused. Nothing in the gate passed `--network` at all, so every container + had outbound access by omission and several fetched dependencies mid-run -- + the difference between proving a build reproduces and proving it reproduces + today. `Mount` refuses a source inside the checkout, because + `-v :/src` let a host step churning hardlinks and a container + reading the same inodes over virtiofs share a filesystem neither declared, + which killed a release run with an intermittent `Permission denied` on a + file that was `0644` before and after. The two privileged install + containers still need both and say so through `Mount.unmigrated`, which a + test enumerates so the count can only shrink. + - Every `Call` answers, in a form a machine can read, why it is not an ordinary declared action and what it can affect. It carried a required kind before; now it carries a closed kind, a reason its author wrote, and a declared diff --git a/config/gate.toml b/config/gate.toml index 248ceb0ff..ac3ba9678 100644 --- a/config/gate.toml +++ b/config/gate.toml @@ -88,6 +88,11 @@ proc_stat_template = "/proc/{pid}/stat" # --------------------------------------------------------------------------- [install] +# Explicit, because nothing in the gate passed `--network` at all and every +# container therefore had outbound access by omission. This lane genuinely +# needs it -- `dpkg -i || apt-get install -f`, and a pnpm install -- so it +# says so until those inputs are baked into the image. +network = "bridge" container = "capsem-install-test" image = "capsem-install-test" dockerfile = "docker/Dockerfile.install-test" @@ -952,6 +957,7 @@ blocks_clippy = "frontend" source_contract = [ "tests/test_gate_observation.py", "tests/test_gate_plan_purity.py", + "tests/test_gate_docker_boundary.py", "tests/test_gate_git_worktree_mount.py", "tests/test_rust_filesystem_chokepoint.py", "tests/test_agent_skill_index.py", diff --git a/src/capsem/gate/debproof.py b/src/capsem/gate/debproof.py index 3cbddc431..cb9dd36f9 100644 --- a/src/capsem/gate/debproof.py +++ b/src/capsem/gate/debproof.py @@ -124,6 +124,7 @@ def _start(self, devices: list[str]) -> None: tmpfs = [f for path in self._install.tmpfs_paths for f in ("--tmpfs", path)] self._docker.remove(self._proof.container) self._docker.run_detached( + network=self._install.network, name=self._proof.container, image=self._install.image, command=[self._install.systemd_command], @@ -139,7 +140,7 @@ def _start(self, devices: list[str]) -> None: Mount(cgroup, cgroup, "rw"), # Read-only: this proof must not be able to influence the tree # it is proving. - Mount(str(self.root), self._install.mount, "ro"), + Mount.unmigrated(str(self.root), self._install.mount, "ro"), ], ) await_systemd( diff --git a/src/capsem/gate/docker.py b/src/capsem/gate/docker.py index 7605fde72..3e07aeec8 100644 --- a/src/capsem/gate/docker.py +++ b/src/capsem/gate/docker.py @@ -20,12 +20,59 @@ @dataclass(frozen=True) class Mount: - """A bind mount or named volume, in `-v` order.""" + """A bind mount or named volume, in `-v` order. + + Refuses the checkout. `-v :/src` let a host step churning + hardlinks and a container reading the same inodes over virtiofs share a + filesystem neither declared, which killed a release run with an + intermittent `Permission denied` on a file that was `0644` before and + after. Containers get their source copied into an image instead, so there + is no mount to police. + """ source: str target: str options: str = "" + #: Set only by `unmigrated`. A mount of the checkout is a defect; until the + #: four modules that still do it are converted to `COPY`, each one says so + #: at its call site rather than the guard being switched off globally. + legacy: bool = False + + @classmethod + def unmigrated(cls, source: str, target: str, options: str = "") -> Mount: + """A checkout mount that has not been converted to an image copy yet. + + Deliberately ugly and deliberately greppable. `tests/ + test_gate_docker_boundary.py` counts these and refuses a new one, so + the list can only shrink. + """ + return cls(source=source, target=target, options=options, legacy=True) + + def __post_init__(self) -> None: + if self.legacy: + return + # The checkout this package was imported from -- the same derivation + # `sourcestate.gate_source()` uses, because a `Mount` is constructed + # before any config is in hand and asking for one would put the check + # back at the call sites it exists to remove. + root = Path(__file__).resolve().parents[3] + # Docker's own rule: a source with no separator is a *named volume*, + # not a path. Resolving one relative to the cwd puts it inside the + # checkout and refuses every legitimate cache volume. + if "/" not in self.source: + return + try: + candidate = Path(self.source).resolve() + except (OSError, ValueError): + return + if candidate == root or root in candidate.parents: + raise GateError( + f"{self.source} is inside the checkout: a container that mounts the " + "working tree shares inodes with every host step, which is a race " + "no declaration can constrain. COPY the source into the image." + ) + def __str__(self) -> str: return f"{self.source}:{self.target}" + (f":{self.options}" if self.options else "") @@ -53,15 +100,72 @@ def run_detached( name: str, image: str, command: list[str], + network: str, options: list[str] | None = None, mounts: list[Mount] | None = None, ) -> None: - argv = ["docker", "run", "-d", "--name", name, *(options or [])] + """Start a container in the background. + + `network` has no default on purpose. Nothing in the gate passed + `--network` at all, so every container had outbound access and several + fetched mid-run -- which is the difference between a gate that proves + a build reproduces and one that proves it reproduces today. + """ + argv = ["docker", "run", "-d", "--name", name, "--network", network, *(options or [])] for mount in mounts or []: argv += ["-v", str(mount)] argv += [image, *command] self._runner.run(argv) + def run_once( + self, + *, + image: str, + command: list[str], + network: str, + options: list[str] | None = None, + mounts: list[Mount] | None = None, + check: bool = True, + ) -> None: + """Run a container to completion and remove it.""" + argv = ["docker", "run", "--rm", "--network", network, *(options or [])] + for mount in mounts or []: + argv += ["-v", str(mount)] + argv += [image, *command] + self._runner.run(argv, check=check) + + # -- images ------------------------------------------------------------ + + def build( + self, *, tag: str, dockerfile: str, context: str, args: list[str] | None = None + ) -> None: + """Build an image. The context streams from the CLI, so it does not + have to be visible inside the Lima VM the way a bind mount does.""" + argv = ["docker", "build", "-t", tag, "-f", dockerfile] + for value in args or []: + argv += ["--build-arg", value] + argv.append(context) + self._runner.run(argv) + + def image_exists(self, tag: str) -> bool: + return self._runner.succeeds(["docker", "image", "inspect", tag]) + + # -- extraction -------------------------------------------------------- + + def create(self, *, name: str, image: str, command: list[str]) -> None: + """Create a container without starting it, so `copy_out` has something + to read. `--rm` and `docker cp` are mutually exclusive: a removed + container has nothing left to copy from, which is why extraction + cannot reuse `run_once`.""" + self._runner.run(["docker", "create", "--name", name, image, *command]) + + def start(self, container: str) -> None: + self._runner.run(["docker", "start", "-a", container]) + + def copy_out(self, container: str, source: str, destination: str) -> None: + """Take bytes out of a container without a writable mount.""" + self._runner.run(["docker", "cp", f"{container}:{source}", destination]) + # -- exec -------------------------------------------------------------- def _exec_argv( diff --git a/src/capsem/gate/installcontainer.py b/src/capsem/gate/installcontainer.py index 498a5aec7..f1f7add80 100644 --- a/src/capsem/gate/installcontainer.py +++ b/src/capsem/gate/installcontainer.py @@ -96,13 +96,14 @@ def start(self, *, options: list[str]) -> None: # a Colima OOM, for instance. self._docker.remove(self.name) self._docker.run_detached( + network=self._settings.network, name=self.name, image=self._settings.image, command=[self._settings.systemd_command], options=["--privileged", "--cgroupns=host", *options, *self._tmpfs()], mounts=[ Mount(cgroup, cgroup, "rw"), - Mount(str(self._config.root), self._settings.mount), + Mount.unmigrated(str(self._config.root), self._settings.mount), *(Mount(v.source, v.target) for v in self._settings.volumes), ], ) diff --git a/src/capsem/gate/productschema.py b/src/capsem/gate/productschema.py index 62822f9b4..d454eb8e1 100644 --- a/src/capsem/gate/productschema.py +++ b/src/capsem/gate/productschema.py @@ -78,6 +78,9 @@ class InstallConfig(Strict): dockerfile: str venv: str mount: str + #: Declared rather than defaulted: every container had outbound access by + #: omission, and this lane is the one that genuinely still needs it. + network: str channel: str manifest_version: str systemd_ready_attempts: int diff --git a/tests/test_gate_docker.py b/tests/test_gate_docker.py index 905548c98..a999cc750 100644 --- a/tests/test_gate_docker.py +++ b/tests/test_gate_docker.py @@ -72,6 +72,7 @@ def test_mounts_render_in_docker_order(tmp_path: Path) -> None: name="box", image="capsem-install-test", command=["/usr/lib/systemd/systemd"], + network="bridge", options=["--privileged"], mounts=[ Mount("/sys/fs/cgroup", "/sys/fs/cgroup", "rw"), @@ -80,7 +81,7 @@ def test_mounts_render_in_docker_order(tmp_path: Path) -> None: ) assert runner.rendered[0] == ( - f"docker run -d --name box --privileged " + f"docker run -d --name box --network bridge --privileged " f"-v /sys/fs/cgroup:/sys/fs/cgroup:rw -v {tmp_path}:/src " f"capsem-install-test /usr/lib/systemd/systemd" ) diff --git a/tests/test_gate_docker_boundary.py b/tests/test_gate_docker_boundary.py new file mode 100644 index 000000000..56a210786 --- /dev/null +++ b/tests/test_gate_docker_boundary.py @@ -0,0 +1,162 @@ +"""Containers get their source copied in, not the developer's checkout mounted. + +Every gate container does `-v :/src`, which is two defects wearing +one flag. + +It is an isolation hole: the container can read and often write the live +checkout, so "the gate runs in a container" says nothing about what the +container can reach. + +And it is a *race*. `rust-coverage` runs on the host and churns hardlinks +inside that tree while `linux-rust` reads the same inodes through virtiofs. +A release run died on it -- `Permission denied` opening +`config/profiles/code/root/root/.gemini/projects.json`, a file that was `0644` +before and `0644` after -- and no amount of declaring `contends` would have +prevented it, because the two steps genuinely share nothing except the +filesystem nobody wrote down. + +Copying the source into an image removes the class rather than guarding it. +There is no mount to police, no host path to rewrite, and nothing for two +steps to contend over: each container holds its own bytes. + +These are argv-level assertions on purpose. The mount is a flag, the network +mode is a flag, and a flag is what a future change will quietly reintroduce. +""" + +from __future__ import annotations + +import ast +from pathlib import Path + +PROJECT_ROOT = Path(__file__).resolve().parents[1] +GATE = PROJECT_ROOT / "src" / "capsem" / "gate" + +#: The one module allowed to spell `docker` as a command name. Everything else +#: asks it, so that mounts and network mode have a single place to be decided. +WRAPPER = {"docker.py", "dockerimage.py"} + + +def _modules() -> list[Path]: + return sorted(path for path in GATE.glob("*.py") if path.name not in WRAPPER) + + +def _docker_argv_literals(tree: ast.AST) -> list[ast.List]: + """Every list literal whose first element is the string `docker`.""" + found: list[ast.List] = [] + for node in ast.walk(tree): + if not isinstance(node, ast.List) or not node.elts: + continue + first = node.elts[0] + if isinstance(first, ast.Constant) and first.value == "docker": + found.append(node) + return found + + +#: Modules that still build docker argv by hand. A ratchet rather than an +#: `xfail`: an `xfail` says "this is broken" and hides how broken, whereas +#: this refuses a tenth site while the nine are migrated. Each removal here is +#: a module that can no longer choose its own mount or network mode. +UNMIGRATED = { + "crossexec.py": 1, + "hostimage.py": 5, + "installimage.py": 2, + "packagerail.py": 1, +} + + +def test_no_new_module_builds_docker_argv_by_hand() -> None: + """One place decides the flags, or every call site decides them again. + + The mount, the network mode and the removal policy have to be identical + everywhere; spread across four modules they were made differently in nine + places -- including the `-v :/src` that raced a host step and + killed a release run. + """ + counted: dict[str, int] = {} + for path in _modules(): + tree = ast.parse(path.read_text(encoding="utf-8")) + found = _docker_argv_literals(tree) + if found: + counted[path.name] = len(found) + + new = {name: count for name, count in counted.items() if name not in UNMIGRATED} + assert not new, f"new modules building docker argv directly: {new}" + + grown = { + name: (count, UNMIGRATED[name]) + for name, count in counted.items() + if count > UNMIGRATED[name] + } + assert not grown, f"these grew new hand-built docker argv (now, allowed): {grown}" + + # And the debt only shrinks: a migrated module leaves the list. + assert set(counted) <= set(UNMIGRATED), sorted(set(counted) - set(UNMIGRATED)) + + +def test_a_mount_cannot_point_at_the_checkout() -> None: + """The specific hole: `-v :/src`. + + Refused at construction rather than reviewed, because this is the flag + that made a host step and a container step share inodes. + """ + import pytest + + from capsem.gate import config as gate_config + from capsem.gate.docker import Mount + from capsem.gate.errors import GateError + + root = gate_config.load(PROJECT_ROOT).root + + with pytest.raises(GateError, match="checkout"): + Mount(source=str(root), target="/src") + + with pytest.raises(GateError, match="checkout"): + Mount(source=str(root / "config"), target="/src/config") + + # A named volume and a path outside the checkout stay legal: this refuses + # the checkout, not mounting. + assert Mount(source="capsem-cargo-registry", target="/usr/local/cargo/registry") + assert Mount(source="/tmp/capsem-scratch", target="/scratch") + + +def test_every_container_declares_its_network() -> None: + """No default, so `--network` is a decision rather than an omission. + + Nothing in the gate passes `--network` today, which means every container + has outbound access and several use it mid-run. A required keyword makes + that visible at each call site instead of invisible everywhere. + """ + import inspect + + from capsem.gate.docker import Docker + + for name in ("run_detached", "run_once"): + method = getattr(Docker, name, None) + assert method is not None, f"Docker.{name} is missing" + parameter = inspect.signature(method).parameters.get("network") + assert parameter is not None, f"Docker.{name} does not take a network mode" + assert parameter.default is inspect.Parameter.empty, ( + f"Docker.{name} defaults its network mode, so a call site can omit " + "the decision and get outbound access without saying so" + ) + + +def test_the_checkout_mounts_are_enumerated_and_shrinking() -> None: + """Two mounts of the working tree remain, and both say so at the call site. + + `Mount.unmigrated` is deliberately ugly and deliberately greppable. The + alternative was switching the guard off globally while the four modules + are converted, which is how a temporary exemption becomes the permanent + behaviour. + """ + remaining: dict[str, int] = {} + for path in _modules(): + count = path.read_text(encoding="utf-8").count("Mount.unmigrated(") + if count: + remaining[path.name] = count + + assert remaining == {"debproof.py": 1, "installcontainer.py": 1}, ( + f"the checkout-mount debt changed: {remaining}. It may shrink -- update " + "this expectation when a module moves to COPY -- but a new one is the " + "race that killed a release run coming back." + ) From 630bb90b2139bdb207c72016dda60901ef2c76d7 Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Tue, 4 Aug 2026 18:05:17 -0400 Subject: [PATCH 11/18] feat(gate): give the Linux parity lane its own bytes, and seal it The lane bind-mounted the live checkout, grafted two writable mounts back through it to retrieve coverage, inherited four named volumes that survive between runs, and ran with outbound network because nothing in the gate ever passed `--network`. A release died here on an intermittent `Permission denied` reading a file that was `0644` before and after, because `rust-coverage` was churning hardlinks in the tree this lane was reading. It now builds its source into an image, runs `--network none`, and returns coverage through `docker cp`. Nothing is shared, so nothing can be raced; nothing is inherited, so a cold machine and a warm one run the same thing. `cache-ownership`, `output-ownership` and `linux-rust-mountpoints` are gone -- they existed only to repair what root-owned shared state left behind. Sealing it found two fetches nobody had written down. The lane ran `pnpm install` mid-run whenever `frontend/dist` was missing, which it always is inside an image that excludes build output. And `ort` -- ONNX Runtime, pulled in by `magika` for file typing -- downloads a binary from `cdn.pyke.io` inside a build script on every cold build. Cache warming cannot fix the second: a build script re-runs on any fingerprint miss and coverage instrumentation changes every fingerprint. Both now come from the image, with ONNX Runtime taken from Microsoft's official release under `ORT_STRATEGY=system` and `ORT_PREFER_DYNAMIC_LINK=1` (it defaults to static and wants a single-file archive that release does not ship). Proof: with the base image built, `capsem-gate linux-rust` exits 0 in 106s and produces `codecov-linux.json` at 844182 bytes -- the same size as the warm bind-mount lane, over an identical set of 165 files, with 162 of them byte-identical in coverage. Three differ (`capsem-agent/src/main.rs`, `capsem-logger/src/reader.rs`, `capsem-logger/src/schema.rs`); that is consistent with run-to-run nondeterminism rather than a different proof, but it is not the byte-identical result this phase set out to get and is worth checking. `scripts/test-linux-rust.sh` is deliberately untouched: the concurrent test refactoring owns it, and `CAPSEM_LINUX_RUST_OUTPUT_DIR` already parameterizes where it writes. 2015 gate tests. --- .dockerignore | 33 +++--- CHANGELOG.md | 17 +++ config/gate.toml | 23 ++++ docker/Dockerfile.linux-rust | 13 +++ docker/Dockerfile.linux-rust-base | 111 ++++++++++++++++++ src/capsem/gate/buildschema.py | 10 ++ src/capsem/gate/docker.py | 16 ++- src/capsem/gate/hostimage.py | 102 ++-------------- src/capsem/gate/linuxrust.py | 124 ++++++++++++++++++++ tests/test_gate_hostimage_composition.py | 40 ++++--- tests/test_gate_linuxrust_hermetic.py | 143 +++++++++++++++++++++++ 11 files changed, 510 insertions(+), 122 deletions(-) create mode 100644 docker/Dockerfile.linux-rust create mode 100644 docker/Dockerfile.linux-rust-base create mode 100644 src/capsem/gate/linuxrust.py create mode 100644 tests/test_gate_linuxrust_hermetic.py diff --git a/.dockerignore b/.dockerignore index fe713d949..343c2cbc1 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,16 +1,17 @@ -# Ignore massive host build artifacts -target/ -node_modules/ -.venv/ -.pytest_cache/ -.ruff_cache/ -.astro/ -coverage/ -dist/ -site/node_modules/ -frontend/node_modules/ -frontend/dist/ -*.db -*.sqlite -*.log -.DS_Store +# Build context for the thin lane image. +# +# Patterns are `**`-prefixed deliberately. A bare `target` matches only the +# root, and this repository contains agent worktrees under `.claude/worktrees/` +# that carry their own multi-gigabyte `target/` trees -- the first build with a +# root-only pattern swept one in and failed with `no space left on device` +# after nineteen minutes. +**/target +**/node_modules +**/.venv +**/dist +**/assets +**/packages +.claude +.git +**/.pnpm-store +**/test-artifacts diff --git a/CHANGELOG.md b/CHANGELOG.md index 940436d8c..77d9036c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- The Linux parity lane holds its own bytes. It bind-mounted the live checkout, + grafted two writable mounts back through it to retrieve coverage, inherited + four named volumes that survive between runs, and ran with outbound network + because nothing ever passed `--network`. It now builds its source into an + image, runs with `--network none`, and returns coverage through `docker cp`. + Dependencies live in a base image keyed by `Cargo.lock`, + `rust-toolchain.toml` and `frontend/pnpm-lock.yaml`; a lockfile change makes + a new tag and the gate refuses to start rather than rebuilding multiple + gigabytes at minute four. + + Sealing it surfaced two fetches nobody had recorded. The lane built the + frontend with `pnpm install` mid-run whenever `frontend/dist` was absent, and + `ort` -- ONNX Runtime, under `magika` -- downloaded a binary from + `cdn.pyke.io` inside a build script on every cold build. Both now come from + the image: the frontend is built there, and ONNX Runtime is Microsoft's + official release with `ORT_STRATEGY=system` and `ORT_PREFER_DYNAMIC_LINK`. + - Containers now declare their network, and a mount of the working tree is refused. Nothing in the gate passed `--network` at all, so every container had outbound access by omission and several fetched dependencies mid-run -- diff --git a/config/gate.toml b/config/gate.toml index ac3ba9678..5d4390733 100644 --- a/config/gate.toml +++ b/config/gate.toml @@ -827,6 +827,28 @@ ready_interval_seconds = 0.5 log_level = "capsem=debug" [hostimage] +# The Linux parity lane's dependencies, resolved once with network and then +# never again. These four named volumes used to hold them and persisted +# between runs, which is what made a warm machine and a clean checkout +# disagree about the same commit. +base_dockerfile = "docker/Dockerfile.linux-rust-base" +lane_dockerfile = "docker/Dockerfile.linux-rust" +base_tag_template = "capsem-linux-rust-base:{digest}" +lane_tag = "capsem-linux-rust:latest" +# What decides the dependency set, and therefore the base image's identity. +lockfile_inputs = ["Cargo.lock", "rust-toolchain.toml", "frontend/pnpm-lock.yaml"] +# Denied, which is what proves the base image is complete rather than believed +# to be complete. +network = "none" +# Copied out with `docker cp`, because `--rm` and `docker cp` are mutually +# exclusive and a writable bind mount is the thing being removed. +container_output_dir = "/linux-rust-output" +# The `docker cp` source. The trailing `/.` copies the directory's *contents*; +# without it the coverage nests at `/linux-rust-output/` where nothing +# looks for it. +container_output_contents = "/linux-rust-output/." +extract_to = "target/linux-rust-coverage" +lane_container = "capsem-linux-rust-run" # The Linux builder image, and the Linux-Rust parity lane that runs inside it. tag = "capsem-host-builder:latest" dockerfile = "docker/Dockerfile.host-builder" @@ -958,6 +980,7 @@ source_contract = [ "tests/test_gate_observation.py", "tests/test_gate_plan_purity.py", "tests/test_gate_docker_boundary.py", + "tests/test_gate_linuxrust_hermetic.py", "tests/test_gate_git_worktree_mount.py", "tests/test_rust_filesystem_chokepoint.py", "tests/test_agent_skill_index.py", diff --git a/docker/Dockerfile.linux-rust b/docker/Dockerfile.linux-rust new file mode 100644 index 000000000..cf70ca351 --- /dev/null +++ b/docker/Dockerfile.linux-rust @@ -0,0 +1,13 @@ +# The lane's source, copied rather than mounted. +# +# `-v :/src` let a host step churning hardlinks and this container +# reading the same inodes over virtiofs share a filesystem neither declared, +# and a release run died on the resulting intermittent `Permission denied`. +# A copy cannot be raced: the container holds its own bytes. +ARG BASE +FROM ${BASE} + +COPY --chown=1000:1000 . /src +WORKDIR /src +USER 1000:1000 +ENV HOME=/home/lane diff --git a/docker/Dockerfile.linux-rust-base b/docker/Dockerfile.linux-rust-base new file mode 100644 index 000000000..2ce88196a --- /dev/null +++ b/docker/Dockerfile.linux-rust-base @@ -0,0 +1,111 @@ +# The Linux parity lane's dependencies, resolved once and never at run time. +# +# The lane used to carry four named Docker volumes -- cargo registry, cargo +# git, rustup, and an 11 GB target -- which persisted between runs. That is +# what made a warm machine and a cold CI runner disagree about the same +# commit, and it is why `just test` could pass locally on state no clean +# checkout has. +# +# Everything those volumes held is baked here instead, keyed by the lockfiles +# that determine it. A dependency change produces a different tag; an +# unchanged tree reuses the image. `just warm` builds it with network, and the +# lane itself then runs with `--network none`, which is what proves this file +# is complete rather than merely believed to be. +ARG BASE=capsem-host-builder:latest +FROM ${BASE} + +# Only the files that decide the dependency set. Copying the whole tree here +# would rebuild this image on every source edit and defeat the caching it +# exists for. +COPY Cargo.toml Cargo.lock rust-toolchain.toml /src/ +COPY crates /src/crates + +WORKDIR /src + +# ONNX Runtime, from Microsoft, installed rather than downloaded at build time. +# +# `capsem-service` depends on `magika`, which runs its model on `ort`. With +# `ort`'s `download-binaries` feature that pulls a custom-packaged blob from +# `cdn.pyke.io` inside a build script -- and a build script re-runs on any +# fingerprint miss, so no amount of cargo cache warming makes it reliable. It +# is also a third-party CDN in the build path that nothing in the repo +# recorded, which `--network none` is what surfaced. +# +# `ORT_STRATEGY=system` points the crate at a library that is simply present. +# The version tracks what `ort 2.0.0-rc.11` expects; a mismatch fails loudly +# here, with network, rather than inside a sealed lane. +ARG ORT_VERSION=1.23.2 +ARG TARGETARCH=arm64 +RUN set -eux; \ + case "$TARGETARCH" in \ + arm64) ort_arch=aarch64 ;; \ + amd64) ort_arch=x64 ;; \ + *) echo "unsupported TARGETARCH=$TARGETARCH" >&2; exit 1 ;; \ + esac; \ + url="https://github.com/microsoft/onnxruntime/releases/download/v${ORT_VERSION}/onnxruntime-linux-${ort_arch}-${ORT_VERSION}.tgz"; \ + curl -fsSL "$url" -o /tmp/ort.tgz; \ + mkdir -p /opt/onnxruntime; \ + tar -xzf /tmp/ort.tgz -C /opt/onnxruntime --strip-components=1; \ + rm /tmp/ort.tgz; \ + test -e /opt/onnxruntime/lib/libonnxruntime.so +# Dynamic, because Microsoft ships shared `.so` libraries and `ort-sys` +# otherwise "does full static linking since no single-file library was found" +# and fails. The build script names this switch in its own debug output. +ENV ORT_PREFER_DYNAMIC_LINK=1 +ENV ORT_STRATEGY=system +ENV ORT_LIB_LOCATION=/opt/onnxruntime/lib +ENV LD_LIBRARY_PATH=/opt/onnxruntime/lib + +# `--locked` so a resolution difference fails here, with network, rather than +# inside a lane that has none. +RUN cargo fetch --locked + +# The dependency graph, compiled. `cargo fetch` gets the sources; this gets +# the artifacts, which is the part that made a cold run 135s instead of 58s. +RUN cargo build --locked --workspace --all-targets || true + +# Warmed a second time with the coverage instrumentation the lane actually +# uses. `cargo llvm-cov show-env` is the documented way to get exactly the +# RUSTFLAGS and target directory the coverage run will use, so a build under +# them produces artifacts the lane reuses rather than discards. +# +# This matters because `ort-sys` downloads prebuilt ONNX Runtime from +# cdn.pyke.io inside its build script (`capsem-service` depends on `ort` with +# `download-binaries`). A build script re-runs whenever its fingerprint +# misses, and instrumentation changes every fingerprint -- so warming without +# these flags left the lane rebuilding it, reaching for a CDN, and failing +# under `--network none`. +# +# An earlier attempt used `--no-run`, which is deprecated: it exited in 0.3s +# with "not found *.profraw files" and `|| true` swallowed the failure, so +# nothing was compiled at all and three subsequent "fixes" were built on a +# step that had silently done nothing. Hence `echo` rather than `true`. +# +# Worth recording as a supply-chain fact: this lane has been silently pulling +# a binary from cdn.pyke.io on every cold build, and `--network none` is what +# made it visible. +RUN bash -lc 'eval "$(cargo llvm-cov show-env --export-prefix)" \ + && cargo build --locked --workspace --all-targets' \ + || echo "prewarm incomplete; the lane will surface anything missing" + +# `capsem-app` embeds `frontend/dist` at compile time, so +# `scripts/test-linux-rust.sh` builds the frontend when that directory is +# missing -- with `pnpm install`, over the network. Sealed, that is a hard +# failure, which is how it was found: every previous run of this lane had been +# quietly pulling from npm. Built here instead, with network, like every other +# dependency; the lane's own check then sees `frontend/dist/index.html` and +# skips the fetch entirely. +COPY frontend /src/frontend +RUN cd /src/frontend \ + && CI=true pnpm install --frozen-lockfile \ + && pnpm run build \ + && test -s /src/frontend/dist/index.html + +# Ownership last, so it covers everything every step above created as root. +# A non-root user is required: the suite chmods an asset to 0o000 and demands +# the read fail, and root ignores permissions. `ubuntu:24.04` already ships a +# uid-1000 account, so this uses the numeric id rather than creating one -- +# `useradd --uid 1000` exits 4 on a UID that already exists. +RUN mkdir -p /cargo-target /home/lane /linux-rust-output \ + && chown -R 1000:1000 /src /cargo-target /home/lane /linux-rust-output \ + /usr/local/cargo /usr/local/rustup diff --git a/src/capsem/gate/buildschema.py b/src/capsem/gate/buildschema.py index ef67a0c4d..4f4704444 100644 --- a/src/capsem/gate/buildschema.py +++ b/src/capsem/gate/buildschema.py @@ -73,6 +73,16 @@ class NamedVolume(Strict): class HostImageConfig(Strict): + base_dockerfile: str + lane_dockerfile: str + base_tag_template: str + lane_tag: str + lockfile_inputs: tuple[str, ...] + network: str + container_output_dir: str + container_output_contents: str + extract_to: str + lane_container: str tag: str dockerfile: str context: str diff --git a/src/capsem/gate/docker.py b/src/capsem/gate/docker.py index 3e07aeec8..c57fa6a74 100644 --- a/src/capsem/gate/docker.py +++ b/src/capsem/gate/docker.py @@ -152,12 +152,24 @@ def image_exists(self, tag: str) -> bool: # -- extraction -------------------------------------------------------- - def create(self, *, name: str, image: str, command: list[str]) -> None: + def create( + self, + *, + name: str, + image: str, + command: list[str], + network: str, + env: dict[str, str] | None = None, + ) -> None: """Create a container without starting it, so `copy_out` has something to read. `--rm` and `docker cp` are mutually exclusive: a removed container has nothing left to copy from, which is why extraction cannot reuse `run_once`.""" - self._runner.run(["docker", "create", "--name", name, image, *command]) + argv = ["docker", "create", "--name", name, "--network", network] + for key, value in (env or {}).items(): + argv += ["-e", f"{key}={value}"] + argv += [image, *command] + self._runner.run(argv) def start(self, container: str) -> None: self._runner.run(["docker", "start", "-a", container]) diff --git a/src/capsem/gate/hostimage.py b/src/capsem/gate/hostimage.py index 993769e28..c1cea60f1 100644 --- a/src/capsem/gate/hostimage.py +++ b/src/capsem/gate/hostimage.py @@ -15,17 +15,15 @@ from __future__ import annotations -import os from pathlib import Path -from . import host +from . import host, linuxrust from .actions import Action, Run from .command import GateCommand from .config import GateConfig from .context import Context from .errors import GateError from .execution import Step, step -from .fileactions import MakeDir from .gitmetadata import docker_git_metadata_mount from .plan import Plan @@ -129,7 +127,6 @@ def __init__(self, plan: Plan, config: GateConfig) -> None: def build(self, after: tuple[Step, ...]) -> Step: config = self._config - settings = config.hostimage plan = self._plan if host.on_linux(): @@ -137,97 +134,20 @@ def build(self, after: tuple[Step, ...]) -> Step: step( "linux-rust", Run( - ["bash", settings.script], + ["bash", config.hostimage.script], env={config.environment.linux_rust.output_dir: str(config.root)}, ), ), after=after, ) - if not host.on_macos(): - raise GateError("Linux Rust parity runs natively on Linux or in Docker on macOS") - - built = fragment(plan, config, after=after) - output = config.path(settings.output_dir) - uid, gid = os.getuid(), os.getgid() - docker = config.exclusive("docker_daemon") - - # The cached volumes belong to root until they are handed over; the - # suite then runs as the host user, because running it as container - # root makes chmod-based permission regressions impossible to observe. - owned = plan.add( - step( - "cache-ownership", - Run( - [ - "docker", - "run", - "--rm", - *_volumes(config), - settings.tag, - "sh", - "-c", - f"chown -R {uid}:{gid} " - + " ".join(v.target for v in settings.cached_volumes), - ], - ), - contends=(docker,), - ), - after=(built,), - ) - - mountpoints = plan.add( - step( - "linux-rust-mountpoints", - MakeDir(config.path(settings.nextest_mount)), - MakeDir(output / settings.nextest_dir), - *( - action - for volume in settings.writable_source_mounts - for action in ( - MakeDir(config.path(volume.source)), - MakeDir(config.path(volume.target)), - ) - ), - ), - after=(owned,), - ) - - suite = plan.add( - step( - "linux-rust", - _LinuxRustSuite( - output, - source=config.root, - mount=settings.mount, - script=settings.script, - ), - contends=(docker,), - ), - after=(mountpoints,), - ) - - return plan.add( - step( - "output-ownership", - Run( - [ - "docker", - "run", - "--rm", - "-v", - f"{output}:{settings.container_output}", - settings.alpine, - "chown", - "-R", - f"{uid}:{gid}", - settings.container_output, - ] - ), - contends=(docker,), - ), - after=(suite,), - ) + # macOS: the same checked-in script, in a container that holds its own + # copy of the source. `cache-ownership`, `linux-rust-mountpoints` and + # `output-ownership` are gone with the mounts and volumes that + # required them -- they existed only to repair what root-owned shared + # state left behind. + built = plan.shared(image(config), after=after) + return linuxrust.lane(plan, config, after=(built,)) class _LinuxRustSuite(Action, name="linux-rust-suite"): @@ -271,8 +191,7 @@ def perform(self, context: Context) -> None: for volume in settings.writable_source_mounts for flag in ( "-v", - f"{context.config.path(volume.source)}:" - f"{settings.mount}/{volume.target}", + f"{context.config.path(volume.source)}:{settings.mount}/{volume.target}", ) ], *_volumes(context.config), @@ -284,6 +203,7 @@ def perform(self, context: Context) -> None: ] ) + class LinuxRustCommand( GateCommand, name="linux-rust", diff --git a/src/capsem/gate/linuxrust.py b/src/capsem/gate/linuxrust.py new file mode 100644 index 000000000..e86428761 --- /dev/null +++ b/src/capsem/gate/linuxrust.py @@ -0,0 +1,124 @@ +"""The Linux parity lane, holding its own bytes. + +Native Linux exercises the `cfg(target_os = "linux")` branches directly; a Mac +host runs the same checked-in script in Docker, or Linux-only regressions stay +out of the local gate entirely. + +What changed, and why it is not a refactor. The lane bind-mounted the live +checkout read-only, grafted two writable mounts back through it to retrieve +coverage, and inherited four named volumes that survive between runs. That +combination is what let a warm machine and a clean checkout disagree about one +commit, and the mount is what raced a host step churning hardlinks in the same +tree -- a release died here on an intermittent `Permission denied` reading a +file that was `0644` before and after. + +Now: dependencies live in a base image keyed by the lockfiles that determine +them, the source is copied into a thin image on top, the container runs with +`--network none`, and the coverage comes back through `docker cp`. Nothing is +shared, so nothing can be raced, and nothing is inherited, so a cold machine +and a warm one run the same thing. +""" + +from __future__ import annotations + +import hashlib + +from .actions import Action +from .config import GateConfig +from .context import Context +from .docker import Docker +from .errors import GateError +from .execution import Step, step +from .filesystem import make_dir +from .plan import Plan + + +def base_tag(config: GateConfig) -> str: + """The base image's identity: its dependency inputs, hashed. + + Keyed by content rather than by channel or date, so a dependency change + cannot reuse a stale image and an unchanged tree cannot be forced to + rebuild one. + """ + settings = config.hostimage + digest = hashlib.blake2b(digest_size=8) + for name in settings.lockfile_inputs: + path = config.path(name) + if not path.is_file(): + raise GateError(f"lockfile input {name} is missing, so the base image has no identity") + digest.update(path.read_bytes()) + return settings.base_tag_template.format(digest=digest.hexdigest()) + + +def require_base(config: GateConfig, docker: Docker) -> str: + """Refuse to start rather than rebuild a multi-gigabyte image mid-gate. + + A `Cargo.lock` bump changes the tag. Building it here would turn a cached + five-minute fetch into a forty-minute surprise at minute four, with + network, inside a lane that is supposed to have none. + """ + tag = base_tag(config) + if not docker.image_exists(tag): + raise GateError( + f"no Linux parity base image for {tag}. Its dependencies changed; " + f"run `just warm` to build it with network before the gate runs " + "without." + ) + return tag + + +class RunLane(Action, name="linux-rust-lane"): + """Build the source into an image, run it sealed, copy the coverage out. + + One action rather than five steps because the container is a single + resource with a lifetime: it must be removed on every path, and the copy + must happen before the removal. Splitting that across steps would put the + ordering in the graph, where a reshuffle can break it silently, instead of + in a `finally` where it cannot. + """ + + def render(self) -> str: + return "build, run and extract the Linux parity lane with no mounts and no network" + + def perform(self, context: Context) -> None: + config = context.config + settings = config.hostimage + docker = Docker(context.runner) + + base = require_base(config, docker) + docker.build( + tag=settings.lane_tag, + dockerfile=config.path(settings.lane_dockerfile).as_posix(), + context=str(config.root), + args=[f"BASE={base}"], + ) + + container = settings.lane_container + destination = config.path(settings.extract_to) + docker.remove(container) + docker.create( + name=container, + image=settings.lane_tag, + command=["bash", settings.script], + network=settings.network, + env={config.environment.linux_rust.output_dir: settings.container_output_dir}, + ) + try: + docker.start(container) + finally: + # Before the removal, and on the failure path too: a lane that + # fails is exactly when its coverage and nextest output are worth + # having, and `--rm` would have destroyed both. + make_dir(destination) + # Contents, not the directory: `docker cp` nests otherwise, and the + # coverage lands where nothing looks for it. + docker.copy_out(container, settings.container_output_contents, str(destination)) + docker.remove(container) + + +def lane(plan: Plan, config: GateConfig, *, after: tuple[Step, ...] = ()) -> Step: + """Compose the parity lane into a plan.""" + return plan.add( + step("linux-rust", RunLane(), contends=(config.exclusive("docker_daemon"),)), + after=after, + ) diff --git a/tests/test_gate_hostimage_composition.py b/tests/test_gate_hostimage_composition.py index 71e082422..5e149cef6 100644 --- a/tests/test_gate_hostimage_composition.py +++ b/tests/test_gate_hostimage_composition.py @@ -139,20 +139,34 @@ def test_linux_rust_suite_mounts_linked_worktree_metadata( assert f"-v {source}:{target}" in suite -def test_linux_rust_materializes_nested_mountpoints_before_read_only_source() -> None: - plan = _plan("linux-rust") - order = list(plan.labels) +def test_the_lane_has_somewhere_to_put_its_output_before_it_runs() -> None: + """Reimplemented, not deleted. The claim survives; its mechanism does not. + + This asserted that nested mountpoints were materialized before the + read-only source mount, because grafting a writable path through a `:ro` + mount fails if the directory underneath does not already exist. There are + no mounts now -- the lane copies its source into an image and copies its + coverage back out -- so the equivalent claim is that the destination + exists before `docker cp` writes into it, and that the copy happens on the + failure path too, since a lane that failed is exactly when its coverage is + worth having. + """ + from capsem.gate import linuxrust - assert order.index("linux-rust-mountpoints") < order.index("linux-rust") - mountpoints = next(step for step in plan.steps if step.label == "linux-rust-mountpoints") - rendered = "\n".join(action.render() for action in mountpoints.actions) - assert str(CONFIG.path(CONFIG.hostimage.nextest_mount)) in rendered - assert str( - CONFIG.path(CONFIG.hostimage.output_dir) / CONFIG.hostimage.nextest_dir - ) in rendered - for volume in CONFIG.hostimage.writable_source_mounts: - assert str(CONFIG.path(volume.source)) in rendered - assert str(CONFIG.path(volume.target)) in rendered + source = (PROJECT_ROOT / "src/capsem/gate/linuxrust.py").read_text(encoding="utf-8") + body = source[source.index("def perform") :] + + assert body.index("make_dir(destination)") < body.index("copy_out("), ( + "the coverage destination is created after the copy that writes to it" + ) + assert "finally:" in body and body.index("finally:") < body.index("copy_out("), ( + "the extraction is not on the failure path, so a failed lane loses the " + "coverage that explains it" + ) + assert body.index("copy_out(") < body.rindex("remove("), ( + "the container is removed before its output is copied out" + ) + assert linuxrust.RunLane().render() def test_chained_lanes_do_not_make_the_builder_depend_on_them() -> None: diff --git a/tests/test_gate_linuxrust_hermetic.py b/tests/test_gate_linuxrust_hermetic.py new file mode 100644 index 000000000..0c204e333 --- /dev/null +++ b/tests/test_gate_linuxrust_hermetic.py @@ -0,0 +1,143 @@ +"""The Linux parity lane holds its own bytes. + +It carried every defect this work exists for: it bind-mounted the live +checkout read-only, grafted two writable mounts through that to get output +back, depended on four named volumes that persist between runs, and ran with +outbound network because nothing ever passed `--network`. A release died in +this lane with `Permission denied` on a file that was `0644` before and after, +because `rust-coverage` was churning hardlinks in the tree it was reading. + +Copying the source into an image removes all four at once. There is no mount +to race over, no volume to inherit, and the dependencies live in a base image +keyed by the lockfiles that determine them. +""" + +from __future__ import annotations + +from pathlib import Path + +PROJECT_ROOT = Path(__file__).resolve().parents[1] + + +def _issued() -> str: + import sys + + sys.path.insert(0, str(PROJECT_ROOT / "tests")) + from helpers.gate import gate_issued + + return gate_issued("linux-rust") + + +def test_the_lane_mounts_nothing() -> None: + """No `-v` at all. Not a rewritten mount -- none.""" + issued = _issued() + docker_lines = [line for line in issued.splitlines() if line.startswith("docker ")] + assert docker_lines, f"no docker command was issued:\n{issued}" + mounted = [line for line in docker_lines if " -v " in line] + assert not mounted, "the lane still mounts something:\n " + "\n ".join(mounted) + + +def test_the_lane_runs_with_no_network() -> None: + """It compiles and runs tests. Fetching mid-run is what the base image is + for, and denying it is what proves the base image is complete.""" + issued = _issued() + # `docker create` rather than `docker run`: the container has to outlive + # its own exit so the coverage can be copied out of it. + created = [line for line in issued.splitlines() if line.startswith("docker create")] + assert created, f"the lane does not create a container:\n{issued}" + for line in created: + assert "--network none" in line, f"the lane can still reach the network: {line}" + + +def test_the_base_image_is_keyed_by_the_lockfiles() -> None: + """A dependency change must produce a different tag, or a stale base image + silently qualifies the wrong dependency set.""" + from capsem.gate import config as gate_config + from capsem.gate import linuxrust + + config = gate_config.load(PROJECT_ROOT) + first = linuxrust.base_tag(config) + assert ":" in first, first + + # The digest covers the files that determine the dependencies. + inputs = [config.path(name) for name in config.hostimage.lockfile_inputs] + assert inputs, "no lockfile inputs are configured" + assert all(path.is_file() for path in inputs), [str(p) for p in inputs if not p.is_file()] + + +def test_the_ownership_steps_are_gone() -> None: + """`cache-ownership` and `output-ownership` existed only because root-owned + volumes and bind mounts left files the host could not read. Without either, + they are ceremony -- and a ratchet keeps them from coming back.""" + from helpers.gate import gate_labels + + labels = set(gate_labels("test-static")) | set(gate_labels("linux-rust")) + assert "cache-ownership" not in labels, sorted(labels) + assert "output-ownership" not in labels, sorted(labels) + + +def test_the_coverage_output_is_copied_out_before_the_container_is_removed() -> None: + """`--rm` and `docker cp` are mutually exclusive: a removed container has + nothing left to copy from. The edge is the assertion.""" + issued = _issued() + lines = issued.splitlines() + + def last_index_of(fragment: str) -> int: + for position in reversed(range(len(lines))): + if fragment in lines[position]: + return position + raise AssertionError(f"{fragment!r} was never issued:\n{issued}") + + # The *last* removal: the lane also removes a predecessor before creating + # its own, and comparing against that one would pass while the teardown + # still destroyed the evidence. + assert last_index_of("docker cp") < last_index_of("docker rm"), issued + + +def test_the_build_context_is_bounded() -> None: + """The image copies the source, so `.dockerignore` decides what "source" is. + + A bare `target` pattern matches only the repository root. This checkout + carries agent worktrees under `.claude/` -- 55 GB of them, each with its + own `target/` -- so the first build swept them in and died with `no space + left on device` after nineteen minutes. Every exclusion is `**`-prefixed + now, and this asserts the outcome rather than the patterns: a new cache + directory nobody thought to exclude fails here in a second instead of + filling the Docker disk. + """ + import fnmatch + + patterns = [ + line.strip() + for line in (PROJECT_ROOT / ".dockerignore").read_text(encoding="utf-8").splitlines() + if line.strip() and not line.startswith("#") + ] + + def ignored(relative: str) -> bool: + parts = relative.split("/") + for pattern in patterns: + bare = pattern.removeprefix("**/") + if pattern.startswith("**/"): + if any(fnmatch.fnmatch(segment, bare) for segment in parts): + return True + elif fnmatch.fnmatch(parts[0], bare): + return True + return False + + total = 0 + for path in PROJECT_ROOT.rglob("*"): + relative = str(path.relative_to(PROJECT_ROOT)) + if ignored(relative): + continue + try: + if path.is_file(): + total += path.stat().st_size + except OSError: + continue + + megabytes = total / 1048576 + assert megabytes < 400, ( + f"the docker build context is {megabytes:.0f} MB. Something large is no " + "longer excluded; a build with this context fills the Docker disk " + "rather than failing fast." + ) From 7805bd8e52003a93eea5bdd777301f47b4ab8a91 Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Tue, 4 Aug 2026 20:13:10 -0400 Subject: [PATCH 12/18] chore(gate): remove what the sealed lane stopped needing `just release-profile nightly code` fail-stopped in `contracts.release` on `test_just_test_owns_linux_rust_platform_coverage_through_docker`, which asserts `docker run --rm` is how the parity lane runs. It is not, since 630bb90b: the lane creates a container, starts it, copies coverage out and removes it. Six assertions in that test pinned the mechanism rather than the property, and I had run only `tests/test_gate_*.py` before committing the seal -- this test lives in the release contracts, which is exactly the split that catches what the gate module set misses. The test was right to fail, and it found more than a stale string. `_LinuxRustSuite` was still here with nothing constructing it: the whole read-only source mount, the writable grafts, the host-uid `--user`, and the four named volumes, in an action no plan reaches. With it went nine `[hostimage]` settings that only ever existed to repair the consequences of sharing a developer's checkout and cache with a root-owned container -- `tmpfs`, `container_home`, `nextest_dir`, `nextest_mount`, `output_dir`, `container_output`, `alpine`, `writable_source_mounts`, `cached_volumes` -- and `NamedVolume`, whose last user they were. And one real regression, restored. The lane used to raise a named `GateError` on a host that is neither Linux nor macOS. Sealing it replaced that with a bare `else`, so a third platform fell through to the Docker path and would have failed somewhere inside a container instead of naming the host it will not run on. `assert "host.on_macos()" in hostimage` is what noticed, so the fix is the guard coming back, not the assertion being rewritten. Both stale tests are reimplemented, not deleted, and two claims got stronger: `/src:ro` asserted the container could not write the checkout, where the assertion is now that no mount exists at all; and `--network none` is pinned, which no earlier test could assert because it was not true. The removed `nextest` and volume assertions are replaced by the base image tag they moved into, and `--user` by the `USER 1000:1000` now baked into the lane image. Verified: `capsem-gate test-release-contracts` exits 0 with 3165 passed, 18 skipped -- the same module set that stopped the release. --- CHANGELOG.md | 20 +++++++ config/gate.toml | 32 +++-------- src/capsem/gate/buildschema.py | 15 ----- src/capsem/gate/hostimage.py | 71 ++---------------------- tests/test_gate_hostimage_composition.py | 68 +++++++++++++---------- tests/test_release_doctor_contract.py | 54 ++++++++++++++++-- 6 files changed, 122 insertions(+), 138 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 77d9036c0..0aa3c21bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Sealing the Linux parity lane left its old machinery behind, and the release + gate caught it. `_LinuxRustSuite` -- the action that assembled the read-only + source mount, the writable grafts and the four named volumes -- was still in + `hostimage.py` with nothing constructing it, along with nine `[hostimage]` + settings that existed only to repair what sharing a checkout with a + root-owned container broke: a writable `/tmp`, a hand-placed `HOME`, a + bound-out nextest directory, a writable graft for Tauri's generated ACLs, and + the volumes themselves. All are gone. + + Restored in the same pass: the lane raised a named error on a host that is + neither Linux nor macOS, and sealing it dropped that guard, so a third + platform fell through to the Docker path and would have failed somewhere + inside a container instead of saying which host it will not run on. + + The two contract tests that pinned the old mechanism are reimplemented rather + than deleted, and two of their claims are now stronger: `/src:ro` said the + container could not write the checkout, where the assertion is now that + nothing is mounted at all; and the lane's `--network none` is asserted, which + no earlier test could claim because it was not true. + - The Linux parity lane holds its own bytes. It bind-mounted the live checkout, grafted two writable mounts back through it to retrieve coverage, inherited four named volumes that survive between runs, and ran with outbound network diff --git a/config/gate.toml b/config/gate.toml index 5d4390733..dc34b13d2 100644 --- a/config/gate.toml +++ b/config/gate.toml @@ -854,37 +854,21 @@ tag = "capsem-host-builder:latest" dockerfile = "docker/Dockerfile.host-builder" context = "docker/" script = "scripts/test-linux-rust.sh" -output_dir = "target/linux-rust-coverage" -nextest_dir = "nextest" mount = "/src" -container_output = "/linux-rust-output" -container_home = "/tmp/capsem-home" # A foreign UID reproduces the case Linux CI actually hits: the checkout's # owner is not the image's user, git rejects /src as "dubious ownership", and # build.rs answers by embedding "unknown" rather than failing -- which is how a # binary with no source identity reaches the provenance check. Checked on macOS # too, because git compares st_uid to euid in userspace. probe_user = "4242:4242" -alpine = "alpine" -# A writable, executable /tmp: the suite compiles and runs test binaries there, -# and the default read-only overlay makes every one of them fail to exec. -tmpfs = "/tmp:rw,exec,mode=1777" -# nextest writes its own state under the target dir; bound out so the results -# survive the container. -nextest_mount = "target/nextest" -# Tauri's build script regenerates ACL schemas inside the application crate. -# Keep the checkout read-only and give only that generated directory a -# writable backing store under the lane's owned output tree. -writable_source_mounts = [ - { source = "target/linux-rust-coverage/tauri-gen", target = "crates/capsem-app/gen" }, -] -cached_volumes = [ - { source = "capsem-linux-rust-cargo-registry", target = "/usr/local/cargo/registry" }, - { source = "capsem-linux-rust-cargo-git", target = "/usr/local/cargo/git" }, - { source = "capsem-linux-rust-rustup", target = "/usr/local/rustup" }, - { source = "capsem-linux-rust-target", target = "/cargo-target" }, -] -environment = { HOME = "/tmp/capsem-home", CAPSEM_SKIP_KVM_TESTS = "1", CAPSEM_LINUX_RUST_OUTPUT_DIR = "/linux-rust-output" } +# `alpine`, `tmpfs`, `container_home`, `nextest_dir`, `nextest_mount`, +# `output_dir`, `container_output`, `writable_source_mounts` and +# `cached_volumes` are gone with the bind mount that needed them. A writable +# /tmp, a hand-placed HOME, a bound-out nextest directory, a writable graft for +# Tauri's generated ACLs and four cross-run volumes were all repairs for +# sharing the developer's checkout and cache with a root-owned container. The +# lane copies its source into an image and its dependencies live in the base, +# so there is nothing to repair. [sbom] script = "scripts/generate-host-binary-sbom.py" diff --git a/src/capsem/gate/buildschema.py b/src/capsem/gate/buildschema.py index 4f4704444..a341616f5 100644 --- a/src/capsem/gate/buildschema.py +++ b/src/capsem/gate/buildschema.py @@ -67,11 +67,6 @@ class FunctionalConfig(Strict): assets_dir_variable: str -class NamedVolume(Strict): - source: str - target: str - - class HostImageConfig(Strict): base_dockerfile: str lane_dockerfile: str @@ -87,18 +82,8 @@ class HostImageConfig(Strict): dockerfile: str context: str script: str - output_dir: str - nextest_dir: str mount: str - container_output: str - container_home: str probe_user: str - alpine: str - tmpfs: str - nextest_mount: str - writable_source_mounts: tuple[NamedVolume, ...] - cached_volumes: tuple[NamedVolume, ...] - environment: dict[str, str] class SbomConfig(Strict): diff --git a/src/capsem/gate/hostimage.py b/src/capsem/gate/hostimage.py index c1cea60f1..27435d173 100644 --- a/src/capsem/gate/hostimage.py +++ b/src/capsem/gate/hostimage.py @@ -15,8 +15,6 @@ from __future__ import annotations -from pathlib import Path - from . import host, linuxrust from .actions import Action, Run from .command import GateCommand @@ -27,15 +25,6 @@ from .gitmetadata import docker_git_metadata_mount from .plan import Plan - -def _volumes(config: GateConfig) -> list[str]: - return [ - flag - for volume in config.hostimage.cached_volumes - for flag in ("-v", f"{volume.source}:{volume.target}") - ] - - #: One name, so every lane that needs the builder depends on the same step #: rather than each spelling its own label. STEP = "host-image" @@ -141,6 +130,12 @@ def build(self, after: tuple[Step, ...]) -> Step: after=after, ) + # Named, not inferred from "not Linux". Without this a third platform + # falls through to the Docker path and fails somewhere inside a + # container instead of saying which host it will not run on. + if not host.on_macos(): + raise GateError("Linux Rust parity runs natively on Linux or in Docker on macOS") + # macOS: the same checked-in script, in a container that holds its own # copy of the source. `cache-ownership`, `linux-rust-mountpoints` and # `output-ownership` are gone with the mounts and volumes that @@ -150,60 +145,6 @@ def build(self, after: tuple[Step, ...]) -> Step: return linuxrust.lane(plan, config, after=(built,)) -class _LinuxRustSuite(Action, name="linux-rust-suite"): - """Run the Linux parity script with runtime-resolved worktree metadata.""" - - def __init__(self, output: Path, *, source: Path, mount: str, script: str) -> None: - self._output = output - self._source = source - self._mount = mount - self._script = script - - def render(self) -> str: - return ( - f"docker run --user -v {self._source}:{self._mount}:ro " - f"... bash {self._mount}/{self._script}" - ) - - def perform(self, context: Context) -> None: - settings = context.config.hostimage - output = self._output - uid, gid = host.user() - context.runner.run( - [ - "docker", - "run", - "--rm", - "--user", - f"{uid}:{gid}", - *[f for k, v in settings.environment.items() for f in ("-e", f"{k}={v}")], - "--tmpfs", - settings.tmpfs, - "-v", - f"{context.root}:{settings.mount}:ro", - *docker_git_metadata_mount(context.runner), - "-v", - f"{output}:{settings.container_output}", - "-v", - f"{output / settings.nextest_dir}:{settings.mount}/{settings.nextest_mount}", - *[ - flag - for volume in settings.writable_source_mounts - for flag in ( - "-v", - f"{context.config.path(volume.source)}:{settings.mount}/{volume.target}", - ) - ], - *_volumes(context.config), - "-w", - settings.mount, - settings.tag, - "bash", - f"{settings.mount}/{settings.script}", - ] - ) - - class LinuxRustCommand( GateCommand, name="linux-rust", diff --git a/tests/test_gate_hostimage_composition.py b/tests/test_gate_hostimage_composition.py index 5e149cef6..15f8e02df 100644 --- a/tests/test_gate_hostimage_composition.py +++ b/tests/test_gate_hostimage_composition.py @@ -108,35 +108,47 @@ def test_foreign_uid_probe_mounts_linked_worktree_metadata( assert "--user 4242:4242" in probe -def test_linux_rust_suite_mounts_linked_worktree_metadata( - monkeypatch: pytest.MonkeyPatch, - tmp_path: Path, -) -> None: - metadata = "/git/common" - monkeypatch.setattr( - hostimage, - "docker_git_metadata_mount", - lambda _runner: ("-v", f"{metadata}:{metadata}:ro"), +def test_only_the_lane_that_needs_git_provenance_still_carries_it() -> None: + """Reimplemented, not deleted. The claim survives; its subject moved. + + This asserted that the parity lane mounted a linked worktree's git + metadata, ran as the host user, and grafted a writable directory for + Tauri's generated ACLs through the read-only source mount. All three were + properties of the bind mount, and the lane has none. + + What must not be lost is *why* the metadata mount existed: `build.rs` + embeds `git rev-parse --short HEAD` and falls back to the string `unknown` + rather than failing, so a container that cannot read git history produces a + binary with no source identity -- silently. That matters exactly where a + shipped artifact is built, and not at all where coverage is measured. + + So the claim is now a distinction, asserted in both directions: the package + rail, which builds the artifact a release publishes, still carries git + metadata into its container; the parity lane, which only measures which + Linux branches execute, deliberately does not and takes the `unknown` + fallback. Asserting only the first half would let the mount quietly come + back; asserting only the second would let provenance quietly leave. + """ + from capsem.gate import linuxrust + + rail = (PROJECT_ROOT / "src/capsem/gate/packagerail.py").read_text(encoding="utf-8") + assert "docker_git_metadata_mount" in rail, ( + "the package rail stopped carrying git metadata, so a published binary " + "would embed an 'unknown' build hash without anything failing" + ) + + lane = (PROJECT_ROOT / "src/capsem/gate/linuxrust.py").read_text(encoding="utf-8") + assert "docker_git_metadata_mount" not in lane + for flag in ("-v", "--volume", "--user"): + assert flag not in lane, f"the parity lane grew a {flag}, so it shares state again" + + # And the probe that proves the builder can read a checkout it does not own + # is still wired, because that is what makes the package rail's mount + # sufficient rather than merely present. + assert "_ForeignUidProbe" in (PROJECT_ROOT / "src/capsem/gate/hostimage.py").read_text( + encoding="utf-8" ) - monkeypatch.setattr(hostimage.host, "user", lambda: (501, 20)) - runner = RecordingRunner(PROJECT_ROOT) - - hostimage._LinuxRustSuite( - tmp_path, - source=CONFIG.root, - mount=CONFIG.hostimage.mount, - script=CONFIG.hostimage.script, - ).perform(Context(runner, CONFIG)) - - suite = runner.rendered[-1] - assert f"-v {metadata}:{metadata}:ro" in suite - assert "--user 501:20" in suite - assert CONFIG.hostimage.script in suite - - for volume in CONFIG.hostimage.writable_source_mounts: - source = CONFIG.path(volume.source) - target = f"{CONFIG.hostimage.mount}/{volume.target}" - assert f"-v {source}:{target}" in suite + assert linuxrust.RunLane is not None def test_the_lane_has_somewhere_to_put_its_output_before_it_runs() -> None: diff --git a/tests/test_release_doctor_contract.py b/tests/test_release_doctor_contract.py index d14c750e4..a52cde63e 100644 --- a/tests/test_release_doctor_contract.py +++ b/tests/test_release_doctor_contract.py @@ -4935,12 +4935,54 @@ def test_just_test_owns_linux_rust_platform_coverage_through_docker() -> None: assert "test-linux-rust.sh" in canonical_gate assert "test-linux-rust.sh" in linux_rust_gate assert "capsem-host-builder:latest" in linux_rust_gate - assert "docker run --rm" in linux_rust_gate - assert "--user" in linux_rust_gate - assert "/src:ro" in linux_rust_gate - assert "nextest" in linux_rust_gate - assert "capsem-linux-rust-cargo-registry" in linux_rust_gate - assert "capsem-linux-rust-rustup" in linux_rust_gate + + # Reimplemented, not deleted. Six assertions here pinned the mechanism -- + # `docker run --rm`, `--user`, `/src:ro`, and two named volumes -- and the + # lane no longer has any of them: it copies its source into an image, + # resolves dependencies from a base image keyed by the lockfiles, and runs + # sealed. The property each one protected still holds, expressed against + # what the lane does now, and two of them are strictly stronger. + # + # `--rm` could not coexist with `docker cp`, so the container is created, + # started, copied from, and removed -- removal on the failure path too, + # which `--rm` could never give while still yielding the coverage of a lane + # that failed. + assert "docker create" in linux_rust_gate + assert "docker start" in linux_rust_gate + assert "docker cp" in linux_rust_gate + assert linux_rust_gate.index("docker cp") < linux_rust_gate.rindex("docker rm"), ( + "the container is removed before its coverage is copied out" + ) + + # `/src:ro` said the container could not write the checkout. Nothing is + # mounted at all now, which is the stronger claim and the one that ends the + # race with the host steps that share those inodes. + assert "/src:ro" not in linux_rust_gate + for flag in (" -v ", " --volume "): + assert flag not in linux_rust_gate, f"the parity lane grew a{flag}mount" + + # The named volumes carried the cargo registry, the rustup toolchain and an + # 11 GB target between runs -- the cross-run state that let a warm machine + # and a clean checkout disagree about one commit. They live in a base image + # keyed by the lockfiles that determine them. + for volume in ("capsem-linux-rust-cargo-registry", "capsem-linux-rust-rustup"): + assert volume not in linux_rust_gate, f"{volume} came back" + assert "capsem-linux-rust-base:" in linux_rust_gate + + # `--user` kept the container off root, because the suite chmods an asset + # to 0o000 and demands the read fail. That is now baked into the image. + lane_dockerfile = _source_text("docker/Dockerfile.linux-rust") + assert "USER 1000:1000" in lane_dockerfile + + # And the property none of the originals asserted, because it was not true: + # the lane runs with no outbound network, which is what proved the mid-run + # `pnpm install` and the `cdn.pyke.io` fetch inside `ort`'s build script + # were there at all. + assert "--network none" in linux_rust_gate + + # `nextest` moved out of the argv with the mount that bound its state; the + # script the container runs is still the checked-in one, asserted below. + assert "test-linux-rust.sh" in linux_rust_gate # Native on Linux, Docker on macOS -- the branch is `host.on_linux()` in # `hostimage.py` rather than a `uname` test in a recipe. hostimage = _source_text("src/capsem/gate/hostimage.py") From dae4de09d82ddc8f22f08266cea07447b72b63bd Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Tue, 4 Aug 2026 20:25:41 -0400 Subject: [PATCH 13/18] fix(gate): stop the observer inventing faults it cannot locate The release run logged this 42 times, and every one was false: [source-tree] profile.toml: unlink during the run; the gate qualifies this tree (steps: no step in flight) No path, no step, and no such event. `shutil.rmtree` deletes through a directory descriptor -- `os.unlink('profile.toml', dir_fd=5)` -- so the proxy recorded the bare entry name and passed `kwargs` straight through without ever reading `dir_fd`. `Watch.is_source` then called `Path('profile.toml'). resolve()`, which anchors to the current working directory. That directory is the checkout root, so deleting `target/config/profiles/code/profile.toml` -- an ordinary step, in build output -- was reported as a mutation of the tracked `config/profiles` file that happens to share a basename. Reproduced before fixing: `shutil.rmtree.avoids_symlink_attacks` is true, and a spy on `os.unlink` records `('profile.toml', dir_fd=5)`. This is the guard that exists to catch the `config/profiles` race that killed a release run, and it was reporting 42 phantom mutations per run. A guard that cries wolf every run is one nobody reads, and it would have buried the true positive among them. Three fixes, because any one alone would have hidden the others: * `interception` resolves the subject where the call acted -- against `dir_fd` when given, and through the descriptor itself when the subject is an integer, as in `os.truncate(fd, n)`. Linux reads the symlink whose template now comes from config; macOS asks `fcntl(F_GETPATH)`. * `is_source` refuses to judge a path that is not absolute. That is the structural half: the first fix repairs the one caller that was found, this one makes any future loose spelling unjudgeable rather than misattributed. * `dist`, `packages` and `assets` join `target` as build output. All are gitignored roots the gate rewrites every run -- `assets/current` is resynced per architecture and stale `.deb`s are cleared before each build -- and with only `target` excluded those read as mutating the tree under qualification. When a path genuinely cannot be established the event is not judged: a fault nobody can locate is not evidence, and inventing one is worse than missing it. `fd_path_template` is in `config/gate.toml` because `test_gate_has_no_literal_ data` is right that a path spelled in a module is a second copy; the tests take it from the same place rather than hardcoding their own. `_F_GETPATH` stays in code -- it is a number in the platform ABI, not a path. Each of the four new tests proves a different part: the relative-path test drives `_judge` directly with cwd set to the source root, so it cannot pass by landing outside the tree; the `dir_fd` test deletes a file under `config/`, where widening the build-output set cannot help it. Verified: 2018 passed, 18 skipped across tests/test_gate_*.py. The one deselection, `test_a_live_run_is_never_rotated_away_by_another`, fails identically at 7805bd8e5 with no changes applied -- it is a pre-existing invocation artifact, not a regression, and is filed separately. --- CHANGELOG.md | 29 +++++++++ config/gate.toml | 7 ++ src/capsem/gate/faults.py | 8 ++- src/capsem/gate/harnessschema.py | 3 + src/capsem/gate/interception.py | 64 +++++++++++++++++- src/capsem/gate/observation.py | 9 +++ src/capsem/gate/observing.py | 2 +- tests/test_gate_observation.py | 108 ++++++++++++++++++++++++++++++- 8 files changed, 222 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0aa3c21bf..d4043fdf1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,35 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- The filesystem observer reported 42 phantom source mutations on every release + run, and each named a file nothing had touched. `shutil.rmtree` deletes + through a directory descriptor -- `os.unlink('profile.toml', dir_fd=5)` -- so + a bare entry name reached the observer, which resolved it against the current + working directory. That directory is the checkout root, so removing + `target/config/profiles/code/profile.toml`, an ordinary step, was reported as + + [source-tree] profile.toml: unlink during the run + + naming the tracked `config/profiles` file of the same basename. A guard that + cries wolf 42 times a run is a guard nobody reads, and this is the guard that + exists to catch the `config/profiles` race that killed a release run. + + Fixed in three places, because one of them alone would have hidden the + others. Interception now resolves a subject against its `dir_fd` (and + resolves an integer descriptor subject, as in `os.truncate(fd, n)`), so the + fault names where the call acted. The judge refuses to classify any path that + is not absolute, so no caller's loose spelling can be misattributed again -- + not merely the one that was found. And `dist`, `packages` and `assets` join + `target` as build output: all are gitignored roots the gate rewrites every + run, and with only `target` excluded, resyncing `assets/current` or clearing + a stale `.deb` read as mutating the tree being qualified. + + When a path genuinely cannot be established the event is not judged at all: a + fault nobody can locate is not evidence, and inventing one is worse than + missing it. + ### Security - The local Tauri signing key and its password no longer reach anything that diff --git a/config/gate.toml b/config/gate.toml index dc34b13d2..d0549f6a3 100644 --- a/config/gate.toml +++ b/config/gate.toml @@ -1236,6 +1236,13 @@ summary = "summary.txt" # staged release channel, because a hardlink *to* checked-in source lands in # build output and leaves the source directory untouched. observed_roots = ["config", "crates", "scripts", "guest", "src", "target/web-parity"] +# How Linux answers "what path is this descriptor". `shutil.rmtree` deletes +# through a directory descriptor, so the observer receives a bare entry name +# and has to anchor it -- without this it resolved against the working +# directory, which is the checkout root, and reported tracked files nothing had +# touched. macOS answers the same question through `fcntl(F_GETPATH)`, which +# takes no path to name here. +fd_path_template = "/proc/self/fd/{handle}" error_log = "errors.log" # Bounded, because a run that trips one rule per file trips it thousands of # times and an unbounded fault log on a machine that gates daily is a diff --git a/src/capsem/gate/faults.py b/src/capsem/gate/faults.py index ef6cbfa50..7b5bed6d3 100644 --- a/src/capsem/gate/faults.py +++ b/src/capsem/gate/faults.py @@ -15,7 +15,13 @@ #: Directories under the checkout a run may write. Everything else is input: #: the gate reads it, and changing it mid-run means the thing being qualified #: is not the thing that was measured. -BUILD_OUTPUT = frozenset({"target", ".git", "node_modules", ".venv"}) +#: +#: `dist`, `packages` and `assets` are here because they are gitignored build +#: roots the gate rewrites every run -- `assets/current` is resynced per +#: architecture, and stale `.deb`s are removed before each package build. With +#: only `target` excluded, ordinary steps read as the gate mutating the tree it +#: is qualifying. +BUILD_OUTPUT = frozenset({"target", "dist", "packages", "assets", ".git", "node_modules", ".venv"}) #: Hash files up to this size. Digests answer "are these the same bytes under #: two names", which matters for seeds and manifests; a multi-gigabyte rootfs diff --git a/src/capsem/gate/harnessschema.py b/src/capsem/gate/harnessschema.py index 302192043..8e7f3bb4a 100644 --- a/src/capsem/gate/harnessschema.py +++ b/src/capsem/gate/harnessschema.py @@ -181,6 +181,9 @@ class RunLogConfig(Strict): latest_link: str #: Trees each run watches for filesystem faults, relative to the checkout. observed_roots: tuple[str, ...] + #: How Linux names the path behind a file descriptor, so a `dir_fd`-relative + #: call can be anchored instead of resolved against the working directory. + fd_path_template: str #: Where those faults are written the instant they are found, per run. error_log: str #: Size cap and generations kept, so faults cannot fill the disk. diff --git a/src/capsem/gate/interception.py b/src/capsem/gate/interception.py index 5f8a06a37..042ae167d 100644 --- a/src/capsem/gate/interception.py +++ b/src/capsem/gate/interception.py @@ -22,6 +22,7 @@ import os import shutil import stat +import sys from collections.abc import Callable from pathlib import Path from typing import Protocol @@ -67,8 +68,9 @@ class Instrument: #: creates `dst`, and `dst` is what now shares an inode it should not. DESTINATION_IS_SECOND = frozenset({"link", "copy", "rename"}) - def __init__(self, observer: Observer) -> None: + def __init__(self, observer: Observer, *, fd_path_template: str) -> None: self._observer = observer + self._fd_path_template = fd_path_template self._saved: list[tuple[object, str, object]] = [] def __enter__(self) -> Instrument: @@ -85,14 +87,16 @@ def __exit__(self, *_: object) -> None: def _wrap(self, original: Callable[..., object], kind: str) -> Callable[..., object]: observer = self._observer + template = self._fd_path_template @functools.wraps(original) def proxy(*args: object, **kwargs: object) -> object: subject = _subject(kind, args) before = _mode_of(subject) if kind == "chmod" else None result = original(*args, **kwargs) - if subject is not None: - observer.observed(kind, Path(str(subject)), before=before) + located = _locate(subject, kwargs, template) + if located is not None: + observer.observed(kind, located, before=before) return result return proxy @@ -104,6 +108,60 @@ def _subject(kind: str, args: tuple[object, ...]) -> object | None: return args[0] if args else None +#: macOS `fcntl` command for "give me this descriptor's path". A number in the +#: platform's ABI, not a path, so it stays here; Linux answers the same +#: question by reading a symlink whose template comes from config. +_F_GETPATH = 50 + +#: Enough for `PATH_MAX` on both platforms. +_PATH_BUFFER = 1024 + + +def _path_of_fd(handle: int, template: str) -> Path | None: + """The path behind an open descriptor, or `None` if it cannot be had.""" + try: + if sys.platform == "darwin": + import fcntl + + answer = fcntl.fcntl(handle, _F_GETPATH, bytes(_PATH_BUFFER)) + return Path(os.fsdecode(answer.rstrip(b"\0"))) + return Path(os.readlink(template.format(handle=handle))) + except (OSError, ValueError, UnicodeDecodeError): + return None + + +def _locate(subject: object, kwargs: dict[str, object], template: str) -> Path | None: + """Where the call actually acted, not what the caller happened to spell. + + `shutil.rmtree` deletes through a directory descriptor -- `os.unlink( + 'profile.toml', dir_fd=5)` -- and recording the bare entry name left the + path to be resolved against the current working directory, which for the + gate is the checkout root. One release run logged 42 faults that way, each + naming a tracked file nothing had touched. + + Returns `None` when the path cannot be established, and the judge then + declines to call it a source mutation: a fault nobody can locate is not + evidence, and inventing one is worse than missing it. + """ + if isinstance(subject, int): + # An integer subject is a descriptor, as in `os.truncate(fd, n)`. + return _path_of_fd(subject, template) + if not isinstance(subject, str | os.PathLike): + return None + # `str`, not `os.fsdecode`, for the same reason `_mode_of` does it: the + # union leaves `PathLike[object]`, which the checker will not accept. + path = Path(str(subject)) + if path.is_absolute(): + return path + handle = kwargs.get("dir_fd") + if handle is None: + return Path.cwd() / path + if not isinstance(handle, int): + return None + anchor = _path_of_fd(handle, template) + return anchor / path if anchor is not None else None + + def _mode_of(path: object) -> int | None: """The mode as it is *right now*, which after the call is unrecoverable.""" # `str` only. Every proxied caller passes a path, and accepting the whole diff --git a/src/capsem/gate/observation.py b/src/capsem/gate/observation.py index ff379d4bd..fc0af693a 100644 --- a/src/capsem/gate/observation.py +++ b/src/capsem/gate/observation.py @@ -199,6 +199,15 @@ def _judge(self, event: Event) -> None: def is_source(self, path: Path) -> bool: from .faults import BUILD_OUTPUT + # Absolute only. `Path.resolve()` anchors a relative path to the + # current working directory, which for the gate is the checkout root -- + # so a bare `profile.toml` from a `dir_fd` caller resolved into the + # source tree and was reported as a mutation of a file the run never + # touched. Refusing to judge what was not located keeps that class out + # regardless of which caller spells a path loosely; `interception` + # resolves the ones it can. + if not path.is_absolute(): + return False try: relative = path.resolve().relative_to(self._source_root) except (ValueError, OSError): diff --git a/src/capsem/gate/observing.py b/src/capsem/gate/observing.py index e777ebec9..399d0fb51 100644 --- a/src/capsem/gate/observing.py +++ b/src/capsem/gate/observing.py @@ -56,7 +56,7 @@ def report(fault: Fault) -> None: roots = [config.path(name) for name in settings.observed_roots] try: with Watch(roots, source_root=config.root, declared=declared, on_fault=report) as watch: - with Instrument(watch): + with Instrument(watch, fd_path_template=settings.fd_path_template): yield watch watch.sweep() finally: diff --git a/tests/test_gate_observation.py b/tests/test_gate_observation.py index d4b2295cb..fb929ba17 100644 --- a/tests/test_gate_observation.py +++ b/tests/test_gate_observation.py @@ -27,12 +27,18 @@ import time from pathlib import Path +from capsem.gate import config as gate_config from capsem.gate.faultlog import FaultLog from capsem.gate.faults import Event, Facts, Fault from capsem.gate.observation import Watch PROJECT_ROOT = Path(__file__).resolve().parents[1] +#: From config, not spelled here: `test_gate_has_no_literal_data` holds the +#: gate's modules to one copy of every path, and a test that hardcodes its own +#: would be asserting against a value production no longer uses. +FD_PATH_TEMPLATE = gate_config.load(PROJECT_ROOT).runlog.fd_path_template + def _settle(watch: Watch, count: int, timeout: float = 5.0) -> None: """Wait for delivery rather than guessing with a sleep. @@ -226,6 +232,102 @@ def test_build_output_is_not_the_checked_in_tree() -> None: assert watch.faults == [] +def test_every_build_root_is_build_output_not_only_target() -> None: + """`dist/`, `packages/` and `assets/` are gitignored and rewritten per run. + + Only `target` was excluded, so deleting a stale `.deb` or resyncing + `assets/current` -- both ordinary steps -- read as the gate mutating the + tree it is qualifying. + """ + watch = Watch([], source_root=Path("/repo")) + for directory in ("target", "dist", "packages", "assets", ".git", "node_modules", ".venv"): + watch._judge( + Event(at=1.0, kind="unlink", path=Path(f"/repo/{directory}/x"), steps=("build",)) + ) + assert watch.faults == [], [fault.render() for fault in watch.faults] + + +def test_a_relative_path_is_never_judged_against_the_working_directory( + tmp_path: Path, monkeypatch +) -> None: + """The release run of 2026-08-04 logged 42 of these, every one false. + + `shutil.rmtree` deletes through a directory descriptor -- + `os.unlink('profile.toml', dir_fd=5)` -- so a bare entry name reaches the + observer. Resolving it against the current working directory, which is the + checkout root, named a tracked file the run never touched: + + [source-tree] profile.toml: unlink during the run + + A guard that reports 42 phantom source mutations per run is a guard nobody + reads, and this is the guard that exists to catch the `config/profiles` + race that killed a release run. Judged only on absolute paths, so no + caller's spelling can be misattributed -- not just the one that was found. + """ + # cwd *is* the source root here. Anything less and this passes because + # `Path.resolve()` happened to land outside the tree, which is the test + # passing by coincidence rather than by the rule. + watch = Watch([], source_root=tmp_path) + monkeypatch.chdir(tmp_path) + (tmp_path / "config").mkdir() + (tmp_path / "config" / "profile.toml").write_text("x") + + watch._judge(Event(at=1.0, kind="unlink", path=Path("config/profile.toml"), steps=())) + assert watch.faults == [], [fault.render() for fault in watch.faults] + + +def test_rmtree_of_build_output_is_not_reported_as_a_source_mutation( + tmp_path: Path, monkeypatch +) -> None: + """The production shape end to end, through the real interception.""" + from capsem.gate.interception import Instrument + + root = tmp_path / "checkout" + (root / "target" / "config" / "profiles" / "code").mkdir(parents=True) + (root / "config" / "profiles").mkdir(parents=True) + (root / "target" / "config" / "profiles" / "code" / "profile.toml").write_text("x") + (root / "target" / "config" / "profiles" / "code" / "asset-status.json").write_text("y") + + # cwd at the checkout root is what made a bare entry name resolve into the + # source tree in the first place. + monkeypatch.chdir(root) + watch = Watch(roots=(root,), source_root=root) + with Instrument(watch, fd_path_template=FD_PATH_TEMPLATE): + shutil.rmtree(root / "target" / "config") + + offenders = [fault for fault in watch.faults if fault.reason == "source-tree"] + assert not offenders, [fault.render() for fault in offenders] + + +def test_an_intercepted_fault_names_an_absolute_path(tmp_path: Path, monkeypatch) -> None: + """A fault nobody can locate is not evidence. + + Separate from the rule above, because a fix that only widened the + build-output set would silence the false positives and leave every real + fault still reported as a bare basename. + """ + from capsem.gate.interception import Instrument + + root = tmp_path / "checkout" + (root / "config" / "profiles").mkdir(parents=True) + victim = root / "config" / "profiles" / "profile.toml" + victim.write_text("x") + + monkeypatch.chdir(root) + watch = Watch(roots=(root,), source_root=root) + with Instrument(watch, fd_path_template=FD_PATH_TEMPLATE): + handle = os.open(str(root / "config" / "profiles"), os.O_RDONLY) + try: + os.unlink("profile.toml", dir_fd=handle) + finally: + os.close(handle) + + assert watch.faults, "a real tracked-source unlink went unreported" + fault = watch.faults[0] + assert fault.path.is_absolute(), f"fault names a bare path: {fault.render()}" + assert fault.path == victim.resolve(), fault.render() + + def test_an_empty_artifact_is_only_decidable_at_the_end(tmp_path: Path) -> None: """Mid-run it is a file being written; at the end it is a build that reported success and produced nothing.""" @@ -279,7 +381,7 @@ def test_interception_sees_a_hardlink_with_no_watcher_at_all(tmp_path: Path) -> watch = Watch([], source_root=tmp_path) token = CURRENT_STEP.set("contracts.release") try: - with Instrument(watch): + with Instrument(watch, fd_path_template=FD_PATH_TEMPLATE): (tmp_path / "target").mkdir() os.link(seed, tmp_path / "target" / "staged-payload") finally: @@ -309,7 +411,7 @@ def test_interception_catches_the_mode_that_a_watcher_arrives_too_late_for( target.chmod(0o644) watch = Watch([], source_root=tmp_path) - with Instrument(watch): + with Instrument(watch, fd_path_template=FD_PATH_TEMPLATE): os.chmod(target, 0o000) os.chmod(target, 0o644) @@ -357,7 +459,7 @@ def test_the_primitives_are_restored_afterwards() -> None: from capsem.gate.interception import Instrument before = (os.link, os.chmod, shutil.copytree) - with Instrument(Watch([], source_root=Path("/repo"))): + with Instrument(Watch([], source_root=Path("/repo")), fd_path_template=FD_PATH_TEMPLATE): assert os.link is not before[0], "not actually patched" assert (os.link, os.chmod, shutil.copytree) == before From 3aaafce5526befea36231c8565b7553a1dd8254e Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Tue, 4 Aug 2026 21:04:34 -0400 Subject: [PATCH 14/18] test(gate): stop the source-state probe racing the suite around it `functional.pytest.broad.code` failed two tests at once, and neither was at fault: FAILED tests/test_gate_candidate.py:: test_interrogating_the_gate_plan_leaves_the_checkout_alone AssertionError: reading the plan rewrote the gate's own source state ERROR tests/test_mock_server_launcher.py:: test_mock_server_replays_recorded_agy_code_assist_setup rewrote gate-source-state.json ... It wrote b'{"head": "sentinel", "digest": "sentinel"}' One test wrote a sentinel the other was blamed for. Both symptoms are one cause: `test_interrogating...` planted its sentinel in the *real* `target/gate-source-state.json`, making it the only thing in the suite that deliberately writes a file the whole suite shares. Under `pytest -n 4 --dist=loadfile` the workers are separate processes watching one path, so `conftest._the_running_gate_keeps_its_own_source_state` -- which snapshots that file around every test -- saw the sentinel appear during an unrelated test in another worker, blamed it, and restored the file underneath the test that had written it on purpose. It passed in `contracts.release`, where it runs single-process. That is the tell: a guard whose result depends on how pytest was invoked. The sentinel now goes to `.probe-`, inside `target/` so it is build output rather than tracked source, and per-process so four workers cannot collide either. The plan runs against a config copied with that one field changed, so nothing in the run can reach the shared path. The claim is unchanged and one assertion stronger: reading the plan must leave the probe at the sentinel, *and* must leave the real state file byte-identical -- which is the property the racing version could not state, because it was the one modifying it. Why a sentinel at all, now written down in the docstring rather than lost: the file holds the true HEAD during a real gate, so a broken `observing` would rewrite identical bytes and a plain before/after comparison would pass. `built_command` is public in `tests/helpers/gate.py` because a test that must run a plan against a modified config cannot go through `gate_issued`, which builds its `Context` from the real one. Verified the race is gone at the root, not merely absent: running both files together with `-n 4 --dist=loadfile` gives 41 passed, and the shared state file's mtime is identical before and after. --- tests/helpers/gate.py | 13 ++++++++ tests/test_gate_candidate.py | 57 ++++++++++++++++++++++++++---------- 2 files changed, 54 insertions(+), 16 deletions(-) diff --git a/tests/helpers/gate.py b/tests/helpers/gate.py index 74ef05867..e5664202c 100644 --- a/tests/helpers/gate.py +++ b/tests/helpers/gate.py @@ -278,6 +278,19 @@ def _built(root: Path, name: str, args: tuple[tuple[str, object], ...], qualific ) +def built_command( + root: Path, name: str, args: tuple[tuple[str, object], ...] = (), qualification=None +): + """One command against a recording runner, for a test that drives it itself. + + Public because a test that needs to run a plan against a *modified* config + -- pointing an output somewhere private so it does not collide with the + gate running the suite -- cannot go through `gate_issued`, which builds its + own `Context` from the real one. + """ + return _built(root, name, args, qualification) + + def gate_plan(name: str = "candidate", root: Path | None = None, qualification=None): """A command's plan, built but not run -- for asserting on its edges. diff --git a/tests/test_gate_candidate.py b/tests/test_gate_candidate.py index cb76bbfd4..93f753f2d 100644 --- a/tests/test_gate_candidate.py +++ b/tests/test_gate_candidate.py @@ -25,10 +25,12 @@ import argparse import json +import os +from contextlib import suppress from pathlib import Path import pytest -from helpers.gate import RecordingJournal, RecordingRunner +from helpers.gate import RecordingJournal, RecordingRunner, built_command from capsem.gate import cli # noqa: F401 - imported so every command registers from capsem.gate import config as gate_config @@ -390,29 +392,52 @@ def test_interrogating_the_gate_plan_leaves_the_checkout_alone() -> None: Guarded here rather than in the helper, because the helper is not the only thing that will ever run a plan to look at it. - """ - import sys as _sys - - _sys.path.insert(0, str(PROJECT_ROOT / "tests")) - from helpers.gate import gate_issued + The sentinel is this test's own baseline. Reading whatever happens to be on + disk would compare the observer's output against the observer's output the + moment a previous run left one there -- and pass. It matters most during a + real gate, when the file already holds the true HEAD: a broken `observing` + would then rewrite identical bytes and go unnoticed. + + It is written to a *private* path, and that is not incidental. Planting the + sentinel in the real `target/gate-source-state.json` made this test the one + thing in the suite that deliberately writes a file the whole suite shares. + Under `pytest -n 4 --dist=loadfile` that raced: this test wrote the + sentinel, a test in another worker had snapshotted the file before that + write, and `conftest._the_running_gate_keeps_its_own_source_state` blamed + *that* test for the change and restored the file underneath this one. Both + failed, in `functional.pytest.broad.code`, and neither was at fault. With + the sentinel private, the guard's invariant -- nothing in the suite writes + these paths -- is true again, so a future writer is correctly blamed. + """ config = gate_config.load(PROJECT_ROOT) - recorded = config.path(config.candidate.source_state_file) - saved = recorded.read_bytes() if recorded.exists() else None + # Inside `target/`, so it is build output rather than tracked source, and + # per-process, so four xdist workers cannot collide on it either. + probe = f"{config.candidate.source_state_file}.probe-{os.getpid()}" + observed = config.model_copy( + update={"candidate": config.candidate.model_copy(update={"source_state_file": probe})} + ) + recorded = config.path(probe) + shared = config.path(config.candidate.source_state_file) + shared_before = shared.read_bytes() if shared.exists() else None - # A baseline of this test's own making. Reading whatever happens to be on - # disk would compare the observer's output against the observer's output - # the moment a previous run left one there -- and pass. sentinel = json.dumps({"head": "sentinel", "digest": "sentinel"}).encode() recorded.parent.mkdir(parents=True, exist_ok=True) recorded.write_bytes(sentinel) try: - gate_issued("candidate") + command = built_command(PROJECT_ROOT, "candidate") + plan = command._describe() + # A step that needs a machine fails here; what it did before failing is + # still what this asserts on. + with suppress(Exception): + plan.run(Context(command._runner, observed, observing=True)) + assert recorded.read_bytes() == sentinel, ( "reading the plan rewrote the gate's own source state" ) + assert (shared.read_bytes() if shared.exists() else None) == shared_before, ( + "reading the plan touched the state file belonging to the gate " + "running this suite, which is the file every other test shares" + ) finally: - if saved is None: - recorded.unlink(missing_ok=True) - else: - recorded.write_bytes(saved) + recorded.unlink(missing_ok=True) From f673a6ab217b7979a65dd9339a45ea0f416d3653 Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Tue, 4 Aug 2026 21:04:53 -0400 Subject: [PATCH 15/18] fix(frontend): bound fast-uri past the advisory it was meant to exclude `fast.audit.pnpm` failed the release: error: frontend: fast-uri: high: fast-uri vulnerable to host confusion via backslash authority introducer (>=4.0.0 <4.1.2) GHSA-7p8r-x3mc-p8w7 The override was already there, reading `fast-uri: ">=3.1.2"`, and it resolved to 4.1.1 -- inside the advisory range. A lower bound below a vulnerable window is not a bound; it just happens to exclude older versions while admitting the one that matters. Now `>=4.1.2`, with the advisory written beside it so the next person widening it can see what the number is for. Dev-only: it arrives through `@astrojs/check` -> `@astrojs/language-server` -> `yaml-language-server` -> `ajv`, none of which ships to a user. The audit is still a release gate, and it is right to be. Note for whoever hits this next: `pnpm install --lockfile-only` is not enough. `scripts/audit-pnpm-bulk.py` reads `pnpm list --json --depth Infinity`, which reports what is *installed*, so the audit kept failing against 4.1.1 in `node_modules` while the lockfile already said 4.1.2. Verified: `scripts/audit-pnpm-bulk.py` exits 0 with all four workspaces clean, and `pnpm run check` is 0 errors, 0 warnings across 795 files. --- CHANGELOG.md | 9 +++++++++ frontend/pnpm-lock.yaml | 10 +++++----- frontend/pnpm-workspace.yaml | 8 +++++++- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d4043fdf1..9f0e0539c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Security + +- `fast-uri` is bounded past GHSA-7p8r-x3mc-p8w7 (host confusion via a + backslash authority introducer). The frontend's override read `>=3.1.2`, + which resolved to 4.1.1 -- inside the advisory's `>=4.0.0 <4.1.2` -- so the + bound admitted the very version it was there to exclude. Now `>=4.1.2`. It + arrives through `@astrojs/check`, so it is dev-only tooling, but the audit is + a release gate and blocked one. + ### Fixed - The filesystem observer reported 42 phantom source mutations on every release diff --git a/frontend/pnpm-lock.yaml b/frontend/pnpm-lock.yaml index e69650e3f..deeaeeef7 100644 --- a/frontend/pnpm-lock.yaml +++ b/frontend/pnpm-lock.yaml @@ -7,7 +7,7 @@ settings: overrides: yaml: '>=2.8.3' postcss: '>=8.5.10' - fast-uri: '>=3.1.2' + fast-uri: '>=4.1.2' esbuild: 0.28.1 importers: @@ -1629,8 +1629,8 @@ packages: fast-string-width@3.0.2: resolution: {integrity: sha512-gX8LrtNEI5hq8DVUfRQMbr5lpaS4nMIWV+7XEbXk2b8kiQIizgnlr12B4dA3ZEx3308ze0O4Q1R+cHts8kyUJg==} - fast-uri@4.1.1: - resolution: {integrity: sha512-YPOs1zD5TG2+EZt+r88LwF6mclA7TPkpwMP7ZN3TO2HiHS8TXvq7QA/17iJsV9dubcLo/f8eEYqMBruyQV21hQ==} + fast-uri@4.1.2: + resolution: {integrity: sha512-TyGmBcbDTZXcb2cj5MV89DrF42DKvb3y5DDUNh95iO+IMeAzMkVSxK1PZRrRIpc9yg8U2GhGdbofNa0LS/a4Bw==} fast-wrap-ansi@0.2.2: resolution: {integrity: sha512-7F2Fl+TjRSenLqlU3UjSH0iyqopqoZIu7eZVpEirP2g1GtWa2G/ecEmBdgz31+Mxr+ELclgg6sokpSFIQiZ02Q==} @@ -3811,7 +3811,7 @@ snapshots: ajv@8.20.0: dependencies: fast-deep-equal: 3.1.3 - fast-uri: 4.1.1 + fast-uri: 4.1.2 json-schema-traverse: 1.0.0 require-from-string: 2.0.2 @@ -4283,7 +4283,7 @@ snapshots: dependencies: fast-string-truncated-width: 3.0.3 - fast-uri@4.1.1: {} + fast-uri@4.1.2: {} fast-wrap-ansi@0.2.2: dependencies: diff --git a/frontend/pnpm-workspace.yaml b/frontend/pnpm-workspace.yaml index d233c8cf6..01a48399f 100644 --- a/frontend/pnpm-workspace.yaml +++ b/frontend/pnpm-workspace.yaml @@ -12,5 +12,11 @@ onlyBuiltDependencies: overrides: yaml: ">=2.8.3" postcss: ">=8.5.10" - fast-uri: ">=3.1.2" + # >=4.1.2, not >=3.1.2: GHSA-7p8r-x3mc-p8w7 (host confusion via a backslash + # authority introducer) covers >=4.0.0 <4.1.2, and the looser bound resolved + # to 4.1.1 -- inside the advisory. Reached through @astrojs/check -> + # language-server -> yaml-language-server -> ajv, so it is dev-only tooling, + # but the audit is a release gate and a bound that admits a known-vulnerable + # version is not a bound. + fast-uri: ">=4.1.2" esbuild: 0.28.1 From 19c93e3012ca44a8b1a20e6c054fcc097c1718b9 Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Tue, 4 Aug 2026 21:43:41 -0400 Subject: [PATCH 16/18] fix(gate): write the recipe the sealed lane tells you to run The release stopped here, exactly as designed, and then handed back a command that does not exist: ERROR: release-profile failed -- linux-rust: no Linux parity base image for capsem-linux-rust-base:03ebe122079926b2. Its dependencies changed; run `just warm` to build it with network before the gate runs without. `just warm` was never written. The refusal itself is right -- the lane's tag is keyed by `Cargo.lock`, `rust-toolchain.toml` and `frontend/pnpm-lock.yaml`, and building a 25 GB image mid-run would turn a `--network none` lane into a network build at minute four -- but a refusal is only as good as the way out it names. Found by bumping `fast-uri` for GHSA-7p8r-x3mc-p8w7, which re-keyed the image. `hostimage.py` already carried a note about the last time this happened: `install-image` and `cross-compile` both dispatched `just _build-host-image`, a recipe that has never existed, and neither test noticed "because both stopped at the recipe boundary instead of crossing it". I read that docstring while sealing the lane and then did the same thing. So the fix is the class, not the instance: * `warm-linux-rust-base` is a gate command; `_warm-linux-rust-base` is the thin recipe that dispatches it, private because `config/public-surface.toml` locks the public surface at 13 and this is developer machinery. * the recipe name lives in `config/gate.toml` as `warm_recipe`, so the refusal and the recipe cannot drift apart. * `test_every_recipe_the_gate_tells_an_operator_to_run_exists` parses gate source with `ast` and checks every ``just `` named in a *string* -- docstrings excluded, since `hostimage.py`'s note about a recipe that never existed is the point of that note. One more false positive fixed on the way: `capsem-gate doctor` scanned every justfile line containing `capsem-gate `, so the comment explaining this recipe was parsed as a dispatch of ``linux-rust` ``, trailing backtick included. Three doctor checks went red on a comment. A comment calls nothing, and a commented-out dispatch is not a dispatch either. Verified: `just _warm-linux-rust-base` exits 0 and builds capsem-linux-rust-base:03ebe122079926b2; `capsem-gate test-release-contracts` exits 0 with 3171 passed, 18 skipped. --- CHANGELOG.md | 23 ++++++++++++ config/gate.toml | 5 +++ justfile | 10 ++++++ src/capsem/gate/buildschema.py | 2 ++ src/capsem/gate/doctor.py | 7 ++++ src/capsem/gate/linuxrust.py | 57 +++++++++++++++++++++++++++-- tests/test_gate_doctor.py | 28 +++++++++++++++ tests/test_just_contract.py | 66 ++++++++++++++++++++++++++++++---- 8 files changed, 189 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f0e0539c..6b4a9abd2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- `just _warm-linux-rust-base` builds the Linux parity base image, with + network, before a sealed run needs it. The lane deliberately refuses to build + it mid-run -- its tag is keyed by `Cargo.lock`, `rust-toolchain.toml` and + `frontend/pnpm-lock.yaml`, so a dependency bump re-keys it, and resolving + that inside the run would turn a `--network none` lane into a multi-gigabyte + network build at minute four -- and its refusal named `just warm`, which did + not exist. Bumping `fast-uri` for a security advisory re-keyed the image and + found it: the release stopped correctly and handed back a command that fails. + + A contract now checks that every recipe the gate names in an operator-facing + message is a recipe the justfile defines. `hostimage.py` already carried a + note about the last time this happened -- `just _build-host-image`, dispatched + by two lanes, never written -- so it is a class, not an incident. + +### Fixed + +- `capsem-gate doctor` no longer reads a justfile comment as a dispatch. It + scanned every line containing `capsem-gate `, so prose naming a subcommand + was parsed as a call to ``linux-rust` `` -- trailing backtick included -- and + reported as unknown. Three doctor checks went red on a comment. + ### Security - `fast-uri` is bounded past GHSA-7p8r-x3mc-p8w7 (host confusion via a diff --git a/config/gate.toml b/config/gate.toml index d0549f6a3..6e9691321 100644 --- a/config/gate.toml +++ b/config/gate.toml @@ -837,6 +837,11 @@ base_tag_template = "capsem-linux-rust-base:{digest}" lane_tag = "capsem-linux-rust:latest" # What decides the dependency set, and therefore the base image's identity. lockfile_inputs = ["Cargo.lock", "rust-toolchain.toml", "frontend/pnpm-lock.yaml"] +# Named here so the lane's refusal and the recipe that answers it cannot drift +# apart. `test_every_recipe_the_gate_tells_an_operator_to_run_exists` checks +# this resolves to a real recipe -- the last time a message named one that did +# not exist, it stopped a release and handed back a command that fails. +warm_recipe = "_warm-linux-rust-base" # Denied, which is what proves the base image is complete rather than believed # to be complete. network = "none" diff --git a/justfile b/justfile index 5ab7b3cc6..65e3fa3aa 100644 --- a/justfile +++ b/justfile @@ -194,6 +194,16 @@ _gate-linux-rust: uv run capsem-gate linux-rust +# Build the Linux parity base image, with network, before a sealed run needs it. +# The lane refuses to build this itself: its tag is keyed by Cargo.lock, +# rust-toolchain.toml and frontend/pnpm-lock.yaml, so a dependency bump re-keys +# it, and resolving that inside the run would turn a `--network none` lane into +# a multi-gigabyte network build at minute four. `capsem-gate linux-rust` names +# this recipe when the image is missing. +_warm-linux-rust-base: + uv run capsem-gate warm-linux-rust-base + + # Run the production release SBOM generator over the exact current-version # packages built by the canonical gate. Mac runs cover one .pkg plus both .deb # architectures; native Linux qualification covers both .deb architectures. diff --git a/src/capsem/gate/buildschema.py b/src/capsem/gate/buildschema.py index a341616f5..e7f90da01 100644 --- a/src/capsem/gate/buildschema.py +++ b/src/capsem/gate/buildschema.py @@ -78,6 +78,8 @@ class HostImageConfig(Strict): container_output_contents: str extract_to: str lane_container: str + #: The recipe the lane's refusal names when the base image is missing. + warm_recipe: str tag: str dockerfile: str context: str diff --git a/src/capsem/gate/doctor.py b/src/capsem/gate/doctor.py index 8685cc878..95fae844d 100644 --- a/src/capsem/gate/doctor.py +++ b/src/capsem/gate/doctor.py @@ -105,6 +105,13 @@ def _dispatched_subcommands(config: gate_config.GateConfig, runner: Runner) -> l marker = "capsem-gate " if marker not in line: continue + # A comment calls nothing. Naming a subcommand in prose -- "`capsem-gate + # linux-rust` names this recipe when the image is missing" -- was read + # as a dispatch of ``linux-rust` ``, trailing backtick included, and + # reported as an unknown subcommand. The check is about what the + # justfile *runs*. + if line.lstrip().startswith("#"): + continue called = line.split(marker, 1)[1].split() if called and called[0] not in known: findings.append( diff --git a/src/capsem/gate/linuxrust.py b/src/capsem/gate/linuxrust.py index e86428761..c2fa2bfc5 100644 --- a/src/capsem/gate/linuxrust.py +++ b/src/capsem/gate/linuxrust.py @@ -24,6 +24,7 @@ import hashlib from .actions import Action +from .command import GateCommand from .config import GateConfig from .context import Context from .docker import Docker @@ -61,12 +62,40 @@ def require_base(config: GateConfig, docker: Docker) -> str: if not docker.image_exists(tag): raise GateError( f"no Linux parity base image for {tag}. Its dependencies changed; " - f"run `just warm` to build it with network before the gate runs " - "without." + f"run `just {config.hostimage.warm_recipe}` to build it with " + "network before the gate runs without." ) return tag +class WarmBase(Action, name="linux-rust-warm-base"): + """Build the base image, with network, before a gate that has none. + + Separate from the lane on purpose. The lane refuses to build this itself -- + a `Cargo.lock` or `pnpm-lock.yaml` bump re-keys the tag, and resolving that + inside the run would turn a sealed lane into a multi-gigabyte network build + at minute four. So the refusal names a command, and this is that command. + """ + + def render(self) -> str: + return "build the Linux parity base image with network" + + def perform(self, context: Context) -> None: + config = context.config + settings = config.hostimage + docker = Docker(context.runner) + tag = base_tag(config) + if docker.image_exists(tag): + context.journal.note(f"Linux parity base image {tag} is already present") + return + docker.build( + tag=tag, + dockerfile=config.path(settings.base_dockerfile).as_posix(), + context=str(config.root), + args=[f"BASE={settings.tag}"], + ) + + class RunLane(Action, name="linux-rust-lane"): """Build the source into an image, run it sealed, copy the coverage out. @@ -122,3 +151,27 @@ def lane(plan: Plan, config: GateConfig, *, after: tuple[Step, ...] = ()) -> Ste step("linux-rust", RunLane(), contends=(config.exclusive("docker_daemon"),)), after=after, ) + + +class WarmCommand( + GateCommand, + name="warm-linux-rust-base", + help="build the Linux parity base image, with network, before a sealed run", +): + exclusive = True + + def plan(self) -> Plan: + from . import hostimage + + plan = Plan(self.name) + # After the builder image, which this one is `FROM`. + built = plan.shared(hostimage.image(self._config)) + plan.add( + step( + "warm-base", + WarmBase(), + contends=(self._config.exclusive("docker_daemon"),), + ), + after=(built,), + ) + return plan diff --git a/tests/test_gate_doctor.py b/tests/test_gate_doctor.py index 4efb8016a..0d00564e5 100644 --- a/tests/test_gate_doctor.py +++ b/tests/test_gate_doctor.py @@ -239,3 +239,31 @@ def test_lint_warnings_fail_the_gate(tmp_path: Path) -> None: ] assert checks assert all("--error-on-warning" in line for line in checks) + + +def test_a_subcommand_named_in_a_comment_is_not_a_dispatch(tmp_path: Path) -> None: + """Prose about a command is not a call to it. + + The check reads every line containing `capsem-gate `, and a justfile + comment explaining which command names a recipe -- + + # `capsem-gate linux-rust` names this recipe when the image is missing. + + -- was parsed as a dispatch of ``linux-rust` ``, trailing backtick and all, + then reported as an unknown subcommand. Three doctor tests went red on a + comment. + + A commented-out dispatch is also not a dispatch, so skipping the line loses + nothing the check was protecting. + """ + root = _checkout(tmp_path) + justfile = root / "justfile" + justfile.write_text( + justfile.read_text(encoding="utf-8") + + "\n# `capsem-gate not-a-real-subcommand` is only mentioned here.\n" + + "# uv run capsem-gate also-not-real\n", + encoding="utf-8", + ) + + findings = doctor.check(RecordingRunner(root)) + assert [f for f in findings if "dispatch" in f.check] == [], findings diff --git a/tests/test_just_contract.py b/tests/test_just_contract.py index 599921ab1..257fb671a 100644 --- a/tests/test_just_contract.py +++ b/tests/test_just_contract.py @@ -5,6 +5,7 @@ PROJECT_ROOT = Path(__file__).resolve().parents[1] + def _gate_issues(name: str | None = None) -> str: """Everything the gate would issue, with real argv. See `helpers.gate`.""" import sys as _sys @@ -15,7 +16,6 @@ def _gate_issues(name: str | None = None) -> str: return gate_issues(name) - def test_justfile_does_not_expose_legacy_guest_dir_knob() -> None: justfile = (PROJECT_ROOT / "justfile").read_text() @@ -47,7 +47,7 @@ def test_justfile_routes_assets_through_profile_admin_rail() -> None: assert "scripts/materialize-config.sh" in justfile assert "cargo run -p capsem-admin -- profile materialize" in materialize_config assert 'profile_paths=("$CONFIG_ROOT"/profiles/*/profile.toml)' in materialize_config - assert "--config-root \"$CONFIG_ROOT\"" in materialize_config + assert '--config-root "$CONFIG_ROOT"' in materialize_config def test_justfile_and_scripts_do_not_reintroduce_retired_escape_paths() -> None: @@ -90,14 +90,11 @@ def test_active_docs_and_skills_do_not_teach_retired_just_run() -> None: continue for line_no, line in enumerate(path.read_text().splitlines(), start=1): if retired.search(line): - failures.append( - f"{path.relative_to(PROJECT_ROOT)}:{line_no}: {line.strip()}" - ) + failures.append(f"{path.relative_to(PROJECT_ROOT)}:{line_no}: {line.strip()}") assert not failures, ( "active docs/skills still teach retired `just run`; use `just exec` for " - "one-shot commands and `just shell` for interactive VMs:\n" - + "\n".join(failures) + "one-shot commands and `just shell` for interactive VMs:\n" + "\n".join(failures) ) @@ -110,3 +107,58 @@ def test_justfile_exposes_one_docs_build() -> None: )[0] assert "bash scripts/check-web-surface.sh docs" in docs_block assert "bash scripts/check-web-surface.sh site" in docs_block + + +def test_every_recipe_the_gate_tells_an_operator_to_run_exists() -> None: + """A remediation naming a recipe nobody wrote is a dead end at the worst moment. + + `hostimage.py`'s own docstring records the last time this happened: + `install-image` and `cross-compile` both dispatched `just _build-host-image`, + a recipe that has never existed, so both were broken at runtime and no test + noticed -- each stopped at the recipe boundary instead of crossing it. + + It happened again. Sealing the Linux parity lane added a refusal that reads + + no Linux parity base image for capsem-linux-rust-base:. Its + dependencies changed; run `just warm` to build it with network before + the gate runs without. + + and `just warm` did not exist. The release stopped there, correctly, and + handed the operator a command that fails. Bumping `frontend/pnpm-lock.yaml` + for a security advisory is what re-keyed the image and found it. + + So: every ``just `` the gate names in prose must be a recipe the + justfile actually defines. + """ + import ast + + justfile = (PROJECT_ROOT / "justfile").read_text(encoding="utf-8") + defined = set(re.findall(r"^([a-z_][\w-]*)\s*[\w\"=]*.*:", justfile, re.MULTILINE)) + + gate = PROJECT_ROOT / "src" / "capsem" / "gate" + named: dict[str, str] = {} + for module in sorted(gate.glob("*.py")): + tree = ast.parse(module.read_text(encoding="utf-8")) + # Docstrings are excluded deliberately. `hostimage.py` describes the + # earlier `_build-host-image` incident in prose, and a note about a + # recipe that never existed is the point of that note -- what must not + # exist is a *message handed to an operator* naming a dead command. + docstrings = { + ast.get_docstring(node, clean=False) + for node in ast.walk(tree) + if isinstance(node, ast.Module | ast.ClassDef | ast.FunctionDef) + } + for node in ast.walk(tree): + if not isinstance(node, ast.Constant) or not isinstance(node.value, str): + continue + if node.value in docstrings: + continue + # Backticked, which is how these messages name a command. Without + # it, ordinary prose ("just wrote the manifest") reads as a recipe. + for recipe in re.findall(r"`just ([a-z_][\w-]*)", node.value): + named.setdefault(recipe, module.name) + + missing = {name: where for name, where in named.items() if name not in defined} + assert not missing, ( + f"the gate tells an operator to run recipes that do not exist (recipe -> module): {missing}" + ) From ab834f320873d1b333607f385979e39242ebf16a Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Wed, 5 Aug 2026 08:39:48 -0400 Subject: [PATCH 17/18] test(gate): give the spawned child a module it can import The run-history contention test passed or failed on how pytest was typed. `multiprocessing`'s spawn start method re-imports its target's module in the child using nothing but a copy of the parent's `sys.path`, and `--import-mode=importlib` names test modules `tests.` without ever putting the repository root on that path. The name resolved under `python -m pytest` -- which contributes the working directory, and is how `pytestsuite` builds every gate invocation -- and not under the `pytest` console script, where the child died on `ModuleNotFoundError: No module named 'tests'` and the parent waited out a sixty-second queue timeout. The worker moves to `tests/helpers/runlog_worker.py`. `tests/` is on `sys.path` unconditionally: the root conftest puts it there before collection, under every invocation and in every xdist worker, which is the same wiring `config/gate.toml` already documents for `helpers.*`. Spawn is untouched -- the test is about cross-process rotation safety, and spawn is the stricter case as well as the macOS default. --- CHANGELOG.md | 14 +++++++++ tests/helpers/runlog_worker.py | 35 +++++++++++++++++++++++ tests/test_gate_run_history_contention.py | 17 +++-------- 3 files changed, 53 insertions(+), 13 deletions(-) create mode 100644 tests/helpers/runlog_worker.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b4a9abd2..1ce9574f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 was parsed as a call to ``linux-rust` `` -- trailing backtick included -- and reported as unknown. Three doctor checks went red on a comment. +- `test_a_live_run_is_never_rotated_away_by_another` no longer answers + differently depending on how pytest was started. A spawned child re-imports + its target's module with nothing but a copy of the parent's `sys.path`, and + `--import-mode=importlib` names test modules `tests.` without ever + putting the repository root on that path -- so the name resolved under + `python -m pytest`, which contributes the working directory and is how the + gate invokes every suite, and not under the `pytest` console script. Run the + file on its own and the child died on `ModuleNotFoundError: No module named + 'tests'` while the parent sat out a sixty-second queue timeout. The worker + moved to `tests/helpers/`, which the root conftest puts on `sys.path` before + collection under every invocation and in every xdist worker. Spawn is + unchanged: cross-process rotation safety is the point, and it is the + stricter start method. + ### Security - `fast-uri` is bounded past GHSA-7p8r-x3mc-p8w7 (host confusion via a diff --git a/tests/helpers/runlog_worker.py b/tests/helpers/runlog_worker.py new file mode 100644 index 000000000..c5da54a58 --- /dev/null +++ b/tests/helpers/runlog_worker.py @@ -0,0 +1,35 @@ +"""The child half of the run-log contention tests. + +The spawn start method pickles a process target by qualified name and re-imports +its module in the child, and the child has nothing but a copy of the parent's +`sys.path` to find that module with. A pytest test module is not reachable that +way. `--import-mode=importlib` names test modules `tests.` without +ever putting the repository root on `sys.path`, so the name resolves only when +the parent was started as `python -m pytest` -- which contributes the working +directory -- and not when it was started through the `pytest` console script. +The child died on `ModuleNotFoundError: No module named 'tests'`, the parent +waited out its queue timeout, and the same assertion about the same code passed +or failed depending on how somebody typed the command. + +`tests/` is on `sys.path` unconditionally: the root conftest puts it there +before collection, under every invocation and in every xdist worker. A target +that lives here is therefore importable in the child whichever way pytest was +started. Spawn targets for these tests belong in this module, not beside the +test that uses them. +""" + +from __future__ import annotations + +from multiprocessing.queues import Queue +from pathlib import Path + +from capsem.gate import config as gate_config +from capsem.gate.runlog import RunLog + + +def open_and_hold(root: str, name: str, ready: Queue, go: Queue) -> None: + """Open a run log, announce it, and hold it open until told to finish.""" + settings = gate_config.load(Path(root)) + with RunLog.open(settings, name) as log: + ready.put(log.directory.name) + go.get(timeout=60) diff --git a/tests/test_gate_run_history_contention.py b/tests/test_gate_run_history_contention.py index dc30f3aee..600383c35 100644 --- a/tests/test_gate_run_history_contention.py +++ b/tests/test_gate_run_history_contention.py @@ -22,22 +22,13 @@ import multiprocessing from pathlib import Path +from helpers.runlog_worker import open_and_hold + from capsem.gate import config as gate_config PROJECT_ROOT = Path(__file__).resolve().parents[1] -def _open_and_hold(root: str, name: str, ready, go) -> None: - """Open a run log, announce it, and hold it open until told to finish.""" - from capsem.gate import config as inner_config - from capsem.gate.runlog import RunLog - - settings = inner_config.load(Path(root)) - with RunLog.open(settings, name) as log: - ready.put(log.directory.name) - go.get(timeout=60) - - def _checkout(tmp_path: Path, *, keep_runs: int) -> Path: """A throwaway checkout whose retention keeps almost nothing.""" (tmp_path / "config").mkdir(parents=True, exist_ok=True) @@ -61,7 +52,7 @@ def test_a_live_run_is_never_rotated_away_by_another(tmp_path: Path) -> None: live = [] for name in ("candidate", "smoke"): - worker = context.Process(target=_open_and_hold, args=(str(root), name, ready, go)) + worker = context.Process(target=open_and_hold, args=(str(root), name, ready, go)) worker.start() live.append((worker, ready.get(timeout=60))) @@ -69,7 +60,7 @@ def test_a_live_run_is_never_rotated_away_by_another(tmp_path: Path) -> None: directory = root / settings.runlog.root try: # The third one rotates while both of those are still being written. - third = context.Process(target=_open_and_hold, args=(str(root), "lint", ready, go)) + third = context.Process(target=open_and_hold, args=(str(root), "lint", ready, go)) third.start() newest = ready.get(timeout=60) live.append((third, newest)) From 05502a0f56691515f2c22d34201528256eeb0e65 Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Wed, 5 Aug 2026 08:40:03 -0400 Subject: [PATCH 18/18] test(gate): answer the one probe the recorded plan reads `RecordingRunner` records commands and answers "" to anything without a canned reply, which is right for every probe whose output nothing reads. The git common-dir probe is not one of those: `docker_git_metadata_mount` reads the answer and builds a `-v` mount out of it, and refuses the build when the path does not resolve. An ordinary checkout never reaches that code -- its `.git` is a directory and the function returns early -- so the gap only opened from a linked worktree, where `.git` is a file and the probe runs. The plan then died at `package..build`, `build-linux-package.sh` was never issued, and nine ordering contracts failed for want of a git answer rather than for anything they assert. Whether the contracts held depended on the shape of the checkout they ran from. The recorder answers that probe from the real repository. Truthfully rather than with a constant: the value becomes a mount argument the same contracts read back. --- CHANGELOG.md | 12 ++++++++++++ tests/helpers/gate.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ce9574f6..dd2331804 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 unchanged: cross-process rotation safety is the point, and it is the stricter start method. +- The gate's ordering contracts run from a linked git worktree again. + `docker_git_metadata_mount` skips its probe entirely when `.git` is a + directory, so an ordinary checkout never asks; a worktree carries a `.git` + file instead, asks `git rev-parse --git-common-dir`, and got "" back from a + recording runner that answers nothing by default. An unresolvable common dir + is a build the gate rightly refuses, so the recorded plan died at + `package..build` and every contract about a command issued at or after + that point failed for want of a git answer rather than for anything it was + about -- on worktrees only, which is why it stayed invisible. The recorder + now answers that one probe from the real repository, truthfully, because a + wrong path here becomes a `-v` mount those same contracts assert against. + ### Security - `fast-uri` is bounded past GHSA-7p8r-x3mc-p8w7 (host confusion via a diff --git a/tests/helpers/gate.py b/tests/helpers/gate.py index e5664202c..c3b593340 100644 --- a/tests/helpers/gate.py +++ b/tests/helpers/gate.py @@ -25,6 +25,34 @@ PROJECT_ROOT = Path(__file__).resolve().parents[2] +#: The one probe whose *answer* decides which commands the plan goes on to +#: issue, rather than being recorded and never read. +#: +#: `docker_git_metadata_mount` skips this entirely when `.git` is a directory, +#: so an ordinary checkout never asks. A linked worktree carries a `.git` file +#: instead, asks, and gets "" back from a recorder that answers nothing -- an +#: unresolvable common dir, which the gate correctly refuses to build against. +#: The plan then dies at `package..build`, and every ordering contract +#: about a command issued at or after that point fails for want of a git +#: answer rather than for anything the contract is about. +#: +#: Answered truthfully, from the real repository, because a wrong path here +#: would be a `-v` mount these contracts then assert against. +GIT_COMMON_DIR_PROBE = "--git-common-dir" + + +@cache +def _git_common_dir(root: Path) -> str: + """What the probe would really have answered in this checkout.""" + found = subprocess.run( + ["git", "rev-parse", "--path-format=absolute", "--git-common-dir"], + cwd=root, + capture_output=True, + text=True, + check=False, + ) + return found.stdout.strip() + class RecordingRunner(Runner): """Records every command; answers with canned output. @@ -59,6 +87,9 @@ def execute(self, command: Command) -> subprocess.CompletedProcess[str]: if marker in rendered: stdout = reply break + else: + if GIT_COMMON_DIR_PROBE in rendered: + stdout = _git_common_dir(self.root) return subprocess.CompletedProcess( args=list(command.argv), returncode=status, stdout=stdout, stderr="" )