mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-12 07:09:59 +00:00
fix(core): redact any path in telemetry, not an allowlist of roots
redactTelemetryString enumerated roots — /Users, /home, /opt, /tmp and a handful more — so a project on /data, /Volumes, an NFS mount or any root a user invented reached telemetry verbatim. Relative paths and bare basenames were never redacted at all, and audioPadTrim routes raw ffprobe stderr through this on every probe failure. Now redacts by shape: absolute paths under any root (two or more segments, so N/A and a 24/1 frame rate are not mistaken for one), relative paths including dash-prefixed ones, and bare basenames with an asset extension. URLs still keep their host and drop only the query. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
91a7cb1f5b
commit
d04569e37f
@@ -16,4 +16,53 @@ describe("redactTelemetryString", () => {
|
|||||||
),
|
),
|
||||||
).toBe("[path] [path] [path] [path] [file-url] https://example.com/video.mp4?…");
|
).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");
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -9,13 +9,70 @@ function redactUrlQueryStrings(value: string): string {
|
|||||||
return value.replace(/\b(https?:\/\/[^\s?]+)\?[^\s]*/g, "$1?…");
|
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.
|
||||||
|
*/
|
||||||
|
const SEGMENT = 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
|
||||||
|
* 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`(?<![:\w/\\])(?:[A-Za-z]:)?(?:[\\/]${SEGMENT}){2,}${TOKEN_TAIL}`,
|
||||||
|
"g",
|
||||||
|
);
|
||||||
|
|
||||||
|
/** `./assets/bgm.mp3`, `../out.wav`, `.\tmp\x` — relative paths leak the same
|
||||||
|
* project structure absolute ones do, and were previously untouched. */
|
||||||
|
const RELATIVE_PATH = new RegExp(
|
||||||
|
String.raw`(?<![\w/\\.])\.{1,2}(?:[\\/]${SEGMENT})+${TOKEN_TAIL}`,
|
||||||
|
"g",
|
||||||
|
);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A bare basename with an asset extension. ffprobe reports the input by the
|
||||||
|
* name it was given, so a caller that passes a basename (or a path this
|
||||||
|
* flattened to its last segment) still names the user's file.
|
||||||
|
*
|
||||||
|
* The lookbehind excludes a slash so this cannot re-redact the tail of a URL
|
||||||
|
* whose host we deliberately keep.
|
||||||
|
*/
|
||||||
|
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;
|
||||||
|
|
||||||
function redactFilePaths(value: string): string {
|
function redactFilePaths(value: string): string {
|
||||||
return value
|
return (
|
||||||
.replace(/file:\/\/[^\s'")]+/g, "[file-url]")
|
value
|
||||||
.replace(/\/Users\/[^\s'")]+/g, "[path]")
|
.replace(/file:\/\/[^\s'")]+/g, "[file-url]")
|
||||||
.replace(/\/(?:home|root|opt|app|workspace|srv|mnt)\/[^\s'")]+/g, "[path]")
|
// Relative BEFORE absolute: `./assets/x.mp3` has an absolute-looking tail
|
||||||
.replace(/\/(?:private\/)?(?:var|tmp)\/[^\s'")]+/g, "[path]")
|
// (`/assets/x.mp3`), so the absolute rule would consume it and leave the
|
||||||
.replace(/[A-Za-z]:\\[^\s'")]+/g, "[path]");
|
// leading `.` stranded outside the redaction.
|
||||||
|
.replace(RELATIVE_PATH, "[path]")
|
||||||
|
.replace(ABSOLUTE_PATH, "[path]")
|
||||||
|
.replace(ASSET_BASENAME, "[file]")
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
export function redactTelemetryString(
|
export function redactTelemetryString(
|
||||||
|
|||||||
Reference in New Issue
Block a user