Skip to content

Commit 6ddf50a

Browse files
committed
fix(mcp): match the MCP host by full authority; keep legacy API keys off the OAuth token prefix
1 parent c60ef06 commit 6ddf50a

4 files changed

Lines changed: 40 additions & 6 deletions

File tree

‎apps/sim/lib/api-key/crypto.test.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@
1010
*/
1111
import { randomBytes } from 'crypto'
1212
import { resetEnvMock, setEnv } from '@sim/testing'
13-
import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'
13+
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
14+
15+
const { mockGenerateSecureToken } = vi.hoisted(() => ({ mockGenerateSecureToken: vi.fn() }))
16+
vi.mock('@sim/security/tokens', () => ({ generateSecureToken: mockGenerateSecureToken }))
1417

1518
beforeAll(() => {
1619
setEnv({ API_ENCRYPTION_KEY: undefined })
@@ -21,6 +24,7 @@ afterAll(resetEnvMock)
2124
import {
2225
decryptApiKey,
2326
encryptApiKey,
27+
generateApiKey,
2428
hashApiKey,
2529
isEncryptedApiKeyFormat,
2630
isLegacyApiKeyFormat,
@@ -86,3 +90,11 @@ describe('api-key format helpers', () => {
8690
expect(isEncryptedApiKeyFormat('sim_abc')).toBe(false)
8791
})
8892
})
93+
94+
describe('generateApiKey', () => {
95+
it('never issues a legacy key that reads as an OAuth access token', () => {
96+
mockGenerateSecureToken.mockReturnValueOnce('oat_collision').mockReturnValueOnce('plain_token')
97+
expect(generateApiKey()).toBe('sim_plain_token')
98+
expect(mockGenerateSecureToken).toHaveBeenCalledTimes(2)
99+
})
100+
})

‎apps/sim/lib/api-key/crypto.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { decrypt, encrypt } from '@sim/security/encryption'
33
import { sha256Hex } from '@sim/security/hash'
44
import { generateSecureToken } from '@sim/security/tokens'
55
import { toError } from '@sim/utils/errors'
6+
import { OAUTH_ACCESS_TOKEN_PREFIX } from '@/lib/auth/oauth-provider'
67
import { env } from '@/lib/core/config/env'
78

89
const logger = createLogger('ApiKeyCrypto')
@@ -60,10 +61,17 @@ export async function decryptApiKey(encryptedValue: string): Promise<{ decrypted
6061

6162
/**
6263
* Generates a standardized API key with the 'sim_' prefix (legacy format)
64+
*
65+
* Never one starting with the OAuth access-token prefix: base64url can spell
66+
* `sim_oat_`, and a bearer credential's prefix is what tells an OAuth token
67+
* from an API key.
6368
* @returns A new API key string
6469
*/
6570
export function generateApiKey(): string {
66-
return `sim_${generateSecureToken(24)}`
71+
for (;;) {
72+
const key = `sim_${generateSecureToken(24)}`
73+
if (!key.startsWith(OAUTH_ACCESS_TOKEN_PREFIX)) return key
74+
}
6775
}
6876

6977
/**

‎apps/sim/lib/api/mcp/host-routing.test.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,13 @@ describe('Sim MCP host routing', () => {
6161
}
6262
)
6363

64+
it('tells the MCP host from an app on the same hostname but another port', () => {
65+
mocks.mcpUrl = 'http://localhost:3001/mcp'
66+
expect(resolveSimMcpHostPath('localhost:3000', '/workspace')).toBeNull()
67+
expect(resolveSimMcpHostPath('localhost:3001', '/mcp')).toBe('/api/mcp')
68+
expect(resolveSimMcpHostPath('localhost:3001', '/workspace')).toBe('not_found')
69+
})
70+
6471
it('serves the app host as before, without a second MCP URL', () => {
6572
expect(resolveSimMcpHostPath('sim.ai', '/mcp')).toBeNull()
6673
expect(resolveSimMcpHostPath('sim.ai', '/workspace')).toBeNull()

‎apps/sim/lib/api/mcp/host-routing.ts‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,16 @@ import { getBaseUrl } from '@/lib/core/utils/urls'
44
const PROTECTED_RESOURCE_METADATA = '/.well-known/oauth-protected-resource'
55
const AUTHORIZATION_SERVER_METADATA = '/.well-known/oauth-authorization-server'
66

7-
/** A `Host` header's hostname: lower-cased, without port or the trailing root dot. */
8-
function hostnameOf(host: string): string {
9-
return host.toLowerCase().replace(/:\d+$/, '').replace(/\.$/, '')
7+
/**
8+
* A `Host` header as a URL authority under `protocol`: lower-cased, without the
9+
* trailing root dot, and without the scheme's default port, so it compares
10+
* equal to `URL.host`. `null` when the header is not a valid authority.
11+
*/
12+
function authorityOf(host: string, protocol: string): string | null {
13+
const normalized = host.replace(/\.(?=:\d+$|$)/, '')
14+
return URL.canParse(`${protocol}//${normalized}`)
15+
? new URL(`${protocol}//${normalized}`).host
16+
: null
1017
}
1118

1219
/**
@@ -30,7 +37,7 @@ export function resolveSimMcpHostPath(
3037
const mcp = new URL(getSimMcpUrl())
3138
const dedicated = mcp.origin !== new URL(getBaseUrl()).origin
3239
const internalMetadataPath = `${PROTECTED_RESOURCE_METADATA}${SIM_MCP_ROUTE_PATH}`
33-
if (!host || hostnameOf(host) !== mcp.hostname) {
40+
if (!host || authorityOf(host, mcp.protocol) !== mcp.host) {
3441
return dedicated && (pathname === SIM_MCP_ROUTE_PATH || pathname === internalMetadataPath)
3542
? 'not_found'
3643
: null

0 commit comments

Comments
 (0)