Skip to main content

max / alloy

Route every child spawn through one place, and give it the sbin directories Problem d0dc9dad measured fw12's session PATH carrying neither /sbin nor /usr/sbin and concluded that bare-name spawns of cryptsetup, wipefs, useradd, chpasswd, chroot, setfiles, udevadm and rfkill would fail. That conclusion was never tested and it is false on the image Alloy ships: Fedora 42 merged /usr/sbin into /usr/bin, both are symlinks there, and all eight resolve under the measured PATH. What lands here is the insurance rather than the fix. cli::child_command becomes the crate's only Command::new and appends both directories to every child's PATH, so a host that did not merge them, a future base, or a tool that moves is covered. bluetooth.rs drops its ad hoc /usr/sbin/rfkill fallback list so one mechanism is the only mechanism.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-08-19 16:59 UTC
Signed with PGP, not checked
Commit: cd9d67a9b3695f4472874dc22f358a3a9d8b1ba1
Parent: bb27b4d
3 files changed, +376 insertions, -39 deletions
M crates/alloy/src/bluetooth.rs +111 -32
@@ -345,10 +345,16 @@
345 345 /// Everything bluez keeps about one device.
346 346 fn info(&self, address: &str, log: &mut CommandLog) -> Result<Device>;
347 347
348 - /// The radio, read without logging. rfkill is the console's own
348 + /// The radio, read without logging the command. rfkill is the console's own
349 349 /// bookkeeping: the user asked about Bluetooth, not about rfkill, and the
350 350 /// answer only ever appears as a phrase in the header.
351 - fn radio(&self) -> Radio {
351 + ///
352 + /// The log is still taken, for the one case that is not bookkeeping. A read
353 + /// that succeeds and names no Bluetooth line is a real answer of
354 + /// [`Radio::Unknown`] and stays quiet; a read that could not run at all is a
355 + /// fault, and the header phrase it produces reads identically. See
356 + /// [`radio_from`].
357 + fn radio(&self, _log: &mut CommandLog) -> Radio {
352 358 Radio::Unknown
353 359 }
354 360
@@ -453,11 +459,8 @@
453 459 parse_info(&Self::ctl().args(["info", address]).run(log)?)
454 460 }
455 461
456 - fn radio(&self) -> Radio {
457 - RFKILL_PROGRAMS
458 - .iter()
459 - .find_map(|program| rfkill_invocation(program).capture_quiet().ok())
460 - .map_or(Radio::Unknown, |raw| parse_rfkill(&raw))
462 + fn radio(&self, log: &mut CommandLog) -> Radio {
463 + radio_from(rfkill_invocation().capture_quiet(), log)
461 464 }
462 465
463 466 fn pair(&self, device: &Device) -> Option<Invocation> {
@@ -561,27 +564,30 @@
561 564 }
562 565 }
563 566
564 - /// Where to look for rfkill, in order.
567 + /// rfkill, by bare name.
565 568 ///
566 - /// The bare name first, and an absolute path after it, because the console
567 - /// cannot count on `PATH` carrying `/usr/sbin`. A booted Alloy session was
568 - /// measured on fw12 with a `PATH` of exactly
569 - /// `~/.local/bin:~/.cargo/bin:/usr/local/bin:/usr/bin:/usr/local/sbin`, no
570 - /// `/sbin` and no `/usr/sbin` (GoingsOn problem `d0dc9dad`), and rfkill ships in
571 - /// `/usr/sbin`. Fedora 43 unifies bin and sbin, so the bare name probably
572 - /// resolves there and this costs one extra spawn on a machine where it does not.
569 + /// It was a two-element list, `["rfkill", "/usr/sbin/rfkill"]`, tried in order
570 + /// because the console cannot count on `PATH` carrying `/usr/sbin` (GoingsOn
571 + /// problem `d0dc9dad`, measured on fw12). The second element was never observed
572 + /// to be the one that worked: on the shipped image `/usr/sbin` is a symlink to
573 + /// `bin` and the bare name resolves, which
574 + /// `cli::SBIN_DIRS` sets out with the measurement. What
575 + /// replaced it is [`child_command`](crate::cli::child_command), which puts the
576 + /// sbin directories on the `PATH` of every child rather than on this one call
577 + /// site, so the pane shows the name a user would type and no tool added later
578 + /// has to remember a second spelling of itself.
579 + ///
580 + /// The spare spawn also carried a guard, and dropping the spawn is not a reason
581 + /// to drop the guard: what it covered was that a failed read is indistinguishable
582 + /// on screen from a successful read of a machine with no Bluetooth rfkill line.
583 + /// [`radio_from`] is where that lives now, and it reports rather than retries.
573 584 ///
574 - /// Worth the spawn because the failure is silent: a radio read that errors
575 - /// becomes [`Radio::Unknown`], which reads on screen as "no rfkill here" and
576 - /// quietly drops the hard-block detection rather than reporting a fault.
577 - const RFKILL_PROGRAMS: [&str; 2] = ["rfkill", "/usr/sbin/rfkill"];
578 -
579 585 /// `--noheadings --output` rather than the default listing or `--json`:
580 586 /// util-linux 2.39 accepts `--json` and ignores it, printing the human format,
581 587 /// so a JSON parser here would fail on the machine it was written for. Measured
582 588 /// on fw13, 2026-08-05.
583 - fn rfkill_invocation(program: &str) -> Invocation {
584 - Invocation::new(program).args([
589 + fn rfkill_invocation() -> Invocation {
590 + Invocation::new("rfkill").args([
585 591 "--noheadings",
586 592 "--output",
587 593 "TYPE,SOFT,HARD",
@@ -590,6 +596,48 @@
590 596 ])
591 597 }
592 598
599 + /// A radio verdict from an rfkill read, with a failed read said out loud.
600 + ///
601 + /// [`Radio::Unknown`] is two different facts wearing one name. rfkill ran and
602 + /// listed no Bluetooth line, which is an answer: the machine has no Bluetooth
603 + /// radio rfkill knows about, and the header correctly says nothing about
604 + /// blocking. Or rfkill did not run, in which case the console knows nothing and
605 + /// the header says nothing about blocking anyway, which is the same screen for
606 + /// the opposite reason. The second case also drops hard-block detection, so an
607 + /// adapter that is off because of a physical switch reads as an adapter that is
608 + /// simply off, and the advice the user gets is advice that cannot work.
609 + ///
610 + /// Widening [`Radio`] with a `Failed` variant was the other option and was not
611 + /// taken: nothing on screen would render it differently from `Unknown`, since
612 + /// there is nothing useful to tell a user about the console's own bookkeeping
613 + /// tool, and a variant every match arm has to name for no visible difference is
614 + /// cost without a reader. The log pane is where the console says what it did, so
615 + /// the fault goes there as commentary, in the form the mock backends already use
616 + /// for a line that is not a command.
617 + ///
618 + /// Split out from [`Backend::radio`] so the failure path has a test. Spawning an
619 + /// rfkill that fails would mean depending on the machine the tests run on, and a
620 + /// test that passes because this machine happens to have rfkill is a test that
621 + /// proves nothing.
622 + ///
623 + /// One limit, stated rather than hidden: two of the refresh paths run inside
624 + /// [`CommandLog::quiet`](crate::cli::CommandLog::quiet), so a fault they hit is
625 + /// suppressed with everything else on those paths. The first read of the screen
626 + /// and the manual `r` are both loud, which is where a persistent fault will be
627 + /// seen; a fault that only ever appears under a quiet refresh will not be.
628 + fn radio_from(read: Result<String>, log: &mut CommandLog) -> Radio {
629 + match read {
630 + Ok(raw) => parse_rfkill(&raw),
631 + Err(err) => {
632 + log.record(
633 + format!("# could not read the radio: rfkill did not run ({err})"),
634 + Severity::Warn,
635 + );
636 + Radio::Unknown
637 + }
638 + }
639 + }
640 +
593 641 // ---- parsing ----
594 642
595 643 /// One `Key: value` field from `show` or `info` output.
@@ -873,7 +921,7 @@
873 921 /// A failed read keeps the previous rows and reports the error, on `disk`'s
874 922 /// rule: stale rows are still the best information available.
875 923 fn refresh(&mut self, log: &mut CommandLog) {
876 - self.radio = self.backend.radio();
924 + self.radio = self.backend.radio(log);
877 925
878 926 match self.backend.adapter(log) {
879 927 Ok(adapter) => {
@@ -1793,19 +1841,50 @@
1793 1841 assert!(device.caveat().unwrap().contains("does not survive"));
1794 1842 }
1795 1843
1796 - /// rfkill lives in `/usr/sbin`, and a booted Alloy session has no
1797 - /// `/usr/sbin` in `PATH` (problem `d0dc9dad`). The absolute path is the
1798 - /// fallback, and its absence would silently drop hard-block detection
1799 - /// rather than reporting anything.
1844 + /// rfkill ships in `/usr/sbin`, which on the shipped image is a symlink to
1845 + /// `bin`, so the bare name resolves; finding it on a host where it does not
1846 + /// is `cli::child_command`'s job. Asserted here so the line cannot drift
1847 + /// back to an absolute path and split that across two mechanisms again;
1848 + /// `tests/sbin_path.rs` is the same rule for the rest of the crate.
1800 1849 #[test]
1801 - fn rfkill_is_looked_for_outside_path_as_well() {
1802 - assert_eq!(RFKILL_PROGRAMS, ["rfkill", "/usr/sbin/rfkill"]);
1850 + fn rfkill_is_asked_for_by_bare_name() {
1803 1851 assert_eq!(
1804 - rfkill_invocation("/usr/sbin/rfkill").display(),
1805 - "/usr/sbin/rfkill --noheadings --output TYPE,SOFT,HARD list bluetooth"
1852 + rfkill_invocation().display(),
1853 + "rfkill --noheadings --output TYPE,SOFT,HARD list bluetooth"
1806 1854 );
1807 1855 }
1808 1856
1857 + /// A read that could not run is a fault, and it produces the same header as
1858 + /// a machine with no Bluetooth rfkill line. The old second spawn existed to
1859 + /// cover that; nothing covers it if the failure stays quiet.
1860 + #[test]
1861 + fn a_radio_read_that_did_not_run_says_so() {
1862 + let mut log = CommandLog::new();
1863 + let radio = radio_from(Err(anyhow::anyhow!("No such file or directory")), &mut log);
1864 + assert_eq!(radio, Radio::Unknown);
1865 +
1866 + let entry = log.entries().last().expect("the fault is recorded");
1867 + assert!(
1868 + entry.command.contains("rfkill did not run"),
1869 + "{}",
1870 + entry.command
1871 + );
1872 + assert!(entry.command.contains("No such file or directory"));
1873 + assert_eq!(entry.outcome, Severity::Warn);
1874 + }
1875 +
1876 + /// And a read that ran and listed nothing is an answer, not a fault, so it
1877 + /// stays out of a pane the user is reading about Bluetooth.
1878 + #[test]
1879 + fn a_radio_read_that_found_no_bluetooth_line_stays_quiet() {
1880 + let mut log = CommandLog::new();
1881 + assert_eq!(
1882 + radio_from(Ok("wlan unblocked unblocked\n".to_string()), &mut log),
1883 + Radio::Unknown
1884 + );
1885 + assert!(log.entries().is_empty(), "nothing to report");
1886 + }
1887 +
1809 1888 #[test]
1810 1889 fn rfkill_reports_hard_ahead_of_soft() {
1811 1890 assert_eq!(
@@ -2135,7 +2214,7 @@
2135 2214 fn reads_this_machines_real_devices() {
2136 2215 let mut log = CommandLog::new();
2137 2216 let backend = BluetoothCtl { pactl: false };
2138 - println!("radio: {:?}", backend.radio());
2217 + println!("radio: {:?}", backend.radio(&mut log));
2139 2218 match backend.adapter(&mut log) {
2140 2219 Ok(adapter) => println!("{adapter:?}"),
2141 2220 Err(err) => println!("no adapter: {err}"),
M crates/alloy/src/cli.rs +262 -4
@@ -15,9 +15,17 @@
15 15 //! log by not being a command, [`Effect`] widens what the log can describe.
16 16 //! The promise is that the console shows what it did, and a write is something
17 17 //! it did.
18 + //!
19 + //! The same shape carries a second job. [`child_command`] is the crate's only
20 + //! `Command::new`, so the environment every child is given is decided in one
21 + //! place, and what it decides today is that the sbin directories are on every
22 + //! child's `PATH`. Nothing was failing without that; [`SBIN_DIRS`] holds the
23 + //! measurement, and `tests/sbin_path.rs` holds the rule.
18 24
19 25 use std::collections::VecDeque;
26 + use std::ffi::{OsStr, OsString};
20 27 use std::io::Write;
28 + use std::os::unix::ffi::OsStrExt;
21 29 use std::path::{Path, PathBuf};
22 30 use std::process::{Command, Stdio};
23 31
@@ -147,6 +155,153 @@
147 155 }
148 156 }
149 157
158 + // ---- the child environment ----
159 +
160 + /// Directories appended to the `PATH` of every child the console spawns.
161 + ///
162 + /// **Nothing was failing when this was written.** All eight of the sbin tools
163 + /// the console runs resolve by bare name on the shipped image, and what follows
164 + /// is the measurement rather than a guess about it.
165 + ///
166 + /// What was measured, and what was not. A booted Alloy session was measured on
167 + /// fw12 with a `PATH` of exactly
168 + /// `/var/home/max/.local/bin:/var/home/max/.cargo/bin:/usr/local/bin:/usr/bin:/usr/local/sbin`,
169 + /// carrying neither `/sbin` nor `/usr/sbin` (GoingsOn problem `d0dc9dad`). That
170 + /// much is real. The problem then inferred that the console's sbin tools fail to
171 + /// start from such a session, and that inference was never run. It is false on
172 + /// the image Alloy builds. Fedora 42 unified `/usr/sbin` into `/usr/bin`, and
173 + /// `fedora-bootc:43` inherits it: in the image `/usr/sbin` is a symlink to
174 + /// `bin` and `/sbin` a symlink to `usr/sbin`, so every sbin tool has a real name
175 + /// under `/usr/bin`, which that session `PATH` does carry. Re-measured
176 + /// 2026-08-19 by resolving each of `cryptsetup`, `wipefs`, `useradd`,
177 + /// `chpasswd`, `chroot`, `setfiles`, `udevadm` and `rfkill` inside
178 + /// `localhost/alloy:clip-client` under the system half of the fw12 `PATH`. All
179 + /// eight resolved, each to `/usr/bin`.
180 + ///
181 + /// So this list is insurance and a chokepoint, not a repair. It is kept for
182 + /// three cases the merge does not cover: a host that never merged, which is
183 + /// where the console runs when Alloy is built or debugged from another distro;
184 + /// a future base image, since the merge is Fedora policy and not a law; and a
185 + /// tool that moves out of `/usr/bin` later, which the eight names above cannot
186 + /// be checked against at compile time. The second half of its value is
187 + /// structural and independent of any of that, and it is the half that pays
188 + /// today: [`child_command`] being the only `Command::new` means every child's
189 + /// environment is decided in one readable place, which is the shape a future
190 + /// change to that environment needs.
191 + ///
192 + /// Both directories are named even though one is a symlink to the other here,
193 + /// because which of the two a host merged is exactly what this cannot assume.
194 + ///
195 + /// **Appended rather than prepended, and the order is a real decision.** These
196 + /// are privileged tools: `cryptsetup` writes headers, `useradd` and `chpasswd`
197 + /// edit the target root's accounts, and the console runs them under polkit. A
198 + /// user-writable entry earlier on the `PATH`, `~/.local/bin` in the measured
199 + /// session, therefore resolves them ahead of `/usr/sbin`, and appending leaves
200 + /// that so. Prepending was weighed and rejected on two grounds. It buys almost
201 + /// nothing: the hazard is already there without this code, since `~/.local/bin`
202 + /// precedes `/usr/bin` in the session `PATH` and always did, and prepending
203 + /// `/usr/sbin` would shadow it only for the tools that live there while leaving
204 + /// every `/usr/bin` tool exposed. And on the merged image it costs a great deal
205 + /// more than it looks like: `/usr/sbin` is `/usr/bin`, so prepending it would
206 + /// put the whole of `/usr/bin` ahead of the user's own entries for every child
207 + /// the console spawns, quietly overriding a `PATH` the user wrote on purpose.
208 + /// The honest statement of the limit is that `PATH` order is not the defense
209 + /// against a hostile entry and this mechanism is not trying to be one. What
210 + /// would be is an absolute path, or a `PATH` built from nothing, on the
211 + /// privileged spawns specifically. That is a separate change with its own cost
212 + /// (see [`child_command`]'s note on `chroot <root> useradd`), and it is not
213 + /// this one.
214 + const SBIN_DIRS: [&str; 2] = ["/usr/sbin", "/sbin"];
215 +
216 + /// What a child gets when the console inherited no `PATH` at all.
217 + ///
218 + /// Reached only by a console started from an environment that carries none, a
219 + /// systemd unit with no `PATH=` being the realistic case. Without this,
220 + /// [`child_path`] would hand a child `/usr/sbin:/sbin` and nothing else, which
221 + /// is worse than the compiled-in default std would otherwise have searched: the
222 + /// act of setting `PATH` for the sbin dirs is what takes that default away, so
223 + /// the system half of the measured session `PATH` is written out to replace it.
224 + const FALLBACK_PATH: &str = "/usr/local/bin:/usr/bin";
225 +
226 + /// The `PATH` a child of the console is given, built from the one it inherited.
227 + ///
228 + /// Takes the inherited value as an argument rather than reading the environment
229 + /// itself, so a test can drive it with a session `PATH` that is not this
230 + /// machine's. `set_var` is unsafe in a threaded test binary for the reason
231 + /// [`pkg`](crate::pkg) and [`settings`](crate::settings) already state, and a
232 + /// pure function needs neither the mutation nor the caveat.
233 + ///
234 + /// Byte-wise rather than through `to_string_lossy`: a `PATH` is not required to
235 + /// be UTF-8, and replacing an undecodable directory with U+FFFD would hand the
236 + /// child a `PATH` naming a directory that does not exist.
237 + fn child_path(inherited: Option<&OsStr>) -> OsString {
238 + let base = match inherited {
239 + Some(path) if !path.is_empty() => path.to_os_string(),
240 + _ => OsString::from(FALLBACK_PATH),
241 + };
242 +
243 + let mut out = base.clone();
244 + for dir in SBIN_DIRS {
245 + // Skipping what is already there keeps the installer's own `PATH` —
246 + // root on a live ISO, which does carry both — coming back unchanged,
247 + // so the value a child sees is the value it would have seen before
248 + // this existed on every machine where nothing was wrong.
249 + if base
250 + .as_bytes()
251 + .split(|byte| *byte == b':')
252 + .any(|entry| entry == dir.as_bytes())
253 + {
254 + continue;
255 + }
256 + out.push(":");
257 + out.push(dir);
258 + }
259 + out
260 + }
261 +
262 + /// A [`Command`] for `program`, with the console's child `PATH` already set.
263 + ///
264 + /// **This is the crate's only `Command::new`**, and `tests/sbin_path.rs` is what
265 + /// keeps that true. A bare program name is resolved against the `PATH` of the
266 + /// child rather than of the parent: std does the lookup itself rather than
267 + /// handing it to `posix_spawnp` once the environment has been touched, so a
268 + /// command built here finds an sbin tool that a command built anywhere else
269 + /// would not. See [`SBIN_DIRS`] for what that is worth on the shipped image,
270 + /// which is less than the problem that prompted it claimed.
271 + ///
272 + /// The alternative was an absolute path per tool, `/usr/sbin/cryptsetup` in
273 + /// place of `cryptsetup` at each call site, which is how `rfkill` was handled
274 + /// when the problem was first hit. It was not chosen for three reasons:
275 + ///
276 + /// - **The log pane shows a name, and a name is what the docs teach.**
277 + /// [`Invocation::display`] renders the line a user can paste into a shell, and
278 + /// the docs tell them to run `cryptsetup`, not `/usr/sbin/cryptsetup`. Note
279 + /// what this bullet cannot claim: on a host where the sbin directories really
280 + /// were missing from the user's `PATH`, the pasted bare name is the one that
281 + /// fails and the absolute form is the one that works, so the copy-paste
282 + /// argument runs the other way there. It holds on the merged image, where the
283 + /// bare name resolves for the user exactly as it does for the console, and it
284 + /// is a readability argument rather than a correctness one. The reasons below
285 + /// are the load-bearing ones.
286 + /// - **It cannot reach the tools that matter most.** The installer runs
287 + /// `chroot <root> useradd`, and `useradd` there is resolved by `chroot` inside
288 + /// the target root. No path this process could write is right for both the
289 + /// live ISO and the deployment; a `PATH` entry is interpreted on both sides.
290 + /// - **It is a per-tool decision, taken again on every tool added.** A path
291 + /// table is a second thing to keep correct next to the call sites, and the
292 + /// failure mode of forgetting an entry is the silent one this exists to stop.
293 + ///
294 + /// The cost of the mechanism chosen is that every child sees a `PATH` two
295 + /// entries longer than the console's own, including an interactive shell a
296 + /// [`Flow::Suspend`](crate::shell::Flow::Suspend) hands the terminal to. That is
297 + /// visible in `distrobox enter`, and it is the same `PATH` a root shell on the
298 + /// machine would have.
299 + pub(crate) fn child_command(program: impl AsRef<OsStr>) -> Command {
300 + let mut command = Command::new(program);
301 + command.env("PATH", child_path(std::env::var_os("PATH").as_deref()));
302 + command
303 + }
304 +
150 305 /// A command line, held as argv rather than a string so it is executed exactly
151 306 /// as displayed — no shell, no quoting round-trip, no injection surface.
152 307 #[derive(Debug)]
@@ -192,8 +347,21 @@
192 347 /// Not shown by [`display`](Self::display), for the same reason
193 348 /// [`stdin`](Self::stdin) is not. See the field's own note for why this
194 349 /// carrier exists at all when stdin is the better one.
350 + ///
351 + /// `PATH` is refused. [`apply_env`](Self::apply_env) runs after
352 + /// [`child_command`] has set the child's `PATH`, so an entry named `PATH`
353 + /// would replace it and take the sbin directories away from that one
354 + /// command, with nothing on screen to say so. A full `assert` rather than a
355 + /// `debug_assert`: what the check is for is a mistake that produces no
356 + /// symptom in release, which is where a `debug_assert` is worth least.
195 357 pub(crate) fn env_secret(mut self, name: impl Into<String>, value: Secret) -> Self {
196 - self.env.push((name.into(), value));
358 + let name = name.into();
359 + assert_ne!(
360 + name, "PATH",
361 + "a command cannot carry its own PATH; `child_command` sets it and \
362 + `apply_env` runs after"
363 + );
364 + self.env.push((name, value));
197 365 self
198 366 }
199 367
@@ -310,7 +478,7 @@
310 478 self.env.is_empty(),
311 479 "a suspended command drops its secret environment"
312 480 );
313 - let mut command = Command::new(&self.program);
481 + let mut command = child_command(&self.program);
314 482 command.args(&self.args);
315 483 command
316 484 }
@@ -349,7 +517,7 @@
349 517 /// and it does so through the run screen rather than by amending a log line
350 518 /// that has already scrolled.
351 519 pub(crate) fn spawn_streaming(&self, log: &mut CommandLog) -> Result<std::process::Child> {
352 - let mut command = Command::new(&self.program);
520 + let mut command = child_command(&self.program);
353 521 command
354 522 .args(&self.args)
355 523 .stdout(Stdio::piped())
@@ -412,7 +580,7 @@
412 580 /// does) waits for input that never ends while we wait for it to exit.
413 581 /// Dropping the handle is what closes it, hence the inner scope.
414 582 fn spawn(&self) -> Result<std::process::Output> {
415 - let mut command = Command::new(&self.program);
583 + let mut command = child_command(&self.program);
416 584 command
417 585 .args(&self.args)
418 586 .stdout(Stdio::piped())
@@ -442,6 +610,12 @@
442 610
443 611 /// Put [`env`](Self::env) onto a command about to be spawned.
444 612 ///
613 + /// Runs after [`child_command`], so an entry here wins over anything that
614 + /// set the same name. `PATH` is the one that would matter and
615 + /// [`env_secret`](Self::env_secret) refuses it at construction, which is
616 + /// where a rejected name can still be attributed to the call site that
617 + /// wrote it.
618 + ///
445 619 /// Non-UTF-8 is not a concern: every value here is a passphrase this
446 620 /// process generated or read from a [`TextField`](alloy_tui::TextField),
447 621 /// both of which are `String`s already.
@@ -935,4 +1109,88 @@
935 1109 assert_eq!(entry.command, format!("cmd {}", LOG_CAPACITY + offset));
936 1110 }
937 1111 }
1112 +
1113 + /// The measured fw12 session `PATH`, verbatim from problem `d0dc9dad`.
1114 + const SESSION_PATH: &str = "/var/home/max/.local/bin:/var/home/max/.cargo/bin:\
1115 + /usr/local/bin:/usr/bin:/usr/local/sbin";
1116 +
1117 + // The session the problem was measured in carries neither directory. It does
1118 + // not follow that a bare `cryptsetup` from it fails, and on the image Alloy
1119 + // ships it does not: see [`SBIN_DIRS`]. What this pins is the shape of the
1120 + // built PATH, appended, so the copies the user's own PATH already found keep
1121 + // winning.
1122 + #[test]
1123 + fn a_session_path_gains_the_sbin_directories() {
1124 + let path = child_path(Some(OsStr::new(SESSION_PATH)));
1125 + assert_eq!(
1126 + path.to_str().expect("built from UTF-8 input"),
1127 + format!("{SESSION_PATH}:/usr/sbin:/sbin")
1128 + );
1129 + }
1130 +
1131 + // The installer runs as root on a live ISO, whose PATH has both already.
1132 + // Appending them again would work and would also mean the console changes
1133 + // the environment of every child on a machine where nothing was wrong.
1134 + #[test]
1135 + fn a_path_that_already_has_them_comes_back_unchanged() {
1136 + let full = "/usr/local/bin:/usr/bin:/usr/local/sbin:/usr/sbin:/sbin";
1137 + assert_eq!(child_path(Some(OsStr::new(full))), OsString::from(full));
1138 +
1139 + // One of the two present is not both: the missing one is still added,
1140 + // and a substring match would have called this done.
1141 + assert_eq!(
1142 + child_path(Some(OsStr::new("/usr/bin:/sbin"))),
1143 + OsString::from("/usr/bin:/sbin:/usr/sbin")
1144 + );
1145 + assert_eq!(
1146 + child_path(Some(OsStr::new("/usr/sbindir:/usr/bin"))),
1147 + OsString::from("/usr/sbindir:/usr/bin:/usr/sbin:/sbin")
1148 + );
1149 + }
1150 +
1151 + // Setting PATH at all is what takes std's compiled-in default away, so the
1152 + // no-PATH case has to put a bin directory back or every bare name breaks.
1153 + #[test]
1154 + fn an_absent_path_still_names_somewhere_to_find_bin_tools() {
1155 + for inherited in [None, Some(OsStr::new(""))] {
1156 + let path = child_path(inherited);
1157 + assert_eq!(
1158 + path,
1159 + OsString::from(format!("{FALLBACK_PATH}:/usr/sbin:/sbin")),
1160 + "{inherited:?}"
1161 + );
1162 + }
1163 + }
1164 +
1165 + // The environment the console was started in is not this machine's, so this
1166 + // asserts the wiring rather than the value: whatever `child_path` computes
1167 + // is what the command carries.
1168 + #[test]
1169 + fn every_command_carries_the_computed_path() {
1170 + let command = child_command("cryptsetup");
1171 + let expected = child_path(std::env::var_os("PATH").as_deref());
1172 + let path = command
1173 + .get_envs()
1174 + .find_map(|(name, value)| (name == "PATH").then_some(value));
1175 + assert_eq!(path, Some(Some(expected.as_os_str())));
1176 + }
1177 +
1178 + // `apply_env` runs after `child_command`, so an entry named PATH would take
1179 + // the sbin directories away from that one command and nothing would say so.
1180 + // The refusal is at construction; this is what proves it is there.
1181 + #[test]
1182 + #[should_panic(expected = "a command cannot carry its own PATH")]
1183 + fn a_command_cannot_carry_its_own_path() {
1184 + let _ = Invocation::new("cryptsetup").env_secret("PATH", Secret::new("/tmp"));
1185 + }
1186 +
1187 + // Every other name still goes through, so the refusal above is about PATH
1188 + // and not about `env_secret` having stopped working.
1189 + #[test]
1190 + fn another_name_is_still_carried() {
1191 + let invocation =
1192 + Invocation::new("cryptsetup").env_secret("PASSWORD", Secret::new("hunter2"));
1193 + assert_eq!(invocation.env.len(), 1);
1194 + assert_eq!(invocation.env[0].0, "PASSWORD");
1195 + }
938 1196 }
M crates/alloy/src/status.rs +3 -3
@@ -24,7 +24,7 @@
24 24
25 25 use std::fmt::Write as _;
26 26 use std::io::{BufRead, BufReader, Write};
27 - use std::process::{Command, Stdio};
27 + use std::process::Stdio;
28 28 use std::sync::Arc;
29 29 use std::sync::atomic::{AtomicBool, Ordering};
30 30 use std::time::Duration;
@@ -33,7 +33,7 @@
33 33 use makeover::SemanticTokens;
34 34
35 35 use crate::audio::{self, Device};
36 - use crate::cli::CommandLog;
36 + use crate::cli::{CommandLog, child_command};
37 37 use crate::net::{self, Interface, Kind, State};
38 38 use crate::theme;
39 39
@@ -155,7 +155,7 @@
155 155 /// ends when the child's stdout closes, which is when the process this belongs
156 156 /// to is ending anyway.
157 157 fn subscribe_audio() -> Option<Arc<AtomicBool>> {
158 - let mut child = Command::new("pactl")
158 + let mut child = child_command("pactl")
159 159 .arg("subscribe")
160 160 .stdin(Stdio::null())
161 161 .stdout(Stdio::piped())