# MPV.Rocks Installer — Deep Codebase Review & Audit (glm-5.3)

- **Date:** 2026-08-31
- **Model:** glm-5.3 (max effort, 8 parallel reviewer sub-agents + lead verification)
- **Baseline commit:** `bdfb31b` (`docs: record v1.3 native qualification`)
- **Scope:** all 247 Go files (~90k lines), `cmd/`, `pkg/`, `internal/`, web frontend (templates + ~8.5k lines JS), build/CI/release tooling, and docs claims
- **Method:** eight parallel deep reviews (web/security, installer, self-update stack, TUI, core utilities, frontend, supporting packages + main, build/CI/deps/docs), followed by independent lead spot-verification of the highest-severity findings and a full dynamic validation run
- **Companion artifacts:** full per-area reviews in `.opencode/reviews/*.md` (8 files, ~1,750 lines)

---

## 1. Executive summary

The codebase is in notably good shape for its size: all Go tests pass, `go vet ./...` is clean, the web auth/token design is solid, the signed-manifest updater core uses standard crypto correctly, and the 7z extractor is genuinely hardened. However, this audit found **154 findings (1 Critical, 26 High, 73 Medium, 49 Low, 5 Info)** concentrated in four systemic weaknesses:

1. **Update/relaunch lifecycle is not actually safe end-to-end.** The relaunched TUI renders into `helper.log` instead of the terminal (Critical), health is acknowledged before the TUI initializes, the swap protocol has crash windows with no launchable executable, and integrity checks run *after* executing staged files.
2. **Destructive file operations are not transactional or ownership-scoped.** Windows uninstall recursively deletes a user-chosen (possibly shared) directory; config reset/restore and FFmpeg rollback can destroy the only known-good copy on ordinary I/O failures.
3. **Cancellation and shutdown don't own their workers.** Web job cancellation releases the method slot before the worker stops; TUI Escape/Ctrl+C abandon goroutines and child package-manager processes; Web/TUI shutdown can orphan destructive jobs mid-transaction.
4. **Release provenance has gaps.** The signed manifest is computed from re-downloaded registry bytes rather than pipeline build artifacts; release immutability/authorization depends on unverified external GitLab settings; `BLAKE3SUMS.txt` is incompatible with the documented `b3sum -c` command.

Several findings recur across 2–4 areas (config restore, sudo timestamp validation, job-history cross-process safety); these are consolidated in §6.

### Severity totals (as reported per area)

| Area | Critical | High | Medium | Low | Info |
|---|---:|---:|---:|---:|---:|
| Web server/API/security (`pkg/web`) | 0 | 2 | 5 | 6 | 1 |
| Installer (`pkg/installer`) | 0 | 4 | 8 | 4 | 1 |
| Self-update / release stack (`pkg/version`, `pkg/releasemanifest`, release cmds) | 0 | 5 | 12 | 10 | 1 |
| TUI (`pkg/tui`) | 1 | 9 | 15 | 6 | 1 |
| Core utilities (`pkg/platform`, `config`, `keyring`, `hotkeys`, `locale`, `log`, …) | 0 | 1 | 9 | 5 | 0 |
| Web frontend (`internal/webassets`) | 0 | 0 | 6 | 4 | 0 |
| Supporting packages + main (`internal/scriptopts`, `fileops`, `modernzconf`, `uoscconf`, `cmd/mpv-manager`) | 0 | 2 | 10 | 11 | 0 |
| Build / CI / dependencies / docs | 0 | 3 | 8 | 3 | 1 |
| **Total** | **1** | **26** | **73** | **49** | **5** |

### Dynamic validation performed by the lead (unlike sub-agent static-only reviews)

- `go vet ./...` — **clean** (exit 0)
- `go test ./...` — **all packages pass** (exit 0; `pkg/version` 18.1s, `pkg/web` 3.9s)
- Independent source verification of the Critical finding and the top High findings (see §3.1 markers)

The passing suite means every finding below is a **latent** logic/security/lifecycle defect not currently caught by tests — the test-gap pattern is itself a documented finding (§7).

---

## 2. Critical

### C-1 — Relaunched self-updated TUI renders into `helper.log`, stealing terminal input

