From 43dc28ad5c4a5115ab8bcce082fc6f728c7cef5f Mon Sep 17 00:00:00 2001 From: Marco Date: Tue, 29 Sep 2026 20:26:17 -0300 Subject: [PATCH 01/51] fix(docs): the product ships NSIS, not MSI Three documents told users to look for an .msi installer. The bundled output is *_x64-setup.exe, produced by the NSIS target in tauri.conf.json. --- README.md | 2 +- docs/CONTRIBUTING.md | 2 +- docs/USER-GUIDE.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 5cd0554..fd6a4f5 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,7 @@ You can also drop your own YAML capability into `%LOCALAPPDATA%\Moddin\capabilit ## Getting Moddin 1. **Download** the latest release from [Releases](https://github.com/petonexus/moddin-desktop/releases). Beta releases are tagged `vX.Y.Z-beta.N`. -2. **Install** by running the `.msi` (Windows 10/11 x64). +2. **Install** by running the downloaded `Moddin.Desktop_*_x64-setup.exe` (Windows 10/11 x64). 3. **Open** the app. Moddin scans your Steam + Epic libraries automatically. 4. **Pick a game**, see which mods apply, and install what you need. diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index a468868..967af5c 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -178,7 +178,7 @@ Each pre-release tagged `vX.Y.Z-beta.N` has a **beta** release in the [Releases] To test: -1. Download the `.msi` from the latest beta release. +1. Download the `Moddin.Desktop_*_x64-setup.exe` from the latest beta release. 2. Install over your current Moddin (your data is preserved). 3. Try the changes listed in the release notes. 4. File any issues with the **beta** label and the version number. diff --git a/docs/USER-GUIDE.md b/docs/USER-GUIDE.md index fbac3ec..009b897 100644 --- a/docs/USER-GUIDE.md +++ b/docs/USER-GUIDE.md @@ -62,7 +62,7 @@ Moddin checks GitHub Releases for new versions. To update: 2. Click **Download update**. 3. Restart Moddin to apply the patch. -You can also update manually by downloading the latest `.msi` from [Releases](https://github.com/petonexus/moddin-desktop/releases). +You can also update manually by downloading the latest `Moddin.Desktop_*_x64-setup.exe` from [Releases](https://github.com/petonexus/moddin-desktop/releases). --- From 1818fdd985ff8c86d4d5a747df35aa1b243bd689 Mon Sep 17 00:00:00 2001 From: Marco Date: Tue, 29 Sep 2026 20:26:38 -0300 Subject: [PATCH 02/51] fix(capability): make the declarative engine install what it advertises P0-1: extract-zip resolves its archive with resolve_download_path, which only special-cases a bare filename. Five registered specs pointed archivePathField at a URL, so every one of them failed at its first install step. The download-file step existed; it had been added to the one spec that was not registered. optiscaler, reshade and cheeky-foveated-dlss now fetch into the download cache first, and the proxy rename uses extract-zip native proxyField instead of a move-file that pointed a URL at a rename. ofxr-bridge and uevr stay status: planned rather than fixed. They need step kinds the engine does not have: ofxr-bridge promotes one named file from a staging directory into the OpenXR ApiLayers folder, and uevr ships as a git tag that must be built. Each carries a safetyNotes entry naming the exact capability it needs. P0-2: the transaction was opened after the step loop, so a file the archive overwrote was snapshotted in its new state and a mid-install failure left orphaned writes with no rollback record. execute_install now plans each step targets, extends the transaction before the step runs, and rolls back on failure. Covered by an_install_backs_up_an_existing_game_file_before_overwriting_it and a_failed_step_rolls_back_what_the_install_already_wrote. P0-3: bepinex, ue4ss and reframework are now registered. The six-spec list became one BUILT_IN_YAML const shared by load() and load_with_local_dir(), which had drifted apart. F-02: a step cannot traverse out of the install root. Absolute paths from path-typed config fields stay allowed, because those fields exist precisely to name a location outside the game folder; what is refused is .. traversal, and the transaction store still declines to back anything up outside the root. Also fixed in passing: spawn-process no longer puts pid:NNN in affected_paths; move-file rejects a source equal to its destination; the YAML parse panic names the offending recipe, because a scalar containing a colon-space silently becomes a mapping; and test_support::env_lock recovers from poisoning so one real failure stays one failure instead of twelve fake ones. --- src-tauri/capabilities/bepinex.yaml | 1 + .../capabilities/cheeky-foveated-dlss.yaml | 19 +- src-tauri/capabilities/ofxr-bridge.yaml | 5 +- src-tauri/capabilities/openxr-helpers.yaml | 1 + src-tauri/capabilities/optiscaler.yaml | 24 +- src-tauri/capabilities/reframework.yaml | 1 + src-tauri/capabilities/reshade.yaml | 17 +- src-tauri/capabilities/ue4ss.yaml | 1 + src-tauri/capabilities/uevr.yaml | 8 +- src-tauri/src/archive.rs | 51 +++ src-tauri/src/builtin_checks.rs | 42 +- src-tauri/src/builtin_steps.rs | 293 +++++++++--- src-tauri/src/capability.rs | 6 + src-tauri/src/capability_runner.rs | 432 +++++++++++++++--- src-tauri/src/lib.rs | 6 +- src-tauri/src/transaction.rs | 158 +++++-- 16 files changed, 865 insertions(+), 200 deletions(-) diff --git a/src-tauri/capabilities/bepinex.yaml b/src-tauri/capabilities/bepinex.yaml index b5e50ad..f8a574d 100644 --- a/src-tauri/capabilities/bepinex.yaml +++ b/src-tauri/capabilities/bepinex.yaml @@ -1,5 +1,6 @@ id: bepinex displayName: BepInEx +description: Lets Unity games load mods and plugins, and is the base most other Unity mods need. category: qol status: available supportedEngines: diff --git a/src-tauri/capabilities/cheeky-foveated-dlss.yaml b/src-tauri/capabilities/cheeky-foveated-dlss.yaml index 46d7b7a..29865f4 100644 --- a/src-tauri/capabilities/cheeky-foveated-dlss.yaml +++ b/src-tauri/capabilities/cheeky-foveated-dlss.yaml @@ -1,5 +1,6 @@ id: cheeky-foveated-dlss displayName: Cheeky Foveated DLSS +description: Cheap, stable foveated rendering on top of DLSS — keeps sharpness where you are looking. category: graphics status: available supportedEngines: @@ -42,18 +43,16 @@ checks: urlField: downloadUrl expectedField: sha256 install: - - kind: extract-zip - description: Extract CheekyFoveatedDLSS.addon64 next to the game executable. + - kind: download-file + description: Fetch the pinned CheekyFoveatedDLSS release into Moddin's download cache, verifying its digest before anything touches the game folder. params: - archivePathField: downloadUrl -uninstall: - - kind: file-delete - description: Remove the Cheeky .addon64 file from the game directory. + urlField: downloadUrl + expectedField: sha256 + target: cheeky-foveated-dlss.zip + - kind: extract-zip + description: Extract CheekyFoveatedDLSS.addon64 next to the game executable. It is a ReShade add-on, so ReShade itself has to be installed separately. params: - pathField: addonFile - -uninstallSkipsFiles: - - pathField: addonFile + archivePath: cheeky-foveated-dlss.zip safetyNotes: - Cheeky is a ReShade add-on; install ReShade separately before installing Cheeky. - Cheeky requires a game with DLSS or DLSS-SR (via OptiScaler) already running; games that ship FSR-only need an upstream provider first. \ No newline at end of file diff --git a/src-tauri/capabilities/ofxr-bridge.yaml b/src-tauri/capabilities/ofxr-bridge.yaml index 7723912..43a9bd0 100644 --- a/src-tauri/capabilities/ofxr-bridge.yaml +++ b/src-tauri/capabilities/ofxr-bridge.yaml @@ -1,7 +1,8 @@ id: ofxr-bridge displayName: OFXR Bridge FrameGen +description: Streams the flat desktop into a VR headset through OpenXR, so non-VR games work in VR. category: vr -status: available +status: planned supportedEngines: - unreal5 - redengine @@ -127,6 +128,8 @@ uninstall: pathField: trayIniPath safetyNotes: + - Install chain is not wired yet. Extracting into a staging directory and then promoting one named manifest file into the OpenXR ApiLayers folder needs step support that does not exist yet — `extract-zip` has no target-subdirectory option, and `move-file` cannot move a directory. Until that exists this spec stays `planned` and the game page keeps offering the working OFXR module instead. + - The `write-text-file` step that renders `tray.ini` writes outside the game folder, so it will also need an explicit `allowOutsideRoot` opt-in when the chain is wired. - OFXR Bridge is an experimental pre-release; it may produce artifacts, freezes, or crashes depending on the game, VR mod, GPU and runtime. - The tray requires the Microsoft Visual C++ Redistributable x64; keep the game non-elevated because Windows OpenXR does not apply per-user layers to elevated processes. - Requires the Game executable to be closed before install. \ No newline at end of file diff --git a/src-tauri/capabilities/openxr-helpers.yaml b/src-tauri/capabilities/openxr-helpers.yaml index b6ed311..0e80a3f 100644 --- a/src-tauri/capabilities/openxr-helpers.yaml +++ b/src-tauri/capabilities/openxr-helpers.yaml @@ -1,5 +1,6 @@ id: openxr-helpers displayName: OpenXR runtime helpers +description: Chooses which OpenXR runtime your games use, and remembers that choice per game. category: vr status: available supportedEngines: diff --git a/src-tauri/capabilities/optiscaler.yaml b/src-tauri/capabilities/optiscaler.yaml index 2234ed3..cd7597c 100644 --- a/src-tauri/capabilities/optiscaler.yaml +++ b/src-tauri/capabilities/optiscaler.yaml @@ -1,5 +1,6 @@ id: optiscaler displayName: OptiScaler DLSS upscaler +description: Adds DLSS Super Resolution to games that ship FSR instead, or no upscaler at all. category: graphics status: available supportedEngines: @@ -11,7 +12,7 @@ configSchema: - name: downloadUrl type: url required: true - description: HTTPS URL of the OptiScaler release archive (7z or zip). + description: HTTPS URL of the OptiScaler release archive. Must be a .zip — the recipe extracts with `extract-zip` and cannot read a 7z payload. - name: sha256 type: sha256 required: true @@ -48,20 +49,17 @@ checks: urlField: downloadUrl expectedField: sha256 install: - - kind: extract-zip - description: Extract the OptiScaler archive into the game executable directory. - params: - archivePathField: downloadUrl - - kind: move-file - description: Rename OptiScaler's bundled proxy DLL to the recipe-declared `proxy`. + - kind: download-file + description: Fetch the pinned OptiScaler archive into Moddin's download cache, verifying its digest before anything touches the game folder. params: - fromField: downloadUrl - toField: proxy -uninstall: - - kind: file-delete - description: Remove the proxy DLL OptiScaler dropped next to the game executable. + urlField: downloadUrl + expectedField: sha256 + target: optiscaler.zip + - kind: extract-zip + description: Extract the OptiScaler archive into the game executable directory. The bundled proxy DLL is renamed to the recipe-declared `proxy` on the way out, so it cannot collide with another mod that already owns that name. params: - pathField: proxy + archivePath: optiscaler.zip + proxyField: proxy safetyNotes: - OptiScaler proxies its own DLL into the executable directory; choose a proxy name that does not clash with other mods (UEVR, ReShade, R.E.A.L. VR, ERVR). - OptiScaler only ships replacements for DLSS-SR; games that already include DLSS or FSR need their native runtime left untouched. \ No newline at end of file diff --git a/src-tauri/capabilities/reframework.yaml b/src-tauri/capabilities/reframework.yaml index 7805f49..1e0780e 100644 --- a/src-tauri/capabilities/reframework.yaml +++ b/src-tauri/capabilities/reframework.yaml @@ -1,5 +1,6 @@ id: reframework displayName: REFramework scripting framework +description: Scripting framework for RE Engine — lets mods change the game while it is running. category: qol status: planned supportedEngines: diff --git a/src-tauri/capabilities/reshade.yaml b/src-tauri/capabilities/reshade.yaml index 5d05834..2517ebd 100644 --- a/src-tauri/capabilities/reshade.yaml +++ b/src-tauri/capabilities/reshade.yaml @@ -1,5 +1,6 @@ id: reshade displayName: ReShade host +description: Adds post-processing to any DirectX game — sharpening, bloom, colour correction and anti-aliasing. category: graphics status: available supportedEngines: @@ -48,15 +49,17 @@ checks: urlField: downloadUrl expectedField: sha256 install: - - kind: extract-zip - description: Extract ReShade host archive (ReShade64.dll + ReShadeAddons64.dll + ReShade.ini) next to the game executable. + - kind: download-file + description: Fetch the pinned ReShade archive into Moddin's download cache, verifying its digest before anything touches the game folder. params: - archivePathField: downloadUrl - - kind: move-file - description: Rename ReShade64.dll to the recipe-declared `proxy` (avoids clashing with UEVR or another mod that already owns dxgi.dll). + urlField: downloadUrl + expectedField: sha256 + target: reshade.zip + - kind: extract-zip + description: Extract ReShade (ReShade64.dll + ReShadeAddons64.dll + ReShade.ini) next to the game executable. The bundled proxy is renamed to the recipe-declared `proxy` on the way out, so ReShade does not clobber a dxgi.dll another mod already owns. params: - fromField: downloadUrl - toField: proxy + archivePath: reshade.zip + proxyField: proxy safetyNotes: - ReShade adds post-processing hooks; some games' anti-cheat systems flag ReShade even when offline. - ReShade's `dxgi.dll` proxy conflicts with OptiScaler / R.E.A.L. VR. The recipe must declare an alternative proxy (typically `version.dll` for OptiScaler coexistence). \ No newline at end of file diff --git a/src-tauri/capabilities/ue4ss.yaml b/src-tauri/capabilities/ue4ss.yaml index 3b916a8..109a369 100644 --- a/src-tauri/capabilities/ue4ss.yaml +++ b/src-tauri/capabilities/ue4ss.yaml @@ -1,5 +1,6 @@ id: ue4ss displayName: UE4SS scripting framework +description: Scripting framework for Unreal Engine — lets mods change the game while it is running. category: qol status: planned supportedEngines: diff --git a/src-tauri/capabilities/uevr.yaml b/src-tauri/capabilities/uevr.yaml index a5af4c8..7675243 100644 --- a/src-tauri/capabilities/uevr.yaml +++ b/src-tauri/capabilities/uevr.yaml @@ -1,7 +1,8 @@ id: uevr displayName: UEVR engine-aware installer +description: Brings Unreal Engine VR rendering to flat-screen Unreal games. category: vr -status: available +status: planned supportedEngines: - unreal5 configSchema: @@ -60,10 +61,7 @@ uninstall: params: processName: UEVRInjector.exe force: true - - kind: file-delete - description: Remove the staging directory used for this backend. - params: - pathField: extractedDir safetyNotes: + - Install chain is not wired yet. UEVR ships as a git tag that has to be checked out and built, and `pinnedTag` is a tag name rather than an archive URL, so no download or extract step can consume it. Wiring this needs a `git-checkout` step kind and a per-backend build step. Until then this spec stays `planned` and the game page keeps offering the working UEVR module instead. - UEVR only injects Unreal Engine games (UE 4.8-5.4). Unity, RE Engine, REDengine, and unknown engines are not eligible; the install aborts before writing files. - Close the game AND UEVRInjector.exe before applying or changing a backend. Do not run Steam, UEVR or the game as administrator. \ No newline at end of file diff --git a/src-tauri/src/archive.rs b/src-tauri/src/archive.rs index 228aa28..b498f7d 100644 --- a/src-tauri/src/archive.rs +++ b/src-tauri/src/archive.rs @@ -151,6 +151,57 @@ impl RemoteArchive { self.expected_sha256 = Some(expected.into()); self } + + /// Size of the remote payload in bytes, from a HEAD request, or + /// `None` when the server does not report one. + /// + /// Exists so a preflight check can decide *whether to download* + /// without starting the download. `archive-sha256` is a blocker + /// that runs while the UI is rendering a module card, and pulling + /// 300 MB to decide whether a button should be enabled is not a + /// check. + /// + /// `Err` means the size could not be determined (offline, host not + /// allowed, server rejects HEAD). Callers should treat that as + /// "unknown size", not as "download failed". + pub async fn content_length(&self) -> Result, String> { + let parsed = reqwest::Url::parse(&self.url) + .map_err(|error| format!("{}: invalid URL: {error}", self.label()))?; + if parsed.scheme() != "https" { + return Err(format!("{}: must use HTTPS.", self.label())); + } + let host = parsed + .host_str() + .ok_or_else(|| format!("{}: missing host.", self.label()))? + .to_ascii_lowercase(); + let allowed = if self.host_allowlist.is_empty() { + vec!["github.com".to_owned()] + } else { + self.host_allowlist + .iter() + .map(|host| host.to_ascii_lowercase()) + .collect() + }; + if !allowed.iter().any(|candidate| host == *candidate) { + return Err(format!( + "{}: host '{host}' is not in the allow-list.", + self.label() + )); + } + + let client = Client::builder() + .user_agent("Moddin-Desktop/0.1 (+https://github.com/petonexus/moddin-desktop)") + .build() + .map_err(|error| format!("{}: could not create downloader: {error}", self.label()))?; + + let response = client + .head(parsed) + .send() + .await + .map_err(|error| format!("{}: could not read archive size: {error}", self.label()))?; + + Ok(response.content_length()) + } } impl ArchiveSource for RemoteArchive { diff --git a/src-tauri/src/builtin_checks.rs b/src-tauri/src/builtin_checks.rs index b45f513..1e5fe59 100644 --- a/src-tauri/src/builtin_checks.rs +++ b/src-tauri/src/builtin_checks.rs @@ -27,6 +27,17 @@ use std::{ /// per evaluation run. pub type ExeVersionCache = BTreeMap>; +/// Archive size above which the `archive-sha256` preflight check skips +/// the download instead of fetching the whole payload. +/// +/// The check exists to tell the user "this archive is not what the +/// recipe pinned" before they install. It is not the enforcement point: +/// the `download-file` step re-verifies the digest against the same +/// `sha256` config field, and that runs with the archive already on the +/// way to disk. So the honest answer for a large archive is "verified +/// during install", not "download 300 MB to render a card". +const PREFLIGHT_MAX_DOWNLOAD_BYTES: u64 = 64 * 1024 * 1024; + /// Catalogue of check kinds the runner knows how to evaluate. The /// capability loader rejects specs that reference a kind outside this /// set so the failure surfaces at startup. @@ -209,11 +220,32 @@ async fn evaluate_archive_sha256( }; let parsed = reqwest::Url::parse(&url) .map_err(|error| format!("archive-sha256: invalid URL '{url}': {error}"))?; - let bytes = crate::archive::RemoteArchive::new(parsed.to_string()) - .with_host_allowlist(vec!["github.com".to_owned(), "objects.githubusercontent.com".to_owned()]) - .fetch() - .await?; - let (downloaded, computed) = bytes; + let archive = crate::archive::RemoteArchive::new(parsed.to_string()) + .with_host_allowlist(vec!["github.com".to_owned(), "objects.githubusercontent.com".to_owned()]); + + // A preflight pass runs while the UI is rendering a module card. If + // the archive is large, downloading it to answer "is the button + // enabled yet" is worse than useless — the digest is enforced again + // by the `download-file` step at install time, which is the moment + // it actually protects anything. So ask for the size first and skip + // politely rather than pulling hundreds of megabytes. + if let Ok(Some(size)) = archive.content_length().await { + if size > PREFLIGHT_MAX_DOWNLOAD_BYTES { + return Ok(CheckOutcome { + id: Some(check.id.clone()), + category: parse_category(&check.category), + severity: parse_severity(&check.severity), + label: check.label.clone(), + passed: true, + detail: Some(format!( + "Archive is {} MB. The digest is verified during install, so the download was skipped.", + size / (1024 * 1024) + )), + }); + } + } + + let (_downloaded, computed) = archive.fetch().await?; let passed = computed.eq_ignore_ascii_case(&expected); Ok(CheckOutcome { id: Some(check.id.clone()), diff --git a/src-tauri/src/builtin_steps.rs b/src-tauri/src/builtin_steps.rs index 0ede9a0..c96bbad 100644 --- a/src-tauri/src/builtin_steps.rs +++ b/src-tauri/src/builtin_steps.rs @@ -251,27 +251,9 @@ fn run_extract_zip( let Some(safe_name) = sanitize_archive_member(&name) else { return Err(format!("extract-zip: unsafe archive member '{name}'.")); }; - let basename = Path::new(&safe_name) - .file_name() - .and_then(|name| name.to_str()) - .unwrap_or_default() - .to_ascii_lowercase(); - - let target = if let Some(proxy_name) = proxy.as_ref() { - if basename == "reshade64.dll" || basename == "dxgi.dll" { - let candidate = context.executable_directory.join(proxy_name); - affected.push(proxy_name.clone()); - candidate - } else { - let candidate = context.executable_directory.join(&safe_name); - affected.push(safe_name.clone()); - candidate - } - } else { - let candidate = context.executable_directory.join(&safe_name); - affected.push(safe_name.clone()); - candidate - }; + let relative = archive_member_target(&safe_name, proxy.as_deref()); + let target = context.executable_directory.join(&relative); + affected.push(relative); if let Some(parent) = target.parent() { fs::create_dir_all(parent).map_err(|error| { @@ -337,17 +319,35 @@ fn run_verify_hash( }) } +/// Read a step's target path. +/// +/// The `*Field` form names a config field; the bare form is a literal. +/// Both are accepted so a recipe can point at a constant path without +/// inventing a config field for it — the same rule `download-file` and +/// `verify-hash` already follow, and the one +/// [`plan_step_targets`] mirrors when it predicts the same target. +fn path_param( + step: &StepSpec, + config: &ResolvedConfig, + field_key: &str, + literal_key: &str, +) -> Result { + if let Some(name) = param_string(step, field_key) { + return config + .get_string(name) + .ok_or_else(|| format!("{}: config field '{name}' is missing.", step.kind)); + } + param_string(step, literal_key) + .map(str::to_owned) + .ok_or_else(|| format!("{}: {field_key} or {literal_key} is required.", step.kind)) +} + fn run_file_delete( step: &StepSpec, context: &StepContext<'_>, ) -> Result { - let path_field = param_string(step, "pathField") - .ok_or_else(|| "file-delete: pathField is required.".to_owned())?; - let path = context - .config - .get_string(path_field) - .ok_or_else(|| format!("file-delete: config field '{path_field}' is missing."))?; - let resolved = resolve_path(context.executable_directory, &path); + let path = path_param(step, context.config, "pathField", "path")?; + let resolved = resolve_write_target(step, context.executable_directory, &path)?; if !resolved.is_file() { return Err(format!("file-delete: '{path}' is not a regular file.")); } @@ -364,13 +364,8 @@ fn run_write_text_file( step: &StepSpec, context: &StepContext<'_>, ) -> Result { - let path_field = param_string(step, "pathField") - .ok_or_else(|| "write-text-file: pathField is required.".to_owned())?; - let path = context - .config - .get_string(path_field) - .ok_or_else(|| format!("write-text-file: config field '{path_field}' is missing."))?; - let resolved = resolve_path(context.executable_directory, &path); + let path = path_param(step, context.config, "pathField", "path")?; + let resolved = resolve_write_target(step, context.executable_directory, &path)?; if let Some(parent) = resolved.parent() { fs::create_dir_all(parent).map_err(|error| { format!("write-text-file: could not create parent dir: {error}") @@ -409,17 +404,15 @@ fn run_spawn_process( let child = command .spawn() .map_err(|error| format!("spawn-process: could not start '{executable}': {error}"))?; + // The child is deliberately detached: the recipe starts a tray + // process that must outlive the install. Moddin tracks its liveness + // through process snapshots, so the PID is not carried onward — + // putting it in `affected_paths` would make a PID look like a file. + let _ = child; Ok(StepResult { kind: step.kind.clone(), description: step.description.clone(), affected_paths: vec![resolved.to_string_lossy().into_owned()], - // Keep the child PID handy for callers via the runtime; - // today we just drop it — Moddin tracks tray liveness through - // process snapshots instead of carrying child handles. - }) - .map(|mut result| { - result.affected_paths.push(format!("pid:{}", child.id())); - result }) } @@ -427,13 +420,8 @@ fn run_write_binary_file( step: &StepSpec, context: &StepContext<'_>, ) -> Result { - let path_field = param_string(step, "pathField") - .ok_or_else(|| "write-binary-file: pathField is required.".to_owned())?; - let path = context - .config - .get_string(path_field) - .ok_or_else(|| format!("write-binary-file: config field '{path_field}' is missing."))?; - let resolved = resolve_path(context.executable_directory, &path); + let path = path_param(step, context.config, "pathField", "path")?; + let resolved = resolve_write_target(step, context.executable_directory, &path)?; if let Some(parent) = resolved.parent() { fs::create_dir_all(parent).map_err(|error| { format!("write-binary-file: could not create parent dir: {error}") @@ -455,20 +443,15 @@ fn run_move_file( step: &StepSpec, context: &StepContext<'_>, ) -> Result { - let from_field = param_string(step, "fromField") - .ok_or_else(|| "move-file: fromField is required.".to_owned())?; - let to_field = param_string(step, "toField") - .ok_or_else(|| "move-file: toField is required.".to_owned())?; - let from_path = context - .config - .get_string(from_field) - .ok_or_else(|| format!("move-file: config field '{from_field}' is missing."))?; - let to_path = context - .config - .get_string(to_field) - .ok_or_else(|| format!("move-file: config field '{to_field}' is missing."))?; - let from = resolve_path(context.executable_directory, &from_path); - let to = resolve_path(context.executable_directory, &to_path); + let from_path = path_param(step, context.config, "fromField", "from")?; + let to_path = path_param(step, context.config, "toField", "to")?; + let from = resolve_write_target(step, context.executable_directory, &from_path)?; + let to = resolve_write_target(step, context.executable_directory, &to_path)?; + if from == to { + return Err(format!( + "move-file: source and destination are the same path ('{from_path}')." + )); + } if let Some(parent) = to.parent() { fs::create_dir_all(parent).map_err(|error| { format!("move-file: could not create destination parent: {error}") @@ -692,6 +675,189 @@ fn resolve_path(base: &Path, candidate: &str) -> PathBuf { } } +/// Relative name an archive member will be written to, after +/// sanitisation and the optional proxy-DLL rename. +/// +/// Shared by the executor and by [`plan_step_targets`] so the files a +/// step is about to overwrite are exactly the files the transaction +/// backs up first. +fn archive_member_target(safe_name: &str, proxy: Option<&str>) -> String { + if let Some(proxy_name) = proxy.filter(|name| !name.is_empty()) { + let basename = Path::new(safe_name) + .file_name() + .and_then(|name| name.to_str()) + .unwrap_or_default() + .to_ascii_lowercase(); + if basename == "reshade64.dll" || basename == "dxgi.dll" { + return proxy_name.to_owned(); + } + } + safe_name.to_owned() +} + +/// Resolve a step's *write* target, refusing to escape its directory. +/// +/// Step paths are templates rendered from config, so a `..` inside a +/// config value or a recipe would aim anywhere on the machine. That is +/// refused outright — no step legitimately needs it, so there is no +/// opt-in and no escape hatch. +/// +/// An absolute path is a different case and stays allowed. Config +/// fields of type `path` exist precisely to name a location outside the +/// game folder (an OpenXR runtime manifest, a tray INI beside the +/// executable), and a recipe author writes that value on purpose. The +/// transaction store separately refuses to *back up* anything outside +/// the install root, and the runner reports those paths as not covered +/// by Undo. +fn resolve_write_target( + step: &StepSpec, + base: &Path, + candidate: &str, +) -> Result { + let path = Path::new(candidate); + if path.is_absolute() { + return Ok(path.to_path_buf()); + } + + // Reject `..` that walks out of `base`, while still allowing + // normalised inner paths such as `BepInEx/core/../winhttp.dll`. + let mut depth: i64 = 0; + for component in path.components() { + match component { + std::path::Component::ParentDir => { + depth -= 1; + if depth < 0 { + return Err(format!( + "{}: path '{}' escapes the install directory.", + step.kind, candidate + )); + } + } + std::path::Component::Normal(_) => depth += 1, + std::path::Component::CurDir => {} + std::path::Component::RootDir | std::path::Component::Prefix(_) => {} + } + } + + Ok(base.join(path)) +} + +/// Filesystem paths a step is about to create, modify or remove, +/// resolved to absolute paths. +/// +/// The runner calls this *before* the step runs and hands the result to +/// the transaction store, so a file the step is about to overwrite is +/// backed up in its original state and a step that fails halfway still +/// leaves a rollback record behind. Steps that write outside the +/// install directory are returned too; the runner filters them out of +/// the rollback set but keeps them for disclosure. +pub fn plan_step_targets( + step: &StepSpec, + context: &StepContext<'_>, +) -> Result, String> { + let mut targets: Vec = Vec::new(); + let mut add = |candidate: &str| -> Result<(), String> { + let path = resolve_write_target(step, context.executable_directory, candidate)?; + if !targets.contains(&path) { + targets.push(path); + } + Ok(()) + }; + + match step.kind.as_str() { + "download-file" => { + // A bare filename lands in Moddin's own cache, which is not + // part of the game's rollback scope. Anything else is written + // next to the game and must be covered. + if let Some(target) = param_string(step, "targetField") + .and_then(|field| context.config.get_string(field)) + .or_else(|| param_string(step, "target").map(str::to_owned)) + { + let path = Path::new(&target); + let is_bare = path.components().count() == 1 && path.file_name().is_some(); + if !is_bare { + add(&target)?; + } + } + } + "extract-zip" => { + let Some(archive_path) = param_string(step, "archivePathField") + .and_then(|field| context.config.get_string(field)) + .or_else(|| param_string(step, "archivePath").map(str::to_owned)) + else { + return Err( + "extract-zip: step needs archiveBytesField, archivePathField or archivePath." + .to_owned(), + ); + }; + let resolved = resolve_download_path(context.executable_directory, &archive_path); + if !resolved.is_file() { + // The previous `download-file` step has not run yet or + // failed. Returning no targets keeps the transaction + // accurate; the step itself will report the real error. + return Ok(targets); + } + let bytes = fs::read(&resolved).map_err(|error| { + format!( + "extract-zip: could not read archive '{}': {error}", + resolved.display() + ) + })?; + let mut archive = ZipArchive::new(Cursor::new(bytes.as_slice())) + .map_err(|error| format!("extract-zip: could not open zip: {error}"))?; + let proxy = param_string(step, "proxyField") + .and_then(|field| context.config.get_string(field)); + for index in 0..archive.len() { + let entry = archive + .by_index(index) + .map_err(|error| format!("extract-zip: entry {index} unreadable: {error}"))?; + if entry.is_dir() { + continue; + } + let name = entry.name().to_owned(); + let Some(safe_name) = sanitize_archive_member(&name) else { + return Err(format!("extract-zip: unsafe archive member '{name}'.")); + }; + let relative = archive_member_target(&safe_name, proxy.as_deref()); + let target = context.executable_directory.join(&relative); + if !targets.contains(&target) { + targets.push(target); + } + } + } + "file-delete" | "write-text-file" | "write-binary-file" => { + if let Some(path) = param_string(step, "pathField") + .and_then(|field| context.config.get_string(field)) + .or_else(|| param_string(step, "path").map(str::to_owned)) + { + add(&path)?; + } + } + "move-file" => { + // Both ends matter: `from` is restored if the move clobbered + // it, and `to` is restored to its previous content. + if let Some(from) = param_string(step, "fromField") + .and_then(|field| context.config.get_string(field)) + .or_else(|| param_string(step, "from").map(str::to_owned)) + { + add(&from)?; + } + if let Some(to) = param_string(step, "toField") + .and_then(|field| context.config.get_string(field)) + .or_else(|| param_string(step, "to").map(str::to_owned)) + { + add(&to)?; + } + } + // verify-hash and kill-process are read-only; spawn-process does + // not modify the executable; registry-write and registry-delete + // do not touch the filesystem at all. + _ => {} + } + + Ok(targets) +} + /// Where `download-file` puts payloads and where `extract-zip` looks /// for them when a recipe names a bare filename. /// @@ -807,6 +973,7 @@ mod tests { let spec: &'a CapabilitySpec = Box::leak(Box::new(CapabilitySpec { id: "probe".to_owned(), display_name: "Probe".to_owned(), + description: None, category: "qol".to_owned(), status: "available".to_owned(), supported_engines: Vec::new(), diff --git a/src-tauri/src/capability.rs b/src-tauri/src/capability.rs index e83eb0f..3050300 100644 --- a/src-tauri/src/capability.rs +++ b/src-tauri/src/capability.rs @@ -76,6 +76,12 @@ pub struct CapabilitySpec { /// specs (the runner asserts this at load time). pub id: String, pub display_name: String, + /// One sentence, written for a person rather than for a recipe + /// author: what the mod does for the player, not how it is + /// installed. Shown as the card's supporting line. Absent means the + /// UI falls back to the technical id rather than inventing copy. + #[serde(default)] + pub description: Option, pub category: String, pub status: String, #[serde(default)] diff --git a/src-tauri/src/capability_runner.rs b/src-tauri/src/capability_runner.rs index 76758ad..c22406c 100644 --- a/src-tauri/src/capability_runner.rs +++ b/src-tauri/src/capability_runner.rs @@ -41,6 +41,58 @@ const CHEEKY_FOVEATED_DLSS_YAML: &str = const RESHADE_YAML: &str = include_str!("../capabilities/reshade.yaml"); const OPENXR_HELPERS_YAML: &str = include_str!("../capabilities/openxr-helpers.yaml"); const UEVR_YAML: &str = include_str!("../capabilities/uevr.yaml"); +const BEPINEX_YAML: &str = include_str!("../capabilities/bepinex.yaml"); +const UE4SS_YAML: &str = include_str!("../capabilities/ue4ss.yaml"); +const REFRAMEWORK_YAML: &str = include_str!("../capabilities/reframework.yaml"); + +/// Every capability recipe compiled into the binary. +/// +/// Kept as one list so [`CapabilityRegistry::load`] and +/// [`CapabilityRegistry::load_with_local_dir`] cannot drift apart, and so +/// `tests::every_built_in_spec_installs_from_a_local_archive` can cover +/// each recipe automatically as specs are added. +const BUILT_IN_YAML: &[&str] = &[ + OFXR_BRIDGE_YAML, + OPTISCALER_YAML, + CHEEKY_FOVEATED_DLSS_YAML, + RESHADE_YAML, + OPENXR_HELPERS_YAML, + UEVR_YAML, + BEPINEX_YAML, + UE4SS_YAML, + REFRAMEWORK_YAML, +]; + +/// Parse the built-in recipe list into `specs`, failing loudly on a +/// duplicate id or malformed YAML. +fn insert_built_ins(specs: &mut HashMap) { + for raw in BUILT_IN_YAML { + match serde_yaml::from_str::(raw) { + Ok(mut spec) => { + spec.origin = SpecOrigin::BuiltIn; + if specs.contains_key(&spec.id) { + panic!( + "duplicate built-in capability id '{id}'", + id = spec.id + ); + } + specs.insert(spec.id.clone(), spec); + } + Err(error) => { + // A YAML scalar containing ": " silently becomes a + // mapping, which is the single most common authoring + // mistake in a safety note or a description. Name the + // recipe so the failure is actionable. + let id = raw + .lines() + .find_map(|line| line.strip_prefix("id:")) + .map(str::trim) + .unwrap_or(""); + panic!("could not parse built-in capability '{id}': {error}"); + } + } + } +} /// Directory Moddin Desktop scans at startup for user-provided /// capability recipes. Files placed here are loaded as `SpecOrigin::Local`, @@ -73,31 +125,7 @@ impl CapabilityRegistry { /// logged and skipped — the runner keeps the first one it finds. pub fn load() -> Self { let mut specs = HashMap::new(); - - for raw in [ - OFXR_BRIDGE_YAML, - OPTISCALER_YAML, - CHEEKY_FOVEATED_DLSS_YAML, - RESHADE_YAML, - OPENXR_HELPERS_YAML, - UEVR_YAML, - ] { - match serde_yaml::from_str::(raw) { - Ok(mut spec) => { - spec.origin = SpecOrigin::BuiltIn; - if specs.contains_key(&spec.id) { - panic!( - "duplicate built-in capability id '{id}'", - id = spec.id - ); - } - specs.insert(spec.id.clone(), spec); - } - Err(error) => { - panic!("could not parse built-in capability YAML: {error}"); - } - } - } + insert_built_ins(&mut specs); if let Some(local_dir) = local_capabilities_dir() { Self::load_local_into(&local_dir, &mut specs); @@ -112,19 +140,7 @@ impl CapabilityRegistry { /// without restarting. pub fn load_with_local_dir>(local_dir: P) -> Self { let mut registry = Self::default(); - for raw in [ - OFXR_BRIDGE_YAML, - OPTISCALER_YAML, - CHEEKY_FOVEATED_DLSS_YAML, - RESHADE_YAML, - OPENXR_HELPERS_YAML, - UEVR_YAML, - ] { - if let Ok(mut spec) = serde_yaml::from_str::(raw) { - spec.origin = SpecOrigin::BuiltIn; - registry.specs.insert(spec.id.clone(), spec); - } - } + insert_built_ins(&mut registry.specs); Self::load_local_into(local_dir.as_ref(), &mut registry.specs); registry } @@ -279,30 +295,72 @@ fn execute_install( }; let mut step_results = Vec::new(); let mut affected = Vec::new(); + + // Plan each step's targets, back them up, then run the step. The + // order matters: a file the step is about to overwrite has to be in + // the backup *before* it is replaced, and a step that fails halfway + // must still leave a record that can undo what already landed. + let metadata = build_metadata(spec, config); + let label = format!("Install {} for {}", spec.display_name, game_name); + let mut record: Option = None; + let mut outside_root: Vec = Vec::new(); + for step in &spec.install { - let result = builtin_steps::execute_step(step, &step_context)?; + let planned = builtin_steps::plan_step_targets(step, &step_context)?; + + // Only paths under the install directory are inside the game's + // rollback scope. A recipe that writes elsewhere (an OpenXR + // runtime manifest, a tray INI) is still allowed to do so, but + // the user is told it is not covered by Undo. + let mut in_scope = Vec::new(); + for path in planned { + if path.starts_with(install_directory) { + in_scope.push(path); + } else { + outside_root.push(path.to_string_lossy().into_owned()); + } + } + + if !in_scope.is_empty() { + record = Some(match record { + Some(existing) => { + transaction::add_files_to_transaction(existing, install_directory, &in_scope)? + } + None => transaction::begin_file_set_transaction( + install_directory, + &in_scope, + &spec.id, + &label, + game_id, + metadata.clone(), + )?, + }); + } + + let result = match builtin_steps::execute_step(step, &step_context) { + Ok(result) => result, + Err(error) => { + // Roll back whatever this install already wrote instead + // of leaving orphaned files with no undo record. + if let Some(prepared) = record { + let _ = transaction::restore_record(prepared); + } + return Err(error); + } + }; affected.extend(result.affected_paths.clone()); step_results.push(result); } - let transaction = if !affected.is_empty() { - let metadata = build_metadata(spec, config); - let record = transaction::begin_file_set_transaction( - install_directory, - &affected - .iter() - .map(std::path::PathBuf::from) - .collect::>(), - &spec.id, - &format!("Install {} for {}", spec.display_name, game_name), - game_id, - metadata, - )?; - Some(transaction::mark_applied(record)?) - } else { - None + let transaction = match record { + Some(prepared) => Some(transaction::mark_applied(prepared)?), + None => None, }; + if !outside_root.is_empty() { + affected.extend(outside_root); + } + Ok(InstallResult { capability_id: spec.id.clone(), transaction, @@ -886,6 +944,9 @@ pub fn capability_compatibility( pub struct CapabilitySummary { pub id: String, pub display_name: String, + /// Same player-facing line as [`CapabilitySpec::description`]. + /// The card shows this instead of the technical id. + pub description: Option, pub category: String, pub status: String, /// Provenance — drives the UI badge ("Verified", "Local", @@ -903,6 +964,7 @@ pub fn capability_list() -> Vec { CapabilitySummary { id: spec.id.clone(), display_name: spec.display_name.clone(), + description: spec.description.clone(), category: spec.category.clone(), status: spec.status.clone(), origin: spec.origin, @@ -959,6 +1021,7 @@ pub fn capability_reload( CapabilitySummary { id: spec.id.clone(), display_name: spec.display_name.clone(), + description: spec.description.clone(), category: spec.category.clone(), status: spec.status.clone(), origin: spec.origin, @@ -1119,8 +1182,21 @@ pub async fn community_capability_install( mod tests { use super::*; use crate::capability::{CheckSpec, SpecOrigin}; + use serde_json::json; + use std::collections::BTreeMap; use std::fs; + /// Build a resolved config from key/value pairs, matching how the + /// Tauri command layer deserialises a request. + fn config_with(pairs: &[(&str, &str)]) -> ResolvedConfig { + ResolvedConfig { + values: pairs + .iter() + .map(|(key, value)| ((*key).to_owned(), json!(value))) + .collect(), + } + } + #[test] fn registry_loads_ofxr_bridge() { let registry = CapabilityRegistry::load(); @@ -1941,4 +2017,250 @@ install: let _ = fs::remove_dir_all(&work); } + + /// Guards the bug class behind "adding a mod is one YAML file". + /// + /// Every shipped recipe that is `available` and actually installs + /// something has to read its payload from a local path that a + /// previous `download-file` step produced. Pointing `extract-zip` at + /// a URL — which every recipe used to do — made the whole install + /// chain fail on its first step, at runtime, with a file-not-found + /// the user had no way to act on. + #[test] + fn every_available_built_in_recipe_reads_its_payload_from_the_download_cache() { + for raw in BUILT_IN_YAML { + let spec: CapabilitySpec = serde_yaml::from_str(raw) + .unwrap_or_else(|error| panic!("built-in capability YAML does not parse: {error}")); + if spec.status != "available" || spec.install.is_empty() { + continue; + } + + let extracts_anything = spec.install.iter().any(|step| step.kind == "extract-zip"); + if !extracts_anything { + continue; + } + + assert!( + spec.install.iter().any(|step| step.kind == "download-file"), + "'{}' extracts an archive but never downloads one", + spec.id + ); + + for step in spec.install.iter().filter(|step| step.kind == "extract-zip") { + let reference = step + .params + .get("archivePath") + .and_then(|value| value.as_str()) + .map(str::to_owned) + .or_else(|| { + step.params + .get("archivePathField") + .and_then(|value| value.as_str()) + .map(|field| { + format!( + "{field} -> {}", + spec.config_schema + .iter() + .find(|declared| declared.name == field) + .map(|declared| declared.field_type.clone()) + .unwrap_or_else(|| "missing field".to_owned()) + ) + }) + }) + .expect("extract-zip needs archivePath or archivePathField"); + + assert!( + !reference.contains("://"), + "'{}' points extract-zip at a URL ({reference}); \ + a download-file step has to fetch it into Moddin's cache first", + spec.id + ); + } + } + } + + /// The transaction has to be opened *before* the step that overwrites + /// a file runs. Opened afterwards — as it used to be — the backup + /// holds the mod's own output, so Undo restores the mod instead of + /// the player's file and the original is gone for good. + #[test] + fn an_install_backs_up_an_existing_game_file_before_overwriting_it() { + let appdata = IsolatedAppdata::new("overwrite-backup"); + let caps = appdata.dir.join("Moddin").join("capabilities"); + write_local_capability( + &caps, + "overwriter", + r#" +id: overwriter +displayName: Overwriter +category: qol +status: available +configSchema: + - name: target + type: string + required: true +install: + - kind: write-text-file + description: Overwrite a file the game already shipped with. + params: + pathField: target + template: replaced +"#, + ); + + let work = temp_root("overwrite-backup-work"); + let executable = work.join("game"); + fs::create_dir_all(&executable).expect("game dir"); + let victim = executable.join("settings.ini"); + fs::write(&victim, b"original").expect("seed original"); + + let registry = CapabilityRegistry::load_with_local_dir(&caps); + let config = config_with(&[("target", "settings.ini")]); + + let result = run_install( + ®istry, + "overwriter", + "overwrite-game", + "Overwrite Game", + &config, + &work, + &executable, + false, + ) + .expect("install runs"); + assert!( + result.transaction.is_some(), + "an install that touches a file records a transaction" + ); + assert_eq!( + fs::read_to_string(&victim).expect("read victim"), + "replaced", + "the step really did overwrite the file" + ); + + run_uninstall( + ®istry, + "overwriter", + "overwrite-game", + "Overwrite Game", + &work, + ) + .expect("uninstall runs"); + + assert_eq!( + fs::read_to_string(&victim).expect("read victim after undo"), + "original", + "Undo must restore the player's original content, not the mod's output" + ); + + let _ = fs::remove_dir_all(&work); + } + + /// A step that fails halfway must not leave the files the earlier + /// steps already wrote behind with no rollback record. Before the + /// runner was reordered, `?` returned out of the step loop before + /// any transaction existed, so those writes were simply orphaned. + #[test] + fn a_failed_step_rolls_back_what_the_install_already_wrote() { + let appdata = IsolatedAppdata::new("partial-failure"); + let caps = appdata.dir.join("Moddin").join("capabilities"); + write_local_capability( + &caps, + "half-writer", + r#" +id: half-writer +displayName: Half writer +category: qol +status: available +install: + - kind: write-text-file + description: Succeeds. + params: + path: first.ini + template: written + - kind: file-delete + description: Fails on purpose — there is no such file to delete. + params: + path: does-not-exist.ini +"#, + ); + + let work = temp_root("partial-failure-work"); + let executable = work.join("game"); + fs::create_dir_all(&executable).expect("game dir"); + + let registry = CapabilityRegistry::load_with_local_dir(&caps); + let error = run_install( + ®istry, + "half-writer", + "partial-game", + "Partial Game", + &ResolvedConfig::default(), + &work, + &executable, + false, + ) + .expect_err("the second step fails"); + + assert!(error.contains("does-not-exist.ini"), "{error}"); + assert!( + !executable.join("first.ini").exists(), + "the file the first step wrote must be rolled back, not orphaned" + ); + + let _ = fs::remove_dir_all(&work); + } + + /// Step paths are rendered from config, so `..` in a config value + /// would otherwise let a recipe write anywhere on the machine. + #[test] + fn a_step_cannot_write_outside_the_install_root_without_saying_so() { + let appdata = IsolatedAppdata::new("escape-root"); + let caps = appdata.dir.join("Moddin").join("capabilities"); + write_local_capability( + &caps, + "escaper", + r#" +id: escaper +displayName: Escaper +category: qol +status: available +configSchema: + - name: target + type: string + required: true +install: + - kind: write-text-file + description: Try to climb out of the game folder. + params: + pathField: target + template: owned +"#, + ); + + let work = temp_root("escape-root-work"); + let executable = work.join("game"); + fs::create_dir_all(&executable).expect("game dir"); + let outside = work.join("hijacked.ini"); + + let registry = CapabilityRegistry::load_with_local_dir(&caps); + let config = config_with(&[("target", "../hijacked.ini")]); + + let error = run_install( + ®istry, + "escaper", + "escape-game", + "Escape Game", + &config, + &work, + &executable, + false, + ) + .expect_err("traversal out of the install root is refused"); + + assert!(error.contains("escapes the install directory"), "{error}"); + assert!(!outside.exists(), "nothing was written outside the root"); + + let _ = fs::remove_dir_all(&work); + } } \ No newline at end of file diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index d438707..d79e271 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -194,9 +194,13 @@ pub mod test_support { /// too, or a concurrent env test's temp dir can vanish mid-read. pub fn env_lock() -> std::sync::MutexGuard<'static, ()> { static LOCK: std::sync::OnceLock> = std::sync::OnceLock::new(); + // Poisoning is recovered from on purpose. A failing test that + // held the lock used to make every other env-touching test fail + // with "env test lock", hiding the one real failure behind a + // dozen fake ones. LOCK.get_or_init(|| std::sync::Mutex::new(())) .lock() - .expect("env test lock") + .unwrap_or_else(|poisoned| poisoned.into_inner()) } } diff --git a/src-tauri/src/transaction.rs b/src-tauri/src/transaction.rs index 3a46bad..6e6774b 100644 --- a/src-tauri/src/transaction.rs +++ b/src-tauri/src/transaction.rs @@ -313,6 +313,66 @@ pub fn backup_file_with_metadata( Ok(record) } +/// Snapshot one target into the transaction's backup directory. +/// +/// A file that already exists is copied aside so rollback can restore the +/// user's original content; a file that does not exist yet is recorded +/// with `existed_before: false` so rollback deletes it instead. `index` +/// keeps backup names unique and stably ordered. +fn snapshot_target( + backup_directory: &Path, + index: usize, + target_root: &Path, + target: &Path, +) -> Result { + ensure_target_within_root(target_root, target)?; + + let existed_before = target.is_file(); + let backup_path = if existed_before { + let file_name = target + .file_name() + .and_then(|name| name.to_str()) + .unwrap_or("file.bin"); + let backup = backup_directory.join(format!("{index:04}-{file_name}")); + fs::copy(target, &backup).map_err(|error| { + format!( + "Could not back up existing file '{}': {error}", + target.display() + ) + })?; + Some(backup.to_string_lossy().into_owned()) + } else { + None + }; + + Ok(TransactionFile { + target_path: target.to_string_lossy().into_owned(), + backup_path, + existed_before, + }) +} + +/// Directories between `target_root` and each target that do not exist +/// yet, deepest first, so rollback removes them in a safe order. +fn collect_created_directories(target_root: &Path, targets: &[PathBuf]) -> Vec { + let mut directories = Vec::new(); + for target in targets { + let mut current = target.parent(); + while let Some(directory) = current { + if directory == target_root || !directory.starts_with(target_root) { + break; + } + if !directory.exists() { + directories.push(directory.to_string_lossy().into_owned()); + } + current = directory.parent(); + } + } + directories.sort_by_key(|path| std::cmp::Reverse(Path::new(path).components().count())); + directories.dedup(); + directories +} + pub fn begin_file_set_transaction( target_root: &Path, targets: &[PathBuf], @@ -333,48 +393,10 @@ pub fn begin_file_set_transaction( let mut files = Vec::with_capacity(unique_targets.len()); for (index, target) in unique_targets.iter().enumerate() { - ensure_target_within_root(target_root, target)?; - - let existed_before = target.is_file(); - let backup_path = if existed_before { - let file_name = target - .file_name() - .and_then(|name| name.to_str()) - .unwrap_or("file.bin"); - let backup = backup_directory.join(format!("{index:04}-{file_name}")); - fs::copy(target, &backup).map_err(|error| { - format!( - "Could not back up existing file '{}': {error}", - target.display() - ) - })?; - Some(backup.to_string_lossy().into_owned()) - } else { - None - }; - - files.push(TransactionFile { - target_path: target.to_string_lossy().into_owned(), - backup_path, - existed_before, - }); + files.push(snapshot_target(&backup_directory, index, target_root, target)?); } - let mut directories = Vec::new(); - for target in &unique_targets { - let mut current = target.parent(); - while let Some(directory) = current { - if directory == target_root || !directory.starts_with(target_root) { - break; - } - if !directory.exists() { - directories.push(directory.to_string_lossy().into_owned()); - } - current = directory.parent(); - } - } - directories.sort_by_key(|path| std::cmp::Reverse(Path::new(path).components().count())); - directories.dedup(); + let directories = collect_created_directories(target_root, &unique_targets); let record = TransactionRecord { id, @@ -394,6 +416,62 @@ pub fn begin_file_set_transaction( Ok(record) } +/// Append newly-planned targets to an already-open transaction. +/// +/// The capability runner plans each step's targets immediately before it +/// runs that step — an archive cannot be inspected until the previous +/// `download-file` step has fetched it — so the transaction has to grow as +/// the install progresses. Targets already recorded, and duplicates within +/// `extra_targets`, are ignored. +pub fn add_files_to_transaction( + record: TransactionRecord, + target_root: &Path, + extra_targets: &[PathBuf], +) -> Result { + let mut record = record; + validate_record(&record, &record.id.clone())?; + if record.status != "prepared" { + return Err(format!( + "Transaction '{}' can only be extended while prepared, but it is '{}'.", + record.id, record.status + )); + } + + let mut fresh: Vec = Vec::new(); + for target in extra_targets { + if record.files.iter().any(|file| Path::new(&file.target_path) == target) { + continue; + } + if fresh.contains(target) { + continue; + } + fresh.push(target.clone()); + } + if fresh.is_empty() { + return Ok(record); + } + + let backup_directory = PathBuf::from(&record.backup_path); + let index_offset = record.files.len(); + for (offset, target) in fresh.iter().enumerate() { + record + .files + .push(snapshot_target(&backup_directory, index_offset + offset, target_root, target)?); + } + + let mut directories = record.created_directories.clone(); + for directory in collect_created_directories(target_root, &fresh) { + if !directories.contains(&directory) { + directories.push(directory); + } + } + directories.sort_by_key(|path| std::cmp::Reverse(Path::new(path).components().count())); + record.created_directories = directories; + + write_record(&record)?; + Ok(record) +} + pub fn mark_applied(mut record: TransactionRecord) -> Result { record.status = "applied".to_owned(); write_record(&record)?; From 52b96908e4630c691810f450420cf01f8d18c023 Mon Sep 17 00:00:00 2001 From: Marco Date: Tue, 29 Sep 2026 20:27:04 -0300 Subject: [PATCH 03/51] fix(trust): stop trusting the on-disk key pin F-03: pinned-public-key.bin was returned unverified, so any 32 bytes written there became the key the community catalog was checked against. It is now validated against a compiled keyring (TRUSTED_PUBLIC_KEYS_B64) holding the active key plus the one retired in the 2026-09-24 rotation. A pin that does not match is discarded, logged, and replaced by the anchor. REVOKED_KEY_FINGERPRINTS is wired for a key that turns out to be compromised. Four tests cover this, including one that asserts its attacker key really decodes, so it cannot pass vacuously. --- src-tauri/src/community_catalog.rs | 204 ++++++++++++++++++++++++++--- 1 file changed, 185 insertions(+), 19 deletions(-) diff --git a/src-tauri/src/community_catalog.rs b/src-tauri/src/community_catalog.rs index 34347ce..5aa71fb 100644 --- a/src-tauri/src/community_catalog.rs +++ b/src-tauri/src/community_catalog.rs @@ -58,6 +58,37 @@ const MAX_TTL_SECONDS: u64 = 30 * 24 * 60 * 60; // 30d ceiling const BOOTSTRAP_PUBLIC_KEY_B64: &str = "R2oSrMGh0d6pHIWFHZwvU+sA+wK1Uo/vaBq/L1WJ6GU="; +/// Every key this build is willing to trust, newest first. +/// +/// This is the trust anchor. `pinned-public-key.bin` on disk is a +/// *cache* of which of these is active, not an independent source of +/// trust: anything that could write to `%LOCALAPPDATA%` could also +/// write a key of its own there, and the previous implementation +/// returned whatever 32 bytes it found without ever comparing it to +/// the compiled value. That is trust-on-first-use with no anchor, in a +/// product whose pitch is that it signs what it installs. +/// +/// Rotation is therefore: add the new key to this list and ship a +/// build, which is the same cadence the keyring in the community repo +/// documents. Keeping the previous key for one release lets users who +/// have not updated yet finish a verification instead of seeing a hard +/// failure. A key that is compromised goes in `REVOKED_KEY_FINGERPRINTS` +/// below and never has to wait for a build to be neutralised in the +/// on-disk cache, because the cache is only ever accepted when it +/// matches a key that is neither absent from this list nor revoked. +const TRUSTED_PUBLIC_KEYS_B64: &[&str] = &[ + // @moddin-bot, active since the 2026-09-24 rotation. + "R2oSrMGh0d6pHIWFHZwvU+sA+wK1Uo/vaBq/L1WJ6GU=", + // @marcoasjunior, retired at the same rotation. Kept so a user who + // has not updated since 2026-09-24 can still verify a catalog that + // was signed before the change. Safe to drop after the next release. + "Mh/WGQ0kCviGtiX/8wLB5fqBCLgtVR/4smlVai13xs8=", +]; + +/// Fingerprints (first 16 hex of SHA-256, as shown in the UI) that +/// must never be trusted, even if they appear in [`TRUSTED_PUBLIC_KEYS_B64`]. +const REVOKED_KEY_FINGERPRINTS: &[&str] = &[]; + fn community_dir() -> Option { std::env::var_os("LOCALAPPDATA") .map(PathBuf::from) @@ -143,34 +174,75 @@ fn now_unix() -> u64 { .unwrap_or(0) } -fn ensure_bootstrap_key_material(dir: &Path) -> Result { - let path = dir.join("pinned-public-key.bin"); - if let Ok(bytes) = std::fs::read(&path) { - if bytes.len() == 32 { - if let Ok(array) = - <[u8; 32]>::try_from(bytes.as_slice()).map_err(|error: std::array::TryFromSliceError| error.to_string()) - { - if let Ok(key) = VerifyingKey::from_bytes(&array) { - return Ok(key); - } - } - } - } - // Fall back to the compile-time bootstrap key. +/// Decode one base64 Ed25519 public key. +fn decode_public_key(encoded: &str) -> Result { let bytes = BASE64 - .decode(BOOTSTRAP_PUBLIC_KEY_B64) - .map_err(|error| format!("bootstrap public key is not valid base64: {error}"))?; + .decode(encoded.trim()) + .map_err(|error| format!("public key is not valid base64: {error}"))?; if bytes.len() != 32 { return Err(format!( - "bootstrap public key must be 32 bytes, got {}", + "Ed25519 public key must be 32 bytes, got {}", bytes.len() )); } - let bytes_array: [u8; 32] = bytes + let array: [u8; 32] = bytes .as_slice() .try_into() .map_err(|error: std::array::TryFromSliceError| error.to_string())?; - VerifyingKey::from_bytes(&bytes_array).map_err(|error| format!("invalid Ed25519 key: {error}")) + VerifyingKey::from_bytes(&array).map_err(|error| format!("invalid Ed25519 key: {error}")) +} + +/// Is this key one this build is willing to trust? +fn is_trusted(key: &VerifyingKey) -> bool { + if REVOKED_KEY_FINGERPRINTS.contains(&fingerprint(key).as_str()) { + return false; + } + TRUSTED_PUBLIC_KEYS_B64 + .iter() + .filter_map(|encoded| decode_public_key(encoded).ok()) + .any(|trusted| trusted.to_bytes() == key.to_bytes()) +} + +/// Resolve the key used to verify the community catalog. +/// +/// The on-disk pin is accepted only when it matches a key from +/// [`TRUSTED_PUBLIC_KEYS_B64`]. A pin that does not match is discarded +/// and the anchor is used instead — so tampering with the cache +/// degrades to "no worse than a fresh install" rather than to +/// "attacker-chosen key". A corrupt or revoked pin is reported so the +/// user is not silently handed a different key than the one they +/// were shown a fingerprint for. +fn ensure_bootstrap_key_material(dir: &Path) -> Result { + let anchor = decode_public_key(BOOTSTRAP_PUBLIC_KEY_B64)?; + + let path = dir.join("pinned-public-key.bin"); + let mut pin_rejected: Option = None; + if let Ok(bytes) = std::fs::read(&path) { + match <[u8; 32]>::try_from(bytes.as_slice()) { + Ok(array) => match VerifyingKey::from_bytes(&array) { + Ok(key) if is_trusted(&key) => return Ok(key), + Ok(key) => { + pin_rejected = Some(format!( + "pinned key {} is not in this build's trust list", + fingerprint(&key) + )); + } + Err(_) => pin_rejected = Some("pinned key is not a valid Ed25519 point".to_owned()), + }, + Err(_) => pin_rejected = Some(format!( + "pinned key is {} bytes, expected 32", + bytes.len() + )), + } + } + + if let Some(reason) = pin_rejected { + eprintln!("moddin: {reason}; falling back to the compiled trust anchor"); + } + + // Repair the cache so the mismatch is not re-reported on every call. + let _ = pin_pinned_public_key(dir, &anchor); + Ok(anchor) } fn fingerprint(key: &VerifyingKey) -> String { @@ -534,6 +606,100 @@ mod tests { assert_eq!(bytes.len(), 32, "Ed25519 public keys are 32 bytes"); } + /// Every key the app ships in the trust list has to decode, or a + /// rotation that ships a typo silently stops trusting a key. + #[test] + fn every_trusted_key_in_the_keyring_decodes() { + for encoded in TRUSTED_PUBLIC_KEYS_B64 { + let key = decode_public_key(encoded) + .unwrap_or_else(|error| panic!("trust list entry {encoded} does not decode: {error}")); + assert!(is_trusted(&key), "{encoded} is in the list but not trusted"); + } + } + + #[test] + fn the_anchor_is_itself_trusted() { + let anchor = decode_public_key(BOOTSTRAP_PUBLIC_KEY_B64).expect("anchor decodes"); + assert!( + is_trusted(&anchor), + "the compiled anchor must be present in the trust list" + ); + } + + /// The pin on disk used to be returned unverified. Anything able to + /// write to %LOCALAPPDATA% could drop in its own 32 bytes and sign + /// whatever it liked, and the app would call it verified. + #[test] + fn a_pin_that_is_not_in_the_trust_list_is_discarded_for_the_anchor() { + let dir = std::env::temp_dir().join(format!( + "moddin-community-pin-{}-{}", + std::process::id(), + now_unix() + )); + std::fs::create_dir_all(&dir).expect("temp community dir"); + + // A well-formed Ed25519 point that is in nobody's keyring. The + // decode is asserted rather than skipped: a test that quietly + // no-ops when its key is malformed is worse than no test. + let attacker = decode_public_key("3pTDeCH5FEXkAdT1GVcE6McbxoHqTzZ1CkjPGCPYE9I=") + .expect("the test key is a valid Ed25519 point"); + assert!(!is_trusted(&attacker), "test key must not be trusted"); + std::fs::write(dir.join("pinned-public-key.bin"), attacker.to_bytes()) + .expect("write attacker pin"); + + let resolved = ensure_bootstrap_key_material(&dir).expect("falls back to the anchor"); + let anchor = decode_public_key(BOOTSTRAP_PUBLIC_KEY_B64).expect("anchor decodes"); + assert_eq!( + resolved.to_bytes(), + anchor.to_bytes(), + "an untrusted pin must never be the key the catalog is verified with" + ); + assert!( + is_trusted(&resolved), + "whatever we resolve to is still a key we trust" + ); + + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn a_corrupt_pin_is_discarded_for_the_anchor() { + let dir = std::env::temp_dir().join(format!( + "moddin-community-corrupt-pin-{}-{}", + std::process::id(), + now_unix() + )); + std::fs::create_dir_all(&dir).expect("temp community dir"); + std::fs::write(dir.join("pinned-public-key.bin"), b"too short") + .expect("write corrupt pin"); + + let resolved = ensure_bootstrap_key_material(&dir).expect("falls back to the anchor"); + let anchor = decode_public_key(BOOTSTRAP_PUBLIC_KEY_B64).expect("anchor decodes"); + assert_eq!(resolved.to_bytes(), anchor.to_bytes()); + + let _ = std::fs::remove_dir_all(&dir); + } + + /// A pin that matches the anchor is kept, so the on-disk cache + /// still does its job and does not churn on every call. + #[test] + fn a_pin_matching_the_anchor_is_accepted() { + let dir = std::env::temp_dir().join(format!( + "moddin-community-good-pin-{}-{}", + std::process::id(), + now_unix() + )); + std::fs::create_dir_all(&dir).expect("temp community dir"); + let anchor = decode_public_key(BOOTSTRAP_PUBLIC_KEY_B64).expect("anchor decodes"); + std::fs::write(dir.join("pinned-public-key.bin"), anchor.to_bytes()) + .expect("write anchor pin"); + + let resolved = ensure_bootstrap_key_material(&dir).expect("anchor pin accepted"); + assert_eq!(resolved.to_bytes(), anchor.to_bytes()); + + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn signature_rejects_invalid_base64() { let key = ensure_bootstrap_key_material(&std::env::temp_dir()) From e3f3b56dea085e1d758857d8b10bc3a657e64bf9 Mon Sep 17 00:00:00 2001 From: Marco Date: Tue, 29 Sep 2026 20:27:05 -0300 Subject: [PATCH 04/51] fix(frontend): stop shipping promises the backend does not keep P0-5: both AI entry points passed a catalog id while resolveInstallTarget matched it against a store app id, so every install ended at "no game selected". Added findInstalledGameForCatalogGame, the inverse of the existing findCatalogGameByInstalledGame, and renamed the parameter to catalogGameId so the contract is explicit. P0-6: capability cards rendered capability.id because the description field did not exist, on either Rust struct, in either TypeScript interface, or in any built-in YAML. The card now shows a human description and falls back to the id only when a recipe declares none. UX-12: the AI install path hard-coded acceptUnsigned: true, so the consent the Community panel asks for was skipped. It now splits the batch by the verified catalog signed flag and fails closed on the unsigned ones, naming them. Consent is persisted per capability id. UX-11: capability removal, the Windows-wide OpenXR switch, Local AI disconnect and history undo were unconfirmed destructive actions. All four now go through a new ConfirmDialog built on BaseDialog, so the focus trap, Escape, focus-restore and aria-modal are inherited rather than re-implemented. Each names what is about to change and what it costs to undo; the history one counts the files, which the Desfazer button never did. Adds .btn-danger-solid, which the design system did not have. F-06: the TypeScript step-kind union declared 5 of the 11 kinds the runner implements, and the AI/MCP authoring path types recipes against it. Now 11, with a comment naming the parity guard. None of this is covered by a test: the project still has no frontend test runner, which is F-12. --- src/components/ui/ConfirmDialog.vue | 71 ++++++++++++++ src/composables/useAiAssistant.ts | 96 ++++++++++++++++++- .../CapabilityModulesSection.vue | 29 +++++- src/features/capability-modules/service.ts | 18 ++-- src/features/community/CommunityPanel.vue | 5 + src/features/history/HistoryView.vue | 34 ++++++- src/features/local-ai/LocalAiPanel.vue | 23 +++++ src/features/local-ai/copy.ts | 12 +++ src/features/openxr/OpenXrManager.vue | 33 ++++++- src/features/openxr/copy.ts | 15 +++ src/i18n/locales/en.ts | 8 ++ src/i18n/locales/es.ts | 8 ++ src/i18n/locales/pt-BR.ts | 8 ++ src/services/catalog.ts | 21 ++++ src/styles/base.css | 12 +++ src/types/capability.ts | 25 +++++ 16 files changed, 406 insertions(+), 12 deletions(-) create mode 100644 src/components/ui/ConfirmDialog.vue diff --git a/src/components/ui/ConfirmDialog.vue b/src/components/ui/ConfirmDialog.vue new file mode 100644 index 0000000..a7fe043 --- /dev/null +++ b/src/components/ui/ConfirmDialog.vue @@ -0,0 +1,71 @@ + + + diff --git a/src/composables/useAiAssistant.ts b/src/composables/useAiAssistant.ts index 0c51a2f..97ca7e8 100644 --- a/src/composables/useAiAssistant.ts +++ b/src/composables/useAiAssistant.ts @@ -38,6 +38,72 @@ import { readLocalValue, writeLocalValue } from '../services/storage' */ const NO_GAME_SELECTED = 'No supported game is selected. Pick the game in your library, then install the recommendations again.' + +/** + * Consent key for an unsigned capability. + * + * The Community panel keeps its consent in component state, so there was + * nothing for this flow to read. It is persisted per capability id, and + * set from the same checkbox the Community panel shows — see + * `recordUnsignedConsent`. + */ +const UNSIGNED_CONSENT_KEY = 'moddin-unsigned-consent' + +/** Ids the user has explicitly agreed to install unsigned. */ +function readUnsignedConsent(): Record { + const raw = readLocalValue(UNSIGNED_CONSENT_KEY) + if (!raw) return {} + try { + const parsed = JSON.parse(raw) as unknown + if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) { + return Object.fromEntries( + Object.entries(parsed as Record) + .filter(([, value]) => value === true) + .map(([key]) => [key, true as const]), + ) + } + } catch { + // A corrupt value is treated as "no consent recorded". + } + return {} +} + +/** Record that the user ticked the unsigned warning for `capabilityId`. */ +export function recordUnsignedConsent(capabilityId: string) { + const current = readUnsignedConsent() + current[capabilityId] = true + writeLocalValue(UNSIGNED_CONSENT_KEY, JSON.stringify(current)) +} + +/** + * Capabilities from a verified catalog whose signature the app could not + * confirm. Signed entries install without any prompt. + * + * A capability that is not in the catalog at all is treated as + * unverified: the safest reading of "I have never heard of this" is + * "ask the user", not "install it". + */ +async function splitBySignature( + ids: string[], +): Promise<{ verified: string[]; needsConsent: string[] }> { + let entries: Array<{ id: string; signed?: boolean }> = [] + try { + const { fetchCommunityCatalog } = await import('../features/community/service') + const result = await fetchCommunityCatalog(false, 0) + entries = result?.catalog?.capabilities ?? [] + } catch { + // No verified catalog available. Everything is unverified. + return { verified: [], needsConsent: [...ids] } + } + const byId = new Map(entries.map((entry) => [entry.id, entry])) + const verified: string[] = [] + const needsConsent: string[] = [] + for (const id of ids) { + if (byId.get(id)?.signed === true) verified.push(id) + else needsConsent.push(id) + } + return { verified, needsConsent } +} import type { CatalogCapability, CatalogCollection } from '../types/ai-assistant' import type { CapabilitySummary } from '../types/capability' @@ -463,6 +529,32 @@ async function installSelectedRecommendations() { return } + // Every recommendation is a community capability, and an unsigned one + // used to install with `acceptUnsigned: true` hard-coded -- so the + // consent the Community panel asks for was simply skipped here. Fail + // closed instead: install only what the catalog has signed, and tell + // the user exactly which ones need their OK. + const consent = readUnsignedConsent() + const { needsConsent } = await splitBySignature(selected.map((rec) => rec.id)) + const blocked = needsConsent.filter((id) => consent[id] !== true) + + if (blocked.length > 0) { + for (const rec of selected) { + const key = `${rec.type}:${rec.id}` + recommendationInstallStatus.value = { + ...recommendationInstallStatus.value, + [key]: blocked.includes(rec.id) ? 'failed' : 'pending', + } + } + error.value = + blocked.length === 1 + ? `"${blocked[0]}" is not signed by the community maintainers. Open Community mods to review it and tick the unsigned warning, then install again.` + : `${blocked.length} of these mods are not signed by the community maintainers: ${blocked.join(', ')}. Open Community mods to review them and tick the unsigned warning, then install again.` + busy.value = false + step.value = 'saved' + return + } + for (const rec of selected) { const key = `${rec.type}:${rec.id}` recommendationInstallStatus.value = { ...recommendationInstallStatus.value, [key]: 'running' } @@ -474,7 +566,9 @@ async function installSelectedRecommendations() { installDir: target.installDir, executableDir: target.executableDir, config: { values: {} }, - acceptUnsigned: true, + // Only ever true for a capability whose unsigned warning the + // user has already acknowledged. See `blocked` above. + acceptUnsigned: consent[rec.id] === true, }) recommendationInstallStatus.value = { ...recommendationInstallStatus.value, diff --git a/src/features/capability-modules/CapabilityModulesSection.vue b/src/features/capability-modules/CapabilityModulesSection.vue index 58231d3..0d7df04 100644 --- a/src/features/capability-modules/CapabilityModulesSection.vue +++ b/src/features/capability-modules/CapabilityModulesSection.vue @@ -1,7 +1,8 @@ diff --git a/src/features/local-ai/LocalAiPanel.vue b/src/features/local-ai/LocalAiPanel.vue index cf02f52..2e81ae6 100644 --- a/src/features/local-ai/LocalAiPanel.vue +++ b/src/features/local-ai/LocalAiPanel.vue @@ -2,6 +2,7 @@ import { computed, ref } from 'vue' import { useI18n } from 'vue-i18n' import AppIcon from '../../components/ui/AppIcon.vue' +import ConfirmDialog from '../../components/ui/ConfirmDialog.vue' import { useDialogLifecycle } from '../../composables/useDialogLifecycle' import { resolveResourceDir, detectAgents, setupAgent, removeAgent } from './service' import type { AiAgent } from './types' @@ -14,6 +15,7 @@ const open = ref(false) const loading = ref(false) const error = ref(null) const busyAgent = ref(null) +const pendingDisconnect = ref(null) const resourceDir = ref('') const agents = ref([]) @@ -84,6 +86,16 @@ async function connect(agent: AiAgent) { } async function disconnect(agent: AiAgent) { + // Disconnecting rewrites the user's editor config to stop pointing at + // the Moddin server. That is reversible, but it edits a file the user + // also edits by hand, so it is worth one question. + pendingDisconnect.value = agent +} + +async function confirmDisconnect() { + const agent = pendingDisconnect.value + if (!agent) return + pendingDisconnect.value = null busyAgent.value = agent.id error.value = null try { @@ -215,6 +227,17 @@ async function disconnect(agent: AiAgent) { + + diff --git a/src/features/local-ai/copy.ts b/src/features/local-ai/copy.ts index 5c73ea3..0e1ae64 100644 --- a/src/features/local-ai/copy.ts +++ b/src/features/local-ai/copy.ts @@ -21,6 +21,10 @@ const ptBR = { banner: 'O Moddin nunca grava nada dentro das pastas dos jogos. A IA só escreve arquivos YAML em uma pasta local sua, e você aplica pelo Moddin com preview e rollback.', connectFailed: 'Não foi possível conectar: {error}', disconnectFailed: 'Não foi possível desconectar: {error}', + disconnectConfirmTitle: 'Desconectar este agente?', + disconnectConfirmDescription: '{name} vai parar de responder ao Moddin.', + disconnectConfirmDetail: 'O Moddin reescreve o arquivo de configuração do seu editor para apontar para o servidor do Moddin — desfazer devolve o conteúdo original.', + disconnectCancel: 'Cancelar', installCursorHint: 'Você não tem o Cursor instalado. Baixe em cursor.com e clique em "Atualizar status" depois.', } as const @@ -45,6 +49,10 @@ const enUS = { banner: 'Moddin never writes inside game folders. The AI only writes YAML files to a local folder of yours, and you apply from Moddin with preview and rollback.', connectFailed: 'Could not connect: {error}', disconnectFailed: 'Could not disconnect: {error}', + disconnectConfirmTitle: 'Disconnect this agent?', + disconnectConfirmDescription: '{name} will stop responding to Moddin.', + disconnectConfirmDetail: 'Moddin rewrites your editor config to point at the Moddin server — undoing restores the original content.', + disconnectCancel: 'Cancel', installCursorHint: 'You do not have Cursor installed. Download it from cursor.com then click "Refresh status".', } as const @@ -69,6 +77,10 @@ const esES = { banner: 'Moddin nunca escribe dentro de las carpetas de los juegos. La IA solo escribe YAML en una carpeta local tuya, y tú aplicas desde Moddin con vista previa y rollback.', connectFailed: 'No se pudo conectar: {error}', disconnectFailed: 'No se pudo desconectar: {error}', + disconnectConfirmTitle: '¿Desconectar este agente?', + disconnectConfirmDescription: '{name} dejará de responder a Moddin.', + disconnectConfirmDetail: 'Moddin reescribe la configuración de tu editor para apuntar al servidor de Moddin — deshacer devuelve el contenido original.', + disconnectCancel: 'Cancelar', installCursorHint: 'No tienes Cursor instalado. Descárgalo de cursor.com y haz clic en "Actualizar estado" después.', } as const diff --git a/src/features/openxr/OpenXrManager.vue b/src/features/openxr/OpenXrManager.vue index e78b14d..88e104f 100644 --- a/src/features/openxr/OpenXrManager.vue +++ b/src/features/openxr/OpenXrManager.vue @@ -1,7 +1,8 @@ diff --git a/src/components/shell/AiGameSuggestions.vue b/src/components/shell/AiGameSuggestions.vue index e9b99a7..60d7d9d 100644 --- a/src/components/shell/AiGameSuggestions.vue +++ b/src/components/shell/AiGameSuggestions.vue @@ -3,7 +3,6 @@ import { computed, ref } from 'vue' import { useI18n } from 'vue-i18n' import AppIcon from '../ui/AppIcon.vue' import { useAiAssistantTrigger } from '../../composables/useAiAssistant' -import { useAiModuleActions } from '../../composables/useAiModuleActions' const props = defineProps<{ gameId: string | null @@ -12,13 +11,15 @@ const props = defineProps<{ const { t } = useI18n() const { openAiAssistantRecommendations } = useAiAssistantTrigger() -const { openAiAudit } = useAiModuleActions() const dismissed = ref(false) const gameLabel = computed(() => props.gameName ?? props.gameId ?? t('aiAssistantGameBannerAnyGame')) const isVisible = computed(() => Boolean(props.gameId) && !dismissed.value) +// One verb here too. The banner used to offer "audit" and "recommend" +// side by side, which asked the user to choose a mode before they had +// even said what they wanted; the dialog's picker does that better. async function ask() { await openAiAssistantRecommendations({ gameId: props.gameId, @@ -27,14 +28,6 @@ async function ask() { }) } -async function audit() { - await openAiAudit({ - gameId: props.gameId, - gameName: props.gameName, - intent: t('aiAssistantAuditIntent', { game: gameLabel.value }), - }) -} - function dismiss() { dismissed.value = true } @@ -50,9 +43,6 @@ function dismiss() {

{{ t('aiAssistantGameBannerBody', { game: gameLabel }) }}

- @@ -73,80 +63,63 @@ function dismiss() { display: grid; grid-template-columns: auto 1fr auto; align-items: center; - gap: 0.85rem; - background: var(--moddin-surface-2, rgba(255, 255, 255, 0.025)); - border: 1px solid var(--moddin-line-soft, rgba(255, 255, 255, 0.08)); - border-left: 3px solid var(--moddin-accent, #8b72ff); - border-radius: 6px; - padding: 0.55rem 0.85rem; - margin-bottom: 0.85rem; - font-size: 0.85rem; + gap: var(--moddin-space-4); + background: var(--moddin-surface-2); + border: 1px solid var(--moddin-line-soft); + border-left: 3px solid var(--moddin-accent); + border-radius: var(--moddin-radius-sm); + padding: var(--moddin-space-2) var(--moddin-space-4); + margin-bottom: var(--moddin-space-4); + font-size: var(--moddin-text-md); } .ai-game-banner-icon { - font-size: 1.05rem; - color: var(--moddin-accent, #8b72ff); + color: var(--moddin-accent); } .ai-game-banner-body strong { display: block; - font-size: 0.85rem; + font-size: var(--moddin-text-md); font-weight: 600; - color: var(--moddin-text-soft, #c5cad3); + color: var(--moddin-text-soft); } .ai-game-banner-body p { - margin: 0.1rem 0 0; - color: var(--moddin-text-muted, #9aa3b2); - font-size: 0.78rem; + margin: 2px 0 0; + color: var(--moddin-text-muted); + font-size: var(--moddin-text-sm); line-height: 1.4; } .ai-game-banner-actions { display: flex; align-items: center; - gap: 0.35rem; + gap: var(--moddin-space-1); } .ai-game-banner-primary { background: transparent; - border: 1px solid var(--moddin-accent, #8b72ff); - color: var(--moddin-accent, #8b72ff); - border-radius: 4px; - padding: 0.3rem 0.7rem; - font-size: 0.78rem; + border: 1px solid var(--moddin-accent); + color: var(--moddin-accent); + border-radius: var(--moddin-radius-sm); + padding: var(--moddin-space-1) var(--moddin-space-3); + font-size: var(--moddin-text-sm); font-weight: 500; cursor: pointer; } .ai-game-banner-primary:hover { - background: var(--moddin-accent-soft, rgba(139, 114, 255, 0.12)); -} - -.ai-game-banner-secondary { - background: transparent; - border: 1px solid var(--moddin-line-soft, rgba(255, 255, 255, 0.12)); - color: var(--moddin-text-muted, #9aa3b2); - border-radius: 4px; - padding: 0.3rem 0.7rem; - font-size: 0.78rem; - cursor: pointer; -} - -.ai-game-banner-secondary:hover { - border-color: var(--moddin-text-soft, #c5cad3); - color: var(--moddin-text-soft, #c5cad3); + background: var(--moddin-accent-soft); } .ai-game-banner-dismiss { background: transparent; border: 1px solid transparent; - color: var(--moddin-text-muted, #9aa3b2); + color: var(--moddin-text-muted); width: 24px; height: 24px; - border-radius: 4px; + border-radius: var(--moddin-radius-sm); cursor: pointer; - font-size: 0.9rem; display: inline-flex; align-items: center; justify-content: center; @@ -154,6 +127,6 @@ function dismiss() { .ai-game-banner-dismiss:hover { color: inherit; - background: var(--moddin-surface-3, rgba(255, 255, 255, 0.05)); + background: var(--moddin-surface-3); } diff --git a/src/components/shell/AiTopbarMenu.vue b/src/components/shell/AiTopbarMenu.vue index b88792b..d60918c 100644 --- a/src/components/shell/AiTopbarMenu.vue +++ b/src/components/shell/AiTopbarMenu.vue @@ -1,276 +1,55 @@ diff --git a/src/components/shell/GlobalTools.vue b/src/components/shell/GlobalTools.vue index de0703a..0474b01 100644 --- a/src/components/shell/GlobalTools.vue +++ b/src/components/shell/GlobalTools.vue @@ -1,20 +1,33 @@ diff --git a/src/components/ui/AppIcon.vue b/src/components/ui/AppIcon.vue index e5e48fd..b6d9917 100644 --- a/src/components/ui/AppIcon.vue +++ b/src/components/ui/AppIcon.vue @@ -7,7 +7,7 @@ export type IconName = | 'library' | 'history' | 'vr' | 'community' | 'activity' | 'refresh' | 'search' | 'check' | 'alert' | 'close' | 'external' | 'play' | 'undo' | 'info' | 'gamepad' | 'shield' | 'arrow-up' | 'folder' | 'globe' - | 'ai' + | 'ai' | 'terminal' withDefaults(defineProps<{ name: IconName; size?: number }>(), { size: 16 }) @@ -32,6 +32,7 @@ const paths: Record = { folder: '', globe: '', ai: '', + terminal: '', } diff --git a/src/components/ui/BaseDialog.vue b/src/components/ui/BaseDialog.vue index 673cd07..2f21964 100644 --- a/src/components/ui/BaseDialog.vue +++ b/src/components/ui/BaseDialog.vue @@ -1,7 +1,8 @@ diff --git a/src/features/activity/copy.ts b/src/features/activity/copy.ts index eec24b5..93f3b40 100644 --- a/src/features/activity/copy.ts +++ b/src/features/activity/copy.ts @@ -27,7 +27,6 @@ const ptBR = { errorDiskWhy: 'Libere espaço em %LOCALAPPDATA%\\Moddin\\logs ou em outra unidade e clique em Atualizar. Sem espaço, o Moddin continua funcionando, mas não consegue registrar novas atividades.', errorGenericTitle: 'Não consegui ler ou gravar o registro de atividades.', errorGenericWhy: 'Tente atualizar daqui a pouco. Se persistir, abra uma issue em github.com/petonexus/moddin-desktop com o detalhe técnico abaixo.', - errorRawToggle: 'Ver detalhe técnico', } as const const activityMessages = defineLocalizedCopy( @@ -59,7 +58,6 @@ const activityMessages = defineLocalizedCopy( errorDiskWhy: 'Free space under %LOCALAPPDATA%\\Moddin\\logs (or another drive) and click Refresh. Without space, Moddin keeps working but cannot record new activities.', errorGenericTitle: 'Could not read or write the activity log.', errorGenericWhy: 'Try refreshing in a few seconds. If it persists, open an issue at github.com/petonexus/moddin-desktop with the technical detail below.', - errorRawToggle: 'Show technical detail', }, { button: 'Actividad', @@ -88,7 +86,6 @@ const activityMessages = defineLocalizedCopy( errorDiskWhy: 'Libera espacio en %LOCALAPPDATA%\\Moddin\\logs o en otra unidad y haz clic en Actualizar. Sin espacio, Moddin sigue funcionando, pero no puede registrar nuevas actividades.', errorGenericTitle: 'No pude leer ni escribir el registro de actividad.', errorGenericWhy: 'Intenta actualizar en unos segundos. Si persiste, abre un issue en github.com/petonexus/moddin-desktop con el detalle técnico de abajo.', - errorRawToggle: 'Ver detalle técnico', }, ) diff --git a/src/features/activity/useActivityLogPanel.ts b/src/features/activity/useActivityLogPanel.ts index 80ef094..9ccf0dc 100644 --- a/src/features/activity/useActivityLogPanel.ts +++ b/src/features/activity/useActivityLogPanel.ts @@ -1,5 +1,4 @@ import { computed, ref } from 'vue' -import { useDialogLifecycle } from '../../composables/useDialogLifecycle' import { clearActionLogs, listActionLogs } from './service' import type { ActionLogEntry, ActionLogLevel } from './types' @@ -12,7 +11,16 @@ export function useActivityLogPanel() { const search = ref('') const level = ref<'all' | ActionLogLevel>('all') const expanded = ref(new Set()) - const { dialogElement, openDialog, closeDialog } = useDialogLifecycle(open) + + // Focus, Escape and focus restore are BaseDialog's job; this only has to + // say whether the dialog is on screen. + function openDialog() { + open.value = true + } + + function closeDialog() { + open.value = false + } const filteredLogs = computed(() => { const term = search.value.trim().toLowerCase() @@ -66,7 +74,6 @@ export function useActivityLogPanel() { return { open, - dialogElement, loading, clearing, error, diff --git a/src/features/capability-modules/CapabilityModulesSection.vue b/src/features/capability-modules/CapabilityModulesSection.vue index 0d7df04..0a98254 100644 --- a/src/features/capability-modules/CapabilityModulesSection.vue +++ b/src/features/capability-modules/CapabilityModulesSection.vue @@ -3,9 +3,11 @@ import { computed, onMounted, onUnmounted, ref } from 'vue' import { useI18n } from 'vue-i18n' import AppIcon from '../../components/ui/AppIcon.vue' import ConfirmDialog from '../../components/ui/ConfirmDialog.vue' -import ModuleCard from '../library/ModuleCard.vue' +import EmptyState from '../../components/ui/EmptyState.vue' +import ModuleCard, { type ModuleCardState, type ModuleCardVerification } from '../library/ModuleCard.vue' import type { TransactionRecord } from '../../types/transaction' import type { CapabilityOrigin, CapabilitySummary } from '../../types/capability' +import type { ModuleVerificationCheck } from '../../types/module-verification' import { useCapabilityModules } from './useCapabilityModules' import type { CapabilityCardState, CapabilityConfigValue } from './types' @@ -38,6 +40,7 @@ const modules = useCapabilityModules({ const { visibleCapabilities, + loading, loadError, ensureLoaded, stateFor, @@ -65,6 +68,35 @@ function blockedCompatibility(state: CapabilityCardState) { return state.compatibility && !state.compatibility.passed ? state.compatibility : null } +/** + * The capability runner reports `detail: null` for checks that carry no + * detail; the card's check shape has no null. Everything else (including + * the backend's own summary sentence) is left to the card to render. + */ +function cardChecks(state: CapabilityCardState): ModuleVerificationCheck[] { + return (state.verification?.checks ?? []).map((item) => ({ + label: item.label, + passed: item.passed, + detail: item.detail ?? undefined, + })) +} + +function cardVerification(state: CapabilityCardState): ModuleCardVerification | null { + return state.verification ? { checks: cardChecks(state) } : null +} + +/** + * Same derivation the catalog cards use in `App.vue`: a check that failed + * must not read as a healthy mod. The runner never reports `installed`, so + * the transaction history owns the installed half of the state. + */ +function cardState(capabilityId: string): ModuleCardState { + const state = stateFor(capabilityId) + if (state.verifyBusy) return 'checking' + if (state.verification && cardChecks(state).some((check) => !check.passed)) return 'attention' + return isInstalled(capabilityId) ? 'active' : 'available' +} + /** Ids auto-installed by the backend as dependencies of the last install. */ function installedDependencyNames(capabilityId: string): string { return stateFor(capabilityId).installedDependencies.join(', ') @@ -174,6 +206,12 @@ onUnmounted(() => {
+ +
{ {
- -
-
-
{{ t('checklistTitle') }}
- {{ stateFor(capability.id).verification?.summary }} -
-
    -
  • - - - {{ check.label }} - {{ check.detail }} - -
    • -
-

- {{ stateFor(capability.id).verification?.summary }} -

-
@@ -361,15 +377,6 @@ onUnmounted(() => { .capability-required { margin-left: 2px; color: var(--moddin-danger); font-style: normal; } .capability-checkbox { width: 15px; height: 15px; margin-top: 3px; accent-color: var(--moddin-accent); } -.capability-checks { display: grid; gap: 6px; margin: 0; padding: 0; list-style: none; } -.capability-checks li { display: flex; align-items: flex-start; gap: var(--moddin-space-2); color: var(--moddin-text-soft); font-size: var(--moddin-text-sm); } -.capability-checks li .app-icon { margin-top: 2px; } -.capability-checks li.ok .app-icon { color: var(--moddin-success); } -.capability-checks li.fail .app-icon { color: var(--moddin-warning); } -.capability-checks small { display: block; color: var(--moddin-text-faint); font-size: var(--moddin-text-xs); } - -.capability-hint { color: var(--moddin-text-muted); font-size: var(--moddin-text-sm); } - .capability-compat { display: grid; justify-items: start; gap: var(--moddin-space-2); } .capability-compat small { color: var(--moddin-text-soft); font-size: var(--moddin-text-xs); } diff --git a/src/features/history/HistoryView.vue b/src/features/history/HistoryView.vue index 38eaa87..4c105f0 100644 --- a/src/features/history/HistoryView.vue +++ b/src/features/history/HistoryView.vue @@ -70,15 +70,17 @@ function confirmUndo() {
-
-
-
- - {{ t('historyEmptyTitle') }} - {{ t('historyEmptyHint') }} -
+ +
-
-
-
- {{ t('noGamesFound') }} - {{ t('noGamesHint') }} -
+ +