fix(engine): reject stdin input, decode stdout correctly, bound its size

[vanceingalls]

Stack 5/6, on top of #2915. The - case is the residual hole in #2740's -- fix; the stream handling is pre-existing.

A filePath of exactly - hangs for 30 seconds

-- stops option parsing, so -intro.mp4 is safe. But ffprobe rewrites - to fd: after option parsing and then reads stdin — and spawn passed no stdio, so the child's stdin was a pipe the parent never writes to and never ends.

Verified against current main:

input	outcome
extractMediaMetadata("-")	throws after 30010 ms, ffprobe deadline with code null:
normal missing file	throws after 28 ms with a real message

The diagnostic is empty because ffprobe never errored, so stderr is blank. Rejected up front now, and the child gets stdio: ["ignore", "pipe", "pipe"] so no future invocation can block on stdin either.

The test added in #2740 covers /tmp/-dangerous-name.mp4, which -- genuinely protects. Bare - was the untested gap.

stdout was decoded per chunk

stdout += data.toString();

Each 64 KiB pipe chunk decodes independently, so a multi-byte character straddling a boundary becomes U+FFFD on both sides. Reproduced: 200,001 bytes of 3-byte characters produced 15 replacement characters and a string 9 chars longer than the source. -show_format output above ~64 KiB with non-ASCII tag text returns silently mangled values — JSON.parse still succeeds, so nothing surfaces it. Now accumulated through StringDecoder.

On testing this one: U+FFFD is valid JSON string content, and nothing on extractMediaMetadata's public surface exposes a tag value, so there is no assertion that fails against the old implementation. Rather than add a test that cannot fail, it is documented and the bound below is what the new test covers. Happy to export a seam if a reviewer would rather have it pinned.

stdout was unbounded

stderr is capped by ManagedChildProcess; stdout was not. analyzeKeyframeIntervals emits one line per frame, so an all-intra ProRes proxy can produce an arbitrarily large string. Capped at 8M characters — real -show_streams JSON is nowhere near it.

Verification

- rejected without spawning, the stdio shape, and the size bound. Reverting the stdin guards fails 1. Engine suite: 1280 pass.

My first draft of the bound checked before appending, so a single oversized chunk passed straight through — the test caught it.

🤖 Generated with Claude Code

[miga-heygen]

Stack 5/6: stdin rejection, StringDecoder, stdout bound — Review

Clean fix, no concerns.

Verified

• filePath === "-" rejected before spawn — correct. -- doesn't help here because ffprobe rewrites - to fd: after option parsing. The error message is clear: stdin is not a supported input path.
• stdio: ["ignore", "pipe", "pipe"] — closes stdin pipe. Prevents any future stdin-reading invocation from blocking, not just the - case.
• StringDecoder replaces per-chunk toString() — fixes multi-byte character corruption at 64 KiB pipe boundaries. Good call NOT adding an untestable assertion: U+FFFD is valid JSON string content, and no public surface exposes mangled tag values.
• Stdout bound: 8M chars, checked AFTER appending (the author's own first draft caught the off-by-one — pre-append check lets a single oversized chunk through). Truncated stdout throws rather than parsing garbage.
• decoder.end() called after the process exits to flush any trailing partial character.
• Tests: - rejection (0 spawn calls), stdio shape assertion, size bound exceeded.

Ships clean.

[james-russo-rames-d-jusso]

Clean series of three related engine-helper hardenings. - rejected up front rather than hanging 30s on unwritten stdin; stdio: ["ignore", "pipe", "pipe"] so no future invocation can block on stdin either; StringDecoder("utf8") preserving multi-byte character boundaries across pipe chunks; post-append size bound with a truncated-then-thrown recovery. The write-up on why the bound is checked after appending (a single chunk can already exceed it) is worth its own comment and does its job.

Nits

• Truncation leaves the child running until the deadline. On stdoutTruncated, subsequent chunks are dropped and the buffer is cleared, but the child process keeps producing output until either managed.wait() returns naturally or the 30s deadline trips. Signalling managed (or proc.kill()) at the truncation point would end early and skip the wasted disk/CPU. Byte-frugal, non-correctness — just tidier.
• The rejection guard is filePath === "-" specifically, not "all stdin protocols". pipe:0, pipe:, fd:0 also make ffprobe read stdin — but those are protocol strings a user would only reach deliberately, and stdio: ["ignore", ...] now turns them into a fast error rather than a hang, so the belt-and-braces holds. Calling out only so it's explicit that the guard closes the accidental case, not the deliberate one.

What I didn't verify

• The StringDecoder-corruption claim is documented in the commit body rather than tested — your reasoning (U+FFFD survives JSON.parse, and no public surface exposes the mangled tag value) is fine. If a future reviewer wants it pinned, a small test seam exposing raw stdout plus one multi-byte-boundary fixture would do it — not a defect, additive coverage.

— Review by Rames D Jusso

[miguel-heygen]

Approved exact head b150c488a3f10d6d96323ea699bd7481e04e320c against stack parent #2915. The bare-- rejection, ignored stdin, split-UTF-8 decoding, bounded stdout, and existing ManagedChildProcess abort/error lifecycle compose correctly. The stream remains drained after truncation, so there is no pipe-backpressure deadlock. Exact-head CI is terminal green, the PR is mergeable, and there are no unresolved review threads.

Non-blocking residuals: oversized output is discarded but the child runs until exit/deadline; the multibyte-boundary fix still lacks a regression assertion; and the size-bound error can mask a later abort/nonzero-exit diagnostic. Approval is for this exact head and remains dependent on the lower stack landing cleanly.

[miguel-heygen]

Re-review

Approved at exact rebased head fbb27353ac0a0b3d4625476f7e5331be05b2cabc.

git range-diff confirms the reviewed invocation patch is byte-equivalent to the previously approved commit. Bare stdin rejection, ignored child stdin, multibyte-safe decoding, the 8 MiB stdout bound, and abort/timeout behavior remain intact. Exact-head CI is terminal green. No P0-P2 findings.

The base branch was changed.