fix(studio-server): publish waveform caches atomically

[jrusso1020]

Waveform cache generation could follow a symlink planted at the cache filename while audio decoding was running, overwriting or creating the symlink target. Both the prewarming helper and the HTTP route now write complete JSON into a private directory beside the cache, then rename it into place. This addresses CodeQL alerts #690 and #691 and prevents readers from seeing partially written peaks.

Cache-directory symlinks are rejected before cache reuse and staging, including links planted during decoding. Directory creation is nonrecursive so dangling parent links are not traversed. The route still returns decoded peaks if caching fails; the helper propagates write errors. Cleanup is best-effort and does not hide the original error or turn a successful publication into failure. Cache keys, normal existing-cache reuse, corrupt-cache regeneration, decoding and response bodies are unchanged.

The existing Windows lane additionally runs the focused waveform tests without changing its existing test selection. All 531 Studio server tests, Studio server typecheck/build, dependency builds, lint/format and signed commit hooks pass locally. The 25 new cases cover leaf and directory symlinks, existing/dangling targets, concurrent misses, corrupt-cache replacement, partial-write/rename/cleanup failures, and caller error contracts. Four leaf witnesses fail on main; four additional directory witnesses and two cleanup cases fail on the initial PR head. Windows and CodeQL must pass before merge.

[miguel-heygen]

The two leaf writers are correctly consolidated. packages/studio-server/src/helpers/waveform.ts:100-124 gives helper and route one publication owner; staging is complete before rename, and route/helper error differences remain at their callers. The new cases convincingly cover cache-file symlinks, concurrent complete publication, corrupt-cache replacement, write/rename cleanup, and existing error contracts. I ran the focused Windows command locally (21/21) plus changed-file lint/format; exact-head CI currently has no failures, with full Test/Typecheck/Build, Windows, and JavaScript CodeQL still pending.

Blocker — a symlink at the cache directory still redirects the write outside the project. packages/studio-server/src/helpers/waveform.ts:116-123 calls mkdirSync(cacheDir, { recursive: true }), creates the “private sibling” inside that resolved directory, then renames to cachePath. If the project already contains .waveform-cache as a symlink to an outside directory, all three operations follow the parent symlink; replacing only a symlink at the final filename does not protect this shape.

I reproduced it against this exact head with the real caller-derived key: created project/.waveform-cache -> outside/, seeded outside/v2_audio.wav_<size>-<mtime>.json with untouched, then called writeWaveformCache(join(project, ".waveform-cache", key), [0.25, 1]). It returned successfully, left .waveform-cache as a symlink, and changed the outside file to [0.25,1]. The current four symlink witnesses plant cachePath, not cacheDir, so they cannot detect this.

Please make the cache-directory boundary owned/non-symlinked before staging or publishing, and add existing-target plus dangling-parent-symlink witnesses through both helper and route. The key requirement is that no cache operation can create or replace bytes outside projectDir through .waveform-cache itself.

Non-blocking: the rmSync in finally at waveform.ts:124 can mask a primary write/rename error, or make a successful publish appear failed if cleanup alone throws. The route absorbs that; the helper does not. A cleanup-failure test or primary-error preservation would make the stated error contract exact.

Verdict: REQUEST CHANGES
Reasoning: Atomic leaf replacement prevents following a planted cache-file symlink, but the same write can still be redirected through a planted cache-directory symlink and overwrite an outside file.

— Magi

[jrusso1020]

Addressed in b162649. Both callers now exclude symlinked cache directories from reuse, and the shared writer uses nonrecursive mkdir followed by lstat directory validation before staging. Existing/dangling directory symlinks and links planted during decoding are covered through both callers; no outside file is created or replaced in those cases. The route still treats cache failure as nonfatal. Cleanup now preserves the primary error and successful publication, with tests for both.

All 531 Studio server tests, typecheck/build, lint/format and signed hooks pass. Four new directory witnesses plus two cleanup cases fail on the prior head. Windows runs the expanded focused suite.

Boundary for review: this rejects planted directory symlinks at the cache entry points; lstat is not a directory-descriptor-relative sandbox against a local process concurrently renaming parent directories between filesystem calls. Please assess that limitation against the intended project-directory trust boundary when re-reviewing this head.

[miguel-heygen]

Re-review at the revised head: the cache-directory blocker and cleanup note are closed.

packages/studio-server/src/helpers/waveform.ts:108-143 now rejects directory links on both the reuse and publication paths, uses nonrecursive creation, revalidates with lstat after decoding, and stages only after the cache entry is a real directory. routes/waveform.ts:29-38 applies the same predicate before reading cached JSON, so route and prewarmer agree. The exact parent-link reproduction from my first review now throws Waveform cache must be a directory, not a symlink, preserves the outside file byte-for-byte, and leaves the link untouched.

The expanded tests cover existing/dangling directory targets and links planted during decoding through both callers. Cleanup is now explicitly best-effort: a failed cleanup neither masks the original write error nor turns an already successful publication into failure. I ran the focused Windows command (29/29) and changed-file lint/format; all pass. Exact-head check-runs have no failures—the zero-duration failures in gh pr checks are from the superseded head. Full Test/Typecheck/Build, Windows, and JavaScript CodeQL are still running and remain landing gates.

The disclosed residual boundary is acceptable: this protects against planted project entries and benign concurrent cache writers, not a hostile same-user process renaming parent directories in the syscall-sized window after lstat. Node does not provide a cross-platform directory-fd-relative rename sandbox here, and such a process can already mutate the trusted project tree and audio source directly. Keep that trust assumption explicit; do not describe this as an adversarial local-filesystem sandbox.

Verdict: APPROVE
Reasoning: Both callers now refuse planted cache-directory links before reuse or publication, the previously reproduced outside overwrite is closed, cleanup preserves error semantics, and the remaining post-lstat race is explicitly bounded by the project-directory trust model.

— Magi