Skip to content

Commit 09775ce

Browse files
committed
fix(credentials): prove a Chat managed-credential use only from a real tool call, and list or mint only live group bindings
1 parent f52b352 commit 09775ce

5 files changed

Lines changed: 96 additions & 23 deletions

File tree

apps/sim/executor/utils/credential-token.test.ts

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -128,16 +128,22 @@ describe('resolveExecutorCredentialToken', () => {
128128
})
129129

130130
it('leaves managed credentials unproven for a context that is not a trusted Chat call', async () => {
131-
await resolveExecutorCredentialToken({
132-
requestId: 'req-1',
133-
credentialId: 'cred-1',
134-
userId: 'user-1',
135-
copilotExecutionContext: { userId: 'user-1', workspaceId: 'ws-1' },
136-
})
131+
for (const copilotExecutionContext of [
132+
{ userId: 'user-1', workspaceId: 'ws-1' },
133+
{ userId: 'user-1', workspaceId: 'ws-1', copilotToolExecution: true as const },
134+
]) {
135+
mockResolveCredentialAccessToken.mockClear()
136+
await resolveExecutorCredentialToken({
137+
requestId: 'req-1',
138+
credentialId: 'cred-1',
139+
userId: 'user-1',
140+
copilotExecutionContext,
141+
})
137142

138-
expect(
139-
mockResolveCredentialAccessToken.mock.calls[0][0].resolveManagedPrincipal
140-
).toBeUndefined()
143+
expect(
144+
mockResolveCredentialAccessToken.mock.calls[0][0].resolveManagedPrincipal
145+
).toBeUndefined()
146+
}
141147
})
142148

