diff --git a/packages/producer/src/renderRequest.test.ts b/packages/producer/src/renderRequest.test.ts index 55d84b342..e7926b89b 100644 --- a/packages/producer/src/renderRequest.test.ts +++ b/packages/producer/src/renderRequest.test.ts @@ -25,6 +25,7 @@ function request() { fps: { num: 30, den: 1 }, quality: "high", format: "mp4", + gifLoop: 0, workers: 3, useGpu: true, strictness: "best-effort", @@ -72,9 +73,12 @@ describe("RenderRequest", () => { height: 1080, format: "mp4", chunkSize: 120, + strictness: "best-effort", + outputResolutionAspectAgnostic: true, variables: value.options.variables, - producerConfig: { protocolTimeout: 123_456 }, + engineConfig: { protocolTimeout: 123_456 }, }); + expect(distributed).not.toHaveProperty("producerConfig"); }); it("round-trips distributed adapter fields back into the shared request", () => { @@ -90,6 +94,12 @@ describe("RenderRequest", () => { fps: value.options.fps, quality: value.options.quality, format: value.options.format, + crf: value.options.crf, + videoFrameFormat: value.options.videoFrameFormat, + outputResolution: value.options.outputResolution, + outputResolutionAspectAgnostic: value.options.outputResolutionAspectAgnostic, + hdrMode: value.options.hdrMode, + strictness: value.options.strictness, entryFile: value.options.entryFile, variables: value.options.variables, distributed: value.options.distributed, @@ -114,4 +124,50 @@ describe("RenderRequest", () => { value.options.fps = { num: 30_000, den: 1_001 }; expect(() => distributedConfigFromRequest(value)).toThrow("does not support fps"); }); + + it("rejects JSON-unsafe request values before serialization", () => { + const cyclic: Record = {}; + cyclic.self = cyclic; + for (const variables of [ + { missing: undefined }, + { callback: () => undefined }, + { identifier: 1n }, + { ratio: Number.NaN }, + { when: new Date(0) }, + cyclic, + ]) { + expect(() => + createRenderRequest({ + projectDir: "/project", + outputPath: "/output/video.mp4", + options: { fps: { num: 30, den: 1 }, quality: "standard", format: "mp4", variables }, + }), + ).toThrow(); + } + }); + + it("rejects malformed optional and distributed fields", () => { + const value = request(); + expect(() => + parseRenderRequest({ ...value, options: { ...value.options, workers: "many" } }), + ).toThrow("workers"); + expect(() => + parseRenderRequest({ + ...value, + options: { ...value.options, distributed: { ...value.options.distributed, width: "wide" } }, + }), + ).toThrow("distributed.width"); + }); + + it("validates the reverse distributed adapter at the wire boundary", () => { + const distributed = distributedConfigFromRequest(request()); + distributed.width = 1921; + expect(() => + renderRequestFromDistributedConfig({ + projectDir: "/project", + outputPath: "/output/video.mp4", + config: distributed, + }), + ).toThrow("must be even"); + }); }); diff --git a/packages/producer/src/renderRequest.ts b/packages/producer/src/renderRequest.ts index a686835f2..ef45e38cf 100644 --- a/packages/producer/src/renderRequest.ts +++ b/packages/producer/src/renderRequest.ts @@ -1,9 +1,18 @@ -import { resolveConfig, type EngineConfig, type VideoFrameFormat } from "@hyperframes/engine"; -import type { CanvasResolution, Fps } from "@hyperframes/core"; +import { + isVideoFrameFormat, + resolveConfig, + type EngineConfig, + type VideoFrameFormat, +} from "@hyperframes/engine"; +import { VALID_CANVAS_RESOLUTIONS, type CanvasResolution, type Fps } from "@hyperframes/core"; import type { ProducerLogger } from "./logger.js"; import type { RenderConfig } from "./services/renderOrchestrator.js"; import type { DistributedRenderConfig } from "./services/distributed/plan.js"; -import type { SerializableDistributedRenderConfig } from "./services/distributed/renderConfigValidation.js"; +import { + validateDistributedRenderConfig, + validateJsonSafeValue, + type SerializableDistributedRenderConfig, +} from "./services/distributed/renderConfigValidation.js"; export const RENDER_REQUEST_VERSION = 1 as const; @@ -59,22 +68,123 @@ export interface CreateRenderRequestInput { } function isPlainObject(value: unknown): value is Record { - return typeof value === "object" && value !== null && !Array.isArray(value); + if (typeof value !== "object" || value === null || Array.isArray(value)) return false; + const proto = Object.getPrototypeOf(value); + return proto === Object.prototype || proto === null; } function assertPositiveFps(options: Record): void { const fps = options.fps; if ( !isPlainObject(fps) || + typeof fps.num !== "number" || + typeof fps.den !== "number" || !Number.isInteger(fps.num) || !Number.isInteger(fps.den) || - (fps.num as number) <= 0 || - (fps.den as number) <= 0 + fps.num <= 0 || + fps.den <= 0 ) { throw new Error("Render request fps must be a positive rational"); } } +function assertOptionalBoolean(options: Record, field: string): void { + if (options[field] !== undefined && typeof options[field] !== "boolean") { + throw new Error(`Render request ${field} must be a boolean`); + } +} + +function assertOptionalInteger(options: Record, field: string, min = 0): void { + const value = options[field]; + if ( + value !== undefined && + (typeof value !== "number" || !Number.isInteger(value) || value < min) + ) { + throw new Error(`Render request ${field} must be an integer >= ${min}`); + } +} + +function assertOptionalString(options: Record, field: string): void { + if (options[field] !== undefined && typeof options[field] !== "string") { + throw new Error(`Render request ${field} must be a string`); + } +} + +function assertOptionalEnum( + options: Record, + field: string, + allowed: readonly string[], +): void { + const value = options[field]; + if (value !== undefined && (typeof value !== "string" || !allowed.includes(value))) { + throw new Error(`Render request ${field} is invalid`); + } +} + +function isCanvasResolution(value: unknown): value is CanvasResolution { + return ( + typeof value === "string" && VALID_CANVAS_RESOLUTIONS.some((resolution) => resolution === value) + ); +} + +function assertDistributedOptions(value: unknown): void { + if (!isPlainObject(value)) throw new Error("Render request distributed must be an object"); + for (const field of ["width", "height"] as const) { + if (typeof value[field] !== "number" || !Number.isInteger(value[field]) || value[field] <= 0) { + throw new Error(`Render request distributed.${field} must be a positive integer`); + } + } + assertOptionalEnum(value, "codec", ["h264", "h265"]); + for (const field of [ + "chunkSize", + "maxParallelChunks", + "targetChunkFrames", + "planDirSizeLimitBytes", + ] as const) { + assertOptionalInteger(value, field, 1); + } + assertOptionalEnum(value, "runtimeCap", [ + "lambda", + "temporal", + "cloud-run-job", + "k8s-job", + "none", + ]); + for (const field of ["rejectOnSystemFonts", "failClosedFontFetch", "cfr"] as const) { + assertOptionalBoolean(value, field); + } +} + +function assertRequestOptionScalars(options: Record): void { + assertOptionalInteger(options, "gifLoop"); + assertOptionalInteger(options, "workers", 1); + assertOptionalInteger(options, "crf"); + for (const field of ["useGpu", "debug", "outputResolutionAspectAgnostic"] as const) { + assertOptionalBoolean(options, field); + } + for (const field of ["entryFile", "videoBitrate"] as const) { + assertOptionalString(options, field); + } + assertOptionalEnum(options, "strictness", ["strict", "best-effort"]); + assertOptionalEnum(options, "hdrMode", ["auto", "force-hdr", "force-sdr"]); + if (options.videoFrameFormat !== undefined && !isVideoFrameFormat(options.videoFrameFormat)) { + throw new Error("Render request videoFrameFormat is invalid"); + } + if (options.outputResolution !== undefined && !isCanvasResolution(options.outputResolution)) { + throw new Error("Render request outputResolution is invalid"); + } +} + +function assertRequestOptionObjects(options: Record): void { + if (!isPlainObject(options.engineConfig)) { + throw new Error("Render request must contain a resolved engineConfig snapshot"); + } + if (options.variables !== undefined && !isPlainObject(options.variables)) { + throw new Error("Render request variables must be a JSON object"); + } + if (options.distributed !== undefined) assertDistributedOptions(options.distributed); +} + function assertRequestOptions(options: unknown): asserts options is RenderRequestOptions { if (!isPlainObject(options)) throw new Error("Render request options must be an object"); assertPositiveFps(options); @@ -84,12 +194,8 @@ function assertRequestOptions(options: unknown): asserts options is RenderReques if (!["mp4", "webm", "mov", "png-sequence", "gif"].includes(String(options.format))) { throw new Error("Render request format is invalid"); } - if (!isPlainObject(options.engineConfig)) { - throw new Error("Render request must contain a resolved engineConfig snapshot"); - } - if (options.variables !== undefined && !isPlainObject(options.variables)) { - throw new Error("Render request variables must be a JSON object"); - } + assertRequestOptionScalars(options); + assertRequestOptionObjects(options); } function assertNonEmptyPath(value: unknown, field: "projectDir" | "outputPath"): void { @@ -106,6 +212,7 @@ function assertRenderRequest(value: unknown): asserts value is RenderRequest { assertNonEmptyPath(value.projectDir, "projectDir"); assertNonEmptyPath(value.outputPath, "outputPath"); assertRequestOptions(value.options); + validateJsonSafeValue(value, "renderRequest"); } export function parseRenderRequest(serialized: string | unknown): RenderRequest { @@ -129,9 +236,9 @@ export function createRenderRequest(input: CreateRenderRequestInput): RenderRequ engineConfig: input.engineConfig ?? resolveConfig(input.engineOverrides), }, } satisfies RenderRequest; - // A JSON round-trip both proves serializability and detaches caller-owned - // variables/config objects before asynchronous adapters receive them. - return parseRenderRequest(JSON.stringify(request)); + // Validate before serialization so JSON never silently drops or normalizes + // caller data at this boundary, then detach it for asynchronous adapters. + return parseRenderRequest(serializeRenderRequest(request)); } export function renderConfigFromRequest( @@ -173,6 +280,7 @@ export function distributedConfigFromRequest( bitrate: options.videoBitrate, videoFrameFormat: options.videoFrameFormat, outputResolution: options.outputResolution, + outputResolutionAspectAgnostic: options.outputResolutionAspectAgnostic, chunkSize: distributed.chunkSize, maxParallelChunks: distributed.maxParallelChunks, targetChunkFrames: distributed.targetChunkFrames, @@ -182,49 +290,61 @@ export function distributedConfigFromRequest( hdrMode: options.hdrMode === "auto" ? "auto" : "force-sdr", cfr: distributed.cfr, logger: runtime.logger, - producerConfig: options.engineConfig, engineConfig: options.engineConfig, entryFile: options.entryFile, + strictness: options.strictness, abortSignal: runtime.abortSignal, planDirSizeLimitBytes: distributed.planDirSizeLimitBytes, variables: options.variables, }; } +function optionalProperty( + key: Key, + value: Value | undefined, +): Partial> { + return value === undefined ? {} : ({ [key]: value } as Partial>); +} + export function renderRequestFromDistributedConfig(input: { projectDir: string; outputPath: string; config: SerializableDistributedRenderConfig; }): RenderRequest { const { config } = input; + validateDistributedRenderConfig(config); + const distributed = { + width: config.width, + height: config.height, + ...optionalProperty("codec", config.codec), + ...optionalProperty("chunkSize", config.chunkSize), + ...optionalProperty("maxParallelChunks", config.maxParallelChunks), + ...optionalProperty("targetChunkFrames", config.targetChunkFrames), + ...optionalProperty("runtimeCap", config.runtimeCap), + ...optionalProperty("rejectOnSystemFonts", config.rejectOnSystemFonts), + ...optionalProperty("failClosedFontFetch", config.failClosedFontFetch), + ...optionalProperty("cfr", config.cfr), + ...optionalProperty("planDirSizeLimitBytes", config.planDirSizeLimitBytes), + } satisfies DistributedRenderOptions; + const options = { + fps: { num: config.fps, den: 1 }, + quality: config.quality ?? "standard", + format: config.format, + distributed, + ...optionalProperty("crf", config.crf), + ...optionalProperty("videoBitrate", config.bitrate), + ...optionalProperty("videoFrameFormat", config.videoFrameFormat), + ...optionalProperty("outputResolution", config.outputResolution), + ...optionalProperty("outputResolutionAspectAgnostic", config.outputResolutionAspectAgnostic), + ...optionalProperty("hdrMode", config.hdrMode), + ...optionalProperty("strictness", config.strictness), + ...optionalProperty("entryFile", config.entryFile), + ...optionalProperty("variables", config.variables), + } satisfies CreateRenderRequestInput["options"]; return createRenderRequest({ projectDir: input.projectDir, outputPath: input.outputPath, engineConfig: config.engineConfig ?? resolveConfig(), - options: { - fps: { num: config.fps, den: 1 }, - quality: config.quality ?? "standard", - format: config.format, - crf: config.crf, - videoBitrate: config.bitrate, - videoFrameFormat: config.videoFrameFormat, - outputResolution: config.outputResolution, - hdrMode: config.hdrMode ?? "force-sdr", - entryFile: config.entryFile, - variables: config.variables, - distributed: { - width: config.width, - height: config.height, - codec: config.codec, - chunkSize: config.chunkSize, - maxParallelChunks: config.maxParallelChunks, - targetChunkFrames: config.targetChunkFrames, - runtimeCap: config.runtimeCap, - rejectOnSystemFonts: config.rejectOnSystemFonts, - failClosedFontFetch: config.failClosedFontFetch, - cfr: config.cfr, - planDirSizeLimitBytes: config.planDirSizeLimitBytes, - }, - }, + options, }); } diff --git a/packages/producer/src/services/distributed/renderConfigValidation.ts b/packages/producer/src/services/distributed/renderConfigValidation.ts index 0d3168566..fbbf63b4c 100644 --- a/packages/producer/src/services/distributed/renderConfigValidation.ts +++ b/packages/producer/src/services/distributed/renderConfigValidation.ts @@ -229,7 +229,12 @@ export function validateVariablesPayload(value: unknown): void { `must be a plain JSON object (got ${describeValue(value)})`, ); } - walkVariables(value, "config.variables", new WeakSet()); + validateJsonSafeValue(value, "config.variables"); +} + +/** Validate any JSON-boundary value without silently normalizing it. */ +export function validateJsonSafeValue(value: unknown, field: string): void { + walkVariables(value, field, new WeakSet()); } /** Per-typeof rejection messages for JSON-unsafe leaves. */