From b9700bb8f82a3aca7efc7fc1424bcfda4795ad64 Mon Sep 17 00:00:00 2001 From: "Youri T. K. K. Mattar" Date: Tue, 11 Aug 2026 16:07:58 -0300 Subject: [PATCH] chore: enforce the invariants the documents assert MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pass that produced M6, M11, M13 and M14 was reading a document and checking the code against it. This is the same pass applied to the invariants rather than the behaviours. "A reviewer should be able to audit all unsafe in the project by reading one file." ARCHITECTURE has said that since before there was code, and it was true by everyone's discipline — nothing stopped a second unsafe block appearing in the supervisor or the unit parser. The oxinit crate now denies unsafe_code with sys::raw as the single #[allow], and every crate that should contain none carries forbid, which nothing downstream can relax. Checked by adding an unsafe block to reap.rs and watching the build fail, and again in oxinit-unit. The two fixtures keep theirs: reconstructing an inherited descriptor is from_raw_fd, and they ship only in a test image. rust-version = "1.95" is a promise to people on older toolchains, and CI ran whatever stable was that week — a dependency bump raising the real floor would have been found by whoever it broke. A job builds on the declared minimum now; verified locally against 1.95 first. "The dependency tree of PID 1 is part of its attack surface" is in CLAUDE.md and every dependency's commit message justifies it, and nothing watched them afterwards. cargo audit runs on every push, and blocks: an advisory against something inside PID 1 is not a warning to look at later. Thirty-five crates in the lockfile, none affected today. One thing was actually wrong rather than merely unguarded: `tests/` appeared in the workspace layout in both CLAUDE.md and ARCHITECTURE.md and has never existed. The integration tests are the xtask suites, and the directory that does exist and was not listed is units/. --- .github/workflows/ci.yml | 41 ++++++++++++++++++++++++++++ ARCHITECTURE.md | 22 ++++++++++++--- CLAUDE.md | 2 +- README.md | 14 +++++++--- ROADMAP.md | 46 ++++++++++++++++++++++++++++++++ crates/oxctl/src/main.rs | 2 ++ crates/oxinit-cgroup/src/lib.rs | 1 + crates/oxinit-graph/src/lib.rs | 1 + crates/oxinit-ipc/src/lib.rs | 1 + crates/oxinit-log/src/lib.rs | 1 + crates/oxinit-service/src/lib.rs | 1 + crates/oxinit-timer/src/lib.rs | 1 + crates/oxinit-unit/src/lib.rs | 1 + crates/oxinit-user/src/lib.rs | 1 + crates/oxinit/src/main.rs | 8 +++++- crates/oxinit/src/sys/mod.rs | 3 +++ crates/oxlogd/src/main.rs | 2 ++ crates/xtask/src/main.rs | 2 ++ 18 files changed, 141 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8796354..8029b52 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -52,6 +52,47 @@ jobs: - name: Host tests run: cargo test -p oxinit-unit -p oxinit-graph -p oxinit-service -p oxinit-cgroup -p oxinit-user -p oxinit-ipc -p oxinit-log -p oxinit-timer + msrv: + name: builds on the declared MSRV + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + # `rust-version` in Cargo.toml is a promise to anyone on an older + # toolchain, and CI otherwise runs whatever `stable` is that week — so + # the promise was never checked. A dependency bump or a language feature + # raising the real floor would go unnoticed until someone on the + # declared minimum tried to build. + - name: Install the declared minimum + run: | + MSRV=$(grep '^rust-version' Cargo.toml | cut -d'"' -f2) + echo "MSRV is $MSRV" + rustup toolchain install "$MSRV" --profile minimal + rustup target add x86_64-unknown-linux-musl --toolchain "$MSRV" + echo "MSRV=$MSRV" >> "$GITHUB_ENV" + + - uses: Swatinem/rust-cache@v2 + + # `rustup run` rather than `cargo +TOOLCHAIN`: the latter needs cargo to + # be rustup's shim, and it is not on every developer's machine. + - run: rustup run "$MSRV" cargo check --workspace --all-targets --target x86_64-unknown-linux-musl + + advisories: + name: dependency advisories + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: Swatinem/rust-cache@v2 + + # CLAUDE.md: "The dependency tree of PID 1 is part of its attack + # surface." Seven direct dependencies, each justified in the commit that + # added it — and nothing was watching them for advisories afterwards. + # + # Blocking on purpose. An advisory published against something inside + # PID 1 is not a warning to look at later. + - run: cargo install cargo-audit --locked + - run: cargo audit + boot: name: boot (${{ matrix.arch }}) runs-on: ubuntu-latest diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 4f04e7f..05ba050 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -27,7 +27,9 @@ PID 1. PID 1 cannot exit. If it does, the kernel panics. Everything below follows from that. -**No panic.** The `oxinit` crate denies the lints that produce them: +**No panic.** Every crate that runs inside PID 1 denies the lints that produce +them — not just the binary, because a panic in the unit parser is a panic in +PID 1 exactly as much as one in the event loop: ```rust #![deny( @@ -38,6 +40,10 @@ that. )] ``` +`oxctl`, `oxlogd` and `xtask` do not carry it. They are separate processes, and +a panic in one kills that process and nothing else — which is the whole reason +they are separate. + Errors are values. Fallible operations return `Result`. Error types are defined with `thiserror` per crate. @@ -83,7 +89,17 @@ Some things `rustix` does not cover — `fork` semantics between fork and exec, invariant holds at that call site. A reviewer should be able to audit all unsafe in the project by reading one -file. +file — and that is enforced rather than asked for. The `oxinit` crate carries +`#![deny(unsafe_code)]`, and `sys::raw` is the single module that relaxes it +with an `#[allow]`; a second `unsafe` block anywhere else in the crate stops +compiling. Every crate that should contain none at all carries +`#![forbid(unsafe_code)]`, which cannot be relaxed by anything downstream of +it. + +The two test fixtures, `notify-probe` and `listen-probe`, are the exception: +they reconstruct inherited descriptors, which is `from_raw_fd` and therefore +`unsafe`. They are not part of the running system and are installed only in a +test image. ## Concurrency @@ -906,7 +922,7 @@ oxinit/ │ ├─ listen-probe/ # test fixture: a socket-activated service │ └─ xtask/ # build and boot automation ├─ docs/ -└─ tests/ +└─ units/ # the test image's unit files ``` `oxctl` and `oxinit-ipc` arrive in M5. Everything else exists. diff --git a/CLAUDE.md b/CLAUDE.md index a4f1aa3..506ed90 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -21,7 +21,7 @@ crates/oxinit-cgroup/ cgroup v2 crates/oxinit-ipc/ control protocol types crates/xtask/ build and boot automation docs/ specifications -tests/ integration tests +units/ the test image's unit files ``` `oxinit-unit`, `oxinit-graph`, and `oxinit-service` must stay free of diff --git a/README.md b/README.md index fbafd2b..af3ac86 100644 --- a/README.md +++ b/README.md @@ -225,15 +225,21 @@ container's PID 1. | **M12** | CI: every suite above, on every push. | | **M13** | Coverage for seven behaviours the milestones claimed and nothing re-ran. | | **M14** | Calendar schedules on the wall clock; datagram sockets; the list closed. | +| **M15** | The documents' invariants enforced by the build, not by discipline. | One `epoll` loop multiplexes the signalfd, the timerfd, the notify socket, the control socket, every socket unit's listening descriptor and every service cgroup's `cgroup.events`. One thread. No async runtime. -Nothing is scheduled after M14, and [ROADMAP.md](ROADMAP.md) says which two -remaining ideas are deliberately not being taken and why. [ROADMAP.md](ROADMAP.md) has the breakdown, -including what each milestone was verified against and what was deferred out -of it. +Nothing is scheduled after M15. [ROADMAP.md](ROADMAP.md) has the breakdown — +what each milestone was verified against, what was deferred out of it, and +which remaining ideas are deliberately not being taken. + +Every `unsafe` block in the project lives in one file, and that is a property +of the build rather than of anyone's discipline: `oxinit` denies `unsafe_code` +with a single module relaxing it, and every crate that should contain none +carries `forbid`. CI also builds on the declared MSRV and checks the dependency +tree for advisories, because both were promises nothing was keeping. ## Running it diff --git a/ROADMAP.md b/ROADMAP.md index f28b213..760d773 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -848,6 +848,52 @@ do. A fixture for it would test the fixture. Verified: 41 checks per architecture, 43 against the distribution image, 148 host tests. +## M15 — Enforcing what the documents assert + +**Done.** + +The pattern that produced M6, M11, M13 and M14 was reading a document and +checking the code against it. This is the same pass applied to the *invariants* +rather than the behaviours — the things the documents state as properties of +the project, which nothing was checking. + +- [x] `#![forbid(unsafe_code)]` on every crate that should contain none, and + `#![deny(unsafe_code)]` on `oxinit` with exactly one module relaxing it. +- [x] A CI job that builds on the declared MSRV. +- [x] A CI job that checks the dependency tree for advisories. +- [x] `tests/`, listed in two documents and never created. + +**"A reviewer should be able to audit all unsafe in the project by reading one +file."** ARCHITECTURE has said that since before there was code. It was true, +and it was true by everyone's discipline: nothing stopped a second `unsafe` +block appearing in the supervisor or in the unit parser. Now the crate denies +`unsafe_code` and `sys::raw` is the single `#[allow]`, so a second one stops +compiling. Checked by adding one to `reap.rs` and watching the build fail, and +again in `oxinit-unit`, which cannot even be relaxed — `forbid` is not +overridable downstream. + +The two test fixtures keep theirs. Reconstructing an inherited descriptor is +`from_raw_fd`, which is `unsafe` by construction, and they are installed only +in a test image. + +**The MSRV was a promise to strangers and nothing kept it.** `rust-version = +"1.95"` is a claim about people on older toolchains, and CI ran whatever +`stable` was that week — so a dependency bump raising the real floor would have +been discovered by whoever it broke. It builds on 1.95 today; from now on that +is a fact the build reports rather than an assumption. + +**"The dependency tree of PID 1 is part of its attack surface"** is in +CLAUDE.md, and every dependency's commit message justifies it. Nothing watched +them afterwards. `cargo audit` runs on every push, blocking — an advisory +against something inside PID 1 is not a warning to look at later. Thirty-five +crates in the lockfile, no advisories today. + +Nothing was broken here either. Like M13, this bought enforcement rather than +repair — with one exception: `tests/` appeared in the workspace layout in both +CLAUDE.md and ARCHITECTURE.md and had never existed. The integration tests are +the `xtask` suites, and the directory that does exist and was not listed is +`units/`. + ## Not doing, and why These were on the list. They are coming off it with a reason rather than diff --git a/crates/oxctl/src/main.rs b/crates/oxctl/src/main.rs index a0d098b..45bb293 100644 --- a/crates/oxctl/src/main.rs +++ b/crates/oxctl/src/main.rs @@ -7,6 +7,8 @@ //! Not PID 1. This process may panic, may exit, and is held to none of the //! rules the `oxinit` crate carries. +#![forbid(unsafe_code)] + use std::path::Path; use std::process::ExitCode; diff --git a/crates/oxinit-cgroup/src/lib.rs b/crates/oxinit-cgroup/src/lib.rs index 6d862f2..b4fa891 100644 --- a/crates/oxinit-cgroup/src/lib.rs +++ b/crates/oxinit-cgroup/src/lib.rs @@ -14,6 +14,7 @@ //! //! This runs inside PID 1, so a panic here is a panic in PID 1. +#![forbid(unsafe_code)] #![deny( clippy::unwrap_used, clippy::expect_used, diff --git a/crates/oxinit-graph/src/lib.rs b/crates/oxinit-graph/src/lib.rs index 48e7c7c..3038002 100644 --- a/crates/oxinit-graph/src/lib.rs +++ b/crates/oxinit-graph/src/lib.rs @@ -9,6 +9,7 @@ //! This code runs inside PID 1, so the same failure policy applies: a panic //! here is a panic in PID 1. +#![forbid(unsafe_code)] #![deny( clippy::unwrap_used, clippy::expect_used, diff --git a/crates/oxinit-ipc/src/lib.rs b/crates/oxinit-ipc/src/lib.rs index 5cfe11b..0291409 100644 --- a/crates/oxinit-ipc/src/lib.rs +++ b/crates/oxinit-ipc/src/lib.rs @@ -15,6 +15,7 @@ //! //! This runs inside PID 1, so a panic here is a panic in PID 1. +#![forbid(unsafe_code)] #![deny( clippy::unwrap_used, clippy::expect_used, diff --git a/crates/oxinit-log/src/lib.rs b/crates/oxinit-log/src/lib.rs index cd76272..c9e7160 100644 --- a/crates/oxinit-log/src/lib.rs +++ b/crates/oxinit-log/src/lib.rs @@ -11,6 +11,7 @@ //! //! `oxinit` depends on this crate, so a panic here is a panic in PID 1. +#![forbid(unsafe_code)] #![deny( clippy::unwrap_used, clippy::expect_used, diff --git a/crates/oxinit-service/src/lib.rs b/crates/oxinit-service/src/lib.rs index 63d58a4..a4d4df7 100644 --- a/crates/oxinit-service/src/lib.rs +++ b/crates/oxinit-service/src/lib.rs @@ -9,6 +9,7 @@ //! //! This runs inside PID 1, so a panic here is a panic in PID 1. +#![forbid(unsafe_code)] #![deny( clippy::unwrap_used, clippy::expect_used, diff --git a/crates/oxinit-timer/src/lib.rs b/crates/oxinit-timer/src/lib.rs index eac5576..f65b128 100644 --- a/crates/oxinit-timer/src/lib.rs +++ b/crates/oxinit-timer/src/lib.rs @@ -13,6 +13,7 @@ //! //! `oxinit` depends on this crate, so a panic here is a panic in PID 1. +#![forbid(unsafe_code)] #![deny( clippy::unwrap_used, clippy::expect_used, diff --git a/crates/oxinit-unit/src/lib.rs b/crates/oxinit-unit/src/lib.rs index 42c8895..78a2baa 100644 --- a/crates/oxinit-unit/src/lib.rs +++ b/crates/oxinit-unit/src/lib.rs @@ -9,6 +9,7 @@ //! The same failure policy as the `oxinit` binary applies: this code runs //! inside PID 1, so a panic here is a panic in PID 1. +#![forbid(unsafe_code)] #![deny( clippy::unwrap_used, clippy::expect_used, diff --git a/crates/oxinit-user/src/lib.rs b/crates/oxinit-user/src/lib.rs index 6d2c050..c4ccb3f 100644 --- a/crates/oxinit-user/src/lib.rs +++ b/crates/oxinit-user/src/lib.rs @@ -14,6 +14,7 @@ //! //! This runs inside PID 1, so a panic here is a panic in PID 1. +#![forbid(unsafe_code)] #![deny( clippy::unwrap_used, clippy::expect_used, diff --git a/crates/oxinit/src/main.rs b/crates/oxinit/src/main.rs index d7fb13b..711beb5 100644 --- a/crates/oxinit/src/main.rs +++ b/crates/oxinit/src/main.rs @@ -11,7 +11,13 @@ clippy::unwrap_used, clippy::expect_used, clippy::panic, - clippy::indexing_slicing + clippy::indexing_slicing, + // ARCHITECTURE.md says a reviewer should be able to audit every `unsafe` + // in the project by reading one file. This is what makes that a property + // of the build rather than of everyone's discipline: the crate denies + // `unsafe`, and exactly one module relaxes it. Adding a second `unsafe` + // block anywhere else stops compiling. + unsafe_code )] // Without this, building on a non-Linux host fails deep inside the signalfd diff --git a/crates/oxinit/src/sys/mod.rs b/crates/oxinit/src/sys/mod.rs index 9d0f005..68a4a3d 100644 --- a/crates/oxinit/src/sys/mod.rs +++ b/crates/oxinit/src/sys/mod.rs @@ -3,4 +3,7 @@ //! Everything here goes through rustix except what lives in [`raw`], which is //! the only module in the crate allowed to contain `unsafe`. +// The one relaxation of the crate-level `deny(unsafe_code)`, and the whole of +// why the claim above is checkable rather than merely asserted. +#[allow(unsafe_code)] pub mod raw; diff --git a/crates/oxlogd/src/main.rs b/crates/oxlogd/src/main.rs index 90abed1..e984119 100644 --- a/crates/oxlogd/src/main.rs +++ b/crates/oxlogd/src/main.rs @@ -20,6 +20,8 @@ //! dropped. `epoll` pays for a registration call per change to make waiting //! cheap on a large stable set, and there is neither here. +#![forbid(unsafe_code)] + use std::collections::HashMap; use std::fs::{File, OpenOptions}; use std::io::{IoSliceMut, Write as _}; diff --git a/crates/xtask/src/main.rs b/crates/xtask/src/main.rs index 1515f6f..c707c26 100644 --- a/crates/xtask/src/main.rs +++ b/crates/xtask/src/main.rs @@ -6,6 +6,8 @@ //! This runs on the developer's machine. The rules that apply to the `oxinit` //! crate — no panic, no unsafe, no async — do not apply here. +#![forbid(unsafe_code)] + use std::env; use std::fs; use std::path::{Path, PathBuf};