Redact tool output secrets (#85196)

amknight · web-flow · commit 3faf669801d0 · 2026-06-11T07:34:50.000+10:00
* redact tool output secrets

* Expand tool-output secret redaction

* fix(security): keep redaction prefilter in sync with expanded defaults

- build DEFAULT_REDACT_PREFILTER_RE from sources covering every default
  pattern family: new vendor prefixes, webhook hosts, bare query/form keys,
  userinfo/connection-string passwords, and percent/plus/invisible
  obfuscated keys (including trailing separator splices)
- run default-pattern redaction tests through the default options path and
  redact the vendor corpus per token so prefilter gaps fail tests
- fix quoted standalone assignment values containing the other quote char
  or an unterminated quote; never re-mask *** placeholders
- align net-policy URL query-name separator stripping with logging key
  normalization (Hangul fillers)

* fix(security): keep base64-prefix redaction out of media payloads

- pure-base64-alphabet token prefixes (gAAAA, AKIA, ASIA, dapi,
  ATCTT3xFfG, ATATT, ATBB) now require a non-alphanumeric left boundary,
  skip explicit ;base64, payload spans, and run unchunked so chunk starts
  cannot fake the boundary or hide the container from the lookbehind
- tokens after URL/path delimiters or assignments still mask; data-URL
  media survives redaction byte-identical (fixes chat media mirror CI)
- regression tests: tiny-PNG data URL, in-blob plus boundary,
  chunk-aligned large data URL, reset-path Fernet token, path AWS key

---------

Co-authored-by: Alex Knight &lt;15041791+amknight@users.noreply.github.com&gt;
diff --git a/packages/net-policy/src/redact-sensitive-url.test.ts b/packages/net-policy/src/redact-sensitive-url.test.ts
@@ -22,6 +22,24 @@ describe("redactSensitiveUrl", () => {
     );
   });
 
