Skip to content

Commit 2fda338

Browse files
Waleed Latifwaleedlatif1
authored andcommitted
fix(security): close ReDoS, zip-bomb, SSRF and redaction gaps found by audit
A dependency re-audit asked "does this library actually deliver the guarantee the calling code assumes" rather than "is it popular". These are the results. Security: - Copilot VFS glob() handed a caller-controlled pattern to micromatch, whose picomatch backend compiles `*` into a backtracking regex. A 25-char pattern took 41s; 29 chars exceeded 180s. Matching now runs on RE2, mirroring the grep() sibling in the same file that already did this. micromatch.makeRe() emits lookaheads RE2 cannot express, so picomatch's dot-assertions are translated into character exclusions; a differential test covers 519,688 pattern/path pairs against micromatch with no mismatches. Patterns that still carry an assertion fail closed rather than falling back. - doc-parser was the only OOXML-adjacent parser without the zip-bomb guard. officeparser sniffs content rather than extension, so a docx renamed .doc reached an unguarded unzip and could OOM the shared server. Genuine OLE2 .doc files are unaffected. Same guard added to the copilot style reader. - json-yaml-chunker called yaml.load with no alias-expansion cap. js-yaml has no maxAliasCount, so this is routed through assertYamlWithinLimits like the file parser already was. - Video tools and falai fetched provider-supplied URLs raw, including one that replayed an Authorization header to a polled status_url. Those URLs are now origin-pinned and revalidated on every poll. Cross-origin redirects drop credential headers by default, keyed on registrable domain rather than exact origin so same-site subdomain hops and http->https upgrades keep working. - Redaction key patterns were fully anchored, so openai_api_key, x-api-key, set-cookie, secretAccessKey and session_id were logged in the clear. The matcher now tokenizes keys, with exemptions so token-usage fields such as promptTokens stay readable. - drawtext escaping did not match ffmpeg's quoting grammar; a quote in agent supplied text escaped into filter options. Text is passed via textfile= so it never enters the filter string. - Loop conditions interpolated a resolved value into a JS string by hand. Supply chain: - Vendor free-email-domains' data. Its postinstall fetched a CDN CSV and overwrote the shipped list, so the lockfile hash covered the tarball but not the data actually used. It is inert under Bun today only because the package is outside trustedDependencies. - Document why xlsx is pinned to the SheetJS CDN: the npm copy is frozen at 0.18.5 with two unfixed CVEs, and a URL dependency is invisible to audit tooling, so upgrades have to be tracked by hand. Dependencies: - @daytonaio/sdk is deprecated in favour of @daytona/sdk (same API). - The @react-email/components 0.x line is deprecated; 1.0.12 needs no source changes here since no template uses <Tailwind> and render() already moved. - Load the pptx preview lazily so echarts leaves the Home, Files and share page bundles. - Drop autoprefixer (absent from the PostCSS config, so it never ran) and tailwind-merge (no importers; @sim/emcn declares its own).
1 parent cb3611b commit 2fda338

37 files changed

Lines changed: 8789 additions & 422 deletions

