Tests and CI: run the root suite on PRs, make skips visible, assert on output, fix tautological tests, tighten CI hygiene

[pierre-warnier]

Findings on the test suite, CI and build system.

Coverage that does not exist

• The PAM path had no automated coverage until the e2e image was repaired; keep it that way and add #[cfg(feature = "pam")] tests that drive authenticate_caller against pam_permit/pam_deny stacks in the Docker images.
• ~120 of 162 integration tests return silently when not root and are reported ok; the only root run (Docker matrix) is if: github.event_name == 'push', so on a pull request about 75 % of the suite does not execute. Run the matrix on PRs (each job is under 4 min), make skips visible (SHADOW_TEST_REQUIRE_ROOT=1 in the containers turns a skip into a failure), and un-gate the pwck/grpck/chage tests that never needed root.
• The Docker matrix runs without --features pam; root+PAM is never unit-tested.
• Tests call uumain() in-process and assert on nothing but exit codes: zero stdout/stderr assertions across the suite, so "bit-for-bit identical output" is unverified. Spawn CARGO_BIN_EXE_shadow-rs instead, which also stops harden_process, setuid(0), Landlock and umask changes from leaking between tests in one process.
• Tautological tests: test_chage.rs and test_chpasswd.rs never invoke their tool (they set a shadow field and read it back); test_pwck accepts 0 || 2; test_chsh passes because PAM fails first; test_chfn discards its fixture; test_passwd discards eight return codes and races four threads on a PID-keyed lock. Rewrite against spawned binaries once chage/chpasswd/chfn/chsh have a --prefix.
• No test that the multicall binary registers every tool (one table now drives dispatch and --list; a test that spawns shadow-rs <tool> --help for all fourteen names closes it).
• Fuzz targets are not built in CI, have no subid target, and assert only "no panic"; add parse→Display→parse and no-separator invariants. The corpus is tracked but gitignored — decide.

CI

