From bf741f8693b22b367e51a7b1c01f098a9c47d770 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Fri, 14 Aug 2026 15:28:41 +0200 Subject: [PATCH] fix(library): a per-user Playnite install is invisible to a SYSTEM host, and one tile killed the library MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Syncing the Playnite plugin failed outright: PUT /library/provider/playnite failed: entries[9]: launch.value for kind launcher_ui names a launcher this host cannot open (playnite) Two defects, and the second is why it cost every game rather than one tile. 1. The host looked for Playnite in the wrong registry hive and the wrong profile. `playnite_fullscreen_exe()` read HKEY_CURRENT_USER, then fell back to %LOCALAPPDATA% — but the Windows host is a LocalSystem service, so its HKCU is the SYSTEM hive (S-1-5-18) and its %LOCALAPPDATA% is C:\Windows\System32\config\systemprofile\AppData\Local. Playnite installs per-user by default, so both lookups miss on a default install. The doc comment reasoned correctly that Playnite is per-user and then read the one HKCU that cannot see it. It also hardcoded `…\Uninstall\Playnite`. Playnite ships an Inno Setup installer, and Inno registers `_is1` — measured on a Windows box where Git and Inno itself appear as `Git_is1` and `Inno Setup 6_is1` — so that key matched nothing anywhere. Now: every loaded hive under HKEY_USERS plus both HKLM views, matched on DisplayName rather than key name, then `C:\Users\*\AppData\Local\Playnite` for the conventional install (and for a user whose hive is not loaded). 2. One unopenable tile 400'd the whole reconcile. The Playnite plugin appends a single launcher tile beside its games, so refusing the payload cost the operator the entire library — the same shape as the unservable-cover bug that sanitize_art_paths was introduced to fix, on the launch side this time. `valid_launcher_ui` conflated two different failures. Split into `known_launcher_ui` (vocabulary — a plugin bug, still a hard 400, because the author has no other way to find out) and `resolvable_launcher_ui` (environment — the launcher just is not installed here, which is a fact about the box). `sanitize_launcher_entries` drops only the latter, with one warn, and the games sync. --- crates/punktfunk-host/src/library/custom.rs | 84 +++++++- crates/punktfunk-host/src/library/launch.rs | 212 ++++++++++++++++---- crates/punktfunk-host/src/mgmt/library.rs | 12 ++ 3 files changed, 261 insertions(+), 47 deletions(-) diff --git a/crates/punktfunk-host/src/library/custom.rs b/crates/punktfunk-host/src/library/custom.rs index c2d47708..dbabbb68 100644 --- a/crates/punktfunk-host/src/library/custom.rs +++ b/crates/punktfunk-host/src/library/custom.rs @@ -442,6 +442,35 @@ pub fn validate_store_claim(store: &str) -> Result<(), String> { } } +/// Drop every `launcher_ui` entry naming a launcher this host cannot actually open, returning the +/// `(title, value)` pairs removed. +/// +/// The launch-side counterpart to [`sanitize_art_paths`], and it exists for the same reason: a +/// plugin reconciles its **whole** entry set at once, so anything that fails the payload costs the +/// operator every game in it. The Playnite plugin appends one launcher tile beside the games, so a +/// host that could not resolve `Playnite.FullscreenApp.exe` refused the lot — the operator saw an +/// empty grid and a `HostRequestError` naming `entries[9]`, with nothing to say the other entries +/// were fine. +/// +/// Only the *unresolvable* case is dropped. A value outside the platform's vocabulary is still a +/// hard 400 in [`validate_provider_payload`]: that one is a bug in the plugin, and silently +/// swallowing it would leave the author with a tile that never appears and no reason why. +/// +/// Dropping the whole entry rather than clearing its `launch` is deliberate — a launcher tile with +/// no launch is a dead tile, which is strictly worse than no tile. +pub fn sanitize_launcher_entries(inputs: &mut Vec) -> Vec<(String, String)> { + let mut dropped = Vec::new(); + inputs.retain(|e| { + let Some(launch) = &e.launch else { return true }; + if launch.kind != "launcher_ui" || resolvable_launcher_ui(&launch.value) { + return true; + } + dropped.push((e.title.clone(), launch.value.clone())); + false + }); + dropped +} + /// Validate a reconcile payload: non-empty titles and unique, non-empty external ids (the /// diff key — a duplicate would make ownership of the surviving entry ambiguous). pub fn validate_provider_payload(inputs: &[ProviderEntryInput]) -> Result<(), String> { @@ -467,12 +496,13 @@ pub fn validate_provider_payload(inputs: &[ProviderEntryInput]) -> Result<(), St "entries[{i}]: `launch.value` for kind `steam_ui` must be `bigpicture` or `desktop`" )); } - // Refused rather than silently accepted, because the failure is otherwise invisible - // until a user clicks the tile: an unresolvable value yields no command at launch time. - if launch.kind == "launcher_ui" && !valid_launcher_ui(&launch.value) { + // Only the VOCABULARY is refused here. Whether the launcher is actually installed on + // this box is not the payload's fault, and 400ing over it threw away every game in the + // reconcile — see `sanitize_launcher_entries`, which drops just the tile instead. + if launch.kind == "launcher_ui" && !known_launcher_ui(&launch.value) { return Err(format!( - "entries[{i}]: `launch.value` for kind `launcher_ui` names a launcher this host \ - cannot open (`{}`)", + "entries[{i}]: `launch.value` for kind `launcher_ui` is not a launcher this \ + host's platform supports (`{}`)", launch.value )); } @@ -1065,6 +1095,14 @@ mod tests { // Other kinds are unconstrained here (the host validates them per-kind at launch). assert!(validate_provider_payload(&[with_launch("command", "anything")]).is_ok()); + // `launcher_ui` is checked for VOCABULARY only. A launcher that is merely not installed + // must pass here and be dropped later — see `an_unopenable_launcher_tile_costs_only_itself`. + assert!(validate_provider_payload(&[with_launch("launcher_ui", "nonesuch")]).is_err()); + #[cfg(windows)] + assert!(validate_provider_payload(&[with_launch("launcher_ui", "playnite")]).is_ok()); + #[cfg(target_os = "linux")] + assert!(validate_provider_payload(&[with_launch("launcher_ui", "lutris")]).is_ok()); + let with_env = |key: &str, value: Option<&str>| { let mut i = input("a", "A"); i.detect.env_marker = Some(EnvMarker { @@ -1129,4 +1167,40 @@ mod tests { "duplicate external_id" ); } + + /// The regression `sanitize_launcher_entries` exists for: a launcher tile this host cannot open + /// must cost that tile, not the games reconciled beside it. + /// + /// Field shape — the Playnite plugin appends exactly one `launcher_ui` tile after its games, so + /// `entries[N]` failing validation used to refuse the entire payload and leave the operator with + /// an empty grid and a `HostRequestError` that named only the index. + #[test] + fn an_unopenable_launcher_tile_costs_only_itself() { + let mut tile = input("launcher", "Playnite"); + tile.role = GameRole::Launcher; + tile.launch = Some(LaunchSpec { + kind: "launcher_ui".into(), + value: "playnite".into(), + }); + + let mut inputs = vec![input("a", "A"), tile, input("b", "B")]; + let dropped = sanitize_launcher_entries(&mut inputs); + + if resolvable_launcher_ui("playnite") { + // A Windows box with Playnite actually installed keeps all three. + assert!(dropped.is_empty()); + assert_eq!(inputs.len(), 3); + } else { + // Everywhere else the tile goes and both games survive — the whole point of the split. + assert_eq!(dropped.len(), 1); + assert_eq!(dropped[0].1, "playnite"); + assert_eq!(inputs.len(), 2); + assert!(inputs.iter().all(|e| e.external_id != "launcher")); + } + + // A payload of nothing but games is untouched on every OS. + let mut only_games = vec![input("a", "A"), input("b", "B")]; + assert!(sanitize_launcher_entries(&mut only_games).is_empty()); + assert_eq!(only_games.len(), 2); + } } diff --git a/crates/punktfunk-host/src/library/launch.rs b/crates/punktfunk-host/src/library/launch.rs index cf2cd601..277de2fb 100644 --- a/crates/punktfunk-host/src/library/launch.rs +++ b/crates/punktfunk-host/src/library/launch.rs @@ -478,13 +478,31 @@ fn launcher_ui_stores() -> &'static [&'static str] { } } -/// Is this a `launcher_ui` value this host can resolve? +/// Is `value` a launcher this host's platform knows about at all? /// -/// On Windows, Playnite is validated by *resolution* rather than by being on the list: a host -/// without Playnite installed refuses the entry (a 400 the plugin author can act on) instead of -/// publishing a tile that does nothing when a user clicks it. -pub(crate) fn valid_launcher_ui(value: &str) -> bool { - if !launcher_ui_stores().contains(&value) { +/// The *vocabulary* half of the old `valid_launcher_ui`. A value outside this set is a plugin +/// author's mistake — a typo, or a launcher this OS has no support for — and no amount of +/// installing things on the box will make it resolve, so the reconcile refuses the payload. +pub(crate) fn known_launcher_ui(value: &str) -> bool { + launcher_ui_stores().contains(&value) +} + +/// Can this host open `value`'s launcher **right now**? +/// +/// The *environment* half. Deliberately separate from [`known_launcher_ui`], because the two +/// failures are not the same kind of thing and must not get the same answer: +/// +/// - an unknown value is a bug in the plugin, and a 400 is the only way its author finds out; +/// - a known value that will not resolve means the launcher simply is not installed here, which is +/// an ordinary fact about the box, not a defect in the payload. +/// +/// Conflating them cost a real library: the Playnite plugin publishes one launcher tile alongside +/// every game, so a host that could not resolve Playnite 400'd the whole reconcile and the operator +/// got **no games at all** — the same shape as the unservable-cover bug that +/// [`super::sanitize_art_paths`] was introduced to fix. The tile is dropped now (see +/// [`super::sanitize_launcher_entries`]) and the games sync. +pub(crate) fn resolvable_launcher_ui(value: &str) -> bool { + if !known_launcher_ui(value) { return false; } #[cfg(windows)] @@ -502,36 +520,141 @@ pub(crate) fn valid_launcher_ui(value: &str) -> bool { /// directly, which is also why nothing here is interpolated from the entry: the whole value is the /// literal `"playnite"`. /// -/// Playnite installs per-user by default, so the install directory comes from its own uninstall -/// entry (HKCU first, then HKLM for a machine-wide install), falling back to the default -/// `%LOCALAPPDATA%\Playnite`. `None` when nothing resolves, which is what refuses the tile. +/// `None` when nothing resolves, which is what drops the tile. #[cfg(windows)] fn playnite_fullscreen_exe() -> Option { - use winreg::enums::{HKEY_CURRENT_USER, HKEY_LOCAL_MACHINE}; - use winreg::RegKey; - const KEY: &str = r"SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall\Playnite"; const EXE: &str = "Playnite.FullscreenApp.exe"; - - let from_registry = [HKEY_CURRENT_USER, HKEY_LOCAL_MACHINE] + playnite_install_dirs() .into_iter() - .find_map(|root| { - RegKey::predef(root) - .open_subkey(KEY) - .ok()? - .get_value::("InstallLocation") - .ok() - }) - .map(std::path::PathBuf::from); - - from_registry - .into_iter() - .chain( - std::env::var_os("LOCALAPPDATA").map(|l| std::path::PathBuf::from(l).join("Playnite")), - ) .map(|dir| dir.join(EXE)) .find(|p| p.is_file()) } +/// Windows: every directory that might hold a Playnite install, best candidates first. +/// +/// **Playnite installs per-user by default, and this host is a LocalSystem service** — which +/// invalidates all three of the obvious lookups, and is why this is not a two-liner: +/// +/// - `HKEY_CURRENT_USER` is *SYSTEM's own* hive (`S-1-5-18`), never the person's, so a per-user +/// install is invisible there. Every **loaded** hive under `HKEY_USERS` is read instead: only +/// logged-on users' hives are loaded, which is exactly the set that can be streaming, and it +/// avoids a `WTSQueryUserToken` dance for what is a best-effort probe. Same trade-off +/// [`crate::procscan::steam_running_hint`] makes, for the same reason. +/// - The uninstall subkey is matched by its **`DisplayName`**, not by key name. Playnite ships an +/// Inno Setup installer and Inno registers `_is1` — measured on a Windows box where Git +/// and Inno itself appear as `Git_is1` and `Inno Setup 6_is1`. The hardcoded +/// `…\Uninstall\Playnite` this replaced matched nothing on any box. +/// - `%LOCALAPPDATA%` for a SYSTEM service is `C:\Windows\System32\config\systemprofile\AppData\ +/// Local`, so the default-install fallback cannot trust the variable — it enumerates the profiles +/// under the users base instead, the same breadth [`super::art::art_roots`] already allows. +/// +/// Order matters only as a preference: a registry `InstallLocation` is what the installer actually +/// did, so it is consulted before the conventional path. Every candidate is probed for the exe, so +/// a stale entry costs one `is_file` and nothing else. +#[cfg(windows)] +fn playnite_install_dirs() -> Vec { + use winreg::enums::{HKEY_LOCAL_MACHINE, HKEY_USERS, KEY_READ}; + use winreg::RegKey; + + // 64-bit and 32-bit views. HKCU/HKU `Software` is not redirected (only `Software\Classes` is), + // so the WOW view is a machine-hive concern only. + const UNINSTALL: &str = r"Software\Microsoft\Windows\CurrentVersion\Uninstall"; + const UNINSTALL_WOW: &str = r"Software\WOW6432Node\Microsoft\Windows\CurrentVersion\Uninstall"; + + let mut dirs: Vec = Vec::new(); + + let hklm = RegKey::predef(HKEY_LOCAL_MACHINE); + playnite_dirs_from_uninstall(&hklm, UNINSTALL, &mut dirs); + playnite_dirs_from_uninstall(&hklm, UNINSTALL_WOW, &mut dirs); + + let users = RegKey::predef(HKEY_USERS); + for sid in users.enum_keys().flatten() { + // The `…_Classes` companion hives carry file associations, never uninstall entries. + if sid.ends_with("_Classes") { + continue; + } + if let Ok(hive) = users.open_subkey_with_flags(&sid, KEY_READ) { + playnite_dirs_from_uninstall(&hive, UNINSTALL, &mut dirs); + } + } + + // The conventional per-user location, for every profile on the box — this is where Playnite's + // own default install lands, and it covers a user whose hive is not currently loaded. + for profile in windows_user_profiles() { + push_unique(&mut dirs, profile.join(r"AppData\Local\Playnite")); + } + dirs +} + +/// Collect `InstallLocation` from every Playnite-looking uninstall entry under `root\path`. +/// +/// Matched on `DisplayName` because the key name is the installer's `AppId` (see +/// [`playnite_install_dirs`]). `starts_with` rather than equality so a versioned or suffixed display +/// name still counts; the value is only ever used as a directory to probe for the exe, so a false +/// positive costs one failed `is_file`. +#[cfg(windows)] +fn playnite_dirs_from_uninstall( + root: &winreg::RegKey, + path: &str, + out: &mut Vec, +) { + use winreg::enums::KEY_READ; + + let Ok(uninstall) = root.open_subkey_with_flags(path, KEY_READ) else { + return; + }; + for name in uninstall.enum_keys().flatten() { + let Ok(entry) = uninstall.open_subkey_with_flags(&name, KEY_READ) else { + continue; + }; + let display: String = entry.get_value("DisplayName").unwrap_or_default(); + if !display.starts_with("Playnite") { + continue; + } + if let Ok(location) = entry.get_value::("InstallLocation") { + let location = location.trim(); + if !location.is_empty() { + push_unique(out, std::path::PathBuf::from(location)); + } + } + } +} + +/// Every user profile directory on the box (`C:\Users\*`), minus the shared `Public` pseudo-profile. +/// +/// `%PUBLIC%`'s parent is the users base on every supported Windows — the same derivation +/// [`super::art::art_roots`] uses — with `%SystemDrive%\Users` as the fallback when the variable is +/// missing from a service's environment. +#[cfg(windows)] +fn windows_user_profiles() -> Vec { + let base = std::env::var_os("PUBLIC") + .map(std::path::PathBuf::from) + .and_then(|p| p.parent().map(std::path::Path::to_path_buf)) + .or_else(|| { + std::env::var_os("SystemDrive").map(|d| std::path::PathBuf::from(d).join("Users")) + }); + let Some(base) = base else { + return Vec::new(); + }; + let Ok(entries) = std::fs::read_dir(&base) else { + return Vec::new(); + }; + entries + .flatten() + .map(|e| e.path()) + .filter(|p| p.is_dir() && !p.ends_with("Public")) + .collect() +} + +/// Push `path` unless an equal one is already there — the candidate lists are a handful of entries, +/// so a linear check beats carrying a set around. +#[cfg(windows)] +fn push_unique(out: &mut Vec, path: std::path::PathBuf) { + if !out.contains(&path) { + out.push(path); + } +} + /// Map a `heroic` LaunchSpec value (`:`) to the Heroic launch command, run nested in /// gamescope. The host owns this mapping; the client only ever sends the id. CAVEAT: Heroic is a /// single-instance Electron app — in a fresh per-session gamescope it boots, launches the game (which @@ -800,33 +923,38 @@ mod tests { fn launcher_ui_accepts_only_launchers_this_host_can_open() { #[cfg(target_os = "linux")] { - assert!(valid_launcher_ui("heroic")); - assert!(valid_launcher_ui("lutris")); - // Not wired on this OS — refused inbound rather than becoming a tile that does nothing. - assert!(!valid_launcher_ui("gog")); + assert!(known_launcher_ui("heroic")); + assert!(known_launcher_ui("lutris")); + // Not wired on this OS — outside the vocabulary, so it is refused inbound rather than + // becoming a tile that does nothing. + assert!(!known_launcher_ui("gog")); } #[cfg(windows)] { - // Playnite is accepted only when this host can actually FIND its Fullscreen app: - // validation is resolution, so a box without Playnite refuses the entry rather than - // publishing a tile that does nothing when clicked. + // Playnite is in the vocabulary unconditionally — whether this particular box has it + // installed is a separate question, answered by `resolvable_launcher_ui` below. Keeping + // them separate is the fix for the reconcile that 400'd a whole library over one tile. + assert!(known_launcher_ui("playnite")); assert_eq!( - valid_launcher_ui("playnite"), + resolvable_launcher_ui("playnite"), playnite_fullscreen_exe().is_some() ); // The Linux launchers, and the Windows ones whose activation is still unverified // (Epic, GOG Galaxy, the Xbox app), stay refused. - assert!(!valid_launcher_ui("heroic")); - assert!(!valid_launcher_ui("gog")); + assert!(!known_launcher_ui("heroic")); + assert!(!known_launcher_ui("gog")); } #[cfg(not(any(target_os = "linux", windows)))] { // No launcher UIs are wired on this OS, so every value is refused. - assert!(!valid_launcher_ui("heroic")); - assert!(!valid_launcher_ui("gog")); + assert!(!known_launcher_ui("heroic")); + assert!(!known_launcher_ui("gog")); } - assert!(!valid_launcher_ui("")); - assert!(!valid_launcher_ui("lutris; rm -rf ~")); + // Junk is outside the vocabulary on every OS, so it never reaches a resolver. + assert!(!known_launcher_ui("")); + assert!(!known_launcher_ui("lutris; rm -rf ~")); + assert!(!resolvable_launcher_ui("")); + assert!(!resolvable_launcher_ui("lutris; rm -rf ~")); } /// The `xbox` kind is what a library PLUGIN can publish: the runner's principal cannot read diff --git a/crates/punktfunk-host/src/mgmt/library.rs b/crates/punktfunk-host/src/mgmt/library.rs index 9512cc81..fc5f5ad0 100644 --- a/crates/punktfunk-host/src/mgmt/library.rs +++ b/crates/punktfunk-host/src/mgmt/library.rs @@ -524,6 +524,18 @@ pub(crate) async fn reconcile_provider_entries( return denied; } } + // A launcher this box cannot open is a fact about the box, not a defect in the payload, so it + // costs its own tile and nothing else. Before this, the Playnite plugin's single launcher entry + // 400'd every game it shipped alongside. + for (title, value) in crate::library::sanitize_launcher_entries(&mut inputs) { + tracing::warn!( + provider, + launcher = %value, + title = %title, + "library reconcile: dropped a launcher tile this host cannot open — the rest of the \ + payload still syncs. Install the launcher, or turn the tile off in the plugin's config" + ); + } // One aggregated line, not one per entry: a root mismatch misses EVERY cover in the payload, and // a per-entry warn would bury the rest of the log under a thousand copies of one fact. let mut dropped_art = 0usize; -- 2.54.0