MPV.Rocks Installer — Combined Cross-Referenced Review (glm-5.3)

Date: 2026-08-31 · Baseline: bdfb31b95030c2d992e5cfc4cb0ba5bada368d1b (master)
Sources cross-referenced: Review A (glm-5.3) = CODEBASE_REVIEW_2026-08-31_glm-5.3.md (154 findings) · Review B (prior) = CODEBASE_REVIEW_2026-08-31.md (50 findings)
This report cross-references every finding, independently verifies all single-sourced claims, resolves severity disagreements, and merges the remediation roadmaps. Markdown companion: CODEBASE_REVIEW_COMBINED_2026-08-31-glm-5.3.md

Corroborated clusters

Unique to prior — all re-verified

~104

Unique to glm-5.3

Refuted claims

Severity disagreements resolved

Combined severity totals — deduplicated across both reviews (204 raw findings → 153 distinct issues), harmonized per §5

Critical

High

Medium

Low

Info

153

Total distinct issues

1. Cross-reference outcome

The two reviews agree on every mutually-inspected high-risk area and disagree on almost nothing factually — where they overlap, file/line references match within a few lines. The prior review's dynamic tooling (Staticcheck, race, shuffle, live Chromium, coverage) and the glm-5.3 audit's broader static depth are complementary: Review B caught 9 issues glm-5.3 missed (notably a privilege-escalation path and a config-tree divergence), while Review A caught ~104 issues Review B missed (notably the updater's exec-before-hash ordering, crash-consistency defects, and large TUI/frontend correctness surfaces).

Combined dynamic validation (union of both reviews + this session)

Check	Result	Source
`go vet ./...`	Pass	B; re-run this session (A)
`go test ./...`	Pass, all packages	A (this session) + B
`go test -race -count=1 ./...`	Pass	B
`go test -shuffle=on -count=3 ./...`	Pass	B
gofmt / tidy -diff / mod verify / git diff --check	Pass	B
`govulncheck@v1.7.0` (Go 1.27)	0 vulnerabilities	B
Staticcheck 0.8.1 (`SA*`)	13 diagnostics; 4 production correctness findings	B
`npm test` (157 assertions, shuffled too)	Pass	B
`npm audit --audit-level=moderate`	0 vulnerabilities	B
Alpine/htmx vendor freshness	Pass, byte-for-byte	B
Six release cross-builds	Pass	B
Local Web route/API/auth smoke test	Pass (cookie auth, hostile Host/Origin, graceful shutdown)	B
Live Chromium jobs-modal focus reproduction	Reproduced	B
Coverage (one repo-wide run)	43.7% overall; installer 84.1%, platform 92.3%, version 72.9%, config 70.3%, web 28.5%, TUI 26.0%, cmd 11.9%	B
Source re-verification of all 9 Review-B-unique claims	All confirmed	This session

2. Tier A — Corroborated findings (41 clusters, independently found by both reviews)

Highest-confidence findings of the audit. Severity harmonized (§5).

A.1 Corroborated Critical/High (13 clusters)

#	Finding	A (glm-5.3)	B (prior)	Verification
1	CRITICALRelaunched self-updated TUI renders into `helper.log` while consuming terminal input; health acknowledged before any real initialization, so the broken relaunch can be committed as "healthy"	C-1 + H-3	H-15	Source-verified by both; B notes ack placement in main.go
2	HIGHWindows uninstall recursively deletes the install directory with no ownership proof; per-entry errors are warnings; success returned regardless	H-6	H-02	A lead verified `windows.go:103-157`
3	HIGHTUI update/uninstall drops `AppID`/`InstallPath`; operations run against the global installer destination — selecting install B can update/delete install A	H-14	H-03	Both traced models_types.go / models_update.go
4	HIGHJob cancellation archives/removes the job before the worker stops; a conflicting destructive job is accepted while side effects continue	H-11	H-05	Both traced jobs.go:482-543
5	HIGHJob conflict detection protects method IDs, not shared resources; different methods (or direct config APIs) can concurrently mutate the same config/UI tree	I-M5 (Med→High)	H-06	Both traced jobs.go:256-279
6	HIGHPhysical success reported when authoritative persistence fails (untracked installs, tracked uninstalls, stale adoption/UI metadata)	T-M2 (Med→High)	H-07	Both traced install/adopt workers
7	HIGHConfig setters mutate in-memory state and return errors without rollback; failed mutations later persisted by unrelated successful saves	P-M2 (Med→High)	H-08	Both enumerated setter families
8	HIGHConfig restore renames the live file aside before validating/staging the replacement; no rollback; date-only backup names collide same-day	H-20	H-10	Both traced installer.go / common.go
9	HIGHPackage-version "10 s timeout" ineffective: unlabeled `break` exits only the `select`; unkillable probes; run at startup and under the cache write lock	H-10	H-11	A lead verified line 313; Staticcheck SA4011 flags the same line (B)
10	HIGHIINA install discards `hdiutil attach` output, assumes `/Volumes/IINA`; volume collision copies an unverified bundle; detaches the wrong path	H-9	H-12	Both traced macos.go
11	HIGHUnauthenticated upstream "latest" artifacts hashed and signed into MPV.Rocks trust (no upstream signature/allowlist check)	U-M12 (Med→High)	H-14	Both traced generate-info
12	HIGHFFmpeg replacement: deferred deletion of `ffmpeg.old` runs even when the rollback rename failed — destroys the only known-good binary	H-7	H-16	Both traced installer.go:525-614
13	HIGHTUI language apply renames the entire `mpv.conf` away for a single-field write; editor writes fresh defaults (all user settings vanish); failed rename ignored	H-16	H-17	Both traced language_preferences.go / editor.go

A.2 Corroborated Medium (23 clusters)

#	Finding	A	B
14	uOSC/ZIP extraction runs external `Expand-Archive -Force`/`unzip -o` directly over live config; rollback allowlist covers only known UI paths	I-M6	M-01
15	Crash before the first updater journal write permanently blocks future updates (orphan dir; recovery stops at first bad dir)	U-M2	M-03
16	Windows discovery executes every discovered `mpv.exe --version` from user-writable/PATH/registry locations before adoption, unbounded	I-M1	M-04
17	Multi-field config/language API writes validated together, persisted field-by-field; partial apply on mid-failure (batch writer exists, unused)	W-M5	M-05
18	Job-history serialization per-`Store` instance; TUI creates fresh stores; fixed `.tmp` name; cross-process record loss	P-M7/S-M8/T-M10	M-06
19	Persistent SSE handlers defeat graceful shutdown: 2 s timeout → fatal → exit 1 with a connected browser	S-M1	M-07
20	SSE terminal-event delivery/reconnect gaps: narrow concurrent-sender drop window; reconnect snapshots only active jobs; client never reconciles	F-M2 + Low	M-08
21	TUI Ctrl+C quits without cancelling/joining backend work (`context.Background` workers; children outlive the TUI)	H-18	M-09
22	UI migration resolution spans two stores with check-then-act, no CAS; file changed while resolution says Keep	L-2 (Low→Med)	M-10
23	Jobs modal opens focus controller while still `hidden`; initial focus lands on inert body	F-M6	M-11 (reproduced in live Chromium)
24	Windows resources: only amd64 `.syso`, hardcoded 1.0.0.0/Win7; generator failures suppressed; arm64 has none	B-M2 + Low	M-12
25	Manager-data reset ignores backup failure, overwrites a fixed `.backup`, logs success; TUI claims "backup saved"	T-M13	M-13
26	Backup validation misses intermediate symlinks; restore/delete can escape the backup dir	W-M2/P-M1	M-14
27	File/config locks process-local while Web/TUI/CLI share the same files (snapshot `Write` parses before locking)	S-M6/S-M7	M-15
28	Privileged release jobs run mutable container tags (`alpine:latest`, `release-cli:latest`)	B-M6	M-16
29	TUI PATH add/remove: non-idempotent aliases, failures reset to nil, removal never removes aliases, Windows broken	T-M9	M-18
30	Release-generator artifact downloads unbounded in time and size	U-M9/B-M4	M-19
31	Signed manager manifest computed from re-downloaded registry bytes, not the pipeline's build artifacts	H-21 (High; B Med)	M-20
32	`BLAKE3SUMS.txt` emits `blake3:` prefix — incompatible with documented `b3sum -c`; never exercised by CI	U-M11/B-M8	M-21
33	Stored-password Web installs dispatch the privileged request twice (`ensurePassword` invokes callback and returns true)	F-M1	M-22
34	Config Apply baseline rebuilt from current controls, not the submitted payload; concurrent edits silently marked saved	F-M3	M-23
35	UI-option saves and regional-language requests allow stale responses to overwrite newer intent	F-M4 + F-M5	M-24
36	CLI `--path` ignored by ModernZ/uOSC/FFmpeg dispatch but recorded as `InstalledApp.InstallPath`	S-M9	M-25

A.3 Corroborated Low (5 clusters)

#	Finding	A	B
37	No CSP on the privileged loopback UI	Frontend Low	L-02
38	Public parsed script-options `Write` can overwrite fresher edits (no revision check)	Support Low	L-05
39	Reset text claims language preferences cleared; they live in `mpv.conf`	T-M13 (partial)	L-06
40	Staticcheck-exposed TUI defects (SA4005 value receiver, SA4014 duplicate branches, SA9003 empty branches)	TUI Lows	L-07
41	Docs claim macOS universal output that CI contradicts; release metadata/native gates unenforced	B-Low + U-Low + H-23 context	L-08 + coverage gaps

3. Tier B — Unique to the prior review (9 findings — all re-verified this session)

VERIFIEDEvery finding only Review B reported was independently confirmed against source during this cross-reference session. None is refuted. Net effect: the prior review contributed 4 new High findings (B-1 privilege escalation, B-2 config-tree divergence, B-3 defaults-on-read-failure, B-4 signing-key separation) that materially expand the risk picture beyond the glm-5.3 audit.

HIGHB-1 — Windows uninstall elevates a mutable install-tree batch file

pkg/installer/windows.go:160-212 · common_handler.go:165-184 · models_messages.go:419-423 (B: H-01)

mpv-unregister.bat is copied from the (possibly adopted, user-writable) MPV tree and executed via Start-Process -Verb RunAs with no digest/ownership/ACL check before UAC. An unprivileged process can replace the .bat and wait for the user to approve the expected prompt → admin code execution.

This session: confirmed — copy from install tree, fallback to the original path, elevated Start-Process, zero verification.

HIGHB-2 — Custom installs and settings APIs resolve different `portable_config` trees

pkg/constants/paths.go:52-72 · pkg/web/api_settings.go:479-518 (B: H-04)

Installer writes <custom>\portable_config, but config/hotkeys/ModernZ/uOSC/migration/backup resolvers use %APPDATA%\mpv or legacy home; the common resolver never consults the configured custom path (and caches with sync.Once). UI reports success against a tree MPV never reads.

This session: confirmed — getWindowsMPVBaseDir checks only APPDATA/home existence via windowsPathOnce.

HIGHB-3 — Config read failures treated as "missing file" and replaced with defaults

pkg/config/config.go:85-149 (B: H-09)

Permission errors, transient I/O failures, and unavailable mounts all fall through to default configuration and are saved over the existing file. The parse-error log prints the shadowed outer err (always <nil>).

This session: confirmed — any ReadFile error reaches "Start fresh defaults" + saveLocked(); inner err shadows the read error used in the log.

HIGHB-4 — Tagged generator code receives the long-lived release signing key

.gitlab-ci.yml:495-496 · cmd/generate-info/main.go:517-527 (B: H-13)

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 do not separate signer trust from the code being authorized.

This session: confirmed — the generator job requires MANIFEST_SIGNING_KEY_ID/MANIFEST_SIGNING_KEY.

#	Finding	B	This session's verification
B-5	MEDMulti-target self-update locks only the primary executable; concurrent primary/secondary updates can both mutate the same secondary	M-02	CONFIRMED`transaction.go:205-210`: single `tryAcquireUpdateLock(updateLockPath(primaryPath))`
B-6	MEDReduced-motion preferences don't cover dialog animations (generic/job/password/UI-select modals)	M-17	CONFIRMED`style.css:430-455` covers toast/badge/progress/priority-list only
B-7	LOWManifest-status network exceptions leave install controls enabled (catch only logs)	L-01	CONFIRMED`manifest-status.js:34-48`: `catch` does `console.error` only
B-8	LOWLocal `make release` can package stale frontend assets (no frontend deps/freshness/tests in the chain)	L-03	CONFIRMED`Makefile:227` chain; also corroborates A's B-M3 (`release-build` omits trust check)
B-9	LOWDuplicate hotkey lines only partially edited (first-match Set/Remove)	L-04	Consistent with the first-match parser traced by A's hotkeys review; not line-by-line re-verified

4. Tier C — Unique to the glm-5.3 review (~104 findings)

Single-sourced but lead-verified during the original audit. Grouped summary (full detail in the glm-5.3 report and .opencode/reviews/):

Updater / release stack (12)

Exec-before-BLAKE3-recheck TOCTOU at target-adjacent staging (H-1)
Apply/rollback crash windows with no launchable primary executable (H-2)
Missing backups treated as successful rollback (H-4)
v1.1/v1.2 legacy bootstrap unauthenticated (H-5)
No target-directory durability ordering with the journal (U-M1)
Forgeable PrepareSelfUpdateFromCheck DTO (U-M3)
No anti-replay/revocation/threshold on signed metadata (U-M4, U-M5)
Secondary-path identity never proven before replacement (U-M6)
Detached helper failures invisible to initiating UX (U-M7)
Kill-without-wait rollback race on Windows (U-M8)
Qualifier --qualifier-driver uncontained destructive mode (U-M10)
Release tag grammar validated only after publication begins (H-22/B-M7)

Installer / TUI / Web / core / frontend / main / CI (headline items)

Installer: config reset fail-open on preservation/backup failure (H-8); MPC-QT Start-Process without -Wait (I-M3); detection collapses uncertainty into "not installed" (I-M2); runtime file transactions not crash-durable (I-M7)
TUI: Escape abandons workers, Abort unreachable (H-12); nondeterministic stream completion — stuck "installing" or dropped output (H-13); "Change UI" full-reinstall + duplicate records (H-15); Enter-while-filtering triggers destructive actions (H-17); interactive sudo fights Bubble Tea for the raw terminal (H-19); stale "updates" menu ID (T-M1); progress bar 0–100 vs 0–1 unit bug (T-M5); quadratic output rendering (T-M7); Unicode backspace byte-slicing (T-M14); terminal control-sequence injection (T-M15); hardcoded version overlay (T-M11); ~33 s non-offline-first startup (T-M12)
Web: cached sudo timestamp accepts any password — sudo -S -v without -k (W-M1); locale cache data race (W-M4); ui_type unvalidated (Low); downgrades labeled updates (Low)
Core: quote-unaware # parsing in mpv.conf/input.conf/script-opts — three independent implementations (P-M3, P-M8, S-M4); Linux GPU model truncation and hybrid-order dependence (P-M4); macOS AV1 over-reporting incl. M2 (P-M5); arm64 labeled x86-64-v2 (P-M6); locale dataset/selection disagreements (P-M9); BOM/CRLF round-trip corruption (S-M5)
Frontend: SSE reconnect never reconciles (F-M2, corroborated); mobile drawer not focus-contained (Low); formatKeys template.HTML trust boundary (Low)
Main: subcommand/flag misrouting (--verbose cli starts Web) (S-M2); --verbose/--debug no-op console logging (S-M3); symlink-replacing atomic writes (S-M6); browser-launch/bind race (S-M10); os.Exit cleanup bypass; zombie browser helper
Build/CI: archives omit LICENSE/notices (B-M1); coverage regex not aggregate; make lint unpinned; browser E2E absence (B-M5)

5. Severity disagreements (resolved)

Topic	A (glm-5.3)	B (prior)	Resolution
Helper-log relaunch + early health ack	CRITICALC-1 + H-3	HIGHH-15	Critical. Both agree on facts. No supported recovery after commit, terminal silently captured, shipped qualification cannot detect it. Not attacker-triggerable RCE, but by impact-and-permanence the single worst defect.
Manifest signs re-downloaded registry bytes	HIGHH-21	MEDM-20	High. GitLab permits duplicate generic-package files by default; the signature is the root of trust and must bind to pipeline-produced bytes. B's own remediation implies the same.
Job conflict detection scope	MEDI-M5	HIGHH-06	High. Overlapping config/UI mutations can roll back committed work — authoritative-state failure.
Persistence-failure false success	MEDT-M2	HIGHH-07	High. Tasks UI as untrustworthy state is a core-flow failure.
Config setter rollback	MEDP-M2	HIGHH-08	High. Failed mutations later persisted by unrelated saves.
UI migration CAS	LOWL-2	MEDM-10	Medium. Two-store check-then-act verified this session.
TUI Ctrl+C	HIGHH-18	MEDM-09	High. Combined with Escape abandonment and shutdown-orphaned jobs, one systemic lifecycle failure (Theme 3).

6. Combined cross-cutting themes

Update/relaunch lifecycle unsafe end-to-end — helper.log relaunch, early health ack, exec-before-hash, no-launchable-exe windows, dishonest rollback, secondary-lock gap, unauthenticated legacy bootstrap, signing-key separation (B-4), registry-bytes provenance, upstream auto-promotion.
Destructive operations without ownership or transaction — uninstall sweep + elevated mutable .bat (B-1), wrong-target TUI operations, config-tree divergence (B-2), fail-open config reset/restore, FFmpeg backup deletion, language-apply rename, defaults-on-read-failure (B-3), date-colliding backups.
Worker lifecycle not owned by cancellors — Web cancel-then-accept, TUI Escape/Ctrl+C, shutdown abandoning jobs/SSE/children.
Authoritative state can lie — persistence-failure false success, setter non-rollback, partial multi-field writes, detection collapsing uncertainty, CLI --path recording fiction.
Coordination is process-local — method-ID job slots, per-Store history, path locks, migration CAS.
Release trust boundary too broad — tag-built generator with the signing key, mutable CI images, tag grammar after publication, unenforced metadata/native gates, checksum-file incompatibility.
Quote-unaware parsers (×3) — mpv.conf, input.conf, script-opts all split on # without quote tracking.

7. Unified remediation roadmap (merged)

P0 — release blockers

Terminal-preserving relaunch + post-first-render health acknowledgement + PTY qualification (C-1/H-15, H-3).
Windows destructive/privileged wrong-target fixes: ownership manifests (H-02/H-6), authenticated fixed helper instead of elevated mutable .bat (H-01/B-1), app-identity-carrying TUI operations (H-03/H-14), one installed-app-aware config resolver (H-04/B-2).
Updater integrity: hash-before-exec, platform-atomic never-unlinked replacement, honest rollback on missing backups, per-target locks (H-1/H-2/H-4, M-02/B-5).
Fail-closed transactional config reset/restore with unique fsynced backups (H-8, H-10/H-20, H-16/H-17), defaults-only-on-os.IsNotExist (H-09/B-3).
FFmpeg staged replacement (H-16/H-7); IINA mount-bound copy (H-12/H-9).

P1 — authoritative state and lifecycle

Worker-acknowledged cancellation; root lifecycle contexts; SSE-aware graceful shutdown (H-05/H-11, M-09/H-18, M-07/S-M1).
Resource-keyed operation leases covering config/UI trees and direct APIs (H-06/I-M5).
Persistence as part of the terminal result; setter rollback via one primitive (H-07/T-M2, H-08/P-M2).
Package-probe timeout fix + blocked-probe test (H-11/H-10, SA4011).
Release trust boundary: sign pipeline-local artifacts, isolated minimal signer/KMS separate from tagged code (M-20/H-21, H-13/B-4), pinned upstream digests + approval boundary (H-14/U-M12), protected semantic tags + duplicate preflight + serialized publication, pinned image digests.
TUI stream protocol, filter-state guards, interactive-sudo suspension (H-13, H-17, H-19).

P2 — reliability, portability, compliance

Native extraction policy for ZIP/tar; crash-durable installer transactions (M-01/I-M6, I-M7).
Cross-process locks for history/config; migration CAS (M-06, M-10, M-15).
Frontend contracts: ensurePassword single-owner, SSE reconnect reconciliation, latest-intent saves, $nextTick modal focus, reduced-motion dialogs (M-22–M-24, M-11, M-17).
b3sum -c-compatible checksums + tests; license notices in archives; Windows resources for both arches; tag grammar before publication; generator download bounds (M-21, M-19, M-12).
Discovery without executing untrusted binaries (M-04/I-M1); MPC-QT wait/verify (I-M3).

P3 — quality and cleanup

Browser E2E suite in CI (both reviews' top test gap); fault-injection tests at commit boundaries; language-workflow tests; coverage floors for Web (28.5%) / TUI (26.0%) / cmd (11.9%).
All Low/Info items; docs reconciliation; Staticcheck SA* in CI.

8. Combined strengths (union)

Signed manifest fails closed on empty trust, invalid signatures, malformed schema, missing/tampered size-hash, wrong staged identity (both reviews).
Native 7z extraction: count/size/path/device/symlink/case-collision/exclusive-create defenses, os.Root writes.
Web auth: constant-time token compare, loopback Host enforcement, Origin checks, request limits, keyring throttling — sound in review and live smoke tests.
Job snapshots copied under locks; relay goroutines drained/joined; race suite found no shared-pointer race.
ModernZ/uOSC/Hotkeys production writers reparse under a process-wide path lock and atomically replace.
Keyring: native OS stores, bounded probes, no weak fallback; passwords via stdin only.
No confirmed DOM XSS; escaping discipline held; no CDN runtime dependency; vendor byte-for-byte freshness.
Full dynamic suite green: tests, race, shuffle, vet, gofmt, tidy, govulncheck, npm audit, six cross-builds.

9. Review limitations

Review B ran on Linux; Windows/macOS outputs cross-built and inspected, not executed natively. Review A's sub-agents were static-only; the A lead ran vet/tests only. This session's verification was source-reading plus vet/tests — no native destructive flows.
Neither review performed real install/uninstall, production signing/publication, or privileged package-manager flows.
Tier C findings are single-sourced (though lead-verified); reconfirm individual line numbers during remediation.
Static review can miss environment-dependent behavior; both reviews limited findings to evidence-backed reachable paths.

Generated 2026-08-31 by glm-5.3. Cross-references docs/CODEBASE_REVIEW_2026-08-31_glm-5.3.md (Review A) and docs/CODEBASE_REVIEW_2026-08-31.md (Review B) at baseline bdfb31b, with independent source re-verification of all single-sourced Review B claims, go vet/go test re-run (clean), and severity-dispute resolution.
Markdown companion: docs/CODEBASE_REVIEW_COMBINED_2026-08-31-glm-5.3.md · Per-area artifacts: .opencode/reviews/*.md