Skip to content

Commit b274f9d

Browse files
committed
fix(credentials): keep Snowflake, Microsoft app, and Google keyless subjects unique
The subject-uniqueness scan from #9866 covered only AWS and Databricks. It now also scans Snowflake, Microsoft app, and Google service account credentials, so no two live credentials of an owner hold one keyless subject. Their keyed secrets that carry no config (a Google JSON key, a Snowflake programmatic access token) hold no subject and no longer read as unreadable. The three keyless save paths report a newly bound subject so create checks it.
1 parent a635fdf commit b274f9d

9 files changed

Lines changed: 177 additions & 6 deletions

‎apps/sim/lib/credentials/google-workload-identity.server.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ describe('saving a keyless Google Cloud credential', () => {
264264

265265
await expect(
266266
save({ subject: grant.subject, subjectProof: grant.subjectProof }, owner)
267-
).resolves.toMatchObject({ serviceAccountEmail: SA_EMAIL })
267+
).resolves.toMatchObject({ serviceAccountEmail: SA_EMAIL, newSubject: grant.subject })
268268
})
269269

270270
it.each([

‎apps/sim/lib/credentials/google-workload-identity.server.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,8 @@ export interface GoogleWorkloadIdentitySaveResult {
347347
serviceAccountEmail: string
348348
/** The workload identity provider's resource name. */
349349
workloadIdentityProvider: string
350+
/** A subject this save binds that the credential did not already hold. */
351+
newSubject?: string
350352
}
351353

352354
/**
@@ -405,6 +407,9 @@ export async function verifyAndEncryptGoogleWorkloadIdentityCredential(
405407
encryptedServiceAccountKey: encrypted,
406408
serviceAccountEmail: config.serviceAccountEmail,
407409
workloadIdentityProvider: googleWorkloadIdentityProviderName(config),
410+
...(config.subject !== readStoredSubject(owner.storedSecret)
411+
? { newSubject: config.subject }
412+
: {}),
408413
}
409414
}
410415

‎apps/sim/lib/credentials/microsoft-app-service-account.server.test.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,21 @@ describe('saving a Microsoft app credential', () => {
231231
expect(used).toContain(`sim-oidc|api://AzureADTokenExchange|${SUBJECT}`)
232232
})
233233

234+
it('reports a newly issued subject so the save can check no other credential holds it', async () => {
235+
const grant = issueWorkloadIdentitySubject('microsoft', { workspaceId: 'ws-1' }, 'user-1')
236+
fakeMicrosoft(fetch, {
237+
acceptAssertion: (assertion) =>
238+
assertion.endsWith(`|${grant.subject}`) || assertion.endsWith(`|${SUBJECT}`),
239+
})
240+
241+
await expect(
242+
save({ authType: 'federated', subject: grant.subject, subjectProof: grant.subjectProof })
243+
).resolves.toMatchObject({ newSubject: grant.subject })
244+
await expect(save({ authType: 'federated' }, reconnectOwner)).resolves.not.toHaveProperty(
245+
'newSubject'
246+
)
247+
})
248+
234249
it('refuses an app that also accepts a subject no credential holds', async () => {
235250
fakeMicrosoft(fetch, { acceptAssertion: () => true })
236251

‎apps/sim/lib/credentials/microsoft-app-service-account.server.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,8 @@ export interface MicrosoftAppCredentialSaveResult {
521521
/** The app's service principal object ID in the tenant. */
522522
servicePrincipalId: string
523523
tenantId: string
524+
/** A subject this save binds that the credential did not already hold. */
525+
newSubject?: string
524526
}
525527

526528
/**
@@ -584,6 +586,9 @@ export async function verifyAndEncryptMicrosoftAppCredential(
584586
displayName: verified.value.displayName ?? config.clientId,
585587
servicePrincipalId: verified.value.id,
586588
tenantId: config.tenantId,
589+
...(config.authType === 'federated' && config.subject !== readStoredSubject(owner.storedSecret)
590+
? { newSubject: config.subject }
591+
: {}),
587592
}
588593
}
589594

‎apps/sim/lib/credentials/service-account-secret.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,8 @@ export interface ServiceAccountSecretResult {
120120
/** Slack custom bot: the derived bot user id (for reaction self-drop). */
121121
botUserId?: string
122122
/**
123-
* Keyless AWS or Databricks: a subject this save binds that the credential did
124-
* not already hold, which must not belong to another credential of the owner.
123+
* Keyless AWS, Databricks, Snowflake, Microsoft app, or Google: a subject this save binds that
124+
* the credential did not already hold, which must not belong to another credential of the owner.
125125
*/
126126
newWorkloadIdentitySubject?: string
127127
}
@@ -301,6 +301,7 @@ async function buildGoogleWorkloadIdentitySecret(
301301
...serviceAccountPrincipalMetadata(principal),
302302
},
303303
principal,
304+
newWorkloadIdentitySubject: result.newSubject,
304305
}
305306
}
306307

