Feed Wi-Fi PSK to nmcli --ask on stdin, never argv

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.

README.md

@@ -10,7 +10,7 @@ breadcrumbs sits on top of NetworkManager (`nmcli`) and manages your Wi-Fi based
 - **Bootstrap + Tailscale gating** — connect to an interim network first, bring up Tailscale, then move to the target network
 - **Self-healing watch daemon** — monitors for drops, auto-recovers, reacts within seconds via `nmcli monitor`
 - **Auto-detection** — scans visible SSIDs and guesses your location from config-defined markers
-- **Credential handling** — a saved network's password is only needed the *first* time breadcrumbs connects to it. Once that connect succeeds, NetworkManager durably owns the credential (a new connection profile, or an updated PSK on an existing one), so breadcrumbs clears its own local copy and stops writing it to disk. Both config files are `0600` (owner-only); saved networks live in a separate `networks.toml` from settings/profiles (see [Configuration](#configuration)). Note: on that first connect, the PSK is still passed to `nmcli` as a command argument, so it's briefly visible to other local users via `/proc/<pid>/cmdline` for the lifetime of that `nmcli` child — a known limitation (see the note in `src/nm.rs`); a `nmcli --ask`/D-Bus secret-agent path that avoids argv exposure entirely is not yet wired up. In practice this window now only exists once per network, not on every connect.
+- **Credential handling** — a saved network's password is only needed the *first* time breadcrumbs connects to it. Once that connect succeeds, NetworkManager durably owns the credential (a new connection profile, or an updated PSK on an existing one), so breadcrumbs clears its own local copy and stops writing it to disk. Both config files are `0600` (owner-only); saved networks live in a separate `networks.toml` from settings/profiles (see [Configuration](#configuration)). On that first connect the PSK is fed to `nmcli --ask` on stdin, never as a command argument, so it does not appear in `/proc/<pid>/cmdline`.
 - **Desktop notifications** via `notify-send` (optional)
 - **systemd user service** generation via `breadcrumbs install-service`
 

src/app.rs

