🔐 fix: Keep OpenID Metadata Out of Logs and Traces (#15470)
Some checks are pending
Backend Unit Tests / Build packages (push) Waiting to run
Backend Unit Tests / Codegraph select (push) Waiting to run
Backend Unit Tests / TypeScript type checks (push) Blocked by required conditions
Backend Unit Tests / Circular dependency checks (push) Waiting to run
Backend Unit Tests / Tests: api (shard 1/3) (push) Blocked by required conditions
Backend Unit Tests / Tests: api (shard 2/3) (push) Blocked by required conditions
Backend Unit Tests / Tests: api (shard 3/3) (push) Blocked by required conditions
Backend Unit Tests / Tests: data-provider (push) Blocked by required conditions
Backend Unit Tests / Tests: data-schemas (push) Blocked by required conditions
Backend Unit Tests / Tests: @librechat/api (shard 1/4) (push) Blocked by required conditions
Backend Unit Tests / Tests: @librechat/api (shard 2/4) (push) Blocked by required conditions
Backend Unit Tests / Tests: @librechat/api (shard 3/4) (push) Blocked by required conditions
Backend Unit Tests / Tests: @librechat/api (shard 4/4) (push) Blocked by required conditions
Codegraph E2E Votes / vote (full suite) (push) Waiting to run
Docker Dev Branch Images Build / publish (push) Waiting to run
Frontend Unit Tests / Codegraph select (push) Waiting to run
Frontend Unit Tests / Build packages (push) Waiting to run
Frontend Unit Tests / TypeScript type checks (client) (push) Blocked by required conditions
Frontend Unit Tests / Tests: @librechat/client (push) Blocked by required conditions
Frontend Unit Tests / Tests: Ubuntu (shard 1/2) (push) Blocked by required conditions
Frontend Unit Tests / Tests: Ubuntu (shard 2/2) (push) Blocked by required conditions
Frontend Unit Tests / Vite build verification (push) Blocked by required conditions

* 🔒 fix: Keep Secrets Out of Debug Logs and Spans

* 🧯 fix: Derive Safe Exception Types

* 🛡️ fix: Close Diagnostic Redaction Gaps

* 🧼 fix: Redact Escaped JSON Secrets
This commit is contained in:
Danny Avila 2026-09-01 13:22:12 -04:00 • committed by GitHub
parent d83958d3b3
commit 67e2107d74
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 333 additions and 59 deletions

View file

@ -9,7 +9,7 @@ const { CacheKeys, ErrorTypes, SystemRoles } = require('librechat-data-provider'
const {
isEnabled,
logHeaders,
safeStringify,
logOpenIdRequestBody,
findOpenIDUser,
getOpenIdEmail,
getOpenIdIssuer,
@ -49,15 +49,7 @@ async function customFetch(url, options) {
logger.debug(`[openidStrategy] Request method: ${options.method || 'GET'}`);
logger.debug(`[openidStrategy] Request headers: ${logHeaders(options.headers)}`);
if (options.body) {
let bodyForLogging = '';
if (options.body instanceof URLSearchParams) {
bodyForLogging = options.body.toString();
} else if (typeof options.body === 'string') {
bodyForLogging = options.body;
} else {
bodyForLogging = safeStringify(options.body);
}
logger.debug(`[openidStrategy] Request body: ${bodyForLogging}`);
logger.debug(`[openidStrategy] Request body: ${logOpenIdRequestBody(options.body)}`);
}
}

View file

@ -143,7 +143,7 @@ describe('createAgentStartupTelemetry', () => {
mockTracer(span);
let now = 10;
const telemetry = createAgentStartupTelemetry({ now: () => now })!;
const error = new Error('startup failed');
const error = new TypeError('startup failed: sk-secret-canary');
now = 25;
telemetry.end('error', error);
@ -151,7 +151,11 @@ describe('createAgentStartupTelemetry', () => {
telemetry.end('aborted');
telemetry.mark('client_initialized');
expect(span.recordException).toHaveBeenCalledWith(error);
expect(span.recordException).toHaveBeenCalledWith({
message: 'Error details withheld',
name: 'TypeError',
});
expect(JSON.stringify(span.recordException.mock.calls)).not.toContain('sk-secret-canary');
expect(span.setStatus).toHaveBeenCalledWith({ code: SpanStatusCode.ERROR });
expect(recordAgentStartupResult).toHaveBeenCalledTimes(1);
expect(recordAgentStartupResult).toHaveBeenCalledWith('error');

View file

@ -10,6 +10,7 @@ import {
recordAgentStartupResult,
} from '~/app/metrics';
import { agentStartupMilestones, agentStartupResults } from './phases';
import { getSafeSpanException } from '~/telemetry/safeException';
const SPAN_NAME = 'librechat.agent.startup';
const MILESTONES = new Set<string>(agentStartupMilestones);
@ -170,7 +171,7 @@ export function createAgentStartupTelemetry(
}
if (tracingEnabled && error) {
span.recordException(error);
span.recordException(getSafeSpanException(error));
}
if (tracingEnabled && (normalizedResult === 'aborted' || normalizedResult === 'error')) {
span.setStatus({ code: SpanStatusCode.ERROR });

View file

@ -338,15 +338,25 @@ describe('telemetryErrorMiddleware', () => {
jest.restoreAllMocks();
});
it('records exceptions and forwards the error', () => {
it('records safe exception metadata and forwards the original error', () => {
const span = createSpan();
const error = new TypeError('boom');
const error = new TypeError('Bearer exception-token-canary');
error.name = 'refresh_token=error-name-canary';
Object.defineProperty(error, 'constructor', {
value: { name: 'client_secret=constructor-canary' },
});
const next = jest.fn();
jest.spyOn(trace, 'getActiveSpan').mockReturnValue(span);
telemetryErrorMiddleware(error, createRequest(), createResponse() as Response, next);
expect(span.recordException).toHaveBeenCalledWith(error);
expect(span.recordException).toHaveBeenCalledWith({
message: 'Error details withheld',
name: 'TypeError',
});
expect(JSON.stringify(span.recordException.mock.calls)).not.toContain('exception-token-canary');
expect(JSON.stringify(span.recordException.mock.calls)).not.toContain('error-name-canary');
expect(JSON.stringify(span.recordException.mock.calls)).not.toContain('constructor-canary');
expect(span.setStatus).toHaveBeenCalledWith({ code: SpanStatusCode.ERROR });
expect(span.setAttributes).toHaveBeenCalledWith({
'enduser.id': 'user-1',
@ -359,6 +369,24 @@ describe('telemetryErrorMiddleware', () => {
expect(next).toHaveBeenCalledWith(error);
});
it('falls back to Error for untrusted custom error prototypes', () => {
const span = createSpan();
const SecretNamedError = class refresh_token_supersecret extends Error {};
const error = new SecretNamedError('client_secret=message-canary');
const next = jest.fn();
jest.spyOn(trace, 'getActiveSpan').mockReturnValue(span);
telemetryErrorMiddleware(error, createRequest(), createResponse() as Response, next);
expect(span.recordException).toHaveBeenCalledWith({
message: 'Error details withheld',
name: 'Error',
});
expect(JSON.stringify(span.recordException.mock.calls)).not.toContain('supersecret');
expect(JSON.stringify(span.recordException.mock.calls)).not.toContain('message-canary');
expect(next).toHaveBeenCalledWith(error);
});
it('records exceptions on the stored request span when available', () => {
const activeSpan = createSpan();
const requestSpan = createSpan();
@ -371,7 +399,10 @@ describe('telemetryErrorMiddleware', () => {
expect(trace.getActiveSpan).not.toHaveBeenCalled();
expect(activeSpan.recordException).not.toHaveBeenCalled();
expect(requestSpan.recordException).toHaveBeenCalledWith(error);
expect(requestSpan.recordException).toHaveBeenCalledWith({
message: 'Error details withheld',
name: 'TypeError',
});
expect(requestSpan.setStatus).toHaveBeenCalledWith({ code: SpanStatusCode.ERROR });
expect(next).toHaveBeenCalledWith(error);
});
@ -381,9 +412,18 @@ describe('telemetryErrorMiddleware', () => {
const next = jest.fn();
jest.spyOn(trace, 'getActiveSpan').mockReturnValue(span);
telemetryErrorMiddleware('boom', createRequest(), createResponse() as Response, next);
telemetryErrorMiddleware(
'refresh_token=non-error-token-canary',
createRequest(),
createResponse() as Response,
next,
);
expect(span.recordException).toHaveBeenCalledWith('boom');
expect(span.recordException).toHaveBeenCalledWith({
message: 'Error details withheld',
name: 'string',
});
expect(JSON.stringify(span.recordException.mock.calls)).not.toContain('non-error-token-canary');
expect(span.setStatus).toHaveBeenCalledWith({ code: SpanStatusCode.ERROR });
expect(span.setAttributes).toHaveBeenCalledWith(
expect.objectContaining({
@ -391,7 +431,7 @@ describe('telemetryErrorMiddleware', () => {
'http.route': '/api/messages/:conversationId',
}),
);
expect(next).toHaveBeenCalledWith('boom');
expect(next).toHaveBeenCalledWith('refresh_token=non-error-token-canary');
});
it('handles null error values without throwing', () => {

View file

@ -7,6 +7,7 @@ import {
finishRedisRequestTelemetry,
runWithRedisRequestTelemetry,
} from '~/cache/redisTelemetry';
import { getErrorType, getSafeSpanException } from './safeException';
import { getTelemetryRequestSpan } from './sdk';
import { DEFAULT_HEALTH_PATH } from './config';
@ -153,7 +154,7 @@ export function telemetryErrorMiddleware(
if (span) {
const routePath = getRoutePath(req);
if (err) {
span.recordException(err instanceof Error ? err : String(err));
span.recordException(getSafeSpanException(err));
}
span.setStatus({ code: SpanStatusCode.ERROR });
setIdentityAttributes(span, req);
@ -165,15 +166,3 @@ export function telemetryErrorMiddleware(
next(err);
}
function getErrorType(err: ExpressErrorValue): string {
if (err instanceof Error) {
return err.name || err.constructor.name;
}
if (err === null) {
return 'null';
}
return typeof err;
}

View file

@ -0,0 +1,47 @@
const SAFE_EXCEPTION_MESSAGE = 'Error details withheld';
export interface SafeSpanException {
message: string;
name: string;
}
const TRUSTED_ERROR_TYPES = new Map<object, string>([
[Error.prototype, 'Error'],
[EvalError.prototype, 'EvalError'],
[RangeError.prototype, 'RangeError'],
[ReferenceError.prototype, 'ReferenceError'],
[SyntaxError.prototype, 'SyntaxError'],
[TypeError.prototype, 'TypeError'],
[URIError.prototype, 'URIError'],
]);
function getErrorConstructorName(error: Error): string {
try {
return TRUSTED_ERROR_TYPES.get(Object.getPrototypeOf(error) as object) ?? 'Error';
} catch {
return 'Error';
}
}
export function getErrorType(error: unknown): string {
try {
if (error instanceof Error) {
return getErrorConstructorName(error);
}
} catch {
return 'object';
}
if (error === null) {
return 'null';
}
return typeof error;
}
export function getSafeSpanException(error: unknown): SafeSpanException {
return {
message: SAFE_EXCEPTION_MESSAGE,
name: getErrorType(error),
};
}

View file

@ -0,0 +1,85 @@
import { logHeaders, logOpenIdRequestBody, safeStringify } from './openid';
describe('OpenID logging helpers', () => {
it('fully masks secret-bearing object fields', () => {
const output = safeStringify({
client_secret: 'client-secret-canary',
code: 'authorization-code-canary',
code_verifier: 'pkce-verifier-canary',
nested: {
refresh_token: 'refresh-token-canary',
connectionString: 'mongodb://user:password@example.test/db',
},
});
expect(output).not.toContain('client-secret-canary');
expect(output).not.toContain('authorization-code-canary');
expect(output).not.toContain('pkce-verifier-canary');
expect(output).not.toContain('refresh-token-canary');
expect(output).not.toContain('mongodb://');
expect(output).toContain('***MASKED***');
});
it('masks credential-bearing headers case-insensitively', () => {
const output = logHeaders(
new Headers({
Authorization: 'Bearer authorization-canary',
Cookie: 'session=cookie-canary',
Location: 'https://example.test/callback?code=location-code-canary',
'Set-Cookie': 'refreshToken=set-cookie-canary',
'X-Amz-Security-Token': 'aws-token-canary',
'X-API-Key': 'api-key-canary',
'Content-Type': 'application/json',
}),
);
expect(output).not.toContain('authorization-canary');
expect(output).not.toContain('cookie-canary');
expect(output).not.toContain('set-cookie-canary');
expect(output).not.toContain('aws-token-canary');
expect(output).not.toContain('location-code-canary');
expect(output).not.toContain('api-key-canary');
expect(output).toContain('application/json');
});
it('logs URL-encoded request shape without logging values', () => {
const body = new URLSearchParams({
grant_type: 'authorization_code',
code: 'authorization-code-canary',
code_verifier: 'pkce-verifier-canary',
client_secret: 'client-secret-canary',
});
const output = logOpenIdRequestBody(body);
expect(output).toContain('URLSearchParams');
expect(output).toContain('fieldCount');
expect(output).not.toContain('grant_type');
expect(output).not.toContain('client_secret');
expect(output).not.toContain('authorization-code-canary');
expect(output).not.toContain('pkce-verifier-canary');
expect(output).not.toContain('client-secret-canary');
});
it('logs opaque string body metadata without logging content', () => {
const output = logOpenIdRequestBody('refresh_token=refresh-token-canary');
expect(output).toContain('string');
expect(output).toContain('length');
expect(output).not.toContain('refresh-token-canary');
});
it('never lets hostile debug values interrupt an OpenID request', () => {
const hostileBody = new Proxy(
{},
{
ownKeys() {
throw new Error('refresh_token=proxy-canary');
},
},
);
expect(() => logOpenIdRequestBody(hostileBody)).not.toThrow();
expect(logOpenIdRequestBody(hostileBody)).not.toContain('proxy-canary');
});
});

View file

@ -1,3 +1,25 @@
const MASKED_VALUE = '***MASKED***';
const SAFE_OPENID_HEADER_VALUES = new Set(['content-length', 'content-type']);
function isSensitiveFieldName(key: string): boolean {
const normalized = key.toLowerCase().replace(/[^a-z0-9]/g, '');
return (
normalized === 'code' ||
normalized.includes('authorization') ||
normalized.includes('assertion') ||
normalized.includes('clientid') ||
normalized.includes('connectionstring') ||
normalized.includes('cookie') ||
normalized.includes('credential') ||
normalized.includes('password') ||
normalized.includes('signature') ||
normalized.includes('secret') ||
normalized.includes('token') ||
normalized.includes('verifier') ||
normalized.includes('apikey')
);
}
/**
* Helper function to safely log sensitive data when debug mode is enabled
* @param obj - Object to stringify
@ -7,26 +29,40 @@
export function safeStringify(obj: unknown, maxLength = 1000): string {
try {
const str = JSON.stringify(obj, (key, value) => {
// Mask sensitive values
if (
key === 'client_secret' ||
key === 'Authorization' ||
key.toLowerCase().includes('token') ||
key.toLowerCase().includes('password')
) {
return typeof value === 'string' && value.length > 6
? `${value.substring(0, 3)}...${value.substring(value.length - 3)}`
: '***MASKED***';
if (isSensitiveFieldName(key)) {
return MASKED_VALUE;
}
return value;
});
if (str && str.length > maxLength) {
return `${str.substring(0, maxLength)}... (truncated)`;
const serialized = str ?? '[Unserializable value]';
if (serialized.length > maxLength) {
return `${serialized.substring(0, maxLength)}... (truncated)`;
}
return str;
} catch (error) {
return `[Error stringifying object: ${(error as Error).message}]`;
return serialized;
} catch {
return '[Error stringifying object]';
}
}
/**
* Describes an OpenID request body without serializing any values. OAuth form
* bodies routinely contain authorization codes, refresh tokens, PKCE verifiers,
* and client secrets, so even partially masked values are unsafe to log.
*/
export function logOpenIdRequestBody(body: unknown): string {
try {
if (body instanceof URLSearchParams) {
return safeStringify({ type: 'URLSearchParams', fieldCount: body.size });
}
if (typeof body === 'string') {
return safeStringify({ type: 'string', length: body.length });
}
return safeStringify({ type: body == null ? typeof body : 'object' });
} catch {
return '[OpenID request body metadata unavailable]';
}
}
@ -36,16 +72,16 @@ export function safeStringify(obj: unknown, maxLength = 1000): string {
* @returns Stringified headers with sensitive data masked
*/
export function logHeaders(headers: Headers | undefined | null): string {
const headerObj: Record<string, string> = {};
if (!headers || typeof headers.entries !== 'function') {
return 'No headers available';
}
for (const [key, value] of headers.entries()) {
if (key.toLowerCase() === 'authorization' || key.toLowerCase().includes('secret')) {
headerObj[key] = '***MASKED***';
} else {
headerObj[key] = value;
try {
const headerObj: Record<string, string> = {};
if (!headers || typeof headers.entries !== 'function') {
return 'No headers available';
}
for (const [key, value] of headers.entries()) {
headerObj[key] = SAFE_OPENID_HEADER_VALUES.has(key.toLowerCase()) ? value : MASKED_VALUE;
}
return safeStringify(headerObj);
} catch {
return '[OpenID request headers unavailable]';
}
return safeStringify(headerObj);
}

View file

@ -48,6 +48,9 @@ describe('redactMessage', () => {
expect(redactMessage('token: sk-abc123def')).toBe('token: sk-[REDACTED]');
expect(redactMessage('auth Bearer secretvalue')).toBe('auth Bearer [REDACTED]');
expect(redactMessage('api-key: secretvalue')).toBe('api-key: [REDACTED]');
expect(redactMessage('Authorization: Basic basic-credentials')).toBe(
'Authorization: [REDACTED]',
);
expect(redactMessage('https://example.test/?key=secretvalue&next=true')).toBe(
'https://example.test/?key=[REDACTED]&next=true',
);
@ -61,6 +64,53 @@ describe('redactMessage', () => {
expect(redactMessage('mask-value computed')).toBe('mask-value computed');
expect(redactMessage('monkey=10 bananas')).toBe('monkey=10 bananas');
});
it('redacts OAuth and signed URL parameters', () => {
const message =
'https://example.test/callback?code=auth-code&code_verifier=pkce-secret&client_secret=oauth-secret&X-Amz-Signature=aws-signature&X-Amz-Security-Token=session-token&next=true';
expect(redactMessage(message)).toBe(
'https://example.test/callback?code=[REDACTED]&code_verifier=[REDACTED]&client_secret=[REDACTED]&X-Amz-Signature=[REDACTED]&X-Amz-Security-Token=[REDACTED]&next=true',
);
});
it('redacts OAuth secrets in JSON-formatted strings', () => {
const message =
'oauth failed: {"refresh_token":"refresh-json-canary","client_secret": "client-json-canary","client_assertion":"assertion-json-canary","code":"code-json-canary","safe":"visible"}';
expect(redactMessage(message)).toBe(
'oauth failed: {"refresh_token":"[REDACTED]","client_secret": "[REDACTED]","client_assertion":"[REDACTED]","code":"[REDACTED]","safe":"visible"}',
);
});
it('redacts escaped JSON OAuth secret values through the closing quote', () => {
const secret = 'prefix"secret-tail\\suffix';
const message = `oauth failed: ${JSON.stringify({ refresh_token: secret, safe: 'visible' })}`;
const redacted = redactMessage(message);
expect(redacted).toBe('oauth failed: {"refresh_token":"[REDACTED]","safe":"visible"}');
expect(redacted).not.toContain('secret-tail');
expect(redacted).not.toContain('suffix');
});
it('redacts complete JWTs, cookies, and URI userinfo', () => {
const jwt = 'eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiJzZWNyZXQtdXNlciJ9.c2lnbmF0dXJlLXNlY3JldA';
const message =
`token=${jwt} Cookie: session=cookie-secret; refresh=refresh-secret\n` +
'database=postgresql://service-user:database-secret@example.test/app\n' +
'cache=redis://:password-only-canary@example.test:6379';
const redacted = redactMessage(message);
expect(redacted).not.toContain(jwt);
expect(redacted).not.toContain('cookie-secret');
expect(redacted).not.toContain('refresh-secret');
expect(redacted).not.toContain('database-secret');
expect(redacted).not.toContain('password-only-canary');
expect(redacted).toContain('token=[REDACTED]');
expect(redacted).toContain('Cookie: [REDACTED]');
expect(redacted).toContain('postgresql://[REDACTED]@example.test/app');
expect(redacted).toContain('redis://[REDACTED]@example.test:6379');
});
});
describe('redactFormat', () => {
@ -114,6 +164,9 @@ describe('redactFormat', () => {
nested: { token: 'secretvalue' },
safe: 'secretvalue',
'x-api-key': 'secretvalue',
cookie: 'session=secretvalue',
code_verifier: 'secretvalue',
connectionString: 'postgresql://user:secretvalue@example.test/db',
};
const info = runRedactFormat({
level: 'info',
@ -126,6 +179,9 @@ describe('redactFormat', () => {
expect(splat[0].authorization).toBe('[REDACTED]');
expect(splat[0].nested.token).toBe('[REDACTED]');
expect(splat[0]['x-api-key']).toBe('[REDACTED]');
expect(splat[0].cookie).toBe('[REDACTED]');
expect(splat[0].code_verifier).toBe('[REDACTED]');
expect(splat[0].connectionString).toBe('[REDACTED]');
expect(splat[0].safe).toBe('secretvalue');
expect(metadata.apiKey).toBe('secretvalue');
expect(metadata.nested.token).toBe('secretvalue');
@ -240,6 +296,24 @@ describe('redactFormat', () => {
expect(error.stack).toContain('Bearer secretvalue');
});
it('redacts JSON OAuth bodies and password-only URIs inside errors', () => {
const error = new Error(
'OAuth failed: {"refresh_token":"error-json-canary"} redis://:error-uri-canary@example.test',
);
const info = runRedactFormat({
level: 'error',
message: 'token refresh failed',
[SPLAT_SYMBOL]: [error],
});
const redactedError = (info[SPLAT_SYMBOL] as Error[])[0];
expect(redactedError.message).toContain('"refresh_token":"[REDACTED]"');
expect(redactedError.message).toContain('redis://[REDACTED]@example.test');
expect(redactedError.message).not.toContain('error-json-canary');
expect(redactedError.message).not.toContain('error-uri-canary');
});
it('preserves contextual messages when redacting error splat arguments', () => {
const error = new Error('Bearer secretvalue');

View file

@ -44,10 +44,16 @@ const sensitiveKeys: RegExp[] = [
/\b(api-key:? )[^\s"']+/gi, // Header: API key pattern
/\b(api_key=)[^\s"'&]+/gi, // URL query param: API key pattern
/\b(key=)[^\s"'&]+/g, // URL query param: sensitive key pattern
/(\b(?:access_token|assertion|client_assertion|client_secret|code|code_verifier|credential|id_token|refresh_token|sig|signature|x-amz-security-token|x-amz-signature|x-goog-signature)\b"?\s*:\s*")(?:\\.|[^"\\])*/gi,
/\b((?:access_token|assertion|client_assertion|client_secret|code|code_verifier|credential|id_token|refresh_token|sig|signature|x-amz-security-token|x-amz-signature|x-goog-signature)=)[^\s"'&]+/gi,
/(\b)eyJ[a-zA-Z0-9_-]{5,}\.[a-zA-Z0-9_-]{5,}\.[a-zA-Z0-9_-]+/g,
/\b((?:Proxy-)?Authorization:\s*)[^\r\n]+/gi,
/\b((?:Cookie|Set-Cookie):\s*)[^\r\n]+/gi,
/([a-z][a-z0-9+.-]*:\/\/)[^/\s:@]*:[^/\s@]+(?=@)/gi,
];
const sensitiveMetadataKey =
/^(authorization|proxy-authorization|x-api-key|api[-_]?key|access[-_]?token|refresh[-_]?token|id[-_]?token|token|secret|password)$/i;
/^(authorization|proxy-authorization|cookie|set-cookie|x-api-key|api[-_]?key|access[-_]?token|refresh[-_]?token|id[-_]?token|token|secret|password|assertion|client[-_]?assertion|client[-_]?secret|code[-_]?verifier|credential|signature|connection[-_]?string)$/i;
const errorStringProperties = new Set(['name', 'message', 'stack']);
/**