Skip to content

Commit eb19a95

Browse files
committed
fix(credentials): close final-audit findings on keyless wording, issuer logs, and WhatsApp IDs
The Databricks refusal names the real cause (a subject_claim other than sub), the issuer failure log points at the key or issuer, the AWS troubleshooting leads with the message users see, rotation steps cover background workers, and a WhatsApp phone number ID can no longer leave its path segment.
1 parent cd58d5b commit eb19a95

7 files changed

Lines changed: 25 additions & 17 deletions

File tree

‎apps/docs/content/docs/integrations/aws-credentials.mdx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,15 @@ Generate a key with `openssl genpkey -algorithm RSA -pkeyopt rsa_keygen_bits:204
6464

6565
To rotate the signing key without failed runs:
6666

67-
1. Set the new key as `WORKLOAD_IDENTITY_PREVIOUS_KEY` and redeploy. Sim publishes it but keeps signing with the current key.
67+
1. Set the new key as `WORKLOAD_IDENTITY_PREVIOUS_KEY` and redeploy (including any background workers that run workflows). Sim publishes it but keeps signing with the current key.
6868
2. Wait at least 5 minutes, so every cached copy of the published keys includes the new one.
69-
3. Swap them: set the new key as `WORKLOAD_IDENTITY_SIGNING_KEY` and the old key as `WORKLOAD_IDENTITY_PREVIOUS_KEY`, and redeploy.
70-
4. After 5 more minutes, remove `WORKLOAD_IDENTITY_PREVIOUS_KEY`.
69+
3. Swap them: set the new key as `WORKLOAD_IDENTITY_SIGNING_KEY` and the old key as `WORKLOAD_IDENTITY_PREVIOUS_KEY`, and redeploy everything that runs workflows.
70+
4. Once every instance signs with the new key, wait 5 more minutes, then remove `WORKLOAD_IDENTITY_PREVIOUS_KEY`.
7171

7272
Without a signing key, the keyless option is unavailable and access keys work as usual.
7373

7474
## Troubleshooting
7575

7676
- **`InvalidIdentityToken` or "No OpenIDConnect provider found":** the IAM identity provider's URL or audience doesn't match the dialog exactly, or AWS can't reach the issuer. Open `<issuer>/.well-known/openid-configuration` from outside your network; it must load over `https`.
77-
- **`AccessDenied` when saving:** the role's trust policy doesn't trust this credential's subject. Paste the trust policy from the dialog again; if you chose **Issue a new subject**, update the policy to the new one.
77+
- **"Sim could not assume this role: Not authorized to perform sts:AssumeRoleWithWebIdentity":** the role's trust policy doesn't trust this credential's subject. Paste the trust policy from the dialog again; if you chose **Issue a new subject**, update the policy to the new one.
7878
- **"This role trusts every Sim subject":** the trust policy is missing the `sub` condition. Use the policy from the dialog.

‎apps/docs/content/docs/integrations/databricks-credentials.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,4 +45,4 @@ Keyless connections need `WORKLOAD_IDENTITY_SIGNING_KEY` set and an `https` issu
4545
## Troubleshooting
4646

4747
- **"Databricks rejected the token request":** the federation policy's issuer, audience, or subject doesn't match the dialog, or the policy is on a different service principal than the application ID you entered.
48-
- **"This service principal's federation policy trusts every Sim subject":** the policy has no `subject`. Recreate it with the command from the dialog.
48+
- **"This service principal's federation policy also trusts other Sim credentials":** the policy's `subject_claim` isn't `sub`, so it matches more than this connection. Recreate it with the command from the dialog.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ describe('saving a keyless Databricks connection', () => {
115115

116116
await expect(
117117
verifyAndEncryptDatabricksCredential(JSON.stringify(config), reconnectOwner)
118-
).rejects.toThrow(/every Sim subject/)
118+
).rejects.toThrow(/also trusts other Sim credentials/)
119119
expect(encryptionMockFns.mockEncryptSecret).not.toHaveBeenCalled()
120120
})
121121

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -255,9 +255,10 @@ async function verifyWorkspaceIdentity(
255255

256256
/**
257257
* Sim signs tokens for any service principal a member names, and every Sim
258-
* tenant shares the issuer and audience, so a federation policy that doesn't
259-
* pin `subject` would let any Sim workspace act as the principal. Exchanging a
260-
* subject no credential holds must be refused before the credential is saved.
258+
* tenant shares the issuer and audience, so a federation policy that matches on
259+
* a claim other than `sub` (a `subject_claim` of `sim_workspace_id`, say) would
260+
* let other Sim credentials act as the principal. Exchanging a subject no
261+
* credential holds must be refused before the credential is saved.
261262
*/
262263
async function assertPolicyPinsSubject(
263264
config: Extract<DatabricksCredentialConfig, { authType: 'oidc_federation' }>,
@@ -280,7 +281,7 @@ async function assertPolicyPinsSubject(
280281
)
281282
}
282283
throw new DatabricksCredentialError(
283-
`This service principal's federation policy trusts every Sim subject. Use the policy shown in the dialog, whose subject is ${config.subject}, so only this credential can use it.`
284+
`This service principal's federation policy also trusts other Sim credentials. Use the policy shown in the dialog, which matches the sub claim to ${config.subject}, so only this credential can use it.`
284285
)
285286
}
286287

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,12 @@ function loadSigningMaterial(): Promise<SigningMaterial> {
109109
return { privateKey, kid: current.kid as string, jwks: { keys } }
110110
})()
111111
signingMaterial.catch((error: unknown) => {
112-
logger.error('Workload identity signing key is set but unusable', { error })
112+
logger.error(
113+
'Workload identity is configured but unusable (check the signing key and issuer)',
114+
{
115+
error,
116+
}
117+
)
113118
})
114119
}
115120
return signingMaterial