@@ -391,6 +392,7 @@ async function buildMicrosoftAppServiceAccountSecret(
391392
...serviceAccountPrincipalMetadata(principal),
392393
},
393394
principal,
395+
newWorkloadIdentitySubject: result.newSubject,
394396
}
395397
}
396398

‎apps/sim/lib/credentials/snowflake-service-account.server.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ describe('saving a keyless Snowflake connection', () => {
117117
)
118118

119119
expect(saved.displayName).toBe('SIM_WORKFLOWS (MYORG-MYACCOUNT)')
120+
expect(saved.newWorkloadIdentitySubject).toBe(grant.subject)
120121
expect(secureFetch.mock.calls[0][0]).toBe(`https://${HOST}/api/v2/statements`)
121122
expect(secureFetch.mock.calls[0][1]).toMatchObject({ maxRedirects: 0 })
122123
expect(fetch).not.toHaveBeenCalled()

‎apps/sim/lib/credentials/snowflake-service-account.server.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,8 @@ export interface SnowflakeKeylessSecret {
130130
displayName: string
131131
auditMetadata: Record<string, string>
132132
principal: ServiceAccountPrincipal | null
133+
/** A subject this save binds that the credential did not already hold. */
134+
newWorkloadIdentitySubject?: string
133135
}
134136

135137
/**
@@ -194,6 +196,9 @@ export async function verifyAndEncryptSnowflakeKeylessCredential(
194196
displayName: validation.displayName,
195197
auditMetadata: { ...validation.auditMetadata, ...principalMetadata },
196198
principal: validation.principal,
199+
...(subject !== readStoredSubject(owner.storedSecret)
200+
? { newWorkloadIdentitySubject: subject }
201+
: {}),
197202
}
198203
}
199204

‎apps/sim/lib/credentials/workload-identity/subject.test.ts‎

Lines changed: 117 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,20 @@
11
/**
22
* @vitest-environment node
33
*/
4-
import { describe, expect, it } from 'vitest'
4+
import { flattenMockConditions } from '@sim/testing'
5+
import { encryptionMock, encryptionMockFns } from '@sim/testing/mocks/encryption.mock'
6+
import { beforeEach, describe, expect, it, vi } from 'vitest'
7+
8+
vi.mock('@/lib/core/security/encryption', () => encryptionMock)
9+
510
import {
11+
assertWorkloadIdentitySubjectUnclaimed,
612
bindWorkloadIdentitySubject,
713
issueWorkloadIdentitySubject,
814
verifyWorkloadIdentitySubject,
915
WorkloadIdentitySubjectError,
1016
} from '@/lib/credentials/workload-identity/subject'
17+
import type { DbTransaction } from '@/lib/db/types'
1118

1219
const owner = { workspaceId: 'ws-1' }
1320
const now = Date.UTC(2026, 10, 6)
@@ -170,3 +177,112 @@ describe('binding a subject to a saved credential', () => {
170177
).toThrow(WorkloadIdentitySubjectError)
171178
})
172179
})
180+
181+
describe('keyless subject uniqueness across providers', () => {
182+
const SUBJECT = 'sim:workspace:ws-1:credential:abcdef0123456789'
183+
184+
interface StoredCredential {
185+
id: string
186+
providerId: string
187+
secret: object
188+
}
189+
190+
/**
191+
* A transaction whose credential query returns only the rows its provider
192+
* filter admits, so a provider missing from the scan is never checked.
193+
*/
194+
function transactionWith(rows: StoredCredential[]): DbTransaction {
195+
encryptionMockFns.mockDecryptSecret.mockImplementation(async (encrypted: string) => ({
196+
decrypted: JSON.stringify(rows.find((row) => row.id === encrypted)?.secret),
197+
}))
198+
const tx = {
199+
execute: async () => [],
200+
select: () => ({
201+
from: () => ({
202+
where: async (condition: unknown) => {
203+
const providerFilter = flattenMockConditions(condition).find(
204+
(node) => node.type === 'inArray'
205+
)
206+
const admitted = Array.isArray(providerFilter?.values) ? providerFilter.values : []
207+
return rows
208+
.filter((row) => admitted.includes(row.providerId))
209+
.map((row) => ({
210+
id: row.id,
211+
displayName: row.id,
212+
encryptedServiceAccountKey: row.id,
213+
}))
214+
},
215+
}),
216+
}),
217+
}
218+
return tx as unknown as DbTransaction
219+
}
220+
221+
function claim(rows: StoredCredential[]) {
222+
return assertWorkloadIdentitySubjectUnclaimed(transactionWith(rows), {
223+
subject: SUBJECT,
224+
scope: { kind: 'workspace', workspaceId: 'ws-1' },
225+
})
226+
}
227+
228+
beforeEach(() => {
229+
encryptionMockFns.mockDecryptSecret.mockReset()
230+
})
231+
232+
it.each([
233+
{
234+
providerId: 'snowflake-service-account',
235+
secret: {
236+
type: 'snowflake_workload_identity_v1',
237+
config: {
238+
authType: 'workload_identity',
239+
host: 'org-acct.snowflakecomputing.com',
240+
subject: SUBJECT,
241+
},
242+
},
243+
},
244+
{
245+
providerId: 'microsoft-app-service-account',
246+
secret: {
247+
type: 'microsoft_app_credential_v1',
248+
config: { authType: 'federated', tenantId: 't', clientId: 'c', subject: SUBJECT },
249+
},
250+
},
251+
{
252+
providerId: 'google-service-account',
253+
secret: {
254+
type: 'google_workload_identity_v1',
255+
config: { projectNumber: '1', poolId: 'pool', providerId: 'prov', subject: SUBJECT },
256+
},
257+
},
258+
])(
259+
'refuses a subject a live $providerId credential already holds',
260+
async ({ providerId, secret }) => {
261+
await expect(claim([{ id: 'held', providerId, secret }])).rejects.toThrow(
262+
/Another credential already uses this trust setup/
263+
)
264+
}
265+
)
266+
267+
it('is not blocked by key-based Google and Snowflake credentials, which hold no subject', async () => {
268+
await expect(
269+
claim([
270+
{
271+
id: 'json-key',
272+
providerId: 'google-service-account',
273+
secret: { type: 'service_account', client_email: 'sa@p.iam.gserviceaccount.com' },
274+
},
275+
{
276+
id: 'pat',
277+
providerId: 'snowflake-service-account',
278+
secret: { type: 'token_service_account', apiToken: 'pat', domain: 'x' },
279+
},
280+
{
281+
id: 'client-secret',
282+
providerId: 'microsoft-app-service-account',
283+
secret: { type: 'microsoft_app_credential_v1', config: { authType: 'client_secret' } },
284+
},
285+
])
286+
).resolves.toBeUndefined()
287+
})
288+
})

