Skip to main content

max / alloy

cli: teach Invocation what a secret is The installer's user step will run `chpasswd`, and the command log pane shows every command the console runs. That coverage is structural — the pane cannot miss an invocation, which is the whole point of routing everything through this module — and it means the default behavior of every value here is to end up on screen. For a password that default is wrong, and "remember not to log this one" is exactly the kind of discipline the pane was designed not to depend on. So a secret is a type. Secret has no Display, its Debug redacts, and getting the bytes back takes a call to `expose`, which is named to be conspicuous in review: it is the only place a secret can leave the type, so it is the only place worth checking. Nothing prevents a determined caller from printing it; nothing does it by accident. Held as bytes rather than a String for two reasons. A pipe takes bytes, so there is no conversion at the moment of use, and a Vec<u8> can be zeroed on drop without unsafe, which a String cannot. The scrub is documented as best-effort rather than claimed as more: the password was a String in the text field before it got here, and reallocation along the way left copies this cannot reach. What it closes is a long-lived Invocation sitting in memory holding a readable password, which is the case worth closing in a process about to reboot the machine. stdin rather than an argument because argv is world-readable — anything passed as a flag is visible in `ps` to every user on the box for as long as the command runs. That is why chpasswd reads stdin in the first place, and why a secret cannot be modelled as just another arg. capture() grows a spawn path, since Command::output offers no way to write to the child first. The write handle is dropped before waiting: a child reading to EOF hangs forever otherwise, and chpasswd is such a child. `the_pipe_is_closed_so_a_reader_can_finish` would hang rather than fail if that regressed, which is a worse failure mode than a red test but a much louder one than silence. The displayed line says `# input withheld` rather than showing nothing. A chpasswd with no visible input reads as a command that did nothing, and the pane's honesty is about the shape of what happened, the same reasoning Effect::Write already uses for a file it cannot round-trip. Effect loses its Clone derive. Nothing cloned it — only a String field inside one — and duplicating something that may hold a Secret means another buffer to scrub. command() gains a debug_assert: suspending inherits stdio, so there is no pipe, and silently dropping the input would hand the child a command missing the half that mattered. Two tests run against a real `cat` rather than asserting on argv, so the bytes are proven to arrive and the pipe is proven to close. Secret::new and Invocation::stdin carry allow(dead_code) until the user step calls them. 192 tests pass, 7 ignored, clippy clean. The three rustfmt diffs in this file pre-date the change; verified none were added.
Co-Authored-By
Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-20 00:45 UTC
Signed with PGP, not checked
Commit: 0476d9f0348aeeede69231cfe889e9c34edbba6a
Parent: 28116b6
1 file changed, +244 insertions, -6 deletions
M crates/alloy/src/cli.rs +244 -6
@@ -16,8 +16,9 @@
16 16 //! it did.
17 17
18 18 use std::collections::VecDeque;
19 + use std::io::Write;
19 20 use std::path::{Path, PathBuf};
20 - use std::process::Command;
21 + use std::process::{Command, Stdio};
21 22
22 23 use alloy_tui::{LogEntry, Severity};
23 24 use anyhow::{Context, Result, bail};
@@ -88,12 +89,76 @@
88 89 }
89 90 }
90 91
92 + /// Input piped to a command that must never be displayed.
93 + ///
94 + /// The installer's user step is the first thing Alloy runs that has a value the
95 + /// log pane must not see. That pane's promise — every command the console runs
96 + /// appears in it — is structural rather than remembered, which is exactly what
97 + /// makes a password dangerous here: the default behavior of every other value
98 + /// in this module is to be shown.
99 + ///
100 + /// So the secret is a type rather than a convention. It has no [`Display`], its
101 + /// [`Debug`] redacts, and reading it back takes a method named to be conspicuous
102 + /// at a call site. Nothing stops a determined caller from printing
103 + /// [`expose`](Self::expose), but nothing does it by accident.
104 + ///
105 + /// Held as bytes, not a `String`, for two reasons. A pipe takes bytes anyway, so
106 + /// no conversion happens at the moment of use, and a `Vec<u8>` can be zeroed on
107 + /// drop without `unsafe`, which a `String` cannot.
108 + pub struct Secret(Vec<u8>);
109 +
110 + impl Secret {
111 + // Constructed only by the tests below until the installer's user step lands,
112 + // which is the one place in the console that has a password to carry.
113 + #[allow(dead_code)]
114 + pub fn new(value: impl Into<Vec<u8>>) -> Self {
115 + Self(value.into())
116 + }
117 +
118 + /// The bytes, for writing to a child's stdin.
119 + ///
120 + /// Named to stand out in review: a call to this is the only place a secret
121 + /// can leave the type, so it is the only place worth checking.
122 + pub fn expose(&self) -> &[u8] {
123 + &self.0
124 + }
125 + }
126 +
127 + /// Redacted, so a `{:?}` of anything holding one cannot leak it. [`Invocation`]
128 + /// derives `Debug` and is printed in test failures.
129 + impl std::fmt::Debug for Secret {
130 + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
131 + f.write_str("Secret(<redacted>)")
132 + }
133 + }
134 +
135 + /// Best-effort scrub.
136 + ///
137 + /// Overwrites the buffer this value owns. Deliberately not claimed as more than
138 + /// that: the password was a `String` in the text field before it arrived here,
139 + /// and any reallocation along the way left copies this cannot reach. It closes
140 + /// the window where a long-lived `Invocation` sits in memory holding a readable
141 + /// password, which is the case actually worth closing in a process that is about
142 + /// to reboot the machine.
143 + impl Drop for Secret {
144 + fn drop(&mut self) {
145 + self.0.fill(0);
146 + }
147 + }
148 +
91 149 /// A command line, held as argv rather than a string so it is executed exactly
92 150 /// as displayed — no shell, no quoting round-trip, no injection surface.
93 - #[derive(Debug, Clone)]
151 + #[derive(Debug)]
94 152 pub struct Invocation {
95 153 program: String,
96 154 args: Vec<String>,
155 + /// Piped to the child on stdin. Never displayed, never logged.
156 + ///
157 + /// stdin rather than an argument because argv is world-readable: anything
158 + /// passed as a flag is visible in `ps` to every user on the machine for as
159 + /// long as the command runs. `chpasswd` reads from stdin for this reason,
160 + /// and it is why a secret cannot be modelled as just another arg.
161 + stdin: Option<Secret>,
97 162 }
98 163
99 164 impl Invocation {
@@ -101,9 +166,25 @@
101 166 Self {
102 167 program: program.into(),
103 168 args: Vec::new(),
169 + stdin: None,
104 170 }
105 171 }
106 172
173 + /// Pipe `secret` to the command on stdin.
174 + ///
175 + /// The value is not shown by [`display`](Self::display) and therefore never
176 + /// reaches the log pane. What the pane shows instead is the argv plus a note
177 + /// that input was withheld, so the line stays honest about the fact that
178 + /// something was piped in without being honest about what.
179 + ///
180 + /// Called only by the tests below until the user step lands; see
181 + /// [`Secret::new`].
182 + #[allow(dead_code)]
183 + pub fn stdin(mut self, secret: Secret) -> Self {
184 + self.stdin = Some(secret);
185 + self
186 + }
187 +
107 188 pub fn arg(mut self, arg: impl Into<String>) -> Self {
108 189 self.args.push(arg.into());
109 190 self
@@ -121,6 +202,12 @@
121 202 /// The command as a user would type it. Arguments containing whitespace are
122 203 /// quoted so the displayed line is copy-pasteable into a shell and means
123 204 /// the same thing there as it did here.
205 + ///
206 + /// A command with [`stdin`](Self::stdin) cannot round-trip that way, so it
207 + /// says so rather than rendering a line that would do nothing if pasted.
208 + /// The `#` marks commentary the way the mock backends already do, and the
209 + /// same reasoning applies as for [`Effect::Write`]: being honest about the
210 + /// shape beats contorting the pane into showing something it must not.
124 211 pub fn display(&self) -> String {
125 212 let mut out = String::from(&self.program);
126 213 for arg in &self.args {
@@ -133,6 +220,9 @@
133 220 out.push_str(arg);
134 221 }
135 222 }
223 + if self.stdin.is_some() {
224 + out.push_str(" # input withheld");
225 + }
136 226 out
137 227 }
138 228
@@ -173,16 +263,23 @@
173 263 /// rather than collecting stdout. Callers log the invocation themselves
174 264 /// before handing it over, since the command outlives this call and its
175 265 /// outcome is not a captured result.
266 + /// A [`Secret`] cannot travel this way. Suspending inherits the terminal's
267 + /// stdio, so there is no pipe to write into, and silently dropping the
268 + /// input would hand the child a command missing the half that mattered.
269 + /// Nothing does this today; the assertion is here so nothing starts to.
176 270 pub fn command(&self) -> Command {
271 + debug_assert!(
272 + self.stdin.is_none(),
273 + "a suspended command inherits stdio and cannot carry a secret"
274 + );
177 275 let mut command = Command::new(&self.program);
178 276 command.args(&self.args);
179 277 command
180 278 }
181 279
182 280 fn capture(&self) -> Result<String> {
183 - let output = Command::new(&self.program)
184 - .args(&self.args)
185 - .output()
281 + let output = self
282 + .spawn()
186 283 .with_context(|| format!("failed to invoke `{}`", self.display()))?;
187 284
188 285 if !output.status.success() {
@@ -200,6 +297,41 @@
200 297 String::from_utf8(output.stdout)
201 298 .with_context(|| format!("`{}` emitted non-UTF-8 output", self.display()))
202 299 }
300 +
301 + /// Run to completion, writing [`stdin`](Self::stdin) if there is any.
302 + ///
303 + /// `Command::output` would be enough without a secret — it is what this was
304 + /// before — but it gives no way to write to the child first. The pipe has to
305 + /// be closed before waiting, or a child that reads until EOF (`chpasswd`
306 + /// does) waits for input that never ends while we wait for it to exit.
307 + /// Dropping the handle is what closes it, hence the inner scope.
308 + fn spawn(&self) -> Result<std::process::Output> {
309 + let mut command = Command::new(&self.program);
310 + command
311 + .args(&self.args)
312 + .stdout(Stdio::piped())
313 + .stderr(Stdio::piped())
314 + .stdin(if self.stdin.is_some() {
315 + Stdio::piped()
316 + } else {
317 + // What `Command::output` does, kept so a command with no input
318 + // behaves exactly as it did before this existed: a child reading
319 + // stdin sees EOF rather than the console's own terminal.
320 + Stdio::null()
321 + });
322 +
323 + let mut child = command.spawn()?;
324 +
325 + if let Some(secret) = &self.stdin {
326 + let mut pipe = child
327 + .stdin
328 + .take()
329 + .context("stdin was piped but no handle came back")?;
330 + pipe.write_all(secret.expose())?;
331 + }
332 +
333 + Ok(child.wait_with_output()?)
334 + }
203 335 }
204 336
205 337 /// Something the console does, which is usually but not always a command.
@@ -208,7 +340,10 @@
208 340 /// [`Invocation`] is returned rather than run: it keeps the log's coverage
209 341 /// structural, and it keeps the backends testable on a machine with none of
210 342 /// the tools installed. See the module docs for why the enum exists at all.
211 - #[derive(Debug, Clone)]
343 + ///
344 + /// Not `Clone`, since [`Invocation`] is not: duplicating something that may
345 + /// hold a [`Secret`] would mean another buffer to scrub, and nothing needs it.
346 + #[derive(Debug)]
212 347 pub enum Effect {
213 348 /// Run a command.
214 349 Run(Invocation),
@@ -392,6 +527,109 @@
392 527 assert_eq!(commands, ["after"], "both nested levels stayed muted");
393 528 }
394 529
530 + // ---- secrets ----
531 +
532 + /// A password-shaped value distinctive enough that a leak into any string
533 + /// is unambiguous rather than a coincidence.
534 + const PASSWORD: &str = "hunter2-correct-horse-battery";
535 +
536 + // The guarantee the type exists for. `display()` feeds the log pane
537 + // directly, so anything it returns is on screen.
538 + #[test]
539 + fn the_displayed_line_never_carries_the_secret() {
540 + let invocation = Invocation::new("chpasswd").stdin(Secret::new(PASSWORD));
541 + let shown = invocation.display();
542 +
543 + assert!(!shown.contains(PASSWORD), "the pane would show it: {shown}");
544 + assert!(shown.starts_with("chpasswd"), "{shown}");
545 + }
546 +
547 + // The line still has to say that something was piped in. A `chpasswd` with
548 + // no visible input reads as a command that did nothing.
549 + #[test]
550 + fn the_displayed_line_admits_that_input_was_withheld() {
551 + let invocation = Invocation::new("chpasswd").stdin(Secret::new(PASSWORD));
552 + assert!(invocation.display().contains("withheld"));
553 + }
554 +
555 + // A command with no secret must display exactly as it always did, or every
556 + // other view's log lines change underneath them.
557 + #[test]
558 + fn a_command_without_a_secret_is_unchanged() {
559 + let invocation = Invocation::new("lsblk").args(["-J", "-b"]);
560 + assert_eq!(invocation.display(), "lsblk -J -b");
561 + }
562 +
563 + // `Invocation` derives Debug and is printed by assertion failures, so a
564 + // plain derived Debug on the secret would leak it into CI output.
565 + #[test]
566 + fn debug_output_redacts_the_secret() {
567 + let invocation = Invocation::new("chpasswd").stdin(Secret::new(PASSWORD));
568 + let debug = format!("{invocation:?}");
569 +
570 + assert!(!debug.contains(PASSWORD), "leaked through Debug: {debug}");
571 + assert!(debug.contains("redacted"), "{debug}");
572 + }
573 +
574 + // What the log actually ends up holding, which is the thing that matters.
575 + // Asserted through `run` rather than `display` so the whole path is covered:
576 + // a future change that logs something else about the command is caught here.
577 + #[test]
578 + fn running_with_a_secret_logs_no_trace_of_it() {
579 + let mut log = CommandLog::new();
580 + let secret = Secret::new(PASSWORD);
581 + let _ = Invocation::new("cat").stdin(secret).run(&mut log);
582 +
583 + let logged: Vec<&str> = log.entries().iter().map(|e| e.command.as_str()).collect();
584 + assert_eq!(logged.len(), 1, "the command was recorded");
585 + assert!(
586 + !logged[0].contains(PASSWORD),
587 + "the log holds the password: {}",
588 + logged[0]
589 + );
590 + }
591 +
592 + // The point of piping at all: the child has to actually receive it. `cat`
593 + // echoes stdin to stdout, so its output is proof the bytes arrived.
594 + #[test]
595 + fn the_child_receives_the_secret_on_stdin() {
596 + let mut log = CommandLog::new();
597 + let out = Invocation::new("cat")
598 + .stdin(Secret::new(PASSWORD))
599 + .run(&mut log)
600 + .expect("cat ran");
601 +
602 + assert_eq!(out, PASSWORD);
603 + }
604 +
605 + // A child that reads to EOF hangs forever if the pipe is never closed.
606 + // `cat` is exactly such a child, so this test would time out rather than
607 + // fail if the write handle were held open.
608 + #[test]
609 + fn the_pipe_is_closed_so_a_reader_can_finish() {
610 + let mut log = CommandLog::new();
611 + let out = Invocation::new("cat")
612 + .stdin(Secret::new("line one\nline two\n"))
613 + .run(&mut log)
614 + .expect("cat saw EOF and exited");
615 +
616 + assert_eq!(out, "line one\nline two\n");
617 + }
618 +
619 + // Scrubbed in place, so the buffer is not left readable in the process's
620 + // memory after the value goes away. Best-effort by construction — earlier
621 + // copies are unreachable — and this pins the part that is not.
622 + #[test]
623 + fn dropping_a_secret_scrubs_its_buffer() {
624 + let mut secret = Secret::new(PASSWORD);
625 + assert_eq!(secret.expose(), PASSWORD.as_bytes());
626 +
627 + // What `Drop` does, on the same buffer, since observing after a real
628 + // drop would be a use-after-free.
629 + secret.0.fill(0);
630 + assert!(secret.expose().iter().all(|byte| *byte == 0));
631 + }
632 +
395 633 // `entries()` straightens the deque; a wrapped ring must still read back in
396 634 // order, or the pane shows the transcript spliced at the wrap point.
397 635 #[test]