Skip to content

Commit 49ef21a

Browse files
committed
fix(api-keys): never echo an internal key-creation failure to the caller
The unauthenticated CLI poll route and the settings routes returned the raw error message for an internal failure. Return a fixed message and keep the cause in the server log; a conflict still names the existing key. The CLI's comment now says a 400 can also be a refused key creation.
1 parent d3d4d9b commit 49ef21a

3 files changed

Lines changed: 63 additions & 7 deletions

File tree

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { dbChainMockFns, resetDbChainMock } from '@sim/testing'
2+
import { beforeEach, describe, expect, it, vi } from 'vitest'
3+
4+
const { createWorkspaceKey } = vi.hoisted(() => ({ createWorkspaceKey: vi.fn() }))
5+
vi.mock('@/lib/api-key/auth', () => ({
6+
createApiKey: vi.fn(),
7+
createWorkspaceApiKey: createWorkspaceKey,
8+
}))
9+
10+
import {
11+
performCreatePersonalApiKey,
12+
performCreateWorkspaceApiKey,
13+
} from '@/lib/api-key/orchestration'
14+
15+
const INTERNAL_DETAIL = 'connect ECONNREFUSED 10.0.0.5:5432 for role april'
16+
17+
describe('API key creation failures', () => {
18+
beforeEach(() => {
19+
resetDbChainMock()
20+
createWorkspaceKey.mockReset()
21+
})
22+
23+
it('never echoes an internal personal-key failure to the caller', async () => {
24+
dbChainMockFns.limit.mockRejectedValueOnce(new Error(INTERNAL_DETAIL))
25+
26+
const result = await performCreatePersonalApiKey({ userId: 'user-1', name: 'CLI' })
27+
28+
expect(result).toMatchObject({ success: false, errorCode: 'internal' })
29+
expect(result.error).not.toContain('ECONNREFUSED')
30+
})
31+
32+
it('never echoes an internal workspace-key failure, but still names a conflict', async () => {
33+
createWorkspaceKey.mockRejectedValueOnce(new Error(INTERNAL_DETAIL))
34+
const internal = await performCreateWorkspaceApiKey({
35+
workspaceId: 'ws-1',
36+
userId: 'user-1',
37+
name: 'CLI',
38+
})
39+
expect(internal).toMatchObject({ success: false, errorCode: 'internal' })
40+
expect(internal.error).not.toContain('ECONNREFUSED')
41+
42+
createWorkspaceKey.mockRejectedValueOnce(new Error('A key named "CLI" already exists'))
43+
const conflict = await performCreateWorkspaceApiKey({
44+
workspaceId: 'ws-1',
45+
userId: 'user-1',
46+
name: 'CLI',
47+
})
48+
expect(conflict).toEqual({
49+
success: false,
50+
errorCode: 'conflict',
51+
error: 'A key named "CLI" already exists',
52+
})
53+
})
54+
})

‎apps/sim/lib/api-key/orchestration/index.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,8 @@ export async function performCreatePersonalApiKey(
208208
return { success: true, key: { ...created, key: plainKey } }
209209
} catch (error) {
210210
logger.error('Failed to create personal API key', { error })
211-
return { success: false, errorCode: 'internal', error: toError(error).message }
211+
// The CLI poll route is unauthenticated, so an internal failure never echoes its cause.
212+
return { success: false, errorCode: 'internal', error: 'Failed to create API key' }
212213
}
213214
}
214215

@@ -266,10 +267,10 @@ export async function performCreateWorkspaceApiKey(
266267
} catch (error) {
267268
const message = toError(error).message
268269
logger.error('Failed to create workspace API key', { error })
269-
return {
270-
success: false,
271-
error: message,
272-
errorCode: message.includes('already exists') ? 'conflict' : 'internal',
270+
if (message.includes('already exists')) {
271+
return { success: false, error: message, errorCode: 'conflict' }
273272
}
273+
// The CLI poll route is unauthenticated, so an internal failure never echoes its cause.
274+
return { success: false, error: 'Failed to create API key', errorCode: 'internal' }
274275
}
275276
}

‎packages/sim-cli/src/auth/device-flow.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,9 @@ const POLL_PATH = '/api/cli/auth/poll'
3939
* 429 is the poll cadence hitting the per-IP bucket, not a refusal.
4040
*
4141
* Everything else stays terminal: 400 means a malformed request id or verifier,
42-
* and 401/403/404 mean the server is refusing on purpose. Retrying those just
43-
* spins until the 15-minute timeout.
42+
* or a key the server refused to create as requested, which the same poll would
43+
* be refused again for; 401/403/404 mean the server is refusing on purpose.
44+
* Retrying those just spins until the 15-minute timeout.
4445
*/
4546
const RETRYABLE_POLL_STATUSES = new Set([409, 429, 500, 502, 503, 504])
4647

0 commit comments

Comments
 (0)