diff --git a/.gitignore b/.gitignore index 7d98ca0..e899c91 100644 --- a/.gitignore +++ b/.gitignore @@ -10,3 +10,6 @@ dist # Secrets — never commit .gitea-token + +# VS Code user settings (workspace settings/.vscode/launch.json are committed) +.vscode/settings.json diff --git a/.prettierrc b/.prettierrc new file mode 100644 index 0000000..baa9d29 --- /dev/null +++ b/.prettierrc @@ -0,0 +1,6 @@ +{ + "semi": false, + "singleQuote": true, + "printWidth": 100, + "trailingComma": "none" +} diff --git a/.claude/launch.json b/.vscode/launch.json similarity index 100% rename from .claude/launch.json rename to .vscode/launch.json diff --git a/CODE-AUDIT.md b/CODE-AUDIT.md index 43bd463..36cb924 100644 --- a/CODE-AUDIT.md +++ b/CODE-AUDIT.md @@ -1,342 +1,2425 @@ -# AeroFetch Code Audit +# Code Audit -**Date:** 2026-06-23 -**Scope:** Full security & correctness review of main process, preload, IPC contract, and renderer stores -**Overall:** Strong security fundamentals (context isolation, sandboxing, argument-injection defense); findings are refinements, not foundational gaps +Living checklist of audit findings for AeroFetch. Security & correctness were reviewed +2026-06-23 (all fixed — see Completed). The architectural review (2026-06-29) added the +structural items; a second polish pass (2026-06-29) added the smaller inconsistencies. +Items carry stable IDs so we can check them off as they land this session. + +Severity = structural leverage / risk of future bugs, not "app is broken" (it isn't). + +**Session 2026-06-30 (correctness/safety/strictness pass):** landed B3 B4 B5 B7 (checksum- +filename match, newline-safe meta probe, canceled-event guards, cookie-login never-resolve), +L88 L146 (queue/trim edge cases), **L168 + L169** (`noUncheckedIndexedAccess` + +`noFallthroughCasesInSwitch` now on — 0 errors after 8 real edge-case fixes), R5 (settings +write failure handled), W1 W5 W6 (min window size, seeded folder picker, parented sign-in +window), CL1 (shared stdout markers), L147 (dead macOS branches removed), M8 (StatusChip +already unified the status→label map), M36 (Library selection only counts actionable rows), +L11 (Queue header counts the live queue), L50 (no "saved" for 0 cookies), L156 + L57 +(schedule picker `min`), L159 L15 L3 (dead-code/comment cleanup), package/builder metadata +(L4 L41 L42 L43 L45 L58), user-facing copy (L66 L154; partial M37/SR9 jargon), doc +reconciliation (M25 M26 L80 L81), and new unit tests (L35 isValidMediaItem, L36 +compareVersions). All verified: typecheck + 234 tests + eslint + prettier green. + +**Session 2026-06-30 pass 2 (quality/format/UX polish):** H5 (history re-download now stores +formatId/formatHasAudio + strips compound quality labels; thumbnail also passed — L72), H6 +(TerminalView log capped at 2000 lines), M21 (skip --audio-quality for lossless flac/wav), +L166 (fmtEta hour rollover fixed in download.ts, store/downloads.ts, queueStats.ts — "2:00:00" +not "120:00"), L165 (formatSpeed precision aligned with fmtBytes), L55 (QueueItem suppresses +sizeLabel when already in probed-format quality label), L47 (probeMeta null sends empty meta +event to clear "Resolving…"), L68 (notifiedBackground resets on window show), L76 (dup warning +falls back to URL when title is placeholder), L91 (SettingsView unsafe tuple cast fixed), L98 +(embed-chapters hint added), L108 (window title set), L37 (formatters extracted to lib/formatters.ts; +fmtBytes/fmtEta/parseProgress unit-tested including NA paths), L38 (webm thumbnail exclusion +test), L39 (CROP_SQUARE_PPA structural test), L40 (looksLikeUrl/looksLikeSingleVideo extracted +to lib/urlHelpers.ts; test imports pure module directly). All verified: typecheck + 242 tests + eslint + prettier green. + +**Session 2026-06-30 pass 3 (reliability — B/R/L races & silent failures):** L140 + L148 (the `active`-map +teardown race — `cancelDownload`/`pauseDownload` release the concurrency slot synchronously and an +identity-guarded `releaseActive` + per-spawn cookie jar make same-id retry/resume safe), L141 (renderer +history capped at 500 + url de-dup to mirror main), R6 (`writeJsonAtomic` reports + logs write failures and +keeps the data dirty for retry instead of an empty catch — log half; toast → CC8), R7 (`encryptSecret` warns +before the plaintext fallback). All verified: typecheck + 243 tests + eslint green; the 5 touched files are +prettier-clean. *Deferred this pass (need UI/live-app or larger design): B2/B6 (cancel buttons + abort +wiring), R4 (can't safely identify a download's `.part` files at cancel time — needs the yt-dlp "Destination" +path tracked), L139 (needs a spawn↔auto-update interlock), L142 (multi-site IPC-return reconciliation, +generalises M34), L157 (cross-store cancel-on-remove, ties to C2), R9 (clock-skew — "note for awareness").* + +**Session 2026-06-30 pass 4 (Critical — store architecture):** **C1** (single source of truth for the +preview mock + Settings defaults — `DEFAULT_SETTINGS` in `@shared`, one typed `mockApi.ts`, centralized +`isPreview`) and **C2** (broke the `downloads ↔ sources` circular import via a pure typed event bus, +`store/coordinator.ts` — neither store imports the other now). All verified: typecheck + 243 tests + +eslint green; the touched files are prettier-clean (3 pre-existing format violations in +DownloadOptionsForm/LibraryView/SettingsView are untouched and out of scope). + +**Session 2026-07-01 (main-process structure — the H1-adjacent decompose batch):** **L2** (the ~200-line +flat IPC block moved into one `src/main/ipc.ts` `registerIpcHandlers(getMainWindow)`; index.ts is now +lifecycle + window creation and imports the shared theme helpers from ipc.ts), **L69** (path helpers split +into `src/main/paths.ts`; settings.ts is now purely the electron-store layer), **L10** (a main-process +`src/main/constants.ts` centralizes the spawn timeouts, execFile buffers, stderr tail, and the three store +caps — nine modules migrated off bare literals), **CL2** (`buildArgs` → single `BuildArgsInput` object), +**CL3** (`startDownload`'s stream/close/error/watchdog wiring extracted into `wireChildProcess`), and **L1** +(the scheduled-download promoter tick moved from store-module load into an App `useEffect` so a store import +no longer starts a stray timer). All verified: typecheck (node+web) + 253 tests + eslint + production build +green; the touched files are prettier-clean (the 8 remaining prettier warnings are all pre-existing, +untouched files — DownloadOptionsForm/SettingsView/settings/*Card). + +**Session 2026-07-01 pass 2 (the "finish the audit" batch — phase-by-phase PRs):** landed **Phase 1** +(doc/stale-checkbox reconciliation — L82, L89, plus ticks for wire-or-remove M5/M6/UX1, MAX_ENQUEUE_BATCH, +and the UX/ref duplicates whose roots already shipped), **Phase 2** (build hygiene — L83 `.blockmap` off via +`nsis.differentialPackage:false`, L84 `dist/` clean step), and **Phase 3 (main foundation)**: **CC8** +(in-house file-backed leveled logger [logger.ts](src/main/logger.ts) + renderer `log:write` sink), the +**`execFileAsync`** consolidation ([lib/exec.ts](src/main/lib/exec.ts) — the five hand-rolled spawn-and-read +wrappers in probe/ytdlp×2/ffmpeg/indexer/download.probeMeta; SIMP2, and the foundation for CC4/CC5), +**L136** (incognito is now enforced in main — no errorlog entry, no title/URL in the OS toast, no login/ +browser cookies), and **R9** (the yt-dlp update throttle no longer skips forever after a backward clock +correction). Each phase is its own branch/PR stacked on `refactor/h1-decompose-god-files`. All green: +typecheck + 263 tests + eslint + prettier. **Next:** M4 (queue persistence — its own focused PR), the +download-engine lifecycle items (R4/L65/L139/L157/B2/B6 — need a live download to verify), the CC4/CC5 +tails (SIMP5 line-buffer + SIMP16 net.request), then Phases 4–8 (renderer tokens+primitives, logic/perf, +UX/Windows, copy/a11y). Per the run's policy, subjective-visual and live-verify-only items are implemented +but left **unchecked** with a flagged list for a watched pass. + +**Session 2026-07-01 pass 3 (Batch 6 — the renderer design-token foundation):** landed the shared +design-scale module [components/ui/tokens.ts](src/renderer/src/components/ui/tokens.ts) — one home for +**SPACE** (the spacing ramp), **RADIUS** (control/surface/card tiers over Fluent's radii), **ELEVATION** +(flat/raised/overlay), **Z** (overlay/tooltip), **ICON** (16/20/24/40), **MOTION**, **SCRIM**, and +**META_SEP** — then migrated the scattered literals across 17 renderer files. Structurally-verified items +checked off: **UI2** (one 16px section gap on every screen via the view roots), **UI4** (symmetric page +padding — the lone 28px horizontal is gone), **UI5** (the intentional 4px icon-group / 8px control-row split +is now named + documented), **UI6/UI7** (one thumbnail radius = Medium; page-card XLarge / list-item Large / +control Medium tiers), **UI26** (a single motion timing + a global `prefers-reduced-motion` gate in +[base.css](src/renderer/src/assets/base.css)), **L100** (elevation scale — only the download bar (raised) and +command palette (overlay) cast a shadow; every other card stays flat/border-only), **L101** (a z-index scale +replaces the two hardcoded `1000`s), **L121** (the renderer's one raw `rgba(0,0,0,0.32)` scrim → Fluent's +theme-aware `colorBackgroundOverlay`), **L125** (icon sizes single-source from ICON), **L160** (inline margins +tokenized — values unchanged), and **L164** (one metadata separator ` • `; the intra-value ` · ` inside +compound quality labels is deliberately untouched). **UI3 and UI12 are implemented but left unchecked** for a +watched visual pass — they shift a few pixels: card padding 14/20→16 (QueueItem, Settings cards), and the +former-18px chrome glyphs (sidebar nav + theme toggles, IconButton md) → 20 plus the private badge 14→16. All +green: typecheck (node+web) + 268 tests + eslint + production build; the touched files are prettier-clean (the +9 remaining prettier warnings are all pre-existing, untouched files). + +**Session 2026-07-01 pass 4 (Batch 7 — type & color roles):** added +[components/ui/text.ts](src/renderer/src/components/ui/text.ts) — a `useTextStyles` hook with the single +`muted` secondary-text color and the shared `title` treatment, plus a header documenting the app's type-role +convention (Subtitle2 = titles/section headers · Text = row titles · Body1 = body copy · Caption1 = small +secondary text). Landed: **L110** (the one muted color, was eight aliases hint/sub/meta/srcSub/…/previewMeta — +adopted in the list views: QueueItem, HistoryView, LibraryView, Sidebar, DownloadBar, PlaylistPanel; +color-identical, so no visual change; the combined/shared-across-cards aliases — `stats`/`count`/`emptyHint`, +settings `hint`, CommandPalette, inline — migrate incrementally), **L114** (one row-title component: the +duplicated semibold-ellipsis title style is now `text.title`, and Library's three `` titles became +`` — size-identical since Fluent's base font is already `fontSizeBase300`), **L123/L124** (the +Caption1/Body1/Text roles are now written down in text.ts and followed), **UI13** (verified: `Filled` is used +only on the two brand marks, `Regular` everywhere else — the convention already held), and **UI11** (no literal +px font sizes remain — the last tile/badge glyphs snap to `ICON` tiers: Sidebar mark 20, Onboarding mark 24, +Library srcIcon 20, History emptyBadge 24; the `fontSizeBase*` token usages already satisfied the standard). +The four tile-glyph deltas (≤2px) join UI12's watched visual pass. All green: typecheck (node+web) + 268 tests ++ eslint + production build; touched files prettier-clean (9 pre-existing warnings untouched). + +**Session 2026-07-01 pass 5 (Batch 8 — form primitives):** **L112** (removed the redundant dynamic +`On`/`Off` label from all 15 ``-wrapped ``es in DownloadOptionsForm + the settings cards — the +Field already names each switch, and the toggle's position + `aria-checked` already convey state, matching +Library's switches), **L133** (made the custom [Select](src/renderer/src/components/Select.tsx) ``-aware +via `useFieldControlProps_unstable({ supportsLabelFor: true })`, so a Field's visible label names the native +`` accessibility.** QueueItem/DownloadsView + `ProgressBar`s have no accessible name; several `Select`s carry both a `` and an + `aria-label` (double announcement). +- [x] **M21 — `--audio-quality` always emitted.** `buildArgs` passes `--audio-quality` even for + lossless `audioFormat` (flac/wav), where it's meaningless; pair with M18's preset/format mismatch. +- [x] **M22 — Backup export writes secrets in cleartext, silently.** [backup.ts](src/main/backup.ts) + `exportBackup` serializes the *decrypted* settings (proxy creds, PO token, update token) to a + plain JSON file; the UI caption only says "Does not include download history" — no warning that + the file contains credentials. *Fixed: `proxy`/`youtubePoToken`/`updateToken` are stripped (set to + `''`) before writing; SettingsView caption updated to say credentials are not included.* +- [x] **M23 — `MAX_ITEMS` truncation can silently drop a source's items.** [sources.ts](src/main/sources.ts) + `replaceMediaItems` does `[...new, ...others].slice(0, 20000)`; once the global total exceeds the + cap, the *tail* (another source's items) is dropped on write and only reappears when that source + is re-indexed. Cap per-source, or surface it. +- [x] **M24 — `setSettings` accepts unvalidated free strings.** [settings.ts](src/main/settings.ts) + stores `rateLimit`/`proxy`/`defaultVideoQuality`/`defaultAudioQuality`/`youtubePlayerClient` as + any string. A bad `rateLimit` ("abc") only fails at download time; a `defaultQuality` not in + `QUALITY_OPTIONS` leaves the Settings dropdown's controlled value blank. *Fixed: `VIDEO_QUALITY_OPTIONS` + / `AUDIO_QUALITY_OPTIONS` added to `shared/ipc.ts` and imported in both `settings.ts` (allowlist + validation) and `store/downloads.ts` (single source of truth). `rateLimit` validated against yt-dlp + rate format regex; `youtubePlayerClient` trimmed.* +- [x] **M25 — ROADMAP marks unwired features as ✅ shipped.** [ROADMAP.md](ROADMAP.md) Phase C + ("Command preview … Surfaced as a **Preview command** button in the download bar") and Phase D + ("Private / incognito mode — a download bar toggle") both carry `[x]`, but neither is wired in + the UI (corroborates M5/M6). The roadmap overstates completion — reconcile the docs or finish the wiring. +- [x] **M26 — ROADMAP accent palette is wholesale stale.** [ROADMAP.md](ROADMAP.md) Phase E + describes "four accent presets — Toffee (original), Slate, Evergreen, Lavender" with a default of + Toffee; the shipped [theme.ts](src/renderer/src/theme.ts) is rose / coral / amber / teal + ("Sunset-to-sea") with a default of **teal**. The whole section documents a palette that no + longer exists. +- [x] **M27 — Library batch-enqueue ignores per-item kind.** [sources.ts](src/renderer/src/store/sources.ts) + `enqueueItems` forces `settings.defaultKind` (+ its quality) for *every* item, so a channel can't + be downloaded as audio without flipping the global default — inconsistent with the DownloadBar + playlist panel, which offers a per-item video/audio toggle. +- [x] **M28 — Primary text inputs have no accessible name.** The URL field (DownloadBar), add-source + (Library), search (History/Settings), and the Terminal args `Textarea` rely on `placeholder` only — + which is not an accessible name. Add `aria-label`/`