Skip to content

Harden sort pipeline and audio endpoint against traversal/OOM/data-loss

jedmund requested to merge jedmund/codebase-review into main

A codebase review surfaced three high-severity issues; this MR fixes all three. Commits are split per issue.

1. Path traversal via `category` (95f94c53)

`/api/preview` accepted an unvalidated `category` that flowed into `formatTargetPath` → `path.join(SORTED_PATH, ...)`. A request with `category: "../../something"` combined with `overwriteExisting: true` could `rm -rf` and write outside the music library.

  • New `VALID_CATEGORIES` / `isValidCategory` in `src/lib/types/index.ts`.
  • `formatTargetPath` now throws on any non-enum category and on empty artist/album.
  • `/api/preview` POST validates `albumId`, `artistName`, `albumName`, year/month/day, `category`, and `overwriteExisting` for all three actions (`preview`, `execute`, `batch`) and returns 400 on bad input.

2. Sort execution containment + copy verification (676acf30)

`executeSort` previously compared only the source/destination file counts and then unconditionally deleted the source. A copy that lost data but happened to match file count → permanent data loss.

  • New `resolveAndValidateTargetPath` in `sorter.ts` resolves the relative path under `SORTED_PATH` and throws if it escapes. Used by both `previewSort` and `executeSort` as defense-in-depth.
  • New `getFileManifest` / `diffManifests` in `fileUtils.ts` build `{relativePath → size}` maps. `executeSort` now compares manifests after `fs.cp` and only deletes the source on an exact match. Mismatches are logged with samples of `missing`/`extra`/`sizeMismatch` and the partial destination is cleaned up. `executeSortWithRetry` will pick up transient failures.

3. Audio endpoint OOM (53dea201)

`Range: bytes=0-9999999999999` → `Buffer.alloc(1e13)` → process crash. Full-file responses also loaded entire FLACs into memory.

  • Strict `bytes=N-M` / `bytes=N-` / suffix `bytes=-N` parsing.
  • 416 with `Content-Range: bytes */` on anything malformed or out of range.
  • Served chunk clamped to 8 MiB (`MAX_RANGE_BYTES`); browsers re-issue subsequent ranges transparently.
  • Both range and full-file branches now stream via `fs.createReadStream` + `Readable.toWeb` instead of allocating buffers.

Verification

  • `pnpm run build` passes locally. (`pnpm check` blocked locally by Node 26 / better-sqlite3; Docker uses Node 22, so this will run on deploy.)
  • Suggested end-to-end checks before merging:
    • `curl -X POST /api/preview` with `category: "../../etc"` returns 400.
    • Sort a real album with `SORTED_PATH` pointed at a scratch dir; verify source is removed on success and preserved on simulated copy failure (e.g. pre-create a wrong-size file at the destination).
    • `curl -H 'Range: bytes=0-9999999999999' /api/audio/` returns a 206 with ≤ 8 MiB body (not OOM); `Range: bytes=abc` returns 416; in-app audio playback and seek still work.

Merge request reports