Skip to content

Commit 7567216

Browse files
committed
fix(security): close glob translation fail-opens and zip guard bypasses
Glob matcher (RE2 translation layer): - reject picomatch escape passthrough (\A, \z, \p{L}) that RE2 reads as anchors and Unicode classes, bypassing the dot:false guarantee - reject character-class ranges straddling the private-use segment markers - restore negated (!) glob semantics instead of silently matching nothing - parse zero-padded repeat bounds numerically Zip guard: - scan the whole buffer for EOCD candidates, matching JSZip and SheetJS rather than the spec window - treat a resolvable-but-empty central directory as unverifiable - keep stray EOCD byte sequences in non-ZIP documents a no-op so the OLE2 and plaintext fallbacks still run - bound central-directory scanning with a shared record budget Redaction: - fix quadratic acronym-boundary backtracking (73s to 1ms at 400k chars)
1 parent f1db97f commit 7567216

17 files changed

Lines changed: 1057 additions & 142 deletions

File tree

apps/sim/app/api/tools/video/route.test.ts

Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,17 @@ function videoResponse() {
5757
}
5858
}
5959

60+
function errorResponse(status: number) {
61+
return {
62+
ok: false,
63+
status,
64+
headers: { get: () => null },
65+
body: null,
66+
text: async () => 'denied',
67+
arrayBuffer: async () => new ArrayBuffer(0),
68+
}
69+
}
70+
6071
const baseBody = {
6172
provider: 'falai',
6273
apiKey: 'fal-key',
@@ -134,3 +145,192 @@ describe('POST /api/tools/video (Fal.ai queue)', () => {
134145
])
135146
})
136147
})
148+
149+
/**
150+
* Runway, Veo, Luma and MiniMax all download the finished asset through
151+
* `downloadVideoFromUrl` (the SSRF-guarded client), each with its own label and
152+
* error prefix. These cover that plumbing per provider.
153+
*/
154+
describe('POST /api/tools/video (provider download paths)', () => {
155+
const fetchMock = vi.fn()
156+
157+
beforeEach(() => {
158+
vi.clearAllMocks()
159+
vi.stubGlobal('fetch', fetchMock)
160+
hybridAuthMockFns.mockCheckInternalAuth.mockResolvedValue({
161+
success: true,
162+
userId: 'user-1',
163+
authType: 'internal_jwt',
164+
})
165+
mockValidateUrlWithDNS.mockResolvedValue({ isValid: true, resolvedIP: '8.8.8.8' })
166+
mockUploadFile.mockResolvedValue({ path: '/api/files/serve/video.mp4' })
167+
})
168+
169+
function apiResponse(payload: unknown) {
170+
return new Response(JSON.stringify(payload), { status: 200 })
171+
}
172+
173+
function veoFetches(uri: string) {
174+
fetchMock.mockResolvedValueOnce(apiResponse({ name: 'operations/op-1' })).mockResolvedValueOnce(
175+
apiResponse({
176+
done: true,
177+
response: { generateVideoResponse: { generatedSamples: [{ video: { uri } }] } },
178+
})
179+
)
180+
}
181+
182+
it('downloads the Runway asset through the guarded client', async () => {
183+
fetchMock
184+
.mockResolvedValueOnce(apiResponse({ id: 'task-1' }))
185+
.mockResolvedValueOnce(
186+
apiResponse({ status: 'SUCCEEDED', output: ['https://cdn.runwayml.test/a.mp4'] })
187+
)
188+
mockSecureFetchWithPinnedIP.mockResolvedValueOnce(videoResponse())
189+
190+
const response = await POST(
191+
createMockRequest('POST', {
192+
provider: 'runway',
193+
apiKey: 'runway-key',
194+
model: 'gen-4',
195+
prompt: 'a cat riding a bike',
196+
})
197+
)
198+
199+
expect(response.status).toBe(200)
200+
expect(mockSecureFetchWithPinnedIP).toHaveBeenCalledTimes(1)
201+
expect(mockSecureFetchWithPinnedIP.mock.calls[0][0]).toBe('https://cdn.runwayml.test/a.mp4')
202+
expect(mockSecureFetchWithPinnedIP.mock.calls[0][2].headers).toBeUndefined()
203+
})
204+
205+
it('surfaces the default download error prefix when the Runway asset fetch fails', async () => {
206+
fetchMock
207+
.mockResolvedValueOnce(apiResponse({ id: 'task-1' }))
208+
.mockResolvedValueOnce(
209+
apiResponse({ status: 'SUCCEEDED', output: ['https://cdn.runwayml.test/a.mp4'] })
210+
)
211+
mockSecureFetchWithPinnedIP.mockResolvedValueOnce(errorResponse(401))
212+
213+
const response = await POST(
214+
createMockRequest('POST', {
215+
provider: 'runway',
216+
apiKey: 'runway-key',
217+
model: 'gen-4',
218+
prompt: 'a cat riding a bike',
219+
})
220+
)
221+
222+
expect(response.status).toBe(500)
223+
expect((await response.json()).error).toBe('Failed to download video: 401')
224+
})
225+
226+
it('downloads the Luma asset through the guarded client', async () => {
227+
fetchMock
228+
.mockResolvedValueOnce(apiResponse({ id: 'gen-1' }))
229+
.mockResolvedValueOnce(
230+
apiResponse({ state: 'completed', assets: { video: 'https://cdn.lumalabs.test/a.mp4' } })
231+
)
232+
mockSecureFetchWithPinnedIP.mockResolvedValueOnce(videoResponse())
233+
234+
const response = await POST(
235+
createMockRequest('POST', {
236+
provider: 'luma',
237+
apiKey: 'luma-key',
238+
model: 'ray-2',
239+
prompt: 'a cat riding a bike',
240+
})
241+
)
242+
243+
expect(response.status).toBe(200)
244+
expect(mockSecureFetchWithPinnedIP.mock.calls[0][0]).toBe('https://cdn.lumalabs.test/a.mp4')
245+
expect(mockSecureFetchWithPinnedIP.mock.calls[0][2].headers).toBeUndefined()
246+
})
247+
248+
it('keeps the MiniMax-specific download error prefix', async () => {
249+
fetchMock
250+
.mockResolvedValueOnce(apiResponse({ base_resp: { status_code: 0 }, task_id: 'task-1' }))
251+
.mockResolvedValueOnce(
252+
apiResponse({ base_resp: { status_code: 0 }, status: 'Success', file_id: 'file-1' })
253+
)
254+
.mockResolvedValueOnce(
255+
apiResponse({ file: { download_url: 'https://cdn.minimax.test/a.mp4' } })
256+
)
257+
mockSecureFetchWithPinnedIP.mockResolvedValueOnce(errorResponse(401))
258+
259+
const response = await POST(
260+
createMockRequest('POST', {
261+
provider: 'minimax',
262+
apiKey: 'minimax-key',
263+
model: 'hailuo-2.3',
264+
prompt: 'a cat riding a bike',
265+
})
266+
)
267+
268+
expect(response.status).toBe(500)
269+
expect((await response.json()).error).toBe('Failed to download video from URL: 401')
270+
})
271+
272+
it('downloads the MiniMax asset through the guarded client', async () => {
273+
fetchMock
274+
.mockResolvedValueOnce(apiResponse({ base_resp: { status_code: 0 }, task_id: 'task-1' }))
275+
.mockResolvedValueOnce(
276+
apiResponse({ base_resp: { status_code: 0 }, status: 'Success', file_id: 'file-1' })
277+
)
278+
.mockResolvedValueOnce(
279+
apiResponse({ file: { download_url: 'https://cdn.minimax.test/a.mp4' } })
280+
)
281+
mockSecureFetchWithPinnedIP.mockResolvedValueOnce(videoResponse())
282+
283+
const response = await POST(
284+
createMockRequest('POST', {
285+
provider: 'minimax',
286+
apiKey: 'minimax-key',
287+
model: 'hailuo-2.3',
288+
prompt: 'a cat riding a bike',
289+
})
290+
)
291+
292+
expect(response.status).toBe(200)
293+
expect(mockSecureFetchWithPinnedIP.mock.calls[0][0]).toBe('https://cdn.minimax.test/a.mp4')
294+
})
295+
296+
it('attaches the Veo API key only for a genuine https Google API host', async () => {
297+
veoFetches('https://generativelanguage.googleapis.com/v1beta/files/a:download')
298+
mockSecureFetchWithPinnedIP.mockResolvedValueOnce(videoResponse())
299+
300+
const response = await POST(
301+
createMockRequest('POST', {
302+
provider: 'veo',
303+
apiKey: 'veo-key',
304+
model: 'veo-3',
305+
prompt: 'a cat riding a bike',
306+
})
307+
)
308+
309+
expect(response.status).toBe(200)
310+
expect(mockSecureFetchWithPinnedIP.mock.calls[0][2].headers).toEqual({
311+
'x-goog-api-key': 'veo-key',
312+
})
313+
})
314+
315+
it.each([
316+
['a suffix-spoofed host', 'https://evil.googleapis.com.attacker.test/a.mp4'],
317+
['a prefix-spoofed host', 'https://xgoogleapis.com/a.mp4'],
318+
['plaintext http', 'http://generativelanguage.googleapis.com/a.mp4'],
319+
])('withholds the Veo API key for %s', async (_label, uri) => {
320+
veoFetches(uri)
321+
mockSecureFetchWithPinnedIP.mockResolvedValueOnce(videoResponse())
322+
323+
const response = await POST(
324+
createMockRequest('POST', {
325+
provider: 'veo',
326+
apiKey: 'veo-key',
327+
model: 'veo-3',
328+
prompt: 'a cat riding a bike',
329+
})
330+
)
331+
332+
expect(response.status).toBe(200)
333+
expect(mockSecureFetchWithPinnedIP.mock.calls[0][0]).toBe(uri)
334+
expect(mockSecureFetchWithPinnedIP.mock.calls[0][2].headers).toBeUndefined()
335+
})
336+
})

