mirror of https://github.com/ospab/ostp.git
fix(gui): relax the helper-task check, and make its decision observable
The installer's task is correct on the reporting machine — right path, right principal, and running it by hand starts the helper with no prompt — yet the app still fell back to an elevated launch on every connect. The exact-path comparison is the only thing that can reject it, and it was never worth its strictness: what the check exists to catch is a task left pointing at a binary that is gone, since `schtasks /Run` reports success merely for accepting such a request and the app would then wait on a helper that never starts. Testing that the registered file exists and is the helper catches exactly that case, without charging a prompt for any other difference. The reason this took several rounds to narrow down is the real defect: every failure on this path went to `eprintln!`, and the GUI is a windowed binary with no console, so the one decision that determines whether the user gets a consent prompt was completely unobservable on their machine. It now appends to %LOCALAPPDATA%\OSTP\helper-launch.log — what was registered, whether it exists, what schtasks /Run answered, and whether the fallback was taken.
This commit is contained in:
parent
1ab55fcdd0
commit
b18852ac07
|
|
@ -962,19 +962,47 @@ fn helper_task_command() -> Option<String> {
|
||||||
#[cfg(target_os = "windows")]
|
#[cfg(target_os = "windows")]
|
||||||
fn helper_task_matches(exe: &std::path::Path) -> bool {
|
fn helper_task_matches(exe: &std::path::Path) -> bool {
|
||||||
let Some(registered) = helper_task_command() else {
|
let Some(registered) = helper_task_command() else {
|
||||||
|
diag_log("task: schtasks /Query returned nothing usable — no task, or its XML had no <Command>");
|
||||||
return false;
|
return false;
|
||||||
};
|
};
|
||||||
let registered = registered.trim().trim_matches('"');
|
let registered = registered.trim().trim_matches('"');
|
||||||
|
let path = std::path::Path::new(registered);
|
||||||
|
|
||||||
// Canonicalize both sides when possible so `..`, short 8.3 names and
|
// Requiring the registered path to equal the helper we would have launched
|
||||||
// casing differences do not read as a mismatch. A missing file cannot be
|
// was too strict, and bought nothing. What the check exists to catch is a
|
||||||
// canonicalized — which is itself a mismatch worth re-registering over.
|
// task left pointing at a binary that is gone — `schtasks /Run` reports
|
||||||
match (
|
// success merely for accepting such a request, so the app would then wait
|
||||||
std::fs::canonicalize(registered),
|
// on a helper that never starts. Testing that the file exists catches
|
||||||
std::fs::canonicalize(exe),
|
// exactly that, while a task registered by the installer against an
|
||||||
) {
|
// equivalent copy of the helper no longer costs the user a prompt.
|
||||||
(Ok(a), Ok(b)) => a == b,
|
let same_program = path
|
||||||
_ => registered.eq_ignore_ascii_case(&exe.display().to_string()),
|
.file_name()
|
||||||
|
.map(|n| n.eq_ignore_ascii_case(HELPER_EXE_NAME))
|
||||||
|
.unwrap_or(false);
|
||||||
|
let exists = path.is_file();
|
||||||
|
|
||||||
|
diag_log(&format!(
|
||||||
|
"task: registered={registered:?} exists={exists} same_program={same_program} wanted={:?}",
|
||||||
|
exe.display().to_string()
|
||||||
|
));
|
||||||
|
exists && same_program
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Appends a line to a small log beside the helper's argument file.
|
||||||
|
///
|
||||||
|
/// The GUI is a windowed binary with no console, so every `eprintln!` on this
|
||||||
|
/// path went nowhere — which left the one decision that matters, whether the
|
||||||
|
/// scheduled task gets used or the user gets a consent prompt, completely
|
||||||
|
/// unobservable from a user's machine.
|
||||||
|
#[cfg(target_os = "windows")]
|
||||||
|
fn diag_log(msg: &str) {
|
||||||
|
let path = helper_args_file().with_file_name("helper-launch.log");
|
||||||
|
if let Some(dir) = path.parent() {
|
||||||
|
let _ = std::fs::create_dir_all(dir);
|
||||||
|
}
|
||||||
|
if let Ok(mut f) = std::fs::OpenOptions::new().create(true).append(true).open(&path) {
|
||||||
|
use std::io::Write;
|
||||||
|
let _ = writeln!(f, "{msg}");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -1004,18 +1032,24 @@ fn launch_as_admin(exe: &std::path::PathBuf, token: &str, port: u16) -> anyhow::
|
||||||
.args(["/Run", "/TN", HELPER_TASK_NAME])
|
.args(["/Run", "/TN", HELPER_TASK_NAME])
|
||||||
.output();
|
.output();
|
||||||
match run {
|
match run {
|
||||||
Ok(o) if o.status.success() => return Ok(()),
|
Ok(o) if o.status.success() => {
|
||||||
Ok(o) => eprintln!(
|
diag_log("run: schtasks /Run accepted — no consent prompt");
|
||||||
"[OSTP] schtasks /Run failed: {}",
|
return Ok(());
|
||||||
|
}
|
||||||
|
Ok(o) => diag_log(&format!(
|
||||||
|
"run: schtasks /Run failed ({:?}): {} {}",
|
||||||
|
o.status.code(),
|
||||||
|
String::from_utf8_lossy(&o.stdout).trim(),
|
||||||
String::from_utf8_lossy(&o.stderr).trim()
|
String::from_utf8_lossy(&o.stderr).trim()
|
||||||
),
|
)),
|
||||||
Err(e) => eprintln!("[OSTP] schtasks /Run could not start: {e}"),
|
Err(e) => diag_log(&format!("run: schtasks /Run could not start: {e}")),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
// Falling through: remove the file so a stale token is not left behind.
|
// Falling through: remove the file so a stale token is not left behind.
|
||||||
let _ = std::fs::remove_file(&args_file);
|
let _ = std::fs::remove_file(&args_file);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
diag_log("falling back to a direct elevated launch — this is the consent prompt");
|
||||||
launch_as_admin_direct(exe, token, port)
|
launch_as_admin_direct(exe, token, port)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue