# MPV.Rocks Installer deep codebase review

**Review date:** 2026-08-31  
**Reviewed commit:** `bdfb31b95030c2d992e5cfc4cb0ba5bada368d1b` (`master`)  
**Companion report:** [Standalone HTML report](CODEBASE_REVIEW_2026-08-31.html)

## Executive summary

This review found **no P0/critical issue**, but it did identify **17 P1/high**, **25 P2/medium**, and **8 P3/low** findings. The most important risks are concentrated in four areas:

1. Windows uninstall and multi-install targeting can elevate mutable code, delete unrelated files, or operate on the wrong installation.
2. Configuration and background-job operations do not consistently make side effects, persistence, and terminal status one transaction.
3. Release signing authenticates bytes, but the signing job currently trusts tagged generator code and unauthenticated upstream “latest” assets too broadly.
4. Several advertised timeouts, recovery paths, and cross-process coordination mechanisms do not cover the full operation they claim to bound.

The codebase also has substantial strengths. All normal and race-enabled Go tests passed, all 157 frontend assertions passed, six release cross-builds succeeded, dependency vulnerability scans were clean, and the signed manifest/self-update path fails closed on missing trust, invalid signatures, hash/size mismatches, and staged identity mismatches. The Web auth cookie, Host/Origin validation, request limits, native archive defenses for 7z, keyring backends, and most shared dialog behavior were sound in the reviewed paths.

The first release-blocking remediation pass should address H-01 through H-04 and H-12 through H-17. The job/config consistency findings H-05 through H-10 should follow before relying on the Tasks UI as authoritative state.

## Scope and method

The review covered Go commands and packages, Web/TUI frontends, embedded assets, platform installers, self-update/release-manifest code, CI, build scripts, and maintained documentation. Three independent focused reviews examined:

- security, installers, release manifests, self-update, keyring, and platform behavior;
- Web jobs, configuration, concurrency, persistence, hotkeys, and UI-config packages;
- frontend behavior, accessibility, embedded dependencies, build/release CI, and documentation.

The primary review then re-read and validated the strongest findings, ran repository-wide checks, cross-built every published manager target, and exercised the local Web server’s routes and security middleware.

Severity means:

- **P0 / Critical:** directly exploitable or unrecoverably destructive without an additional intended trust/privilege decision.
- **P1 / High:** plausible privilege, supply-chain, destructive-data, wrong-target, indefinite-availability, or authoritative-state failure.
- **P2 / Medium:** material reliability, security-hardening, concurrency, portability, or accessibility defect.
- **P3 / Low:** limited-impact defect, misleading UX/docs, defense-in-depth gap, or maintainability issue.

## Validation results

| Check | Result |
|---|---|
| `go test -count=1 ./...` | Pass |
| `go test -race -count=1 ./...` | Pass |
| `go test -shuffle=on -count=3 ./...` | Pass |
| `go vet ./...` | Pass |
| `gofmt` diff, `go mod tidy -diff`, `go mod verify`, `git diff --check` | Pass |
| `govulncheck@v1.7.0 ./...` built with Go 1.27 | No vulnerabilities found |
| Staticcheck v0.8.1, correctness family `SA*` | 13 diagnostics; four production correctness findings are discussed below |
| `npm test` | Pass: 12 files, 157 tests |
| Shuffled frontend tests | Pass: 157 tests |
| `npm audit --audit-level=moderate` | 0 vulnerabilities |
| npm-managed Alpine/htmx vendor freshness | Pass, byte-for-byte |
| Linux amd64/arm64, Windows amd64/arm64, Darwin amd64/arm64 builds | Pass |
| Local Web route/API/auth smoke test | Pass; normal routes, cookie auth, hostile Host/Origin rejection, and graceful no-client shutdown behaved as expected |

Statement coverage from one repository-wide run was **43.7% overall**. Notable package values were installer 84.1%, platform 92.3%, job history 90.7%, version 72.9%, config 70.3%, Web 28.5%, TUI 26.0%, and `cmd/mpv-manager` 11.9%. These figures are a directional map, not a quality score: cross-package execution is not fully attributed by the per-package coverage mode.

The locally installed `govulncheck` was initially built with Go 1.26 and could not analyze this Go 1.27 module. The review rebuilt the pinned tool with the active Go 1.27 toolchain before recording the clean result.

## P1 / High findings

### H-01 — Windows elevates a mutable install-tree batch file

**Evidence:** `pkg/installer/windows.go:160-212`, `pkg/installer/common_handler.go:165-184`, `pkg/tui/models_messages.go:419-423`.

The Windows uninstall flow copies `installer/mpv-unregister.bat` from the selected MPV tree and invokes it through PowerShell `Start-Process -Verb RunAs`. It performs no digest, signature, ownership, ACL, or reparse-point check immediately before elevation. Detected and adopted installations may be user-controlled, and the TUI exposes tracked records without a managed-install gate.

