From 542e3a540778fcf1b194e2b915c5a88068dfbe9c Mon Sep 17 00:00:00 2001 From: Ghost Scripter Date: Wed, 27 May 2026 17:10:42 +0530 Subject: [PATCH 1/3] fix(tauri): verify openhuman:// registry registration on Windows (#2699) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `tauri-plugin-deep-link::register_all` writes HKCU\Software\Classes\openhuman at first launch so the browser can hand `openhuman://auth?...` OAuth callbacks back to the running instance. When that write silently fails — or when the value goes stale because the install was moved or upgraded out-of-place — the plugin only emits a `log::warn!` and the user is left with an OAuth flow that never returns to the app. Issue #2699 has multiple users hitting exactly that failure mode on native Windows 11. Add a read-back verification after `register_all()` returns: open HKCU\Software\Classes\openhuman\shell\open\command, parse the `(Default)` value, and compare against `std::env::current_exe()`. A mismatch, missing key, or read error is now surfaced via `log::error!` (Sentry-level) with the actual registry state so support and telemetry can distinguish "never registered" from "stale" from "ACL-blocked". The handler is deliberately read-only — auto-repair could brick a working install by writing the wrong exe path; the troubleshooting doc now carries the documented manual repair script. The cross-platform string-parsing helpers (`extract_first_token`, `paths_equal_loose`, `command_references_exe`) live in a single new module behind the cfg gate so they run under `cargo test` on the macOS dev host even though the registry read itself is Windows-only. --- CLAUDE.md | 1 + app/src-tauri/Cargo.toml | 6 + .../src/deep_link_registration_check.rs | 326 ++++++++++++++++++ app/src-tauri/src/lib.rs | 29 +- gitbooks/overview/troubleshooting-sign-in.md | 24 ++ 5 files changed, 384 insertions(+), 2 deletions(-) create mode 100644 app/src-tauri/src/deep_link_registration_check.rs diff --git a/CLAUDE.md b/CLAUDE.md index 003bbcbff3..08985a39d8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -355,5 +355,6 @@ Specify → prove in Rust → prove over RPC → surface in the UI → test. - **Vendored CEF-aware `tauri-cli`**: runtime is CEF; only the vendored CLI at `app/src-tauri/vendor/tauri-cef/crates/tauri-cli` bundles Chromium into `Contents/Frameworks/`. Stock `@tauri-apps/cli` produces a broken bundle (panic in `cef::library_loader::LibraryLoader::new`). `pnpm dev:app` and all `cargo tauri` scripts call `pnpm tauri:ensure` which runs [`scripts/ensure-tauri-cli.sh`](scripts/ensure-tauri-cli.sh). If overwritten, reinstall with `cargo install --locked --path app/src-tauri/vendor/tauri-cef/crates/tauri-cli`. - **macOS deep links**: often require a built `.app` bundle, not just `tauri dev`. +- **Windows deep links**: `openhuman://` is registered to `HKCU\Software\Classes\openhuman\shell\open\command` by `tauri-plugin-deep-link::register_all` at first launch (per-user, no UAC). The Tauri shell now reads that key back after `register_all` returns and emits `log::error!` with the actual state (`NotRegistered` / `MissingCommand` / `Stale` / `ReadError`) when the value is missing or doesn't point at the running exe — without it, OAuth callbacks via `openhuman://auth?…` never reach the app (issue #2699). The check lives in [`app/src-tauri/src/deep_link_registration_check.rs`](app/src-tauri/src/deep_link_registration_check.rs); a manual repair script for affected users is in [`gitbooks/overview/troubleshooting-sign-in.md`](gitbooks/overview/troubleshooting-sign-in.md). - **Tauri environment guard**: use `isTauri()` (from `app/src/services/webviewAccountService.ts`) or wrap `invoke(...)` in `try/catch`; do not check `window.__TAURI__` directly — it is not present at module load and bypasses the established wrapper contract. - **Core is in-process** (no sidecar): `core_rpc` reaches the embedded server at `http://127.0.0.1:/rpc` with bearer auth via `OPENHUMAN_CORE_TOKEN`. `scripts/stage-core-sidecar.mjs` no longer exists; `pnpm core:stage` is a no-op echo. To run the core standalone for debugging, use `./target/debug/openhuman-core serve` (token at `{workspace}/core.token`, default `~/.openhuman-staging/core.token` under `OPENHUMAN_APP_ENV=staging`). diff --git a/app/src-tauri/Cargo.toml b/app/src-tauri/Cargo.toml index dd93b7c3c8..048a3c3ab7 100644 --- a/app/src-tauri/Cargo.toml +++ b/app/src-tauri/Cargo.toml @@ -172,6 +172,12 @@ windows-sys = { version = "0.59", features = [ "Win32_Storage_FileSystem", "Win32_System_IO", "Win32_System_Pipes", + # RegOpenKeyExW / RegQueryValueExW / RegCloseKey — used by + # deep_link_registration_check::verify_protocol_registration to read + # back HKCU\Software\Classes\openhuman\shell\open\command after + # `tauri-plugin-deep-link::register_all` so a silently-failed write + # surfaces in the Sentry / user logs (issue #2699). + "Win32_System_Registry", ] } [features] diff --git a/app/src-tauri/src/deep_link_registration_check.rs b/app/src-tauri/src/deep_link_registration_check.rs new file mode 100644 index 0000000000..a55f5f4fbd --- /dev/null +++ b/app/src-tauri/src/deep_link_registration_check.rs @@ -0,0 +1,326 @@ +//! Read-back verification for the `openhuman://` URL-scheme registration. +//! +//! `tauri-plugin-deep-link::register_all` writes +//! `HKCU\Software\Classes\openhuman\shell\open\command` on Windows so the +//! browser can hand `openhuman://auth?...` OAuth callbacks back to the running +//! desktop instance. When that write silently fails — or when the value +//! becomes stale because the install was moved out from under itself — the +//! Tauri plugin only surfaces a `warn` and the user is left with an OAuth +//! flow that never returns to the app (issue #2699). +//! +//! This module verifies the registration after `register_all` returns so the +//! actual state is logged loudly enough to be picked up by Sentry and end-user +//! support logs. We do **not** auto-repair the registry — writing the wrong +//! exe path can brick a working install — but the diagnostic surface is now +//! sufficient to point users at the documented manual repair in +//! `gitbooks/overview/troubleshooting-sign-in.md`. +//! +//! The string-parsing helpers are cross-platform so the developer host (macOS +//! / Linux) can run their unit tests; the actual registry read sits behind +//! `#[cfg(target_os = "windows")]`. The whole module is dead code on +//! non-Windows targets outside of tests, so the dead-code lint is suppressed +//! there only. + +#![cfg_attr(not(target_os = "windows"), allow(dead_code))] + +use std::path::Path; + +/// Subkey under `HKEY_CURRENT_USER` that holds the `openhuman://` URL-scheme +/// handler command. Matches what `tauri-plugin-deep-link::register_all` +/// writes on Windows (HKCU, not HKLM, so no UAC elevation is involved). +pub(crate) const HKCU_OPEN_COMMAND_SUBKEY: &str = r"Software\Classes\openhuman\shell\open\command"; + +/// Outcome of inspecting the `openhuman://` protocol handler. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum RegistrationStatus { + /// HKCU key exists and references the running executable. + Valid { command: String }, + /// HKCU key exists but its command points at a different exe than the one + /// that's running. Typical after the install was moved/copied to a new + /// path without re-running the installer. + Stale { + registered_command: String, + expected_exe: String, + }, + /// HKCU command subkey exists with no `(Default)` value or an empty one. + MissingCommand, + /// `HKCU\Software\Classes\openhuman` doesn't exist — the scheme has never + /// been registered for this user. + NotRegistered, + /// Couldn't read the registry at all (permissions, ACL on a locked-down + /// Windows image, transient failure). + ReadError(String), +} + +impl RegistrationStatus { + pub(crate) fn is_healthy(&self) -> bool { + matches!(self, Self::Valid { .. }) + } +} + +/// Pull the first whitespace-delimited token out of a Windows-style command +/// string, honouring double-quoted paths. The registry stores values like +/// `"C:\Program Files\OpenHuman\OpenHuman.exe" "%1"` — we want the exe path. +pub(crate) fn extract_first_token(command: &str) -> &str { + let trimmed = command.trim_start(); + if let Some(rest) = trimmed.strip_prefix('"') { + match rest.find('"') { + Some(end) => &rest[..end], + None => rest, + } + } else { + match trimmed.find(char::is_whitespace) { + Some(end) => &trimmed[..end], + None => trimmed, + } + } +} + +/// Compare two Windows path strings case-insensitively after normalizing +/// directory separators. Windows treats `/` and `\` interchangeably and is +/// case-insensitive on path comparisons, so this matches OS behavior closely +/// enough to detect "registry points at the right exe." +pub(crate) fn paths_equal_loose(a: &str, b: &str) -> bool { + fn norm(s: &str) -> String { + s.replace('/', "\\").to_lowercase() + } + norm(a) == norm(b) +} + +/// True iff `command` (as found in the registry) references `exe`. +pub(crate) fn command_references_exe(command: &str, exe: &Path) -> bool { + let token = extract_first_token(command); + paths_equal_loose(token, &exe.to_string_lossy()) +} + +/// Read `HKCU\Software\Classes\openhuman\shell\open\command\(Default)` and +/// classify the registration. Windows-only — all other targets get a stub +/// that returns [`RegistrationStatus::NotRegistered`] (the verification is a +/// no-op outside Windows since macOS / Linux use different mechanisms). +#[cfg(target_os = "windows")] +pub(crate) fn verify_protocol_registration() -> RegistrationStatus { + use windows_sys::Win32::Foundation::{ERROR_FILE_NOT_FOUND, ERROR_SUCCESS}; + use windows_sys::Win32::System::Registry::{ + RegCloseKey, RegOpenKeyExW, RegQueryValueExW, HKEY, HKEY_CURRENT_USER, KEY_READ, REG_SZ, + }; + + let exe = match std::env::current_exe() { + Ok(p) => p, + Err(err) => { + return RegistrationStatus::ReadError(format!("current_exe failed: {err}")); + } + }; + + let subkey_wide: Vec = HKCU_OPEN_COMMAND_SUBKEY + .encode_utf16() + .chain(std::iter::once(0)) + .collect(); + let mut hkey: HKEY = std::ptr::null_mut(); + // SAFETY: subkey_wide is NUL-terminated UTF-16; hkey is written iff result == 0. + let open_result = unsafe { + RegOpenKeyExW( + HKEY_CURRENT_USER, + subkey_wide.as_ptr(), + 0, + KEY_READ, + &mut hkey, + ) + }; + + if open_result as u32 == ERROR_FILE_NOT_FOUND { + return RegistrationStatus::NotRegistered; + } else if open_result as u32 != ERROR_SUCCESS { + return RegistrationStatus::ReadError(format!( + "RegOpenKeyExW failed: win32 error {open_result}" + )); + } + + // Probe size first. value_name = NUL pointer would select the (Default) value, + // but RegQueryValueExW also accepts an empty NUL-terminated string for the same. + let value_name: [u16; 1] = [0]; + let mut value_type: u32 = 0; + let mut needed: u32 = 0; + // SAFETY: hkey is a valid open HKEY; we pass null for data to get the size only. + let size_probe = unsafe { + RegQueryValueExW( + hkey, + value_name.as_ptr(), + std::ptr::null_mut(), + &mut value_type, + std::ptr::null_mut(), + &mut needed, + ) + }; + + if size_probe as u32 != ERROR_SUCCESS { + // SAFETY: hkey is a valid open HKEY from the matching RegOpenKeyExW call. + unsafe { + RegCloseKey(hkey); + } + if size_probe as u32 == ERROR_FILE_NOT_FOUND { + return RegistrationStatus::MissingCommand; + } + return RegistrationStatus::ReadError(format!( + "RegQueryValueExW (size probe) failed: win32 error {size_probe}" + )); + } + + if value_type != REG_SZ || needed == 0 { + // SAFETY: hkey is a valid open HKEY from the matching RegOpenKeyExW call. + unsafe { + RegCloseKey(hkey); + } + return RegistrationStatus::MissingCommand; + } + + // `needed` is in bytes; REG_SZ uses UTF-16 so each code unit is 2 bytes. Round + // up to accommodate values that aren't NUL-terminated on disk. + let units = needed.div_ceil(2) as usize; + let mut buf: Vec = vec![0u16; units]; + let mut buf_bytes: u32 = needed; + // SAFETY: buf is sized for `needed` bytes; buf_bytes is updated by the call. + let query_result = unsafe { + RegQueryValueExW( + hkey, + value_name.as_ptr(), + std::ptr::null_mut(), + &mut value_type, + buf.as_mut_ptr().cast::(), + &mut buf_bytes, + ) + }; + // SAFETY: hkey is a valid open HKEY from the matching RegOpenKeyExW call. + unsafe { + RegCloseKey(hkey); + } + + if query_result as u32 != ERROR_SUCCESS { + return RegistrationStatus::ReadError(format!( + "RegQueryValueExW failed: win32 error {query_result}" + )); + } + + let end = buf.iter().position(|&c| c == 0).unwrap_or(buf.len()); + let command = String::from_utf16_lossy(&buf[..end]); + let command_trimmed = command.trim(); + + if command_trimmed.is_empty() { + return RegistrationStatus::MissingCommand; + } + + if command_references_exe(command_trimmed, &exe) { + RegistrationStatus::Valid { + command: command_trimmed.to_string(), + } + } else { + RegistrationStatus::Stale { + registered_command: command_trimmed.to_string(), + expected_exe: exe.to_string_lossy().to_string(), + } + } +} + +/// Non-Windows stub so the setup-time wiring can call this unconditionally +/// behind `cfg(windows)` without polluting the call site with cfg gates. +#[cfg(not(target_os = "windows"))] +#[allow(dead_code)] +pub(crate) fn verify_protocol_registration() -> RegistrationStatus { + RegistrationStatus::NotRegistered +} + +#[cfg(test)] +mod tests { + use super::*; + use std::path::PathBuf; + + #[test] + fn extract_first_token_quoted_exe_with_args() { + assert_eq!( + extract_first_token("\"C:\\Program Files\\OpenHuman\\OpenHuman.exe\" \"%1\""), + "C:\\Program Files\\OpenHuman\\OpenHuman.exe" + ); + } + + #[test] + fn extract_first_token_unquoted_exe_with_args() { + assert_eq!( + extract_first_token("C:\\OpenHuman\\OpenHuman.exe %1"), + "C:\\OpenHuman\\OpenHuman.exe" + ); + } + + #[test] + fn extract_first_token_handles_leading_whitespace() { + assert_eq!( + extract_first_token(" C:\\OpenHuman\\OpenHuman.exe %1"), + "C:\\OpenHuman\\OpenHuman.exe" + ); + } + + #[test] + fn extract_first_token_single_value_no_args() { + assert_eq!(extract_first_token("OpenHuman.exe"), "OpenHuman.exe"); + } + + #[test] + fn extract_first_token_unterminated_quote_falls_through() { + // Defensive: malformed REG_SZ should not panic. We return the rest of + // the string instead of slicing past a missing terminator. + assert_eq!( + extract_first_token("\"C:\\OpenHuman\\OpenHuman.exe %1"), + "C:\\OpenHuman\\OpenHuman.exe %1" + ); + } + + #[test] + fn paths_equal_loose_is_case_insensitive_and_slash_agnostic() { + assert!(paths_equal_loose( + "C:\\Program Files\\OpenHuman\\OpenHuman.exe", + "c:/program files/openhuman/openhuman.exe" + )); + } + + #[test] + fn paths_equal_loose_distinguishes_different_paths() { + assert!(!paths_equal_loose( + "C:\\OldPath\\OpenHuman.exe", + "C:\\NewPath\\OpenHuman.exe" + )); + } + + #[test] + fn command_references_exe_matches_quoted_command_with_percent_one() { + let exe = PathBuf::from("C:\\Program Files\\OpenHuman\\OpenHuman.exe"); + assert!(command_references_exe( + "\"C:\\Program Files\\OpenHuman\\OpenHuman.exe\" \"%1\"", + &exe + )); + } + + #[test] + fn command_references_exe_detects_stale_install_path() { + // Repro of the "user moved the install" failure mode: registry still + // points at the old location. + let exe = PathBuf::from("C:\\NewLocation\\OpenHuman.exe"); + assert!(!command_references_exe( + "\"C:\\OldLocation\\OpenHuman.exe\" \"%1\"", + &exe + )); + } + + #[test] + fn is_healthy_only_for_valid_variant() { + assert!(RegistrationStatus::Valid { + command: "x".into() + } + .is_healthy()); + assert!(!RegistrationStatus::MissingCommand.is_healthy()); + assert!(!RegistrationStatus::NotRegistered.is_healthy()); + assert!(!RegistrationStatus::Stale { + registered_command: "x".into(), + expected_exe: "y".into() + } + .is_healthy()); + assert!(!RegistrationStatus::ReadError("foo".into()).is_healthy()); + } +} diff --git a/app/src-tauri/src/lib.rs b/app/src-tauri/src/lib.rs index e532a4a6c6..e9b9626e2e 100644 --- a/app/src-tauri/src/lib.rs +++ b/app/src-tauri/src/lib.rs @@ -14,6 +14,10 @@ mod core_rpc; mod deep_link_ipc; #[cfg(target_os = "windows")] mod deep_link_ipc_windows; +// Cross-platform module: the registry-reading function is windows-only, but +// the parsing helpers compile (and test) everywhere so `cargo test` on the +// developer host covers them. +mod deep_link_registration_check; mod dictation_hotkeys; mod discord_scanner; mod fake_camera; @@ -2587,8 +2591,29 @@ pub fn run() { .setup(move |app| { #[cfg(windows)] { - if let Err(err) = app.deep_link().register_all() { - log::warn!("[deep-link] register_all failed (non-fatal): {err}"); + // `register_all` writes HKCU\Software\Classes\openhuman so the + // browser can hand `openhuman://auth?...` callbacks back to + // the running instance. The plugin only returns an Err — and + // it only logs at `warn` — when its single internal write + // fails outright; it does not verify what's on disk. Issue + // #2699 reports OAuth callbacks silently disappearing on + // some Windows installs, which traced back to a missing or + // stale `command` value here. Read it back and log loudly + // (Sentry-level `error`) so the failure mode is observable + // in support logs; we deliberately do NOT auto-repair — + // writing the wrong exe path can brick a working install. + let register_err = app.deep_link().register_all().err(); + let status = deep_link_registration_check::verify_protocol_registration(); + if register_err.is_none() && status.is_healthy() { + log::info!("[deep-link] openhuman:// scheme registered ({status:?})"); + } else { + log::error!( + "[deep-link] openhuman:// scheme registration unhealthy — \ + OAuth callbacks may never reach the app. \ + register_all_error={register_err:?}, hkcu_status={status:?}. \ + See gitbooks/overview/troubleshooting-sign-in.md \ + (\"Windows: openhuman:// handler not registered\") for the manual repair." + ); } deep_link_ipc_windows::drain_pending_urls(app.app_handle()); } diff --git a/gitbooks/overview/troubleshooting-sign-in.md b/gitbooks/overview/troubleshooting-sign-in.md index 5d6f82a852..7af392b1ab 100644 --- a/gitbooks/overview/troubleshooting-sign-in.md +++ b/gitbooks/overview/troubleshooting-sign-in.md @@ -41,6 +41,30 @@ Successful desktop OAuth ends with an `openhuman://auth?...` callback. If the br 2. Restart the app, keep the same remote-core settings, and retry sign-in. 3. If using a remote core, check whether the core receives `openhuman.auth_store_session`. +## Windows: `openhuman://` handler not registered + +On Windows the `openhuman://` URL scheme is registered to the running executable via `HKEY_CURRENT_USER\Software\Classes\openhuman\shell\open\command` at first launch. If that registration silently failed — or if the install was moved/copied after first launch — the browser cannot hand the OAuth callback back to the app, and sign-in stalls after the provider step (issue #2699). + +The Tauri shell now emits a `log::error!` line at startup when this happens. Look for it in your log file (default `%USERPROFILE%\.openhuman\logs\openhuman.*.log`): + +``` +[deep-link] openhuman:// scheme registration unhealthy — OAuth callbacks may never reach the app. +register_all_error=…, hkcu_status=NotRegistered|MissingCommand|Stale { … }|ReadError(…) +``` + +To repair manually, open PowerShell **as the same user that runs OpenHuman** (no admin required — HKCU is per-user) and replace the path with your actual install location: + +```powershell +$exe = 'C:\Path\To\OpenHuman.exe' # update this +New-Item -Path 'HKCU:\Software\Classes\openhuman' -Force | Out-Null +Set-ItemProperty -Path 'HKCU:\Software\Classes\openhuman' -Name '(Default)' -Value 'URL:OpenHuman Protocol' +New-ItemProperty -Path 'HKCU:\Software\Classes\openhuman' -Name 'URL Protocol' -Value '' -Force | Out-Null +New-Item -Path 'HKCU:\Software\Classes\openhuman\shell\open\command' -Force | Out-Null +Set-ItemProperty -Path 'HKCU:\Software\Classes\openhuman\shell\open\command' -Name '(Default)' -Value ('"' + $exe + '" "%1"') +``` + +Restart OpenHuman afterwards and retry sign-in. If `register_all_error` is non-`None` in the log — for example because antivirus or a locked-down image is blocking writes to `HKCU\Software\Classes` — fixing the underlying policy is required; the manual script above will hit the same block. + For a remote core, a temporary manual injection can confirm the core is otherwise healthy: ```bash From edc54e5ed5bb024fcafe6e3ea348551c8328e991 Mon Sep 17 00:00:00 2001 From: Ghost Scripter Date: Wed, 27 May 2026 18:23:08 +0530 Subject: [PATCH 2/3] refactor(tauri): RAII the HKEY in deep-link registration check + fill test gaps Self-review follow-up on #2699 (#2757): - Replace the three manual `RegCloseKey(hkey)` calls with an `OwnedHkey(HKEY)` Drop wrapper, mirroring the `OwnedMutex` pattern in `lib.rs::run()`. Now any future early-return between the open and the value reads falls through Drop instead of having to remember to close the handle on every branch. The wrapper starts null so Drop is a no-op before `RegOpenKeyExW` succeeds. - Add three small tests for previously-uncovered parser cases: empty input (the caller short-circuits this, but the parser stays total), quoted exe with no trailing `"%1"` (some HKCU writers omit it), and the unquoted code path of `command_references_exe` (was only exercised transitively via `extract_first_token`). All 13 unit tests pass; `cargo clippy` is clean on the module. --- .../src/deep_link_registration_check.rs | 72 ++++++++++++++----- 1 file changed, 54 insertions(+), 18 deletions(-) diff --git a/app/src-tauri/src/deep_link_registration_check.rs b/app/src-tauri/src/deep_link_registration_check.rs index a55f5f4fbd..11ddd40c0e 100644 --- a/app/src-tauri/src/deep_link_registration_check.rs +++ b/app/src-tauri/src/deep_link_registration_check.rs @@ -104,6 +104,21 @@ pub(crate) fn verify_protocol_registration() -> RegistrationStatus { RegCloseKey, RegOpenKeyExW, RegQueryValueExW, HKEY, HKEY_CURRENT_USER, KEY_READ, REG_SZ, }; + // RAII wrapper for the open HKEY. Mirrors the `OwnedMutex` pattern used + // in `lib.rs::run()` for the pre-CEF mutex handle: any early return after + // the first successful `RegOpenKeyExW` falls through Drop instead of + // having to remember to call `RegCloseKey` on every branch. + struct OwnedHkey(HKEY); + impl Drop for OwnedHkey { + fn drop(&mut self) { + if !self.0.is_null() { + // SAFETY: self.0 is only set via a successful `RegOpenKeyExW` + // and is not aliased elsewhere — this Drop is the sole closer. + unsafe { RegCloseKey(self.0) }; + } + } + } + let exe = match std::env::current_exe() { Ok(p) => p, Err(err) => { @@ -115,15 +130,16 @@ pub(crate) fn verify_protocol_registration() -> RegistrationStatus { .encode_utf16() .chain(std::iter::once(0)) .collect(); - let mut hkey: HKEY = std::ptr::null_mut(); - // SAFETY: subkey_wide is NUL-terminated UTF-16; hkey is written iff result == 0. + // Start null so Drop is a no-op if `RegOpenKeyExW` never succeeds. + let mut hkey = OwnedHkey(std::ptr::null_mut()); + // SAFETY: subkey_wide is NUL-terminated UTF-16; hkey.0 is written iff result == 0. let open_result = unsafe { RegOpenKeyExW( HKEY_CURRENT_USER, subkey_wide.as_ptr(), 0, KEY_READ, - &mut hkey, + &mut hkey.0, ) }; @@ -140,10 +156,10 @@ pub(crate) fn verify_protocol_registration() -> RegistrationStatus { let value_name: [u16; 1] = [0]; let mut value_type: u32 = 0; let mut needed: u32 = 0; - // SAFETY: hkey is a valid open HKEY; we pass null for data to get the size only. + // SAFETY: hkey.0 is a valid open HKEY; we pass null for data to get the size only. let size_probe = unsafe { RegQueryValueExW( - hkey, + hkey.0, value_name.as_ptr(), std::ptr::null_mut(), &mut value_type, @@ -153,10 +169,6 @@ pub(crate) fn verify_protocol_registration() -> RegistrationStatus { }; if size_probe as u32 != ERROR_SUCCESS { - // SAFETY: hkey is a valid open HKEY from the matching RegOpenKeyExW call. - unsafe { - RegCloseKey(hkey); - } if size_probe as u32 == ERROR_FILE_NOT_FOUND { return RegistrationStatus::MissingCommand; } @@ -166,10 +178,6 @@ pub(crate) fn verify_protocol_registration() -> RegistrationStatus { } if value_type != REG_SZ || needed == 0 { - // SAFETY: hkey is a valid open HKEY from the matching RegOpenKeyExW call. - unsafe { - RegCloseKey(hkey); - } return RegistrationStatus::MissingCommand; } @@ -181,7 +189,7 @@ pub(crate) fn verify_protocol_registration() -> RegistrationStatus { // SAFETY: buf is sized for `needed` bytes; buf_bytes is updated by the call. let query_result = unsafe { RegQueryValueExW( - hkey, + hkey.0, value_name.as_ptr(), std::ptr::null_mut(), &mut value_type, @@ -189,10 +197,8 @@ pub(crate) fn verify_protocol_registration() -> RegistrationStatus { &mut buf_bytes, ) }; - // SAFETY: hkey is a valid open HKEY from the matching RegOpenKeyExW call. - unsafe { - RegCloseKey(hkey); - } + + // From here on we no longer need the handle; Drop closes it at function exit. if query_result as u32 != ERROR_SUCCESS { return RegistrationStatus::ReadError(format!( @@ -262,6 +268,24 @@ mod tests { assert_eq!(extract_first_token("OpenHuman.exe"), "OpenHuman.exe"); } + #[test] + fn extract_first_token_empty_string() { + // Defensive guard: an empty REG_SZ value must not panic. The caller + // (`verify_protocol_registration`) classifies this as `MissingCommand` + // before reaching the parser, but the parser itself stays total. + assert_eq!(extract_first_token(""), ""); + } + + #[test] + fn extract_first_token_quoted_exe_with_no_trailing_args() { + // Some installers register the command without the `"%1"` argv + // placeholder. The first token is still the quoted exe path. + assert_eq!( + extract_first_token("\"C:\\OpenHuman\\OpenHuman.exe\""), + "C:\\OpenHuman\\OpenHuman.exe" + ); + } + #[test] fn extract_first_token_unterminated_quote_falls_through() { // Defensive: malformed REG_SZ should not panic. We return the rest of @@ -297,6 +321,18 @@ mod tests { )); } + #[test] + fn command_references_exe_matches_unquoted_command() { + // Some HKCU writers omit the quotes when the path has no spaces. The + // matcher must still resolve to the exe via the unquoted code path in + // `extract_first_token` rather than relying only on the quoted path. + let exe = PathBuf::from("C:\\OpenHuman\\OpenHuman.exe"); + assert!(command_references_exe( + "C:\\OpenHuman\\OpenHuman.exe %1", + &exe + )); + } + #[test] fn command_references_exe_detects_stale_install_path() { // Repro of the "user moved the install" failure mode: registry still From c8dd8bb63b8b41d50a43834c08ac0ef25839ed4f Mon Sep 17 00:00:00 2001 From: Ghost Scripter Date: Wed, 27 May 2026 18:40:32 +0530 Subject: [PATCH 3/3] refactor(tauri): redact install paths from deep-link error log (#2699) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses an inline review comment on #2757: the `log::error!` line for an unhealthy `openhuman://` registration printed `hkcu_status={status:?}`, which for the `Stale` and `Valid` variants embedded the full `registered_command` / `expected_exe` strings. On Windows those contain `C:\Users\\AppData\Local\...` for per-user installs, so the username landed in Sentry events and end-user support logs verbatim. Add a `RegistrationStatus::redacted()` renderer that keeps the variant name and the exe basename (so the diagnostic still tells the reader *what* exe is registered) but drops the directory portion. The `MissingCommand` / `NotRegistered` / `ReadError` variants pass through unchanged since they don't carry user paths. Swap the call site in `lib.rs::setup` to use `status.redacted()` instead of the debug format. Implementation note: basename extraction scans for `\\` and `/` manually rather than via `Path::file_name`, because `std::path::Path` uses host-OS separator semantics — on a macOS / Linux dev host a Windows-style `"C:\\foo\\bar.exe"` is a single component and `file_name` would return the full path unchanged. The unit test for the Stale variant caught this on the first run. Tests: 3 new (redaction strips dirs for Stale, preserves basename for Valid, pathless variants pass through unchanged). 16/16 pass. --- .../src/deep_link_registration_check.rs | 97 +++++++++++++++++++ app/src-tauri/src/lib.rs | 9 +- 2 files changed, 104 insertions(+), 2 deletions(-) diff --git a/app/src-tauri/src/deep_link_registration_check.rs b/app/src-tauri/src/deep_link_registration_check.rs index 11ddd40c0e..51cab0ab2f 100644 --- a/app/src-tauri/src/deep_link_registration_check.rs +++ b/app/src-tauri/src/deep_link_registration_check.rs @@ -56,6 +56,54 @@ impl RegistrationStatus { pub(crate) fn is_healthy(&self) -> bool { matches!(self, Self::Valid { .. }) } + + /// Render the status as a single-line string with full filesystem paths + /// reduced to just their final component. The `Stale` and `Valid` + /// variants carry registry / current-exe paths that on Windows include + /// `C:\Users\\AppData\Local\...` for per-user installs — that + /// username lands in Sentry / user log lines unless we strip it. We keep + /// the basename so the diagnostic still tells the reader *what* exe is + /// registered, just not *where*. Used in place of `Debug` at the log + /// call site. + pub(crate) fn redacted(&self) -> String { + match self { + Self::Valid { command } => { + format!("Valid {{ exe: {} }}", basename_of_first_token(command)) + } + Self::Stale { + registered_command, + expected_exe, + } => format!( + "Stale {{ registered_exe: {}, expected_exe: {} }}", + basename_of_first_token(registered_command), + basename_of_first_token(expected_exe), + ), + Self::MissingCommand => "MissingCommand".into(), + Self::NotRegistered => "NotRegistered".into(), + // `ReadError` carries a win32-error string like + // "RegOpenKeyExW failed: win32 error 5" — no user paths. + Self::ReadError(msg) => format!("ReadError({msg})"), + } + } +} + +/// Take the first whitespace-delimited token out of `s` (handling quoted +/// command strings via [`extract_first_token`]) and return only its file +/// name. Drops the directory component so the log line doesn't leak the +/// running user's install path. +/// +/// We scan for `\\` and `/` manually rather than using [`Path::file_name`] +/// because `std::path::Path` uses **host-OS** separator semantics, so on a +/// macOS / Linux dev host a Windows-style `"C:\\foo\\bar.exe"` would come +/// back as a single component and defeat the redaction. +fn basename_of_first_token(s: &str) -> String { + let token = extract_first_token(s); + token + .rsplit(['\\', '/']) + .next() + .filter(|seg| !seg.is_empty()) + .unwrap_or("") + .to_string() } /// Pull the first whitespace-delimited token out of a Windows-style command @@ -344,6 +392,55 @@ mod tests { )); } + #[test] + fn redacted_drops_directory_components_for_stale_paths() { + // Reproduce the Sentry-leak case: a Stale status carrying the running + // user's home directory must produce a log line that contains the + // exe basenames but neither the username nor the parent dirs. + let status = RegistrationStatus::Stale { + registered_command: + "\"C:\\Users\\joe\\AppData\\Local\\OpenHuman\\OpenHuman.exe\" \"%1\"".into(), + expected_exe: "C:\\Users\\joe\\AppData\\Local\\OpenHuman_new\\OpenHuman.exe".into(), + }; + let rendered = status.redacted(); + assert!( + rendered.contains("OpenHuman.exe"), + "basename should survive redaction: {rendered}" + ); + assert!( + !rendered.contains("joe"), + "username must not leak: {rendered}" + ); + assert!( + !rendered.contains("AppData"), + "directory path must not leak: {rendered}" + ); + } + + #[test] + fn redacted_preserves_valid_variant_label_and_basename() { + let status = RegistrationStatus::Valid { + command: "\"C:\\Program Files\\OpenHuman\\OpenHuman.exe\" \"%1\"".into(), + }; + assert_eq!(status.redacted(), "Valid { exe: OpenHuman.exe }"); + } + + #[test] + fn redacted_passes_through_pathless_variants() { + assert_eq!( + RegistrationStatus::MissingCommand.redacted(), + "MissingCommand" + ); + assert_eq!( + RegistrationStatus::NotRegistered.redacted(), + "NotRegistered" + ); + assert_eq!( + RegistrationStatus::ReadError("win32 error 5".into()).redacted(), + "ReadError(win32 error 5)" + ); + } + #[test] fn is_healthy_only_for_valid_variant() { assert!(RegistrationStatus::Valid { diff --git a/app/src-tauri/src/lib.rs b/app/src-tauri/src/lib.rs index e9b9626e2e..caea9c3af8 100644 --- a/app/src-tauri/src/lib.rs +++ b/app/src-tauri/src/lib.rs @@ -2604,13 +2604,18 @@ pub fn run() { // writing the wrong exe path can brick a working install. let register_err = app.deep_link().register_all().err(); let status = deep_link_registration_check::verify_protocol_registration(); + let status_log = status.redacted(); if register_err.is_none() && status.is_healthy() { - log::info!("[deep-link] openhuman:// scheme registered ({status:?})"); + log::info!("[deep-link] openhuman:// scheme registered ({status_log})"); } else { + // Use the redacted form so per-user install paths + // (`C:\Users\\...`) do not land in Sentry / user + // logs — basenames are kept so the diagnostic still + // identifies the registered exe. log::error!( "[deep-link] openhuman:// scheme registration unhealthy — \ OAuth callbacks may never reach the app. \ - register_all_error={register_err:?}, hkcu_status={status:?}. \ + register_all_error={register_err:?}, hkcu_status={status_log}. \ See gitbooks/overview/troubleshooting-sign-in.md \ (\"Windows: openhuman:// handler not registered\") for the manual repair." );