Fix bugs from a full audit: Tailscale recovery, SSID verification, captive portals, config races

The watch loop could spin forever on a stopped Tailscale daemon (the
auto-start path was unreachable) while re-notifying every retry, report
"connected" when NM autoconnect won a race onto a different SSID, and
classify captive portals as healthy (200/302 accepted as internet). The
bread-bus subscription thread also read/wrote config and state files
concurrently with the watch loop. Fix all of those plus: EDITOR values
with arguments, scan --to silently ignoring unknown profiles, deleted
core profiles being resurrected, inline-network migration data loss,
login never retried, XDG-unaware install-service, detect persisting a
stale default profile, password length leaking through the mask, a
stdin/stdout pipe deadlock, and several minor UI/robustness issues.

Generated with Codebuff 🤖
Co-Authored-By: Codebuff <noreply@codebuff.com>
This commit is contained in:
Breadway 2026-08-26 12:33:07 +08:00
parent 02e96126e0
commit c02360a873
12 changed files with 538 additions and 189 deletions

View file

@ -4,6 +4,7 @@
//! consumers (including the integration tests under `tests/`). //! consumers (including the integration tests under `tests/`).
use std::io::{BufRead, Write}; use std::io::{BufRead, Write};
use std::path::PathBuf;
use std::process::Command; use std::process::Command;
use std::time::Duration; use std::time::Duration;
@ -66,9 +67,11 @@ enum Cmd {
ssid: String, ssid: String,
/// Password (prompted if omitted) /// Password (prompted if omitted)
password: Option<String>, password: Option<String>,
/// Network is hidden (does not broadcast its SSID) /// Network is hidden (does not broadcast its SSID).
#[arg(long)] /// `--hidden` sets it; `--hidden=false` clears it on an existing
hidden: bool, /// entry; omitted leaves an existing entry's flag untouched.
#[arg(long, num_args = 0..=1, default_missing_value = "true")]
hidden: Option<bool>,
/// Attach this SSID to a profile's priority list /// Attach this SSID to a profile's priority list
#[arg(long)] #[arg(long)]
to: Option<String>, to: Option<String>,
@ -321,8 +324,15 @@ fn detect_profile(cfg: &Config) -> Option<String> {
} }
} }
// Fall back to the default profile if no markers matched. // Fall back to the default profile if no markers matched — but only if
// it actually exists: a stale `default_profile` name is a config error,
// not a detection result, and persisting it would wedge the watcher in
// UnknownProfile forever.
if cfg.profiles.contains_key(&cfg.settings.default_profile) {
Some(cfg.settings.default_profile.clone()) Some(cfg.settings.default_profile.clone())
} else {
None
}
} }
fn cmd_detect(cfg: &mut Config, apply: bool) -> Result<i32, String> { fn cmd_detect(cfg: &mut Config, apply: bool) -> Result<i32, String> {
@ -330,18 +340,19 @@ fn cmd_detect(cfg: &mut Config, apply: bool) -> Result<i32, String> {
Some(p) => { Some(p) => {
println!("{p}"); println!("{p}");
if apply { if apply {
State { // Route through state::set_profile (like the CLI and the
profile: p.clone(), // bread bus do) so an unknown fallback is rejected with a
updated: crate::util::timestamp(), // proper error instead of being persisted as active.
} state::set_profile(cfg, &p)?;
.save()?;
let outcome = flow::run(cfg, &p); let outcome = flow::run(cfg, &p);
print_outcome(&p, &outcome); print_outcome(&p, &outcome);
return Ok(if outcome.ok() { 0 } else { 1 }); return Ok(if outcome.ok() { 0 } else { 1 });
} }
Ok(0) Ok(0)
} }
None => Err("could not detect a profile (no Wi-Fi adapter?)".into()), None => Err("could not detect a profile (no Wi-Fi adapter, or no \
profile matches and the default is misconfigured)"
.into()),
} }
} }
@ -385,7 +396,7 @@ fn cmd_add(
cfg: &mut Config, cfg: &mut Config,
ssid: String, ssid: String,
password: Option<String>, password: Option<String>,
hidden: bool, hidden: Option<bool>,
to: Option<String>, to: Option<String>,
at: Option<usize>, at: Option<usize>,
) -> Result<i32, String> { ) -> Result<i32, String> {
@ -397,12 +408,17 @@ fn cmd_add(
match cfg.networks.iter_mut().find(|n| n.ssid == ssid) { match cfg.networks.iter_mut().find(|n| n.ssid == ssid) {
Some(n) => { Some(n) => {
n.password = password; n.password = password;
n.hidden = hidden || n.hidden; // `--hidden` / `--hidden=false` set the flag explicitly; when
// the flag is omitted, leave an existing entry's hidden state
// alone (a password-only update must not un-hide a network).
if let Some(h) = hidden {
n.hidden = h;
}
} }
None => cfg.networks.push(NetworkDef { None => cfg.networks.push(NetworkDef {
ssid: ssid.clone(), ssid: ssid.clone(),
password, password,
hidden, hidden: hidden.unwrap_or(false),
}), }),
} }
if let Some(prof_name) = to { if let Some(prof_name) = to {
@ -443,6 +459,14 @@ fn cmd_forget(cfg: &mut Config, ssid: &str) -> Result<i32, String> {
} }
fn cmd_scan(cfg: &mut Config, to: Option<String>) -> Result<i32, String> { fn cmd_scan(cfg: &mut Config, to: Option<String>) -> Result<i32, String> {
// Validate `--to` up front, before any side effects (connecting is
// one): `add --to` errors on an unknown profile, so `scan --to` must
// too instead of silently saving a network that never gets attached.
if let Some(prof_name) = &to {
if !cfg.profiles.contains_key(prof_name) {
return Err(format!("unknown profile '{prof_name}'"));
}
}
let iface = nm::wifi_interface().ok_or("no Wi-Fi adapter")?; let iface = nm::wifi_interface().ok_or("no Wi-Fi adapter")?;
nm::radio_on(); nm::radio_on();
nm::rescan(&iface, &[]); nm::rescan(&iface, &[]);
@ -502,12 +526,13 @@ fn cmd_scan(cfg: &mut Config, to: Option<String>) -> Result<i32, String> {
Ok(0) Ok(0)
} }
/// Mask a secret for display. Operates on chars (not bytes) so multi-byte /// Mask a secret for display. Always renders the same fixed-length
/// UTF-8 passwords don't panic on a mid-character byte slice, and never /// placeholder so the output reveals neither the secret's length nor any
/// echoes back any real character of the secret (previously the first byte /// character of it (a fixed placeholder is what password managers show;
/// was shown unmasked). /// length-hiding also means multi-byte UTF-8 passwords need no special
fn mask(p: &str) -> String { /// handling).
"".repeat(p.chars().count().max(2)) fn mask(_p: &str) -> String {
"".repeat(8)
} }
fn cmd_list(cfg: &Config, show_pw: bool) -> Result<i32, String> { fn cmd_list(cfg: &Config, show_pw: bool) -> Result<i32, String> {
@ -567,7 +592,15 @@ fn cmd_list(cfg: &Config, show_pw: bool) -> Result<i32, String> {
fn cmd_edit() -> Result<i32, String> { fn cmd_edit() -> Result<i32, String> {
let editor = std::env::var("EDITOR").unwrap_or_else(|_| "nano".into()); let editor = std::env::var("EDITOR").unwrap_or_else(|_| "nano".into());
let path = config::config_path(); let path = config::config_path();
let status = Command::new(&editor) // EDITOR values routinely carry arguments ("code -w", "subl -w"), so
// split on whitespace: the first token is the program, the rest are its
// arguments. The path stays a separate argument — never interpolated
// into a shell string — so it can't be used for injection.
let mut parts = editor.split_whitespace();
let prog = parts.next().unwrap_or("nano");
let mut cmd = Command::new(prog);
cmd.args(parts);
let status = cmd
.arg(&path) .arg(&path)
.status() .status()
.map_err(|e| format!("launching {editor}: {e}"))?; .map_err(|e| format!("launching {editor}: {e}"))?;
@ -685,7 +718,13 @@ fn exec_replace(prog: &str, dir: &std::path::Path) -> String {
} }
fn cmd_install_service(enable: bool) -> Result<i32, String> { fn cmd_install_service(enable: bool) -> Result<i32, String> {
let unit_dir = home_dir().join(".config/systemd/user"); // Honor XDG_CONFIG_HOME like the rest of the app: systemd --user units
// live in $XDG_CONFIG_HOME/systemd/user (default ~/.config/systemd/user).
let unit_dir = std::env::var_os("XDG_CONFIG_HOME")
.map(PathBuf::from)
.unwrap_or_else(|| home_dir().join(".config"))
.join("systemd")
.join("user");
std::fs::create_dir_all(&unit_dir) std::fs::create_dir_all(&unit_dir)
.map_err(|e| format!("creating {}: {e}", unit_dir.display()))?; .map_err(|e| format!("creating {}: {e}", unit_dir.display()))?;
let bin = std::env::current_exe().map_err(|e| format!("resolving current executable: {e}"))?; let bin = std::env::current_exe().map_err(|e| format!("resolving current executable: {e}"))?;
@ -746,24 +785,15 @@ mod tests {
use super::*; use super::*;
#[test] #[test]
fn mask_empty_password() { fn mask_is_fixed_length_regardless_of_secret() {
// len() == 0 <= 2 branch: still at least 2 dots so an empty saved // Fixed-length masking: the output must reveal neither the secret's
// password doesn't visually collapse to nothing in `list`. // length nor any character of it — for empty, short, and long
assert_eq!(mask(""), "••"); // secrets alike.
} assert_eq!(mask(""), "".repeat(8));
assert_eq!(mask("a"), "".repeat(8));
#[test] assert_eq!(mask("ab"), "".repeat(8));
fn mask_short_passwords_reveal_nothing() { assert_eq!(mask("hunter2"), "".repeat(8));
assert_eq!(mask("a"), "••"); assert!(!mask("hunter2").contains('h'));
assert_eq!(mask("ab"), "••");
}
#[test]
fn mask_never_echoes_a_real_character() {
let pw = "hunter2";
let masked = mask(pw);
assert_eq!(masked, "".repeat(pw.len()));
assert!(!masked.contains('h'), "masked output leaked the first char");
} }
#[test] #[test]
@ -773,14 +803,13 @@ mod tests {
// emoji or accented character), since byte index 1 can land mid-char. // emoji or accented character), since byte index 1 can land mid-char.
let pw = "日本語パスワード"; let pw = "日本語パスワード";
let masked = mask(pw); let masked = mask(pw);
assert_eq!(masked.chars().count(), pw.chars().count()); assert_eq!(masked, "".repeat(8));
assert!(masked.chars().all(|c| c == '•')); assert!(masked.chars().all(|c| c == '•'));
} }
#[test] #[test]
fn mask_emoji_first_character_does_not_panic() { fn mask_emoji_first_character_does_not_panic() {
let pw = "🔒password123"; let pw = "🔒password123";
let masked = mask(pw); assert_eq!(mask(pw), "".repeat(8));
assert_eq!(masked.chars().count(), pw.chars().count());
} }
} }

View file

@ -37,37 +37,54 @@ pub fn emit_health_changed(client: &BreadClient, profile: &str, health: &str, ss
); );
} }
/// What a `bread.command.crumbs.*` event asks the watch loop to do.
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum CommandAction {
/// Persist this profile via [`state::set_profile`]. Applied on the
/// watch loop thread — the single owner of config/state file access —
/// never on the subscription thread, which would race the loop's own
/// `Config::load`/`save`.
SetProfile(String),
/// Nothing to do: unknown verb, or a validation failure already
/// reported via `bread.crumbs.set_profile.failed`.
Ignore,
}
/// Reacts to `bread.command.crumbs.*` verbs. Only `set_profile` maps to /// Reacts to `bread.command.crumbs.*` verbs. Only `set_profile` maps to
/// real, existing breadcrumbs functionality today — there is no pin/select /// real, existing breadcrumbs functionality today — there is no pin/select
/// (or other) verb because breadcrumbs has no such concept. Unrecognized /// (or other) verb because breadcrumbs has no such concept. Unrecognized
/// verbs are ignored, not stubbed as no-ops that pretend to succeed. /// verbs are ignored, not stubbed as no-ops that pretend to succeed.
/// ///
/// Returns `true` when a profile was actually persisted, so the watch loop /// This only *parses and validates* the command — it performs no file I/O
/// can wake immediately and re-evaluate instead of waiting out the current /// (that would race the watch loop's own config access from a second
/// poll interval. /// thread). The returned [`CommandAction`] is forwarded to the loop, which
/// /// applies it via [`apply_set_profile`].
/// Emits `bread.crumbs.set_profile.done`/`.failed` per the confirmation pub fn handle_command(event: &BreadEvent) -> CommandAction {
/// convention in bread's Documentation.md.
pub fn handle_command(event: &BreadEvent) -> bool {
let Some(verb) = event.event.strip_prefix("bread.command.crumbs.") else { let Some(verb) = event.event.strip_prefix("bread.command.crumbs.") else {
return false; return CommandAction::Ignore;
}; };
match verb { match verb {
"set_profile" => handle_set_profile(event), "set_profile" => match event.data.get("profile").and_then(|v| v.as_str()) {
Some(name) if !name.trim().is_empty() => CommandAction::SetProfile(name.to_string()),
_ => {
emit_set_profile_failed("missing string \"profile\" in command data");
CommandAction::Ignore
}
},
other => { other => {
crate::notify::log(&format!( crate::notify::log(&format!(
"watch: ignoring unrecognized bread.command.crumbs.{other}" "watch: ignoring unrecognized bread.command.crumbs.{other}"
)); ));
false CommandAction::Ignore
} }
} }
} }
fn handle_set_profile(event: &BreadEvent) -> bool { /// Apply a `set_profile` command on the watch loop thread and emit the
let Some(name) = event.data.get("profile").and_then(|v| v.as_str()) else { /// `done`/`failed` confirmation. Kept separate from [`handle_command`] so
emit_set_profile_failed("missing string \"profile\" in command data"); /// the bread subscription thread never touches config/state files
return false; /// concurrently with the loop.
}; pub fn apply_set_profile(name: &str) {
match Config::load().and_then(|cfg| state::set_profile(&cfg, name)) { match Config::load().and_then(|cfg| state::set_profile(&cfg, name)) {
Ok(()) => { Ok(()) => {
crate::notify::log(&format!( crate::notify::log(&format!(
@ -77,11 +94,9 @@ fn handle_set_profile(event: &BreadEvent) -> bool {
"bread.crumbs.set_profile.done", "bread.crumbs.set_profile.done",
serde_json::json!({ "profile": name }), serde_json::json!({ "profile": name }),
); );
true
} }
Err(e) => { Err(e) => {
emit_set_profile_failed(&e); emit_set_profile_failed(&e);
false
} }
} }
} }

View file

@ -28,6 +28,10 @@ fn default_ping_host() -> String {
"1.1.1.1".to_string() "1.1.1.1".to_string()
} }
fn is_false(b: &bool) -> bool {
!b
}
#[derive(Debug, Clone, Serialize, Deserialize)] #[derive(Debug, Clone, Serialize, Deserialize)]
pub struct Settings { pub struct Settings {
#[serde(default = "default_dns")] #[serde(default = "default_dns")]
@ -44,6 +48,14 @@ pub struct Settings {
pub connectivity_url: String, pub connectivity_url: String,
#[serde(default = "default_ping_host")] #[serde(default = "default_ping_host")]
pub ping_host: String, pub ping_host: String,
/// Set the first time the config is saved. Core profiles (`home` /
/// `work` / `away`) are only backfilled for genuinely fresh or legacy
/// configs; once the user owns the file, a profile they deliberately
/// deleted stays deleted instead of being silently resurrected on the
/// next load. Omits itself from the TOML until set, so existing
/// configs keep parsing exactly as before.
#[serde(default, skip_serializing_if = "is_false")]
pub core_profiles_initialized: bool,
} }
impl Default for Settings { impl Default for Settings {
@ -56,6 +68,7 @@ impl Default for Settings {
watch_interval: default_watch_interval(), watch_interval: default_watch_interval(),
connectivity_url: default_connectivity_url(), connectivity_url: default_connectivity_url(),
ping_host: default_ping_host(), ping_host: default_ping_host(),
core_profiles_initialized: false,
} }
} }
} }
@ -175,7 +188,7 @@ impl Config {
pub fn load() -> Result<Config, String> { pub fn load() -> Result<Config, String> {
let path = config_path(); let path = config_path();
if !path.exists() { if !path.exists() {
let cfg = build_initial_config(); let mut cfg = build_initial_config();
cfg.save()?; cfg.save()?;
return Ok(cfg); return Ok(cfg);
} }
@ -192,14 +205,31 @@ impl Config {
.map_err(|e| format!("reading {}: {e}", net_path.display()))?; .map_err(|e| format!("reading {}: {e}", net_path.display()))?;
let nf: NetworksFile = toml::from_str(&net_text) let nf: NetworksFile = toml::from_str(&net_text)
.map_err(|e| format!("parsing {}: {e}", net_path.display()))?; .map_err(|e| format!("parsing {}: {e}", net_path.display()))?;
cfg.networks = nf.networks; // Merge, don't overwrite: a legacy config can still carry an
// inline `[[networks]]` block, and those entries must survive
// even when networks.toml already exists — otherwise the next
// save() (which writes only networks.toml) would silently drop
// hand-added inline networks. networks.toml wins on SSID
// conflicts; inline-only entries are appended and migrated.
let mut merged = nf.networks;
for def in std::mem::take(&mut cfg.networks) {
if !merged.iter().any(|n| n.ssid == def.ssid) {
merged.push(def);
}
}
cfg.networks = merged;
} }
// else: no networks.toml yet — keep whatever legacy inline networks // else: no networks.toml yet — keep whatever legacy inline networks
// were read from breadcrumbs.toml above (or none, on a genuinely // were read from breadcrumbs.toml above (or none, on a genuinely
// fresh config). The next `save()` writes them to networks.toml and // fresh config). The next `save()` writes them to networks.toml and
// stops writing them into breadcrumbs.toml, completing the migration. // stops writing them into breadcrumbs.toml, completing the migration.
// Self-heal: guarantee the three core profiles always exist. // Enforce the documented minimum so `list` and the watch loop agree
// on the poll interval (watch silently clamps to 4 otherwise).
if cfg.settings.watch_interval < 4 {
cfg.settings.watch_interval = 4;
}
ensure_core_profiles(&mut cfg); ensure_core_profiles(&mut cfg);
Ok(cfg) Ok(cfg)
} }
@ -209,7 +239,12 @@ impl Config {
/// `forget`, `scan`, `profile set`, and `flow::run`'s own credential /// `forget`, `scan`, `profile set`, and `flow::run`'s own credential
/// clearing) goes through this single method so the two files never /// clearing) goes through this single method so the two files never
/// drift out of sync with each other. /// drift out of sync with each other.
pub fn save(&self) -> Result<(), String> { pub fn save(&mut self) -> Result<(), String> {
// The first save marks the config as user-owned: core profiles are
// backfilled only for genuinely fresh/legacy configs, never
// resurrected after the user has edited (or deleted) them.
self.settings.core_profiles_initialized = true;
let dir = config_dir(); let dir = config_dir();
fs::create_dir_all(&dir).map_err(|e| format!("creating {}: {e}", dir.display()))?; fs::create_dir_all(&dir).map_err(|e| format!("creating {}: {e}", dir.display()))?;
@ -296,6 +331,13 @@ fn core_profiles() -> BTreeMap<String, Profile> {
} }
fn ensure_core_profiles(cfg: &mut Config) { fn ensure_core_profiles(cfg: &mut Config) {
// Backfill missing core profiles only until the user has taken
// ownership of the config (`core_profiles_initialized` is set by the
// first save). After that, a profile the user deliberately deleted
// stays deleted.
if cfg.settings.core_profiles_initialized {
return;
}
for (name, prof) in core_profiles() { for (name, prof) in core_profiles() {
cfg.profiles.entry(name).or_insert(prof); cfg.profiles.entry(name).or_insert(prof);
} }
@ -350,6 +392,23 @@ mod tests {
assert!(cfg.profile("away").is_some()); assert!(cfg.profile("away").is_some());
} }
#[test]
fn ensure_core_profiles_skips_backfill_once_initialized() {
// After the first save the config is user-owned: a deliberately
// deleted core profile must stay deleted instead of being
// resurrected on every load.
let mut cfg = Config {
settings: Settings {
core_profiles_initialized: true,
..Default::default()
},
networks: vec![],
profiles: BTreeMap::new(),
};
ensure_core_profiles(&mut cfg);
assert!(cfg.profiles.is_empty(), "no backfill once user-owned");
}
#[test] #[test]
fn ensure_core_profiles_preserves_user_customized_core_profile() { fn ensure_core_profiles_preserves_user_customized_core_profile() {
// A user-edited "home" (custom SSIDs) must not be clobbered by the // A user-edited "home" (custom SSIDs) must not be clobbered by the

View file

@ -1,3 +1,6 @@
use std::thread;
use std::time::Duration;
use crate::config::{Config, NetworkDef}; use crate::config::{Config, NetworkDef};
use crate::nm; use crate::nm;
use crate::notify::{log, notify, Urgency}; use crate::notify::{log, notify, Urgency};
@ -74,14 +77,45 @@ fn clear_password_if_used(cfg: &mut Config, ssid: &str) {
} }
} }
/// Try to connect + confirm it actually carries traffic. /// Try to connect + confirm the device actually landed on the *requested*
/// Returns Ok(()) on success, Err(reason) on failure. /// SSID. Returns Ok(()) on success, Err(reason) on failure.
fn connect_and_verify(iface: &str, def: &NetworkDef, cfg: &Config) -> Result<(), String> { fn connect_and_verify(iface: &str, def: &NetworkDef, cfg: &Config) -> Result<(), String> {
nm::connect_verbose(iface, def, cfg.settings.nmcli_wait, &cfg.settings.dns)?; nm::connect_verbose(iface, def, cfg.settings.nmcli_wait, &cfg.settings.dns)?;
// Confirm the SSID, not just "device connected": NM autoconnect can win
// a race and leave the device on a different network, and the wifi list
// can lag activation by a moment — so poll briefly before giving up.
for _ in 0..8 {
match nm::active_ssid(iface) {
// Explicitly on the requested network — success.
Some(active) if active == def.ssid => return Ok(()),
// Associated with a *different* network — the failure this
// check exists to catch.
Some(_) => break,
// Scan list stale right after activation — keep polling while
// the device is at least connected.
None => {
if !nm::device_connected(iface) { if !nm::device_connected(iface) {
return Err("device not connected after nmcli success".into()); break;
} }
Ok(()) }
}
thread::sleep(Duration::from_millis(250));
}
Err(format!("not associated with '{}' after connect", def.ssid))
}
/// Run the connection state machine for `profile_name`, with desktop
/// notifications enabled. See [`run_quiet`] for the daemon-facing variant.
pub fn run(cfg: &mut Config, profile_name: &str) -> Outcome {
run_inner(cfg, profile_name, true)
}
/// Same state machine, but suppresses desktop notifications. Used by the
/// watch loop, which does its own transition-gated notifications — without
/// this, a persistent failure (e.g. a stopped Tailscale daemon) would
/// re-notify on every recovery retry instead of once per state change.
pub fn run_quiet(cfg: &mut Config, profile_name: &str) -> Outcome {
run_inner(cfg, profile_name, false)
} }
/// Run the connection state machine for `profile_name`. /// Run the connection state machine for `profile_name`.
@ -91,15 +125,17 @@ fn connect_and_verify(iface: &str, def: &NetworkDef, cfg: &Config) -> Result<(),
/// immediately (see [`clear_password_if_used`]) — this is the only way that /// immediately (see [`clear_password_if_used`]) — this is the only way that
/// clearing happens for the `init` / `profile set --apply` / `detect --apply` /// clearing happens for the `init` / `profile set --apply` / `detect --apply`
/// commands and the watch loop, all of which route through here. /// commands and the watch loop, all of which route through here.
pub fn run(cfg: &mut Config, profile_name: &str) -> Outcome { fn run_inner(cfg: &mut Config, profile_name: &str, notify_user: bool) -> Outcome {
let profile = match cfg.profile(profile_name) { let profile = match cfg.profile(profile_name) {
Some(p) => p.clone(), Some(p) => p.clone(),
None => { None => {
if notify_user {
notify( notify(
"breadcrumbs: unknown profile", "breadcrumbs: unknown profile",
&format!("'{profile_name}' is not defined in breadcrumbs.toml"), &format!("'{profile_name}' is not defined in breadcrumbs.toml"),
Urgency::Critical, Urgency::Critical,
); );
}
return Outcome::UnknownProfile(profile_name.to_string()); return Outcome::UnknownProfile(profile_name.to_string());
} }
}; };
@ -107,11 +143,13 @@ pub fn run(cfg: &mut Config, profile_name: &str) -> Outcome {
let iface = match nm::wifi_interface() { let iface = match nm::wifi_interface() {
Some(i) => i, Some(i) => i,
None => { None => {
if notify_user {
notify( notify(
"breadcrumbs: no Wi-Fi adapter", "breadcrumbs: no Wi-Fi adapter",
"Hardware issue — Wi-Fi device not found. Manual check needed.", "Hardware issue — Wi-Fi device not found. Manual check needed.",
Urgency::Critical, Urgency::Critical,
); );
}
return Outcome::NoInterface; return Outcome::NoInterface;
} }
}; };
@ -173,6 +211,7 @@ pub fn run(cfg: &mut Config, profile_name: &str) -> Outcome {
let ts = tailscale::ensure_exit_node(&exit_node); let ts = tailscale::ensure_exit_node(&exit_node);
if !ts.is_ok() { if !ts.is_ok() {
let ssid = nm::active_ssid(&iface).or_else(|| profile.bootstrap.clone()); let ssid = nm::active_ssid(&iface).or_else(|| profile.bootstrap.clone());
if notify_user {
notify( notify(
"Tailscale Error", "Tailscale Error",
&format!( &format!(
@ -182,6 +221,7 @@ pub fn run(cfg: &mut Config, profile_name: &str) -> Outcome {
), ),
Urgency::Critical, Urgency::Critical,
); );
}
return Outcome::TailscaleError { ssid, health: ts }; return Outcome::TailscaleError { ssid, health: ts };
} }
log(&format!("tailscale healthy via exit node {exit_node}")); log(&format!("tailscale healthy via exit node {exit_node}"));
@ -205,7 +245,7 @@ pub fn run(cfg: &mut Config, profile_name: &str) -> Outcome {
} else { } else {
Some("associated but no internet yet".to_string()) Some("associated but no internet yet".to_string())
}; };
finish_connected(&def.ssid, profile_name, &note); finish_connected(&def.ssid, profile_name, &note, notify_user);
return Outcome::Connected { return Outcome::Connected {
ssid: def.ssid.clone(), ssid: def.ssid.clone(),
note, note,
@ -227,7 +267,7 @@ pub fn run(cfg: &mut Config, profile_name: &str) -> Outcome {
} else { } else {
Some("associated but no internet yet".to_string()) Some("associated but no internet yet".to_string())
}; };
finish_connected(&def.ssid, profile_name, &note); finish_connected(&def.ssid, profile_name, &note, notify_user);
return Outcome::Connected { return Outcome::Connected {
ssid: def.ssid.clone(), ssid: def.ssid.clone(),
note, note,
@ -272,7 +312,9 @@ pub fn run(cfg: &mut Config, profile_name: &str) -> Outcome {
} else { } else {
format!("target network not in range — staying on {bs_ssid} (Tailscale OK)") format!("target network not in range — staying on {bs_ssid} (Tailscale OK)")
}; };
if notify_user {
notify("breadcrumbs: using bootstrap", &reason, Urgency::Normal); notify("breadcrumbs: using bootstrap", &reason, Urgency::Normal);
}
log(&format!("flow end: on bootstrap {bs_ssid}; {reason}")); log(&format!("flow end: on bootstrap {bs_ssid}; {reason}"));
return Outcome::Connected { return Outcome::Connected {
ssid: bs_ssid, ssid: bs_ssid,
@ -286,33 +328,40 @@ pub fn run(cfg: &mut Config, profile_name: &str) -> Outcome {
.map(|c| c.ssid.as_str()) .map(|c| c.ssid.as_str())
.collect::<Vec<_>>() .collect::<Vec<_>>()
.join(", "); .join(", ");
notify( let msg = if candidates.is_empty() {
"breadcrumbs: no known networks", format!("profile '{profile_name}' has no networks configured")
&format!("profile '{profile_name}': none of [{names}] are in range"), } else {
Urgency::Critical, format!("profile '{profile_name}': none of [{names}] are in range")
); };
if notify_user {
notify("breadcrumbs: no known networks", &msg, Urgency::Critical);
}
log(&format!( log(&format!(
"flow end: no networks connected (profile={profile_name})" "flow end: no networks connected (profile={profile_name})"
)); ));
Outcome::NoNetworks Outcome::NoNetworks
} }
fn finish_connected(ssid: &str, profile: &str, note: &Option<String>) { fn finish_connected(ssid: &str, profile: &str, note: &Option<String>, notify_user: bool) {
match note { match note {
None => { None => {
if notify_user {
notify( notify(
"breadcrumbs: connected", "breadcrumbs: connected",
&format!("{ssid} ({profile})"), &format!("{ssid} ({profile})"),
Urgency::Low, Urgency::Low,
); );
}
log(&format!("flow end: connected {ssid} (profile={profile})")); log(&format!("flow end: connected {ssid} (profile={profile})"));
} }
Some(n) => { Some(n) => {
if notify_user {
notify( notify(
"breadcrumbs: connected (degraded)", "breadcrumbs: connected (degraded)",
&format!("{ssid} ({profile}) — {n}"), &format!("{ssid} ({profile}) — {n}"),
Urgency::Normal, Urgency::Normal,
); );
}
log(&format!( log(&format!(
"flow end: connected {ssid} (profile={profile}) note={n}" "flow end: connected {ssid} (profile={profile}) note={n}"
)); ));

View file

@ -114,6 +114,15 @@ pub struct ScanEntry {
pub security: String, pub security: String,
} }
/// Parse an nmcli SIGNAL value ("72" or "72 %") into a comparable number.
fn signal_strength(s: &str) -> i32 {
s.trim()
.trim_end_matches('%')
.trim()
.parse::<i32>()
.unwrap_or(-100)
}
pub fn scan_list(iface: &str) -> Vec<ScanEntry> { pub fn scan_list(iface: &str) -> Vec<ScanEntry> {
let o = run( let o = run(
"nmcli", "nmcli",
@ -129,8 +138,7 @@ pub fn scan_list(iface: &str) -> Vec<ScanEntry> {
], ],
Duration::from_secs(12), Duration::from_secs(12),
); );
let mut seen = HashSet::new(); let mut out: Vec<ScanEntry> = Vec::new();
let mut out = Vec::new();
if !o.success { if !o.success {
return out; return out;
} }
@ -140,14 +148,29 @@ pub fn scan_list(iface: &str) -> Vec<ScanEntry> {
continue; continue;
} }
let ssid = fields[0].trim().to_string(); let ssid = fields[0].trim().to_string();
if ssid.is_empty() || !seen.insert(ssid.clone()) { if ssid.is_empty() {
// Hidden networks have no SSID in the scan; they're not
// selectable here anyway (see `cmd_scan`), so skip them.
continue; continue;
} }
out.push(ScanEntry { let signal = fields.get(1).cloned().unwrap_or_default();
let security = fields.get(2).cloned().unwrap_or_default();
// One line per BSSID: dedup by SSID keeping the *strongest* signal,
// so a network broadcast by several APs shows once (at its best
// signal) instead of N times at the first listing.
match out.iter_mut().find(|e| e.ssid == ssid) {
Some(existing) => {
if signal_strength(&signal) > signal_strength(&existing.signal) {
existing.signal = signal;
existing.security = security;
}
}
None => out.push(ScanEntry {
ssid, ssid,
signal: fields.get(1).cloned().unwrap_or_default(), signal,
security: fields.get(2).cloned().unwrap_or_default(), security,
}); }),
}
} }
out out
} }

View file

@ -21,8 +21,12 @@ pub fn internet_ok(cfg: &Config) -> bool {
], ],
Duration::from_secs(6), Duration::from_secs(6),
); );
let code = o.stdout.trim(); // Only a 204 counts as real internet. Captive/guest portals answer
if code == "204" || code == "200" || code == "301" || code == "302" { // 200 (a login page) or 302 (a redirect to it) — accepting those
// would classify a portal-trapped device as "Up". The default
// endpoint is generate_204, which returns 204 precisely when
// traffic isn't being intercepted.
if o.stdout.trim() == "204" {
return true; return true;
} }
} }
@ -66,7 +70,10 @@ pub fn gather(cfg: &Config, profile_name: &str) -> Status {
let iface = nm::wifi_interface(); let iface = nm::wifi_interface();
let ssid = iface.as_deref().and_then(nm::active_ssid); let ssid = iface.as_deref().and_then(nm::active_ssid);
let ip = iface.as_deref().and_then(ipv4); let ip = iface.as_deref().and_then(ipv4);
let internet = internet_ok(cfg); // Skip the (potentially 4s-blocking) connectivity probe when there's no
// Wi-Fi interface at all: the watch loop classifies NoAdapter and would
// otherwise burn a network round-trip (curl/ping) every tick for nothing.
let internet = iface.is_some() && internet_ok(cfg);
let prof = cfg.profile(profile_name); let prof = cfg.profile(profile_name);
let ts_required = prof.map(|p| p.tailscale).unwrap_or(false); let ts_required = prof.map(|p| p.tailscale).unwrap_or(false);

View file

@ -21,6 +21,10 @@ pub enum TsHealth {
ExitNodeMissing, ExitNodeMissing,
/// The exit node exists but is offline. /// The exit node exists but is offline.
ExitNodeOffline, ExitNodeOffline,
/// The profile requires an exit node but none is configured
/// (`settings.exit_node` / per-profile `exit_node` empty). Cannot be
/// auto-fixed — the user must configure one.
NoExitNode,
Error(String), Error(String),
} }
@ -37,6 +41,7 @@ impl TsHealth {
TsHealth::Stopped => "backend stopped".into(), TsHealth::Stopped => "backend stopped".into(),
TsHealth::ExitNodeMissing => "exit node not found in tailnet".into(), TsHealth::ExitNodeMissing => "exit node not found in tailnet".into(),
TsHealth::ExitNodeOffline => "exit node is offline".into(), TsHealth::ExitNodeOffline => "exit node is offline".into(),
TsHealth::NoExitNode => "no exit node configured".into(),
TsHealth::Error(e) => format!("error: {e}"), TsHealth::Error(e) => format!("error: {e}"),
} }
} }
@ -225,10 +230,30 @@ pub fn ensure_exit_node(node: &str) -> TsHealth {
if !installed() { if !installed() {
return TsHealth::NotInstalled; return TsHealth::NotInstalled;
} }
if node.trim().is_empty() {
// Never run `tailscale set --exit-node=` with an empty node — that
// would clear the user's current exit-node selection. This is a
// config error, surfaced as its own health state.
return TsHealth::NoExitNode;
}
let v = match status_json() { let v = match status_json() {
Some(v) => v, Some(v) => v,
None => {
// Daemon unreachable — usually *not running*: a stopped daemon
// prints its error to stderr and leaves stdout empty, which is
// exactly why this branch used to be dead code (the
// BackendState "Stopped" case below only fires when the daemon
// is up but the backend is stopped). Try to bring it up before
// giving up; `tailscale up` is idempotent when already running
// and fails fast (no sudo prompt — stdin is /dev/null) when
// the caller lacks permission to manage the daemon.
let _ = run("tailscale", &["up"], Duration::from_secs(20));
match status_json() {
Some(v2) => v2,
None => return TsHealth::Error("could not read tailscale status".into()), None => return TsHealth::Error("could not read tailscale status".into()),
}
}
}; };
match backend_state(&v).as_str() { match backend_state(&v).as_str() {
@ -286,6 +311,11 @@ pub fn check(node: &str) -> TsHealth {
if !installed() { if !installed() {
return TsHealth::NotInstalled; return TsHealth::NotInstalled;
} }
if node.trim().is_empty() {
// Read-only check, so never runs `tailscale set` — an empty node is
// a config error, not something this probe can fix.
return TsHealth::NoExitNode;
}
let v = match status_json() { let v = match status_json() {
Some(v) => v, Some(v) => v,
None => return TsHealth::Error("status unavailable".into()), None => return TsHealth::Error("status unavailable".into()),
@ -433,6 +463,7 @@ mod tests {
assert!(!TsHealth::Stopped.is_ok()); assert!(!TsHealth::Stopped.is_ok());
assert!(!TsHealth::ExitNodeMissing.is_ok()); assert!(!TsHealth::ExitNodeMissing.is_ok());
assert!(!TsHealth::ExitNodeOffline.is_ok()); assert!(!TsHealth::ExitNodeOffline.is_ok());
assert!(!TsHealth::NoExitNode.is_ok());
assert!(!TsHealth::Error("x".into()).is_ok()); assert!(!TsHealth::Error("x".into()).is_ok());
} }
@ -443,6 +474,7 @@ mod tests {
TsHealth::NeedsLogin.describe(), TsHealth::NeedsLogin.describe(),
"not logged in (run: tailscale up)" "not logged in (run: tailscale up)"
); );
assert_eq!(TsHealth::NoExitNode.describe(), "no exit node configured");
assert_eq!(TsHealth::Error("boom".into()).describe(), "error: boom"); assert_eq!(TsHealth::Error("boom".into()).describe(), "error: boom");
} }

View file

@ -139,13 +139,6 @@ fn spawn_run(prog: &str, args: &[&str], stdin: Option<&str>, timeout: Duration)
Err(_) => return Output::failed(), Err(_) => return Output::failed(),
}; };
if let Some(data) = stdin {
if let Some(mut sink) = child.stdin.take() {
let _ = sink.write_all(data.as_bytes());
// Drop closes the pipe so the child's read sees EOF.
}
}
let mut stdout_pipe = child.stdout.take(); let mut stdout_pipe = child.stdout.take();
let mut stderr_pipe = child.stderr.take(); let mut stderr_pipe = child.stderr.take();
@ -164,6 +157,16 @@ fn spawn_run(prog: &str, args: &[&str], stdin: Option<&str>, timeout: Duration)
buf buf
}); });
// Feed stdin only now that the reader threads are draining stdout and
// stderr: a chatty child could otherwise fill its stdout pipe while we
// block writing stdin, deadlocking both sides.
if let Some(data) = stdin {
if let Some(mut sink) = child.stdin.take() {
let _ = sink.write_all(data.as_bytes());
// Drop closes the pipe so the child's read sees EOF.
}
}
let start = Instant::now(); let start = Instant::now();
let status = loop { let status = loop {
match child.try_wait() { match child.try_wait() {

View file

@ -64,9 +64,11 @@ pub fn classify(cfg: &Config, profile: &str) -> (Health, Option<String>) {
if s.tailscale_required { if s.tailscale_required {
match s.tailscale { match s.tailscale {
Some(TsHealth::Ok) => (Health::Up, ssid), Some(TsHealth::Ok) => (Health::Up, ssid),
Some(TsHealth::NeedsLogin) | Some(TsHealth::NotInstalled) => { // NeedsLogin / NotInstalled / NoExitNode all need human action:
(Health::DownTailscaleManual, ssid) // a missing exit-node config can't be auto-fixed either.
} Some(TsHealth::NeedsLogin)
| Some(TsHealth::NotInstalled)
| Some(TsHealth::NoExitNode) => (Health::DownTailscaleManual, ssid),
Some(_) => (Health::DownTailscaleOther, ssid), Some(_) => (Health::DownTailscaleOther, ssid),
None => (Health::DownTailscaleManual, ssid), None => (Health::DownTailscaleManual, ssid),
} }
@ -84,9 +86,18 @@ fn debounce_ready(last: Option<Instant>, gap: Duration) -> bool {
last.map(|t| t.elapsed() > gap).unwrap_or(true) last.map(|t| t.elapsed() > gap).unwrap_or(true)
} }
/// A wake signal for the watch loop. `SetProfile` is an *action* (applied on
/// the loop thread), `LinkChurn` is just "go look" — the distinction keeps
/// every config/state file access on the single loop thread, so the bread
/// subscription thread can never race the loop's own `Config::load`/`save`.
enum Wake {
LinkChurn,
SetProfile(String),
}
/// Tail `nmcli monitor` and ping the channel on link-state churn so we react /// Tail `nmcli monitor` and ping the channel on link-state churn so we react
/// to drops within a second instead of waiting out the poll interval. /// to drops within a second instead of waiting out the poll interval.
fn spawn_nm_monitor(tx: mpsc::Sender<()>) { fn spawn_nm_monitor(tx: mpsc::Sender<Wake>) {
thread::spawn(move || loop { thread::spawn(move || loop {
let child = Command::new("nmcli") let child = Command::new("nmcli")
.arg("monitor") .arg("monitor")
@ -113,11 +124,18 @@ fn spawn_nm_monitor(tx: mpsc::Sender<()>) {
let mut last: Option<Instant> = None; let mut last: Option<Instant> = None;
for line in reader.lines().map_while(Result::ok) { for line in reader.lines().map_while(Result::ok) {
let l = line.to_lowercase(); let l = line.to_lowercase();
let interesting = // `connectivity` lines catch drops that keep the device
l.contains("disconnect") || l.contains("unavailable") || l.contains("failed"); // "connected" but lose the internet (captive portal, DHCP
// failure); `deactivating` covers teardown. Everything else
// waits out the poll interval.
let interesting = l.contains("disconnect")
|| l.contains("unavailable")
|| l.contains("failed")
|| l.contains("deactivating")
|| l.contains("connectivity");
if interesting && debounce_ready(last, Duration::from_millis(1500)) { if interesting && debounce_ready(last, Duration::from_millis(1500)) {
last = Some(Instant::now()); last = Some(Instant::now());
let _ = tx.send(()); let _ = tx.send(Wake::LinkChurn);
} }
} }
} }
@ -127,17 +145,32 @@ fn spawn_nm_monitor(tx: mpsc::Sender<()>) {
}); });
} }
/// Sleep up to `dur`, but wake early if `nmcli monitor` signals link churn. /// Sleep up to `dur`, but wake early if `nmcli monitor` signals link churn or
fn wait_for_tick(rx: &Receiver<()>, dur: Duration) { /// a `set_profile` command arrives. Returns the pending action, if any.
fn wait_for_tick(rx: &Receiver<Wake>, dur: Duration) -> Option<Wake> {
match rx.recv_timeout(dur) { match rx.recv_timeout(dur) {
Ok(()) => { Ok(first) => {
// Drain any burst of events so we don't re-fire immediately. // Drain any burst of churn signals so we don't re-fire
while rx.try_recv().is_ok() {} // immediately, but never drop a queued set_profile — it's an
// action, not a signal, and the earliest one wins.
let mut pending = match &first {
Wake::SetProfile(_) => Some(first),
Wake::LinkChurn => None,
};
while let Ok(w) = rx.try_recv() {
if pending.is_none() && matches!(&w, Wake::SetProfile(_)) {
pending = Some(w);
} }
Err(mpsc::RecvTimeoutError::Timeout) => {} }
pending
}
Err(mpsc::RecvTimeoutError::Timeout) => None,
// Monitor thread gone (shouldn't happen: we hold the sender) — fall // Monitor thread gone (shouldn't happen: we hold the sender) — fall
// back to a plain sleep so we don't busy-spin. // back to a plain sleep so we don't busy-spin.
Err(mpsc::RecvTimeoutError::Disconnected) => thread::sleep(dur), Err(mpsc::RecvTimeoutError::Disconnected) => {
thread::sleep(dur);
None
}
} }
} }
@ -150,21 +183,25 @@ pub fn run(mut cfg: Config, run_initial: bool) -> i32 {
); );
log("watch: started"); log("watch: started");
let (tx, rx) = mpsc::channel::<()>(); let (tx, rx) = mpsc::channel::<Wake>();
spawn_nm_monitor(tx.clone()); spawn_nm_monitor(tx.clone());
// Long-lived, so this uses BreadClient::subscribe (a persistent // Long-lived, so this uses BreadClient::subscribe (a persistent
// background thread with its own reconnect/backoff loop). breadd being // background thread with its own reconnect/backoff loop). breadd being
// absent or restarting is transparent: the subscription just quietly // absent or restarting is transparent: the subscription just quietly
// stops delivering commands until it reconnects. A successful // stops delivering commands until it reconnects. The callback only
// `set_profile` wakes this loop the same way `nmcli monitor` does, so // *validates* the command and forwards an action through the channel —
// the new profile is applied on the next tick instead of waiting out // it never touches config/state files itself (that would race this
// the current poll interval. // loop's own Config::load/save), so all file access stays on this one
// thread.
let bread = BreadClient::connect(bread_events::APP_ID); let bread = BreadClient::connect(bread_events::APP_ID);
let wake = tx; let wake = tx;
let _commands = bread.subscribe("bread.command.crumbs.**", move |event| { let _commands = bread.subscribe("bread.command.crumbs.**", move |event| {
if bread_events::handle_command(&event) { match bread_events::handle_command(&event) {
let _ = wake.send(()); bread_events::CommandAction::SetProfile(name) => {
let _ = wake.send(Wake::SetProfile(name));
}
bread_events::CommandAction::Ignore => {}
} }
}); });
@ -178,7 +215,7 @@ pub fn run(mut cfg: Config, run_initial: bool) -> i32 {
)); ));
} else { } else {
log(&format!("watch: initial flow for profile={profile}")); log(&format!("watch: initial flow for profile={profile}"));
let _ = flow::run(&mut cfg, &profile); let _ = flow::run_quiet(&mut cfg, &profile);
} }
} }
@ -193,9 +230,10 @@ pub fn run(mut cfg: Config, run_initial: bool) -> i32 {
// This always runs *before* `flow::run` below (never after, within // This always runs *before* `flow::run` below (never after, within
// the same tick), so a password `flow::run` clears-and-saves this // the same tick), so a password `flow::run` clears-and-saves this
// iteration is durably on disk by the time the *next* iteration's // iteration is durably on disk by the time the *next* iteration's
// reload runs — there's no window where a stale (still-has-password) // reload runs. All config/state file access happens on this loop
// reload could clobber the save, since the two never race on // thread: `set_profile` commands from the bread bus are queued as
// different threads: everything here is sequential on this loop. // [`Wake::SetProfile`] and applied here (see the bottom of the
// loop), never on the subscription thread.
if let Ok(fresh) = Config::load() { if let Ok(fresh) = Config::load() {
cfg = fresh; cfg = fresh;
} }
@ -248,29 +286,43 @@ pub fn run(mut cfg: Config, run_initial: bool) -> i32 {
fail_streak = fail_streak.saturating_add(1); fail_streak = fail_streak.saturating_add(1);
} }
Health::UnknownProfile => { Health::UnknownProfile => {
// flow::run() already notifies + logs the "unknown profile" // flow::run() is quiet from here, so surface the misconfig
// critical error; re-running it here would just spam that on // ourselves — once per transition/change, not every tick.
// every tick, so only surface it once per transition.
if transition || profile_changed { if transition || profile_changed {
let _ = flow::run(&mut cfg, &profile); notify(
"breadcrumbs: unknown profile",
&format!("'{profile}' is not defined in breadcrumbs.toml"),
Urgency::Critical,
);
} }
fail_streak = fail_streak.saturating_add(1); fail_streak = fail_streak.saturating_add(1);
} }
Health::DownTailscaleManual => { Health::DownTailscaleManual => {
// Can't be auto-fixed (login / not installed). Notify once. // Can't be auto-fixed (login / install / exit-node config).
// Notify once per transition.
if transition { if transition {
notify( notify(
"Tailscale Error", "Tailscale Error",
"Tailscale needs manual attention (login / install). \ "Tailscale needs manual attention (login / install / \
Other Wi-Fi automation paused until resolved.", exit node config). Other Wi-Fi automation paused \
until resolved.",
Urgency::Critical, Urgency::Critical,
); );
} }
// Re-run flow only on transition so we land on the bootstrap net. // Re-attempt periodically and on the transition into this
if transition || profile_changed { // state: login may have completed since the last attempt, or
let _ = flow::run(&mut cfg, &profile); // the user may have missed the browser window. Quiet — a
// still-broken state must not re-notify on every retry.
let elapsed = last_flow_at.map(|t| t.elapsed().as_secs()).unwrap_or(u64::MAX);
if elapsed >= FLOW_COOLDOWN {
let outcome = flow::run_quiet(&mut cfg, &profile);
last_flow_at = Some(Instant::now());
fail_streak = if outcome.ok() {
0
} else {
fail_streak.saturating_add(1)
};
} }
fail_streak = fail_streak.saturating_add(1);
} }
Health::DownNoNet | Health::DownTailscaleOther => { Health::DownNoNet | Health::DownTailscaleOther => {
if transition { if transition {
@ -288,7 +340,7 @@ pub fn run(mut cfg: Config, run_initial: bool) -> i32 {
"watch: down ({:?}) profile={profile} ssid={:?} — running flow", "watch: down ({:?}) profile={profile} ssid={:?} — running flow",
health, ssid health, ssid
)); ));
let outcome = flow::run(&mut cfg, &profile); let outcome = flow::run_quiet(&mut cfg, &profile);
log(&format!("watch: recovery outcome = {:?}", outcome)); log(&format!("watch: recovery outcome = {:?}", outcome));
last_flow_at = Some(Instant::now()); last_flow_at = Some(Instant::now());
fail_streak = if outcome.ok() { fail_streak = if outcome.ok() {
@ -310,7 +362,12 @@ pub fn run(mut cfg: Config, run_initial: bool) -> i32 {
// Adaptive backoff: healthy -> base; failing -> grow up to ~6x. // Adaptive backoff: healthy -> base; failing -> grow up to ~6x.
let mult = 1 + fail_streak.min(5); let mult = 1 + fail_streak.min(5);
let dur = Duration::from_secs(base * mult as u64); let dur = Duration::from_secs(base * mult as u64);
wait_for_tick(&rx, dur); // Apply a queued set_profile on this thread — the single owner of
// config/state file access — and emit the confirmation. The next
// iteration's reload sees the new profile and recovers accordingly.
if let Some(Wake::SetProfile(name)) = wait_for_tick(&rx, dur) {
bread_events::apply_set_profile(&name);
}
} }
} }

View file

@ -317,7 +317,10 @@ fn install_service_no_enable_writes_valid_unit_file() {
let o = sb.cmd(&["install-service", "--no-enable"]); let o = sb.cmd(&["install-service", "--no-enable"]);
assert!(o.status.success(), "stderr: {}", stderr(&o)); assert!(o.status.success(), "stderr: {}", stderr(&o));
let unit_path = sb.root.join(".config/systemd/user/breadcrumbs.service"); // The sandbox sets XDG_CONFIG_HOME=$root/config, so the unit lands
// under $XDG_CONFIG_HOME/systemd/user — the whole point of the fix is
// honoring XDG rather than hardcoding ~/.config.
let unit_path = sb.root.join("config/systemd/user/breadcrumbs.service");
assert!(unit_path.exists()); assert!(unit_path.exists());
let text = fs::read_to_string(unit_path).unwrap(); let text = fs::read_to_string(unit_path).unwrap();
assert!(text.contains("ExecStart=")); assert!(text.contains("ExecStart="));
@ -423,6 +426,8 @@ case "$args" in
"device wifi rescan"*) ;; "device wifi rescan"*) ;;
"-t -f SSID device wifi list ifname wlan0") "-t -f SSID device wifi list ifname wlan0")
echo "TestNet" ;; echo "TestNet" ;;
"-t -f ACTIVE,SSID device wifi list ifname wlan0")
echo "yes:TestNet" ;;
"-t -f NAME,TYPE connection show") "-t -f NAME,TYPE connection show")
if [ -f "$marker" ]; then if [ -f "$marker" ]; then
echo "TestNet:802-11-wireless" echo "TestNet:802-11-wireless"