apps/sim/app/api/tools/video/route.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -550,6 +550,15 @@ async function generateWithRunway(
550550
throw new Error('Runway generation timed out')
551551
}
552552

553+
/** Host of a provider-supplied URI for logging, without leaking the path or query. */
554+
function safeHostname(url: string): string {
555+
try {
556+
return new URL(url).host
557+
} catch {
558+
return 'unparseable'
559+
}
560+
}
561+
553562
/**
554563
* True when a provider-supplied URI is served by Google. The Veo download URI comes out of
555564
* the operation status body, so the `x-goog-api-key` credential is only attached when the
@@ -662,9 +671,17 @@ async function generateWithVeo(
662671
throw new Error('No video URI in response')
663672
}
664673

674+
const isGoogleHosted = isGoogleApiHost(videoUri)
675+
if (!isGoogleHosted) {
676+
logger.warn(
677+
`[${requestId}] Veo download URI is not a Google API host; sending the request unauthenticated. A 401 here means the URI host is wrong, not the API key.`,
678+
{ host: safeHostname(videoUri) }
679+
)
680+
}
681+
665682
return {
666683
buffer: await downloadVideoFromUrl(videoUri, 'Veo video', {
667-
headers: isGoogleApiHost(videoUri) ? { 'x-goog-api-key': apiKey } : undefined,
684+
headers: isGoogleHosted ? { 'x-goog-api-key': apiKey } : undefined,
668685
}),
669686
width: dimensions.width,
670687
height: dimensions.height,

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/file-viewer.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ const PdfViewerCore = dynamic(() => import('./pdf-viewer').then((m) => m.PdfView
3737
* `lib/pptx-renderer/renderer/chart-renderer`) stays out of every bundle that
3838
* statically imports this viewer - the Files page, the Home view and the public
3939
* share page - instead of only the visitors who open a PowerPoint file. The
40-
* fallback matches {@link PptxPreview}'s own pre-fetch frame, so the chunk load
40+
* fallback matches the frame `./pptx-preview` renders while it fetches, so the chunk load
4141
* and the binary fetch look like one continuous loading state. Rendered inside a
4242
* {@link PreviewErrorBoundary} so a rejected chunk load degrades to the preview
4343
* fallback — which offers a page reload, the only way to refetch a chunk whose

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/preview-shared.test.tsx

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,20 @@ describe('PreviewErrorBoundary', () => {
8484
expect(actionLabels()).toEqual(['Reload page'])
8585
})
8686

87+
it('offers a page reload for a failed CSS chunk, whose message omits "Loading chunk"', async () => {
88+
const Broken = lazy(() => Promise.reject(new Error('Loading CSS chunk 4821 failed')))
89+
90+
await render(
91+
<PreviewErrorBoundary label='PowerPoint'>
92+
<Suspense fallback={<span>loading</span>}>
93+
<Broken />
94+
</Suspense>
95+
</PreviewErrorBoundary>
96+
)
97+
98+
expect(actionLabels()).toEqual(['Reload page'])
99+
})
100+
87101
it('renders children when nothing throws', async () => {
88102
await render(
89103
<PreviewErrorBoundary label='PowerPoint'>

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/preview-shared.tsx

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,21 @@ export function PreviewError({ label, error, action }: PreviewErrorProps) {
2828
</p>
2929
<p className='text-[13px] text-[var(--text-muted)]'>{error}</p>
3030
{action ? (
31-
<Chip className='mt-[4px]' onClick={action.onClick}>
31+
<Chip variant='primary' className='mt-[4px]' onClick={action.onClick}>
3232
{action.label}
3333
</Chip>
3434
) : null}
3535
</div>
3636
)
3737
}
3838

39+
/**
40+
* webpack's chunk-load failure messages. The JS form is `Loading chunk 5 failed`
41+
* and the stylesheet form is `Loading CSS chunk 5 failed`, so a plain
42+
* `Loading chunk` substring test misses every CSS chunk 404.
43+
*/
44+
const CHUNK_LOAD_MESSAGE = /Loading (?:CSS )?chunk/
45+
3946
/**
4047
* A `next/dynamic` / `React.lazy` chunk fetch that rejected. The module system
4148
* caches the rejection on the lazy component itself, so re-rendering it throws
@@ -45,7 +52,7 @@ function isChunkLoadError(error: Error | undefined): boolean {
4552
if (!error) return false
4653
if (error.name === 'ChunkLoadError') return true
4754
return (
48-
error.message.includes('Loading chunk') || error.message.includes('dynamically imported module')
55+
CHUNK_LOAD_MESSAGE.test(error.message) || error.message.includes('dynamically imported module')
4956
)
5057
}
5158

apps/sim/executor/orchestrators/loop.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -732,10 +732,8 @@ export class LoopOrchestrator {
732732
if (lower === 'true' || lower === 'false') {
733733
return lower
734734
}
735-
/**
736-
* Serialized rather than hand-quoted: the value is a block output, so a `"` or
737-
* newline would close the literal early and run as code in the condition VM.
738-
*/
735+
// Serialized rather than hand-quoted: the value is a block output, so a `"` or
736+
// newline would close the literal early and run as code in the condition VM.
739737
return JSON.stringify(resolved)
740738
}
741739
return JSON.stringify(resolved)

apps/sim/lib/copilot/vfs/document-style.test.ts

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* @vitest-environment node
33
*/
44
import JSZip from 'jszip'
5-
import { describe, expect, it } from 'vitest'
5+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
66
import { extractDocumentStyle } from '@/lib/copilot/vfs/document-style'
77

88
const CENTRAL_DIRECTORY_HEADER_SIGNATURE = 0x02014b50
@@ -37,14 +37,56 @@ function forgeDeclaredUncompressedSize(zipBuffer: Buffer, declaredBytes: number)
3737
}
3838

3939
describe('extractDocumentStyle zip-bomb guard', () => {
40+
/**
41+
* `extractDocumentStyle` swallows every failure and returns `null`, and a
42+
* forged archive is one JSZip rejects on its own — so asserting `null` passes
43+
* with the guard deleted. Spying on the decompressor is what actually proves
44+
* the buffer never reached it.
45+
*/
46+
let loadAsync: ReturnType<typeof vi.spyOn<typeof JSZip, 'loadAsync'>>
47+
48+
beforeEach(() => {
49+
loadAsync = vi.spyOn(JSZip, 'loadAsync')
50+
})
51+
52+
afterEach(() => {
53+
loadAsync.mockRestore()
54+
})
55+
4056
it('refuses an archive whose declared expansion exceeds the limit', async () => {
4157
const bomb = forgeDeclaredUncompressedSize(await buildDocxArchive(), 0xfffffff0)
58+
4259
await expect(extractDocumentStyle(bomb, 'docx')).resolves.toBeNull()
60+
expect(loadAsync).not.toHaveBeenCalled()
61+
})
62+
63+
it('refuses a bomb whose EOCD lies about the entry count behind an empty-EOCD decoy', async () => {
64+
const bomb = forgeDeclaredUncompressedSize(await buildDocxArchive(), 0xfffffff0)
65+
const eocdOffset = bomb.lastIndexOf(Buffer.from([0x50, 0x4b, 0x05, 0x06]))
66+
const cdOffset = bomb.readUInt32LE(eocdOffset + 16)
67+
const decoy = Buffer.alloc(22)
68+
decoy.writeUInt32LE(0x06054b50, 0)
69+
70+
const realEocd = Buffer.from(bomb.subarray(eocdOffset))
71+
realEocd.writeUInt16LE(bomb.readUInt16LE(eocdOffset + 10) + 1, 8)
72+
realEocd.writeUInt16LE(bomb.readUInt16LE(eocdOffset + 10) + 1, 10)
73+
realEocd.writeUInt32LE(cdOffset + decoy.length, 16)
74+
75+
const attack = Buffer.concat([
76+
bomb.subarray(0, cdOffset),
77+
decoy,
78+
bomb.subarray(cdOffset, eocdOffset),
79+
realEocd,
80+
])
81+
82+
await expect(extractDocumentStyle(attack, 'docx')).resolves.toBeNull()
83+
expect(loadAsync).not.toHaveBeenCalled()
4384
})
4485

4586
it('still extracts style from a well-formed archive', async () => {
4687
const summary = await extractDocumentStyle(await buildDocxArchive(), 'docx')
4788

89+
expect(loadAsync).toHaveBeenCalledTimes(1)
4890
expect(summary).not.toBeNull()
4991
expect(summary?.theme?.fonts.minor).toBe('Calibri')
5092
expect(summary?.theme?.colors.accent1).toBe('4472C4')

0 commit comments

Comments
 (0)