Skip to content

Commit dfa8567

Browse files
committed
fix(bedrock): build role-session Bedrock clients per request instead of filling the shared client cache
An assumed or keyless role resolves to a fresh STS session on every call, so a session-keyed cache entry was never read again and pushed other providers' warm clients out of the shared LRU. Long-lived saved keys stay cached. The session type now derives from AwsSessionCredentials, and authorization refusals carry a reason.
1 parent 3cf2717 commit dfa8567

4 files changed

Lines changed: 69 additions & 53 deletions

File tree

‎apps/sim/lib/internal/llm/bedrock-credential.ts‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,21 @@ import { recordCredentialAccess } from '@/lib/oauth/token-resolution'
66
import { AWS_SERVICE_ACCOUNT_PROVIDER_ID } from '@/lib/oauth/types'
77
import type { BedrockCredentialSession } from '@/providers/types'
88

9-
/** A saved AWS credential could not be used for a Bedrock request. */
9+
/**
10+
* A saved AWS credential could not be used for a Bedrock request: the caller is
11+
* not allowed to use it (`unauthorized`), or it is not a live AWS credential or
12+
* AWS refused it (`unusable`).
13+
*/
1014
export class BedrockCredentialError extends Error {
11-
constructor(message: string, options?: { cause?: unknown }) {
12-
super(message, options)
15+
readonly reason: 'unauthorized' | 'unusable'
16+
17+
constructor(
18+
message: string,
19+
options?: { reason?: 'unauthorized' | 'unusable'; cause?: unknown }
20+
) {
21+
super(message, { cause: options?.cause })
1322
this.name = 'BedrockCredentialError'
23+
this.reason = options?.reason ?? 'unusable'
1424
}
1525
}
1626

@@ -50,15 +60,19 @@ export async function resolveBedrockCredentialSession({
5060
signal,
5161
}: ResolveBedrockCredentialParams): Promise<BedrockCredentialSession> {
5262
if (!actingUserId) {
53-
throw new BedrockCredentialError('Using an AWS credential requires an authenticated user')
63+
throw new BedrockCredentialError('Using an AWS credential requires an authenticated user', {
64+
reason: 'unauthorized',
65+
})
5466
}
5567

5668
const access = await authorizeCredentialUseForAuth(
5769
{ success: true, userId: actingUserId, authType: AuthType.INTERNAL_JWT },
5870
{ credentialId, workflowId, workspaceId, callerUserId: actingUserId }
5971
)
6072
if (!access.ok) {
61-
throw new BedrockCredentialError(access.error || 'Not authorized to use this AWS credential')
73+
throw new BedrockCredentialError(access.error || 'Not authorized to use this AWS credential', {
74+
reason: 'unauthorized',
75+
})
6276
}
6377
if (
6478
access.credentialType !== 'service_account' ||
@@ -99,6 +113,5 @@ export async function resolveBedrockCredentialSession({
99113
accessKeyId: session.accessKeyId,
100114
secretAccessKey: session.secretAccessKey,
101115
...(session.sessionToken ? { sessionToken: session.sessionToken } : {}),
102-
...(session.expiration ? { expiration: session.expiration } : {}),
103116
}
104117
}

‎apps/sim/providers/bedrock/index.test.ts‎

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,6 @@ describe('bedrockProvider credential handling', () => {
169169
accessKeyId: 'ASIASESSIONKEY000001',
170170
secretAccessKey: 'session-secret',
171171
sessionToken: 'session-token',
172-
expiration: new Date('2030-01-01T00:00:00Z'),
173172
},
174173
})
175174

@@ -202,30 +201,29 @@ describe('bedrockProvider credential handling', () => {
202201
expect(mockGetAwsCredentials).not.toHaveBeenCalled()
203202
})
204203

205-
it('reuses one client per session and builds a new one when the session changes', async () => {
206-
const session = {
207-
credentialId: 'cred-1',
208-
accessKeyId: 'ASIASESSIONKEY000001',
209-
secretAccessKey: 'session-secret',
210-
sessionToken: 'session-token',
211-
expiration: new Date('2030-01-01T00:00:00Z'),
204+
it("keeps other clients warm while a role credential's STS sessions come and go", async () => {
205+
const inlineKeys = {
206+
bedrockAccessKeyId: 'AKIAIOSFODNN7EXAMPLE',
207+
bedrockSecretKey: 'wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY',
212208
}
209+
await bedrockProvider.executeRequest({ ...baseRequest, ...inlineKeys })
213210
vi.mocked(BedrockRuntimeClient).mockClear()
214211

215-
await bedrockProvider.executeRequest({ ...baseRequest, bedrockSession: session })
216-
await bedrockProvider.executeRequest({ ...baseRequest, bedrockSession: { ...session } })
217-
expect(BedrockRuntimeClient).toHaveBeenCalledTimes(1)
212+
for (let call = 0; call < 1_000; call++) {
213+
await bedrockProvider.executeRequest({
214+
...baseRequest,
215+
bedrockSession: {
216+
credentialId: 'cred-role',
217+
accessKeyId: `ASIASESSIONKEY${call}`,
218+
secretAccessKey: `session-secret-${call}`,
219+
sessionToken: `session-token-${call}`,
220+
},
221+
})
222+
}
223+
vi.mocked(BedrockRuntimeClient).mockClear()
218224

219-
await bedrockProvider.executeRequest({
220-
...baseRequest,
221-
bedrockSession: {
222-
...session,
223-
accessKeyId: 'ASIASESSIONKEY000002',
224-
sessionToken: 'next-token',
225-
expiration: new Date('2030-01-01T01:00:00Z'),
226-
},
227-
})
228-
expect(BedrockRuntimeClient).toHaveBeenCalledTimes(2)
225+
await bedrockProvider.executeRequest({ ...baseRequest, ...inlineKeys })
226+
expect(BedrockRuntimeClient).not.toHaveBeenCalled()
229227
})
230228

231229
it('builds a new client when a long-lived saved key pair is corrected', async () => {

‎apps/sim/providers/bedrock/index.ts‎

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -125,28 +125,31 @@ function enrichLastModelSegmentFromBedrockResponse(
125125

126126
/**
127127
* The credentials a Bedrock client signs with, and the part of its cache key
128-
* that names them. A saved credential's session is used alone, so selecting one
129-
* never falls back to inline keys or the deployment's own AWS credentials. Its
130-
* key names the credential and session expiry plus a digest of the key material,
131-
* so a new session or a corrected key pair gets a fresh client without the
132-
* cache key holding a secret.
128+
* that names them, or `null` when the client must not be cached. A saved
129+
* credential's session is used alone, so selecting one never falls back to
130+
* inline keys or the deployment's own AWS credentials. An STS session (assumed
131+
* or keyless role) is fresh on every resolve, so its client is built for the one
132+
* request rather than filling the shared client cache with entries never read
133+
* again. A long-lived saved key pair is keyed by credential plus a digest of the
134+
* key material, so a corrected pair gets a fresh client without the cache key
135+
* holding a secret.
133136
*/
134137
function resolveBedrockClientCredentials(request: ProviderRequest): {
135138
credentials: BedrockRuntimeClientConfig['credentials']
136-
credentialKey: string
139+
credentialKey: string | null
137140
} {
138141
const session = request.bedrockSession
139142
if (session) {
140143
const { accessKeyId, secretAccessKey, sessionToken } = session
144+
if (sessionToken) {
145+
return { credentials: { accessKeyId, secretAccessKey, sessionToken }, credentialKey: null }
146+
}
141147
return {
142-
credentials: sessionToken
143-
? { accessKeyId, secretAccessKey, sessionToken }
144-
: { accessKeyId, secretAccessKey },
148+
credentials: { accessKeyId, secretAccessKey },
145149
credentialKey: [
146150
'credential',
147151
session.credentialId,
148-
session.expiration?.getTime() ?? 'long-lived',
149-
sha256Hex([accessKeyId, secretAccessKey, sessionToken ?? ''].join('\0')),
152+
sha256Hex([accessKeyId, secretAccessKey].join('\0')),
150153
].join(':'),
151154
}
152155
}
@@ -204,10 +207,12 @@ export const bedrockProvider: ProviderConfig = {
204207
})
205208

206209
const { credentials, credentialKey } = resolveBedrockClientCredentials(request)
207-
const client = getCachedProviderClient(
208-
`bedrock::${region}::${credentialKey}`,
209-
() => new BedrockRuntimeClient({ region, credentials, requestHandler: awsRequestHandler() })
210-
)
210+
const buildClient = () =>
211+
new BedrockRuntimeClient({ region, credentials, requestHandler: awsRequestHandler() })
212+
const client =
213+
credentialKey === null
214+
? buildClient()
215+
: getCachedProviderClient(`bedrock::${region}::${credentialKey}`, buildClient)
211216

212217
const { messages, systemContent } = convertBedrockRequestHistory(request)
213218
const canonicalModelId = `bedrock/${getBedrockBaseModelId(request.model)}`

‎apps/sim/providers/types.ts‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { BillingAttributionSnapshot } from '@/lib/billing/core/attribution'
22
import { UserFailure } from '@/lib/core/errors/user-failure'
3+
import type { AwsSessionCredentials } from '@/lib/credentials/aws-service-account.server'
34
import type { CustomBlockInputFieldType } from '@/blocks/custom/build-config'
45
import type { ProviderTimingSegment, StreamingExecution, UserFile } from '@/executor/types'
56

@@ -193,17 +194,16 @@ export interface EvaluationInput {
193194
questions: unknown
194195
}
195196

196-
/** Signing credentials resolved from a saved AWS credential for one Bedrock request. */
197-
export interface BedrockCredentialSession {
198-
/** The saved credential the session came from; scopes the provider client cache. */
199-
credentialId: string
200-
accessKeyId: string
201-
secretAccessKey: string
202-
/** Present for STS sessions (assumed and keyless roles); absent for long-lived keys. */
203-
sessionToken?: string
204-
/** When an STS session ends; absent for long-lived keys. */
205-
expiration?: Date
206-
}
197+
/**
198+
* Signing credentials resolved from a saved AWS credential for one Bedrock
199+
* request. `sessionToken` is present for STS sessions (assumed and keyless
200+
* roles) and absent for long-lived keys; `credentialId` names the saved
201+
* credential the session came from.
202+
*/
203+
export type BedrockCredentialSession = Pick<
204+
AwsSessionCredentials,
205+
'accessKeyId' | 'secretAccessKey' | 'sessionToken'
206+
> & { credentialId: string }
207207

208208
export interface ProviderRequest {
209209
evaluation?: EvaluationInput

0 commit comments

Comments
 (0)