Bundle ffprobe.exe and complete the real-download smoke-test pass

A live smoke test of buildArgs' argv against the bundled yt-dlp + ffmpeg found
that ffprobe.exe was never bundled (resources/bin/ shipped only ffmpeg.exe).
yt-dlp resolves both from --ffmpeg-location, so duration-aware post-processing
failed at runtime for every user with "ffprobe not found": --sponsorblock-remove,
--force-keyframes-at-cuts, and --split-chapters (CODE-AUDIT C1).

- Bundle ffprobe.exe (matching n8.1.2 LGPL build, sha256-verified against the
  already-bundled ffmpeg.exe) and document it in resources/bin/README.md as a
  required binary; correct the stale "not committed to git" note.
- Guard startDownload against a missing ffmpeg.exe/ffprobe.exe up front so the
  failure is a clear AeroFetch error, not a cryptic yt-dlp postprocessing one
  (new getFfprobePath() in binaries.ts).
- Expand test/real-download.integration.test.ts from 2 to 8 live cases: crop,
  sponsorblock-remove, audio opus re-encode, mkv+vp9 merge, subtitle+chapter
  embed, restrict-filenames, download-archive skip, and Phase C extra-args.
  All 8 pass against live yt-dlp + ffmpeg.
- ROADMAP: mark the Phase A/B/C smoke-test caveats done. CODE-AUDIT: add C1.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-06-23 10:57:12 -04:00
parent b08d76a986
commit 18e1d9a24d
7 changed files with 305 additions and 21 deletions
+24 -1
View File
@@ -16,7 +16,7 @@ AeroFetch's security posture is notably thoughtful:
- **Settings validation**: Type-checked before persistence; `spawn` used without `shell: true`
- **Good test coverage** of risky logic (especially [buildArgs.test.ts](test/buildArgs.test.ts))
The findings below are opportunities to tighten existing defenses and fix performance gaps. None are foundational breaks.
The findings below are opportunities to tighten existing defenses and fix performance gaps. None are foundational breaks — with one exception added later: **C1**, a missing bundled `ffprobe.exe` that broke duration-dependent post-processing, surfaced by the real-download smoke test and now fixed.
---
@@ -132,6 +132,28 @@ function isValidHistoryEntry(obj: unknown): obj is HistoryEntry {
---
### Correctness
#### C1 — Bundled `ffprobe.exe` was missing — duration-dependent post-processing failed for every user
**Files:** [resources/bin/README.md](resources/bin/README.md), [binaries.ts](src/main/binaries.ts), [download.ts:235](src/main/download.ts)
**Severity:** High
**Status:** Fixed — 2026-06-23
**Description:**
`resources/bin/` shipped `ffmpeg.exe` but **not** `ffprobe.exe`, and the README only documented copying `ffmpeg.exe`. yt-dlp resolves *both* binaries from `--ffmpeg-location <dir>`; without `ffprobe.exe` it cannot read media durations, so any post-processor that needs one fails at runtime with `ERROR: Postprocessing: Unable to determine video duration: ffprobe not found`. That breaks `--sponsorblock-remove`, `--force-keyframes-at-cuts`, and `--split-chapters` for **every** user — end-user machines have no system ffprobe either. It slipped past review because thumbnail/crop/metadata post-processing only uses ffmpeg, and typecheck can't see a missing binary.
**Found by:**
The real-download smoke test ([real-download.integration.test.ts](test/real-download.integration.test.ts)) — the SponsorBlock-remove case failed with exit 1 (`ffprobe not found`) until ffprobe was bundled. Everything else (crop, audio re-encode, container/codec, subs, chapters, restrict-filenames, archive, extra-args) passed.
**Fix:**
Copied the matching `ffprobe.exe` (same `n8.1.2` LGPL build, verified by SHA-256 against the already-bundled `ffmpeg.exe`) into `resources/bin/`, and updated the README to list it as a required binary alongside `ffmpeg.exe`. The integration suite now also asserts `ffprobe.exe` is present in `beforeAll`.
**Hardening (done — 2026-06-23):**
`startDownload` now asserts `ffmpeg.exe` and `ffprobe.exe` presence up front ([download.ts](src/main/download.ts)), alongside the existing `yt-dlp.exe` check — a future missing binary returns a clear AeroFetch error naming the file, instead of a cryptic mid-download yt-dlp postprocessing failure. (`getFfprobePath()` added to [binaries.ts](src/main/binaries.ts).)
---
### Performance
#### P1 — `getSettings()` writes to disk on every read
@@ -305,6 +327,7 @@ The following security-critical functions lack tests:
| M1 | Maint | 15m | Trivial | Extract `cleanError` to shared module | **Fixed 2026-06-23** |
| M2 | Maint | 15m | Trivial | Document `parseExtraArgs` limitations | **Fixed 2026-06-23** |
| M3 | Polish | 30m | Trivial | Add icon asset before release | **Deferred** (pre-release TODO) |
| C1 | Correctness | 30m | High | Bundle missing `ffprobe.exe` + document it (found by smoke test) | **Fixed 2026-06-23** |
---