Skip to content

Commit d86041b

Browse files
committed
fix(providers): enforce access checks and strip forged ids in the upload path
uploadLargeFilesToProvider runs on raw request messages for every caller (incl. the internal providers passthrough), so harden it independently of the agent path: - verifyFileAccess on each file's storage key before reading its bytes, so a forged key can't exfiltrate another user's file. - clear any inbound providerFileId/providerFileUri up front (legit ids are only set by the upload itself), so a forged id can't reference a file in a hosted account.
1 parent e47d983 commit d86041b

1 file changed

Lines changed: 34 additions & 0 deletions

File tree

apps/sim/providers/file-attachments.server.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,13 +98,24 @@ export async function attachLargeFileRemoteUrls(
9898
* For `files-api` providers, uploads each large attachment (already carrying a signed
9999
* `remoteUrl`) to the provider Files API and records the returned handle on the file.
100100
* Runs after the request's API key is resolved so hosted and BYOK keys both work.
101+
*
102+
* Any `providerFileId`/`providerFileUri` present on input is cleared first — legitimate ids
103+
* are only assigned by the upload below, so a forged id (which could otherwise reference an
104+
* arbitrary file in a shared hosted provider account) can never reach a message builder.
101105
*/
102106
export async function uploadLargeFilesToProvider(
103107
request: ProviderRequest,
104108
providerId: ProviderId | string
105109
): Promise<void> {
106110
if (getProviderFileStrategy(providerId) !== 'files-api') return
107111

112+
for (const message of request.messages ?? []) {
113+
for (const file of message.files ?? []) {
114+
file.providerFileId = undefined
115+
file.providerFileUri = undefined
116+
}
117+
}
118+
108119
const groups = groupUploadableFiles(request.messages)
109120
if (groups.length === 0) return
110121

@@ -114,6 +125,7 @@ export async function uploadLargeFilesToProvider(
114125

115126
for (const group of groups) {
116127
const [representative] = group
128+
await assertFileAccessForUpload(representative, request.userId)
117129
if (openai) {
118130
await uploadOpenAIFile(representative, openai, maxBytes, request.abortSignal)
119131
} else if (ai) {
@@ -126,6 +138,28 @@ export async function uploadLargeFilesToProvider(
126138
}
127139
}
128140

141+
/**
142+
* Verifies the caller may read this file before its bytes are uploaded to a provider. Enforced
143+
* for every caller of {@link uploadLargeFilesToProvider} (not just the agent path), so a forged
144+
* storage key in a passthrough request cannot exfiltrate another user's file.
145+
*/
146+
async function assertFileAccessForUpload(
147+
file: UserFile,
148+
userId: string | undefined
149+
): Promise<void> {
150+
if (!file.key) {
151+
throw new Error(`File "${file.name}" has no storage key`)
152+
}
153+
if (!userId) {
154+
throw new Error(`File "${file.name}" requires an authenticated user to upload`)
155+
}
156+
const context = (file.context as StorageContext) || inferContextFromKey(file.key)
157+
const hasAccess = await verifyFileAccess(file.key, userId, undefined, context, false)
158+
if (!hasAccess) {
159+
throw new Error(`File "${file.name}" is not accessible`)
160+
}
161+
}
162+
129163
/**
130164
* Groups large files needing a Files API upload by storage key so a file referenced across
131165
* multiple messages uploads once; the resulting handle is then applied to every occurrence.

0 commit comments

Comments
 (0)