Skip to content

Commit 519917a

Browse files
committed
fix(input-format): validate recovered internal-url keys via parseInternalFileUrl
Addresses review round 5 (Greptile): isInternalFileUrl only checks the URL prefix, but the executor parses+decodes the key. A malformed internal URL (no extractable key) passed the prefix check, got collected into workflowInput.files, then normalizeStartFile rejected it and dropped the whole files array. Now mirror the executor exactly — attempt parseInternalFileUrl and require a real key. Tests use a realistic recoverable internal URL.
1 parent f2718e9 commit 519917a

3 files changed

Lines changed: 39 additions & 21 deletions

File tree

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/starter/input-format-files.test.ts

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
const file: InputFormatFile = {
1111
id: 'f1',
1212
name: 'doc.pdf',
13-
url: '/api/files/serve/key',
13+
url: '/api/files/serve/workspace%2Fws-1%2F1700000000000-doc.pdf?context=workspace',
1414
key: 'key',
1515
size: 10,
1616
type: 'application/pdf',
@@ -20,11 +20,11 @@ describe('filesToControlValue', () => {
2020
it.concurrent('maps url -> path for the FileUpload value shape', () => {
2121
expect(filesToControlValue([file])).toEqual([
2222
{
23-
name: 'doc.pdf',
24-
path: '/api/files/serve/key',
25-
key: 'key',
26-
size: 10,
27-
type: 'application/pdf',
23+
name: file.name,
24+
path: file.url,
25+
key: file.key,
26+
size: file.size,
27+
type: file.type,
2828
},
2929
])
3030
})
@@ -43,9 +43,7 @@ describe('controlValueToFiles', () => {
4343
})
4444

4545
it.concurrent('matches an existing file by url when key is absent', () => {
46-
const control = [
47-
{ name: 'doc.pdf', path: '/api/files/serve/key', size: 10, type: 'application/pdf' },
48-
]
46+
const control = [{ name: 'doc.pdf', path: file.url, size: 10, type: 'application/pdf' }]
4947
expect(controlValueToFiles(control, [file])[0].id).toBe('f1')
5048
})
5149

@@ -67,11 +65,11 @@ describe('controlValueToFiles', () => {
6765

6866
it.concurrent('normalizes a single object or null to an array', () => {
6967
const single = {
70-
name: 'doc.pdf',
71-
path: '/api/files/serve/key',
72-
key: 'key',
73-
size: 10,
74-
type: 'application/pdf',
68+
name: file.name,
69+
path: file.url,
70+
key: file.key,
71+
size: file.size,
72+
type: file.type,
7573
}
7674
expect(controlValueToFiles(single, [file])).toEqual([file])
7775
expect(controlValueToFiles(null, [file])).toEqual([])

apps/sim/lib/workflows/input-format.test.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ describe('parseInputFormatFiles', () => {
252252
const file = {
253253
id: 'f1',
254254
name: 'doc.pdf',
255-
url: '/api/files/serve/key',
255+
url: '/api/files/serve/workspace%2Fws-1%2F1700000000000-doc.pdf?context=workspace',
256256
key: 'key',
257257
size: 10,
258258
type: 'application/pdf',
@@ -312,6 +312,13 @@ describe('parseInputFormatFiles', () => {
312312
expect(parseInputFormatFiles(JSON.stringify([noKey]))).toEqual([noKey])
313313
})
314314

315+
it.concurrent('rejects a key-less file whose internal url yields no key', () => {
316+
const { key, ...noKey } = file
317+
expect(parseInputFormatFiles(JSON.stringify([{ ...noKey, url: '/api/files/serve/' }]))).toEqual(
318+
[]
319+
)
320+
})
321+
315322
it.concurrent('rejects empty-string id/name/url/type (executor treats them as falsy)', () => {
316323
expect(parseInputFormatFiles(JSON.stringify([{ ...file, id: '' }]))).toEqual([])
317324
expect(parseInputFormatFiles(JSON.stringify([{ ...file, name: '' }]))).toEqual([])
@@ -324,7 +331,7 @@ describe('collectInputFormatFiles', () => {
324331
const file = {
325332
id: 'f1',
326333
name: 'doc.pdf',
327-
url: '/api/files/serve/key',
334+
url: '/api/files/serve/workspace%2Fws-1%2F1700000000000-doc.pdf?context=workspace',
328335
key: 'key',
329336
size: 10,
330337
type: 'application/pdf',

apps/sim/lib/workflows/input-format.ts

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { generateId } from '@sim/utils/id'
2-
import { isInternalFileUrl } from '@/lib/uploads/utils/file-utils'
2+
import { isInternalFileUrl, parseInternalFileUrl } from '@/lib/uploads/utils/file-utils'
33
import { isInputDefinitionTrigger } from '@/lib/workflows/triggers/input-definition-triggers'
44
import type { InputFormatField } from '@/lib/workflows/types'
55
import type { UserFile } from '@/executor/types'
@@ -62,6 +62,22 @@ export function isFileFieldType(type: string | null | undefined): boolean {
6262
export type InputFormatFile = Pick<UserFile, 'id' | 'name' | 'url' | 'size' | 'type'> &
6363
Pick<Partial<UserFile>, 'key'>
6464

65+
/**
66+
* Whether a file's key is usable at run time: an explicit non-empty `key`, or an
67+
* internal `/api/files/serve/...` URL the key can actually be parsed from. This
68+
* mirrors `normalizeStartFile` exactly (including the parse, so a malformed
69+
* internal URL is rejected rather than accepted on the prefix alone).
70+
*/
71+
function hasRecoverableFileKey(file: InputFormatFile): boolean {
72+
if (typeof file.key === 'string' && file.key.length > 0) return true
73+
if (typeof file.url !== 'string' || !isInternalFileUrl(file.url)) return false
74+
try {
75+
return parseInternalFileUrl(file.url).key.length > 0
76+
} catch {
77+
return false
78+
}
79+
}
80+
6581
/**
6682
* Tolerantly parses a file field's stored value (a JSON string, or an already
6783
* materialized array) into run-ready file objects. Returns an empty array for
@@ -91,9 +107,6 @@ export function parseInputFormatFiles(value: unknown): InputFormatFile[] {
91107
// as normalizeStartFile). A partial object or external/signed URL without a
92108
// key is rejected so it never opens in uploader mode or reaches the files
93109
// channel only to be dropped; it falls back to the JSON editor instead.
94-
const hasRecoverableKey =
95-
(typeof f.key === 'string' && f.key.length > 0) ||
96-
(typeof f.url === 'string' && isInternalFileUrl(f.url))
97110
// Non-empty strings: normalizeStartFile rejects falsy id/name/url/type, and
98111
// file normalization is all-or-nothing, so one empty-string field would drop
99112
// every file from the run.
@@ -108,7 +121,7 @@ export function parseInputFormatFiles(value: unknown): InputFormatFile[] {
108121
Number.isFinite(f.size) &&
109122
typeof f.type === 'string' &&
110123
f.type.length > 0 &&
111-
hasRecoverableKey
124+
hasRecoverableFileKey(f)
112125
)
113126
})
114127
}

0 commit comments

Comments
 (0)