Independent code, security, concurrency, frontend, CI, and release audit

MPV.Rocks Installer deep codebase review

31 August 2026 · commit bdfb31b95030c2d992e5cfc4cb0ba5bada368d1b · branch master

Read the Markdown edition

0 P0 / critical
17 P1 / high
25 P2 / medium
8 P3 / low

Executive summary

No critical issue was confirmed. The largest risks are Windows uninstall/elevation and multi-install targeting; non-transactional job/config outcomes; release-signing trust boundaries; and timeout/recovery/coordination gaps around otherwise strong signed-update machinery.

All normal and race-enabled Go suites passed, all 157 frontend assertions passed, six release targets cross-built, and dependency vulnerability scans were clean. The signed manifest path, Web authentication boundary, native 7z defenses, keyring backends, and most shared dialog behavior held up well.

Recommended release blockers: H-01 through H-04 and H-12 through H-17. Address H-05 through H-10 next before treating Tasks/history and manager configuration as authoritative.

Scope and method

The review covered all Go commands/packages, Web and TUI code, embedded frontend assets, platform installers, signed manifests/self-update, CI/build scripts, and maintained documentation. Three gpt-5.6-sol/xhigh focused reviewers independently covered security/updater/platform code; backend/config/jobs/concurrency; and frontend/accessibility/CI. The primary reviewer validated and deduplicated their strongest findings and ran repository-wide checks.

Severity definitions: P0 is directly exploitable or unrecoverably destructive without an additional intended trust decision; P1 is plausible privilege, supply-chain, destructive-data, wrong-target, indefinite-availability, or authoritative-state failure; P2 is a material reliability/security/accessibility issue; P3 is limited impact or defense-in-depth.

Validation

CheckResult
go test -count=1 ./...Pass
go test -race -count=1 ./...Pass
go test -shuffle=on -count=3 ./...Pass
go vet ./..., formatting, tidy diff, module verification, diff checkPass
govulncheck@v1.7.0 ./... built with Go 1.27No vulnerabilities
Staticcheck v0.8.1 SA*13 diagnostics; four production defects captured below
npm test and shuffled frontend run12 files / 157 tests pass
npm audit --audit-level=moderate0 vulnerabilities
Alpine/htmx vendor freshness and Tailwind rebuildByte-identical
Linux amd64/arm64, Windows amd64/arm64, Darwin amd64/arm64All cross-build
Loopback Web routes, cookie auth, hostile Host/Origin, OPTIONS, no-client shutdownExpected behavior

Repository-wide statement coverage was 43.7%. Installer was 84.1%, platform 92.3%, job history 90.7%, version 72.9%, config 70.3%, Web 28.5%, TUI 26.0%, and the main command 11.9%. This is directional because per-package coverage does not fully attribute cross-package execution.

P1 / High findings

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

pkg/installer/windows.go:160-212; pkg/installer/common_handler.go:165-184; pkg/tui/models_messages.go:419-423

The uninstall flow copies an install-tree mpv-unregister.bat and runs it with Start-Process -Verb RunAs, without digest, signature, ownership, ACL, or reparse-point validation at elevation time. An unprivileged process can replace it and gain administrator execution when the expected UAC prompt is approved.

Fix: use fixed native cleanup or an embedded authenticated helper in an ACL-protected location; verify immediately before elevation and prohibit manager uninstall for unmanaged records.

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

pkg/installer/windows.go:121-142; pkg/installer/windows_detection.go:30-53; pkg/web/api_adopt.go:447-476

Uninstall recursively removes every direct child except a small preserve list, while detection/adoption accepts shared custom, PATH, Scoop, and Chocolatey locations. An adopted MPV in C:\Tools can cause unrelated siblings to be removed.

Fix: require a manager ownership marker and per-install manifest; delete only owned paths and use upstream uninstall for adopted/package-managed installs.

H-03 — TUI update/uninstall can target another installation

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

The TUI drops AppID and InstallPath from update items and dispatches through a global installer. With multiple/custom/adopted Windows installs, choosing B can update or delete A.

Fix: carry stable identity/path through every message and build an immutable per-operation installer for the exact selected record.

H-04 — Custom Windows installs and settings resolve different config trees