‎apps/sim/lib/credentials/workload-identity/subject.ts‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,26 @@ import { env } from '@/lib/core/config/env'
99
import type { ResourceScope } from '@/lib/core/resource-scope'
1010
import { resourceScopeCondition } from '@/lib/core/resource-scope.server'
1111
import { decryptSecret } from '@/lib/core/security/encryption'
12+
import {
13+
SNOWFLAKE_SERVICE_ACCOUNT_PROVIDER_ID,
14+
TOKEN_SERVICE_ACCOUNT_SECRET_TYPE,
15+
} from '@/lib/credentials/token-service-accounts/descriptors'
1216
import type { WorkloadIdentityProvider } from '@/lib/credentials/workload-identity/audiences'
1317
import type { WorkloadIdentityOwner } from '@/lib/credentials/workload-identity/issuer'
1418
import { acquireAdvisoryXactLock } from '@/lib/db/advisory-locks'
1519
import type { DbTransaction } from '@/lib/db/types'
1620
import {
1721
AWS_SERVICE_ACCOUNT_PROVIDER_ID,
1822
DATABRICKS_SERVICE_ACCOUNT_PROVIDER_ID,
23+
GOOGLE_SERVICE_ACCOUNT_PROVIDER_ID,
24+
MICROSOFT_APP_SERVICE_ACCOUNT_PROVIDER_ID,
1925
} from '@/lib/oauth/types'
2026

