Skip to content

Commit 7b6e43d

Browse files
waleedlatif1waleedlatif1
committed
fix(security): address audit findings from security fix review
- SMTP send: restructure attachment loop from Promise.all to sequential for...of so verifyFileAccess denial returns 404 instead of propagating as a generic 500 via the SMTP error classifier - Supabase tools: extend table-name validation and encodeURIComponent to the five previously missed tools — insert, upsert, count, query, text_search — completing coverage across all nine Supabase tools - Credential routes: remove unnecessary `request as any` casts in Gmail, OneDrive, and Wealthbox routes; authorizeCredentialUse already accepts NextRequest directly - Form soft delete: also set isActive=false alongside archivedAt so that any future code paths querying by isActive see a consistent state - SSH utils: fix exit code fallback from 0 to -1 so an abnormally closed connection that supplies no exit code is not reported as success - Workspace env: capitalize EXCLUDED.variables in the onConflictDoUpdate set clause to make the pseudo-table reference unambiguous
1 parent 7b84a79 commit 7b6e43d

13 files changed

Lines changed: 64 additions & 53 deletions

File tree

‎apps/sim/app/api/auth/oauth/wealthbox/items/route.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
3232
const { credentialId, type } = parsed.data.query
3333
const query = parsed.data.query.query ?? ''
3434

35-
const authz = await authorizeCredentialUse(request as any, {
35+
const authz = await authorizeCredentialUse(request, {
3636
credentialId,
3737
requireWorkflowIdForInternal: false,
3838
})

‎apps/sim/app/api/form/manage/[id]/route.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ export const DELETE = withRouteHandler(
199199

200200
await db
201201
.update(form)
202-
.set({ archivedAt: new Date(), updatedAt: new Date() })
202+
.set({ archivedAt: new Date(), isActive: false, updatedAt: new Date() })
203203
.where(eq(form.id, id))
204204

205205
logger.info(`Form ${id} soft deleted`)

‎apps/sim/app/api/tools/gmail/labels/route.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
4040
return NextResponse.json({ error: credentialIdValidation.error }, { status: 400 })
4141
}
4242

43-
const authz = await authorizeCredentialUse(request as any, {
43+
const authz = await authorizeCredentialUse(request, {
4444
credentialId,
4545
requireWorkflowIdForInternal: false,
4646
})

‎apps/sim/app/api/tools/onedrive/folders/route.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
4343
return NextResponse.json({ error: credentialIdValidation.error }, { status: 400 })
4444
}
4545

46-
const authz = await authorizeCredentialUse(request as any, {
46+
const authz = await authorizeCredentialUse(request, {
4747
credentialId,
4848
requireWorkflowIdForInternal: false,
4949
})

‎apps/sim/app/api/tools/smtp/send/route.ts‎

Lines changed: 33 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -120,44 +120,39 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
120120
)
121121
}
122122

123-
const attachmentBuffers = await Promise.all(
124-
attachments.map(async (file) => {
125-
try {
126-
if (typeof file.key !== 'string' || file.key.length === 0) {
127-
logger.warn(`[${requestId}] File access check rejected: missing key`)
128-
throw new Error('File not found')
129-
}
130-
if (!authResult.userId) {
131-
logger.warn(`[${requestId}] File access check requires userId but none available`)
132-
throw new Error('File not found')
133-
}
134-
const hasAccess = await verifyFileAccess(file.key, authResult.userId)
135-
if (!hasAccess) {
136-
logger.warn(`[${requestId}] File access denied for user`, {
137-
userId: authResult.userId,
138-
key: file.key,
139-
})
140-
throw new Error('File not found')
141-
}
142-
logger.info(
143-
`[${requestId}] Downloading attachment: ${file.name} (${file.size} bytes)`
144-
)
145-
146-
const buffer = await downloadFileFromStorage(file, requestId, logger)
147-
148-
return {
149-
filename: file.name,
150-
content: buffer,
151-
contentType: file.type || 'application/octet-stream',
152-
}
153-
} catch (error) {
154-
logger.error(`[${requestId}] Failed to download attachment ${file.name}:`, error)
155-
throw new Error(
156-
`Failed to download attachment "${file.name}": ${error instanceof Error ? error.message : 'Unknown error'}`
157-
)
158-
}
159-
})
160-
)
123+
const attachmentBuffers: { filename: string; content: Buffer; contentType: string }[] = []
124+
for (const file of attachments) {
125+
if (typeof file.key !== 'string' || file.key.length === 0) {
126+
logger.warn(`[${requestId}] File access check rejected: missing key`)
127+
return NextResponse.json({ success: false, error: 'File not found' }, { status: 404 })
128+
}
129+
if (!authResult.userId) {
130+
logger.warn(`[${requestId}] File access check requires userId but none available`)
131+
return NextResponse.json({ success: false, error: 'File not found' }, { status: 404 })
132+
}
133+
const hasAccess = await verifyFileAccess(file.key, authResult.userId)
134+
if (!hasAccess) {
135+
logger.warn(`[${requestId}] File access denied for user`, {
136+
userId: authResult.userId,
137+
key: file.key,
138+
})
139+
return NextResponse.json({ success: false, error: 'File not found' }, { status: 404 })
140+
}
141+
try {
142+
logger.info(`[${requestId}] Downloading attachment: ${file.name} (${file.size} bytes)`)
143+
const buffer = await downloadFileFromStorage(file, requestId, logger)
144+
attachmentBuffers.push({
145+
filename: file.name,
146+
content: buffer,
147+
contentType: file.type || 'application/octet-stream',
148+
})
149+
} catch (error) {
150+
logger.error(`[${requestId}] Failed to download attachment ${file.name}:`, error)
151+
throw new Error(
152+
`Failed to download attachment "${file.name}": ${error instanceof Error ? error.message : 'Unknown error'}`
153+
)
154+
}
155+
}
161156

162157
logger.info(`[${requestId}] Processed ${attachmentBuffers.length} attachment(s)`)
163158
mailOptions.attachments = attachmentBuffers

‎apps/sim/app/api/tools/ssh/utils.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ export function executeSSHCommand(client: Client, command: string): Promise<SSHC
202202
stderr: stderrTruncated
203203
? `${stderr}\n[stderr truncated: exceeded 16MB limit]`
204204
: stderr.trim(),
205-
exitCode: code ?? 0,
205+
exitCode: code ?? -1,
206206
})
207207
})
208208