@@ -357,9 +357,8 @@ fn prompt_line(msg: &str) -> String {
 /// response) means "this network has no password" (open Wi-Fi) — normalize
 /// it to `None` right at the point of entry so it flows the same way a
 /// genuinely absent/cleared password does. Without this, `Some("")` would
-/// make `nm::connect_verbose` send an empty PSK argument, which nmcli treats
-/// as "secured with a blank password" rather than "open", and the connect
-/// fails against a real open SSID.
+/// make `nm::connect_verbose` treat it as a (blank) secret rather than an
+/// open network, and the connect fails against a real open SSID.
 fn non_empty(s: String) -> Option<String> {
     if s.is_empty() {
         None

src/config.rs

@@ -70,7 +70,8 @@ pub struct NetworkDef {
     /// rather than writing a plaintext copy that's no longer needed. `None`
     /// means either "NetworkManager already owns this secret" or "this is
     /// an open (unsecured) network" — both cases behave the same way on
-    /// connect: no password argument is ever sent.
+    /// connect: no PSK is sent to nmcli at all. When `Some`, the PSK is
+    /// fed to `nmcli --ask` on stdin, never as an argv element.
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub password: Option<String>,
     #[serde(default)]

src/flow.rs

@@ -54,12 +54,11 @@ fn resolve_candidates(cfg: &Config, p: &crate::config::Profile) -> Vec<NetworkDe
 
 /// A network was just connected to using a local password, and NetworkManager
 /// now durably holds that secret — either in a freshly created connection
-/// profile (`device wifi connect`) or an existing one whose PSK we just
-/// updated (`connection modify`). Either way breadcrumbs no longer needs its
-/// own plaintext copy: clear it and persist immediately, so it can't be
-/// re-sent as an argv argument on the next connect and doesn't sit on disk
-/// any longer than necessary. A no-op (no save) if the network has no local
-/// password to begin with.
+/// profile (`device wifi connect --ask`) or an existing one whose PSK we just
+/// supplied via `--ask connection up`. Either way breadcrumbs no longer needs
+/// its own plaintext copy: clear it and persist immediately so it doesn't sit
+/// on disk any longer than necessary. A no-op (no save) if the network has no
+/// local password to begin with.
 fn clear_password_if_used(cfg: &mut Config, ssid: &str) {
     let Some(def) = cfg.networks.iter_mut().find(|n| n.ssid == ssid) else {
         return;
@@ -157,7 +156,9 @@ pub fn run(cfg: &mut Config, profile_name: &str) -> Outcome {
                                 log(&format!("bootstrap connected: {}", bdef.ssid));
                                 clear_password_if_used(cfg, &bdef.ssid);
                             }
-                            Err(e) => log(&format!("bootstrap connect failed: {} — {e}", bdef.ssid)),
+                            Err(e) => {
+                                log(&format!("bootstrap connect failed: {} — {e}", bdef.ssid))
+                            }
                         }
                     } else {
                         log(&format!("bootstrap not in range: {}", bdef.ssid));
@@ -248,7 +249,10 @@ pub fn run(cfg: &mut Config, profile_name: &str) -> Outcome {
         if !nm::device_connected(&iface) {
             // Owned clone (not a borrow of `cfg`) so a successful reconnect
             // is free to mutate `cfg` to clear the used password.
-            if let Some(bdef) = profile.bootstrap.as_deref().and_then(|s| cfg.network(s).cloned())
+            if let Some(bdef) = profile
+                .bootstrap
+                .as_deref()
+                .and_then(|s| cfg.network(s).cloned())
             {
                 match connect_and_verify(&iface, &bdef, cfg) {
                     Ok(()) => {

src/nm.rs

@@ -2,7 +2,7 @@ use std::collections::HashSet;
 use std::time::Duration;
 
 use crate::config::NetworkDef;
-use crate::util::{run, run_ok};
+use crate::util::{run, run_ok, run_with_stdin};
 
 /// nmcli `-t` escapes `:` and `\` in field values; undo that.
 fn unescape(s: &str) -> String {
@@ -284,8 +284,8 @@ pub fn connect(iface: &str, net: &NetworkDef, wait: u32, dns: &str) -> bool {
 
 /// Connect to a network and pin DNS. Returns the nmcli error on failure.
 ///
-/// Reuses an existing saved profile for the SSID when one exists (updating its
-/// PSK) so that repeated connections do not accumulate numbered duplicates in
+/// Reuses an existing saved profile for the SSID when one exists so that
+/// repeated connections do not accumulate numbered duplicates in
 /// NetworkManager ("NCC", "NCC 1", "NCC 2", …). Falls back to
 /// `nmcli device wifi connect` — which creates a new profile — only when no
 /// saved profile is found.
@@ -293,41 +293,18 @@ pub fn connect(iface: &str, net: &NetworkDef, wait: u32, dns: &str) -> bool {
 /// `net.password` is only sent when `Some`: on the reuse path, `None` means
 /// "leave the saved PSK alone" (either NetworkManager already durably owns
 /// it, or the network is open); on the create path it means "no password
-/// argument at all", which is also how a genuinely open (no-security) SSID
-/// is connected. See the field doc on [`NetworkDef::password`] for how a
-/// local secret transitions to `None` after its first successful use.
+/// at all", which is also how a genuinely open (no-security) SSID is
+/// connected. See the field doc on [`NetworkDef::password`] for how a local
+/// secret transitions to `None` after its first successful use.
 ///
-/// KNOWN LIMITATION (credential exposure): when a password *is* sent, it's
-/// passed to `nmcli` as a plain command-line argument
-/// (`802-11-wireless-security.psk <pw>` on the reuse path, `password <pw>`
-/// on the create path). For the lifetime of that `nmcli` child, the secret
-/// is readable by other local users via `/proc/<pid>/cmdline`.
-/// `util::run_with_stdin` exists to feed secrets on stdin instead, but
-/// wiring it up correctly needs either verified `nmcli --ask` piped-stdin
-/// behavior or NetworkManager's D-Bus secret-agent API — neither of which
-/// can be validated without a live NetworkManager connection — so this is
-/// left as documented tech debt rather than a guess. In practice this
-/// exposure window now only exists on a network's *first* connect: once
-/// NetworkManager has the credential, breadcrumbs clears its local copy, so
-/// there's nothing left to pass on argv for every subsequent connect.
+/// When a password *is* sent it goes to `nmcli --ask` on stdin, never on
+/// argv — `/proc/<pid>/cmdline` is world-readable. After the first success
+/// breadcrumbs clears its local copy, so subsequent connects pass nothing.
 pub fn connect_verbose(iface: &str, net: &NetworkDef, wait: u32, dns: &str) -> Result<(), String> {
     let wait_s = wait.to_string();
+    let timeout = Duration::from_secs(wait as u64 + 15);
 
     if let Some(profile) = first_profile_for_ssid(&net.ssid) {
-        // Update the saved PSK and, for hidden networks, ensure the flag is set.
-        if let Some(pw) = &net.password {
-            let _ = run(
-                "nmcli",
-                &[
-                    "connection",
-                    "modify",
-                    &profile,
-                    "802-11-wireless-security.psk",
-                    pw.as_str(),
-                ],
-                Duration::from_secs(6),
-            );
-        }
         if net.hidden {
             let _ = run(
                 "nmcli",
@@ -341,11 +318,52 @@ pub fn connect_verbose(iface: &str, net: &NetworkDef, wait: u32, dns: &str) -> R
                 Duration::from_secs(6),
             );
         }
-        let o = run(
-            "nmcli",
-            &["--wait", &wait_s, "connection", "up", &profile, "ifname", iface],
-            Duration::from_secs(wait as u64 + 15),
-        );
+        let o = if let Some(pw) = &net.password {
+            // WHY: a stored PSK makes NM skip the secret agent, so --ask
+            // would never read stdin. Resetting the property (empty value,
+            // not a secret) forces a request; the new PSK arrives on stdin.
+            let _ = run(
+                "nmcli",
+                &[
+                    "connection",
+                    "modify",
+                    &profile,
+                    "802-11-wireless-security.psk",
+                    "",
+                ],
+                Duration::from_secs(6),
+            );
+            let stdin = format!("{pw}\n");
+            run_with_stdin(
+                "nmcli",
+                &[
+                    "--ask",
+                    "--wait",
+                    &wait_s,
+                    "connection",
+                    "up",
+                    &profile,
+                    "ifname",
+                    iface,
+                ],
+                Some(&stdin),
+                timeout,
+            )
+        } else {
+            run(
+                "nmcli",
+                &[
+                    "--wait",
+                    &wait_s,
+                    "connection",
+                    "up",
+                    &profile,
+                    "ifname",
+                    iface,
+                ],
+                timeout,
+            )
+        };
         if !o.success {
             let detail = o.stderr.trim().to_string();
             return Err(if detail.is_empty() {
@@ -362,29 +380,51 @@ pub fn connect_verbose(iface: &str, net: &NetworkDef, wait: u32, dns: &str) -> R
 
     // No saved profile — create one via device wifi connect.
     let hidden = if net.hidden { "yes" } else { "no" };
-    let mut args: Vec<&str> = vec![
-        "--wait",
-        &wait_s,
-        "device",
-        "wifi",
-        "connect",
-        net.ssid.as_str(),
-    ];
-    // Only pass `password` when we actually have one. An empty/missing PSK
-    // argument makes nmcli treat the network as open (no security), which is
-    // what we want both for genuinely open SSIDs and for a network whose
-    // secret NetworkManager should already hold — though the latter case
-    // only succeeds if a saved profile in fact exists, which is why we only
-    // reach this branch (no saved profile found) when that assumption held.
-    if let Some(pw) = &net.password {
-        args.push("password");
-        args.push(pw.as_str());
-    }
-    args.push("hidden");
-    args.push(hidden);
-    args.push("ifname");
-    args.push(iface);
-    let o = run("nmcli", &args, Duration::from_secs(wait as u64 + 15));
+    let o = if let Some(pw) = &net.password {
+        // WHY: never put the PSK on argv — /proc/<pid>/cmdline is
+        // world-readable. `nmcli --ask` registers as a secret agent and
+        // nmc_readline reads the PSK from stdin (one line). Open networks
+        // stay on the no-ask path so we don't hang on a prompt.
+        let stdin = format!("{pw}\n");
+        run_with_stdin(
+            "nmcli",
+            &[
+                "--ask",
+                "--wait",
+                &wait_s,
+                "device",
+                "wifi",
+                "connect",
+                net.ssid.as_str(),
+                "hidden",
+                hidden,
+                "ifname",
+                iface,
+            ],
+            Some(&stdin),
+            timeout,
+        )
+    } else {
+        // No local PSK: either the SSID is open, or NM should already hold
+        // the secret — the latter only succeeds if a saved profile exists,
+        // which is why we only reach this branch when that assumption held.
+        run(
+            "nmcli",
+            &[
+                "--wait",
+                &wait_s,
+                "device",
+                "wifi",
+                "connect",
+                net.ssid.as_str(),
+                "hidden",
+                hidden,
+                "ifname",
+                iface,
+            ],
+            timeout,
+        )
+    };
     if !o.success {
         let detail = o.stderr.trim().to_string();
         return Err(if detail.is_empty() {

src/watch.rs

@@ -113,9 +113,8 @@ fn spawn_nm_monitor(tx: mpsc::Sender<()>) {
             let mut last: Option<Instant> = None;
             for line in reader.lines().map_while(Result::ok) {
                 let l = line.to_lowercase();
-                let interesting = l.contains("disconnect")
-                    || l.contains("unavailable")
-                    || l.contains("failed");
+                let interesting =
+                    l.contains("disconnect") || l.contains("unavailable") || l.contains("failed");
                 if interesting && debounce_ready(last, Duration::from_millis(1500)) {
                     last = Some(Instant::now());
                     let _ = tx.send(());
@@ -281,7 +280,9 @@ pub fn run(mut cfg: Config, run_initial: bool) -> i32 {
                         Urgency::Normal,
                     );
                 }
-                let elapsed = last_flow_at.map(|t| t.elapsed().as_secs()).unwrap_or(u64::MAX);
+                let elapsed = last_flow_at
+                    .map(|t| t.elapsed().as_secs())
+                    .unwrap_or(u64::MAX);
                 if elapsed >= FLOW_COOLDOWN {
                     log(&format!(
                         "watch: down ({:?}) profile={profile} ssid={:?} — running flow",

tests/cli.rs

@@ -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}");
 }
 

tests/flow_watch.rs

@@ -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:?}");
+}