Merge pull request #12611 from Budibase/limit-js-execution-per-request

Implement per-request JS execution limiting.

packages/backend-core/src/context/mainContext.ts

@@ -335,3 +335,11 @@ export function isScim(): boolean {
   const scimCall = context?.isScim
   return !!scimCall
 }
+
+export function getCurrentContext(): ContextMap | undefined {
+  try {
+    return Context.get()
+  } catch (e) {
+    return undefined
+  }
+}

packages/backend-core/src/context/types.ts

@@ -1,4 +1,5 @@
 import { IdentityContext } from "@budibase/types"
+import { ExecutionTimeTracker } from "../timers"
 
 // keep this out of Budibase types, don't want to expose context info
 export type ContextMap = {
@@ -9,4 +10,5 @@ export type ContextMap = {
   isScim?: boolean
   automationId?: string
   isMigrating?: boolean
+  jsExecutionTracker?: ExecutionTimeTracker
 }

packages/backend-core/src/timers/timers.ts

@@ -20,3 +20,37 @@ export function cleanup() {
   }
   intervals = []
 }
+
+export class ExecutionTimeoutError extends Error {
+  public readonly name = "ExecutionTimeoutError"
+}
+
+export class ExecutionTimeTracker {
+  static withLimit(limitMs: number) {
+    return new ExecutionTimeTracker(limitMs)
+  }
+
+  constructor(private limitMs: number) {}
+
+  private totalTimeMs = 0
+
+  track<T>(f: () => T): T {
+    this.checkLimit()
+    const start = process.hrtime.bigint()
+    try {
+      return f()
+    } finally {
+      const end = process.hrtime.bigint()
+      this.totalTimeMs += Number(end - start) / 1e6
+      this.checkLimit()
+    }
+  }
+
+  private checkLimit() {
+    if (this.totalTimeMs > this.limitMs) {
+      throw new ExecutionTimeoutError(
+        `Execution time limit of ${this.limitMs}ms exceeded: ${this.totalTimeMs}ms`
+      )
+    }
+  }
+}

packages/backend-core/src/utils/Duration.ts

@@ -49,4 +49,8 @@ export class Duration {
   static fromDays(duration: number) {
     return Duration.from(DurationType.DAYS, duration)
   }
+
+  static fromMilliseconds(duration: number) {
+    return Duration.from(DurationType.MILLISECONDS, duration)
+  }
 }

packages/server/src/api/routes/tests/row.spec.ts

@@ -2086,4 +2086,112 @@ 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: 20 }, 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: 20,
+          JS_PER_REQUEST_TIME_LIMIT_MS: 40,
+        },
+        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" })
+          }
+
+          // Run this test 3 times to make sure that there's no cross-request
+          // pollution of the execution time tracking.
+          for (let reqs = 0; reqs < 3; reqs++) {
+            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
+              }
+            }
+
+            // Given the execution times are not deterministic, we can't be sure
+            // of the exact number of rows that were executed before the timeout
+            // but it should absolutely be at least 1.
+            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")
+            }
+          }
+        }
+      )
+    })
+  })
 })

packages/server/src/environment.ts

@@ -70,6 +70,11 @@ const environment = {
   SELF_HOSTED: process.env.SELF_HOSTED,
   HTTP_MB_LIMIT: process.env.HTTP_MB_LIMIT,
   FORKED_PROCESS_NAME: process.env.FORKED_PROCESS_NAME || "main",
+  JS_PER_EXECUTION_TIME_LIMIT_MS:
+    parseIntSafe(process.env.JS_PER_EXECUTION_TIME_LIMIT_MS) || 1000,
+  JS_PER_REQUEST_TIME_LIMIT_MS: parseIntSafe(
+    process.env.JS_PER_REQUEST_TIME_LIMIT_MS
+  ),
   // old
   CLIENT_ID: process.env.CLIENT_ID,
   _set(key: string, value: any) {

packages/server/src/jsRunner.ts

@@ -0,0 +1,36 @@
+import vm from "vm"
+import env from "./environment"
+import { setJSRunner } from "@budibase/string-templates"
+import { context, timers } from "@budibase/backend-core"
+
+type TrackerFn = <T>(f: () => T) => T
+
+export function init() {
+  setJSRunner((js: string, ctx: vm.Context) => {
+    const perRequestLimit = env.JS_PER_REQUEST_TIME_LIMIT_MS
+    let track: TrackerFn = f => f()
+    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 = {
+      ...ctx,
+      alert: undefined,
+      setInterval: undefined,
+      setTimeout: undefined,
+    }
+    vm.createContext(ctx)
+    return track(() =>
+      vm.runInNewContext(js, ctx, {
+        timeout: env.JS_PER_EXECUTION_TIME_LIMIT_MS,
+      })
+    )
+  })
+}

packages/server/src/startup.ts

@@ -23,6 +23,7 @@ import { automationsEnabled, printFeatures } from "./features"
 import Koa from "koa"
 import { Server } from "http"
 import { AddressInfo } from "net"
+import * as jsRunner from "./jsRunner"
 
 let STARTUP_RAN = false
 
@@ -152,4 +153,6 @@ export async function startup(app?: Koa, server?: Server) {
       }
     })
   }
+
+  jsRunner.init()
 }

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"
   }
 }

packages/string-templates/src/index.cjs

@@ -18,6 +18,7 @@ module.exports.doesContainString = templates.doesContainString
 module.exports.disableEscaping = templates.disableEscaping
 module.exports.findHBSBlocks = templates.findHBSBlocks
 module.exports.convertToJS = templates.convertToJS
+module.exports.setJSRunner = templates.setJSRunner
 module.exports.FIND_ANY_HBS_REGEX = templates.FIND_ANY_HBS_REGEX
 
 if (!process.env.NO_JS) {

packages/string-templates/src/index.js

@@ -9,6 +9,7 @@ const {
   findDoubleHbsInstances,
 } = require("./utilities")
 const { convertHBSBlock } = require("./conversion")
+const javascript = require("./helpers/javascript")
 
 const hbsInstance = handlebars.create()
 registerAll(hbsInstance)
@@ -362,6 +363,8 @@ module.exports.doesContainString = (template, string) => {
   return exports.doesContainStrings(template, [string])
 }
 
+module.exports.setJSRunner = javascript.setJSRunner
+
 module.exports.convertToJS = hbs => {
   const blocks = exports.findHBSBlocks(hbs)
   let js = "return `",

packages/string-templates/src/index.mjs

@@ -1,6 +1,5 @@
 import vm from "vm"
 import templates from "./index.js"
-import { setJSRunner } from "./helpers/javascript"
 
 /**
  * ES6 entrypoint for rollup
@@ -20,6 +19,7 @@ export const doesContainString = templates.doesContainString
 export const disableEscaping = templates.disableEscaping
 export const findHBSBlocks = templates.findHBSBlocks
 export const convertToJS = templates.convertToJS
+export const setJSRunner = templates.setJSRunner
 export const FIND_ANY_HBS_REGEX = templates.FIND_ANY_HBS_REGEX
 
 if (process && !process.env.NO_JS) {