pkg/web/api_settings.go:479-518; pkg/installer/windows.go:410-423; pkg/constants/paths.go:47-79,306-319; config/hotkeys/scriptopts/uiconfig resolvers

Install writes <custom>\portable_config, but later settings, hotkeys, ModernZ/uOSC, migration, backup, and restore resolve AppData/legacy paths. Successful changes may affect a tree MPV never reads.

Fix: use one installed-app-aware resolver across every configuration surface.

H-05 — Cancellation archives “cancelled” before workers finish commits

pkg/web/jobs.go:482-543; install/uninstall/adopt/UI workers

CancelJob immediately marks, persists, removes, and broadcasts a terminal job. A worker can pass its last cancellation check and still commit physical/config side effects; later completion updates miss the archived job.

Fix: cancellation only requests cancellation; the worker owns one compare-and-set terminal transition at a defined commit boundary.

H-06 — Job conflicts are method-based, not resource-based

pkg/web/jobs.go:256-279; Linux installers; component jobs; pkg/web/api_config.go:563-615

Different method IDs and direct reset/restore/migration endpoints mutate shared config/UI/install resources concurrently. One rollback can undo another successful operation.

Fix: add deterministic resource-keyed leases for config trees, targets, package managers, and app identities.

H-07 — Physical success is reported despite tracking persistence failure

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 and TUI flows warn or log when installed-app metadata cannot be saved, then report success. Tasks/history can contradict the physical system.

Fix: include persistence in terminal outcome; expose structured partial success and reconciliation when physical rollback is impossible.

H-08 — Failed manager-config writes leak into memory

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

Many setters mutate globalConfig before saving and do not restore it on failure. A failed mutation is immediately observable and can be persisted by a later unrelated save.

Fix: mutate clones and publish after successful persistence, or route all setters through the rollback-capable update primitive.

H-09 — Config read errors are treated as a missing file

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

Permission, I/O, and unavailable-mount errors fall through to defaults and a nil return. Later writes can replace valid state. A shadowed decode error also logs <nil> before quarantine.

Fix: default only on IsNotExist; retain last known-good state and return other errors.

H-10 — Config restore has duplicate rename steps and no rollback

pkg/installer/installer.go:1165-1195; pkg/installer/common.go:161-194; date-only backup names

The live file is renamed before a second helper tries to back it up again. Copy failure can leave mpv.conf absent, and repeated same-day restores collide.

Fix: stage/fsync/swap once, restore the original on failure, and use unique names.

H-11 — Package-version timeout does not terminate its loop

pkg/web/server_version_cache.go:249-364; unbounded commands in pkg/web/package_version.go; synchronous startup

An unlabelled break exits the select, not the collector loop. The one-shot timer is then drained and an unbounded package probe can prevent Web startup or freeze refresh indefinitely. Staticcheck reports SA4011.

Fix: context-bound probes, shared cancellation, a real return/labelled break, and probing outside the cache lock.

H-12 — IINA install trusts a hard-coded global DMG mount

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

The code discards hdiutil output, assumes /Volumes/IINA, and validates only structure. A name collision/alternate mount can copy an unrelated app and detach the wrong volume.

Fix: parse hdiutil attach -plist, use an owned random mount, validate bundle ID/architecture/code signature/Team ID, and detach by returned device.

H-13 — Tagged generator code receives the release private key

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

The signing job executes a generator built from the tag with the long-lived key in its environment. Compromised tagged code can exfiltrate the key and forge future releases.

Fix: isolate signing behind a pinned minimal signer and non-exportable KMS/HSM key; tagged application code must never receive key material.

H-14 — Upstream “latest” bytes become first-party signed assets without provenance verification

cmd/generate-info/main.go:358-448,546-634,948-977,1174-1232,1260-1269

The generator discovers upstream latest releases, downloads bytes, hashes them, and immediately signs/publishes those hashes without upstream signature/checksum/native-signature verification or a reviewed digest allowlist.

Fix: pin reviewed versions/digests, verify upstream and platform-native signatures, and require approval before isolated signing.

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

pkg/version/transaction.go:349-358,466-485; cmd/mpv-manager/main.go:101-180,273-275

The helper is launched with stdout/stderr redirected to helper.log, then launches the new TUI using its inherited stdout/stderr while stdin remains the terminal. The new TUI can render invisibly into the log. Health is acknowledged before Bubble Tea initialization or first render, so this can still commit as healthy.

