From 8bf096a516c4a25377e6eea6b4d2535ec8c73d5a Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 20 Jul 2026 11:43:39 -0700 Subject: [PATCH] improvement(mcp): trace OAuth state writes to diagnose invalid_state clobber MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The MCP OAuth callback fails with invalid_state because the authorization state is cleared/missing by the time the user authorizes — but clearState was silent, so the clobber never surfaced in logs. Log every state save and clear with a caller context so the exact source is visible on the next repro. --- apps/sim/app/api/mcp/oauth/callback/route.ts | 4 ++-- apps/sim/lib/mcp/oauth/provider.ts | 4 ++-- apps/sim/lib/mcp/oauth/storage.ts | 6 ++++-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/apps/sim/app/api/mcp/oauth/callback/route.ts b/apps/sim/app/api/mcp/oauth/callback/route.ts index f0b99eb522b..e6d5b73186c 100644 --- a/apps/sim/app/api/mcp/oauth/callback/route.ts +++ b/apps/sim/app/api/mcp/oauth/callback/route.ts @@ -90,7 +90,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => { if (errorParam) { logger.warn(`MCP OAuth callback received error: ${errorParam}`) - if (initialRow) await clearState(initialRow.id).catch(() => {}) + if (initialRow) await clearState(initialRow.id, 'callback:provider_error').catch(() => {}) return respond(`Authorization failed: ${errorParam}`, false, 'provider_error', stateRowServerId) } if (!state || !code) { @@ -157,7 +157,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => { } // Burn state before token exchange so a replayed callback cannot reuse it. - await clearState(row.id) + await clearState(row.id, 'callback:burn-before-exchange') const preregistered = await loadPreregisteredClient(server.id) const provider = new SimMcpOauthProvider({ row, preregistered }) diff --git a/apps/sim/lib/mcp/oauth/provider.ts b/apps/sim/lib/mcp/oauth/provider.ts index 9b130e06453..f567607e066 100644 --- a/apps/sim/lib/mcp/oauth/provider.ts +++ b/apps/sim/lib/mcp/oauth/provider.ts @@ -77,7 +77,7 @@ export class SimMcpOauthProvider implements OAuthClientProvider { async state(): Promise { const state = generateId() - await saveState(this.row.id, state) + await saveState(this.row.id, state, 'provider.state') return state } @@ -140,7 +140,7 @@ export class SimMcpOauthProvider implements OAuthClientProvider { } if (scope === 'all' || scope === 'verifier') { await clearVerifier(this.row.id) - await clearState(this.row.id) + await clearState(this.row.id, `invalidateCredentials:${scope}`) this.row.codeVerifier = null } } diff --git a/apps/sim/lib/mcp/oauth/storage.ts b/apps/sim/lib/mcp/oauth/storage.ts index b2e0a7b13b5..3db4ffda083 100644 --- a/apps/sim/lib/mcp/oauth/storage.ts +++ b/apps/sim/lib/mcp/oauth/storage.ts @@ -192,12 +192,13 @@ export async function saveCodeVerifier(rowId: string, verifier: string): Promise .where(eq(mcpServerOauth.id, rowId)) } -export async function saveState(rowId: string, state: string): Promise { +export async function saveState(rowId: string, state: string, context = 'unknown'): Promise { const now = new Date() await db .update(mcpServerOauth) .set({ state: hashState(state), stateCreatedAt: now, updatedAt: now }) .where(eq(mcpServerOauth.id, rowId)) + logger.info('MCP OAuth authorization state saved', { rowId, context }) } export async function clearTokens(rowId: string): Promise { @@ -221,11 +222,12 @@ export async function clearVerifier(rowId: string): Promise { .where(eq(mcpServerOauth.id, rowId)) } -export async function clearState(rowId: string): Promise { +export async function clearState(rowId: string, context = 'unknown'): Promise { await db .update(mcpServerOauth) .set({ state: null, stateCreatedAt: null, updatedAt: new Date() }) .where(eq(mcpServerOauth.id, rowId)) + logger.info('MCP OAuth authorization state cleared', { rowId, context }) } /**