[michaelwang11394]

What

Hardening for the producer and engine render paths:

• Build the render job before a render slot is taken, so a rejected request returns 400 and frees its temporary project instead of holding the slot. workers is validated up front (0, fractional or negative values now return 400 instead of 500).
• Write compiled sub-compositions only inside the compiled directory, whatever query or fragment suffix data-composition-src carries.
• Map media element ids to a single safe path segment before they name frame directories or audio work files. Ordinary ids keep their current paths. Ids containing /, \ or : get a generated name: a readable part capped at 64 UTF-8 bytes, then ~ and a 12-character digest of the whole id. A plain id that already ends like a generated name (~ plus 12 hex characters) also gets a generated name, so a plain id can never share a folder or work file with an unsafe one. Generated names stay within the 255-byte filename limit for long or multibyte ids.
• The engine file server listens on 127.0.0.1 by default, matching the producer file server, with an opt-in hostname option.
• The standalone render server refuses POSTs that a browser sends from a page on another origin. The browser's Sec-Fetch-Site header decides when it is present, so a same-origin page behind a reverse proxy that rewrites Host still works. Browsers without that header fall back to comparing Origin with Host. Requests without an Origin header (curl, server-to-server) are unchanged, and the server never sent CORS headers, so no working cross-origin client exists. Embedders that mount createRenderHandlers on their own app are not affected.

Why

Each change keeps untrusted request or composition input from reaching paths, slots or listeners it should not. Valid compositions render the same.

Related work

The <script src> inlining guard is handled separately in #4599.

How

Small, local changes in server.ts, render/shared.ts, the media path helpers and the engine file server. One existing planV2.test.ts case expected an id like ../escape to be rejected; that id is now kept inside the frame folder instead, so the case was updated.

Test plan

• Unit tests added/updated: producer 316 (vitest) + 120 (bun), engine 150 pass for the touched areas. Each new test fails with its fix removed.
• Typecheck passes for engine and producer; lefthook hooks and the comment ratchet pass.
• fails without the fix: mediaIdPathSegment > never gives a plain id the name generated for another id, on disk > keeps the audio work files of a colliding pair apart, on disk > names a frame folder and work file for a long multi-component id, on disk > names a frame folder and work file for a long multibyte id, shown by running mediaPathSegment.test.ts against the previous helper (4 failed, 15 passed). A reviewer separately confirmed that the cross-origin, workers, render-slot, sub-composition (writes sub-compositions only inside compileDir) and loopback (answers on loopback and not on another interface) tests go red with the base server.ts, render/shared.ts and engine fileServer.ts restored.
• Integration lanes and the full-render regression harness were not run locally.
• Media ids that contain /, \ or : now get a different frame folder name. A distributed plan written by this version and read by an older chunk worker (or the reverse) would not find the frames for such ids. Plain ids keep their paths, so this only matters during a mixed-version rollout.
• 10 engine video-sampling tests fail locally on unmodified main too (local ffmpeg 8.0.1); this change adds no failures.

🤖 Generated with Claude Code

[github-actions]

Edit accuracy: accurate 267 (base branch 267), smooth 102 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (8)