+  it("redacts encoded and invisible-spliced sensitive query param names", () => {
+    expect(
+      redactSensitiveUrl("https://example.com/mcp?client%5Fse%E2%80%8Bcret=secret&safe=value"),
+    ).toBe("https://example.com/mcp?client_se%E2%80%8Bcret=***&safe=value");
+  });
+
+  it("redacts encoded sensitive query names with decoded whitespace and control separators", () => {
+    expect(
+      redactSensitiveUrl("https://example.com/mcp?client%5Fse%20cret=space&client%5Fse%00cret=nul"),
+    ).toBe("https://example.com/mcp?client_se+cret=***&client_se%00cret=***");
+  });
+
+  it("redacts query names with plus-encoded separators", () => {
+    expect(redactSensitiveUrl("https://example.com/mcp?client_se+cret=secret&safe=value")).toBe(
+      "https://example.com/mcp?client_se+cret=***&safe=value",
+    );
+  });
+
   it("keeps non-sensitive URLs unchanged", () => {
     expect(redactSensitiveUrl("https://example.com/mcp?safe=value")).toBe(
       "https://example.com/mcp?safe=value",
@@ -36,6 +54,26 @@ describe("redactSensitiveUrlLikeString", () => {
     );
   });
 
+  it("redacts encoded and invisible-spliced query names in invalid URL-like strings", () => {
+    expect(
+      redactSensitiveUrlLikeString("//example.com/mcp?client%5Fse%E2%80%8Bcret=secret&safe=value"),
+    ).toBe("//example.com/mcp?client%5Fse%E2%80%8Bcret=***&safe=value");
+  });
+
+  it("redacts encoded query names with decoded whitespace and control separators in invalid URL-like strings", () => {
+    expect(
+      redactSensitiveUrlLikeString(
+        "//example.com/mcp?client%5Fse%20cret=space&client%5Fse%00cret=nul",
+      ),
+    ).toBe("//example.com/mcp?client%5Fse%20cret=***&client%5Fse%00cret=***");
+  });
+
+  it("redacts plus-spliced query names in invalid URL-like strings", () => {
+    expect(redactSensitiveUrlLikeString("//example.com/mcp?client_se+cret=secret&safe=value")).toBe(
+      "//example.com/mcp?client_se+cret=***&safe=value",
+    );
+  });
+
   it("redacts every URL-like userinfo occurrence in arbitrary text", () => {
     expect(
       redactSensitiveUrlLikeString(
@@ -61,6 +99,17 @@ describe("isSensitiveUrlQueryParamName", () => {
     expect(isSensitiveUrlQueryParamName("hook-token")).toBe(true);
     expect(isSensitiveUrlQueryParamName("passwd")).toBe(true);
     expect(isSensitiveUrlQueryParamName("signature")).toBe(true);
+    expect(isSensitiveUrlQueryParamName("code")).toBe(true);
+    expect(isSensitiveUrlQueryParamName("x-amz-signature")).toBe(true);
+    expect(isSensitiveUrlQueryParamName("X-Amz-Security-Token")).toBe(true);
+    expect(isSensitiveUrlQueryParamName("id_token")).toBe(true);
+    expect(isSensitiveUrlQueryParamName("app_secret")).toBe(true);
+    expect(isSensitiveUrlQueryParamName("client%5Fse\u200Bcret")).toBe(true);
+    expect(isSensitiveUrlQueryParamName("client%5Fse%20cret")).toBe(true);
+    expect(isSensitiveUrlQueryParamName("client%5Fse%00cret")).toBe(true);
+    expect(isSensitiveUrlQueryParamName("client_se+cret")).toBe(true);
+    expect(isSensitiveUrlQueryParamName("client_se\u3164cret")).toBe(true);
+    expect(isSensitiveUrlQueryParamName("credential")).toBe(true);
     expect(isSensitiveUrlQueryParamName("safe")).toBe(false);
   });
 });
diff --git a/packages/net-policy/src/redact-sensitive-url.ts b/packages/net-policy/src/redact-sensitive-url.ts
@@ -22,15 +22,39 @@ const SENSITIVE_URL_QUERY_PARAM_NAMES = new Set([
   "pass",
   "passwd",
   "auth",
+  "jwt",
+  "session",
+  "id_token",
+  "code",
   "client_secret",
+  "app_secret",
   "hook_token",
   "refresh_token",
   "signature",
+  "x_amz_signature",
+  "x_amz_security_token",
+  "private_key",
+  "credential",
+  "authorization",
 ]);
+// Keep in sync with FORM_BODY_KEY_SEPARATOR_RE in src/logging/redact.ts: Hangul fillers are
+// category Lo, so \p{C}\p{Z} alone would let them splice sensitive key names.
+const URL_QUERY_NAME_SEPARATOR_RE = /[\p{C}\p{Z}\u115F\u1160\u3164\uFFA0+]/gu;
+
+function normalizeUrlQueryParamName(name: string): string {
+  const stripped = name.replace(URL_QUERY_NAME_SEPARATOR_RE, "");
+  try {
+    return normalizeLowercaseStringOrEmpty(
+      decodeURIComponent(stripped).replace(URL_QUERY_NAME_SEPARATOR_RE, ""),
+    ).replaceAll("-", "_");
+  } catch {
+    return normalizeLowercaseStringOrEmpty(stripped).replaceAll("-", "_");
+  }
+}
 
 /** True for auth-like URL query parameter names that should be redacted. */
 export function isSensitiveUrlQueryParamName(name: string): boolean {
-  const normalized = normalizeLowercaseStringOrEmpty(name).replaceAll("-", "_");
+  const normalized = normalizeUrlQueryParamName(name);
   return SENSITIVE_URL_QUERY_PARAM_NAMES.has(normalized);
 }
 
diff --git a/src/cli/gateway-cli/run.option-collisions.test.ts b/src/cli/gateway-cli/run.option-collisions.test.ts
@@ -648,6 +648,6 @@ describe("gateway run option collisions", () => {
         ).rejects.toThrow("__exit__:1");
       },
     );
-    expect(runtimeErrors[0]).toContain("Use either --passw***d or --password-file.");
+    expect(runtimeErrors[0]).toContain("Use either --password or --password-file.");
   });
 });
