Fix TUN no-internet: terminate helper cleanly and harden bypass routes

The helper logged "exiting" but never terminated: the WinTun blocking
receive runs on a thread that task.abort() cannot cancel, so it kept the
ostp_tun adapter (and its metric-0 default route) alive and hung the tokio
runtime as a zombie. The next connect then faced two competing default
routes and failed to install the per-server /32 bypass, so the client's own
handshake packets looped back into the dead tunnel — every OSTP handshake
timed out and there was no internet.

- ostp-tun-helper: std::process::exit(0) after run_server returns so the
  kernel reclaims the adapter and all routes bound to it.
- ostp-tun/windows_route: dedupe bypass IPs, purge any stale /32 for the
  dest before adding (enumerate + delete), and log add failures at warn!
  instead of debug! so the cause is visible in the INFO-level helper log.
- ostp-tun/windows: keep .destination() LUID default route (reliable
  capture) alongside the racy friendly-name route; retry create() through
  the transient ERROR_INVALID_PARAMETER window.
- ostp-client: wire BridgeMetrics.connection_state through runner and
  inbounds so the GUI reflects connecting/connected/disconnected.
- ostp-gui: parse JSONC config (strip // and /* */) in the settings view.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Cargo.lock

@@ -1591,12 +1591,13 @@ name = "ostp-tun-helper"
 version = "0.3.12"
 dependencies = [
  "anyhow",
- "chrono",
+ "hex",
  "ostp-client",
  "portable-atomic",
  "serde",
  "serde_json",
  "tokio",
+ "tracing",
  "winres",
 ]
 
@@ -2589,6 +2590,7 @@ dependencies = [
  "sharded-slab",
  "smallvec",
  "thread_local",
+ "time",
  "tracing",
  "tracing-core",
  "tracing-log",

ostp-client/src/runner.rs

@@ -9,12 +9,18 @@ use crate::tunnel::router::Router;
 
 pub async fn run_client_core(
     config: ClientConfig,
-    _metrics: Arc<crate::bridge::BridgeMetrics>,
+    metrics: Arc<crate::bridge::BridgeMetrics>,
     mut shutdown_rx_ext: watch::Receiver<bool>,
     _config_rx: Option<watch::Receiver<ClientConfig>>,
 ) -> Result<()> {
+    use portable_atomic::Ordering;
     tracing::info!("starting client core");
 
+    // Report "connecting" until an inbound has successfully bound. Each inbound
+    // flips this to 2 (connected) once it is ready; if they all fail, the
+    // select! below returns and we reset to 0 (disconnected).
+    metrics.connection_state.store(1, Ordering::Relaxed);
+
     let router = Arc::new(Router::new(config.routing.clone()));
     let balancer = Arc::new(Balancer::new(&config));
     
@@ -29,6 +35,7 @@ pub async fn run_client_core(
         let outbound_manager_clone = outbound_manager.clone();
         let shutdown_rx = shutdown_rx_ext.clone();
         let config_clone = config.clone();
+        let metrics_clone = metrics.clone();
 
         match inbound.clone() {
             InboundConfig::Tun { .. } => {
@@ -39,6 +46,7 @@ pub async fn run_client_core(
                         router_clone,
                         outbound_manager_clone,
                         shutdown_rx,
+                        metrics_clone,
                     ).await {
                         tracing::error!("TUN inbound failed: {}", e);
                     }
@@ -52,6 +60,7 @@ pub async fn run_client_core(
                         router_clone,
                         outbound_manager_clone,
                         shutdown_rx,
+                        metrics_clone,
                     ).await {
                         tracing::error!("SOCKS inbound failed: {}", e);
                     }
@@ -69,5 +78,6 @@ pub async fn run_client_core(
         }
     }
 
+    metrics.connection_state.store(0, Ordering::Relaxed);
     Ok(())
 }

ostp-client/src/tunnel/inbounds/local_proxy.rs

@@ -13,7 +13,9 @@ pub async fn run_socks_inbound(
     router: Arc<Router>,
     outbound_manager: Arc<OutboundManager>,
     mut shutdown: watch::Receiver<bool>,
+    metrics: Arc<crate::bridge::BridgeMetrics>,
 ) -> Result<()> {
+    use portable_atomic::Ordering;
     let InboundConfig::LocalProxy { tag, protocol, listen, port, set_system_proxy } = inbound_config else {
         return Err(anyhow!("Invalid config for LocalProxy inbound"));
     };
@@ -30,6 +32,10 @@ pub async fn run_socks_inbound(
 
     let listener = TcpListener::bind(&bind_addr).await?;
 
+    // Listener bound successfully — the proxy is ready to accept connections.
+    metrics.connection_state.store(2, Ordering::Relaxed);
+    tracing::info!("{} proxy inbound ready on {}, connection state = connected", protocol, bind_addr);
+
     loop {
         tokio::select! {
             _ = shutdown.changed() => {

ostp-client/src/tunnel/inbounds/tun.rs

@@ -13,9 +13,11 @@ pub async fn run_tun_inbound(
     router: Arc<Router>,
     outbound_manager: Arc<OutboundManager>,
     mut shutdown: watch::Receiver<bool>,
+    metrics: Arc<crate::bridge::BridgeMetrics>,
 ) -> Result<()> {
 
     use netstack_smoltcp::StackBuilder;
+    use portable_atomic::Ordering;
     use tokio::io::{AsyncReadExt, AsyncWriteExt};
     use futures::{StreamExt, SinkExt};
 
@@ -179,6 +181,11 @@ pub async fn run_tun_inbound(
         }
     };
 
+    // TUN device is up and the default route has been installed inside
+    // OstpTunInterface::create — the tunnel is now carrying traffic.
+    metrics.connection_state.store(2, Ordering::Relaxed);
+    tracing::info!("TUN inbound ready, connection state = connected");
+
     // ── TCP Handler ──
     let outbound_manager_tcp = outbound_manager.clone();
     let router_tcp = router.clone();
@@ -296,6 +303,7 @@ pub async fn run_tun_inbound(
     _router: Arc<Router>,
     _outbound_manager: Arc<OutboundManager>,
     _shutdown: watch::Receiver<bool>,
+    _metrics: Arc<crate::bridge::BridgeMetrics>,
 ) -> Result<()> {
     Err(anyhow!("TUN is only supported on Windows and Linux"))
 }

ostp-core/src/protocol.rs

@@ -265,7 +265,7 @@ impl ProtocolMachine {
                 noise_len, raw_vec.len() - 6
             )));
         }
-        tracing::info!("handle_inbound: raw_vec.len()={}, noise_len={}, raw_vec[0..6]={:?}", raw_vec.len(), noise_len, &raw_vec[0..6]);
+        tracing::debug!("handle_inbound: raw_vec.len()={}, noise_len={}, raw_vec[0..6]={:?}", raw_vec.len(), noise_len, &raw_vec[0..6]);
         
         let mut read_out = vec![0_u8; 1024];
         let n = self.noise.read_handshake(&raw_vec[6..6 + noise_len], &mut read_out).map_err(|e| {

ostp-gui/src-tauri/Cargo.lock

@@ -4544,6 +4544,7 @@ dependencies = [
  "sharded-slab",
  "smallvec",
  "thread_local",
+ "time",
  "tracing",
  "tracing-core",
  "tracing-log",

ostp-gui/src/main.js

@@ -6,6 +6,33 @@ if (window.__TAURI__?.core) {
   invoke = window.__TAURI__.core.invoke;
 }
 
+// ── JSONC parsing ─────────────────────────────────────────────────────────────
+// config.json is JSONC: save_config prepends a `// OSTP Configuration` header
+// and the default template carries comments. Strip // line and /* */ block
+// comments before JSON.parse, while preserving them inside string literals
+// (config values contain URLs like https:// and paths like ./wintun.dll).
+function parseJsonc(raw) {
+  let out = '';
+  let inStr = false, inLine = false, inBlock = false, escaped = false;
+  for (let i = 0; i < raw.length; i++) {
+    const c = raw[i], n = raw[i + 1];
+    if (inLine) { if (c === '\n') { inLine = false; out += c; } continue; }
+    if (inBlock) { if (c === '*' && n === '/') { inBlock = false; i++; } continue; }
+    if (inStr) {
+      out += c;
+      if (escaped) escaped = false;
+      else if (c === '\\') escaped = true;
+      else if (c === '"') inStr = false;
+      continue;
+    }
+    if (c === '"') { inStr = true; out += c; continue; }
+    if (c === '/' && n === '/') { inLine = true; i++; continue; }
+    if (c === '/' && n === '*') { inBlock = true; i++; continue; }
+    out += c;
+  }
+  return JSON.parse(out);
+}
+
 // ── State ────────────────────────────────────────────────────────────────────
 let appState    = 'disconnected'; // 'disconnected' | 'connecting' | 'connected'
 let pollTimer   = null;
@@ -384,7 +411,7 @@ async function handleToggle() {
   if (appState === 'disconnected') {
     try {
       const raw = await invoke('get_config');
-      const cfg = JSON.parse(raw);
+      const cfg = parseJsonc(raw);
       serverAddr = cfg.server || '';
     } catch { serverAddr = ''; }
 
@@ -434,7 +461,7 @@ function showScreen(name) {
 async function loadConfigIntoForm() {
   try {
     const raw = await invoke('get_config');
-    rawConfig = JSON.parse(raw);
+    rawConfig = parseJsonc(raw);
     const c = rawConfig.mode === 'client' ? rawConfig : null;
     if (!c) return;
 
@@ -695,7 +722,7 @@ window.addEventListener('DOMContentLoaded', async () => {
   // Auto-connect on startup
   try {
     const raw = await invoke('get_config');
-    rawConfig = JSON.parse(raw);
+    rawConfig = parseJsonc(raw);
     if (rawConfig?.gui?.autoconnect) {
       setTimeout(() => {
         if (appState === 'disconnected') handleToggle();
@@ -716,7 +743,7 @@ window.addEventListener('DOMContentLoaded', async () => {
       
       try {
         const raw = await invoke('get_config');
-        rawConfig = JSON.parse(raw);
+        rawConfig = parseJsonc(raw);
       } catch {
         showToast('Please save your config first', 'error');
         return;

ostp-tun-helper/src/main.rs

@@ -74,7 +74,19 @@ async fn main() -> Result<()> {
         tracing::error!("fatal: {}", e);
     }
     tracing::info!("helper exiting");
-    Ok(())
+
+    // The WinTun blocking `receive` runs on a thread that `task.abort()` cannot
+    // cancel, so it keeps the adapter handle — and the default route bound to it
+    // — alive and prevents the tokio runtime from shutting down. Without this the
+    // process lingers as a zombie: `ostp_tun` stays Up, its metric-0 default
+    // route competes with the physical one, and the NEXT connect fails to install
+    // the server bypass route, so traffic loops back into a dead tunnel (no
+    // internet). The GUI launches a fresh helper for every connect, so this
+    // process has no more work once run_server returns. Give the synchronous
+    // route/firewall teardown a moment to finish, then force the process to exit
+    // so the kernel reclaims the adapter and every route bound to it.
+    tokio::time::sleep(Duration::from_millis(800)).await;
+    std::process::exit(0);
 }
 
 async fn run_server(expected_token: String, port: u16) -> Result<()> {

ostp-tun/src/windows.rs

@@ -68,36 +68,73 @@ pub async fn create(opts: OstpTunOptions) -> Result<OstpTunInterface> {
         .tun_name("ostp_tun")
         .address((10, 1, 0, 2))
         .netmask((255, 255, 255, 0))
+        // `.destination()` makes the `tun` crate install the default route via
+        // the adapter's LUID (available immediately after creation). This is
+        // the RELIABLE default route that captures traffic — the manual
+        // add_ipv4_route below depends on a friendly-name interface-index
+        // lookup that races on a freshly created adapter. Keep both: the LUID
+        // route guarantees connectivity even when the index lookup is slow.
+        // The retry loop around tun::create absorbs the transient
+        // ERROR_INVALID_PARAMETER (os error 87) this call can hit during the
+        // post-creation registration window.
         .destination((10, 1, 0, 1))
         .mtu(opts.mtu)
         .up();
 
-    let dev = tun::create(&tun_cfg).map_err(|e| anyhow!("Failed to create TUN device: {}", e))?;
+    // The IpHelper calls the `tun` crate performs right after Adapter::create
+    // (set address / mtu) can transiently fail with ERROR_INVALID_PARAMETER
+    // (os error 87) when the freshly created interface is not yet registered
+    // in the IP stack. Retry a few times; on retry the crate reuses the
+    // existing adapter via Adapter::open.
+    let dev = {
+        let mut attempt = 0;
+        loop {
+            attempt += 1;
+            match tun::create(&tun_cfg) {
+                Ok(d) => break d,
+                Err(e) if attempt < 5 => {
+                    tracing::warn!(
+                        "TUN device creation attempt {}/5 failed: {} — retrying in 300ms",
+                        attempt, e
+                    );
+                    tokio::time::sleep(std::time::Duration::from_millis(300)).await;
+                }
+                Err(e) => return Err(anyhow!("Failed to create TUN device after {} attempts: {}", attempt, e)),
+            }
+        }
+    };
     let dev = tun::AsyncDevice::new(dev).map_err(|e| anyhow!("TUN device async failed: {}", e))?;
     tracing::info!("TUN device 'ostp_tun' created.");
 
     let current_exe = std::env::current_exe()?.to_string_lossy().into_owned();
 
+    // A freshly created WinTun adapter can take several seconds to appear in
+    // GetAdaptersAddresses (it only shows up once it has an operational IPv4
+    // binding). The default route via the TUN is what actually captures
+    // traffic, so this lookup is critical — give it a generous window
+    // (~15s) before giving up rather than the previous 2s.
     let mut tun_index = None;
-    for _ in 0..20 {
+    for _ in 0..75 {
         if let Some(idx) = windows_route::sys::get_interface_index("ostp_tun") {
             tun_index = Some(idx);
             break;
         }
-        tokio::time::sleep(std::time::Duration::from_millis(100)).await;
+        tokio::time::sleep(std::time::Duration::from_millis(200)).await;
     }
 
     if let Some(idx) = tun_index {
-        let _ = windows_route::sys::add_ipv4_route(
+        match windows_route::sys::add_ipv4_route(
             std::net::Ipv4Addr::new(0, 0, 0, 0),
             std::net::Ipv4Addr::new(0, 0, 0, 0),
             std::net::Ipv4Addr::new(10, 1, 0, 1),
             idx,
             5,
-        );
-        tracing::info!("Default route via TUN (if_index={idx}, metric=5) added.");
+        ) {
+            Ok(()) => tracing::info!("Default route via TUN (if_index={idx}, metric=5) added."),
+            Err(e) => tracing::error!("Failed to add default route via TUN (if_index={idx}): {e} — traffic will NOT be captured."),
+        }
     } else {
-        tracing::warn!("Could not find ostp_tun index in routing table — traffic may not be captured.");
+        tracing::error!("Could not find ostp_tun index in routing table after 15s — traffic will NOT be captured.");
     }
 
     let exe1 = current_exe.clone();

ostp-tun/src/windows_route.rs

@@ -168,6 +168,34 @@ pub mod sys {
         }
     }
 
+    /// Delete every routing-table entry whose destination is `dest`/`mask`,
+    /// regardless of its gateway or interface. Used to purge stale bypass routes
+    /// left by a previous session (possibly pointing at an old gateway after a
+    /// network change) so a fresh, correct one can be installed.
+    pub fn delete_routes_for_dest(dest: Ipv4Addr, mask: Ipv4Addr) {
+        unsafe {
+            let mut size: ULONG = 0;
+            if GetIpForwardTable(ptr::null_mut(), &mut size, 0) != ERROR_INSUFFICIENT_BUFFER {
+                return;
+            }
+            let mut buf: Vec<u8> = vec![0; size as usize];
+            let table = buf.as_mut_ptr() as *mut MIB_IPFORWARDTABLE;
+            if GetIpForwardTable(table, &mut size, 0) != NO_ERROR {
+                return;
+            }
+            let want_dest = ipv4_to_dword(dest);
+            let want_mask = ipv4_to_dword(mask);
+            let entries =
+                std::slice::from_raw_parts_mut((*table).table.as_mut_ptr(), (*table).dwNumEntries as usize);
+            for row in entries {
+                if row.dwForwardDest == want_dest && row.dwForwardMask == want_mask {
+                    // Delete the exact existing row (its own nexthop/ifindex).
+                    let _ = DeleteIpForwardEntry(row);
+                }
+            }
+        }
+    }
+
     /// Add bypass routes for a list of resolved IP addresses (typically from exclusion config).
     /// Each IP gets a /32 host route via the physical gateway so it bypasses the TUN.
     /// Returns list of (ip, gw, if_index) that were successfully added, for later cleanup.
@@ -178,15 +206,24 @@ pub mod sys {
         metric: u32,
     ) -> Vec<(Ipv4Addr, Ipv4Addr, u32)> {
         let mut added = Vec::new();
+        let mut seen = std::collections::HashSet::new();
+        let mask = Ipv4Addr::new(255, 255, 255, 255);
         for &ip in ips {
-            let mask = Ipv4Addr::new(255, 255, 255, 255);
+            // The server IP is passed both as server_ip and inside bypass_ips, so
+            // dedupe to avoid a guaranteed "already exists" failure on the second add.
+            if !seen.insert(ip) {
+                continue;
+            }
+            // Purge any pre-existing /32 for this dest (e.g. a stale route via an
+            // old gateway from a previous session) so CreateIpForwardEntry below
+            // installs the correct one instead of failing with ERROR_OBJECT_ALREADY_EXISTS.
+            delete_routes_for_dest(ip, mask);
             match add_ipv4_route(ip, mask, gw, if_index, metric) {
                 Ok(()) => {
                     added.push((ip, gw, if_index));
                 }
                 Err(e) => {
-                    // 87 = ERROR_INVALID_PARAMETER (route may already exist)
-                    tracing::debug!("bypass route add {ip}/32 via {gw}: {e}");
+                    tracing::warn!("bypass route add {ip}/32 via {gw} (if {if_index}) failed: {e}");
                 }
             }
         }