From 66368c9d0fccb6ece327754d7a4823f1640d27c9 Mon Sep 17 00:00:00 2001 From: ospab Date: Fri, 7 Aug 2026 22:24:40 +0300 Subject: [PATCH] fix(gui): helper task checked only its name, not the exe it points at MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Scheduled Task stores an absolute path. Checking that a task named "OSTP TUN Helper" exists said nothing about whether its still points at the helper we are about to run, and the paths do drift: a dev build registers target\debug\ostp-tun-helper.exe, an installer registers Program Files, and moving or reinstalling the app leaves the old path behind. That failed silently in the worst way. schtasks /Run reports success for merely ACCEPTING the launch request — a task whose exe no longer exists fails afterwards, out of band, with nothing returned to us. So launch_as_admin returned Ok, and the caller then sat in its 60-second connect loop before reporting "Timeout connecting to helper." On every connect, permanently, with no way out except deleting the task by hand. The check now reads the registered back and compares it to the exe, re-registering through the existing /F overwrite when they differ: one consent prompt, once, instead of a permanent silent breakage. The path is read via /Query /XML rather than /FO LIST /V because the list format's field labels are localized — "Task To Run" is "Задача для запуска" on a Russian Windows — while XML tag names are not. schtasks emits UTF-16LE with a BOM there, which is decoded explicitly, with UTF-8 tolerated as a fallback. Both paths are canonicalized before comparison so casing, `..` and 8.3 short names do not read as a mismatch; a path that cannot be canonicalized no longer exists, which is itself grounds to re-register. --- ostp-gui/src-tauri/src/lib.rs | 80 ++++++++++++++++++++++++++++++----- 1 file changed, 70 insertions(+), 10 deletions(-) diff --git a/ostp-gui/src-tauri/src/lib.rs b/ostp-gui/src-tauri/src/lib.rs index df3d09f..4f53689 100644 --- a/ostp-gui/src-tauri/src/lib.rs +++ b/ostp-gui/src-tauri/src/lib.rs @@ -849,14 +849,74 @@ fn xml_escape(s: &str) -> String { .replace('\'', "'") } -/// Whether the elevated-launch Scheduled Task already exists. +/// Reverse of [`xml_escape`]. `&` must be undone last or `&lt;` would +/// come back as `<`. #[cfg(target_os = "windows")] -fn helper_task_exists() -> bool { - quiet_command("schtasks") - .args(["/Query", "/TN", HELPER_TASK_NAME]) +fn xml_unescape(s: &str) -> String { + s.replace(""", "\"") + .replace("'", "'") + .replace("<", "<") + .replace(">", ">") + .replace("&", "&") +} + +/// The exe path currently baked into the registered task, if any. +/// +/// Queried as XML rather than `/FO LIST /V`: the list format's field labels are +/// localized (on a Russian Windows "Task To Run" is "Задача для запуска"), +/// whereas XML tag names are fixed. schtasks writes UTF-16LE with a BOM here, +/// but tolerate UTF-8 in case that ever changes. +#[cfg(target_os = "windows")] +fn helper_task_command() -> Option { + let out = quiet_command("schtasks") + .args(["/Query", "/TN", HELPER_TASK_NAME, "/XML"]) .output() - .map(|o| o.status.success()) - .unwrap_or(false) + .ok()?; + if !out.status.success() { + return None; + } + + let text = if out.stdout.starts_with(&[0xFF, 0xFE]) { + let units: Vec = out.stdout[2..] + .chunks_exact(2) + .map(|c| u16::from_le_bytes([c[0], c[1]])) + .collect(); + String::from_utf16_lossy(&units) + } else { + String::from_utf8_lossy(&out.stdout).into_owned() + }; + + let start = text.find("")? + "".len(); + let end = text[start..].find("")? + start; + Some(xml_unescape(text[start..end].trim())) +} + +/// Whether a task is registered AND still points at the exe we are about to run. +/// +/// The path matters as much as the name. A task registered by a dev build (or +/// by an install that has since moved) keeps its original ``, and +/// `schtasks /Run` reports success merely for *accepting* the request — a task +/// whose exe no longer exists fails asynchronously and silently. Trusting the +/// name alone therefore bought a 60-second "Timeout connecting to helper" on +/// every single connect, permanently, until the task was deleted by hand. +/// Re-registering costs one consent prompt and fixes it for good. +#[cfg(target_os = "windows")] +fn helper_task_matches(exe: &std::path::Path) -> bool { + let Some(registered) = helper_task_command() else { + return false; + }; + let registered = registered.trim().trim_matches('"'); + + // Canonicalize both sides when possible so `..`, short 8.3 names and + // casing differences do not read as a mismatch. A missing file cannot be + // canonicalized — which is itself a mismatch worth re-registering over. + match ( + std::fs::canonicalize(registered), + std::fs::canonicalize(exe), + ) { + (Ok(a), Ok(b)) => a == b, + _ => registered.eq_ignore_ascii_case(&exe.display().to_string()), + } } /// Register the Scheduled Task. This is the ONLY step that needs elevation, and @@ -966,10 +1026,10 @@ fn install_helper_task(exe: &std::path::Path) -> anyhow::Result<()> { Err(e) => anyhow::bail!("could not run powershell to register the task: {e}"), } - if helper_task_exists() { + if helper_task_matches(exe) { Ok(()) } else { - anyhow::bail!("schtasks reported success but the task is not present") + anyhow::bail!("schtasks reported success but the task does not point at {}", exe.display()) } } @@ -994,12 +1054,12 @@ fn launch_as_admin(exe: &std::path::PathBuf, token: &str, port: u16) -> anyhow:: let wrote_args = std::fs::write(&args_file, payload.to_string()).is_ok(); if wrote_args { - if !helper_task_exists() { + if !helper_task_matches(exe) { if let Err(e) = install_helper_task(exe) { eprintln!("[OSTP] could not register the helper task ({e}); falling back to a direct elevated launch"); } } - if helper_task_exists() { + if helper_task_matches(exe) { let run = quiet_command("schtasks") .args(["/Run", "/TN", HELPER_TASK_NAME]) .output();