Skip to content

Commit 6503671

Browse files
waleedlatif1waleedlatif1
authored
fix(security): harden findings — path traversal, SSRF, IDOR, file auth, credential access (#4571)
* fix(security): harden HIGH deepsec findings across multiple attack surfaces - Supabase tools (get_row, delete, update): validate table name with strict identifier regex and encodeURIComponent to prevent LLM-controlled path traversal to admin endpoints; add missing empty-filter guard to update matching the delete.ts pattern - SFTP/SMTP/SharePoint upload routes: add verifyFileAccess ownership check before downloadFileFromStorage, matching the WordPress reference pattern; rejects files the requesting user does not own with 404 - Gmail labels, OneDrive folders, Wealthbox items (×2): replace bare resolveOAuthAccountId + workspace-only membership check with authorizeCredentialUse which enforces credentialMember table; use credentialOwnerUserId for token refresh instead of bare accountRow.userId - A2A utils: thread pre-resolved IP from validateUrlWithDNS into A2A SDK via pinnedFetch (secureFetchWithPinnedIP) for JsonRpcTransportFactory, RestTransportFactory, and DefaultAgentCardResolver, closing the TOCTOU DNS rebinding window - SSH utils: cap stdout/stderr accumulation at 16 MB with truncation marker to prevent OOM from unbounded command output - Form DELETE route: replace db.delete() with db.update({archivedAt}) for true soft delete matching the schema's archivedAt column - Workflow admin import: fix Array.isArray() guard that silently dropped all variables (export format is Record, not Array) - Multipart upload: apply checkStorageQuota and MAX_WORKSPACE_FILE_SIZE to mothership context, closing the quota bypass for workspace-scoped storage * fix(security): eliminate workspace env lost-update race with atomic JSONB ops PUT: use `variables || excluded.variables` in onConflictDoUpdate so concurrent writes merge atomically in the DB instead of last-writer-wins at the application layer. DELETE: replace the read-modify-write upsert with a single UPDATE that removes keys via the JSONB `-` operator, preventing concurrent deletes from resurrecting previously-removed secrets. * 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 * fix(security): address PR review comments and harden deepsec fixes - fix(env): replace jsonb operators with transaction+FOR UPDATE read-modify-write - PUT: uses db.transaction + SELECT FOR UPDATE + JS merge to avoid lost-update race - DELETE: same pattern; fixes variable scope bug where current was referenced outside tx - removes broken || and - jsonb operators that fail on json-typed column - fix(ssh): trim truncated output consistently with non-truncated path - fix(gmail): remove redundant resolveOAuthAccountId call - adds credentialType field to CredentialAccessResult - authorizeCredentialUse now returns credentialType in all success paths - gmail/labels route uses authz.credentialType and authz.resolvedCredentialId directly - fix(supabase): centralize table identifier validation - adds validateDatabaseIdentifier() to input-validation.ts - all 8 supabase tools use the shared util instead of inline regex * fix(workflows): fix VariableType assignment in admin workflow import route The intermediate Record cast used 'string' for the type field which TypeScript correctly rejected — WorkflowVariable.type is 'VariableType', not string. Changed the cast to use VariableType so both branches typecheck correctly. * fix(a2a): handle Request objects in pinnedFetch URL extraction * fix(security): extract shared file-access guard; merge workspace/mothership branch * fix(security): advisory lock for env first-insert race; handle all BodyInit types in pinnedFetch * chore: remove inline comment from advisory lock * fix(security): remove stray comment; narrow credentialType to literal union * fix(security): add credentialId validation to wealthbox oauth route; fix null body override in pinnedFetch * fix(security): stream A2A response body to unblock SSE; keep text/json/arrayBuffer for non-streaming callers * fix(security): resolve credentialId guard on OneDrive, use assertToolFileAccess in WordPress, memoize body buffer to prevent silent empty reads, fix ArrayBuffer type cast * fix(security): handle string[][] HeadersInit format in pinnedFetch * fix(security): keep abort listener alive during body streaming; clean up in stream end/error/cancel * chore: remove extraneous inline comment * fix(security): cleanup abort listener when maxResponseBytes limit is exceeded
1 parent ec936be commit 6503671

26 files changed

Lines changed: 468 additions & 354 deletions

File tree

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

Lines changed: 21 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,12 @@
1-
import { db } from '@sim/db'
2-
import { account } from '@sim/db/schema'
31
import { createLogger } from '@sim/logger'
4-
import { eq } from 'drizzle-orm'
52
import { type NextRequest, NextResponse } from 'next/server'
63
import { wealthboxOAuthItemsContract } from '@/lib/api/contracts/selectors/wealthbox'
74
import { parseRequest } from '@/lib/api/server'
8-
import { getSession } from '@/lib/auth'
5+
import { authorizeCredentialUse } from '@/lib/auth/credential-access'
6+
import { validatePathSegment } from '@/lib/core/security/input-validation'
97
import { generateRequestId } from '@/lib/core/utils/request'
108
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
11-
import { refreshAccessTokenIfNeeded, resolveOAuthAccountId } from '@/app/api/auth/oauth/utils'
9+
import { refreshAccessTokenIfNeeded } from '@/app/api/auth/oauth/utils'
1210

1311
export const dynamic = 'force-dynamic'
1412

@@ -30,51 +28,34 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
3028
const requestId = generateRequestId()
3129

3230
try {
33-
const session = await getSession()
34-
35-
if (!session?.user?.id) {
36-
logger.warn(`[${requestId}] Unauthenticated request rejected`)
37-
return NextResponse.json({ error: 'User not authenticated' }, { status: 401 })
38-
}
39-
4031
const parsed = await parseRequest(wealthboxOAuthItemsContract, request, {})
4132
if (!parsed.success) return parsed.response
4233
const { credentialId, type } = parsed.data.query
4334
const query = parsed.data.query.query ?? ''
4435

45-
const resolved = await resolveOAuthAccountId(credentialId)
46-
if (!resolved) {
47-
return NextResponse.json({ error: 'Credential not found' }, { status: 404 })
48-
}
49-
50-
if (resolved.workspaceId) {
51-
const { getUserEntityPermissions } = await import('@/lib/workspaces/permissions/utils')
52-
const perm = await getUserEntityPermissions(
53-
session.user.id,
54-
'workspace',
55-
resolved.workspaceId
56-
)
57-
if (perm === null) {
58-
return NextResponse.json({ error: 'Forbidden' }, { status: 403 })
59-
}
36+
const credentialIdValidation = validatePathSegment(credentialId, {
37+
paramName: 'credentialId',
38+
maxLength: 100,
39+
allowHyphens: true,
40+
allowUnderscores: true,
41+
allowDots: false,
42+
})
43+
if (!credentialIdValidation.isValid) {
44+
logger.warn(`[${requestId}] Invalid credentialId format: ${credentialId}`)
45+
return NextResponse.json({ error: credentialIdValidation.error }, { status: 400 })
6046
}
6147

62-
const credentials = await db
63-
.select()
64-
.from(account)
65-
.where(eq(account.id, resolved.accountId))
66-
.limit(1)
67-
68-
if (!credentials.length) {
69-
logger.warn(`[${requestId}] Credential not found`, { credentialId })
70-
return NextResponse.json({ error: 'Credential not found' }, { status: 404 })
48+
const authz = await authorizeCredentialUse(request, {
49+
credentialId,
50+
requireWorkflowIdForInternal: false,
51+
})
52+
if (!authz.ok || !authz.credentialOwnerUserId) {
53+
return NextResponse.json({ error: authz.error || 'Unauthorized' }, { status: 403 })
7154
}
7255

73-
const accountRow = credentials[0]
74-
7556
const accessToken = await refreshAccessTokenIfNeeded(
76-
resolved.accountId,
77-
accountRow.userId,
57+
credentialId,
58+
authz.credentialOwnerUserId,
7859
requestId
7960
)
8061

‎apps/sim/app/api/files/authorization.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { db } from '@sim/db'
22
import { document, knowledgeBase, workspaceFile } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
44
import { and, eq, isNull, like, or } from 'drizzle-orm'
5+
import { NextResponse } from 'next/server'
56
import { getFileMetadata } from '@/lib/uploads'
67
import type { StorageContext } from '@/lib/uploads/config'
78
import { BLOB_CHAT_CONFIG, S3_CHAT_CONFIG } from '@/lib/uploads/config'
@@ -587,6 +588,36 @@ async function authorizeFileAccess(
587588
}
588589
}
589590

591+
/**
592+
* Guard helper for tool routes that download user files from storage.
593+
*
594+
* Validates that `key` is a non-empty string, that `userId` is present, and
595+
* that the authenticated user owns the file. Returns a 404 `NextResponse` on
596+
* any failure so callers can `return` it immediately; returns `null` when
597+
* access is granted.
598+
*/
599+
export async function assertToolFileAccess(
600+
key: unknown,
601+
userId: string | undefined,
602+
requestId: string,
603+
routeLogger: ReturnType<typeof createLogger>
604+
): Promise<NextResponse | null> {
605+
if (typeof key !== 'string' || key.length === 0) {
606+
routeLogger.warn(`[${requestId}] File access check rejected: missing key`)
607+
return NextResponse.json({ success: false, error: 'File not found' }, { status: 404 })
608+
}
609+
if (!userId) {
610+
routeLogger.warn(`[${requestId}] File access check requires userId but none available`)
611+
return NextResponse.json({ success: false, error: 'File not found' }, { status: 404 })
612+
}
613+
const hasAccess = await verifyFileAccess(key, userId)
614+
if (!hasAccess) {
615+
routeLogger.warn(`[${requestId}] File access denied for user`, { userId, key })
616+
return NextResponse.json({ success: false, error: 'File not found' }, { status: 404 })
617+
}
618+
return null
619+
}
620+
590621
/**
591622
* Get chat storage configuration based on current storage provider
592623
*/

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

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
136136
const config = getStorageConfig(storageContext)
137137

138138
let customKey: string | undefined
139-
if (context === 'workspace') {
139+
if (context === 'workspace' || context === 'mothership') {
140140
const { MAX_WORKSPACE_FILE_SIZE } = await import('@/lib/uploads/shared/types')
141141
if (typeof fileSize === 'number' && fileSize > MAX_WORKSPACE_FILE_SIZE) {
142142
return NextResponse.json(
@@ -158,11 +158,6 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
158158
{ status: 413 }
159159
)
160160
}
161-
} else if (context === 'mothership') {
162-
const { generateWorkspaceFileKey } = await import(
163-
'@/lib/uploads/contexts/workspace/workspace-file-manager'
164-
)
165-
customKey = generateWorkspaceFileKey(workspaceId, fileName)
166161
} else if (context === 'execution') {
167162
const workflowId = (data as { workflowId?: unknown }).workflowId
168163
const executionId = (data as { executionId?: unknown }).executionId

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,9 +197,12 @@ export const DELETE = withRouteHandler(
197197
return createErrorResponse('Form not found or access denied', 404)
198198
}
199199

200-
await db.delete(form).where(eq(form.id, id))
200+
await db
201+
.update(form)
202+
.set({ archivedAt: new Date(), isActive: false, updatedAt: new Date() })
203+
.where(eq(form.id, id))
201204

202-
logger.info(`Form ${id} deleted (soft delete)`)
205+
logger.info(`Form ${id} soft deleted`)
203206

204207
recordAudit({
205208
workspaceId: formWorkspaceId ?? null,

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

Lines changed: 11 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,15 @@
1-
import { db } from '@sim/db'
2-
import { account } from '@sim/db/schema'
31
import { createLogger } from '@sim/logger'
4-
import { eq } from 'drizzle-orm'
52
import { type NextRequest, NextResponse } from 'next/server'
63
import { gmailLabelsSelectorContract } from '@/lib/api/contracts/selectors/google'
74
import { parseRequest } from '@/lib/api/server'
8-
import { getSession } from '@/lib/auth'
5+
import { authorizeCredentialUse } from '@/lib/auth/credential-access'
96
import { validateAlphanumericId } from '@/lib/core/security/input-validation'
107
import { generateRequestId } from '@/lib/core/utils/request'
118
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
129
import { getScopesForService } from '@/lib/oauth/utils'
1310
import {
1411
getServiceAccountToken,
1512
refreshAccessTokenIfNeeded,
16-
resolveOAuthAccountId,
1713
ServiceAccountTokenError,
1814
} from '@/app/api/auth/oauth/utils'
1915
export const dynamic = 'force-dynamic'
@@ -32,13 +28,6 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
3228
const requestId = generateRequestId()
3329

3430
try {
35-
const session = await getSession()
36-
37-
if (!session?.user?.id) {
38-
logger.warn(`[${requestId}] Unauthenticated labels request rejected`)
39-
return NextResponse.json({ error: 'User not authenticated' }, { status: 401 })
40-
}
41-
4231
const parsed = await parseRequest(gmailLabelsSelectorContract, request, {})
4332
if (!parsed.success) return parsed.response
4433
const { credentialId, query } = parsed.data.query
@@ -50,52 +39,26 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
5039
return NextResponse.json({ error: credentialIdValidation.error }, { status: 400 })
5140
}
5241

53-
const resolved = await resolveOAuthAccountId(credentialId)
54-
if (!resolved) {
55-
return NextResponse.json({ error: 'Credential not found' }, { status: 404 })
56-
}
57-
58-
if (resolved.workspaceId) {
59-
const { getUserEntityPermissions } = await import('@/lib/workspaces/permissions/utils')
60-
const perm = await getUserEntityPermissions(
61-
session.user.id,
62-
'workspace',
63-
resolved.workspaceId
64-
)
65-
if (perm === null) {
66-
return NextResponse.json({ error: 'Forbidden' }, { status: 403 })
67-
}
42+
const authz = await authorizeCredentialUse(request, {
43+
credentialId,
44+
requireWorkflowIdForInternal: false,
45+
})
46+
if (!authz.ok || !authz.credentialOwnerUserId || !authz.resolvedCredentialId) {
47+
return NextResponse.json({ error: authz.error || 'Unauthorized' }, { status: 403 })
6848
}
6949

7050
let accessToken: string | null = null
7151

72-
if (resolved.credentialType === 'service_account' && resolved.credentialId) {
52+
if (authz.credentialType === 'service_account') {
7353
accessToken = await getServiceAccountToken(
74-
resolved.credentialId,
54+
authz.resolvedCredentialId,
7555
getScopesForService('gmail'),
7656
impersonateEmail
7757
)
7858
} else {
79-
const credentials = await db
80-
.select()
81-
.from(account)
82-
.where(eq(account.id, resolved.accountId))
83-
.limit(1)
84-
85-
if (!credentials.length) {
86-
logger.warn(`[${requestId}] Credential not found`)
87-
return NextResponse.json({ error: 'Credential not found' }, { status: 404 })
88-
}
89-
90-
const accountRow = credentials[0]
91-
92-
logger.info(
93-
`[${requestId}] Using credential: ${accountRow.id}, provider: ${accountRow.providerId}`
94-
)
95-
9659
accessToken = await refreshAccessTokenIfNeeded(
97-
resolved.accountId,
98-
accountRow.userId,
60+
credentialId,
61+
authz.credentialOwnerUserId,
9962
requestId,
10063
getScopesForService('gmail')
10164
)

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

Lines changed: 10 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,12 @@
1-
import { db } from '@sim/db'
2-
import { account } from '@sim/db/schema'
31
import { createLogger } from '@sim/logger'
42
import { generateId } from '@sim/utils/id'
5-
import { eq } from 'drizzle-orm'
63
import { type NextRequest, NextResponse } from 'next/server'
74
import { onedriveFoldersQuerySchema } from '@/lib/api/contracts/selectors/microsoft'
85
import { getValidationErrorMessage } from '@/lib/api/server'
9-
import { getSession } from '@/lib/auth'
6+
import { authorizeCredentialUse } from '@/lib/auth/credential-access'
107
import { validateMicrosoftGraphId } from '@/lib/core/security/input-validation'
118
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
12-
import { refreshAccessTokenIfNeeded, resolveOAuthAccountId } from '@/app/api/auth/oauth/utils'
9+
import { refreshAccessTokenIfNeeded } from '@/app/api/auth/oauth/utils'
1310
import type { MicrosoftGraphDriveItem } from '@/tools/onedrive/types'
1411

1512
export const dynamic = 'force-dynamic'
@@ -23,11 +20,6 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
2320
const requestId = generateId().slice(0, 8)
2421

2522
try {
26-
const session = await getSession()
27-
if (!session?.user?.id) {
28-
return NextResponse.json({ error: 'User not authenticated' }, { status: 401 })
29-
}
30-
3123
const { searchParams } = new URL(request.url)
3224
const validation = onedriveFoldersQuerySchema.safeParse({
3325
credentialId: searchParams.get('credentialId') ?? '',
@@ -51,37 +43,17 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
5143
return NextResponse.json({ error: credentialIdValidation.error }, { status: 400 })
5244
}
5345

54-
const resolved = await resolveOAuthAccountId(credentialId)
55-
if (!resolved) {
56-
return NextResponse.json({ error: 'Credential not found' }, { status: 404 })
57-
}
58-
59-
if (resolved.workspaceId) {
60-
const { getUserEntityPermissions } = await import('@/lib/workspaces/permissions/utils')
61-
const perm = await getUserEntityPermissions(
62-
session.user.id,
63-
'workspace',
64-
resolved.workspaceId
65-
)
66-
if (perm === null) {
67-
return NextResponse.json({ error: 'Forbidden' }, { status: 403 })
68-
}
69-
}
70-
71-
const credentials = await db
72-
.select()
73-
.from(account)
74-
.where(eq(account.id, resolved.accountId))
75-
.limit(1)
76-
if (!credentials.length) {
77-
return NextResponse.json({ error: 'Credential not found' }, { status: 404 })
46+
const authz = await authorizeCredentialUse(request, {
47+
credentialId,
48+
requireWorkflowIdForInternal: false,
49+
})
50+
if (!authz.ok || !authz.credentialOwnerUserId || !authz.resolvedCredentialId) {
51+
return NextResponse.json({ error: authz.error || 'Unauthorized' }, { status: 403 })
7852
}
7953

80-
const accountRow = credentials[0]
81-
8254
const accessToken = await refreshAccessTokenIfNeeded(
83-
resolved.accountId,
84-
accountRow.userId,
55+
credentialId,
56+
authz.credentialOwnerUserId,
8557
requestId
8658
)
8759
if (!accessToken) {

‎apps/sim/app/api/tools/sftp/upload/route.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { generateRequestId } from '@/lib/core/utils/request'
77
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
88
import { processFilesToUserFiles } from '@/lib/uploads/utils/file-utils'
99
import { downloadFileFromStorage } from '@/lib/uploads/utils/file-utils.server'
10+
import { assertToolFileAccess } from '@/app/api/files/authorization'
1011
import {
1112
createSftpConnection,
1213
getSftp,
@@ -95,6 +96,13 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
9596

9697
for (const file of userFiles) {
9798
try {
99+
const denied = await assertToolFileAccess(
100+
file.key,
101+
authResult.userId,
102+
requestId,
103+
logger
104+
)
105+
if (denied) return denied
98106
logger.info(
99107
`[${requestId}] Downloading file for upload: ${file.name} (${file.size} bytes)`
100108
)

‎apps/sim/app/api/tools/sharepoint/upload/route.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { generateRequestId } from '@/lib/core/utils/request'
99
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1010
import { processFilesToUserFiles } from '@/lib/uploads/utils/file-utils'
1111
import { downloadFileFromStorage } from '@/lib/uploads/utils/file-utils.server'
12+
import { assertToolFileAccess } from '@/app/api/files/authorization'
1213
import type { MicrosoftGraphDriveItem } from '@/tools/onedrive/types'
1314
import type { SharepointSkippedFile, SharepointUploadError } from '@/tools/sharepoint/types'
1415

@@ -82,6 +83,8 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
8283
const errors: SharepointUploadError[] = []
8384

8485
for (const userFile of userFiles) {
86+
const denied = await assertToolFileAccess(userFile.key, authResult.userId, requestId, logger)
87+
if (denied) return denied
8588
logger.info(`[${requestId}] Uploading file: ${userFile.name}`)
8689

8790
const buffer = await downloadFileFromStorage(userFile, requestId, logger)

0 commit comments

Comments
 (0)