View file

@ -50,6 +50,7 @@ impl RecordedCall {
} }
type Matcher = Box<dyn Fn(&str, &[&str]) -> bool>; type Matcher = Box<dyn Fn(&str, &[&str]) -> bool>;
type DynamicRule = (Matcher, Box<dyn Fn(&str, &[&str]) -> Output>);
/// A canned, rule-based [`Runner`]. Rules are tried in registration order; /// A canned, rule-based [`Runner`]. Rules are tried in registration order;
/// the first whose matcher returns `true` supplies the response. No rule /// the first whose matcher returns `true` supplies the response. No rule
@ -58,6 +59,7 @@ type Matcher = Box<dyn Fn(&str, &[&str]) -> bool>;
/// (a wrong exit code) rather than silently returning success. /// (a wrong exit code) rather than silently returning success.
pub struct FakeRunner { pub struct FakeRunner {
rules: Vec<(Matcher, Output)>, rules: Vec<(Matcher, Output)>,
dynamic_rules: Vec<DynamicRule>,
commands: HashSet<String>, commands: HashSet<String>,
calls: Rc<RefCell<Vec<RecordedCall>>>, calls: Rc<RefCell<Vec<RecordedCall>>>,
} }
@ -66,6 +68,7 @@ impl FakeRunner {
pub fn new() -> Self { pub fn new() -> Self {
FakeRunner { FakeRunner {
rules: Vec::new(), rules: Vec::new(),
dynamic_rules: Vec::new(),
commands: HashSet::new(), commands: HashSet::new(),
calls: Rc::new(RefCell::new(Vec::new())), calls: Rc::new(RefCell::new(Vec::new())),
} }
@ -99,6 +102,20 @@ impl FakeRunner {
output, output,
) )
} }
/// Register a rule whose *response* is computed at call time (rather
/// than canned), enabling stateful fakes — e.g. answering "which SSID is
/// active?" with the SSID of the most recently dialed connection.
/// Dynamic rules are tried after the static ones.
pub fn on_dynamic(
mut self,
matcher: impl Fn(&str, &[&str]) -> bool + 'static,
out: impl Fn(&str, &[&str]) -> Output + 'static,
) -> Self {
self.dynamic_rules
.push((Box::new(matcher), Box::new(out)));
self
}
} }
impl Default for FakeRunner { impl Default for FakeRunner {
@ -119,6 +136,11 @@ impl Runner for FakeRunner {
return out.clone(); return out.clone();
} }
} }
for (matcher, out) in &self.dynamic_rules {
if matcher(prog, args) {
return out(prog, args);
}
}
Output::failed() Output::failed()
} }