apps/sim/app/api/tools/onedrive/upload/route.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { createLogger } from '@sim/logger'
22
import { getErrorMessage } from '@sim/utils/errors'
33
import { type NextRequest, NextResponse } from 'next/server'
4+
/** Pinned to the SheetJS CDN, not npm — see the note in `lib/file-parsers/xlsx-parser.ts`. */
45
import * as XLSX from 'xlsx'
56
import { onedriveUploadContract } from '@/lib/api/contracts/tools/microsoft'
67
import { parseRequest } from '@/lib/api/server'
Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import {
5+
createMockRequest,
6+
hybridAuthMockFns,
7+
inputValidationMock,
8+
inputValidationMockFns,
9+
} from '@sim/testing'
10+
import { beforeEach, describe, expect, it, vi } from 'vitest'
11+
import { MAX_FAL_QUEUE_JSON_BYTES } from '@/lib/media/falai'
12+
13+
const { mockUploadFile } = vi.hoisted(() => ({
14+
mockUploadFile: vi.fn(),
15+
}))
16+
17+
vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock)
18+
vi.mock('@/lib/core/execution-limits', () => ({ getMaxExecutionTimeout: () => 30_000 }))
19+
vi.mock('@sim/utils/helpers', () => ({ sleep: () => Promise.resolve() }))
20+
vi.mock('@/app/api/files/authorization', () => ({
21+
assertToolFileAccess: vi.fn().mockResolvedValue(null),
22+
}))
23+
vi.mock('@/lib/uploads', () => ({
24+
StorageService: { uploadFile: mockUploadFile },
25+
}))
26+
vi.mock('@/lib/core/utils/urls', () => ({ getBaseUrl: () => 'https://sim.test' }))
27+
28+
import { POST } from '@/app/api/tools/video/route'
29+
30+
const { mockValidateUrlWithDNS, mockSecureFetchWithPinnedIP } = inputValidationMockFns
31+
32+
function jsonResponse(payload: unknown) {
33+
const text = JSON.stringify(payload)
34+
return {
35+
ok: true,
36+
status: 200,
37+
headers: { get: () => null },
38+
body: null,
39+
text: async () => text,
40+
arrayBuffer: async () => new ArrayBuffer(0),
41+
}
42+
}
43+
44+
function videoResponse() {
45+
return {
46+
ok: true,
47+
status: 200,
48+
headers: {
49+
get: (name: string) => {
50+
if (name === 'content-type') return 'video/mp4'
51+
return name === 'content-length' ? '8' : null
52+
},
53+
},
54+
body: null,
55+
text: async () => '',
56+
arrayBuffer: async () => new ArrayBuffer(8),
57+
}
58+
}
59+
60+
const baseBody = {
61+
provider: 'falai',
62+
apiKey: 'fal-key',
63+
model: 'kling-v3-pro',
64+
prompt: 'a cat riding a bike',
65+
}
66+
67+
describe('POST /api/tools/video (Fal.ai queue)', () => {
68+
const fetchMock = vi.fn()
69+
70+
beforeEach(() => {
71+
vi.clearAllMocks()
72+
vi.stubGlobal('fetch', fetchMock)
73+
hybridAuthMockFns.mockCheckInternalAuth.mockResolvedValue({
74+
success: true,
75+
userId: 'user-1',
76+
authType: 'internal_jwt',
77+
})
78+
mockValidateUrlWithDNS.mockResolvedValue({ isValid: true, resolvedIP: '8.8.8.8' })
79+
mockUploadFile.mockResolvedValue({ path: '/api/files/serve/video.mp4' })
80+
})
81+
82+
it('normalizes the echoed /response URL and bounds queue reads with the shared Fal cap', async () => {
83+
fetchMock.mockResolvedValue(
84+
new Response(
85+
JSON.stringify({
86+
request_id: 'req-1',
87+
status_url: 'https://queue.fal.run/fal-ai/kling-video/requests/req-1/status',
88+
response_url: 'https://queue.fal.run/fal-ai/kling-video/requests/req-1/response',
89+
}),
90+
{ status: 200 }
91+
)
92+
)
93+
mockSecureFetchWithPinnedIP
94+
.mockResolvedValueOnce(jsonResponse({ status: 'COMPLETED' }))
95+
.mockResolvedValueOnce(jsonResponse({ video: { url: 'https://cdn.fal.media/a.mp4' } }))
96+
.mockResolvedValueOnce(videoResponse())
97+
98+
const response = await POST(createMockRequest('POST', baseBody))
99+
expect(response.status).toBe(200)
100+
101+
const [statusCall, resultCall, downloadCall] = mockSecureFetchWithPinnedIP.mock.calls
102+
expect(statusCall[0]).toBe('https://queue.fal.run/fal-ai/kling-video/requests/req-1/status')
103+
// `/response` is not a GET route on queue.fal.run — it must be stripped.
104+
expect(resultCall[0]).toBe('https://queue.fal.run/fal-ai/kling-video/requests/req-1')
105+
expect(downloadCall[0]).toBe('https://cdn.fal.media/a.mp4')
106+
107+
expect(statusCall[2].maxResponseBytes).toBe(MAX_FAL_QUEUE_JSON_BYTES)
108+
expect(resultCall[2].maxResponseBytes).toBe(MAX_FAL_QUEUE_JSON_BYTES)
109+
})
110+
111+
it('falls back to the constructed multi-segment queue URL when the candidate is off-origin', async () => {
112+
fetchMock.mockResolvedValue(
113+
new Response(
114+
JSON.stringify({
115+
request_id: 'req-2',
116+
status_url: 'https://evil.example.net/steal',
117+
response_url: 'https://evil.example.net/steal',
118+
}),
119+
{ status: 200 }
120+
)
121+
)
122+
mockSecureFetchWithPinnedIP
123+
.mockResolvedValueOnce(jsonResponse({ status: 'COMPLETED' }))
124+
.mockResolvedValueOnce(jsonResponse({ video: { url: 'https://cdn.fal.media/a.mp4' } }))
125+
.mockResolvedValueOnce(videoResponse())
126+
127+
const response = await POST(createMockRequest('POST', baseBody))
128+
expect(response.status).toBe(200)
129+
130+
// `fal-ai/kling-video/v3/pro/text-to-video` polls under the app id only.
131+
expect(mockSecureFetchWithPinnedIP.mock.calls.slice(0, 2).map(([url]) => url)).toEqual([
132+
'https://queue.fal.run/fal-ai/kling-video/requests/req-2/status',
133+
'https://queue.fal.run/fal-ai/kling-video/requests/req-2',
134+
])
135+
})
136+
})

0 commit comments

Comments
 (0)