Skip to content

Commit 27684e0

Browse files
MaxwellCalkinclaude
andcommitted
fix(security): enforce write permission on target workspace in folder duplicate
The folder duplicate endpoint only checked permissions on the source workspace. When a different workspaceId was provided in the request body, folders (and their entire nested structure) were created in the target workspace without verifying the user had write access to it. The workflow duplicate helper (lib/workflows/persistence/duplicate.ts) already enforces this check, but the folder duplicate route bypassed it for folder creation. Add a permission check on the target workspace when it differs from the source, requiring write or admin access before proceeding. Add tests for cross-workspace authorization scenarios. > **AI Disclosure:** This PR was authored by an AI (Claude, Anthropic). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 8c0a2e0 commit 27684e0

File tree

2 files changed

+256
-0
lines changed

2 files changed

+256
-0
lines changed
Lines changed: 238 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,238 @@
1+
/**
2+
* Tests for folder duplicate API route (/api/folders/[id]/duplicate)
3+
*
4+
* @vitest-environment node
5+
*/
6+
import { auditMock, createMockRequest, type MockUser } from '@sim/testing'
7+
import { beforeEach, describe, expect, it, vi } from 'vitest'
8+
9+
const { mockGetSession, mockGetUserEntityPermissions, mockLogger, mockDbRef, mockDuplicateWorkflow } =
10+
vi.hoisted(() => {
11+
const logger = {
12+
info: vi.fn(),
13+
warn: vi.fn(),
14+
error: vi.fn(),
15+
debug: vi.fn(),
16+
trace: vi.fn(),
17+
fatal: vi.fn(),
18+
child: vi.fn(),
19+
}
20+
return {
21+
mockGetSession: vi.fn(),
22+
mockGetUserEntityPermissions: vi.fn(),
23+
mockLogger: logger,
24+
mockDbRef: { current: null as any },
25+
mockDuplicateWorkflow: vi.fn(),
26+
}
27+
})
28+
29+
vi.mock('@/lib/audit/log', () => auditMock)
30+
vi.mock('@/lib/auth', () => ({
31+
getSession: mockGetSession,
32+
}))
33+
vi.mock('@sim/logger', () => ({
34+
createLogger: vi.fn().mockReturnValue(mockLogger),
35+
}))
36+
vi.mock('@/lib/workspaces/permissions/utils', () => ({
37+
getUserEntityPermissions: mockGetUserEntityPermissions,
38+
}))
39+
vi.mock('@/lib/workflows/persistence/duplicate', () => ({
40+
duplicateWorkflow: mockDuplicateWorkflow,
41+
}))
42+
vi.mock('@sim/db', () => ({
43+
get db() {
44+
return mockDbRef.current
45+
},
46+
}))
47+
48+
import { POST } from '@/app/api/folders/[id]/duplicate/route'
49+
50+
const TEST_USER: MockUser = {
51+
id: 'user-123',
52+
email: 'test@example.com',
53+
name: 'Test User',
54+
}
55+
56+
const SOURCE_WORKSPACE_ID = 'workspace-source'
57+
const TARGET_WORKSPACE_ID = 'workspace-target'
58+
59+
const mockFolder = {
60+
id: 'folder-1',
61+
name: 'Source Folder',
62+
userId: TEST_USER.id,
63+
workspaceId: SOURCE_WORKSPACE_ID,
64+
parentId: null,
65+
color: '#6B7280',
66+
sortOrder: 1,
67+
isExpanded: false,
68+
createdAt: new Date('2024-01-01T00:00:00Z'),
69+
updatedAt: new Date('2024-01-01T00:00:00Z'),
70+
}
71+
72+
function createDuplicateDbMock(options: { folderLookupResult?: any; throwError?: boolean } = {}) {
73+
const { folderLookupResult = mockFolder, throwError = false } = options
74+
75+
const mockSelect = vi.fn().mockImplementation(() => ({
76+
from: vi.fn().mockImplementation(() => ({
77+
where: vi.fn().mockImplementation(() => ({
78+
then: vi.fn().mockImplementation((callback) => {
79+
if (throwError) {
80+
throw new Error('Database error')
81+
}
82+
const result = folderLookupResult ? [folderLookupResult] : []
83+
return Promise.resolve(callback(result))
84+
}),
85+
})),
86+
})),
87+
}))
88+
89+
const mockTransactionInsert = vi.fn().mockReturnValue({
90+
values: vi.fn().mockResolvedValue(undefined),
91+
})
92+
93+
const minSelectMock = vi.fn().mockReturnValue({
94+
from: vi.fn().mockReturnValue({
95+
where: vi.fn().mockResolvedValue([{ minSortOrder: 0 }]),
96+
}),
97+
})
98+
99+
const txMock = {
100+
select: minSelectMock,
101+
insert: mockTransactionInsert,
102+
}
103+
104+
const mockTransaction = vi.fn().mockImplementation(async (fn: any) => {
105+
return fn(txMock)
106+
})
107+
108+
return {
109+
select: mockSelect,
110+
transaction: mockTransaction,
111+
}
112+
}
113+
114+
function mockAuthenticatedUser(user?: MockUser) {
115+
mockGetSession.mockResolvedValue({ user: user || TEST_USER })
116+
}
117+
118+
function mockUnauthenticated() {
119+
mockGetSession.mockResolvedValue(null)
120+
}
121+
122+
describe('Folder Duplicate API Route', () => {
123+
beforeEach(() => {
124+
vi.clearAllMocks()
125+
mockGetUserEntityPermissions.mockResolvedValue('admin')
126+
mockDbRef.current = createDuplicateDbMock()
127+
mockDuplicateWorkflow.mockResolvedValue({
128+
id: 'new-workflow-1',
129+
name: 'Duplicated Workflow',
130+
})
131+
})
132+
133+
describe('POST /api/folders/[id]/duplicate', () => {
134+
it('should reject unauthenticated requests', async () => {
135+
mockUnauthenticated()
136+
137+
const req = createMockRequest('POST', {
138+
name: 'Duplicate Folder',
139+
})
140+
const params = Promise.resolve({ id: 'folder-1' })
141+
142+
const response = await POST(req as any, { params })
143+
144+
expect(response.status).toBe(401)
145+
})
146+
147+
it('should reject when user has read-only access to source workspace', async () => {
148+
mockAuthenticatedUser()
149+
mockGetUserEntityPermissions.mockResolvedValue('read')
150+
151+
const req = createMockRequest('POST', {
152+
name: 'Duplicate Folder',
153+
})
154+
const params = Promise.resolve({ id: 'folder-1' })
155+
156+
const response = await POST(req as any, { params })
157+
158+
expect(response.status).toBe(403)
159+
})
160+
161+
it('should reject when user lacks access to a different target workspace', async () => {
162+
mockAuthenticatedUser()
163+
164+
mockGetUserEntityPermissions
165+
.mockResolvedValueOnce('admin') // source workspace check
166+
.mockResolvedValueOnce('read') // target workspace check - read only
167+
168+
const req = createMockRequest('POST', {
169+
name: 'Duplicate Folder',
170+
workspaceId: TARGET_WORKSPACE_ID,
171+
})
172+
const params = Promise.resolve({ id: 'folder-1' })
173+
174+
const response = await POST(req as any, { params })
175+
176+
expect(response.status).toBe(403)
177+
const data = await response.json()
178+
expect(data.error).toBe('Write or admin access required for target workspace')
179+
})
180+
181+
it('should reject when user has no permission on target workspace', async () => {
182+
mockAuthenticatedUser()
183+
184+
mockGetUserEntityPermissions
185+
.mockResolvedValueOnce('admin') // source workspace check
186+
.mockResolvedValueOnce(null) // target workspace check - no access
187+
188+
const req = createMockRequest('POST', {
189+
name: 'Duplicate Folder',
190+
workspaceId: TARGET_WORKSPACE_ID,
191+
})
192+
const params = Promise.resolve({ id: 'folder-1' })
193+
194+
const response = await POST(req as any, { params })
195+
196+
expect(response.status).toBe(403)
197+
})
198+
199+
it('should not check target workspace permission when duplicating within same workspace', async () => {
200+
mockAuthenticatedUser()
201+
mockGetUserEntityPermissions.mockResolvedValue('admin')
202+
203+
const req = createMockRequest('POST', {
204+
name: 'Duplicate Folder',
205+
})
206+
const params = Promise.resolve({ id: 'folder-1' })
207+
208+
await POST(req as any, { params })
209+
210+
expect(mockGetUserEntityPermissions).toHaveBeenCalledTimes(1)
211+
expect(mockGetUserEntityPermissions).toHaveBeenCalledWith(
212+
TEST_USER.id,
213+
'workspace',
214+
SOURCE_WORKSPACE_ID
215+
)
216+
})
217+
218+
it('should check target workspace permission when workspaceId differs', async () => {
219+
mockAuthenticatedUser()
220+
mockGetUserEntityPermissions.mockResolvedValue('admin')
221+
222+
const req = createMockRequest('POST', {
223+
name: 'Duplicate Folder',
224+
workspaceId: TARGET_WORKSPACE_ID,
225+
})
226+
const params = Promise.resolve({ id: 'folder-1' })
227+
228+
await POST(req as any, { params })
229+
230+
expect(mockGetUserEntityPermissions).toHaveBeenCalledTimes(2)
231+
expect(mockGetUserEntityPermissions).toHaveBeenCalledWith(
232+
TEST_USER.id,
233+
'workspace',
234+
TARGET_WORKSPACE_ID
235+
)
236+
})
237+
})
238+
})

apps/sim/app/api/folders/[id]/duplicate/route.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,24 @@ export async function POST(req: NextRequest, { params }: { params: Promise<{ id:
5959

6060
const targetWorkspaceId = workspaceId || sourceFolder.workspaceId
6161

62+
if (targetWorkspaceId !== sourceFolder.workspaceId) {
63+
const targetPermission = await getUserEntityPermissions(
64+
session.user.id,
65+
'workspace',
66+
targetWorkspaceId
67+
)
68+
69+
if (!targetPermission || targetPermission === 'read') {
70+
logger.warn(
71+
`[${requestId}] User ${session.user.id} denied write access to target workspace ${targetWorkspaceId}`
72+
)
73+
return NextResponse.json(
74+
{ error: 'Write or admin access required for target workspace' },
75+
{ status: 403 }
76+
)
77+
}
78+
}
79+
6280
const { newFolderId, folderMapping } = await db.transaction(async (tx) => {
6381
const newFolderId = crypto.randomUUID()
6482
const now = new Date()

0 commit comments

Comments
 (0)