Skip to content

Commit 110f604

Browse files
committed
fix(credentials): age-gate 403 token eviction and stop memoizing transient mint failures
An internal operation's 403 now drops this instance's cached client-credential token only once it is older than a minute, matching the external-request path, so a 403 from a file-access refusal or per-record permission no longer re-mints on every call. The coordinated token's failure memo keeps only refusals of the credentials themselves; provider_unavailable propagates with no memo row.
1 parent d026560 commit 110f604

6 files changed

Lines changed: 169 additions & 32 deletions

File tree

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { clientCredentialToken } from '@sim/db/schema'
5+
import { dbChainMockFns, queueTableRows, resetDbChainMock } from '@sim/testing/mocks/database.mock'
6+
import { encryptionMock, encryptionMockFns } from '@sim/testing/mocks/encryption.mock'
7+
import { beforeEach, describe, expect, it, vi } from 'vitest'
8+
import {
9+
type CoordinatedClientTokenInput,
10+
resolveCoordinatedClientToken,
11+
} from '@/lib/credentials/coordinated-client-token'
12+
import {
13+
type TokenServiceAccountValidationCode,
14+
TokenServiceAccountValidationError,
15+
} from '@/lib/credentials/token-service-accounts/errors'
16+
17+
vi.mock('@/lib/core/security/encryption', () => encryptionMock)
18+
19+
beforeEach(() => {
20+
resetDbChainMock()
21+
encryptionMockFns.mockEncryptSecret.mockImplementation(async (secret: string) => ({
22+
encrypted: secret,
23+
}))
24+
encryptionMockFns.mockDecryptSecret.mockImplementation(async (value: string) => ({
25+
decrypted: value,
26+
}))
27+
})
28+
29+
function refusing(code: TokenServiceAccountValidationCode, status: number) {
30+
return vi.fn<CoordinatedClientTokenInput['exchange']>(async () => {
31+
throw new TokenServiceAccountValidationError(code, status, { step: 'mint' })
32+
})
33+
}
34+
35+
/**
36+
* Resolves once, then again against the rows the first resolution wrote, so the second call sees
37+
* any failure memo exactly as another worker reading the shared table would.
38+
*/
39+
async function resolveTwice(exchange: CoordinatedClientTokenInput['exchange']) {
40+
const input: CoordinatedClientTokenInput = {
41+
namespace: 'fixture',
42+
application: ['application'],
43+
clientSecret: 'secret',
44+
step: 'mint',
45+
exchange,
46+
}
47+
const first = await resolveCoordinatedClientToken(input).catch((error: unknown) => error)
48+
const written = dbChainMockFns.values.mock.calls.map(([row]) => row)
49+
queueTableRows(clientCredentialToken, [])
50+
queueTableRows(clientCredentialToken, written)
51+
const second = await resolveCoordinatedClientToken(input).catch((error: unknown) => error)
52+
return { first, second }
53+
}
54+
55+
describe('resolveCoordinatedClientToken failure memo', () => {
56+
it('replays a refused exchange instead of calling the provider again', async () => {
57+
const exchange = refusing('invalid_credentials', 401)
58+
59+
const { first, second } = await resolveTwice(exchange)
60+
61+
expect(first).toMatchObject({ code: 'invalid_credentials', status: 401 })
62+
expect(second).toMatchObject({ code: 'invalid_credentials', status: 401 })
63+
expect(exchange).toHaveBeenCalledOnce()
64+
})
65+
66+
it.each([
67+
['a timeout or 5xx', 503],
68+
['a 429', 429],
69+
])('asks the provider again after %s instead of replaying it', async (_label, status) => {
70+
const exchange = refusing('provider_unavailable', status)
71+
72+
const { first, second } = await resolveTwice(exchange)
73+
74+
expect(first).toMatchObject({ code: 'provider_unavailable', status })
75+
expect(second).toMatchObject({ code: 'provider_unavailable', status })
76+
expect(exchange).toHaveBeenCalledTimes(2)
77+
})
78+
})

