From c7ada345e6f63b3b9568888a93c312d47f18e72e Mon Sep 17 00:00:00 2001 From: "Aaron K. Clark" Date: Tue, 19 May 2026 11:58:30 -0500 Subject: [PATCH] chore(redact-url): redact OAuth `code` + RFC 7521/7523 assertions in logs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The redact-url middleware already covered access_token, refresh_token, id_token, and client_secret. Three more OAuth query-string params can ride into our logs via redirect bounces and were not on the allowlist: - `code` (RFC 6749 §4.1) — single-use OAuth authorization code. Short-lived but a leaked log line containing one is enough to attempt a replay before the legitimate exchange completes. - `assertion` (RFC 7521 §4.1) — generic bearer-assertion param. - `client_assertion` (RFC 7521 §4.2 / RFC 7523) — JWT used as client credential. Doesn't expire for minutes to hours and is unambiguously credential-bearing. We don't issue OAuth tokens ourselves, but operators fronting this API with an OAuth proxy may have error handlers that bounce through /v1/* paths with these params still attached. Add two table-driven assertions in tests/unit/redact-url.test.js to document the new redactions and the rationale. Co-Authored-By: Claude Opus 4.7 (1M context) --- app/middleware/redact-url.js | 17 +++++++++++++++++ tests/unit/redact-url.test.js | 22 ++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/app/middleware/redact-url.js b/app/middleware/redact-url.js index b03b51d..3322ed2 100644 --- a/app/middleware/redact-url.js +++ b/app/middleware/redact-url.js @@ -32,6 +32,23 @@ const SENSITIVE_PARAM_NAMES = new Set([ 'refresh_token', 'id_token', 'client_secret', + // OAuth authorization code (RFC 6749 §4.1) and the RFC 7521 + // generic-assertion / RFC 7523 JWT-bearer client-assertion + // params. The auth code rides on the redirect URI's query + // string in the standard browser flow; on error redirects it + // occasionally lands back on an arbitrary path with the value + // still attached. The code is single-use and short-lived, but + // a leaked log line with one in it is enough to attempt a + // replay before the legitimate exchange completes. The + // assertion params carry signed JWT/SAML credentials that + // don't expire for minutes to hours and are unambiguously + // sensitive. We don't issue OAuth tokens ourselves, but if an + // operator fronts this API with an OAuth proxy whose error + // handlers bounce through /v1/* we shouldn't be the source of + // the leak. + 'code', + 'assertion', + 'client_assertion', 'password', 'secret', ]); diff --git a/tests/unit/redact-url.test.js b/tests/unit/redact-url.test.js index ac91e51..0eece53 100644 --- a/tests/unit/redact-url.test.js +++ b/tests/unit/redact-url.test.js @@ -70,6 +70,28 @@ describe('redactUrl', () => { expect(redactUrl(42)).toBe(42); }); + test('redacts OAuth authorization code on a redirect-bounce URL', () => { + // OAuth `code` parameter is single-use but a leaked log line + // containing one can be replayed before the legitimate + // exchange completes. RFC 6749 §4.1 puts it on the redirect + // URI's query string verbatim. + const out = redactUrl('/v1/whoami?code=AUTH_CODE_VALUE&state=xyz'); + expect(out).toContain('code='); + expect(out).not.toContain('AUTH_CODE_VALUE'); + // state is a CSRF nonce, not sensitive itself — leave it alone + expect(out).toContain('state=xyz'); + }); + + test('redacts the RFC 7521 / 7523 client-assertion JWT', () => { + // RFC 7523 puts a signed JWT in `client_assertion` and + // `assertion`; both are credential-bearing and don't expire + // for minutes to hours. + expect(redactUrl('/x?client_assertion=eyJ.signed.jwt&grant_type=client_credentials')) + .toContain('client_assertion='); + expect(redactUrl('/x?assertion=eyJ.SAML.creds')) + .toBe('/x?assertion='); + }); + test('does not throw on malformed percent-encoding in param name', () => { // decodeURIComponent throws URIError on `%FF` (incomplete UTF-8 // sequence) or `%ZZ` (invalid hex). pino-http would invoke this