From 688d88345ffda554d756ce3e73eb688d7f23b9ef Mon Sep 17 00:00:00 2001 From: Omid Rade-PC Date: Sun, 8 Nov 2026 11:53:46 +0330 Subject: [PATCH 1/5] fix: verify network release and retain incomplete proxy handoffs --- AGENTS.md | 15 +++ Cargo.lock | 30 ++--- Cargo.toml | 2 +- apps/desktop/package.json | 2 +- crates/iran-split-core/src/lib.rs | 125 ++++++++++++++++-- crates/iran-split-core/src/proxy_restore.rs | 53 +++++++- .../src/system_proxy.rs | 6 +- .../src/system_proxy.rs | 6 +- .../src/system_proxy.rs | 6 +- docs/adr/0133-verified-network-restoration.md | 54 ++++++++ docs/adr/README.md | 1 + e2e/primary-flows.spec.ts | 2 + package.json | 2 +- src-tauri/src/environment/apps.rs | 11 +- src-tauri/tauri.conf.json | 2 +- version | 2 +- 16 files changed, 280 insertions(+), 39 deletions(-) create mode 100644 docs/adr/0133-verified-network-restoration.md diff --git a/AGENTS.md b/AGENTS.md index 88d1fdc..f92e083 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -71,6 +71,21 @@ If a required command fails or emits a warning from project code, fix it in the ## Lessons +- An unavailable original proxy client is an incomplete handoff, not a safe + reason to delete its baseline. Keep the record, return a cleanup error, and + allow Disconnect/Quit to retry after the client returns; never re-enable a + dead proxy. Verify the owned process as well as the TUN before handoff, even + when the helper reports cleanup complete. Expose pending restoration in Error + diagnostics too (ADR 0133, 6.2.85). +- A queued retry initially retains the previous Error phase. Phase waiters must + wait for lifecycle work to become idle before accepting a terminal phase or + error; otherwise Restart/Disconnect immediately returns the old failure while + its retry is still queued. Cover the actual Stop retry, not only direct + shutdown, in cleanup regressions (ADR 0133). +- A missing Playwright download is not an e2e failure in application behavior. + Install its pinned browser or select the installed test browser explicitly + through `BIFLOW_CHROMIUM_PATH`, then rerun the failed test. + - A native DMG fixture must select its filesystem and use a unique volume name; do not depend on a runner's image-creation defaults. Preserve hdiutil exit status and stderr in fixture assertions. A generic attach assertion hid the diff --git a/Cargo.lock b/Cargo.lock index 31614c6..190d0f7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2046,7 +2046,7 @@ dependencies = [ [[package]] name = "iran-split-cli" -version = "6.2.84" +version = "6.2.85" dependencies = [ "async-trait", "chrono", @@ -2064,7 +2064,7 @@ dependencies = [ [[package]] name = "iran-split-clients" -version = "6.2.84" +version = "6.2.85" dependencies = [ "async-trait", "ipnet", @@ -2080,7 +2080,7 @@ dependencies = [ [[package]] name = "iran-split-config" -version = "6.2.84" +version = "6.2.85" dependencies = [ "hex", "rand 0.9.5", @@ -2093,7 +2093,7 @@ dependencies = [ [[package]] name = "iran-split-core" -version = "6.2.84" +version = "6.2.85" dependencies = [ "async-trait", "chrono", @@ -2110,7 +2110,7 @@ dependencies = [ [[package]] name = "iran-split-desktop" -version = "6.2.84" +version = "6.2.85" dependencies = [ "chrono", "dirs 6.0.0", @@ -2152,7 +2152,7 @@ dependencies = [ [[package]] name = "iran-split-helper" -version = "6.2.84" +version = "6.2.85" dependencies = [ "clap", "hex", @@ -2177,7 +2177,7 @@ dependencies = [ [[package]] name = "iran-split-helper-winacl" -version = "6.2.84" +version = "6.2.85" dependencies = [ "tokio", "windows", @@ -2185,7 +2185,7 @@ dependencies = [ [[package]] name = "iran-split-ipc" -version = "6.2.84" +version = "6.2.85" dependencies = [ "bytes", "chrono", @@ -2198,7 +2198,7 @@ dependencies = [ [[package]] name = "iran-split-mihomo" -version = "6.2.84" +version = "6.2.85" dependencies = [ "chrono", "futures-util", @@ -2221,7 +2221,7 @@ dependencies = [ [[package]] name = "iran-split-netstats" -version = "6.2.84" +version = "6.2.85" dependencies = [ "libc", "tempfile", @@ -2230,7 +2230,7 @@ dependencies = [ [[package]] name = "iran-split-platform-linux" -version = "6.2.84" +version = "6.2.85" dependencies = [ "async-trait", "chrono", @@ -2254,7 +2254,7 @@ dependencies = [ [[package]] name = "iran-split-platform-macos" -version = "6.2.84" +version = "6.2.85" dependencies = [ "async-trait", "chrono", @@ -2278,7 +2278,7 @@ dependencies = [ [[package]] name = "iran-split-platform-win" -version = "6.2.84" +version = "6.2.85" dependencies = [ "async-trait", "chrono", @@ -2301,7 +2301,7 @@ dependencies = [ [[package]] name = "iran-split-rules" -version = "6.2.84" +version = "6.2.85" dependencies = [ "async-trait", "chrono", @@ -2323,7 +2323,7 @@ dependencies = [ [[package]] name = "iran-split-taskbar" -version = "6.2.84" +version = "6.2.85" dependencies = [ "iran-split-config", "objc2", diff --git a/Cargo.toml b/Cargo.toml index bce8499..f1d4a96 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,7 +19,7 @@ members = [ ] [workspace.package] -version = "6.2.84" +version = "6.2.85" edition = "2021" license = "MIT OR Apache-2.0" rust-version = "1.88" diff --git a/apps/desktop/package.json b/apps/desktop/package.json index 335ca85..7ec68df 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -1,6 +1,6 @@ { "name": "@iran-split/desktop", - "version": "6.2.84", + "version": "6.2.85", "private": true, "type": "module", "scripts": { diff --git a/crates/iran-split-core/src/lib.rs b/crates/iran-split-core/src/lib.rs index f02d94e..09c8fe9 100644 --- a/crates/iran-split-core/src/lib.rs +++ b/crates/iran-split-core/src/lib.rs @@ -18,7 +18,8 @@ mod proxy_match; pub use proxy_match::{proxy_endpoint_matches, proxy_server_matches}; mod proxy_restore; pub use proxy_restore::{ - proxy_endpoint_ready, proxy_host_is_loopback, proxy_restore_decision, ProxyRestoreDecision, + proxy_endpoint_ready, proxy_host_is_loopback, proxy_restore_decision, retire_proxy_snapshot, + ProxyRestoreDecision, }; #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] @@ -641,12 +642,30 @@ pub trait PlatformBackend: Send + Sync + 'static { ) -> Result; async fn cleanup_owned_state(&self) -> Result; async fn verify_not_intercepting(&self) -> Result<(), CoreError> { - let tun = self.tun_status().await?; - if tun.active { + let (process, tun) = tokio::try_join!(self.core_process(), self.tun_status())?; + if process.running || tun.active { + warn!( + event = "network.cleanup_verification_failed", + section = "engine", + initiator = "cleanup_verification", + cause = "owned_capture_remaining", + trace_route = "engine->platform_backend->verify_not_intercepting", + process_running = process.running, + tun_active = tun.active, + "owned process or tunnel remains after cleanup" + ); return Err(CoreError::TunCleanupFailed( - "TUN is still active after cleanup".into(), + "owned process or TUN is still active after cleanup".into(), )); } + info!( + event = "network.cleanup_verified", + section = "engine", + initiator = "cleanup_verification", + cause = "owned_capture_absent", + trace_route = "engine->platform_backend->verify_not_intercepting", + "verified the owned core and tunnel are stopped before proxy handoff" + ); Ok(()) } } @@ -2053,6 +2072,7 @@ impl Engine { report.warnings.join("; ") ))); } + self.backend.verify_not_intercepting().await?; self.backend.restore_hiddify_system_proxy().await?; Ok(()) } @@ -2110,6 +2130,7 @@ impl Engine { ))); } self.backend.forget_client_egress().await; + self.backend.verify_not_intercepting().await?; self.backend.restore_hiddify_system_proxy().await?; proxy_stop?; proxy_clear?; @@ -2214,6 +2235,7 @@ impl Engine { report.warnings.join("; ") ))); } + self.backend.verify_not_intercepting().await?; self.backend.restore_hiddify_system_proxy().await?; Ok(()) } @@ -2276,7 +2298,7 @@ impl Engine { self.snapshots.send_replace(snapshot); } - /// Waits until the stack reaches `desired`, reports an error phase, or times out. + /// Waits for an idle `desired` phase, a completed error, or a timeout. /// /// # Errors /// @@ -2292,10 +2314,12 @@ impl Engine { tokio::time::timeout(timeout, async { loop { let snapshot = receiver.borrow().clone(); - if snapshot.phase == desired { + if snapshot.phase == desired && snapshot.busy.is_none() { return Ok(snapshot); } - if snapshot.phase == StackPhase::Error { + // An accepted retry can still display the preceding Error + // until its queued worker starts. Wait for that retry to finish. + if snapshot.phase == StackPhase::Error && snapshot.busy.is_none() { return Err(CoreError::Platform(snapshot.last_error.map_or_else( || "unknown error".into(), |error| error.technical_details.unwrap_or(error.message_key), @@ -2517,6 +2541,8 @@ mod tests { auto_client_sets_proxy: AtomicBool, fail_proxy_clear: AtomicBool, fail_proxy_stop: AtomicBool, + fail_proxy_restore: AtomicBool, + process_survives_cleanup: AtomicBool, } #[async_trait] @@ -2709,6 +2735,11 @@ mod tests { "core must stop before proxy handoff" ); self.proxy_restores.fetch_add(1, Ordering::SeqCst); + if self.fail_proxy_restore.load(Ordering::SeqCst) { + return Err(CoreError::Platform( + "original proxy client unavailable".into(), + )); + } if self.proxy_lease.load(Ordering::SeqCst) { self.proxy_active .store(self.proxy_baseline.load(Ordering::SeqCst), Ordering::SeqCst); @@ -2757,7 +2788,10 @@ mod tests { async fn cleanup_owned_state(&self) -> Result { self.cleanups.fetch_add(1, Ordering::SeqCst); - self.process.store(false, Ordering::SeqCst); + self.process.store( + self.process_survives_cleanup.load(Ordering::SeqCst), + Ordering::SeqCst, + ); self.tun.store(false, Ordering::SeqCst); Ok(CleanupReport { process_stopped: true, @@ -2769,6 +2803,81 @@ mod tests { } } + #[tokio::test] + async fn disconnect_and_quit_retry_an_incomplete_proxy_restore() { + for quit in [false, true] { + let backend = Arc::new(FakeBackend::default()); + backend.proxy_active.store(true, Ordering::SeqCst); + let engine = Engine::new(Arc::clone(&backend), &tokio::runtime::Handle::current()); + engine.start_stack(None).await.unwrap(); + engine + .wait_for_phase(StackPhase::Running, Duration::from_secs(2)) + .await + .unwrap(); + backend.fail_proxy_restore.store(true, Ordering::SeqCst); + if quit { + assert!(engine.shutdown().await.is_err()); + } else { + engine.stop_stack().await.unwrap(); + engine + .wait_for_phase(StackPhase::Error, Duration::from_secs(2)) + .await + .unwrap(); + } + assert_eq!(engine.snapshot().phase, StackPhase::Error); + assert!(!backend.process.load(Ordering::SeqCst)); + assert!(!backend.tun.load(Ordering::SeqCst)); + assert!(!backend.proxy_active.load(Ordering::SeqCst)); + assert!(backend.proxy_lease.load(Ordering::SeqCst)); + backend.fail_proxy_restore.store(false, Ordering::SeqCst); + if quit { + engine.shutdown().await.unwrap(); + } else { + engine.stop_stack().await.unwrap(); + engine + .wait_for_phase(StackPhase::Stopped, Duration::from_secs(2)) + .await + .unwrap(); + } + assert!(backend.proxy_active.load(Ordering::SeqCst)); + assert!(!backend.proxy_lease.load(Ordering::SeqCst)); + } + } + + #[tokio::test] + async fn disconnect_and_quit_reject_a_clean_report_when_the_core_survives() { + for quit in [false, true] { + let backend = Arc::new(FakeBackend::default()); + backend.proxy_active.store(true, Ordering::SeqCst); + let engine = Engine::new(Arc::clone(&backend), &tokio::runtime::Handle::current()); + engine.start_stack(None).await.unwrap(); + engine + .wait_for_phase(StackPhase::Running, Duration::from_secs(2)) + .await + .unwrap(); + backend + .process_survives_cleanup + .store(true, Ordering::SeqCst); + if quit { + assert!(engine.shutdown().await.is_err()); + } else { + engine.stop_stack().await.unwrap(); + engine + .wait_for_phase(StackPhase::Error, Duration::from_secs(2)) + .await + .unwrap(); + } + assert_eq!(engine.snapshot().phase, StackPhase::Error); + assert_eq!(backend.proxy_restores.load(Ordering::SeqCst), 0); + assert!(backend.proxy_lease.load(Ordering::SeqCst)); + backend + .process_survives_cleanup + .store(false, Ordering::SeqCst); + engine.shutdown().await.unwrap(); + assert!(backend.proxy_active.load(Ordering::SeqCst)); + } + } + #[tokio::test] async fn disconnect_returns_the_original_proxy_after_detaching_capture() { let backend = Arc::new(FakeBackend::default()); diff --git a/crates/iran-split-core/src/proxy_restore.rs b/crates/iran-split-core/src/proxy_restore.rs index da34fd5..59685ba 100644 --- a/crates/iran-split-core/src/proxy_restore.rs +++ b/crates/iran-split-core/src/proxy_restore.rs @@ -1,6 +1,7 @@ //! Restore only a managed, reachable proxy after releasing the TUN. -use std::time::Duration; +use crate::CoreError; +use std::{path::Path, time::Duration}; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ProxyRestoreDecision { @@ -39,6 +40,29 @@ pub const fn proxy_restore_decision( } } +/// Retire a completed handoff without losing a temporarily unavailable client. +/// +/// # Errors +/// Returns an error and retains the baseline when the original client is down, +/// or when the completed record cannot be removed. +pub fn retire_proxy_snapshot(path: &Path, decision: ProxyRestoreDecision) -> Result<(), CoreError> { + if decision == ProxyRestoreDecision::ClientUnavailable { + tracing::warn!( + event = "system_proxy.handoff_deferred", + section = "system_proxy", + initiator = "platform_backend", + cause = decision.cause(), + trace_route = "disconnect->restore_system_proxy->retain_baseline", + "original proxy client is unavailable; retaining its baseline for retry" + ); + return Err(CoreError::Platform( + "BiFlow routing is detached, but the original proxy client is unavailable. Start that client and retry Disconnect or Quit to restore its proxy.".into(), + )); + } + std::fs::remove_file(path) + .map_err(|_| CoreError::Platform("could not retire the saved system proxy".into())) +} + /// Only local client endpoints belong to the system proxy handoff. #[must_use] pub fn proxy_host_is_loopback(host: &str) -> bool { @@ -64,6 +88,33 @@ pub async fn proxy_endpoint_ready(host: &str, port: u16) -> bool { mod tests { use super::*; + #[test] + fn unavailable_client_keeps_the_baseline_until_a_verified_retry() { + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("proxy.json"); + let baseline = b"original private proxy baseline"; + std::fs::write(&path, baseline).unwrap(); + assert!(retire_proxy_snapshot(&path, ProxyRestoreDecision::ClientUnavailable).is_err()); + assert_eq!(std::fs::read(&path).unwrap(), baseline); + retire_proxy_snapshot(&path, ProxyRestoreDecision::Restore).unwrap(); + assert!(!path.exists()); + } + + #[test] + fn deliberate_settings_changes_and_unmanaged_baselines_can_be_retired() { + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("proxy.json"); + for decision in [ + ProxyRestoreDecision::SettingsChanged, + ProxyRestoreDecision::NotManaged, + ] { + std::fs::write(&path, b"baseline").unwrap(); + retire_proxy_snapshot(&path, decision).unwrap(); + assert!(!path.exists()); + } + assert!(retire_proxy_snapshot(&path, ProxyRestoreDecision::Restore).is_err()); + } + #[test] fn only_an_unchanged_managed_live_proxy_is_restored() { for unchanged in [false, true] { diff --git a/crates/iran-split-platform-linux/src/system_proxy.rs b/crates/iran-split-platform-linux/src/system_proxy.rs index d830510..0a8b5e2 100644 --- a/crates/iran-split-platform-linux/src/system_proxy.rs +++ b/crates/iran-split-platform-linux/src/system_proxy.rs @@ -1,5 +1,6 @@ use iran_split_core::{ - proxy_endpoint_ready, proxy_restore_decision, CoreError, ProxyRestoreDecision, + proxy_endpoint_ready, proxy_restore_decision, retire_proxy_snapshot, CoreError, + ProxyRestoreDecision, }; use serde::{Deserialize, Serialize}; use std::path::{Path, PathBuf}; @@ -135,8 +136,7 @@ pub async fn restore(persist: &Path, endpoints: &[(String, u16)]) -> Result<(), )); } } - std::fs::remove_file(persist) - .map_err(|_| CoreError::Platform("could not retire the saved system proxy".into()))?; + retire_proxy_snapshot(persist, decision)?; info!( event = "system_proxy.handoff_completed", section = "system_proxy", diff --git a/crates/iran-split-platform-macos/src/system_proxy.rs b/crates/iran-split-platform-macos/src/system_proxy.rs index 75f7e34..0dba1bd 100644 --- a/crates/iran-split-platform-macos/src/system_proxy.rs +++ b/crates/iran-split-platform-macos/src/system_proxy.rs @@ -7,7 +7,8 @@ //! service (Wi-Fi, Ethernet, …). use iran_split_core::{ - proxy_endpoint_ready, proxy_restore_decision, CoreError, ProxyRestoreDecision, + proxy_endpoint_ready, proxy_restore_decision, retire_proxy_snapshot, CoreError, + ProxyRestoreDecision, }; use serde::{Deserialize, Serialize}; use std::{ @@ -171,8 +172,7 @@ pub async fn restore(persist: &Path, endpoints: &[(String, u16)]) -> Result<(), )); } } - std::fs::remove_file(persist) - .map_err(|_| CoreError::Platform("could not retire the saved system proxy".into()))?; + retire_proxy_snapshot(persist, decision)?; info!( event = "system_proxy.handoff_completed", section = "system_proxy", diff --git a/crates/iran-split-platform-win/src/system_proxy.rs b/crates/iran-split-platform-win/src/system_proxy.rs index ecaa7a8..6add5eb 100644 --- a/crates/iran-split-platform-win/src/system_proxy.rs +++ b/crates/iran-split-platform-win/src/system_proxy.rs @@ -1,5 +1,6 @@ use iran_split_core::{ - proxy_endpoint_ready, proxy_restore_decision, CoreError, ProxyRestoreDecision, + proxy_endpoint_ready, proxy_restore_decision, retire_proxy_snapshot, CoreError, + ProxyRestoreDecision, }; use serde::{Deserialize, Serialize}; use std::path::{Path, PathBuf}; @@ -123,8 +124,7 @@ pub async fn restore(persist: &Path, endpoints: &[(String, u16)]) -> Result<(), )); } } - std::fs::remove_file(persist) - .map_err(|_| CoreError::Platform("could not retire the saved system proxy".into()))?; + retire_proxy_snapshot(persist, decision)?; info!( event = "system_proxy.handoff_completed", section = "system_proxy", diff --git a/docs/adr/0133-verified-network-restoration.md b/docs/adr/0133-verified-network-restoration.md new file mode 100644 index 0000000..a84296c --- /dev/null +++ b/docs/adr/0133-verified-network-restoration.md @@ -0,0 +1,54 @@ +# ADR 0133: Preserve incomplete proxy restoration and verify network release + +Status: Accepted (6.2.85) + +## Context + +The Windows diagnostic log recorded `saved_client_unavailable` during a +Disconnect. All native proxy backends then deleted the original proxy baseline +and logged the handoff as completed. A temporary client outage therefore lost +the record needed to restore the user's ordinary proxy on a later retry. + +Live checks also reproduced an untrusted-root TLS error for a direct request to +`rade.ir`, while the same request through Hiddify passed certificate validation. +Chrome's Proxy Switcher was on No Proxy and bypassed the restored system proxy. +With System Proxy selected, the site opened while BiFlow was disconnected. +BiFlow does not manage that browser extension or change certificate validation. + +## Decision + +- Preserve ADR 0132's ownership boundary: return the original managed proxy + only after releasing capture, while the client is reachable and settings + still match the state BiFlow left. Respect intervening user or corporate + settings and leave clients that were already running alone. +- An unavailable original client now returns an error and retains the private + baseline on Windows, macOS, and Linux. It is never enabled while unreachable. + A retry of Disconnect or Quit can complete restoration. Quit keeps the app + open when that restoration is incomplete. Deliberate outside settings changes + and unmanaged baselines remain safe reasons to retire a record. +- Independently check the owned core process and TUN after cleanup, before + proxy restoration on Stop, Pause, rollback, and startup reconciliation. A + successful helper report alone does not prove the running process is gone. +- Phase waiters accept a terminal phase/error only after lifecycle work is idle. + A queued retry must not immediately return the previous Error before it runs. +- Log an incomplete handoff as `system_proxy.handoff_deferred`, without proxy + settings. Include `saved_user_proxy_handoff_pending` in Error diagnostics as + well as stopped/paused diagnostics. +- "Normal" means the state before Connect: an existing healthy Hiddify proxy + stays available; an originally disabled proxy stays disabled. It does not + mean forcing ISP access or changing client configuration, browser proxy + extensions, DNS providers, security products, or certificate trust. + +## Validation + +Filesystem tests assert that a failed handoff retains the exact record and a +verified retry retires it. Shared engine tests exercise Disconnect and Quit with +a failed proxy restore and with a surviving core despite a clean helper report; +they assert capture cleanup, baseline retention, blocked completion, and retry. +The primary-flow e2e checks continued internet status and absent traffic routes +after a second Connect/Disconnect cycle. Environment tests cover pending +restoration in Error. Native CI validates the shared policy on every platform. + +These checks do not guarantee that an independently stopped client or an ISP's +invalid certificate can be repaired by BiFlow. They ensure those conditions do +not silently discard recovery state or produce a successful cleanup result. diff --git a/docs/adr/README.md b/docs/adr/README.md index c571d3c..b1b8212 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -136,3 +136,4 @@ Keep this index current. Add a new ADR for each non-obvious change, or update th | [0130](./0130-resume-recovery-and-serialized-quit.md) | Recover after unlock and serialize network release on Quit | Accepted | | [0131](./0131-verified-macos-install-and-relaunch.md) | Install and verify macOS updates before a user relaunch | Accepted | | [0132](./0132-return-user-proxy-on-disconnect.md) | Return the original live proxy after Disconnect | Accepted | +| [0133](./0133-verified-network-restoration.md) | Preserve incomplete proxy restoration and verify network release | Accepted | diff --git a/e2e/primary-flows.spec.ts b/e2e/primary-flows.spec.ts index 4a2bf6e..e27d7f5 100644 --- a/e2e/primary-flows.spec.ts +++ b/e2e/primary-flows.spec.ts @@ -185,6 +185,8 @@ test.describe("primary BiFlow flows", () => { await expect( page.getByRole("heading", { name: "Not connected" }), ).toBeVisible(); + await expect(statusBar).toContainText("Internet connected"); + await expect(page.locator(".traffic-flow-route")).toHaveCount(0); }); test("adds a pasted site from Home and reroutes it in place", async ({ diff --git a/package.json b/package.json index e86bd13..114b4f3 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "iran-split-desktop-workspace", - "version": "6.2.84", + "version": "6.2.85", "private": true, "packageManager": "pnpm@9.0.1", "engines": { diff --git a/src-tauri/src/environment/apps.rs b/src-tauri/src/environment/apps.rs index b585296..341e119 100644 --- a/src-tauri/src/environment/apps.rs +++ b/src-tauri/src/environment/apps.rs @@ -634,7 +634,10 @@ pub(super) fn app_findings( findings.push("mihomo_binary_missing".into()); } if report.apps.system_proxy_saved_by_pause - && phase_is(report, &[StackPhase::Paused, StackPhase::Stopped]) + && phase_is( + report, + &[StackPhase::Paused, StackPhase::Stopped, StackPhase::Error], + ) { findings.push("pause_cleared_hiddify_system_proxy".into()); if !report.system_proxy.is_empty() && report.system_proxy.iter().all(|proxy| !proxy.enabled) @@ -826,6 +829,12 @@ mod tests { }); let mut findings = Vec::new(); app_findings(&report, None, &mut findings); + assert!(findings + .iter() + .any(|finding| finding == "saved_user_proxy_handoff_pending")); + report.biflow.stack_phase = "error".into(); + findings.clear(); + app_findings(&report, None, &mut findings); assert!(findings .iter() .any(|finding| finding == "saved_user_proxy_handoff_pending")); diff --git a/src-tauri/tauri.conf.json b/src-tauri/tauri.conf.json index 66d1f95..21e58ec 100644 --- a/src-tauri/tauri.conf.json +++ b/src-tauri/tauri.conf.json @@ -1,7 +1,7 @@ { "$schema": "https://schema.tauri.app/config/2", "productName": "BiFlow", - "version": "6.2.84", + "version": "6.2.85", "identifier": "app.biflow.desktop", "build": { "beforeDevCommand": "pnpm bundle:check && pnpm --dir apps/desktop dev", diff --git a/version b/version index 3ea7ad3..b37aa10 100644 --- a/version +++ b/version @@ -1 +1 @@ -6.2.84 +6.2.85 From dd62e1544072582f457dcda4989950f0ba1d047f Mon Sep 17 00:00:00 2001 From: Omid Rade-PC Date: Sun, 8 Nov 2026 12:34:38 +0330 Subject: [PATCH 2/5] docs: replace user-reported sites with generic examples --- AGENTS.md | 9 +++++++-- apps/desktop/src/components/Diagnostics.test.tsx | 4 ++-- apps/desktop/src/lib/host.test.ts | 2 +- apps/desktop/src/lib/host.ts | 4 ++-- docs/adr/0133-verified-network-restoration.md | 2 +- e2e/primary-flows.spec.ts | 14 +++++++------- 6 files changed, 20 insertions(+), 15 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index f92e083..1500b8a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -71,6 +71,11 @@ If a required command fails or emits a warning from project code, fix it in the ## Lessons +- Keep user-reported domains, URLs, account identifiers, and infrastructure out + of documentation, example comments, test fixtures, and PR descriptions. Use + generic wording, reserved example domains, or synthetic offline fixtures when + a particular suffix is essential to a test. + - An unavailable original proxy client is an incomplete handoff, not a safe reason to delete its baseline. Keep the record, return a cleanup error, and allow Disconnect/Quit to retry after the client returns; never re-enable a @@ -718,7 +723,7 @@ already in progress"`. Cache the last `UpdateInfo` (never log asset URLs). allow rustfmt to split `.try_begin(` onto the next line. - A `.deb` install puts the helper at `/usr/lib/biflow/iran-split-helper`, which is also `install-helper.sh`'s destination, so `install(1)` aborts with "are the same file" and the in-app Install fails. Compare `readlink -f` of source and destination and re-apply ownership and mode in place instead of copying. - Dashboard and every other page grow inside the shell's `overflow-y-auto`. A page root with `h-full overflow-y-auto` locks height to the viewport and traps the live-routing SVG. Use `flex flex-col gap-4 pb-2` like Direct Rules. -- Diagnostics accepts any pasted form (`https://www.rade.ir/`, `host:port`, IPv6) and reduces it with `extractHost` before calling `test_route`; the rule parser rejects URLs outright. The flow result offers the opposite move — Add to direct for a VPN host, Add to VPN for a direct one — and re-tests afterwards so the card shows the new routing. +- Diagnostics accepts any pasted form (`https://www.example.ir/`, `host:port`, IPv6) and reduces it with `extractHost` before calling `test_route`; the rule parser rejects URLs outright. The flow result offers the opposite move — Add to direct for a VPN host, Add to VPN for a direct one — and re-tests afterwards so the card shows the new routing. - User route pins are one list (ADR 0034/0068): `RoutePinsDocument.pins` with `Outbound::Direct | Client { client_id }`. `pinRoute` takes `"direct"` or a ClientId uuid. Disabled-client pins stay in the document and are grey in Direct Rules but are not emitted to Mihomo. Deleting a client confirms with pin count and can move pins; MATCH `default_route` falls back to Direct. Pins store the PSL registrable root (private suffixes included); `api.shop.example.com` is `example.com`, `user.github.io` is not all of `github.io`. `pin` must not wait on DoH. Writers emit `+.example.com` and never copy `resolved_ips` into IP providers. Precedence in `RuleSet::decide` must match generated Mihomo rules — private/LAN first, then enabled client pins, then DIRECT pins, then bundled Iran domains, then curated `iran-business-domains`, then `MATCH` to `default_route`. Never let a private, loopback, or CGNAT address onto a LocalProxy. Helper `GENERATION_FILES` accept the fixed names plus `custom--domains.txt` / `custom--ips.txt` where `` is `[0-9a-f-]{36}`. Live apply when the stack is running or degraded; persist-only when stopped. - `resources/rules/manifest.json` `commit` is the **upstream Chocolate4U** revision the rules came from, not a BiFlow commit. Building a `raw.githubusercontent.com/devlifeX/BiFlow//…` URL from it always 404s and surfaces as "cloud rule download failed". Fetch snapshot files from the same branch the manifest itself is fetched from; the per-file SHA-256 in the manifest is what guarantees integrity, not the ref. - Windows `.join("biflow/debug.log")` keeps the `/`, so Explorer `/select,C:\…\biflow/debug.log` is ignored and opens This PC. Join `biflow` and `debug.log` as separate components, normalize leftover `/` to `\`, and pass `/select,` with `raw_arg`. The same class of bug is `mihomo_file_name()` returning `"bin/mihomo.exe"` — join `bin` and the file name separately. Helper Install must not elevate `C:\ProgramData\iran-split\bin\iran-split-helper.exe`: a leftover copy `fs::copy`s onto itself and a GUI-subsystem helper leaves stderr empty. Elevate the packaged helper, skip same-file copy, write `ProgramData\iran-split\install.log` on failure, and use NSIS `$PROGRAMDATA` (not `$COMMONPROGRAMDATA`) for helper/Mihomo install destinations. Packaged Windows helper staging is `$PROGRAMDATA\iran-split\staging` (ADR 0064). NSIS `perMachine` `$LOCALAPPDATA` expands to `C:\ProgramData`, so `$LOCALAPPDATA\biflow\runtime\generations` recorded `C:\ProgramData\biflow\runtime\generations` while the desktop wrote under the user's LocalAppData. The elevated installer ignores a mismatched `--staging-dir`, grants Builtin\Users modify with `icacls`, and the desktop stages into `WindowsPaths.generation_staging_dir`. @@ -763,7 +768,7 @@ SHA-256")`); launchd `KeepAlive` then restarts it in a crash loop and the - `pnpm format:check` includes `docs/considering/*.md`. Compact table separators (`|---|---|`) fail Prettier and the frontend GitHub Action. Run `pnpm exec prettier --write` on new markdown before merge. - Bundled rule bytes are pinned with `resources/rules/*.json -text`, which covers both `manifest.json` and `iran-business-domains.sources.json`. Contract tests must match that glob; requiring a literal `manifest.json -text` line fails after the curated catalog lands. - `RuleSet::from_sources` takes the curated catalog as a fourth iterator. Merging those names into `iran_domains` hides the Mihomo order (Iran list, then business catalog, then `MATCH`) from `test_route` and the CLI. -- Playwright `listitem` text for a subdomain pin is the registrable root (`rade.ir`, not `www.rade.ir`). Responsive screenshots prefer `/opt/cursor/artifacts/screenshots` and fall back to `test-results/responsive-screenshots` when that path is not writable. +- Playwright `listitem` text for a subdomain pin is the registrable root (`example.ir`, not `www.example.ir`). Responsive screenshots prefer `/opt/cursor/artifacts/screenshots` and fall back to `test-results/responsive-screenshots` when that path is not writable. - Pause never calls `stop_user_proxy` (Hiddify stays up) but must `clear_hiddify_system_proxy` when the OS proxy still points at Hiddify. Connect must not set the OS proxy; Resume restores the user-data snapshot without logging the endpoint. CLI `DemoBackend` must implement those required trait methods as no-ops; Windows `cargo test --workspace` compiles `iran-split-cli` and fails first if they are missing. - Basic mobile always shows `data-testid="bottom-nav"`. Tapping Rules, Diagnostics, or Settings writes Advanced and opens that page; Dashboard and About stay in Basic. - `list_active_connections` may log a row count, never hosts or URLs. Outbound is the last Mihomo `chains` entry (`DIRECT` → `"direct"`, otherwise the client uuid). diff --git a/apps/desktop/src/components/Diagnostics.test.tsx b/apps/desktop/src/components/Diagnostics.test.tsx index 1b88166..9bdfb03 100644 --- a/apps/desktop/src/components/Diagnostics.test.tsx +++ b/apps/desktop/src/components/Diagnostics.test.tsx @@ -131,12 +131,12 @@ describe("Diagnostics", () => { renderOn("test"); await userEvent.type( screen.getByLabelText("Test IP or domain"), - "https://www.rade.ir/some/path?a=1", + "https://www.example.ir/some/path?a=1", ); await userEvent.click(screen.getByRole("button", { name: "Test flow" })); const { desktop } = await import("../api/desktop"); - expect(desktop.testRoute).toHaveBeenCalledWith("www.rade.ir"); + expect(desktop.testRoute).toHaveBeenCalledWith("www.example.ir"); }); it("offers to move a VPN host to direct and re-tests it", async () => { diff --git a/apps/desktop/src/lib/host.test.ts b/apps/desktop/src/lib/host.test.ts index 74ed5e8..29c0d8a 100644 --- a/apps/desktop/src/lib/host.test.ts +++ b/apps/desktop/src/lib/host.test.ts @@ -3,7 +3,7 @@ import { extractHost } from "./host"; describe("extractHost", () => { it("reduces a full URL to its host", () => { - expect(extractHost("https://www.rade.ir/")).toBe("www.rade.ir"); + expect(extractHost("https://www.example.ir/")).toBe("www.example.ir"); expect(extractHost("http://digikala.com/product/1?a=2#top")).toBe( "digikala.com", ); diff --git a/apps/desktop/src/lib/host.ts b/apps/desktop/src/lib/host.ts index 0700294..97f0791 100644 --- a/apps/desktop/src/lib/host.ts +++ b/apps/desktop/src/lib/host.ts @@ -1,8 +1,8 @@ /** * Reduces anything a user might paste to the host a routing rule matches on. * - * The rule parser rejects URLs, so `https://www.rade.ir/` has to become - * `www.rade.ir` before it reaches the backend. Everything that is not a host — + * The rule parser rejects URLs, so `https://www.example.ir/` has to become + * `www.example.ir` before it reaches the backend. Everything that is not a host — * scheme, credentials, port, path, query, fragment — is dropped. */ export function extractHost(input: string): string { diff --git a/docs/adr/0133-verified-network-restoration.md b/docs/adr/0133-verified-network-restoration.md index a84296c..f6c15aa 100644 --- a/docs/adr/0133-verified-network-restoration.md +++ b/docs/adr/0133-verified-network-restoration.md @@ -10,7 +10,7 @@ and logged the handoff as completed. A temporary client outage therefore lost the record needed to restore the user's ordinary proxy on a later retry. Live checks also reproduced an untrusted-root TLS error for a direct request to -`rade.ir`, while the same request through Hiddify passed certificate validation. +the reported site, while the same request through Hiddify passed certificate validation. Chrome's Proxy Switcher was on No Proxy and bypassed the restored system proxy. With System Proxy selected, the site opened while BiFlow was disconnected. BiFlow does not manage that browser extension or change certificate validation. diff --git a/e2e/primary-flows.spec.ts b/e2e/primary-flows.spec.ts index e27d7f5..e4138bc 100644 --- a/e2e/primary-flows.spec.ts +++ b/e2e/primary-flows.spec.ts @@ -474,21 +474,21 @@ test.describe("primary BiFlow flows", () => { }) => { await openFresh(page); await goTo(page, "Troubleshoot", "Test"); - await page.getByLabel("Test IP or domain").fill("https://www.rade.ir/"); + await page.getByLabel("Test IP or domain").fill("https://www.example.ir/"); await page.getByRole("button", { name: "Test flow" }).click(); // The bundled Iran list keeps every .ir host direct until it is pinned. const flow = page.locator('[role="status"]').filter({ - hasText: "www.rade.ir", + hasText: "www.example.ir", }); - await expect(flow.getByText("www.rade.ir → DIRECT")).toBeVisible(); + await expect(flow.getByText("www.example.ir → DIRECT")).toBeVisible(); await flow.locator("select").selectOption({ label: "Hiddify" }); - await expect(flow.getByText("www.rade.ir → Hiddify")).toBeVisible(); + await expect(flow.getByText("www.example.ir → Hiddify")).toBeVisible(); await goTo(page, "Routing", "Sites"); // Pins stay exactly as typed; the moved host keeps its www label. const pinned = page.getByRole("row").filter({ - has: page.getByText("www.rade.ir", { exact: true }), + has: page.getByText("www.example.ir", { exact: true }), }); await expect(pinned.locator("select")).toHaveValue( "11111111-1111-1111-1111-111111111111", @@ -497,9 +497,9 @@ test.describe("primary BiFlow flows", () => { await pinned.locator("select").selectOption("direct"); await expect(pinned.locator("select")).toHaveValue("direct"); await goTo(page, "Troubleshoot"); - await page.getByLabel("Test IP or domain").fill("www.rade.ir"); + await page.getByLabel("Test IP or domain").fill("www.example.ir"); await page.getByRole("button", { name: "Test flow" }).click(); - await expect(page.getByText("www.rade.ir → DIRECT")).toBeVisible(); + await expect(page.getByText("www.example.ir → DIRECT")).toBeVisible(); }); test("restarts Hiddify on clean state from diagnostics", async ({ page }) => { From bbc24b8b60dbd234887aa4001adfe88a6bc2d5e2 Mon Sep 17 00:00:00 2001 From: Omid Rade-PC Date: Sun, 8 Nov 2026 16:01:26 +0330 Subject: [PATCH 3/5] fix: restore native networking after disconnect and quit --- AGENTS.md | 51 ++ Cargo.lock | 30 +- Cargo.toml | 2 +- apps/desktop/package.json | 2 +- crates/iran-split-core/src/lib.rs | 128 ++++- crates/iran-split-helper/src/commands.rs | 5 +- crates/iran-split-helper/src/lib.rs | 510 +++++++++++++----- crates/iran-split-helper/src/macos_dns.rs | 353 ++++++------ crates/iran-split-helper/src/macos_tun.rs | 157 ++++++ .../iran-split-helper/src/windows_network.rs | 217 ++++++++ crates/iran-split-ipc/src/lib.rs | 43 +- docs/adr/0121-restore-dns-without-loopback.md | 4 + docs/adr/0134-native-network-release.md | 59 ++ docs/adr/0135-serialized-native-lifecycle.md | 42 ++ .../0136-preserve-completed-proxy-handoff.md | 37 ++ docs/adr/0137-inactive-host-dns-evidence.md | 37 ++ docs/adr/README.md | 4 + e2e/primary-flows.spec.ts | 13 + package.json | 2 +- src-tauri/src/environment/dns.rs | 174 ++++++ src-tauri/src/environment/macos.rs | 57 ++ src-tauri/src/environment/mod.rs | 98 +++- src-tauri/tauri.conf.json | 2 +- version | 2 +- 24 files changed, 1675 insertions(+), 354 deletions(-) create mode 100644 crates/iran-split-helper/src/macos_tun.rs create mode 100644 crates/iran-split-helper/src/windows_network.rs create mode 100644 docs/adr/0134-native-network-release.md create mode 100644 docs/adr/0135-serialized-native-lifecycle.md create mode 100644 docs/adr/0136-preserve-completed-proxy-handoff.md create mode 100644 docs/adr/0137-inactive-host-dns-evidence.md create mode 100644 src-tauri/src/environment/dns.rs create mode 100644 src-tauri/src/environment/macos.rs diff --git a/AGENTS.md b/AGENTS.md index 1500b8a..f6a24f4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -71,6 +71,57 @@ If a required command fails or emits a warning from project code, fix it in the ## Lessons +- Configured DNS addresses do not prove usable resolution. A detached host can + have no owned tunnel or proxy while its original resolver returns private + answers for a public control name. Record answer classes, empty responses, + lookup failures, and timeouts without logging names or addresses. A successful + lookup is not proof of HTTP reachability, and intermittent HTTP success is not + proof that a previous timeout was caused by DNS or the app (ADR 0137). +- Initialize a diagnostic fixture's top-level fields in its struct literal. + Assigning them immediately after `Default::default()` fails strict Clippy's + `field_reassign_with_default`, even when neighboring nested-field setup passes. +- Native packet-monitor text can carry a UTF-16 BOM. Detect its encoding before + parsing a private diagnostic artifact; do not assume UTF-8 or treat a decoder + failure as evidence about network behavior. + +- A completed Stop already handed the original proxy client back. Its private + baseline has been retired, so calling `stop_user_proxy` again during Quit or + repeated Stop can mistake a retained child handle for ownership and kill the + returned client behind an enabled system proxy. Verify absent capture and + retry pending handoff without reclaiming the client. Cover Stop followed by + Quit and by Stop, not only Quit directly from Running (ADR 0136). +- Give concurrent verification invocations distinct output files on Windows. + PowerShell cannot truncate a gate log while an earlier process still owns its + redirected handle; await completion before reusing that log path. + +- Native Windows `MSFT_NetAdapter.AdminStatus` is numeric (Up=1, Down=2), not + the formatted `Up`/`Down` text. Use native-shaped fixtures, skip an already + disabled adapter, and reenable it before reconnect. Frame IO's five seconds + is not a native lifecycle budget. Serialize the helper through the end of + release so a timed-out Stop cannot disable a later Connect (ADR 0135). +- Launch detached installation scripts with the same known PowerShell runtime + used to prepare them. A legacy Windows PowerShell child can inherit an + incompatible module path and fail to find `Get-FileHash` before installation. + +- Missing native inspection is not successful cleanup. Windows must not use + `ifconfig` or a no-op adapter remover; verify its owned adapter, routes, and + DNS with native commands. A disabled retained adapter must be reenabled before + reconnect. On macOS, record the core's actual `utunN` with a per-launch UUID, + retain incomplete DNS/device recovery after process exit, reject an empty live + resolver, and run idempotent startup cleanup without requiring a controller. + Exercise production scripts with mocked native commands so network regression + tests cannot disconnect the developer (ADR 0134). +- PowerShell functions with `[Parameter]` already have common parameters. + Declaring `ErrorAction` again breaks a native-command fixture before it tests + behavior; shadow cmdlets with compatible parameter signatures. +- A LAN gateway is not proof of a DNS service. Do not replace restored macOS + DHCP/static resolvers with a guessed gateway when native resolver state is + delayed; retain the original recovery record and retry (ADR 0134). +- Apple-target Rust std alone cannot cross-check C dependencies from Windows. + Use an Apple-target compiler; a Zig wrapper must translate cc-rs's + `arm64-apple-macosx` target to `aarch64-macos`. PowerShell also does not expand + wildcard path arguments for `rg`; pass the directory plus `-g` instead. + - Keep user-reported domains, URLs, account identifiers, and infrastructure out of documentation, example comments, test fixtures, and PR descriptions. Use generic wording, reserved example domains, or synthetic offline fixtures when diff --git a/Cargo.lock b/Cargo.lock index 190d0f7..594cf21 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2046,7 +2046,7 @@ dependencies = [ [[package]] name = "iran-split-cli" -version = "6.2.85" +version = "6.2.89" dependencies = [ "async-trait", "chrono", @@ -2064,7 +2064,7 @@ dependencies = [ [[package]] name = "iran-split-clients" -version = "6.2.85" +version = "6.2.89" dependencies = [ "async-trait", "ipnet", @@ -2080,7 +2080,7 @@ dependencies = [ [[package]] name = "iran-split-config" -version = "6.2.85" +version = "6.2.89" dependencies = [ "hex", "rand 0.9.5", @@ -2093,7 +2093,7 @@ dependencies = [ [[package]] name = "iran-split-core" -version = "6.2.85" +version = "6.2.89" dependencies = [ "async-trait", "chrono", @@ -2110,7 +2110,7 @@ dependencies = [ [[package]] name = "iran-split-desktop" -version = "6.2.85" +version = "6.2.89" dependencies = [ "chrono", "dirs 6.0.0", @@ -2152,7 +2152,7 @@ dependencies = [ [[package]] name = "iran-split-helper" -version = "6.2.85" +version = "6.2.89" dependencies = [ "clap", "hex", @@ -2177,7 +2177,7 @@ dependencies = [ [[package]] name = "iran-split-helper-winacl" -version = "6.2.85" +version = "6.2.89" dependencies = [ "tokio", "windows", @@ -2185,7 +2185,7 @@ dependencies = [ [[package]] name = "iran-split-ipc" -version = "6.2.85" +version = "6.2.89" dependencies = [ "bytes", "chrono", @@ -2198,7 +2198,7 @@ dependencies = [ [[package]] name = "iran-split-mihomo" -version = "6.2.85" +version = "6.2.89" dependencies = [ "chrono", "futures-util", @@ -2221,7 +2221,7 @@ dependencies = [ [[package]] name = "iran-split-netstats" -version = "6.2.85" +version = "6.2.89" dependencies = [ "libc", "tempfile", @@ -2230,7 +2230,7 @@ dependencies = [ [[package]] name = "iran-split-platform-linux" -version = "6.2.85" +version = "6.2.89" dependencies = [ "async-trait", "chrono", @@ -2254,7 +2254,7 @@ dependencies = [ [[package]] name = "iran-split-platform-macos" -version = "6.2.85" +version = "6.2.89" dependencies = [ "async-trait", "chrono", @@ -2278,7 +2278,7 @@ dependencies = [ [[package]] name = "iran-split-platform-win" -version = "6.2.85" +version = "6.2.89" dependencies = [ "async-trait", "chrono", @@ -2301,7 +2301,7 @@ dependencies = [ [[package]] name = "iran-split-rules" -version = "6.2.85" +version = "6.2.89" dependencies = [ "async-trait", "chrono", @@ -2323,7 +2323,7 @@ dependencies = [ [[package]] name = "iran-split-taskbar" -version = "6.2.85" +version = "6.2.89" dependencies = [ "iran-split-config", "objc2", diff --git a/Cargo.toml b/Cargo.toml index f1d4a96..ace1fd5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,7 +19,7 @@ members = [ ] [workspace.package] -version = "6.2.85" +version = "6.2.89" edition = "2021" license = "MIT OR Apache-2.0" rust-version = "1.88" diff --git a/apps/desktop/package.json b/apps/desktop/package.json index 7ec68df..dfa5953 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -1,6 +1,6 @@ { "name": "@iran-split/desktop", - "version": "6.2.85", + "version": "6.2.89", "private": true, "type": "module", "scripts": { diff --git a/crates/iran-split-core/src/lib.rs b/crates/iran-split-core/src/lib.rs index 09c8fe9..7d66c5c 100644 --- a/crates/iran-split-core/src/lib.rs +++ b/crates/iran-split-core/src/lib.rs @@ -1452,7 +1452,14 @@ impl Engine { ) && snapshot.mihomo.phase != ComponentPhase::Running && snapshot.tun.phase != ComponentPhase::Running { - self.backend.stop_user_proxy().await?; + info!( + event = "network.released_client_preserved", + section = "lifecycle", + initiator = "shutdown", + cause = "stack_already_detached", + trace_route = "shutdown->preserve_completed_handoff", + "leaving a previously returned proxy client alone during Quit" + ); return self.backend.restore_hiddify_system_proxy().await; } self.run_stop(Uuid::new_v4()).await @@ -1773,10 +1780,13 @@ impl Engine { trace_route = "startup->engine->platform_backend->cleanup", "found owned runtime state during startup reconciliation" ); - let report = self.backend.cleanup_owned_state().await?; - if !report.clean() { - return Err(CoreError::TunCleanupFailed(report.warnings.join("; "))); - } + } + // DNS recovery can remain after the process and TUN are gone + // (including a helper restart). Cleanup is idempotent and must not + // depend on a live controller to discover this persisted state. + let report = self.backend.cleanup_owned_state().await?; + if !report.clean() { + return Err(CoreError::TunCleanupFailed(report.warnings.join("; "))); } self.backend.verify_not_intercepting().await?; self.backend.restore_hiddify_system_proxy().await?; @@ -2078,6 +2088,16 @@ impl Engine { } async fn run_stop(&self, operation_id: Uuid) -> Result<(), CoreError> { + if self.snapshot().phase == StackPhase::Stopped { + self.backend.verify_not_intercepting().await?; + self.backend.restore_hiddify_system_proxy().await?; + info!(event = "network.released_client_preserved", section = "lifecycle", + initiator = "run_stop", cause = "stack_already_detached", trace_id = %operation_id, + trace_route = "stop->verify_detached->preserve_completed_handoff", + "repeated Stop preserved the completed user proxy handoff"); + self.set_stopped(self.backend.runtime_health().await); + return Ok(()); + } self.announce(StackPhase::Stopping, operation_id).await; self.announce_stage(OperationStage::StoppingCore).await; if let Err(cause) = self.backend.stop_core().await { @@ -2543,6 +2563,9 @@ mod tests { fail_proxy_stop: AtomicBool, fail_proxy_restore: AtomicBool, process_survives_cleanup: AtomicBool, + dns_restore_fails: AtomicBool, + kill_proxy_after_handoff: AtomicBool, + proxy_client_alive: AtomicBool, } #[async_trait] @@ -2704,6 +2727,13 @@ mod tests { if self.fail_proxy_stop.load(Ordering::SeqCst) { return Err(CoreError::Platform("proxy stop failed".into())); } + // A native Child handle can outlive the retired proxy baseline. + // A second stop must not reclaim the client after handoff. + if self.kill_proxy_after_handoff.load(Ordering::SeqCst) + && !self.proxy_lease.load(Ordering::SeqCst) + { + self.proxy_client_alive.store(false, Ordering::SeqCst); + } Ok(()) } @@ -2796,13 +2826,57 @@ mod tests { Ok(CleanupReport { process_stopped: true, tun_removed: true, - dns_restored: true, + dns_restored: !self.dns_restore_fails.load(Ordering::SeqCst), routes_removed: 2, warnings: vec![], }) } } + #[tokio::test] + async fn stop_and_quit_retain_proxy_ownership_until_dns_cleanup_succeeds() { + for quit in [false, true] { + let backend = Arc::new(FakeBackend::default()); + backend.proxy_active.store(true, Ordering::SeqCst); + let engine = Engine::new(Arc::clone(&backend), &tokio::runtime::Handle::current()); + engine.start_stack(None).await.unwrap(); + engine + .wait_for_phase(StackPhase::Running, Duration::from_secs(2)) + .await + .unwrap(); + backend.dns_restore_fails.store(true, Ordering::SeqCst); + if quit { + assert!(engine.shutdown().await.is_err()); + } else { + engine.stop_stack().await.unwrap(); + engine + .wait_for_phase(StackPhase::Error, Duration::from_secs(2)) + .await + .unwrap(); + } + assert_eq!(engine.snapshot().phase, StackPhase::Error); + assert_eq!(backend.proxy_restores.load(Ordering::SeqCst), 0); + assert!(backend.proxy_lease.load(Ordering::SeqCst)); + backend.dns_restore_fails.store(false, Ordering::SeqCst); + engine.shutdown().await.unwrap(); + assert!(backend.proxy_active.load(Ordering::SeqCst)); + assert!(!backend.proxy_lease.load(Ordering::SeqCst)); + } + } + + #[tokio::test] + async fn reconcile_cleans_persisted_dns_without_a_live_process_or_tun() { + let backend = Arc::new(FakeBackend::default()); + let engine = Engine::new(Arc::clone(&backend), &tokio::runtime::Handle::current()); + engine.reconcile_startup().await.unwrap(); + engine + .wait_for_phase(StackPhase::Stopped, Duration::from_secs(2)) + .await + .unwrap(); + assert!(backend.cleanups.load(Ordering::SeqCst) > 0); + assert_eq!(backend.starts.load(Ordering::SeqCst), 0); + } + #[tokio::test] async fn disconnect_and_quit_retry_an_incomplete_proxy_restore() { for quit in [false, true] { @@ -2878,6 +2952,48 @@ mod tests { } } + #[tokio::test] + async fn stop_then_quit_or_repeated_stop_keeps_the_returned_client_alive() { + for quit in [false, true] { + let backend = Arc::new(FakeBackend::default()); + backend.proxy_active.store(true, Ordering::SeqCst); + backend.proxy_client_alive.store(true, Ordering::SeqCst); + backend + .kill_proxy_after_handoff + .store(true, Ordering::SeqCst); + let engine = Engine::new(Arc::clone(&backend), &tokio::runtime::Handle::current()); + engine.start_stack(None).await.unwrap(); + engine + .wait_for_phase(StackPhase::Running, Duration::from_secs(2)) + .await + .unwrap(); + engine.stop_stack().await.unwrap(); + engine + .wait_for_phase(StackPhase::Stopped, Duration::from_secs(2)) + .await + .unwrap(); + assert!(backend.proxy_client_alive.load(Ordering::SeqCst)); + assert!(backend.proxy_active.load(Ordering::SeqCst)); + assert!(!backend.proxy_lease.load(Ordering::SeqCst)); + if quit { + engine.shutdown().await.unwrap(); + } else { + engine.stop_stack().await.unwrap(); + engine + .wait_for_phase(StackPhase::Stopped, Duration::from_secs(2)) + .await + .unwrap(); + } + assert!( + backend.proxy_client_alive.load(Ordering::SeqCst), + "returned client was stopped after its proxy baseline was retired" + ); + assert!(backend.proxy_active.load(Ordering::SeqCst)); + assert!(!backend.process.load(Ordering::SeqCst)); + assert!(!backend.tun.load(Ordering::SeqCst)); + } + } + #[tokio::test] async fn disconnect_returns_the_original_proxy_after_detaching_capture() { let backend = Arc::new(FakeBackend::default()); diff --git a/crates/iran-split-helper/src/commands.rs b/crates/iran-split-helper/src/commands.rs index 7966346..dda93d9 100644 --- a/crates/iran-split-helper/src/commands.rs +++ b/crates/iran-split-helper/src/commands.rs @@ -118,10 +118,7 @@ async fn dispatch( HelperCommand::RestartMihomo { generation_id, config_sha256, - } => { - supervisor.stop().await?; - HelperReply::ProcessStatus(supervisor.start(generation_id, &config_sha256).await?) - } + } => HelperReply::ProcessStatus(supervisor.restart(generation_id, &config_sha256).await?), HelperCommand::GetMihomoProcessStatus => { HelperReply::ProcessStatus(supervisor.status().await?) } diff --git a/crates/iran-split-helper/src/lib.rs b/crates/iran-split-helper/src/lib.rs index ad1ecdf..cdb3193 100644 --- a/crates/iran-split-helper/src/lib.rs +++ b/crates/iran-split-helper/src/lib.rs @@ -10,7 +10,7 @@ use std::{ path::{Path, PathBuf}, process::Stdio, sync::{ - atomic::{AtomicUsize, Ordering}, + atomic::{AtomicBool, AtomicUsize, Ordering}, Arc, LazyLock, }, time::{Duration, Instant}, @@ -178,11 +178,13 @@ struct ManagedChild { pub struct Supervisor { settings: HelperSettings, child: Mutex>, + lifecycle: Mutex<()>, registered: Mutex>, logs: Arc>>, pub(crate) side_tunnels: Mutex>, desktop_contact: Mutex>, commands_in_flight: AtomicUsize, + network_release_pending: AtomicBool, started_at: Instant, } @@ -192,11 +194,13 @@ impl Supervisor { Self { settings, child: Mutex::new(None), + lifecycle: Mutex::new(()), registered: Mutex::new(HashMap::new()), logs: Arc::new(Mutex::new(VecDeque::with_capacity(MAX_LOG_ENTRIES))), side_tunnels: Mutex::new(HashMap::new()), desktop_contact: Mutex::new(None), commands_in_flight: AtomicUsize::new(0), + network_release_pending: AtomicBool::new(false), started_at: Instant::now(), } } @@ -205,6 +209,18 @@ impl Supervisor { *self.desktop_contact.lock().await = Some(Instant::now()); } + async fn lock_lifecycle(&self, operation: &'static str) -> tokio::sync::MutexGuard<'_, ()> { + let trace_id = Uuid::new_v4(); + tracing::info!(event = "helper.lifecycle_waiting", section = "helper_lifecycle", + initiator = "helper_command", cause = "serialize_native_network_changes", %trace_id, operation, + trace_route = "helper_command->lifecycle_lock->native_network", "waiting for owned network lifecycle work"); + let guard = self.lifecycle.lock().await; + tracing::info!(event = "helper.lifecycle_acquired", section = "helper_lifecycle", + initiator = "helper_command", cause = "previous_native_work_finished", %trace_id, operation, + trace_route = "helper_command->lifecycle_lock->native_network", "acquired owned network lifecycle work"); + guard + } + pub(crate) fn begin_command(&self) -> CommandInFlight<'_> { self.commands_in_flight.fetch_add(1, Ordering::SeqCst); CommandInFlight { supervisor: self } @@ -303,6 +319,15 @@ impl Supervisor { &self, generation_id: Uuid, expected_sha256: &str, + ) -> Result { + let _lifecycle = self.lock_lifecycle("start").await; + self.start_inner(generation_id, expected_sha256).await + } + + async fn start_inner( + &self, + generation_id: Uuid, + expected_sha256: &str, ) -> Result { self.verify_binary()?; let registered = self.registered.lock().await; @@ -334,12 +359,26 @@ impl Supervisor { )); } orphan::cleanup(&self.settings, generation_id).await?; + #[cfg(target_os = "macos")] + if macos_dns::recovery_pending(&self.settings.runtime_dir) + || macos_tun::saved(&self.settings.runtime_dir).map_or(true, |device| device.is_some()) + { + self.release_macos_network().await?; + } + self.network_release_pending.store(true, Ordering::SeqCst); + #[cfg(windows)] + windows_network::prepare(&self.settings.tun_name).await?; + // Persist DNS before the core can change native resolver state. + #[cfg(target_os = "macos")] + macos_dns::prepare(&self.settings.runtime_dir)?; + let started_at = now_string(); + let launch_id = Uuid::new_v4(); let mut child = spawn_mihomo(&self.settings, &generation_root, &config_path)?; if let Some(stdout) = child.stdout.take() { - capture_lines(stdout, Arc::clone(&self.logs), "info"); + capture_lines(stdout, Arc::clone(&self.logs), "info", launch_id); } if let Some(stderr) = child.stderr.take() { - capture_lines(stderr, Arc::clone(&self.logs), "warn"); + capture_lines(stderr, Arc::clone(&self.logs), "warn", launch_id); } tokio::time::sleep(MIHOMO_STAY_ALIVE).await; if let Some(status) = child.try_wait()? { @@ -352,7 +391,7 @@ impl Supervisor { let managed = ManagedChild { child, generation_id, - started_at: now_string(), + started_at, }; let status = process_status(Some(&managed)); *current = Some(managed); @@ -364,7 +403,16 @@ impl Supervisor { // sees it. Point every network service at `127.0.0.1` (Mihomo listens // on port 53 as root) so DNS flows through the TUN. #[cfg(target_os = "macos")] - macos_dns::apply(&self.settings.runtime_dir)?; + { + macos_tun::remember_started_device( + &self.settings.runtime_dir, + &self.logs, + launch_id, + Duration::from_secs(4), + ) + .await?; + macos_dns::apply(&self.settings.runtime_dir)?; + } Ok(status) } @@ -383,6 +431,7 @@ impl Supervisor { generation_id: Uuid, expected_sha256: &str, ) -> Result { + let _lifecycle = self.lock_lifecycle("overlay").await; let registered = self.registered.lock().await; if registered.get(&generation_id).map(String::as_str) != Some(expected_sha256) { return Err(HelperServiceError::InvalidGeneration( @@ -437,6 +486,11 @@ impl Supervisor { /// /// Returns an error when the process cannot be inspected or stopped. pub async fn stop(&self) -> Result { + let _lifecycle = self.lock_lifecycle("stop").await; + self.stop_inner().await + } + + async fn stop_inner(&self) -> Result { let mut current = self.child.lock().await; if let Some(managed) = current.as_mut() { stop_managed_child(managed).await?; @@ -463,7 +517,13 @@ impl Supervisor { // Start takes the same lock before it spawns, and the desktop does // not overlap stop with start. #[cfg(target_os = "macos")] - self.release_macos_network().await; + self.release_macos_network().await?; + #[cfg(windows)] + if windows_network::captured(&self.settings.tun_name, true).await? { + return Err(HelperServiceError::Process( + "Windows tunnel capture remains after stop".into(), + )); + } Ok(ProcessStatus { running: false, pid: None, @@ -472,12 +532,35 @@ impl Supervisor { }) } + pub(crate) async fn restart( + &self, + generation_id: Uuid, + expected_sha256: &str, + ) -> Result { + let _lifecycle = self.lock_lifecycle("restart").await; + self.stop_inner().await?; + self.start_inner(generation_id, expected_sha256).await + } + /// Drops macOS tunnel routes and points system DNS back at the network /// the machine had before Connect. #[cfg(target_os = "macos")] - async fn release_macos_network(&self) { - clear_owned_routes(&self.settings.tun_name).await; - if let Err(cause) = macos_dns::restore(&self.settings.runtime_dir) { + async fn release_macos_network(&self) -> Result<(), HelperServiceError> { + let routes = async { + let device = macos_tun::saved(&self.settings.runtime_dir)?; + let interface = device.as_deref().unwrap_or(&self.settings.tun_name); + if interface_exists(interface)? { + // A utun name can be reused after our process exits. Leave a live + // device alone and retain recovery instead of touching a new owner. + return Err(HelperServiceError::Process( + "macOS native tunnel is still present; release remains pending".into(), + )); + } + clear_owned_routes(interface).await + } + .await; + let dns = macos_dns::restore(&self.settings.runtime_dir); + if let Err(ref cause) = dns { tracing::warn!( event = "helper.dns_restore_after_stop_failed", section = "helper_dns", @@ -487,7 +570,11 @@ impl Supervisor { "macOS DNS was not restored when Mihomo stopped" ); } - flush_dns_cache(); + let cache = flush_dns_cache(); + routes?; + dns?; + cache?; + macos_tun::retire(&self.settings.runtime_dir) } /// Returns the current helper-owned process status. @@ -511,18 +598,36 @@ impl Supervisor { /// /// Returns an error when the process or network cleanup fails. pub async fn cleanup(&self) -> Result { + let _lifecycle = self.lock_lifecycle("cleanup").await; + self.cleanup_inner().await + } + + async fn cleanup_inner(&self) -> Result { + self.network_release_pending.store(true, Ordering::SeqCst); self.stop_all_side_tunnels().await; - let process_stopped = !self.stop().await?.running; + // A DNS/route failure must not skip the remaining cleanup. Stop may + // have already reaped the process before network restoration failed. + let stop_result = self.stop_inner().await; + let process_stopped = !self.status().await?.running; + #[cfg(target_os = "linux")] let was_active = interface_exists(&self.settings.tun_name); + #[cfg(target_os = "linux")] if was_active { - #[cfg(unix)] delete_owned_interface(&self.settings.tun_name).await?; - #[cfg(not(unix))] - delete_owned_interface(&self.settings.tun_name); } #[cfg(target_os = "macos")] - clear_owned_routes(&self.settings.tun_name).await; + let native_release = if stop_result.is_ok() { + Ok(()) + } else { + self.release_macos_network().await + }; + #[cfg(target_os = "macos")] + let tun_removed = native_release.is_ok(); + #[cfg(target_os = "linux")] let tun_removed = !interface_exists(&self.settings.tun_name); + #[cfg(windows)] + let tun_removed = stop_result.is_ok() + || !windows_network::captured(&self.settings.tun_name, true).await?; let warnings = if tun_removed { Vec::new() } else { @@ -531,13 +636,17 @@ impl Supervisor { self.settings.tun_name )] }; - // Restore the macOS system DNS (router resolver) now that the TUN is - // down; Mihomo's `127.0.0.1:53` listener is gone with the process. - #[cfg(target_os = "macos")] - let dns_result = macos_dns::restore(&self.settings.runtime_dir); - flush_dns_cache(); + #[cfg(not(target_os = "macos"))] + let cache_result = flush_dns_cache(); #[cfg(target_os = "macos")] - dns_result?; + native_release?; + #[cfg(not(target_os = "macos"))] + cache_result?; + // A second pass may repair a transient network failure from Stop. A + // process-stop failure still cannot be reported as successful cleanup. + if !process_stopped { + stop_result?; + } let report = CleanupReport { process_stopped, tun_removed, @@ -547,6 +656,13 @@ impl Supervisor { }; self.push_log("info", "network_cleanup_finished", BTreeMap::new()) .await; + if report.process_stopped + && report.tun_removed + && report.dns_restored + && report.warnings.is_empty() + { + self.network_release_pending.store(false, Ordering::SeqCst); + } Ok(report) } @@ -758,14 +874,26 @@ fn request_graceful_stop(child: &Child) { } /// Drops cached fake-ip answers (`198.18.0.0/16`) so lookups work again after -/// the TUN is gone. A missing tool is ignored; a failed flush is not fatal. -fn flush_dns_cache() { +/// the TUN is gone. Windows/macOS cache failures keep cleanup retryable. +fn flush_dns_cache() -> Result<(), HelperServiceError> { + let mut flushed = false; + let mut failed = false; for (program, args) in dns_flush_commands() { if !Path::new(program).is_file() { + failed = true; + tracing::warn!( + event = "helper.dns_cache_flush_unavailable", + section = "helper_dns", + initiator = "helper_process", + cause = "native_tool_missing", + trace_route = "helper_process->cleanup->dns_cache", + "native DNS cache tool is unavailable" + ); continue; } match dns_flush_status(program, args) { Ok(status) if status.success() => { + flushed = true; tracing::info!( event = "helper.dns_cache_flushed", section = "helper_dns", @@ -775,25 +903,39 @@ fn flush_dns_cache() { "flushed the system DNS cache after releasing the tunnel" ); } - Ok(status) => tracing::warn!( - event = "helper.dns_cache_flush_failed", - section = "helper_dns", - initiator = "helper_process", - cause = "flush_command_failed", - trace_route = "helper_process->cleanup->dns_cache", - status = %status, - "system DNS cache was not flushed" - ), - Err(cause) => tracing::warn!( - event = "helper.dns_cache_flush_failed", - section = "helper_dns", - initiator = "helper_process", - cause = %cause, - trace_route = "helper_process->cleanup->dns_cache", - "system DNS cache flush could not start" - ), + Ok(status) => { + failed = true; + tracing::warn!( + event = "helper.dns_cache_flush_failed", + section = "helper_dns", + initiator = "helper_process", + cause = "flush_command_failed", + trace_route = "helper_process->cleanup->dns_cache", + status = %status, + "system DNS cache was not flushed" + ); + } + Err(cause) => { + failed = true; + tracing::warn!( + event = "helper.dns_cache_flush_failed", + section = "helper_dns", + initiator = "helper_process", + cause = %cause, + trace_route = "helper_process->cleanup->dns_cache", + "system DNS cache flush could not start" + ); + } } } + // Linux resolver tools are optional alternatives and may be installed + // while systemd-resolved is disabled. Preserve that platform's policy. + if !cfg!(target_os = "linux") && (!flushed || failed) { + return Err(HelperServiceError::Process( + "system DNS cache cleanup remains pending".into(), + )); + } + Ok(()) } #[cfg(target_os = "macos")] @@ -873,19 +1015,24 @@ fn abandoned_capture( impl Supervisor { /// Stops Mihomo and restores DNS when the desktop process has disappeared. pub(crate) async fn release_if_desktop_absent(&self) { + let Ok(_lifecycle) = self.lifecycle.try_lock() else { + return; + }; let in_flight = self.commands_in_flight.load(Ordering::SeqCst) > 0; let contact = *self.desktop_contact.lock().await; - let captured = self.network_captured().await; if !abandoned_capture( contact, self.started_at, Instant::now(), in_flight, - captured, + true, DESKTOP_ABSENCE_LIMIT, ) { return; } + if !self.network_captured().await { + return; + } tracing::warn!( event = "helper.desktop_absent", section = "helper_lifecycle", @@ -894,7 +1041,7 @@ impl Supervisor { trace_route = "helper_process->absence_watch->cleanup", "desktop contact expired while the tunnel was still up; releasing the network" ); - if let Err(cause) = self.cleanup().await { + if let Err(cause) = self.cleanup_inner().await { tracing::warn!( event = "helper.desktop_absent_cleanup_failed", section = "helper_lifecycle", @@ -910,12 +1057,43 @@ impl Supervisor { } async fn network_captured(&self) -> bool { - let running = self - .status() + let running = if let Ok(status) = self.status().await { + status.running + } else { + tracing::warn!( + event = "helper.capture_status_unknown", + section = "helper_network", + initiator = "absence_watch", + cause = "process_inspection_failed", + trace_route = "helper_process->absence_watch->native_state", + "process inspection failed; keeping network release pending" + ); + true + }; + #[cfg(windows)] + let captured = windows_network::captured(&self.settings.tun_name, false) .await - .map(|status| status.running) - .unwrap_or(false); - running || interface_exists(&self.settings.tun_name) + .unwrap_or(true); + #[cfg(target_os = "linux")] + let captured = interface_exists(&self.settings.tun_name); + #[cfg(target_os = "macos")] + let captured = if let (Ok(active), Ok(device)) = ( + interface_exists(&self.settings.tun_name), + macos_tun::saved(&self.settings.runtime_dir), + ) { + active || device.is_some() || macos_dns::recovery_pending(&self.settings.runtime_dir) + } else { + tracing::warn!( + event = "helper.capture_status_unknown", + section = "helper_network", + initiator = "absence_watch", + cause = "native_inspection_failed", + trace_route = "helper_process->absence_watch->native_state", + "native inspection failed; keeping network release pending" + ); + true + }; + running || captured || self.network_release_pending.load(Ordering::SeqCst) } } @@ -1203,8 +1381,12 @@ fn now_string() -> String { ) } -fn capture_lines(reader: R, logs: Arc>>, level: &'static str) -where +fn capture_lines( + reader: R, + logs: Arc>>, + level: &'static str, + launch_id: Uuid, +) where R: tokio::io::AsyncRead + Unpin + Send + 'static, { tokio::spawn(async move { @@ -1218,7 +1400,10 @@ where timestamp: now_string(), level: level.into(), event: "mihomo_output".into(), - fields: BTreeMap::from([("message".into(), redact(&line))]), + fields: BTreeMap::from([ + ("message".into(), redact(&line)), + ("launch_id".into(), launch_id.to_string()), + ]), }); } }); @@ -1302,38 +1487,58 @@ fn owned_routes(table: &str, interface: &str) -> Vec { #[cfg(any(target_os = "macos", test))] fn route_delete_args(route: &OwnedRoute) -> Vec { - if route.destination == "default" { - return vec![ - "-n".into(), - "delete".into(), - "default".into(), - route.gateway.clone(), - ]; - } - let kind = if route.host_route { "-host" } else { "-net" }; - vec![ - "-n".into(), - "delete".into(), - kind.into(), - route.destination.clone(), - route.gateway.clone(), - ] + let mut args = vec!["-n".into(), "delete".into()]; + if route.gateway.contains(':') || route.destination.contains(':') { + args.push("-inet6".into()); + } + if route.destination != "default" { + args.push(if route.host_route { "-host" } else { "-net" }.into()); + } + args.push(route.destination.clone()); + args.push(route.gateway.clone()); + args +} + +#[cfg(any(target_os = "macos", test))] +fn verify_owned_routes_absent(table: &str, interface: &str) -> Result<(), HelperServiceError> { + if !owned_routes(table, interface).is_empty() { + return Err(HelperServiceError::Process( + "macOS owned routes remain after cleanup".into(), + )); + } + Ok(()) } #[cfg(target_os = "macos")] -async fn clear_owned_routes(interface: &str) { - let mut removed = 0usize; - let mut failed = 0usize; - for family in ["inet", "inet6"] { - let output = Command::new("/usr/sbin/netstat") - .args(["-rn", "-f", family]) +async fn route_command( + program: &str, + args: &[String], +) -> Result { + tokio::time::timeout( + Duration::from_secs(5), + Command::new(program) + .args(args) .env_clear() .env("LC_ALL", "C") .stdin(Stdio::null()) .stdout(Stdio::piped()) .stderr(Stdio::null()) - .output() - .await; + .kill_on_drop(true) + .output(), + ) + .await + .map_err(|_| HelperServiceError::Process("macOS route command timed out".into()))? + .map_err(HelperServiceError::Io) +} + +#[cfg(target_os = "macos")] +async fn clear_owned_routes(interface: &str) -> Result<(), HelperServiceError> { + let trace_id = Uuid::new_v4(); + let mut removed = 0usize; + let mut failed = 0usize; + for family in ["inet", "inet6"] { + let args = ["-rn".into(), "-f".into(), family.into()]; + let output = route_command("/usr/sbin/netstat", &args).await; let Ok(output) = output else { failed += 1; continue; @@ -1342,65 +1547,34 @@ async fn clear_owned_routes(interface: &str) { failed += 1; continue; } - let table = String::from_utf8_lossy(&output.stdout); - for route in owned_routes(&table, interface) { - let deleted = Command::new("/sbin/route") - .args(route_delete_args(&route)) - .env_clear() - .env("LC_ALL", "C") - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .output() - .await; - match deleted { + for route in owned_routes(&String::from_utf8_lossy(&output.stdout), interface) { + match route_command("/sbin/route", &route_delete_args(&route)).await { Ok(result) if result.status.success() => removed += 1, _ => failed += 1, } } + match route_command("/usr/sbin/netstat", &args).await { + Ok(result) + if result.status.success() + && verify_owned_routes_absent( + &String::from_utf8_lossy(&result.stdout), + interface, + ) + .is_ok() => {} + _ => failed += 1, + } } - if removed > 0 || failed > 0 { - tracing::info!( - event = "helper.tun_routes_cleared", - section = "helper_network", - initiator = "helper_process", - cause = "network_cleanup", - removed, - failed, - trace_route = "helper_process->stop->route", - "removed routes that still pointed at the tunnel" - ); - } -} - -#[cfg(target_os = "macos")] -async fn delete_owned_interface(name: &str) -> Result<(), HelperServiceError> { - // Mihomo creates a `utun` device and destroys it when the process exits, so - // a leftover interface is rare. `ifconfig delete` removes the - // address; the kernel reaps the interface once it has no addresses and no - // controlling process. - let output = Command::new("/sbin/ifconfig") - .args([name, "delete"]) - .env_clear() - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::piped()) - .output() - .await?; - if !output.status.success() { + tracing::info!(event = "helper.tun_routes_cleared", section = "helper_network", + initiator = "helper_process", cause = "network_cleanup", %trace_id, removed, failed, + trace_route = "helper_process->stop->route", "verified owned tunnel route release"); + if failed > 0 { return Err(HelperServiceError::Process( - String::from_utf8_lossy(&output.stderr) - .chars() - .take(1_024) - .collect(), + "macOS owned route cleanup could not be verified".into(), )); } Ok(()) } -#[cfg(not(unix))] -fn delete_owned_interface(_name: &str) {} - /// Returns `true` when a network interface with `name` is present. #[cfg(target_os = "linux")] fn interface_exists(name: &str) -> bool { @@ -1408,23 +1582,33 @@ fn interface_exists(name: &str) -> bool { } /// Returns `true` when a network interface with `name` is present. macOS has no -/// `/sys/class/net`; `ifconfig ` exiting successfully is the portable -/// equivalent. -#[cfg(not(target_os = "linux"))] -fn interface_exists(name: &str) -> bool { - std::process::Command::new("ifconfig") - .arg(name) - .stdout(Stdio::null()) +/// `/sys/class/net`; enumerate exact names and propagate inspection failures. +#[cfg(target_os = "macos")] +fn interface_exists(name: &str) -> Result { + let output = std::process::Command::new("/sbin/ifconfig") + .arg("-l") .stderr(Stdio::null()) - .status() - .is_ok_and(|status| status.success()) + .output()?; + if !output.status.success() { + return Err(HelperServiceError::Process( + "macOS interface enumeration failed".into(), + )); + } + Ok(String::from_utf8_lossy(&output.stdout) + .split_whitespace() + .any(|interface| interface == name)) } mod commands; mod orphan; +#[cfg(any(windows, test))] +mod windows_network; + #[cfg(any(target_os = "macos", test))] mod macos_dns; +#[cfg(any(target_os = "macos", test))] +mod macos_tun; #[cfg(unix)] mod unix_peer; @@ -1464,6 +1648,57 @@ pub fn clap_install_error_message(error: &clap::Error) -> Option { mod tests { use super::*; + #[tokio::test] + async fn mutating_commands_wait_until_previous_native_release_finishes() { + let root = tempfile::tempdir().unwrap(); + let supervisor = Supervisor::new(HelperSettings { + authorized_uid: 1000, + authorized_gid: 1000, + socket_path: root.path().join("helper.sock"), + staging_dir: root.path().join("staging"), + runtime_dir: root.path().join("runtime"), + mihomo_binary: root.path().join("missing-core"), + mihomo_sha256: "a".repeat(64), + tun_name: "biflow-test".into(), + }); + let release = supervisor.lifecycle.lock().await; + let budget = Duration::from_millis(20); + assert!( + tokio::time::timeout(budget, supervisor.start(Uuid::new_v4(), &"a".repeat(64))) + .await + .is_err() + ); + assert!(tokio::time::timeout(budget, supervisor.stop()) + .await + .is_err()); + assert!(tokio::time::timeout(budget, supervisor.cleanup()) + .await + .is_err()); + assert!( + tokio::time::timeout(budget, supervisor.restart(Uuid::new_v4(), &"a".repeat(64))) + .await + .is_err() + ); + assert!(tokio::time::timeout( + budget, + supervisor.overlay_running(Uuid::new_v4(), &"a".repeat(64)) + ) + .await + .is_err()); + // Skip busy work rather than queue a stale absence cleanup after Connect. + tokio::time::timeout( + Duration::from_secs(1), + supervisor.release_if_desktop_absent(), + ) + .await + .unwrap(); + drop(release); + assert!(matches!( + supervisor.start(Uuid::new_v4(), &"a".repeat(64)).await, + Err(HelperServiceError::Io(_)) + )); + } + #[test] fn absence_releases_only_an_idle_capture() { let now = Instant::now(); @@ -1542,6 +1777,14 @@ mod tests { ); } + #[test] + fn successful_route_command_is_not_proof_of_route_release() { + let table = "default 192.0.2.1 UGSc en0\n128.0/1 198.18.0.1 UGSc utun9\n"; + assert!(verify_owned_routes_absent(table, "utun9").is_err()); + assert!(verify_owned_routes_absent("default 192.0.2.1 UGSc en0\n", "utun9").is_ok()); + assert!(verify_owned_routes_absent(table, "utun8").is_ok()); + } + #[test] fn tun_routes_are_removed_without_touching_the_physical_default() { let table = "\ @@ -1591,7 +1834,7 @@ default fe80::1%utun9 UGcIg utun9 1 ); assert_eq!( route_delete_args(&routes[3]), - vec!["-n", "delete", "default", "fe80::1%utun9"] + vec!["-n", "delete", "-inet6", "default", "fe80::1%utun9"] ); let physical = owned_routes(table, "en0"); assert_eq!(physical.len(), 1); @@ -1653,7 +1896,6 @@ tun_name = "clash-iran" let path = Path::new("unused"); set_file_permissions(path); set_directory_permissions(path); - delete_owned_interface("unused"); } #[cfg(windows)] diff --git a/crates/iran-split-helper/src/macos_dns.rs b/crates/iran-split-helper/src/macos_dns.rs index 2a875da..cd4991d 100644 --- a/crates/iran-split-helper/src/macos_dns.rs +++ b/crates/iran-split-helper/src/macos_dns.rs @@ -7,6 +7,21 @@ use tracing::warn; type Snapshot = BTreeMap>; const SNAPSHOT_FILE: &str = "system-dns.toml"; +#[cfg(target_os = "macos")] +pub(crate) fn prepare(runtime: &Path) -> Result<(), HelperServiceError> { + let result = capture_with(runtime, &mut networksetup).map(|_| ()); + tracing::info!(event = "helper.dns_baseline_prepared", section = "helper_dns", + initiator = "helper_process", cause = "before_core_spawn", trace_id = %uuid::Uuid::new_v4(), + verified = result.is_ok(), trace_route = "helper_process->start->dns_baseline", + "prepared the DNS recovery baseline without logging settings"); + result +} + +pub(crate) fn recovery_pending(runtime: &Path) -> bool { + // A metadata/read error is not evidence that recovery is finished. + !matches!(fs::metadata(runtime.join(SNAPSHOT_FILE)), Err(error) if error.kind() == std::io::ErrorKind::NotFound) +} + #[cfg(target_os = "macos")] pub(crate) fn apply(runtime: &Path) -> Result<(), HelperServiceError> { run_action(runtime, true) @@ -25,19 +40,9 @@ fn run_action(runtime: &Path, apply: bool) -> Result<(), HelperServiceError> { initiator = "helper_process", cause = "dns_lifecycle", %trace_id, operation, trace_route = "helper_process->macos_dns->networksetup", "updating macOS system DNS"); let result = if apply { - apply_with( - runtime, - &mut networksetup, - &mut scutil_dns, - &mut default_gateway_text, - ) + apply_with(runtime, &mut networksetup, &mut scutil_dns) } else { - restore_with( - runtime, - &mut networksetup, - &mut scutil_dns, - &mut default_gateway_text, - ) + restore_with(runtime, &mut networksetup, &mut scutil_dns) }; if let Ok(services) = &result { tracing::info!(event = "helper.dns_completed", section = "helper_dns", @@ -69,8 +74,25 @@ fn apply_with( runtime: &Path, command: &mut impl FnMut(&[String]) -> Result, live_dns: &mut impl FnMut() -> Result, - gateway: &mut impl FnMut() -> Result, ) -> Result { + let services = capture_with(runtime, command)?; + for service in &services { + if let Err(error) = command(&dns_arguments(service, &["127.0.0.1".into()])) { + if restore_with(runtime, command, live_dns).is_err() { + return Err(HelperServiceError::Process( + "macOS DNS apply failed; recovery remains pending".into(), + )); + } + return Err(error); + } + } + Ok(services.len()) +} + +fn capture_with( + runtime: &Path, + command: &mut impl FnMut(&[String]) -> Result, +) -> Result, HelperServiceError> { let output = command(&["-listallnetworkservices".into()])?; let services = parse_services(&output); if services.is_empty() { @@ -100,24 +122,13 @@ fn apply_with( snapshot.insert(service.clone(), recoverable); } write_snapshot(runtime, &snapshot)?; - for service in &services { - if let Err(error) = command(&dns_arguments(service, &["127.0.0.1".into()])) { - if restore_with(runtime, command, live_dns, gateway).is_err() { - return Err(HelperServiceError::Process( - "macOS DNS apply failed; recovery remains pending".into(), - )); - } - return Err(error); - } - } - Ok(services.len()) + Ok(services) } fn restore_with( runtime: &Path, command: &mut impl FnMut(&[String]) -> Result, live_dns: &mut impl FnMut() -> Result, - gateway: &mut impl FnMut() -> Result, ) -> Result { let Some(snapshot) = read_snapshot(runtime)? else { return Ok(0); @@ -137,35 +148,30 @@ fn restore_with( return Err(error); } if still_loopback(command, live_dns, snapshot.keys())? { - let gateway = parse_default_gateway(&gateway()?); - let Some(gateway) = gateway else { - warn!( - event = "helper.dns_still_loopback", - section = "helper_dns", - initiator = "helper_process", - cause = "no_private_gateway", - trace_route = "helper_process->macos_dns->restore", - "macOS DNS still uses the tunnel resolver" - ); - return Err(HelperServiceError::Process( - "macOS DNS still uses the tunnel resolver".into(), - )); - }; warn!( - event = "helper.dns_router_fallback", + event = "helper.dns_still_loopback", section = "helper_dns", initiator = "helper_process", - cause = "live_resolver_still_loopback", - services = snapshot.len(), + cause = "native_resolver_not_restored", trace_route = "helper_process->macos_dns->restore", - "live macOS DNS stayed on the tunnel resolver; using the LAN gateway" + "macOS DNS recovery remains pending without replacing the original resolver" ); - for service in snapshot.keys() { - command(&dns_arguments(service, &[gateway.clone()]))?; - } - if still_loopback(command, live_dns, snapshot.keys())? { + return Err(HelperServiceError::Process( + "macOS DNS still uses the tunnel resolver".into(), + )); + } + if primary_nameservers(&live_dns()?).is_empty() { + return Err(HelperServiceError::Process( + "macOS primary DNS resolver is not ready; recovery remains pending".into(), + )); + } + for (service, servers) in &snapshot { + if !servers.is_empty() + && parse_dns(&command(&["-getdnsservers".into(), service.clone()])?)? != *servers + { return Err(HelperServiceError::Process( - "macOS DNS still uses the tunnel resolver".into(), + "macOS original DNS settings could not be verified; recovery remains pending" + .into(), )); } } @@ -231,7 +237,10 @@ fn without_loopback(servers: &[String]) -> Vec { fn is_loopback_resolver(server: &str) -> bool { server .parse::() - .is_ok_and(|address| address.is_loopback()) + .is_ok_and(|address| { + address.is_loopback() + || matches!(address, IpAddr::V4(ip) if ip.octets()[0] == 198 && matches!(ip.octets()[1], 18 | 19)) + }) } /// The first `scutil --dns` resolver is the one unqualified names use. @@ -270,25 +279,6 @@ fn primary_is_loopback(scutil_dns: &str) -> bool { .any(|server| is_loopback_resolver(server)) } -/// A private IPv4 default gateway can answer DNS when DHCP restore does not -/// clear a stuck `127.0.0.1`. The tunnel gateway `198.18.0.1` is not private. -fn parse_default_gateway(output: &str) -> Option { - for line in output.lines() { - let Some(rest) = line.trim().strip_prefix("gateway:") else { - continue; - }; - let candidate = rest.trim(); - let Ok(addr) = candidate.parse::() else { - return None; - }; - return match addr { - IpAddr::V4(ip) if ip.is_private() => Some(candidate.to_owned()), - _ => None, - }; - } - None -} - #[cfg(target_os = "macos")] fn scutil_dns() -> Result { let output = std::process::Command::new("/usr/sbin/scutil") @@ -303,20 +293,6 @@ fn scutil_dns() -> Result { Ok(String::from_utf8_lossy(&output.stdout).into_owned()) } -#[cfg(target_os = "macos")] -fn default_gateway_text() -> Result { - let output = std::process::Command::new("/sbin/route") - .args(["-n", "get", "default"]) - .env("LC_ALL", "C") - .output()?; - if !output.status.success() { - return Err(HelperServiceError::Process( - "macOS default route could not be read".into(), - )); - } - Ok(String::from_utf8_lossy(&output.stdout).into_owned()) -} - fn parse_services(output: &str) -> Vec { output .lines() @@ -401,9 +377,90 @@ fn write_snapshot(runtime: &Path, snapshot: &Snapshot) -> Result<(), HelperServi #[cfg(test)] mod tests { + #[test] + fn successful_set_does_not_retire_a_mismatched_static_dns_baseline() { + let root = tempfile::tempdir().unwrap(); + super::capture_with(root.path(), &mut |args| { + if args[0] == "-getdnsservers" { + Ok("9.9.9.9\n".into()) + } else { + network_response(args) + } + }) + .unwrap(); + assert!(super::restore_with( + root.path(), + &mut |args| { + if args[0] == "-getdnsservers" { + Ok("192.0.2.53\n".into()) + } else { + Ok(String::new()) + } + }, + &mut healthy_resolver, + ) + .is_err()); + assert!(super::recovery_pending(root.path())); + super::restore_with( + root.path(), + &mut |args| { + if args[0] == "-getdnsservers" { + Ok("9.9.9.9\n".into()) + } else { + Ok(String::new()) + } + }, + &mut healthy_resolver, + ) + .unwrap(); + assert!(!super::recovery_pending(root.path())); + } + #[test] + fn empty_live_resolver_keeps_recovery_until_verified_retry() { + let root = tempfile::tempdir().unwrap(); + assert!(!super::recovery_pending(root.path())); + super::capture_with(root.path(), &mut network_response).unwrap(); + assert!(super::recovery_pending(root.path())); + assert!( + super::restore_with(root.path(), &mut network_response, &mut || Ok( + "DNS configuration\n".into() + ),) + .is_err() + ); + assert!(super::recovery_pending(root.path())); + super::restore_with(root.path(), &mut network_response, &mut healthy_resolver).unwrap(); + assert!(!super::recovery_pending(root.path())); + } + + #[test] + fn prepare_captures_before_any_network_mutation_and_reuses_baseline() { + let root = tempfile::tempdir().unwrap(); + super::capture_with(root.path(), &mut |args| { + assert_ne!(args[0], "-setdnsservers"); + if args[0] == "-getdnsservers" { + Ok("9.9.9.9\n".into()) + } else { + network_response(args) + } + }) + .unwrap(); + super::capture_with(root.path(), &mut |args| { + assert_ne!(args[0], "-getdnsservers", "recaptured after takeover"); + network_response(args) + }) + .unwrap(); + assert_eq!( + super::read_snapshot(root.path()).unwrap().unwrap()["Wi-Fi"], + vec!["9.9.9.9"] + ); + assert!(super::is_loopback_resolver("198.18.0.2")); + assert!(super::is_loopback_resolver("198.19.255.254")); + assert!(!super::is_loopback_resolver("192.168.1.1")); + } + use super::{ - apply_with, dns_arguments, parse_default_gateway, parse_dns, parse_services, - primary_is_loopback, read_snapshot, restore_with, SNAPSHOT_FILE, + apply_with, dns_arguments, parse_dns, parse_services, primary_is_loopback, read_snapshot, + restore_with, SNAPSHOT_FILE, }; use crate::HelperServiceError; #[cfg(unix)] @@ -425,13 +482,7 @@ mod tests { #[test] fn dhcp_dns_is_restored_with_empty_and_the_snapshot_is_private() { let root = tempfile::tempdir().expect("tempdir"); - apply_with( - root.path(), - &mut network_response, - &mut healthy_resolver, - &mut no_gateway, - ) - .expect("apply"); + apply_with(root.path(), &mut network_response, &mut healthy_resolver).expect("apply"); #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; @@ -452,7 +503,6 @@ mod tests { network_response(args) }, &mut healthy_resolver, - &mut no_gateway, ) .expect("restore"); assert!(calls @@ -467,28 +517,15 @@ mod tests { #[test] fn failed_restore_keeps_recovery_for_a_new_helper() { let root = tempfile::tempdir().expect("tempdir"); - apply_with( - root.path(), - &mut network_response, - &mut healthy_resolver, - &mut no_gateway, - ) - .expect("apply"); + apply_with(root.path(), &mut network_response, &mut healthy_resolver).expect("apply"); assert!(restore_with( root.path(), &mut |_| Err(HelperServiceError::Process("fixture".into())), &mut healthy_resolver, - &mut no_gateway, ) .is_err()); assert!(root.path().join(SNAPSHOT_FILE).is_file()); - restore_with( - root.path(), - &mut network_response, - &mut healthy_resolver, - &mut no_gateway, - ) - .expect("retry"); + restore_with(root.path(), &mut network_response, &mut healthy_resolver).expect("retry"); assert!(!root.path().join(SNAPSHOT_FILE).exists()); } @@ -505,7 +542,6 @@ mod tests { } }, &mut healthy_resolver, - &mut no_gateway, ) .expect("first start"); apply_with( @@ -515,7 +551,6 @@ mod tests { network_response(args) }, &mut healthy_resolver, - &mut no_gateway, ) .expect("restarted helper"); let snapshot = read_snapshot(root.path()) @@ -524,9 +559,14 @@ mod tests { assert_eq!(snapshot["Wi-Fi"], vec!["8.8.8.8", "2001:4860:4860::8888"]); restore_with( root.path(), - &mut network_response, + &mut |args| { + if args[0] == "-getdnsservers" { + Ok("8.8.8.8\n2001:4860:4860::8888\n".into()) + } else { + network_response(args) + } + }, &mut healthy_resolver, - &mut no_gateway, ) .expect("restore"); } @@ -550,7 +590,6 @@ mod tests { network_response(args) }, &mut healthy_resolver, - &mut no_gateway, ); assert!(result.is_err()); assert_eq!(restores, vec!["Ethernet", "Wi-Fi"]); @@ -571,7 +610,6 @@ mod tests { } }, &mut healthy_resolver, - &mut no_gateway, ) .is_err()); assert!(!root.path().join(SNAPSHOT_FILE).exists()); @@ -599,7 +637,6 @@ mod tests { } }, &mut healthy_resolver, - &mut no_gateway, ) .expect("apply"); let snapshot = read_snapshot(root.path()) @@ -616,78 +653,35 @@ mod tests { network_response(args) }, &mut healthy_resolver, - &mut no_gateway, ) .expect("restore"); assert_eq!(sets, vec![vec!["-setdnsservers", "Wi-Fi", "Empty"]]); } #[test] - fn stuck_loopback_resolver_uses_the_lan_gateway() { - use std::cell::{Cell, RefCell}; - use std::rc::Rc; - - let root = tempfile::tempdir().expect("tempdir"); - apply_with( + fn delayed_dhcp_preserves_the_original_mode_until_retry() { + let root = tempfile::tempdir().unwrap(); + super::capture_with(root.path(), &mut network_response).unwrap(); + let mut written = Vec::new(); + assert!(restore_with( root.path(), &mut |args| { - if args[0] == "-getdnsservers" { - Ok("127.0.0.1\n".into()) - } else { - network_response(args) - } - }, - &mut healthy_resolver, - &mut no_gateway, - ) - .expect("apply"); - let using_gateway = Rc::new(Cell::new(false)); - let sets = Rc::new(RefCell::new(Vec::new())); - restore_with( - root.path(), - &mut { - let using_gateway = Rc::clone(&using_gateway); - let sets = Rc::clone(&sets); - move |args| { - if args[0] == "-setdnsservers" { - sets.borrow_mut().push(args[2..].to_vec()); - if args[2] == "10.0.0.1" { - using_gateway.set(true); - } - } - if args[0] == "-getdnsservers" { - return if using_gateway.get() { - Ok("10.0.0.1\n".into()) - } else { - Ok("127.0.0.1\n".into()) - }; - } - Ok(String::new()) - } - }, - &mut { - let using_gateway = Rc::clone(&using_gateway); - move || { - if using_gateway.get() { - Ok("resolver #1\n nameserver[0] : 10.0.0.1\n".into()) - } else { - Ok("resolver #1\n nameserver[0] : 127.0.0.1\n".into()) - } + if args[0] == "-setdnsservers" { + written.push(args[2..].to_vec()); } + network_response(args) }, - &mut || Ok(" gateway: 10.0.0.1\n".into()), + &mut || Ok("resolver #1\n nameserver[0] : 127.0.0.1\n".into()) ) - .expect("restore"); - let sets = sets.borrow(); - assert!(sets.iter().any(|servers| servers == &["Empty".to_owned()])); - assert!(sets - .iter() - .any(|servers| servers == &["10.0.0.1".to_owned()])); - assert!(!root.path().join(SNAPSHOT_FILE).exists()); + .is_err()); + assert_eq!(written, vec![vec!["Empty"]]); + assert!(super::recovery_pending(root.path())); + restore_with(root.path(), &mut network_response, &mut healthy_resolver).unwrap(); + assert!(!super::recovery_pending(root.path())); } #[test] - fn stuck_loopback_without_a_lan_gateway_keeps_recovery() { + fn stuck_loopback_keeps_recovery() { let root = tempfile::tempdir().expect("tempdir"); apply_with( root.path(), @@ -699,7 +693,6 @@ mod tests { } }, &mut healthy_resolver, - &mut no_gateway, ) .expect("apply"); let error = restore_with( @@ -712,7 +705,6 @@ mod tests { } }, &mut || Ok("resolver #1\n nameserver[0] : 127.0.0.1\n".into()), - &mut || Ok(" gateway: 198.18.0.1\n".into()), ) .expect_err("tunnel resolver remains"); assert!(error.to_string().contains("tunnel resolver")); @@ -722,19 +714,13 @@ mod tests { } #[test] - fn live_dns_and_gateway_parsers_ignore_the_tunnel() { + fn live_dns_parser_does_not_hide_the_tunnel_behind_a_scoped_resolver() { assert!(primary_is_loopback( "resolver #1\n nameserver[0] : 127.0.0.1\n\nresolver #2\n domain : local\n nameserver[0] : 8.8.8.8\n" )); assert!(!primary_is_loopback( "resolver #1\n nameserver[0] : 9.9.9.9\n" )); - assert_eq!( - parse_default_gateway(" gateway: 10.0.0.1\n interface: en0\n").as_deref(), - Some("10.0.0.1") - ); - assert_eq!(parse_default_gateway(" gateway: 198.18.0.1\n"), None); - assert_eq!(parse_default_gateway(" gateway: 8.8.8.8\n"), None); } // Production DNS callbacks return `Result`. These fixtures match that @@ -743,9 +729,4 @@ mod tests { fn healthy_resolver() -> Result { Ok("resolver #1\n nameserver[0] : 9.9.9.9\n".into()) } - - #[allow(clippy::unnecessary_wraps)] - fn no_gateway() -> Result { - Ok(String::new()) - } } diff --git a/crates/iran-split-helper/src/macos_tun.rs b/crates/iran-split-helper/src/macos_tun.rs new file mode 100644 index 0000000..39c0c2c --- /dev/null +++ b/crates/iran-split-helper/src/macos_tun.rs @@ -0,0 +1,157 @@ +//! Persist the kernel-assigned device from this core's startup output. Never +//! enumerate or delete unrelated `utun` devices by name prefix. +use super::HelperServiceError; +use std::{fs, io::Write, path::Path}; + +const FILE: &str = "owned-macos-tun"; + +pub(crate) async fn remember_started_device( + runtime: &Path, + logs: &tokio::sync::Mutex>, + launch_id: uuid::Uuid, + budget: std::time::Duration, +) -> Result<(), HelperServiceError> { + let deadline = std::time::Instant::now() + budget; + let launch = launch_id.to_string(); + loop { + let device = logs + .lock() + .await + .iter() + .rev() + .filter(|entry| entry.fields.get("launch_id") == Some(&launch)) + .find_map(|entry| { + entry + .fields + .get("message") + .and_then(|message| device_from_message(message)) + .map(str::to_owned) + }); + if let Some(device) = device { + remember(runtime, &device)?; + tracing::info!(event = "helper.native_tun_recorded", section = "helper_network", + initiator = "helper_process", cause = "owned_core_startup_output", trace_id = %launch_id, + trace_route = "helper_process->start->native_tun_identity", "recorded the owned kernel tunnel identity"); + return Ok(()); + } + if std::time::Instant::now() >= deadline { + return Err(HelperServiceError::Process( + "macOS core did not report its native tunnel identity".into(), + )); + } + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + } +} + +pub(crate) fn device_from_message(message: &str) -> Option<&str> { + let suffix = message.split_once("Tun adapter listening at: ")?.1; + let end = suffix + .find(|c: char| !c.is_ascii_alphanumeric()) + .unwrap_or(suffix.len()); + let name = &suffix[..end]; + valid_device(name).then_some(name) +} + +fn valid_device(name: &str) -> bool { + name.strip_prefix("utun").is_some_and(|index| { + !index.is_empty() && index.len() <= 5 && index.bytes().all(|c| c.is_ascii_digit()) + }) +} + +pub(crate) fn remember(runtime: &Path, name: &str) -> Result<(), HelperServiceError> { + if !valid_device(name) { + return Err(HelperServiceError::Process( + "invalid native macOS tunnel identity".into(), + )); + } + fs::create_dir_all(runtime)?; + let mut staged = tempfile::NamedTempFile::new_in(runtime)?; + staged.write_all(name.as_bytes())?; + staged.as_file().sync_all()?; + staged + .persist(runtime.join(FILE)) + .map_err(|error| error.error)?; + Ok(()) +} + +pub(crate) fn saved(runtime: &Path) -> Result, HelperServiceError> { + match fs::read_to_string(runtime.join(FILE)) { + Ok(name) if valid_device(&name) => Ok(Some(name)), + Ok(_) => Err(HelperServiceError::Process( + "invalid saved macOS tunnel identity".into(), + )), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(error) => Err(error.into()), + } +} + +pub(crate) fn retire(runtime: &Path) -> Result<(), HelperServiceError> { + match fs::remove_file(runtime.join(FILE)) { + Ok(()) => Ok(()), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(error) => Err(error.into()), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn repeated_generation_cannot_reuse_a_previous_process_device() { + use iran_split_ipc::ServiceLogEntry; + use std::{ + collections::{BTreeMap, VecDeque}, + time::Duration, + }; + let runtime = tempfile::tempdir().unwrap(); + let previous = uuid::Uuid::new_v4(); + let launch = uuid::Uuid::new_v4(); + let entry = |id: uuid::Uuid, device| ServiceLogEntry { + timestamp: "fixture".into(), + level: "info".into(), + event: "mihomo_output".into(), + fields: BTreeMap::from([ + ("launch_id".into(), id.to_string()), + ( + "message".into(), + format!("[TUN] Tun adapter listening at: {device}([fixture])"), + ), + ]), + }; + let logs = tokio::sync::Mutex::new(VecDeque::from([entry(previous, "utun7")])); + assert!( + remember_started_device(runtime.path(), &logs, launch, Duration::ZERO) + .await + .is_err() + ); + assert_eq!(saved(runtime.path()).unwrap(), None); + logs.lock().await.push_back(entry(launch, "utun9")); + remember_started_device(runtime.path(), &logs, launch, Duration::ZERO) + .await + .unwrap(); + assert_eq!(saved(runtime.path()).unwrap().as_deref(), Some("utun9")); + } + + #[test] + fn only_a_reported_kernel_device_can_be_persisted_and_retired() { + let runtime = tempfile::tempdir().unwrap(); + assert_eq!(saved(runtime.path()).unwrap(), None); + for message in [ + "tun enabled", + "Tun adapter listening at: en0", + "Tun adapter listening at: utun", + "Tun adapter listening at: ../utun9", + ] { + assert_eq!(device_from_message(message), None); + } + let name = device_from_message("time=fixture level=info msg=\"[TUN] Tun adapter listening at: utun9([fixture], mtu: 9000)\"").unwrap(); + remember(runtime.path(), name).unwrap(); + assert_eq!(saved(runtime.path()).unwrap().as_deref(), Some("utun9")); + assert!(remember(runtime.path(), "en0").is_err()); + assert_eq!(saved(runtime.path()).unwrap().as_deref(), Some("utun9")); + retire(runtime.path()).unwrap(); + retire(runtime.path()).unwrap(); + assert_eq!(saved(runtime.path()).unwrap(), None); + } +} diff --git a/crates/iran-split-helper/src/windows_network.rs b/crates/iran-split-helper/src/windows_network.rs new file mode 100644 index 0000000..5c484bc --- /dev/null +++ b/crates/iran-split-helper/src/windows_network.rs @@ -0,0 +1,217 @@ +//! Release only the configured Mihomo adapter, then read back native state. +use super::HelperServiceError; + +// Executed by the privileged helper. Tests replace every native cmdlet with +// in-memory functions; no regression test changes the host network. +const SCRIPT: &str = r" +$ErrorActionPreference = 'Stop' +$stage = 'adapter_enumeration' +trap { [Console]::Out.WriteLine('failed:' + $stage); exit 1 } +function Owned-Adapter { + $items = @(Get-NetAdapter -IncludeHidden -ErrorAction Stop | Where-Object { $_.Name -eq $tunName }) + if ($items.Count -gt 1) { throw 'ambiguous adapter' } + if ($items.Count -eq 0) { return } + $item = $items[0] + if ($item.InterfaceDescription -notmatch '^(Meta Tunnel|Mihomo Tunnel|Wintun Userspace Tunnel)( #\d+)?$') { throw 'adapter ownership mismatch' } + return $item +} +function Capture-Remains($item) { + if ($null -eq $item) { return $false } + $routes = @(Get-NetRoute -ErrorAction Stop | Where-Object { $_.InterfaceIndex -eq $item.ifIndex }) + $dns = @(Get-DnsClientServerAddress -ErrorAction Stop | Where-Object { $_.InterfaceIndex -eq $item.ifIndex } | ForEach-Object { $_.ServerAddresses }) + return ([int]$item.AdminStatus -ne 2 -or $routes.Count -gt 0 -or $dns.Count -gt 0) +} +$item = Owned-Adapter +if ($prepare -and $null -ne $item -and [int]$item.AdminStatus -eq 2) { + $stage = 'adapter_enable' + $item | Enable-NetAdapter -Confirm:$false -ErrorAction Stop +} +if ($release -and $null -ne $item) { + $identity = $item.InterfaceGuid + $current = Owned-Adapter + if ($null -eq $current -or $current.InterfaceGuid -ne $identity) { throw 'adapter identity changed' } + # A persistent Wintun device can survive TerminateProcess. Do not uninstall + # its driver or touch physical adapters, DHCP, other VPNs, or global routes. + $stage = 'route_release' + Get-NetRoute -ErrorAction Stop | Where-Object { $_.InterfaceIndex -eq $item.ifIndex } | Remove-NetRoute -Confirm:$false -ErrorAction Stop + $stage = 'dns_reset' + $dns = @(Get-DnsClientServerAddress -ErrorAction Stop | Where-Object { $_.InterfaceIndex -eq $item.ifIndex } | ForEach-Object { $_.ServerAddresses }) + if ($dns.Count -gt 0) { + Set-DnsClientServerAddress -InterfaceIndex $item.ifIndex -ResetServerAddresses -ErrorAction Stop + } + if ([int]$item.AdminStatus -ne 2) { + $stage = 'adapter_disable' + $item | Disable-NetAdapter -Confirm:$false -ErrorAction Stop + } +} +$stage = 'native_verification' +if (Capture-Remains (Owned-Adapter)) { 'active' } else { 'clear' } +"; + +fn script(name: &str, release: bool, prepare: bool) -> Result { + if name.is_empty() + || name.len() > 15 + || !name + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || byte == b'-' || byte == b'_') + { + return Err(HelperServiceError::UnsafeConfig( + "unsafe tunnel name".into(), + )); + } + Ok(format!( + "$tunName = '{name}'; $release = ${release}; $prepare = ${prepare};\n{SCRIPT}" + )) +} + +#[cfg(windows)] +pub(crate) async fn captured(name: &str, release: bool) -> Result { + run(name, release, false).await +} + +#[cfg(windows)] +pub(crate) async fn prepare(name: &str) -> Result<(), HelperServiceError> { + run(name, false, true).await.map(|_| ()) +} + +#[cfg(windows)] +async fn run(name: &str, release: bool, prepare: bool) -> Result { + use std::{process::Stdio, time::Duration}; + use tokio::process::Command; + let trace_id = uuid::Uuid::new_v4(); + let script = script(name, release, prepare)?; + let output = tokio::time::timeout( + Duration::from_secs(15), + Command::new("powershell.exe") + .args(["-NoProfile", "-NonInteractive", "-Command", &script]) + .creation_flags(0x0800_0000) + .kill_on_drop(true) + .stdin(Stdio::null()) + .output(), + ) + .await; + let mut failure_stage = "native_command"; + let result = match output { + Ok(Ok(output)) if output.status.success() => { + match String::from_utf8_lossy(&output.stdout).trim() { + "active" => Ok(true), + "clear" => Ok(false), + _ => Err(HelperServiceError::Process( + "Windows network verification returned invalid state".into(), + )), + } + } + Ok(Ok(output)) => { + let stdout = String::from_utf8_lossy(&output.stdout); + failure_stage = match stdout.trim() { + "failed:adapter_enumeration" => "adapter_enumeration", + "failed:adapter_enable" => "adapter_enable", + "failed:route_release" => "route_release", + "failed:dns_reset" => "dns_reset", + "failed:adapter_disable" => "adapter_disable", + "failed:native_verification" => "native_verification", + _ => "native_command", + }; + Err(HelperServiceError::Process(format!( + "Windows owned network {failure_stage} failed" + ))) + } + _ => Err(HelperServiceError::Process( + "Windows owned network command failed or timed out".into(), + )), + }; + tracing::info!(event = "helper.windows_network_checked", section = "helper_network", + initiator = "helper_process", cause = "native_adapter_routes_dns_check", %trace_id, + release, prepare, failure_stage, verified = result.is_ok(), captured = result.as_ref().ok().copied(), + trace_route = "helper_process->windows_network->native_state", + "checked Windows capture without logging network settings"); + result +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn unsafe_names_cannot_become_powershell() { + for name in ["", "*", "tun'; exit", "a/b", "1234567890123456"] { + assert!(script(name, true, false).is_err()); + } + let text = script("biflow-test", true, false).unwrap(); + assert!(!text.contains("ifconfig")); + assert!(text.contains("Get-NetAdapter -IncludeHidden -ErrorAction Stop")); + } + + #[cfg(windows)] + #[test] + fn native_errors_report_only_the_safe_stage() { + let production = script("biflow-test", false, false).unwrap(); + let fixture = + format!("function Get-NetAdapter {{ throw 'private fixture payload' }}; {production}"); + let output = std::process::Command::new("powershell.exe") + .args(["-NoProfile", "-NonInteractive", "-Command", &fixture]) + .output() + .unwrap(); + assert!(!output.status.success()); + assert_eq!( + String::from_utf8_lossy(&output.stdout).trim(), + "failed:adapter_enumeration" + ); + assert!(!String::from_utf8_lossy(&output.stderr).contains("private fixture payload")); + } + + #[cfg(windows)] + #[test] + fn native_release_is_scoped_verified_retryable_and_idempotent() { + // Every OS command in SCRIPT is shadowed before production code runs. + let fixture = r" +$script:adapter = [pscustomobject]@{ Name='biflow-test'; InterfaceDescription='Meta Tunnel'; ifIndex=42; InterfaceGuid='fixture'; AdminStatus=1 } +$script:routes = 2 +$script:dns = @('198.18.0.2') +$script:failDns = $true +$script:ignoreDisable = $false +function Get-NetAdapter { param([switch]$IncludeHidden, $ErrorAction) $script:adapter } +function Get-NetRoute { param($ErrorAction) for ($i=0; $i -lt $script:routes; $i++) { [pscustomobject]@{InterfaceIndex=42;Index=$i} }; [pscustomobject]@{InterfaceIndex=99;Index=99} } +function Remove-NetRoute { param([Parameter(ValueFromPipeline=$true)]$InputObject, $Confirm) process { if ($InputObject.InterfaceIndex -ne 42) { throw 'touched other route' }; $script:routes = 0 } } +function Get-DnsClientServerAddress { param($ErrorAction) [pscustomobject]@{ InterfaceIndex=42; ServerAddresses=$script:dns }; [pscustomobject]@{InterfaceIndex=99; ServerAddresses=@('192.0.2.53')} } +function Set-DnsClientServerAddress { param($InterfaceIndex, [switch]$ResetServerAddresses, $ErrorAction) if ($script:dns.Count -eq 0) { throw 'DNS interface already absent' }; if ($script:failDns) { throw 'fixture failure' }; if ($InterfaceIndex -ne 42 -or !$ResetServerAddresses) { throw 'wrong DNS reset' }; $script:dns=@() } +function Disable-NetAdapter { param([Parameter(ValueFromPipeline=$true)]$InputObject, $Confirm) process { if ($InputObject.ifIndex -ne 42) { throw 'wrong adapter' }; if ([int]$script:adapter.AdminStatus -eq 2) { throw 'already disabled' }; if (!$script:ignoreDisable) { $script:adapter.AdminStatus=2 } } } +function Enable-NetAdapter { param([Parameter(ValueFromPipeline=$true)]$InputObject, $Confirm) process { if ($InputObject.ifIndex -ne 42) { throw 'wrong adapter' }; $script:adapter.AdminStatus=1 } } +"; + let production = script("biflow-test", true, false).unwrap().replace( + "trap { [Console]::Out.WriteLine('failed:' + $stage); exit 1 }", + "", + ); + let probe = script("biflow-test", false, false).unwrap().replace( + "trap { [Console]::Out.WriteLine('failed:' + $stage); exit 1 }", + "", + ); + let prepare = script("biflow-test", false, true).unwrap().replace( + "trap { [Console]::Out.WriteLine('failed:' + $stage); exit 1 }", + "", + ); + let test = format!( + "{fixture}\n$action = {{ {production} }}; $probe = {{ {probe} }}; $prepare = {{ {prepare} }};\n\ + if ((& $probe) -ne 'active') {{ throw 'missed controller-independent capture' }};\n\ + $failed=$false; try {{ & $action }} catch {{ $failed=$true }}; if (!$failed) {{ throw 'ignored DNS failure' }};\n\ + $script:failDns=$false; $script:ignoreDisable=$true; if ((& $action) -ne 'active') {{ throw 'trusted failed disable' }};\n\ + $script:ignoreDisable=$false; if ((& $action) -ne 'clear') {{ throw 'retry failed' }};\n\ + if ((& $action) -ne 'clear') {{ throw 'not idempotent' }};\n\ + $script:adapter.InterfaceDescription='Meta Tunnel #2'; if ((& $action) -ne 'clear') {{ throw 'driver suffix rejected' }};\n\ + if ((& $prepare) -ne 'active') {{ throw 'adapter not reenabled for reconnect' }};\n\ + if ((& $action) -ne 'clear') {{ throw 'second release failed' }};\n\ + $script:adapter.InterfaceDescription='Physical Ethernet'; $failed=$false; try {{ & $action }} catch {{ $failed=$true }}; if (!$failed) {{ throw 'touched physical adapter' }};\n\ + $script:adapter=$null; if ((& $action) -ne 'clear') {{ throw 'absent adapter failed' }};\n\ + function Get-NetAdapter {{ throw 'enumeration failed' }}; $failed=$false; try {{ & $probe }} catch {{ $failed=$true }}; if (!$failed) {{ throw 'inspection failure counted as clear' }}" + ); + let output = std::process::Command::new("powershell.exe") + .args(["-NoProfile", "-NonInteractive", "-Command", &test]) + .output() + .unwrap(); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + } +} diff --git a/crates/iran-split-ipc/src/lib.rs b/crates/iran-split-ipc/src/lib.rs index 010ec10..46303ee 100644 --- a/crates/iran-split-ipc/src/lib.rs +++ b/crates/iran-split-ipc/src/lib.rs @@ -11,6 +11,8 @@ pub const MAX_MESSAGE_BYTES: usize = 1024 * 1024; /// Per-frame write/connect budget for helper IPC. pub const HELPER_IPC_FRAME_TIMEOUT_SECS: u64 = 5; +/// Native process, adapter, route, and DNS cleanup can outlive frame IO. +pub const HELPER_IPC_LIFECYCLE_TIMEOUT_SECS: u64 = 90; /// Small margin beyond [`HelperCommand::StartSideTunnel`]'s /// `timeout_seconds` so route installation and reply framing do not @@ -25,6 +27,16 @@ pub const HELPER_IPC_SIDE_TUNNEL_MARGIN_SECS: u64 = 15; #[must_use] pub fn helper_ipc_reply_timeout(command: &HelperCommand) -> Duration { match command { + HelperCommand::RestartMihomo { .. } => { + Duration::from_secs(2 * HELPER_IPC_LIFECYCLE_TIMEOUT_SECS) + } + HelperCommand::StartMihomo { .. } + | HelperCommand::StopMihomo + | HelperCommand::CleanupOwnedNetworkState + | HelperCommand::PrepareForUpdate + | HelperCommand::OverlayRuntimeGeneration { .. } => { + Duration::from_secs(HELPER_IPC_LIFECYCLE_TIMEOUT_SECS) + } HelperCommand::StartSideTunnel { timeout_seconds, .. } => { @@ -465,9 +477,34 @@ mod tests { helper_ipc_reply_timeout(&HelperCommand::GetServiceStatus), Duration::from_secs(HELPER_IPC_FRAME_TIMEOUT_SECS) ); - assert_eq!( + } + + #[tokio::test] + async fn lifecycle_reply_can_arrive_after_the_frame_deadline() { + let (mut desktop, mut helper) = tokio::io::duplex(4096); + let reply = Envelope::new(HelperReply::Ack); + let expected = reply.clone(); + let server = tokio::spawn(async move { + tokio::time::sleep(Duration::from_secs(HELPER_IPC_FRAME_TIMEOUT_SECS + 1)).await; + write_frame(&mut helper, &reply).await.unwrap(); + }); + let actual: Envelope = tokio::time::timeout( helper_ipc_reply_timeout(&HelperCommand::StopMihomo), - Duration::from_secs(HELPER_IPC_FRAME_TIMEOUT_SECS) - ); + read_frame(&mut desktop), + ) + .await + .expect("native cleanup reply must outlive frame IO") + .unwrap(); + assert_eq!(actual, expected); + server.await.unwrap(); + for command in [ + HelperCommand::CleanupOwnedNetworkState, + HelperCommand::PrepareForUpdate, + ] { + assert_eq!( + helper_ipc_reply_timeout(&command), + Duration::from_secs(HELPER_IPC_LIFECYCLE_TIMEOUT_SECS) + ); + } } } diff --git a/docs/adr/0121-restore-dns-without-loopback.md b/docs/adr/0121-restore-dns-without-loopback.md index 894a5ee..620280d 100644 --- a/docs/adr/0121-restore-dns-without-loopback.md +++ b/docs/adr/0121-restore-dns-without-loopback.md @@ -4,6 +4,10 @@ Accepted +The LAN-gateway fallback below is superseded by +[ADR 0134](./0134-native-network-release.md). Restoration now preserves the +original static resolvers or DHCP and retains recovery until verification. + ## Context Disconnect on macOS reported a successful DNS restore and then every site diff --git a/docs/adr/0134-native-network-release.md b/docs/adr/0134-native-network-release.md new file mode 100644 index 0000000..34203d8 --- /dev/null +++ b/docs/adr/0134-native-network-release.md @@ -0,0 +1,59 @@ +# ADR 0134: Verify native network release on Windows and macOS + +Status: Accepted (6.2.86) + +## Context + +The Windows helper used `ifconfig` to detect an owned adapter and a no-op to +remove it. A missing Unix command therefore looked like successful cleanup. +Controller shutdown also does not prove that native routing or resolver state +has disappeared. On macOS, the configured tunnel alias is replaced by a +kernel-assigned `utunN` name. Cleanup using the alias misses that identity. +Separately, startup reconciliation skipped cleanup once the core and TUN were +gone, even if a private DNS recovery record remained. An empty live resolver +was accepted as restored DNS, and some route/cache failures were only warnings. + +## Decision + +- Windows release uses native adapter, route, and DNS cmdlets. It selects only + the configured name with a known Mihomo/Wintun description, rechecks its GUID, + removes routes on that interface, resets its DNS, disables the persistent + adapter, and reads back state. A retained device is reenabled before the next + core start. Missing tools, inspection errors, and partial release are failures. + Physical adapters, other VPNs, global proxy choices, and drivers are untouched. +- macOS saves the actual `utunN` emitted by the owned core, tagged with a fresh + launch UUID so an earlier launch cannot supply its identity. The private + record survives helper restart. Cleanup checks native interface enumeration; + it retains recovery while that device remains instead of deleting a possibly + reused live tunnel. Route-command failures propagate. No prefix-wide `utun` + cleanup is permitted. +- Save macOS DNS before spawning the core. Retain recovery when the native + primary resolver is empty or still uses a tunnel resolver, including fake-IP + answers. Read back saved static resolvers before retiring the baseline. DNS + restoration still runs when route verification fails. Supersede ADR 0121's + guessed LAN-gateway fallback: a gateway is not evidence of a DNS service. + Restore the original static settings or DHCP and retain recovery for retry. +- Startup always runs the helper's idempotent cleanup when the helper is ready. + A live controller is not a prerequisite. The absence watchdog also tracks + pending release and private macOS DNS/device recovery records. +- Windows/macOS cache-flush failures keep cleanup retryable. Stop/Pause/Quit + cannot complete a proxy handoff while the helper reports incomplete cleanup. +- macOS environment snapshots now collect classified primary DNS evidence and + report a missing resolver while stopped/paused. Raw resolver addresses are + never written into diagnostics. + +## Validation + +Tests replace every Windows network cmdlet with in-memory fixtures and execute +the production script through PowerShell. They cover a surviving capture, +partial DNS failure, a no-op disable, retry, repeated release, reconnect, +unrelated routes, a physical-adapter name collision, and enumeration failure. +Portable macOS tests cover pre-spawn DNS capture, empty resolver retry, private +device persistence, launch isolation, and classified environment findings. +Engine tests cover startup without a process/TUN and blocked Stop/Quit followed +by successful DNS recovery. The existing connect/pause/resume/disconnect e2e +flow also asserts absent traffic routes while paused. + +These tests do not stop the installed app or change the host network. The helper +also passes Apple Silicon target Clippy from Windows using Zig for its C +dependencies. Native end-to-end macOS behavior still requires validation on a Mac. diff --git a/docs/adr/0135-serialized-native-lifecycle.md b/docs/adr/0135-serialized-native-lifecycle.md new file mode 100644 index 0000000..0aa0c9f --- /dev/null +++ b/docs/adr/0135-serialized-native-lifecycle.md @@ -0,0 +1,42 @@ +# ADR 0135: Serialize native release and budget lifecycle replies + +Status: Accepted (6.2.87) + +## Context + +The first native Windows release used textual `AdminStatus` fixtures. The actual +MSFT adapter property is numeric: 1 means Up and 2 means Down. Comparing it with +`Down` missed reenabling a retained adapter and repeated an unsupported disable. +The desktop also abandoned lifecycle replies after the five-second frame budget, +while bounded native work continued in the helper. A subsequent Connect could +then overlap that release and have its new tunnel disabled by the previous Stop. + +## Decision + +- Use numeric adapter administration state in production and executable fixtures. + Reenable Down before start and skip an already completed disable. Keep GUID + checks and native readback; accept only the known driver description and its + Windows-added numeric suffix. Skip DNS reset when the interface's DNS state + is already absent; a down Wintun device can retain an adapter without a DNS row. +- Give lifecycle replies a separate 90-second budget, and an atomic Restart twice + that budget. Connection, frame IO, and routine status retain their short + deadlines. Native commands remain independently bounded. +- A helper lifecycle mutex spans Start, Stop, Cleanup, Restart, and generation + overlay, including the native release after the child lock is dropped. Cleanup + calls the internal stop under that same lock. A successful verified Stop need + not repeat native release inside Cleanup. Status stays independent of the + lifecycle lock. The absence watchdog skips busy work and rechecks desktop + contact while holding the lock before it can clean up. +- Native failures return only a whitelisted stage, never PowerShell stderr or + network settings. Environment findings expose a disabled owned native adapter + even when the controller still claims Running. + +## Validation + +The production PowerShell script runs against numeric in-memory adapter fixtures, +including reconnect, repeated release, and an already-disabled failure trap. +Another test verifies the production safe error framing. Public lifecycle methods +must wait behind a held native-release lock without executing network commands. +A duplex IPC test delivers an actual framed reply after the old five-second +deadline. Environment tests reject controller-only health for a disabled adapter. +No test disconnects the developer's network. diff --git a/docs/adr/0136-preserve-completed-proxy-handoff.md b/docs/adr/0136-preserve-completed-proxy-handoff.md new file mode 100644 index 0000000..43213fb --- /dev/null +++ b/docs/adr/0136-preserve-completed-proxy-handoff.md @@ -0,0 +1,37 @@ +# ADR 0136: Preserve a completed proxy handoff on repeated Stop and Quit + +Status: Accepted (6.2.88) + +## Context + +A successful Stop restores the original local proxy and retires its private +baseline. The Stopped shutdown path nevertheless called `stop_user_proxy` again. +A backend still holding a child handle could then see no saved baseline, stop +the returned client, and leave the restored system proxy pointing at a dead +socket. A repeated Stop had the same hazard, including another unnecessary +clear/restore cycle. This is a global ownership bug, independent of destinations. + +## Decision + +- Quit from an already detached Stopped/Uninitialized stack only retries pending + restoration; it does not reclaim the returned client. +- Repeated Stop verifies that capture is absent and retries pending restoration + without stopping clients or clearing their already restored system proxy. +- Keep full cleanup and error retry for active and Error states. Never reenable + a proxy whose client is unavailable. An uninitialized app does not own a + preexisting user client merely because Quit was requested. +- Diagnose an enabled loopback system proxy whose configured client refuses both + address families as `system_proxy_points_to_unavailable_client`. A process name + or a successful old handoff event is not evidence of current socket readiness. +- Do not add per-domain routes, hosts entries, DNS exceptions, or site fixtures. + Native capture cleanup, system proxy state, DNS policy, and external network + reachability are distinct system-wide checks. + +## Validation + +An engine regression starts with an original live proxy, performs Stop, retires +the baseline through the production handoff flow, then exercises both repeated +Stop and Quit. The backend fixture models a retained process handle that would +kill the returned client after baseline retirement. This test fails before the +fix and passes after it. Environment fixtures cover the global dead-proxy finding +and its absence for a disabled proxy. No real tunnel is disconnected by tests. diff --git a/docs/adr/0137-inactive-host-dns-evidence.md b/docs/adr/0137-inactive-host-dns-evidence.md new file mode 100644 index 0000000..0de4332 --- /dev/null +++ b/docs/adr/0137-inactive-host-dns-evidence.md @@ -0,0 +1,37 @@ +# ADR 0137: Record classified host DNS answers while disconnected + +Status: Accepted (6.2.89) + +## Context + +Verified native cleanup and DNS server configuration do not establish that the +host resolver returns usable addresses. An original resolver can return a +private answer for independent public destinations after the owned TUN and +proxy have been released. Conversely, a valid DNS answer or TLS handshake does +not prove that an HTTP response will arrive. Intermittent HTTP success cannot +establish the cause of an earlier failure. + +## Decision + +- While Stopped or Paused, environment snapshots perform a bounded, read-only + host resolver lookup for the fixed reserved example domain `example.com`. + No user target, direct rule, route, proxy, hosts entry, or resolver is changed. +- Record only resolution status and deduplicated address classes. Never retain + the target, returned addresses, or raw OS resolver error in diagnostics. +- Distinguish empty replies, resolver failures, timeouts, and non-public answers + with `stopped_public_dns_*` findings. Do not run this probe in Running/Degraded + snapshots, where fake-IP answers are expected. +- Findings describe evidence, not ownership. They do not block native cleanup, + replace a user's DNS/firewall/proxy policy, or claim that all Internet access + is healthy after one successful lookup. A recovery of an external connection + without a configuration change is not a new application fix. + +## Validation + +Injected resolver futures exercise private, fake-IP, loopback, link-local, +shared-address, multicast, broadcast, ordinary, empty, failed, and timed-out +responses without accessing the network. Serialization regressions ensure +addresses and raw error payloads cannot reach the snapshot. Environment tests +verify phase-dependent findings, including mixed ordinary/non-public answers. +The production probe only resolves through the existing host policy and makes +no HTTP requests. Live tunnel toggling is not required by these tests. diff --git a/docs/adr/README.md b/docs/adr/README.md index b1b8212..64bdf2c 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -137,3 +137,7 @@ Keep this index current. Add a new ADR for each non-obvious change, or update th | [0131](./0131-verified-macos-install-and-relaunch.md) | Install and verify macOS updates before a user relaunch | Accepted | | [0132](./0132-return-user-proxy-on-disconnect.md) | Return the original live proxy after Disconnect | Accepted | | [0133](./0133-verified-network-restoration.md) | Preserve incomplete proxy restoration and verify network release | Accepted | +| [0134](./0134-native-network-release.md) | Verify native network release on Windows and macOS | Accepted | +| [0135](./0135-serialized-native-lifecycle.md) | Serialize native release and budget lifecycle replies | Accepted | +| [0136](./0136-preserve-completed-proxy-handoff.md) | Preserve a completed proxy handoff on repeated Stop and Quit | Accepted | +| [0137](./0137-inactive-host-dns-evidence.md) | Record classified host DNS answers while disconnected | Accepted | diff --git a/e2e/primary-flows.spec.ts b/e2e/primary-flows.spec.ts index e4138bc..c8a9601 100644 --- a/e2e/primary-flows.spec.ts +++ b/e2e/primary-flows.spec.ts @@ -166,6 +166,7 @@ test.describe("primary BiFlow flows", () => { ).toBeVisible(); await expect(page.getByRole("button", { name: "Resume" })).toBeVisible(); await expect(statusBar).toContainText("Internet connected"); + await expect(page.locator(".traffic-flow-route")).toHaveCount(0); await page.getByRole("button", { name: "Resume" }).click(); await expect( page.getByRole("heading", { name: "Connected", exact: true }), @@ -187,6 +188,18 @@ test.describe("primary BiFlow flows", () => { ).toBeVisible(); await expect(statusBar).toContainText("Internet connected"); await expect(page.locator(".traffic-flow-route")).toHaveCount(0); + // A tray Stop may arrive after the page has already finished Disconnect. + const repeatedStop = await page.evaluate(async () => { + const modulePath = "/src/api/mock.ts"; + const { mockApi } = await import(modulePath); + return mockApi.stop(); + }); + expect(repeatedStop.already_complete).toBe(true); + await expect( + page.getByRole("heading", { name: "Not connected" }), + ).toBeVisible(); + await expect(statusBar).toContainText("Internet connected"); + await expect(page.locator(".traffic-flow-route")).toHaveCount(0); }); test("adds a pasted site from Home and reroutes it in place", async ({ diff --git a/package.json b/package.json index 114b4f3..6af43a6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "iran-split-desktop-workspace", - "version": "6.2.85", + "version": "6.2.89", "private": true, "packageManager": "pnpm@9.0.1", "engines": { diff --git a/src-tauri/src/environment/dns.rs b/src-tauri/src/environment/dns.rs new file mode 100644 index 0000000..0ffcb49 --- /dev/null +++ b/src-tauri/src/environment/dns.rs @@ -0,0 +1,174 @@ +//! Address-free evidence from the host resolver while capture is inactive. +use super::classify_host; +use serde::Serialize; +use std::{collections::BTreeSet, future::Future, io, net::IpAddr, time::Duration}; +use uuid::Uuid; + +#[derive(Debug, Clone, Serialize)] +pub struct DnsProbe { + pub status: &'static str, + pub answer_classes: Vec<&'static str>, +} + +pub(super) async fn collect() -> DnsProbe { + // A fixed reserved example domain, independent of user rules or diagnostics + // targets. Query only the host resolver; never pin an address or send HTTP. + let lookup = async { + tokio::net::lookup_host(("example.com", 443)) + .await + .map(|addresses| addresses.map(|address| address.ip()).collect()) + }; + let probe = collect_with(lookup, Duration::from_secs(2)).await; + tracing::info!( + event = "environment.dns_probe_completed", + section = "environment", + initiator = "environment_collector", + cause = probe.status, + trace_id = %Uuid::new_v4(), + trace_route = "environment_snapshot->inactive_dns_probe->host_resolver", + answer_classes = ?probe.answer_classes, + "checked host resolution without logging targets or addresses" + ); + probe +} + +async fn collect_with(lookup: F, budget: Duration) -> DnsProbe +where + F: Future>>, +{ + let (status, addresses) = match tokio::time::timeout(budget, lookup).await { + Ok(Ok(addresses)) if addresses.is_empty() => ("empty", Vec::new()), + Ok(Ok(addresses)) => ("resolved", addresses), + Ok(Err(_)) => ("failed", Vec::new()), + Err(_) => ("timed_out", Vec::new()), + }; + let answer_classes = addresses + .into_iter() + .map(|address| { + if address.is_multicast() { + "multicast" + } else if matches!(address, IpAddr::V4(ip) if ip.is_broadcast()) { + "broadcast" + } else { + classify_host(&address.to_string()) + } + }) + .collect::>() + .into_iter() + .collect(); + DnsProbe { + status, + answer_classes, + } +} + +pub(super) fn findings(probe: Option<&DnsProbe>, findings: &mut Vec) { + let Some(probe) = probe else { return }; + if probe.status != "resolved" { + findings.push(format!("stopped_public_dns_{}", probe.status)); + } else if probe + .answer_classes + .iter() + .any(|class| *class != "public_ip") + { + // This is evidence about resolution, not proof of an app leftover or + // permission to replace the user's DNS, corporate proxy, or firewall. + findings.push("stopped_public_dns_non_public_answer".into()); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn non_public_answers_are_visible_without_recording_addresses() { + let addresses = [ + "10.0.0.7", + "127.0.0.1", + "198.18.0.7", + "100.64.0.7", + "224.0.0.1", + "255.255.255.255", + "::1", + "fe80::1", + "10.0.0.7", + ]; + let probe = collect_with( + async { + Ok(addresses + .iter() + .map(|value| value.parse().unwrap()) + .collect()) + }, + Duration::from_secs(1), + ) + .await; + let mut found = Vec::new(); + findings(Some(&probe), &mut found); + assert_eq!(found, ["stopped_public_dns_non_public_answer"]); + assert_eq!( + probe.answer_classes, + [ + "broadcast", + "cgnat", + "fake_ip", + "link_local", + "loopback", + "multicast", + "private" + ] + ); + let json = serde_json::to_string(&probe).unwrap(); + for address in addresses { + assert!(!json.contains(address)); + } + } + + #[tokio::test] + async fn public_answers_do_not_claim_http_reachability_or_raise_a_dns_failure() { + let probe = collect_with( + async { + Ok(vec![ + "192.0.2.7".parse().unwrap(), + "2001:db8::7".parse().unwrap(), + ]) + }, + Duration::from_secs(1), + ) + .await; + assert_eq!(probe.status, "resolved"); + assert_eq!(probe.answer_classes, ["public_ip"]); + let mut found = Vec::new(); + findings(Some(&probe), &mut found); + findings(None, &mut found); + assert!(found.is_empty()); + } + + #[tokio::test] + async fn lookup_failure_empty_answer_and_timeout_are_distinct_and_redacted() { + let probes = [ + collect_with( + async { Err(io::Error::other("private resolver failure payload")) }, + Duration::from_secs(1), + ) + .await, + collect_with(async { Ok(Vec::new()) }, Duration::from_secs(1)).await, + collect_with(std::future::pending(), Duration::ZERO).await, + ]; + let mut found = Vec::new(); + for probe in &probes { + assert!(probe.answer_classes.is_empty()); + assert!(!serde_json::to_string(probe).unwrap().contains("payload")); + findings(Some(probe), &mut found); + } + assert_eq!( + found, + [ + "stopped_public_dns_failed", + "stopped_public_dns_empty", + "stopped_public_dns_timed_out" + ] + ); + } +} diff --git a/src-tauri/src/environment/macos.rs b/src-tauri/src/environment/macos.rs new file mode 100644 index 0000000..87976d6 --- /dev/null +++ b/src-tauri/src/environment/macos.rs @@ -0,0 +1,57 @@ +//! Native DNS evidence on macOS, with addresses classified before logging. +use super::{classify_host, DnsServers, EnvironmentReport}; + +pub(super) fn parse_dns(text: &str, report: &mut EnvironmentReport) { + let mut in_primary = false; + let mut servers = Vec::new(); + for line in text.lines().map(str::trim) { + if line.starts_with("resolver #") { + if in_primary { + break; + } + in_primary = true; + } else if in_primary && line.starts_with("nameserver[") { + if let Some((_, address)) = line.split_once(':') { + servers.push(classify_host(address.trim())); + } + } + } + report.primary_dns_present = Some(!servers.is_empty()); + report.dns_servers.push(DnsServers { + interface: "primary_resolver".into(), + servers, + }); +} + +#[cfg(target_os = "macos")] +pub(super) async fn collect_platform( + report: &mut EnvironmentReport, + _tun_name: &str, +) -> super::PlatformData { + match super::run_command("/usr/sbin/scutil", &["--dns"]).await { + Ok(text) => parse_dns(&text, report), + Err(error) => report.collection_errors.push(error), + } + super::PlatformData::default() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn primary_dns_is_classified_without_logging_addresses_or_scoped_resolvers() { + let mut report = EnvironmentReport::default(); + parse_dns("resolver #1\n nameserver[0] : 127.0.0.1\n nameserver[1] : 198.18.0.2\nresolver #2\n nameserver[0] : 192.0.2.53\n", &mut report); + assert_eq!(report.primary_dns_present, Some(true)); + assert_eq!(report.dns_servers[0].servers, vec!["loopback", "fake_ip"]); + report.biflow.stack_phase = "stopped".into(); + assert!(super::super::findings(&report, None).contains(&"leftover_fake_ip_dns".into())); + let mut empty = EnvironmentReport::default(); + parse_dns("No DNS configuration available\n", &mut empty); + empty.biflow.stack_phase = "stopped".into(); + assert_eq!(empty.primary_dns_present, Some(false)); + assert!(super::super::findings(&empty, None) + .contains(&"stopped_without_primary_dns_resolver".into())); + } +} diff --git a/src-tauri/src/environment/mod.rs b/src-tauri/src/environment/mod.rs index 845bfb0..d8021e7 100644 --- a/src-tauri/src/environment/mod.rs +++ b/src-tauri/src/environment/mod.rs @@ -12,11 +12,14 @@ //! hosts. Ports are kept because they are what identifies a conflict. mod apps; +mod dns; // Each collector's parsers are unit-tested on every host; off their own // platform only the tests call them, so unused-item lints are expected there. #[cfg(any(target_os = "linux", test))] #[cfg_attr(not(target_os = "linux"), allow(dead_code, unused_imports))] mod linux; +#[cfg(any(target_os = "macos", test))] +mod macos; #[cfg(any(windows, test))] #[cfg_attr(not(windows), allow(dead_code, unused_imports))] mod windows; @@ -148,6 +151,10 @@ pub struct EnvironmentReport { pub adapters: Vec, pub default_routes: Vec, pub dns_servers: Vec, + /// Native primary resolver availability, distinct from configured servers. + pub primary_dns_present: Option, + /// Classified host resolver answers while stopped/paused; no target or IPs. + pub inactive_dns_probe: Option, pub system_proxy: Vec, pub env_proxy: Vec, pub vpn_processes: Vec, @@ -483,6 +490,9 @@ async fn collect(context: CollectContext) -> EnvironmentReport { |stack| format!("{:?}", stack.phase).to_ascii_lowercase(), ); report.biflow = biflow_info(context.config.as_ref(), &stack_phase); + if matches!(stack_phase.as_str(), "stopped" | "paused") { + report.inactive_dns_probe = Some(dns::collect().await); + } report.runtime = apps::runtime_info(context.stack.as_ref(), context.helper.as_ref()); report.install = apps::install_info(context.install_kind); let data_dir = context.data_dir.as_deref(); @@ -659,6 +669,14 @@ fn loopback_findings(report: &EnvironmentReport, findings: &mut Vec) { "client_port_refused_on_loopback:{}:{}", client.preset, client.port )); + if report.system_proxy.iter().any(|proxy| { + proxy.enabled && proxy.host == Some("loopback") && proxy.port == Some(client.port) + }) { + findings.push(format!( + "system_proxy_points_to_unavailable_client:{}", + client.port + )); + } } else if client.ipv4 && !client.ipv6 && v6_first { // `localhost:` tries ::1 first; an IPv4-only listener then // depends on the caller falling back to 127.0.0.1. @@ -1263,6 +1281,10 @@ fn idle_findings( config: Option<&AppConfig>, findings: &mut Vec, ) { + dns::findings(report.inactive_dns_probe.as_ref(), findings); + if report.primary_dns_present == Some(false) { + findings.push("stopped_without_primary_dns_resolver".into()); + } let is_tun = |name: &str| is_biflow_tun(config, name); { for adapter in report @@ -1314,6 +1336,11 @@ fn running_findings( config: Option<&AppConfig>, findings: &mut Vec, ) { + if report.adapters.iter().any(|adapter| { + is_biflow_tun(config, &adapter.name) && adapter.status.eq_ignore_ascii_case("disabled") + }) { + findings.push("active_stack_native_tun_disabled".into()); + } if let Some(stack) = &report.runtime.stack { for component in ["mihomo", "tun", "dns"] { if stack @@ -1671,10 +1698,12 @@ async fn run_command_timeout( #[cfg(target_os = "linux")] use linux::collect_platform; +#[cfg(target_os = "macos")] +use macos::collect_platform; #[cfg(windows)] use windows::collect_platform; -#[cfg(not(any(windows, target_os = "linux")))] +#[cfg(not(any(windows, target_os = "linux", target_os = "macos")))] #[allow(clippy::unused_async)] async fn collect_platform(_report: &mut EnvironmentReport, _tun_name: &str) -> PlatformData { PlatformData::default() @@ -1682,6 +1711,73 @@ async fn collect_platform(_report: &mut EnvironmentReport, _tun_name: &str) -> P #[cfg(test)] mod tests { + #[test] + fn inactive_dns_evidence_is_not_mistaken_for_an_active_tunnel_failure() { + let mut report = EnvironmentReport { + inactive_dns_probe: Some(dns::DnsProbe { + status: "resolved", + answer_classes: vec!["public_ip", "private"], + }), + ..EnvironmentReport::default() + }; + for phase in ["stopped", "paused"] { + report.biflow.stack_phase = phase.into(); + assert!( + findings(&report, None).contains(&"stopped_public_dns_non_public_answer".into()) + ); + } + for phase in ["running", "degraded", "error", "unknown"] { + report.biflow.stack_phase = phase.into(); + assert!(!findings(&report, None) + .iter() + .any(|finding| finding.starts_with("stopped_public_dns_"))); + } + } + + #[test] + fn an_enabled_proxy_to_a_dead_local_client_is_a_global_network_finding() { + let mut report = EnvironmentReport::default(); + report.loopback.clients.push(ClientLoopback { + preset: "hiddify", + port: 12334, + ipv4: false, + ipv6: false, + }); + report.system_proxy.push(ProxySetting { + source: "fixture".into(), + enabled: true, + scheme: Some("http".into()), + host: Some("loopback"), + port: Some(12334), + detail: None, + }); + let mut found = Vec::new(); + loopback_findings(&report, &mut found); + assert!(found.contains(&"system_proxy_points_to_unavailable_client:12334".into())); + report.system_proxy[0].enabled = false; + found.clear(); + loopback_findings(&report, &mut found); + assert!(!found + .iter() + .any(|finding| finding.starts_with("system_proxy_points_to_unavailable_client:"))); + } + #[test] + fn controller_running_cannot_hide_a_disabled_native_tunnel() { + let config = iran_split_config::AppConfig::default(); + let mut report = super::EnvironmentReport::default(); + report.biflow.stack_phase = "running".into(); + report.adapters.push(super::Adapter { + name: config.mihomo.tun_name.clone(), + description: "Meta Tunnel".into(), + status: "Disabled".into(), + kind: "tun", + }); + assert!(super::findings(&report, Some(&config)) + .contains(&"active_stack_native_tun_disabled".into())); + report.biflow.stack_phase = "stopped".into(); + assert!(!super::findings(&report, Some(&config)) + .contains(&"active_stack_native_tun_disabled".into())); + } #[test] fn active_stack_reports_missing_tun_and_dns_after_resume() { let mut report = EnvironmentReport::default(); diff --git a/src-tauri/tauri.conf.json b/src-tauri/tauri.conf.json index 21e58ec..6452a5d 100644 --- a/src-tauri/tauri.conf.json +++ b/src-tauri/tauri.conf.json @@ -1,7 +1,7 @@ { "$schema": "https://schema.tauri.app/config/2", "productName": "BiFlow", - "version": "6.2.85", + "version": "6.2.89", "identifier": "app.biflow.desktop", "build": { "beforeDevCommand": "pnpm bundle:check && pnpm --dir apps/desktop dev", diff --git a/version b/version index b37aa10..e8839d3 100644 --- a/version +++ b/version @@ -1 +1 @@ -6.2.85 +6.2.89 From 18e9126898b1fd13a726b93fe854eedd22de35d5 Mon Sep 17 00:00:00 2001 From: Omid Rade-PC Date: Sun, 8 Nov 2026 16:10:09 +0330 Subject: [PATCH 4/5] fix: compile the native macOS DNS collector --- AGENTS.md | 5 +++++ Cargo.lock | 30 ++++++++++++------------- Cargo.toml | 2 +- apps/desktop/package.json | 2 +- docs/adr/0134-native-network-release.md | 5 +++++ package.json | 2 +- src-tauri/src/environment/mod.rs | 7 ++++-- src-tauri/tauri.conf.json | 2 +- version | 2 +- 9 files changed, 35 insertions(+), 22 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index f6a24f4..da3a507 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -71,6 +71,11 @@ If a required command fails or emits a warning from project code, fix it in the ## Lessons +- A native collector's caller and shared command runner must have matching OS + coverage. Adding a macOS `scutil` caller while retaining a Windows/Linux-only + `run_command` cfg breaks the desktop on macOS even when the portable parser + tests and helper/core cross-Clippy pass. Include macOS in the runner and its + timeout constant's cfg policy; native desktop CI remains a required check. - Configured DNS addresses do not prove usable resolution. A detached host can have no owned tunnel or proxy while its original resolver returns private answers for a public control name. Record answer classes, empty responses, diff --git a/Cargo.lock b/Cargo.lock index 594cf21..4ef6628 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2046,7 +2046,7 @@ dependencies = [ [[package]] name = "iran-split-cli" -version = "6.2.89" +version = "6.2.90" dependencies = [ "async-trait", "chrono", @@ -2064,7 +2064,7 @@ dependencies = [ [[package]] name = "iran-split-clients" -version = "6.2.89" +version = "6.2.90" dependencies = [ "async-trait", "ipnet", @@ -2080,7 +2080,7 @@ dependencies = [ [[package]] name = "iran-split-config" -version = "6.2.89" +version = "6.2.90" dependencies = [ "hex", "rand 0.9.5", @@ -2093,7 +2093,7 @@ dependencies = [ [[package]] name = "iran-split-core" -version = "6.2.89" +version = "6.2.90" dependencies = [ "async-trait", "chrono", @@ -2110,7 +2110,7 @@ dependencies = [ [[package]] name = "iran-split-desktop" -version = "6.2.89" +version = "6.2.90" dependencies = [ "chrono", "dirs 6.0.0", @@ -2152,7 +2152,7 @@ dependencies = [ [[package]] name = "iran-split-helper" -version = "6.2.89" +version = "6.2.90" dependencies = [ "clap", "hex", @@ -2177,7 +2177,7 @@ dependencies = [ [[package]] name = "iran-split-helper-winacl" -version = "6.2.89" +version = "6.2.90" dependencies = [ "tokio", "windows", @@ -2185,7 +2185,7 @@ dependencies = [ [[package]] name = "iran-split-ipc" -version = "6.2.89" +version = "6.2.90" dependencies = [ "bytes", "chrono", @@ -2198,7 +2198,7 @@ dependencies = [ [[package]] name = "iran-split-mihomo" -version = "6.2.89" +version = "6.2.90" dependencies = [ "chrono", "futures-util", @@ -2221,7 +2221,7 @@ dependencies = [ [[package]] name = "iran-split-netstats" -version = "6.2.89" +version = "6.2.90" dependencies = [ "libc", "tempfile", @@ -2230,7 +2230,7 @@ dependencies = [ [[package]] name = "iran-split-platform-linux" -version = "6.2.89" +version = "6.2.90" dependencies = [ "async-trait", "chrono", @@ -2254,7 +2254,7 @@ dependencies = [ [[package]] name = "iran-split-platform-macos" -version = "6.2.89" +version = "6.2.90" dependencies = [ "async-trait", "chrono", @@ -2278,7 +2278,7 @@ dependencies = [ [[package]] name = "iran-split-platform-win" -version = "6.2.89" +version = "6.2.90" dependencies = [ "async-trait", "chrono", @@ -2301,7 +2301,7 @@ dependencies = [ [[package]] name = "iran-split-rules" -version = "6.2.89" +version = "6.2.90" dependencies = [ "async-trait", "chrono", @@ -2323,7 +2323,7 @@ dependencies = [ [[package]] name = "iran-split-taskbar" -version = "6.2.89" +version = "6.2.90" dependencies = [ "iran-split-config", "objc2", diff --git a/Cargo.toml b/Cargo.toml index ace1fd5..ee47bcc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,7 +19,7 @@ members = [ ] [workspace.package] -version = "6.2.89" +version = "6.2.90" edition = "2021" license = "MIT OR Apache-2.0" rust-version = "1.88" diff --git a/apps/desktop/package.json b/apps/desktop/package.json index dfa5953..551618b 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -1,6 +1,6 @@ { "name": "@iran-split/desktop", - "version": "6.2.89", + "version": "6.2.90", "private": true, "type": "module", "scripts": { diff --git a/docs/adr/0134-native-network-release.md b/docs/adr/0134-native-network-release.md index 34203d8..ab46761 100644 --- a/docs/adr/0134-native-network-release.md +++ b/docs/adr/0134-native-network-release.md @@ -57,3 +57,8 @@ flow also asserts absent traffic routes while paused. These tests do not stop the installed app or change the host network. The helper also passes Apple Silicon target Clippy from Windows using Zig for its C dependencies. Native end-to-end macOS behavior still requires validation on a Mac. + +The native desktop CI matrix also checks that the macOS collector can call the +shared bounded command runner. Portable DNS parser tests and helper/core +cross-checks do not compile this desktop boundary; both the runner and its +timeout policy must include macOS (6.2.90). diff --git a/package.json b/package.json index 6af43a6..7cafff5 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "iran-split-desktop-workspace", - "version": "6.2.89", + "version": "6.2.90", "private": true, "packageManager": "pnpm@9.0.1", "engines": { diff --git a/src-tauri/src/environment/mod.rs b/src-tauri/src/environment/mod.rs index d8021e7..4a4372d 100644 --- a/src-tauri/src/environment/mod.rs +++ b/src-tauri/src/environment/mod.rs @@ -43,7 +43,10 @@ use std::{ use tracing::{info, warn}; use uuid::Uuid; -#[cfg_attr(not(any(windows, target_os = "linux")), allow(dead_code))] +#[cfg_attr( + not(any(windows, target_os = "linux", target_os = "macos")), + allow(dead_code) +)] const COMMAND_TIMEOUT: Duration = Duration::from_secs(25); /// Service and scheduled-task names worth reporting. Same matching rules as @@ -1651,7 +1654,7 @@ fn host_findings( // Platform collectors. // --------------------------------------------------------------------------- -#[cfg(any(windows, target_os = "linux"))] +#[cfg(any(windows, target_os = "linux", target_os = "macos"))] async fn run_command(program: &str, args: &[&str]) -> Result { run_command_timeout(OsStr::new(program), args, COMMAND_TIMEOUT).await } diff --git a/src-tauri/tauri.conf.json b/src-tauri/tauri.conf.json index 6452a5d..dfb44fe 100644 --- a/src-tauri/tauri.conf.json +++ b/src-tauri/tauri.conf.json @@ -1,7 +1,7 @@ { "$schema": "https://schema.tauri.app/config/2", "productName": "BiFlow", - "version": "6.2.89", + "version": "6.2.90", "identifier": "app.biflow.desktop", "build": { "beforeDevCommand": "pnpm bundle:check && pnpm --dir apps/desktop dev", diff --git a/version b/version index e8839d3..064ddf9 100644 --- a/version +++ b/version @@ -1 +1 @@ -6.2.89 +6.2.90 From 749a35467e03cb2d5c97bcafdac1f1df056cb978 Mon Sep 17 00:00:00 2001 From: Omid Rade-PC Date: Sun, 8 Nov 2026 16:19:41 +0330 Subject: [PATCH 5/5] test: synchronize graceful-stop child readiness --- AGENTS.md | 5 +++++ Cargo.lock | 30 ++++++++++++------------- Cargo.toml | 2 +- apps/desktop/package.json | 2 +- crates/iran-split-helper/src/lib.rs | 30 ++++++++++++++++++------- docs/adr/0134-native-network-release.md | 5 +++++ package.json | 2 +- src-tauri/tauri.conf.json | 2 +- version | 2 +- 9 files changed, 52 insertions(+), 28 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index da3a507..dd12bf2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -71,6 +71,11 @@ If a required command fails or emits a warning from project code, fix it in the ## Lessons +- A fixed sleep does not prove a child has installed its signal handler on a + busy native runner. The graceful-stop fixture must read an explicit flushed + readiness message after handler installation, keep the child alive through + ignored signals, and assert the eventual SIGKILL status. Preserve the real + graceful wait assertion; do not rerun the race or relax it into a false pass. - A native collector's caller and shared command runner must have matching OS coverage. Adding a macOS `scutil` caller while retaining a Windows/Linux-only `run_command` cfg breaks the desktop on macOS even when the portable parser diff --git a/Cargo.lock b/Cargo.lock index 4ef6628..c332b48 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2046,7 +2046,7 @@ dependencies = [ [[package]] name = "iran-split-cli" -version = "6.2.90" +version = "6.2.91" dependencies = [ "async-trait", "chrono", @@ -2064,7 +2064,7 @@ dependencies = [ [[package]] name = "iran-split-clients" -version = "6.2.90" +version = "6.2.91" dependencies = [ "async-trait", "ipnet", @@ -2080,7 +2080,7 @@ dependencies = [ [[package]] name = "iran-split-config" -version = "6.2.90" +version = "6.2.91" dependencies = [ "hex", "rand 0.9.5", @@ -2093,7 +2093,7 @@ dependencies = [ [[package]] name = "iran-split-core" -version = "6.2.90" +version = "6.2.91" dependencies = [ "async-trait", "chrono", @@ -2110,7 +2110,7 @@ dependencies = [ [[package]] name = "iran-split-desktop" -version = "6.2.90" +version = "6.2.91" dependencies = [ "chrono", "dirs 6.0.0", @@ -2152,7 +2152,7 @@ dependencies = [ [[package]] name = "iran-split-helper" -version = "6.2.90" +version = "6.2.91" dependencies = [ "clap", "hex", @@ -2177,7 +2177,7 @@ dependencies = [ [[package]] name = "iran-split-helper-winacl" -version = "6.2.90" +version = "6.2.91" dependencies = [ "tokio", "windows", @@ -2185,7 +2185,7 @@ dependencies = [ [[package]] name = "iran-split-ipc" -version = "6.2.90" +version = "6.2.91" dependencies = [ "bytes", "chrono", @@ -2198,7 +2198,7 @@ dependencies = [ [[package]] name = "iran-split-mihomo" -version = "6.2.90" +version = "6.2.91" dependencies = [ "chrono", "futures-util", @@ -2221,7 +2221,7 @@ dependencies = [ [[package]] name = "iran-split-netstats" -version = "6.2.90" +version = "6.2.91" dependencies = [ "libc", "tempfile", @@ -2230,7 +2230,7 @@ dependencies = [ [[package]] name = "iran-split-platform-linux" -version = "6.2.90" +version = "6.2.91" dependencies = [ "async-trait", "chrono", @@ -2254,7 +2254,7 @@ dependencies = [ [[package]] name = "iran-split-platform-macos" -version = "6.2.90" +version = "6.2.91" dependencies = [ "async-trait", "chrono", @@ -2278,7 +2278,7 @@ dependencies = [ [[package]] name = "iran-split-platform-win" -version = "6.2.90" +version = "6.2.91" dependencies = [ "async-trait", "chrono", @@ -2301,7 +2301,7 @@ dependencies = [ [[package]] name = "iran-split-rules" -version = "6.2.90" +version = "6.2.91" dependencies = [ "async-trait", "chrono", @@ -2323,7 +2323,7 @@ dependencies = [ [[package]] name = "iran-split-taskbar" -version = "6.2.90" +version = "6.2.91" dependencies = [ "iran-split-config", "objc2", diff --git a/Cargo.toml b/Cargo.toml index ee47bcc..d019f7e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,7 +19,7 @@ members = [ ] [workspace.package] -version = "6.2.90" +version = "6.2.91" edition = "2021" license = "MIT OR Apache-2.0" rust-version = "1.88" diff --git a/apps/desktop/package.json b/apps/desktop/package.json index 551618b..409c4f3 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -1,6 +1,6 @@ { "name": "@iran-split/desktop", - "version": "6.2.90", + "version": "6.2.91", "private": true, "type": "module", "scripts": { diff --git a/crates/iran-split-helper/src/lib.rs b/crates/iran-split-helper/src/lib.rs index cdb3193..ca0de19 100644 --- a/crates/iran-split-helper/src/lib.rs +++ b/crates/iran-split-helper/src/lib.rs @@ -1749,14 +1749,27 @@ mod tests { #[cfg(unix)] #[tokio::test] async fn stop_waits_for_a_graceful_exit_before_killing() { - // Perl installs the ignore handler before it sleeps. A shell `trap` - // loses the race: stop's SIGTERM arrives before the trap is parsed. - let child = Command::new("perl") - .args(["-e", "$SIG{TERM} = 'IGNORE'; sleep 30;"]) + use std::os::unix::process::ExitStatusExt; + + // Readiness follows handler installation. A fixed startup sleep can + // still send SIGTERM before a busy native runner has started Perl. + let mut child = Command::new("perl") + .args([ + "-e", + "$| = 1; $SIG{TERM} = 'IGNORE'; print qq(ready\\n); while (1) { sleep 30; }", + ]) + .stdout(Stdio::piped()) .kill_on_drop(true) .spawn() .expect("process that ignores SIGTERM"); - tokio::time::sleep(Duration::from_millis(200)).await; + let stdout = child.stdout.take().expect("readiness pipe"); + let mut readiness = BufReader::new(stdout).lines(); + let ready = tokio::time::timeout(PROCESS_STOP_TIMEOUT, readiness.next_line()) + .await + .expect("child readiness deadline") + .expect("read child readiness") + .expect("child stayed alive until ready"); + assert_eq!(ready, "ready"); let mut managed = ManagedChild { child, generation_id: Uuid::new_v4(), @@ -1771,9 +1784,10 @@ mod tests { elapsed >= GRACEFUL_STOP_TIMEOUT, "stop killed the process before the graceful wait: {elapsed:?}" ); - assert!( - elapsed < PROCESS_STOP_TIMEOUT, - "stop waited for the force-kill timeout: {elapsed:?}" + assert_eq!( + managed.child.wait().await.expect("reaped child").signal(), + Some(libc::SIGKILL), + "the ready child must survive SIGTERM until the bounded kill fallback" ); } diff --git a/docs/adr/0134-native-network-release.md b/docs/adr/0134-native-network-release.md index ab46761..ad6a6a9 100644 --- a/docs/adr/0134-native-network-release.md +++ b/docs/adr/0134-native-network-release.md @@ -62,3 +62,8 @@ The native desktop CI matrix also checks that the macOS collector can call the shared bounded command runner. Portable DNS parser tests and helper/core cross-checks do not compile this desktop boundary; both the runner and its timeout policy must include macOS (6.2.90). + +The Unix graceful-stop fixture synchronizes on a flushed child readiness message +after signal-handler installation, rather than a fixed startup delay. It verifies +both the actual graceful wait and the final SIGKILL fallback, keeping native +runner scheduling out of the ownership assertions (6.2.91). diff --git a/package.json b/package.json index 7cafff5..ecf9153 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "iran-split-desktop-workspace", - "version": "6.2.90", + "version": "6.2.91", "private": true, "packageManager": "pnpm@9.0.1", "engines": { diff --git a/src-tauri/tauri.conf.json b/src-tauri/tauri.conf.json index dfb44fe..afe9c0f 100644 --- a/src-tauri/tauri.conf.json +++ b/src-tauri/tauri.conf.json @@ -1,7 +1,7 @@ { "$schema": "https://schema.tauri.app/config/2", "productName": "BiFlow", - "version": "6.2.90", + "version": "6.2.91", "identifier": "app.biflow.desktop", "build": { "beforeDevCommand": "pnpm bundle:check && pnpm --dir apps/desktop dev", diff --git a/version b/version index 064ddf9..61d04b7 100644 --- a/version +++ b/version @@ -1 +1 @@ -6.2.90 +6.2.91