Merge pull request #13075 from Budibase/fix-context-in-js

Fix re-used context in JS runner.

packages/server/src/api/controllers/script.ts

@@ -3,9 +3,10 @@ import { IsolatedVM } from "../../jsRunner/vm"
 
 export async function execute(ctx: Ctx) {
   const { script, context } = ctx.request.body
-  const runner = new IsolatedVM().withContext(context)
-
-  const result = runner.execute(`(function(){\n${script}\n})();`)
+  const vm = new IsolatedVM()
+  const result = vm.withContext(context, () =>
+    vm.execute(`(function(){\n${script}\n})();`)
+  )
   ctx.body = result
 }
 

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

@@ -2135,5 +2135,48 @@ describe.each([
         }
       )
     })
+
+    it("should not carry over context between formulas", async () => {
+      const js = Buffer.from(`return $("[text]");`).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: FormulaType.DYNAMIC,
+          },
+        },
+      })
+
+      for (let i = 0; i < 10; i++) {
+        await config.api.row.save(table._id!, { text: `foo${i}` })
+      }
+
+      const { rows } = await config.api.row.search(table._id!)
+      expect(rows).toHaveLength(10)
+
+      const formulaValues = rows.map(r => r.formula)
+      expect(formulaValues).toEqual(
+        expect.arrayContaining([
+          "foo0",
+          "foo1",
+          "foo2",
+          "foo3",
+          "foo4",
+          "foo5",
+          "foo6",
+          "foo7",
+          "foo8",
+          "foo9",
+        ])
+      )
+    })
   })
 })

packages/server/src/jsRunner/index.ts

@@ -23,25 +23,20 @@ export function init() {
       try {
         const bbCtx = context.getCurrentContext()
 
-        let vm = bbCtx?.vm
-        if (!vm) {
-          // Can't copy the native helpers into the isolate. We just ignore them as they are handled properly from the helpersSource
-          const { helpers, ...ctxToPass } = ctx
+        const vm = bbCtx?.vm
+          ? bbCtx.vm
+          : new IsolatedVM({
+              memoryLimit: env.JS_RUNNER_MEMORY_LIMIT,
+              invocationTimeout: env.JS_PER_INVOCATION_TIMEOUT_MS,
+              isolateAccumulatedTimeout: env.JS_PER_REQUEST_TIMEOUT_MS,
+            }).withHelpers()
 
-          vm = new IsolatedVM({
-            memoryLimit: env.JS_RUNNER_MEMORY_LIMIT,
-            invocationTimeout: env.JS_PER_INVOCATION_TIMEOUT_MS,
-            isolateAccumulatedTimeout: env.JS_PER_REQUEST_TIMEOUT_MS,
-          })
-            .withContext(ctxToPass)
-            .withHelpers()
-
-          if (bbCtx) {
-            // If we have a context, we want to persist it to reuse the isolate
-            bbCtx.vm = vm
-          }
+        if (bbCtx) {
+          // If we have a context, we want to persist it to reuse the isolate
+          bbCtx.vm = vm
         }
-        return vm.execute(js)
+        const { helpers, ...rest } = ctx
+        return vm.withContext(rest, () => vm.execute(js))
       } catch (error: any) {
         if (error.message === "Script execution timed out.") {
           throw new JsErrorTimeout()

packages/server/src/jsRunner/vm/builtin-vm.ts

@@ -15,6 +15,17 @@ export class BuiltInVM implements VM {
     this.span = span
   }
 
+  withContext<T>(context: Record<string, any>, executeWithContext: () => T): T {
+    this.ctx = vm.createContext(context)
+    try {
+      return executeWithContext()
+    } finally {
+      for (const key in context) {
+        delete this.ctx[key]
+      }
+    }
+  }
+
   execute(code: string) {
     const perRequestLimit = env.JS_PER_REQUEST_TIMEOUT_MS
     let track: TrackerFn = f => f()

packages/server/src/jsRunner/vm/isolated-vm.ts

@@ -97,10 +97,14 @@ export class IsolatedVM implements VM {
     return this
   }
 
-  withContext(context: Record<string, any>) {
+  withContext<T>(context: Record<string, any>, executeWithContext: () => T) {
     this.addToContext(context)
 
-    return this
+    try {
+      return executeWithContext()
+    } finally {
+      this.removeFromContext(Object.keys(context))
+    }
   }
 
   withParsingBson(data: any) {
@@ -224,6 +228,12 @@ export class IsolatedVM implements VM {
     }
   }
 
+  private removeFromContext(keys: string[]) {
+    for (let key of keys) {
+      this.jail.deleteSync(key)
+    }
+  }
+
   private getFromContext(key: string) {
     const ref = this.vm.global.getSync(key, { reference: true })
     const result = ref.copySync()

packages/server/src/jsRunner/vm/vm2.ts

@@ -7,16 +7,26 @@ export class VM2 implements VM {
   vm: vm2.VM
   results: { out: string }
 
-  constructor(context: any) {
+  constructor() {
     this.vm = new vm2.VM({
       timeout: JS_TIMEOUT_MS,
     })
     this.results = { out: "" }
-    this.vm.setGlobals(context)
     this.vm.setGlobal("fetch", fetch)
     this.vm.setGlobal("results", this.results)
   }
 
+  withContext<T>(context: Record<string, any>, executeWithContext: () => T): T {
+    this.vm.setGlobals(context)
+    try {
+      return executeWithContext()
+    } finally {
+      for (const key in context) {
+        this.vm.setGlobal(key, undefined)
+      }
+    }
+  }
+
   execute(script: string) {
     const code = `let fn = () => {\n${script}\n}; results.out = fn();`
     const vmScript = new vm2.VMScript(code)

packages/server/src/threads/query.ts

@@ -131,24 +131,21 @@ class QueryRunner {
     if (transformer) {
       let runner: VM
       if (!USE_ISOLATED_VM) {
-        runner = new VM2({
-          data: rows,
-          params: enrichedParameters,
-        })
+        runner = new VM2()
       } else {
         transformer = `(function(){\n${transformer}\n})();`
-        let isolatedVm = new IsolatedVM().withContext({
-          data: rows,
-          params: enrichedParameters,
-        })
+        let vm = new IsolatedVM()
         if (datasource.source === SourceName.MONGODB) {
-          isolatedVm = isolatedVm.withParsingBson(rows)
+          vm = vm.withParsingBson(rows)
         }
-
-        runner = isolatedVm
+        runner = vm
       }
 
-      rows = runner.execute(transformer)
+      const ctx = {
+        data: rows,
+        params: enrichedParameters,
+      }
+      rows = runner.withContext(ctx, () => runner.execute(transformer))
     }
 
     // if the request fails we retry once, invalidating the cached value

packages/types/src/sdk/vm.ts

@@ -1,3 +1,4 @@
 export interface VM {
   execute(code: string): any
+  withContext<T>(context: Record<string, any>, executeWithContext: () => T): T
 }