An unprivileged process can replace the batch file and wait for the user to approve the expected MPV Manager UAC prompt. That converts a trusted-looking uninstall operation into administrator execution.

**Remediation:** replace the batch file with fixed native registry cleanup or an embedded authenticated helper staged in an ACL-protected location. Verify the exact helper immediately before elevation and prohibit manager uninstall for unmanaged records.

### H-02 — Windows uninstall deletes an entire directory without ownership proof

**Evidence:** `pkg/installer/windows.go:121-142`, `pkg/installer/windows_detection.go:30-53`, `pkg/web/api_adopt.go:447-476`.

`UninstallWithOutput` enumerates the selected install directory and recursively removes every direct child except a small preserve list. Detection accepts AppData, home, Scoop, Chocolatey, custom, and PATH locations; adoption can mark an external record as managed.

If `mpv.exe` lives in a shared directory such as `C:\Tools`, uninstalling MPV can remove unrelated sibling files and directories. Package-managed installations are also treated like manager-owned directory layouts.

**Remediation:** require a manager-created ownership marker and a per-install manifest. Delete only paths recorded as manager-owned. For adopted/package-managed installs, use the upstream uninstaller or remove only manager-owned configuration.

### H-03 — TUI update/uninstall drops the selected app identity and can target another install

**Evidence:** `pkg/version/updates.go:24-33`, `pkg/tui/models_types.go:141-148`, `pkg/tui/models_update.go:438-446`, `pkg/tui/models_messages.go:40-64`, `pkg/installer/common_handler.go:165-184`.

The update domain object includes `AppID` and `InstallPath`, but the TUI’s `updateItem` drops both. Update dispatch uses the model’s global installer. Uninstall retains the selected record, but `ExecuteUninstall` invokes the platform installer already bound to the global path rather than `app.InstallPath`.

With multiple, custom, or adopted Windows installations, selecting installation B can update or delete installation A. Combined with H-02, this is destructive.

**Remediation:** carry stable app ID and canonical install path through every TUI item/message. Construct an immutable per-operation installer for the exact record, as the Web flow already does, and gate unmanaged records.

### H-04 — Windows custom installs and settings APIs resolve different `portable_config` trees

**Evidence:** `pkg/web/api_settings.go:479-518`, `pkg/installer/windows.go:410-423`, `pkg/constants/paths.go:47-79,306-319`, `pkg/config/editor.go:15-22`, `pkg/hotkeys/inputconf.go:223-232`, `internal/scriptopts/scriptopts.go:24-32`, `pkg/uiconfig/uiconfig.go:51-72`.

A custom location is persisted and the installer writes `<custom>\portable_config`. Later Config, Hotkeys, ModernZ/uOSC, migration, and backup operations resolve `%APPDATA%\mpv` or the legacy home path; the common resolver never considers the configured custom path.

The UI can therefore report successful changes to a configuration tree the installed MPV never reads.

**Remediation:** define one installed-app-aware MPV configuration resolver. Route config, hotkeys, script options, UI discovery/migration, backup, and restore through the exact selected installation identity.

### H-05 — Job cancellation can archive “cancelled” before irreversible commits finish

**Evidence:** `pkg/web/jobs.go:482-543`, `pkg/web/api_install.go:362-393,459-540`, `pkg/web/api_adopt.go:214-248,447-476`.

`CancelJob` cancels, marks terminal, archives, persists, and removes a job immediately. Workers check cancellation and then perform install/uninstall/config commits without a shared compare-and-set terminal boundary. If cancellation wins after the last check, history/SSE say “cancelled” while physical or configuration side effects still commit. Later completion/error updates silently miss the removed job.

**Remediation:** cancellation should request cancellation but leave the job active. The worker should own the sole terminal transition at a defined commit boundary and represent “cancellation requested after irreversible completion” explicitly.

### H-06 — Job conflict detection protects method IDs, not shared resources

**Evidence:** `pkg/web/jobs.go:256-279`, `pkg/installer/common_handler.go:68-82`, `pkg/installer/linux.go:296-303`, `pkg/installer/linux_package.go:258-272`, `pkg/web/api_config.go:563-615`.

`CreateJobIfNoneActive` only compares `MethodID`. Flatpak MPV, package MPV, ModernZ/uOSC component jobs, reset, restore, migration, and direct configuration APIs can all mutate the same MPV configuration/UI tree under different method IDs or outside the job manager.

Concurrent operations can overwrite each other, and one rollback can undo another successful job.

**Remediation:** introduce resource-keyed operation leases for config/UI trees, install targets, package managers, and app identities. Acquire multiple keys in deterministic order and include reset/restore/migration/direct writes in the coordinator.

### H-07 — Physical success is reported even when authoritative state persistence fails

**Evidence:** `pkg/web/api_install.go:382-394,507-540`, `pkg/web/api_adopt.go:228-249,460-477`, `pkg/tui/models_update.go:524-618,960-990,1024-1038`.

Web install, uninstall, adoption, and UI-change workers warn on tracking failures and then complete the job. The TUI records success before mutating installed-app state and ignores or only logs persistence errors.

