Feed Wi-Fi PSK to nmcli --ask on stdin, never argv
All checks were successful
check / check (push) Successful in 1m7s
dev release / build (push) Successful in 1m28s

First connect (and reuse-with-password) no longer puts the secret on
nmcli's command line, so it is not visible in /proc/<pid>/cmdline.
networks.toml stays 0600; the local copy is still cleared after first
success.
This commit is contained in:
Breadway 2026-08-23 14:44:23 +08:00
parent 92fb40d69b
commit 02e96126e0
8 changed files with 246 additions and 97 deletions

View file

@ -194,7 +194,9 @@ fn profile_list_marks_exactly_the_current_profile() {
let out = stdout(&o);
assert!(out.contains("* home"), "out: {out}");
assert_eq!(
out.lines().filter(|l| l.trim_start().starts_with('*')).count(),
out.lines()
.filter(|l| l.trim_start().starts_with('*'))
.count(),
1,
"expected exactly one marked profile, got: {out}"
);
@ -262,7 +264,10 @@ fn forget_removes_network_from_config() {
);
// ...and it should never have been in breadcrumbs.toml to begin with.
let text = fs::read_to_string(sb.config_file()).unwrap();
assert!(!text.contains("CafeWifi"), "network leaked into config: {text}");
assert!(
!text.contains("CafeWifi"),
"network leaked into config: {text}"
);
}
#[test]
@ -270,7 +275,11 @@ fn detect_without_wifi_adapter_errors() {
let sb = Sandbox::new();
let o = sb.cmd(&["detect"]);
assert!(!o.status.success());
assert!(stderr(&o).contains("could not detect"), "stderr: {}", stderr(&o));
assert!(
stderr(&o).contains("could not detect"),
"stderr: {}",
stderr(&o)
);
}
#[test]
@ -364,7 +373,11 @@ fn networks_are_stored_separately_from_settings_and_profiles() {
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
let mode = fs::metadata(sb.networks_file()).unwrap().permissions().mode() & 0o777;
let mode = fs::metadata(sb.networks_file())
.unwrap()
.permissions()
.mode()
& 0o777;
assert_eq!(mode, 0o600, "networks.toml should be owner-only");
}
}
@ -374,8 +387,8 @@ fn add_with_empty_password_is_stored_as_no_password() {
// An explicitly empty password (e.g. `add SSID ""`, or a blank response
// at the interactive prompt) means "this is an open network" — it must
// round-trip as an absent `password` key, the same as a cleared one,
// not as `password = ""` (which `nm::connect_verbose` would send as a
// literal empty PSK and fail against a real open SSID).
// not as `password = ""` (which `nm::connect_verbose` would treat as a
// blank secret rather than an open network).
let sb = Sandbox::new();
let o = sb.cmd(&["add", "OpenCafe", ""]);
assert!(o.status.success(), "stderr: {}", stderr(&o));
@ -446,14 +459,19 @@ fn password_is_cleared_after_first_connect_and_never_sent_again() {
let record = sb.root.join(".nmcli-calls");
// First connect: no saved NM profile yet, so breadcrumbs creates one via
// `device wifi connect ... password hunter2 ...`.
// `nmcli --ask device wifi connect ...` with the PSK on stdin, not argv.
let first = sb.cmd(&["init"]);
assert!(first.status.success(), "stderr: {}", stderr(&first));
let first_calls = fs::read_to_string(&record).unwrap_or_default();
assert!(
first_calls.contains("device wifi connect TestNet") && first_calls.contains("hunter2"),
"first connect should create a new NM profile with the password: {first_calls}"
first_calls.contains("device wifi connect TestNet") && first_calls.contains("--ask"),
"first connect should create a new NM profile via --ask: {first_calls}"
);
assert!(
!first_calls.contains("hunter2"),
"PSK must not appear on nmcli argv: {first_calls}"
);
// stdin payload is asserted in-process via FakeRunner (see flow_watch).
// The local copy is gone from disk immediately after.
let networks = fs::read_to_string(sb.networks_file()).unwrap();
@ -461,7 +479,10 @@ fn password_is_cleared_after_first_connect_and_never_sent_again() {
!networks.contains("hunter2"),
"password should have been cleared from networks.toml: {networks}"
);
assert!(networks.contains("TestNet"), "network entry itself should remain");
assert!(
networks.contains("TestNet"),
"network entry itself should remain"
);
// Reset the recording so the second run's argv can be checked in isolation.
fs::write(&record, "").unwrap();
@ -524,7 +545,10 @@ fn doctor_reports_present_when_nmcli_and_tailscale_are_on_path() {
let o = sb.cmd(&["doctor"]);
assert!(o.status.success(), "stderr: {}", stderr(&o));
let out = stdout(&o);
assert!(out.contains("nmcli") && out.contains("present"), "out: {out}");
assert!(
out.contains("nmcli") && out.contains("present"),
"out: {out}"
);
assert!(!out.contains("MISSING"), "out: {out}");
}

View file

@ -14,6 +14,7 @@ use bread_utils::bread_client::BreadEvent;
use breadcrumbs::bread_events;
use breadcrumbs::config::{Config, NetworkDef, Profile, Settings};
use breadcrumbs::flow;
use breadcrumbs::nm;
use breadcrumbs::state::{self, State};
use breadcrumbs::util::with_runner;
use breadcrumbs::watch::{classify, Health};
@ -83,10 +84,7 @@ fn flow_run_connects_to_first_visible_candidate_in_priority_order() {
let _env = EnvSandbox::new();
let mut cfg = base_config();
cfg.networks = vec![
net("First", Some("pw1")),
net("Second", Some("pw2")),
];
cfg.networks = vec![net("First", Some("pw1")), net("Second", Some("pw2"))];
cfg.profiles.insert(
"home".into(),
Profile {
@ -112,10 +110,11 @@ fn flow_run_connects_to_first_visible_candidate_in_priority_order() {
// Priority order actually mattered: "Second" was never dialed even
// though it was visible and would have succeeded too.
let dialed_second = calls
.borrow()
.iter()
.any(|c| c.prog == "nmcli" && c.args.contains(&"connect".to_string()) && c.args.iter().any(|a| a == "Second"));
let dialed_second = calls.borrow().iter().any(|c| {
c.prog == "nmcli"
&& c.args.contains(&"connect".to_string())
&& c.args.iter().any(|a| a == "Second")
});
assert!(!dialed_second, "connected to Second when First should win");
// The password used for the winning connect is now NM's problem, not
@ -573,3 +572,84 @@ fn handle_command_ignores_events_outside_its_own_command_namespace() {
)));
assert_eq!(State::load("away").profile, "away");
}
// ---------------------------------------------------------------------
// PSK never on argv (first connect feeds nmcli --ask on stdin)
// ---------------------------------------------------------------------
fn assert_psk_not_on_argv(calls: &[common::RecordedCall], psk: &str) {
for c in calls {
if c.prog != "nmcli" {
continue;
}
assert!(
!c.args.iter().any(|a| a == psk),
"PSK leaked onto nmcli argv: {:?}",
c.args
);
}
}
#[test]
fn connect_verbose_create_feeds_psk_on_stdin_never_argv() {
let runner = FakeRunner::new()
.on_contains("nmcli", "NAME,TYPE", ok(""))
.on_contains("nmcli", "connect", ok(""))
.on_contains("nmcli", "GENERAL.CON-UUID", ok("uuid-1"))
.on_contains("nmcli", "ipv4.ignore-auto-dns", ok(""))
.on_contains("nmcli", "device reapply", ok(""));
let calls = runner.calls_handle();
let net = net("Cafe", Some("super-secret-psk"));
let result = with_runner(runner, || nm::connect_verbose("wlan0", &net, 8, "1.1.1.1"));
assert!(result.is_ok(), "{result:?}");
let calls = calls.borrow();
assert_psk_not_on_argv(&calls, "super-secret-psk");
let connect = calls
.iter()
.find(|c| c.prog == "nmcli" && c.args.iter().any(|a| a == "connect"))
.expect("expected device wifi connect");
assert!(
connect.args.iter().any(|a| a == "--ask"),
"create path must use --ask: {:?}",
connect.args
);
assert_eq!(connect.stdin.as_deref(), Some("super-secret-psk\n"));
}
#[test]
fn connect_verbose_reuse_feeds_psk_on_stdin_never_argv() {
let runner = FakeRunner::new()
.on_contains("nmcli", "NAME,TYPE", ok("Cafe:802-11-wireless"))
.on_contains("nmcli", "connection modify", ok(""))
.on_contains("nmcli", "connection up", ok(""))
.on_contains("nmcli", "GENERAL.CON-UUID", ok("uuid-1"))
.on_contains("nmcli", "ipv4.ignore-auto-dns", ok(""))
.on_contains("nmcli", "device reapply", ok(""));
let calls = runner.calls_handle();
let net = net("Cafe", Some("super-secret-psk"));
let result = with_runner(runner, || nm::connect_verbose("wlan0", &net, 8, "1.1.1.1"));
assert!(result.is_ok(), "{result:?}");
let calls = calls.borrow();
assert_psk_not_on_argv(&calls, "super-secret-psk");
let up = calls
.iter()
.find(|c| c.prog == "nmcli" && c.args.iter().any(|a| a == "up"))
.expect("expected connection up");
assert!(
up.args.iter().any(|a| a == "--ask"),
"reuse path must use --ask: {:?}",
up.args
);
assert_eq!(up.stdin.as_deref(), Some("super-secret-psk\n"));
// Clearing the stored PSK uses an empty argv value, never the secret.
let cleared = calls.iter().any(|c| {
c.prog == "nmcli"
&& c.args.iter().any(|a| a == "802-11-wireless-security.psk")
&& c.args.last().is_some_and(|a| a.is_empty())
});
assert!(cleared, "reuse+password should reset stored PSK: {calls:?}");
}