Skip to content

Commit 119d5a5

Browse files
committed
Fix auth bypass, SSRF, and wrong size limit comment
- Add 'patch' to workspace_file WRITE_ACTIONS — patch operation was missing, letting read-only users modify file content - Add download_to_workspace_file to WRITE_ACTIONS with '*' wildcard — tool was completely ungated, letting read-only users write workspace files - Update isActionAllowed to handle '*' (always-write tools) and undefined action (tools with no operation/action field) - Block private/internal URLs in download_to_workspace_file to prevent SSRF against RFC 1918 ranges, loopback, and cloud metadata endpoints - Fix file-reader.ts image size limit comment and error message (was 20MB, actual constant is 5MB)
1 parent cc50d1e commit 119d5a5

File tree

3 files changed

+49
-9
lines changed

3 files changed

+49
-9
lines changed

apps/sim/lib/copilot/tools/server/files/download-to-workspace-file.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,26 @@ const DownloadToWorkspaceFileResultSchema = z.object({
3030
type DownloadToWorkspaceFileArgs = z.infer<typeof DownloadToWorkspaceFileArgsSchema>
3131
type DownloadToWorkspaceFileResult = z.infer<typeof DownloadToWorkspaceFileResultSchema>
3232

33+
function isPrivateUrl(url: string): boolean {
34+
try {
35+
const { hostname, protocol } = new URL(url)
36+
if (protocol !== 'https:' && protocol !== 'http:') return true
37+
if (hostname === 'localhost' || hostname === '::1') return true
38+
const ipv4 = hostname.match(/^(\d+)\.(\d+)\.(\d+)\.(\d+)$/)
39+
if (ipv4) {
40+
const [a, b] = [Number(ipv4[1]), Number(ipv4[2])]
41+
// Loopback, RFC 1918, link-local (including cloud metadata 169.254.169.254)
42+
if (a === 127 || a === 10 || a === 0) return true
43+
if (a === 172 && b >= 16 && b <= 31) return true
44+
if (a === 192 && b === 168) return true
45+
if (a === 169 && b === 254) return true
46+
}
47+
return false
48+
} catch {
49+
return true
50+
}
51+
}
52+
3353
function sanitizeFileName(fileName: string): string {
3454
return fileName.replace(/[\\/:*?"<>|\u0000-\u001f]+/g, '_').trim()
3555
}
@@ -134,6 +154,13 @@ export const downloadToWorkspaceFileServerTool: BaseServerTool<
134154
try {
135155
assertServerToolNotAborted(context)
136156

157+
if (isPrivateUrl(params.url)) {
158+
return {
159+
success: false,
160+
message: 'Downloading from private or internal URLs is not allowed',
161+
}
162+
}
163+
137164
const response = await fetch(params.url, {
138165
redirect: 'follow',
139166
signal: context.abortSignal,

apps/sim/lib/copilot/tools/server/router.ts

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -64,17 +64,29 @@ const WRITE_ACTIONS: Record<string, string[]> = {
6464
manage_mcp_tool: ['add', 'edit', 'delete'],
6565
manage_skill: ['add', 'edit', 'delete'],
6666
manage_credential: ['rename', 'delete'],
67-
workspace_file: ['write', 'update', 'delete', 'rename'],
67+
workspace_file: ['write', 'update', 'delete', 'rename', 'patch'],
68+
download_to_workspace_file: ['*'],
6869
generate_visualization: ['generate'],
6970
generate_image: ['generate'],
7071
}
7172

72-
function isActionAllowed(toolName: string, action: string, userPermission: string): boolean {
73-
const writeActions = WRITE_ACTIONS[toolName]
74-
if (!writeActions || !writeActions.includes(action)) return true
73+
function isWritePermission(userPermission: string): boolean {
7574
return userPermission === 'write' || userPermission === 'admin'
7675
}
7776

77+
function isActionAllowed(
78+
toolName: string,
79+
action: string | undefined,
80+
userPermission: string
81+
): boolean {
82+
const writeActions = WRITE_ACTIONS[toolName]
83+
if (!writeActions) return true
84+
// '*' means the tool is always a write operation regardless of action field
85+
if (writeActions.includes('*')) return isWritePermission(userPermission)
86+
if (action && writeActions.includes(action)) return isWritePermission(userPermission)
87+
return true
88+
}
89+
7890
/** Registry of all server tools. Tools self-declare their validation schemas. */
7991
const serverToolRegistry: Record<string, BaseServerTool> = {
8092
[getBlocksMetadataServerTool.name]: getBlocksMetadataServerTool,
@@ -115,10 +127,11 @@ export async function routeExecution(
115127
// Action-level permission enforcement for mixed read/write tools
116128
if (context?.userPermission && WRITE_ACTIONS[toolName]) {
117129
const p = payload as Record<string, unknown>
118-
const action = (p?.operation ?? p?.action) as string
119-
if (action && !isActionAllowed(toolName, action, context.userPermission)) {
130+
const action = (p?.operation ?? p?.action) as string | undefined
131+
if (!isActionAllowed(toolName, action, context.userPermission)) {
132+
const actionLabel = action ? `'${action}' on ` : ''
120133
throw new Error(
121-
`Permission denied: '${action}' on ${toolName} requires write access. You have '${context.userPermission}' permission.`
134+
`Permission denied: ${actionLabel}${toolName} requires write access. You have '${context.userPermission}' permission.`
122135
)
123136
}
124137
}

apps/sim/lib/copilot/vfs/file-reader.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { isImageFileType } from '@/lib/uploads/utils/file-utils'
66
const logger = createLogger('FileReader')
77

88
const MAX_TEXT_READ_BYTES = 5 * 1024 * 1024 // 5 MB
9-
const MAX_IMAGE_READ_BYTES = 5 * 1024 * 1024 // 20 MB
9+
const MAX_IMAGE_READ_BYTES = 5 * 1024 * 1024 // 5 MB
1010

1111
const TEXT_TYPES = new Set([
1212
'text/plain',
@@ -54,7 +54,7 @@ export async function readFileRecord(record: WorkspaceFileRecord): Promise<FileR
5454
if (isImageFileType(record.type)) {
5555
if (record.size > MAX_IMAGE_READ_BYTES) {
5656
return {
57-
content: `[Image too large: ${record.name} (${(record.size / 1024 / 1024).toFixed(1)}MB, limit 20MB)]`,
57+
content: `[Image too large: ${record.name} (${(record.size / 1024 / 1024).toFixed(1)}MB, limit 5MB)]`,
5858
totalLines: 1,
5959
}
6060
}

0 commit comments

Comments
 (0)