MPV.Rocks Installer — Combined, Cross-Referenced Codebase Review
Review date: 2026-08-31 Reviewer: GPT-5.6-sol, xhigh reasoning Reviewed baseline: bdfb31b95030c2d992e5cfc4cb0ba5bada368d1b (master) Inputs: GPT-5.6-sol review and GLM-5.3 review HTML companion: Standalone HTML report
Executive result
The two source reports agree on the codebase's strongest qualities and on most of its highest-risk weaknesses. After independently tracing claims back to the reviewed source, correcting stale paths, combining repeated root causes, checking external-format/platform assertions against primary documentation, and normalizing severity, this review retains 101 actionable entries or tightly related defect clusters:
| Severity | Count | Interpretation |
|---|---|---|
| Critical | 0 | No issue met the defined threshold of direct exploitability or unrecoverable destruction without an additional intended trust/privilege decision. |
| High | 25 | Release-blocking destructive-data, wrong-target, privilege, supply-chain, availability, or authoritative-state defects. |
| Medium | 56 | Material correctness, recovery, concurrency, portability, accessibility, or defense-in-depth defects. |
| Low / hardening | 20 | Limited-impact defects and consolidated maintenance, test, documentation, or hardening themes. |
These counts must not be obtained by adding the source totals. The first report contains 50 individually deduplicated findings. The GLM report's 154 total is explicitly a sum of per-area reviewer labels and repeats the same root cause across packages; its own cross-cutting section identifies several of those duplicates. This combined report counts a repeated root cause once, promotes or downgrades it only after source verification, and groups closely coupled low-impact observations.
The most urgent work is concentrated in five systems:
- Windows ownership and target identity: elevation of mutable install-tree code, broad recursive uninstall, selected-app identity loss, and split custom-config resolution.
- Updater replacement and relaunch: exec-before-recheck ordering, absent-primary crash windows, dishonest rollback on a missing backup, helper-log terminal inheritance, and readiness acknowledged before the TUI is ready.
- Configuration transactions: fail-open preservation, non-atomic reset/restore, leaked in-memory mutations after failed persistence, and read errors silently becoming defaults.
- Worker lifecycle: Web cancellation archives before workers stop; TUI Escape/Ctrl+C abandon active workers; multi-channel completion can be lost.
- Release provenance: tagged application code receives the signing key, unauthenticated upstream “latest” bytes become first-party trusted, and the signed manager manifest is derived from registry downloads rather than the pipeline's local build artifacts.
Reconciliation and verification method
The following disposition vocabulary is used throughout:
- Verified: the described control flow and impact are present in the reviewed source.
- Verified, severity changed: the defect is real, but the source report's severity did not match the common definitions.
- Conditional: the repository exposes a risk, but exploitability or enforcement depends on external state that is not present in this checkout.
- Historical limitation: accurate for immutable older clients, but not a current-code defect.
- Not upheld: the cited behavior exists but the claimed security/correctness impact does not follow.
Verification included direct inspection of every retained High finding and the source paths behind the Medium crosswalk, plus focused checks of claims on updater ordering, Windows file ownership, job state transitions, config persistence, TUI channel ordering, frontend async ownership, release jobs, checksum format, CPU/GPU heuristics, and cross-process storage. Stale source paths in the GLM report were corrected—for example, the MPC-QT code is in pkg/installer/windows.go, not a nonexistent windows_mpcqt.go.
External claims were checked against primary sources:
- The official
b3sumdocumentation defines--checkfor ordinary BLAKE3 check files; the repository'sblake3:prefix is not accepted check-file syntax. - GitLab documents that duplicate generic-package publication is allowed by default and can be disabled with the Allow duplicates setting. Because project settings are not in the repository, this is a conditional release-control gap, not proof that the deployed project permits overwrites. See GitLab generic package documentation.
- Apple's M2 specification lists hardware H.264, HEVC, and ProRes but not AV1, while the M3 specification explicitly lists AV1 decode. See Apple M2 specifications and Apple M3 specifications.
Validation rerun for this reconciliation
| Check | Result |
|---|---|
go test -count=1 ./... | Pass |
go test -race -count=1 ./pkg/version ./pkg/installer ./pkg/web ./pkg/tui ./pkg/config ./pkg/jobhistory ./pkg/platform ./pkg/hotkeys ./pkg/uiconfig | Pass |
go vet ./... | Pass |
npm test -- --run | Pass: 12 files, 157 tests |
| Direct source verification | All 25 High entries and all Medium crosswalk paths reviewed |
b3sum -c live compatibility | Not run: b3sum is not installed locally; verified against official syntax instead |
Passing tests do not invalidate the findings. Most concern crash boundaries, inter-process contention, native Windows/macOS behavior, async ordering, terminal descriptor ownership, or failure injection that the current suite does not exercise. This run did not repeat the first report's six cross-builds, govulncheck, full race suite, or live browser smoke tests; those remain results of that source review, not newly claimed reruns here.
High findings — normalized cross-reference
| ID | Combined finding | Source cross-reference | Verification and required correction |
|---|---|---|---|
| CH-01 | Windows elevates a mutable install-tree batch file. | GPT H-01 | Verified. pkg/installer/windows.go:160-212 copies mpv-unregister.bat from a selected/adopted install tree and invokes it with Start-Process -Verb RunAs without authenticating the file or ownership immediately before elevation. Replace it with fixed native cleanup or an embedded authenticated helper in an ACL-protected location; never elevate adopted install content. |
| CH-02 | Windows uninstall recursively removes unrelated contents from a user-selectable directory. | GPT H-02; GLM H-6 | Verified. pkg/installer/windows.go:103-157 deletes every direct child except a small preserve list; accepted custom/detected/adopted locations are not proven manager-owned, and deletion failures can still end in success. Require an ownership marker and per-install manifest, delete only owned entries, and fail or report partial completion accurately. |
| CH-03 | TUI update/uninstall loses the selected app identity and can target another installation. | GPT H-03; GLM H-14 | Verified. pkg/tui/models_types.go:141-148, models_update.go:438-446, and models_messages.go:40-64 discard or bypass AppID/InstallPath and use a globally bound installer. Carry immutable app identity/path and clone the installer per operation. |
| CH-04 | Windows custom installs and settings APIs resolve different portable_config trees. | GPT H-04 | Verified. The installer writes <custom>\portable_config, while config/hotkey/script-option/UI-discovery paths in pkg/constants/paths.go:47-79,306-319 resolve AppData/home locations. Introduce one installed-app-aware config resolver used by every editor, backup, restore, and migration path. |
| CH-05 | Web cancellation archives a terminal “cancelled” result before the worker reaches or rejects its commit. | GPT H-05; GLM H-11 | Verified. pkg/web/jobs.go:482-543 removes/archives immediately; workers can still install, uninstall, adopt, or write final configuration. Cancellation must remain an active cancelling state until the worker acknowledges termination and owns the sole terminal transition. |
| CH-06 | Job exclusion is method-based rather than resource-based. | GPT H-06; GLM I-M5 | Verified. pkg/web/jobs.go:256-279 allows different method IDs and direct endpoints to mutate the same install/config/UI/package-manager resources. Add deterministic multi-resource leases covering jobs and direct mutation APIs. |
| CH-07 | Operations report success when authoritative tracking persistence fails. | GPT H-07; GLM T-M2 | Verified. Web and TUI physical operations warn or log after failed installed-app/config persistence and still produce success. Make persistence part of completion or expose a durable partial-success/reconciliation state. |
| CH-08 | Failed manager-config writes leak rejected mutations into live process state. | GPT H-08; GLM P-M2 | Verified. Multiple setters in pkg/config/config.go mutate globalConfig before saveLocked and do not restore it on failure, so a later unrelated save can persist a change previously reported as failed. Mutate a clone and publish only after successful atomic persistence. |
| CH-09 | Manager-config read errors are treated as absence and can be replaced with defaults. | GPT H-09 | Verified. pkg/config/config.go:85-149 defaults on permission/transient I/O errors as well as IsNotExist; reload inherits the behavior. Default only for a missing file and preserve the last known-good state on all other errors. |
| CH-10 | mpv.conf restore is non-transactional and can leave the active file absent. | GPT H-10; GLM H-20, W-M3, I-M4, T-M3 | Verified and heavily duplicated. The live file is renamed before the replacement is validated/durable, restore performs a second backup step, and date-only backup names collide. Use one same-directory staged/fsynced atomic replace with a unique recovery snapshot and rollback on every failure. |
| CH-11 | The package-version “10 second” timeout cannot terminate blocked probes. | GPT H-11; GLM H-10 | Verified. The unlabeled break in pkg/web/server_version_cache.go:249-326 exits only the select; probes use exec.Command, and refresh holds the cache write lock. Use context-bounded probes, exit the collection loop on deadline, and publish a rebuilt cache outside the lock. |
| CH-12 | IINA installation validates one DMG but copies from a global hard-coded mount path. | GPT H-12; GLM H-9 | Verified. pkg/installer/macos.go:422-475 ignores hdiutil attach output and assumes /Volumes/IINA, so a name collision can redirect copy/detach to another volume. Parse plist output, bind validation/copy/detach to the returned device/mount, and verify bundle identity/signature. |
| CH-13 | Tagged application code receives the long-lived release signing key. | GPT H-13 | Verified. .gitlab-ci.yml:284-317,477-506 builds the generator from the tag, injects the key, and executes it. Move signing to separately pinned minimal code with a non-exportable key and restricted network; tagged code should never receive key material. |
| CH-14 | Unauthenticated upstream “latest” bytes are promoted into first-party signed trust. | GPT H-14; GLM U-M12 | Verified. cmd/generate-info discovers/downloads current upstream assets but does not require reviewed digests or upstream/native signature verification before signing their metadata. Pin reviewed versions/digests and require an approval/signature-verification boundary. |
| CH-15 | Relaunched TUI inherits helper-log output and acknowledges health before real readiness. | GPT H-15; GLM C-1 and H-3 | Verified; GLM Critical downgraded to High. pkg/version/transaction.go:349-358,466-488 redirects the helper and gives the child those descriptors; cmd/mpv-manager/main.go:154-184 acknowledges before platform/release/model/terminal initialization and first render. Preserve/reopen controlling-terminal handles and acknowledge from a post-first-render readiness event. Add a native PTY test. |
| CH-16 | FFmpeg replacement failure can delete the only known-good backup. | GPT H-16; GLM H-7 | Verified. pkg/installer/installer.go:525-614 renames the live binary, copies directly, and can remove the backup even when restoration fails. Stage and verify before touching the live path; retain backup until post-swap validation and successful rollback proof. |
| CH-17 | TUI language apply can replace the full live mpv.conf with embedded defaults. | GPT H-17; GLM H-16 | Verified. pkg/tui/language_preferences.go:823-859 renames the source away, then the shared editor sees no file and creates a default-based config; the date-only backup can collide. Edit the existing source atomically and make a unique copy backup without removing it. |
| CH-18 | Updater executes staged/replaced code before its helper-side digest recheck. | GLM H-1 | Verified; exploitability is conditional on target-adjacent replacement access. pkg/version/transaction.go:512-519 invokes identity verification before verifyUpdateArtifact; the installed target is likewise executed before the post-swap check. Hash/size-check immediately before every exec and bind execution to protected staging/verified file identity. |
| CH-19 | Apply and rollback create kill windows with no executable at the primary pathname. | GLM H-2 | Verified. pkg/version/transaction.go:521-526,575-597 first renames/removes the live pathname and only then installs/restores its successor. A crash prevents startup recovery because there is no primary executable to run it. Use platform-native atomic replacement that never exposes an absent live pathname, with directory durability barriers. |
| CH-20 | A missing backup for an applied updater target is treated as successful rollback. | GLM H-4 | Verified. rollbackUpdateTargets ignores IsNotExist even for an applied target, allowing the journal to claim rollback without restored bytes. Missing required backup evidence must fail rollback; verify restored digest and identity before recording success. |
| CH-21 | TUI Escape/Ctrl+C abandon active destructive work instead of cancelling and joining it. | GPT M-09; GLM H-12 and H-18 | Verified; GPT Medium promoted to High as one lifecycle root cause. Global Escape precedes the manager-update abort branch; Ctrl+C quits; workers use context.Background. Introduce one owned operation context/state machine that cancels child processes, drains output, aborts prepared updates, waits for the worker, then changes screens or exits. |
| CH-22 | TUI's multi-channel stream can nondeterministically lose completion or trailing output. | GLM H-13 | Verified. pkg/tui/models_messages.go:324-375 selects among a buffered terminal result and a closing output channel; either can win, leaving the UI stuck or dropping output, and the tick path is not reliably re-armed. Use one ordered event channel or drain all streams until exactly one terminal result is consumed. |
| CH-23 | “Change MPV UI” runs a full install against the first MPV record and can duplicate tracking. | GLM H-15 | Verified. pkg/tui/models_update.go:1148-1170,1724-1740 silently selects the first record and routes through the full install/save path instead of a targeted UI transaction. Require explicit selected app identity and call the safe UI installer for that app's config tree. |
| CH-24 | Enter used to accept a TUI list filter can also activate the selected action. | GLM H-17 | Verified. Top-level Enter dispatch and several list handlers run before/alongside filter-state handling, including install/uninstall screens. Centralize key routing and prohibit application actions while a filter is being edited or accepted. |
| CH-25 | Recommended-config installation/reset proceeds after preservation or backup failure and writes non-atomically. | GLM H-8 | Verified. pkg/installer/installer.go:1060-1107 logs and continues after preservation/backup errors, overwrites mpv.conf, and treats reapply failure as a warning. Fail closed before the destructive step; stage, fsync, atomically replace, and roll back on any post-swap failure. |
Medium findings — deduplicated cross-reference
Transactions, storage, jobs, and Web lifecycle
| ID | Finding | Source mapping and verification | Corrective direction |
|---|---|---|---|
| CM-01 | ZIP/tar/uOSC extraction can write unexpected entries directly into live destinations outside the rollback allowlist. | GPT M-01; GLM I-M6. Verified. | Preflight every archive format with the 7z policy, extract to a private staging root, validate inventory, then transact only approved paths. |
| CM-02 | A multi-target self-update locks only the primary executable. | GPT M-02. Verified. | Lock every canonical target in deterministic order or use a transaction-wide lock keyed by the complete target set. |
| CM-03 | A crash before the first updater journal leaves an orphan transaction that blocks recovery. | GPT M-03; GLM U-M2. Verified. | Journal intent before fallible staging or make startup safely recognize and clean pre-journal transaction directories without stopping at the first malformed entry. |
| CM-04 | Windows “read-only” install discovery executes every candidate mpv.exe without a bound. | GPT M-04; GLM I-M1. Verified. | Prefer metadata parsing; otherwise use strict context deadlines, constrained execution, canonical/reparse validation, and no execution of untrusted discovery candidates. |
| CM-05 | Multi-field config/language API requests commit one field at a time. | GPT M-05; GLM W-M5. Verified. | Validate into a snapshot and atomically persist the complete new file once, with rollback and one terminal result. |
| CM-06 | Job-history coordination is per Store instance/process and uses a shared fixed temporary pathname. | GPT M-06; GLM P-M7, S-M8, T-M10. Verified. | Add a cross-process lock and revision/CAS semantics; use unique same-directory temp files and durable rename. |
| CM-07 | Persistent SSE clients can turn ordinary graceful shutdown into a timeout/error exit. | GPT M-07; GLM S-M1. Verified. | Close broadcasters first, make handlers observe a root shutdown context, then call bounded Shutdown and wait for completion before deriving exit status. |
| CM-08 | SSE terminal delivery and reconnect reconciliation are not guaranteed. | GPT M-08; GLM F-M2. Verified. | Add event IDs/replay or an active-plus-recent snapshot handshake, bounded per-client queues, and terminal-event priority. |
| CM-09 | UI migration resolution spans manager config and script config without one transaction. | GPT M-10. Verified. | Serialize with compare-and-set and journal/rollback changes across both stores. |
| CM-10 | The job-output modal attempts focus before Alpine removes hidden. | GPT M-11; GLM F-M6. Verified and previously reproduced in Chromium. | Open the dialog after $nextTick or synchronously remove hidden; assert initial focus and focus return in a real browser. |
| CM-11 | Windows amd64 resource metadata is stale/fail-open and arm64 has no resource object. | GPT M-12; GLM B-M2. Verified. | Generate versioned resources for both architectures, fail generation errors, and inspect PE resources in tag CI. |
| CM-12 | Manager-data reset ignores backup failure while promising recovery. | GPT M-13; GLM T-M13/P-M2. Verified. | Create a unique atomic durable backup, return its path, and abort reset if it cannot be made. |
| CM-13 | Backup containment follows intermediate symlinks outside the nominal root. | GPT M-14; GLM W-M2/P-M1. Verified. | Require canonical parent equality for flat backups and use descriptor-relative no-follow operations. |
| CM-14 | Shared config/path locks are process-local even though Web, TUI, and CLI share files. | GPT M-15; GLM S-M7. Verified. | Enforce a per-user singleton or add advisory locks plus file revisions/CAS across every writer. |
| CM-15 | Credential-bearing release jobs use mutable container tags. | GPT M-16; GLM B-M6. Verified. | Pin images by immutable digest and update through reviewed automation. |
| CM-16 | Reduced-motion preferences do not disable all dialog animations. | GPT M-17. Verified. | Centralize dialog motion and add computed-style browser checks under prefers-reduced-motion: reduce. |
| CM-17 | TUI PATH add/remove hides partial failure; Unix aliases accumulate and Windows behavior is not a real PATH install. | GPT M-18; GLM T-M9. Verified. | Use marked idempotent shell blocks or actual platform PATH integration, propagate partial errors, roll back, and verify command resolution. |
| CM-18 | Release-generator downloads lack total time/size/low-speed bounds and omit environment proxy handling. | GPT M-19; GLM U-M9/B-M4. Verified. | Add context deadlines, byte caps, content-length checks, idle limits, partial cleanup, and ProxyFromEnvironment where policy permits. |
| CM-19 | The signed manager manifest is derived from registry re-downloads rather than the pipeline's local build artifacts. | GPT M-20; GLM H-21. Verified; GLM High normalized to Medium because the signer still authenticates the bytes it actually fetched. | Pass exact build artifacts into the signing job, hash/sign locally, upload once, and compare published bytes to local provenance. |
| CM-20 | BLAKE3SUMS.txt is incompatible with the documented b3sum -c. | GPT M-21; GLM U-M11/B-M8. Verified against source and official b3sum documentation. | Emit raw 64-hex GNU-style lines, keep blake3: only in JSON fields, add generator tests, and execute the documented command in release CI. |
| CM-21 | Stored-password Web flows dispatch a privileged operation twice. | GPT M-22; GLM F-M1. Verified. | Give ensurePassword one contract—return a result or invoke a callback, never both—and test stored/missing/cancelled/failed flows. |
| CM-22 | Config Apply can mark edits made during an in-flight request as saved without submitting them. | GPT M-23; GLM F-M3. Verified. | Set the success baseline to the captured submitted payload and immediately recompute dirty state against current controls. |
| CM-23 | UI-setting and regional-language requests allow stale responses/controllers to overwrite newer intent. | GPT M-24; GLM F-M4/F-M5. Verified. | Serialize per key or use monotonic generations and controller ownership checks; ignore every stale response. |
| CM-24 | CLI --path is ignored by component installers but recorded as the actual install path. | GPT M-25; GLM S-M9. Verified. | Plumb the exact destination through every component or reject unsupported path overrides and record only the path actually used. |
| CM-25 | Cached sudo timestamps allow an incorrect submitted password to appear valid. | GLM W-M1/I-M8. Verified. | Invalidate the timestamp (sudo -k) before validating a supplied password, keep the validation context bounded, and share one implementation. |
| CM-26 | The Web locale cache has unsynchronized lazy initialization and sorts a shared backing slice in place. | GLM W-M4. Verified. | Use sync.Once/immutable storage, return copies, and never sort shared backing arrays. |
| CM-27 | Install detection collapses multiple installs by method and can sync records away after uncertain probes. | GLM I-M2. Verified. | Represent detection as found/not-found/unknown, preserve records on unknown, key identity by canonical install, and bound locale-stable probes. |
| CM-28 | Elevated MPC-QT install returns when PowerShell exits, not when the installer completes. | GLM I-M3. Verified in pkg/installer/windows.go:516-572; the GLM filename was stale. | Use Start-Process -Wait -PassThru, propagate the child exit code, and verify the installed result before success. |
| CM-29 | Runtime installer/UI rollback transactions are in-memory only and are not recovered after a crash. | GLM I-M7. Verified. | Add a durable transaction journal and startup recovery for .txn-*/backup artifacts, or reduce mutations to atomic single-path swaps. |
Self-update, TUI, platform, parser, and release correctness
| ID | Finding | Source mapping and verification | Corrective direction |
|---|---|---|---|
| CM-30 | Updater rename/metadata ordering is not durably tied to the journal. | GLM U-M1. Verified. | Fsync affected files/directories where supported, record phases only after durable barriers, and revalidate committed targets during recovery. |
| CM-31 | A configured secondary updater path can overwrite an unrelated regular file without proving its current identity. | GLM U-M6. Verified. | Store and verify the secondary's expected product/component identity and canonical ownership before admitting it to a transaction. |
| CM-32 | Detached update-helper failure never reaches the initiating Web/CLI result. | GLM U-M7. Verified. | Persist helper outcome in a durable transaction status and surface it on next start/tasks; do not label handoff as final update success. |
| CM-33 | Health-failure rollback kills the child without waiting, racing locked executables on Windows. | GLM U-M8. Verified. | Kill the process tree, wait for confirmed exit with a bound, then restore; report rollback failure honestly. |
| CM-34 | Detached TUI worker panics bypass Bubble Tea terminal recovery. | GLM T-M4. Verified. | Wrap operation goroutines with panic-to-message recovery and restore/join terminal state through the main program lifecycle. |
| CM-35 | TUI progress supplies 0–100 values to a 0–1 ViewAs API. | GLM T-M5. Verified. | Normalize once to [0,1], clamp, and test representative percentages. |
| CM-36 | TUI output handling is unscrollable/unbounded/quadratic and resize logic omits active lists/minimums. | GLM T-M6/T-M7/T-M8. Verified as a related interaction/performance cluster. | Use a bounded line ring and incremental viewport, wire keyboard/mouse scrolling, reset auto-scroll on user navigation, and clamp/reflow all active views. |
| CM-37 | TUI update bookkeeping uses a stale menu ID and can re-offer successfully updated apps. | GLM T-M1. Verified. | Persist by stable app ID/method from the selected update result and cover every supported selector, including MPC-QT. |
| CM-38 | TUI “offline” startup performs long, repeated manifest fetches before first render. | GLM T-M12. Verified. | Render from cached/local state first and perform bounded refresh asynchronously with explicit offline status. |
| CM-39 | TUI language search backspace removes one byte rather than one Unicode code point. | GLM T-M14. Verified. | Edit rune/grapheme-aware text or use a maintained text-input component. |
| CM-40 | Raw subprocess/history output can inject terminal CSI/OSC control sequences into the TUI. | GLM T-M15. Verified. | Strip or safely visualize control sequences before display and persistence; preserve raw logs only in a non-terminal-safe artifact if needed. |
| CM-41 | mpv.conf, input.conf, and script-options parsers split comments without tracking quoted #. | GLM P-M3/P-M8/S-M4. Verified. | Share a quote/escape-aware line lexer and regression-test round trips, duplicates, whitespace, and comment-only lines. |
| CM-42 | Linux GPU detection truncates model names and prematurely skips alternate enumeration/fallbacks. | GLM P-M4. Verified. | Preserve complete adapter identity, aggregate PCI/Vulkan/GLX evidence, implement PCI resolution, and derive codecs across all adapters deterministically. |
| CM-43 | macOS AV1 heuristic fails open and incorrectly groups M2 with M3+. | GLM P-M5. Verified in source and against Apple M2/M3 specifications. | Use runtime VideoToolbox capability where possible; otherwise maintain conservative model-generation data and return unknown rather than AV1 on probe failure. |
| CM-44 | Linux arm64 CPUs can be labeled x86-64-v2 because asimd is not recognized and GOARCH is not seeded. | GLM P-M6. Verified. | Branch on runtime.GOARCH, map Linux asimd to NEON, and use architecture-appropriate baseline labels/tests. |
| CM-45 | Hotkey parsing/editing corrupts quoted # arguments, permits # as a serialized comment key, and edits only one duplicate. | GPT L-04; GLM P-M8. Verified; promoted from GPT Low because valid commands can be corrupted. | Reuse the quote-aware lexer, reject comment-only key syntax, define effective duplicate semantics, and normalize all duplicates. |
| CM-46 | Locale selection metadata contains unresolvable common IDs and incorrect region/flag mappings. | GLM P-M9. Verified. | Add referential-integrity tests from every curated list into locales.json and correct hif/te/fil, Punjabi-region, and es-419 data. |
| CM-47 | CLI mode parsing accepts/misroutes conflicting positionals, and verbose/debug do not deliver promised console logging. | GLM S-M2/S-M3. Verified as one command-contract cluster. | Use explicit subcommand parsing with rejected trailing/conflicting args, hide internal flags, and connect verbosity to the logger before initialization. |
| CM-48 | Script-option writes mishandle BOM/CRLF and replace symlinks/metadata with mode 0644. | GLM S-M5/S-M6. Verified. | Preserve or deliberately normalize BOM/newline style, define symlink policy, and retain safe mode/ownership metadata during atomic replacement. |
| CM-49 | Browser launch/banner can race listener bind and the server pointer has unsynchronized publication. | GLM S-M10. Verified. | Bind first, publish the server under synchronization, obtain the actual address, then launch the browser and announce readiness. |
| CM-50 | Release tag grammar, publication serialization, duplicate preflight, and protected-ref enforcement are insufficient in-repo. | GLM H-22/B-M7. Verified as a conditional control gap; downgraded from High. GitLab's external duplicate/protected-tag settings cannot be observed here. | Enforce semantic protected tags in every publishing job, require CI_COMMIT_REF_PROTECTED, add resource_group, fail on existing package files, and audit the project settings GitLab documents. |
| CM-51 | Release creation is not gated by native signing/native execution evidence for the exact Windows/macOS artifacts. | GLM H-23/B-M5. Verified; downgraded to Medium because it is an acknowledged release-readiness gap, not a defect in an already native-qualified artifact. | Add native sign/verify/smoke jobs or a protected approval tied to immutable evidence and exact artifact digests; add live job/SSE browser E2E. |
| CM-52 | make release-build can produce a “release” binary with an empty trust ring. | GLM B-M3. Verified. | Make release targets require and validate embedded trust, and name non-trusted output as development artifacts. |
| CM-53 | Distributed archives omit LICENSE and third-party notices. | GLM B-M1. Verified. | Include both in every archive and assert their presence in packaging tests. |
| CM-54 | Signed release metadata has no enforced freshness/anti-replay state and a static single-key authorization model. | GLM U-M4/U-M5. Verified as design hardening. | Persist the highest accepted sequence/version per channel, define expiry/rollback policy, and support key validity epochs/revocation or threshold rotation. |
| CM-55 | PrepareSelfUpdateFromCheck accepts a mutable DTO whose nonempty key ID stands in for proof of authentication. | GLM U-M3. Verified at the API boundary; no current production caller was found forging it. Downgraded from the GLM implication of an active exploit. | Make the verified result opaque/unforgeable within the package or reverify the manifest/proof at preparation. |
| CM-56 | Interactive sudo competes with Bubble Tea for the same raw terminal. | GLM H-19. Verified; downgraded to Medium because impact is environment-dependent and not destructive by itself. | Pre-authenticate in a secure TUI flow or suspend/restore Bubble Tea and give the child exclusive terminal ownership. |
Low and hardening findings — consolidated
These entries intentionally consolidate closely related low-impact observations. Their count is therefore not comparable to the GLM report's per-reviewer Low/Info labels.
| ID | Consolidated finding | Sources / disposition | Direction |
|---|---|---|---|
| CL-01 | Manifest-status fetch exceptions leave install controls visually enabled even though the backend fails closed. | GPT L-01. Verified. | Route network rejection through the unavailable state and test it. |
| CL-02 | The privileged loopback UI has no Content Security Policy. | GPT L-02; GLM frontend Low. Verified defense-in-depth gap; no XSS was found. | Remove inline behavior, then deploy a tested nonce/hash-based CSP. |
| CL-03 | Local make release can package stale frontend assets. | GPT L-03. Verified; tag CI has separate freshness protection. | Make the local target run locked install, vendor/CSS freshness, and frontend tests. |
| CL-04 | Public parsed script-option snapshot Write can overwrite fresher edits. | GPT L-05; GLM S-M7. Verified API hazard; current single-value production paths reparse under lock. | Deprecate it or add revision/CAS rejection. |
| CL-05 | Manager reset text claims language preferences are cleared although they live in mpv.conf. | GPT L-06; GLM T-M13. Verified. | Correct the text or include a separately confirmed transactional language reset. |
| CL-06 | Staticcheck identifies persistent TUI state/unreachable-branch/empty-branch defects. | GPT L-07. Verified. | Fix value-receiver mutation and unreachable/empty branches; enforce the correctness family in CI. |
| CL-07 | Maintained macOS docs disagree about universal binaries and .app bundles. | GPT L-08; GLM build/docs Low. Verified. | Align all maintained docs with actual published artifacts. |
| CL-08 | Several Web routes have weak method/state validation and minor error-contract defects. | GLM Web Low index. Verified as a cluster: GET state sync, unvalidated ui_type, racy backup enumeration, downgrade-as-update labeling, and raw keyring detail. | Make state sync explicit/mutation-gated, validate enums, tolerate disappearing entries, compare versions semantically, and normalize errors. |
| CL-09 | SSE clients/writes have no explicit cap or write deadline. | GLM Web Low index. Verified hardening gap. | Bound clients/queues and enforce write deadlines/backpressure policy. |
| CL-10 | Installer text/config/output cleanup has minor correctness leaks. | GLM Installer Low index. Verified cluster: duplicate OSC entries, leaked uOSC temp config, and chunk-not-line output framing. | Normalize effective keys, defer cleanup reliably, and line-buffer output. |
| CL-11 | Generated shortcut/wrapper paths are not robustly encoded for shell/VBS syntax. | GLM Installer Low index. Verified path-dependent risk. | Avoid shell interpolation or use platform-native quoting/shortcut APIs with adversarial path tests. |
| CL-12 | Updater process/path hardening is incomplete. | GLM Updater Low index. Verified cluster: PID reuse, replaceable pathname lock, unbounded identity output/process group, long cumulative retry window, and retained obsolete updater path. | Bind handoff to stronger process identity, harden lock ownership, bound/kill probes, cap total retry time, and remove dead high-risk code. |
| CL-13 | Signed asset policy fields and legacy/v2 field consistency are not fully enforced. | GLM Updater Low index; GPT coverage gaps. Verified. | Enforce or remove NativeSigning, format/scope/strategy/CPU policy fields and validate legacy/v2 equivalence. |
| CL-14 | Several small TUI input/view states contradict displayed behavior. | GLM TUI Low index. Verified cluster: error Enter, screenshot quoting/sticky warnings, re-entrant preset action, inert selectable rows, tiny-terminal and dead-state behavior. | Simplify state ownership and add table-driven key/viewport tests. |
| CL-15 | Core catalogs/log retention/data models contain bounded quality issues. | GLM Core Low index. Verified cluster: regionless filter behavior, ambiguous dual-UI representation, silently dropped log bursts, unbounded hotkey backups, and contradictory static descriptions. | Clarify model semantics, bound/measure queues and backups, and add catalog integrity tests. |
| CL-16 | Frontend mobile focus containment and trusted-fragment HTML composition need hardening; orchestration test coverage is thin. | GLM Frontend Low index. Verified. | Use the shared dialog controller for the drawer, escape every fragment, and add behavioral/browser tests around orchestration seams. |
| CL-17 | Shared file helpers lack full directory durability and can widen/lose metadata; some exported globals are mutable. | GLM Supporting Low index. Verified cluster. | Fsync parent directories, preserve safe metadata, bound backups, and return copies of public option catalogs. |
| CL-18 | Main-process lifecycle has small cleanup/diagnostic defects. | GLM Supporting Low index. Verified cluster: os.Exit bypasses defers, signals start late, production panics print stacks, help/version initialize logging, browser helpers may not be reaped, and exit-code semantics vary. | Centralize lifecycle/exit handling and make identity/help paths side-effect free. |
| CL-19 | Build/CI quality checks and maintained documentation have minor drift. | GLM Build Low/Info index. Verified cluster: coverage parsing, unpinned local lint installer, obsolete URLs/targets/line counts, and routine dependency drift. | Parse aggregate coverage, pin tooling, and add doc/link/inventory checks. |
| CL-20 | The development-only qualifier driver can perform destructive operations without a controller capability token. | GLM U-M10. Verified but downgraded from Medium. It requires hidden, explicit qualification flags and is not shipped as the updater. | Require a controller-created capability/owned disposable root and reject paths outside it. |
Claims changed, qualified, or not upheld
| Source claim | Final disposition | Reason |
|---|---|---|
| GLM C-1 is Critical. | Verified as CH-15, High. | The descriptor/readiness bug is serious and release-blocking, but it requires the intended TUI self-update path and does not independently produce direct privilege compromise or unrecoverable destruction under the shared severity definition. |
| GLM H-5: v1.1/v1.2 bootstrap remains unauthenticated. | Historical limitation; not counted as a current-code defect. | Immutable historical clients cannot retroactively validate the new signature. Current documentation already requires a one-time manual v1.3 replacement for affected Windows clients. Keep the warning and retire legacy automation when migration permits. |
| GLM Low: unknown JSON fields are “outside the signature.” | Not upheld as a current vulnerability. | Verification unmarshals into the known schema and signs/canonicalizes every field the consumer uses. Ignored unknown fields are inert to current behavior. Strict decoding remains optional schema-hardening, not evidence of a signature bypass. |
| GLM Low: HTTPS is not revalidated across redirects. | Not upheld as a demonstrated security finding. | Redirect handling can be tightened, but release bytes and manifests are independently authenticated by signature/hash/size. No path was shown where a redirect changes trusted behavior while those checks hold. |
| GLM H-22 assumes overwrite-capable GitLab settings/roles. | Conditional CM-50, Medium. | GitLab documents duplicate publication as the default, but this checkout cannot prove the live project's toggle, protected-tag configuration, or member roles. The pipeline should still preflight and enforce what it can locally. |
| GLM U-M3 implies callers can turn any URL into an authenticated update. | Verified boundary weakness as CM-55, Medium. | The DTO is forgeable in-package/API terms and the qualifier uses synthetic data, but current production call paths originate from verified checks. Make the proof opaque rather than claiming an observed external exploit. |
| GLM U-M10 rates the qualifier driver Medium. | Verified as CL-20, Low. | It is development-only and requires explicit hidden qualification flags; containment is still worth adding. |
| GLM P-M5 says M2 is wrongly grouped with M3 for AV1. | Upheld as CM-43. | Apple lists AV1 decode for M3 and omits it from M2 hardware/video support, matching the source-level heuristic defect. |
Cross-report agreement summary
| Theme | GPT report | GLM report | Combined conclusion |
|---|---|---|---|
| Windows ownership/wrong target | H-01–H-04 | H-6, H-14 plus installer findings | Strong agreement; four distinct High fixes. |
| Config transactional integrity | H-08–H-10, H-17, M-05/M-13 | H-8/H-16/H-20, W-M3, I-M4, T-M3, P-M2 | Strongest duplicated root cause; four High items plus supporting Medium items. |
| Web cancellation/jobs | H-05–H-07, M-06–M-08 | H-11, F-M2, S-M1, history duplicates | Strong agreement; lifecycle and resource locking must be redesigned together. |
| Updater/relaunch | H-15, M-02/M-03 | C-1, H-1–H-4, U-M1–U-M8 | GLM added four verified High updater findings; Critical label normalized. |
| TUI operation lifecycle | H-03/H-17, M-09 | H-12–H-19, T-M1–T-M15 | GLM materially expands the TUI defect set; Escape/Ctrl+C combined into one High root cause. |
| Release provenance | H-13/H-14, M-16/M-19–M-21 | H-21–H-23, U-M9/U-M11/U-M12, B-M1–B-M8 | Broad agreement; actual artifact provenance is real, external GitLab configuration remains conditional. |
| Frontend async/focus | M-11, M-22–M-24 | F-M1–F-M6 | Near-exact agreement and useful independent corroboration. |
| Platform detection/parsers | Limited | P-M3–P-M9, S-M4–S-M6 | GLM adds several verified Medium portability/data-integrity findings. |
Areas that held up well
- Web API auth uses a per-start random token, constant-time comparison,
HttpOnly/SameSite=Strict, loopback Host enforcement, exact Origin checks, method restrictions on core mutations, request limits, and bounded keyring-auth attempts. No auth bypass was found. - Release manifest verification fails closed for missing/unknown trust, malformed or unsigned data, signature failure, missing authenticated size/hash, asset tampering, and staged manager identity mismatch.
- The 7z extractor has substantial path, device, symlink, duplicate/case-collision, entry-count, expanded-size, and exclusive-create defenses.
- Keyring backends use native OS stores; reviewed password transport avoids argv/env/log exposure and uses bounded probes.
- Reviewed Web rendering generally uses Go template escaping,
textContent, Alpinex-text, or explicit escaping. No confirmed DOM XSS was found. - Job snapshots are copied under lock, relay goroutines are normally drained, and the rerun race tests passed. The remaining job defects are lifecycle/transaction ordering rather than an already detected data race.
- npm-managed Alpine/htmx assets are pinned and embedded for offline use; the source review's vendor freshness and dependency audits were clean.
Recommended remediation sequence
Release blocker A — never lose ownership, target identity, or a launchable primary
Fix CH-01–CH-04, CH-12, CH-16–CH-20, and CH-25. Add native Windows tests for ownership-scoped uninstall and exact selected-app targeting, native macOS mount-identity tests, and kill-point updater tests that prove the primary pathname is always launchable.
Release blocker B — make relaunch health real
Fix CH-15 together with CH-19/CH-20. Add a native PTY qualification that confirms visible output, usable input, first-render acknowledgement, bounded startup, rollback before readiness, and retained diagnostics without descriptor inheritance.
Correctness gate — one owner for every operation and terminal result
Fix CH-05–CH-11 and CH-21–CH-24. Introduce resource leases, worker-acknowledged cancellation, one ordered TUI event stream, explicit partial-success states, and transactional config snapshots. Fault-inject disk-full, permission, cancel-at-commit, blocked probe, process contention, and connected-SSE shutdown cases.
Supply-chain gate — shorten the path from reviewed bytes to signed bytes
Fix CH-13/CH-14 and CM-15/CM-18–CM-20/CM-30–CM-33/CM-50–CM-55. Sign pipeline-local artifacts with isolated signer code, pin upstream digests and CI images, enforce protected semantic tags and publication serialization, make checksums executable by the documented tool, and gate release on native evidence.
Portability and UX gate
Address CM-10/CM-16/CM-21–CM-29/CM-34–CM-49/CM-56, then the Low/hardening clusters. Add real-browser lifecycle tests, quote-aware parser property tests, architecture-specific detection fixtures, and cross-process file/history contention tests.
Review limitations
- Reconciliation was performed on Linux. Windows UAC/uninstall/MPC-QT, macOS DMG/signature/quarantine, native updater crash points, and actual package-manager mutations were not executed here.
- No production signing key, release registry, GitLab project settings, membership/role state, protected-tag rules, or stable publication endpoint was accessed. Findings depending on those controls are explicitly conditional.
- No destructive install/uninstall, production publication, or external account mutation was performed.
- This review verifies control flow at commit
bdfb31b; it does not claim every environment-dependent impact will reproduce on every supported platform. - Low/Info observations from the GLM per-area reports are consolidated by root cause, so combined counts are actionable work-item counts rather than a claim of atom-for-atom equivalence with 154 raw labels.
Bottom line
The two reviews are broadly consistent once duplicate labels and severity rhetoric are removed. The current code has a solid cryptographic manifest core, strong loopback Web authentication, hardened 7z extraction, good dependency hygiene, and a passing normal/race test baseline. The release should nevertheless be blocked on the ownership/targeting, updater atomicity/relaunch, destructive config, worker-lifecycle, and signing-boundary High findings above. None requires abandoning the architecture, but several require replacing “best effort plus logging” with explicit transaction ownership, durable proof, and one authoritative terminal outcome.