fix(render): serve engine and producer files through checked descriptors

[jrusso1020]

The engine and producer render servers checked file paths, then reopened them to serve content. A replacement could change the HTML or binary response; producer range headers could also describe a different file from the streamed body. Both servers now select a regular file through one read-only descriptor and serve that same descriptor.

Compiled-file preference, project fallback, empty files, symlinked assets, MIME types and script injection remain intact. Producer HTML reads stay asynchronous, and full/ranged binary responses stay streamed. Streams own descriptor cleanup on completion, error and cancellation; HTML, HEAD, 416 and pre-stream failures close in the request. Nonblocking opens avoid FIFO hangs, and failed directory/socket opens preserve non-file fallback behavior.

Addresses CodeQL #277 and #278, including the corresponding active producer render path.

Validation: 111 tests pass (20 engine file-server, 27 producer descriptor/HTTP regressions, 64 existing producer server/timing tests), plus producer test classification. Ten replacement witnesses fail on the prior implementations across project/compiled HTML, full binary and ranged binary. Coverage includes streaming errors/cancellation, HEAD/416, fallback, symlinks, FIFO/socket handling, empty files, range/MIME/injection behavior and descriptor cleanup. Engine/producer typechecks, parser/core/lint builds, lint/format and signed commit hooks passed. Fresh CI, Windows, regression renders and CodeQL must pass before merge.

[miguel-heygen]

The changed engine path is well implemented. packages/engine/src/services/fileServer.ts:62-84 gives one descriptor ownership of open → regular-file check → read → close, keeps empty files distinct from null, and uses the pathname stat only to classify a failed open—never to authorize a later read. packages/engine/src/services/fileServer.file-race.test.ts:86-209 covers all four project/compiled × HTML/binary replacements, fallbacks, FIFOs/sockets, empty files, symlinks, injection, and cleanup. I ran both server suites (20/20), engine typecheck, and changed-file lint/format; all pass. Current exact-head CI has no failures, with Build/Typecheck/Windows/JavaScript CodeQL still pending.

Blocker — the active producer render server still has the identical check/reopen race. packages/producer/src/services/fileServer.ts:760-798 chooses compiled/project content with existsSync + statSync, then reopens the pathname with readFile for HTML. Its binary path checks size at :823-826, then reopens again with createReadStream at :845 or :862; a replacement can therefore serve bytes from a different inode, and a range response can pair one file's size/headers with another file's body. This is not a dormant cousin: renderOrchestrator.ts:2621-2630, distributed/renderChunk.ts:699-710, and render/stages/probeStage.ts:296-302 invoke that separate createFileServer for actual render work. The new four witnesses exercise only the engine implementation, so the production render route remains exposed to the same root contract violation.

Please apply the checked-descriptor ownership to the producer server too (or centralize a primitive that preserves its async HTML and descriptor-backed full/range streaming semantics), with replacement and close/error coverage for HTML, full binary, and ranged binary responses.

Verdict: REQUEST CHANGES
Reasoning: The exact engine implementation is correct, but the independently implemented server used by the render pipeline still checks one pathname target and serves another, leaving the same security bug in the active sibling path.

— Magi

[jrusso1020]

Addressed the producer sibling in the latest revision. Its candidate selection now opens and fstats one descriptor, uses that descriptor for asynchronous HTML reads and full/range streams, and derives range headers from the same fstat. Streaming remains asynchronous; each descriptor has an explicit request or stream owner. HEAD and 416 close without constructing a stream, and failure/cancellation paths are covered.

Added 27 producer regression tests, including six HTML/full/range replacement witnesses that fail on the prior producer implementation and coverage for stream errors, cancellation, creation/conversion failures and bodyless responses. All 111 engine/producer server and timing tests, both typechecks and test classification pass. The new real-HTTP suite is registered in the producer integration lane. Please re-review the updated head; fresh CI remains a merge gate.

[miguel-heygen]

Re-review at the revised head: the producer blocker is closed.

packages/producer/src/services/fileServer.ts:730-755 now returns ownership only after the opened descriptor itself is proven regular, closing every rejected candidate. The handler preserves compiled-first/project fallback and lexical containment at :801-813; HTML reads asynchronously from that descriptor at :821-836; and binary range metadata comes from its fstat while createReadStream receives the same fd at :852-890. The ownership handoff is explicit and balanced: the request closes HTML, HEAD, 416, stat/read/construction failures, while the stream closes on completion, error, conversion failure, and cancellation (:816-898). No check-then-pathname-read remains in either changed server.

The added real-HTTP suite is substantive: fileServer.file-race.test.ts:106-138 covers project/compiled × HTML/full/range replacements; :250-333 covers stream errors, construction/conversion failures, client cancellation, HEAD/416, and HTML stat/read failures. It is registered in the producer integration lane. I independently ran all 111 advertised engine/producer server and timing tests, both package typechecks, classification, and changed-file lint/format; all pass. The apparent zero-duration failures in gh pr checks belong to the superseded head; exact-head check-runs have no failures. Fresh Build, integration, Windows, regressions, and JavaScript CodeQL are still running and remain landing gates.

Non-blocking residual: as with the caption reader, one descriptor pins inode identity but not immutable bytes. Concurrent in-place mutation can still make fstat-derived length/range headers diverge from the eventual stream. That limitation predates this PR and does not reopen the pathname-replacement defects addressed here.

Verdict: APPROVE
Reasoning: The active producer render path now serves the exact descriptor it checked, with correct request-versus-stream cleanup across every response and failure mode, and the focused tests independently verify those ownership boundaries.

— Magi