fix(producer): transport safe extraction failure metadata

[jrusso1020]

Summary

• expose producer-authored public error data through a generic errorMetadata envelope; the OSS server does not know or enforce caller schemas
• attach bounded extraction facts under errorMetadata.extractionFailure: failure-kind counts, status class, exhausted local retry evidence, normalized source host, and a query-free SHA-256 source fingerprint
• emit the same metadata in blocking JSON and SSE responses
• remove every HeyGen/ImageKit host constant and pass normalized host information through for downstream policy

Boundary ownership

OSS owns fact production: classification, bounds, deterministic grouping, fingerprinting, and removal of raw URLs/paths. The server only transports explicitly public metadata.

The internal adapter owns trust-policy validation and host allowlisting/redaction. EF owns the final retry decision. No workflow retry policy exists in this PR.

Safety

• signed URLs, URL paths, local paths, video IDs, and raw upstream status text never enter producer-authored public metadata
• only the explicit publicMetadata object on an allowlisted typed error is emitted; arbitrary error properties remain private
• host values contain only the URL parser normalized hostname, never the URL, path, query, or fragment
• downstream consumers must validate errorMetadata before making policy decisions

Validation

• 205 focused engine/producer tests passed
• producer TypeScript typecheck passed
• Oxlint, Oxfmt, fallow structural audit, and all pre-commit gates passed
• internal producer built successfully against this OSS checkout through HYPERFRAMES_OSS_DIR

Rollout

Publish this first. Then update the internal PR to the resulting @hyperframes/producer release and configure its host policy before deployment. Older consumers ignore the additive errorMetadata field.

Incident context: https://heygen.slack.com/archives/C09JCPXF3HS/p1788299364334449

Linked PRs:

• internal transport/policy: https://github.com/heygen-com/hyperframes-internal/pull/1022
• EF retry policy: https://github.com/heygen-com/experiment-framework/pull/49227

[terencecho]

