Skip to main content

max / alloy

install: keep the password off the summary's render path render_summary called plan(), which handed the password to install_plan, which copied it onto the heap and moved the copy into a boxed closure. The summary redraws on the one-second tick and on every keypress, so each frame made a fresh unscrubbed copy of the password and dropped it. That is the case Secret exists to prevent, reintroduced by the one caller that had no use for the value. The summary needs display strings only, and no displayed stage carries a password: every stage that does is built by a resolver, after the deploy, from arguments that do not exist yet. So build the summary's plan without one. plan() now takes the password as a parameter, which leaves confirmed() as the only caller that names it. the_summary_shows_the_same_commands_the_install_runs pins the displayed lines to the real plan's, so a stage moving out of a resolver cannot quietly make the summary wrong.
Author: Max Johnson <me@maxj.phd> · 2026-07-22 21:51 UTC
Signed with PGP, not checked
Commit: e289049827557e7067b82980e872675084aba2b6
Parent: 037decb
1 file changed, +47 insertions, -11 deletions
@@ -1454,9 +1454,8 @@
1454 1454 Line::from(text::muted(theme, "This runs:")),
1455 1455 ];
1456 1456
1457 - for invocation in self.plan() {
1458 - let shown = format!(" {}", invocation.display());
1459 - lines.push(Line::from(text::secondary(theme, shown)));
1457 + for shown in self.plan_display() {
1458 + lines.push(Line::from(text::secondary(theme, format!(" {shown}"))));
1460 1459 }
1461 1460
1462 1461 frame.render_widget(Paragraph::new(lines), area);
@@ -1529,10 +1528,14 @@
1529 1528
1530 1529 /// The commands this install will run, from the answers collected.
1531 1530 ///
1531 + /// The password is a parameter rather than read from `self` so that the one
1532 + /// caller who needs it has to say so. Everything else asks for
1533 + /// [`plan_display`](Self::plan_display) instead.
1534 + ///
1532 1535 /// Empty if an answer is missing, which cannot happen from the summary step
1533 - /// — every earlier step gates on its own validation — but returning nothing
1536 + /// (every earlier step gates on its own validation) but returning nothing
1534 1537 /// beats rendering a command line with a hole in it.
1535 - fn plan(&self) -> Vec<Stage> {
1538 + fn plan(&self, password: &str) -> Vec<Stage> {
1536 1539 let (Some(disk), Some(hostname), Some(username)) = (
1537 1540 self.answers.disk.as_deref(),
1538 1541 self.answers.hostname.as_deref(),
@@ -1541,7 +1544,25 @@
1541 1544 return Vec::new();
1542 1545 };
1543 1546
1544 - install_plan(disk, hostname, username, self.password.value())
1547 + install_plan(disk, hostname, username, password)
1548 + }
1549 +
1550 + /// The same commands, as the summary shows them.
1551 + ///
1552 + /// Built from [`plan`](Self::plan) rather than from its own list, so the
1553 + /// summary cannot drift from what runs. No password reaches it: every stage
1554 + /// that carries one is built by a resolver, after the deploy, from
1555 + /// arguments that do not exist yet, and [`Stage::display`] does not descend
1556 + /// into a resolver. So the summary renders the same lines either way, and
1557 + /// this way the render thread stops copying the password onto the heap once
1558 + /// per frame and dropping it unscrubbed, which is the case the [`Secret`]
1559 + /// type exists to prevent.
1560 + ///
1561 + /// `the_summary_shows_the_same_commands_the_install_runs` pins the two
1562 + /// together, and `no_line_of_the_plan_carries_the_password` pins the claim
1563 + /// above it.
1564 + fn plan_display(&self) -> Vec<String> {
1565 + self.plan("").iter().map(Stage::display).collect()
1545 1566 }
1546 1567
1547 1568 /// The hostname pane: a prompt, the field with its caret, and what the
@@ -1704,7 +1725,7 @@
1704 1725 /// to-disk` takes minutes and the frame has to keep drawing for all of them.
1705 1726 fn confirmed(&mut self, _log: &mut CommandLog) {
1706 1727 self.error = None;
1707 - self.running = Some(Sequence::new(self.plan()));
1728 + self.running = Some(Sequence::new(self.plan(self.password.value())));
1708 1729 }
1709 1730
1710 1731 /// Drive the running install and advance the activity light.
@@ -2345,7 +2366,7 @@
2345 2366 #[test]
2346 2367 fn the_plan_starts_with_the_deploy_then_discovers() {
2347 2368 let (view, _log) = at_summary();
2348 - let shown: Vec<String> = view.plan().iter().map(Stage::display).collect();
2369 + let shown = view.plan_display();
2349 2370
2350 2371 assert_eq!(shown.len(), 4, "{shown:#?}");
2351 2372 assert_eq!(shown[0], "bootc install to-disk --wipe /dev/sda");
@@ -2361,7 +2382,7 @@
2361 2382 #[test]
2362 2383 fn the_plan_shown_up_front_stops_at_the_first_discovery() {
2363 2384 let (view, _log) = at_summary();
2364 - let shown: Vec<String> = view.plan().iter().map(Stage::display).collect();
2385 + let shown = view.plan_display();
2365 2386
2366 2387 assert!(
2367 2388 !shown.iter().any(|line| line.contains("firstboot")),
@@ -2369,12 +2390,27 @@
2369 2390 );
2370 2391 }
2371 2392
2393 + // The summary is rendered from a plan built without the password, because
2394 + // the render thread runs on every tick and every keypress and a plan built
2395 + // there copied the password onto the heap and dropped it unscrubbed each
2396 + // time. That is only sound while the lines are the same either way, which
2397 + // holds because every stage carrying a password is built by a resolver from
2398 + // arguments discovered later. This is what would catch a stage moving out
2399 + // of a resolver and into the plan itself.
2400 + #[test]
2401 + fn the_summary_shows_the_same_commands_the_install_runs() {
2402 + let (view, _log) = at_summary();
2403 + let real: Vec<String> = view.plan("hunter2").iter().map(Stage::display).collect();
2404 +
2405 + assert_eq!(view.plan_display(), real);
2406 + }
2407 +
2372 2408 // The whole reason Secret exists: the summary renders these and the log
2373 2409 // pane records them.
2374 2410 #[test]
2375 2411 fn no_line_of_the_plan_carries_the_password() {
2376 2412 let (view, _log) = at_summary();
2377 - for stage in view.plan() {
2413 + for stage in view.plan("hunter2") {
2378 2414 assert!(!stage.display().contains("hunter2"), "{}", stage.display());
2379 2415 }
2380 2416 for shown in configured() {
@@ -2831,7 +2867,7 @@
2831 2867 fn an_incomplete_plan_is_empty_rather_than_partial() {
2832 2868 let (mut view, _log) = at_summary();
2833 2869 view.answers.hostname = None;
2834 - assert!(view.plan().is_empty());
2870 + assert!(view.plan("hunter2").is_empty());
2835 2871 }
2836 2872
2837 2873 // ---- the run screen ----