A disk-full or permissions failure can leave an installed app untracked, an uninstalled app still tracked, or UI/adoption metadata stale while Tasks/history reports success.

**Remediation:** make persistence part of the terminal result. If physical rollback is impossible, expose a structured partial-success state with reconciliation/retry instructions rather than unconditional success.

### H-08 — Failed manager-config writes leak into live in-memory state

**Evidence:** `pkg/config/config.go:325-345,459-515,672-825,921-930,985-993,1059-1092`.

The general `Update` path rolls back on save failure, but many setters mutate `globalConfig` before `saveLocked` and return an error without restoring the prior value. This affects versions/UI type, PATH metadata, app add/remove, shortcut state, app UI/managed state, and HWA preference.

A failed `AddInstalledApp`, for example, is immediately visible through getters and can be persisted later by an unrelated successful save.

**Remediation:** mutate a detached clone and publish only after atomic persistence succeeds, or route every setter through the rollback-capable update primitive. Add write-failure injection tests by mutation family.

### H-09 — Config read failures are treated as a missing file and replaced with defaults

**Evidence:** `pkg/config/config.go:85-149,258-263`.

`Load` only distinguishes the successful `ReadFile` path. Permission errors, transient I/O failures, and unavailable mounts all fall through to default configuration and a nil return. `Reload` inherits that behavior. A later write may replace a previously valid file/state with defaults.

The parse-error log also references the shadowed outer `err`, producing a misleading `<nil>` diagnostic before quarantine.

**Remediation:** default only on `os.IsNotExist`. Return every other read error while retaining the last known-good in-memory config. Keep the decode error in an unshadowed variable and fail if initial default persistence fails.

### H-10 — Config restore renames the live file twice and has no rollback

**Evidence:** `pkg/installer/installer.go:1165-1195`, `pkg/installer/common.go:161-194`, `pkg/constants/constants.go:88-92`, `pkg/web/api_config.go:586-615`.

`RestoreConfigWithOutput` renames the live config aside, then calls `RestoreBackup`, which attempts another backup/rename before copying. Copy failure after the first rename leaves `mpv.conf` absent. Both recovery names use a date-only suffix, so repeated restores on one day collide; on Windows, a rename failure is only warned about before copy continues.

**Remediation:** perform one transaction: validate and copy the requested backup to a same-directory staged file, fsync it, atomically swap it with the live file, and restore the original on every failure. Use unique timestamp/UUID backup names.

### H-11 — The package-version “10 second” timeout does not terminate the loop

**Evidence:** `pkg/web/server_version_cache.go:249-326,332-364`, `pkg/web/package_version.go`, `pkg/web/server.go:98-100`.

The timeout branch contains an unlabelled `break`, which exits only the `select`, not the surrounding collection loop. `time.After` sends once, so the next iteration can wait forever. Package probes use unbounded `exec.Command`, initialization is synchronous before the listener starts, and refresh holds the version-cache write lock across probing.

A stalled package manager can prevent the Web UI from starting or freeze refresh/cache readers indefinitely. Staticcheck independently reports SA4011 at the same line.

**Remediation:** give every probe a context deadline, use a shared cancellable context and timer, return or use a labelled break on timeout, publish only completed results, and never hold the cache lock while probing. Add a permanently blocked probe test.

### H-12 — IINA installation trusts a global hard-coded DMG mount path

**Evidence:** `pkg/installer/macos.go:199-216,438-475`.

The code discards `hdiutil attach` output, assumes `/Volumes/IINA`, copies `IINA.app` from there, and detaches that path. A volume-name collision or alternate mount name can cause it to copy a pre-existing unrelated bundle and detach the wrong volume. Validation checks only basic bundle structure.

**Remediation:** use `hdiutil attach -plist` and the returned device/mount point, preferably with `-mountRandom` under an owned temporary directory. Validate bundle ID, architecture, and code signature/Team ID before replacement; detach by the returned device.

### H-13 — Tagged generator code receives the long-lived release signing key

**Evidence:** `.gitlab-ci.yml:284-317,477-506`, `cmd/generate-info/main.go:517-527`.

CI builds `cmd/generate-info` from the tag, injects `MANIFEST_SIGNING_KEY`, and executes that artifact. A malicious or compromised tagged generator can export the private key and forge future manifests. Protected-tag rules constrain who can trigger the job but do not separate signer trust from the code being authorized.

**Remediation:** move signing into an isolated, network-restricted job using a non-exportable KMS/HSM key or a pinned, separately reviewed minimal signer. Tagged application code should produce an unsigned, rigorously validated manifest/digest but never receive key material.

### H-14 — Unauthenticated upstream “latest” bytes are promoted into first-party signed assets

**Evidence:** `cmd/generate-info/main.go:358-448,546-634,948-977,1174-1232,1260-1269`, `.gitlab-ci.yml:502-522`.

