Skip to content

Commit 59e43ed

Browse files
fix(auth): preserve runtime authority across resumed operations
1 parent c26bc2c commit 59e43ed

12 files changed

Lines changed: 311 additions & 26 deletions

File tree

‎apps/sim/executor/handlers/credential-group/credential-group-handler.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ describe('CredentialGroupBlockHandler', () => {
190190
principal,
191191
input: {
192192
credentialGroupId: 'group-1',
193+
assertedWorkspaceId: 'workspace-1',
193194
email: 'person@example.com',
194195
mcpServerId: 'mcp-server-1',
195196
limit: 25,

‎apps/sim/executor/handlers/credential-group/credential-group-handler.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ export class CredentialGroupBlockHandler implements BlockHandler {
140140
principal,
141141
input: {
142142
credentialGroupId: credentialGroupId!,
143+
assertedWorkspaceId: executionWorkspaceId,
143144
limit: parseLimit(inputs.limit),
144145
cursor: parseOptionalString(inputs.cursor, 'Cursor'),
145146
email: parseOptionalString(inputs.email, 'Email'),

‎apps/sim/lib/auth/internal.test.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,20 @@ describe('internal executor delegation claims', () => {
7474
name: 'workspace API key',
7575
principal: { kind: 'workspace_api_key', workspaceId: 'workspace-1', keyId: 'key-1' },
7676
},
77+
...(['copilot', 'realtime'] as const).map((serviceId) => ({
78+
name: `${serviceId} delegation`,
79+
principal: {
80+
kind: 'delegated' as const,
81+
serviceId,
82+
subjectUserId: 'delegated-user',
83+
workspaceId: 'workspace-1',
84+
delegationId: 'delegation-1',
85+
audience: 'workflow-execution',
86+
issuedAt: new Date(),
87+
expiresAt: new Date(Date.now() + 60_000),
88+
},
89+
expectedSubject: 'delegated-user',
90+
})),
7791
{
7892
name: 'schedule',
7993
principal: {

‎apps/sim/lib/core/application/workspace-operation.test.ts‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,45 @@
11
/**
22
* @vitest-environment node
33
*/
4-
import { describe, expect, it } from 'vitest'
5-
import { defineWorkspaceOperation } from '@/lib/core/application/workspace-operation'
4+
import type { BoundWorkflowExecutionPrincipal, SessionPrincipal } from '@sim/auth/principal'
5+
import { describe, expect, expectTypeOf, it } from 'vitest'
6+
import {
7+
defineWorkspaceOperation,
8+
type PrincipalForOperation,
9+
} from '@/lib/core/application/workspace-operation'
610
import { CREDENTIAL_GROUP_CREDENTIAL_USE_ACTION } from '@/lib/resource-policies/registry'
711

12+
describe('defineWorkspaceOperation workflow execution policy', () => {
13+
it('infers bound runtime principals for a workflow-only operation', () => {
14+
const operation = defineWorkspaceOperation({
15+
id: 'test.workflow_only',
16+
minimumRole: 'read',
17+
workspaceApiKey: 'deny',
18+
principalKinds: [],
19+
workflowExecution: 'allow',
20+
capability: 'none',
21+
})
22+
23+
expect(operation.workflowExecution).toBe('allow')
24+
expectTypeOf<
25+
PrincipalForOperation<typeof operation>
26+
>().toEqualTypeOf<BoundWorkflowExecutionPrincipal>()
27+
})
28+
29+
it('does not widen an operation that omits workflow execution', () => {
30+
const operation = defineWorkspaceOperation({
31+
id: 'test.session_only',
32+
minimumRole: 'read',
33+
workspaceApiKey: 'deny',
34+
principalKinds: ['session'],
35+
capability: 'none',
36+
})
37+
38+
expect(operation.workflowExecution).toBeUndefined()
39+
expectTypeOf<PrincipalForOperation<typeof operation>>().toEqualTypeOf<SessionPrincipal>()
40+
})
41+
})
42+
843
describe('defineWorkspaceOperation delegated service policy', () => {
944
it('preserves and freezes an explicit delegated service allowlist', () => {
1045
const operation = defineWorkspaceOperation({

‎apps/sim/lib/core/application/workspace-operation.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ type DelegatedPrincipalForOperation<
3333
: never
3434

3535
type WorkflowExecutionPrincipalForOperation<O extends { readonly workflowExecution?: 'allow' }> =
36-
O['workflowExecution'] extends 'allow' ? BoundWorkflowExecutionPrincipal : never
36+
Extract<O['workflowExecution'], 'allow'> extends never ? never : BoundWorkflowExecutionPrincipal
3737

3838
export type PrincipalForOperation<
3939
O extends {

‎apps/sim/lib/credential-groups/application/list-mcp-connections.test.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ const workspaceContext = {
6363
allowPersonalApiKeys: true,
6464
billedAccountUserId: 'billing-owner-1',
6565
}
66-
const input = { credentialGroupId: 'group-1', limit: 50 }
66+
const input = { credentialGroupId: 'group-1', assertedWorkspaceId: 'workspace-1', limit: 50 }
6767

6868
function executorPrincipal(
6969
principal?: WorkflowExecutionPrincipal
@@ -157,6 +157,18 @@ describe('listCredentialGroupMcpConnections', () => {
157157
})
158158
})
159159

160+
it('conceals groups outside the execution workspace even when the user has access', async () => {
161+
mocks.resolvePermission.mockResolvedValue('admin')
162+
mocks.loadGroup.mockResolvedValue({ ...groupContext, workspaceId: 'workspace-2' })
163+
164+
await expect(
165+
listCredentialGroupMcpConnections.execute({ principal: executorPrincipal(), input })
166+
).rejects.toMatchObject({ code: 'not_found' })
167+
expect(mocks.loadWorkspace).not.toHaveBeenCalled()
168+
expect(mocks.resolvePermission).not.toHaveBeenCalled()
169+
expect(mocks.listMcpConnections).not.toHaveBeenCalled()
170+
})
171+
160172
it('rejects invalid filters before querying MCP connections', async () => {
161173
await expect(
162174
listCredentialGroupMcpConnections.execute({

‎apps/sim/lib/credential-groups/application/list-mcp-connections.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515

1616
export interface ListCredentialGroupMcpConnectionsInput {
1717
credentialGroupId: string
18+
assertedWorkspaceId: string
1819
limit: number
1920
cursor?: string
2021
email?: string
@@ -31,7 +32,7 @@ export interface ListCredentialGroupMcpConnectionsResult {
3132
export const listCredentialGroupMcpConnections = defineAuthorizedWorkspaceUseCase({
3233
operation: credentialGroupOperations.listMcpConnections,
3334
resolveContext: ({ input }: { input: ListCredentialGroupMcpConnectionsInput }) =>
34-
resolveCredentialGroupContext(input.credentialGroupId),
35+
resolveCredentialGroupContext(input.credentialGroupId, input.assertedWorkspaceId),
3536
authorizationOptions: {},
3637
execute: async ({ input, context }): Promise<ListCredentialGroupMcpConnectionsResult> => {
3738
if (

‎apps/sim/lib/logs/api/route-policies.test.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,19 +56,23 @@ describe('internal logs route authentication', () => {
5656
})
5757
})
5858

59-
it('keeps workflow-scoped executor tokens unscoped to one execution', async () => {
59+
it('preserves the canonical workspace separately from the signed execution identity', async () => {
6060
const token = await generateInternalDelegationToken({
6161
principal: createTestRuntimePrincipal(),
6262
})
6363

64-
const principal = await internalLogsSessionOrExecutorAuth.authenticate(
64+
const admission = await internalLogsSessionOrExecutorAuth.authenticateWithTransport(
6565
new NextRequest('http://localhost/api/logs/log-1', {
6666
headers: { authorization: `Bearer ${token}` },
6767
}),
6868
{ id: 'log-1' }
6969
)
7070

71-
expect(principal.executionMetadata.executionId).toBe('execution-1')
71+
expect(admission).toMatchObject({
72+
transport: 'executor_jwt',
73+
executionWorkspaceId: 'canonical-workspace',
74+
principal: { executionMetadata: { executionId: 'execution-1' } },
75+
})
7276
})
7377

7478
it('rejects an executor delegation without canonical workflow execution context', async () => {

‎apps/sim/lib/uploads/upload-session/service.test.ts‎

Lines changed: 118 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
/**
22
* @vitest-environment node
33
*/
4-
import { type Principal, serializePrincipal } from '@sim/auth/principal'
4+
import {
5+
type Principal,
6+
serializePrincipal,
7+
type WorkflowExecutionPrincipal,
8+
} from '@sim/auth/principal'
59
import { sha256Hex } from '@sim/security/hash'
610
import { dbChainMockFns, queueTableRows, resetDbChainMock, schemaMock } from '@sim/testing'
711
import { eq, inArray, isNull } from 'drizzle-orm'
@@ -550,6 +554,119 @@ describe('upload sessions', () => {
550554
).toThrow('Upload session not found')
551555
})
552556

557+
it.each<WorkflowExecutionPrincipal>([
558+
{ kind: 'workspace_api_key', workspaceId: WORKSPACE_ID, keyId: 'workspace-key' },
559+
{
560+
kind: 'system',
561+
serviceId: 'schedule',
562+
workspaceId: WORKSPACE_ID,
563+
workflowId: 'workflow-1',
564+
},
565+
{
566+
kind: 'system',
567+
serviceId: 'webhook',
568+
workspaceId: WORKSPACE_ID,
569+
workflowId: 'workflow-1',
570+
webhookId: 'webhook-1',
571+
provider: 'generic',
572+
},
573+
{
574+
kind: 'system',
575+
serviceId: 'webhook',
576+
workspaceId: WORKSPACE_ID,
577+
workflowId: 'workflow-1',
578+
webhookId: 'slack-webhook',
579+
provider: 'slack',
580+
subject: { kind: 'external_user', provider: 'slack', tenantId: 'T123', subjectId: 'U123' },
581+
},
582+
{
583+
kind: 'system',
584+
serviceId: 'chat',
585+
workspaceId: WORKSPACE_ID,
586+
workflowId: 'workflow-1',
587+
subject: { kind: 'authenticated_email', email: 'person@example.com' },
588+
},
589+
])(
590+
'binds and validates table-import uploads without inventing a Sim user: $kind $serviceId',
591+
async (actor) => {
592+
const principal = createTestRuntimePrincipal({ principal: actor })
593+
dbChainMockFns.returning.mockResolvedValueOnce([uploadRow({ purpose: 'table_import' })])
594+
595+
await createUploadSession({
596+
id: 'upload-1',
597+
workspaceId: WORKSPACE_ID,
598+
userId: 'billing-attribution-only',
599+
principal,
600+
purpose: 'table_import',
601+
fileName: 'data.csv',
602+
contentType: 'text/csv',
603+
fileSize: 4,
604+
})
605+
606+
const authBinding = dbChainMockFns.values.mock.calls[0][0].metadata.authBinding
607+
expect(authBinding).toEqual({
608+
version: 2,
609+
workspaceId: WORKSPACE_ID,
610+
principal: serializePrincipal(principal, 2),
611+
})
612+
expect(authBinding.principal.principal).toEqual(actor)
613+
expect(JSON.stringify(authBinding)).not.toContain('billing-attribution-only')
614+
const session = sessionRecord({ purpose: 'table_import', metadata: { authBinding } })
615+
expect(() => assertUploadSessionAuthBinding(session, principal)).not.toThrow()
616+
expect(() =>
617+
createUploadSessionAuthBinding(principal, 'other-workspace', { workflowExecution: 'allow' })
618+
).toThrow('Workflow execution cannot create this upload')
619+
expect(() =>
620+
assertUploadSessionAuthBinding(
621+
session,
622+
createTestRuntimePrincipal({ principal: actor, executionId: 'other-execution' })
623+
)
624+
).toThrow('Upload session not found')
625+
}
626+
)
627+
628+
it.each([
629+
{ workspaceId: WORKSPACE_ID, audience: 'uploads' },
630+
{ workspaceId: 'other-workspace', audience: 'uploads' },
631+
{ workspaceId: WORKSPACE_ID, audience: 'other-audience' },
632+
])(
633+
'rejects legacy executor bindings instead of accepting a different delegated identity: %j',
634+
(scope) => {
635+
const principal = createTestRuntimePrincipal({
636+
principal: {
637+
kind: 'delegated',
638+
serviceId: 'copilot',
639+
subjectUserId: 'user-1',
640+
...scope,
641+
delegationId: 'copilot-1',
642+
issuedAt: new Date(),
643+
expiresAt: new Date(Date.now() + 60_000),
644+
},
645+
})
646+
const session = sessionRecord({
647+
purpose: 'table_import',
648+
metadata: {
649+
authBinding: {
650+
version: 1,
651+
workspaceId: WORKSPACE_ID,
652+
principal: {
653+
kind: 'delegated',
654+
serviceId: 'executor',
655+
subjectUserId: 'user-1',
656+
audience: 'uploads',
657+
workflowId: 'workflow-1',
658+
executionId: 'execution-1',
659+
},
660+
},
661+
},
662+
})
663+
664+
expect(() => assertUploadSessionAuthBinding(session, principal)).toThrow(
665+
'Upload session not found'
666+
)
667+
}
668+
)
669+
553670
it('compares persisted executor bindings independently of JSON object key order', () => {
554671
const session = sessionRecord({
555672
purpose: 'table_import',

‎apps/sim/lib/uploads/upload-session/service.ts‎

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ import {
33
type Principal,
44
parsePrincipal,
55
requirePrincipalExecutionMetadata,
6-
requirePrincipalSubjectUserId,
7-
resolvePrincipalSubject,
86
type SerializedPrincipalV2,
97
serializePrincipal,
108
} from '@sim/auth/principal'
@@ -167,7 +165,7 @@ function isExecutorWorkflowExecutionPrincipal(
167165
} catch {
168166
return false
169167
}
170-
return resolvePrincipalSubject(principal)?.kind === 'sim_user'
168+
return true
171169
}
172170

173171
interface CreateUploadSessionBaseParams {
@@ -509,6 +507,8 @@ export function assertUploadSessionAuthBinding(
509507
return
510508
}
511509
const bound = candidate.principal
510+
/** Version-1 executor bindings cannot prove the canonical runtime principal identity. */
511+
if (bound.kind === 'delegated') throw uploadNotFound()
512512
const matches =
513513
bound.kind === principal.kind &&
514514
(bound.kind === 'session'
@@ -519,15 +519,9 @@ export function assertUploadSessionAuthBinding(
519519
? principal.kind === 'personal_api_key' &&
520520
bound.userId === principal.userId &&
521521
bound.keyId === principal.keyId
522-
: bound.kind === 'workspace_api_key'
523-
? principal.kind === 'workspace_api_key' &&
524-
bound.workspaceId === principal.workspaceId &&
525-
bound.keyId === principal.keyId
526-
: isExecutorWorkflowExecutionPrincipal(principal) &&
527-
resolvePrincipalSubject(principal)?.kind === 'sim_user' &&
528-
requirePrincipalSubjectUserId(principal) === bound.subjectUserId &&
529-
principal.executionMetadata.rootWorkflowId === bound.workflowId &&
530-
principal.executionMetadata.executionId === bound.executionId)
522+
: principal.kind === 'workspace_api_key' &&
523+
bound.workspaceId === principal.workspaceId &&
524+
bound.keyId === principal.keyId)
531525
if (!matches) throw uploadNotFound()
532526
}
533527

0 commit comments

Comments
 (0)