fix(cli): redact masked values by merged match ranges so overlapping secrets leave no fragment

Replacing one masked value after another left a remainder whenever two values overlapped in the text: a prefix of a longer token printed ***DEF, and "ABC" with "BCD" across "ABCD" printed ***D. The redactor now locates every occurrence of every masked value in the original text, merges overlapping or touching ranges, and replaces each merged range with one ***, for both streams and the summary file.

Author: Vivswan

src/cli/io.ts

@@ -51,20 +51,53 @@ class RedactingStream extends Writable {
   }
 }
 
+/**
+ * `text` with every occurrence of every masked value replaced by `***`.
+ * Occurrences are located in the original text and overlapping or touching
+ * ones are merged, so two values that overlap (a prefix of another, or
+ * "ABC" and "BCD" across "ABCD") leave no fragment, as replacing one value
+ * after another would.
+ */
+function redactRanges(text: string, masked: ReadonlySet<string>): string {
+  const ranges: Array<[number, number]> = [];
+  for (const value of masked) {
+    if (value === "") {
+      continue;
+    }
+    for (let at = text.indexOf(value); at !== -1; at = text.indexOf(value, at + 1)) {
+      ranges.push([at, at + value.length]);
+    }
+  }
+  ranges.sort((a, b) => a[0] - b[0]);
+  let out = "";
+  let cursor = 0;
+  let open: [number, number] | undefined;
+  for (const [start, end] of ranges) {
+    if (open !== undefined && start <= open[1]) {
+      open[1] = Math.max(open[1], end);
+      continue;
+    }
+    if (open !== undefined) {
+      out += `${text.slice(cursor, open[0])}***`;
+      cursor = open[1];
+    }
+    open = [start, end];
+  }
+  if (open !== undefined) {
+    out += `${text.slice(cursor, open[0])}***`;
+    cursor = open[1];
+  }
+  return out + text.slice(cursor);
+}
+
 /**
  * Every write to the returned streams is redacted; register a value before
  * anything can print it. One registry serves the parser, the Io, and the
  * file-only commands alike, so no writer can bypass it.
  */
 export function maskedStreams(streams: CliStreams): MaskedStreams {
   const registry = maskRegistry(() => {});
-  const redact = (text: string): string => {
-    let out = text;
-    for (const value of registry.masked()) {
-      out = out.replaceAll(value, "***");
-    }
-    return out;
-  };
+  const redact = (text: string): string => redactRanges(text, registry.masked());
   return {
     stdout: new RedactingStream(streams.stdout, redact),
     stderr: new RedactingStream(streams.stderr, redact),

test/cli/io.test.ts

@@ -98,6 +98,29 @@ describe("the CLI Io", () => {
     expect(readFileSync(summary, "utf8")).toBe("# run by ***\n");
   });
 
+  test.each<[string, string[], string, string]>([
+    [
+      "a prefix of another value",
+      ["github_pat_ABC", "github_pat_ABCDEF"],
+      "argument 'github_pat_ABCDEF' repeats github_pat_ABC",
+      "argument '***' repeats ***",
+    ],
+    ["two values overlapping across the text", ["ABC", "BCD"], "x ABCD y", "x *** y"],
+    ["an infix of another value", ["BC", "xABCDEy"], "see xABCDEy and BC", "see *** and ***"],
+    ["two values touching end to start", ["ABC", "DEF"], "ABCDEF", "***"],
+    ["separate occurrences of one value", ["ABC"], "ABC and ABC", "*** and ***"],
+    ["a value absent from the line", ["ABC"], "nothing here", "nothing here"],
+  ])("masking leaves no fragment of %s", (_case, masks, line, redacted) => {
+    // Replacing one value after another would leave "***D" for the overlap and
+    // "***DEF" for the prefix; the ranges are merged in the original text instead.
+    const { io, stderr } = open();
+    for (const value of masks) {
+      io.mask(value);
+    }
+    io.annotate("error", line);
+    expect(stderr()).toBe(`error: ${redacted}\n`);
+  });
+
   test("summary blocks append to the named file and are dropped without one", () => {
     const dir = mkdtempSync(join(tmpdir(), "gsac-io-"));
     scratch.push(dir);