‎apps/sim/lib/credentials/coordinated-client-token.ts‎

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -44,18 +44,27 @@ interface StoredApplicationToken {
4444
secretFingerprint: string
4545
}
4646

47+
type StoredFailureCode = 'invalid_credentials' | 'grant_not_permitted' | 'site_not_found'
48+
4749
interface StoredFailure {
48-
code: 'invalid_credentials' | 'grant_not_permitted' | 'site_not_found' | 'provider_unavailable'
50+
code: StoredFailureCode
4951
status: number
5052
}
5153

52-
const STORED_FAILURE_CODES: ReadonlySet<string> = new Set([
54+
/**
55+
* Refusals that say the credentials themselves are wrong, so replaying them is safe. A transient
56+
* failure (`provider_unavailable`: a timeout, 5xx, 429, or malformed reply) is never remembered.
57+
*/
58+
const STORED_FAILURE_CODES: ReadonlySet<string> = new Set<StoredFailureCode>([
5359
'invalid_credentials',
5460
'grant_not_permitted',
5561
'site_not_found',
56-
'provider_unavailable',
5762
])
5863

64+
function isStoredFailureCode(code: string): code is StoredFailureCode {
65+
return STORED_FAILURE_CODES.has(code)
66+
}
67+
5968
function digest(value: unknown): string {
6069
return createHash('sha256').update(JSON.stringify(value)).digest('hex')
6170
}
@@ -132,11 +141,11 @@ function decodeFailure(value: string, step: string): StoredFailure {
132141
if (
133142
!isRecordLike(parsed) ||
134143
typeof parsed.code !== 'string' ||
135-
!STORED_FAILURE_CODES.has(parsed.code) ||
144+
!isStoredFailureCode(parsed.code) ||
136145
typeof parsed.status !== 'number'
137146
)
138147
throw unavailable(step, 'stored token failure is malformed')
139-
return { code: parsed.code as StoredFailure['code'], status: parsed.status }
148+
return { code: parsed.code, status: parsed.status }
140149
}
141150

142151
function remainingSeconds(expiresAt: Date): number {
@@ -154,9 +163,10 @@ function isServable(expiresAt: Date): boolean {
154163
* to reuse one token rather than accumulate them. Minting happens only under a
155164
* transaction-scoped advisory lock keyed by the application, so two workers never race two
156165
* tokens into existence; a token minted with a different secret is never served, and a token the
157-
* provider rejects is expired through {@link invalidateCoordinatedClientToken}. A rejected
158-
* exchange is remembered for 30 seconds so a hot workflow on a revoked secret does not hammer the
159-
* token endpoint. Tokens are encrypted at rest.
166+
* provider rejects is expired through {@link invalidateCoordinatedClientToken}. An exchange the
167+
* provider refuses for the credentials themselves is remembered for 30 seconds so a hot workflow on
168+
* a revoked secret does not hammer the token endpoint; a transient failure is not remembered.
169+
* Tokens are encrypted at rest.
160170
*/
161171
export async function resolveCoordinatedClientToken(
162172
input: CoordinatedClientTokenInput
@@ -225,13 +235,12 @@ export async function resolveCoordinatedClientToken(
225235
expiresInSeconds: remainingSeconds(exchanged.expiresAt),
226236
}
227237
} catch (error) {
228-
if (!(error instanceof TokenServiceAccountValidationError)) throw error
229-
const failure: StoredFailure = {
230-
code: STORED_FAILURE_CODES.has(error.code)
231-
? (error.code as StoredFailure['code'])
232-
: 'provider_unavailable',
233-
status: error.status,
234-
}
238+
if (
239+
!(error instanceof TokenServiceAccountValidationError) ||
240+
!isStoredFailureCode(error.code)
241+
)
242+
throw error
243+
const failure: StoredFailure = { code: error.code, status: error.status }
235244
const { encrypted } = await encryptSecret(JSON.stringify(failure))
236245
const expiresAt = new Date(Date.now() + FAILURE_TTL_MS)
237246
await tx

‎apps/sim/lib/oauth/credential-service-cancellation.test.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,4 +125,30 @@ describe('client credential token cache', () => {
125125
await expect(resolve()).resolves.toEqual({ accessToken: 'replacement-token' })
126126
expect(mint).toHaveBeenCalledTimes(2)
127127
})
128+
it('keeps a token minted within the minimum age, and drops it once it is older', async () => {
129+
vi.useFakeTimers({ toFake: ['Date'] })
130+
try {
131+
const mint = vi.fn<clientAccounts.ClientCredentialAccountMinter>()
132+
mint
133+
.mockResolvedValueOnce({ ...token, accessToken: 'refused-token' })
134+
.mockResolvedValueOnce({ ...token, accessToken: 'replacement-token' })
135+
vi.spyOn(clientAccounts, 'getClientCredentialAccountMinter').mockReturnValue(mint)
136+
const resolve = () => {
137+
queueTableRows(credential, [storedCredential])
138+
return resolveServiceAccountToken('ramp-refused-aged', RAMP_SERVICE_ACCOUNT_PROVIDER_ID)
139+
}
140+
141+
await expect(resolve()).resolves.toEqual({ accessToken: 'refused-token' })
142+
vi.advanceTimersByTime(30_000)
143+
forgetRejectedClientCredentialToken('refused-token', { minimumAgeMs: 60_000 })
144+
await expect(resolve()).resolves.toEqual({ accessToken: 'refused-token' })
145+
146+
vi.advanceTimersByTime(31_000)
147+
forgetRejectedClientCredentialToken('refused-token', { minimumAgeMs: 60_000 })
148+
await expect(resolve()).resolves.toEqual({ accessToken: 'replacement-token' })
149+
expect(mint).toHaveBeenCalledTimes(2)
150+
} finally {
151+
vi.useRealTimers()
152+
}
153+
})
128154
})

‎apps/sim/lib/oauth/credential-service.ts‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -624,6 +624,7 @@ export interface ServiceAccountTokenResult {
624624
interface CachedClientCredentialToken {
625625
/** The minted token and the destination fields it is bound to, exactly as tools receive them. */
626626
token: ServiceAccountTokenResult
627+
mintedAtMs: number
627628
expiresAtMs: number
628629
/**
629630
* Fingerprint of the encrypted secret the token was minted from (see
@@ -694,11 +695,19 @@ function pruneExpiredClientCredentialCaches(nowMs: number): void {
694695
* refused it (a revoked API client, or a token minted before the client was
695696
* granted a scope), so the next resolution mints a replacement instead of
696697
* serving the refused token until it nears expiry. Matching on the token value
697-
* leaves alone a replacement another call has already minted.
698+
* leaves alone a replacement another call has already minted. With
699+
* `minimumAgeMs`, a token minted more recently is kept: a refusal that may come
700+
* from the endpoint rather than the token then re-mints at most once per that
701+
* interval instead of once per call.
698702
*/
699-
export function forgetRejectedClientCredentialToken(accessToken: string): void {
703+
export function forgetRejectedClientCredentialToken(
704+
accessToken: string,
705+
options: { minimumAgeMs?: number } = {}
706+
): void {
707+
const mintedBeforeMs = Date.now() - (options.minimumAgeMs ?? 0)
700708
for (const [identity, entry] of clientCredentialTokenCache) {
701-
if (entry.token.accessToken === accessToken) clientCredentialTokenCache.delete(identity)
709+
if (entry.token.accessToken === accessToken && entry.mintedAtMs <= mintedBeforeMs)
710+
clientCredentialTokenCache.delete(identity)
702711
}
703712
}
704713

@@ -778,6 +787,7 @@ async function resolveMintedServiceAccountToken(
778787
if (!sharedToken)
779788
clientCredentialTokenCache.set(cacheIdentity, {
780789
token: minted.token,
790+
mintedAtMs: Date.now(),
781791
expiresAtMs: Date.now() + minted.expiresInSeconds * 1000,
782792
secretFingerprint,
783793
})

‎apps/sim/tools/index.test.ts‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3823,15 +3823,21 @@ describe('Internal Route Trust', () => {
38233823
)
38243824
}
38253825

3826-
it.each([401, 403])(
3827-
'drops the cached token after a %i so the next call mints one',
3828-
async (status) => {
3829-
const result = await runWith('service_account', status)
3826+
it('drops the cached token after a 401 so the next call mints one', async () => {
3827+
const result = await runWith('service_account', 401)
38303828

3831-
expect(result.success).toBe(false)
3832-
expect(mockForgetRejectedClientCredentialToken).toHaveBeenCalledWith('minted-token')
3833-
}
3834-
)
3829+
expect(result.success).toBe(false)
3830+
expect(mockForgetRejectedClientCredentialToken).toHaveBeenCalledWith('minted-token')
3831+
})
3832+
3833+
it('drops the cached token after a 403 only once it is old enough to blame', async () => {
3834+
const result = await runWith('service_account', 403)
3835+
3836+
expect(result.success).toBe(false)
3837+
expect(mockForgetRejectedClientCredentialToken).toHaveBeenCalledWith('minted-token', {
3838+
minimumAgeMs: 60_000,
3839+
})
3840+
})
38353841

38363842
it('keeps the token after a call the provider accepts', async () => {
38373843
const result = await runWith('service_account', 200)

‎apps/sim/tools/index.ts‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2957,7 +2957,11 @@ async function executeDeclaredInternalOperation({
29572957
authorizedServiceAccountCredential &&
29582958
typeof params.accessToken === 'string'
29592959
) {
2960-
await forgetRefusedServiceAccountToken(params.accessToken, requestId)
2960+
await forgetRefusedServiceAccountToken(
2961+
params.accessToken,
2962+
requestId,
2963+
result.status === 403 ? REJECTED_TOKEN_MINIMUM_AGE_MS : undefined
2964+
)
29612965
}
29622966
if (result instanceof Response && result.status === 401 && refreshCredential) {
29632967
await result.body?.cancel()
@@ -3042,8 +3046,8 @@ async function executeDeclaredInternalOperation({
30423046
*/
30433047
const SCOPE_REFUSAL_PATTERN = /scope does not match|insufficient_scope/i
30443048
/**
3045-
* A tool call's 401 can come from the endpoint rather than the token, so a token minted within the
3046-
* last minute is kept: a call that keeps failing re-mints its application's token at most once a
3049+
* A tool call's 401 (and an internal operation's 403) can come from the endpoint rather than the
3050+
* token, so a token minted within the last minute is kept: a call that keeps failing re-mints its application's token at most once a
30473051
* minute, instead of once per call.
30483052
*/
30493053
const REJECTED_TOKEN_MINIMUM_AGE_MS = 60_000
@@ -3094,15 +3098,19 @@ async function expireRejectedServiceAccountToken(
30943098
* Drops a refused service-account token from this instance's client-credential cache: an internal
30953099
* operation answers 401 or 403, and an external request 401, when the provider refused it. The
30963100
* next call mints a replacement: providers such as CrowdStrike Falcon and SailPoint fix a token's
3097-
* scopes when it is minted, so a scope granted since then only reaches a new token.
3101+
* scopes when it is minted, so a scope granted since then only reaches a new token. A 403 can also
3102+
* come from Sim's own file-access refusal or a per-record permission, so the internal-operation
3103+
* path passes `minimumAgeMs` for it and a token minted within that window is kept.
30983104
*/
30993105
async function forgetRefusedServiceAccountToken(
31003106
accessToken: string,
3101-
requestId: string
3107+
requestId: string,
3108+
minimumAgeMs?: number
31023109
): Promise<void> {
31033110
try {
31043111
const { forgetRejectedClientCredentialToken } = await import('@/lib/oauth/credential-service')
3105-
forgetRejectedClientCredentialToken(accessToken)
3112+
if (minimumAgeMs === undefined) forgetRejectedClientCredentialToken(accessToken)
3113+
else forgetRejectedClientCredentialToken(accessToken, { minimumAgeMs })
31063114
} catch (error) {
31073115
logger.warn('Could not drop a service-account token the provider refused', {
31083116
requestId,

0 commit comments

Comments
 (0)