• seqplainkeys-none-center-r0-nested-z100 (fixed by studio: undo/redo on a keyframed plain element sometimes leaves the box 230-400px off (edit-accuracy seqplainkeys-none-*) #6989): pass, passes
• seqplainkeys-none-center-r0-root-z100 (fixed by studio: undo/redo on a keyframed plain element sometimes leaves the box 230-400px off (edit-accuracy seqplainkeys-none-*) #6989): fail / pass / pass, passes
• seqplainkeys-none-center-r30-root-z100 (fixed by studio: undo/redo on a keyframed plain element sometimes leaves the box 230-400px off (edit-accuracy seqplainkeys-none-*) #6989): not run
• seqplainkeys-none-pct-r0-root-z100 (fixed by studio: undo/redo on a keyframed plain element sometimes leaves the box 230-400px off (edit-accuracy seqplainkeys-none-*) #6989): pass, passes
• seqplainkeys-none-pct-r30-root-z100 (fixed by studio: undo/redo on a keyframed plain element sometimes leaves the box 230-400px off (edit-accuracy seqplainkeys-none-*) #6989): not run
• seqplainkeys-none-px-r0-nested-z100 (fixed by studio: undo/redo on a keyframed plain element sometimes leaves the box 230-400px off (edit-accuracy seqplainkeys-none-*) #6989): pass, passes
• seqplainkeys-none-px-r30-nested-z100 (fixed by studio: undo/redo on a keyframed plain element sometimes leaves the box 230-400px off (edit-accuracy seqplainkeys-none-*) #6989): not run
• seqplainkeys-none-px-r30-root-z100 (fixed by studio: undo/redo on a keyframed plain element sometimes leaves the box 230-400px off (edit-accuracy seqplainkeys-none-*) #6989): not run

[jrusso1020]

Reviewed at 38b41067f794a0753a6a028da0ac9b938b532614.

Important: generated media path segments need a disjoint, bounded namespace. packages/engine/src/utils/mediaPathSegment.ts:8-11 maps a/b to a_b~c14cddc033f6, but a distinct authored ID literally equal to a_b~c14cddc033f6 passes through unchanged. Both now use the same audio scratch files and frame directories. For example, audioMixer's ${mediaIdPathSegment(element.id)}-trimmed.wav writes the second clip over the first. This is deterministic and does not require finding a hash collision. The prior two paths were distinct.

The same generated-name branch keeps the entire readable ID: three 100-character components separated by / become one 315-byte filename. On this filesystem the prior nested directories can be created, while the new path throws ENAMETOOLONG. This adds a render failure for an otherwise representable ID.

Fix the helper with a reserved generated prefix (escape/hash any otherwise-safe ID that uses that prefix too), and bound the readable portion by filename bytes before appending the digest. Alternatively enforce unique mapped names before any writes, but retaining distinct names is preferable. Keep all existing writer/reader call sites on this single helper. Add tests for the generated-name-as-another-ID pair, long multi-component IDs and multibyte readable names.

Evidence on Node v22.22.3 through pytest-gate.sh: all 15 added mediaPathSegment tests pass, but two controlled probes using the actual transpiled helper fail. The first printed mappedA: a_b~c14cddc033f6, mappedB: a_b~c14cddc033f6, samePath: true, firstNow: second audio after real temporary-file writes. The second printed baseline nested id: success; mapped segment byte length: 315, then ENAMETOOLONG. These are helper/consumer-path probes, not a full render run. Independent adversarial source review confirmed the naming is otherwise propagated through extraction, deferred fill, compiled staging and distributed readers.

The job-construction cleanup, workers validation and compiler containment look sound. Pre-existing follow-up: the browser guard compares the Origin host with Host; it does not solve DNS rebinding or distinguish schemes. That does not block the added refusal of ordinary cross-origin requests.

Need: yes, confine render paths and release rejected jobs · Size: ok · Reuse: existing isPathInside and shared render preparation · Simpler: fix the single naming helper · Structure: fine (checked shared media writers/readers and render handlers; no split)
Risk: medium - path aliases can replace one clip's scratch output with another.

Verdict: REQUEST_CHANGES.
Reasoning: make generated media names distinct and filesystem-safe before using them across all render paths.

Reviewed by hf-review.

[heygengenesis]

Verdict: REQUEST CHANGES · 0 new blockers · 2 important (open from @jrusso1020) · 1 nit · head f4339e9

+1 with @jrusso1020. Both of their mediaIdPathSegment findings are still open at this head: mediaPathSegment.ts hasn't changed since 38b4106, and the only later commit touches server.ts and its test.

• important packages/engine/src/utils/mediaPathSegment.ts:7-11: a generated name for an unsafe id can equal a plain id. Ids a/b and a_b~c14cddc033f6 both map to a_b~c14cddc033f6, so the second clip's -trimmed.wav and frame dir overwrite the first (checked with a probe of the helper). Fix: reserve the generated form, so a plain id that matches it also takes the hash branch. Add a test for this pair.
• important same lines: the readable part has no byte cap. x*100/y*100/z*100 produces a 315-byte segment, and 视频×30/视频×30 produces 374 bytes. Both throw ENAMETOOLONG where the old nested path worked. Fix: truncate by UTF-8 bytes before the digest.
• nit: the body has no fails without the fix: line naming tests. I checked by putting back the base server.ts, render/shared.ts and engine fileServer.ts: the new cross-origin, workers, slot, sub-composition and loopback tests go red on base and green on head.

Scope

I read the full diff. Cross-origin guard, render-slot ordering (both acquire sites), sub-composition containment and the engine 127.0.0.1 bind all hold. Every media-id path site goes through the helper. Ran engine vitest 49/49, producer vitest 313/313 and producer bun planV2 68/68. CI is green.

— Genesis

[michaelwang11394]

Thanks, jrusso1020 and Genesis. Replies to each finding:

• Generated name can equal a plain id (important): fixed in 110d3d1. A plain id that already ends like a generated name (~ plus 12 hex characters) now takes the hashed branch too, so a/b and a_b~c14cddc033f6 map to different names. The pass-through names and generated names no longer overlap. Tests: never gives a plain id the name generated for another id and on disk > keeps the audio work files of a colliding pair apart. The second one is the overwrite repro, and it fails on the previous helper.
• No byte cap on the readable part (important): fixed in 110d3d1. The readable part is cut at 64 UTF-8 bytes on a character boundary before the digest. Tests: names a frame folder and work file for a long multi-component id (x*100/y*100/z*100) and ... for a long multibyte id (视频×30/视频×30). Each one creates the folder and a -extracted.wav file on disk and checks the work-file name stays within 255 bytes. Both fail on the previous helper. All call sites still go through the one helper.
• Missing "fails without the fix" line (nit): fixed. The PR body now names the four new tests and how they were shown to fail.
• Origin vs Host does not cover DNS rebinding (pre-existing follow-up): not changed in this PR. Since f4339e9, the guard trusts the browser's Sec-Fetch-Site header when it is present. That keeps a same-origin page behind a proxy that rewrites Host working. It still does not address DNS rebinding.

[heygengenesis]

Verdict: APPROVE — 2 resolved, 0 open, 0 new · f4339e9 → 110d3d1. A plain id that ends in ~<12 hex> now goes through the hash branch, so plain and generated names can no longer meet. The readable part is capped at 64 UTF-8 bytes on code-point boundaries. Probe: a/b, a_b~c14cddc033f6, 3×100-char, CJK, emoji, .., . and empty ids all give distinct segments of 77 bytes or less, and mkdir works for each. The 4 new tests fail on the previous helper; 19/19 pass at head. All CI is green, Windows included.

— Genesis

[jrusso1020]

Reviewed at 110d3d18d4cbe55ef9a50107488d2e2e7093d3e7. Delta since my previous review, including the follow-up browser-header change, read.

Both prior naming concerns are resolved in packages/engine/src/utils/mediaPathSegment.ts: generated-form IDs enter the same mapping branch, and the readable generated portion is capped by UTF-8 bytes on character boundaries. Writer/reader call sites continue to share this helper. The server predicate handles browser site metadata and retains the Origin/Host fallback.

Validation through pytest-gate.sh, Node 22.22.3: mediaPathSegment.test.ts 19/19 pass, original filesystem probes now preserve separate outputs and create the formerly overlong generated directory (77 bytes), and 9 actual server-predicate cases pass (same-origin/proxy accepted; cross-site, same-site and unknown metadata refused; absent-metadata Origin/Host and malformed-Origin cases checked). The native cross-origin route suite could not load because this checkout's reused dependencies lack hono; the predicate probe is not an HTTP integration test. No full render or mixed-version worker run.

Overlap check: all 143 open PRs and their paginated file lists inspected. Shared-file patches in #7083, #6612, #5820, #5815, #5698, #5334 and #4599 address different behavior; none duplicates or changes the naming helper or browser predicate. Same-file work will still need normal integration/rebase care.

Pre-existing follow-up: plain IDs still retain their original filenames, including platform and length constraints. The mixed-version naming limitation remains documented in the PR.

Need: yes, bounded and consistent render paths · Size: ok · Reuse: existing shared media naming helper and render app predicate · Simpler: already minimal · Structure: fine (checked extraction, staging and distributed readers; no split)
Risk: medium - shared scratch paths and standalone request handling; full render integration not exercised locally.

Verdict: APPROVE
Reasoning: Both previous blockers are fixed and their original filesystem probes now pass. No new blocker found in the delta.

Reviewed by hf-review.