fix(logging): redactSecrets regex requires non-alphanumeric prefix to avoid false positives

The original `/sk-(?:ant-)?[A-Za-z0-9_-]+/g` redacted any word ending
in `sk-…`, including common English compounds in error messages and
PR descriptions like `task-force`, `risk-management`, `disk-io`.
Server logs ended up reading "ta[REDACTED] ABC" / "ri[REDACTED]
note" / "di[REDACTED] 1234", which is unhelpful even though
over-redaction is the safer-than-leaking failure mode.

Tightens the regex to require either the start of the string OR a
non-alphanumeric prefix character (whitespace, `=`, `{`, `,`, `:`,
etc.) so the `sk-` token must actually start a secret, mirroring the
v1.4.6 P12 idempotency-cache guard. Adds a minimum tail length of 8
characters to defend against `sk-1`-style false-positives in code
identifiers.

Capture group 1 is the leading separator (preserved in the
substitution) so the redaction reads "apiKey=[REDACTED]" rather than
"apiKey[REDACTED]".

Adds tests covering the three v1.4.6-QA false-positive examples
(task-force, risk-management, disk-io) plus mid-word negatives, and
extends the positive coverage to confirm `sk-1234567890abcdef` and
`sk-ant-1234567890abcdef` still redact correctly with various
preceding separators.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Marc-André BombeckandClaude Opus 4.7 committed 2026-05-09 16:05:53 +02:00
1 parent 54038216e4
commit d6696cf02b
2 files changed
+45 -1

No files matched your search

+33
View File
@@ -52,6 +52,39 @@ describe("redactSecrets", () => {
);
});
// v1.4.6 P11 hardening: the original `/sk-(?:ant-)?[A-Za-z0-9_-]+/`
// regex over-matched any word ending in `sk-…`, redacting common
// English compounds in error messages and PR descriptions. v1.5
// tightens the regex to require a non-alphanumeric prefix so the
// `sk-` token must actually start a secret.
it("does NOT redact common English compounds containing 'sk-'", () => {
expect(redactSecrets("task-force ABC")).toBe("task-force ABC");
expect(redactSecrets("risk-management note")).toBe("risk-management note");
expect(redactSecrets("disk-io 1234")).toBe("disk-io 1234");
// Mid-word matches must also be ignored (the `sk-` is preceded by a
// letter / digit, so it's part of an identifier, not a secret).
expect(redactSecrets("workflow:risk-management")).toBe(
"workflow:risk-management",
);
expect(redactSecrets("e2e_disk-io_check")).toBe("e2e_disk-io_check");
});
it("still redacts genuine sk- and sk-ant- API keys", () => {
// Start of string.
expect(redactSecrets("sk-1234567890abcdef")).toBe("[REDACTED]");
expect(redactSecrets("sk-ant-1234567890abcdef")).toBe("[REDACTED]");
// Preceded by whitespace, equals, brace, comma, etc. — the
// non-alphanumeric-prefix requirement.
expect(redactSecrets("apiKey=sk-1234567890abcdef end")).toBe(
"apiKey=[REDACTED] end",
);
expect(redactSecrets("OPENAI_API_KEY=sk-prod-abcDEF123456789012")).toBe(
"OPENAI_API_KEY=[REDACTED]",
);
// Trailing context preserved, leading separator preserved.
expect(redactSecrets(",sk-1234567890abcdef,")).toBe(",[REDACTED],");
});
it("handles multiple secrets in one string", () => {
// Bearer matches `\S+` so it greedily consumes `hlk_x;` — that's
// safe (over-redaction is fine when the alternative is leaking).
+12 -1
View File
@@ -26,7 +26,18 @@ export function redactSecrets(input: string): string {
return input
.replace(/Bearer\s+\S+/gi, "Bearer [REDACTED]")
.replace(/bot\d+:[A-Za-z0-9_-]+/g, "bot[REDACTED]")
.replace(/sk-(?:ant-)?[A-Za-z0-9_-]+/g, "[REDACTED]")
// `sk-` is a common English-prefix substring (task-force, risk-
// management, disk-io). Without a word-boundary, the original
// `/sk-(?:ant-)?[A-Za-z0-9_-]+/g` over-matched and made server logs
// unreadable. Require the `sk-` token to be at the start of the
// string OR preceded by a non-alphanumeric character (whitespace,
// `=`, `{`, `,`, `:`, etc.). The minimum length-8 tail keeps a
// tiny safety margin against `sk-1`-style false-positives in code
// identifiers. Capture group 1 = the leading separator (preserved).
.replace(
/(^|[^A-Za-z0-9])sk-(?:ant-)?[A-Za-z0-9_-]{8,}/g,
"$1[REDACTED]",
)
.replace(
/([?&])(secret|code|token|api[_-]?key)=[^&\s]+/gi,
"$1$2=[REDACTED]",