diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 8da2e39fe..108b87bb8 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/telemetryRedaction.test.ts b/packages/core/src/telemetryRedaction.test.ts index bd5737033..0420ec3fe 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", () => { @@ -65,4 +65,52 @@ describe("redactTelemetryString", () => { 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]", + ); + }); +}); + +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"); + }); }); diff --git a/packages/core/src/telemetryRedaction.ts b/packages/core/src/telemetryRedaction.ts index 3a2de994e..941f0f981 100644 --- a/packages/core/src/telemetryRedaction.ts +++ b/packages/core/src/telemetryRedaction.ts @@ -17,6 +17,9 @@ function redactUrlQueryStrings(value: string): string { */ const SEGMENT = String.raw`[\w.\-@+()~]+`; +/** Same, minus the dot, so a trailing `.ext` can be matched separately. */ +const SEGMENT_NODOT = String.raw`[\w\-@+()~]+`; + /** * 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 @@ -62,6 +65,53 @@ const RELATIVE_PATH = new RegExp( const ASSET_BASENAME = /(? v.length > 2)) { + out = out.split(literal).join("[path]"); + } + } + return out; +} + function redactFilePaths(value: string): string { return ( value @@ -71,6 +121,7 @@ function redactFilePaths(value: string): string { // leading `.` stranded outside the redaction. .replace(RELATIVE_PATH, "[path]") .replace(ABSOLUTE_PATH, "[path]") + .replace(BARE_RELATIVE_PATH, "[path]") .replace(ASSET_BASENAME, "[file]") ); } diff --git a/packages/producer/src/services/render/audioPadTrim.ts b/packages/producer/src/services/render/audioPadTrim.ts index 7ba6f37b1..fe0346b7d 100644 --- a/packages/producer/src/services/render/audioPadTrim.ts +++ b/packages/producer/src/services/render/audioPadTrim.ts @@ -30,7 +30,7 @@ import { trackChildProcess, type AudioMetadata, } from "@hyperframes/engine"; -import { redactTelemetryString } from "@hyperframes/core"; +import { redactKnownPaths, redactTelemetryString } from "@hyperframes/core"; /** * Tolerance used to decide whether an audio file is already short enough to @@ -461,9 +461,14 @@ async function runFfprobeJson(args: string[], signal?: AbortSignal): Promise< throw outcome.error ?? new Error(outcome.stderr); } if (outcome.reason !== "exit" || outcome.exitCode !== 0) { - // Redacted: raw ffprobe stderr echoes the input path, and this message - // reaches logs and telemetry. - throw new Error(`ffprobe ${outcome.reason}: ${redactTelemetryString(outcome.stderr, 2000)}`); + // 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;