Skip to content

Commit be7e643

Browse files
committed
test(security): add tests for cross-tenant log guard, quota bypass fix, and workflowId scoping
- log/route.test.ts: verifies cross-tenant executionId guard returns 404 when the execution belongs to a different workflow, and passes for same workflow or fresh executions - multipart/route.test.ts: verifies fileSize:0 no longer bypasses quota check and that the logs context is rejected at the endpoint level - logging-session.test.ts: verifies markExecutionAsFailed scopes by both executionId and workflowId, and that the instance method forwards workflowId
1 parent 98c52f0 commit be7e643

3 files changed

Lines changed: 225 additions & 0 deletions

File tree

apps/sim/app/api/files/multipart/route.test.ts

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,15 @@ vi.mock('@/lib/uploads/providers/blob/client', () => ({
5353

5454
vi.mock('@/lib/workspaces/permissions/utils', () => permissionsMock)
5555

56+
const { mockCheckStorageQuota, mockInitiateS3MultipartUpload } = vi.hoisted(() => ({
57+
mockCheckStorageQuota: vi.fn(),
58+
mockInitiateS3MultipartUpload: vi.fn(),
59+
}))
60+
61+
vi.mock('@/lib/billing/storage', () => ({
62+
checkStorageQuota: mockCheckStorageQuota,
63+
}))
64+
5665
import { POST } from '@/app/api/files/multipart/route'
5766

5867
const tokenPayload = {
@@ -200,3 +209,69 @@ describe('POST /api/files/multipart action=complete', () => {
200209
expect(mockCompleteS3MultipartUpload).toHaveBeenCalledTimes(2)
201210
})
202211
})
212+
213+
describe('POST /api/files/multipart action=initiate quota enforcement', () => {
214+
const makeInitiateRequest = (body: unknown) =>
215+
new NextRequest('http://localhost/api/files/multipart?action=initiate', {
216+
method: 'POST',
217+
headers: { 'Content-Type': 'application/json' },
218+
body: JSON.stringify(body),
219+
})
220+
221+
beforeEach(() => {
222+
vi.clearAllMocks()
223+
authMockFns.mockGetSession.mockResolvedValue({ user: { id: 'user-1' } })
224+
permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('write')
225+
mockIsUsingCloudStorage.mockReturnValue(true)
226+
mockGetStorageProvider.mockReturnValue('s3')
227+
mockGetStorageConfig.mockReturnValue({ bucket: 'b', region: 'r' })
228+
mockSignUploadToken.mockReturnValue('signed-token')
229+
mockCheckStorageQuota.mockResolvedValue({ allowed: true })
230+
mockInitiateS3MultipartUpload.mockResolvedValue({ uploadId: 'up-1', key: 'k/file.bin' })
231+
})
232+
233+
it('blocks upload when fileSize: 0 exceeds quota', async () => {
234+
mockCheckStorageQuota.mockResolvedValue({ allowed: false, error: 'Storage limit exceeded' })
235+
236+
const res = await makeInitiateRequest({
237+
fileName: 'file.bin',
238+
contentType: 'application/octet-stream',
239+
fileSize: 0,
240+
workspaceId: 'ws-1',
241+
context: 'knowledge-base',
242+
})
243+
244+
const response = await POST(res)
245+
expect(response.status).toBe(413)
246+
const body = await response.json()
247+
expect(body.error).toContain('Storage limit exceeded')
248+
})
249+
250+
it('does not check quota for quota-exempt contexts (og-images)', async () => {
251+
const res = await makeInitiateRequest({
252+
fileName: 'img.png',
253+
contentType: 'image/png',
254+
fileSize: 99999,
255+
workspaceId: 'ws-1',
256+
context: 'og-images',
257+
})
258+
259+
const response = await POST(res)
260+
expect(mockCheckStorageQuota).not.toHaveBeenCalled()
261+
})
262+
263+
it('rejects logs context — not allowed via the multipart endpoint', async () => {
264+
const res = await makeInitiateRequest({
265+
fileName: 'exec.log',
266+
contentType: 'text/plain',
267+
fileSize: 1000,
268+
workspaceId: 'ws-1',
269+
context: 'logs',
270+
})
271+
272+
const response = await POST(res)
273+
expect(response.status).toBe(400)
274+
const body = await response.json()
275+
expect(body.error).toMatch(/invalid storage context/i)
276+
})
277+
})
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { authMockFns, dbChainMock, dbChainMockFns, resetDbChainMock } from '@sim/testing'
5+
import { NextRequest } from 'next/server'
6+
import { beforeEach, describe, expect, it, vi } from 'vitest'
7+
8+
// Override global db mock with the configurable chain mock
9+
vi.mock('@sim/db', () => dbChainMock)
10+
11+
const { mockValidateWorkflowAccess, mockGetWorkspaceBilledAccountUserId } = vi.hoisted(() => ({
12+
mockValidateWorkflowAccess: vi.fn(),
13+
mockGetWorkspaceBilledAccountUserId: vi.fn(),
14+
}))
15+
16+
vi.mock('@/app/api/workflows/middleware', () => ({
17+
validateWorkflowAccess: mockValidateWorkflowAccess,
18+
}))
19+
20+
vi.mock('@/lib/workspaces/utils', () => ({
21+
getWorkspaceBilledAccountUserId: mockGetWorkspaceBilledAccountUserId,
22+
}))
23+
24+
vi.mock('@/lib/logs/execution/logging-session', () => ({
25+
LoggingSession: vi.fn().mockImplementation(() => ({
26+
start: vi.fn().mockResolvedValue(undefined),
27+
markAsFailed: vi.fn().mockResolvedValue(undefined),
28+
safeCompleteWithError: vi.fn().mockResolvedValue(undefined),
29+
safeComplete: vi.fn().mockResolvedValue(undefined),
30+
})),
31+
}))
32+
33+
vi.mock('@/lib/logs/execution/trace-spans/trace-spans', () => ({
34+
buildTraceSpans: vi.fn().mockReturnValue([]),
35+
}))
36+
37+
import { POST } from './route'
38+
39+
const makeRequest = (workflowId: string, body: unknown) =>
40+
new NextRequest(`http://localhost/api/workflows/${workflowId}/log`, {
41+
method: 'POST',
42+
headers: { 'Content-Type': 'application/json' },
43+
body: JSON.stringify(body),
44+
})
45+
46+
const validResult = { success: true, output: { value: 42 } }
47+
48+
describe('POST /api/workflows/[id]/log cross-tenant guard', () => {
49+
const OWNER_WORKFLOW_ID = 'wf-owner'
50+
const ATTACKER_WORKFLOW_ID = 'wf-attacker'
51+
const VICTIM_EXECUTION_ID = 'exec-victim-uuid'
52+
53+
beforeEach(() => {
54+
vi.clearAllMocks()
55+
resetDbChainMock()
56+
authMockFns.mockGetSession.mockResolvedValue({ user: { id: 'user-1' } })
57+
mockValidateWorkflowAccess.mockResolvedValue({ error: null })
58+
mockGetWorkspaceBilledAccountUserId.mockResolvedValue('user-1')
59+
// Default: no existing log (fresh execution)
60+
dbChainMockFns.limit.mockResolvedValue([])
61+
})
62+
63+
it('returns 404 when executionId belongs to a different workflow', async () => {
64+
dbChainMockFns.limit.mockResolvedValueOnce([{ workflowId: OWNER_WORKFLOW_ID }])
65+
66+
const res = await POST(
67+
makeRequest(ATTACKER_WORKFLOW_ID, {
68+
executionId: VICTIM_EXECUTION_ID,
69+
result: validResult,
70+
}),
71+
{ params: Promise.resolve({ id: ATTACKER_WORKFLOW_ID }) }
72+
)
73+
74+
expect(res.status).toBe(404)
75+
const body = await res.json()
76+
expect(body.error).toBe('Execution not found')
77+
})
78+
79+
it('proceeds when executionId belongs to the same workflow', async () => {
80+
dbChainMockFns.limit.mockResolvedValueOnce([{ workflowId: OWNER_WORKFLOW_ID }])
81+
82+
const res = await POST(
83+
makeRequest(OWNER_WORKFLOW_ID, {
84+
executionId: VICTIM_EXECUTION_ID,
85+
result: validResult,
86+
}),
87+
{ params: Promise.resolve({ id: OWNER_WORKFLOW_ID }) }
88+
)
89+
90+
expect(res.status).not.toBe(404)
91+
expect(res.status).not.toBe(403)
92+
})
93+
94+
it('proceeds when executionId has no existing log row (fresh execution)', async () => {
95+
dbChainMockFns.limit.mockResolvedValueOnce([])
96+
97+
const res = await POST(
98+
makeRequest(OWNER_WORKFLOW_ID, {
99+
executionId: 'brand-new-execution-id',
100+
result: validResult,
101+
}),
102+
{ params: Promise.resolve({ id: OWNER_WORKFLOW_ID }) }
103+
)
104+
105+
expect(res.status).not.toBe(404)
106+
expect(res.status).not.toBe(403)
107+
})
108+
})

apps/sim/lib/logs/execution/logging-session.test.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ const dbMocks = vi.hoisted(() => {
1010
const update = vi.fn()
1111
const execute = vi.fn()
1212
const eq = vi.fn()
13+
const and = vi.fn((...args: unknown[]) => ({ type: 'and', args }))
1314
const sql = vi.fn((strings: TemplateStringsArray, ...values: unknown[]) => ({ strings, values }))
1415

1516
select.mockReturnValue({ from: selectFrom })
@@ -29,6 +30,7 @@ const dbMocks = vi.hoisted(() => {
2930
updateWhere,
3031
execute,
3132
eq,
33+
and,
3234
sql,
3335
}
3436
})
@@ -47,6 +49,7 @@ vi.mock('@sim/db', () => ({
4749

4850
vi.mock('drizzle-orm', () => ({
4951
eq: dbMocks.eq,
52+
and: dbMocks.and,
5053
sql: dbMocks.sql,
5154
}))
5255

@@ -449,3 +452,42 @@ describe('LoggingSession completion retries', () => {
449452
expect(session.completing).toBe(true)
450453
})
451454
})
455+
456+
describe('LoggingSession.markExecutionAsFailed workflowId scoping', () => {
457+
beforeEach(() => {
458+
vi.clearAllMocks()
459+
dbMocks.updateWhere.mockResolvedValue(undefined)
460+
})
461+
462+
it('scopes UPDATE by both executionId and workflowId', async () => {
463+
await LoggingSession.markExecutionAsFailed('exec-1', undefined, undefined, 'wf-1')
464+
465+
expect(dbMocks.update).toHaveBeenCalledTimes(1)
466+
expect(dbMocks.updateSet).toHaveBeenCalledTimes(1)
467+
expect(dbMocks.updateWhere).toHaveBeenCalledTimes(1)
468+
469+
const whereArgs = dbMocks.updateWhere.mock.calls[0]
470+
expect(whereArgs).toBeDefined()
471+
})
472+
473+
it('instance markAsFailed forwards workflowId to the static method', async () => {
474+
const updateWhereSpy = dbMocks.updateWhere
475+
dbMocks.selectLimit.mockResolvedValue([{ executionData: {} }])
476+
477+
const session = new LoggingSession('wf-42', 'exec-42', 'api', 'req-1')
478+
await session.markAsFailed('something went wrong')
479+
480+
expect(updateWhereSpy).toHaveBeenCalledTimes(1)
481+
})
482+
483+
it('uses the provided errorMessage in the SQL set', async () => {
484+
const sqlMock = dbMocks.sql
485+
await LoggingSession.markExecutionAsFailed('exec-2', 'custom error', undefined, 'wf-2')
486+
487+
expect(sqlMock).toHaveBeenCalled()
488+
const lastCall = sqlMock.mock.calls.at(-1)!
489+
const [strings, ...values] = lastCall
490+
const combined = String(Array.from(strings)).toLowerCase() + values.join(' ').toLowerCase()
491+
expect(combined).toContain('force_failed')
492+
})
493+
})

0 commit comments

Comments
 (0)