Fix: hand the child real controlling-terminal descriptors and acknowledge only after a visible first render; qualify in a native PTY.

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

pkg/installer/installer.go:593-608

The updater renames ffmpeg.exe to .bak, defers deletion of that backup, and then copies the replacement. If copy fails, deferred cleanup removes the backup and leaves no executable.

Fix: stage/verify before touching live state, atomically swap, roll back on every failure, and delete backup only after verification.

H-17 — TUI language apply replaces the full live config from defaults

pkg/tui/language_preferences.go:823-859; pkg/config/editor.go:135-177

The TUI renames mpv.conf to a date-only backup before setting one language field. Seeing no live file, the shared editor loads the embedded default and writes it, dropping every other user setting from the active config.

Fix: remove the rename path and update the existing file through the shared atomic backup/edit transaction.

P2 / Medium findings

IDFinding and evidenceRecommended action
M-01uOSC ZIP extracts over live user config outside rollback scope. pkg/installer/installer.go:681-717, archive helpers, managed UI snapshots.Extract natively into an owned temp root, strictly allowlist, then transactionally overlay owned files.
M-02Multi-target self-update locks only the primary. pkg/version/transaction.go:205-210,265-316,417-451,750-776.Lock every canonical target in deterministic order or use a per-user global update lock.
M-03Crash before first journal blocks future updates. Transaction directory precedes journal; recovery returns on unreadable orphan.Write/fsync an initial journal immediately and quarantine safe pre-journal orphans while continuing recovery.
M-04Windows discovery executes every found mpv.exe. pkg/installer/windows_detection.go:30-53,69-105,133-163.Read PE metadata; require adoption/provenance before execution and never probe while elevated.
M-05Multi-field mpv.conf APIs are not transactional. Config Apply/languages write fields separately despite an existing batch writer.Build one validated batch per file and commit once.
M-06Job-history locks are per Store instance and temp name is fixed. pkg/jobhistory/jobhistory.go:64-157.Canonical path lock, unique temp files, advisory process lock, and one retained TUI store.
M-07Global SSE can make graceful shutdown time out and exit 1. Page-global stream, request-context-only exit, two-second Shutdown.Cancel a server base context, close SSE clients, then shut down.
M-08Terminal SSE delivery/reconnect snapshot is not guaranteed. Nonblocking send can abandon terminal events; filtered reconnect sees active jobs only.Per-client serialized queue with terminal priority and active/recent snapshot.
M-09TUI Ctrl+C does not cancel or join active work. Immediate quit plus context.Background() operations.Model-owned cancellation and worker/child cleanup before quit.
M-10UI migration resolution is split across two unsynchronized stores. Concurrent Apply/Keep and persistence failure can disagree.Serialize with CAS and add rollback/recovery.
M-11Job modal becomes visible after focus was attempted. Live Chromium left focus on body while background was inert.Open controller after Alpine nextTick; add mounted browser focus assertions.
M-12Windows amd64 resources say 1.0.0; arm64 has no resource table. One amd64 syso, swallowed generation failures, direct CI builds.Generate versioned per-arch resources and inspect both PE outputs in tag CI.
M-13Manager-data reset ignores backup failure while promising recovery. pkg/config/config.go:1095-1143.Require a unique, atomic, durable backup before reset.
M-14Backup containment misses intermediate symlinks. Final-component Lstat plus lexical Abs/Rel.Require canonical parent equality or descriptor-relative no-follow operations.
M-15Shared config/file locks are process-local. Concurrent Web/TUI/CLI instances can last-writer-win.Enforce singleton or add advisory locks plus revisions/CAS.
M-16Privileged CI jobs use mutable image tags. Moving Go/Node, alpine:latest, release-cli:latest.Pin by digest and update through reviewed automation.
M-17Reduced-motion does not disable dialog animations. Generic/job/password/UI-select rules remain active.Centralize motion overrides and add browser computed-style checks.
M-18TUI PATH management hides partial errors and leaves aliases. It resets errors to nil and does not remove shell additions.Managed idempotent shell blocks, rollback, propagated errors, real lookup verification.
M-19Release-generator downloads are unbounded in time and size. No total deadline and unrestricted copy.Deadlines, idle limits, byte caps, length checks, and partial cleanup.
M-20Signed manager metadata is derived from re-downloaded registry bytes, not direct build artifacts. The signing job receives the generator and registry URLs rather than all six provenance artifacts.Hash local build artifacts, publish once, and compare registry bytes before signing.
M-21BLAKE3SUMS.txt is incompatible with documented b3sum -c. It emits a blake3: prefix instead of raw hex.Emit standard check-file hex and run the documented consumer in release CI.
M-22Stored-password installs dispatch twice. ensurePassword invokes the callback and returns true; callers start in both paths.Give password gating one ownership contract and test every authentication state.
M-23Config Apply can mark an in-flight edit clean without submitting it. Success baseline is recollected from current controls rather than the submitted payload.Baseline the captured payload and recompute dirty state after completion.
M-24UI-option and regional-language requests allow stale results to win. Overlapping saves lack generations; an aborted request can clear a newer controller.Serialize or generation-tag mutations, abort safely, and ignore stale responses.
M-25CLI --path is ignored by component installers but recorded as used. ModernZ/uOSC/FFmpeg resolve global paths.Plumb the actual destination or reject path for unsupported methods.

