Security, perf, a11y & CI hardening from end-to-end audit

kleb commented

2026-06-23 22:14:23 +02:00

Owner

Remediation from the end-to-end audit. 26 commits, one per fix, each validated
with typecheck + lint (+ test where applicable). Branch builds on
electron-port.

P0 — release / ship-blockers

publish.owner + win.publisherName set to kleb (was the literal
PLACEHOLDER_OWNER, which pointed the auto-updater at an unclaimed GitHub org).
New ci.yml + a gating check job in release.yml (typecheck/lint/test);
both fail if PLACEHOLDER_OWNER reappears. bun-version pinned.
Production CSP now enforced via a build-only injected <meta> tag (header-only
CSP never applied to file://). Dev/HMR untouched.
Filename-format template path-traversal closed in both builders.
macOS hardened-runtime entitlements plist (was killing bundled ffmpeg/ffprobe).
Vitest + 20 unit tests (sanitize/path-safety, Spotify-URL parser, rate-limit).

P1 — security / correctness / perf

SSRF guard (assertPublicHttpUrl + beforeRedirect re-validation, maxRedirects:5)
on cover / Qobuz / Amazon / Tidal media + segment fetches. Localhost custom-API
path unaffected.
OpenFolder directory-only via realpath; ReadTextFile/ReadImageAsBase64
realpath-resolve before the extension gate.
Destructive library ops (delete/move/deleteEmptyFolders) confined to the
last in-process scan's results and symlink-rejected.
safeStorage plaintext-fallback now warns once; settings.json written 0600;
Amazon gateway filenames use the full sanitizer.
base-cache: drop per-write COUNT(*), memoize prepared statements, quarantine
- recreate a corrupt DB, sweep expired rows at startup; external-DB handle reuse.
Dependabot for both workspaces + actions.

manualChunks fix (framework chunk 306→214 kB), stable TrackList keys,
QueueItem React.memo + stable onRemove.
Global :focus-visible; progress-toast / sidebar (aria-current) / spectrogram
(role=img) a11y; SettingsPage header aligned to the page-header pattern.
Download-queue control errors now surface as toasts; inline Spotify-URL validation.
Disk log sink (rotating) + local-only crashReporter (no uploads).

Deferred (need GUI/runtime verification; too risky to land blind)

Push-based IPC migration, download state-machine rewrite, list virtualization,
React Compiler, the 5-file type-contract collapse, FFT worker pooling, partial-file
.part restructure, double-ISRC-rescan removal, cache request-coalescing /
negative-caching / schema-versioning.

Verify before release

Packaged script-src 'self' meta CSP needs a real GUI smoke test (file://
origin) — could not be verified without a desktop runtime.

Remediation from the end-to-end audit. 26 commits, one per fix, each validated with `typecheck` + `lint` (+ `test` where applicable). Branch builds on `electron-port`. ## P0 — release / ship-blockers - `publish.owner` + `win.publisherName` set to `kleb` (was the literal `PLACEHOLDER_OWNER`, which pointed the auto-updater at an unclaimed GitHub org). - New `ci.yml` + a gating `check` job in `release.yml` (typecheck/lint/test); both fail if `PLACEHOLDER_OWNER` reappears. `bun-version` pinned. - Production CSP now enforced via a build-only injected `<meta>` tag (header-only CSP never applied to `file://`). Dev/HMR untouched. - Filename-format template path-traversal closed in both builders. - macOS hardened-runtime entitlements plist (was killing bundled ffmpeg/ffprobe). - Vitest + 20 unit tests (sanitize/path-safety, Spotify-URL parser, rate-limit). ## P1 — security / correctness / perf - SSRF guard (`assertPublicHttpUrl` + `beforeRedirect` re-validation, `maxRedirects:5`) on cover / Qobuz / Amazon / Tidal media + segment fetches. Localhost custom-API path unaffected. - `OpenFolder` directory-only via `realpath`; `ReadTextFile`/`ReadImageAsBase64` realpath-resolve before the extension gate. - Destructive library ops (`delete`/`move`/`deleteEmptyFolders`) confined to the last in-process scan's results and symlink-rejected. - safeStorage plaintext-fallback now warns once; `settings.json` written `0600`; Amazon gateway filenames use the full sanitizer. - `base-cache`: drop per-write `COUNT(*)`, memoize prepared statements, quarantine + recreate a corrupt DB, sweep expired rows at startup; external-DB handle reuse. - Dependabot for both workspaces + actions. ## P2 / P3 — renderer / UI / a11y / UX / observability - `manualChunks` fix (framework chunk 306→214 kB), stable `TrackList` keys, `QueueItem` `React.memo` + stable `onRemove`. - Global `:focus-visible`; progress-toast / sidebar (`aria-current`) / spectrogram (`role=img`) a11y; SettingsPage header aligned to the page-header pattern. - Download-queue control errors now surface as toasts; inline Spotify-URL validation. - Disk log sink (rotating) + local-only `crashReporter` (no uploads). ## Deferred (need GUI/runtime verification; too risky to land blind) Push-based IPC migration, download state-machine rewrite, list virtualization, React Compiler, the 5-file type-contract collapse, FFT worker pooling, partial-file `.part` restructure, double-ISRC-rescan removal, cache request-coalescing / negative-caching / schema-versioning. ## Verify before release Packaged `script-src 'self'` meta CSP needs a real GUI smoke test (`file://` origin) — could not be verified without a desktop runtime.

kleb added 26 commits

2026-06-23 22:14:23 +02:00

fix(release): set update publish owner and Windows publisherName to kleb cdaa1acded

The publish owner was the literal PLACEHOLDER_OWNER (baked into shipped
app-update.yml), which points electron-updater at an unclaimed, attacker-
registerable GitHub org -> repo-takeover RCE. win.publisherName was unset,
disarming the Windows Authenticode publisher check on updates.

fix(security): enforce production CSP via build-injected meta tag 3f149d77f4

The CSP delivered only through onHeadersReceived does not apply to file://
document loads, so packaged builds shipped with no enforced CSP. Inject a
<meta http-equiv> CSP at build time (dev/HMR untouched) mirroring the
window.ts header policy.

Note: needs a packaged smoke test to confirm script-src 'self' resolves
sibling assets under file:// (cannot be verified without a GUI runtime).

fix(security): strip path separators from filename-format templates a1ac80cccf

buildExpectedFilename and generateFilename interpolated the user/settings-
controlled format template verbatim while sanitizing only the substituted
values, so a custom format like '../../{title}' escaped the output directory
via path.join(). Strip separators from the composed name; built-in presets
are unaffected.

fix(build): add macOS hardened-runtime entitlements f2aa16a5c1

hardenedRuntime was enabled with no entitlements plist, which kills the
bundled ffmpeg/ffprobe child processes (and JIT) at launch on notarized
builds. Add entitlements (allow-jit, allow-unsigned-executable-memory,
disable-library-validation), wire mac.entitlements/entitlementsInherit, and
narrow the build/ gitignore so the static plist is tracked while generated
icons stay ignored.

test: add Vitest with unit tests for path-safety and parsers 10510d2906

Adds Vitest (none existed) and 20 tests covering the highest-risk pure
logic: sanitizeFilename/sanitizeFolderPath/buildExpectedFilename (including
the filename-template traversal guard), parseSpotifyURI (host allowlist),
and the rate-limit backoff/Retry-After helpers.

ci: add quality gate and block releases on placeholder owner 60942ccce8

Adds ci.yml (typecheck + lint + test hard gates on push/PR; audit/format/
knip informational) and a gating 'check' job in release.yml that the build
job now needs:. Both fail if electron-builder.yml still contains
PLACEHOLDER_OWNER. Pins bun-version (was 'latest') for reproducibility.
Previously a v* tag published signed installers with zero checks.

fix(security): add SSRF guard for provider-supplied URLs eae25c5812

Provider/song.link/cover/manifest responses are attacker-influenced and their
URLs were fetched with no scheme/host validation, and got followed redirects
with no re-validation. Add assertPublicHttpUrl() (https/http only; reject
loopback/private/link-local incl. 169.254.169.254) at the cover, Qobuz/Amazon
stream, and Tidal media/segment fetch sites, plus a global beforeRedirect
re-check and maxRedirects:5. The user's localhost custom Tidal API path is
unaffected (it does not flow through these media fetches).

fix(security): harden OpenFolder and file-read IPC gates aacdd15622

OpenFolder passed any string to shell.openPath, which shell-executes
.exe/.bat/.lnk/documents; now resolve symlinks and require an actual
directory. ReadTextFile/ReadImageAsBase64 gated only on the raw extname,
bypassable via symlink/NTFS-ADS/trailing-dot; now realpath-resolve first and
read the resolved target.

fix(security): confine destructive library ops to last-scan results ee74be6d6d

deleteFilteredTracks/moveFilteredTracks/deleteEmptyFolders acted on arbitrary
renderer-supplied path lists with no root scoping and no symlink guard, so a
compromised renderer could delete/move/recursively-remove any file the user
can. Restrict each op to paths the most recent in-process scan actually
returned, and refuse symlinked targets (resolved absolute-path allowlist).

fix(security): warn on unencrypted token fallback; 0600 settings.json fd4132c9d0

encodeTokenData silently stored Tidal tokens in plaintext when safeStorage
had no backend (e.g. Linux without a keyring); now warn once (without token
contents). settings.json holds the Spotify client secret and was written
world/group-readable; write it 0600 (+ chmod on POSIX).

chore(deps): add Dependabot for npm workspaces and actions 8baf3d59ac

No update automation existed; weekly grouped minor/patch PRs for the root
and frontend workspaces plus GitHub Actions. Pairs with the bun audit step
added to CI.

perf(cache): drop per-write COUNT(*) and memoize prepared statements 9ab3bc4b89

updateTotal() ran an O(rows) COUNT(*) on every cache insert, blocking the
synchronous node:sqlite engine on the main thread during downloads — yet the
value is never read (getStats recomputes it live). Replace with a timestamp
bump. Add a prepared-statement cache so the constant per-instance SQL is not
re-parsed/re-planned on every hit.

fix(cache): quarantine and recreate a corrupt cache DB 899124c930

new DatabaseSync + schema exec were unguarded, so a corrupt cache file
(power loss mid-WAL-checkpoint, disk error) threw on first access and broke
every dependent feature with no self-heal. Quarantine the file (+ WAL
sidecars) and recreate once on open failure.

fix(cache): sweep expired entries at startup bea11c2535

cleanExpired() ran only on explicit user action, so expiry bounded freshness
but never disk size and dead rows grew without bound. Sweep the expiring
caches (track/album/artist/songlink) once on app startup.

perf(db): reuse one read-only handle for the external Spotify DB f6d4d33b6c

Every ISRC/cover lookup opened and closed the user DB (re-reading headers and
re-parsing the schema), called per-track in ISRC-fallback and the 10-worker
library-verification loop. Cache one read-only handle keyed by path.

fix(security): sanitize Amazon gateway filenames with full sanitizer 57078ae267

The gateway-supplied track/artist names used a separator-only stripper that
let through control chars, Windows reserved device names, and trailing dots.
Use sanitizeFilename instead and drop the redundant stripGatewayFilenameChars.

perf(build): fix manualChunks so lucide-react lands in the ui chunk 94e3759689

id.includes('react') matched 'lucide-react' first, pulling the icon set into
the stable framework chunk. Match @radix-ui/lucide-react before the react
test so an icon-set bump no longer busts the framework chunk.

fix(a11y): add global :focus-visible fallback d04af25919

Focus visibility was opt-in per component, so controls without an explicit
ring (frameless titlebar buttons, command-palette rows, bare buttons) showed
no keyboard-focus indicator. Add a global outline fallback (WCAG 2.4.7).

fix(ux): surface download-queue control errors as toasts a5c0d1ef80

Pause/Resume/Stop/Retry/Clear/Remove swallowed failures to console.error, so
a failed action looked like it did nothing. Show a toast.error on failure
(console.error kept for debugging).

fix(perf): key TrackList rows by stable id, not array index 7f359150f5

With key={index}, sorting/filtering the track list re-associated row state
and cover <img> to whatever track moved into each slot, forcing full subtree
re-renders and cover reloads. Key by isrc/spotify_id/coverKey instead.

fix(ui): align SettingsPage header with the page-header pattern d0c1a8cd84

Settings rendered its title in the body font at font-bold with no kicker,
unlike every sibling page (font-display, font-semibold, spec kicker). Match
the canonical header so it looks like part of the redesign.

fix(a11y): name the progress toast and expose nav state 98faaf14d8

DownloadProgressToast button had no accessible name (icon + MB number only);
add aria-label, role=progressbar with aria-value*, and aria-hidden on the
decorative icons. Sidebar nav now sets aria-current=page on the active item
and labels the nav landmark.

feat(observability): persist logs to disk and capture local crash dumps 2fa11167d0

Logs lived only in a RAM ring buffer, lost on the crash you most need to
debug, and native crashes produced no artifact at all. Add a best-effort
rotating disk sink (userData/logs/main.log, injected so logger.ts stays
electron-free/testable) and start crashReporter in local-only mode
(uploadToServer:false) for minidumps. No data leaves the machine.

fix(ux): validate Spotify URL before fetch d119d3d8a7

A malformed or wrong-service URL was sent to the backend and surfaced as a
raw API error after a round-trip. Reject non-spotify.com http(s) links inline
with a clear message; spotify: URIs and bare ids still pass through.

perf(renderer): memoize QueueItem and stabilize its onRemove 9cfe486137

The queue list re-rendered every row on each poll tick (~5/s during a
download) even when only one item changed. Wrap QueueItem in React.memo and
useCallback the parent's onRemove so only changed rows reconcile. Also label
the remove button (aria-label) and hide its decorative icon.

fix(a11y): add text alternative to the spectrogram canvas

CI / check (pull_request) Waiting to run

Details

e003d3165f

The analyzer's primary output was an unlabeled <canvas> (axis labels painted
into the bitmap, invisible to assistive tech). Add role=img with an aria-label
summarizing sample rate, bit depth, and frequency ceiling.

