diff --git a/packages/core/src/telemetryRedaction.test.ts b/packages/core/src/telemetryRedaction.test.ts index 670f1a4ea..bd5737033 100644 --- a/packages/core/src/telemetryRedaction.test.ts +++ b/packages/core/src/telemetryRedaction.test.ts @@ -16,4 +16,53 @@ describe("redactTelemetryString", () => { ), ).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"); + }); }); diff --git a/packages/core/src/telemetryRedaction.ts b/packages/core/src/telemetryRedaction.ts index 5e05ab7a1..3a2de994e 100644 --- a/packages/core/src/telemetryRedaction.ts +++ b/packages/core/src/telemetryRedaction.ts @@ -9,13 +9,70 @@ function redactUrlQueryStrings(value: string): string { 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`(?