• No permissions: blocks; actions pinned by tag, not digest (helpers:pinGitHubActionDigests in Renovate).
• MSRV job runs cargo check --workspace without --all-targets/--all-features; completions and (until shadow-core: keep ownership on rewrite, refuse field injection, never abort on '..' #238) selinux were compiled by nobody. Add an --all-features clippy pass.
• no-ai-traces.sh: the regex is not word-boundaried (llm matches smallmap, anthropic matches philanthropic), so it will block legitimate PRs touching CONTRIBUTING.md's tooling policy paragraph, and it misses gpt-4o, cursor, aider, co-authored-by: *[bot]. ${{ github.base_ref }} is interpolated into run:; pass it through env:.
• audit.yml duplicates the deny job; keep one scheduled advisory check.
• Two hook systems (hooks/ in Docker vs .pre-commit-config.yaml on the host with a different feature set); pre-push hook runs the full multi-distro suite and gets bypassed #231 already tracks the pre-push cost. Keep one, and encode the CI commands in make check/make test so README, CONTRIBUTING, hooks and ci.yml stop repeating them.

Build and dependencies

• clap/derive is enabled and unused; it alone pulls clap_derive + syn 3 next to syn 2 and is why deny.toml skips the duplicate.
• Makefile installs all fourteen tools into $(PREFIX)/sbin; GNU puts passwd chage chfn chsh newgrp in bin (non-root users rarely have sbin in PATH). make test runs without pam while install ships it.
• shadow-rs.spec (version 0.2.0, %files lists a binary make install no longer produces) and debian/ are stale and would not build; tests/gnu-compat.sh compares full output for two commands only. Fix or delete.
• rust:latest, rust:alpine, fedora:latest and get.nexte.st/latest are unpinned (nextest is never used); toolchain drift lands on main with no PR signal.
• deny.toml allows MPL-2.0, ISC, BSD-2-Clause, CC0, Zlib that nothing uses — tighten to the graph; document that self_cell is accepted under its Apache option; unknown-registry/unknown-git → deny.
• Fourteen [[test]] binaries each linking all fourteen crates, with fn run, read_shadow, setup_prefix … duplicated up to 14×; the uutils layout (tests/tests.rs + mod by_util + a real common) is the fix.

[pierre-warnier]

Partly done in #259:

• the Docker matrix (where the root-only tests actually run) now runs on pull requests, not only on push — roughly three quarters of the integration suite was skipping silently and being reported as ok;
• the MSRV job covers --all-targets;
• both workflows declare least-privilege token permissions;
• the trace guard's patterns are genuinely word-boundaried (they matched substrings inside ordinary words and failed unrelated changes) and cover two more artifact forms;
• the unused clap derive feature is gone, the license allowlist no longer pre-approves MPL-2.0, and a crate from outside crates.io is denied rather than warned about.

The end-to-end image was repaired and wired into CI separately, in #251.

Still open here: the test architecture — spawn the binary and assert on stdout/stderr instead of calling uumain in-process with no output assertions, make root-only skips visible instead of silently ok, replace the tautological tests (test_chage.rs and test_chpasswd.rs never invoke their tool), give chage/chpasswd/chfn/chsh a prefix option so they can be tested at all, one test binary instead of fourteen, and wire the fuzz targets into CI.

[pierre-warnier]

Done across #273, #275 and #281. Three real defects came out of the work, which
is the argument for it.

Coverage

The suite asserted almost nothing. Every test called uumain in-process and
checked an exit code; not one assertion looked at a byte of output. common now
spawns the real binary and asserts on both streams. Within the hour that found:

• passwd -l was not idempotent — eight concurrent locks left eight markers,
  and passwd -u refuses to unlock an account that would stay locked, so an
  account locked twice could not be unlocked with the tool at all.
• chage exited 3 for an invalid date where 3 means an unexpected failure.
• pwck's duplicate-UID test accepted either 0 or 2, so it asserted nothing.

Skips that looked like passes. SHADOW_TEST_REQUIRE_ROOT turns a skip into
a failure and the containers set it. Run as an ordinary user with it on, 27
useradd tests that reported ok fail immediately — that is how much was
invisible.

Tests that never ran their tool. test_chage and test_chpasswd set a
field through shadow-core and read it back. Both drive the binary through
--prefix now.

The concurrency test raced four threads against a lock keyed on the process
id, which same-process threads never contend for, and discarded all eight return
codes. Eight processes now, every code checked.

Root and PAM together were exercised by no job; the matrix runs both
configurations. PAM had no negative test — the deployment image can only
authenticate a real account. Two throwaway services, pam_permit and
pam_deny, now cover the refusal path and distinguish it from a conversation
that never reached the stack.

New: the multicall binary is tested for the property its dispatch table is
supposed to guarantee, and the GNU comparison suite is real — passwd -S and
chage -l compared byte for byte across eleven aging configurations, with one
deliberate divergence asserted as such.

One test binary instead of fourteen, each of which linked all fourteen
crates.

Fuzzing

Targets asserted only "does not panic"; they now check that a rendered record
carries exactly the separators its format has and parses back identically.
/etc/subuid had no target. The crate is a separate workspace nothing built, so
it had rotted — a stable cargo check in CI catches that. The corpus was both
gitignored and tracked; it is kept, and the main suite replays its 667 inputs on
every pull request.

CI

Actions pinned by digest with Renovate keeping them current. One advisory check,
on a schedule. --all-features clippy, which nothing compiled before. make check is the single definition of what CI gates on, and the hooks and
CONTRIBUTING refer to it. The two hook systems are kept and the reason
written down: they do not overlap.

Build and dependencies

Base images pinned; an unused, unpinned cargo-nextest download removed. The
licence allow list narrowed to exactly the graph. clap's derive feature was
already absent — the syn duplicate comes from landlock and uucore, as
deny.toml says. make install puts the five user-facing tools in bin, as
GNU does; passwd was previously unreachable for the people it is setuid for.
The RPM spec and debian/ are removed: neither could build, and debian/ had
no changelog at all.