-
Notifications
You must be signed in to change notification settings - Fork 133
fix: mask filesystem paths in telemetry error text #1117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
167a5f2
4899189
9d56682
40cb7fe
b466d17
12728e0
acef14f
fabd76b
ab8417d
7b794e7
6e1d639
a514316
8801300
f64496d
a880343
7d8678e
be0daf9
f8939f1
c963b71
499e956
7eb8382
a5f79d8
6a0dfd3
43b5d68
454cb28
78f92a4
adce061
96dbe0e
7565405
b75d618
7707270
af81386
1e1e2a3
6bdbd1b
ddf1dad
0b43716
f902eea
a811a23
2e1275f
abd0ff4
aa5d6d1
ced57f4
a32d716
35320ed
22f0c54
4acad5e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -45,6 +45,7 @@ const log = Log.create({ service: "telemetry" }) | |||||||||||||||||||||||||||||||
| * | extend err = tostring(customDimensions.error_class) | ||||||||||||||||||||||||||||||||
| * | summarize count() by err | ||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // altimate_change end | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| /** True when a test runner is driving the process rather than a real user session. | ||||||||||||||||||||||||||||||||
|
|
@@ -64,6 +65,91 @@ function isAutomatedRun(): boolean { | |||||||||||||||||||||||||||||||
| return Boolean(process.env.BUN_TEST || process.env.VITEST || process.env.JEST_WORKER_ID) | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // altimate_change start — composed path-masking rules (shared fragments) | ||||||||||||||||||||||||||||||||
| // R: one path-run character — anything but whitespace/separators/quotes, | ||||||||||||||||||||||||||||||||
| // plus an apostrophe when a word character follows (O'Connor vs closing '). | ||||||||||||||||||||||||||||||||
| const PM_R = "(?:[^\\s\\/\\\\'\"`]|'(?=[\\p{L}\\p{N}_]))" | ||||||||||||||||||||||||||||||||
| // slash-delimited variant: backslash is path content, not a separator | ||||||||||||||||||||||||||||||||
| const PM_R_P = "(?:[^\\s\\/'\"`]|'(?=[\\p{L}\\p{N}_]))" | ||||||||||||||||||||||||||||||||
| const PM_WORD = "(?:[\\p{L}\\p{M}\\p{N}_‘’-]|'(?=[\\p{L}\\p{N}_]))" | ||||||||||||||||||||||||||||||||
| const PM_ANCHOR = "(^|[\\s\"'`=(,[{:;<|>)\\]}&])" | ||||||||||||||||||||||||||||||||
| const PM_SP = "[^\\S\\t\\n\\r\\v\\f]" | ||||||||||||||||||||||||||||||||
| const PM_EXT = "\\.[\\p{L}\\p{M}\\p{N}-]{0,29}[\\p{L}\\p{M}\\p{N}]" | ||||||||||||||||||||||||||||||||
| // span char: path content incl. delimiters (, ; ) ] } >) that a later | ||||||||||||||||||||||||||||||||
| // separator — or an attached dotted terminal filename (;draft.sql) — | ||||||||||||||||||||||||||||||||
| // proves is path content — multi-word spaced runs allowed, all | ||||||||||||||||||||||||||||||||
| // quantified units space- or separator-anchored with disjoint inner classes | ||||||||||||||||||||||||||||||||
| // (unambiguous parse => linear time; the nested-quantifier ReDoS shape is | ||||||||||||||||||||||||||||||||
| // banned here). | ||||||||||||||||||||||||||||||||
| const SEP_P = "\\/" | ||||||||||||||||||||||||||||||||
| const SEP_W = "[\\\\\\/]" | ||||||||||||||||||||||||||||||||
| // per-letter case expansion — used instead of the i flag on home/cloud | ||||||||||||||||||||||||||||||||
| // rules: under /iu, conformant engines case-fold \p{Lu}, which would turn | ||||||||||||||||||||||||||||||||
| // the capitalized-tail gate into "match any word" (V8 folds; JSC does not — | ||||||||||||||||||||||||||||||||
| // never rely on the divergence) | ||||||||||||||||||||||||||||||||
| const pmCI = (w: string) => w.split("").map((c) => ("[" + c + c.toUpperCase() + "]")).join("") | ||||||||||||||||||||||||||||||||
| const pmR = (sep: string) => (sep === SEP_P ? PM_R_P : PM_R) | ||||||||||||||||||||||||||||||||
| const PM_WR = "(?:[^\\s\\/\\\\'\"`,;:\\]}>]|'(?=[\\p{L}\\p{N}_]))" | ||||||||||||||||||||||||||||||||
| const PM_WR_P = "(?:[^\\s\\/'\"`,;:\\]}>]|'(?=[\\p{L}\\p{N}_]))" | ||||||||||||||||||||||||||||||||
| const pmWR = (sep: string) => (sep === SEP_P ? PM_WR_P : PM_WR) | ||||||||||||||||||||||||||||||||
| const PM_SEG_L = "(?=[^\\s'\"`]{0,128}[\\p{L}\\p{M}]|[\\s'\"`,;)\\]}>]|$)" | ||||||||||||||||||||||||||||||||
| const pmSpan = (sep: string) => | ||||||||||||||||||||||||||||||||
| "(?:[^\\s'\"`)\\]},;>]|'(?=[\\p{L}\\p{N}_])|[,;)\\]}>](?=" + pmR(sep) + "{0,256}(?:" + PM_SP + "{1,2}" + pmWR(sep) + "{1,64}){0,2}(?:" + sep + PM_SEG_L + "|" + PM_EXT + "(?=$|[\\s.,;:)\\]}!?])))|[\"'`](?=" + pmR(sep) + "{1,256}(?:" + PM_SP + "{1,2}" + pmWR(sep) + "{1,64}){0,2}(?:" + sep + PM_SEG_L + "|" + PM_EXT + "(?=$|[\\s.,;:)\\]}!?]))))" | ||||||||||||||||||||||||||||||||
| const pmChunks = (sep: string) => | ||||||||||||||||||||||||||||||||
| "(?:(?:" + PM_SP + "{1,2}" + pmWR(sep) + "{1,64}){1,2}" + sep + PM_SEG_L + pmSpan(sep) + "*){0,2}" | ||||||||||||||||||||||||||||||||
| // terminal dotted filename: up to four spaced words that END in an extension | ||||||||||||||||||||||||||||||||
| const pmSpFile = (sep: string) => "(?:(?:" + PM_SP + "{1,2}" + pmR(sep) + "+){1,4}(?<=" + PM_EXT + "))?" | ||||||||||||||||||||||||||||||||
|
Comment on lines
+100
to
+101
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When an explicit shallow path has five or more spaced continuations, for example Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in |
||||||||||||||||||||||||||||||||
| const PM_TERM_COND = "(?:(?<!" + PM_EXT + ")" + PM_SP + "{1,2}" + PM_WORD + "+(?=$|[.,;:)\\]}!?]))?" | ||||||||||||||||||||||||||||||||
| const PM_TERM_UNC = | ||||||||||||||||||||||||||||||||
| "(?:(?<!" + PM_EXT + ")" + PM_SP + "{1,2}" + PM_WORD + "+(?:" + PM_SP + "{1,2}(?:[\\p{Lu}\\p{Lo}]" + PM_WORD + "*|(?:v[ao]n|de[nrl]?|d[aiou]|dos|la|les?|los|bin|ibn|al|el|te[nr])(?=" + PM_SP + ")))*)?" | ||||||||||||||||||||||||||||||||
| const PM_WC = "(?:[\\p{L}\\p{N}_#@().'-]{2,}|[\\p{L}\\p{N}_#@().'+&-]{3,})" | ||||||||||||||||||||||||||||||||
| const PM_WCC = "[\\p{L}\\p{N}_#@().'+&-]+" | ||||||||||||||||||||||||||||||||
| const pmSpFileX = (sep: string) => "(?:(?:" + PM_SP + "{1,2}" + pmR(sep) + "{1,64}){1,12}(?<=" + PM_EXT + "))?" | ||||||||||||||||||||||||||||||||
| const pmTail = (sep: string, term: string) => pmSpan(sep) + "*" + pmChunks(sep) + pmSpFile(sep) + term | ||||||||||||||||||||||||||||||||
| // Known-prefix literals — the local user's home and cwd are KNOWN values, | ||||||||||||||||||||||||||||||||
| // replaced by exact match AFTER the structural rules (structure must see the | ||||||||||||||||||||||||||||||||
| // original string: stripping the prefix first orphans terminal spaced | ||||||||||||||||||||||||||||||||
| // components). The literal pass mops up whatever structure missed. Exact | ||||||||||||||||||||||||||||||||
| // matching handles every username shape (spaces, NBSP, unicode) with zero | ||||||||||||||||||||||||||||||||
| // false positives; the structural rules below remain for paths the | ||||||||||||||||||||||||||||||||
| // literals cannot know (other drives, UNC shares, cloud URIs, relative | ||||||||||||||||||||||||||||||||
| // forms, WSL-mounted homes). Variants cover JSON-doubled backslashes and | ||||||||||||||||||||||||||||||||
| // swapped separators. Same approach as Salesforce's telemetry GDPR scrub | ||||||||||||||||||||||||||||||||
| // (os.homedir() literal) and gatsby-telemetry's cleanPaths (cwd prefixes). | ||||||||||||||||||||||||||||||||
| const pmEscape = (v: string) => v.replace(/[-[\]/{}()*+?.\\^$|]/g, "\\$&") | ||||||||||||||||||||||||||||||||
| const pmPrefixVariants = (root: string): RegExp[] => { | ||||||||||||||||||||||||||||||||
| if (!root || root.length < 4) return [] | ||||||||||||||||||||||||||||||||
| const out: RegExp[] = [] | ||||||||||||||||||||||||||||||||
| for (const v of new Set([root, root.replace(/\\/g, "\\\\"), root.replace(/\\/g, "/")])) { | ||||||||||||||||||||||||||||||||
| out.push(new RegExp("(?<![\\w.-])" + pmEscape(v), "gi")) | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| return out | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| const PM_HOME_PREFIXES = pmPrefixVariants(os.homedir()) | ||||||||||||||||||||||||||||||||
| // The CLI chdirs into the project after this module loads (tui/attach/run), | ||||||||||||||||||||||||||||||||
| // so the cwd literals are rebuilt whenever process.cwd() changes — a stale | ||||||||||||||||||||||||||||||||
| // import-time cwd would miss exactly the shallow, extensionless project root | ||||||||||||||||||||||||||||||||
| // the structural rules cannot mask. | ||||||||||||||||||||||||||||||||
| let pmCwdCache = "" | ||||||||||||||||||||||||||||||||
| let pmCwdPrefixes: RegExp[] = [] | ||||||||||||||||||||||||||||||||
| function pmKnownPrefixes(): RegExp[] { | ||||||||||||||||||||||||||||||||
| const cwd = process.cwd() | ||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: When the process cwd has been deleted or becomes inaccessible, Prompt for AI agents
Suggested change
|
||||||||||||||||||||||||||||||||
| if (cwd !== pmCwdCache) { | ||||||||||||||||||||||||||||||||
| pmCwdCache = cwd | ||||||||||||||||||||||||||||||||
| pmCwdPrefixes = pmPrefixVariants(cwd) | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| return [...pmCwdPrefixes, ...PM_HOME_PREFIXES] | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| const PATH_RULES = { | ||||||||||||||||||||||||||||||||
| cloud: new RegExp(PM_ANCHOR + "(?:(?:" + [pmCI("gs"), pmCI("s3") + "[anAN]?", pmCI("abfs") + "[sS]?", pmCI("wasb") + "[sS]?", pmCI("adl"), pmCI("dbfs"), pmCI("hdfs")].join("|") + "):\\/\\/|" + pmCI("file") + ":\\/{1,3})" + pmTail(SEP_P, PM_TERM_UNC), "gu"), | ||||||||||||||||||||||||||||||||
| windowsHome: new RegExp(PM_ANCHOR + "(?:(?:\\\\\\\\\\?\\\\)?[A-Za-z]:" + SEP_W + "{0,2}|(?:\\\\\\\\(?:\\?\\\\" + pmCI("unc") + "\\\\)?|(?<!:)\\/\\/(?=[^\\s\\/\\\\]+" + SEP_W + "))(?:" + PM_R + "+(?:" + PM_SP + "{1,2}" + PM_WR + "{1,64})*" + SEP_W + "{1,2})*|" + SEP_W + "{1,2})(?:" + pmCI("users") + "|" + pmCI("documents") + " " + pmCI("and") + " " + pmCI("settings") + ")" + SEP_W + "{1,2}" + pmTail(SEP_W, PM_TERM_UNC), "gu"), | ||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocking — The commit bounded The inner spaced-word repetition is still Isolated to I want to be precise about what's new here, because most of it isn't. Checked against One caveat on reproducing this: the trailing path has to be attached to the last prose word by a separator. With a space before it ( Suggested fix: compose this prefix from the same bounded primitives the other rules now use. Also worth adding a bridging control per family — the new tests in this area are POSIX-only, which is why nothing caught this.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocking — FQDN UNC The reply says the disambiguator is " So the two spellings still disagree on the same path — the same defect as last round, moved from Suggested fix: add case-expanded Every schemed public URL is protected by the |
||||||||||||||||||||||||||||||||
| windows: new RegExp(PM_ANCHOR + "(?:[A-Za-z]:" + SEP_W + "|(?<!:)\\/\\/(?=[^\\s\\/\\\\.]+" + SEP_W + ")|[A-Za-z]:(?=" + PM_R + "{1,256}(?:" + PM_SP + "{1,2}" + PM_R + "{1,256})*\\\\|(?=[^\\s\\/\\\\]{0,256}[\\p{L}])" + PM_R + "{1,256}(?:" + PM_SP + "{1,2}" + PM_R + "{1,256})*\\/|" + PM_R + "{1,256}(?:" + PM_SP + "{1,2}" + PM_R + "{1,256}){0,6}(?<=\\.[\\p{L}\\p{M}\\p{N}-]{0,29})(?<=[\\p{L}\\p{M}][\\p{L}\\p{M}\\p{N}-]{0,29})(?=$|[\\s.,;:)\\]}!?]))|\\\\\\\\|\\.{1,2}\\\\(?=" + PM_R + "{1,256}(?:" + PM_SP + "{1,2}" + PM_R + "{1,256})*\\\\|[^\\s\\\\]{1,256}" + PM_EXT + "(?=$|[\\s.,;:)\\]}!?]))|\\\\(?=(?:" + PM_WC + "(?:" + PM_SP + "{1,2}" + PM_WCC + ")*\\\\){2}|" + PM_WC + "(?:" + PM_SP + "{1,2}" + PM_WCC + ")*\\\\[^\\s\\\\]{1,256}" + PM_EXT + "(?=$|[\\s.,;:)\\]}!?])|" + PM_WC + "(?:" + PM_SP + "{1,2}" + PM_WCC + ")+\\\\[\\p{L}\\p{N}]|[\\p{L}\\p{N}_-]\\\\(?:[\\p{L}\\p{N}_-]{2,}|\\.[\\p{L}\\p{N}_-]{2,})|[^\\s\\\\]{1,256}" + PM_EXT + "(?=$|[\\s.,;:)\\]}!?])))" + pmSpan(SEP_W) + "+" + pmChunks(SEP_W) + pmSpFile(SEP_W) + PM_TERM_COND, "gu"), | ||||||||||||||||||||||||||||||||
| posixHome: new RegExp(PM_ANCHOR + "\\/(?:" + PM_R_P + "+(?:" + PM_SP + "{1,2}" + PM_WR_P + "{1,64}){0,2}\\/" + PM_SEG_L + ")*(?:" + pmCI("users") + "|" + pmCI("home") + ")\\/" + pmTail(SEP_P, PM_TERM_UNC), "gu"), | ||||||||||||||||||||||||||||||||
| posix: new RegExp(PM_ANCHOR + "(?:\\.{0,2}\\/(?:" + PM_R_P + "+(?:" + PM_SP + "{1,2}" + PM_WR_P + "{1,64}){0,2}\\/" + PM_SEG_L + ")+" + pmTail(SEP_P, PM_TERM_COND) + "|(?:\\.{1,2}\\/|\\/(?!\\/))" + pmSpan(SEP_P) + "+" + pmSpFileX(SEP_P) + "(?<=" + PM_EXT + ")(?=$|[\\s.,;:)\\]}!?]))", "gu"), | ||||||||||||||||||||||||||||||||
| tilde: new RegExp(PM_ANCHOR + "~[\\p{L}\\p{M}\\p{N}_.-]*(?:\\/|\\\\(?=" + PM_WC + "(?:" + PM_SP + "{1,2}" + PM_WCC + ")*[\\\\\\/]|\\.[\\p{L}\\p{N}_-]{2,}[\\\\\\/]|[\\p{L}\\p{N}_-]\\\\(?:[\\p{L}\\p{N}_-]{2,}|\\.[\\p{L}\\p{N}_-]{2,})))" + pmTail(SEP_W, PM_TERM_UNC), "gu"), | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| // altimate_change end | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| export namespace Telemetry { | ||||||||||||||||||||||||||||||||
| const FLUSH_INTERVAL_MS = 5_000 | ||||||||||||||||||||||||||||||||
| const MAX_BUFFER_SIZE = 200 | ||||||||||||||||||||||||||||||||
|
|
@@ -1381,12 +1467,64 @@ export namespace Telemetry { | |||||||||||||||||||||||||||||||
| // Bearer … Authorization headers leaked in error text | ||||||||||||||||||||||||||||||||
| // Each match replaces with a fixed redaction so length-based fingerprinting | ||||||||||||||||||||||||||||||||
| // can't reconstruct the original token. | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| export function maskString(s: string): string { | ||||||||||||||||||||||||||||||||
| return s | ||||||||||||||||||||||||||||||||
| // Consumers truncate masked output to <= 2000 chars; masking beyond 8 KB | ||||||||||||||||||||||||||||||||
| // of input buys nothing and unbounded input is what turns any super- | ||||||||||||||||||||||||||||||||
| // linear rule into a stall (a wide generated SELECT reached seconds). | ||||||||||||||||||||||||||||||||
| if (s.length > 8192) s = s.slice(0, 8192) | ||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocking — the 8 KB entry truncation runs before redaction, so a credential straddling the cut ships in the clear. This one is on me: I suggested this truncation in the last round, and it was the wrong advice.
Reproduced with a long quoted literal in front — it collapses to The whole masked output is 11–21 characters in each case, so the Path rules are unaffected. I walked a The truncation also isn't buying anything. Your
Linear everywhere, 8.8 ms worst case on half a megabyte. Suggested fix: mask first, truncate the masked result. A whitespace-boundary cut is not a sufficient substitute — an unterminated quoted value containing spaces still loses its closing quote. If you want to keep an input cap for defence in depth, run the three cheap credential rules on the full string first and truncate only before the path stack. Two smaller consequences worth a line in the comment: two long SQL strings identical for their first 8,192 characters now produce identical masked text and therefore an identical Separately, |
||||||||||||||||||||||||||||||||
| let out = s | ||||||||||||||||||||||||||||||||
| // ANSI CSI sequences (colored subprocess stderr) would otherwise split | ||||||||||||||||||||||||||||||||
| // tokens so neither credential nor path rules can see them | ||||||||||||||||||||||||||||||||
| .replace(/\x1b(?:\[[0-?]*[ -\/]*[@-~]|\][^\x07\x1b]*(?:\x07|\x1b\\))/g, "") | ||||||||||||||||||||||||||||||||
| .replace(/sk-(?:ant-)?[A-Za-z0-9_-]{20,}/g, "sk-***") | ||||||||||||||||||||||||||||||||
| .replace(/Bearer\s+[A-Za-z0-9._-]{20,}/gi, "Bearer ***") | ||||||||||||||||||||||||||||||||
| // Fast path: a string with no separator cannot contain a path — skip the | ||||||||||||||||||||||||||||||||
| // whole path stack (most telemetry strings carry no path at all). | ||||||||||||||||||||||||||||||||
| if (out.includes("/") || out.includes("\\") || /(?<![A-Za-z0-9])[A-Za-z]:[^\s:]{1,255}(?: [^\s:]{1,255}){0,2}\.[A-Za-z]/.test(out)) { | ||||||||||||||||||||||||||||||||
| out = out | ||||||||||||||||||||||||||||||||
| // altimate_change start — mask filesystem paths in error text | ||||||||||||||||||||||||||||||||
| // Six masking rules (cloud URIs, Windows home, Windows/UNC incl. .\ and | ||||||||||||||||||||||||||||||||
| // ..\, POSIX home, POSIX incl. ./ and ../, ~ incl. ~username), composed from shared | ||||||||||||||||||||||||||||||||
| // fragments below (PATH_RULES) — one source of truth after repeated | ||||||||||||||||||||||||||||||||
| // lockstep edits drifted (see PR history). Ordered after the credential | ||||||||||||||||||||||||||||||||
| // rules and BEFORE the email/internal-host rules so whole URIs mask | ||||||||||||||||||||||||||||||||
| // before userinfo can fragment into <email>. Public URL interiors are | ||||||||||||||||||||||||||||||||
| // structurally safe: after "https:" comes "//", which cannot start a | ||||||||||||||||||||||||||||||||
| // segment chain. Doctrine: over-masking is the correct failure mode. | ||||||||||||||||||||||||||||||||
| // HOME-ROOTED paths and CLOUD URIs consume one unconditional trailing | ||||||||||||||||||||||||||||||||
| // word (spaced usernames / object keys — the high-PII classes), | ||||||||||||||||||||||||||||||||
| // suppressed after a dotted extension so "x.sql was deleted" prose | ||||||||||||||||||||||||||||||||
| // survives; other rules consume a trailing word only at end-of-string / | ||||||||||||||||||||||||||||||||
|
ralphstodomingo marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||
| // before punctuation — except spaced terminal FILENAMES, which may span | ||||||||||||||||||||||||||||||||
| // interior words when the run ends in a dotted extension (up to 4 words | ||||||||||||||||||||||||||||||||
| // on deep paths, 12 on explicit shallow ./-style paths). Residue (by design): one prose word may be | ||||||||||||||||||||||||||||||||
| // over-masked after extensionless home/cloud paths; a non-home, | ||||||||||||||||||||||||||||||||
| // non-cloud path's terminal spaced component can leak ONE structure | ||||||||||||||||||||||||||||||||
| // word mid-sentence (no personal names in that class); a delimiter | ||||||||||||||||||||||||||||||||
| // followed by neither a further separator nor a dotted terminal | ||||||||||||||||||||||||||||||||
| // filename is a permanent boundary. | ||||||||||||||||||||||||||||||||
| .replace(/(^|[\s"'`=(,[{:;<|>)\]}&])[\\/]{4,}(?=$|[\s"'`,;)\]}<>|&])/g, "$1<path>") | ||||||||||||||||||||||||||||||||
| .replace(PATH_RULES.cloud, "$1<path>") | ||||||||||||||||||||||||||||||||
| // the windows rules carry the widest opener alternation — they cannot | ||||||||||||||||||||||||||||||||
| // match without a backslash, a boundary drive-colon, or a non-scheme // | ||||||||||||||||||||||||||||||||
| if (out.includes("\\") || /(?<![A-Za-z0-9])[A-Za-z]:/.test(out) || /(?<!:)\/\//.test(out)) { | ||||||||||||||||||||||||||||||||
| out = out.replace(PATH_RULES.windowsHome, "$1<path>").replace(PATH_RULES.windows, "$1<path>") | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| out = out | ||||||||||||||||||||||||||||||||
| .replace(PATH_RULES.posixHome, "$1<path>") | ||||||||||||||||||||||||||||||||
| .replace(PATH_RULES.posix, "$1<path>") | ||||||||||||||||||||||||||||||||
| .replace(PATH_RULES.tilde, "$1<path>") | ||||||||||||||||||||||||||||||||
| for (const re of pmKnownPrefixes()) out = out.replace(re, "<path>") | ||||||||||||||||||||||||||||||||
| out = out | ||||||||||||||||||||||||||||||||
| // a literal-prefix mask followed by a structurally-masked remainder | ||||||||||||||||||||||||||||||||
| // collapses to one marker | ||||||||||||||||||||||||||||||||
| .replace(/<path>(?:[\\/]?<path>)+/g, "<path>") | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| return out | ||||||||||||||||||||||||||||||||
| // altimate_change end | ||||||||||||||||||||||||||||||||
| // Email addresses — providers occasionally echo caller identity in error text. | ||||||||||||||||||||||||||||||||
| .replace(/[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}/g, "<email>") | ||||||||||||||||||||||||||||||||
| .replace(/(?<![A-Za-z0-9._%+-])[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}/g, "<email>") | ||||||||||||||||||||||||||||||||
| // Internal hostnames in URLs — keeps parity with `parseAPICallError`'s | ||||||||||||||||||||||||||||||||
| // `maskInternalHost` so an error message containing the same URL doesn't | ||||||||||||||||||||||||||||||||
| // leak through telemetry while metadata.url is masked. Covers: | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blocking — the two-word bridge cap re-opens the spaced-path leak: a directory component of 4+ words now leaks its tail, in every rule family.
pmChunksis(?:(?:SP{1,2} WR{1,64}){1,2} sep SEG_L span*){0,2}— at most two bridge words per chunk, at most two chunks. A directory component with four or more space-separated words exhausts that budget mid-component; the match stops there and the remainder is emitted raw. This regresses4899189e7, the commit that fixed the original spaced-path finding.22f0c5414acad5earead /data/my big client folder/models/x.sqlread <path>read <path> big client folder/models/x.sqlread /srv/acme corp data warehouse/models/x.sqlread <path>read <path> corp data warehouse/models/x.sqlread C:\data\my big client folder\models\x.sqlread <path>read <path> big client folder\models\x.sqls3://bucket/my big client folder/models/x.parquet<path><path> client folder/models/x.parquetread ~/w0 w1 w2 w3/models/x.sqlread <path>read <path> w2 w3/models/x.sqlread //srv/share/Users/jdoe/w0 w1 w2 w3/x.sqlread <path>read <path> w2 w3/x.sqlSwept component width 1→6: 1–3 mask, 4+ leak. Strictly per-component —
/data/w0 w1 w2/w3 w4 w5/x.sql(six words over two components) masks cleanly,/data/w0 w1 w2 w3/x.sql(four in one) does not. A second, rarer trigger is the{0,2}chunk count, which is what breaks/Users/Jane Doe/client repo/deep nested dir/model.sqleven though no component exceeds three words.Nine realistic folder names from ordinary macOS/Windows/cloud layouts — 6 of 9 regressed, two leaking a client organisation name outright:
All nine mask completely on
22f0c541. The known-prefix literal layer doesn't rescue it either — it masks the prefix and leaves the client-named tail, including under$HOMEand the active project cwd.No test covers a 4-word directory component. The 4-word allowance in
pmSpFileand the 12-word allowance inpmSpFileXare about terminal filenames; the directory-component tests top out at three words, so the boundary that broke sits exactly in the untested gap.On the fix — I don't think "raise the bound" is right, and your earlier reasoning is why. You rejected
{1,8}onpmSpFilewith a concrete counterexample (read /opt/x error reading the project config.yml hereconsuming five prose words), and the same objection applies here: the bridge cap is load-bearing, and widening it plainly re-opens what I raised last round.What does separate the two classes is whether the path so far already ended in a dotted extension. Every one of the seven shapes I gave you last round ends in one (
/app/x.sql,C:\proj\x.sql,~/proj/x.sql,s3://b/k.parquet), and none of the leak cases above do — they're mid-path directory components. So: keep the tight bound after a dotted extension, and allow a wider one ({1,8}) when the run so far has no extension — the same extension-lookbehind discriminatorPM_TERM_CONDalready uses.I checked this against both sets: it covers 5/5 of the leak class and keeps 4/6 of the prose class safe. The two it doesn't cover (
dbt deps failed in /Users/j/p: package hub/dbt-utils not found, and your own/opt/x … config.ymlexample) are extensionless and would still bridge — that's the undecidable residue you've already documented, not a regression. Your call whether the trade is worth it, but it's a strictly better position than the current bound.Related, same root cause:
PM_SEG_L's{0,128}scan means a component whose first letter falls past position 128 after a bridge breaks the chain (/opt/client repo/<129 digits>a/private.sqlleaks), andPM_WR{1,64}does the same for a bridge word over 64 characters. Both narrow, both the same defect. And the narrowed fast-path gate costs the same way:c:my very secret file.sqlandc:a b c d.sqlmasked before and are now untouched, because(?: [^\s:]{1,255}){0,2}allows only two spaces.