From 59f1d6f795cc2c8c9ac25920dd5da2052d626b42 Mon Sep 17 00:00:00 2001 From: pelikhan Date: Thu, 15 Oct 2026 22:13:00 -0700 Subject: [PATCH 01/11] Fix runtime-mask parsing for large and malformed log records Split the secret-parsing and redaction changes from #69235 without backend ownership or path changes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../patch-large-record-mask-redaction.md | 5 + actions/setup/js/add_mask_redaction.cjs | 97 +++++++- actions/setup/js/add_mask_redaction.test.cjs | 212 +++++++++++++++++- actions/setup/js/redact_secrets.test.cjs | 56 +++++ docs/src/content/docs/reference/imports.md | 2 + 5 files changed, 359 insertions(+), 13 deletions(-) create mode 100644 .changeset/patch-large-record-mask-redaction.md diff --git a/.changeset/patch-large-record-mask-redaction.md b/.changeset/patch-large-record-mask-redaction.md new file mode 100644 index 00000000000..0ab130ef290 --- /dev/null +++ b/.changeset/patch-large-record-mask-redaction.md @@ -0,0 +1,5 @@ +--- +"gh-aw": patch +--- + +Avoid regular-expression stack exhaustion when redacting runtime masks from multi-megabyte JSON and mixed-log artifacts, while preserving fail-closed cleanup for genuine redaction failures. diff --git a/actions/setup/js/add_mask_redaction.cjs b/actions/setup/js/add_mask_redaction.cjs index 6c2cde4ba7c..5b524ab0c3a 100644 --- a/actions/setup/js/add_mask_redaction.cjs +++ b/actions/setup/js/add_mask_redaction.cjs @@ -138,6 +138,63 @@ function applyAddMaskRedaction(text, maskedValues) { return redactMaskedValues(withoutCommands, maskedValues); } +/** + * Visit quoted JSON tokens without a repeated regex alternation, which can + * exhaust the RegExp stack on multi-megabyte log records. + * Malformed prefixes are preserved as text; their quote pairing is ambiguous. + * @param {string} text + * @param {(token: string) => string} transformString + * @param {(text: string) => string} [transformText] + * @returns {string} + */ +function transformJSONStrings(text, transformString, transformText = value => value) { + const parts = []; + let cursor = 0; + while (cursor < text.length) { + const start = text.indexOf('"', cursor); + if (start === -1) { + parts.push(transformText(text.slice(cursor))); + break; + } + let end = start + 1; + let invalidString = false; + while (end < text.length && text[end] !== '"') { + if (text.charCodeAt(end) < 0x20) { + invalidString = true; + break; + } + if (text[end] === "\\") { + end++; + const escaped = text[end]; + if (escaped === "u") { + if (!/^[0-9a-fA-F]{4}$/.test(text.slice(end + 1, end + 5))) { + invalidString = true; + break; + } + end += 4; + } else if (!escaped || !'"\\/bfnrt'.includes(escaped)) { + invalidString = true; + break; + } + } + end++; + } + if (invalidString) { + parts.push(transformText(text.slice(cursor, start + 1))); + cursor = start + 1; + continue; + } + if (end >= text.length) { + parts.push(transformText(text.slice(cursor))); + break; + } + parts.push(transformText(text.slice(cursor, start))); + parts.push(transformString(text.slice(start, end + 1))); + cursor = end + 1; + } + return parts.join(""); +} + /** * Sanitize artifact sources while runtime masks are still available in memory. * Decode JSON strings first so escaping cannot hide a registered value, redact @@ -160,11 +217,15 @@ function redactArtifactMaskedValues(content, maskedValues) { let value = JSON.parse(text); // Skip quoted strings and match original numeric tokens at every depth so // rounding or exponent notation cannot hide a mask. - const numericRedacted = text.replace(/"(?:[^"\\]|\\.)*"|-?\d+(?:\.\d+)?(?:[eE][+-]?\d+)?/g, token => { - if (token.startsWith('"')) return token; - const redacted = redactMaskedValues(token, maskedValues); - return redacted === token ? token : JSON.stringify(redacted); - }); + const numericRedacted = transformJSONStrings( + text, + token => token, + plain => + plain.replace(/-?\d+(?:\.\d+)?(?:[eE][+-]?\d+)?/g, token => { + const redacted = redactMaskedValues(token, maskedValues); + return redacted === token ? token : JSON.stringify(redacted); + }) + ); if (numericRedacted !== text) value = JSON.parse(numericRedacted); const redacted = JSON.stringify(redactValue(value)); return redacted === JSON.stringify(value) ? numericRedacted : text.match(/^\s*/)[0] + redacted + text.match(/\s*$/)[0]; @@ -181,16 +242,27 @@ function redactArtifactMaskedValues(content, maskedValues) { const jsonLine = redactJson(line); if (jsonLine !== undefined) return jsonLine; // Mixed or malformed logs can still contain recoverable JSON strings. - const decoded = line.replace(/"(?:[^"\\]|\\.)*"/g, encoded => { + const decoded = transformJSONStrings(line, encoded => { + let value; try { - const value = JSON.parse(encoded); - const redacted = redactValue(value); - return redacted === value ? encoded : JSON.stringify(redacted); - } catch { - return encoded; + value = JSON.parse(encoded); + } catch (error) { + throw new Error("Unable to decode JSON log string during secret redaction", { cause: error }); } + const redacted = redactValue(value); + return redacted === value ? encoded : JSON.stringify(redacted); + }); + const redacted = redactMaskedValues(decoded, maskedValues); + // Malformed quotes have no unique pairing. Decode remaining escapes once + // conservatively so an alternate pairing cannot hide an encoded mask. + const escapes = { '"': '"', "\\": "\\", "/": "/", b: "\b", f: "\f", n: "\n", r: "\r", t: "\t" }; + const unescaped = redacted.replace(/\\(["\\/bfnrt]|u[0-9a-fA-F]{4})/g, (_, escape) => { + if (escape[0] === "u") return String.fromCharCode(parseInt(escape.slice(1), 16)); + return escapes[escape]; }); - return redactMaskedValues(decoded, maskedValues); + const conservative = applyAddMaskRedaction(unescaped, maskedValues); + // Only re-encode an ambiguous record when the extra decoding finds a mask. + return conservative === unescaped ? redacted : JSON.stringify(conservative); }) .join("\n"); } @@ -203,5 +275,6 @@ module.exports = { isAddMaskCommandLine, redactArtifactMaskedValues, redactMaskedValues, + transformJSONStrings, unescapeWorkflowCommandValue, }; diff --git a/actions/setup/js/add_mask_redaction.test.cjs b/actions/setup/js/add_mask_redaction.test.cjs index 81355292823..c7b4f0edc94 100644 --- a/actions/setup/js/add_mask_redaction.test.cjs +++ b/actions/setup/js/add_mask_redaction.test.cjs @@ -3,9 +3,57 @@ import { describe, it, expect, vi } from "vitest"; import { createRequire } from "module"; const require = createRequire(import.meta.url); -const { applyAddMaskRedaction, collectAddMaskedValues, isAddMaskCommandLine, redactArtifactMaskedValues, redactMaskedValues, unescapeWorkflowCommandValue } = require("./add_mask_redaction.cjs"); +const { applyAddMaskRedaction, collectAddMaskedValues, isAddMaskCommandLine, redactArtifactMaskedValues, redactMaskedValues, transformJSONStrings, unescapeWorkflowCommandValue } = require("./add_mask_redaction.cjs"); describe("add_mask_redaction", () => { + describe("transformJSONStrings", () => { + it("passes complete quoted tokens and plain spans to separate callbacks", () => { + const strings = vi.fn(token => `[${token}]`); + const plain = vi.fn(text => `{${text}}`); + expect(transformJSONStrings('head "one" between "two" tail', strings, plain)).toBe('{head }["one"]{ between }["two"]{ tail}'); + expect(strings.mock.calls.map(([token]) => token)).toEqual(['"one"', '"two"']); + expect(plain.mock.calls.map(([text]) => text)).toEqual(["head ", " between ", " tail"]); + }); + + it("visits adjacent and empty tokens without separators or trailing text", () => { + const strings = vi.fn(token => token); + const content = '"""one""two"""'; + expect(transformJSONStrings(content, strings)).toBe(content); + expect(strings.mock.calls.map(([token]) => token)).toEqual(['""', '"one"', '"two"', '""']); + }); + + it.each(['"unterminated', '"ends\\', '"ends\\u', '"ends\\u123', 'plain "unterminated', '"safe" "ends\\'])("preserves truncated input and never transforms incomplete strings (%j)", content => { + const strings = vi.fn(token => token); + const plain = vi.fn(text => text); + expect(transformJSONStrings(content, strings, plain)).toBe(content); + expect(strings.mock.calls.map(([token]) => token)).toEqual(content.startsWith('"safe"') ? ['"safe"'] : []); + expect(plain.mock.calls.map(([text]) => text).join("")).toBe(content.startsWith('"safe"') ? content.slice(6) : content); + }); + + it.each(['"bad\t ', '"bad\\q ', '"bad\\u12gg '])("recovers valid tokens after invalid unterminated prefixes without losing bytes (%j)", prefix => { + const strings = vi.fn(token => token); + const plain = vi.fn(text => text); + expect(transformJSONStrings(prefix + '"\\u006fpaque" tail', strings, plain)).toBe(prefix + '"\\u006fpaque" tail'); + expect(strings.mock.calls.map(([token]) => token)).toEqual(['"\\u006fpaque"']); + expect(plain.mock.calls.map(([text]) => text).join("")).toBe(prefix + " tail"); + }); + + it("preserves bytes even when malformed terminated input makes quote pairing ambiguous", () => { + const strings = vi.fn(token => token); + const content = '"bad\t" "\\u006fpaque"'; + expect(transformJSONStrings(content, strings)).toBe(content); + expect(strings.mock.calls.map(([token]) => token)).toEqual(['" "']); + }); + + it.each(["string", "plain"])("propagates %s callback failures without adding source text", callback => { + const error = new Error("synthetic callback failure"); + const fail = () => { + throw error; + }; + expect(() => transformJSONStrings('plain "value"', callback === "string" ? fail : token => token, callback === "plain" ? fail : text => text)).toThrow(error); + }); + }); + describe("isAddMaskCommandLine", () => { it("detects add-mask command lines", () => { expect(isAddMaskCommandLine("::add-mask::secret")).toBe(true); @@ -134,6 +182,168 @@ describe("add_mask_redaction", () => { }); describe("redactArtifactMaskedValues", () => { + describe("JSON string scanner boundaries", () => { + const encodedMask = '"\\u006fpaque"'; + const validStrings = [ + "", + "ordinary ASCII", + 'quote"then opaque', + "backslash\\then opaque", + "slash/then opaque", + "backspace\bthen opaque", + "formfeed\fthen opaque", + "newline\nthen opaque", + "return\rthen opaque", + "tab\tthen opaque", + "\u0000opaque\u001f", + "caf\u00e9 opaque \u2603", + "\ud83d\ude80opaque", + "\ud800opaque\udfff", + "\u2028opaque\u2029", + '""opaque""', + ]; + + it.each(validStrings.map(value => [JSON.stringify(value), value]))("matches decoded-string masking for valid token %s", (encoded, value) => { + const expected = JSON.stringify(value.replaceAll("opaque", "***")); + expect(redactArtifactMaskedValues(encoded, ["opaque"])).toBe(expected); + expect(redactArtifactMaskedValues(`before ${encoded} after`, ["opaque"])).toBe(`before ${expected} after`); + }); + + it.each(Array.from({ length: 17 }, (_, count) => count))("recognizes closing quotes after %i pairs of escaped backslashes", count => { + const safe = JSON.stringify("\\".repeat(count)); + const escapedQuote = JSON.stringify("\\".repeat(count) + '"opaque'); + const content = `prefix ${safe} ${encodedMask} ${escapedQuote} suffix`; + const expected = `prefix ${safe} "***" ${JSON.stringify("\\".repeat(count) + '"***')} suffix`; + expect(redactArtifactMaskedValues(content, ["opaque"])).toBe(expected); + }); + + it.each(["\\u006fpaque", "op\\u0061que", "\\u006F\\u0070\\u0061\\u0071\\u0075\\u0065"])("decodes Unicode escapes at every mask position (%s)", encoded => { + expect(redactArtifactMaskedValues(`log "${encoded}"`, ["opaque"])).toBe('log "***"'); + }); + + it.each(Array.from({ length: 32 }, (_, code) => code))("recovers after unescaped control character U+%i", code => { + const prefix = '"invalid' + String.fromCharCode(code) + " "; + expect(redactArtifactMaskedValues(prefix + encodedMask, ["opaque"])).toBe(prefix + '"***"'); + }); + + it.each(["a", "c", "e", "v", "x", "0", "1", "'", "U", " ", "\t", "\r", "\n", "\u2028", "\u2029"])("recovers after illegal escaped character %j", escaped => { + const prefix = '"invalid\\' + escaped + " "; + expect(redactArtifactMaskedValues(prefix + encodedMask, ["opaque"])).toBe(prefix + '"***"'); + }); + + it.each(["", "0", "00", "000", "gggg", "000g", "00g0", "0g00", "g000", "+000", "-000", " 000"])("recovers after malformed Unicode escape %j", digits => { + const prefix = '"invalid\\u' + digits + " "; + expect(redactArtifactMaskedValues(prefix + encodedMask, ["opaque"])).toBe(prefix + '"***"'); + }); + + it.each(['"bad\t"', '"bad\u0001"', '"bad\\q"', '"bad\\u12gg"'])("conservatively redacts alternate quote pairings after terminated malformed tokens (%j)", prefix => { + const content = `${prefix} ${encodedMask}`; + const redacted = redactArtifactMaskedValues(content, ["opaque"]); + expect(JSON.parse(redacted)).toBe(`${prefix} "***"`); + expect(redacted).not.toContain("\\u006fpaque"); + expect(redactArtifactMaskedValues(content, ["absent"])).toBe(content); + }); + + it("conservatively redacts escapes before invalid controls and in unterminated strings", () => { + for (const content of ['"\\u006fpaque\t"', '"\\u006fpaque', '"\\u006fpaque\\']) { + const redacted = redactArtifactMaskedValues(content, ["opaque"]); + expect(JSON.parse(redacted)).toBe(content.replace("\\u006fpaque", "***")); + } + }); + + it("redacts encoded masks crossing quote boundaries in an alternate pairing", () => { + const content = '"bad\t" "foo\\"bar"'; + expect(JSON.parse(redactArtifactMaskedValues(content, ['foo"bar']))).toBe('"bad\t" "***"'); + }); + + it("does not recursively decode literal Unicode escapes after malformed prefixes", () => { + const content = '"bad\t" ' + JSON.stringify("\\u006fpaque"); + expect(redactArtifactMaskedValues(content, ["opaque"])).toBe(content); + }); + + it.each(["", "plain", '"', '"unterminated', '"ends\\', '"ends\\u', '"ends\\u123', '"bad\\q', '""', '"" ""', '"safe"tail', '"safe" "unterminated'])("preserves unmatched or truncated input byte-for-byte (%j)", content => { + expect(redactArtifactMaskedValues(content, ["absent"])).toBe(content); + }); + + it.each(['"unterminated opaque', '"ends opaque\\', '"bad\\q opaque', 'opaque "safe" opaque', '"safe" opaque "unterminated'])("still applies raw masks outside recoverable tokens (%j)", content => { + expect(redactArtifactMaskedValues(content, ["opaque"])).toBe(content.replaceAll("opaque", "***")); + }); + + it("handles adjacent strings, empty strings, invalid prefixes, and trailing text together", () => { + const content = `start ""${encodedMask}""${encodedMask} "bad\\q ${encodedMask} trailing`; + const redactedToken = JSON.stringify("***"); + expect(redactArtifactMaskedValues(content, ["opaque"])).toBe(`start ""${redactedToken}""${redactedToken} "bad\\q ${redactedToken} trailing`); + }); + + it("keeps literal escaped Unicode distinct from an actual encoded mask", () => { + const literal = JSON.stringify("\\u006fpaque"); + expect(redactArtifactMaskedValues(`log ${literal} ${encodedMask}`, ["opaque"])).toBe(`log ${literal} "***"`); + }); + + it("keeps numeric-looking string tokens separate from original numeric tokens", () => { + const content = ' \t{"escaped":"\\u0031e400","quoted":"9007199254740993","ids":[1e400,9007199254740993,-0,1e+09]}\r\n'; + const redacted = redactArtifactMaskedValues(content, ["1e400", "9007199254740993"]); + expect(JSON.parse(redacted)).toEqual({ escaped: "***", quoted: "***", ids: ["***", "***", 0, 1000000000] }); + expect(redacted.startsWith(" \t")).toBe(true); + expect(redacted.endsWith("\r\n")).toBe(true); + }); + + it("matches a deterministic decoded-string oracle across generated escape combinations", () => { + const alphabet = ["a", '"', "\\", "/", "\b", "\f", "\n", "\r", "\t", "\u0000", "\u001f", "\u2028", "\ud800", "\ud83d\ude80"]; + for (let index = 0; index < 256; index++) { + const left = alphabet[index % alphabet.length]; + const right = alphabet[Math.floor(index / alphabet.length) % alphabet.length]; + const value = left + right + "opaque" + right + left; + const encoded = JSON.stringify(value); + const expected = JSON.stringify(value.replaceAll("opaque", "***")); + expect(redactArtifactMaskedValues(`record ${encoded} end`, ["opaque"]), `generated record ${index}`).toBe(`record ${expected} end`); + const untouched = `record ${encoded} end`; + expect(redactArtifactMaskedValues(untouched, ["absent"]), `unchanged record ${index}`).toBe(untouched); + } + }); + + it("redacts thousands of adjacent tokens without losing delimiters or unchanged bytes", () => { + const safe = '"safe\\\\\\"value"'; + const record = `prefix ${safe}:${encodedMask}:`; + expect(redactArtifactMaskedValues(record.repeat(4096) + "tail", ["opaque"])).toBe(`prefix ${safe}:"***":`.repeat(4096) + "tail"); + }); + }); + + it.each(["json", "mixed"])("redacts multi-megabyte %s records without exhausting the regex stack", shape => { + const mask = 'synthetic-"quoted"\\mask'; + const value = "x".repeat(10 * 1024 * 1024) + mask; + const record = JSON.stringify({ text: value, number: shape === "mixed" ? 42 : 12345678 }); + const prefix = shape === "mixed" ? "tool output: " : ""; + const redacted = redactArtifactMaskedValues(prefix + record, [mask, "12345678"]); + expect(redacted.includes(mask)).toBe(false); + expect(redacted.includes(JSON.stringify(mask).slice(1, -1))).toBe(false); + const parsed = JSON.parse(redacted.slice(prefix.length)); + expect(parsed.text.length).toBe(10 * 1024 * 1024 + 3); + expect(parsed.text.endsWith("***")).toBe(true); + expect(parsed.number).toBe(shape === "mixed" ? 42 : "***"); + }); + + it("handles many escaped quotes and both odd and even backslash runs", () => { + const mask = 'synthetic-"quoted"\\mask'; + const value = '\\"'.repeat(1024 * 1024) + mask + "\\\\"; + const record = JSON.stringify({ text: value, safe: "12345678" }); + const redacted = redactArtifactMaskedValues(record, [mask]); + const parsed = JSON.parse(redacted); + expect(parsed.text.length).toBe(2 * 1024 * 1024 + 5); + expect(parsed.text.endsWith("***\\\\")).toBe(true); + expect(parsed.safe).toBe("12345678"); + }); + + it.each(["\n", "\r", "\u2028", "\u2029"])("recovers encoded masks after an invalid escaped line terminator", terminator => { + const mask = 'synthetic-"quoted"\\mask'; + const prefix = '"invalid\\' + terminator; + expect(redactArtifactMaskedValues(prefix + JSON.stringify(mask), [mask])).toBe(prefix + '"***"'); + }); + + it.each(['"bad\\q ', '"bad\\u12gg ', '"bad\\u12 ', '"bad\u0001 ', '"bad\t ', '"bad\\v '])("recovers Unicode-encoded masks after invalid JSON string prefixes (%j)", prefix => { + expect(redactArtifactMaskedValues(prefix + '"\\u006fpaque"', ["opaque"])).toBe(prefix + '"***"'); + }); + it("redacts decoded JSON strings and keys without mutating syntax or primitives", () => { const secret = 'quote"\\percent%0A'; const value = { [secret]: [secret, 0, false, null], nested: { output: "first\nsecond" } }; diff --git a/actions/setup/js/redact_secrets.test.cjs b/actions/setup/js/redact_secrets.test.cjs index ad0adf21f58..947927a2165 100644 --- a/actions/setup/js/redact_secrets.test.cjs +++ b/actions/setup/js/redact_secrets.test.cjs @@ -127,6 +127,62 @@ describe("redact_secrets.cjs", () => { } }), describe("file handling", () => { + it("retains and sanitizes a private multi-megabyte artifact instead of failing closed on regex stack exhaustion", () => { + const { processFile } = require("./redact_secrets.cjs"); + const file = path.join(tempDir, "large-record.jsonl"); + const mask = 'synthetic-"quoted"\\mask'; + fs.writeFileSync(file, JSON.stringify({ text: "x".repeat(10 * 1024 * 1024) + mask }), { mode: 0o600 }); + expect(processFile(file, [], [mask])).toBe(1); + expect(fs.existsSync(file)).toBe(true); + const parsed = JSON.parse(fs.readFileSync(file, "utf8")); + expect(parsed.text.endsWith("***")).toBe(true); + expect(parsed.text.length).toBe(10 * 1024 * 1024 + 3); + expect(fs.statSync(file).mode & 0o777).toBe(0o600); + expect(mockCore.setFailed).not.toHaveBeenCalled(); + }); + + it.each(['"bad\\q ', '"bad\\u12gg ', '"bad\\u12 ', '"bad\u0001 ', '"bad\t ', '"bad\\v '])("sanitizes recoverable encoded masks after invalid JSON string prefixes (%j)", prefix => { + const { processFile } = require("./redact_secrets.cjs"); + const file = path.join(tempDir, "malformed.log"); + fs.writeFileSync(file, prefix + '"\\u006fpaque"', { mode: 0o600 }); + expect(processFile(file, [], ["opaque"])).toBe(1); + expect(fs.readFileSync(file, "utf8")).toBe(prefix + '"***"'); + expect(fs.statSync(file).mode & 0o777).toBe(0o600); + expect(mockCore.setFailed).not.toHaveBeenCalled(); + }); + + it.each(['"bad\t"', '"bad\\q"', '"bad\\u12gg"'])("conservatively sanitizes alternate quote pairings after malformed terminated tokens (%j)", prefix => { + const { processFile } = require("./redact_secrets.cjs"); + const file = path.join(tempDir, "ambiguous.log"); + fs.writeFileSync(file, prefix + ' "\\u006fpaque"', { mode: 0o600 }); + expect(processFile(file, [], ["opaque"])).toBe(1); + const redacted = fs.readFileSync(file, "utf8"); + expect(JSON.parse(redacted)).toBe(prefix + ' "***"'); + expect(redacted).not.toContain("\\u006fpaque"); + expect(fs.statSync(file).mode & 0o777).toBe(0o600); + expect(mockCore.setFailed).not.toHaveBeenCalled(); + }); + + it("removes the artifact if decoding a validated mixed-log token unexpectedly fails", () => { + const { processFile } = require("./redact_secrets.cjs"); + const file = path.join(tempDir, "mixed.log"); + fs.writeFileSync(file, 'output: "\\u006fpaque"'); + const parse = JSON.parse; + const parseSpy = vi.spyOn(JSON, "parse").mockImplementation(text => { + if (text === '"\\u006fpaque"') throw new Error("opaque runtime-mask"); + return parse(text); + }); + try { + expect(processFile(file, [], ["opaque"])).toBe(0); + expect(fs.existsSync(file)).toBe(false); + expect(mockCore.warning).toHaveBeenCalledWith("Secret redaction failed (stage=redact, code=unknown)"); + expect(mockCore.warning.mock.calls.flat().join("\n")).not.toMatch(/opaque|runtime-mask/); + expect(mockCore.setFailed).toHaveBeenCalled(); + } finally { + parseSpy.mockRestore(); + } + }); + it.each(["EACCES", "EPERM", "ENOSPC"])("reports only safe read diagnostics for secret-bearing %s errors", code => { const { processFile } = require("./redact_secrets.cjs"); const file = path.join(tempDir, "secret-value.log"); diff --git a/docs/src/content/docs/reference/imports.md b/docs/src/content/docs/reference/imports.md index 5da2f90e468..4f5588d0ac8 100644 --- a/docs/src/content/docs/reference/imports.md +++ b/docs/src/content/docs/reference/imports.md @@ -388,6 +388,8 @@ Steps from imports run **before** steps defined in the main workflow. Imported p Built-in redaction scans supported text files under `/tmp/gh-aw` and `${RUNNER_TEMP}/gh-aw`. Files without secret matches are left unchanged unless `secret-masking.steps` configures custom redactors, which require writable files. When redaction or custom-hook preparation requires a write, a readable non-writable file, such as a container-owned log, is atomically replaced with a runner-owned copy with owner-only read/write permissions. If the parent directory is also read-only, such as a Go module cache directory, redaction instead makes the file owner-readable and writable in place without changing directory permissions. This requires permission to change the file's mode; it does not bypass file locks or inaccessible directories. +Runtime-mask redaction handles multi-megabyte JSON strings and mixed log records without relying on recursive regular-expression matching. If malformed quotes hide an encoded mask, a conservative escape-decoding pass redacts it and represents that record as a JSON string; records without additional mask matches retain their original bytes. This does not change GitHub's step-summary size limit. + Custom hooks must tolerate absent optional files and files that disappear between discovery and reading (`ENOENT`). Earlier cleanup or redaction can remove files, including symbolic links and artifacts that could not be sanitized. Do not create empty placeholders for missing logs or outputs. Custom hooks run during the initial workspace redaction, before graders execute. The later grader-output redaction pass sanitizes newly generated results but does not rerun custom hooks, so it does not prepare unchanged files for them. From f1ee74c4b30c49aa1b1ae45672344dbc11adce35 Mon Sep 17 00:00:00 2001 From: pelikhan Date: Thu, 15 Oct 2026 22:17:29 -0700 Subject: [PATCH 02/11] Expand runtime-mask parser coverage with seeded fuzz tests Exercise 7,000 reproducible generated cases across token mutations, escaped nested and mixed records, malformed quotes, and high-precision numeric lexemes; check every truncation boundary for representative quoted tokens. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- actions/setup/js/add_mask_redaction.test.cjs | 138 +++++++++++++++++++ 1 file changed, 138 insertions(+) diff --git a/actions/setup/js/add_mask_redaction.test.cjs b/actions/setup/js/add_mask_redaction.test.cjs index c7b4f0edc94..e217e9b5b1e 100644 --- a/actions/setup/js/add_mask_redaction.test.cjs +++ b/actions/setup/js/add_mask_redaction.test.cjs @@ -52,6 +52,144 @@ describe("add_mask_redaction", () => { }; expect(() => transformJSONStrings('plain "value"', callback === "string" ? fail : token => token, callback === "plain" ? fail : text => text)).toThrow(error); }); + + it.each(["", "ordinary", 'quote"slash\\', "\u0000\b\f\n\r\t\u001f", "\ud800\udfff", "\\u006fpaque", '\\"'.repeat(32)])("only transforms a complete token at every possible truncation boundary (%j)", value => { + const encoded = JSON.stringify(value); + for (let length = 0; length <= encoded.length; length++) { + const strings = vi.fn(token => `[${token}]`); + const content = `prefix ${encoded.slice(0, length)}`; + expect(transformJSONStrings(content, strings)).toBe(length === encoded.length ? `prefix [${encoded}]` : content); + expect(strings.mock.calls.map(([token]) => token)).toEqual(length === encoded.length ? [encoded] : []); + } + }); + }); + + describe("seeded parser fuzz tests", () => { + const seeds = [1, 42, 2026, 0xdeadbeef, 0xffffffff]; + const alphabet = ["a", "z", "0", '"', "\\", "/", "\b", "\f", "\n", "\r", "\t", "\u0000", "\u001f", "\u2028", "\u2029", "\ud800", "\udfff", "\ud83d\ude80"]; + + function randomFor(seed) { + let state = seed; + return max => { + state = (Math.imul(state, 1664525) + 1013904223) >>> 0; + return Math.floor((state / 0x100000000) * max); + }; + } + + function randomText(random) { + return Array.from({ length: random(24) }, () => alphabet[random(alphabet.length)]).join(""); + } + + function encodeString(value, random) { + let encoded = '"'; + for (let index = 0; index < value.length; index++) { + const character = value[index]; + encoded += random(2) ? `\\u${value.charCodeAt(index).toString(16).padStart(4, "0")}` : JSON.stringify(character).slice(1, -1); + } + return encoded + '"'; + } + + it.each(seeds)("matches an independent bounded token grammar after random insertions, deletions, and truncations (seed %i)", seed => { + const random = randomFor(seed); + const fragments = ['"', "\\", "\\u", "\\u0000", "\\u12gg", "\\q", '""', "\t", "\n", "\r", "plain", ":", ","]; + // The regex oracle only sees short fuzz records, never the large production inputs. + const tokenPattern = /"(?:[^"\\\x00-\x1f]|\\(?:["\\/bfnrt]|u[0-9a-fA-F]{4}))*"/g; + for (let iteration = 0; iteration < 500; iteration++) { + let content = Array.from({ length: 1 + random(8) }, () => (random(2) ? encodeString(randomText(random), random) : fragments[random(fragments.length)])).join(""); + const position = random(content.length + 1); + switch (random(3)) { + case 0: + content = content.slice(0, position) + String.fromCharCode(random(0x10000)) + content.slice(position); + break; + case 1: + content = content.slice(0, position) + content.slice(position + 1); + break; + case 2: + content = content.slice(0, position); + break; + } + const tokens = []; + const spans = []; + const result = transformJSONStrings( + content, + token => { + expect(typeof JSON.parse(token), `seed ${seed}, iteration ${iteration}`).toBe("string"); + tokens.push(token); + spans.push(token); + return token; + }, + plain => { + spans.push(plain); + return plain; + } + ); + const context = `seed ${seed}, iteration ${iteration}, input ${JSON.stringify(content)}`; + expect(tokens, context).toEqual(Array.from(content.matchAll(tokenPattern), match => match[0])); + expect(spans.join(""), context).toBe(content); + expect(result, context).toBe(content); + expect(redactArtifactMaskedValues(content, ["ABSENT_RUNTIME_MASK"]), context).toBe(content); + } + }); + + it.each(seeds)("redacts randomly escaped masks in nested JSON, JSONL, and mixed records against a decoded oracle (seed %i)", seed => { + const random = randomFor(seed); + for (let iteration = 0; iteration < 300; iteration++) { + const mask = `MASK-${seed}-${iteration}-` + randomText(random); + const left = randomText(random); + const right = randomText(random); + const value = left + mask + right + mask; + const expected = value.replaceAll(mask, "***"); + const key = `key-${mask}`; + const encodedValue = encodeString(value, random); + const encodedKey = encodeString(key, random); + const nested = `{${encodedKey}:[${encodedValue},{"value":${encodedValue}},null,true,false,42]}`; + const context = `seed ${seed}, iteration ${iteration}`; + const expectedNested = { "key-***": [expected, { value: expected }, null, true, false, 42] }; + expect(JSON.parse(redactArtifactMaskedValues(nested, [mask])), context).toEqual(expectedNested); + const jsonl = ` \t${nested}\r\n${encodedValue}\n`; + const redactedLines = redactArtifactMaskedValues(jsonl, [mask]).trim().split("\n"); + expect( + redactedLines.map(line => JSON.parse(line)), + context + ).toEqual([expectedNested, expected]); + const mixed = `output: ${encodedValue} end`; + expect(redactArtifactMaskedValues(mixed, [mask]), context).toBe(`output: ${JSON.stringify(expected)} end`); + expect(redactArtifactMaskedValues(nested, ["ABSENT_RUNTIME_MASK"]), context).toBe(nested); + expect(redactArtifactMaskedValues(jsonl, ["ABSENT_RUNTIME_MASK"]), context).toBe(jsonl); + expect(redactArtifactMaskedValues(mixed, ["ABSENT_RUNTIME_MASK"]), context).toBe(mixed); + } + }); + + it.each(seeds)("finds randomly encoded masks after ambiguous malformed quotes without rewriting unmatched records (seed %i)", seed => { + const random = randomFor(seed); + const prefixes = ['"bad\\q"', '"bad\\u12gg"', '"bad\\u123"', '"bad\t"', '"bad\u0000"']; + for (let iteration = 0; iteration < 300; iteration++) { + const prefix = prefixes[random(prefixes.length)]; + const before = "a".repeat(random(16)); + const after = "z".repeat(random(16)); + const encodedMask = "\\u006f" + encodeString("paque", random).slice(1, -1); + const content = `${prefix} "${before}${encodedMask}${after}"`; + const context = `seed ${seed}, iteration ${iteration}, input ${JSON.stringify(content)}`; + expect(JSON.parse(redactArtifactMaskedValues(content, ["opaque"])), context).toBe(`${prefix} "${before}***${after}"`); + expect(redactArtifactMaskedValues(content, ["ABSENT_RUNTIME_MASK"]), context).toBe(content); + } + }); + + it.each(seeds)("preserves original numeric lexemes while masking generated high-precision tokens (seed %i)", seed => { + const random = randomFor(seed); + for (let iteration = 0; iteration < 300; iteration++) { + const integer = String(1 + random(9)) + Array.from({ length: 20 + random(20) }, () => String(random(10))).join(""); + const fraction = random(2) ? "." + String(random(10000)) : ""; + const exponent = random(2) ? `e${random(2) ? "+" : "-"}${random(1000)}` : ""; + const token = (random(2) ? "-" : "") + integer + fraction + exponent; + const content = ` \t[${token},-0,1e+09,9007199254740993,1e400]\r\n`; + const context = `seed ${seed}, iteration ${iteration}, token ${token}`; + const redacted = redactArtifactMaskedValues(content, [token]); + expect(redacted, context).toBe(' \t["***",-0,1e+09,9007199254740993,1e400]\r\n'); + expect(JSON.parse(redacted)[0], context).toBe("***"); + expect(redactArtifactMaskedValues(content, ["ABSENT_RUNTIME_MASK"]), context).toBe(content); + } + }); }); describe("isAddMaskCommandLine", () => { From 116a12807404f197664fdac6c1742647a75dd6cc Mon Sep 17 00:00:00 2001 From: pelikhan Date: Thu, 15 Oct 2026 22:48:55 -0700 Subject: [PATCH 03/11] Preserve safe-output protocol schemas during secret redaction Keep bundled public field names and enum values intact while sanitizing payload data. Protect post-redaction collector configuration, fail closed on unsafe control changes, and cover the short-mask failure through collection and real review handlers with mocked GitHub. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../patch-large-record-mask-redaction.md | 2 + actions/setup/js/add_mask_redaction.cjs | 42 +++-- actions/setup/js/redact_secrets.cjs | 21 ++- .../js/runtime_mask_publication.test.cjs | 104 +++++++++- actions/setup/js/safe_output_redaction.cjs | 177 ++++++++++++++++++ .../setup/js/safe_output_redaction.test.cjs | 123 ++++++++++++ docs/src/content/docs/reference/imports.md | 2 + 7 files changed, 448 insertions(+), 23 deletions(-) create mode 100644 actions/setup/js/safe_output_redaction.cjs create mode 100644 actions/setup/js/safe_output_redaction.test.cjs diff --git a/.changeset/patch-large-record-mask-redaction.md b/.changeset/patch-large-record-mask-redaction.md index 0ab130ef290..6594ed9144c 100644 --- a/.changeset/patch-large-record-mask-redaction.md +++ b/.changeset/patch-large-record-mask-redaction.md @@ -3,3 +3,5 @@ --- Avoid regular-expression stack exhaustion when redacting runtime masks from multi-megabyte JSON and mixed-log artifacts, while preserving fail-closed cleanup for genuine redaction failures. + +Preserve bundled safe-output schema vocabulary during redaction so short masks do not corrupt review message fields, dispatch types, or collector configuration. Continue redacting payload data and fail closed on malformed executable records, unsafe custom routing changes, or conflicts with control data. diff --git a/actions/setup/js/add_mask_redaction.cjs b/actions/setup/js/add_mask_redaction.cjs index 5b524ab0c3a..16b417cd7c4 100644 --- a/actions/setup/js/add_mask_redaction.cjs +++ b/actions/setup/js/add_mask_redaction.cjs @@ -202,10 +202,11 @@ function transformJSONStrings(text, transformString, transformText = value => va * byte-for-byte. * @param {string} content * @param {string[]} maskedValues + * @param {(value: any) => any} [redactJSONValue] - Structured protocol redactor for valid JSON records * @returns {string} */ -function redactArtifactMaskedValues(content, maskedValues) { - if (!content || !maskedValues.length) return content; +function redactArtifactMaskedValues(content, maskedValues, redactJSONValue) { + if (!content || (!maskedValues.length && !redactJSONValue)) return content; const redactValue = value => { if (typeof value === "string") return applyAddMaskRedaction(value, maskedValues); if (Array.isArray(value)) return value.map(redactValue); @@ -213,25 +214,32 @@ function redactArtifactMaskedValues(content, maskedValues) { return value; }; const redactJson = text => { + let value; try { - let value = JSON.parse(text); - // Skip quoted strings and match original numeric tokens at every depth so - // rounding or exponent notation cannot hide a mask. - const numericRedacted = transformJSONStrings( - text, - token => token, - plain => - plain.replace(/-?\d+(?:\.\d+)?(?:[eE][+-]?\d+)?/g, token => { - const redacted = redactMaskedValues(token, maskedValues); - return redacted === token ? token : JSON.stringify(redacted); - }) - ); - if (numericRedacted !== text) value = JSON.parse(numericRedacted); - const redacted = JSON.stringify(redactValue(value)); - return redacted === JSON.stringify(value) ? numericRedacted : text.match(/^\s*/)[0] + redacted + text.match(/\s*$/)[0]; + value = JSON.parse(text); } catch { return undefined; } + // Skip quoted strings and match original numeric tokens at every depth so + // rounding or exponent notation cannot hide a mask. + const numericRedacted = transformJSONStrings( + text, + token => token, + plain => + plain.replace(/-?\d+(?:\.\d+)?(?:[eE][+-]?\d+)?/g, token => { + const redacted = redactMaskedValues(token, maskedValues); + return redacted === token ? token : JSON.stringify(redacted); + }) + ); + if (numericRedacted !== text) { + try { + value = JSON.parse(numericRedacted); + } catch (error) { + throw new Error("Unable to decode redacted JSON log record during secret redaction", { cause: error }); + } + } + const redacted = JSON.stringify((redactJSONValue || redactValue)(value)); + return redacted === JSON.stringify(value) ? numericRedacted : text.match(/^\s*/)[0] + redacted + text.match(/\s*$/)[0]; }; const json = redactJson(content); if (json !== undefined) return json; diff --git a/actions/setup/js/redact_secrets.cjs b/actions/setup/js/redact_secrets.cjs index f1d6e4c7c18..1a61417dd99 100644 --- a/actions/setup/js/redact_secrets.cjs +++ b/actions/setup/js/redact_secrets.cjs @@ -10,7 +10,8 @@ const fs = require("fs"); const path = require("path"); const { getErrorMessage } = require("./error_helpers.cjs"); const { ERR_VALIDATION } = require("./error_codes.cjs"); -const { collectAddMaskedValues, redactArtifactMaskedValues } = require("./add_mask_redaction.cjs"); +const { applyAddMaskRedaction, collectAddMaskedValues, redactArtifactMaskedValues } = require("./add_mask_redaction.cjs"); +const { redactSafeOutputContent } = require("./safe_output_redaction.cjs"); const { redactPiSessionHTML } = require("./pi_session_redaction.cjs"); const { getOTLPSecretValues } = require("./otlp_header_helpers.cjs"); const { redactSensitiveConfig } = require("./safe_outputs_config_redact.cjs"); @@ -499,6 +500,24 @@ function processFile(filePath, secretValues, maskedValues = [], prepareForCustom try { const content = fs.readFileSync(filePath, "utf8"); stage = "redact"; + const structured = redactSafeOutputContent( + filePath, + content, + [...maskedValues, ...secretValues.filter(value => value && value.length >= 6)].sort((a, b) => b.length - a.length), + text => { + const custom = redactSecrets(redactBuiltInPatterns(text).content, secretValues); + return applyAddMaskRedaction(custom.content, maskedValues); + } + ); + if (structured !== null) { + const redactionCount = structured === content ? 0 : 1; + if (redactionCount || prepareForCustomMasking) { + stage = "write"; + writeProcessedFile(filePath, structured, Boolean(redactionCount)); + } + if (redactionCount) core.info(`Processed ${filePath}: ${redactionCount} redaction(s)`); + return redactionCount; + } const encodedResult = path.extname(filePath).toLowerCase() === ".html" ? redactPiSessionHTML(content, text => { diff --git a/actions/setup/js/runtime_mask_publication.test.cjs b/actions/setup/js/runtime_mask_publication.test.cjs index ab244725431..0867acdb632 100644 --- a/actions/setup/js/runtime_mask_publication.test.cjs +++ b/actions/setup/js/runtime_mask_publication.test.cjs @@ -7,6 +7,7 @@ import { publishUnifiedSessionSummary } from "./unified_session_render.cjs"; const redactScript = fs.readFileSync(path.join(import.meta.dirname, "redact_secrets.cjs"), "utf8"); const bootstrapScript = fs.readFileSync(path.join(import.meta.dirname, "log_parser_bootstrap.cjs"), "utf8"); +const collectorScript = fs.readFileSync(path.join(import.meta.dirname, "collect_ndjson_output.cjs"), "utf8"); describe("runtime mask publication boundary", () => { let root, otherRoot, originalEnv, originalCore; @@ -111,12 +112,12 @@ describe("runtime mask publication boundary", () => { it("sanitizes sources with oversized runtime masks without removing artifacts", async () => { const mask = "x".repeat(200000); const stdio = write("agent-stdio.log", `::add-mask::${mask}\noutput ${mask}\n`); - const source = write("safeoutputs.jsonl", { body: mask }); + const source = write("safeoutputs.jsonl", { type: "add_comment", body: mask }); await redactSources(); expect(core.setFailed).not.toHaveBeenCalled(); expect(core.warning).not.toHaveBeenCalled(); expect(fs.readFileSync(stdio, "utf8")).toBe("output ***\n"); - expect(JSON.parse(fs.readFileSync(source, "utf8"))).toEqual({ body: "***" }); + expect(JSON.parse(fs.readFileSync(source, "utf8"))).toEqual({ type: "add_comment", body: "***" }); }); it("sanitizes regex-shaped multiline gateway text without removing artifacts", async () => { @@ -130,18 +131,111 @@ describe("runtime mask publication boundary", () => { ]; const mask = actualGatewayLines.join("\n"); const stdio = write("agent-stdio.log", `::add-mask::${mask.replace(/\n/g, "%0A")}\n${mask}\n`); - const source = write("safeoutputs.jsonl", { body: mask }); + const source = write("safeoutputs.jsonl", { type: "add_comment", body: mask }); await redactSources(); expect(core.setFailed).not.toHaveBeenCalled(); expect(core.warning).not.toHaveBeenCalled(); expect(fs.readFileSync(stdio, "utf8")).toBe("***\n***\n***\n***\n"); - expect(JSON.parse(fs.readFileSync(source, "utf8"))).toEqual({ body: "***\n***\n***\n***" }); + expect(JSON.parse(fs.readFileSync(source, "utf8"))).toEqual({ type: "add_comment", body: "***\n***\n***\n***" }); }); + it("keeps review messages executable after short-mask redaction across live, fallback, and collected outputs", async () => { + const originalGithub = global.github; + const originalContext = global.context; + try { + core.debug = vi.fn(); + core.setOutput = vi.fn(); + core.exportVariable = vi.fn(); + global.github = { rest: { pulls: { get: vi.fn().mockResolvedValue({ data: {} }) } } }; + global.context = { + eventName: "pull_request", + runId: 123, + repo: { owner: "test-owner", repo: "test-repo" }, + payload: { pull_request: { number: 12, head: { sha: "test-sha" } } }, + }; + const messages = [ + { type: "create_pull_request_review_comment", path: "src/file.js", line: 12, side: "RIGHT", pull_request_number: 12, body: "bad body" }, + { type: "submit_pull_request_review", event: "COMMENT", pull_request_number: 12, body: "bad body" }, + ]; + write("agent-stdio.log", "::add-mask::b\n"); + process.env.GH_AW_SAFE_OUTPUTS = write("runner-temp/safeoutputs/outputs.jsonl", messages.map(item => JSON.stringify(item)).join("\n") + "\n"); + const config = { create_pull_request_review_comment: { max: 10 }, submit_pull_request_review: { max: 1, allowed_events: ["COMMENT"] } }; + process.env.GH_AW_SAFE_OUTPUTS_CONFIG_PATH = write("runner-temp/safeoutputs/config.json", config); + const fields = { body: { required: true, type: "string" }, pull_request_number: { type: "number" } }; + const validation = { + create_pull_request_review_comment: { defaultMax: 10, fields: { ...fields, path: { type: "string" }, line: { type: "number" }, side: { type: "string", enum: ["LEFT", "RIGHT"] } } }, + submit_pull_request_review: { defaultMax: 1, fields: { ...fields, event: { type: "string", enum: ["COMMENT"] } } }, + }; + process.env.GH_AW_VALIDATION_CONFIG_PATH = write("runner-temp/safeoutputs/validation.json", validation); + const fallback = write("safeoutputs.jsonl", messages.map(item => JSON.stringify(item)).join("\n") + "\n"); + const collected = write("agent_output.json", { items: messages, errors: [] }); + const generic = write("mcp-logs/rpc.jsonl", messages[1]); + await redactSources(); + expect(core.setFailed).not.toHaveBeenCalled(); + const expected = messages.map(item => ({ ...item, body: "***ad ***ody" })); + for (const file of [process.env.GH_AW_SAFE_OUTPUTS, fallback]) { + expect(fs.readFileSync(file, "utf8").trim().split("\n").map(JSON.parse)).toEqual(expected); + } + const output = JSON.parse(fs.readFileSync(collected, "utf8")); + expect(output).toEqual({ items: expected, errors: [] }); + expect(JSON.parse(fs.readFileSync(generic, "utf8"))).toEqual({ type: "su***mit_pull_request_review", event: "COMMENT", "pull_request_num***er": 12, "***ody": "***ad ***ody" }); + expect(JSON.parse(fs.readFileSync(process.env.GH_AW_SAFE_OUTPUTS_CONFIG_PATH, "utf8"))).toEqual(config); + expect(JSON.parse(fs.readFileSync(process.env.GH_AW_VALIDATION_CONFIG_PATH, "utf8"))).toEqual(validation); + const script = collectorScript + .replace('const { AGENT_OUTPUT_FILENAME, TMP_GH_AW_PATH } = require("./constants.cjs");', 'const { AGENT_OUTPUT_FILENAME } = require("./constants.cjs"); const TMP_GH_AW_PATH = root;') + .replaceAll("/tmp/gh-aw", root); + await eval(`(async () => { ${script}; await main(); })()`); + const collectedOutput = JSON.parse(fs.readFileSync(collected, "utf8")); + expect(collectedOutput.errors).toEqual([]); + expect(collectedOutput.items).toEqual(expected); + const { createReviewBuffer } = require("./pr_review_buffer.cjs"); + const buffer = createReviewBuffer(); + for (const [index, module] of ["./create_pr_review_comment.cjs", "./submit_pr_review.cjs"].entries()) { + const handler = await require(module).main({ _prReviewBuffer: buffer }); + expect((await handler(collectedOutput.items[index], {})).success).toBe(true); + } + expect(buffer.getBufferedCount()).toBe(1); + expect(core.warning).not.toHaveBeenCalled(); + } finally { + global.github = originalGithub; + global.context = originalContext; + require("./safe_output_type_validator.cjs").resetValidationConfigCache(); + } + }); + + it("redacts built-in, custom, encoded, and numeric payload secrets without changing protocol keys", async () => { + const builtin = "ghs_" + "b".repeat(40); + process.env.GH_AW_SECRET_NAMES = "CUSTOM"; + process.env.SECRET_CUSTOM = "body-value"; + write("agent-stdio.log", "::add-mask::b\n::add-mask::9007199254740993\n"); + const source = write("safeoutputs.jsonl", `{"type":"create_pull_request_review_comment","path":"src/file.js","line":9007199254740993,"body":"${builtin} \\u0062ody-value"}\n`); + await redactSources(); + expect(core.setFailed).not.toHaveBeenCalled(); + const message = JSON.parse(fs.readFileSync(source, "utf8")); + expect(message.type).toBe("create_pull_request_review_comment"); + expect(message.line).toBe("***"); + expect(message.body).not.toContain(builtin); + expect(message.body).not.toContain("body-value"); + expect(message.body).not.toContain("ody-value"); + expect(message.body).not.toContain("b"); + }); + + it.each(['{"type":"add_comment","body":"opaque"', '{"type":"opaque_job","body":"opaque"}', '{"type":"add_comment","opaque_field":"value","body":"opaque"}'])( + "removes unsafe executable output instead of publishing a malformed or rerouted message", + content => { + const { processFile } = require("./redact_secrets.cjs"); + const source = write("safeoutputs.jsonl", content); + expect(processFile(source, [], ["opaque"])).toBe(0); + expect(fs.existsSync(source)).toBe(false); + expect(core.setFailed).toHaveBeenCalledWith(expect.stringContaining("Removed artifact source after secret redaction failed")); + expect(core.warning.mock.calls.flat().join("\n")).not.toContain("opaque"); + } + ); + it("removes a source on failed runtime-mask writes without logging masks and continues sanitizing the other files", async () => { write("agent-stdio.log", `::add-mask::${opaque.replace(/%/g, "%25")}\n`); const failing = write("mcp-logs/first.jsonl", { credential: opaque }); - const remaining = write("safeoutputs.jsonl", { body: opaque }); + const remaining = write("safeoutputs.jsonl", { type: "add_comment", body: opaque }); const originalWrite = fs.writeFileSync; vi.spyOn(fs, "writeFileSync").mockImplementation((file, ...args) => { if (file === failing) throw new Error(`write denied: ${opaque}`); diff --git a/actions/setup/js/safe_output_redaction.cjs b/actions/setup/js/safe_output_redaction.cjs new file mode 100644 index 00000000000..a7cc9ba6c90 --- /dev/null +++ b/actions/setup/js/safe_output_redaction.cjs @@ -0,0 +1,177 @@ +// @ts-check + +const path = require("path"); +const tools = require("./safe_outputs_tools.json"); +const { redactArtifactMaskedValues } = require("./add_mask_redaction.cjs"); +const { ERR_VALIDATION } = require("./error_codes.cjs"); + +const schemas = new Map(tools.map(tool => [tool.name, tool.inputSchema])); +const controlKeys = new Set([ + ...schemas.keys(), + ...Array.from(schemas.keys(), name => name.replace(/_/g, "-")), + ...tools.flatMap(tool => Object.keys(tool.inputSchema.properties || {})), + "max", + "min", + "side", + "allowed_events", + "allowed-events", + "defaultMax", + "fields", + "customValidation", + "dataEnabled", + "dataSchema", + "collapseData", + "required", + "type", + "typeHint", + "sanitize", + "maxLength", + "minLength", + "positiveInteger", + "optionalPositiveInteger", + "allowAuto", + "allowNull", + "allowEmpty", + "issueOrPRNumber", + "issueNumberOrTemporaryId", + "enum", + "itemType", + "itemSanitize", + "itemMaxLength", + "pattern", + "patternError", + "temporaryId", + "rejectIfOversized", + "x-strip-on-error", +]); +const publicEnums = new Set(tools.flatMap(tool => Object.values(tool.inputSchema.properties || {}).flatMap(property => property.enum || []))); + +/** + * Config and validation files are consumed after the redaction step too. + * Preserve their public vocabulary, but never rewrite a constraint or target. + * @param {any} value + * @param {(text: string) => string} redactText + * @param {string} [field] + */ +function assertSafeControl(value, redactText, field = "") { + if (typeof value === "string") { + if (["type", "itemType"].includes(field) && ["string", "number", "integer", "boolean", "array", "object", "null"].includes(value)) return; + if (["enum", "allowed_events", "allowed-events"].includes(field) && publicEnums.has(value)) return; + if (redactText(value) !== value) throw new Error(`${ERR_VALIDATION}: Secret redaction would change safe-output control data`); + } else if (typeof value === "number") { + if (redactText(String(value)) !== String(value)) throw new Error(`${ERR_VALIDATION}: Secret redaction would change a safe-output constraint`); + } else if (Array.isArray(value)) { + value.forEach(item => assertSafeControl(item, redactText, field)); + } else if (value && typeof value === "object") { + for (const [key, nested] of Object.entries(value)) { + if (!controlKeys.has(key) && redactText(key) !== key) throw new Error(`${ERR_VALIDATION}: Secret redaction would change a custom safe-output control field`); + assertSafeControl(nested, redactText, key); + } + } +} + +/** + * Public schema vocabulary is not secret payload data. Only schemas shipped + * with the action can authorize preserving a key or enum, never agent files. + * @param {any} value + * @param {any} schema + * @param {(text: string) => string} redactText + * @returns {any} + */ +function redactBySchema(value, schema, redactText) { + if (typeof value === "string") return schema?.enum?.includes(value) ? value : redactText(value); + if (Array.isArray(value)) return value.map(item => redactBySchema(item, schema?.items, redactText)); + if (value && typeof value === "object") { + return Object.fromEntries( + Object.entries(value).map(([key, nested]) => { + const property = schema?.properties && Object.hasOwn(schema.properties, key) ? schema.properties[key] : undefined; + return [property ? key : redactText(key), redactBySchema(nested, property, redactText)]; + }) + ); + } + return value; +} + +/** + * @param {any} item + * @param {(text: string) => string} redactText + * @returns {any} + */ +function redactMessage(item, redactText) { + if (!item || typeof item !== "object" || Array.isArray(item) || typeof item.type !== "string") { + throw new Error(`${ERR_VALIDATION}: Invalid safe-output message during secret redaction`); + } + const schema = schemas.get(item.type); + if (!schema && redactText(item.type) !== item.type) { + throw new Error(`${ERR_VALIDATION}: Secret redaction would change a custom safe-output type`); + } + const properties = { ...schema?.properties, type: { enum: [item.type] } }; + for (const property of Object.values(schema?.properties || {})) { + for (const alias of property["x-synonyms"] || []) properties[alias] = property; + } + // Unknown top-level fields may be custom job inputs or Claim routing data. + // Do not silently rename them and then execute a different message. + for (const key of Object.keys(item)) { + if (!Object.hasOwn(properties, key) && redactText(key) !== key) { + throw new Error(`${ERR_VALIDATION}: Secret redaction would change a custom safe-output field`); + } + } + return redactBySchema(item, { properties }, redactText); +} + +/** + * Return null for ordinary artifacts; recognized executable outputs must be + * valid JSON/JSONL or throw so processFile removes them before publication. + * @param {string} filePath + * @param {string} content + * @param {string[]} maskedValues + * @param {(text: string) => string} redactText + * @returns {string | null} + */ +function redactSafeOutputContent(filePath, content, maskedValues, redactText) { + const name = path.basename(filePath); + const control = + filePath === process.env.GH_AW_SAFE_OUTPUTS_CONFIG_PATH || filePath === process.env.GH_AW_VALIDATION_CONFIG_PATH || (path.basename(path.dirname(filePath)) === "safeoutputs" && ["config.json", "validation.json"].includes(name)); + const envelope = name === "agent_output.json"; + if (!control && !envelope && name !== "safeoutputs.jsonl" && filePath !== process.env.GH_AW_SAFE_OUTPUTS) return null; + const parse = record => { + try { + return JSON.parse(record); + } catch (error) { + throw new Error(`${ERR_VALIDATION}: Invalid JSON safe-output record during secret redaction`, { cause: error }); + } + }; + if (control) { + const value = parse(content); + if (!value || typeof value !== "object" || Array.isArray(value)) throw new Error(`${ERR_VALIDATION}: Invalid safe-output control object`); + const redacted = redactArtifactMaskedValues(content, maskedValues, parsed => { + assertSafeControl(parsed, redactText); + return parsed; + }); + if (redacted !== content) throw new Error(`${ERR_VALIDATION}: Secret redaction would change a safe-output constraint`); + return content; + } + const transform = value => { + if (!envelope) return redactMessage(value, redactText); + if (!value || typeof value !== "object" || !Array.isArray(value.items)) { + throw new Error(`${ERR_VALIDATION}: Invalid agent-output envelope during secret redaction`); + } + const properties = Object.fromEntries(["errors", "collectorEmptyOutputCause", "collectorFailureCause", "collectorDriverExitCode", "collectorRetryCount", "collectorEngineErrorType"].map(key => [key, {}])); + return Object.fromEntries( + Object.entries(value).map(([key, nested]) => + key === "items" ? [key, nested.map(item => redactMessage(item, redactText))] : [Object.hasOwn(properties, key) ? key : redactText(key), redactBySchema(nested, properties[key], redactText)] + ) + ); + }; + const redactRecord = record => { + parse(record); + return redactArtifactMaskedValues(record, maskedValues, transform); + }; + if (envelope) return redactRecord(content); + return content + .split("\n") + .map(line => (line.trim() ? redactRecord(line) : line)) + .join("\n"); +} + +module.exports = { redactSafeOutputContent }; diff --git a/actions/setup/js/safe_output_redaction.test.cjs b/actions/setup/js/safe_output_redaction.test.cjs new file mode 100644 index 00000000000..f5e48d69a8d --- /dev/null +++ b/actions/setup/js/safe_output_redaction.test.cjs @@ -0,0 +1,123 @@ +// @ts-check +import { describe, expect, it } from "vitest"; +import { createRequire } from "node:module"; + +const require = createRequire(import.meta.url); +const { redactSafeOutputContent } = require("./safe_output_redaction.cjs"); +const { applyAddMaskRedaction } = require("./add_mask_redaction.cjs"); +const tools = require("./safe_outputs_tools.json"); + +function redact(file, content, masks) { + return redactSafeOutputContent(file, content, masks, text => applyAddMaskRedaction(text, masks)); +} + +describe("safe-output schema redaction", () => { + it.each(["b", "body", "submit_pull_request_review", "COMMENT", "pull_request_number"])("preserves public review vocabulary while redacting the same text in payloads (%s)", mask => { + const message = { type: "submit_pull_request_review", event: "COMMENT", pull_request_number: 12, body: `prefix ${mask} suffix` }; + const content = JSON.stringify(message) + "\n"; + expect(JSON.parse(redact("safeoutputs.jsonl", content, [mask]))).toEqual({ ...message, body: "prefix *** suffix" }); + expect(JSON.parse(redact("agent_output.json", JSON.stringify({ items: [message], errors: [] }), [mask]))).toEqual({ items: [{ ...message, body: "prefix *** suffix" }], errors: [] }); + }); + + it("preserves only declared nested keys, never arbitrary payload keys or values", () => { + const message = { type: "ledger_append", record: { body: "body", nested: [{ body: "body" }] } }; + expect(JSON.parse(redact("safeoutputs.jsonl", JSON.stringify(message), ["body"]))).toEqual({ type: "ledger_append", record: { "***": "***", nested: [{ "***": "***" }] } }); + }); + + it.each(["body", "type", "event", "items", "COMMENT"])("does not treat public words as exempt inside free-form content (%s)", mask => { + const message = { type: "add_comment", body: `before ${mask} after` }; + const result = JSON.parse(redact("safeoutputs.jsonl", JSON.stringify(message), [mask])); + expect(result).toEqual({ type: "add_comment", body: "before *** after" }); + }); + + it("redacts escaped strings and original high-precision numeric tokens", () => { + const content = '{"type":"create_pull_request_review_comment","body":"\\u006fpaque","path":"src/file.js","line":9007199254740993}'; + expect(JSON.parse(redact("safeoutputs.jsonl", content, ["opaque", "9007199254740993"]))).toEqual({ + type: "create_pull_request_review_comment", + body: "***", + path: "src/file.js", + line: "***", + }); + }); + + it("retains unchanged JSON/JSONL formatting and large records", () => { + const content = ' \t{"type":"add_comment", "body":"' + "x".repeat(10 * 1024 * 1024) + '"}\r\n\n'; + expect(redact("safeoutputs.jsonl", content, ["absent"])).toBe(content); + const masked = redact("safeoutputs.jsonl", content, ["xxx"]); + expect(JSON.parse(masked.trim()).type).toBe("add_comment"); + expect(JSON.parse(masked.trim()).body).not.toContain("xxx"); + }); + + it("keeps unknown custom types and fields only when redaction does not alter their routing", () => { + const content = '{"type":"custom_job","custom_input":"opaque"}'; + expect(JSON.parse(redact("safeoutputs.jsonl", content, ["opaque"]))).toEqual({ type: "custom_job", custom_input: "***" }); + expect(() => redact("safeoutputs.jsonl", content, ["custom_job"])).toThrow("Secret redaction would change a custom safe-output type"); + expect(() => redact("safeoutputs.jsonl", content, ["custom_input"])).toThrow("Secret redaction would change a custom safe-output field"); + }); + + it.each(['{"type":"add_comment","body":"opaque"', '{"body":"opaque"}', "null", "[]", '"opaque"'])("rejects malformed or invalid executable records instead of using mixed-log recovery (%s)", content => { + expect(() => redact("safeoutputs.jsonl", content, ["opaque"])).toThrow(); + }); + + it("rejects an invalid agent-output envelope and propagates redactor failures", () => { + expect(() => redact("agent_output.json", '{"items":{},"body":"opaque"}', ["opaque"])).toThrow("Invalid agent-output envelope"); + expect(() => + redactSafeOutputContent("safeoutputs.jsonl", '{"type":"add_comment","body":"opaque"}', ["opaque"], () => { + throw new Error("synthetic redactor failure"); + }) + ).toThrow("synthetic redactor failure"); + }); + + it("never exempts unknown enum values or fields from masking", () => { + expect(JSON.parse(redact("safeoutputs.jsonl", '{"type":"submit_pull_request_review","event":"opaque","body":"opaque"}', ["opaque"]))).toEqual({ + type: "submit_pull_request_review", + event: "***", + body: "***", + }); + expect(() => redact("safeoutputs.jsonl", '{"type":"add_comment","opaque":"text","body":"text"}', ["opaque"])).toThrow("Secret redaction would change a custom safe-output field"); + }); + + it("preserves declared aliases without exempting their values", () => { + const content = '{"type":"submit_pull_request_review","action":"COMMENT","pullRequestNumber":"12","body":"opaque"}'; + expect(JSON.parse(redact("safeoutputs.jsonl", content, ["action", "pullRequestNumber", "opaque"]))).toEqual({ + type: "submit_pull_request_review", + action: "COMMENT", + pullRequestNumber: "12", + body: "***", + }); + }); + + it("does not enable protocol exceptions for arbitrary artifacts or similarly named files", () => { + for (const file of ["log.json", "agent_output.json.bak", "safeoutputs.txt", "safe-output-items.jsonl"]) { + expect(redact(file, '{"type":"add_comment","body":"opaque"}', ["opaque"])).toBeNull(); + } + }); + + it("keeps public configuration and validation vocabulary without disabling real constraints", () => { + const config = '{"submit_pull_request_review":{"max":1,"allowed_events":["COMMENT"]}}'; + const validation = '{"submit_pull_request_review":{"defaultMax":1,"fields":{"body":{"type":"string","maxLength":65000},"event":{"type":"string","enum":["COMMENT"]}}}}'; + expect(redact("/tmp/test/safeoutputs/config.json", config, ["b"])).toBe(config); + expect(redact("/tmp/test/safeoutputs/validation.json", validation, ["b"])).toBe(validation); + expect(() => redact("/tmp/test/safeoutputs/config.json", config, ["1"])).toThrow("safe-output constraint"); + expect(() => redact("/tmp/test/safeoutputs/config.json", '{"add_comment":{"repo":"opaque/repo"}}', ["opaque"])).toThrow("safe-output control data"); + expect(() => redact("/tmp/test/safeoutputs/config.json", '{"custom_job":{"max":1}}', ["custom_job"])).toThrow("custom safe-output control field"); + expect(() => redact("/tmp/test/safeoutputs/config.json", '{"add_comment":{"max":9007199254740993}}', ["9007199254740993"])).toThrow("safe-output constraint"); + }); + + it("covers each shipped tool and property against seeded short-mask collisions", () => { + let state = 0x5eed; + for (const tool of tools) { + const properties = tool.inputSchema.properties || {}; + for (const key of Object.keys(properties)) { + state = (Math.imul(state, 1664525) + 1013904223) >>> 0; + const vocabulary = tool.name + key; + const mask = vocabulary[state % vocabulary.length]; + const item = { type: tool.name, [key]: `payload ${mask} payload` }; + const result = JSON.parse(redact("safeoutputs.jsonl", JSON.stringify(item), [mask])); + expect(result.type, `${tool.name}.${key}`).toBe(tool.name); + expect(Object.hasOwn(result, key), `${tool.name}.${key}`).toBe(true); + expect(result[key], `${tool.name}.${key}`).toBe(applyAddMaskRedaction(item[key], [mask])); + } + } + }); +}); diff --git a/docs/src/content/docs/reference/imports.md b/docs/src/content/docs/reference/imports.md index 4f5588d0ac8..7ecdf0649fc 100644 --- a/docs/src/content/docs/reference/imports.md +++ b/docs/src/content/docs/reference/imports.md @@ -390,6 +390,8 @@ Built-in redaction scans supported text files under `/tmp/gh-aw` and `${RUNNER_T Runtime-mask redaction handles multi-megabyte JSON strings and mixed log records without relying on recursive regular-expression matching. If malformed quotes hide an encoded mask, a conservative escape-decoding pass redacts it and represents that record as a JSON string; records without additional mask matches retain their original bytes. This does not change GitHub's step-summary size limit. +Executable safe-output JSON/JSONL preserves public field names, aliases, and enum values declared by the action's bundled tool schemas, so short masks cannot rename `body` or change a review's message type. Free-form values and arbitrary nested keys are still redacted, even when they contain those same public words. The configuration and validation files consumed by the output collector also retain public protocol vocabulary; redaction must not rewrite their constraints or targets. Malformed executable records and mask conflicts with custom message types, custom top-level fields, or control data fail closed: the source is removed and redaction fails rather than publishing a corrupted message. Ordinary log artifacts do not receive these protocol exceptions. + Custom hooks must tolerate absent optional files and files that disappear between discovery and reading (`ENOENT`). Earlier cleanup or redaction can remove files, including symbolic links and artifacts that could not be sanitized. Do not create empty placeholders for missing logs or outputs. Custom hooks run during the initial workspace redaction, before graders execute. The later grader-output redaction pass sanitizes newly generated results but does not rerun custom hooks, so it does not prepare unchanged files for them. From 5ff7b3848c7e879766194b56defb518a0d763ca5 Mon Sep 17 00:00:00 2001 From: pelikhan Date: Thu, 15 Oct 2026 23:14:28 -0700 Subject: [PATCH 04/11] Fix malformed-record redaction and credential overlap Preserve unrelated escapes, bound rejected-token scanning, and resolve configured, built-in, and runtime masks without encoding or overlap bypasses. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../patch-large-record-mask-redaction.md | 2 + actions/setup/js/add_mask_redaction.cjs | 336 +++++++++++++++--- actions/setup/js/add_mask_redaction.test.cjs | 152 +++++++- actions/setup/js/redact_secrets.cjs | 74 ++-- actions/setup/js/redact_secrets.test.cjs | 166 +++++++++ docs/src/content/docs/reference/imports.md | 2 +- 6 files changed, 666 insertions(+), 66 deletions(-) diff --git a/.changeset/patch-large-record-mask-redaction.md b/.changeset/patch-large-record-mask-redaction.md index 6594ed9144c..7665535a184 100644 --- a/.changeset/patch-large-record-mask-redaction.md +++ b/.changeset/patch-large-record-mask-redaction.md @@ -4,4 +4,6 @@ Avoid regular-expression stack exhaustion when redacting runtime masks from multi-megabyte JSON and mixed-log artifacts, while preserving fail-closed cleanup for genuine redaction failures. +Keep malformed-token recovery linear, preserve unrelated plain-text escapes when masking encoded values, and match configured secrets together with runtime masks to prevent fragmentation or encoding changes from bypassing redaction. + Preserve bundled safe-output schema vocabulary during redaction so short masks do not corrupt review message fields, dispatch types, or collector configuration. Continue redacting payload data and fail closed on malformed executable records, unsafe custom routing changes, or conflicts with control data. diff --git a/actions/setup/js/add_mask_redaction.cjs b/actions/setup/js/add_mask_redaction.cjs index 16b417cd7c4..984a76a841b 100644 --- a/actions/setup/js/add_mask_redaction.cjs +++ b/actions/setup/js/add_mask_redaction.cjs @@ -68,14 +68,14 @@ function collectAddMaskedValues(logContent) { } /** - * Replace all occurrences of the masked values with `***`. - * + * Yield non-overlapping matches, breaking equal-position ties by mask order. * @param {string} text * @param {string[]} maskedValues - * @returns {string} + * @param {(match: {start: number, end: number, value: string}) => boolean} [allowed] + * @param {boolean} [overlapping] + * @yields {{start: number, end: number, value: string}} */ -function redactMaskedValues(text, maskedValues) { - if (!text || !maskedValues || maskedValues.length === 0) return text; +function* maskedValueMatches(text, maskedValues, allowed = () => true, overlapping = false) { const pending = []; for (let order = 0; order < maskedValues.length; order++) { const value = maskedValues[order]; @@ -95,28 +95,94 @@ function redactMaskedValues(text, maskedValues) { } }; for (let i = Math.floor(pending.length / 2) - 1; i >= 0; i--) siftDown(i); - const parts = []; let cursor = 0; - let replacements = 0; while (pending.length && cursor < text.length) { const next = pending[0]; + let searchStart = cursor; if (next.index >= cursor) { - if (next.index > cursor) { - if (replacements) parts.push(MASK_REPLACEMENT.repeat(replacements)); - replacements = 0; - parts.push(text.slice(cursor, next.index)); + const match = { start: next.index, end: next.index + next.value.length, value: next.value }; + if (allowed(match)) { + yield match; + cursor = overlapping ? match.start + 1 : match.end; + searchStart = cursor; + } else { + searchStart = next.index + 1; } - replacements++; - cursor = next.index + next.value.length; } // Only refresh consumed or overlapping occurrences; later candidates stay cached. - next.index = text.indexOf(next.value, cursor); + next.index = text.indexOf(next.value, searchStart); if (next.index === -1) { const last = pending.pop(); if (pending.length) pending[0] = last; } siftDown(0); } +} + +/** + * @param {Iterable<{start: number, end: number, value: string, ambiguous?: boolean}>} matches + */ +function* overlappingMatchGroups(matches) { + let group; + for (const match of matches) { + if (group && match.start < group.end) { + group.end = Math.max(group.end, match.end); + if (match.value.length > group.value.length) group.value = match.value; + group.ambiguous ||= match.ambiguous; + } else { + if (group) yield group; + group = { ...match }; + } + } + if (group) yield group; +} + +/** + * Coalesce overlapping credentials before choosing their replacement marker. + * @param {string} text + * @param {string[]} maskedValues - Longest credentials first + * @param {(value: string) => string} replacementForValue + * @returns {string} + */ +function redactOverlappingMaskedValues(text, maskedValues, replacementForValue) { + const parts = []; + let cursor = 0; + for (const match of overlappingMatchGroups(maskedValueMatches(text, maskedValues, undefined, true))) { + parts.push(text.slice(cursor, match.start), replacementForValue(match.value)); + cursor = match.end; + } + parts.push(text.slice(cursor)); + return parts.join(""); +} + +/** + * Replace all occurrences of the masked values, defaulting to `***`. + * + * @param {string} text + * @param {string[]} maskedValues + * @param {(value: string) => string} [replacementForValue] - Explicit markers coalesce overlapping credentials + * @returns {string} + */ +function redactMaskedValues(text, maskedValues, replacementForValue) { + if (!text || !maskedValues || maskedValues.length === 0) return text; + if (replacementForValue) + return redactOverlappingMaskedValues( + text, + [...maskedValues].sort((a, b) => b.length - a.length), + replacementForValue + ); + const parts = []; + let cursor = 0; + let replacements = 0; + for (const match of maskedValueMatches(text, maskedValues)) { + if (match.start > cursor) { + if (replacements) parts.push(MASK_REPLACEMENT.repeat(replacements)); + replacements = 0; + parts.push(text.slice(cursor, match.start)); + } + replacements++; + cursor = match.end; + } if (replacements) parts.push(MASK_REPLACEMENT.repeat(replacements)); parts.push(text.slice(cursor)); return parts.join(""); @@ -127,15 +193,16 @@ function redactMaskedValues(text, maskedValues) { * * @param {string} text - Text about to be rendered into generated content * @param {string[]} maskedValues - Values collected via {@link collectAddMaskedValues} + * @param {(value: string) => string} [replacementForValue] * @returns {string} */ -function applyAddMaskRedaction(text, maskedValues) { +function applyAddMaskRedaction(text, maskedValues, replacementForValue) { if (!text) return text; const withoutCommands = text .split("\n") .filter(line => !isAddMaskCommandLine(line)) .join("\n"); - return redactMaskedValues(withoutCommands, maskedValues); + return redactMaskedValues(withoutCommands, maskedValues, replacementForValue); } /** @@ -180,8 +247,9 @@ function transformJSONStrings(text, transformString, transformText = value => va end++; } if (invalidString) { - parts.push(transformText(text.slice(cursor, start + 1))); - cursor = start + 1; + // Do not rescan escaped quotes in the rejected prefix. + parts.push(transformText(text.slice(cursor, end))); + cursor = end; continue; } if (end >= text.length) { @@ -195,6 +263,181 @@ function transformJSONStrings(text, transformString, transformText = value => va return parts.join(""); } +/** + * Decode ambiguous escapes for matching only, mapping matches back to source + * spans so unrelated plain-text paths and literal escapes keep their bytes. + * @param {string} text + * @param {string[]} maskedValues + * @param {(value: string) => string} replacementForValue + * @returns {string} + */ +function redactAmbiguousMaskedValues(text, maskedValues, replacementForValue) { + const escapes = { '"': '"', "\\": "\\", "/": "/", b: "\b", f: "\f", n: "\n", r: "\r", t: "\t" }; + const pattern = /\\(["\\/bfnrt]|u[0-9a-fA-F]{4})/g; + const unescaped = text.replace(pattern, (_, escape) => { + if (escape[0] === "u") return String.fromCharCode(parseInt(escape.slice(1), 16)); + return escapes[escape]; + }); + let nextEscape = pattern.exec(text); + let offset = 0; + const sourceOffset = position => { + while (nextEscape && nextEscape.index - offset < position) { + offset += nextEscape[0].length - 1; + nextEscape = pattern.exec(text); + } + return position + offset; + }; + const parts = []; + let cursor = 0; + for (const match of maskedValueMatches(unescaped, maskedValues)) { + const start = sourceOffset(match.start); + const end = sourceOffset(match.end); + parts.push(text.slice(cursor, start), replacementForValue(match.value)); + cursor = end; + } + // Ambiguous records retain their JSON-string wrapper, but its decoded value + // now changes only at the source spans that actually matched a mask. + return cursor ? JSON.stringify(parts.join("") + text.slice(cursor)) : text; +} + +/** + * @param {string} encoded + * @returns {string} + */ +function decodeJSONString(encoded) { + try { + return JSON.parse(encoded); + } catch (error) { + throw new Error("Unable to decode JSON log string during secret redaction", { cause: error }); + } +} + +/** + * Resolve raw and encoded matches together before a malformed record can be + * rewritten. Quoted-token interiors use decoded semantics; raw matches spanning + * token boundaries remain eligible so a shorter mask cannot fragment them. + * @param {string} text + * @param {string[]} maskedValues + * @param {(value: string) => string} replacementForValue + * @returns {string} + */ +function redactMixedMaskedValues(text, maskedValues, replacementForValue) { + maskedValues = [...maskedValues].sort((a, b) => b.length - a.length); + const tokens = []; + let scanned = 0; + transformJSONStrings( + text, + token => { + decodeJSONString(token); + tokens.push(scanned, scanned + token.length); + scanned += token.length; + return token; + }, + plain => { + scanned += plain.length; + return plain; + } + ); + const tokenAt = position => { + let low = 0; + let high = tokens.length / 2; + while (low < high) { + const mid = (low + high) >>> 1; + if (tokens[mid * 2] <= position) low = mid + 1; + else high = mid; + } + return (low - 1) * 2; + }; + const escapeStarts = []; + const escapeOffsets = []; + const escapes = { '"': '"', "\\": "\\", "/": "/", b: "\b", f: "\f", n: "\n", r: "\r", t: "\t" }; + let offset = 0; + const unescaped = text.replace(/\\(["\\/bfnrt]|u[0-9a-fA-F]{4})/g, (match, escape, index) => { + escapeStarts.push(index - offset); + offset += match.length - 1; + escapeOffsets.push(offset); + return escape[0] === "u" ? String.fromCharCode(parseInt(escape.slice(1), 16)) : escapes[escape]; + }); + const sourceOffset = position => { + let low = 0; + let high = escapeStarts.length; + while (low < high) { + const mid = (low + high) >>> 1; + if (escapeStarts[mid] < position) low = mid + 1; + else high = mid; + } + return position + (low ? escapeOffsets[low - 1] : 0); + }; + const toSource = match => ({ start: sourceOffset(match.start), end: sourceOffset(match.end), value: match.value }); + const rawMatches = maskedValueMatches( + text, + maskedValues, + match => { + const token = tokenAt(match.start); + return token < 0 || match.end > tokens[token + 1]; + }, + true + ); + const decodedMatches = maskedValueMatches( + unescaped, + maskedValues, + match => { + const source = toSource(match); + const token = tokenAt(source.start); + return token < 0 || source.end > tokens[token + 1] || (source.start > tokens[token] && source.end < tokens[token + 1]); + }, + true + ); + function* sourceMatches() { + let raw = rawMatches.next(); + let decoded = decodedMatches.next(); + let decodedSource = decoded.done ? undefined : toSource(decoded.value); + while (!raw.done || !decoded.done) { + const useRaw = !decodedSource || (!raw.done && (raw.value.start < decodedSource.start || (raw.value.start === decodedSource.start && raw.value.end >= decodedSource.end))); + const match = useRaw && !raw.done ? raw.value : decodedSource; + if (!match) throw new Error("Unable to select log spans during secret redaction"); + const token = tokenAt(match.start); + const insideToken = token >= 0 && match.start > tokens[token] && match.end < tokens[token + 1]; + const overlapsToken = (token >= 0 && match.start < tokens[token + 1]) || (token + 2 < tokens.length && tokens[token + 2] < match.end); + yield { ...match, ambiguous: !insideToken && (overlapsToken || text.slice(match.start, match.end) !== match.value) }; + if (useRaw) raw = rawMatches.next(); + else { + decoded = decodedMatches.next(); + decodedSource = decoded.done ? undefined : toSource(decoded.value); + } + } + } + const parts = []; + let cursor = 0; + let ambiguous = false; + let repeated = ""; + let repetitions = 0; + for (const match of overlappingMatchGroups(sourceMatches())) { + const token = tokenAt(match.start); + const insideToken = token >= 0 && match.start > tokens[token] && match.end < tokens[token + 1]; + ambiguous ||= match.ambiguous; + const marker = replacementForValue(match.value); + const replacement = insideToken ? JSON.stringify(marker).slice(1, -1) : marker; + if (match.start > cursor || replacement !== repeated) { + if (repetitions) parts.push(repeated.repeat(repetitions)); + repetitions = 0; + if (match.start > cursor) parts.push(text.slice(cursor, match.start)); + } + repeated = replacement; + repetitions++; + cursor = match.end; + } + if (repetitions) parts.push(repeated.repeat(repetitions)); + parts.push(text.slice(cursor)); + const redacted = transformJSONStrings(parts.join(""), encoded => { + const value = decodeJSONString(encoded); + const withoutCommands = applyAddMaskRedaction(value, []); + return withoutCommands === value ? encoded : JSON.stringify(withoutCommands); + }); + // A registered raw whole-line value removes all of the original syntax. + return ambiguous && !maskedValues.includes(text) ? JSON.stringify(redacted) : redacted; +} + /** * Sanitize artifact sources while runtime masks are still available in memory. * Decode JSON strings first so escaping cannot hide a registered value, redact @@ -203,16 +446,24 @@ function transformJSONStrings(text, transformString, transformText = value => va * @param {string} content * @param {string[]} maskedValues * @param {(value: any) => any} [redactJSONValue] - Structured protocol redactor for valid JSON records + * @param {(value: string) => string} [replacementForValue] - Marker for each matched registered value * @returns {string} */ -function redactArtifactMaskedValues(content, maskedValues, redactJSONValue) { +function redactArtifactMaskedValues(content, maskedValues, redactJSONValue, replacementForValue) { if (!content || (!maskedValues.length && !redactJSONValue)) return content; + const preserveLexemes = !!replacementForValue && !redactJSONValue; + const replacement = replacementForValue || (() => MASK_REPLACEMENT); const redactValue = value => { - if (typeof value === "string") return applyAddMaskRedaction(value, maskedValues); + if (typeof value === "string") return applyAddMaskRedaction(value, maskedValues, replacementForValue); if (Array.isArray(value)) return value.map(redactValue); - if (value && typeof value === "object") return Object.fromEntries(Object.entries(value).map(([key, nested]) => [redactMaskedValues(key, maskedValues), redactValue(nested)])); + if (value && typeof value === "object") return Object.fromEntries(Object.entries(value).map(([key, nested]) => [redactMaskedValues(key, maskedValues, replacementForValue), redactValue(nested)])); return value; }; + const redactString = (encoded, isKey = false) => { + const value = decodeJSONString(encoded); + const redacted = isKey ? redactMaskedValues(value, maskedValues, replacementForValue) : redactValue(value); + return redacted === value ? encoded : JSON.stringify(redacted); + }; const redactJson = text => { let value; try { @@ -227,7 +478,7 @@ function redactArtifactMaskedValues(content, maskedValues, redactJSONValue) { token => token, plain => plain.replace(/-?\d+(?:\.\d+)?(?:[eE][+-]?\d+)?/g, token => { - const redacted = redactMaskedValues(token, maskedValues); + const redacted = redactMaskedValues(token, maskedValues, replacementForValue); return redacted === token ? token : JSON.stringify(redacted); }) ); @@ -238,6 +489,22 @@ function redactArtifactMaskedValues(content, maskedValues, redactJSONValue) { throw new Error("Unable to decode redacted JSON log record during secret redaction", { cause: error }); } } + if (preserveLexemes) { + let cursor = 0; + return transformJSONStrings( + numericRedacted, + token => { + cursor += token.length; + let next = cursor; + while (next < numericRedacted.length && " \t\r\n".includes(numericRedacted[next])) next++; + return redactString(token, numericRedacted[next] === ":"); + }, + plain => { + cursor += plain.length; + return plain; + } + ); + } const redacted = JSON.stringify((redactJSONValue || redactValue)(value)); return redacted === JSON.stringify(value) ? numericRedacted : text.match(/^\s*/)[0] + redacted + text.match(/\s*$/)[0]; }; @@ -249,28 +516,11 @@ function redactArtifactMaskedValues(content, maskedValues, redactJSONValue) { .map(line => { const jsonLine = redactJson(line); if (jsonLine !== undefined) return jsonLine; + if (preserveLexemes) return redactMixedMaskedValues(line, maskedValues, replacement); // Mixed or malformed logs can still contain recoverable JSON strings. - const decoded = transformJSONStrings(line, encoded => { - let value; - try { - value = JSON.parse(encoded); - } catch (error) { - throw new Error("Unable to decode JSON log string during secret redaction", { cause: error }); - } - const redacted = redactValue(value); - return redacted === value ? encoded : JSON.stringify(redacted); - }); - const redacted = redactMaskedValues(decoded, maskedValues); - // Malformed quotes have no unique pairing. Decode remaining escapes once - // conservatively so an alternate pairing cannot hide an encoded mask. - const escapes = { '"': '"', "\\": "\\", "/": "/", b: "\b", f: "\f", n: "\n", r: "\r", t: "\t" }; - const unescaped = redacted.replace(/\\(["\\/bfnrt]|u[0-9a-fA-F]{4})/g, (_, escape) => { - if (escape[0] === "u") return String.fromCharCode(parseInt(escape.slice(1), 16)); - return escapes[escape]; - }); - const conservative = applyAddMaskRedaction(unescaped, maskedValues); - // Only re-encode an ambiguous record when the extra decoding finds a mask. - return conservative === unescaped ? redacted : JSON.stringify(conservative); + const decoded = transformJSONStrings(line, redactString); + const redacted = redactMaskedValues(decoded, maskedValues, replacementForValue); + return redactAmbiguousMaskedValues(redacted, maskedValues, replacement); }) .join("\n"); } diff --git a/actions/setup/js/add_mask_redaction.test.cjs b/actions/setup/js/add_mask_redaction.test.cjs index e217e9b5b1e..67249dcd399 100644 --- a/actions/setup/js/add_mask_redaction.test.cjs +++ b/actions/setup/js/add_mask_redaction.test.cjs @@ -45,6 +45,29 @@ describe("add_mask_redaction", () => { expect(strings.mock.calls.map(([token]) => token)).toEqual(['" "']); }); + it.each([1024, 2048])("bounds character inspections after %i malformed escaped quotes", count => { + const prefix = '"' + '\\"'.repeat(count) + "\\q "; + const content = prefix + '"\\u006fpaque" tail'; + const strings = vi.fn(token => token); + const plain = vi.fn(text => text); + const original = String.prototype.charCodeAt; + let inspections = 0; + const spy = vi.spyOn(String.prototype, "charCodeAt").mockImplementation(function (index) { + inspections++; + return original.call(this, index); + }); + let result; + try { + result = transformJSONStrings(content, strings, plain); + } finally { + spy.mockRestore(); + } + expect(inspections).toBeLessThanOrEqual(content.length); + expect(result).toBe(content); + expect(strings.mock.calls.map(([token]) => token)).toEqual(['"\\u006fpaque"']); + expect(plain.mock.calls.map(([text]) => text).join("")).toBe(prefix + " tail"); + }); + it.each(["string", "plain"])("propagates %s callback failures without adding source text", callback => { const error = new Error("synthetic callback failure"); const fail = () => { @@ -154,9 +177,15 @@ describe("add_mask_redaction", () => { ).toEqual([expectedNested, expected]); const mixed = `output: ${encodedValue} end`; expect(redactArtifactMaskedValues(mixed, [mask]), context).toBe(`output: ${JSON.stringify(expected)} end`); + const selected = redactArtifactMaskedValues(mixed, [mask], undefined, () => "***"); + expect(JSON.parse(selected.slice("output: ".length, -" end".length)), context).toBe(expected); expect(redactArtifactMaskedValues(nested, ["ABSENT_RUNTIME_MASK"]), context).toBe(nested); expect(redactArtifactMaskedValues(jsonl, ["ABSENT_RUNTIME_MASK"]), context).toBe(jsonl); expect(redactArtifactMaskedValues(mixed, ["ABSENT_RUNTIME_MASK"]), context).toBe(mixed); + expect( + redactArtifactMaskedValues(mixed, ["ABSENT_RUNTIME_MASK"], undefined, () => "***"), + context + ).toBe(mixed); } }); @@ -165,12 +194,13 @@ describe("add_mask_redaction", () => { const prefixes = ['"bad\\q"', '"bad\\u12gg"', '"bad\\u123"', '"bad\t"', '"bad\u0000"']; for (let iteration = 0; iteration < 300; iteration++) { const prefix = prefixes[random(prefixes.length)]; - const before = "a".repeat(random(16)); - const after = "z".repeat(random(16)); + const before = (random(2) ? String.raw`C:\temp\file.txt \u0041 \\u006fpaque ` : "") + "a".repeat(random(16)); + const after = "z".repeat(random(16)) + (random(2) ? String.raw` D:\new\report.txt \u0042` : ""); const encodedMask = "\\u006f" + encodeString("paque", random).slice(1, -1); const content = `${prefix} "${before}${encodedMask}${after}"`; const context = `seed ${seed}, iteration ${iteration}, input ${JSON.stringify(content)}`; expect(JSON.parse(redactArtifactMaskedValues(content, ["opaque"])), context).toBe(`${prefix} "${before}***${after}"`); + expect(JSON.parse(redactArtifactMaskedValues(content, ["opaque"], undefined, () => "***")), context).toBe(`${prefix} "${before}***${after}"`); expect(redactArtifactMaskedValues(content, ["ABSENT_RUNTIME_MASK"]), context).toBe(content); } }); @@ -242,6 +272,13 @@ describe("add_mask_redaction", () => { }); describe("redactMaskedValues", () => { + it("preserves per-value replacement markers across adjacent and overlapping matches", () => { + const replacement = value => (value === "configured" ? "***REDACTED***" : "***"); + expect(redactMaskedValues("configuredruntimeconfiguredconfigured runtime", ["configured", "runtime", "figure"], replacement)).toBe("***REDACTED***" + "***" + "***REDACTED***".repeat(2) + " ***"); + expect(redactMaskedValues("ababa", ["aba"])).toBe("***ba"); + expect(redactMaskedValues("ababa", ["aba"], replacement)).toBe("***"); + }); + it("replaces every occurrence with ***", () => { expect(redactMaskedValues("token=abc and abc again", ["abc"])).toBe("token=*** and *** again"); }); @@ -320,6 +357,94 @@ describe("add_mask_redaction", () => { }); describe("redactArtifactMaskedValues", () => { + it.each([ + ['{"configured":["configured",12345678],"live":"runtime","safe":42}', '{"***REDACTED***":["***REDACTED***","***REDACTED***"],"live":"***","safe":42}'], + ['plain configured runtime "\\u0063onfigured" "\\u0072untime"', 'plain ***REDACTED*** *** "***REDACTED***" "***"'], + ['"bad\t" "\\u0063onfigured \\u0072untime" C:\\temp\\file.txt', JSON.stringify('"bad\t" "***REDACTED*** ***" C:\\temp\\file.txt')], + ])("supports per-value replacement markers throughout encoding-aware redaction (%j)", (content, expected) => { + const replacement = value => (value === "runtime" ? "***" : "***REDACTED***"); + expect(redactArtifactMaskedValues(content, ["configured", "12345678", "runtime"], undefined, replacement)).toBe(expected); + }); + + it("preserves original JSON whitespace, safe escapes, and numeric lexemes with explicit replacement markers", () => { + const content = ' \t{ "configured" : "\\u0063onfigured", "safe" : "\\u0041", "values" : [12345678,-0,1e+09,9007199254740993,1e400] }\r\n'; + const expected = ' \t{ "***REDACTED***" : "***REDACTED***", "safe" : "\\u0041", "values" : ["***REDACTED***",-0,1e+09,9007199254740993,1e400] }\r\n'; + expect(redactArtifactMaskedValues(content, ["configured", "12345678"], undefined, () => "***REDACTED***")).toBe(expected); + }); + + it.each([ + ["synthetic\\n", "synthetic\npassword", JSON.stringify("synthetic\npassword")], + ["synthetic\\u006e", "syntheticnpassword", '"synthetic\\u006epassword"'], + ['"password', '"passwordtail', JSON.stringify('"passwordtail')], + ])("resolves longer encoded masks before raw configured prefixes (%j)", (configured, runtime, encoded) => { + const masks = [configured, runtime].sort((a, b) => b.length - a.length); + const replacement = value => (value === configured ? "***REDACTED***" : "***"); + expect(redactArtifactMaskedValues("output: " + encoded, masks, undefined, replacement)).toBe('output: "***"'); + expect(JSON.parse(redactArtifactMaskedValues('"bad\t" ' + encoded, masks, undefined, replacement))).toBe('"bad\t" "***"'); + }); + + it("resolves raw configured masks spanning recoverable token boundaries before shorter decoded masks", () => { + const configured = 'synthetic"\\\tpassword'; + const content = '"bad\t" "\\u006fpaque" token=' + configured; + const replacement = value => (value === configured ? "***REDACTED***" : "***"); + expect(JSON.parse(redactArtifactMaskedValues(content, [configured, "synthetic", "opaque"], undefined, replacement))).toBe('"bad\t" "***" token=***REDACTED***'); + }); + + it.each(["***", "***REDACTED***"])("keeps a fully masked raw shell command as a plain marker (%s)", marker => { + const content = 'export PAYLOAD_DIR="/synthetic/session/payload"'; + expect(redactArtifactMaskedValues(content, [content], undefined, () => marker)).toBe(marker); + expect(redactArtifactMaskedValues(content + "\n", [content], undefined, () => marker)).toBe(marker + "\n"); + }); + + it("coalesces raw and encoded matches that overlap at different source positions", () => { + const content = String.raw`\u0061bc\nf`; + expect(JSON.parse(redactArtifactMaskedValues(content, [String.raw`u0061bc\nf`, String.raw`c\nf`, "ab"], undefined, () => "***"))).toBe("***"); + }); + + it.each([ + ["long-configured-secret", "ablong", "ablong-configured-secret"], + ["12345678", "9123", "912345678"], + ])("coalesces shorter earlier runtime overlaps across text, JSON strings, keys, and numeric lexemes (%j)", (configured, runtime, content) => { + const replacement = value => (value === configured ? "***REDACTED***" : "***"); + const masks = [configured, runtime]; + expect(redactMaskedValues(content, masks, replacement)).toBe("***REDACTED***"); + expect(redactArtifactMaskedValues(content, masks, undefined, replacement)).toBe(/^\d+$/.test(content) ? '"***REDACTED***"' : "***REDACTED***"); + expect(redactArtifactMaskedValues(JSON.stringify({ [content]: content }), masks, undefined, replacement)).toBe('{"***REDACTED***":"***REDACTED***"}'); + expect(redactArtifactMaskedValues("output: " + JSON.stringify(content), masks, undefined, replacement)).toBe('output: "***REDACTED***"'); + }); + + it("keeps literal Unicode escapes distinct with explicit replacement markers", () => { + const content = '"bad\t" ' + JSON.stringify("\\u006fpaque"); + expect(redactArtifactMaskedValues(content, ["opaque"], undefined, () => "***")).toBe(content); + }); + + it("fails closed if decoding an original validated token unexpectedly fails with explicit replacement markers", () => { + const encoded = '"\\u006fpaque"'; + const failure = new Error("synthetic decoding failure"); + const original = JSON.parse; + const spy = vi.spyOn(JSON, "parse").mockImplementation(value => { + if (value === encoded) throw failure; + return original(value); + }); + try { + expect(() => redactArtifactMaskedValues("output: " + encoded, ["opaque"], undefined, () => "***")).toThrow("Unable to decode JSON log string during secret redaction"); + } finally { + spy.mockRestore(); + } + }); + + it("keeps the structured JSON visitor authoritative and propagates its failures", () => { + const visitor = vi.fn(value => ({ wrapped: value })); + expect(redactArtifactMaskedValues('{"safe":42}', [], visitor, () => "***REDACTED***")).toBe('{"wrapped":{"safe":42}}'); + expect(visitor).toHaveBeenCalledExactlyOnceWith({ safe: 42 }); + const error = new Error("synthetic visitor failure"); + expect(() => + redactArtifactMaskedValues('{"safe":42}', [], () => { + throw error; + }) + ).toThrow(error); + }); + describe("JSON string scanner boundaries", () => { const encodedMask = '"\\u006fpaque"'; const validStrings = [ @@ -394,6 +519,20 @@ describe("add_mask_redaction", () => { expect(JSON.parse(redactArtifactMaskedValues(content, ['foo"bar']))).toBe('"bad\t" "***"'); }); + it.each(["opaque", 'foo"bar', "a\tb", "a\\b", "\ud83d\ude80"])("only rewrites source spans participating in conservative matches (%j)", mask => { + const encodedMask = Array.from({ length: mask.length }, (_, index) => `\\u${mask.charCodeAt(index).toString(16).padStart(4, "0")}`).join(""); + const prefix = String.raw`before C:\temp\file.txt \\server\folder \n \t \b \f \r \" \/ \u0041 "bad` + '\t" "'; + const suffix = String.raw`" after D:\new\report.txt \\server\share \u0042 \\u006fpaque`; + const content = prefix + encodedMask + suffix; + expect(JSON.parse(redactArtifactMaskedValues(content, [mask]))).toBe(prefix + "***" + suffix); + expect(redactArtifactMaskedValues(content, ["absent"])).toBe(content); + }); + + it("preserves unmatched escapes between multiple conservative matches and overlapping candidates", () => { + const content = '"bad\t" ' + String.raw`"\u006fpaque" C:\temp\file.txt "\u006fpaque-tail" \\u006fpaque`; + expect(JSON.parse(redactArtifactMaskedValues(content, ["opaque-tail", "opaque"]))).toBe('"bad\t" ' + String.raw`"***" C:\temp\file.txt "***" \\u006fpaque`); + }); + it("does not recursively decode literal Unicode escapes after malformed prefixes", () => { const content = '"bad\t" ' + JSON.stringify("\\u006fpaque"); expect(redactArtifactMaskedValues(content, ["opaque"])).toBe(content); @@ -472,6 +611,15 @@ describe("add_mask_redaction", () => { expect(parsed.safe).toBe("12345678"); }); + it.each([false, true])("redacts a multi-megabyte malformed escaped-quote record without rescanning its rejected prefix (explicit selector %j)", selector => { + const prefix = '"' + '\\"'.repeat(1024 * 1024); + const suffix = String.raw`\q "safe" C:\temp\file.txt`; + const content = prefix + String.raw`\u006fpaque` + suffix; + const replacement = selector ? () => "***" : undefined; + expect(JSON.parse(redactArtifactMaskedValues(content, ["opaque"], undefined, replacement))).toBe(prefix + "***" + suffix); + expect(redactArtifactMaskedValues(content, ["absent"], undefined, replacement)).toBe(content); + }); + it.each(["\n", "\r", "\u2028", "\u2029"])("recovers encoded masks after an invalid escaped line terminator", terminator => { const mask = 'synthetic-"quoted"\\mask'; const prefix = '"invalid\\' + terminator; diff --git a/actions/setup/js/redact_secrets.cjs b/actions/setup/js/redact_secrets.cjs index 1a61417dd99..0c2c60a115d 100644 --- a/actions/setup/js/redact_secrets.cjs +++ b/actions/setup/js/redact_secrets.cjs @@ -10,7 +10,7 @@ const fs = require("fs"); const path = require("path"); const { getErrorMessage } = require("./error_helpers.cjs"); const { ERR_VALIDATION } = require("./error_codes.cjs"); -const { applyAddMaskRedaction, collectAddMaskedValues, redactArtifactMaskedValues } = require("./add_mask_redaction.cjs"); +const { applyAddMaskRedaction, collectAddMaskedValues, redactArtifactMaskedValues, redactMaskedValues } = require("./add_mask_redaction.cjs"); const { redactSafeOutputContent } = require("./safe_output_redaction.cjs"); const { redactPiSessionHTML } = require("./pi_session_redaction.cjs"); const { getOTLPSecretValues } = require("./otlp_header_helpers.cjs"); @@ -304,6 +304,55 @@ function redactSecrets(content, secretValues) { return { content: redacted, redactionCount }; } +/** + * Match all credential sources before any replacement or JSON serialization can + * fragment a longer secret or change its spelling. + * @param {string} content + * @param {string[]} secretValues + * @param {string[]} maskedValues + * @returns {{content: string, redactionCount: number}} + */ +function redactArtifactContent(content, secretValues, maskedValues) { + const configured = new Set(secretValues.filter(value => value && value.length >= 6)); + /** @type {Map} */ + const builtInValues = new Map(); + // Discovery only: include escaped built-in credentials without rewriting the + // original numeric lexemes, malformed records, or unchanged JSON strings. + const escapes = { '"': '"', "\\": "\\", "/": "/", b: "\b", f: "\f", n: "\n", r: "\r", t: "\t" }; + const decoded = content.replace(/\\(["\\/bfnrt]|u[0-9a-fA-F]{4})/g, (_, escape) => (escape[0] === "u" ? String.fromCharCode(parseInt(escape.slice(1), 16)) : escapes[escape])); + for (const text of new Set([content, decoded])) { + for (const { name, pattern } of BUILT_IN_PATTERNS) { + for (const match of text.matchAll(pattern)) builtInValues.set(match[0], name); + } + } + const values = [...new Set([...configured, ...builtInValues.keys(), ...maskedValues])].sort((a, b) => b.length - a.length); + let redactionCount = 0; + const builtInCounts = new Map(); + let configuredCount = 0; + const replacement = value => { + redactionCount++; + const name = builtInValues.get(value); + if (name) builtInCounts.set(name, (builtInCounts.get(name) || 0) + 1); + else if (configured.has(value)) configuredCount++; + return name || configured.has(value) ? "***REDACTED***" : "***"; + }; + // Raw multiline values must be removed before the mixed-record parser splits + // lines; never pre-rewrite valid JSON syntax or its original numeric lexemes. + const multilineValues = values.filter(value => /[\r\n]/.test(value)); + let prepared = content; + if (multilineValues.some(value => content.includes(value))) { + try { + JSON.parse(content); + } catch { + prepared = redactMaskedValues(content, multilineValues, replacement); + } + } + const redacted = redactArtifactMaskedValues(prepared, values, undefined, replacement); + for (const [name, count] of builtInCounts) global.core?.info?.(`Redacted ${count} occurrence(s) of ${name}`); + if (configuredCount) global.core?.info?.(`Redacted ${configuredCount} occurrence(s) of a secret`); + return { content: redacted, redactionCount: redacted === content ? 0 : Math.max(1, redactionCount) }; +} + /** * Validate and read the host-only AWF early configuration artifact. The exact * path contract and metadata checks prevent an environment variable from @@ -518,25 +567,10 @@ function processFile(filePath, secretValues, maskedValues = [], prepareForCustom if (redactionCount) core.info(`Processed ${filePath}: ${redactionCount} redaction(s)`); return redactionCount; } - const encodedResult = - path.extname(filePath).toLowerCase() === ".html" - ? redactPiSessionHTML(content, text => { - const masked = redactArtifactMaskedValues(text, maskedValues); - const builtIn = redactBuiltInPatterns(masked); - const custom = redactSecrets(builtIn.content, secretValues); - return { content: custom.content, redactionCount: builtIn.redactionCount + custom.redactionCount + (masked !== text ? 1 : 0) }; - }) - : { content, redactionCount: 0 }; - const runtimeRedacted = redactArtifactMaskedValues(encodedResult.content, maskedValues); - // First, redact built-in patterns - const builtInResult = redactBuiltInPatterns(runtimeRedacted); - let redacted = builtInResult.content; - let totalRedactions = encodedResult.redactionCount + builtInResult.redactionCount + (runtimeRedacted !== encodedResult.content ? 1 : 0); - - // Then, redact custom secrets - const customResult = redactSecrets(redacted, secretValues); - redacted = customResult.content; - totalRedactions += customResult.redactionCount; + const encodedResult = path.extname(filePath).toLowerCase() === ".html" ? redactPiSessionHTML(content, text => redactArtifactContent(text, secretValues, maskedValues)) : { content, redactionCount: 0 }; + const genericResult = redactArtifactContent(encodedResult.content, secretValues, maskedValues); + const redacted = genericResult.content; + const totalRedactions = encodedResult.redactionCount + genericResult.redactionCount; if (totalRedactions > 0 || prepareForCustomMasking) { stage = "write"; diff --git a/actions/setup/js/redact_secrets.test.cjs b/actions/setup/js/redact_secrets.test.cjs index 947927a2165..c1ecf98420f 100644 --- a/actions/setup/js/redact_secrets.test.cjs +++ b/actions/setup/js/redact_secrets.test.cjs @@ -127,6 +127,172 @@ describe("redact_secrets.cjs", () => { } }), describe("file handling", () => { + it.each(['export SYNTHETIC_DIR="/example/synthetic"', 'mkdir -p "/example/synthetic"'])("keeps complete-line quoted shell masks as plain redaction markers (%s)", line => { + const { processFile } = require("./redact_secrets.cjs"); + const file = path.join(tempDir, "shell-mask.log"); + for (const configured of [false, true]) { + fs.writeFileSync(file, line + "\n"); + + expect(processFile(file, configured ? [line] : [], configured ? [] : [line])).toBeGreaterThan(0); + + expect(fs.readFileSync(file, "utf8")).toBe((configured ? "***REDACTED***" : "***") + "\n"); + expect(mockCore.setFailed).not.toHaveBeenCalled(); + } + }); + it.each(['synthetic"password', "synthetic\\password", "synthetic\\npassword", "synthetic\tpassword", "synthetic\npassword", 'synthetic"\\\tpassword'])( + "redacts configured secrets before runtime masking can serialize malformed records (%j)", + secret => { + const { processFile } = require("./redact_secrets.cjs"); + const file = path.join(tempDir, "combined.log"); + fs.writeFileSync(file, '"bad\t" "\\u006fpaque" token=' + secret); + + expect(processFile(file, [secret], ["opaque", "synthetic"])).toBeGreaterThan(0); + + const output = fs.readFileSync(file, "utf8"); + const decoded = output + .split("\n") + .map(line => { + try { + return JSON.parse(line); + } catch { + return line; + } + }) + .join("\n"); + expect(decoded).not.toContain(secret); + expect(decoded).not.toContain("password"); + expect(decoded).not.toContain("opaque"); + expect(mockCore.setFailed).not.toHaveBeenCalled(); + expect(mockCore.info.mock.calls.flat().join("\n")).not.toContain(secret); + } + ); + + it.each(['synthetic"password', "synthetic\\password", "synthetic\tpassword", "synthetic\npassword"])("redacts decoded configured secrets alongside overlapping runtime masks in JSON and mixed records (%j)", secret => { + const { processFile } = require("./redact_secrets.cjs"); + const record = JSON.stringify({ [secret]: secret, safe: "public" }); + const encoded = record.replace(/synthetic/g, "\\u0073ynthetic"); + for (const [name, input] of [ + ["valid.json", encoded], + ["mixed.log", "output: " + encoded], + ]) { + const file = path.join(tempDir, name); + fs.writeFileSync(file, input); + + expect(processFile(file, [secret], ["synthetic"])).toBeGreaterThan(0); + + const output = fs.readFileSync(file, "utf8"); + const decoded = JSON.parse(name === "valid.json" ? output : output.slice("output: ".length)); + expect(Object.keys(decoded)).toEqual(["***REDACTED***", "safe"]); + expect(Object.values(decoded)).toEqual(["***REDACTED***", "public"]); + expect(mockCore.setFailed).not.toHaveBeenCalled(); + } + }); + + it.each([false, true])("removes full built-in and configured credentials before overlapping runtime masks (encoded=%s)", encoded => { + const { processFile } = require("./redact_secrets.cjs"); + const token = "ghs_" + "A".repeat(40); + const secret = "prefix-" + token + "-configured"; + const file = path.join(tempDir, "overlapping.json"); + const record = JSON.stringify({ token, secret, safe: "public" }); + fs.writeFileSync(file, encoded ? record.replace(/ghs_/g, "\\u0067hs_") : record); + + expect(processFile(file, [secret], ["ghs_", "prefix"])).toBeGreaterThan(0); + + expect(JSON.parse(fs.readFileSync(file, "utf8"))).toEqual({ token: "***REDACTED***", secret: "***REDACTED***", safe: "public" }); + expect(mockCore.setFailed).not.toHaveBeenCalled(); + }); + + it.each([ + ["configured", "long-configured-secret", "ablong", "ab", "-configured-secret"], + ["built-in", "ghs_" + "A".repeat(40), "beforeghs_", "before", "A".repeat(40)], + ["numeric", "12345678", "9123", "9", "45678"], + ["multiline", "long\nconfigured-secret", "ablong\n", "ab", "configured-secret"], + ])("removes full %s credentials when a shorter runtime mask starts before their overlapping prefix", (kind, secret, runtime, prefix, suffix) => { + const { processFile } = require("./redact_secrets.cjs"); + const file = path.join(tempDir, "crossing-overlap.log"); + const text = prefix + secret; + for (const content of [text, JSON.stringify({ [text]: text }), "output: " + JSON.stringify(text)]) { + fs.writeFileSync(file, content); + + expect(processFile(file, kind === "built-in" ? [] : [secret], [runtime])).toBeGreaterThan(0); + + const output = fs.readFileSync(file, "utf8"); + expect(output).not.toContain(suffix); + expect(output).toContain("***"); + expect(mockCore.setFailed).not.toHaveBeenCalled(); + } + }); + + it.each([ + ["synthetic\\n", "synthetic\npassword", false], + ["synthetic\\u006e", "syntheticnpassword", true], + ['"password', '"passwordtail', false], + ])("does not fragment longer encoded runtime masks with raw configured escape prefixes (%j)", (secret, runtime, unicode) => { + const { processFile } = require("./redact_secrets.cjs"); + const file = path.join(tempDir, "encoded-overlap.log"); + const encoded = JSON.stringify(runtime); + for (const prefix of ["output: ", '"bad\t" ']) { + fs.writeFileSync(file, prefix + (unicode ? encoded.replace("syntheticn", "synthetic\\u006e") : encoded)); + + expect(processFile(file, [secret], [runtime])).toBeGreaterThan(0); + + const output = fs.readFileSync(file, "utf8"); + let decoded = output; + try { + decoded = JSON.parse(output); + } catch { + // A mixed record can remain text instead of a quoted JSON string. + } + expect(decoded).toBe(prefix + '"***"'); + expect(mockCore.setFailed).not.toHaveBeenCalled(); + } + }); + + it("redacts original configured numeric lexemes before a shorter runtime mask", () => { + const { processFile } = require("./redact_secrets.cjs"); + const file = path.join(tempDir, "configured-numeric.json"); + fs.writeFileSync(file, '{"ids":[9007199254740993,12345678,1.2345678e+8],"safe":42}'); + + expect(processFile(file, ["9007199254740993", "12345678", "1.2345678e+8"], ["123", "900"])).toBeGreaterThan(0); + + expect(JSON.parse(fs.readFileSync(file, "utf8"))).toEqual({ ids: ["***REDACTED***", "***REDACTED***", "***REDACTED***"], safe: 42 }); + expect(mockCore.setFailed).not.toHaveBeenCalled(); + }); + + it("preserves generic JSON formatting and untouched numeric lexemes when configured values change", () => { + const { processFile } = require("./redact_secrets.cjs"); + const file = path.join(tempDir, "formatted.json"); + fs.writeFileSync(file, '{\n "text": "synthetic-password",\n "ids": [9007199254740993, 1e2],\n "escaped": "\\u0070ublic"\n}\n'); + + expect(processFile(file, ["synthetic-password"])).toBeGreaterThan(0); + + expect(fs.readFileSync(file, "utf8")).toBe('{\n "text": "***REDACTED***",\n "ids": [9007199254740993, 1e2],\n "escaped": "\\u0070ublic"\n}\n'); + }); + + it("redacts decoded HTML session payloads and surrounding mixed text with one combined mask set", () => { + const { processFile } = require("./redact_secrets.cjs"); + const secret = 'synthetic"\\\tpassword'; + const malformed = '"bad\t" "\\u006fpaque" token=' + secret; + const payload = Buffer.from(JSON.stringify({ text: malformed, [secret]: secret }), "utf8").toString("base64"); + const file = path.join(tempDir, "session.html"); + fs.writeFileSync(file, `
${malformed}
`); + + expect(processFile(file, [secret], ["opaque", "synthetic"])).toBeGreaterThan(0); + + const raw = fs.readFileSync(file, "utf8"); + const output = raw.startsWith('"') ? JSON.parse(raw) : raw; + const session = JSON.parse(Buffer.from(output.match(/