diff --git a/.changeset/machine-token-sub-guard.md b/.changeset/machine-token-sub-guard.md new file mode 100644 index 00000000000..ea2ae1bbeb1 --- /dev/null +++ b/.changeset/machine-token-sub-guard.md @@ -0,0 +1,5 @@ +--- +'@clerk/backend': patch +--- + +Prevent an unhandled exception when verifying a machine token whose JWT payload has a missing or non-string `sub`. Such tokens are now classified and rejected with a typed verification error instead of throwing, so a crafted `Authorization` header can no longer surface as an unhandled error during request authentication. diff --git a/.changeset/redact-nested-tokens-debug-formatter.md b/.changeset/redact-nested-tokens-debug-formatter.md new file mode 100644 index 00000000000..a9a472ddfe9 --- /dev/null +++ b/.changeset/redact-nested-tokens-debug-formatter.md @@ -0,0 +1,5 @@ +--- +'@clerk/nextjs': patch +--- + +Harden middleware debug log output: the formatter now recursively truncates known credential keys (`sessionToken`, `tokenInHeader`, `sessionTokenInCookie`, `secretKey`, `jwtKey`) at any nesting depth, so a bearer token can no longer reach the logs even if a debug producer nests one. This is a defense-in-depth backstop alongside the source-level redaction in `@clerk/backend`. diff --git a/.changeset/redact-tokens-debug-output.md b/.changeset/redact-tokens-debug-output.md new file mode 100644 index 00000000000..0fa37151f1c --- /dev/null +++ b/.changeset/redact-tokens-debug-output.md @@ -0,0 +1,5 @@ +--- +'@clerk/backend': patch +--- + +Redact raw bearer credentials from the `auth` object's debug output. The debug payload (surfaced when an SDK enables middleware debug logging) previously included full session, machine, refresh, dev-browser and handshake tokens; each now exposes only a short, non-reconstructable prefix, matching how `secretKey` and `jwtKey` are already handled. diff --git a/packages/backend/src/tokens/__tests__/authObjects.test.ts b/packages/backend/src/tokens/__tests__/authObjects.test.ts index 7f91eddb559..fde4ec53c75 100644 --- a/packages/backend/src/tokens/__tests__/authObjects.test.ts +++ b/packages/backend/src/tokens/__tests__/authObjects.test.ts @@ -47,6 +47,36 @@ describe('signedInAuthObject', () => { expect(token).toBe('token'); }); + it('redacts raw session and machine tokens from debug output', () => { + const rawSessionToken = 'eyJhbGciOiJSUzI1NiJ9.eyJzdWIiOiJ1c2VyXzEyMyJ9.this-segment-must-never-be-logged'; + const mockAuthenticateContext = { + tokenInHeader: 'eyJhbGciOiJSUzI1NiJ9.header-bearer.this-segment-must-never-be-logged', + sessionTokenInCookie: 'eyJhbGciOiJSUzI1NiJ9.cookie-bearer.this-segment-must-never-be-logged', + refreshTokenInCookie: 'eyJhbGciOiJSUzI1NiJ9.refresh-bearer.this-segment-must-never-be-logged', + devBrowserToken: 'eyJhbGciOiJSUzI1NiJ9.devbrowser-bearer.this-segment-must-never-be-logged', + handshakeToken: 'eyJhbGciOiJSUzI1NiJ9.handshake-bearer.this-segment-must-never-be-logged', + } as unknown as AuthenticateContext; + + const authObject = signedInAuthObject(mockAuthenticateContext, rawSessionToken, { + sub: 'userId', + } as unknown as JwtPayload); + + const debug = authObject.debug() as Record; + + // Only a short, non-reconstructable prefix of each bearer credential is exposed. + expect(debug.sessionToken).toBe('eyJhbGc'); + expect(debug.tokenInHeader).toBe('eyJhbGc'); + expect(debug.sessionTokenInCookie).toBe('eyJhbGc'); + expect(debug.refreshTokenInCookie).toBe('eyJhbGc'); + expect(debug.devBrowserToken).toBe('eyJhbGc'); + expect(debug.handshakeToken).toBe('eyJhbGc'); + + // The full tokens must not be recoverable from the serialized debug payload. + const serialized = JSON.stringify(debug); + expect(serialized).not.toContain('this-segment-must-never-be-logged'); + expect(serialized).not.toContain(rawSessionToken); + }); + describe('JWT v1', () => { it('has() for user scope', () => { const mockAuthenticateContext = { sessionToken: 'authContextToken' } as AuthenticateContext; diff --git a/packages/backend/src/tokens/__tests__/verify.test.ts b/packages/backend/src/tokens/__tests__/verify.test.ts index a396d796504..f1bbc1aa72d 100644 --- a/packages/backend/src/tokens/__tests__/verify.test.ts +++ b/packages/backend/src/tokens/__tests__/verify.test.ts @@ -509,6 +509,45 @@ describe('tokens.verifyMachineAuthToken(token, options)', () => { expect(result.errors).toBeDefined(); expect(result.errors?.[0].message).toContain('expired'); }); + + // Regression: `decodedResult.payload.sub.startsWith(...)` previously threw a + // TypeError for a missing or non-string `sub` before OAuth verification ran, so a + // crafted at+jwt bearer token surfaced as an unhandled error in request auth. + it.each([ + ['a missing', undefined], + ['a null', null], + ['a numeric', 123], + ['an object', {}], + ] as Array<[string, unknown]>)( + 'classifies an at+jwt token with %s sub as OAuth instead of throwing', + async (_label, sub) => { + server.use( + http.get( + 'https://api.clerk.test/v1/jwks', + validateHeaders(() => { + return HttpResponse.json(mockJwks); + }), + ), + ); + + const payload: Record = { ...mockOAuthAccessTokenJwtPayload }; + if (sub === undefined) { + delete payload.sub; + } else { + payload.sub = sub; + } + + const oauthJwt = await createSignedOAuthJwt(payload as typeof mockOAuthAccessTokenJwtPayload, 'at+jwt'); + + const result = await verifyMachineAuthToken(oauthJwt, { + apiUrl: 'https://api.clerk.test', + secretKey: 'a-valid-key', + }); + + // Reaching a typed OAuth result proves the M2M `sub` check no longer throws. + expect(result.tokenType).toBe('oauth_token'); + }, + ); }); describe('verifyM2MToken with JWT', () => { diff --git a/packages/backend/src/tokens/authObjects.ts b/packages/backend/src/tokens/authObjects.ts index 8353fa1286d..44391b388da 100644 --- a/packages/backend/src/tokens/authObjects.ts +++ b/packages/backend/src/tokens/authObjects.ts @@ -170,6 +170,19 @@ const createDebug = (data: AuthObjectDebugData | undefined) => { const res = { ...data }; res.secretKey = (res.secretKey || '').substring(0, 7); res.jwtKey = (res.jwtKey || '').substring(0, 7); + // Session and machine tokens are live bearer credentials, so only ever expose a + // short, non-reconstructable prefix here, the same way secretKey/jwtKey are handled + // above. Otherwise enabling debug logging would write usable tokens to logs. + // This also covers the bearer fields carried on AuthenticateContext, which is spread + // wholesale into the debug payload by signedInAuthObject: the refresh token is the + // most sensitive of these, and the dev-browser/handshake tokens are short-lived but + // still credentials. + res.sessionToken = (res.sessionToken || '').substring(0, 7); + res.tokenInHeader = (res.tokenInHeader || '').substring(0, 7); + res.sessionTokenInCookie = (res.sessionTokenInCookie || '').substring(0, 7); + res.refreshTokenInCookie = (res.refreshTokenInCookie || '').substring(0, 7); + res.devBrowserToken = (res.devBrowserToken || '').substring(0, 7); + res.handshakeToken = (res.handshakeToken || '').substring(0, 7); return { ...res }; }; }; diff --git a/packages/backend/src/tokens/verify.ts b/packages/backend/src/tokens/verify.ts index 9954bdc77bd..93c27df42ac 100644 --- a/packages/backend/src/tokens/verify.ts +++ b/packages/backend/src/tokens/verify.ts @@ -261,7 +261,7 @@ export async function verifyMachineAuthToken(token: string, options: VerifyToken } as MachineTokenReturnType; } - if (decodedResult.payload.sub.startsWith(M2M_SUBJECT_PREFIX)) { + if (typeof decodedResult.payload.sub === 'string' && decodedResult.payload.sub.startsWith(M2M_SUBJECT_PREFIX)) { return verifyM2MJwt(token, decodedResult, options); } diff --git a/packages/nextjs/src/utils/__tests__/logFormatter.test.ts b/packages/nextjs/src/utils/__tests__/logFormatter.test.ts new file mode 100644 index 00000000000..fea767287fc --- /dev/null +++ b/packages/nextjs/src/utils/__tests__/logFormatter.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, it } from 'vitest'; + +import { logFormatter } from '../logFormatter'; + +describe('logFormatter', () => { + it('truncates sensitive token keys nested in debug objects', () => { + const entry = [ + 'auth', + { + auth: { userId: 'user_123' }, + debug: { + sessionToken: 'eyJhbGciOiJSUzI1NiJ9.payload.full-session-segment-should-not-appear', + tokenInHeader: 'eyJhbGciOiJSUzI1NiJ9.payload.header-segment-should-not-appear', + sessionTokenInCookie: 'eyJhbGciOiJSUzI1NiJ9.payload.cookie-segment-should-not-appear', + }, + }, + ]; + + const output = logFormatter(entry as any); + + // Full bearer tokens nested under known keys must not survive formatting. + expect(output).not.toContain('full-session-segment-should-not-appear'); + expect(output).not.toContain('header-segment-should-not-appear'); + expect(output).not.toContain('cookie-segment-should-not-appear'); + // Only the short, non-reconstructable prefix remains. + expect(output).toContain('"sessionToken": "eyJhbGc"'); + // Non-sensitive nested data is preserved. + expect(output).toContain('"userId": "user_123"'); + }); + + it('is idempotent for values already truncated at the source', () => { + const entry = ['auth', { debug: { sessionToken: 'eyJhbGc' } }]; + + const output = logFormatter(entry as any); + + expect(output).toContain('"sessionToken": "eyJhbGc"'); + }); +}); diff --git a/packages/nextjs/src/utils/logFormatter.ts b/packages/nextjs/src/utils/logFormatter.ts index 51ed7996ffa..d2029a19293 100644 --- a/packages/nextjs/src/utils/logFormatter.ts +++ b/packages/nextjs/src/utils/logFormatter.ts @@ -1,5 +1,12 @@ import type { LogEntry } from './debugLogger'; +// Keys whose values are live bearer credentials or secrets. Their values are +// truncated at any nesting depth, as a defense-in-depth backstop for debug +// producers that nest sensitive data. The authoritative redaction still happens +// at the source (e.g. @clerk/backend's auth-object debug output already truncates +// these); truncating to the same 7-char prefix here keeps that output stable. +const SENSITIVE_KEYS = new Set(['sessionToken', 'tokenInHeader', 'sessionTokenInCookie', 'secretKey', 'jwtKey']); + // Move to shared once clerk/shared is used in clerk/nextjs const maskSecretKey = (str: any) => { if (!str || typeof str !== 'string') { @@ -13,6 +20,25 @@ const maskSecretKey = (str: any) => { } }; +// Recursively redacts sensitive values. A string under a known sensitive key is +// truncated regardless of depth; every other string is still run through +// maskSecretKey so `sk_*` keys are masked wherever they appear. +const redactSensitive = (value: unknown, key?: string): unknown => { + if (key && SENSITIVE_KEYS.has(key) && typeof value === 'string') { + return value.substring(0, 7); + } + + if (Array.isArray(value)) { + return value.map(item => redactSensitive(item)); + } + + if (value && typeof value === 'object') { + return Object.fromEntries(Object.entries(value).map(([k, v]) => [k, redactSensitive(v, k)])); + } + + return maskSecretKey(value); +}; + export const logFormatter = (entry: LogEntry) => { return (Array.isArray(entry) ? entry : [entry]) .map(entry => { @@ -20,8 +46,7 @@ export const logFormatter = (entry: LogEntry) => { return maskSecretKey(entry); } - const masked = Object.fromEntries(Object.entries(entry).map(([k, v]) => [k, maskSecretKey(v)])); - return JSON.stringify(masked, null, 2); + return JSON.stringify(redactSensitive(entry), null, 2); }) .join(', '); };