refactor(mcp): tighten OAuth callback contract and registration metadata

- Validate callback query params via mcpOauthCallbackContract instead of
  raw searchParams.get, matching the rest of the MCP route surface.
- Drop non-RFC-7591 application_type field from dynamic client registration
  to avoid rejection by strict authorization servers.
- Collapse the pre-lock OAuth row load in createClient — the row is now
  loaded exclusively inside withMcpOauthRefreshLock, removing a redundant
  query and a stale-snapshot path.

Author: waleedlatif1

apps/sim/app/api/mcp/oauth/callback/route.ts

@@ -6,6 +6,8 @@ import { toError } from '@sim/utils/errors'
 import { and, eq, isNull } from 'drizzle-orm'
 import type { NextRequest } from 'next/server'
 import { NextResponse } from 'next/server'
+import { mcpOauthCallbackContract } from '@/lib/api/contracts/mcp'
+import { parseRequest } from '@/lib/api/server'
 import { getSession } from '@/lib/auth'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 import {
@@ -55,10 +57,11 @@ function htmlClose(
 }
 
 export const GET = withRouteHandler(async (request: NextRequest) => {
-  const url = new URL(request.url)
-  const state = url.searchParams.get('state')
-  const code = url.searchParams.get('code')
-  const errorParam = url.searchParams.get('error')
+  const parsed = await parseRequest(mcpOauthCallbackContract, request, {})
+  if (!parsed.success) {
+    return htmlClose('Malformed authorization callback.', false, 'missing_params')
+  }
+  const { state, code, error: errorParam } = parsed.data.query
 
   const initialRow = state ? await loadOauthRowByState(state).catch(() => null) : null
   const stateRowServerId = initialRow?.mcpServerId

apps/sim/lib/api/contracts/mcp.ts

@@ -447,6 +447,27 @@ export const startMcpOauthContract = defineRouteContract({
   },
 })
 
+/**
+ * Provider can return any subset depending on the outcome:
+ * - success: `state` + `code`
+ * - provider error: `error` + optional `error_description` + optional `state`
+ * - malformed callback: nothing
+ * All fields are optional so the route can render an HTML error page itself.
+ */
+export const mcpOauthCallbackQuerySchema = z.object({
+  state: z.string().optional(),
+  code: z.string().optional(),
+  error: z.string().optional(),
+  error_description: z.string().optional(),
+})
+
+export const mcpOauthCallbackContract = defineRouteContract({
+  method: 'GET',
+  path: '/api/mcp/oauth/callback',
+  query: mcpOauthCallbackQuerySchema,
+  response: { mode: 'text' },
+})
+
 export const getAllowedMcpDomainsContract = defineRouteContract({
   method: 'GET',
   path: '/api/settings/allowed-mcp-domains',

apps/sim/lib/mcp/oauth/provider.ts

@@ -64,13 +64,12 @@ export class SimMcpOauthProvider implements OAuthClientProvider {
   }
 
   get clientMetadata(): OAuthClientMetadata {
-    const meta: OAuthClientMetadata & { application_type?: string } = {
+    const meta: OAuthClientMetadata = {
       client_name: 'Sim',
       redirect_uris: [this.redirectUrl],
       grant_types: ['authorization_code', 'refresh_token'],
       response_types: ['code'],
       token_endpoint_auth_method: this.preregistered?.clientSecret ? 'client_secret_post' : 'none',
-      application_type: 'web',
     }
     if (this.scope) meta.scope = this.scope
     return meta

apps/sim/lib/mcp/service.ts

@@ -195,20 +195,12 @@ class McpService {
       throw new Error('OAuth MCP server requires both userId and workspaceId')
     }
 
-    const initialRow = await getOrCreateOauthRow({
-      mcpServerId: config.id,
-      userId,
-      workspaceId: config.workspaceId,
-    })
-    if (!initialRow.tokens) {
-      throw new McpOauthAuthorizationRequiredError(config.id, config.name)
-    }
-
-    // Re-read the row inside the lock so concurrent callers observe tokens
-    // written by a predecessor refresh, rather than the stale snapshot loaded
-    // before lock acquisition. Without this, the second caller's provider holds
-    // a rotated-out refresh token and the SDK trips `invalid_grant`.
-    return withMcpOauthRefreshLock(initialRow.id, async () => {
+    // Load the row inside the refresh lock so concurrent callers observe tokens
+    // written by a predecessor refresh, rather than a stale snapshot. Without
+    // this, the second caller's provider would hold a rotated-out refresh token
+    // and the SDK would trip `invalid_grant`. The lock is keyed on serverId
+    // since the row is per-server.
+    return withMcpOauthRefreshLock(config.id, async () => {
       const row = await getOrCreateOauthRow({
         mcpServerId: config.id,
         userId,