‎apps/sim/tools/whatsapp/utils.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { isRecordLike, toRecord } from '@sim/utils/object'
2+
import { safeUrlPathSegment } from '@/tools/url-path'
23
import type { WhatsAppMediaType, WhatsAppSendResponse } from '@/tools/whatsapp/types'
34

45
/** WhatsApp Cloud API Graph version used by every outbound tool. */
@@ -9,7 +10,7 @@ export function buildMessagesUrl(phoneNumberId: string | undefined): string {
910
if (!phoneNumberId) {
1011
throw new Error('WhatsApp Phone Number ID is required')
1112
}
12-
return `https://graph.facebook.com/${WHATSAPP_GRAPH_VERSION}/${encodeURIComponent(phoneNumberId.trim())}/messages`
13+
return `https://graph.facebook.com/${WHATSAPP_GRAPH_VERSION}/${safeUrlPathSegment(phoneNumberId.trim(), 'phoneNumberId')}/messages`
1314
}
1415

1516
/** Build the media upload endpoint for a given business phone number ID. */

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,12 @@ describe('WhatsApp request URL and headers', () => {
2323
)
2424
})
2525

26-
it('keeps a phone number ID inside its own path segment', () => {
27-
const url = sendMessageTool.request.url({ ...auth, phoneNumberId: '../../999/register#' })
28-
29-
expect(new URL(url as string).pathname).toBe('/v25.0/..%2F..%2F999%2Fregister%23/messages')
30-
})
26+
it.each(['../../999/register#', '..', '123/subscribed_apps?'])(
27+
'refuses a phone number ID that would leave its path segment (%s)',
28+
(phoneNumberId) => {
29+
expect(() => sendMessageTool.request.url({ ...auth, phoneNumberId })).toThrow()
30+
}
31+
)
3132
})
3233

3334
describe('sendMessageTool body', () => {

0 commit comments

Comments
 (0)