The generator discovers latest upstream releases, downloads them, computes hashes, and signs/publishes those hashes in the MPV.Rocks manifest. It does not verify upstream signatures/checksum files/native signatures or require a reviewed digest allowlist. The MPV.Rocks signature therefore proves what CI fetched, not that the upstream bytes were authentic or reviewed.

Compromise or release-process error in an upstream executable/script becomes a trusted MPV.Rocks payload.

**Remediation:** pin reviewed upstream versions and digests in the tagged repository, verify upstream and platform-native signatures, and require an explicit approval boundary before isolated signing.

### H-15 — Post-update TUI relaunch inherits the helper log instead of the terminal

**Evidence:** <code>pkg/version/transaction.go:349-358,466-485</code>, <code>cmd/mpv-manager/main.go:101-180,273-275</code>.

The initiating process launches the update helper with stdout/stderr redirected to the transaction's <code>helper.log</code>. The helper later launches the new TUI with its own stdout/stderr, which are those log descriptors, while stdin still points at the terminal. The updated TUI can therefore accept keys but render invisibly into the log. Health is acknowledged in main before platform detection, release fetch, model construction, terminal initialization, or first render, so this broken relaunch can still be committed as healthy after the short stabilization window.

**Remediation:** detach helper diagnostics from the relaunch terminal contract. Open the user's controlling terminal explicitly for the child or hand off preserved terminal descriptors, and acknowledge health only after Bubble Tea initializes and completes a visible first render. Add a native PTY qualification that verifies rendered output and input after update.

### H-16 — FFmpeg replacement failure deletes the only known-good backup

**Evidence:** <code>pkg/installer/installer.go:593-608</code>.

The updater renames <code>ffmpeg.exe</code> to <code>.bak</code>, immediately defers deletion of that backup, and then copies the replacement. If the copy fails, the function returns and the deferred removal deletes the backup, leaving no executable and no recovery copy.

**Remediation:** stage and verify the new binary before touching the live path, atomically swap, restore the backup on every failure, and delete the backup only after post-swap verification succeeds.

### H-17 — TUI language apply renames away the full live config before a single-field write

**Evidence:** <code>pkg/tui/language_preferences.go:823-859</code>, <code>pkg/config/editor.go:135-177</code>.

Language apply renames <code>mpv.conf</code> to a date-only backup and then calls the shared single-value editor. Because the live file is now absent, the editor loads the embedded default and writes a fresh config containing the language change. All other user settings disappear from the active file. The fixed backup can also collide, and a failed rename is ignored before editing continues.

**Remediation:** remove the TUI-specific rename path and use the shared atomic editor's snapshot/backup transaction. A language change must update existing live content and retain a unique recovery backup without making the source disappear.

## P2 / Medium findings

### M-01 — uOSC ZIP extraction writes directly over live config outside the rollback allowlist

**Evidence:** `pkg/installer/installer.go:681-717`, `pkg/installer/installer_archive.go:15-68`, `pkg/installer/common.go:458-500`.

External `Expand-Archive -Force`/`unzip -o` extracts into the live config directory. The UI transaction snapshots only known managed UI paths. An unexpected in-root entry such as `input.conf` can overwrite unrelated user data and is not restored after a later failure.

**Remediation:** use bounded native extraction into an owned temporary root, reject unsafe/special/colliding entries, allowlist the expected uOSC tree, and transactionally overlay only owned files.

### M-02 — Multi-target self-update locks only the primary executable

**Evidence:** `pkg/version/transaction.go:205-210,265-316,417-451,750-776`.

A configured secondary manager target is journaled and replaced, but only the primary target’s lock is acquired and validated. Concurrent updates started from primary and secondary can hold different locks while both mutate the same secondary target.

**Remediation:** resolve all targets first and acquire one canonical lock per target in deterministic order, or use a per-user global update lock. Persist and validate the complete lock set.

### M-03 — A crash before the first updater journal permanently blocks recovery

**Evidence:** `pkg/version/transaction.go:221-235,300-320,905-916`.

The transaction directory exists during download/staging before the first journal is written. Recovery returns on the first unreadable matching directory, so a kill or power loss in that window can block every later update until manual deletion.

**Remediation:** write and fsync an initial journal immediately after directory creation. Recovery should safely quarantine/remove identified pre-journal orphans and continue scanning other transactions.

### M-04 — Windows discovery executes every found `mpv.exe`

**Evidence:** `pkg/installer/windows_detection.go:30-53,69-105,133-163`.

Discovery scans user-writable/custom/PATH locations and runs each discovered executable with `--version`, before adoption. Opening MPV Manager therefore executes untrusted local code with the manager’s current privilege, including if the manager itself is elevated.

**Remediation:** inspect PE metadata without executing the binary. Require explicit adoption and provenance/signature validation before any live probe; never probe PATH discoveries while elevated.

### M-05 — Multi-field `mpv.conf` API writes are not transactional

**Evidence:** `pkg/web/api_config.go:113-472`, `pkg/web/api_languages.go:28-82`, `pkg/config/editor.go:135-177`.

