-
Notifications
You must be signed in to change notification settings - Fork 645
fix(sdk): redact standalone keys and structured credentials #322
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
dd61f0d
552c382
e8b9589
bf34227
c2d786b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,17 +3,28 @@ import { formatUsd, type ScanCost } from "./cost.js"; | |
| /** Returns an error message with credential-shaped substrings redacted. */ | ||
| export function redactedErrorMessage(error: unknown): string { | ||
| const message = error instanceof Error ? error.message : String(error); | ||
| const withoutPrivateKeys = message.replaceAll( | ||
| /(\b[A-Za-z0-9_-]{0,64}private[_-]?key(?:[_-][A-Za-z0-9_-]{1,64}|(?:value|data|token|secret|credential|password|header|field|id|key)[A-Za-z0-9_-]{0,48})?\b(?:\\?["'])?\s*[:=]\s*)(?:\\?["'])?-----BEGIN [A-Z0-9 ]*PRIVATE KEY-----[\s\S]*?(?:-----END [A-Z0-9 ]*PRIVATE KEY-----(?:\\?["'])?|$)/giu, | ||
| "$1[redacted]", | ||
| ); | ||
| return redactQuotedCredentialValues(withoutPrivateKeys) | ||
| const withoutPrivateKeys = message | ||
| .replaceAll( | ||
| /(\b[A-Za-z0-9_-]{0,64}private[_-]?key(?:[_-][A-Za-z0-9_-]{1,64}|(?:value|data|token|secret|credential|password|header|field|id|key)[A-Za-z0-9_-]{0,48})?\b(?:\\?["'])?\s*[:=]\s*)(?:\\?["'])?-----BEGIN [A-Z0-9 ]*PRIVATE KEY(?: BLOCK)?-----[\s\S]*?(?:-----END [A-Z0-9 ]*PRIVATE KEY(?: BLOCK)?-----(?:\\?["'])?|$)/giu, | ||
| "$1[redacted]", | ||
| ) | ||
| .replaceAll( | ||
| /-----BEGIN [A-Z0-9 ]*PRIVATE KEY(?: BLOCK)?-----[\s\S]*?(?:-----END [A-Z0-9 ]*PRIVATE KEY(?: BLOCK)?-----|$)/giu, | ||
| "[redacted]", | ||
| ); | ||
| return redactQuotedCredentialValues( | ||
| redactCredentialCollections(withoutPrivateKeys), | ||
| ) | ||
| .replaceAll( | ||
| /(\b[A-Za-z0-9_-]{0,64}(?:authorization|auth)(?:[_-][A-Za-z0-9_-]{1,64}|(?:value|data|token|secret|credential|password|header|field|id|key)[A-Za-z0-9_-]{0,48})?\b(?:\\?["'])?\s*[:=]\s*)([A-Za-z0-9][A-Za-z0-9.+~-]{0,63})((?:\s|%20|\+)+)[!#$%&'*+.^_`|~0-9A-Za-z-]{1,65}\s*=\s*(?:"(?:\\.|[^"\\])*"|'(?:\\.|[^'\\])*'|[^\s,;]+)(?:\s*,\s*[!#$%&'*+.^_`|~0-9A-Za-z-]{1,65}\s*=\s*(?:"(?:\\.|[^"\\])*"|'(?:\\.|[^'\\])*'|[^\s,;]+))*/giu, | ||
| "$1$2$3[redacted]", | ||
| ) | ||
| .replaceAll( | ||
| /(\b[A-Za-z0-9_-]{0,64}(?:authorization|auth)(?:[_-][A-Za-z0-9_-]{1,64}|(?:value|data|token|secret|credential|password|header|field|id|key)[A-Za-z0-9_-]{0,48})?\b(?:\\?["'])?\s*[:=]\s*)([A-Za-z][A-Za-z0-9._~-]{0,63})((?:\s|%20|\+)+)(?!\[redacted\]|(?!key\s*=)[A-Za-z_][A-Za-z0-9_-]{0,64}\s*[:=]\s*(?=[^=\s"',;}&\\\]]))[^\s"',;}&\\\]]+/giu, | ||
| "$1$2$3[redacted]", | ||
| ) | ||
| .replaceAll( | ||
| /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?key|access[_-]?key(?:[_-]?id)?|private[_-]?key|authorization|auth|token|secret|credential|signature|sig|password|passwd)(?:[_-][A-Za-z0-9_-]{1,64}|(?:value|data|token|secret|credential|password|header|field|id|key)[A-Za-z0-9_-]{0,48})?\b(?:\\?["'])?\s*[:=]\s*(?:\\?["'])?)(?!\[redacted\]|[A-Za-z][A-Za-z0-9._~-]{0,63}(?:\s|%20|\+)+\[redacted\])[^\s"',;}&\\\]]+/giu, | ||
| /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?keys?|access[_-]?key(?:[_-]?id)?|private[_-]?key|authorization|auth|token|secret|credentials?|signature|sig|password|passwd)(?:[_-][A-Za-z0-9_-]{1,64}|(?:value|data|token|secret|credential|password|header|field|id|key)[A-Za-z0-9_-]{0,48})?\b(?:\\?["'])?\s*[:=]\s*(?:\\?["'])?)(?!\[redacted\]|[A-Za-z0-9][A-Za-z0-9._~-]{0,63}(?:\s|%20|\+)+\[redacted\])(?:\\(?!["'])|[^\s"',;}&\\\]])+/giu, | ||
| "$1[redacted]", | ||
| ) | ||
| .replaceAll(/sk-(?:proj-)?[A-Za-z0-9_*=-]{8,}/gu, "[redacted]") | ||
|
|
@@ -25,14 +36,53 @@ export function redactedErrorMessage(error: unknown): string { | |
| ) | ||
| .replaceAll(/((?:https?|ssh|git\+ssh):\/\/)[^\s/@]+@/giu, "$1[redacted]@") | ||
| .replaceAll( | ||
| /((?:[?&]|%3F|%26)(?:(?!%3F|%26|%3D)(?:[A-Za-z0-9_.%-]|\[|\])){0,64}(?:api[_-]?key|access(?:[_-]|%5F|%2D)?key(?:(?:[_-]|%5F|%2D)?id)?|private(?:[_-]|%5F|%2D)?key|authorization|auth|token|secret|credential|signature|sig|password|passwd)(?:(?:[_-]|%5F|%2D)[A-Za-z0-9_.%-]{1,64}|(?:value|data|token|secret|credential|password|header|field|id|key)[A-Za-z0-9_.%-]{0,48})?(?:\]|%5D)?(?:=|%3D))(?:(?!%26)[^&\s])+/giu, | ||
| /((?:[?&]|%3F|%26)(?:(?!%3F|%26|%3D)(?:[A-Za-z0-9_.%-]|\[|\])){0,64}(?:api(?:[_-]|%5F|%2D)?keys?|access(?:[_-]|%5F|%2D)?key(?:(?:[_-]|%5F|%2D)?id)?|private(?:[_-]|%5F|%2D)?key|authorization|auth|token|secret|credentials?|signature|sig|password|passwd)(?:(?:[_-]|%5F|%2D)[A-Za-z0-9_.%-]{1,64}|(?:value|data|token|secret|credential|password|header|field|id|key)[A-Za-z0-9_.%-]{0,48})?(?:\]|%5D)?(?:=|%3D))(?:(?!%26)[^&\s])+/giu, | ||
| "$1[redacted]", | ||
| ); | ||
| } | ||
|
|
||
| function redactCredentialCollections(message: string): string { | ||
| const assignment = | ||
| /(\b[A-Za-z0-9_-]{0,64}(?:credentials|api[_-]?keys)(?:[_-][A-Za-z0-9_-]{1,64}|(?:value|data|token|secret|credential|password|header|field|id|key)[A-Za-z0-9_-]{0,48})?\b(?:\\*["'])?\s*[:=]\s*)([\[{])/giu; | ||
| let output = ""; | ||
| let consumed = 0; | ||
| for ( | ||
| let match = assignment.exec(message); | ||
| match !== null; | ||
| match = assignment.exec(message) | ||
| ) { | ||
| const closings = [match[2] === "[" ? "]" : "}"]; | ||
| let quote: string | undefined; | ||
| let position = assignment.lastIndex; | ||
| while (position < message.length && closings.length > 0) { | ||
| const character = message[position]!; | ||
| if (quote !== undefined) { | ||
| if (character === "\\") position += 1; | ||
| else if (character === quote) quote = undefined; | ||
| } else if (character === '"' || character === "'") { | ||
| quote = character; | ||
|
Comment on lines
+59
to
+63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When an error contains a JSON-stringified object with a credential collection, the quotes inside the encoded collection are backslash-escaped, but this scanner treats the first escaped quote as an opening structural quote. It therefore never recognizes the closing bracket and discards the remainder of the message; for example, Useful? React with 👍 / 👎. |
||
| } else if (character === "[") { | ||
| closings.push("]"); | ||
| } else if (character === "{") { | ||
| closings.push("}"); | ||
| } else if (character === closings.at(-1)) { | ||
| closings.pop(); | ||
| } | ||
| position += 1; | ||
| } | ||
|
|
||
| const prefix = match[1]!; | ||
| const replacement = /:\s*$/u.test(prefix) ? '"[redacted]"' : "[redacted]"; | ||
| output += `${message.slice(consumed, match.index)}${prefix}${replacement}`; | ||
| consumed = closings.length === 0 ? position : message.length; | ||
| assignment.lastIndex = consumed; | ||
| } | ||
| return output + message.slice(consumed); | ||
| } | ||
|
|
||
| function redactQuotedCredentialValues(message: string): string { | ||
| const assignment = | ||
| /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?key|access[_-]?key(?:[_-]?id)?|private[_-]?key|authorization|auth|token|secret|credential|signature|sig|password|passwd)(?:[_-][A-Za-z0-9_-]{1,64}|(?:value|data|token|secret|credential|password|header|field|id|key)[A-Za-z0-9_-]{0,48})?\b(?:\\*["'])?\s*[:=]\s*)(\\*)(["'])/giu; | ||
| /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?keys?|access[_-]?key(?:[_-]?id)?|private[_-]?key|authorization|auth|token|secret|credentials?|signature|sig|password|passwd)(?:[_-][A-Za-z0-9_-]{1,64}|(?:value|data|token|secret|credential|password|header|field|id|key)[A-Za-z0-9_-]{0,48})?\b(?:\\*["'])?\s*[:=]\s*)(\\*)(["'])/giu; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a newly supported quoted plural field ends in a literal backslash, its valid JSON representation has two backslashes before the closing quote, but the delimiter scanner only accepts a closing quote preceded by exactly Useful? React with 👍 / 👎. |
||
| let output = ""; | ||
| let consumed = 0; | ||
| for ( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| import { describe, expect, test } from "bun:test"; | ||
| import { redactedErrorMessage } from "../src/errors.js"; | ||
|
|
||
| describe("security error redaction", () => { | ||
| test("redacts standalone and truncated PEM private-key envelopes", () => { | ||
| expect( | ||
| redactedErrorMessage( | ||
| "provider failed: -----BEGIN PRIVATE KEY-----\nSYNTHETIC_PRIVATE_KEY\n-----END PRIVATE KEY----- safe=value", | ||
| ), | ||
| ).toBe("provider failed: [redacted] safe=value"); | ||
| expect( | ||
| redactedErrorMessage( | ||
| "provider failed: -----BEGIN RSA PRIVATE KEY-----\nSYNTHETIC_TRUNCATED_KEY", | ||
| ), | ||
| ).toBe("provider failed: [redacted]"); | ||
| expect( | ||
| redactedErrorMessage( | ||
| "provider failed: -----BEGIN PGP PRIVATE KEY BLOCK-----\nSYNTHETIC_PGP_PRIVATE_KEY\n-----END PGP PRIVATE KEY BLOCK----- safe=value", | ||
| ), | ||
| ).toBe("provider failed: [redacted] safe=value"); | ||
| }); | ||
|
|
||
| test("redacts every parameter from structured authorization schemes", () => { | ||
| expect( | ||
| redactedErrorMessage( | ||
| 'Authorization: Digest username="example", response=SYNTHETIC_DIGEST_SECRET', | ||
| ), | ||
| ).toBe("Authorization: Digest [redacted]"); | ||
| expect( | ||
| redactedErrorMessage("auth=Custom response=SYNTHETIC_AUTH_SECRET"), | ||
| ).toBe("auth=Custom [redacted]"); | ||
| expect( | ||
| redactedErrorMessage( | ||
| "Authorization: Custom key=SYNTHETIC_AUTH_SECRET https://example.test/safe", | ||
| ), | ||
| ).toBe("Authorization: Custom [redacted] https://example.test/safe"); | ||
| expect( | ||
| redactedErrorMessage( | ||
| 'Authorization: Digest username="a\\"b", response="SYNTHETIC_DIGEST_SECRET"', | ||
| ), | ||
| ).toBe("Authorization: Digest [redacted]"); | ||
| expect( | ||
| redactedErrorMessage( | ||
| "Authorization: Digest username*=UTF-8''user, response=SYNTHETIC_DIGEST_SECRET", | ||
| ), | ||
| ).toBe("Authorization: Digest [redacted]"); | ||
| expect( | ||
| redactedErrorMessage( | ||
| 'client_authorization_value=Digest username="example", response="SYNTHETIC_DIGEST_SECRET"', | ||
| ), | ||
| ).toBe("client_authorization_value=Digest [redacted]"); | ||
| expect( | ||
| redactedErrorMessage( | ||
| "Authorization: 0-custom 0nonce=visible, !response=SYNTHETIC_DIGEST_SECRET", | ||
| ), | ||
| ).toBe("Authorization: 0-custom [redacted]"); | ||
| }); | ||
|
|
||
| test("redacts encoded API-key names without consuming other parameters", () => { | ||
| for (const separator of ["%5F", "%2D"]) { | ||
| const value = `https://example.test/?api${separator}key%3DSYNTHETIC_API_KEY%26safe%3Dvisible`; | ||
| expect(redactedErrorMessage(value)).toBe( | ||
| `https://example.test/?api${separator}key%3D[redacted]%26safe%3Dvisible`, | ||
| ); | ||
| } | ||
| }); | ||
|
|
||
| test("redacts plural credential assignments and quoted values", () => { | ||
| expect( | ||
| redactedErrorMessage( | ||
| "credentials=SYNTHETIC_CREDENTIAL clientCredentials=SYNTHETIC_CLIENT apiKeys=SYNTHETIC_KEYS", | ||
| ), | ||
| ).toBe( | ||
| "credentials=[redacted] clientCredentials=[redacted] apiKeys=[redacted]", | ||
| ); | ||
| expect( | ||
| redactedErrorMessage('{"credentials":"correct horse battery staple"}'), | ||
| ).toBe('{"credentials":"[redacted]"}'); | ||
| expect( | ||
| redactedErrorMessage( | ||
| "credentials=[alice:SYNTHETIC_ONE,bob:SYNTHETIC_TWO] safe=visible", | ||
| ), | ||
| ).toBe("credentials=[redacted] safe=visible"); | ||
| expect( | ||
| redactedErrorMessage( | ||
| '{"credentials":["SYNTHETIC_ONE","SYNTHETIC_TWO"],"safe":"visible"}', | ||
| ), | ||
| ).toBe('{"credentials":"[redacted]","safe":"visible"}'); | ||
| expect( | ||
| redactedErrorMessage( | ||
| 'credentials_value=["SYNTHETIC_ONE","SYNTHETIC_TWO"] safe=visible', | ||
| ), | ||
| ).toBe("credentials_value=[redacted] safe=visible"); | ||
| expect( | ||
| redactedErrorMessage( | ||
| '{"api_keys_data":[{"key":"SYNTHETIC_ONE"},{"key":"SYNTHETIC_TWO"}]}', | ||
| ), | ||
| ).toBe('{"api_keys_data":"[redacted]"}'); | ||
| expect( | ||
| redactedErrorMessage("password=prefix\\SYNTHETIC_SECRET safe=visible"), | ||
| ).toBe("password=[redacted] safe=visible"); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The updated matcher now accepts additional token punctuation, but the new
{1,65}ceiling still rejects valid HTTP auth-param names longer than 65 characters. In that case the generic fallback redacts only the first fragment; for example,Authorization: Digest <66-character-name>=foo, response=SECRETbecomesAuthorization: Digest [redacted], response=SECRET. Remove the arbitrary token-length limit so later structured credential parameters cannot leak.Useful? React with 👍 / 👎.