Add JavaScript execution safety limits and improve error handling

- Add 1MB code size limit to prevent memory issues
- Improve timeout error handling with proper cleanup logging
- Remove host environment exposure and fix file descriptor leaks in worker
This commit is contained in:
suyao 2025-10-08 07:11:39 +08:00
parent 9f1c8f2c17
commit 71d35eddf7
No known key found for this signature in database
3 changed files with 17 additions and 6 deletions

View File

@ -107,7 +107,7 @@ class JsServer {
timeout: timeout ?? DEFAULT_TIMEOUT timeout: timeout ?? DEFAULT_TIMEOUT
}) })
const { combinedOutput, isError } = formatExecutionResult(result as any) const { combinedOutput, isError } = formatExecutionResult(result)
return { return {
content: [ content: [

View File

@ -41,6 +41,12 @@ export class JsService {
throw new Error('JavaScript code must be a non-empty string') throw new Error('JavaScript code must be a non-empty string')
} }
// Limit code size to 1MB to prevent memory issues
const MAX_CODE_SIZE = 1_000_000
if (code.length > MAX_CODE_SIZE) {
throw new Error(`JavaScript code exceeds maximum size of ${MAX_CODE_SIZE / 1_000_000}MB`)
}
return new Promise<JsExecutionResult>((resolve, reject) => { return new Promise<JsExecutionResult>((resolve, reject) => {
const worker = createJsWorker({ const worker = createJsWorker({
workerData: { code }, workerData: { code },
@ -98,7 +104,9 @@ export class JsService {
timeoutId = setTimeout(() => { timeoutId = setTimeout(() => {
logger.warn(`JavaScript execution timed out after ${timeout}ms`) logger.warn(`JavaScript execution timed out after ${timeout}ms`)
void settleError(new Error('JavaScript execution timed out')) settleError(new Error('JavaScript execution timed out')).catch((err) => {
logger.error('Error during timeout cleanup:', err instanceof Error ? err : new Error(String(err)))
})
}, timeout) }, timeout)
}) })
} }

View File

@ -1,7 +1,6 @@
import { mkdtemp, open, readFile, rm } from 'node:fs/promises' import { mkdtemp, open, readFile, rm } from 'node:fs/promises'
import { tmpdir } from 'node:os' import { tmpdir } from 'node:os'
import { join } from 'node:path' import { join } from 'node:path'
import { env } from 'node:process'
import { WASI } from 'node:wasi' import { WASI } from 'node:wasi'
import { parentPort, workerData } from 'node:worker_threads' import { parentPort, workerData } from 'node:worker_threads'
@ -40,7 +39,7 @@ async function runQuickJsInSandbox(jsCode: string): Promise<JsExecutionResult> {
const wasi = new WASI({ const wasi = new WASI({
version: 'preview1', version: 'preview1',
args: ['qjs', '-e', jsCode], args: ['qjs', '-e', jsCode],
env, env: {}, // Empty environment for security - don't expose host env vars
stdin: 0, stdin: 0,
stdout: stdoutHandle.fd, stdout: stdoutHandle.fd,
stderr: stderrHandle.fd, stderr: stderrHandle.fd,
@ -60,10 +59,14 @@ async function runQuickJsInSandbox(jsCode: string): Promise<JsExecutionResult> {
} }
} }
await stdoutHandle.close() // Close handles before reading files to prevent descriptor leak
const _stdoutHandle = stdoutHandle
stdoutHandle = undefined stdoutHandle = undefined
await stderrHandle.close() await _stdoutHandle.close()
const _stderrHandle = stderrHandle
stderrHandle = undefined stderrHandle = undefined
await _stderrHandle.close()
const capturedStdout = await readFile(stdoutPath, 'utf8') const capturedStdout = await readFile(stdoutPath, 'utf8')
const capturedStderr = await readFile(stderrPath, 'utf8') const capturedStderr = await readFile(stderrPath, 'utf8')