mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-11 14:50:02 +00:00
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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
d04569e37f
commit
e79ab3ab31
@@ -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,
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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 =
|
||||
/(?<![\w/\\])[\w.\-@+()~]+\.(?:mp4|mov|mkv|webm|avi|m4v|mpe?g|ts|mp3|wav|aac|m4a|flac|ogg|opus|png|jpe?g|gif|webp|svg|html?|json|srt|vtt|ass)\b/gi;
|
||||
|
||||
/**
|
||||
* A relative path with NO `./` prefix — `assets/bgm.mp3`,
|
||||
* `customer/acme-secret/video.mp4`. These leak exactly as much as an absolute
|
||||
* path and were missed by both rules above: the absolute rule requires a
|
||||
* leading slash, and the `./` rule requires the dot.
|
||||
*
|
||||
* Qualifying needs either two separators or one plus a file extension, so the
|
||||
* ordinary non-paths in ffprobe stderr — `N/A`, a `24/1` frame rate,
|
||||
* `48000/1001` — do not match. The lookbehind keeps it off URL paths, whose
|
||||
* host is deliberately kept.
|
||||
*/
|
||||
const BARE_RELATIVE_PATH = new RegExp(
|
||||
[
|
||||
// Two or more separators: `customer/acme/video.mp4`. No extension needed —
|
||||
// that much structure is already a path.
|
||||
String.raw`(?<![\w/\\.:@-])(?:${SEGMENT}[\\/]){2,}${SEGMENT}${TOKEN_TAIL}`,
|
||||
// One separator, but the last segment carries a file extension:
|
||||
// `assets/bgm.mp3`. That segment is dot-free on purpose — SEGMENT includes
|
||||
// `.`, so a greedy one swallows the extension this rule needs.
|
||||
String.raw`(?<![\w/\\.:@-])${SEGMENT}[\\/]${SEGMENT_NODOT}\.\w{1,8}\b${TOKEN_TAIL}`,
|
||||
].join("|"),
|
||||
"g",
|
||||
);
|
||||
|
||||
/**
|
||||
* Redact literal strings — the exact paths a caller KNOWS it passed — before
|
||||
* any shape-based rule runs.
|
||||
*
|
||||
* Shape matching is a net with holes by construction; this is not. When the
|
||||
* caller has the path in hand (it built the argv), spell it out rather than
|
||||
* hoping a regex recognises it, and take the basename too since ffprobe often
|
||||
* reports only that.
|
||||
*/
|
||||
export function redactKnownPaths(value: string, paths: readonly string[]): string {
|
||||
let out = value;
|
||||
for (const path of paths) {
|
||||
if (typeof path !== "string" || path.length === 0) continue;
|
||||
// Longest first: replacing the basename before the full path would leave
|
||||
// the directory prefix stranded.
|
||||
const basename = path.split(/[\\/]/).pop() ?? "";
|
||||
for (const literal of [path, basename].filter((v) => 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]")
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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<T>(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;
|
||||
|
||||
Reference in New Issue
Block a user