Config Apply and language endpoints validate a request and then persist fields one at a time. A later I/O failure leaves earlier values committed; concurrent full-form requests can interleave. A batch `SetConfigValues` primitive already exists.

**Remediation:** build one update list per file and call the batch writer once. Define explicit cross-file semantics for manager-only HWA metadata.

### M-06 — Job-history serialization is per `Store` instance, not per path/process

**Evidence:** `pkg/jobhistory/jobhistory.go:64-75,100-157`, `pkg/tui/job_history.go:25-29,91-97`, `pkg/web/jobstore.go:12-31`.

Each store has its own mutex and uses a fixed `.tmp` file for read/modify/write. TUI creates fresh stores while Web owns another. Two stores or processes can lose records or collide on the temp file.

**Remediation:** use a process-wide canonical-path lock, unique same-directory temp files, and an advisory cross-process lock. Retain one TUI store instance.

### M-07 — Normal browser SSE connections can turn graceful shutdown into exit 1

**Evidence:** `internal/webassets/static/jobs.js:61-72`, `pkg/web/sse.go:129-132`, `pkg/web/server.go:239-254`, `cmd/mpv-manager/main.go:380-385`.

Every page opens the global stream. The SSE handler exits only when the request context ends, while `Server.Shutdown` waits two seconds and signal handling treats any timeout as fatal. The primary live smoke test exited 0 without a connected client; the normal connected-browser case remains incompatible with that graceful path.

**Remediation:** create a server base context, cancel it before shutdown, unregister/close SSE clients, then force-close only after the graceful deadline. Add a connected-browser shutdown test.

### M-08 — Terminal SSE delivery and completed-job reconnect snapshots are not guaranteed

**Evidence:** `pkg/web/jobs.go:166-200,595-608`, `pkg/web/sse.go:35-50`.

The nonblocking send path can still abandon a terminal event after a concurrent buffer drain. A filtered reconnect snapshots only active jobs, while terminal jobs have already been removed.

**Remediation:** serialize each client’s dispatch through a bounded queue/ring with terminal priority and snapshot active plus recent persisted jobs on connection.

### M-09 — TUI Ctrl+C neither cancels nor joins active backend work

**Evidence:** `pkg/tui/models_update.go:25-31`, `pkg/tui/models_messages.go:20-64`.

Ctrl+C immediately quits Bubble Tea. Operations use `context.Background`, so spawned downloads/package managers are not cancelled and producers can outlive the consumer during shutdown.

**Remediation:** give the model an operation context/cancel function, stop output acceptance, cancel child commands, and join workers before quitting.

### M-10 — UI migration resolution spans two stores without a transaction

**Evidence:** `pkg/uiconfig/uiconfig.go:137-169`, `pkg/web/api_uiconfig.go:126-159`.

Concurrent Apply/Keep requests can both observe “pending”; the ModernZ file can be changed while the final manager-config resolution says Keep. A later manager-config write failure also leaves a changed file with a pending migration.

**Remediation:** serialize resolution using compare-and-set semantics and add rollback/recovery across both stores.

### M-11 — Job-output modal becomes visible after focus was attempted

**Evidence:** `internal/webassets/templates/base.html:32-38`, `internal/webassets/static/jobs-modal.js:69-72`, `internal/webassets/static/dialog.js:160-184`, `internal/webassets/static/tests/jobs-modal.test.js:66-71`.

The modal retains a literal `hidden` class until Alpine flushes `:class`, but `dialogController.open()` performs its only focus attempt synchronously. Live Chromium reproduced a visible modal with focus on `document.body` while the background was inert.

**Remediation:** invoke the controller from Alpine `$nextTick` or remove `hidden` before opening. Add a mounted browser assertion for initial focus and focus return.

### M-12 — Windows amd64 resources are stale and Windows arm64 has none

**Evidence:** `cmd/mpv-manager/winres/winres.json:13-42`, `cmd/mpv-manager/rsrc_windows_amd64.syso`, `Makefile:64-67`, `.gitlab-ci.yml:188-260`.

The only `.syso` is amd64 and its version metadata is hard-coded to `1.0.0.0`. Tag CI directly builds both Windows targets without generating resources; the Make helper suppresses generator failures. Inspection confirmed an amd64 `.rsrc` section and no arm64 resource table.

**Remediation:** generate architecture-specific resources using the release version, fail on generator errors, and inspect icon, manifest, DPI, and version resources in both PE outputs during tag CI.

### M-13 — Manager-data reset ignores backup failures but promises recovery

**Evidence:** `pkg/config/config.go:1095-1143`, `pkg/tui/reset_data.go:33-65`.

Reset ignores the backup write error, logs success unconditionally, overwrites a fixed `.backup`, and then saves defaults. The TUI says “backup saved.” A permissions/disk failure can therefore destroy the only live tracking/preferences file without the promised backup.

**Remediation:** create a unique, atomic, durable backup and abort reset if it fails. Return the backup path and test failure cases.

