fix(engine): preserve bounded ffprobe diagnostics (#2772)

* fix(engine): preserve bounded ffprobe diagnostics

* test(engine): harden ffprobe diagnostic redaction
This commit is contained in:
James Russo
2026-07-25 17:12:43 -04:00
committed by GitHub
parent 37b88688e7
commit 8c50770684
2 changed files with 113 additions and 17 deletions
+63
View File
@@ -183,6 +183,7 @@ describe("ffprobe missing-binary fallback", () => {
it("spawns the configured absolute FFprobe path when HYPERFRAMES_FFPROBE_PATH is set", async () => { it("spawns the configured absolute FFprobe path when HYPERFRAMES_FFPROBE_PATH is set", async () => {
process.env.HYPERFRAMES_FFPROBE_PATH = "/tools/ffprobe.exe"; process.env.HYPERFRAMES_FFPROBE_PATH = "/tools/ffprobe.exe";
const successfulStderr = "recoverable diagnostic on a successful probe";
const { spawn, calls } = createSpawnSpy([ const { spawn, calls } = createSpawnSpy([
{ {
kind: "exit", kind: "exit",
@@ -191,6 +192,7 @@ describe("ffprobe missing-binary fallback", () => {
streams: [{ codec_type: "audio", codec_name: "aac", sample_rate: "48000", channels: 2 }], streams: [{ codec_type: "audio", codec_name: "aac", sample_rate: "48000", channels: 2 }],
format: { duration: "1.25", bit_rate: "128000" }, format: { duration: "1.25", bit_rate: "128000" },
}), }),
stderr: successfulStderr,
}, },
]); ]);
vi.resetModules(); vi.resetModules();
@@ -200,7 +202,9 @@ describe("ffprobe missing-binary fallback", () => {
const meta = await extractAudioMetadata("/tmp/uses-configured-ffprobe.wav"); const meta = await extractAudioMetadata("/tmp/uses-configured-ffprobe.wav");
expect(meta.durationSeconds).toBe(1.25); expect(meta.durationSeconds).toBe(1.25);
expect(JSON.stringify(meta)).not.toContain(successfulStderr);
expect(calls[0]?.command).toBe(resolve("/tools/ffprobe.exe")); expect(calls[0]?.command).toBe(resolve("/tools/ffprobe.exe"));
expect(calls[0]?.args.slice(0, 2)).toEqual(["-v", "error"]);
}); });
it.each([ it.each([
@@ -363,6 +367,65 @@ describe("ffprobe missing-binary fallback", () => {
await expect(extractMediaMetadataMocked("/tmp/no-such-video.mp4")).rejects.toThrow(/ffprobe/); await expect(extractMediaMetadataMocked("/tmp/no-such-video.mp4")).rejects.toThrow(/ffprobe/);
}); });
it("surfaces bounded ffprobe stderr for invalid media", async () => {
const leadingNoise = "x".repeat(10_000);
const diagnostic = "Invalid data found when processing input";
const inputPath = "/tmp/render/My Secret Video.mp4";
const { spawn, calls } = createSpawnSpy([
{
kind: "exit",
code: 1,
stderr: `${leadingNoise}${inputPath}: ${diagnostic}`,
},
]);
vi.resetModules();
vi.doMock("child_process", () => ({ spawn }));
const { extractAudioMetadata } = await import("./ffprobe.js");
let thrown: unknown;
try {
await extractAudioMetadata(inputPath);
} catch (error) {
thrown = error;
}
expect(String(thrown)).toContain(diagnostic);
expect(String(thrown)).toContain("[input]");
expect(String(thrown)).not.toContain(inputPath);
expect(String(thrown)).not.toContain("My Secret Video.mp4");
expect(String(thrown).length).toBeLessThan(4_500);
expect(calls[0]?.args.slice(0, 2)).toEqual(["-v", "error"]);
expect(calls[0]?.args.at(-1)).toBe(inputPath);
});
it("redacts an input path fragment when the stderr tail starts inside its basename", async () => {
const diagnostic = "Invalid data found when processing input";
const inputPath = "/tmp/render/Confidential Client Preview.mp4";
const retainedPathFragment = "Client Preview.mp4";
const diagnosticPrefix = `: ${diagnostic} `;
const remainingBytes =
8 * 1024 - Buffer.byteLength(retainedPathFragment) - Buffer.byteLength(diagnosticPrefix);
const multibyteCount = Math.floor(remainingBytes / Buffer.byteLength("€"));
const trailingAscii = "x".repeat(remainingBytes - multibyteCount * Buffer.byteLength("€"));
const stderr = `${inputPath}${diagnosticPrefix}${"€".repeat(multibyteCount)}${trailingAscii}`;
const { spawn } = createSpawnSpy([{ kind: "exit", code: 1, stderr }]);
vi.resetModules();
vi.doMock("child_process", () => ({ spawn }));
const { extractAudioMetadata } = await import("./ffprobe.js");
let thrown: unknown;
try {
await extractAudioMetadata(inputPath);
} catch (error) {
thrown = error;
}
expect(String(thrown)).toContain(diagnostic);
expect(String(thrown)).toContain("[input]");
expect(String(thrown)).not.toContain(retainedPathFragment);
expect(String(thrown)).not.toContain("Client Preview.mp4");
});
it("extractAudioMetadata surfaces a ffprobe-missing error verbatim", async () => { it("extractAudioMetadata surfaces a ffprobe-missing error verbatim", async () => {
const { spawn, calls } = createSpawnSpy([{ kind: "missing" }]); const { spawn, calls } = createSpawnSpy([{ kind: "missing" }]);
hidePathBinaries(); hidePathBinaries();
+50 -17
View File
@@ -1,15 +1,54 @@
// fallow-ignore-file code-duplication complexity // fallow-ignore-file code-duplication complexity
import { spawn } from "child_process"; import { spawn } from "child_process";
import { readFileSync } from "fs"; import { readFileSync } from "fs";
import { extname } from "path"; import { basename, extname } from "path";
import { redactTelemetryString } from "@hyperframes/core";
import { FFPROBE_PATH_ENV, getFfprobeBinary } from "./ffmpegBinaries.js"; import { FFPROBE_PATH_ENV, getFfprobeBinary } from "./ffmpegBinaries.js";
import { ManagedChildProcess } from "./managedChildProcess.js"; import { ManagedChildProcess } from "./managedChildProcess.js";
import { trackChildProcess } from "./processTracker.js"; import { trackChildProcess } from "./processTracker.js";
const FFPROBE_STDERR_MAX_BYTES = 8 * 1024;
const FFPROBE_ERROR_MAX_CHARS = 4 * 1024;
function redactFfprobeInput(stderr: string, filePath: string): string {
if (!filePath) return stderr;
let redacted = stderr.split(filePath).join("[input]");
const inputBasename = basename(filePath);
if (inputBasename) redacted = redacted.split(inputBasename).join("[input]");
// ManagedChildProcess retains an 8 KiB byte tail. When that boundary lands
// inside the input path, stderr starts with only a suffix of the path, so
// neither exact-path nor generic absolute-path redaction can recognize it.
if (!redacted.startsWith("[input]")) {
for (let offset = 1; offset < filePath.length; offset += 1) {
const suffix = filePath.slice(offset);
if (suffix.length < 4 || !redacted.startsWith(suffix)) continue;
const boundary = redacted[suffix.length];
if (boundary !== undefined && !/[\s:'")\]}]/.test(boundary)) continue;
redacted = `[input]${redacted.slice(suffix.length)}`;
break;
}
}
return redacted;
}
function sanitizeFfprobeDiagnostic(stderr: string, filePath: string): string {
const stderrWithoutInput = redactFfprobeInput(stderr, filePath);
const redacted = redactTelemetryString(stderrWithoutInput, FFPROBE_STDERR_MAX_BYTES);
if (redacted.length <= FFPROBE_ERROR_MAX_CHARS) return redacted;
return `${redacted.slice(-(FFPROBE_ERROR_MAX_CHARS - 1))}`;
}
/** Spawn ffprobe with given args, return stdout. Throws on non-zero exit or missing binary. */ /** Spawn ffprobe with given args, return stdout. Throws on non-zero exit or missing binary. */
async function runFfprobe(args: string[], signal?: AbortSignal): Promise<string> { async function runFfprobe(
filePath: string,
argsWithoutInput: string[],
signal?: AbortSignal,
): Promise<string> {
const command = getFfprobeBinary(); const command = getFfprobeBinary();
const proc = spawn(command, args); const proc = spawn(command, ["-v", "error", ...argsWithoutInput, filePath]);
trackChildProcess(proc); trackChildProcess(proc);
let stdout = ""; let stdout = "";
proc.stdout.on("data", (data) => { proc.stdout.on("data", (data) => {
@@ -18,6 +57,7 @@ async function runFfprobe(args: string[], signal?: AbortSignal): Promise<string>
const managed = new ManagedChildProcess(proc, { const managed = new ManagedChildProcess(proc, {
signal, signal,
deadlineAtMs: Date.now() + 30_000, deadlineAtMs: Date.now() + 30_000,
stderrMaxBytes: FFPROBE_STDERR_MAX_BYTES,
}); });
const outcome = await managed.wait(); const outcome = await managed.wait();
if (outcome.reason === "spawn_error") { if (outcome.reason === "spawn_error") {
@@ -32,8 +72,9 @@ async function runFfprobe(args: string[], signal?: AbortSignal): Promise<string>
throw outcome.error ?? new Error(outcome.stderr); throw outcome.error ?? new Error(outcome.stderr);
} }
if (outcome.reason !== "exit" || outcome.exitCode !== 0) { if (outcome.reason !== "exit" || outcome.exitCode !== 0) {
const diagnostic = sanitizeFfprobeDiagnostic(outcome.stderr, filePath);
throw new Error( throw new Error(
`[FFmpeg] ffprobe ${outcome.reason} with code ${outcome.exitCode}: ${outcome.stderr}`, `[FFmpeg] ffprobe ${outcome.reason} with code ${outcome.exitCode}: ${diagnostic}`,
); );
} }
return stdout; return stdout;
@@ -263,14 +304,11 @@ export async function extractMediaMetadata(filePath: string): Promise<VideoMetad
let output: FFProbeOutput | null = null; let output: FFProbeOutput | null = null;
try { try {
const stdout = await runFfprobe([ const stdout = await runFfprobe(filePath, [
"-v",
"quiet",
"-print_format", "-print_format",
"json", "json",
"-show_format", "-show_format",
"-show_streams", "-show_streams",
filePath,
]); ]);
output = parseProbeJson(stdout); output = parseProbeJson(stdout);
} catch (error) { } catch (error) {
@@ -361,7 +399,8 @@ export async function extractAudioMetadata(
const probePromise = (async (): Promise<AudioMetadata> => { const probePromise = (async (): Promise<AudioMetadata> => {
const stdout = await runFfprobe( const stdout = await runFfprobe(
["-v", "quiet", "-print_format", "json", "-show_format", "-show_streams", filePath], filePath,
["-print_format", "json", "-show_format", "-show_streams"],
options?.signal, options?.signal,
); );
const output = parseProbeJson(stdout); const output = parseProbeJson(stdout);
@@ -373,9 +412,7 @@ export async function extractAudioMetadata(
const sampleRate = audioStream.sample_rate ? parseInt(audioStream.sample_rate) : 44100; const sampleRate = audioStream.sample_rate ? parseInt(audioStream.sample_rate) : 44100;
const audioCodec = audioStream.codec_name || "unknown"; const audioCodec = audioStream.codec_name || "unknown";
if (audioCodec === "aac" && sampleRate > 0) { if (audioCodec === "aac" && sampleRate > 0) {
const packetStdout = await runFfprobe([ const packetStdout = await runFfprobe(filePath, [
"-v",
"quiet",
"-select_streams", "-select_streams",
"a:0", "a:0",
"-count_packets", "-count_packets",
@@ -383,7 +420,6 @@ export async function extractAudioMetadata(
"stream=nb_read_packets", "stream=nb_read_packets",
"-print_format", "-print_format",
"json", "json",
filePath,
]); ]);
const packetOutput = parseProbeJson(packetStdout); const packetOutput = parseProbeJson(packetStdout);
const packetCount = Number(packetOutput.streams[0]?.nb_read_packets); const packetCount = Number(packetOutput.streams[0]?.nb_read_packets);
@@ -441,9 +477,7 @@ export async function analyzeKeyframeIntervals(filePath: string): Promise<Keyfra
} }
async function analyzeKeyframeIntervalsUncached(filePath: string): Promise<KeyframeAnalysis> { async function analyzeKeyframeIntervalsUncached(filePath: string): Promise<KeyframeAnalysis> {
const stdout = await runFfprobe([ const stdout = await runFfprobe(filePath, [
"-v",
"quiet",
"-select_streams", "-select_streams",
"v:0", "v:0",
"-skip_frame", "-skip_frame",
@@ -452,7 +486,6 @@ async function analyzeKeyframeIntervalsUncached(filePath: string): Promise<Keyfr
"frame=pts_time", "frame=pts_time",
"-of", "-of",
"csv=p=0", "csv=p=0",
filePath,
]); ]);
const timestamps = stdout const timestamps = stdout