From e79ab3ab31a5f789f8c998430f2b58cb95973f07 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Sat, 1 Aug 2026 17:53:23 -0700 Subject: [PATCH] fix(core,producer): redact bare relative paths and the known input path The generic scrub still missed a relative path with no `./` prefix: `customer/acme-secret/video.mp4` and `assets/bgm.mp3` reached telemetry completely unredacted, because the absolute rule needs a leading slash and the `./` rule needs the dot. Adds a rule for them that still leaves `N/A`, `24/1` and `48000/1001` alone. Shape matching is a net with holes by construction, so audioPadTrim now also redacts the exact path it put in the argv, plus its basename, before the generic scrub runs. It built the argv, so it does not have to guess. Co-Authored-By: Claude Opus 5 (1M context) --- packages/core/src/index.ts | 2 +- packages/core/src/telemetryRedaction.test.ts | 50 +++++++++++++++++- packages/core/src/telemetryRedaction.ts | 51 +++++++++++++++++++ .../src/services/render/audioPadTrim.ts | 13 +++-- 4 files changed, 110 insertions(+), 6 deletions(-) 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;