### M-14 — Backup containment misses intermediate symlinks

**Evidence:** `pkg/config/validation.go:15-99`, `pkg/web/api_config.go:586-633`.

Validation rejects a symlink only at the final component and then performs lexical `Abs`/`Rel` containment. A valid-looking file below an intermediate symlink can resolve outside the backup directory and be restored or deleted.

**Remediation:** because backups are flat, require the canonical parent to equal the canonical backup directory. Prefer descriptor-relative no-follow restore/delete operations.

### M-15 — File/config locks are process-local while multiple manager modes share files

**Evidence:** `internal/fileops/fileops.go:1-51`, `pkg/config/config.go:61-67`, `docs/TROUBLESHOOTING.md:103-105`.

The path-lock and manager-config mutexes coordinate goroutines only. Concurrent Web/TUI/CLI processes can each read stale JSON or `mpv.conf` and then atomically replace the other process’s changes. Documentation currently asks users to run one instance rather than enforcing it.

**Remediation:** enforce a per-user singleton or add advisory cross-process locks and revision/CAS checks for every shared mutable file.

### M-16 — Privileged release jobs use mutable container tags

**Evidence:** `.gitlab-ci.yml:53,141,190,286,333,431,479,553`.

Build/signing jobs use moving Go/Node tags, while packaging/upload/release jobs use `alpine:latest` and `release-cli:latest`. Those images can change without repository review while jobs hold package, signing, or release credentials.

**Remediation:** pin every privileged CI image by digest and update pins through reviewed dependency automation.

### M-17 — Reduced-motion preferences do not cover dialog animations

**Evidence:** `internal/webassets/static/style.css:173,379,429`, `internal/webassets/templates/password-modal.html:131`, `internal/webassets/templates/ui-select-modal.html:158`.

The reduced-motion block handles toast/task/progress/drag behavior but leaves generic, job, password, and UI-selection dialog animation/transition rules active.

**Remediation:** centralize dialog motion and disable animation/transition under `prefers-reduced-motion: reduce`. Add browser computed-style assertions.

### M-18 — TUI PATH add/remove hides partial failure and leaves shell aliases behind

**Evidence:** `pkg/tui/models_messages.go:180-315,377-388`.

Add writes an alias every time, ignores symlink/alias failures, and resets the final error to nil. Remove deletes the copied binary/symlink but never removes the shell alias and likewise returns success after removal errors. PATH status only tests file existence, not command resolution.

**Remediation:** use a marked idempotent shell block per supported shell, remove that block, propagate partial errors, roll back failed adds, and verify with `exec.LookPath` in a controlled environment.

### M-19 — Release-generator artifact downloads are unbounded in time and size

**Evidence:** `cmd/generate-info/main.go:56-67,597-634`.

The download client intentionally has no total timeout and copies response bodies without a byte cap. A stalled or oversized upstream can hang the signing job or exhaust runner storage.

**Remediation:** add per-artifact size caps, request/context deadlines, low-speed/idle limits, `Content-Length` validation, and partial-file cleanup.

### M-20 — The signed manager manifest is derived from re-downloaded registry bytes, not pipeline build artifacts

**Evidence:** <code>.gitlab-ci.yml:429-506</code>, <code>cmd/generate-info/main.go:948-977,1234-1267</code>.

The manifest job waits for package publication but receives only the generator artifact, reconstructs public registry URLs, and downloads/hashes those bytes before signing. The signature therefore binds to what the registry returned at signing time rather than directly to the six binaries produced by the build jobs.

**Remediation:** pass the exact raw build artifacts into the signing job, compute metadata locally, upload once, and byte/hash-compare every published object against that provenance before signing or stable publication.

### M-21 — <code>BLAKE3SUMS.txt</code> is not compatible with the documented <code>b3sum -c</code>

**Evidence:** <code>cmd/gen-checksums/main.go:49-90</code>, <code>.gitlab-ci.yml:609-623</code>.

The checksum file writes <code>blake3:&lt;64 hex&gt;</code> before each filename, while standard check files expect the raw 64-character hexadecimal digest. The release notes explicitly tell users to run <code>b3sum -c</code>, and CI does not exercise it.

**Remediation:** emit raw hex in the GNU-style checksum file, retain prefixed values only in the JSON schema, add generator tests, and have release CI run the documented check against all archives.

### M-22 — Stored-password Web installs dispatch the privileged request twice

**Evidence:** <code>internal/webassets/static/password-modal.js:391-416</code>, <code>internal/webassets/static/install.js:287-315,350-390</code>, corresponding UI-select callers.

<code>ensurePassword</code> invokes its callback when a password is already stored and also returns true. Callers start the operation in the callback and then again after the true return. The backend prevents a second same-method job, but the resulting 409 path can re-enable the button while the accepted job is active and produces false conflict feedback.

**Remediation:** give <code>ensurePassword</code> one ownership contract—prefer returning a boolean without invoking the callback on the already-authenticated path—and test stored, missing, cancelled, and failed authentication flows.

