From b004ef5448376c3d5d7868b38ee27b3c2294d13d Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 3 Oct 2024 09:42:08 +0100 Subject: [PATCH] Pull the error object out of isolated-vm when a user script throws an error. --- .../src/jsRunner/tests/jsRunner.spec.ts | 2 +- .../server/src/jsRunner/vm/isolated-vm.ts | 22 ++++++++++++++++++- .../src/helpers/javascript.ts | 3 +++ 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/packages/server/src/jsRunner/tests/jsRunner.spec.ts b/packages/server/src/jsRunner/tests/jsRunner.spec.ts index 525f6d865e..2b9fc08041 100644 --- a/packages/server/src/jsRunner/tests/jsRunner.spec.ts +++ b/packages/server/src/jsRunner/tests/jsRunner.spec.ts @@ -44,7 +44,7 @@ describe("jsRunner (using isolated-vm)", () => { const output = await processJS( `return this.constructor.constructor("return process.env")()` ) - expect(output).toBe("Error while executing JS") + expect(output).toBe("ReferenceError: process is not defined") }) describe("helpers", () => { diff --git a/packages/server/src/jsRunner/vm/isolated-vm.ts b/packages/server/src/jsRunner/vm/isolated-vm.ts index 87d5b966c2..e502b799bd 100644 --- a/packages/server/src/jsRunner/vm/isolated-vm.ts +++ b/packages/server/src/jsRunner/vm/isolated-vm.ts @@ -17,6 +17,15 @@ class ExecutionTimeoutError extends Error { } } +class UserScriptError extends Error { + constructor(readonly userScriptError: Error) { + super( + `error while running user-supplied JavaScript: ${userScriptError.message}`, + { cause: userScriptError } + ) + } +} + export class IsolatedVM implements VM { private isolate: ivm.Isolate private vm: ivm.Context @@ -29,6 +38,7 @@ export class IsolatedVM implements VM { private readonly resultKey = "results" private runResultKey: string + private runErrorKey: string constructor({ memoryLimit, @@ -47,6 +57,7 @@ export class IsolatedVM implements VM { this.jail.setSync("global", this.jail.derefInto()) this.runResultKey = crypto.randomUUID() + this.runErrorKey = crypto.randomUUID() this.addToContext({ [this.resultKey]: { [this.runResultKey]: "" }, }) @@ -216,7 +227,13 @@ export class IsolatedVM implements VM { } } - code = `results['${this.runResultKey}']=${this.codeWrapper(code)}` + code = ` + try { + results['${this.runResultKey}']=${this.codeWrapper(code)} + } catch (e) { + results['${this.runErrorKey}']=e + } + ` const script = this.isolate.compileScriptSync(code) @@ -227,6 +244,9 @@ export class IsolatedVM implements VM { // We can't rely on the script run result as it will not work for non-transferable values const result = this.getFromContext(this.resultKey) + if (result[this.runErrorKey]) { + throw new UserScriptError(result[this.runErrorKey]) + } return result[this.runResultKey] } diff --git a/packages/string-templates/src/helpers/javascript.ts b/packages/string-templates/src/helpers/javascript.ts index 3e16d8a07b..27a71da834 100644 --- a/packages/string-templates/src/helpers/javascript.ts +++ b/packages/string-templates/src/helpers/javascript.ts @@ -100,6 +100,9 @@ export function processJS(handlebars: string, context: any) { if (error.name === "ExecutionTimeoutError") { return "Request JS execution limit hit" } + if ("userScriptError" in error) { + return error.userScriptError.toString() + } return "Error while executing JS" } }