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;