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};