143149
it('fails before dispatch when the origin lacks current workflow authority', async () => {

apps/sim/executor/utils/credential-token.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,15 @@ export async function resolveExecutorCredentialToken(
5757
throw new Error('Managed credential delegation is missing current workflow authority')
5858
}
5959

60+
/**
61+
* A Chat proof needs the per-call id the delegation is minted under; a
62+
* context that lacks it is not a Chat tool call and leaves managed
63+
* credentials unproven, so the resolver answers with its own refusal.
64+
*/
6065
const resolveManagedPrincipal = executorDelegationOrigin
6166
? (managedCredentialId: string) =>
6267
bindExecutorManagedOAuthDelegation(executorDelegationOrigin, managedCredentialId)
63-
: copilotExecutionContext?.copilotToolExecution
68+
: copilotExecutionContext?.copilotToolExecution && copilotExecutionContext.toolCallId
6469
? async (managedCredentialId: string) =>
6570
createCopilotManagedOAuthPrincipal(copilotExecutionContext, managedCredentialId)
6671
: undefined

apps/sim/lib/credential-groups/application/authorization.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,23 @@ import { credentialOperations } from '@/lib/credentials/application/operations'
99

1010
const mocks = vi.hoisted(() => ({
1111
loadEnrollmentAccess: vi.fn(),
12+
loadBinding: vi.fn(),
1213
requirePolicy: vi.fn(),
1314
}))
1415

1516
vi.mock('@/lib/credential-groups/credentials', () => ({
1617
loadCredentialGroupEnrollmentAccessForSubject: mocks.loadEnrollmentAccess,
18+
loadManagedCredentialGroupBinding: mocks.loadBinding,
19+
isManagedCredentialGroupBindingLive: (binding: {
20+
managedOauthStatus: string
21+
enrollmentStatus: string
22+
groupStatus: string
23+
optionStatus: string | null
24+
}) =>
25+
binding.managedOauthStatus === 'active' &&
26+
['in_progress', 'completed'].includes(binding.enrollmentStatus) &&
27+
binding.groupStatus === 'active' &&
28+
binding.optionStatus === 'active',
1729
}))
1830

1931
vi.mock('@/lib/resource-policies/repository', () => ({
@@ -29,10 +41,23 @@ const context = {
2941
workspaceId: 'workspace-1',
3042
workspaceOrganizationId: null,
3143
allowPersonalApiKeys: true,
44+
credentialId: 'credential-1',
3245
credentialGroupId: 'group-1',
3346
credentialGroupEnrollmentId: 'enrollment-1',
3447
}
3548

49+
const liveBinding = {
50+
credentialId: 'credential-1',
51+
workspaceId: 'workspace-1',
52+
providerId: 'google-email',
53+
credentialGroupId: 'group-1',
54+
credentialGroupOptionId: 'option-1',
55+
managedOauthStatus: 'active',
56+
enrollmentStatus: 'completed',
57+
groupStatus: 'active',
58+
optionStatus: 'active',
59+
}
60+
3661
function storedPolicy(allowedWorkflowIds: string[] = []) {
3762
return {
3863
id: 'policy-1',
@@ -112,6 +137,15 @@ describe('requireCredentialGroupCredentialAccess', () => {
112137
enrollmentId: 'enrollment-1',
113138
email: 'person@example.com',
114139
})
140+
mocks.loadBinding.mockResolvedValue(liveBinding)
141+
})
142+
143+
it('denies a Chat turn once the credential group or its option is disabled', async () => {
144+
mocks.loadBinding.mockResolvedValue({ ...liveBinding, optionStatus: 'disabled' })
145+
await expect(requireAccess(copilotPrincipal())).rejects.toMatchObject({ code: 'forbidden' })
146+
147+
mocks.loadBinding.mockResolvedValue({ ...liveBinding, groupStatus: 'disabled' })
148+
await expect(requireAccess(copilotPrincipal())).rejects.toMatchObject({ code: 'forbidden' })
115149
})
116150

117151
it("allows a Chat turn to use only the credential under the signed-in user's own enrollment", async () => {

apps/sim/lib/credential-groups/application/authorization.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,11 @@ import {
1616
evaluateCredentialGroupWorkflowAccess,
1717
} from '@/lib/credential-groups/application/workflow-access-policy'
1818
import type { CredentialGroupCredentialListContext } from '@/lib/credential-groups/credentials'
19-
import { loadCredentialGroupEnrollmentAccessForSubject } from '@/lib/credential-groups/credentials'
19+
import {
20+
isManagedCredentialGroupBindingLive,
21+
loadCredentialGroupEnrollmentAccessForSubject,
22+
loadManagedCredentialGroupBinding,
23+
} from '@/lib/credential-groups/credentials'
2024
import type { ResourcePolicyBindingFor } from '@/lib/resource-policies/registry'
2125
import { requireResourcePolicy } from '@/lib/resource-policies/repository'
2226

@@ -93,23 +97,28 @@ export function requireCredentialGroupWorkflowActor(principal: Principal): Princ
9397
*/
9498
async function requireCredentialGroupActorCredentialAccess(
9599
principal: Extract<Principal, { kind: 'delegated' }>,
96-
context: CredentialGroupAuthorizationContext & { credentialGroupEnrollmentId: string },
100+
context: CredentialGroupAuthorizationContext & {
101+
credentialId: string
102+
credentialGroupEnrollmentId: string
103+
},
97104
resourcePolicy: ResourcePolicyBindingFor<'credential_group'>
98105
): Promise<void> {
99106
const subject = resolvePrincipalSubject(principal)
100107
if (subject?.kind !== 'sim_user' || !subject.userId) {
101108
throw new OrchestrationError('forbidden', 'Credential Group actor access required')
102109
}
103-
const [policy, actorAccess] = await Promise.all([
110+
const [policy, actorAccess, binding] = await Promise.all([
104111
requireResourcePolicy({
105112
workspaceId: context.workspaceId,
106113
resourceType: 'credential_group',
107114
resourceId: context.credentialGroupId,
108115
codec: credentialGroupWorkflowAccessPolicyCodec,
109116
}),
110117
loadCredentialGroupEnrollmentAccessForSubject(context.credentialGroupId, subject),
118+
loadManagedCredentialGroupBinding(context.credentialId),
111119
])
112-
if (!actorAccess) {
120+
/** A disabled group or option denies here, as it does for every other consumer of a binding. */
121+
if (!actorAccess || !binding || !isManagedCredentialGroupBindingLive(binding)) {
113122
throw new OrchestrationError('forbidden', 'Credential Group credential access denied')
114123
}
115124
const decision = evaluateCredentialGroupActorCredentialAccess({
@@ -126,7 +135,10 @@ async function requireCredentialGroupActorCredentialAccess(
126135

127136
export async function requireCredentialGroupCredentialAccess(
128137
principal: Principal,
129-
context: CredentialGroupAuthorizationContext & { credentialGroupEnrollmentId: string },
138+
context: CredentialGroupAuthorizationContext & {
139+
credentialId: string
140+
credentialGroupEnrollmentId: string
141+
},
130142
resourcePolicy: ResourcePolicyBindingFor<'credential_group'>
131143
): Promise<void> {
132144
if (principal.kind === 'delegated' && principal.serviceId === 'copilot') {

apps/sim/lib/credentials/environment.ts

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { chunkArray } from '@sim/utils/helpers'
1414
import { generateId } from '@sim/utils/id'
1515
import { and, asc, eq, inArray, isNotNull, isNull, notInArray, or, sql } from 'drizzle-orm'
1616
import { acquireUserBillingIdentityLock } from '@/lib/billing/organizations/billing-identity-lock'
17-
import { LIVE_ENROLLMENT_STATUSES } from '@/lib/credential-groups/credentials'
17+
import { isManagedCredentialGroupBindingLive } from '@/lib/credential-groups/credentials'
1818
import type { DbOrTx } from '@/lib/db/types'
1919
import {
2020
getEffectiveWorkspacePermission,
@@ -843,10 +843,11 @@ export interface AccessibleOAuthCredential {
843843

844844
/**
845845
* The Credential Group credentials a verified person holds through their own
846-
* live enrollments in the workspace: active managed OAuth rows whose enrollment
847-
* email is the person's. These are theirs to use as themselves; the policy's
848-
* actor statement is what a use is authorized against, so nothing here widens
849-
* access, it only tells the person (and the agent acting for them) what exists.
846+
* enrollments in the workspace and may use right now: the credential, its
847+
* enrollment, its option, and its group are all live, the same bar every mint
848+
* applies. These are theirs to use as themselves; the policy's actor statement
849+
* is what a use is authorized against, so nothing here widens access, it only
850+
* tells the person (and the agent acting for them) what exists.
850851
*/
851852
export async function getEnrolledManagedOAuthCredentials(
852853
workspaceId: string,
@@ -857,7 +858,12 @@ export async function getEnrolledManagedOAuthCredentials(
857858
id: credential.id,
858859
providerId: credential.providerId,
859860
displayName: credential.displayName,
861+
credentialGroupOptionId: credential.credentialGroupOptionId,
862+
managedOauthStatus: credential.managedOauthStatus,
863+
enrollmentStatus: credentialGroupEnrollment.status,
860864
groupName: credentialGroup.name,
865+
groupStatus: credentialGroup.status,
866+
groupOptions: credentialGroup.options,
861867
updatedAt: credential.updatedAt,
862868
})
863869
.from(credential)
@@ -871,15 +877,25 @@ export async function getEnrolledManagedOAuthCredentials(
871877
and(
872878
eq(credential.workspaceId, workspaceId),
873879
eq(credential.type, 'managed_oauth'),
874-
eq(credential.managedOauthStatus, 'active'),
875-
inArray(credentialGroupEnrollment.status, [...LIVE_ENROLLMENT_STATUSES]),
876880
eq(user.id, userId),
877881
eq(user.emailVerified, true)
878882
)
879883
)
880884

881885
return rows
882-
.filter((row): row is typeof row & { providerId: string } => Boolean(row.providerId))
886+
.filter(
887+
(row): row is typeof row & { providerId: string } =>
888+
Boolean(row.providerId) &&
889+
row.managedOauthStatus !== null &&
890+
isManagedCredentialGroupBindingLive({
891+
managedOauthStatus: row.managedOauthStatus,
892+
enrollmentStatus: row.enrollmentStatus,
893+
groupStatus: row.groupStatus,
894+
optionStatus:
895+
row.groupOptions.find((option) => option.id === row.credentialGroupOptionId)?.status ??
896+
null,
897+
})
898+
)
883899
.map((row) => ({
884900
id: row.id,
885901
providerId: row.providerId,

0 commit comments

Comments
 (0)