Skip to content

Commit 6415e24

Browse files
TheodoreSpeaksTheodoreSpeaks
committed
fix(auth): enforce canonical execution scope
1 parent 521d527 commit 6415e24

64 files changed

Lines changed: 591 additions & 509 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎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'
@@ -345,7 +345,8 @@ async function handleWorkspaceFile(
345345
input: { key, assertedWorkspaceId: workspaceId },
346346
request,
347347
})
348-
const ownerKey = `user:${requirePrincipalSubjectUserId(principal)}`
348+
const subjectUserId = resolvePrincipalSubjectUserId(principal)
349+
const ownerKey = subjectUserId ? `user:${subjectUserId}` : `workspace:${workspaceId}`
349350
const resolved = await resolveServableBytes({
350351
buffer: content,
351352
filename: file.name,

‎apps/sim/app/api/knowledge/[id]/connectors/[connectorId]/access/route.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,15 @@ export const PATCH = defineInternalJsonRoute({
2020
reason: 'A settings action an admin performs by hand; the switch itself is bounded',
2121
}),
2222
errorPolicy: internalKnowledgeErrorPolicies.connectors,
23-
mapInput: ({ params, body }, { principal, request }) => ({
23+
mapInput: ({ params, body }, { principal, request, authTransport }) => ({
2424
connectorId: params.connectorId,
2525
knowledgeBaseId: params.id,
2626
accessMode: body.accessMode,
2727
credentialGroupId: body.credentialGroupId,
2828
credentialGroupOptionId: body.credentialGroupOptionId,
2929
credentialId: body.credentialId,
3030
resolveBillingAttribution: (workspaceId: string) =>
31-
resolveInternalKnowledgeBillingAttribution(request, principal, workspaceId),
31+
resolveInternalKnowledgeBillingAttribution(request, principal, workspaceId, authTransport),
3232
source: 'ui' as const,
3333
}),
3434
useCase: updateKnowledgeConnectorAccess,

‎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/app/api/table/[tableId]/query/route.test.ts‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

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

89
const { mocks, MockTableV2FeatureDisabledError } = vi.hoisted(() => {
910
class MockTableV2FeatureDisabledError extends Error {
@@ -73,16 +74,7 @@ function sessionPrincipal() {
7374

7475
function executorPrincipal() {
7576
mocks.authenticate.mockResolvedValue({
76-
principal: {
77-
kind: 'delegated',
78-
serviceId: 'executor',
79-
subjectUserId: 'user-1',
80-
workspaceId: 'workspace-canonical',
81-
delegationId: 'delegation-1',
82-
audience: 'sim:tables',
83-
issuedAt: new Date('2026-01-01'),
84-
expiresAt: new Date('2026-01-02'),
85-
},
77+
principal: createTestRuntimePrincipal(),
8678
transport: 'executor_jwt',
8779
executionWorkspaceId: 'workspace-canonical',
8880
})

‎apps/sim/app/api/table/[tableId]/rows/[rowId]/route.test.ts‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
import { NextRequest } from 'next/server'
1717
import { beforeEach, describe, expect, it, vi } from 'vitest'
18+
import { createTestRuntimePrincipal } from '@/lib/auth/runtime-principal.test-support'
1819

1920
const { mocks } = vi.hoisted(() => ({
2021
mocks: {
@@ -87,16 +88,7 @@ function sessionPrincipal() {
8788

8889
function executorPrincipal() {
8990
mocks.authenticate.mockResolvedValue({
90-
principal: {
91-
kind: 'delegated',
92-
serviceId: 'executor',
93-
subjectUserId: 'user-1',
94-
workspaceId: WORKSPACE_ID,
95-
delegationId: 'delegation-1',
96-
audience: 'table',
97-
issuedAt: new Date('2026-01-01'),
98-
expiresAt: new Date('2026-01-02'),
99-
},
91+
principal: createTestRuntimePrincipal(),
10092
transport: 'executor_jwt',
10193
executionWorkspaceId: WORKSPACE_ID,
10294
})

‎apps/sim/app/api/table/[tableId]/rows/route.test.ts‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import { NextRequest } from 'next/server'
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
authenticate: vi.fn(),
@@ -70,16 +71,7 @@ function sessionPrincipal() {
7071

7172
function executorPrincipal() {
7273
mocks.authenticate.mockResolvedValue({
73-
principal: {
74-
kind: 'delegated',
75-
serviceId: 'executor',
76-
subjectUserId: 'user-1',
77-
workspaceId: 'workspace-canonical',
78-
delegationId: 'delegation-1',
79-
audience: 'sim:tables',
80-
issuedAt: new Date('2026-01-01'),
81-
expiresAt: new Date('2026-01-02'),
82-
},
74+
principal: createTestRuntimePrincipal(),
8375
transport: 'executor_jwt',
8476
executionWorkspaceId: 'workspace-canonical',
8577
})

‎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
@@ -111,6 +111,7 @@ describe('CredentialGroupBlockHandler', () => {
111111
principal,
112112
input: {
113113
credentialGroupId: 'group-1',
114+
assertedWorkspaceId: 'workspace-1',
114115
email: 'person@example.com',
115116
credentialProviderIds: ['google-email'],
116117
limit: 25,
@@ -159,6 +160,7 @@ describe('CredentialGroupBlockHandler', () => {
159160
principal: actorlessPrincipal,
160161
input: {
161162
credentialGroupId: 'group-1',
163+
assertedWorkspaceId: 'workspace-1',
162164
limit: 100,
163165
cursor: undefined,
164166
email: undefined,
@@ -242,7 +244,11 @@ describe('CredentialGroupBlockHandler', () => {
242244
)
243245
expect(mocks.sendInvite).toHaveBeenCalledWith({
244246
principal,
245-
input: { credentialGroupId: 'group-1', email: 'person@example.com' },
247+
input: {
248+
credentialGroupId: 'group-1',
249+
assertedWorkspaceId: 'workspace-1',
250+
email: 'person@example.com',
251+
},
246252
})
247253
})
248254

@@ -273,7 +279,11 @@ describe('CredentialGroupBlockHandler', () => {
273279
)
274280
expect(mocks.createInviteLink).toHaveBeenCalledWith({
275281
principal,
276-
input: { credentialGroupId: 'group-1', email: 'person@example.com' },
282+
input: {
283+
credentialGroupId: 'group-1',
284+
assertedWorkspaceId: 'workspace-1',
285+
email: 'person@example.com',
286+
},
277287
})
278288
expect(mocks.sendInvite).not.toHaveBeenCalled()
279289
expect(result).toEqual({

0 commit comments

Comments
 (0)