Skip to content

Commit 08e5585

Browse files
TheodoreSpeaksTheodoreSpeaks
committed
fix(auth): enforce canonical execution scope
1 parent 10a3213 commit 08e5585

25 files changed

Lines changed: 302 additions & 62 deletions

File tree

‎apps/sim/app/api/files/serve/[...path]/route.test.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import { hybridAuthMockFns, storageServiceMock, storageServiceMockFns } from '@sim/testing'
77
import { NextRequest } from 'next/server'
88
import { beforeEach, describe, expect, it, vi } from 'vitest'
9+
import { createTestRuntimePrincipal } from '@/lib/auth/runtime-principal.test-support'
910
import { MAX_BUFFERED_TRANSFER_BYTES } from '@/lib/uploads/shared/types'
1011

1112
vi.mock('@sim/logger', () => ({
@@ -370,6 +371,39 @@ describe('File Serve API Route', () => {
370371
expect(mockVerifyFileAccess).not.toHaveBeenCalled()
371372
})
372373

374+
it('serves an actorless workflow file without synthesizing a user owner', async () => {
375+
const principal = createTestRuntimePrincipal({
376+
principal: {
377+
kind: 'system',
378+
serviceId: 'schedule',
379+
workspaceId: 'test-workspace-id',
380+
workflowId: 'workflow-1',
381+
},
382+
})
383+
mockResolveStoredFileContext.mockResolvedValue('workspace')
384+
mockParseWorkspaceFileKey.mockReturnValue('test-workspace-id')
385+
mockAuthenticateWorkspaceFile.mockResolvedValue(principal)
386+
387+
const response = await GET(
388+
new NextRequest(
389+
'http://localhost:3000/api/files/serve/workspace/test-workspace-id/report.pdf'
390+
),
391+
{
392+
params: Promise.resolve({
393+
path: ['workspace', 'test-workspace-id', 'report.pdf'],
394+
}),
395+
}
396+
)
397+
398+
expect(response.status).toBe(200)
399+
expect(mockResolveServableDocBytes).toHaveBeenCalledWith(
400+
expect.objectContaining({
401+
filePrincipal: principal,
402+
ownerKey: 'workspace:test-workspace-id',
403+
})
404+
)
405+
})
406+
373407
it('serves a mothership chat attachment stored under a workspace key', async () => {
374408
/**
375409
* The attachment shares the `workspace/…` prefix but is recorded as

‎apps/sim/app/api/files/serve/[...path]/route.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { type Principal, requirePrincipalSubjectUserId } from '@sim/auth/principal'
1+
import { type Principal, resolvePrincipalSubjectUserId } from '@sim/auth/principal'
22
import { createLogger } from '@sim/logger'
33
import { getErrorMessage } from '@sim/utils/errors'
44
import type { NextRequest } from 'next/server'
@@ -328,7 +328,8 @@ async function handleWorkspaceFile(
328328
input: { key, assertedWorkspaceId: workspaceId },
329329
request,
330330
})
331-
const ownerKey = `user:${requirePrincipalSubjectUserId(principal)}`
331+
const subjectUserId = resolvePrincipalSubjectUserId(principal)
332+
const ownerKey = subjectUserId ? `user:${subjectUserId}` : `workspace:${workspaceId}`
332333
const resolved = await resolveServableBytes({
333334
buffer: content,
334335
filename: file.name,

‎apps/sim/app/api/knowledge/[id]/documents/[documentId]/chunks/route.test.ts‎

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import { authMockFns, createMockRequest } from '@sim/testing'
66
import { beforeEach, describe, expect, it, vi } from 'vitest'
7+
import { createTestRuntimePrincipal } from '@/lib/auth/runtime-principal.test-support'
78

89
const mocks = vi.hoisted(() => ({
910
list: vi.fn(),
@@ -32,8 +33,9 @@ vi.mock('@/lib/knowledge/api/secret-provenance', () => ({
3233
resolveKnowledgeWriteSecretProvenance: vi.fn(),
3334
}))
3435

36+
import { internalKnowledgeSessionOrExecutorAuth } from '@/lib/knowledge/api/route-policies'
3537
import { KnowledgeDocumentNotReadyError } from '@/lib/knowledge/application/chunk-errors'
36-
import { GET } from '@/app/api/knowledge/[id]/documents/[documentId]/chunks/route'
38+
import { GET, POST } from '@/app/api/knowledge/[id]/documents/[documentId]/chunks/route'
3739

3840
const params = () => ({
3941
params: Promise.resolve({ id: 'knowledge-1', documentId: 'document-1' }),
@@ -60,4 +62,54 @@ describe('/api/knowledge/[id]/documents/[documentId]/chunks internal route compo
6062
retryAfter: 5,
6163
})
6264
})
65+
66+
it('passes the executor transport workspace into chunk reads', async () => {
67+
const principal = createTestRuntimePrincipal()
68+
vi.spyOn(
69+
internalKnowledgeSessionOrExecutorAuth,
70+
'authenticateWithTransport'
71+
).mockResolvedValueOnce({
72+
principal,
73+
transport: 'executor_jwt',
74+
executionWorkspaceId: 'workspace-canonical',
75+
})
76+
mocks.list.mockResolvedValueOnce({
77+
chunks: [],
78+
pagination: { total: 0, limit: 50, offset: 0, hasMore: false },
79+
workspaceId: 'workspace-canonical',
80+
documentId: 'document-1',
81+
})
82+
83+
const response = await GET(createMockRequest('GET'), params())
84+
85+
expect(response.status).toBe(200)
86+
expect(mocks.list.mock.calls[0][0]).toMatchObject({
87+
principal,
88+
input: { assertedWorkspaceId: 'workspace-canonical' },
89+
})
90+
})
91+
92+
it('passes the executor transport workspace into chunk writes', async () => {
93+
const principal = createTestRuntimePrincipal()
94+
vi.spyOn(
95+
internalKnowledgeSessionOrExecutorAuth,
96+
'authenticateWithTransport'
97+
).mockResolvedValueOnce({
98+
principal,
99+
transport: 'executor_jwt',
100+
executionWorkspaceId: 'workspace-canonical',
101+
})
102+
mocks.create.mockRejectedValueOnce(new Error('stop after input mapping'))
103+
104+
const response = await POST(
105+
createMockRequest('POST', { content: 'hello', enabled: true }),
106+
params()
107+
)
108+
109+
expect(response.status).toBe(500)
110+
expect(mocks.create.mock.calls[0][0]).toMatchObject({
111+
principal,
112+
input: { assertedWorkspaceId: 'workspace-canonical' },
113+
})
114+
})
63115
})

‎apps/sim/app/api/knowledge/[id]/documents/[documentId]/chunks/route.ts‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
defineInternalJsonRoute,
1010
type InternalAuthTransport,
1111
internalRateLimits,
12+
resolveInternalAuthWorkspaceId,
1213
} from '@/lib/api/server/routes'
1314
import { OrchestrationError } from '@/lib/core/orchestration/types'
1415
import {
@@ -65,9 +66,14 @@ export const GET = defineInternalJsonRoute({
6566
operation: knowledgeOperations.listChunks,
6667
rateLimit: internalRateLimits.none({ reason: 'Preserve existing internal chunk-list behavior' }),
6768
errorPolicy: internalKnowledgeErrorPolicies.chunkList,
68-
mapInput: ({ params, query }) => ({
69+
mapInput: ({ params, query }, { authTransport, executionWorkspaceId }) => ({
6970
knowledgeBaseId: params.id,
7071
documentId: params.documentId,
72+
assertedWorkspaceId: resolveInternalAuthWorkspaceId(
73+
authTransport,
74+
executionWorkspaceId,
75+
undefined
76+
),
7177
...query,
7278
}),
7379
useCase: listKnowledgeChunks,
@@ -105,9 +111,14 @@ export const POST = defineInternalJsonRoute({
105111
reason: 'Preserve existing internal chunk-create behavior',
106112
}),
107113
errorPolicy: internalKnowledgeErrorPolicies.chunks,
108-
mapInput: ({ params, body }, { principal, request, authTransport }) => ({
114+
mapInput: ({ params, body }, { principal, request, authTransport, executionWorkspaceId }) => ({
109115
knowledgeBaseId: params.id,
110116
documentId: params.documentId,
117+
assertedWorkspaceId: resolveInternalAuthWorkspaceId(
118+
authTransport,
119+
executionWorkspaceId,
120+
undefined
121+
),
111122
content: body.content,
112123
enabled: body.enabled,
113124
resolveContentProvenance: ({ workspaceId }: { workspaceId?: string }) =>
@@ -132,9 +143,14 @@ export const PATCH = defineInternalJsonRoute({
132143
operation: knowledgeOperations.bulkChunks,
133144
rateLimit: internalRateLimits.none({ reason: 'Preserve existing internal bulk-chunk behavior' }),
134145
errorPolicy: internalKnowledgeErrorPolicies.chunks,
135-
mapInput: ({ params, body }) => ({
146+
mapInput: ({ params, body }, { authTransport, executionWorkspaceId }) => ({
136147
knowledgeBaseId: params.id,
137148
documentId: params.documentId,
149+
assertedWorkspaceId: resolveInternalAuthWorkspaceId(
150+
authTransport,
151+
executionWorkspaceId,
152+
undefined
153+
),
138154
...body,
139155
}),
140156
useCase: bulkUpdateKnowledgeChunks,

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ describe('CredentialGroupBlockHandler', () => {
106106
principal,
107107
input: {
108108
credentialGroupId: 'group-1',
109+
assertedWorkspaceId: 'workspace-1',
109110
email: 'person@example.com',
110111
credentialProviderIds: ['google-email'],
111112
limit: 25,
@@ -154,6 +155,7 @@ describe('CredentialGroupBlockHandler', () => {
154155
principal: actorlessPrincipal,
155156
input: {
156157
credentialGroupId: 'group-1',
158+
assertedWorkspaceId: 'workspace-1',
157159
limit: 100,
158160
cursor: undefined,
159161
email: undefined,
@@ -207,7 +209,11 @@ describe('CredentialGroupBlockHandler', () => {
207209
)
208210
expect(mocks.sendInvite).toHaveBeenCalledWith({
209211
principal,
210-
input: { credentialGroupId: 'group-1', email: 'person@example.com' },
212+
input: {
213+
credentialGroupId: 'group-1',
214+
assertedWorkspaceId: 'workspace-1',
215+
email: 'person@example.com',
216+
},
211217
})
212218
})
213219

@@ -238,7 +244,11 @@ describe('CredentialGroupBlockHandler', () => {
238244
)
239245
expect(mocks.createInviteLink).toHaveBeenCalledWith({
240246
principal,
241-
input: { credentialGroupId: 'group-1', email: 'person@example.com' },
247+
input: {
248+
credentialGroupId: 'group-1',
249+
assertedWorkspaceId: 'workspace-1',
250+
email: 'person@example.com',
251+
},
242252
})
243253
expect(mocks.sendInvite).not.toHaveBeenCalled()
244254
expect(result).toEqual({

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

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,11 @@ export class CredentialGroupBlockHandler implements BlockHandler {
9696
_block: SerializedBlock,
9797
inputs: Record<string, unknown>
9898
): Promise<BlockOutput> {
99-
if (!ctx.workspaceId) throw new Error('workspaceId is required for Credential Group operations')
10099
const operation = parseOperation(inputs.operation)
101100
if (!ctx.principal?.executionMetadata) {
102101
throw new Error('Credential Group operations require an authenticated workflow execution')
103102
}
103+
const executionWorkspaceId = requireExecutorWorkspaceId(ctx)
104104
const credentialGroupId =
105105
operation === 'list_groups'
106106
? undefined
@@ -119,6 +119,7 @@ export class CredentialGroupBlockHandler implements BlockHandler {
119119
principal,
120120
input: {
121121
credentialGroupId: credentialGroupId!,
122+
assertedWorkspaceId: executionWorkspaceId,
122123
limit: parseLimit(inputs.limit),
123124
cursor: parseOptionalString(inputs.cursor, 'Cursor'),
124125
email: parseOptionalString(inputs.email, 'Email'),
@@ -133,11 +134,12 @@ export class CredentialGroupBlockHandler implements BlockHandler {
133134
return result
134135
}
135136
case 'send_invite': {
136-
await enforceCredentialGroupInvitationExecutionRateLimit(requireExecutorWorkspaceId(ctx))
137+
await enforceCredentialGroupInvitationExecutionRateLimit(executionWorkspaceId)
137138
const result = await sendCredentialGroupInvite.execute({
138139
principal,
139140
input: {
140141
credentialGroupId: credentialGroupId!,
142+
assertedWorkspaceId: executionWorkspaceId,
141143
email: requireString(inputs.email, 'Email'),
142144
},
143145
})
@@ -154,11 +156,12 @@ export class CredentialGroupBlockHandler implements BlockHandler {
154156
}
155157
}
156158
case 'get_invite_link': {
157-
await enforceCredentialGroupInvitationExecutionRateLimit(requireExecutorWorkspaceId(ctx))
159+
await enforceCredentialGroupInvitationExecutionRateLimit(executionWorkspaceId)
158160
const result = await createCredentialGroupInviteLink.execute({
159161
principal,
160162
input: {
161163
credentialGroupId: credentialGroupId!,
164+
assertedWorkspaceId: executionWorkspaceId,
162165
email: requireString(inputs.email, 'Email'),
163166
},
164167
})
@@ -185,6 +188,7 @@ export class CredentialGroupBlockHandler implements BlockHandler {
185188
principal,
186189
input: {
187190
credentialGroupId: credentialGroupId!,
191+
assertedWorkspaceId: executionWorkspaceId,
188192
limit: parseLimit(inputs.limit),
189193
cursor: parseOptionalString(inputs.cursor, 'Cursor'),
190194
email: parseOptionalString(inputs.email, 'Email'),
@@ -202,13 +206,13 @@ export class CredentialGroupBlockHandler implements BlockHandler {
202206
const result = await listCredentialGroupsForWorkflow.execute({
203207
principal,
204208
input: {
205-
workspaceId: ctx.workspaceId,
209+
workspaceId: executionWorkspaceId,
206210
limit: parseLimit(inputs.limit),
207211
cursor: parseOptionalString(inputs.cursor, 'Cursor'),
208212
},
209213
})
210214
logger.info('Listed Credential Groups', {
211-
workspaceId: ctx.workspaceId,
215+
workspaceId: executionWorkspaceId,
212216
count: result.count,
213217
hasMore: result.hasMore,
214218
})

‎apps/sim/lib/api/server/routes/internal-json-route.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,11 @@ export function resolveInternalAuthWorkspaceId(
237237
executionWorkspaceId: string | undefined,
238238
sessionWorkspaceId: string
239239
): string
240+
export function resolveInternalAuthWorkspaceId(
241+
authTransport: InternalAuthTransport | undefined,
242+
executionWorkspaceId: string | undefined,
243+
sessionWorkspaceId: string | undefined
244+
): string | undefined
240245
export function resolveInternalAuthWorkspaceId(
241246
authTransport: InternalAuthTransport | undefined,
242247
executionWorkspaceId: string | undefined,

‎apps/sim/lib/auth/internal-delegation.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ export async function bindRuntimeWorkflowExecution(
5656
}
5757
const executionMetadata = requirePrincipalExecutionMetadata(principal)
5858
const subject = resolvePrincipalSubject(principal)
59-
if (subject && options.compatibilityActorUserId) {
59+
if (subject && options.compatibilityActorUserId !== undefined) {
6060
throw new Error('Internal delegation cannot bind a compatibility actor to a subject')
6161
}
6262

@@ -103,9 +103,10 @@ export async function bindRuntimeWorkflowExecution(
103103
}
104104

105105
return {
106-
principal: options.compatibilityActorUserId
107-
? withPrincipalExecutionActor(principal, options.compatibilityActorUserId)
108-
: principal,
106+
principal:
107+
options.compatibilityActorUserId !== undefined
108+
? withPrincipalExecutionActor(principal, options.compatibilityActorUserId)
109+
: principal,
109110
workspaceId,
110111
}
111112
}

‎apps/sim/lib/auth/runtime-principal.test-support.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ export function createTestRuntimePrincipal(
3737
? root
3838
: enterPrincipalWorkflowExecution(root, currentWorkflow)
3939

40-
return options.compatibilityActorUserId
40+
return options.compatibilityActorUserId !== undefined
4141
? withPrincipalExecutionActor(bound, options.compatibilityActorUserId)
4242
: bound
4343
}

‎apps/sim/lib/credential-groups/application/context.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,20 +34,20 @@ export async function resolveCredentialGroupWorkspaceContext(workspaceId: string
3434
}
3535

3636
export async function resolveCredentialGroupContext(
37-
credentialGroupId: string
37+
credentialGroupId: string,
38+
assertedWorkspaceId?: string
3839
): Promise<CredentialGroupApplicationContext> {
3940
const group = await loadCredentialGroupCredentialListContext(credentialGroupId)
4041
if (!group) throw new OrchestrationError('not_found', 'Credential group not found')
42+
if (assertedWorkspaceId !== undefined && group.workspaceId !== assertedWorkspaceId) {
43+
throw new OrchestrationError('not_found', 'Credential group not found')
44+
}
4145
return { ...(await resolveCredentialGroupWorkspaceContext(group.workspaceId)), ...group }
4246
}
4347

4448
export async function resolveCredentialGroupSettingsContext(
4549
credentialGroupId: string,
4650
assertedWorkspaceId: string
4751
): Promise<CredentialGroupApplicationContext> {
48-
const context = await resolveCredentialGroupContext(credentialGroupId)
49-
if (context.workspaceId !== assertedWorkspaceId) {
50-
throw new OrchestrationError('not_found', 'Credential group not found')
51-
}
52-
return context
52+
return resolveCredentialGroupContext(credentialGroupId, assertedWorkspaceId)
5353
}

0 commit comments

Comments
 (0)