‎apps/sim/app/api/tools/wealthbox/items/route.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
4545
return NextResponse.json({ error: credentialIdValidation.error }, { status: 400 })
4646
}
4747

48-
const authz = await authorizeCredentialUse(request as any, {
48+
const authz = await authorizeCredentialUse(request, {
4949
credentialId,
5050
requireWorkflowIdForInternal: false,
5151
})

‎apps/sim/app/api/workspaces/[id]/environment/route.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ export const PUT = withRouteHandler(
125125
.onConflictDoUpdate({
126126
target: [workspaceEnvironment.workspaceId],
127127
set: {
128-
variables: sql`${workspaceEnvironment.variables} || excluded.variables`,
128+
variables: sql`${workspaceEnvironment.variables} || EXCLUDED.variables`,
129129
updatedAt: new Date(),
130130
},
131131
})

‎apps/sim/tools/supabase/count.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,11 @@ export const countTool: ToolConfig<SupabaseCountParams, SupabaseCountResponse> =
5050

5151
request: {
5252
url: (params) => {
53-
let url = `${supabaseBaseUrl(params.projectId)}/rest/v1/${params.table}?select=*`
53+
if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(params.table)) {
54+
throw new Error('Invalid table name: must contain only letters, digits, and underscores')
55+
}
56+
let url = `${supabaseBaseUrl(params.projectId)}/rest/v1/${encodeURIComponent(params.table)}?select=*`
5457

55-
// Add filters if provided
5658
if (params.filter?.trim()) {
5759
url += `&${params.filter.trim()}`
5860
}

‎apps/sim/tools/supabase/insert.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,12 @@ export const insertTool: ToolConfig<SupabaseInsertParams, SupabaseInsertResponse
4343
},
4444

4545
request: {
46-
url: (params) => `${supabaseBaseUrl(params.projectId)}/rest/v1/${params.table}?select=*`,
46+
url: (params) => {
47+
if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(params.table)) {
48+
throw new Error('Invalid table name: must contain only letters, digits, and underscores')
49+
}
50+
return `${supabaseBaseUrl(params.projectId)}/rest/v1/${encodeURIComponent(params.table)}?select=*`
51+
},
4752
method: 'POST',
4853
headers: (params) => {
4954
const headers: Record<string, string> = {

0 commit comments

Comments
 (0)