Skip to content

Commit ea1c7b4

Browse files
committed
fix(mcp): final audit fixes — state TTL, authProvider guard, decrypt fallback
- loadOauthRowByState: enforce 10min TTL on state row to prevent replay of stale unconsumed states. - McpClient: throw on authType=oauth without an authProvider instead of silently falling back to header auth. - loadPreregisteredClient: tolerate decrypt failure by treating as no preregistered client rather than crashing the OAuth flow.
1 parent cf7e2a7 commit ea1c7b4

3 files changed

Lines changed: 27 additions & 5 deletions

File tree

apps/sim/lib/mcp/client.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,10 @@ export class McpClient {
9090
throw new McpError('URL required for Streamable HTTP transport')
9191
}
9292

93-
const useOauth = this.config.authType === 'oauth' && this.authProvider != null
93+
if (this.config.authType === 'oauth' && this.authProvider == null) {
94+
throw new McpError('OAuth MCP server requires an authProvider')
95+
}
96+
const useOauth = this.config.authType === 'oauth'
9497
this.transport = new StreamableHTTPClientTransport(new URL(this.config.url), {
9598
authProvider: useOauth ? this.authProvider : undefined,
9699
requestInit: useOauth ? undefined : { headers: this.config.headers },

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

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import type {
66
} from '@modelcontextprotocol/sdk/shared/auth.js'
77
import { db } from '@sim/db'
88
import { mcpServers } from '@sim/db/schema'
9+
import { createLogger } from '@sim/logger'
10+
import { toError } from '@sim/utils/errors'
911
import { generateId } from '@sim/utils/id'
1012
import { eq } from 'drizzle-orm'
1113
import { decryptSecret } from '@/lib/core/security/encryption'
@@ -22,6 +24,8 @@ import {
2224
saveTokens as saveTokensDb,
2325
} from '@/lib/mcp/oauth/storage'
2426

27+
const logger = createLogger('SimMcpOauthProvider')
28+
2529
export class McpOauthRedirectRequired extends Error {
2630
constructor(public readonly authorizationUrl: string) {
2731
super('MCP OAuth redirect required')
@@ -160,8 +164,16 @@ export async function loadPreregisteredClient(
160164
if (!row?.clientId) return undefined
161165
let clientSecret: string | undefined
162166
if (row.clientSecret) {
163-
const { decrypted } = await decryptSecret(row.clientSecret)
164-
clientSecret = decrypted
167+
try {
168+
const { decrypted } = await decryptSecret(row.clientSecret)
169+
clientSecret = decrypted
170+
} catch (error) {
171+
logger.warn('Failed to decrypt preregistered MCP OAuth client secret; ignoring', {
172+
serverId,
173+
error: toError(error).message,
174+
})
175+
return undefined
176+
}
165177
}
166178
return { clientId: row.clientId, clientSecret }
167179
}

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,15 @@ import type {
66
import { db } from '@sim/db'
77
import { mcpServerOauth } from '@sim/db/schema'
88
import { generateId } from '@sim/utils/id'
9-
import { and, eq } from 'drizzle-orm'
9+
import { and, eq, gt } from 'drizzle-orm'
1010
import { decryptSecret, encryptSecret } from '@/lib/core/security/encryption'
1111

1212
function hashState(state: string): string {
1313
return createHash('sha256').update(state).digest('hex')
1414
}
1515

16+
const STATE_TTL_MS = 10 * 60 * 1000
17+
1618
export interface McpOauthRow {
1719
id: string
1820
mcpServerId: string
@@ -109,7 +111,12 @@ export async function loadOauthRowByState(state: string): Promise<McpOauthRow |
109111
const [row] = await db
110112
.select()
111113
.from(mcpServerOauth)
112-
.where(eq(mcpServerOauth.state, hashState(state)))
114+
.where(
115+
and(
116+
eq(mcpServerOauth.state, hashState(state)),
117+
gt(mcpServerOauth.updatedAt, new Date(Date.now() - STATE_TTL_MS))
118+
)
119+
)
113120
.limit(1)
114121
if (!row) return null
115122
return {

0 commit comments

Comments
 (0)