diff --git a/src/infra/exec-approval-command-display.test.ts b/src/infra/exec-approval-command-display.test.ts
@@ -104,6 +104,21 @@ describe("sanitizeExecApprovalDisplayText", () => {
     expect(result).toContain("https://api.example.com");
   });
 
+  it("masks newly added vendor token prefixes through the default redaction path", () => {
+    const token = "glpat-abcdefghijklmnopqrstuv";
+    const result = sanitizeExecApprovalDisplayText(`deploy --with ${token}`);
+    expect(result).not.toContain(token);
+  });
+
+  it("does not let contextual secret matches hide split-token bypass detection", () => {
+    const discordToken = `${"A".repeat(24)}.${"B".repeat(6)}.${"C".repeat(27)}`;
+    const cmd = `discord sk-abc123\u200B456789012345678 ${discordToken}`;
+    const result = sanitizeExecApprovalDisplayText(cmd);
+    expect(result).not.toContain("sk-abc123");
+    expect(result).not.toContain("456789012345678");
+    expect(result).not.toContain(discordToken);
+  });
+
   it("keeps PEM private-key context visible when raw redaction already covers the key (not a bypass)", () => {
     const cmd =
       "echo -----BEGIN RSA PRIVATE KEY-----\nABCDEF0123456789abcdef\n-----END RSA PRIVATE KEY----- > key.pem";
@@ -157,6 +172,41 @@ describe("sanitizeExecApprovalDisplayText", () => {
     expect(result).not.toContain("456789012345678");
     expect(result).toContain("remainder");
   });
+
+  it("masks form body values whose sensitive key is spliced with an invisible character", () => {
+    const cmd = "client_id=visible&app_se\u200Bcret=opaque-app-secret&safe=value";
+    const result = sanitizeExecApprovalDisplayText(cmd);
+    expect(result).not.toContain("opaque-app-secret");
+    expect(result).toContain("client_id=visible");
+    expect(result).toContain("safe=value");
+  });
+
+  it("masks form body values whose encoded sensitive key is spliced with an invisible character", () => {
+    const cmd = "client_id=visible&client%5Fse\u200Bcret=oauth-secret&safe=value";
+    const result = sanitizeExecApprovalDisplayText(cmd);
+    expect(result).not.toContain("oauth-secret");
+    expect(result).toContain("client_id=visible");
+    expect(result).toContain("safe=value");
+  });
+
+  it("masks form body values whose sensitive key is spliced with a plus separator", () => {
+    const cmd = "client_id=visible&client_se+cret=oauth-secret&safe=value";
+    const result = sanitizeExecApprovalDisplayText(cmd);
+    expect(result).not.toContain("oauth-secret");
+    expect(result).toContain("client_id=visible");
+    expect(result).toContain("safe=value");
+  });
+
+  it("keeps parsed form-body secrets masked when a separate spliced token triggers bypass rendering", () => {
+    const cmd =
+      "client_id=visible&client%5Fsecret=oauth,secret&safe=1 echo sk-abc123\u200B456789012345678";
+    const result = sanitizeExecApprovalDisplayText(cmd);
+    expect(result).not.toContain("oauth,secret");
+    expect(result).not.toContain(",secret");
+    expect(result).not.toContain("456789012345678");
+    expect(result).toContain("client_id=visible");
+    expect(result).toContain("safe=1");
+  });
 });
 
 describe("sanitizeExecApprovalWarningText", () => {
diff --git a/src/infra/exec-approval-command-display.ts b/src/infra/exec-approval-command-display.ts
@@ -1,5 +1,9 @@
 // Sanitizes command text before it is displayed in approval prompts.
-import { redactSensitiveText, resolveRedactOptions } from "../logging/redact.js";
+import {
+  computeSensitiveRedactionBitmap,
+  redactSensitiveText,
+  resolveRedactOptions,
+} from "../logging/redact.js";
 import type { ExecApprovalRequestPayload } from "./exec-approvals.js";
 
 // Escape control characters, Unicode format/line/paragraph separators, and non-ASCII space
@@ -60,31 +64,6 @@ function truncateForDisplay(text: string): SanitizedExecApprovalDisplayText {
   };
 }
 
-// Build a boolean bitmap of positions in `text` that ANY redaction pattern would match.
-// Patterns are applied independently to the raw text (not sequentially against a
-// progressively-redacted view) so later patterns can still find matches that the in-place
-// redaction would have replaced first. That is conservative — it may over-count overlapping
-// matches — but that is acceptable for a coverage check. Indices are UTF-16 code-unit
-// offsets, matching what `matchAll` returns and aligning with `String#length`.
-function computeRedactionBitmap(text: string, patterns: RegExp[]): boolean[] {
-  const bitmap: boolean[] = Array.from({ length: text.length }, () => false);
-  for (const pattern of patterns) {
-    const iter = pattern.flags.includes("g")
-      ? new RegExp(pattern.source, pattern.flags)
-      : new RegExp(pattern.source, `${pattern.flags}g`);
-    for (const match of text.matchAll(iter)) {
-      if (match.index === undefined) {
-        continue;
-      }
-      const end = match.index + match[0].length;
-      for (let i = match.index; i < end; i++) {
-        bitmap[i] = true;
-      }
-    }
-  }
-  return bitmap;
-}
-
 // Iterate by full Unicode code point so astral-plane invisibles (e.g. U+E0061 TAG LATIN
 // SMALL LETTER A, category Cf) are matched as single characters instead of being seen as a
 // surrogate pair whose halves are category Cs and would escape the invisible-char regex.
@@ -126,17 +105,16 @@ function sanitizeExecApprovalDisplayTextInternal(
   if (strippedRedacted === stripped) {
     return truncateForDisplay(escapeInvisibles(rawRedacted, options));
   }
-  // Detect bypass by position-bitmap coverage. Run each redaction pattern independently on
-  // both views and map stripped-view match positions back to original coordinates. If every
-  // position the stripped view would mask is also masked by the raw view, the raw view
-  // already covered everything — for example, an ordinary multi-line PEM private key where
-  // raw produces `BEGIN/…redacted…/END` while stripped collapses to `***`. A real bypass
-  // exists only when the stripped view masks at least one original position raw missed (e.g.
-  // the tail of an `sk-` token whose prefix-boundary was broken by a spliced zero-width or
-  // NBSP character).
-  const { patterns } = resolveRedactOptions({ mode: "tools" });
-  const rawMask = computeRedactionBitmap(commandText, patterns);
-  const strippedMask = computeRedactionBitmap(stripped, patterns);
+  // Detect bypass by position-bitmap coverage. Run the redaction matchers on both views and
+  // map stripped-view match positions back to original coordinates. If every position the
+  // stripped view would mask is also masked by the raw view, the raw view already covered
+  // everything — for example, an ordinary multi-line PEM private key where raw produces
+  // `BEGIN/…redacted…/END` while stripped collapses to `***`. A real bypass exists only when
+  // the stripped view masks at least one original position raw missed (e.g. the tail of an
+  // `sk-` token whose prefix-boundary was broken by a spliced zero-width or NBSP character).
+  const redaction = resolveRedactOptions({ mode: "tools" });
+  const rawMask = computeSensitiveRedactionBitmap(commandText, redaction);
+  const strippedMask = computeSensitiveRedactionBitmap(stripped, redaction);
   let bypassDetected = false;
   for (let i = 0; i < strippedMask.length; i++) {
     if (strippedMask[i] && !rawMask[strippedToOrig[i]]) {
diff --git a/src/logging/redact.test.ts b/src/logging/redact.test.ts
diff --git a/src/logging/redact.ts b/src/logging/redact.ts
