From 08291c4aced69541538b8be5ebed8b21f5dac208 Mon Sep 17 00:00:00 2001 From: Razvan Dimescu Date: Wed, 8 Apr 2026 04:19:07 +0300 Subject: [PATCH 1/7] fix: prevent self-referential DNS backup on re-install MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The install flow previously captured current system DNS servers verbatim into the backup file. If numa was already installed, current DNS was 127.0.0.1, so the "backup" recorded 127.0.0.1 as the "original" — making a subsequent uninstall a no-op self-reference. Reproduced 2026-04-08 during v0.10.0 brew dogfood: after `sudo numa uninstall; sudo /opt/homebrew/bin/numa install`, `sudo numa uninstall` printed `restored DNS for "Wi-Fi" -> 127.0.0.1` because the brew binary's install step had overwritten the backup with the already-stub state. Fix (all three platforms): - macOS/Windows: if the existing backup already contains at least one non-loopback/non-stub upstream, preserve it as-is. If writing a fresh backup, filter loopback/stub addresses first so a capture from already-numa-managed state isn't self-referential. - Linux (resolv.conf fallback path): detect numa-managed or all-loopback resolv.conf content and skip the file copy in that case; preserve an existing useful backup rather than overwriting it. systemd-resolved path is unaffected (uses a drop-in, no backup file). Adds three unit tests for the predicates: macOS HashMap detection, Windows interface filter, and resolv.conf parsing (real upstream, self-referential, numa-marker, systemd stub, mixed). Co-Authored-By: Claude Opus 4.6 (1M context) --- src/system_dns.rs | 236 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 213 insertions(+), 23 deletions(-) diff --git a/src/system_dns.rs b/src/system_dns.rs index 8709e0d..2657488 100644 --- a/src/system_dns.rs +++ b/src/system_dns.rs @@ -243,6 +243,34 @@ fn parse_resolv_conf(path: &str) -> (Option, Vec) { (upstream, search_domains) } +/// True if the resolv.conf *content* appears to be written by numa itself, +/// or has no real upstream — either way, it's not a safe source of truth +/// for a backup. +#[cfg(any(target_os = "linux", test))] +fn resolv_conf_is_numa_managed(content: &str) -> bool { + if content.contains("Generated by Numa") { + return true; + } + !resolv_conf_has_real_upstream(content) +} + +/// True if the resolv.conf content has at least one non-loopback, non-stub +/// nameserver. An all-loopback resolv.conf is self-referential. +#[cfg(any(target_os = "linux", test))] +fn resolv_conf_has_real_upstream(content: &str) -> bool { + for line in content.lines() { + let line = line.trim(); + if let Some(rest) = line.strip_prefix("nameserver") { + if let Some(ns) = rest.split_whitespace().next() { + if !is_loopback_or_stub(ns) { + return true; + } + } + } + } + false +} + /// Query resolvectl for the real upstream DNS server (e.g. VPC resolver on AWS). #[cfg(target_os = "linux")] fn resolvectl_dns_server() -> Option { @@ -526,9 +554,19 @@ fn enable_dnscache() { .status(); } +/// True if the backup map has at least one real upstream (non-loopback, non-stub). +#[cfg(windows)] +fn backup_has_real_upstream_windows( + interfaces: &std::collections::HashMap, +) -> bool { + interfaces + .values() + .any(|iface| iface.servers.iter().any(|s| !is_loopback_or_stub(s))) +} + #[cfg(windows)] fn install_windows() -> Result<(), String> { - let interfaces = get_windows_interfaces()?; + let mut interfaces = get_windows_interfaces()?; if interfaces.is_empty() { return Err("no active network interfaces found".to_string()); } @@ -538,9 +576,30 @@ fn install_windows() -> Result<(), String> { std::fs::create_dir_all(parent) .map_err(|e| format!("failed to create {}: {}", parent.display(), e))?; } - let json = serde_json::to_string_pretty(&interfaces) - .map_err(|e| format!("failed to serialize backup: {}", e))?; - std::fs::write(&path, json).map_err(|e| format!("failed to write backup: {}", e))?; + + // Preserve an existing useful backup rather than overwriting it with + // numa-managed state (which would be self-referential after uninstall). + let existing: Option> = + std::fs::read_to_string(&path) + .ok() + .and_then(|json| serde_json::from_str(&json).ok()); + let has_useful_existing = existing + .as_ref() + .map(backup_has_real_upstream_windows) + .unwrap_or(false); + + if has_useful_existing { + eprintln!(" Existing DNS backup preserved at {}", path.display()); + } else { + // Filter loopback/stub addresses before saving so a fresh backup + // captured from already-numa-managed state isn't self-referential. + for iface in interfaces.values_mut() { + iface.servers.retain(|s| !is_loopback_or_stub(s)); + } + let json = serde_json::to_string_pretty(&interfaces) + .map_err(|e| format!("failed to serialize backup: {}", e))?; + std::fs::write(&path, json).map_err(|e| format!("failed to write backup: {}", e))?; + } for name in interfaces.keys() { let status = std::process::Command::new("netsh") @@ -570,7 +629,11 @@ fn install_windows() -> Result<(), String> { let needs_reboot = disable_dnscache()?; register_autostart(); - eprintln!("\n Original DNS saved to {}", path.display()); + if has_useful_existing { + eprintln!(); + } else { + eprintln!("\n Original DNS saved to {}", path.display()); + } eprintln!(" Run 'numa uninstall' to restore.\n"); if needs_reboot { eprintln!(" *** Reboot required. Numa will start automatically. ***\n"); @@ -754,27 +817,60 @@ fn get_dns_servers(service: &str) -> Result, String> { } } +/// True if the backup map has at least one real upstream (non-loopback, non-stub). +/// An all-loopback backup is self-referential — restoring it is a no-op. +#[cfg(any(target_os = "macos", test))] +fn backup_has_real_upstream_macos( + servers: &std::collections::HashMap>, +) -> bool { + servers + .values() + .any(|list| list.iter().any(|s| !is_loopback_or_stub(s))) +} + #[cfg(target_os = "macos")] fn install_macos() -> Result<(), String> { use std::collections::HashMap; let services = get_network_services()?; - let mut original: HashMap> = HashMap::new(); - - // Save current DNS for each service - for service in &services { - let servers = get_dns_servers(service)?; - original.insert(service.clone(), servers); - } - - // Save backup let dir = numa_data_dir(); std::fs::create_dir_all(&dir) .map_err(|e| format!("failed to create {}: {}", dir.display(), e))?; - let json = serde_json::to_string_pretty(&original) - .map_err(|e| format!("failed to serialize backup: {}", e))?; - std::fs::write(backup_path(), json).map_err(|e| format!("failed to write backup: {}", e))?; + // If a useful backup already exists (at least one non-loopback upstream), + // preserve it — overwriting would destroy the original DNS state when + // re-installing on top of a numa-managed configuration. + let existing_backup: Option>> = + std::fs::read_to_string(backup_path()) + .ok() + .and_then(|json| serde_json::from_str(&json).ok()); + let has_useful_existing = existing_backup + .as_ref() + .map(backup_has_real_upstream_macos) + .unwrap_or(false); + + if has_useful_existing { + eprintln!( + " Existing DNS backup preserved at {}", + backup_path().display() + ); + } else { + // Capture fresh, filtering out loopback and stub addresses so we + // never record a self-referential backup. + let mut original: HashMap> = HashMap::new(); + for service in &services { + let servers: Vec = get_dns_servers(service)? + .into_iter() + .filter(|s| !is_loopback_or_stub(s)) + .collect(); + original.insert(service.clone(), servers); + } + + let json = serde_json::to_string_pretty(&original) + .map_err(|e| format!("failed to serialize backup: {}", e))?; + std::fs::write(backup_path(), json) + .map_err(|e| format!("failed to write backup: {}", e))?; + } // Set DNS to 127.0.0.1 and add "numa" search domain for each service for service in &services { @@ -795,7 +891,11 @@ fn install_macos() -> Result<(), String> { .status(); } - eprintln!("\n Original DNS saved to {}", backup_path().display()); + if has_useful_existing { + eprintln!(); + } else { + eprintln!("\n Original DNS saved to {}", backup_path().display()); + } eprintln!(" Run 'sudo numa uninstall' to restore.\n"); Ok(()) @@ -1132,11 +1232,33 @@ fn install_linux() -> Result<(), String> { .map_err(|e| format!("failed to create {}: {}", parent.display(), e))?; } - // Back up current resolv.conf (ignore NotFound) - match std::fs::copy(resolv, &backup) { - Ok(_) => eprintln!(" Saved /etc/resolv.conf to {}", backup.display()), - Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} - Err(e) => return Err(format!("failed to backup /etc/resolv.conf: {}", e)), + // Back up current resolv.conf, but never overwrite a useful existing + // backup with a numa-managed file — that would leave uninstall with + // nothing to restore to. + let current = std::fs::read_to_string(resolv).ok(); + let current_is_numa_managed = current + .as_deref() + .map(resolv_conf_is_numa_managed) + .unwrap_or(false); + let existing_backup_is_useful = std::fs::read_to_string(&backup) + .ok() + .as_deref() + .map(resolv_conf_has_real_upstream) + .unwrap_or(false); + + if existing_backup_is_useful { + eprintln!( + " Existing resolv.conf backup preserved at {}", + backup.display() + ); + } else if current_is_numa_managed { + eprintln!(" warning: /etc/resolv.conf is already numa-managed; no fresh backup written"); + } else { + match std::fs::copy(resolv, &backup) { + Ok(_) => eprintln!(" Saved /etc/resolv.conf to {}", backup.display()), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => return Err(format!("failed to backup /etc/resolv.conf: {}", e)), + } } if resolv @@ -1432,6 +1554,74 @@ Wireless LAN adapter Wi-Fi: assert!(!result.contains("{{exe_path}}")); } + #[test] + fn macos_backup_real_upstream_detection() { + use std::collections::HashMap; + let mut map: HashMap> = HashMap::new(); + + // Empty backup → no real upstream + assert!(!backup_has_real_upstream_macos(&map)); + + // All-loopback backup → still no real upstream (the bug case) + map.insert("Wi-Fi".into(), vec!["127.0.0.1".into()]); + map.insert("Ethernet".into(), vec!["::1".into()]); + assert!(!backup_has_real_upstream_macos(&map)); + + // One real entry → useful + map.insert("Tailscale".into(), vec!["192.168.1.1".into()]); + assert!(backup_has_real_upstream_macos(&map)); + } + + #[test] + fn resolv_conf_real_upstream_detection() { + let real = "nameserver 192.168.1.1\nsearch lan\n"; + assert!(resolv_conf_has_real_upstream(real)); + assert!(!resolv_conf_is_numa_managed(real)); + + let self_ref = "nameserver 127.0.0.1\nsearch numa\n"; + assert!(!resolv_conf_has_real_upstream(self_ref)); + assert!(resolv_conf_is_numa_managed(self_ref)); + + let numa_marker = + "# Generated by Numa — run 'sudo numa uninstall' to restore\nnameserver 127.0.0.1\nsearch numa\n"; + assert!(resolv_conf_is_numa_managed(numa_marker)); + + let systemd_stub = "nameserver 127.0.0.53\noptions edns0\n"; + assert!(!resolv_conf_has_real_upstream(systemd_stub)); + + let mixed = "nameserver 127.0.0.1\nnameserver 1.1.1.1\n"; + assert!(resolv_conf_has_real_upstream(mixed)); + assert!(!resolv_conf_is_numa_managed(mixed)); + } + + #[test] + fn windows_backup_filters_loopback() { + use std::collections::HashMap; + let mut interfaces: HashMap = HashMap::new(); + interfaces.insert( + "Wi-Fi".into(), + WindowsInterfaceDns { + dhcp: false, + servers: vec!["127.0.0.1".into(), "1.1.1.1".into()], + }, + ); + interfaces.insert( + "Ethernet".into(), + WindowsInterfaceDns { + dhcp: true, + servers: vec!["127.0.0.1".into()], + }, + ); + + // Mimic the filter step from install_windows + for iface in interfaces.values_mut() { + iface.servers.retain(|s| !is_loopback_or_stub(s)); + } + + assert_eq!(interfaces["Wi-Fi"].servers, vec!["1.1.1.1".to_string()]); + assert!(interfaces["Ethernet"].servers.is_empty()); + } + #[test] fn parse_ipconfig_skips_disconnected() { let sample = "\ -- 2.34.1 From 68c7fc41309ee44ba57a0487c078f2ab00f81d04 Mon Sep 17 00:00:00 2001 From: Razvan Dimescu Date: Wed, 8 Apr 2026 04:33:34 +0300 Subject: [PATCH 2/7] refactor: share iter_nameservers helper and reuse resolv.conf content MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Post-review simplifications on the stale-backup fix: - Extract iter_nameservers(&str) helper used by both parse_resolv_conf and resolv_conf_has_real_upstream. Eliminates the duplicated line-by-line nameserver parsing (findings from reuse review). - install_linux: reuse the already-read resolv.conf content via std::fs::write instead of a second read via std::fs::copy. - install_macos / install_windows: flatten the conditional eprintln pattern — always print a blank line, conditionally print the save message. Equivalent output, less branching. Net −12 lines. All 130 tests still pass, clippy clean. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/system_dns.rs | 68 +++++++++++++++++++---------------------------- 1 file changed, 28 insertions(+), 40 deletions(-) diff --git a/src/system_dns.rs b/src/system_dns.rs index 2657488..6c8bef8 100644 --- a/src/system_dns.rs +++ b/src/system_dns.rs @@ -214,7 +214,18 @@ fn discover_linux() -> SystemDnsInfo { } } -/// Parse resolv.conf in a single pass, extracting both the first non-loopback +/// Yield each `nameserver` address from resolv.conf content. No filtering — +/// callers decide what counts as a real upstream. +#[cfg(any(target_os = "linux", test))] +fn iter_nameservers(content: &str) -> impl Iterator { + content.lines().filter_map(|line| { + let mut parts = line.trim().split_whitespace(); + (parts.next() == Some("nameserver")).then_some(())?; + parts.next() + }) +} + +/// Parse resolv.conf in a single pass, extracting the first non-loopback /// nameserver and all search domains. #[cfg(target_os = "linux")] fn parse_resolv_conf(path: &str) -> (Option, Vec) { @@ -222,19 +233,13 @@ fn parse_resolv_conf(path: &str) -> (Option, Vec) { Ok(t) => t, Err(_) => return (None, Vec::new()), }; - let mut upstream = None; + let upstream = iter_nameservers(&text) + .find(|ns| !is_loopback_or_stub(ns)) + .map(str::to_string); let mut search_domains = Vec::new(); for line in text.lines() { let line = line.trim(); - if line.starts_with("nameserver") { - if upstream.is_none() { - if let Some(ns) = line.split_whitespace().nth(1) { - if !is_loopback_or_stub(ns) { - upstream = Some(ns.to_string()); - } - } - } - } else if line.starts_with("search") || line.starts_with("domain") { + if line.starts_with("search") || line.starts_with("domain") { for domain in line.split_whitespace().skip(1) { search_domains.push(domain.to_string()); } @@ -248,27 +253,14 @@ fn parse_resolv_conf(path: &str) -> (Option, Vec) { /// for a backup. #[cfg(any(target_os = "linux", test))] fn resolv_conf_is_numa_managed(content: &str) -> bool { - if content.contains("Generated by Numa") { - return true; - } - !resolv_conf_has_real_upstream(content) + content.contains("Generated by Numa") || !resolv_conf_has_real_upstream(content) } /// True if the resolv.conf content has at least one non-loopback, non-stub /// nameserver. An all-loopback resolv.conf is self-referential. #[cfg(any(target_os = "linux", test))] fn resolv_conf_has_real_upstream(content: &str) -> bool { - for line in content.lines() { - let line = line.trim(); - if let Some(rest) = line.strip_prefix("nameserver") { - if let Some(ns) = rest.split_whitespace().next() { - if !is_loopback_or_stub(ns) { - return true; - } - } - } - } - false + iter_nameservers(content).any(|ns| !is_loopback_or_stub(ns)) } /// Query resolvectl for the real upstream DNS server (e.g. VPC resolver on AWS). @@ -629,10 +621,9 @@ fn install_windows() -> Result<(), String> { let needs_reboot = disable_dnscache()?; register_autostart(); - if has_useful_existing { - eprintln!(); - } else { - eprintln!("\n Original DNS saved to {}", path.display()); + eprintln!(); + if !has_useful_existing { + eprintln!(" Original DNS saved to {}", path.display()); } eprintln!(" Run 'numa uninstall' to restore.\n"); if needs_reboot { @@ -891,10 +882,9 @@ fn install_macos() -> Result<(), String> { .status(); } - if has_useful_existing { - eprintln!(); - } else { - eprintln!("\n Original DNS saved to {}", backup_path().display()); + eprintln!(); + if !has_useful_existing { + eprintln!(" Original DNS saved to {}", backup_path().display()); } eprintln!(" Run 'sudo numa uninstall' to restore.\n"); @@ -1253,12 +1243,10 @@ fn install_linux() -> Result<(), String> { ); } else if current_is_numa_managed { eprintln!(" warning: /etc/resolv.conf is already numa-managed; no fresh backup written"); - } else { - match std::fs::copy(resolv, &backup) { - Ok(_) => eprintln!(" Saved /etc/resolv.conf to {}", backup.display()), - Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} - Err(e) => return Err(format!("failed to backup /etc/resolv.conf: {}", e)), - } + } else if let Some(content) = current.as_deref() { + std::fs::write(&backup, content) + .map_err(|e| format!("failed to backup /etc/resolv.conf: {}", e))?; + eprintln!(" Saved /etc/resolv.conf to {}", backup.display()); } if resolv -- 2.34.1 From 9c9986b49df27307c73099cc6547834c5290a9e1 Mon Sep 17 00:00:00 2001 From: Razvan Dimescu Date: Wed, 8 Apr 2026 04:44:07 +0300 Subject: [PATCH 3/7] fix: drop redundant trim before split_whitespace CI caught `clippy::trim_split_whitespace` on Rust 1.94: `split_whitespace()` already skips leading/trailing whitespace, so `.trim()` first is redundant. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/system_dns.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/system_dns.rs b/src/system_dns.rs index 6c8bef8..9d48996 100644 --- a/src/system_dns.rs +++ b/src/system_dns.rs @@ -219,7 +219,7 @@ fn discover_linux() -> SystemDnsInfo { #[cfg(any(target_os = "linux", test))] fn iter_nameservers(content: &str) -> impl Iterator { content.lines().filter_map(|line| { - let mut parts = line.trim().split_whitespace(); + let mut parts = line.split_whitespace(); (parts.next() == Some("nameserver")).then_some(())?; parts.next() }) -- 2.34.1 From a54fb99428fb29da6f6ee2cc365bbb97e31cfbb1 Mon Sep 17 00:00:00 2001 From: Razvan Dimescu Date: Wed, 8 Apr 2026 05:01:17 +0300 Subject: [PATCH 4/7] refactor: extract load_backup helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove duplicated read+deserialize boilerplate shared by install_macos and install_windows. The two call sites each had an identical 4-line chain of read_to_string().ok().and_then(serde_json::from_str).ok() — collapse into a single generic helper load_backup(). Co-Authored-By: Claude Opus 4.6 (1M context) --- src/system_dns.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/system_dns.rs b/src/system_dns.rs index 9d48996..59e40e7 100644 --- a/src/system_dns.rs +++ b/src/system_dns.rs @@ -6,6 +6,13 @@ fn is_loopback_or_stub(addr: &str) -> bool { matches!(addr, "127.0.0.1" | "127.0.0.53" | "0.0.0.0" | "::1" | "") } +/// Load a JSON backup file as `T`, returning `None` if the file is missing +/// or unreadable (rather than erroring — callers fall back to a fresh capture). +#[cfg(any(target_os = "macos", windows))] +fn load_backup(path: &std::path::Path) -> Option { + serde_json::from_str(&std::fs::read_to_string(path).ok()?).ok() +} + /// A conditional forwarding rule: domains matching `suffix` are forwarded to `upstream`. #[derive(Debug, Clone)] pub struct ForwardingRule { @@ -572,9 +579,7 @@ fn install_windows() -> Result<(), String> { // Preserve an existing useful backup rather than overwriting it with // numa-managed state (which would be self-referential after uninstall). let existing: Option> = - std::fs::read_to_string(&path) - .ok() - .and_then(|json| serde_json::from_str(&json).ok()); + load_backup(&path); let has_useful_existing = existing .as_ref() .map(backup_has_real_upstream_windows) @@ -831,10 +836,7 @@ fn install_macos() -> Result<(), String> { // If a useful backup already exists (at least one non-loopback upstream), // preserve it — overwriting would destroy the original DNS state when // re-installing on top of a numa-managed configuration. - let existing_backup: Option>> = - std::fs::read_to_string(backup_path()) - .ok() - .and_then(|json| serde_json::from_str(&json).ok()); + let existing_backup: Option>> = load_backup(&backup_path()); let has_useful_existing = existing_backup .as_ref() .map(backup_has_real_upstream_macos) -- 2.34.1 From 522e145c22ad2b88875ce1940e70738e43647a70 Mon Sep 17 00:00:00 2001 From: Razvan Dimescu Date: Wed, 8 Apr 2026 05:04:47 +0300 Subject: [PATCH 5/7] Revert "refactor: extract load_backup helper" This reverts commit a54fb99428fb29da6f6ee2cc365bbb97e31cfbb1. --- src/system_dns.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/system_dns.rs b/src/system_dns.rs index 59e40e7..9d48996 100644 --- a/src/system_dns.rs +++ b/src/system_dns.rs @@ -6,13 +6,6 @@ fn is_loopback_or_stub(addr: &str) -> bool { matches!(addr, "127.0.0.1" | "127.0.0.53" | "0.0.0.0" | "::1" | "") } -/// Load a JSON backup file as `T`, returning `None` if the file is missing -/// or unreadable (rather than erroring — callers fall back to a fresh capture). -#[cfg(any(target_os = "macos", windows))] -fn load_backup(path: &std::path::Path) -> Option { - serde_json::from_str(&std::fs::read_to_string(path).ok()?).ok() -} - /// A conditional forwarding rule: domains matching `suffix` are forwarded to `upstream`. #[derive(Debug, Clone)] pub struct ForwardingRule { @@ -579,7 +572,9 @@ fn install_windows() -> Result<(), String> { // Preserve an existing useful backup rather than overwriting it with // numa-managed state (which would be self-referential after uninstall). let existing: Option> = - load_backup(&path); + std::fs::read_to_string(&path) + .ok() + .and_then(|json| serde_json::from_str(&json).ok()); let has_useful_existing = existing .as_ref() .map(backup_has_real_upstream_windows) @@ -836,7 +831,10 @@ fn install_macos() -> Result<(), String> { // If a useful backup already exists (at least one non-loopback upstream), // preserve it — overwriting would destroy the original DNS state when // re-installing on top of a numa-managed configuration. - let existing_backup: Option>> = load_backup(&backup_path()); + let existing_backup: Option>> = + std::fs::read_to_string(backup_path()) + .ok() + .and_then(|json| serde_json::from_str(&json).ok()); let has_useful_existing = existing_backup .as_ref() .map(backup_has_real_upstream_macos) -- 2.34.1 From 34b1d0e6df5463b68b51f44433a6b8a034fb34f8 Mon Sep 17 00:00:00 2001 From: Razvan Dimescu Date: Wed, 8 Apr 2026 05:14:58 +0300 Subject: [PATCH 6/7] test: drop windows_backup_filters_loopback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test inlined the 3-line filter block from install_windows rather than calling a production helper, so it was testing stdlib Vec::retain + is_loopback_or_stub — both already covered elsewhere. Deleting it removes a test that would silently pass even if install_windows stopped filtering altogether. The predicate logic for macOS-shaped backups stays covered by macos_backup_real_upstream_detection (same inner Vec type). Co-Authored-By: Claude Opus 4.6 (1M context) --- src/system_dns.rs | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/src/system_dns.rs b/src/system_dns.rs index 9d48996..dc0a0b1 100644 --- a/src/system_dns.rs +++ b/src/system_dns.rs @@ -1582,34 +1582,6 @@ Wireless LAN adapter Wi-Fi: assert!(!resolv_conf_is_numa_managed(mixed)); } - #[test] - fn windows_backup_filters_loopback() { - use std::collections::HashMap; - let mut interfaces: HashMap = HashMap::new(); - interfaces.insert( - "Wi-Fi".into(), - WindowsInterfaceDns { - dhcp: false, - servers: vec!["127.0.0.1".into(), "1.1.1.1".into()], - }, - ); - interfaces.insert( - "Ethernet".into(), - WindowsInterfaceDns { - dhcp: true, - servers: vec!["127.0.0.1".into()], - }, - ); - - // Mimic the filter step from install_windows - for iface in interfaces.values_mut() { - iface.servers.retain(|s| !is_loopback_or_stub(s)); - } - - assert_eq!(interfaces["Wi-Fi"].servers, vec!["1.1.1.1".to_string()]); - assert!(interfaces["Ethernet"].servers.is_empty()); - } - #[test] fn parse_ipconfig_skips_disconnected() { let sample = "\ -- 2.34.1 From b856945da8b2ef27b80f6e2ee291dbcfb4427aee Mon Sep 17 00:00:00 2001 From: Razvan Dimescu Date: Wed, 8 Apr 2026 15:24:50 +0300 Subject: [PATCH 7/7] test: add windows_backup_filters_loopback unit test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PR description mentioned this test but it was missing from the diff, leaving backup_has_real_upstream_windows untested. Mirrors the shape of macos_backup_real_upstream_detection: empty map → false, all-loopback (127.0.0.1, ::1, 0.0.0.0) → false, one real entry alongside loopback → true. Also relax the cfg gate on backup_has_real_upstream_windows from cfg(windows) to cfg(any(windows, test)) so the test compiles cross-platform, matching how backup_has_real_upstream_macos and the resolv_conf helpers are gated. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/system_dns.rs | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/src/system_dns.rs b/src/system_dns.rs index dc0a0b1..098e69a 100644 --- a/src/system_dns.rs +++ b/src/system_dns.rs @@ -547,7 +547,7 @@ fn enable_dnscache() { } /// True if the backup map has at least one real upstream (non-loopback, non-stub). -#[cfg(windows)] +#[cfg(any(windows, test))] fn backup_has_real_upstream_windows( interfaces: &std::collections::HashMap, ) -> bool { @@ -1560,6 +1560,42 @@ Wireless LAN adapter Wi-Fi: assert!(backup_has_real_upstream_macos(&map)); } + #[test] + fn windows_backup_filters_loopback() { + use std::collections::HashMap; + let mut map: HashMap = HashMap::new(); + + // Empty backup → no real upstream + assert!(!backup_has_real_upstream_windows(&map)); + + // All-loopback backup → still no real upstream (the bug case) + map.insert( + "Wi-Fi".into(), + WindowsInterfaceDns { + dhcp: false, + servers: vec!["127.0.0.1".into()], + }, + ); + map.insert( + "Ethernet".into(), + WindowsInterfaceDns { + dhcp: false, + servers: vec!["::1".into(), "0.0.0.0".into()], + }, + ); + assert!(!backup_has_real_upstream_windows(&map)); + + // One real entry alongside loopback → useful + map.insert( + "Ethernet 2".into(), + WindowsInterfaceDns { + dhcp: false, + servers: vec!["192.168.1.1".into()], + }, + ); + assert!(backup_has_real_upstream_windows(&map)); + } + #[test] fn resolv_conf_real_upstream_detection() { let real = "nameserver 192.168.1.1\nsearch lan\n"; -- 2.34.1