fix: redact OAuth trace error messages

This commit is contained in:
Kai (Tam Nhu) Tran
2026-05-30 15:28:32 -04:00
committed by GitHub
parent 62a4d44b58
commit 14da76abae
4 changed files with 38 additions and 11 deletions
@@ -306,7 +306,11 @@ describe('handlePasteCallbackMode traceability', () => {
}, },
{ {
url: /\/v0\/management\/oauth-callback$/, url: /\/v0\/management\/oauth-callback$/,
response: { status: 'error', error: 'invalid_grant: expired code' }, response: {
status: 'error',
error:
'invalid_grant: bad redirect http://localhost:1455/callback?code=oauth-code-secret&state=upstream-state-secret',
},
status: 400, status: 400,
}, },
]); ]);
@@ -120,9 +120,29 @@ describe('createOAuthTraceRecorder', () => {
expect(snap[0].error).toEqual({ code: 'E1', message: 'boom' }); expect(snap[0].error).toEqual({ code: 'E1', message: 'boom' });
}); });
test('Error instance accepted', () => { test('redacts OAuth secrets from error messages before they reach sinks', () => {
const lines: string[] = [];
const { rec } = makeRecorder(true, lines);
rec.record(OAuthTracePhase.Error, undefined, {
code: 'CALLBACK_REJECTED',
message:
'bad redirect http://localhost:1455/callback?code=AUTHCODE_SECRET&state=STATE_SECRET',
});
const blob = JSON.stringify(rec.snapshot()) + '\n' + lines.join('\n');
expect(blob).not.toContain('AUTHCODE_SECRET');
expect(blob).not.toContain('STATE_SECRET');
expect(blob).toContain(REDACTED_PLACEHOLDER);
});
test('Error instance accepted and redacted', () => {
const { rec } = makeRecorder(); const { rec } = makeRecorder();
rec.record(OAuthTracePhase.Error, undefined, new Error('plain')); rec.record(
expect(rec.snapshot()[0].error?.message).toBe('plain'); OAuthTracePhase.Error,
undefined,
new Error('bad redirect http://localhost:1455/callback?code=AUTHCODE_SECRET')
);
expect(rec.snapshot()[0].error?.message).toContain(REDACTED_PLACEHOLDER);
expect(rec.snapshot()[0].error?.message).not.toContain('AUTHCODE_SECRET');
}); });
}); });
+7 -4
View File
@@ -82,6 +82,7 @@ import { generateSessionId } from './project-selection-handler';
import { createFileSink } from './oauth-trace/sink-file'; import { createFileSink } from './oauth-trace/sink-file';
import { createOAuthTraceRecorder, OAuthTracePhase, type OAuthTraceRecorder } from './oauth-trace'; import { createOAuthTraceRecorder, OAuthTracePhase, type OAuthTraceRecorder } from './oauth-trace';
import { diagnoseFailure, formatErrorMessage } from './oauth-trace/diagnose-failure'; import { diagnoseFailure, formatErrorMessage } from './oauth-trace/diagnose-failure';
import { redactString } from './oauth-trace/redactor';
interface PasteCallbackStartData { interface PasteCallbackStartData {
url?: string; url?: string;
@@ -889,8 +890,9 @@ export async function handlePasteCallbackMode(
if (!callbackResponse.ok || callbackData.status === 'error') { if (!callbackResponse.ok || callbackData.status === 'error') {
const callbackError = const callbackError =
callbackData.error || `OAuth callback failed with status ${callbackResponse.status}`; callbackData.error || `OAuth callback failed with status ${callbackResponse.status}`;
console.log(fail(callbackError)); const redactedCallbackError = redactString(callbackError);
warnPossible403Ban(provider, callbackError); console.log(fail(redactedCallbackError));
warnPossible403Ban(provider, redactedCallbackError);
trace.record( trace.record(
OAuthTracePhase.Error, OAuthTracePhase.Error,
{ status: callbackResponse.status }, { status: callbackResponse.status },
@@ -915,8 +917,9 @@ export async function handlePasteCallbackMode(
); );
if (tokenWaitError) { if (tokenWaitError) {
console.log(fail(tokenWaitError)); const redactedTokenWaitError = redactString(tokenWaitError);
warnPossible403Ban(provider, tokenWaitError); console.log(fail(redactedTokenWaitError));
warnPossible403Ban(provider, redactedTokenWaitError);
trace.record( trace.record(
OAuthTracePhase.Error, OAuthTracePhase.Error,
{}, {},
@@ -1,7 +1,7 @@
import { OAuthTraceEvent, OAuthTracePhase, OAuthTraceSink } from './trace-events'; import { OAuthTraceEvent, OAuthTracePhase, OAuthTraceSink } from './trace-events';
import { createMemorySink } from './sink-memory'; import { createMemorySink } from './sink-memory';
import { createVerboseStdoutSink } from './sink-verbose-stdout'; import { createVerboseStdoutSink } from './sink-verbose-stdout';
import { redactJsonShallow } from './redactor'; import { redactJsonShallow, redactString } from './redactor';
export interface OAuthTraceRecorder { export interface OAuthTraceRecorder {
record( record(
@@ -53,9 +53,9 @@ export function createOAuthTraceRecorder(options: OAuthTraceRecorderOptions): OA
): { code?: string; message: string } | undefined { ): { code?: string; message: string } | undefined {
if (!err) return undefined; if (!err) return undefined;
if (err instanceof Error) { if (err instanceof Error) {
return { message: err.message }; return { message: redactString(err.message) };
} }
return { code: err.code, message: err.message }; return { code: err.code, message: redactString(err.message) };
} }
return { return {