### M-23 — Config Apply can mark edits as saved even though they were never submitted

**Evidence:** <code>internal/webassets/static/config-page.js:74-108</code>.

The request submits one settings snapshot, but on success the baseline is rebuilt from the controls' then-current values. If the user edits while the request is in flight, the server stores the old values while the new local values become the clean baseline. Navigation warning is also disabled during the request even though navigation can abort it.

**Remediation:** capture the submitted payload once, set the success baseline to exactly that payload, immediately recompute dirty state against current controls, and retain unload protection until successful completion.

### M-24 — UI-option and regional-language requests allow stale responses to overwrite newer intent

**Evidence:** <code>internal/webassets/static/ui-settings.js:424-497,523-590</code>, <code>internal/webassets/static/languages.js:424-439,489-515,551-585,875-887</code>.

UI setting saves/resets can overlap without per-key generations or cancellation. In the language flow, aborted request A can clear request B's global controller in <code>finally</code>, and responses are applied without checking the request ID or selected language. Slow requests can restore stale values or render variants for the wrong selection.

**Remediation:** serialize mutations per key or use AbortController plus monotonic generations, ignore every stale response, and clear a controller only if it is still active.

### M-25 — CLI <code>--path</code> is ignored by component installers but recorded as the actual path

**Evidence:** <code>cmd/mpv-manager/main.go:534-577,602-625,649-663</code>, <code>pkg/installer/common_handler.go:68-90</code>.

The CLI constructs an installer with the validated path, but ModernZ/uOSC resolve the global MPV config and FFmpeg resolves the global install path. After success, tracking records the user-supplied destination. Disk state and authoritative metadata can therefore disagree.

**Remediation:** plumb the resolved destination through component dispatch and record the actual path, or reject <code>--path</code> for methods where it has no defined meaning.

## P3 / Low findings

### L-01 — Manifest-status network exceptions leave install controls enabled

**Evidence:** `internal/webassets/static/manifest-status.js:34-48`.

Non-OK responses fail closed in the UI, but a rejected fetch only logs. The banner remains hidden and controls remain enabled; the backend still rejects unsafe installation, so this is misleading degraded UX rather than a security bypass.

**Remediation:** route the catch through the same unavailable/banner/disable path and test both rejection and non-OK responses.

### L-02 — No Content Security Policy protects the privileged loopback UI

**Evidence:** `pkg/web/middleware.go:32-40` and inline Alpine/template behavior.

No exploitable injection was found, and dynamic data was generally escaped or assigned through text APIs. A CSP would still reduce the impact of a future escaping error. Current inline expressions, handlers, styles, and scripts require staged refactoring first.

**Remediation:** remove inline behavior, disable unnecessary HTMX script evaluation, then deploy/report-only-test a policy centered on `default-src 'self'`, `object-src 'none'`, `base-uri 'none'`, and `frame-ancestors 'none'` with nonces/hashes or a CSP-compatible Alpine build.

### L-03 — Local `make release` can package stale frontend assets

**Evidence:** `Makefile:222-227,320-337`.

The local release target does not depend on frontend build/vendor freshness/tests. GitLab tag CI does check freshness, so public CI releases are protected, but the documented local path may embed stale JS or CSS.

**Remediation:** make local release run locked dependency installation, vendor/Tailwind freshness, and frontend tests before packaging.

### L-04 — Duplicate hotkey lines are only partially edited

**Evidence:** `pkg/hotkeys/inputconf.go:314-419`.

Find/Set/Remove operate on the first matching key. Removing can report success while another binding for the same key remains, and overwrite can leave a conflicting duplicate.

**Remediation:** define the effective-binding rule and normalize/remove all duplicates with regression tests.

### L-05 — Public parsed script-options `Write` can overwrite fresher edits

**Evidence:** `internal/scriptopts/scriptopts.go:70-88,150-175`, `pkg/modernzconf/modernzconf.go:65-69`, `pkg/uoscconf/uoscconf.go:61-65`.

Parsing occurs before the path lock; `File.Write` later locks only the stale snapshot write. Current Web/TUI single-value production paths reparse under lock, so this is a public API hazard rather than a demonstrated current call-site bug.

**Remediation:** deprecate snapshot `Write` or add revision/CAS detection.

### L-06 — Reset text claims language preferences are cleared when they are stored elsewhere

**Evidence:** `pkg/tui/reset_data.go:59-65`, `pkg/config/config.go:933-950,1095-1143`.

The prompt says manager-data reset includes language preferences, but audio/subtitle preferences live in `mpv.conf`; `ResetToDefaults` only replaces `mpv-manager.json`.

**Remediation:** correct the wording or transactionally reset both files with explicit backups.

### L-07 — Staticcheck exposes small but real TUI state defects

**Evidence:** `pkg/tui/models_views.go:818-842`, `pkg/tui/models_update.go:240-279`, `pkg/tui/hwaccel_config.go:205-215`, `pkg/installer/validation.go:84`.

