fix: prevent self-referential DNS backup on re-install
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) <noreply@anthropic.com>
This commit is contained in:
@@ -243,6 +243,34 @@ fn parse_resolv_conf(path: &str) -> (Option<String>, Vec<String>) {
|
|||||||
(upstream, search_domains)
|
(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).
|
/// Query resolvectl for the real upstream DNS server (e.g. VPC resolver on AWS).
|
||||||
#[cfg(target_os = "linux")]
|
#[cfg(target_os = "linux")]
|
||||||
fn resolvectl_dns_server() -> Option<String> {
|
fn resolvectl_dns_server() -> Option<String> {
|
||||||
@@ -526,9 +554,19 @@ fn enable_dnscache() {
|
|||||||
.status();
|
.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<String, WindowsInterfaceDns>,
|
||||||
|
) -> bool {
|
||||||
|
interfaces
|
||||||
|
.values()
|
||||||
|
.any(|iface| iface.servers.iter().any(|s| !is_loopback_or_stub(s)))
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(windows)]
|
#[cfg(windows)]
|
||||||
fn install_windows() -> Result<(), String> {
|
fn install_windows() -> Result<(), String> {
|
||||||
let interfaces = get_windows_interfaces()?;
|
let mut interfaces = get_windows_interfaces()?;
|
||||||
if interfaces.is_empty() {
|
if interfaces.is_empty() {
|
||||||
return Err("no active network interfaces found".to_string());
|
return Err("no active network interfaces found".to_string());
|
||||||
}
|
}
|
||||||
@@ -538,9 +576,30 @@ fn install_windows() -> Result<(), String> {
|
|||||||
std::fs::create_dir_all(parent)
|
std::fs::create_dir_all(parent)
|
||||||
.map_err(|e| format!("failed to create {}: {}", parent.display(), e))?;
|
.map_err(|e| format!("failed to create {}: {}", parent.display(), 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::collections::HashMap<String, WindowsInterfaceDns>> =
|
||||||
|
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)
|
let json = serde_json::to_string_pretty(&interfaces)
|
||||||
.map_err(|e| format!("failed to serialize backup: {}", e))?;
|
.map_err(|e| format!("failed to serialize backup: {}", e))?;
|
||||||
std::fs::write(&path, json).map_err(|e| format!("failed to write backup: {}", e))?;
|
std::fs::write(&path, json).map_err(|e| format!("failed to write backup: {}", e))?;
|
||||||
|
}
|
||||||
|
|
||||||
for name in interfaces.keys() {
|
for name in interfaces.keys() {
|
||||||
let status = std::process::Command::new("netsh")
|
let status = std::process::Command::new("netsh")
|
||||||
@@ -570,7 +629,11 @@ fn install_windows() -> Result<(), String> {
|
|||||||
let needs_reboot = disable_dnscache()?;
|
let needs_reboot = disable_dnscache()?;
|
||||||
register_autostart();
|
register_autostart();
|
||||||
|
|
||||||
|
if has_useful_existing {
|
||||||
|
eprintln!();
|
||||||
|
} else {
|
||||||
eprintln!("\n Original DNS saved to {}", path.display());
|
eprintln!("\n Original DNS saved to {}", path.display());
|
||||||
|
}
|
||||||
eprintln!(" Run 'numa uninstall' to restore.\n");
|
eprintln!(" Run 'numa uninstall' to restore.\n");
|
||||||
if needs_reboot {
|
if needs_reboot {
|
||||||
eprintln!(" *** Reboot required. Numa will start automatically. ***\n");
|
eprintln!(" *** Reboot required. Numa will start automatically. ***\n");
|
||||||
@@ -754,27 +817,60 @@ fn get_dns_servers(service: &str) -> Result<Vec<String>, 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<String, Vec<String>>,
|
||||||
|
) -> bool {
|
||||||
|
servers
|
||||||
|
.values()
|
||||||
|
.any(|list| list.iter().any(|s| !is_loopback_or_stub(s)))
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(target_os = "macos")]
|
#[cfg(target_os = "macos")]
|
||||||
fn install_macos() -> Result<(), String> {
|
fn install_macos() -> Result<(), String> {
|
||||||
use std::collections::HashMap;
|
use std::collections::HashMap;
|
||||||
|
|
||||||
let services = get_network_services()?;
|
let services = get_network_services()?;
|
||||||
let mut original: HashMap<String, Vec<String>> = 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();
|
let dir = numa_data_dir();
|
||||||
std::fs::create_dir_all(&dir)
|
std::fs::create_dir_all(&dir)
|
||||||
.map_err(|e| format!("failed to create {}: {}", dir.display(), e))?;
|
.map_err(|e| format!("failed to create {}: {}", dir.display(), 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<HashMap<String, Vec<String>>> =
|
||||||
|
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<String, Vec<String>> = HashMap::new();
|
||||||
|
for service in &services {
|
||||||
|
let servers: Vec<String> = 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)
|
let json = serde_json::to_string_pretty(&original)
|
||||||
.map_err(|e| format!("failed to serialize backup: {}", e))?;
|
.map_err(|e| format!("failed to serialize backup: {}", e))?;
|
||||||
std::fs::write(backup_path(), json).map_err(|e| format!("failed to write 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
|
// Set DNS to 127.0.0.1 and add "numa" search domain for each service
|
||||||
for service in &services {
|
for service in &services {
|
||||||
@@ -795,7 +891,11 @@ fn install_macos() -> Result<(), String> {
|
|||||||
.status();
|
.status();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if has_useful_existing {
|
||||||
|
eprintln!();
|
||||||
|
} else {
|
||||||
eprintln!("\n Original DNS saved to {}", backup_path().display());
|
eprintln!("\n Original DNS saved to {}", backup_path().display());
|
||||||
|
}
|
||||||
eprintln!(" Run 'sudo numa uninstall' to restore.\n");
|
eprintln!(" Run 'sudo numa uninstall' to restore.\n");
|
||||||
|
|
||||||
Ok(())
|
Ok(())
|
||||||
@@ -1132,12 +1232,34 @@ fn install_linux() -> Result<(), String> {
|
|||||||
.map_err(|e| format!("failed to create {}: {}", parent.display(), e))?;
|
.map_err(|e| format!("failed to create {}: {}", parent.display(), e))?;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Back up current resolv.conf (ignore NotFound)
|
// 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) {
|
match std::fs::copy(resolv, &backup) {
|
||||||
Ok(_) => eprintln!(" Saved /etc/resolv.conf to {}", backup.display()),
|
Ok(_) => eprintln!(" Saved /etc/resolv.conf to {}", backup.display()),
|
||||||
Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}
|
Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}
|
||||||
Err(e) => return Err(format!("failed to backup /etc/resolv.conf: {}", e)),
|
Err(e) => return Err(format!("failed to backup /etc/resolv.conf: {}", e)),
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if resolv
|
if resolv
|
||||||
.symlink_metadata()
|
.symlink_metadata()
|
||||||
@@ -1432,6 +1554,74 @@ Wireless LAN adapter Wi-Fi:
|
|||||||
assert!(!result.contains("{{exe_path}}"));
|
assert!(!result.contains("{{exe_path}}"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn macos_backup_real_upstream_detection() {
|
||||||
|
use std::collections::HashMap;
|
||||||
|
let mut map: HashMap<String, Vec<String>> = 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<String, WindowsInterfaceDns> = 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]
|
#[test]
|
||||||
fn parse_ipconfig_skips_disconnected() {
|
fn parse_ipconfig_skips_disconnected() {
|
||||||
let sample = "\
|
let sample = "\
|
||||||
|
|||||||
Reference in New Issue
Block a user