Skip to main content

max / alloy

Clear the recovery verdict per run, and say why passwd is read twice The last of the audit's minor findings. disk_intact outlived the run that set it. Nothing reaches a second run today, since keys are ignored once one starts and the sequence is never cleared, so this was latent rather than live. It is still a flag whose whole meaning is "what this run left on the disk", and one that survives into a run which has not established anything yet would let a recovery phrase be demanded for a volume this run never enrolled. A fresh one per run rather than a reset, so there is no shared state to reason about. The two reads of the target's passwd file are not redundant, which is worth writing down before somebody merges them. useradd writes to that file between them: the first read asks whether there is an account to create, the second asks what ids it turned out to get. Merged, the lookup would run before the line carrying them exists. The too_many_arguments allow on finish_plan was speculative. Clippy is clean without it.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-08-06 17:38 UTC
Signed with PGP, not checked
Commit: eeeb7689dcdc7b4b1a5de53e6aca0546b07a1832
Parent: dd7aca4
1 file changed, +29 insertions, -4 deletions
@@ -1814,7 +1814,8 @@
1814 1814 //
1815 1815 // Asked of the target's own passwd file rather than of getent,
1816 1816 // for the reason the id lookup below states: getent answers
1817 - // about the machine doing the installing.
1817 + // about the machine doing the installing. That lookup reads
1818 + // this file again on purpose, after useradd has written to it.
1818 1819 if passwd_ids(listing, &useradd_user).is_ok() {
1819 1820 return Ok(Vec::new());
1820 1821 }
@@ -1908,8 +1909,14 @@
1908 1909 }
1909 1910
1910 1911 stages.extend([
1911 - // First discovery: which uid and gid useradd picked. Reading the file
1912 - // rather than asking getent, because getent answers about this machine.
1912 + // Which uid and gid useradd picked. Reading the file rather than asking
1913 + // getent, because getent answers about this machine.
1914 + //
1915 + // The second read of this file, and not a redundant one: the account
1916 + // stage reads it first to decide whether it has an account to create,
1917 + // and useradd writes to it in between. The first read asks whether the
1918 + // account exists, this one asks what it turned out to be, and merging
1919 + // them would mean asking for ids before the line carrying them exists.
1913 1920 Stage::Resolve {
1914 1921 invocation: Invocation::new("cat").arg(format!("{ids_root}/etc/passwd")),
1915 1922 then: Box::new(move |listing| {
@@ -2444,7 +2451,6 @@
2444 2451 /// The whole of the install's post-deploy half, in the order the three parts
2445 2452 /// have to happen: nothing can be checked before it is configured, and nothing
2446 2453 /// can be checked after the mount it is read through is gone.
2447 - #[allow(clippy::too_many_arguments)]
2448 2454 fn finish_plan(
2449 2455 hostname: &str,
2450 2456 username: &str,
@@ -4219,6 +4225,10 @@
4219 4225 }
4220 4226
4221 4227 self.error = None;
4228 + // A fresh flag per run. The old one records what was true of the disk
4229 + // the last run left behind, and carrying that into a new run would let
4230 + // a phrase be demanded for a volume this run never enrolled.
4231 + self.disk_intact = Arc::new(AtomicBool::new(false));
4222 4232 let encryption = self.answers.encrypt.then(|| EncryptionChoice {
4223 4233 passphrase: self.passphrase.value().to_string(),
4224 4234 recovery: self.recovery.clone().unwrap_or_default(),
@@ -7038,6 +7048,21 @@
7038 7048 );
7039 7049 }
7040 7050
7051 + // The flag records what was true of the disk a run left behind, so a run
7052 + // that inherited it could demand a phrase for a volume it never enrolled.
7053 + #[test]
7054 + fn a_new_run_does_not_inherit_the_last_ones_verdict() {
7055 + let (mut view, mut log) = view();
7056 + view.answers.disk = Some("/dev/sda".into());
7057 + view.answers.hostname = Some("workshop".into());
7058 + view.answers.username = Some("max".into());
7059 + view.disk_intact.store(true, Ordering::Relaxed);
7060 +
7061 + view.confirmed(&mut log);
7062 +
7063 + assert!(!view.disk_intact.load(Ordering::Relaxed));
7064 + }
7065 +
7041 7066 // ---- the commit boundary ----
7042 7067
7043 7068 /// A header as bootc leaves it: one keyslot, claimed by a systemd-tpm2