-
Notifications
You must be signed in to change notification settings - Fork 51
fix(invoke): adjust redaction regex to allow words following bearer #1480
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,17 +1,34 @@ | ||
| import { redactSensitiveText } from '../command.js'; | ||
| import { describe, expect, it } from 'vitest'; | ||
| import { SignJWT } from 'jose'; | ||
| import { beforeAll, describe, expect, it } from 'vitest'; | ||
|
|
||
| const TEST_SIGNING_SECRET = new TextEncoder().encode('redaction-unit-test-signing-secret-0123456789'); | ||
|
|
||
| async function makeJwt(claims: Record<string, unknown> = { sub: '1234567890', aud: 'client-abc' }): Promise<string> { | ||
| return new SignJWT(claims) | ||
| .setProtectedHeader({ alg: 'HS256', typ: 'JWT' }) | ||
| .setIssuedAt() | ||
| .setExpirationTime('1h') | ||
| .sign(TEST_SIGNING_SECRET); | ||
| } | ||
|
|
||
| describe('redactSensitiveText', () => { | ||
| let jwt: string; | ||
|
|
||
| beforeAll(async () => { | ||
| jwt = await makeJwt(); | ||
| }); | ||
|
|
||
| it('redacts Bearer tokens', () => { | ||
| expect(redactSensitiveText('Authorization: Bearer eyJhbGciOiJSUzI1NiJ9.payload.sig')).toBe( | ||
| 'Authorization: Bearer [REDACTED]' | ||
| ); | ||
| expect(redactSensitiveText(`Authorization: Bearer ${jwt}`)).toBe('Authorization: Bearer [REDACTED]'); | ||
| }); | ||
|
|
||
| it('redacts Bearer tokens in JSON', () => { | ||
| expect(redactSensitiveText('{"header":"Bearer eyJhbGciOiJSUzI1NiJ9.payload.sig"}')).toBe( | ||
| '{"header":"Bearer [REDACTED]"}' | ||
| ); | ||
| expect(redactSensitiveText(`{"header":"Bearer ${jwt}"}`)).toBe('{"header":"Bearer [REDACTED]"}'); | ||
| }); | ||
|
|
||
| it('redacts a JWT by shape even without a "bearer"/key prefix', () => { | ||
| expect(redactSensitiveText(`agent response: ${jwt}`)).toBe('agent response: [REDACTED]'); | ||
| }); | ||
|
|
||
| it('redacts client_secret in key=value form', () => { | ||
|
|
@@ -38,13 +55,24 @@ describe('redactSensitiveText', () => { | |
| expect(redactSensitiveText('client-secret=mysecret')).toBe('client-secret=[REDACTED]'); | ||
| }); | ||
|
|
||
| it('handles multiple sensitive values in one string', () => { | ||
| expect(redactSensitiveText(`Bearer ${jwt} and client_secret=xyz789`)).toBe( | ||
| 'Bearer [REDACTED] and client_secret=[REDACTED]' | ||
| ); | ||
| }); | ||
|
|
||
| it('does not modify text without sensitive content', () => { | ||
| const input = 'Agent responded successfully with 200 OK'; | ||
| expect(redactSensitiveText(input)).toBe(input); | ||
| }); | ||
|
|
||
| it('handles multiple sensitive values in one string', () => { | ||
| const input = 'Bearer abc123 and client_secret=xyz789'; | ||
| expect(redactSensitiveText(input)).toBe('Bearer [REDACTED] and client_secret=[REDACTED]'); | ||
| it('does not redact the literal word "token" after "bearer"', () => { | ||
| const input = "Agent 'E2eJwt123' is configured for CUSTOM_JWT but no bearer token is available."; | ||
| expect(redactSensitiveText(input)).toBe(input); | ||
| }); | ||
|
|
||
| it('does not redact prose like "Invalid Bearer Token"', () => { | ||
| const input = 'Invalid Bearer Token'; | ||
| expect(redactSensitiveText(input)).toBe(input); | ||
|
Comment on lines
+74
to
+76
Contributor
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. nice that our integ/e2e tests caught this! |
||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -82,10 +82,16 @@ | |
| } | ||
|
|
||
| export function redactSensitiveText(value: string): string { | ||
| return value | ||
| .replace(/(bearer\s+)[a-z0-9\-._~+/]+=*/gi, '$1[REDACTED]') | ||
| .replace(/(client[_-]?secret["']?\s*[:=]\s*["']?)([^"',\s}]+)/gi, '$1[REDACTED]') | ||
| .replace(/((?:access[_-]?)?token["']?\s*[:=]\s*["']?)([^"',\s}]+)/gi, '$1[REDACTED]'); | ||
| return ( | ||
| value | ||
| // AgentCore inbound bearer tokens are always CUSTOM_JWT/OIDC JWTs, and a JWT always begins | ||
| // with `eyJ` (base64url of `{"`, the start of its header/payload JSON). Matching by shape | ||
| // redacts the token wherever it appears and never touches prose like "bearer token". | ||
| // See https://stackoverflow.com/a/74181595 (why JWTs start with `eyJ`). | ||
| .replace(/eyJ[A-Za-z0-9_-]+\.eyJ[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+/g, '[REDACTED]') | ||
| .replace(/(client[_-]?secret["']?\s*[:=]\s*["']?)([^"',\s}]+)/gi, '$1[REDACTED]') | ||
| .replace(/((?:access[_-]?)?token["']?\s*[:=]\s*["']?)([^"',\s}]+)/gi, '$1[REDACTED]') | ||
|
Contributor
Author
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. the eslint flag is existing code, and its a false-positive. The rule is prone to false positives, and they call it out here: https://www.npmjs.com/package/safe-regex |
||
| ); | ||
| } | ||
|
|
||
| function printInvokeResult(result: InvokeResult, options: InvokeOptions): void { | ||
|
|
||
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.
TIL about
josepretty neat!