Skip to content

Commit b7d57af

Browse files
waleedlatif1waleedlatif1
committed
fix(security): extract shared file-access guard; merge workspace/mothership branch
1 parent 7e1104e commit b7d57af

5 files changed

Lines changed: 46 additions & 74 deletions

File tree

‎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 & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -136,29 +136,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
136136
const config = getStorageConfig(storageContext)
137137

138138
let customKey: string | undefined
139-
if (context === 'workspace') {
140-
const { MAX_WORKSPACE_FILE_SIZE } = await import('@/lib/uploads/shared/types')
141-
if (typeof fileSize === 'number' && fileSize > MAX_WORKSPACE_FILE_SIZE) {
142-
return NextResponse.json(
143-
{ error: `File size exceeds maximum of ${MAX_WORKSPACE_FILE_SIZE} bytes` },
144-
{ status: 413 }
145-
)
146-
}
147-
148-
const { generateWorkspaceFileKey } = await import(
149-
'@/lib/uploads/contexts/workspace/workspace-file-manager'
150-
)
151-
customKey = generateWorkspaceFileKey(workspaceId, fileName)
152-
153-
const { checkStorageQuota } = await import('@/lib/billing/storage')
154-
const quotaCheck = await checkStorageQuota(userId, fileSize)
155-
if (!quotaCheck.allowed) {
156-
return NextResponse.json(
157-
{ error: quotaCheck.error || 'Storage limit exceeded' },
158-
{ status: 413 }
159-
)
160-
}
161-
} else if (context === 'mothership') {
139+
if (context === 'workspace' || context === 'mothership') {
162140
const { MAX_WORKSPACE_FILE_SIZE } = await import('@/lib/uploads/shared/types')
163141
if (typeof fileSize === 'number' && fileSize > MAX_WORKSPACE_FILE_SIZE) {
164142
return NextResponse.json(

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

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +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 { verifyFileAccess } from '@/app/api/files/authorization'
10+
import { assertToolFileAccess } from '@/app/api/files/authorization'
1111
import {
1212
createSftpConnection,
1313
getSftp,
@@ -96,22 +96,13 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
9696

9797
for (const file of userFiles) {
9898
try {
99-
if (typeof file.key !== 'string' || file.key.length === 0) {
100-
logger.warn(`[${requestId}] File access check rejected: missing key`)
101-
return NextResponse.json({ success: false, error: 'File not found' }, { status: 404 })
102-
}
103-
if (!authResult.userId) {
104-
logger.warn(`[${requestId}] File access check requires userId but none available`)
105-
return NextResponse.json({ success: false, error: 'File not found' }, { status: 404 })
106-
}
107-
const hasAccess = await verifyFileAccess(file.key, authResult.userId)
108-
if (!hasAccess) {
109-
logger.warn(`[${requestId}] File access denied for user`, {
110-
userId: authResult.userId,
111-
key: file.key,
112-
})
113-
return NextResponse.json({ success: false, error: 'File not found' }, { status: 404 })
114-
}
99+
const denied = await assertToolFileAccess(
100+
file.key,
101+
authResult.userId,
102+
requestId,
103+
logger
104+
)
105+
if (denied) return denied
115106
logger.info(
116107
`[${requestId}] Downloading file for upload: ${file.name} (${file.size} bytes)`
117108
)

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

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +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 { verifyFileAccess } from '@/app/api/files/authorization'
12+
import { assertToolFileAccess } from '@/app/api/files/authorization'
1313
import type { MicrosoftGraphDriveItem } from '@/tools/onedrive/types'
1414
import type { SharepointSkippedFile, SharepointUploadError } from '@/tools/sharepoint/types'
1515

@@ -83,22 +83,8 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
8383
const errors: SharepointUploadError[] = []
8484

8585
for (const userFile of userFiles) {
86-
if (typeof userFile.key !== 'string' || userFile.key.length === 0) {
87-
logger.warn(`[${requestId}] File access check rejected: missing key`)
88-
return NextResponse.json({ success: false, error: 'File not found' }, { status: 404 })
89-
}
90-
if (!authResult.userId) {
91-
logger.warn(`[${requestId}] File access check requires userId but none available`)
92-
return NextResponse.json({ success: false, error: 'File not found' }, { status: 404 })
93-
}
94-
const hasAccess = await verifyFileAccess(userFile.key, authResult.userId)
95-
if (!hasAccess) {
96-
logger.warn(`[${requestId}] File access denied for user`, {
97-
userId: authResult.userId,
98-
key: userFile.key,
99-
})
100-
return NextResponse.json({ success: false, error: 'File not found' }, { status: 404 })
101-
}
86+
const denied = await assertToolFileAccess(userFile.key, authResult.userId, requestId, logger)
87+
if (denied) return denied
10288
logger.info(`[${requestId}] Uploading file: ${userFile.name}`)
10389

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

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

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { generateRequestId } from '@/lib/core/utils/request'
1010
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1111
import { processFilesToUserFiles } from '@/lib/uploads/utils/file-utils'
1212
import { downloadFileFromStorage } from '@/lib/uploads/utils/file-utils.server'
13-
import { verifyFileAccess } from '@/app/api/files/authorization'
13+
import { assertToolFileAccess } from '@/app/api/files/authorization'
1414

1515
export const dynamic = 'force-dynamic'
1616

@@ -122,22 +122,8 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
122122

123123
const attachmentBuffers: { filename: string; content: Buffer; contentType: string }[] = []
124124
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-
}
125+
const denied = await assertToolFileAccess(file.key, authResult.userId, requestId, logger)
126+
if (denied) return denied
141127
try {
142128
logger.info(`[${requestId}] Downloading attachment: ${file.name} (${file.size} bytes)`)
143129
const buffer = await downloadFileFromStorage(file, requestId, logger)

0 commit comments

Comments
 (0)