2127
const logger = createLogger('WorkloadIdentitySubject')
2228

29+
/** The `type` of a Google JSON key, stored verbatim. */
30+
const GOOGLE_SERVICE_ACCOUNT_KEY_TYPE = 'service_account'
31+
2332
/**
2433
* Credential-scoped workload-identity subjects.
2534
*
@@ -179,18 +188,31 @@ export function bindWorkloadIdentitySubject(input: {
179188
const KEYLESS_PROVIDER_IDS = [
180189
AWS_SERVICE_ACCOUNT_PROVIDER_ID,
181190
DATABRICKS_SERVICE_ACCOUNT_PROVIDER_ID,
191+
SNOWFLAKE_SERVICE_ACCOUNT_PROVIDER_ID,
192+
MICROSOFT_APP_SERVICE_ACCOUNT_PROVIDER_ID,
193+
GOOGLE_SERVICE_ACCOUNT_PROVIDER_ID,
182194
]
183195

184-
/** The part of an AWS or Databricks stored secret that can hold a bound subject. */
196+
/** The part of a keyless provider's stored secret that can hold a bound subject. */
185197
const storedSubjectSchema = z.object({
186198
config: z.object({ subject: z.string().optional() }),
187199
})
188200

201+
/**
202+
* Keyed secrets of those providers that have no `config` at all: a Google JSON
203+
* key and a Snowflake programmatic access token. They hold no subject.
204+
*/
205+
const keyedSecretSchema = z.object({
206+
type: z.enum([GOOGLE_SERVICE_ACCOUNT_KEY_TYPE, TOKEN_SERVICE_ACCOUNT_SECRET_TYPE]),
207+
})
208+
189209
/** The subject a stored secret holds, `undefined` for a keyed mode, or `null` when unreadable. */
190210
async function readStoredSubject(encrypted: string): Promise<string | undefined | null> {
191211
try {
192212
const { decrypted } = await decryptSecret(encrypted)
193-
const parsed = storedSubjectSchema.safeParse(JSON.parse(decrypted))
213+
const blob: unknown = JSON.parse(decrypted)
214+
if (keyedSecretSchema.safeParse(blob).success) return undefined
215+
const parsed = storedSubjectSchema.safeParse(blob)
194216
return parsed.success ? parsed.data.config.subject : null
195217
} catch {
196218
return null

0 commit comments

Comments
 (0)