# Installer, filesystem, process and package-query audit — 2026-10-03 Read-only audit notes for the parent codebase review. No tracked source or maintained documentation was edited by this reviewer. Tests used a Go overlay pointing an otherwise absent package-local test file to `/tmp/mpv-manager-installer-audit-2026-10-03/probe_test.go`. Fixtures were all disposable temporary directories; no player installation, user app/config directory, registry, mount, or native Windows/macOS state was mutated. ## Evidence and scope - Probe source: `/tmp/mpv-manager-installer-audit-2026-10-03/probe_test.go`. - Overlay: `/tmp/mpv-manager-installer-audit-2026-10-03/overlay.json`. - Results: `/tmp/mpv-manager-installer-audit-2026-10-03/probe-results.txt`. - Command: `go test -overlay=/tmp/mpv-manager-installer-audit-2026-10-03/overlay.json ./pkg/installer -run '^TestOctoberAudit' -v -count=1`. - All six audit probes passed **by asserting the current faulty behavior**. This is defect reproduction, not verification of a fix. - Read AGENTS.md, README.md, docs/ARCHITECTURE.md, September 5 audit/reconciliation and September 8 review/remediation. Reviewed installer public operation dispatch; platform install/update/uninstall paths; downloads and archive preflight/extraction; transactional overlay/swap/file/UI recovery; ownership inventories and narrow Windows legacy migration; config/default/restore preservation; shortcuts; IINA mount cleanup; process ownership; advisory locks and atomic persistence; shared package queries and duplicated detection parsers. - Exact line references below are from the current working tree. Runtime probes ran on Linux amd64. Windows ARM64 and macOS observations are source findings, not native coverage. Parent performs baseline/race/lint/browser/vulnerability/reachability validation. ## Confirmed findings ### I-01 — P1 security: unauthenticated installer recovery accepts a planted journal for an unrelated sibling **Locations:** `pkg/installer/transaction_journal.go:255`, `:298`, `:303`, `:396`, `:529`; `cmd/mpv-manager/main.go:235`; `pkg/config/validation.go:192`. `RecoverInstallerTransactions(paths...)` reduces supplied paths to parent directories and replays every matching journal found there. The only journal-file trust check is that it is a regular non-symlink file. `validateInstallerJournal` permits any absolute `Destination` immediately beneath that scanned parent; it neither binds destinations to the supplied recovery paths nor checks journal/backup ownership and permissions nor authenticates journal contents. A schema-1 `file-swap` journal in `applying` state, with a properly named empty backup directory and `HadOriginal:false`, passes validation. Recovery calls `os.RemoveAll(change.Target)`. **Reproduction:** `TestOctoberAuditForgedJournalDeletesUnrelatedSibling` asks the public API to recover `configured-install`, planting a journal whose destination is a separate `unrelated-private-documents` directory. Recovery returns nil, deletes `unrelated-private-documents/keep.txt`, and removes the forged journal. Output: `recovery requested only configured-install; unrelated victim deleted=true`. **Trigger/threat model:** the user chooses an install/config/app path beneath a parent writable by another OS user, e.g. a custom installation directly beneath a shared temporary or group-writable directory. `ValidateCustomInstallPath` permits writable shared ancestors. Another OS user can plant the journal in the parent without write permission to the target itself. On startup, `recoverInstallerState` scans this parent and performs deletion with the manager user's privileges. A private 0700 parent prevents that cross-user planting precondition. This is not a claim of privilege gain against a process already running as the user. The local probe proves unauthorized sibling selection and destructive replay; it did not create a second OS account. **Impact:** arbitrary sibling files or trees accessible to the manager can be deleted during normal startup; a crafted backup with `HadOriginal:true` can also replace a target. No network input or player execution is necessary. **Recommendation:** bind replay to explicitly requested known destination identities and verify recovery-store ancestry, ownership and permissions. Authenticate installer journal contents and backup evidence with a private key when journals can reside in writable shared parents, or reject those parents for destructive automatic replay. Preserve untrusted or ambiguous evidence and report manual recovery. Do not treat matching filename prefixes or lexical containment as authorization. **Regression tests:** planted unrelated-sibling journal must be rejected with victim bytes intact; same-target journal written by another principal must be rejected; journal/backup symlink, ownership/permission, tampering and repeated-recovery fixtures. Keep the existing legitimate interrupted transaction tests passing. **Historical mapping:** previous R01 repeat-recovery data-loss safeguards are present and useful. This is a distinct remaining installer journal trust boundary, analogous to but separate from the authenticated manager updater journal. ### I-02 — P1 correctness/recovery: automatic Web shortcut setup still overwrites the installed manager directly **Locations:** `cmd/mpv-manager/main.go:399`; `pkg/installer/linux.go:614`, `:627`, `:633`; `pkg/installer/windows_shortcuts.go:304`, `:313`, `:326`. Every Web startup with `CreateManagerShortcut` enabled synchronously calls `CreateWebUIShortcutWithOutput`. Linux copies the running binary into `~/.config/mpv/mpv-manager` whenever its resolved source path differs from the destination path. Windows compares full executable contents and copies whenever bytes differ. Both use `os.WriteFile` on the existing destination: no version ordering, updater lock, staged rename, transaction journal, or rollback. The runner uses `context.Background` through `NewCommandRunner`, so startup cancellation does not own this path. Windows additionally reads both binaries fully and ignores errors from the comparison reads. **Reproduction:** `TestOctoberAuditWebShortcutOverwritesExistingManager` writes an existing-manager sentinel in an isolated Linux home then calls the same shortcut function invoked from Web startup. The sentinel is replaced by the current test executable, and that path is persisted as `ManagerBinPath`. Output: `existing installed manager replaced=true; now equals running executable=true`. This uses only isolated filesystem state; it does not prove native Windows behavior or power-loss behavior. **Trigger:** enable desktop shortcut creation, then launch a downloaded/copy manager from a path different from the installed manager. An older executable can overwrite a newer idle installed executable. Disk-full, interrupted writes or process termination after truncation can leave the installed copy unusable. A simultaneous real updater has no shared exclusion with this path. A running Linux destination can instead fail ETXTBSY; an idle copy is enough for the reproduction. **Recommendation:** remove startup-time executable replacement or route explicitly requested manager installation/refresh through the established coordinated updater/installation protocol. Shortcut creation should not independently decide version ordering from byte inequality. Reuse the configured effective storage path rather than introducing separate hardcoded locations. **Regression tests:** ordinary Web startup with shortcut enabled preserves a newer sentinel; failed copy leaves old bytes intact; manager install/update and shortcut creation coordinate; source/target read errors retain old binary; Windows locked-handle and Linux idle/running cases stay separate. **Historical mapping:** R2-09 removed the TUI startup overwrite path. The Web shortcut path remains, so the broader claim that startup-time direct replacement was retired is incomplete. The setting defaults disabled, but it is exposed and normal supported behavior when enabled. ### I-03 — P2 data loss: FFmpeg update uses an unowned fixed scratch directory and removes its pre-existing contents **Locations:** `pkg/installer/installer.go:561`, `:577`, `:578`, `:580`; live dispatch `pkg/installer/common_handler.go:84`. Standalone FFmpeg update chooses fixed paths `installDir/ffmpeg-update.7z` and `installDir/ffmpeg-temp`, then unconditionally defers removal of the archive and the entire extraction tree. It does not require those paths to be absent or establish ownership. Generic transactional extraction preserves unrelated entries in the destination, but the enclosing deferred `RemoveAll` subsequently deletes them anyway. It runs on extraction failure as well. **Reproduction:** `TestOctoberAuditFFmpegFixedScratchDeletesUnrelatedData` creates `ffmpeg-temp/user-document.txt` in a private install fixture. Injected HTTP returns fixture bytes with a matching manifest BLAKE3 digest; the 7z parser fails. The function returns an extraction error, yet the pre-existing user document is gone. Output: `...not a valid 7-zip file; pre-existing ffmpeg-temp/user-document.txt deleted=true`. The fixture replaces only remote bytes; real filesystem, hashing, archive parser and enclosing cleanup control flow run. No native player install was performed. A valid archive follows the same unconditional cleanup path. **Impact:** unrelated user data under an unfortunate name is deleted even when the update fails before commit. A pre-existing `ffmpeg-update.7z` can likewise be overwritten/deleted by the download/cleanup path. Concurrent manager processes can also share these fixed scratch paths before the commit lock exists. **Recommendation:** reserve a unique private sibling staging directory with `os.MkdirTemp`, place download and extraction paths inside it, and delete only that owned directory. Keep live executable replacement beside the destination and retain the existing transaction mechanism. **Regression tests:** pre-existing `ffmpeg-temp` subtree and archive-name collision survive successful update, parser failure, hash mismatch, cancellation and concurrent staging. ### I-04 — P2 permissions: backup restore and recommended-config replacement change a private mpv.conf to 0644 **Locations:** `pkg/installer/common.go:196`, `:218`; `pkg/installer/installer.go:1101` (recommended-config publication); reachable Web/TUI restores `pkg/web/api_config.go:307`, `pkg/tui/models_update.go:1766`. `CreateFullBackup` captures the existing file's permission mode, but `RestoreBackup` uses `fileops.AtomicWrite(...constants.FilePermission)` for the live file and the rollback path uses the same fixed mode. This discards existing metadata, despite `AtomicWritePreserve` already being available and used by normal config edits. Recommended-config installation uses the same fixed-mode publication for an existing file. Atomic creation also changes Unix ownership to the manager process rather than preserving the target's owner/group. **Reproduction:** `TestOctoberAuditRestorePermissions` restores a valid generated backup into an existing 0600 mpv.conf. It returns nil and resulting mode is 0644: `RestoreBackup original mode=0600 resulting mode=0644`. Mode loss is verified on Linux. Ownership loss is source-derived; no cross-owner fixture was run. **Impact:** another local user can read formerly private config in a traversable directory, including private paths or credentials stored in mpv options. Permission/ownership invariants are lost on successful restore; rollback also does not restore them. A private 0700 ancestor limits immediate disclosure but does not repair the metadata regression. **Recommendation:** inspect/reject non-regular targets and use `AtomicWritePreserve` for existing live files. Preserve original metadata through rollback and explicitly choose metadata for a previously missing destination. If restore semantics should adopt backup metadata instead, encode and apply that policy consistently rather than forcing 0644. **Regression tests:** 0600 and 0640 live targets retain mode; Unix owner/group retention when available; failure/rollback retains both bytes and metadata; missing-target fallback and symlink rejection. Extend recommended-config reset/load-latest tests to the same preservation contract. ### I-05 — P2 architecture: standalone FFmpeg updates replace Windows ARM64 payload with x86-64 **Locations:** `pkg/installer/installer.go:538`, `:547`, `:641`; contrast `pkg/installer/common.go:50` and `pkg/installer/windows.go:578`; dispatch `pkg/installer/common_handler.go:84`. `platform.GetCPULevel()` returns `arm64` on ARM systems, but standalone FFmpeg update handles only `x86-64-v3` and `x86-64`; its default chooses `ReleaseInfo.FFmpeg.X8664` and claims CPU compatibility. Validation accepts only `pe.IMAGE_FILE_MACHINE_AMD64`. Initial Windows MPV staging correctly selects `FFmpeg.Aarch64`, so using the separate FFmpeg update route changes architecture. Supplying a native ARM64 FFmpeg binary to the existing validator is also rejected. **Evidence:** source tracing of the reachable component route and explicit switch/validator. No Windows ARM64 execution or emulation result is claimed. The owner's native ARM64 open-beta coverage waiver is a coverage gap, not authorization to select the wrong manifest asset. **Impact:** native ARM64 FFmpeg becomes x64 and relies on Windows x64 emulation; architecture parity and native performance are lost. On environments without working emulation the component cannot execute. A native payload cannot pass the update validator. **Recommendation:** choose the artifact from OS architecture plus reviewed CPU baseline, using the same architecture selector as Windows install. Validate the expected PE machine for the selected asset. Do not default an unknown CPU architecture to x64 with a compatibility assertion. **Regression tests:** amd64 baseline/v3 and arm64 manifest selection; AMD64 and ARM64 PE validation; mismatch rejects before live replacement. Keep native ARM64 execution explicitly unverified until hardware is available. ### I-06 — P2 lifecycle/performance: the live Flatpak prerequisite query remains unbounded and ignores operation cancellation **Locations:** `pkg/installer/linux.go:67`, `:71`, `:189`, `:232`, `:296`. `InstallFlatpakWithOutput` begins commit, then calls `CheckFlathubEnabled`. The latter uses `runQuietOutput`, whose production fallback is plain `exec.Command(...).Output()`, independent of the owning `CommandRunner` context. It lacks a deadline, output budget and owned process-tree boundary. The bounded `internal/packagequery` and `CommandRunner` implementations do not cover this call. `runQuiet` for icon cache commands is another unbounded fallback on Web shortcut startup. **Reproduction:** `TestOctoberAuditFlatpakQuietProbeIgnoresOperationDeadline` places a harmless fixture `flatpak` executable at the front of PATH. Its `remote-list` sleeps one second. An operation with a 50ms context deadline returns `DeadlineExceeded` only after the full 1.0016s quiet probe. The subsequent installation command is never executed after cancellation. A hung probe has no intrinsic bound; the one-second fixture bounds the audit itself. **Impact:** hanging Flatpak remote discovery can hold a committed job/resource lease and prevent a proper worker drain indefinitely. Cancellation and shared package-query hardening do not bound this live path. Large output accumulates without the 1 MiB parsing budget. **Recommendation:** pass the operation context and use the shared bounded owned-process boundary, with an explicit short read-only prerequisite timeout. Perform read-only repository discovery before the irreversible boundary where possible; do not replace necessary mutation lifetime ownership with a blanket cancellation after commit. **Regression tests:** stalled/noisy remote-list query, context deadline and descendant cleanup; ordinary install, missing remote and disabled/unavailable Flathub results stay distinguishable. ## Confirmed lower-priority performance/cancellation observation ### I-07 — P3: compressed-tar preflight reads the entire expanded stream before checking cancellation or total size **Locations:** `pkg/installer/installer_archive.go:233`, `:326`, `:329`, `:345`; `pkg/installer/installer_sevenzip.go:26`; `pkg/installer/installer.go:650` for separate uncancellable artifact hashing. Tar preflight receives no context and only calls `validateArchiveInventory` after iterating every header. `tar.Reader.Next` must skip/decompress the previous entry's unread contents to reach the next header, so gzip/xz preflight traverses the full expanded data. The 2 GiB aggregate bound is applied only after this work; an oversized first header is not rejected before skipping its data. Actual extraction then decompresses the archive a second time. `extractIntoPrivateStage` checks cancellation only after preflight. Artifact hashing similarly uses `io.Copy` without context despite receiving a runner in its caller. **Evidence:** `TestOctoberAuditTarPreflightIgnoresCancellation` uses an already-canceled context and a 256 MiB zero-filled compressed tar. Preflight completes before returning cancellation (~28ms on this host). An already-canceled malformed gzip returns `open gzip stream: unexpected EOF` instead of `context.Canceled`, proving archive work precedes cancellation. This is not a claim that 28ms is a user-visible outage; it establishes the ordering, which scales with slow xz decode, storage and archive size. No multi-gigabyte stress or memory-exhaustion test was run. **Recommendation/tests:** pass context through preflight and its readers; enforce per-entry/aggregate size limits immediately after header inspection before skipping the body; reject already canceled work before opening parsers. Measure ordinary upstream archive decode before considering one-pass extraction into a private stage. Regression with canceled context and deliberately large header should return promptly without decompressing the oversized body. Keep resource limits independent of authenticated provenance, while recognizing normal remote artifacts are hash-verified first. ## Source-only follow-up candidate; native behavior not reproduced ### I-08 — P3 lifecycle: IINA mount cleanup is registered only after successful hdiutil output **Locations:** `pkg/installer/macos.go:579`, `:584`, `:590`, `:595`, `:601`. When `hdiutil attach` returns nonzero or is canceled, installation returns at line 590 before any detach defer is registered. A successful-but-unparseable plist correctly uses `detachDMGsWithinRoot`, but command failure does not. If hdiutil has already attached a device before cancellation or another error, it can leave a mounted image under the operation's private mount root. The enclosing `RemoveAll(tempDir)` cannot reliably retire a mounted read-only tree and ignores cleanup errors. No native interrupted attach was run, so this remains a specific source-backed follow-up candidate rather than a reproduced stranded-mount defect. Recommendation: register owned-mount cleanup before attach, use an independent bounded cleanup context, discover/detach only devices under that private root on every failure path, and retain/report paths when detachment fails. Native regression should interrupt attach after mount creation and fail plist/exit after attaching, while keeping unrelated mounted images intact. ## Dead code / duplication / maintainability observations - Legacy non-streaming package APIs in `pkg/installer/linux_package.go:19`–`:204` duplicate live `*WithOutput` routes in the same file. They also retain Celluloid PPA installation policy absent from the live path. The parent reachability audit should classify these per-platform supported exports versus cleanup candidates; grep alone is not proof of safe deletion. Exported injection APIs and behavior-test seams must be considered separately. - Linux package parsers remain duplicated between `pkg/installer/detection.go:286`–`:456` and `internal/packagequery/query.go`. Discovery strips Pacman epoch (`detection.go:419`) while the shared installed query retains it. Consolidating parsing/probes in the shared boundary would reduce policy drift and duplicated fallback handling; retain the `Found/Unknown/NotFound` conservative discovery semantics and exact path identity. - Downloader byte-copy, size-limit, cancellation, progress and cleanup logic is duplicated across `Installer.DownloadFileWithProgress`, `Installer.DownloadFileWithProgressToChannel` (`installer.go:194`, `:349`) and `HTTPDownloader.DownloadFile` (`real.go:165`). Shared low-level artifact transfer can retain progress callbacks and test seams without introducing forwarding-only wrappers. The extra public downloader option does not affect `DownloadFileWithProgressToChannel`, which deliberately uses the HTTP client; tests must inject the actual production boundary they exercise. - The Web shortcut code rereads full manager executables (Linux one large allocation; Windows comparison of two binaries then rereads source). This is observable needless memory/I/O once the unsafe overwrite policy is corrected. Do not optimize byte-comparison while leaving its replacement semantics in place. - Production non-Windows shortcuts and convenience exports require applicable-platform reachability checks. Historical dead-code counts are not current safe-deletion counts. ## Prior safeguards rechecked and retained - Installer rollback validates required backup trees before deleting live targets, copies rather than consumes crash-recovery evidence, and publishes a terminal marker before cleanup. Prior R01 missing-backup/repeated-recovery defect is not repeated here. - UI snapshots are read under editor locks at commit time; `.mpv-manager/ui-baselines` and UI version metadata participate in durable rollback. Staging no longer publishes component metadata. Previous R10/R11 are not blindly reopened; standalone FFmpeg uses a different metadata boundary and may warrant a future interruption-consistency test. - Windows legacy/adopted ownership migration claims only validated PE MPV launchers and preserves unrelated collision originals; ownership inventories protect `portable_config` and manager files. Removal does not elevate an old mutable archive batch script. MPC-QT install/uninstall waits for elevated process completion, propagates errors and verifies removal. - Windows update resolves selected app UI state; UI preparation precedes live payload commit. Partial setup failures are explicit. macOS app recovery permits only relative symlinks resolving within the same bundle and uses ditto for native preservation. Icon generation now computes ten distinct regular/Retina paths. - `internal/process` uses Unix process groups and Windows Job Objects, 2-second pipe cleanup, bounded captures and owned synchronous processes. Shared package-query execution uses it with 5-second context bounds and C locale and discards partial timeout output. These useful safeguards do not cover the quiet Flatpak fallback in I-06. - Ordinary installer/update/uninstall paths reviewed do **not** remove `.mpv-manager-update-journal.key` or the release-trust directory. Windows payload preservation protects `portable_config`; UI cleanup lists cover specific config/scripts/fonts/baselines; embedded uninstall scripts remove specific manager binary/JSON/icon paths, not updater authentication state. Forged installer journal I-01 can of course delete a targeted tree and remains a separate trust problem. ## Limits No native Windows, macOS, power-loss, disk-full, cross-user identity, UAC, hdiutil, Intel Mac, or ARM64 hardware run occurred in this review. All direct mutation fixtures were disposable Linux temp directories. Findings distinguish reproduced platform-neutral/Linux behavior from source-derived Windows/macOS paths. Existing native evidence and explicit coverage waivers remain historical/accepted gaps rather than new passing native tests. Fixes were not requested or applied.