APPROVE at 2686b5d7 — reviewed as the top of the 3-PR stack (OSS emit → HFI sanitize/transport (#1022) → EF consume (#49227)).

Schema fields verified emitted:

• ExtractionFailureMetadataV1 = {schemaVersion: 1, kindCounts[1..14], groups[0..8], omittedGroupCount[0..10000]}
• KindCount = {kind: VideoExtractionFailureKind, affectedElementCount[1..10000]}, sorted+unique
• Group = {kind, affectedElementCount, sourceFingerprint?: "sha256:[0-9a-f]{64}", host?: allowlisted-suffix|"other", statusClass?: 5-enum, retry?: {phase:"download", used:0|1, budget:1}}, sorted deterministically

Signed-URL exclusion is clean at every seam. Fingerprint hashes origin+pathname only — urlDownloader.ts:107 strips query + fragment BEFORE sha256, so signed-URL params can't leak via the covert-channel path. Host allowlist is literal-suffix (heygen.ai, heygen.com, imagekit.io + "other") — not regex-permissive — computed at engine and re-validated at the producer response boundary via safeExtractionHost + SAFE_EXTRACTION_HOSTNAME. Producer boundary rejects any host string containing :/?# or uppercase. Group content is enum + hash + count only — no error-message inlining, no stack traces. server.extractionFailureMetadata.test.ts explicitly asserts the serialized body does NOT contain "super-secret", "private-video-id", /tmp/private-render, or the upstream private status text.

Terminal discipline holds. errors.push({group: downloadFailureGroup(...)}) only fires after downloadWithRetry throws AFTER maxTransientRetries exhaustion; non-download errors (invalid_media, ffmpeg_*) never populate group, so downstream can't mistake them for retry-eligible. retry.used=1, budget=1 set only when error.telemetry.attempt >= 2 — the "exhausted producer retry" evidence the EF consumer keys off.

Test coverage is strong. URL-sanitization asserted on both /v1/render (JSON) and /v1/render-stream (SSE) with identical top-level contract. 7 parametrized malformed-payload rejections (oversized counts, empty kindCounts, duplicates, non-canonical ordering, too many groups, oversized omitted, unknown fields). 8 malformed-host cases dropped. 9-group overflow → 8 shown + 2 omitted. Log-format tightening removes raw src from Extracting frames from video ….

Cross-cutting is additive. New packages/producer/src/services/render/extractionFailureMetadata.ts (80 LOC). Engine index adds 1 fn + 4 types export. VideoExtractionFailure.group? and SafeRenderErrorMetadata.extractionFailure? both additive — no OSS public-API breaks.

CI at 2686b5d7 — latest run 33568725600 all SUCCESS: Lint, Typecheck, Format, Build, producer unit + integration, SDK, Test, CLI smoke, Fallow audit, Semantic PR title, Windows render, regression shards 1-9. Earlier CANCELLED entries are superseded pre-force-push runs.

Non-blocking notes:

• MAX_EXTRACTION_FAILURE_KIND_COUNTS = 14 is hardcoded to current VideoExtractionFailureKind enum size — adding a kind needs the bump. A const assertion tying it to SAFE_EXTRACTION_FAILURE_KINDS.size would remove the drift risk.
• Host allowlist is duplicated (engine + producer boundary) — deliberate defense-in-depth, but a drift risk if one list gains a host without the other. Worth a shared constant.

Rollout order: this PR is the head of the stack (#3592 merge → new OSS release → #1022 pin bump → #1022 CI rerun → land internal → EF #49227 merges safely). This PR itself is safe to merge in isolation — no consumer breaks on the current pin.

— Review by tai (pr-review)

[somanshreddy]

Review — OSS producer extraction-failure metadata (independent two-pass, at 3de11c2)

Reviewed as one contract with hyperframes-internal #1022 + experiment-framework #49227. Codex (unbiased) + my own pass, reconciled at source (note: head refreshed a few times during review — verified at 3de11c2). Verdict: LGTM on the design; one nonblocking-but-recommended leak to decide on.

The refresh that generalized this into a generic errorMetadata/publicMetadata transport with no HeyGen/ImageKit host constants or caller-specific server validator is the right call — an OSS package shouldn't carry a caller's host policy; delegating validation + host redaction to the internal sidecar is cleaner and keeps this repo vendor-neutral.

Confirmed sound

• Signed-URL exclusion is structural. safeDownloadUrlIdentity builds canonical = origin + pathname (query + fragment stripped) before the sha256 — a signature/token can never enter the hash input, and the fingerprint is one-way. Query-free canonicalization also means retries of the same signed asset share a fingerprint (grouping works).
• Bounded + terminal discipline. Groups populated only for UrlDownloadError after downloadWithRetry exhausts maxTransientRetries; ≤8 groups (rest → omittedGroupCount), ≤14 kindCounts, counts ≤10,000; non-download failures never carry a group.

Nonblocking (recommended — sibling log leak this PR's own goal targets)

videoFrameExtractor.ts:1595 still emits the raw source verbatim to stderr:

WARNING: video src="${video.src}" could not be resolved on disk (looked for ${videoPath}).

isHttpUrl (urlDownloader.ts:1327) is case-sensitive (startsWith("http://")||startsWith("https://")), so a mixed-case HTTPS://…?X-Amz-Signature=… or a protocol-relative //host/… source is misclassified as local, never downloads, fails existsSync, and its full signed URL is written verbatim (and interpolated again into error: \Video file not found: ${videoPath}`). This is pre-existing and reachability is narrow (requires a non-lowercase/relative scheme — HeyGen's own presigned URLs are lowercase https://), so it's not a hard block — but this PR sanitized the sibling extraction log 40 lines away for exactly this class, so it's worth closing here too: parse/lower-case the scheme in isHttpUrl(and handle protocol-relative) and route this warning + thesource_missingerror throughsafeVideoExtractionSourceLogMetadata/a bounded identity instead of video.src/videoPath`. (Credit Codex — caught by neither my first pass nor the other reviewers.)

Happy to LGTM fully once you've decided whether to fix it here or track it as a pre-existing follow-up. Everything else corroborated by tai + Genesis.

[terencecho]

APPROVE at 3de11c2f6 — re-review at the new head after the "generic errorMetadata transport" fix-up (delta from 2686b5d7, which I stamped as 5084613822).

Delta verified. Wire moved from top-level extractionFailure → nested errorMetadata.extractionFailure on both SSE and blocking-JSON. Producer-side dropped safeExtractionFailureMetadata structural validator, safeExtractionHost allowlist, and the SAFE_EXTRACTION_HOST_SUFFIXES = ["heygen.ai","heygen.com","imagekit.io"] list. Producer now emits errorMetadata as Readonly<Record<string, unknown>> — an opaque bag; the engine (extractVideosStage.ts) is the sole constructor of VideoExtractionStageError.publicMetadata = { extractionFailure }. Engine-side videoFrameExtractor.ts also dropped allowlistedExtractionHost — safeVideoExtractionSourceIdentity now returns identity.host ?? "other". Wire-side host is lowercased URL hostname.

Signed-URL exclusion preserved. urlDownloader.ts:107 still strips query+fragment before sha256(origin+pathname). No regression on the covert-channel path.

Terminal-only emit preserved. VideoExtractionStageError still fires only from assertVideoExtractionSucceeded after per-source downloadWithRetry exhaustion.

Test coverage rebalanced, not weakened. Server-side structural-trust matrix (oversized counts, empty/dup kindCounts, non-canonical ordering, too many groups, unknown fields) and wire-host rejection matrix (evil.heygen.com, a/b.heygen.com, media.heygen.com?signature=secret) moved to the private consumer (HFI #1022). Signed-URL no-leak assertions preserved.

Concur with Somu (nonblocking, worth closing here): videoFrameExtractor.ts:1595 still writes video src="${video.src}" to stderr; isHttpUrl is case-sensitive, so a mixed-case HTTPS://…?X-Amz-Signature=… source or protocol-relative //host/… misclassifies as local and leaks the signed URL. Pre-existing + narrow reachability (HeyGen presigned URLs are lowercase https://), but the sibling extraction log 40 lines away is sanitized in this PR for exactly this class. Same PR is the natural place.

CI at 3de11c2f6 — no failures at latest-per-name. Preflight, Lint, Format, Producer unit tests, Test runtime contract, Studio load smoke, SDK, player-perf, preview-regression, Fallow audit, File size, Semantic PR title all SUCCESS. Typecheck, CLI smoke, Producer integration, Windows lanes, and 9 regression shards still IN_PROGRESS at review time.

Non-blocking (additive to Somu's):

• BC break for internal consumers: wire top-level → nested. HFI #1022 is already re-synced to errorMetadata.extractionFailure / publicMetadata.extractionFailure; EF #49227 consumes SidecarErrorBody (HFI's own shape), so no direct impact. Coordinated via the rollout order.
• Producer no longer validates publicMetadata shape (only isPlainObject). Safe today because the engine is the only writer, but any new writer bypassing extractionFailureMetadataFromResult would flow verbatim. Consider a positive covert-channel test at head asserting constructed URLs don't reach the wire; not required.

— Review by tai (pr-review)

[miguel-heygen]

Cross-repo review at exact head 3de11c2, with HFI #1022 at 8e2b1ad55174bc6deece75a600830f4e24824489 and EF #49227 at d91e29205f010e1a932188e3b7e9ff429957d548.

The vendor-specific policy issue is resolved. OSS now owns a generic errorMetadata transport only; VideoExtractionStageError explicitly opts its bounded publicMetadata in, while internal owns the runtime schema and host-redaction policy. The download evidence producer still emits deterministic kind counts/groups, query-and-fragment-free SHA-256 source identity, normalized hostname, status class, and exact local retry exhaustion. JSON and SSE carry the same top-level envelope.

Signed-URL exclusion checks out on the live path: safeDownloadUrlIdentity hashes origin+pathname after URL parsing, never query/fragment; the extractor response metadata contains no raw source, status text, video id, or local path. The current internal boundary then rejects non-host syntax, bounds every field, and redacts valid unconfigured hosts to other.

Verification: parsers/lint/studio-server/core/engine/producer builds passed locally; 129 focused engine/producer tests passed at this SHA; all required GitHub checks now pass, including Build, Test, Typecheck, runtime contract, Windows render, and semantic title.

Non-blocking existing note: Somu’s HF-F1 points out the older source-missing stderr warning still prints a mixed-case/protocol-relative source verbatim because isHttpUrl is case-sensitive. It is outside the new metadata path and narrow in current reachability, but worth closing in a follow-up so the sibling log has the same safety property.

Verdict: APPROVE
Reasoning: The OSS layer is now vendor-neutral, its producer-owned extraction evidence remains bounded and query-free, and the two downstream layers own strict validation and retry policy without relying on raw diagnostics.

— Magi