diff --git a/packages/cli/src/commands/init.ts b/packages/cli/src/commands/init.ts index b470da0f86..b8905f2d3b 100644 --- a/packages/cli/src/commands/init.ts +++ b/packages/cli/src/commands/init.ts @@ -109,7 +109,7 @@ function probeVideo(filePath: string): VideoMeta | undefined { if (!ffprobePath) return undefined; const raw = execFileSync( ffprobePath, - ["-v", "quiet", "-print_format", "json", "-show_format", "-show_streams", filePath], + ["-v", "quiet", "-print_format", "json", "-show_format", "-show_streams", "--", filePath], { encoding: "utf-8", timeout: 15_000 }, ); diff --git a/packages/cli/src/utils/webmAlphaCheck.ts b/packages/cli/src/utils/webmAlphaCheck.ts index d52f1d72aa..75b8ae159b 100644 --- a/packages/cli/src/utils/webmAlphaCheck.ts +++ b/packages/cli/src/utils/webmAlphaCheck.ts @@ -86,6 +86,7 @@ function probeWebmAlpha(filePath: string): WebmAlphaProbe { "stream=codec_name:stream_tags=alpha_mode", "-of", "json", + "--", filePath, ], { encoding: "utf-8", timeout: 15_000 }, diff --git a/packages/cli/src/whisper/transcribe.ts b/packages/cli/src/whisper/transcribe.ts index 6641edb95e..c511a2b535 100644 --- a/packages/cli/src/whisper/transcribe.ts +++ b/packages/cli/src/whisper/transcribe.ts @@ -171,6 +171,7 @@ function getMediaDurationSeconds(filePath: string): number | null { "format=duration", "-of", "default=noprint_wrappers=1:nokey=1", + "--", filePath, ], { encoding: "utf-8", timeout: 10_000 }, @@ -329,7 +330,7 @@ function isWav16kMono(filePath: string): boolean { if (!ffprobePath) return false; const raw = execFileSync( ffprobePath, - ["-v", "quiet", "-print_format", "json", "-show_streams", filePath], + ["-v", "quiet", "-print_format", "json", "-show_streams", "--", filePath], { encoding: "utf-8", timeout: 10_000 }, ); const parsed: { diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 8da2e39fe1..108b87bb82 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -252,7 +252,7 @@ export { quantizeTimeToFrame, type MediaVisualStyleProperty, } from "./inline-scripts/parityContract"; -export { redactTelemetryString } from "./telemetryRedaction"; +export { redactKnownPaths, redactTelemetryString } from "./telemetryRedaction"; export { isSafePath, resolveWithinProject } from "./safePath"; export type { HyperframePickerApi, diff --git a/packages/core/src/mediaGradeAnalyzer.ts b/packages/core/src/mediaGradeAnalyzer.ts index 7ea693435a..ca6c129135 100644 --- a/packages/core/src/mediaGradeAnalyzer.ts +++ b/packages/core/src/mediaGradeAnalyzer.ts @@ -121,6 +121,7 @@ function probeMedia(mediaPath: string, ffprobePath: string): GradeMediaProbe { "stream=color_space,color_transfer,color_primaries,pix_fmt,duration:format=duration", "-of", "json", + "--", mediaPath, ], { encoding: "utf8", timeout: 5_000, stdio: ["ignore", "pipe", "pipe"] }, diff --git a/packages/core/src/telemetryRedaction.test.ts b/packages/core/src/telemetryRedaction.test.ts index 670f1a4eae..ee98b4b0df 100644 --- a/packages/core/src/telemetryRedaction.test.ts +++ b/packages/core/src/telemetryRedaction.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { redactTelemetryString } from "./telemetryRedaction.js"; +import { redactKnownPaths, redactTelemetryString } from "./telemetryRedaction.js"; describe("redactTelemetryString", () => { it("redacts macOS, Linux, Windows, file URLs, and URL query strings", () => { @@ -16,4 +16,141 @@ describe("redactTelemetryString", () => { ), ).toBe("[path] [path] [path] [path] [file-url] https://example.com/video.mp4?…"); }); + + // The redactor used to enumerate roots (/Users, /home, /opt, /tmp, …). Any + // root outside that list reached telemetry verbatim, which is most of them. + it.each([ + "/data/media/interview.mov", + "/mnt2/nfs/share/take3.wav", + "/srv2/renders/2026/final.mp4", + "/nix/store/abc123/asset.png", + ])("redacts the non-allowlisted absolute root in %s", (path) => { + const out = redactTelemetryString(`ffprobe failed reading ${path}`); + expect(out).not.toContain("/"); + expect(out).toContain("[path]"); + }); + + it("redacts relative paths, including a dash-prefixed one", () => { + expect(redactTelemetryString("could not open ./assets/-weird-name.mp3")).toBe( + "could not open [path]", + ); + expect(redactTelemetryString("could not open ../-out.wav")).toBe("could not open [path]"); + expect(redactTelemetryString("could not open .\\tmp\\-x.aac")).toBe("could not open [path]"); + }); + + it("redacts a bare basename — a caller may pass one instead of a path", () => { + expect(redactTelemetryString("Invalid data found in my-client-cut.mp4")).toBe( + "Invalid data found in [file]", + ); + }); + + // Over-redaction is cheap; these are ordinary in ffprobe stderr and turning + // them into [path] would make a diagnostic string useless. + it.each(["N/A", "24/1", "Stream #0:0", "moov atom not found", "48000/1001"])( + "leaves %s alone", + (text) => { + expect(redactTelemetryString(`ffprobe: ${text}`)).toBe(`ffprobe: ${text}`); + }, + ); + + // A `?` is illegal in a Windows filename, so this is not a query string — + // the whole token is path, and must not survive by hiding behind a `?`. + it("consumes the rest of the token once a path is established", () => { + expect(redactTelemetryString("Navigation failed for C:\\Users\\A\\v.mov?not-a-query")).toBe( + "Navigation failed for [path]", + ); + }); + + it("truncates after redacting, so a long path cannot survive by being cut", () => { + const out = redactTelemetryString(`/data/${"x".repeat(500)}/a.mp4`, 40); + expect(out).not.toContain("xxx"); + }); + + // Named explicitly in review: a relative path with NO `./` prefix was + // missed by both the absolute rule (needs a leading slash) and the `./` + // rule (needs the dot), so it reached telemetry completely unredacted. + it.each([ + "customer/acme-secret/video.mp4", + "assets/bgm.mp3", + "projects/client-name/cut/final.mov", + "a\\b\\c.wav", + ])("redacts the bare relative path %s", (path) => { + const out = redactTelemetryString(`Invalid data found when processing ${path}`); + expect(out).toBe("Invalid data found when processing [path]"); + }); + + it("redacts a dash-prefixed bare basename", () => { + expect(redactTelemetryString("could not open -customer-secret-intro.mp4")).toBe( + "could not open [file]", + ); + }); +}); + +// The segment classes were ASCII `\w`, so a non-Latin path went out verbatim. +// This redactor also feeds CLI telemetry and producer observation messages, +// where no known-path list is supplied to compensate. +describe("non-Latin paths", () => { + it.each([ + "/数据/客户/秘密视频.mp4", + "/данные/клиент/видео.mp4", + "/data/客户/secret.mp4", + "/Users/alice/проект/видео.mp4", + ])("redacts the absolute path %s", (path) => { + const out = redactTelemetryString(`ffprobe failed reading ${path}`); + expect(out).toBe("ffprobe failed reading [path]"); + }); + + it("redacts a non-Latin bare relative path without stranding the first segment", () => { + // The lookbehind used to be `\w`-based, so a match could start mid-token + // when the preceding character was non-ASCII: this redacted to `客户[path]`. + expect(redactTelemetryString("could not open 客户/秘密/视频.mp4")).toBe( + "could not open [path]", + ); + }); + + it.each(["./资产/背景.mp3", "../输出/final.wav"])("redacts the relative path %s", (path) => { + expect(redactTelemetryString(`could not open ${path}`)).toBe("could not open [path]"); + }); + + it("redacts a non-Latin bare basename", () => { + expect(redactTelemetryString("Invalid data found in 秘密视频.mp4")).toBe( + "Invalid data found in [file]", + ); + }); +}); + +describe("redactKnownPaths", () => { + // Shape matching has holes by construction. A caller that built the argv + // knows the exact path, so it can name it instead of hoping a regex does. + it("redacts an exact path a regex would not recognise as one", () => { + const weird = "acme_secret_project"; + expect(redactKnownPaths(`ffprobe: ${weird}: Invalid data`, [weird])).toBe( + "ffprobe: [path]: Invalid data", + ); + }); + + it("redacts the basename too — ffprobe often reports only that", () => { + const out = redactKnownPaths("moov atom not found in secret-cut.mp4", [ + "/data/x/secret-cut.mp4", + ]); + expect(out).toContain("[path]"); + expect(out).not.toContain("secret-cut"); + }); + + it("leaves the message alone when no path was supplied", () => { + expect(redactKnownPaths("moov atom not found", [])).toBe("moov atom not found"); + }); + + // Guards against a one/two-character basename turning every occurrence of + // that letter into [path]. + it("ignores paths too short to be distinctive", () => { + expect(redactKnownPaths("a stream at a rate", ["a"])).toBe("a stream at a rate"); + }); + + // This sits on an error path: throwing here turns a reported failure into an + // unhandled rejection, which is what a non-Error rejection's `undefined` + // message caused. + it.each([undefined, null, 42, {}])("returns a string for the non-string input %s", (value) => { + expect(() => redactKnownPaths(value as unknown as string, ["/tmp/x.mp4"])).not.toThrow(); + }); }); diff --git a/packages/core/src/telemetryRedaction.ts b/packages/core/src/telemetryRedaction.ts index 5e05ab7a12..423a9d29e9 100644 --- a/packages/core/src/telemetryRedaction.ts +++ b/packages/core/src/telemetryRedaction.ts @@ -9,13 +9,141 @@ function redactUrlQueryStrings(value: string): string { return value.replace(/\b(https?:\/\/[^\s?]+)\?[^\s]*/g, "$1?…"); } +/** + * Path characters we treat as part of a single segment. Space is deliberately + * excluded: including it would let a match run past the path and swallow the + * prose after it, and a path with a space still gets its remaining segments + * redacted, which is the part that carries the identifying information. + */ +/** + * A path segment is defined by what ENDS it, not by an alphabet. + * + * `[\w...]` is ASCII-only, so `/数据/客户/秘密视频.mp4` and `/data/客户/secret.mp4` + * passed through completely unredacted — the generic redactor also feeds CLI + * telemetry and producer observation messages, where no known-path list is + * supplied to cover for it. Enumerating Unicode classes instead (`\p{L}\p{N}…`) + * would work but has to be kept correct for marks, joiners and emoji; a + * delimiter-based rule is right for every script by construction. + * + * Whitespace and quotes end a segment; so do the separators themselves. + * Space stays excluded for the original reason: including it would let a match + * run past the path and swallow the prose after it. + */ +const SEGMENT = String.raw`[^\s/\\'"]+`; + +/** Same, minus the dot, so a trailing `.ext` can be matched separately. */ +const SEGMENT_NODOT = String.raw`[^\s/\\'".]+`; + +/** + * Once a match is established as a path, consume the rest of the token. + * Windows forbids `?` in a filename, so `video.mov?not-a-query` is not a real + * query string — but stopping at the `?` would emit the remainder verbatim. + * Redacting to the next delimiter cannot leak; stopping early can. + */ +const TOKEN_TAIL = String.raw`[^\s'")]*`; + +/** + * Absolute path, any root — NOT an allowlist of roots. + * + * The previous version enumerated `/Users`, `/home`, `/opt`, `/tmp`… which + * meant a project on `/data`, `/Volumes/External`, an NFS mount or any root a + * user invented reached telemetry verbatim. Two or more segments are required + * so `N/A` and a `24/1` frame rate — both ordinary in ffprobe stderr — are not + * mistaken for paths. + * + * The lookbehind keeps this off URLs: after `https:` the slash is preceded by + * `:`, the second by `/`, and the path segment by a word character, so no + * position inside a URL can start a match. URLs are handled above, where the + * host is kept and only the query is dropped. + */ +const ABSOLUTE_PATH = new RegExp( + String.raw`(? v.length > 2)) { + out = out.split(literal).join("[path]"); + } + } + return out; +} + function redactFilePaths(value: string): string { - return value - .replace(/file:\/\/[^\s'")]+/g, "[file-url]") - .replace(/\/Users\/[^\s'")]+/g, "[path]") - .replace(/\/(?:home|root|opt|app|workspace|srv|mnt)\/[^\s'")]+/g, "[path]") - .replace(/\/(?:private\/)?(?:var|tmp)\/[^\s'")]+/g, "[path]") - .replace(/[A-Za-z]:\\[^\s'")]+/g, "[path]"); + return ( + value + .replace(/file:\/\/[^\s'")]+/g, "[file-url]") + // Relative BEFORE absolute: `./assets/x.mp3` has an absolute-looking tail + // (`/assets/x.mp3`), so the absolute rule would consume it and leave the + // leading `.` stranded outside the redaction. + .replace(RELATIVE_PATH, "[path]") + // Bare-relative BEFORE absolute: a bare path's interior satisfies the + // absolute rule, which would claim it and strand the first segment. + .replace(BARE_RELATIVE_PATH, "[path]") + .replace(ABSOLUTE_PATH, "[path]") + .replace(ASSET_BASENAME, "[file]") + ); } export function redactTelemetryString( diff --git a/packages/lint/src/hevcPreviewLint.ts b/packages/lint/src/hevcPreviewLint.ts index 9a6cfba853..e0cc086c0b 100644 --- a/packages/lint/src/hevcPreviewLint.ts +++ b/packages/lint/src/hevcPreviewLint.ts @@ -57,6 +57,7 @@ async function probeIsHevc(ffprobePath: string, filePath: string): Promise { "stream=r_frame_rate,avg_frame_rate,duration", "-of", "json", + "--", outputPath, ], { stdio: "pipe" }, diff --git a/packages/producer/src/services/render/audioPadTrim.integration.test.ts b/packages/producer/src/services/render/audioPadTrim.integration.test.ts index 137dbdcd1c..5c7162fe8f 100644 --- a/packages/producer/src/services/render/audioPadTrim.integration.test.ts +++ b/packages/producer/src/services/render/audioPadTrim.integration.test.ts @@ -67,6 +67,7 @@ describe.skipIf(!hasFfmpeg)("audio pad real-media packet contract", () => { "packet=duration_time", "-of", "json", + "--", output, ], { encoding: "utf8" }, diff --git a/packages/producer/src/services/render/audioPadTrim.test.ts b/packages/producer/src/services/render/audioPadTrim.test.ts index 4f79ac5bf9..13b78fbaad 100644 --- a/packages/producer/src/services/render/audioPadTrim.test.ts +++ b/packages/producer/src/services/render/audioPadTrim.test.ts @@ -270,3 +270,112 @@ describe("padOrTrimAudioToVideoFrameCount", () => { expect(result.targetDurationSeconds).toBe(6); }); }); + +// ── Public-path path redaction ──────────────────────────────────────────── +// +// The redaction helpers have their own unit tests, but those pass whether or +// not this module actually CALLS them: deleting the wiring in +// padOrTrimAudioToVideoFrameCount left every one of them green. These drive +// the public entry point and assert on the public `PadTrimAudioResult.error`, +// which is what reaches logs, telemetry, and the caller. +describe("PadTrimAudioResult.error never carries the input path", () => { + const cases: Array<{ name: string; videoPath: string; secret: string }> = [ + { + name: "a dash-prefixed relative path", + videoPath: "./assets/-customer-secret-intro.mp4", + secret: "customer-secret-intro", + }, + { + name: "a non-allowlisted absolute root", + videoPath: "/data/acme-secret/video.mp4", + secret: "acme-secret", + }, + { + name: "a bare relative path", + videoPath: "customer/acme-secret/video.mp4", + secret: "acme-secret", + }, + ]; + + for (const { name, videoPath, secret } of cases) { + it(`redacts ${name} raised by the video probe`, async () => { + const result = await padOrTrimAudioToVideoFrameCount({ + videoPath, + audioPath: "/tmp/audio.aac", + outputPath: "/tmp/out.aac", + // Reproduces the real thrower: defaultProbeVideoFrameInfo raises + // `ffprobe found no video stream in ${videoPath}` with the raw path. + probeVideoFrameInfo: () => + Promise.reject(new Error(`ffprobe found no video stream in ${videoPath}`)), + probeAudioInfo: () => Promise.resolve({ durationSeconds: 1 }), + runFfmpeg: () => Promise.resolve({ success: true }), + }); + + expect(result.success).toBe(false); + expect(result.error).toBeDefined(); + expect(result.error ?? "").not.toContain(secret); + expect(result.error ?? "").not.toContain(videoPath); + // Still diagnosable — the failure mode survives redaction. + expect(result.error ?? "").toContain("failed to probe video"); + }); + } + + it("redacts raw ffprobe stderr surfaced through the audio probe", async () => { + const result = await padOrTrimAudioToVideoFrameCount({ + videoPath: "/tmp/v.mp4", + audioPath: "/data/acme-secret/audio.aac", + outputPath: "/tmp/out.aac", + probeVideoFrameInfo: () => Promise.resolve({ frameCount: 30, fpsNum: 30, fpsDen: 1 }), + probeAudioInfo: () => + Promise.reject( + new Error("/data/acme-secret/audio.aac: Invalid data found when processing input"), + ), + runFfmpeg: () => Promise.resolve({ success: true }), + }); + + expect(result.success).toBe(false); + expect(result.error ?? "").not.toContain("acme-secret"); + expect(result.error ?? "").toContain("failed to probe audio"); + }); + + // An injected probe can reject with anything. Casting the reason to Error and + // reading `.message` yielded undefined, which threw inside the redactor and + // turned a returned failure result into a rejected promise. + describe("a probe that rejects with a non-Error value", () => { + const nonErrors: Array<[string, unknown]> = [ + ["a string", "probe failed"], + ["undefined", undefined], + ["null", null], + ["a number", 42], + ["a plain object", { code: "ENOENT" }], + ]; + + for (const [label, reason] of nonErrors) { + it(`still returns a failed result when the video probe rejects with ${label}`, async () => { + const result = await padOrTrimAudioToVideoFrameCount({ + videoPath: "/data/acme-secret/video.mp4", + audioPath: "/tmp/audio.aac", + outputPath: "/tmp/out.aac", + probeVideoFrameInfo: () => Promise.reject(reason), + probeAudioInfo: () => Promise.resolve({ durationSeconds: 1 }), + runFfmpeg: () => Promise.resolve({ success: true }), + }); + expect(result.success).toBe(false); + expect(result.error ?? "").toContain("failed to probe video"); + }); + + it(`still returns a failed result when the audio probe rejects with ${label}`, async () => { + const result = await padOrTrimAudioToVideoFrameCount({ + videoPath: "/tmp/v.mp4", + audioPath: "/data/acme-secret/audio.aac", + outputPath: "/tmp/out.aac", + probeVideoFrameInfo: () => Promise.resolve({ frameCount: 30, fpsNum: 30, fpsDen: 1 }), + probeAudioInfo: () => Promise.reject(reason), + runFfmpeg: () => Promise.resolve({ success: true }), + }); + expect(result.success).toBe(false); + expect(result.error ?? "").toContain("failed to probe audio"); + }); + } + }); +}); diff --git a/packages/producer/src/services/render/audioPadTrim.ts b/packages/producer/src/services/render/audioPadTrim.ts index 9aa30503f2..381d4d325a 100644 --- a/packages/producer/src/services/render/audioPadTrim.ts +++ b/packages/producer/src/services/render/audioPadTrim.ts @@ -30,6 +30,7 @@ import { trackChildProcess, type AudioMetadata, } from "@hyperframes/engine"; +import { redactKnownPaths, redactTelemetryString } from "@hyperframes/core"; /** * Tolerance used to decide whether an audio file is already short enough to @@ -214,6 +215,31 @@ function formatSeconds(sec: number): string { return sec.toFixed(6); } +/** + * Every probe failure message, sanitized once, at the one place they all pass + * through on their way into the public `PadTrimAudioResult.error`. + * + * `runFfprobeJson` already scrubs the stderr it raises, but it is not the only + * thrower: `defaultProbeVideoFrameInfo` raises + * `ffprobe found no video stream in ${videoPath}` with the raw path, and a + * caller-supplied `probeVideoFrameInfo` / `probeAudioInfo` can raise anything + * at all. Sanitizing per-thrower is a list that will drift; sanitizing at the + * boundary cannot be bypassed by adding a new throw upstream. + * + * Known paths first (this function has them in hand, so no pattern has to + * recognise them), then the generic shape-based scrub for anything the message + * picked up elsewhere. + */ +function sanitizeProbeFailure(reason: unknown, paths: readonly string[]): string { + // Normalized here, not at the call sites. A caller-supplied probe can reject + // with anything — `Promise.reject("probe failed")` has no `.message`, so + // casting to Error yielded `undefined` and threw inside the redactor. That + // turned a returned failure result into a rejected promise, which is a + // behaviour regression the cast introduced. + const message = reason instanceof Error ? reason.message : String(reason); + return redactTelemetryString(redactKnownPaths(message, paths)); +} + /** * Pad or trim `audio.aac` so its exact duration matches `frameCount / fps` * for the assembled video. @@ -234,12 +260,16 @@ export async function padOrTrimAudioToVideoFrameCount( probeAudio(input.audioPath, input.signal), ]); + const probePaths = [input.videoPath, input.audioPath, input.outputPath]; if (videoResult.status === "rejected") { return failResult( input.outputPath, 0, audioResult.status === "fulfilled" ? audioResult.value.durationSeconds : 0, - `audioPadTrim: failed to probe video: ${(videoResult.reason as Error).message}`, + `audioPadTrim: failed to probe video: ${sanitizeProbeFailure( + videoResult.reason, + probePaths, + )}`, ); } if (audioResult.status === "rejected") { @@ -247,7 +277,10 @@ export async function padOrTrimAudioToVideoFrameCount( input.outputPath, 0, 0, - `audioPadTrim: failed to probe audio: ${(audioResult.reason as Error).message}`, + `audioPadTrim: failed to probe audio: ${sanitizeProbeFailure( + audioResult.reason, + probePaths, + )}`, ); } @@ -361,6 +394,7 @@ async function defaultProbeVideoFrameInfo( "stream=nb_frames,r_frame_rate", "-of", "json", + "--", videoPath, ], signal, @@ -381,6 +415,7 @@ async function defaultProbeVideoFrameInfo( "stream=nb_read_packets,r_frame_rate", "-of", "json", + "--", videoPath, ], signal, @@ -434,7 +469,13 @@ async function defaultRunFfmpeg( // ── ffprobe JSON runner (shared between fast/slow video probe paths) ───── async function runFfprobeJson(args: string[], signal?: AbortSignal): Promise { - const proc = spawn(getFfprobeBinary(), args); + // Callers bake the input path into `args` (terminated with "--"), so this + // helper cannot add the terminator itself — assert they did rather than + // let a dash-prefixed path silently reach ffprobe as an option. + if (!args.includes("--")) { + throw new Error('[audioPadTrim] ffprobe args must terminate options with "--".'); + } + const proc = spawn(getFfprobeBinary(), args, { stdio: ["ignore", "pipe", "pipe"] }); trackChildProcess(proc); let stdout = ""; proc.stdout.on("data", (data: Buffer) => { @@ -452,7 +493,14 @@ async function runFfprobeJson(args: string[], signal?: AbortSignal): Promise< throw outcome.error ?? new Error(outcome.stderr); } if (outcome.reason !== "exit" || outcome.exitCode !== 0) { - throw new Error(`ffprobe ${outcome.reason}: ${outcome.stderr}`); + // Redacted twice, deliberately. The shape-based scrub is a net with + // holes — it cannot know that `customer/acme-secret/video.mp4` is a path + // and `48000/1001` is not — but THIS caller knows the exact path it put + // in the argv, so it names it literally first. The message reaches logs, + // telemetry, and `PadTrimAudioResult.error`. + const probed = args[args.length - 1]; + const scrubbed = redactKnownPaths(outcome.stderr, probed === undefined ? [] : [probed]); + throw new Error(`ffprobe ${outcome.reason}: ${redactTelemetryString(scrubbed, 2000)}`); } try { return JSON.parse(stdout) as T; diff --git a/packages/producer/src/utils/audioRegression.ts b/packages/producer/src/utils/audioRegression.ts index 4659d15238..8017b0298a 100644 --- a/packages/producer/src/utils/audioRegression.ts +++ b/packages/producer/src/utils/audioRegression.ts @@ -315,6 +315,7 @@ function probeAudioDuration(file: string): { seconds: number; error?: string } { "stream=duration", "-of", "default=noprint_wrappers=1:nokey=1", + "--", file, ], { encoding: "utf-8" }, diff --git a/packages/producer/src/utils/ffprobeArgvContract.test.ts b/packages/producer/src/utils/ffprobeArgvContract.test.ts new file mode 100644 index 0000000000..66d7a4e4e2 --- /dev/null +++ b/packages/producer/src/utils/ffprobeArgvContract.test.ts @@ -0,0 +1,310 @@ +import { describe, expect, it } from "vitest"; +import { readFileSync, readdirSync, statSync } from "node:fs"; +import { join, relative } from "node:path"; + +/** + * Every ffprobe/ffmpeg invocation must terminate its options with `--` + * IMMEDIATELY before the input path. + * + * Source-level and DISCOVERY-based on purpose. Two prior attempts failed + * differently and both reported the bug class closed: + * + * - #2740 fixed one of ten call sites and asserted the argv of that one site. + * - Its follow-up scanned a hardcoded file list for a format flag followed by + * a bare identifier, which only matched the shape it was written against — + * mutation-testing showed removal of `--` from `engine/utils/ffprobe.ts` + * and `cli/commands/init.ts` did not fail it. + * + * So this walks the tree, finds the callers itself, and checks the TERMINATOR + * POSITION rather than its mere presence. A new caller is discovered rather + * than needing to be remembered. + */ +const REPO_ROOT = join(import.meta.dirname, "..", "..", "..", ".."); + +/** + * Roots to sweep. + * + * `skills/` is here because leaving it out was not a scoping choice, it was a + * hole: the shipped agent tools under `skills/**` spawn ffprobe directly, and + * 17 of those call sites were missing the terminator while this suite reported + * the bug class closed. They are distributed to users, not fixtures. + */ +const SWEEP_ROOTS = ["packages", "skills", "scripts"]; + +/** `.mjs`/`.cjs` are first-class here — the skill scripts are not TypeScript. */ +const SOURCE_EXT = /\.(?:ts|mjs|cjs|js|py|sh)$/; + +/** + * Shell-syntax invocations, checked separately and more weakly. + * + * A JS/Python argv is a bracketed literal, so the parser above can check that + * `--` is the PENULTIMATE entry. A shell command line is not a literal — + * `ffprobe -v error ... "$BG" 2>/dev/null | tr -dc '0-9.'` has redirections, + * pipes and substitutions after the input — so checking position would need a + * shell parser. This asserts the terminator is PRESENT on any ffprobe command + * line, which is weaker but is the part that was missing, and it is honest + * about being weaker rather than implying the same guarantee. + */ +const SHELL_EXT = /\.sh$/; + +/** + * The caller set as of the sweep that introduced this contract. + * + * Discovery is authoritative — a NEW caller is picked up without touching this + * list. The manifest runs the comparison the other way: if a regex change or a + * refactor drops a known caller out of discovery, every assertion for it stops + * running and the suite still reports green. That silent-vacuum failure is how + * the two previous versions of this test stayed passing over a live bug. + */ +const MANIFEST = [ + "packages/cli/src/commands/init.ts", + "packages/cli/src/utils/webmAlphaCheck.ts", + "packages/cli/src/whisper/transcribe.ts", + "packages/core/src/mediaGradeAnalyzer.ts", + "packages/engine/src/utils/ffprobe.ts", + "packages/lint/src/hevcPreviewLint.ts", + "packages/producer/src/plan-parity-analysis.ts", + "packages/producer/src/services/render/audioPadTrim.ts", + "packages/producer/src/utils/audioRegression.ts", + "packages/studio-server/src/helpers/mediaMetadata.ts", + "packages/studio-server/src/helpers/mediaValidation.ts", +]; + +/** Files that actually invoke ffprobe/ffmpeg, found rather than listed. */ +/** + * Runs ffprobe but produced no argv the matcher understood. Not necessarily a + * bug — a file may only pass a probe path around — but it IS the blind spot, + * so it is surfaced rather than dropped. + */ +function mentionsProbe(src: string): boolean { + // The first argument of the spawn must itself name a probe binary. A looser + // "file mentions ffprobe anywhere AND spawns anything" rule flagged four + // files that build no argv at all: binary resolution (`ffBinaries.ts`, + // `browser/ffmpeg.ts`), error-string matching (`videoFrameExtractor.ts`) and + // prose (`mediaCodecMap.ts`). + // + // ponytail: names the binary at the call site; a caller that spawns through + // an opaquely-named variable (`spawn(command, argv)`) is invisible here. + // Those still get caught by argv matching whenever their flags are literal — + // widen this if one ever slips through both. + // Comments and doc prose describing a spawn are not a spawn: + // `tts.test.mjs` explains `ffprobeDuration's spawnSync("ffprobe", ...) call` + // in a comment and was reported as an unclassified caller. + // A probe spawned with an all-literal argv takes no runtime input, so there + // is nothing to terminate: `spawnSync("ffprobe", ["-version"])` is a + // capability check, not a file probe. Dropping those keeps them out of the + // unclassified list without weakening it — an argv carrying a bare + // identifier (a path) still has to be understood. + // + // Split into a linear scan plus a per-body check rather than one regex. + // The obvious `(?:"[^"]*"\s*,?\s*)+` form nests a quantifier inside a + // quantifier with an optional separator, so whitespace can be matched two + // ways and it backtracks exponentially on a long non-matching argv — CodeQL + // flagged it as js/redos, correctly. + const code = stripComments(src); + return CALLS_PROBE.test(withoutNoInputProbes(code)); +} + +const PROBE_SPAWN_HEAD = + /(?:spawn|spawnSync|execFile\w*|exec)\s*\(\s*["'`]ff(?:probe|mpeg)["'`]\s*,\s*\[/g; +const CALLS_PROBE = + /(?:spawn|spawnSync|execFile\w*|exec)\s*\(\s*[^,)]*(?:ffprobe|ffProbe|probeBin|probePath)/i; + +function stripComments(src: string): string { + return src.replace(/\/\*[\s\S]*?\*\//g, "").replace(/\/\/[^\n]*/g, ""); +} + +/** Every entry a string literal → no runtime path in this argv. */ +function isLiteralOnlyArgv(body: string): boolean { + const entries = body + .split(",") + .map((e) => e.trim()) + .filter((e) => e !== ""); + return entries.length > 0 && entries.every((e) => /^"[^"]*"$/.test(e)); +} + +/** Blank out `spawn("ffprobe", [ ...all literals... ])` occurrences. */ +function withoutNoInputProbes(code: string): string { + let out = ""; + let cursor = 0; + PROBE_SPAWN_HEAD.lastIndex = 0; + for (let m = PROBE_SPAWN_HEAD.exec(code); m !== null; m = PROBE_SPAWN_HEAD.exec(code)) { + const bodyStart = m.index + m[0].length; + const bodyEnd = code.indexOf("]", bodyStart); + if (bodyEnd === -1) continue; + if (!isLiteralOnlyArgv(code.slice(bodyStart, bodyEnd))) continue; + out += code.slice(cursor, m.index); + cursor = bodyEnd + 1; + PROBE_SPAWN_HEAD.lastIndex = cursor; + } + return out + code.slice(cursor); +} + +const SKIP_DIRS = new Set(["node_modules", "dist"]); + +function isSourceFile(entry: string): boolean { + if (!SOURCE_EXT.test(entry) || entry.endsWith(".d.ts")) return false; + // This file documents the contract with example argvs, including a + // deliberately misordered one. Scanning itself reports its own prose. + if (entry === "ffprobeArgvContract.test.ts") return false; + // Test files are swept too. A test that probes a rendered output is itself a + // caller, and `dither.test.mjs` was one of the 17 broken sites. + return true; +} + +function discoverCallers(): { found: string[]; unclassified: string[]; shell: string[] } { + const found: string[] = []; + const unclassified: string[] = []; + const shell: string[] = []; + const classify = (abs: string): void => { + const src = readFileSync(abs, "utf8"); + if (SHELL_EXT.test(abs) && /(?:^|[^\w-])ffprobe\s+-/m.test(src)) { + shell.push(relative(REPO_ROOT, abs)); + } + // Discovery is ARGV-shaped, not call-shaped. Matching on spawn/execFile + // misses a dependency-injected runner — `runner("ffprobe", [...])` in + // studio-server's mediaValidation.ts is exactly that, and a call-shaped + // predicate skipped it silently. Anything that BUILDS a probe argv is a + // caller, however it is invoked. + if (argvTails(src).length > 0) found.push(relative(REPO_ROOT, abs)); + else if (mentionsProbe(src)) unclassified.push(relative(REPO_ROOT, abs)); + }; + const walk = (dir: string): void => { + for (const entry of readdirSync(dir)) { + if (SKIP_DIRS.has(entry) || entry.startsWith(".")) continue; + const abs = join(dir, entry); + if (statSync(abs).isDirectory()) walk(abs); + else if (isSourceFile(entry)) classify(abs); + } + }; + for (const root of SWEEP_ROOTS) { + const abs = join(REPO_ROOT, root); + try { + if (statSync(abs).isDirectory()) walk(abs); + } catch { + /* root absent in a partial checkout */ + } + } + return { found: found.sort(), unclassified: unclassified.sort(), shell: shell.sort() }; +} + +/** + * Argv arrays that carry ffprobe/ffmpeg flags, with their trailing tokens. + * + * Deliberately shape-agnostic: it finds `[ ... ]` literals containing a known + * probe flag and reports the last two entries, so `["-v","error",...spread,"--",p]` + * and a multi-line `["-of","json","--",p]` are both covered. + */ +// ffPROBE-shaped only. ffmpeg takes its input via `-i` (already unambiguous, +// the flag consumes the next token) and its OUTPUT as the trailing positional, +// where `--` is not the convention — so matching those produced false +// positives on every encode call (`..., "-y", outputPath`). +const PROBE_ONLY_FLAG = + /"-(?:of|print_format|show_entries|show_format|show_streams|select_streams|count_packets)"/; +/** `-y` (overwrite output) and `-i` (input flag) mark an ffmpeg argv. */ +const FFMPEG_SHAPE = /"-(?:y|i)"/; +/** + * A generic wrapper builds its flags from a spread — `["-v", "error", + * ...argsWithoutInput, "--", filePath]` — so it carries no literal probe flag + * of its own. That is the exact shape `engine/utils/ffprobe.ts` uses, and the + * shape a probe-flag-only matcher silently skipped. + */ +const SPREAD_WRAPPER = /"-v"[\s\S]*\.\.\.[A-Za-z_$]/; + +function argvTails(source: string): Array<{ snippet: string; tail: string[] }> { + const tails: Array<{ snippet: string; tail: string[] }> = []; + // Non-greedy array literal, tolerant of newlines and spreads. + for (const m of source.matchAll(/\[((?:[^[\]]|\[[^\]]*\])*?)\]/gs)) { + const body = m[1] ?? ""; + const isProbeArgv = PROBE_ONLY_FLAG.test(body) || SPREAD_WRAPPER.test(body); + if (!isProbeArgv || FFMPEG_SHAPE.test(body)) continue; + const parts = body + .split(",") + .map((p) => p.replace(/\/\/[^\n]*/g, "").trim()) + .filter((p) => p !== ""); + if (parts.length < 2) continue; + tails.push({ snippet: m[0].replace(/\s+/g, " ").slice(0, 90), tail: parts.slice(-2) }); + } + return tails; +} + +describe("shell ffprobe invocations terminate their options", () => { + const shellFiles = discoverCallers().shell; + + it("finds the shell callers", () => { + // Guards the guard: `frame_strip.sh` and `render-and-composite.sh` both + // shipped un-terminated while the JS-only sweep reported the class closed. + expect(shellFiles.length).toBeGreaterThan(0); + }); + + it.each(shellFiles)("%s passes -- on every ffprobe command line", (relPath) => { + const source = readFileSync(join(REPO_ROOT, relPath), "utf8"); + const offenders = source + .split("\n") + .map((line, index) => ({ line: line.trim(), number: index + 1 })) + // An invocation passes flags. `command -v ffprobe >/dev/null` is a PATH + // check and `echo "ffmpeg/ffprobe not on PATH"` is a message; neither + // takes an input, and both were reported before this narrowed. + .filter(({ line }) => /(?:^|[^\w-])ffprobe\s+-/.test(line) && !line.startsWith("#")) + .filter(({ line }) => !/\b(?:command\s+-v|which|type)\s+ffprobe/.test(line)) + .filter(({ line }) => !/\s--\s/.test(line)) + .map(({ line, number }) => `${number}: ${line.slice(0, 80)}`); + + expect(offenders, `${relPath}: ffprobe command lines missing "--"`).toEqual([]); + }); +}); + +describe("ffprobe argv contract", () => { + const { found: callers, unclassified } = discoverCallers(); + + it("still discovers every caller in the manifest", () => { + const missing = MANIFEST.filter((f) => !callers.includes(f)); + expect(missing, "discovery regressed — these are no longer being checked").toEqual([]); + }); + + it("classifies every file that spawns ffprobe", () => { + // A caller written in a shape the matcher does not recognise is checked by + // nothing. Fail loudly and widen the matcher rather than skip it. + expect(unclassified, "spawns ffprobe but built no argv this test understands").toEqual([]); + }); + + it.each(callers)("%s terminates options immediately before the input", (relPath) => { + const source = readFileSync(join(REPO_ROOT, relPath), "utf8"); + const offenders = argvTails(source) + .filter(({ tail }) => { + const [penultimate, last] = tail; + // A trailing string literal is a flag/value, not a path — those argv + // arrays do not take an input here. + if (last === undefined || /^["'`]/.test(last)) return false; + return penultimate !== '"--"'; + }) + .map(({ snippet, tail }) => `${tail.join(" , ")} in ${snippet}`); + + expect(offenders, `${relPath}: "--" must be immediately before the input`).toEqual([]); + }); + + // MISORDERING, not just removal. `["-of","json",path,"--"]` contains the + // terminator but does nothing, and a presence-only check passes it. + it.each(callers)("%s never places the terminator after the input", (relPath) => { + const source = readFileSync(join(REPO_ROOT, relPath), "utf8"); + const misordered = argvTails(source) + .filter(({ tail }) => tail[1] === '"--"') + .map(({ snippet }) => snippet); + expect(misordered, `${relPath}: "--" must precede the input, not follow it`).toEqual([]); + }); + + // audioPadTrim's runFfprobeJson takes a pre-built argv, so it cannot add the + // terminator itself and instead asserts one is present. Presence is weaker + // than position — pin that the callers put it immediately before the path. + it("audioPadTrim's pre-built argv puts the terminator immediately before the input", () => { + const src = readFileSync( + join(REPO_ROOT, "packages/producer/src/services/render/audioPadTrim.ts"), + "utf8", + ); + const probeArgvs = argvTails(src); + expect(probeArgvs.length).toBeGreaterThan(0); + for (const { tail, snippet } of probeArgvs) { + expect(tail[0], `terminator not penultimate in ${snippet}`).toBe('"--"'); + } + }); +}); diff --git a/packages/producer/tests/distributed/_smoke/webm-concat-copy.test.ts b/packages/producer/tests/distributed/_smoke/webm-concat-copy.test.ts index e5f9e3d69a..0aedc39b89 100644 --- a/packages/producer/tests/distributed/_smoke/webm-concat-copy.test.ts +++ b/packages/producer/tests/distributed/_smoke/webm-concat-copy.test.ts @@ -213,6 +213,7 @@ describe("webm VP9 concat-copy smoke", () => { "stream=codec_name,width,height,pix_fmt,r_frame_rate", "-of", "default=noprint_wrappers=1", + "--", outputPath, ]); if (result.exitCode !== 0) { @@ -268,6 +269,7 @@ describe("webm VP9 concat-copy smoke", () => { "stream=nb_read_frames", "-of", "default=noprint_wrappers=1:nokey=1", + "--", outputPath, ]); if (result.exitCode !== 0) { @@ -423,6 +425,7 @@ describe("webm VP9 concat-copy smoke (yuva420p alpha)", () => { "-select_streams", "v:0", "-show_streams", + "--", alphaOutputPath, ]); expect(probeResult.exitCode).toBe(0); diff --git a/packages/studio-server/src/helpers/mediaMetadata.ts b/packages/studio-server/src/helpers/mediaMetadata.ts index 905abeef48..4082f04ba4 100644 --- a/packages/studio-server/src/helpers/mediaMetadata.ts +++ b/packages/studio-server/src/helpers/mediaMetadata.ts @@ -202,6 +202,7 @@ export async function probeMediaMetadata( "stream=codec_type,codec_name,profile,pix_fmt,color_space,color_transfer,color_primaries,bits_per_raw_sample:stream_disposition=attached_pic", "-of", "json", + "--", filePath, ], { timeout: 15_000, maxBuffer: 1024 * 1024 }, diff --git a/packages/studio-server/src/helpers/mediaValidation.ts b/packages/studio-server/src/helpers/mediaValidation.ts index 1602124152..81bb180362 100644 --- a/packages/studio-server/src/helpers/mediaValidation.ts +++ b/packages/studio-server/src/helpers/mediaValidation.ts @@ -33,6 +33,7 @@ export function validateUploadedMedia( "stream=codec_type", "-of", "json", + "--", filePath, ]); diff --git a/skills-manifest.json b/skills-manifest.json index 11db85e55b..4a101ae6e9 100644 --- a/skills-manifest.json +++ b/skills-manifest.json @@ -2,15 +2,15 @@ "source": "heygen-com/hyperframes", "skills": { "embedded-captions": { - "hash": "ed4dc7b850b92ff5", + "hash": "e8c2c3b6dfd04b39", "files": 140 }, "faceless-explainer": { - "hash": "261a9740ec1378b0", + "hash": "c70b904aa68cf7e5", "files": 24 }, "figma": { - "hash": "517e4dc53c13ea05", + "hash": "4f524b4962bd8d7c", "files": 2 }, "general-video": { @@ -46,11 +46,11 @@ "files": 10 }, "media-use": { - "hash": "6c40be3e8bd6eacc", + "hash": "6fedfe5fe57a9885", "files": 152 }, "motion-graphics": { - "hash": "50db172cad89b1c7", + "hash": "1434e22bb0259bbb", "files": 23 }, "music-to-video": { @@ -58,15 +58,15 @@ "files": 132 }, "pr-to-video": { - "hash": "41171bbed1c5d8f4", + "hash": "7769801640dca521", "files": 30 }, "product-launch-video": { - "hash": "01fc75da8492f749", + "hash": "81953f054fcb9d91", "files": 28 }, "remotion-to-hyperframes": { - "hash": "3a0e6c2affb9f74e", + "hash": "3ecc684432b298dd", "files": 70 }, "slideshow": { diff --git a/skills/embedded-captions/scripts/make-cinematic.cjs b/skills/embedded-captions/scripts/make-cinematic.cjs index fb1d9bacd6..27149504ce 100644 --- a/skills/embedded-captions/scripts/make-cinematic.cjs +++ b/skills/embedded-captions/scripts/make-cinematic.cjs @@ -185,6 +185,7 @@ function main() { "format=duration", "-of", "default=nokey=1:noprint_wrappers=1", + "--", fp, ], { encoding: "utf8" }, diff --git a/skills/embedded-captions/scripts/make-composition.cjs b/skills/embedded-captions/scripts/make-composition.cjs index aa499fabae..efdda1c8eb 100644 --- a/skills/embedded-captions/scripts/make-composition.cjs +++ b/skills/embedded-captions/scripts/make-composition.cjs @@ -50,6 +50,7 @@ function sourceDurationSec(project) { "format=duration", "-of", "default=nokey=1:noprint_wrappers=1", + "--", p, ], { encoding: "utf8" }, diff --git a/skills/embedded-captions/scripts/matte.cjs b/skills/embedded-captions/scripts/matte.cjs index bb0fe23435..315c22897b 100644 --- a/skills/embedded-captions/scripts/matte.cjs +++ b/skills/embedded-captions/scripts/matte.cjs @@ -83,6 +83,7 @@ function probeRates(src) { "stream=r_frame_rate,avg_frame_rate", "-of", "default=nk=1:nw=1", + "--", src, ]) .toString() diff --git a/skills/embedded-captions/scripts/render-and-composite.sh b/skills/embedded-captions/scripts/render-and-composite.sh index c7741d0c74..2c9d04e975 100755 --- a/skills/embedded-captions/scripts/render-and-composite.sh +++ b/skills/embedded-captions/scripts/render-and-composite.sh @@ -352,8 +352,8 @@ else fi # Probe render dims for ffmpeg scale -W="$(ffprobe -v error -select_streams v:0 -show_entries stream=width -of default=nw=1:nk=1 "$BG")" -H="$(ffprobe -v error -select_streams v:0 -show_entries stream=height -of default=nw=1:nk=1 "$BG")" +W="$(ffprobe -v error -select_streams v:0 -show_entries stream=width -of default=nw=1:nk=1 -- "$BG")" +H="$(ffprobe -v error -select_streams v:0 -show_entries stream=height -of default=nw=1:nk=1 -- "$BG")" # Clamp every composite to the matte (= source-video) length. The render uses # plan.duration / data-duration, which can exceed the source (e.g. Whisper word @@ -365,7 +365,7 @@ H="$(ffprobe -v error -select_streams v:0 -show_entries stream=height -of defaul # the a-roll stream ends). The true a-roll duration is authoritative: clamp to # min(matte frames / fps, source duration). MATTE_DUR="$(awk "BEGIN{printf \"%.3f\", $(ls "$PROJECT/frames_fg" | wc -l)/$FPS}")" -SRC_DUR="$(ffprobe -v error -show_entries format=duration -of csv=p=0 "$PROJECT/source.mp4" 2>/dev/null || true)" +SRC_DUR="$(ffprobe -v error -show_entries format=duration -of csv=p=0 -- "$PROJECT/source.mp4" 2>/dev/null || true)" if [[ -n "${SRC_DUR:-}" ]]; then MATTE_DUR="$(awk "BEGIN{m=$MATTE_DUR; s=$SRC_DUR; printf \"%.3f\", (s>0 && s/dev/null | tr -dc '0-9.')" + BG_DUR="$(ffprobe -v error -show_entries format=duration -of default=nokey=1:noprint_wrappers=1 -- "$BG" 2>/dev/null | tr -dc '0-9.')" if [[ -n "$BG_DUR" ]] && awk "BEGIN{exit !($BG_DUR < $MATTE_DUR - 0.3)}"; then echo "[render] ⚠ background plate is ${BG_DUR}s but the clip is ${MATTE_DUR}s — the composition is shorter than the footage." >&2 echo " The tail would show ONLY the foreground subject on black. FIX: set the composition" >&2 diff --git a/skills/faceless-explainer/scripts/assemble-index.mjs b/skills/faceless-explainer/scripts/assemble-index.mjs index e772ddbc45..34059e2665 100644 --- a/skills/faceless-explainer/scripts/assemble-index.mjs +++ b/skills/faceless-explainer/scripts/assemble-index.mjs @@ -81,7 +81,7 @@ function ensureBgmCovers(relPath, hyperframesDir, total) { const abs = join(hyperframesDir, relPath); const probe = spawnSync( "ffprobe", - ["-v", "error", "-show_entries", "format=duration", "-of", "csv=p=0", abs], + ["-v", "error", "-show_entries", "format=duration", "-of", "csv=p=0", "--", abs], { encoding: "utf8" }, ); if (probe.status !== 0) return { looped: false, short: false, reason: "ffprobe unavailable" }; diff --git a/skills/figma/scripts/verify-motion.mjs b/skills/figma/scripts/verify-motion.mjs index ae5376e0c1..38247a8fa4 100644 --- a/skills/figma/scripts/verify-motion.mjs +++ b/skills/figma/scripts/verify-motion.mjs @@ -51,6 +51,7 @@ const ffprobe = (file) => "format=duration", "-of", "csv=p=0", + "--", file, ]) .toString() @@ -69,6 +70,7 @@ const dims = execFileSync("ffprobe", [ "stream=width,height", "-of", "csv=p=0", + "--", reference, ]) .toString() diff --git a/skills/media-use/audio/scripts/lib/tts.mjs b/skills/media-use/audio/scripts/lib/tts.mjs index b708594744..8bb004e138 100644 --- a/skills/media-use/audio/scripts/lib/tts.mjs +++ b/skills/media-use/audio/scripts/lib/tts.mjs @@ -109,7 +109,7 @@ function ffmpegDurationFallback(absPath) { export function ffprobeDuration(absPath) { const r = spawnSync( "ffprobe", - ["-v", "error", "-show_entries", "format=duration", "-of", "default=nw=1:nk=1", absPath], + ["-v", "error", "-show_entries", "format=duration", "-of", "default=nw=1:nk=1", "--", absPath], { encoding: "utf8" }, ); if (r.error?.code === "ENOENT") return ffmpegDurationFallback(absPath); diff --git a/skills/media-use/scripts/dither.mjs b/skills/media-use/scripts/dither.mjs index e844e2b3b3..7e4bfef6ff 100644 --- a/skills/media-use/scripts/dither.mjs +++ b/skills/media-use/scripts/dither.mjs @@ -120,7 +120,7 @@ async function run() { function probe(filePath) { const raw = execFileSync( "ffprobe", - ["-v", "error", "-print_format", "json", "-show_streams", "-show_format", filePath], + ["-v", "error", "-print_format", "json", "-show_streams", "-show_format", "--", filePath], { encoding: "utf8", timeout: 10_000 }, ); const parsed = JSON.parse(raw); diff --git a/skills/media-use/scripts/dither.test.mjs b/skills/media-use/scripts/dither.test.mjs index fd564badfc..1e2d61f270 100644 --- a/skills/media-use/scripts/dither.test.mjs +++ b/skills/media-use/scripts/dither.test.mjs @@ -105,7 +105,7 @@ test("processes moving MP4 frames, audio, and BT.709 metadata", { skip: !HAS_FFM const probe = JSON.parse( execFileSync( "ffprobe", - ["-v", "error", "-print_format", "json", "-show_streams", "-show_format", output], + ["-v", "error", "-print_format", "json", "-show_streams", "-show_format", "--", output], { encoding: "utf8", }, @@ -135,6 +135,7 @@ test("processes moving MP4 frames, audio, and BT.709 metadata", { skip: !HAS_FFM "frame=best_effort_timestamp_time", "-of", "csv=p=0", + "--", output, ], { encoding: "utf8" }, diff --git a/skills/media-use/scripts/lib/grade-analyzer.mjs b/skills/media-use/scripts/lib/grade-analyzer.mjs index b673a65a58..f419d5b3b9 100644 --- a/skills/media-use/scripts/lib/grade-analyzer.mjs +++ b/skills/media-use/scripts/lib/grade-analyzer.mjs @@ -44,6 +44,7 @@ function probeMedia(mediaPath, ffprobePath) { "stream=color_space,color_transfer,color_primaries,pix_fmt,duration:format=duration", "-of", "json", + "--", mediaPath, ], { encoding: "utf8", timeout: 5_000, stdio: ["ignore", "pipe", "pipe"] }, diff --git a/skills/media-use/scripts/lib/probe.mjs b/skills/media-use/scripts/lib/probe.mjs index 27ef28ce7a..f7e0a72224 100644 --- a/skills/media-use/scripts/lib/probe.mjs +++ b/skills/media-use/scripts/lib/probe.mjs @@ -12,7 +12,7 @@ export function probe(filePath) { // can't break out of the quoting — filePath is passed as a literal argv entry. const raw = execFileSync( "ffprobe", - ["-v", "quiet", "-print_format", "json", "-show_format", "-show_streams", filePath], + ["-v", "quiet", "-print_format", "json", "-show_format", "-show_streams", "--", filePath], { encoding: "utf8", timeout: 5000 }, ); const info = JSON.parse(raw); diff --git a/skills/media-use/scripts/lib/tts-local-provider.mjs b/skills/media-use/scripts/lib/tts-local-provider.mjs index b1920bebea..b2058780f1 100644 --- a/skills/media-use/scripts/lib/tts-local-provider.mjs +++ b/skills/media-use/scripts/lib/tts-local-provider.mjs @@ -17,7 +17,7 @@ function probeDurationSeconds(file) { try { const out = execFileSync( "ffprobe", - ["-v", "error", "-show_entries", "format=duration", "-of", "csv=p=0", file], + ["-v", "error", "-show_entries", "format=duration", "-of", "csv=p=0", "--", file], { encoding: "utf8", timeout: 15000 }, ); const d = parseFloat(String(out).trim()); diff --git a/skills/media-use/scripts/transcript-cut.mjs b/skills/media-use/scripts/transcript-cut.mjs index 49210abc9f..d76bd022fb 100644 --- a/skills/media-use/scripts/transcript-cut.mjs +++ b/skills/media-use/scripts/transcript-cut.mjs @@ -209,6 +209,7 @@ function probeDuration(filePath) { "format=duration", "-of", "default=noprint_wrappers=1:nokey=1", + "--", filePath, ], { encoding: "utf8" }, diff --git a/skills/motion-graphics/grounding/locate.mjs b/skills/motion-graphics/grounding/locate.mjs index 136b8074aa..6e65ec22df 100644 --- a/skills/motion-graphics/grounding/locate.mjs +++ b/skills/motion-graphics/grounding/locate.mjs @@ -45,6 +45,7 @@ function probe(img) { "stream=width,height", "-of", "csv=p=0", + "--", img, ]) .toString() diff --git a/skills/pr-to-video/scripts/assemble-index.mjs b/skills/pr-to-video/scripts/assemble-index.mjs index e1c200ef6a..79f15ef2bf 100644 --- a/skills/pr-to-video/scripts/assemble-index.mjs +++ b/skills/pr-to-video/scripts/assemble-index.mjs @@ -82,7 +82,7 @@ function ensureBgmCovers(relPath, hyperframesDir, total) { const abs = join(hyperframesDir, relPath); const probe = spawnSync( "ffprobe", - ["-v", "error", "-show_entries", "format=duration", "-of", "csv=p=0", abs], + ["-v", "error", "-show_entries", "format=duration", "-of", "csv=p=0", "--", abs], { encoding: "utf8" }, ); if (probe.status !== 0) return { looped: false, short: false, reason: "ffprobe unavailable" }; diff --git a/skills/product-launch-video/scripts/assemble-index.mjs b/skills/product-launch-video/scripts/assemble-index.mjs index b8f92f4078..91b64e2aec 100644 --- a/skills/product-launch-video/scripts/assemble-index.mjs +++ b/skills/product-launch-video/scripts/assemble-index.mjs @@ -81,7 +81,7 @@ function ensureBgmCovers(relPath, hyperframesDir, total) { const abs = join(hyperframesDir, relPath); const probe = spawnSync( "ffprobe", - ["-v", "error", "-show_entries", "format=duration", "-of", "csv=p=0", abs], + ["-v", "error", "-show_entries", "format=duration", "-of", "csv=p=0", "--", abs], { encoding: "utf8" }, ); if (probe.status !== 0) return { looped: false, short: false, reason: "ffprobe unavailable" }; diff --git a/skills/remotion-to-hyperframes/scripts/frame_strip.sh b/skills/remotion-to-hyperframes/scripts/frame_strip.sh index 468d50d5ee..73264fe6b0 100755 --- a/skills/remotion-to-hyperframes/scripts/frame_strip.sh +++ b/skills/remotion-to-hyperframes/scripts/frame_strip.sh @@ -53,7 +53,7 @@ probe = subprocess.run( ["ffprobe", "-v", "error", "-select_streams", "v:0", "-show_entries", "stream=r_frame_rate,nb_read_frames,duration", "-show_entries", "format=duration", - "-of", "json", "-count_frames", baseline], + "-of", "json", "-count_frames", "--", baseline], check=True, capture_output=True, text=True, ) data = json.loads(probe.stdout)