Skip to content

Commit b967a3b

Browse files
fix(files): index generated docs without compiling and fix the docx sandbox bundle (#7386)
* fix(files): index generated docs without compiling and fix the docx sandbox bundle * fix(files): surface download aborts and sync the copilot artifact bucket to workers
1 parent 47c0805 commit b967a3b

13 files changed

Lines changed: 723 additions & 162 deletions

File tree

‎apps/sim/lib/execution/sandbox/bundles/_polyfills.ts‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,37 @@
77
* `ivm.Reference` per laverdet/isolated-vm#136) BEFORE the bundle runs, so
88
* `process/browser` picks up the real delegated `setTimeout`.
99
*
10-
* The only thing this file still does is alias `global -> globalThis` for
11-
* UMD-style fallbacks inside the bundles. All other runtime surface
12-
* (`console`, `TextEncoder`, `TextDecoder`, timers) is installed by the
13-
* worker via `ivm.Callback` / `ivm.Reference` bridges to Node's native
14-
* implementations — no hand-rolled polyfill logic lives in the isolate.
10+
* Beyond aliasing `global -> globalThis` for UMD-style fallbacks inside the
11+
* bundles, this file only answers the one name the bundler can leave dangling
12+
* (see below). All other runtime surface (`console`, `TextEncoder`,
13+
* `TextDecoder`, timers) is installed by the worker via `ivm.Callback` /
14+
* `ivm.Reference` bridges to Node's native implementations — no hand-rolled
15+
* polyfill logic lives in the isolate.
1516
*/
1617

17-
const g: typeof globalThis & { global?: typeof globalThis } = globalThis
18+
const g: typeof globalThis & {
19+
global?: typeof globalThis
20+
__require?: (id: string) => never
21+
} = globalThis
1822

1923
if (typeof g.global === 'undefined') g.global = globalThis
2024

25+
/**
26+
* A library that inlines a CommonJS dependency ships esbuild's `__require`
27+
* helper around it (docx >= 9.7.1 does this for JSZip's UMD build). Bun's
28+
* browser/iife build rewrites the bare `require` references inside that helper
29+
* to its own `__require` runtime helper and then never emits it, so the bundle
30+
* throws `ReferenceError: __require is not defined` while it is still being
31+
* evaluated. The isolate has no `require` at all, so the only correct answer
32+
* to a dynamic require is the one esbuild's helper gives when `require` is
33+
* absent: throw. Defining it here keeps every bundle self-contained; `build.ts`
34+
* evaluates each bundle in a bare context so a new variant of the defect fails
35+
* the build instead of shipping.
36+
*/
37+
if (typeof g.__require === 'undefined') {
38+
g.__require = (id: string): never => {
39+
throw new Error(`Dynamic require of "${id}" is not supported in the sandbox`)
40+
}
41+
}
42+
2143
export {}

‎apps/sim/lib/execution/sandbox/bundles/build.ts‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,20 @@
77
* `fs`). The emitted files attach their exports to `globalThis.__bundles[name]`
88
* and are checked in so production images don't need the bundler at runtime.
99
*
10+
* Every bundle is evaluated in a bare context before it is written: the
11+
* bundler can emit a reference to a runtime helper it never defines (Bun does
12+
* this for docx's inlined CommonJS shim), and nothing else loads these files
13+
* before a production document generation does.
14+
*
1015
* Run via: `bun run build:sandbox-bundles`.
1116
*/
1217

1318
import { mkdirSync, rmSync, writeFileSync } from 'node:fs'
1419
import { dirname, join } from 'node:path'
1520
import { fileURLToPath } from 'node:url'
1621
import { createLogger } from '@sim/logger'
22+
import { evaluateSandboxBundle } from '@/lib/execution/sandbox/bundles/verify'
23+
import type { SandboxBundleName } from '@/lib/execution/sandbox/types'
1724

1825
const logger = createLogger('SandboxBundleBuild')
1926

@@ -39,7 +46,7 @@ const APP_SIM_ROOT = join(HERE, '..', '..', '..', '..')
3946

4047
interface BundleSpec {
4148
/** Key on `globalThis.__bundles`. */
42-
name: string
49+
name: SandboxBundleName
4350
/** Short filename written under `bundles/<file>.cjs`. */
4451
outFile: string
4552
/** Source of the entry file bun will bundle. */
@@ -121,8 +128,16 @@ async function main(): Promise<void> {
121128

122129
const code = await result.outputs[0].text()
123130
const banner = `// sandbox bundle: ${spec.name}\n// generated by apps/sim/lib/execution/sandbox/bundles/build.ts\n// do not edit by hand. run \`bun run build:sandbox-bundles\` to regenerate.\n`
124-
writeFileSync(join(BUNDLES_DIR, spec.outFile), banner + code, 'utf-8')
125-
logger.info(`built ${spec.outFile} (${code.length.toLocaleString()} chars)`)
131+
const output = banner + code
132+
try {
133+
evaluateSandboxBundle(output, spec.name)
134+
} catch (error) {
135+
throw new Error(
136+
`Sandbox bundle ${spec.name} does not evaluate in a bare isolate context: ${String(error)}`
137+
)
138+
}
139+
writeFileSync(join(BUNDLES_DIR, spec.outFile), output, 'utf-8')
140+
logger.info(`built and verified ${spec.outFile} (${code.length.toLocaleString()} chars)`)
126141
}
127142

128143
rmSync(ENTRIES_DIR, { recursive: true, force: true })

‎apps/sim/lib/execution/sandbox/bundles/docx.cjs‎

Lines changed: 21 additions & 21 deletions
Large diffs are not rendered by default.

‎apps/sim/lib/execution/sandbox/bundles/pdf-lib.cjs‎

Lines changed: 21 additions & 21 deletions
Large diffs are not rendered by default.

‎apps/sim/lib/execution/sandbox/bundles/pptxgenjs.cjs‎

Lines changed: 62 additions & 63 deletions
Large diffs are not rendered by default.
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { readFileSync } from 'node:fs'
5+
import { describe, expect, it } from 'vitest'
6+
import { evaluateSandboxBundle } from '@/lib/execution/sandbox/bundles/verify'
7+
import type { SandboxBundleName } from '@/lib/execution/sandbox/types'
8+
9+
function loadCheckedInBundle(name: SandboxBundleName): Record<string, unknown> {
10+
const source = readFileSync(new URL(`./${name}.cjs`, import.meta.url), 'utf-8')
11+
return evaluateSandboxBundle(source, name) as Record<string, unknown>
12+
}
13+
14+
/**
15+
* The checked-in bundles are what Trigger.dev workers run verbatim, so this is
16+
* the only place a bundle that throws while being evaluated is caught before a
17+
* deploy. Each case asserts the surface the matching sandbox task's bootstrap
18+
* and finalize scripts reach for.
19+
*/
20+
describe('sandbox bundles', () => {
21+
it('docx evaluates in a bare context and exposes the docx-generate surface', () => {
22+
const docx = loadCheckedInBundle('docx')
23+
expect(typeof docx.Document).toBe('function')
24+
expect(typeof docx.Packer).toBe('function')
25+
expect(typeof docx.ImageRun).toBe('function')
26+
expect(typeof docx.Paragraph).toBe('function')
27+
})
28+
29+
it('pdf-lib evaluates in a bare context and exposes the pdf-generate surface', () => {
30+
const pdfLib = loadCheckedInBundle('pdf-lib')
31+
expect(typeof pdfLib.PDFDocument).toBe('function')
32+
expect(typeof pdfLib.rgb).toBe('function')
33+
expect(typeof pdfLib.StandardFonts).toBe('object')
34+
})
35+
36+
it('pptxgenjs evaluates in a bare context and exposes its constructor', () => {
37+
const source = readFileSync(new URL('./pptxgenjs.cjs', import.meta.url), 'utf-8')
38+
expect(typeof evaluateSandboxBundle(source, 'pptxgenjs')).toBe('function')
39+
})
40+
})
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import vm from 'node:vm'
2+
import type { SandboxBundleName } from '@/lib/execution/sandbox/types'
3+
4+
/**
5+
* Evaluates a built sandbox bundle the way the isolated-vm worker will: as a
6+
* classic script in a context that has timers, `console`, and the text codecs
7+
* but no `require`, `process`, or `Buffer` of its own. Returns the export the
8+
* bundle registered on `globalThis.__bundles`, or throws with the bundle's own
9+
* error, so a bundle that references a helper the bundler never emitted fails
10+
* at build time and in the test suite instead of on the first document
11+
* generated in production.
12+
*/
13+
export function evaluateSandboxBundle(source: string, name: SandboxBundleName): unknown {
14+
const context: Record<string, unknown> = {
15+
setTimeout,
16+
clearTimeout,
17+
setInterval,
18+
clearInterval,
19+
queueMicrotask,
20+
console,
21+
TextEncoder,
22+
TextDecoder,
23+
}
24+
context.globalThis = context
25+
vm.createContext(context)
26+
vm.runInContext(source, context, { filename: `sandbox/${name}.cjs` })
27+
28+
const bundles = context.__bundles
29+
const bundle =
30+
typeof bundles === 'object' && bundles !== null
31+
? (bundles as Record<string, unknown>)[name]
32+
: undefined
33+
if (bundle === undefined || bundle === null) {
34+
throw new Error(
35+
`Sandbox bundle "${name}" evaluated without registering globalThis.__bundles["${name}"]`
36+
)
37+
}
38+
return bundle
39+
}
Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
6+
const { mockDownloadFile } = vi.hoisted(() => ({
7+
mockDownloadFile: vi.fn(),
8+
}))
9+
10+
vi.mock('@/lib/billing/storage', () => ({
11+
decrementStorageUsageForBillingContextInTx: vi.fn(),
12+
incrementStorageUsageForBillingContextInTx: vi.fn(),
13+
maybeNotifyStorageLimitForBillingContext: vi.fn(),
14+
resolveStorageBillingContext: vi.fn(),
15+
}))
16+
17+
vi.mock('@/lib/uploads', () => ({
18+
getServePathPrefix: vi.fn(() => '/api/files/serve/s3/'),
19+
}))
20+
21+
vi.mock('@/lib/uploads/core/storage-service', () => ({
22+
deleteFile: vi.fn(),
23+
downloadFile: mockDownloadFile,
24+
hasCloudStorage: vi.fn(() => false),
25+
headObject: vi.fn(),
26+
uploadFile: vi.fn(),
27+
}))
28+
29+
vi.mock('@/lib/uploads/contexts/workspace/workspace-file-folder-manager', () => ({
30+
assertWorkspaceFileFolderTarget: vi.fn(async () => null),
31+
buildWorkspaceFileFolderPathMap: vi.fn(() => new Map()),
32+
fileNameExistsInWorkspaceFolder: vi.fn(async () => false),
33+
findWorkspaceFileFolderIdByPath: vi.fn(),
34+
getWorkspaceFileFolderPath: vi.fn(),
35+
listWorkspaceFileFolders: vi.fn(async () => []),
36+
normalizeWorkspaceFileItemName: vi.fn((name: string) => name),
37+
resolveWorkspaceFileFolderTarget: vi.fn(async () => null),
38+
}))
39+
40+
import { assertKnownSizeWithinLimit, isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
41+
import {
42+
fetchWorkspaceFileBuffer,
43+
type WorkspaceFileRecord,
44+
} from '@/lib/uploads/contexts/workspace/workspace-file-manager'
45+
46+
const FILE: WorkspaceFileRecord = {
47+
id: 'file-1',
48+
workspaceId: 'workspace-1',
49+
name: 'notes.txt',
50+
key: 'workspace/workspace-1/notes.txt',
51+
path: '/api/files/serve/workspace/workspace-1/notes.txt',
52+
size: 5,
53+
type: 'text/plain',
54+
uploadedBy: 'user-1',
55+
uploadedAt: new Date('2026-09-01T00:00:00.000Z'),
56+
updatedAt: new Date('2026-09-01T00:00:00.000Z'),
57+
}
58+
59+
function sizeLimitError(): unknown {
60+
try {
61+
assertKnownSizeWithinLimit(2, 1, 'test')
62+
} catch (error) {
63+
return error
64+
}
65+
throw new Error('assertKnownSizeWithinLimit did not throw')
66+
}
67+
68+
describe('fetchWorkspaceFileBuffer', () => {
69+
beforeEach(() => {
70+
vi.clearAllMocks()
71+
})
72+
73+
it('forwards the byte ceiling and the cancellation signal to storage', async () => {
74+
const bytes = Buffer.from('hello')
75+
mockDownloadFile.mockResolvedValue(bytes)
76+
const signal = new AbortController().signal
77+
78+
await expect(fetchWorkspaceFileBuffer(FILE, { maxBytes: 10, signal })).resolves.toBe(bytes)
79+
expect(mockDownloadFile).toHaveBeenCalledWith({
80+
key: FILE.key,
81+
context: 'workspace',
82+
maxBytes: 10,
83+
signal,
84+
})
85+
})
86+
87+
it('surfaces a cancelled read as the abort rather than a download failure', async () => {
88+
const controller = new AbortController()
89+
mockDownloadFile.mockImplementation(async () => {
90+
controller.abort()
91+
throw new Error('read interrupted')
92+
})
93+
94+
await expect(
95+
fetchWorkspaceFileBuffer(FILE, { maxBytes: 10, signal: controller.signal })
96+
).rejects.toMatchObject({ name: 'AbortError' })
97+
})
98+
99+
it('rethrows a byte-ceiling breach unwrapped', async () => {
100+
mockDownloadFile.mockRejectedValue(sizeLimitError())
101+
102+
await expect(fetchWorkspaceFileBuffer(FILE, { maxBytes: 10 })).rejects.toSatisfy(
103+
isPayloadSizeLimitError
104+
)
105+
})
106+
107+
it('wraps other transport failures', async () => {
108+
mockDownloadFile.mockRejectedValue(new Error('socket hang up'))
109+
110+
await expect(fetchWorkspaceFileBuffer(FILE, { maxBytes: 10 })).rejects.toThrow(
111+
'Failed to download file: socket hang up'
112+
)
113+
})
114+
})

‎apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1678,7 +1678,7 @@ export async function fetchServableWorkspaceFileBuffer(
16781678
*/
16791679
export async function fetchWorkspaceFileBuffer(
16801680
fileRecord: WorkspaceFileRecord,
1681-
options: { maxBytes: number }
1681+
options: { maxBytes: number; signal?: AbortSignal }
16821682
): Promise<Buffer> {
16831683
logger.info(`Downloading workspace file: ${fileRecord.name}`)
16841684

@@ -1687,12 +1687,16 @@ export async function fetchWorkspaceFileBuffer(
16871687
key: fileRecord.key,
16881688
context: fileRecord.storageContext ?? 'workspace',
16891689
maxBytes: options.maxBytes,
1690+
signal: options.signal,
16901691
})
16911692
logger.info(
16921693
`Successfully downloaded workspace file: ${fileRecord.name} (${buffer.length} bytes)`
16931694
)
16941695
return buffer
16951696
} catch (error) {
1697+
// A cancelled read is not a download failure: surface the abort itself so the
1698+
// caller sees cancellation, not a transport error it might retry or record.
1699+
options.signal?.throwIfAborted()
16961700
logger.error(`Failed to download workspace file ${fileRecord.name}:`, error)
16971701
// Rethrow a `maxBytes` breach unwrapped: callers distinguish "too large" from a
16981702
// transport failure to answer with their own placeholder, and re-wrapping it in a

0 commit comments

Comments
 (0)