**Refs:** `pkg/version/transaction.go:349-358` (helper `Stdout/Stderr = helperLog`), `pkg/version/transaction.go:466-475` (relaunched child inherits helper's `os.Stdout/os.Stderr`), `pkg/tui/models_update.go:876-896`, `cmd/mpv-manager/main.go:209-276`. **Lead-verified in source.**

The TUI hands off via `LaunchUpdateHelper("tui")`; the helper's stdout/stderr are redirected to `helper.log`, and the updated binary is launched with those same descriptors while stdin remains the terminal. The relaunched Bubble Tea program puts the terminal into raw mode and consumes keystrokes while all frames/escape sequences go invisibly to the log. To the user it looks like a return to the shell whose input is being eaten by a hidden TUI. The commit can already be permanent at this point (health acknowledged, stabilization elapsed).

**Fix:** preserve/reopen the controlling terminal output handles for the relaunched TUI (Windows-safe `CONOUT$` path included); keep helper diagnostics on a separate descriptor.

---

## 3. High findings

### 3.1 Self-update / release stack

**H-1. Exec before hash recheck (TOCTOU at target-adjacent staging).** `pkg/version/transaction.go:287-293, 512-519, 533-537`; `pkg/version/version.go:904-932`. The helper executes `target.StagedPath` and later the installed `target.Path` for identity validation *before* `verifyUpdateArtifact`/post-swap BLAKE3 checks. Anyone able to replace a target-adjacent file during the parent-exit window gets code execution at helper privilege; the mismatch is only detected afterward. Can cross a privilege boundary when an elevated primary update includes a user-writable secondary. *Fix:* size+BLAKE3 before every exec; protected staging (open handle / verified inode) and unsafe-directory rejection.

**H-2. Apply and rollback both create a crash window with no launchable primary executable.** `pkg/version/transaction.go:521-526, 575-597`; `cmd/mpv-manager/main.go:154-160`. Apply renames the installed executable to backup, *then* renames the stage into place; a kill between the two leaves the primary pathname absent, and the advertised startup recovery can't run because there is nothing to launch. Rollback repeats the pattern (remove replacement, then rename backup). *Fix:* durably sync the backup without removing the live pathname; use platform-atomic replacement (`rename` over existing path on Unix, `ReplaceFile` on Windows).

**H-3. Health acknowledged before the relaunched process is actually ready.** `cmd/mpv-manager/main.go:113-125, 154-184, 209-276`; `pkg/version/transaction.go:349-358, 466-488, 556-567, 620-657`. The acknowledgement is written after arg parsing — before platform detection, release fetching (up to 3 manifest attempts), model construction, or `tea.NewProgram(...).Run()`. Combined with only a 1.5 s stabilization window, the helper can permanently commit an update whose child is still blocked in startup networking or about to fail during TUI init. The qualifier cannot catch this (its fixture only acknowledges and sleeps). *Fix:* acknowledge from a real post-first-render readiness event; failures before that trigger rollback. *(Also flagged High by the supporting-packages and TUI reviews — consolidated here.)*

**H-4. Missing backups treated as successful rollback.** `pkg/version/transaction.go:546-572, 575-597, 939-946`. `rollbackUpdateTargets` silently continues when the backup stat reports not-exist, including for applied targets. AV/disk/cleanup removing a backup after apply ⇒ rollback "succeeds", journal says `rolled_back`, helper relaunches the rejected binary; recovery can claim success while the primary is absent. *Fix:* missing backup for an applied target = rollback failure; verify restored bytes/identity before recording state.

**H-5. v1.1/v1.2 legacy bootstrap remains unauthenticated.** `pkg/releasemanifest/manifest.go:62-64, 127-139`; `cmd/generate-info/main.go:1234-1258`; historical `ffad438^:pkg/version/release.go`. The manifest deliberately emits legacy URL/hash fields for old clients, but the historical v1.2 decoder performs no signature verification and trusts the BLAKE3 from the same document — an attacker controlling the manifest response supplies both binary and matching hash. *Fix:* stop describing legacy bootstrap as signed; use pinned detached signatures or manual replacement; retire the automatic path after migration.

### 3.2 Installer

**H-6. Windows uninstall recursively deletes an arbitrary writable custom directory and reports success despite deletion failures.** `pkg/installer/windows.go:103-157`; `pkg/config/validation.go:102-168`; `pkg/web/api_settings.go:50-101`. **Lead-verified in source.** `ValidateCustomInstallPath` accepts any existing absolute writable non-root non-`%WINDIR%` directory; uninstall then enumerates and `RemoveAll`s every top-level entry except `portable_config`/`updater`/manager files. Choosing e.g. `C:\Users\name\Downloads` as install destination makes uninstall erase unrelated files; per-entry errors are warnings and the function still returns success. *Fix:* ownership manifest at install time; uninstall only manifest-owned paths; fail if cleanup incomplete.

**H-7. FFmpeg rollback deletes the only known-good binary when the restore rename fails.** `pkg/installer/installer.go:525-614`; `pkg/installer/real.go:65-72`. Active exe is renamed to `ffmpeg.old`, replacement copied directly (not staged+atomic); on copy failure rollback removes the destination and renames the backup back — but a deferred cleanup unconditionally deletes `ffmpeg.old`, including when that rename failed (AV/sharing race on Windows). *Fix:* retain backup until restoration positively confirmed; use destination-adjacent staging + atomic replace.

**H-8. Destructive config reset proceeds after preservation/backup failures (fail-open) and writes non-atomically.** `pkg/installer/installer.go:1060-1107`; `pkg/config_preservation.go:115-234`; `pkg/installer/windows.go:720-762`; `pkg/installer/common.go:131-158`. `InstallMPVConfigWithOutput` logs-and-continues on both `PreserveConfigsWithOutput` and `BackupExistingConfig` failures, then `os.WriteFile`s over `mpv.conf`; failure reapplying saved settings is downgraded to a warning after commit. Tests encode the continuation. *Fix:* fail closed before replacement; staged+fsynced atomic replacement; failure rollback.

**H-9. IINA install validates one DMG but copies from a hard-coded mount that may be another image.** `pkg/installer/macos.go:422-467, 199-230, 128-131`. `hdiutil attach` output is ignored; the code assumes `/Volumes/IINA/IINA.app`. A pre-existing `/Volumes/IINA` makes macOS mount the verified image as `/Volumes/IINA 1`, so the installer validates and copies the *wrong* volume's app (possibly attacker-prepared), then ad-hoc signs and launches it. Cleanup detaches the hard-coded path, not the attached device. *Fix:* attach with `-plist -nobrowse`, parse device/mount point, validate/copy/detach that exact mount.

### 3.3 Web server/API

**H-10. Package-version probe timeout is ineffective and holds the cache write lock.** `pkg/web/server_version_cache.go:183-225, 263-326 (esp. 313), 331-365`; `pkg/web/package_version.go:273-829`. **Lead-verified in source.** The timeout branch's unlabeled `break` exits only the `select`, not the collection loop — with two probes still blocked, the next iteration waits on `resultChan` forever; probes use `exec.Command` (not `CommandContext`) so the timeout can't kill a stuck package manager. Runs synchronously during `NewServer` startup and under `versionCacheMux` write lock on refresh — a hung package manager blocks startup or all dashboard/API readers indefinitely. *Fix:* bounded context per probe, labeled timeout exit, build replacement cache outside the lock.

**H-11. Cancelling a job permits a conflicting destructive job before the worker stops.** `pkg/web/jobs.go:256-278, 482-541, 591-608`; `pkg/web/api_install.go:339-393, 425-540`; `pkg/web/api_adopt.go:175-248, 379-476`. `CancelJob` archives/removes the job immediately; `CreateJobIfNoneActive` then accepts another operation for the same method while the original goroutine still executes (worker progress/error calls no-op; several workers have final config writes after their last cancellation check). Cancel-and-retry can overlap install/uninstall/adopt transactions. *Fix:* explicit `cancelling` state; keep job active until worker acknowledges termination; final context check before every commit.

### 3.4 TUI

**H-12. Escape abandons live work without cancellation; manager-update abort path unreachable.** `pkg/tui/models_update.go:227-239, 870-905, 1040-1067`; `pkg/tui/models_messages.go:20-64`. The global Escape branch resets `StateInstalling` models before the state-specific branch that calls `PreparedSelfUpdate.Abort` can run; workers use `context.Background` and keep running; stale completions can be attributed to a new selection; an escaped prepared self-update retains its lock/transaction until process exit. *Fix:* single operation state machine that cancels, drains, aborts, and only then returns to menu.

**H-13. Multi-channel stream can randomly lose completion or trailing output.** `pkg/tui/models_messages.go:324-375`; `pkg/tui/models_update.go:480-511, 513-675, 677-678`. Worker sends buffered `installDone` then closes `outputChan`; once both are ready `startStreaming` selects nondeterministically — a closed-output win leaves the UI stuck "installing" forever (the `tickMsg` handler doesn't re-arm); a completion win silently drops remaining output. *Fix:* one ordered event channel (closed only after a final result), or receiver tracks closed channels until drained + exactly one terminal result.

**H-14. Install/update/uninstall destinations are stale or discarded.** `pkg/tui/models_update.go:171-187, 438-446`; `pkg/tui/models_types.go:141-148`; `pkg/tui/install_path.go:90-103`; contrast `pkg/web/operation_state.go:19-28`. `executeInstallCmd` accepts/logs `installDir` but never applies it (changing the custom path mid-session installs to the startup path while recording the new one); `updateItem` drops backend `AppID`/`InstallPath`; uninstall uses the global destination — on Windows this can update/delete a *different* MPV installation than selected. Web correctly clones per-operation. *Fix:* carry stable app ID + path; `CloneForOperation` with immutable snapshot for every operation.

**H-15. "Change MPV UI" performs a full reinstall and records a duplicate installation.** `pkg/tui/models_update.go:1148-1170, 1724-1740, 606-612`; contrast `pkg/web/api_adopt.go:182-249`. Selects the first MPV app silently, runs the full install path with `isUpdate=false`, then `saveInstalledApp` can add a duplicate record instead of updating the selected app's UI type. *Fix:* dedicated TUI UI-change operation calling `InstallUISafely` against the selected app's config dir.

**H-16. Applying language preferences can replace the entire active `mpv.conf`.** `pkg/tui/language_preferences.go:828-859`; `pkg/config/editor.go:144-176`. `backupMPVConfig` renames the active config away before `SetConfigValue`, which then sees no file and writes fresh embedded defaults — unrelated user settings vanish; a same-day second apply can overwrite the date-named backup with the generated config, destroying the original. Apply returns no error to the state machine, so failures still show "preferences saved". *Fix:* unique high-resolution copy backup without removing the source; surface errors before success.

**H-17. Accepting a list filter also activates the selected action, including destructive ones.** `pkg/tui/models_update.go:41-224, 1090-1106, 1179-1196, 1280-1311`; guards exist in `modernz_options.go:210-224`/`uosc_options.go:186-200` but not the main lists. Enter intended to accept a Bubbles filter can immediately start an install/uninstall; some handlers pass Enter to `list.Update` (changing filter state) *and* process it as an action; async filter matching can act on a stale item. *Fix:* centralize list key routing; never dispatch application Enter/Escape while a filter is being set.

**H-18. Ctrl+C is presented as cancellation but quits without cancelling or joining work.** `pkg/tui/models_update.go:25-31`; `pkg/tui/models_views.go:690-694`; `pkg/tui/models_messages.go:33-64`. The UI says Ctrl+C cancels; the handler clears a few fields and `tea.Quit`s — no context cancel, no worker join; child package-manager processes can outlive the TUI; a prepared manager transaction is not aborted. *(Consolidated with the supporting-packages finding on Web/TUI shutdown abandoning jobs — same lifecycle root cause, see §6 theme L.)*

**H-19. Interactive sudo competes with Bubble Tea for the raw terminal.** `pkg/tui/models_update.go:870-909`; `pkg/installer/command_runner.go:64-93, 169-203`. Without a keyring password, `RunSudoCommand` falls back to interactive sudo while Bubble Tea's input reader stays active in raw/alternate-screen mode — missing/split password input, broken masking, or a hung install; exactly the first-Linux-install case. *Fix:* pre-authenticate via a secure TUI flow or use Bubble Tea's exec/suspend so the child exclusively owns a restored terminal.

### 3.5 Core utilities

**H-20. Config restore can leave `mpv.conf` missing/partial, and same-day backups collide.** `pkg/installer/installer.go:1176-1189`; `pkg/installer/common.go:161-194`; `pkg/installer/real.go:65-72`; `pkg/constants/constants.go:89-92`. Restore renames the current config away before validating/reading the source, with no rollback on any failure; the copy is `ReadFile`+non-atomic `WriteFile`; safety backups are date-precision and can be overwritten by a second same-day operation. *Fix:* one restore transaction under the path lock — validate, unique fsynced snapshot, atomic replace, restore snapshot on every failure.

### 3.6 Build / CI / release

**H-21. Signed manifest is not cryptographically bound to the pipeline's build artifacts.** `.gitlab-ci.yml:429-468, 477-506`; `cmd/generate-info/main.go:948-977, 1234-1267`. The manifest job receives only generator artifacts; it reconstructs registry URLs, re-downloads the bytes, and signs those. A duplicate upload, registry writer, or concurrent publication that changes the registry object before signing makes the protected signer authenticate bytes this pipeline never built (GitLab allows duplicate generic-package files by default; Developer can publish). *Fix:* pass raw build artifacts to the manifest job, sign those local bytes, upload once, then byte/hash-compare published objects against local provenance before signing.

**H-22. Release immutability/authorization/ordering depend on unverified external settings.** `.gitlab-ci.yml:191-192, 432-433, 480-481, 556-557`; `docs/GITLAB_CI.md:60-68, 81-105`. Every tag (not just protected semantic `v\d+\.\d+\.\d+`) matches release rules; no duplicate-file preflight or `resource_group`; concurrent/retried tag pipelines can republish a tag or race the stable-channel PUT; docs assume protected variables but don't require a protected `v*` tag rule. *Fix:* strict tag grammar + `CI_COMMIT_REF_PROTECTED`; fail if a tag file already exists; serialize stable publication.

**H-23. Final release has no enforced native-signing/native-execution gate.** `.gitlab-ci.yml:551-560`; `docs/GITLAB_CI.md:115-121`; `docs/SELF_UPDATE_AND_WAILS_REVIEW_2026-08-28.md:36-51`. Release needs only `publish:release-manifest`; Linux cross-compilation can publish Windows/macOS artifacts with no machine-verifiable evidence that Authenticode/notarization/native smoke tests passed (acknowledged v1.3 publication blocker, restated here for completeness). *Fix:* gate release creation on native jobs that sign/verify/smoke-test the exact artifacts, or a protected manual approval recording immutable qualification evidence.

---

## 4. Medium findings (73, condensed)

### Web server/API (`pkg/web`)
| ID | Finding | Refs |
|---|---|---|
| W-M1 | Cached sudo credentials let any submitted password "pass" validation (`sudo -S -v` without `-k`) | `api_keyring.go:84-121`; `installer/command_runner.go:194-229` |
| W-M2 | Backup path validation follows intermediate symlinks outside the backup root | `config/validation.go:17-55, 93-99`; `api_config.go:586-635` |
| W-M3 | Failed restore can leave active `mpv.conf` missing (see H-20) | `api_config.go:586-608` |
| W-M4 | Global locale cache is not concurrency-safe (unsynchronized lazy load; shared mutable backing slice sorted in place) | `locale.go:13-33, 57-75, 111-123, 305-313` |
| W-M5 | Multi-setting API writes validated together but committed independently (up to 22 writes; partial apply on mid-failure) | `api_config.go:113-181, 183-466`; `api_languages.go:28-120` |

### Installer (`pkg/installer`)
| ID | Finding | Refs |
|---|---|---|
| I-M1 | Windows discovery executes untrusted candidate binaries during a read-only scan (no context/timeout; PATH/registry-supplied paths) | `windows_detection.go:69-105, 148-164` |
| I-M2 | Detection treats uncertain probes as "not installed" and syncs records away; identity keyed by method collapses multiple installs; unbounded locale-varying probes | `detection.go:167-186, 230-382, 388-465`; `config/config.go:711-744` |
| I-M3 | MPC-QT install uses `Start-Process` without `-Wait -PassThru`; returns before installer completes and ignores its exit | `windows_mpcqt.go:516-572` |
| I-M4 | Restore moves the active config away before validating/staging the replacement (dup of H-20) | `installer.go:1165-1190` |
| I-M5 | No destination-scoped exclusion between installer operations — different methods can concurrently mutate shared config/UI paths | `installer.go:72-120`; `common.go:367-390` |
| I-M6 | ZIP/tar extraction lacks the 7z preflight policy (no entry-count/expanded-size/symlink policy; direct extraction into live destinations) | `installer_archive.go:15-68`; `real.go:191-253`; `installer.go:681-767` |
| I-M7 | Runtime rollback transactions are not crash-durable (in-memory state only; `.txn-*`/`.bak-*` debris not recovered at startup) | `file_transaction.go:23-205`; `ui_config_update.go:269-488` |
| I-M8 | Cached sudo timestamp issue (dup of W-M1) | `command_runner.go:169-229` |

### Self-update / release stack
| ID | Finding | Refs |
|---|---|---|
| U-M1 | Target-directory metadata not durably ordered with journal (no dir fsync after renames; Windows dir sync a no-op; recovery doesn't revalidate committed targets) | `transaction.go:491-540, 803-870, 925-946` |
| U-M2 | Crash before first journal write permanently blocks future updates (orphan `.mpv-manager-update-*` never cleaned; recovery stops at first bad dir) | `transaction.go:225-318, 905-916` |
| U-M3 | `PrepareSelfUpdateFromCheck` accepts a forgeable mutable DTO (any non-empty `ManifestKeyID` = "authenticated"; HTTP URLs/synthetic versions accepted — qualifier proves it) | `version.go:84-99, 174-210`; `transaction.go:175-203` |
| U-M4 | No freshness/anti-replay on signed metadata (replay older-higher release or suppress security updates) | `manifest.go:65-71, 229-245` |
| U-M5 | Static single-key authorization; no revocation/threshold/validity epochs (compromised retiring key keeps signing for embedded clients) | `manifest.go:54-71, 191-224` |
| U-M6 | Stale/tampered `manager_bin_path` secondary can overwrite an unrelated regular file (no identity proof before replacement) | `config/config.go:692-713`; `transaction.go:151-168, 265-284` |
| U-M7 | Detached helper failures never reach the initiating UX (Web job completes before any replacement occurs) | `transaction.go:328-500`; `web/api.go:278-301` |
| U-M8 | Health-failure rollback races a still-running child on Windows (kill without wait; exe still locked) | `transaction.go:479-488, 575-597, 620-657` |
| U-M9 | Release-generator downloads: no whole-transfer deadline, unbounded body to disk, `Proxy` nil (bypasses `ProxyFromEnvironment`) | `generate-info/main.go:56-68, 546-634, 948-977` |
| U-M10 | Qualifier's hidden `--qualifier-driver` mode is destructive outside the controller (no capability token/containment) | `qualify-selfupdate/main.go:97-126, 299-381` |
| U-M11 | `BLAKE3SUMS.txt` not in `b3sum -c` format (`blake3:` prefix emitted; documented verify command unusable) | `gen-checksums/main.go:49-90`; `.gitlab-ci.yml:400-419, 609-623` |
| U-M12 | Auto publication promotes unreviewed upstream "latest" releases into signed trust (no upstream signature/allowlist check) | `generate-info/main.go:358-448, 1088-1094, 1202-1269` |

### TUI
| ID | Finding | Refs |
|---|---|---|
| T-M1 | App-update bookkeeping uses stale main-menu ID `"updates"`; successful updates stay recorded at old version and re-offered; MPC-QT selector unreachable | `models_update.go:43-48, 614-618, 999-1037` |
| T-M2 | Persistence failures ignored or printed straight to stdout (corrupts frames); success rendered anyway | `models_update.go:545-596, 977-989` |
| T-M3 | Restore removes active config before replacement known durable (dup of H-20) | `models_update.go:1588-1606` |
| T-M4 | Detached worker panics bypass Bubble Tea's terminal recovery (10 raw goroutines, no recover boundary) | `models_messages.go:180-309, 353-362` |
| T-M5 | Progress bar fed 0–100 values to `ViewAs`, which expects 0–1 (≥1% renders as 100%) | `models_update.go:491-503`; `models_views.go:572-586` |
| T-M6 | Installation output not keyboard-scrollable; mouse mode never enabled; auto-scroll never reset | `models_views.go:124-128`; `models_update.go:679-689` |
| T-M7 | Output rendering unbounded and quadratic (full transcript re-joined per line) | `models_init.go:345-348`; `job_history.go:70-78` |
| T-M8 | Resize handling omits many active lists; `width-10`/`height-15` without minimums | `models_update.go:360-394`; `models_init.go:350-368` |
| T-M9 | "Install to PATH" broken on Windows (copies without `.exe`, doesn't touch PATH); non-idempotent alias appends on Unix; failures reported as success | `models_messages.go:77-177, 180-309` |
| T-M10 | Shared job history not safe across TUI/Web processes (see §6 theme) | `job_history.go:91-98` |
| T-M11 | Ctrl+V overlay shows hard-coded `0.1.0` / Go `1.21` / `linux/amd64` regardless of build identity | `models_views.go:736-755` |
| T-M12 | Offline startup not offline-first: ~33 s of failed manifest fetches before first render; then another fetch; network actions not gated | `main.go:209-238`; `models_init.go:19-21` |
| T-M13 | Reset erases manager data after backup failure while claiming "backup saved"; wrong reset message re languages | `reset_data.go:33-47`; `config/config.go:1095-1143` |
| T-M14 | Unicode search backspace removes one byte, not one character (invalid UTF-8 afterwards) | `language_preferences.go:264-268, 391-395` |
| T-M15 | Raw subprocess/persisted output rendered without stripping terminal control sequences (CSI/OSC injection into the TUI) | `models_update.go:505-511`; `job_history.go:260-267` |

### Core utilities
| ID | Finding | Refs |
|---|---|---|
| P-M1 | Backup validation symlink/race escape (dup of W-M2) | `config/validation.go:33-55, 93-99` |
| P-M2 | Setters mutate in-memory state then can fail to persist without rollback (later successful save persists a mutation the user was told failed); `ResetToDefaults` ignores backup failure | `config/config.go:459-515, 716-787, 1059-1090` |
| P-M3 | `mpv.conf` editor truncates valid quoted values at `#`, edits first-not-effective duplicate | `config/editor.go:46-89, 193-213` |
| P-M4 | Linux GPU detection truncates models at first `(`, skips Vulkan/PCI enumeration when glxinfo returns anything, first-match codec fallback for hybrids, empty PCI-ID resolver stub | `platform/gpu.go:85-112, 564-623, 1107-1113, 1218-1220` |
| P-M5 | macOS AV1 over-reported: unknown models and probe failure fail open to AV1; M2 wrongly grouped with M3+ (AV1 decode starts at M3); release builds ship the CGO-off heuristic | `platform/gpu_darwin.go:22-64`; `Makefile:106-110` |
| P-M6 | Linux arm64 CPUs can be labeled `x86-64-v2` (`asimd` not recognized as NEON; no `runtime.GOARCH` seeding) | `platform/cpu.go:38-68, 118-177` |
| P-M7 | Job history cross-process safety (see §6 theme) | `jobhistory/jobhistory.go:64-157` |
| P-M8 | Hotkey round-trip corrupts quoted `#` arguments; validation accepts `#` as a key which serializes to a comment line | `hotkeys/inputconf.go:109-138, 188-220`; `validation.go:74-79` |
| P-M9 | Locale selection/data disagree: `hif`/`te`/`fil` declared common but unresolvable; Eastern Punjabi assigned Aruba/Curaçao; `es-419` flag wrong | `locale/selection.go:49-70, 107-111`; `assets/locales.json:3-22, 1447-1455, 3225-3240` |

### Web frontend
| ID | Finding | Refs |
|---|---|---|
| F-M1 | Stored-password installs dispatch the privileged operation twice (ensurePassword invokes callback *and* returns true; second POST 409s and corrupts button state) | `password-modal.js:398-416`; `install.js:287-315` |
| F-M2 | Lost SSE events never reconciled after reconnect (no event IDs/replay/snapshot handshake; stale active jobs/badges/buttons until refresh) | `jobs.js:27-76, 186-220, 272-344`; `sse.go:35-49` |
| F-M3 | Config apply marks concurrent edits as saved (baseline rebuilt from current controls, not the submitted payload; `beforeUnload` suppressed while saving) | `config-page.js:74-108` |
| F-M4 | UI option save/reset requests can overwrite newer intent (no generation/abort per key; stale responses restore old values after reset) | `ui-settings.js:424-475, 483-497, 523-590` |
| F-M5 | Regional-language cancellation loses the current AbortController; out-of-order responses render the wrong language's variants | `languages.js:424-439, 489-515, 551-585, 875-887` |
| F-M6 | Alpine job modal opens its focus controller while still `hidden` (focus targets rejected as invisible; initial focus left on inert invoker) | `base.html:32-45`; `jobs-modal.js:49-78`; `dialog.js:27-60` |

### Supporting packages / main
| ID | Finding | Refs |
|---|---|---|
| S-M1 | Persistent SSE handlers defeat signal-based graceful HTTP shutdown (2 s timeout → "shutdown failure", exit 1; API path may exit before Shutdown completes) | `main.go:345-387`; `server.go:229-254`; `sse.go:31-33` |
| S-M2 | Command parsing misroutes positionals/conflicting modes (`--verbose cli` starts Web mode; `web garbage` silently accepted; last-one-wins `-m`); internal update flags leak into usage | `main.go:56-99, 162-184` |
| S-M3 | `--verbose`/`--debug` don't produce the promised console logging (flags consulted only by `debugPrintf` during already-finished init) | `main.go:127-135`; `log/logger.go:48-196` |
| S-M4 | Accepted string values not round-trip safe (`controls="menu # compact"` reparses as `"menu` + comment; whitespace-trim asymmetry) | `scriptopts/options.go:29-75`; `scriptopts.go:232-280` |
| S-M5 | BOM misread as part of first key; edits produce mixed CRLF/LF files | `scriptopts.go:73-87, 213-219, 239-280` |
| S-M6 | Atomic replacement destroys symlinks and original file metadata (mode forced to 0644; dotfiles-managed `modernz.conf` link replaced) | `fileops.go:66-102`; `scriptopts.go:150-175` |
| S-M7 | Config locking doesn't cover stale public snapshots or multiple processes (exported `Write` parses before locking) | `fileops.go:20-64`; `scriptopts.go:73-87, 150-159` |
| S-M8 | Job history cross-process (dup of P-M7) | `jobhistory.go:64-157` |
| S-M9 | CLI `--path` ignored by uOSC/ModernZ/FFmpeg dispatch but recorded as `InstallPath` (state inconsistent with disk) | `main.go:534-663` |
| S-M10 | Browser launch and banner race the actual bind (port conflict → browser opens unrelated local service); `s.httpServer` assign/read race | `main.go:345-375`; `server.go:116-130, 227-235` |

### Build / CI / docs
| ID | Finding | Refs |
|---|---|---|
| B-M1 | Distributed archives omit `LICENSE` and third-party notices (BSD binary-redistribution requirement quoted in own notices doc) | `.gitlab-ci.yml:331-392`; `docs/THIRD_PARTY_NOTICES.md:16-23` |
| B-M2 | Windows resource generation stale/fail-open/incomplete for arm64 (`|| true`; committed `1.0.0.0`/Win7 metadata; no arm64 `.syso`) | `Makefile:64-67`; `winres/winres.json:13-43` |
| B-M3 | Standalone `make release-build` works without a trust ring → "release" binaries that reject all release metadata | `Makefile:51-54, 197-203, 226-227` |
| B-M4 | Release generator unbounded downloads (dup of U-M9) | `generate-info/main.go:56-67, 594-632` |
| B-M5 | No live-browser job/SSE E2E in CI (jsdom/fakes only; central feature untested end-to-end) | `vitest.config.mjs:3-7`; `docs/TESTING.md:129-155` |
| B-M6 | Release jobs execute mutable container-image tags (`golang:1.27.0`, `alpine:latest`, `release-cli:latest`) | `.gitlab-ci.yml:51-53, 331-333, 551-553` |
| B-M7 | Tag grammar validated only after public package publication begins (non-`v` tags upload then fail later) | `.gitlab-ci.yml:191-192, 334-335`; `generate-info/main.go:1288-1309` |
| B-M8 | `BLAKE3SUMS.txt` format (dup of U-M11; `cmd/gen-checksums` has no tests) | `gen-checksums/main.go:49-90` |

---

## 5. Low / Info findings (49 + 5, one-line index)

**Web:** route-level method restrictions incomplete (GET can trigger state sync on `check-updates`/`refresh-installed`/`settings/shortcut`) · `ui_type` unvalidated on install/update (invalid durable state) · backup listing panics if entry vanishes mid-enumeration · SSE clients/writes unbounded (no cap, no write deadline) · any version difference labeled "update" including downgrades · keyring failures return HTTP 200 with raw backend detail · dead `loggingMiddleware`/`handleUpdatesAPI`/duplicate locale layers (INFO).

**Installer:** OSC edits can create duplicate authoritative entries · uOSC temp config leaked on failed install · output capture splits on read chunks not lines (truncated/merged records) · generated shortcuts embed paths without shell/VBS-safe encoding (macOS wrapper vulnerable to `$`/backtick expansion).

**Updater:** unknown JSON fields outside signature yet verification succeeds (envelope-extension risk) · HTTPS not revalidated across redirects · PID-reuse confusion in handoff/recovery · signed asset strategy/CPU-baseline/`NativeSigning` fields not enforced · legacy manager fields can diverge from v2 assets · pathname-based advisory lock replaceable on Unix · identity probes unbounded output, no process-group kill · obsolete in-process updater retained as dead code (high-risk regression path) · qualification omits the failure boundaries that would invalidate its claims · per-attempt timeouts can extend one self-update to ~45 min.

**TUI:** error-screen Enter advertised but unhandled · screenshot inputs can create malformed quoted config; warning never clears (value receiver) · hotkey preset application re-entrant · package-info rows selectable with no action · `PostInstallMessage` normalized only for history, not control flow · resize-independent wrappers unscrollable/blank in tiny terminals · dead states/messages/duplicated branches (INFO).

**Core utilities:** country filter deliberately leaks regionless languages (test-locked bug) · active-UI detection can't represent ambiguous two-script installs · bursty log lines silently dropped (non-blocking send, empty default) · hotkey backups unbounded, no retention · static catalog IDs contradict descriptions (`volume-up` describes decrease, etc.) · keyring verified clean (OS stores only, stdin passwords, bounded probes).

**Frontend:** mobile drawer visually modal but not focus-contained · `formatKeys` builds `template.HTML` from unescaped fragments (trust-boundary hazard) · no CSP on a privileged local UI (inline scripts/handlers everywhere) · highest-risk orchestration modules (`jobs.js`, `install.js`, `password-modal.js`, …) have no behavioral tests — the Medium findings live exactly in those seams.

**Supporting/main:** temp-file sync not fully durable (no parent-dir fsync) · backups best-effort/permission-widening/unbounded · managed scaling preservation guesses implicit defaults · `uoscconf.KnownOptions` exposes mutable global slices (ModernZ deep-copies; divergence) · `os.Exit` bypasses deferred cleanup; cancellation exit codes inconsistent · signals handled only after slow startup work · production panics print full stacks unconditionally · `--help`/`--version --json` initialize the file logger (stateful identity probes) · browser helper never reaped (zombie) · Windows resources hardcoded `1.0.0.0` · public snapshot writes have no stale-version detection.

**Build/CI:** coverage regex matches per-package lines, not the aggregate · `make lint` installs `golangci-lint@latest`, not mirrored in CI · docs contradictions (macOS "universal" claim, Windows arm64 "future", obsolete troubleshooting URLs/targets, stale AGENTS.md line-count table) · dependencies otherwise current and exactly pinned; Alpine 3.17.0 available (no security requirement found — INFO).

---

## 6. Cross-cutting themes (deduplicated)

These single root causes account for findings reported by multiple reviewers:

1. **Config restore/reset is not transactional** (H-8, H-20, W-M3, I-M4, T-M3): every path that replaces `mpv.conf` moves the original away before the replacement is validated/durable, with fail-open backups and date-collision names.
2. **Worker lifecycle isn't owned by cancellors** (H-11, H-12, H-18, S-M1): Web cancel, TUI Escape/Ctrl+C, and process shutdown all return before the goroutines/subprocesses they ostensibly cancel have stopped — enabling overlapping destructive operations and orphaned children.
3. **Sudo timestamp validation is fake when cached** (W-M1, I-M8): `sudo -S -v` without `-k` accepts any password while credentials are cached; both duplicated code paths share the bug.
4. **Job history is per-instance-locked only** (P-M7, S-M8, T-M10): `Store` mutexes don't span processes; fixed `.tmp` name enables lost records/racing renames between Web and TUI.
5. **Health/relaunch readiness is declared too early** (C-1, H-3): acknowledgement precedes any real initialization, and the relaunched child inherits log-file descriptors — one fix (terminal preservation + post-render ack) addresses both.
6. **Release-tooling trust gaps** (H-21, H-22, U-M9/U-M12, B-M4, B-M6): signing re-downloaded bytes, mutable image tags, unbounded generator downloads, and unreviewed upstream auto-promotion all widen the distance between "what CI built" and "what the key authenticated".
7. **Quote-unaware line parsers** (P-M3, P-M8, S-M4): `mpv.conf`, `input.conf`, and script-opts each split on `#`/`" #"` without tracking quotes — three independent implementations of the same parser bug.

---

## 7. Test-coverage pattern

All findings are latent (suite green, vet clean). The reviews consistently traced *why*: the highest-risk seams are precisely the untested ones — stored-password double dispatch, SSE reconnect reconciliation, cancel/retry overlap, stream channel races, terminal descriptor inheritance, quoted-`#` round-trips, `b3sum -c` compatibility (`cmd/gen-checksums` has no tests), cross-process history, and any browser-level E2E for jobs. Several existing tests *encode the defective behavior* (backup-failure continuation, glxinfo truncation, `#`-key acceptance, regionless country-filter leak), so fixes will need expectation changes, not just new tests.

---

## 8. Verified-clean strengths

- **Auth:** 32-byte `crypto/rand` token, constant-time compare, `HttpOnly`/`SameSite=Strict` cookie, Host allowlist + exact Origin match, OPTIONS rejected, loopback-only listener without DNS resolution, 1 MiB body cap, header-count/timeouts set (`middleware.go`, `listen_address.go`, `request_limits.go`). No bypass found.
- **Injection surface:** method IDs/UI types allowlisted; subprocesses use argument vectors (no shell interpolation); PowerShell archive commands single-quote with doubling; log HTML escaped before HTMX swaps; no Web-controlled SSRF path.
- **7z extraction:** genuine preflight (traversal/absolute/ADS/reserved names, symlinks/special files, duplicate + case-colliding paths, ≤4096 entries, ≤2 GiB expanded) writing through `os.Root`.
- **Manifest core:** standard Ed25519, fail-closed on empty/unknown/malformed trust; deterministic canonicalization for this schema; signed sizes/hashes/identities; verify-before-select on the normal fetch path; 512 MiB streamed cap; `O_EXCL` destination-adjacent staging; flock/`LockFileEx` correctly serialize cooperating processes.
- **Keyring:** OS credential stores only (Secret Service/Keychain/CredMgr), stdin password transport, no password in argv/env/logs, bounded probes.
- **Frontend:** `html/template` escaping + `textContent`/`x-text`/`escapeHtml` discipline held everywhere reviewed; no `eval`/`new Function`/`document.write`; vendor sync byte-for-byte with CI freshness checks; dialog focus traps tested.
- **Repo hygiene:** no secrets/keys/env files committed; embed inventory contains only intended assets; ignored stale logs correctly not treated as evidence.

---

## 9. Recommended remediation roadmap

**P0 — before any further release work (blocks v1.3+ claims)**
1. C-1 + H-3: terminal-preserving relaunch + post-render health acknowledgement, with a PTY integration test (render, ack-after-first-frame, rollback on pre-frame failure).
2. H-1, H-2, H-4: hash-before-exec at every identity probe; platform-atomic replacement that never unlinks the live primary; honest rollback when backup evidence is missing.
3. H-6: manifest-owned Windows uninstall; refuse unmanaged directories; fail on incomplete cleanup.
4. H-8, H-20 (+ theme 1): fail-closed, staged, atomic config reset/restore with unique fsynced backups.

**P1 — correctness of core flows**
5. Theme 2 (H-11, H-12, H-18, S-M1): worker-acknowledged cancellation; root lifecycle context + bounded wait on shutdown for Web and TUI.
6. H-10: context-bounded package probes; labeled timeout; lock-free cache rebuild.
7. H-13, H-14, H-15, H-16, H-17: TUI stream protocol, per-app destinations, dedicated UI-change op, backup-without-removal for language prefs, filter-state guards.
8. H-7, H-9: FFmpeg staged replacement; IINA mount-bound copy.
9. Theme 3 (`sudo -k`), theme 4 (cross-process history lock), locale cache race (W-M4).

**P2 — release/pipeline trust**
10. H-21, H-22, H-23: sign pipeline-local artifacts; protected-tag + duplicate-preflight + serialized publication; native gates or recorded approval.
11. U-M1..U-M6, U-M11/U-M12, B-M1..B-M8: durability barriers, forgeable-DTO removal, anti-replay/revocation design, `b3sum -c`-compatible checksums, notices in archives, pinned image digests, browser E2E.

**P3 — quality/consistency**
12. Themes 5–7 remainder, all Low/Info items, docs reconciliation (AGENTS.md line-count table is materially stale; several docs claims refuted — see the claims matrix in `.opencode/reviews/build-ci-dependency-docs-review.md`).

---

## 10. Per-area artifacts

| Area | Artifact | Counts |
|---|---|---|
| Web security/correctness | `.opencode/reviews/web-security-correctness-review.md` | 2H/5M/6L/1I |
| Installer | `.opencode/reviews/installer-deep-review.md` | 4H/8M/4L/1I |
| Self-update/release stack | `.opencode/reviews/self-update-release-version-stack.md` | 5H/12M/10L/1I |
| TUI | `.opencode/reviews/tui-deep-correctness.md` | 1C/9H/15M/6L/1I |
| Core utilities | `.opencode/reviews/core-utility-packages.md` | 1H/9M/5L |
| Web frontend | `.opencode/reviews/web-frontend-review.md` | 6M/4L |
| Supporting packages + main | `.opencode/reviews/supporting-packages-main-entrypoint.md` | 2H/10M/11L |
| Build/CI/deps/docs | `.opencode/reviews/build-ci-dependency-docs-review.md` | 3H/8M/3L/1I |

*Sub-agent reviews were static-only (their sandbox denied test execution). The lead independently re-verified the Critical and top High findings against source and ran `go vet ./...` and `go test ./...` (both clean) on the reviewed baseline.*