From 1c34147357d46d10d4c51eed789dacdd8139e291 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 18 Dec 2023 17:01:56 +0000 Subject: [PATCH] Add tests for per-request execution timeout. --- packages/backend-core/src/timers/timers.ts | 14 ++- .../server/src/api/routes/tests/row.spec.ts | 104 ++++++++++++++++++ packages/server/src/javascript.ts | 14 ++- .../src/helpers/javascript.js | 4 +- 4 files changed, 125 insertions(+), 11 deletions(-) diff --git a/packages/backend-core/src/timers/timers.ts b/packages/backend-core/src/timers/timers.ts index 7bb23425e9..9a8067ee64 100644 --- a/packages/backend-core/src/timers/timers.ts +++ b/packages/backend-core/src/timers/timers.ts @@ -21,6 +21,10 @@ export function cleanup() { intervals = [] } +export class ExecutionTimeoutError extends Error { + public readonly name = "ExecutionTimeoutError" +} + export class ExecutionTimeTracker { static withLimit(limitMs: number) { return new ExecutionTimeTracker(limitMs) @@ -31,6 +35,7 @@ export class ExecutionTimeTracker { private totalTimeMs = 0 track(f: () => T): T { + this.checkLimit() const [startSeconds, startNanoseconds] = process.hrtime() const startMs = startSeconds * 1000 + startNanoseconds / 1e6 try { @@ -38,15 +43,14 @@ export class ExecutionTimeTracker { } finally { const [endSeconds, endNanoseconds] = process.hrtime() const endMs = endSeconds * 1000 + endNanoseconds / 1e6 - this.increment(endMs - startMs) + this.totalTimeMs += endMs - startMs + this.checkLimit() } } - private increment(byMs: number) { - this.totalTimeMs += byMs - + private checkLimit() { if (this.totalTimeMs > this.limitMs) { - throw new Error( + throw new ExecutionTimeoutError( `Execution time limit of ${this.limitMs}ms exceeded: ${this.totalTimeMs}ms` ) } diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 5b3c69b87a..c1287ddd42 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -2086,4 +2086,108 @@ describe.each([ expect(row.formula).toBe(relatedRow.name) }) }) + + describe("Formula JS protection", () => { + it("should time out JS execution if a single cell takes too long", async () => { + await config.withEnv( + { JS_PER_EXECUTION_TIME_LIMIT_MS: 100 }, + async () => { + const js = Buffer.from( + ` + let i = 0; + while (true) { + i++; + } + return i; + ` + ).toString("base64") + + const table = await config.createTable({ + name: "table", + type: "table", + schema: { + text: { + name: "text", + type: FieldType.STRING, + }, + formula: { + name: "formula", + type: FieldType.FORMULA, + formula: `{{ js "${js}"}}`, + formulaType: FormulaTypes.DYNAMIC, + }, + }, + }) + + await config.api.row.save(table._id!, { text: "foo" }) + const { rows } = await config.api.row.search(table._id!) + expect(rows).toHaveLength(1) + const row = rows[0] + expect(row.text).toBe("foo") + expect(row.formula).toBe("Timed out while executing JS") + } + ) + }) + + it("should time out JS execution if a multiple cells take too long", async () => { + await config.withEnv( + { + JS_PER_EXECUTION_TIME_LIMIT_MS: 100, + JS_PER_REQUEST_TIME_LIMIT_MS: 200, + }, + async () => { + const js = Buffer.from( + ` + let i = 0; + while (true) { + i++; + } + return i; + ` + ).toString("base64") + + const table = await config.createTable({ + name: "table", + type: "table", + schema: { + text: { + name: "text", + type: FieldType.STRING, + }, + formula: { + name: "formula", + type: FieldType.FORMULA, + formula: `{{ js "${js}"}}`, + formulaType: FormulaTypes.DYNAMIC, + }, + }, + }) + + for (let i = 0; i < 10; i++) { + await config.api.row.save(table._id!, { text: "foo" }) + } + + const { rows } = await config.api.row.search(table._id!) + expect(rows).toHaveLength(10) + + let i = 0 + for (; i < 10; i++) { + const row = rows[i] + if (row.formula !== "Timed out while executing JS") { + break + } + } + + expect(i).toBeGreaterThan(0) + expect(i).toBeLessThan(5) + + for (; i < 10; i++) { + const row = rows[i] + expect(row.text).toBe("foo") + expect(row.formula).toBe("Request JS execution limit hit") + } + } + ) + }) + }) }) diff --git a/packages/server/src/javascript.ts b/packages/server/src/javascript.ts index 42d4f439bf..a9301feb60 100644 --- a/packages/server/src/javascript.ts +++ b/packages/server/src/javascript.ts @@ -8,12 +8,16 @@ type TrackerFn = (f: () => T) => T export function init() { setJSRunner((js: string, ctx: vm.Context) => { const perRequestLimit = env.JS_PER_REQUEST_TIME_LIMIT_MS - const bbCtx = context.getCurrentContext() let track: TrackerFn = f => f() - if (perRequestLimit && bbCtx && !bbCtx.jsExecutionTracker) { - bbCtx.jsExecutionTracker = - timers.ExecutionTimeTracker.withLimit(perRequestLimit) - track = bbCtx.jsExecutionTracker.track + if (perRequestLimit) { + const bbCtx = context.getCurrentContext() + if (bbCtx) { + if (!bbCtx.jsExecutionTracker) { + bbCtx.jsExecutionTracker = + timers.ExecutionTimeTracker.withLimit(perRequestLimit) + } + track = bbCtx.jsExecutionTracker.track.bind(bbCtx.jsExecutionTracker) + } } ctx = { diff --git a/packages/string-templates/src/helpers/javascript.js b/packages/string-templates/src/helpers/javascript.js index 53baec8613..585a2ba6b7 100644 --- a/packages/string-templates/src/helpers/javascript.js +++ b/packages/string-templates/src/helpers/javascript.js @@ -56,10 +56,12 @@ module.exports.processJS = (handlebars, context) => { const res = { data: runJS(js, sandboxContext) } return `{{${LITERAL_MARKER} js_result-${JSON.stringify(res)}}}` } catch (error) { - console.log(`JS error: ${typeof error} ${JSON.stringify(error)}`) if (error.code === "ERR_SCRIPT_EXECUTION_TIMEOUT") { return "Timed out while executing JS" } + if (error.name === "ExecutionTimeoutError") { + return "Request JS execution limit hit" + } return "Error while executing JS" } }