fix(mcp): serialize OAuth and XAA refresh across processes (#2093)

* fix(mcp): serialize OAuth and XAA refresh across processes

Normal OAuth refresh, reactive 401 recovery, and silent XAA exchange can otherwise race shared secure-storage writes between processes. Coordinate them on one server-scoped lock and re-read storage so waiters reuse persisted winners.

* fix(mcp): harden refresh follow-up paths

Use asynchronous cache-bypass reads on request paths while preserving the adjacent final record merge and write. Make the XAA concurrency fixtures independent of module import order and extend abort, redaction, and retry coverage.

* fix(mcp): honor aborts after credential reads

Check the active cancellation signal after asynchronous secure-storage reads so fresh-token fast paths cannot return credentials to an aborted request. Cover cancellation while a cache-bypassing read is pending.

src/services/mcp/client.test.ts

@@ -1,12 +1,18 @@
 import assert from 'node:assert/strict'
 import test from 'node:test'
+import { UnauthorizedError } from '@modelcontextprotocol/sdk/client/auth.js'
+import { SSEClientTransport } from '@modelcontextprotocol/sdk/client/sse.js'
+import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js'
 
 import {
   appendBoundedMcpStderr,
+  buildMcpSseEventSourceHeaders,
+  buildMcpSseRequestHeaders,
   cleanupFailedConnection,
   buildMcpStdioCommand,
   logMcpServerStderr,
 } from './client.js'
+import { wrapFetchWithStepUpDetection } from './auth.js'
 import {
   _resetErrorLogForTesting,
   attachErrorLogSink,
@@ -36,6 +42,170 @@ function withCapturedMcpLogEvents(
   }
 }
 
+test('buildMcpSseEventSourceHeaders preserves a refreshed Headers bearer', () => {
+  const headers = buildMcpSseEventSourceHeaders(
+    new Headers({
+      Authorization: 'Bearer refreshed-access-secret',
+      'User-Agent': 'custom-agent',
+      Accept: 'application/json',
+    }),
+  )
+
+  assert.equal(headers.get('Authorization'), 'Bearer refreshed-access-secret')
+  assert.equal(headers.get('User-Agent'), 'custom-agent')
+  assert.equal(headers.get('Accept'), 'text/event-stream')
+})
+
+test('buildMcpSseRequestHeaders preserves SDK headers and explicit precedence', () => {
+  const sdkHeaders = new Headers({
+    Authorization: 'Bearer sdk-stale-secret',
+    'MCP-Protocol-Version': '2025-06-18',
+  })
+  const providerHeaders = buildMcpSseRequestHeaders(sdkHeaders, {
+    'X-Configured': 'configured-value',
+  })
+  const explicitHeaders = buildMcpSseRequestHeaders(sdkHeaders, {
+    Authorization: 'Bearer configured-secret',
+  })
+
+  assert.equal(providerHeaders.get('Authorization'), 'Bearer sdk-stale-secret')
+  assert.equal(providerHeaders.get('MCP-Protocol-Version'), '2025-06-18')
+  assert.equal(providerHeaders.get('X-Configured'), 'configured-value')
+  assert.equal(
+    explicitHeaders.get('Authorization'),
+    'Bearer configured-secret',
+  )
+})
+
+function makeNeedsAuthTransportFixture() {
+  const resourceUrl = 'https://mcp.example.test/mcp'
+  const resourceMetadataUrl =
+    'https://mcp.example.test/.well-known/oauth-protected-resource'
+  const authorizationServerUrl = 'https://auth.example.test'
+  let resourceAuthorization: string | null = null
+  let metadataAuthorization: string | null = null
+  let redirectCalls = 0
+  const provider = {
+    redirectUrl: 'http://127.0.0.1:31337/callback',
+    clientMetadata: {
+      client_name: 'OpenClaude test',
+      redirect_uris: ['http://127.0.0.1:31337/callback'],
+      grant_types: ['authorization_code'],
+      response_types: ['code'],
+      token_endpoint_auth_method: 'none',
+    },
+    clientInformation: async () => ({ client_id: 'test-client' }),
+    tokens: async () => undefined,
+    state: async () => 'test-state',
+    saveCodeVerifier: async () => {},
+    redirectToAuthorization: async () => {
+      redirectCalls++
+    },
+    prepareRequest: async () => ({ access_token: 'resource-access-secret' }),
+    refreshAfterUnauthorized: async () => undefined,
+    markStepUpPending: () => {},
+  }
+  const baseFetch = async (
+    input: string | URL | Request,
+    init?: RequestInit,
+  ): Promise<Response> => {
+    const url = input.toString()
+    const authorization = new Headers(init?.headers).get('Authorization')
+    if (url === resourceUrl) {
+      resourceAuthorization = authorization
+      return new Response(null, {
+        status: 401,
+        headers: {
+          'WWW-Authenticate':
+            `Bearer resource_metadata="${resourceMetadataUrl}"`,
+        },
+      })
+    }
+    metadataAuthorization = authorization
+    if (url === resourceMetadataUrl) {
+      return Response.json({
+        resource: resourceUrl,
+        authorization_servers: [authorizationServerUrl],
+      })
+    }
+    if (
+      url ===
+      `${authorizationServerUrl}/.well-known/oauth-authorization-server`
+    ) {
+      return Response.json({
+        issuer: authorizationServerUrl,
+        authorization_endpoint: `${authorizationServerUrl}/authorize`,
+        token_endpoint: `${authorizationServerUrl}/token`,
+        response_types_supported: ['code'],
+        grant_types_supported: ['authorization_code'],
+        code_challenge_methods_supported: ['S256'],
+      })
+    }
+    return new Response(null, { status: 404 })
+  }
+  const wrappedFetch = wrapFetchWithStepUpDetection(
+    baseFetch,
+    provider as never,
+    { resourceUrl, providerOwnsAuthorization: true },
+  )
+  return {
+    provider,
+    resourceUrl,
+    wrappedFetch,
+    getResourceAuthorization: () => resourceAuthorization,
+    getMetadataAuthorization: () => metadataAuthorization,
+    getRedirectCalls: () => redirectCalls,
+  }
+}
+
+test('HTTP failed recovery reaches UnauthorizedError without leaking the resource bearer to OAuth metadata', async () => {
+  const fixture = makeNeedsAuthTransportFixture()
+  const transport = new StreamableHTTPClientTransport(
+    new URL(fixture.resourceUrl),
+    {
+      authProvider: fixture.provider as never,
+      fetch: fixture.wrappedFetch,
+    },
+  )
+  await transport.start()
+  try {
+    await assert.rejects(
+      transport.send({ jsonrpc: '2.0', id: 1, method: 'ping' }),
+      UnauthorizedError,
+    )
+  } finally {
+    await transport.close()
+  }
+
+  assert.equal(fixture.getMetadataAuthorization(), null)
+  assert.equal(
+    fixture.getResourceAuthorization(),
+    'Bearer resource-access-secret',
+  )
+  assert.equal(fixture.getRedirectCalls(), 1)
+})
+
+test('SSE failed recovery reaches UnauthorizedError without leaking the resource bearer to OAuth metadata', async () => {
+  const fixture = makeNeedsAuthTransportFixture()
+  const transport = new SSEClientTransport(new URL(fixture.resourceUrl), {
+    authProvider: fixture.provider as never,
+    fetch: fixture.wrappedFetch,
+    eventSourceInit: { fetch: fixture.wrappedFetch },
+  })
+  try {
+    await assert.rejects(transport.start(), UnauthorizedError)
+  } finally {
+    await transport.close()
+  }
+
+  assert.equal(fixture.getMetadataAuthorization(), null)
+  assert.equal(
+    fixture.getResourceAuthorization(),
+    'Bearer resource-access-secret',
+  )
+  assert.equal(fixture.getRedirectCalls(), 1)
+})
+
 test('cleanupFailedConnection awaits transport close before resolving', async () => {
   let closed = false
   let resolveClose: (() => void) | undefined

src/services/mcp/client.ts

@@ -574,6 +574,28 @@ type InProcessMcpServer = {
 const MAX_MCP_STDERR_CHARS = 256 * 1024
 const MCP_STDERR_TRUNCATED_MARKER = '\n...[stderr truncated]'
 
+export function buildMcpSseEventSourceHeaders(
+  initHeaders: HeadersInit | undefined,
+): Headers {
+  const headers = new Headers(initHeaders)
+  if (!headers.has('User-Agent')) {
+    headers.set('User-Agent', getMCPUserAgent())
+  }
+  headers.set('Accept', 'text/event-stream')
+  return headers
+}
+
+export function buildMcpSseRequestHeaders(
+  initHeaders: HeadersInit | undefined,
+  combinedHeaders: Record<string, string>,
+): Headers {
+  const headers = new Headers(initHeaders)
+  new Headers(combinedHeaders).forEach((value, key) => {
+    headers.set(key, value)
+  })
+  return headers
+}
+
 export function appendBoundedMcpStderr(
   current: string,
   chunk: Buffer | string,
@@ -680,6 +702,9 @@ export const connectToServer = memoize(
 
         // Get combined headers (static + dynamic)
         const combinedHeaders = await getMcpServerHeaders(name, serverRef)
+        const allowUnauthorizedRefresh = !new Headers(combinedHeaders).has(
+          'Authorization',
+        )
 
         // Use the auth provider with SSEClientTransport
         const transportOptions: SSEClientTransportOptions = {
@@ -688,7 +713,11 @@ export const connectToServer = memoize(
           // Step-up detection wraps innermost so the 403 is seen before the
           // SDK's handler calls auth() → tokens().
           fetch: wrapFetchWithTimeout(
-            wrapFetchWithStepUpDetection(createFetchWithInit(), authProvider),
+            wrapFetchWithStepUpDetection(createFetchWithInit(), authProvider, {
+              allowUnauthorizedRefresh,
+              resourceUrl: serverRef.url,
+              providerOwnsAuthorization: allowUnauthorizedRefresh,
+            }),
           ),
           requestInit: {
             headers: {
@@ -703,27 +732,28 @@ export const connectToServer = memoize(
         // to receive server-sent events), so applying a 60-second timeout would kill it.
         // The timeout is only meant for individual API requests (POST, auth refresh), not
         // the persistent SSE stream.
-        transportOptions.eventSourceInit = {
-          fetch: async (url: string | URL, init?: RequestInit) => {
-            // Get auth headers from the auth provider
-            const authHeaders: Record<string, string> = {}
-            const tokens = await authProvider.tokens()
-            if (tokens) {
-              authHeaders.Authorization = `Bearer ${tokens.access_token}`
-            }
-
+        const eventSourceFetch = wrapFetchWithStepUpDetection(
+          async (url: string | URL, init?: RequestInit) => {
             const proxyOptions = getProxyFetchOptions()
             // eslint-disable-next-line eslint-plugin-n/no-unsupported-features/node-builtins
             return fetch(url, {
               ...init,
               ...proxyOptions,
-              headers: {
-                'User-Agent': getMCPUserAgent(),
-                ...authHeaders,
-                ...init?.headers,
-                ...combinedHeaders,
-                Accept: 'text/event-stream',
-              },
+              headers: buildMcpSseEventSourceHeaders(init?.headers),
+            })
+          },
+          authProvider,
+          {
+            allowUnauthorizedRefresh,
+            resourceUrl: serverRef.url,
+            providerOwnsAuthorization: allowUnauthorizedRefresh,
+          },
+        )
+        transportOptions.eventSourceInit = {
+          fetch: async (url: string | URL, init?: RequestInit) => {
+            return eventSourceFetch(url, {
+              ...init,
+              headers: buildMcpSseRequestHeaders(init?.headers, combinedHeaders),
             })
           },
         }
@@ -861,13 +891,16 @@ export const connectToServer = memoize(
 
         // Get combined headers (static + dynamic)
         const combinedHeaders = await getMcpServerHeaders(name, serverRef)
-
         // Check if this server has stored OAuth tokens. If so, the SDK's
         // authProvider will set Authorization — don't override with the
         // session ingress token (SDK merges requestInit AFTER authProvider).
         // CCR proxy URLs (ccr_shttp_mcp) have no stored OAuth, so they still
         // get the ingress token. See PR #24454 discussion.
         const hasOAuthTokens = !!(await authProvider.tokens())
+        const hasExplicitAuthorization =
+          new Headers(combinedHeaders).has('Authorization') ||
+          Boolean(sessionIngressToken && !hasOAuthTokens)
+        const allowUnauthorizedRefresh = !hasExplicitAuthorization
 
         // Use the auth provider with StreamableHTTPClientTransport
         const proxyOptions = getProxyFetchOptions()
@@ -882,7 +915,11 @@ export const connectToServer = memoize(
           // Step-up detection wraps innermost so the 403 is seen before the
           // SDK's handler calls auth() → tokens().
           fetch: wrapFetchWithTimeout(
-            wrapFetchWithStepUpDetection(createFetchWithInit(), authProvider),
+            wrapFetchWithStepUpDetection(createFetchWithInit(), authProvider, {
+              allowUnauthorizedRefresh,
+              resourceUrl: serverRef.url,
+              providerOwnsAuthorization: allowUnauthorizedRefresh,
+            }),
           ),
           requestInit: {
             ...proxyOptions,

src/services/mcp/refreshLock.ts

@@ -0,0 +1,241 @@
+import { createHash } from 'crypto'
+import { mkdir } from 'fs/promises'
+import { join } from 'path'
+import { raceAbort, throwIfAborted } from '../../utils/boundedAsync.js'
+import { createCombinedAbortSignal } from '../../utils/combinedAbortSignal.js'
+import { logForDebugging } from '../../utils/debug.js'
+import { getClaudeConfigHomeDir } from '../../utils/envUtils.js'
+import { getErrnoCode } from '../../utils/errors.js'
+import * as lockfile from '../../utils/lockfile.js'
+import { logMCPDebug } from '../../utils/log.js'
+import { sleep } from '../../utils/sleep.js'
+
+export const MCP_REFRESH_FRESHNESS_SECONDS = 300
+
+const MAX_LOCK_RETRIES = 5
+const LOCK_STALE_MS = 10_000
+const LOCK_UPDATE_MS = LOCK_STALE_MS / 2
+const LOCK_IO_TIMEOUT_MS = 5_000
+
+export type McpRefreshLockResult<T> = {
+  acquired: boolean
+  value: T
+}
+
+export type McpRefreshLockContext = {
+  acquired: boolean
+  signal: AbortSignal
+}
+
+export class McpRefreshLockUnavailableError extends Error {
+  constructor() {
+    super('MCP credential refresh lock is unavailable')
+    this.name = 'McpRefreshLockUnavailableError'
+  }
+}
+
+function getMcpRefreshLockIdentity(serverKey: string): string {
+  return createHash('sha256').update(serverKey).digest('hex').substring(0, 32)
+}
+
+function logRefreshWarning(lockIdentity: string, message: string): void {
+  try {
+    logForDebugging(`[mcp-refresh:${lockIdentity}] ${message}`, {
+      level: 'warn',
+    })
+  } catch {
+    // Refresh coordination and cleanup must not depend on diagnostics.
+  }
+}
+
+async function runBoundedLockIo<T>(
+  operation: Promise<T>,
+  signal: AbortSignal | undefined,
+): Promise<T> {
+  const combined = createCombinedAbortSignal(signal, {
+    timeoutMs: LOCK_IO_TIMEOUT_MS,
+  })
+  try {
+    return await raceAbort(
+      operation,
+      combined.signal,
+      'MCP refresh lock operation aborted',
+    )
+  } finally {
+    combined.cleanup()
+  }
+}
+
+async function releaseRefreshLock(
+  release: () => Promise<void>,
+): Promise<boolean> {
+  try {
+    // Never race release against a timeout or caller cancellation. A release
+    // that continues after this helper returns could remove a successor's
+    // lock directory and break the serialization boundary.
+    await release()
+    return true
+  } catch {
+    return false
+  }
+}
+
+async function acquireRefreshLock(
+  lockPath: string,
+  options: Parameters<typeof lockfile.lock>[1],
+  signal: AbortSignal | undefined,
+): Promise<() => Promise<void>> {
+  const combined = createCombinedAbortSignal(signal, {
+    timeoutMs: LOCK_IO_TIMEOUT_MS,
+  })
+  const acquisition = lockfile.lock(lockPath, options).then(async release => {
+    if (combined.signal.aborted) {
+      // The caller has already regained control. Clean up a late acquisition
+      // without inheriting the already-aborted request signal.
+      await releaseRefreshLock(release)
+      throw combined.signal.reason
+    }
+    return release
+  })
+  try {
+    return await raceAbort(
+      acquisition,
+      combined.signal,
+      'MCP refresh lock acquisition aborted',
+    )
+  } finally {
+    combined.cleanup()
+  }
+}
+
+export function getMcpRefreshLockPath(
+  serverKey: string,
+  configDir = getClaudeConfigHomeDir(),
+): string {
+  return join(
+    configDir,
+    `mcp-refresh-${getMcpRefreshLockIdentity(serverKey)}.lock`,
+  )
+}
+
+/**
+ * Runs one MCP credential refresh under the canonical server-scoped lock.
+ *
+ * After bounded acquisition failure, the operation gets one final
+ * fresh-storage check but must not perform a network refresh. Callers use the
+ * `acquired` flag to enforce that fail-closed policy consistently.
+ *
+ * This lock protects the provider's proactive normal OAuth refresh and silent
+ * XAA exchange for one server. It is not a global secure-storage lock: login,
+ * logout, manual token replacement, and other servers retain their existing
+ * coordination boundaries.
+ */
+export async function withMcpRefreshLock<T>(
+  serverName: string,
+  serverKey: string,
+  signal: AbortSignal | undefined,
+  operation: (context: McpRefreshLockContext) => Promise<T>,
+): Promise<McpRefreshLockResult<T>> {
+  throwIfAborted(signal, 'MCP token refresh aborted')
+
+  const configDir = getClaudeConfigHomeDir()
+  const lockPath = getMcpRefreshLockPath(serverKey, configDir)
+  const lockIdentity = getMcpRefreshLockIdentity(serverKey)
+  let release: (() => Promise<void>) | undefined
+  let acquisitionFailure: string | undefined
+  let canAttemptLock = true
+  const compromisedController = new AbortController()
+
+  try {
+    await runBoundedLockIo(mkdir(configDir, { recursive: true }), signal)
+  } catch (error) {
+    throwIfAborted(signal, 'MCP token refresh aborted')
+    acquisitionFailure = getErrnoCode(error) ?? 'directory-setup-timeout'
+    canAttemptLock = false
+  }
+
+  for (
+    let retry = 0;
+    canAttemptLock && retry < MAX_LOCK_RETRIES;
+    retry++
+  ) {
+    throwIfAborted(signal, 'MCP token refresh aborted')
+    try {
+      logMCPDebug(serverName, `Acquiring refresh lock (attempt ${retry + 1})`)
+      release = await acquireRefreshLock(
+        lockPath,
+        {
+          realpath: false,
+          retries: 0,
+          stale: LOCK_STALE_MS,
+          update: LOCK_UPDATE_MS,
+          onCompromised: () => {
+            // proper-lockfile invokes this callback from its update timer. Never
+            // allow a diagnostic failure to escape that timer as an unhandled
+            // exception; the active operation still owns its normal cleanup.
+            try {
+              logMCPDebug(serverName, 'Refresh lock was compromised')
+              logRefreshWarning(lockIdentity, 'refresh lock was compromised')
+            } catch {
+              // Diagnostics are best-effort in a compromised-lock callback.
+            }
+            compromisedController.abort(
+              new DOMException('MCP refresh lock compromised', 'AbortError'),
+            )
+          },
+        },
+        signal,
+      )
+      logMCPDebug(serverName, 'Acquired refresh lock')
+      break
+    } catch (error) {
+      throwIfAborted(signal, 'MCP token refresh aborted')
+      const code = getErrnoCode(error)
+      acquisitionFailure = code ?? 'unknown'
+      if (code !== 'ELOCKED') {
+        break
+      }
+      logMCPDebug(
+        serverName,
+        `Refresh lock held by another process, waiting (attempt ${retry + 1}/${MAX_LOCK_RETRIES})`,
+      )
+      if (retry < MAX_LOCK_RETRIES - 1) {
+        await sleep(1000 + Math.random() * 1000, signal)
+        throwIfAborted(signal, 'MCP token refresh aborted')
+      }
+    }
+  }
+
+  const acquired = release !== undefined
+  if (!acquired) {
+    logMCPDebug(
+      serverName,
+      `Could not acquire refresh lock (${acquisitionFailure ?? 'exhausted'}); refresh blocked`,
+    )
+    logRefreshWarning(
+      lockIdentity,
+      `refresh blocked after bounded lock acquisition failure (${acquisitionFailure ?? 'exhausted'})`,
+    )
+  }
+
+  const operationSignal = createCombinedAbortSignal(signal, {
+    signalB: compromisedController.signal,
+  })
+
+  try {
+    throwIfAborted(operationSignal.signal, 'MCP token refresh aborted')
+    return {
+      acquired,
+      value: await operation({ acquired, signal: operationSignal.signal }),
+    }
+  } finally {
+    if (release) {
+      if (await releaseRefreshLock(release)) {
+        logMCPDebug(serverName, 'Released refresh lock')
+      } else {
+        logMCPDebug(serverName, 'Failed to release refresh lock')
+      }
+    }
+    operationSignal.cleanup()
+  }
+}

src/services/mcp/xaa.test.ts

@@ -0,0 +1,123 @@
+import { expect, test } from 'bun:test'
+
+import {
+  exchangeJwtAuthGrant,
+  requestJwtAuthorizationGrant,
+} from './xaa.js'
+
+const ID_JAG_TOKEN_TYPE = 'urn:ietf:params:oauth:token-type:id-jag'
+
+function requestTokenExchange(body: unknown) {
+  return requestJwtAuthorizationGrant({
+    tokenEndpoint: 'https://idp.example.test/token',
+    audience: 'https://as.example.test',
+    resource: 'https://mcp.example.test/mcp',
+    idToken: 'identity-token',
+    clientId: 'idp-client',
+    fetchFn: async () =>
+      new Response(JSON.stringify(body), {
+        status: 200,
+        headers: { 'Content-Type': 'application/json' },
+      }),
+  })
+}
+
+test('XAA token-exchange errors never include provider-controlled secret text', async () => {
+  const echoedSecret = 'identity-secret-value-7Qm2'
+
+  const request = requestJwtAuthorizationGrant({
+    tokenEndpoint: 'https://idp.example.test/token',
+    audience: 'https://as.example.test',
+    resource: 'https://mcp.example.test/mcp',
+    idToken: echoedSecret,
+    clientId: 'idp-client',
+    fetchFn: async () =>
+      new Response(
+        JSON.stringify({
+          error: 'invalid_grant',
+          error_description: `provider echoed ${echoedSecret}`,
+        }),
+        { status: 400 },
+      ),
+  })
+
+  await expect(request).rejects.not.toThrow(echoedSecret)
+  await expect(request).rejects.toThrow(/HTTP 400/)
+})
+
+test('XAA jwt-bearer errors never include provider-controlled secret text', async () => {
+  const echoedSecret = 'assertion-secret-value-9Vr4'
+
+  const request = exchangeJwtAuthGrant({
+    tokenEndpoint: 'https://as.example.test/token',
+    assertion: echoedSecret,
+    clientId: 'as-client',
+    clientSecret: 'client-secret',
+    fetchFn: async () =>
+      new Response(
+        JSON.stringify({
+          error: 'invalid_grant',
+          error_description: `provider echoed ${echoedSecret}`,
+        }),
+        { status: 400 },
+      ),
+  })
+
+  await expect(request).rejects.not.toThrow(echoedSecret)
+  await expect(request).rejects.toThrow(/HTTP 400/)
+})
+
+test('XAA token-exchange schema errors redact successful response data', async () => {
+  const echoedSecret = 'schema-secret-value-3Fs8'
+  const request = requestTokenExchange({
+    access_token: 'id-jag',
+    issued_token_type: ID_JAG_TOKEN_TYPE,
+    expires_in: { echoedSecret },
+  })
+
+  await expect(request).rejects.not.toThrow(echoedSecret)
+  await expect(request).rejects.toThrow(/did not match expected shape/)
+})
+
+test('XAA missing-token errors redact successful response data', async () => {
+  const echoedSecret = 'missing-token-secret-value-4Gt9'
+  const request = requestTokenExchange({
+    issued_token_type: ID_JAG_TOKEN_TYPE,
+    scope: echoedSecret,
+  })
+
+  await expect(request).rejects.not.toThrow(echoedSecret)
+  await expect(request).rejects.toThrow(/missing access_token/)
+})
+
+test('XAA unexpected-token-type errors redact successful response data', async () => {
+  const echoedSecret = 'token-type-secret-value-5Hu0'
+  const request = requestTokenExchange({
+    access_token: echoedSecret,
+    issued_token_type: `unexpected-${echoedSecret}`,
+  })
+
+  await expect(request).rejects.not.toThrow(echoedSecret)
+  await expect(request).rejects.toThrow(/unexpected issued_token_type/)
+})
+
+test('XAA jwt-bearer schema errors redact successful response data', async () => {
+  const echoedSecret = 'jwt-schema-secret-value-6Iv1'
+  const request = exchangeJwtAuthGrant({
+    tokenEndpoint: 'https://as.example.test/token',
+    assertion: 'assertion',
+    clientId: 'as-client',
+    clientSecret: 'client-secret',
+    fetchFn: async () =>
+      new Response(
+        JSON.stringify({ access_token: { echoedSecret } }),
+        {
+          status: 200,
+          headers: { 'Content-Type': 'application/json' },
+        },
+      ),
+  })
+
+  await expect(request).rejects.not.toThrow(echoedSecret)
+  await expect(request).rejects.toThrow(/did not match expected shape/)
+})

src/services/mcp/xaa.ts

@@ -25,7 +25,6 @@ import { z } from 'zod/v4'
 import { createCombinedAbortSignal } from '../../utils/combinedAbortSignal.js'
 import { lazySchema } from '../../utils/lazySchema.js'
 import { logMCPDebug } from '../../utils/log.js'
-import { jsonStringify } from '../../utils/slowOperations.js'
 
 const XAA_REQUEST_TIMEOUT_MS = 30000
 
@@ -83,19 +82,6 @@ export class XaaTokenExchangeError extends Error {
   }
 }
 
-// Matches quoted values for known token-bearing keys regardless of nesting
-// depth. Works on both parsed-then-stringified bodies AND raw text() error
-// bodies from !res.ok paths — a misbehaving AS that echoes the request's
-// subject_token/assertion/client_secret in a 4xx error envelope must not leak
-// into debug logs.
-const SENSITIVE_TOKEN_RE =
-  /"(access_token|refresh_token|id_token|assertion|subject_token|client_secret)"\s*:\s*"[^"]*"/g
-
-function redactTokens(raw: unknown): string {
-  const s = typeof raw === 'string' ? raw : jsonStringify(raw)
-  return s.replace(SENSITIVE_TOKEN_RE, (_, k) => `"${k}":"[REDACTED]"`)
-}
-
 // ─── Zod Schemas ────────────────────────────────────────────────────────────
 
 const TokenExchangeResponseSchema = lazySchema(() =>
@@ -263,12 +249,12 @@ export async function requestJwtAuthorizationGrant(opts: {
     body: params,
   })
   if (!res.ok) {
-    const body = redactTokens(await res.text()).slice(0, 200)
+    await res.body?.cancel().catch(() => {})
     // 4xx → id_token rejected (invalid_grant etc.), clear cache.
     // 5xx → IdP outage, id_token may still be valid, preserve it.
     const shouldClear = res.status < 500
     throw new XaaTokenExchangeError(
-      `XAA: token exchange failed: HTTP ${res.status}: ${body}`,
+      `XAA: token exchange failed: HTTP ${res.status}`,
       shouldClear,
     )
   }
@@ -285,20 +271,20 @@ export async function requestJwtAuthorizationGrant(opts: {
   const exchangeParsed = TokenExchangeResponseSchema().safeParse(rawExchange)
   if (!exchangeParsed.success) {
     throw new XaaTokenExchangeError(
-      `XAA: token exchange response did not match expected shape: ${redactTokens(rawExchange)}`,
+      'XAA: token exchange response did not match expected shape',
       true,
     )
   }
   const result = exchangeParsed.data
   if (!result.access_token) {
     throw new XaaTokenExchangeError(
-      `XAA: token exchange response missing access_token: ${redactTokens(result)}`,
+      'XAA: token exchange response missing access_token',
       true,
     )
   }
   if (result.issued_token_type !== ID_JAG_TOKEN_TYPE) {
     throw new XaaTokenExchangeError(
-      `XAA: token exchange returned unexpected issued_token_type: ${result.issued_token_type}`,
+      'XAA: token exchange returned unexpected issued_token_type',
       true,
     )
   }
@@ -373,8 +359,8 @@ export async function exchangeJwtAuthGrant(opts: {
     body: params,
   })
   if (!res.ok) {
-    const body = redactTokens(await res.text()).slice(0, 200)
-    throw new Error(`XAA: jwt-bearer grant failed: HTTP ${res.status}: ${body}`)
+    await res.body?.cancel().catch(() => {})
+    throw new Error(`XAA: jwt-bearer grant failed: HTTP ${res.status}`)
   }
   let rawTokens: unknown
   try {
@@ -387,7 +373,7 @@ export async function exchangeJwtAuthGrant(opts: {
   const tokensParsed = JwtBearerResponseSchema().safeParse(rawTokens)
   if (!tokensParsed.success) {
     throw new Error(
-      `XAA: jwt-bearer response did not match expected shape: ${redactTokens(rawTokens)}`,
+      'XAA: jwt-bearer response did not match expected shape',
     )
   }
   return tokensParsed.data

src/services/mcp/xaaIdpLogin.test.ts

@@ -1,6 +1,7 @@
 import assert from 'node:assert/strict'
 import test from 'node:test'
 
+import { discoverOidc } from './xaaIdpLogin.js'
 import {
   shouldCompleteXaaIdpCallback,
   validateXaaIdpCallbackParams,
@@ -60,3 +61,38 @@ test('XAA IdP callback accepts authorization codes only when state matches', ()
     { type: 'state_mismatch' },
   )
 })
+
+test('discoverOidc aborts a pending discovery request', async () => {
+  const originalFetch = globalThis.fetch
+  const controller = new AbortController()
+  let markFetchStarted!: () => void
+  const fetchStarted = new Promise<void>(resolve => {
+    markFetchStarted = resolve
+  })
+
+  globalThis.fetch = ((_input: string | URL, init?: RequestInit) =>
+    new Promise<Response>((_resolve, reject) => {
+      markFetchStarted()
+      const signal = init?.signal
+      if (signal?.aborted) {
+        reject(signal.reason)
+        return
+      }
+      signal?.addEventListener('abort', () => reject(signal.reason), {
+        once: true,
+      })
+    })) as typeof globalThis.fetch
+
+  try {
+    const discovery = discoverOidc(
+      'https://idp.example.test',
+      controller.signal,
+    )
+    await fetchStarted
+    controller.abort(new DOMException('cancelled', 'AbortError'))
+
+    await assert.rejects(discovery, { name: 'AbortError' })
+  } finally {
+    globalThis.fetch = originalFetch
+  }
+})

src/services/mcp/xaaIdpLogin.ts

@@ -205,10 +205,11 @@ export function clearIdpClientSecret(idpIssuer: string): void {
 // the fix. Exported because auth.ts needs the same discovery.
 export async function discoverOidc(
   idpIssuer: string,
+  abortSignal?: AbortSignal,
 ): Promise<OpenIdProviderDiscoveryMetadata> {
   const base = idpIssuer.endsWith('/') ? idpIssuer : idpIssuer + '/'
   const url = new URL('.well-known/openid-configuration', base)
-  const { signal, cleanup } = createCombinedAbortSignal(undefined, {
+  const { signal, cleanup } = createCombinedAbortSignal(abortSignal, {
     timeoutMs: IDP_REQUEST_TIMEOUT_MS,
   })
   try {