P3 / Low findings

IDFindingRecommended action
L-01Manifest-status rejected fetch leaves banner hidden and install controls enabled; backend still fails closed.Use the same unavailable path for exceptions and non-OK responses.
L-02No CSP contains a future Web injection; no current DOM XSS was confirmed.Remove inline behavior, then deploy a strict report-only/enforced CSP.
L-03Local make release can package stale frontend assets; GitLab releases are freshness-gated.Make local release run locked install, freshness, build, and tests.
L-04Duplicate hotkey lines are only partially edited.Define effective-binding semantics and normalize all duplicates.
L-05Public parsed script-options Write can overwrite fresher edits; current single-value UI paths reparse safely.Deprecate snapshot Write or add revision/CAS.
L-06Reset text claims language preferences are cleared although they live in mpv.conf.Correct wording or reset both files transactionally.
L-07Staticcheck found a value-receiver state clear, duplicate Escape branches, and empty soft-error branches.Fix state transitions and add Staticcheck correctness checks to CI.
L-08FEATURES claims macOS universal/.app outputs; the platform guide and CI publish separate raw binaries.Align maintained documentation with current artifacts.

Coverage and release-gate gaps

  1. No real browser lifecycle suite in CI. Node/Vitest tests did not observe the Alpine flush/focus failure. Add Chromium route, HTMX, dialog, viewport, console/network, and accessibility coverage.
  2. Language workflows are under-tested. The TUI language test is a placeholder and the main languages.js workflow lacks a direct suite.
  3. Native destructive/platform behavior remains a gate. This review cross-built but did not run Windows UAC/uninstall or macOS DMG/signing/quarantine behavior.
  4. Release policy metadata is not enforced. NativeSigning, Format, InstallScope, and UpdateStrategy are largely nonempty strings; identity execution is Linux amd64 only.
  5. Fault injection/contention is sparse. Add blocked probes, write rollback, cancellation/commit, resource collision, history contention, connected-SSE shutdown, terminal reconnect, symlink, and restore rollback cases.

The npm graph had no reported vulnerability. Alpine.js 3.16.3 has 3.17.0 available; this is routine maintenance, not a security finding.

Areas that held up well

Recommended remediation sequence

  1. Stop destructive/privileged wrong-target behavior: H-01 to H-04, H-16, H-17, and M-04.
  2. Rebuild release trust: H-13, H-14, M-16, M-19.
  3. Make job/config outcomes authoritative: H-05 to H-10, M-05, M-06, M-10, M-13, M-15.
  4. Close updater/installer recovery gaps: H-11, H-12, H-15, M-01 to M-03.
  5. Repair lifecycle/accessibility: M-07 to M-09, M-11, M-17, M-22 to M-24, and browser coverage.
  6. Finish portability/hardening/cleanup: M-12, M-14, M-18, and P3 items.
Regression rule: reproduce every P1 failure in a test before changing behavior. Windows uninstall/targeting and macOS DMG fixes require native release-gate validation; cross-compilation is insufficient.

Limitations