kAgent commented

2026-06-23 22:14:25 +02:00

Collaborator

kReview review

Verdict: no findings

No findings to address in the reviewed diff.

Overall risk is low; no concrete correctness or security regressions were verifiable from the provided diff and context.

Excluded as generated or vendored (not reviewed): build/entitlements.mac.plist, bun.lock.

Reviewed by kReview at 2f91e1a92a. This comment is conservative and based only on the PR diff, metadata, and supplied repository context.

Est. cost ~$0.51 total (97.7k in / 1.3k out) · this run ~$0.25 (49.0k in / 557 out) / gpt-5.5.

## kReview review **Verdict:** no findings No findings to address in the reviewed diff. Overall risk is low; no concrete correctness or security regressions were verifiable from the provided diff and context. _Excluded as generated or vendored (not reviewed):_ `build/entitlements.mac.plist`, `bun.lock`. _Reviewed by kReview at `2f91e1a92a`. This comment is conservative and based only on the PR diff, metadata, and supplied repository context._ _Est. cost ~$0.51 total (97.7k in / 1.3k out) · this run ~$0.25 (49.0k in / 557 out) / gpt-5.5._

kleb added 1 commit

2026-06-23 22:15:54 +02:00

fix(db): reopen external DB when the file at the same path changes

CI / check (pull_request) Has been cancelled

Details

2f91e1a92a

Address PR review: the read-only handle cache keyed only by path string would
keep serving a stale connection if the user rewrites/replaces the SQLite file
at the same path (testDatabaseConnection is exactly that entry point). Key the
cache by path + mtime/size and reopen on change; stat stays far cheaper than
the open + schema-parse it replaces.

kleb referenced this pull request

2026-06-24 00:53:56 +02:00

Architectural & perf rewrites: push IPC, React Compiler, virtualization, worker pool #4

kleb closed this pull request

2026-06-24 08:41:56 +02:00