The screenshot view clears an error on a value receiver, so the real model retains it (SA4005). Duplicate Escape conditions make the later refresh branch unreachable (SA4014). HWA preference persistence and installer validation contain empty branches (SA9003), including a comment promising a warning that is never logged.

**Remediation:** move state changes into Update/pointer-owned transitions, collapse the Escape state machine, handle/log intentional soft failures, and add Staticcheck correctness checks to CI.

### L-08 — macOS maintained documentation disagrees about universal and app-bundle outputs

**Evidence:** `docs/FEATURES.md:28,569-586`, `docs/PLATFORM_GUIDES/MACOS.md:3-23`.

The Features document claims universal and `.app` bundle outputs; the platform guide correctly says v1.3 publishes separate raw Intel/ARM binaries and no universal/app bundle.

**Remediation:** make Features match the platform guide and current release jobs.

## Coverage and release-gate gaps

These are not additional proven production defects, but they materially affect confidence:

1. **No real browser lifecycle suite in CI.** `npm test` is Node/Vitest logic and static-source testing. The live jobs-modal focus defect passed its mocked controller test. Add Chromium coverage for Alpine mount/flush, HTMX swaps, dialog focus/return, mobile navigation, primary routes/viewports, console/network failures, and automated accessibility checks.
2. **Language workflows are under-tested.** `pkg/tui/language_preferences_test.go` is a placeholder despite a large state machine, and the main `languages.js` workflow lacks a direct test suite. Exercise duplicate priorities, search/variant selection, save failures, and HTMX reinitialization.
3. **Native destructive/platform behavior remains a release gate.** Cross-builds passed, but this Linux review did not execute Windows UAC/uninstall/detection, macOS DMG/signing/quarantine, or actual package-manager install/uninstall flows.
4. **Release metadata fields are not enforced.** `NativeSigning`, `Format`, `InstallScope`, and `UpdateStrategy` are validated largely as nonempty strings but are not consumed as policy. CI executes manager identity only on Linux amd64. Add PE/Mach-O identity/resource/signature assertions and platform-native artifact inspection.
5. **Fault-injection and contention cases are missing.** Add tests for config write failure rollback, blocked package probes, cancellation at commit boundaries, shared-resource jobs, same-path multi-store history, connected-SSE shutdown, terminal reconnect, intermediate symlinks, restore rollback, and same-day repeated restores.

The npm graph had no reported vulnerability. `npm outdated` reported Alpine.js 3.16.3 with 3.17.0 available; treat that as routine maintenance, not a security finding.

## Areas that held up well

- Release manifests fail closed for empty trust, missing/invalid signatures, malformed schema, missing authenticated size/hash, tampering, and wrong staged manager identity.
- Native 7z extraction includes count/size/path/device/symlink/case-collision and exclusive-create defenses.
- Web API authentication, constant-time token comparison, loopback Host enforcement, Origin checks, request-size limits, and keyring authentication throttling were sound in reviewed paths and live smoke tests.
- Job snapshots are copied under locks, output relay goroutines are drained/joined, and the race suite found no shared-pointer race in the reviewed job manager paths.
- ModernZ/uOSC/Hotkeys production single-value writers reparse under a process-wide path lock and atomically replace files.
- Keyring implementations use native OS storage with bounded probes and no weak plaintext/file fallback.
- No confirmed DOM XSS was found; reviewed dynamic content generally uses Go template escaping, `textContent`, `x-text`, or explicit escaping.
- Embedded Alpine/htmx assets matched their locked npm distributions and no CDN runtime dependency was present.

## Recommended remediation sequence

1. **Stop destructive/privileged wrong-target behavior:** H-01, H-02, H-03, H-04, H-16, H-17, and M-04.
2. **Rebuild the release trust boundary:** H-13, H-14, M-16, and M-19.
3. **Make job and config outcomes authoritative:** H-05 through H-10, M-05, M-06, M-10, M-13, and M-15.
4. **Close updater/installer recovery gaps:** H-11, H-12, H-15, M-01 through M-03.
5. **Repair lifecycle and accessibility behavior:** M-07 through M-09, M-11, M-17, M-22 through M-24, and the browser test gap.
6. **Finish portability, hardening, and cleanup:** M-12, M-14, M-18, and the P3 findings.

For each P1 fix, add a regression that reproduces the failure before changing behavior. Windows uninstall/targeting and macOS DMG fixes require native validation before release; cross-compilation alone is not sufficient.

## Review limitations

- The review was performed primarily on Linux. Windows and macOS outputs were cross-built and inspected where possible, not executed natively.
- No destructive real install/uninstall, production signing, release publication, external account mutation, or privileged package-manager flow was run.
- The local live-server smoke test used disposable configuration and a loopback port. A focused reviewer separately reproduced the modal-focus failure in Chromium.
- Static review can miss environment-dependent behavior. The findings above are limited to evidence-backed paths; speculative concerns that could not be tied to a reachable impact were not promoted.