View file

@ -52,16 +52,53 @@ fn base_config() -> Config {
/// the device reports connected after any successful connect attempt. /// the device reports connected after any successful connect attempt.
fn base_nm(visible_ssids: &[&str]) -> FakeRunner { fn base_nm(visible_ssids: &[&str]) -> FakeRunner {
let visible = visible_ssids.join("\n"); let visible = visible_ssids.join("\n");
FakeRunner::new() let runner = FakeRunner::new()
.on_contains("nmcli", "DEVICE,TYPE", ok("wlan0:wifi")) .on_contains("nmcli", "DEVICE,TYPE", ok("wlan0:wifi"))
.on_contains("nmcli", "radio wifi on", ok("")) .on_contains("nmcli", "radio wifi on", ok(""))
.on_contains("nmcli", "wifi rescan", ok("")) .on_contains("nmcli", "wifi rescan", ok(""))
.on_contains("nmcli", "-f SSID device wifi list", ok(&visible)) // Exact match: `-f ACTIVE,SSID` queries (which contain the substring
// "SSID device wifi list") must NOT be answered with the visible
// list — they go to the stateful rule below.
.on(
move |_prog, args| args.join(" ") == "-t -f SSID device wifi list ifname wlan0",
ok(&visible),
)
.on_contains("nmcli", "NAME,TYPE", ok("")) // no saved profiles .on_contains("nmcli", "NAME,TYPE", ok("")) // no saved profiles
.on_contains("nmcli", "GENERAL.CON-UUID", ok("uuid-1")) .on_contains("nmcli", "GENERAL.CON-UUID", ok("uuid-1"))
.on_contains("nmcli", "ipv4.ignore-auto-dns", ok("")) .on_contains("nmcli", "ipv4.ignore-auto-dns", ok(""))
.on_contains("nmcli", "device reapply", ok("")) .on_contains("nmcli", "device reapply", ok(""))
.on_contains("nmcli", "DEVICE,STATE", ok("wlan0:connected")) .on_contains("nmcli", "DEVICE,STATE", ok("wlan0:connected"));
let calls = runner.calls_handle();
runner.on_dynamic(
move |_prog, args| args.join(" ") == "-t -f ACTIVE,SSID device wifi list ifname wlan0",
move |_prog, _args| {
// Stateful: answer with the SSID of the most recently dialed
// connection, so connect_and_verify's post-connect SSID check
// sees the network that was just activated (bootstrap first,
// then the target).
let rec = calls.borrow();
let ssid = rec.iter().rev().find_map(|call| {
let j = call.args.join(" ");
if j.contains("connect") {
call.args
.iter()
.position(|a| a == "connect")
.map(|i| call.args[i + 1].clone())
} else if j.contains("connection up") {
call.args
.iter()
.position(|a| a == "up")
.map(|i| call.args[i + 1].clone())
} else {
None
}
});
match ssid {
Some(s) => ok(&format!("yes:{s}")),
None => ok(""),
}
},
)
} }
/// A successful `device wifi connect <ssid> ...` for every ssid in `ssids`. /// A successful `device wifi connect <ssid> ...` for every ssid in `ssids`.
@ -494,12 +531,17 @@ fn set_profile_command_persists_even_with_no_daemon_reachable() {
state::set_profile(&cfg, "away").unwrap(); state::set_profile(&cfg, "away").unwrap();
assert_eq!(State::load("away").profile, "away"); assert_eq!(State::load("away").profile, "away");
let acted = bread_events::handle_command(&command_event( // handle_command only parses/validates (no file I/O on the subscription
// thread); the loop thread then applies the action.
let action = bread_events::handle_command(&command_event(
"bread.command.crumbs.set_profile", "bread.command.crumbs.set_profile",
serde_json::json!({ "profile": "home" }), serde_json::json!({ "profile": "home" }),
)); ));
assert!(
assert!(acted, "known profile must persist"); matches!(action, bread_events::CommandAction::SetProfile(n) if n == "home"),
"a known profile must yield a SetProfile action"
);
bread_events::apply_set_profile("home");
assert_eq!(State::load("away").profile, "home"); assert_eq!(State::load("away").profile, "home");
} }
@ -509,12 +551,14 @@ fn set_profile_command_rejects_unknown_profile() {
let cfg = Config::load().expect("fresh config"); let cfg = Config::load().expect("fresh config");
state::set_profile(&cfg, "away").unwrap(); state::set_profile(&cfg, "away").unwrap();
let acted = bread_events::handle_command(&command_event( let action = bread_events::handle_command(&command_event(
"bread.command.crumbs.set_profile", "bread.command.crumbs.set_profile",
serde_json::json!({ "profile": "bogus" }), serde_json::json!({ "profile": "bogus" }),
)); ));
assert!(matches!(action, bread_events::CommandAction::SetProfile(n) if n == "bogus"));
assert!(!acted); // The rejection happens when the loop thread applies it: state is
// untouched and the failure event is emitted (a no-op without breadd).
bread_events::apply_set_profile("bogus");
assert_eq!( assert_eq!(
State::load("away").profile, State::load("away").profile,
"away", "away",
@ -528,12 +572,11 @@ fn set_profile_command_rejects_missing_profile_field() {
let cfg = Config::load().expect("fresh config"); let cfg = Config::load().expect("fresh config");
state::set_profile(&cfg, "away").unwrap(); state::set_profile(&cfg, "away").unwrap();
let acted = bread_events::handle_command(&command_event( let action = bread_events::handle_command(&command_event(
"bread.command.crumbs.set_profile", "bread.command.crumbs.set_profile",
serde_json::json!({}), serde_json::json!({}),
)); ));
assert!(matches!(action, bread_events::CommandAction::Ignore));
assert!(!acted);
assert_eq!(State::load("away").profile, "away"); assert_eq!(State::load("away").profile, "away");
} }
@ -543,12 +586,11 @@ fn handle_command_ignores_unrecognized_verb() {
let cfg = Config::load().expect("fresh config"); let cfg = Config::load().expect("fresh config");
state::set_profile(&cfg, "away").unwrap(); state::set_profile(&cfg, "away").unwrap();
let acted = bread_events::handle_command(&command_event( let action = bread_events::handle_command(&command_event(
"bread.command.crumbs.pin", "bread.command.crumbs.pin",
serde_json::json!({}), serde_json::json!({}),
)); ));
assert!(matches!(action, bread_events::CommandAction::Ignore));
assert!(!acted);
assert_eq!( assert_eq!(
State::load("away").profile, State::load("away").profile,
"away", "away",
@ -562,14 +604,20 @@ fn handle_command_ignores_events_outside_its_own_command_namespace() {
let cfg = Config::load().expect("fresh config"); let cfg = Config::load().expect("fresh config");
state::set_profile(&cfg, "away").unwrap(); state::set_profile(&cfg, "away").unwrap();
assert!(!bread_events::handle_command(&command_event( assert!(matches!(
bread_events::handle_command(&command_event(
"bread.command.clip.clear", "bread.command.clip.clear",
serde_json::json!({}), serde_json::json!({}),
))); )),
assert!(!bread_events::handle_command(&command_event( bread_events::CommandAction::Ignore
));
assert!(matches!(
bread_events::handle_command(&command_event(
"bread.crumbs.profile.changed", "bread.crumbs.profile.changed",
serde_json::json!({ "from": "away", "to": "home" }), serde_json::json!({ "from": "away", "to": "home" }),
))); )),
bread_events::CommandAction::Ignore
));
assert_eq!(State::load("away").profile, "away"); assert_eq!(State::load("away").profile, "away");
} }