From 2c451d494b0f18c5b0faf1049351d82784ad8828 Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Mon, 7 Oct 2024 12:04:45 +0100
Subject: [PATCH 1/4] Backend/frontend JS parity, deep clone optimisation.

---
 .../src/jsRunner/tests/jsRunner.spec.ts       |  28 +-
 .../src/helpers/javascript.ts                 |  30 +-
 packages/string-templates/src/index.ts        |  40 +-
 packages/string-templates/src/utilities.ts    |   7 +
 .../string-templates/test/javascript.spec.ts  | 363 +++++++++++++++++-
 5 files changed, 427 insertions(+), 41 deletions(-)

diff --git a/packages/server/src/jsRunner/tests/jsRunner.spec.ts b/packages/server/src/jsRunner/tests/jsRunner.spec.ts
index c7ab86f014..006df19fa6 100644
--- a/packages/server/src/jsRunner/tests/jsRunner.spec.ts
+++ b/packages/server/src/jsRunner/tests/jsRunner.spec.ts
@@ -8,7 +8,6 @@ import { init } from ".."
 import TestConfiguration from "../../tests/utilities/TestConfiguration"
 
 const DATE = "2021-01-21T12:00:00"
-
 tk.freeze(DATE)
 
 describe("jsRunner (using isolated-vm)", () => {
@@ -48,6 +47,33 @@ describe("jsRunner (using isolated-vm)", () => {
     ).toEqual("ReferenceError: process is not defined")
   })
 
+  it("should not allow the context to be mutated", async () => {
+    const context = { array: [1] }
+    const result = await processJS(
+      `
+        const array = $("array");
+        array.push(2);
+        return array[1]
+      `,
+      context
+    )
+    expect(result).toEqual(2)
+    expect(context.array).toEqual([1])
+  })
+
+  it("should copy values whenever returning them from $", async () => {
+    const context = { array: [1] }
+    const result = await processJS(
+      `
+        $("array").push(2);
+        return $("array")[1];
+      `,
+      context
+    )
+    expect(result).toEqual(undefined)
+    expect(context.array).toEqual([1])
+  })
+
   describe("helpers", () => {
     runJsHelpersTests({
       funcWrap: (func: any) => config.doInContext(config.getAppId(), func),
diff --git a/packages/string-templates/src/helpers/javascript.ts b/packages/string-templates/src/helpers/javascript.ts
index 2f850b0533..4d53fe335d 100644
--- a/packages/string-templates/src/helpers/javascript.ts
+++ b/packages/string-templates/src/helpers/javascript.ts
@@ -1,13 +1,14 @@
-import { atob, isJSAllowed } from "../utilities"
-import cloneDeep from "lodash/fp/cloneDeep"
+import { atob, isBackendService, isJSAllowed } from "../utilities"
 import { LITERAL_MARKER } from "../helpers/constants"
 import { getJsHelperList } from "./list"
 import { iifeWrapper } from "../iife"
 import { JsTimeoutError, UserScriptError } from "../errors"
+import { cloneDeep } from "lodash/fp"
 
 // The method of executing JS scripts depends on the bundle being built.
 // This setter is used in the entrypoint (either index.js or index.mjs).
-let runJS: ((js: string, context: any) => any) | undefined = undefined
+let runJS: ((js: string, context: Record<string, any>) => any) | undefined =
+  undefined
 export const setJSRunner = (runner: typeof runJS) => (runJS = runner)
 
 export const removeJSRunner = () => {
@@ -31,9 +32,19 @@ const removeSquareBrackets = (value: string) => {
   return value
 }
 
+const isReservedKey = (key: string) =>
+  key === "snippets" ||
+  key === "helpers" ||
+  key.startsWith("snippets.") ||
+  key.startsWith("helpers.")
+
 // Our context getter function provided to JS code as $.
 // Extracts a value from context.
 const getContextValue = (path: string, context: any) => {
+  // We populate `snippets` ourselves, don't allow access to it.
+  if (isReservedKey(path)) {
+    return undefined
+  }
   const literalStringRegex = /^(["'`]).*\1$/
   let data = context
   // check if it's a literal string - just return path if its quoted
@@ -46,7 +57,15 @@ const getContextValue = (path: string, context: any) => {
     }
     data = data[removeSquareBrackets(key)]
   })
-  return data
+
+  // When data is returned from this function in the backend, isolated-vm copies
+  // it across the vm boundary. We need to replicate that behaviour on the
+  // frontend to ensure that JS runs the same in both environments.
+  if (isBackendService()) {
+    return data
+  } else {
+    return cloneDeep(data)
+  }
 }
 
 // Evaluates JS code against a certain context
@@ -70,9 +89,8 @@ export function processJS(handlebars: string, context: any) {
     // Our $ context function gets a value from context.
     // We clone the context to avoid mutation in the binding affecting real
     // app context.
-    const clonedContext = cloneDeep({ ...context, snippets: null })
     const sandboxContext = {
-      $: (path: string) => getContextValue(path, clonedContext),
+      $: (path: string) => getContextValue(path, context),
       helpers: getJsHelperList(),
 
       // Proxy to evaluate snippets when running in the browser
diff --git a/packages/string-templates/src/index.ts b/packages/string-templates/src/index.ts
index c2be63978d..7246bc4942 100644
--- a/packages/string-templates/src/index.ts
+++ b/packages/string-templates/src/index.ts
@@ -1,4 +1,4 @@
-import { Context, createContext, runInNewContext } from "vm"
+import { createContext, runInNewContext } from "vm"
 import { create, TemplateDelegate } from "handlebars"
 import { registerAll, registerMinimum } from "./helpers/index"
 import { postprocess, preprocess } from "./processors"
@@ -455,21 +455,14 @@ export function convertToJS(hbs: string) {
 
 export { JsTimeoutError, UserScriptError } from "./errors"
 
-export function defaultJSSetup() {
-  if (!isBackendService()) {
-    /**
-     * Use polyfilled vm to run JS scripts in a browser Env
-     */
-    setJSRunner((js: string, context: Context) => {
-      context = {
-        ...context,
-        alert: undefined,
-        setInterval: undefined,
-        setTimeout: undefined,
-      }
-      createContext(context)
+export function browserJSSetup() {
+  /**
+   * Use polyfilled vm to run JS scripts in a browser Env
+   */
+  setJSRunner((js: string, context: Record<string, any>) => {
+    createContext(context)
 
-      const wrappedJs = `
+    const wrappedJs = `
         result = {
           result: null,
           error: null,
@@ -484,12 +477,17 @@ export function defaultJSSetup() {
         result;
       `
 
-      const result = runInNewContext(wrappedJs, context, { timeout: 1000 })
-      if (result.error) {
-        throw new UserScriptError(result.error)
-      }
-      return result.result
-    })
+    const result = runInNewContext(wrappedJs, context, { timeout: 1000 })
+    if (result.error) {
+      throw new UserScriptError(result.error)
+    }
+    return result.result
+  })
+}
+
+export function defaultJSSetup() {
+  if (!isBackendService()) {
+    browserJSSetup()
   } else {
     removeJSRunner()
   }
diff --git a/packages/string-templates/src/utilities.ts b/packages/string-templates/src/utilities.ts
index 18e0926b96..779bef3735 100644
--- a/packages/string-templates/src/utilities.ts
+++ b/packages/string-templates/src/utilities.ts
@@ -4,7 +4,14 @@ export const FIND_HBS_REGEX = /{{([^{].*?)}}/g
 export const FIND_ANY_HBS_REGEX = /{?{{([^{].*?)}}}?/g
 export const FIND_TRIPLE_HBS_REGEX = /{{{([^{].*?)}}}/g
 
+const isJest = () => typeof jest !== "undefined"
+
 export const isBackendService = () => {
+  // We consider the tests for string-templates to be frontend, so that they
+  // test the frontend JS functionality.
+  if (isJest()) {
+    return false
+  }
   return typeof window === "undefined"
 }
 
diff --git a/packages/string-templates/test/javascript.spec.ts b/packages/string-templates/test/javascript.spec.ts
index 99e6ee122a..9f2d493e23 100644
--- a/packages/string-templates/test/javascript.spec.ts
+++ b/packages/string-templates/test/javascript.spec.ts
@@ -1,7 +1,13 @@
-import vm from "vm"
-
-import { processStringSync, encodeJSBinding, setJSRunner } from "../src/index"
+import {
+  processStringSync,
+  encodeJSBinding,
+  defaultJSSetup,
+} from "../src/index"
 import { UUID_REGEX } from "./constants"
+import tk from "timekeeper"
+
+const DATE = "2021-01-21T12:00:00"
+tk.freeze(DATE)
 
 const processJS = (js: string, context?: object): any => {
   return processStringSync(encodeJSBinding(js), context)
@@ -9,9 +15,7 @@ const processJS = (js: string, context?: object): any => {
 
 describe("Javascript", () => {
   beforeAll(() => {
-    setJSRunner((js, context) => {
-      return vm.runInNewContext(js, context, { timeout: 1000 })
-    })
+    defaultJSSetup()
   })
 
   describe("Test the JavaScript helper", () => {
@@ -118,8 +122,7 @@ describe("Javascript", () => {
     })
 
     it("should handle errors", () => {
-      const output = processJS(`throw "Error"`)
-      expect(output).toBe("Error while executing JS")
+      expect(processJS(`throw "Error"`)).toEqual("Error")
     })
 
     it("should timeout after one second", () => {
@@ -127,16 +130,18 @@ describe("Javascript", () => {
       expect(output).toBe("Timed out while executing JS")
     })
 
-    it("should prevent access to the process global", () => {
-      const output = processJS(`return process`)
-      expect(output).toBe("Error while executing JS")
+    it("should prevent access to the process global", async () => {
+      expect(processJS(`return process`)).toEqual(
+        "ReferenceError: process is not defined"
+      )
     })
   })
 
   describe("check JS helpers", () => {
     it("should error if using the format helper. not helpers.", () => {
-      const output = processJS(`return helper.toInt(4.3)`)
-      expect(output).toBe("Error while executing JS")
+      expect(processJS(`return helper.toInt(4.3)`)).toEqual(
+        "ReferenceError: helper is not defined"
+      )
     })
 
     it("should be able to use toInt", () => {
@@ -156,4 +161,336 @@ describe("Javascript", () => {
       expect(output).toBe("Custom")
     })
   })
+
+  describe("mutability", () => {
+    it("should not allow the context to be mutated", async () => {
+      const context = { array: [1] }
+      const result = await processJS(
+        `
+        const array = $("array");
+        array.push(2);
+        return array[1]
+      `,
+        context
+      )
+      expect(result).toEqual(2)
+      expect(context.array).toEqual([1])
+    })
+
+    it("should copy values whenever returning them from $", async () => {
+      const context = { array: [1] }
+      const result = await processJS(
+        `
+        $("array").push(2);
+        return $("array")[1];
+      `,
+        context
+      )
+      expect(result).toEqual(undefined)
+      expect(context.array).toEqual([1])
+    })
+  })
+
+  describe("malice", () => {
+    it("should not be able to call JS functions", () => {
+      expect(processJS(`return alert("hello")`)).toEqual(
+        "ReferenceError: alert is not defined"
+      )
+
+      expect(processJS(`return prompt("hello")`)).toEqual(
+        "ReferenceError: prompt is not defined"
+      )
+
+      expect(processJS(`return confirm("hello")`)).toEqual(
+        "ReferenceError: confirm is not defined"
+      )
+
+      expect(processJS(`return setTimeout(() => {}, 1000)`)).toEqual(
+        "ReferenceError: setTimeout is not defined"
+      )
+
+      expect(processJS(`return setInterval(() => {}, 1000)`)).toEqual(
+        "ReferenceError: setInterval is not defined"
+      )
+    })
+  })
+
+  // the test cases here were extracted from templates/real world examples of JS in Budibase
+  describe("real test cases from Budicloud", () => {
+    const context = {
+      "Unit Value": 2,
+      Quantity: 1,
+    }
+    it("handle test case 1", async () => {
+      const result = await processJS(
+        `
+        var Gross = $("[Unit Value]") * $("[Quantity]")
+        return Gross.toFixed(2)`,
+        context
+      )
+      expect(result).toBeDefined()
+      expect(result).toBe("2.00")
+    })
+
+    it("handle test case 2", async () => {
+      const todayDate = new Date()
+      // add a year and a month
+      todayDate.setMonth(new Date().getMonth() + 1)
+      todayDate.setFullYear(todayDate.getFullYear() + 1)
+      const context = {
+        "Purchase Date": DATE,
+        today: todayDate.toISOString(),
+      }
+      const result = await processJS(
+        `
+        var purchase = new Date($("[Purchase Date]"));
+        let purchaseyear = purchase.getFullYear();
+        let purchasemonth = purchase.getMonth();
+
+        var today = new Date($("today"));
+        let todayyear = today.getFullYear();
+        let todaymonth = today.getMonth();
+
+        var age = todayyear - purchaseyear
+
+        if (((todaymonth - purchasemonth) < 6) == true){
+          return age
+        }
+        `,
+        context
+      )
+      expect(result).toBeDefined()
+      expect(result).toBe(1)
+    })
+
+    it("should handle test case 3", async () => {
+      const context = {
+        Escalate: true,
+        "Budget ($)": 1100,
+      }
+      const result = await processJS(
+        `
+        if ($("[Escalate]") == true) {
+          if ($("Budget ($)") <= 1000)
+            {return 2;}
+          if ($("Budget ($)") > 1000) 
+            {return 3;}
+          }
+          else {
+            if ($("Budget ($)") <= 1000)
+              {return 1;}
+            if ($("Budget ($)") > 1000)
+              if ($("Budget ($)") < 10000) 
+                {return 2;}
+              else 
+                {return 3}
+          }
+        `,
+        context
+      )
+      expect(result).toBeDefined()
+      expect(result).toBe(3)
+    })
+
+    it("should handle test case 4", async () => {
+      const context = {
+        "Time Sheets": ["a", "b"],
+      }
+      const result = await processJS(
+        `
+      let hours = 0
+      if (($("[Time Sheets]") != null) == true){
+        for (i = 0; i < $("[Time Sheets]").length; i++){
+          let hoursLogged = "Time Sheets." + i + ".Hours"
+          hours += $(hoursLogged)
+        }
+        return hours
+      }
+      if (($("[Time Sheets]") != null) == false){
+        return hours
+      }
+      `,
+        context
+      )
+      expect(result).toBeDefined()
+      expect(result).toBe("0ab")
+    })
+
+    it("should handle test case 5", async () => {
+      const context = {
+        change: JSON.stringify({ a: 1, primaryDisplay: "a" }),
+        previous: JSON.stringify({ a: 2, primaryDisplay: "b" }),
+      }
+      const result = await processJS(
+        `
+      let change = $("[change]") ? JSON.parse($("[change]")) : {}
+      let previous = $("[previous]") ? JSON.parse($("[previous]")) : {}
+
+      function simplifyLink(originalKey, value, parent) {
+        if (Array.isArray(value)) {
+          if (value.filter(item => Object.keys(item || {}).includes("primaryDisplay")).length > 0) {
+            parent[originalKey] = value.map(link => link.primaryDisplay)
+          }
+        }
+      }
+
+      for (let entry of Object.entries(change)) {
+        simplifyLink(entry[0], entry[1], change)
+      }
+      for (let entry of Object.entries(previous)) {
+        simplifyLink(entry[0], entry[1], previous)
+      }
+      
+      let diff = Object.fromEntries(Object.entries(change).filter(([k, v]) => previous[k]?.toString() !== v?.toString()))
+      
+      delete diff.audit_change
+      delete diff.audit_previous
+      delete diff._id
+      delete diff._rev
+      delete diff.tableId
+      delete diff.audit
+      
+      for (let entry of Object.entries(diff)) {
+        simplifyLink(entry[0], entry[1], diff)
+      }
+      
+      return JSON.stringify(change)?.replaceAll(",\\"", ",\\n\\t\\"").replaceAll("{\\"", "{\\n\\t\\"").replaceAll("}", "\\n}")
+      `,
+        context
+      )
+      expect(result).toBe(`{\n\t"a":1,\n\t"primaryDisplay":"a"\n}`)
+    })
+
+    it("should handle test case 6", async () => {
+      const context = {
+        "Join Date": DATE,
+      }
+      const result = await processJS(
+        `
+        var rate = 5;
+        var today = new Date();
+        
+        // comment
+        function monthDiff(dateFrom, dateTo) {
+         return dateTo.getMonth() - dateFrom.getMonth() + 
+           (12 * (dateTo.getFullYear() - dateFrom.getFullYear()))
+        }
+        var serviceMonths = monthDiff( new Date($("[Join Date]")), today);
+        var serviceYears = serviceMonths / 12;
+        
+        if (serviceYears >= 1 && serviceYears < 5){
+          rate = 10;
+        }
+        if (serviceYears >= 5 && serviceYears < 10){
+          rate = 15;
+        }
+        if (serviceYears >= 10){
+          rate = 15;
+          rate += 0.5 * (Number(serviceYears.toFixed(0)) - 10);
+        }
+        return rate;
+        `,
+        context
+      )
+      expect(result).toBe(10)
+    })
+
+    it("should handle test case 7", async () => {
+      const context = {
+        "P I": "Pass",
+        "PA I": "Pass",
+        "F I": "Fail",
+        "V I": "Pass",
+      }
+      const result = await processJS(
+        `if (($("[P I]") == "Pass") == true)
+              if (($("[  P I]") == "Pass") == true)
+                if (($("[F I]") == "Pass") == true)
+                  if (($("[V I]") == "Pass") == true)
+                    {return "Pass"}
+            
+            if (($("[PA I]") == "Fail") == true)
+              {return "Fail"}
+            if (($("[  P I]") == "Fail") == true)
+              {return "Fail"}
+            if (($("[F I]") == "Fail") == true)
+              {return "Fail"}
+            if (($("[V I]") == "Fail") == true)
+              {return "Fail"}
+            
+            else
+              {return ""}`,
+        context
+      )
+      expect(result).toBe("Fail")
+    })
+
+    it("should handle test case 8", async () => {
+      const context = {
+        "T L": [{ Hours: 10 }],
+        "B H": 50,
+      }
+      const result = await processJS(
+        `var totalHours = 0;
+            if (($("[T L]") != null) == true){
+            for (let i = 0; i < ($("[T L]").length); i++){
+              var individualHours = "T L." + i + ".Hours";
+              var hoursNum = Number($(individualHours));
+              totalHours += hoursNum;
+            }
+            return totalHours.toFixed(2);
+            }
+            if (($("[T L]") != null) == false) {
+              return totalHours.toFixed(2);
+            }
+        `,
+        context
+      )
+      expect(result).toBe("10.00")
+    })
+
+    it("should handle test case 9", async () => {
+      const context = {
+        "T L": [{ Hours: 10 }],
+        "B H": 50,
+      }
+      const result = await processJS(
+        `var totalHours = 0;
+            if (($("[T L]") != null) == true){
+            for (let i = 0; i < ($("[T L]").length); i++){
+              var individualHours = "T L." + i + ".Hours";
+              var hoursNum = Number($(individualHours));
+              totalHours += hoursNum;
+            }
+            return ($("[B H]") - totalHours).toFixed(2);
+            }
+            if (($("[T L]") != null) == false) {
+              return ($("[B H]") - totalHours).toFixed(2);
+            }`,
+        context
+      )
+      expect(result).toBe("40.00")
+    })
+
+    it("should handle test case 10", async () => {
+      const context = {
+        "F F": [{ "F S": 10 }],
+      }
+      const result = await processJS(
+        `var rating = 0;
+          
+          if ($("[F F]") != null){
+          for (i = 0; i < $("[F F]").length; i++){
+            var individualRating = $("F F." + i + ".F S");
+            rating += individualRating;
+            }
+            rating = (rating / $("[F F]").length);
+          }
+          return rating;
+          `,
+        context
+      )
+      expect(result).toBe(10)
+    })
+  })
 })

From 55699cfaffd49de3edd5138ff66263549682face Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Mon, 7 Oct 2024 12:23:22 +0100
Subject: [PATCH 2/4] Don't clone non-object/non-arrays, use default JS in
 manifest.spec.ts.

---
 packages/string-templates/src/helpers/javascript.ts | 6 +++++-
 packages/string-templates/test/manifest.spec.ts     | 8 ++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/packages/string-templates/src/helpers/javascript.ts b/packages/string-templates/src/helpers/javascript.ts
index 4d53fe335d..61fe872103 100644
--- a/packages/string-templates/src/helpers/javascript.ts
+++ b/packages/string-templates/src/helpers/javascript.ts
@@ -64,7 +64,11 @@ const getContextValue = (path: string, context: any) => {
   if (isBackendService()) {
     return data
   } else {
-    return cloneDeep(data)
+    if (typeof data === "object") {
+      return cloneDeep(data)
+    } else {
+      return data
+    }
   }
 }
 
diff --git a/packages/string-templates/test/manifest.spec.ts b/packages/string-templates/test/manifest.spec.ts
index de0e3355fd..2d09bc7871 100644
--- a/packages/string-templates/test/manifest.spec.ts
+++ b/packages/string-templates/test/manifest.spec.ts
@@ -1,5 +1,3 @@
-import vm from "vm"
-
 jest.mock("@budibase/handlebars-helpers/lib/math", () => {
   const actual = jest.requireActual("@budibase/handlebars-helpers/lib/math")
 
@@ -17,7 +15,7 @@ jest.mock("@budibase/handlebars-helpers/lib/uuid", () => {
   }
 })
 
-import { processString, setJSRunner } from "../src/index"
+import { defaultJSSetup, processString } from "../src/index"
 
 import tk from "timekeeper"
 import { getParsedManifest, runJsHelpersTests } from "./utils"
@@ -32,9 +30,7 @@ describe("manifest", () => {
   const manifest = getParsedManifest()
 
   beforeAll(() => {
-    setJSRunner((js, context) => {
-      return vm.runInNewContext(js, context, { timeout: 1000 })
-    })
+    defaultJSSetup()
   })
 
   describe("examples are valid", () => {

From c21d8a8131bd3b3a97f411e35ccc1c8bc4ff5d6d Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Mon, 7 Oct 2024 12:27:17 +0100
Subject: [PATCH 3/4] Remove inaccurate comment.

---
 packages/string-templates/src/helpers/javascript.ts | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/packages/string-templates/src/helpers/javascript.ts b/packages/string-templates/src/helpers/javascript.ts
index 61fe872103..48db4b9809 100644
--- a/packages/string-templates/src/helpers/javascript.ts
+++ b/packages/string-templates/src/helpers/javascript.ts
@@ -90,9 +90,6 @@ export function processJS(handlebars: string, context: any) {
       snippetMap[snippet.name] = snippet.code
     }
 
-    // Our $ context function gets a value from context.
-    // We clone the context to avoid mutation in the binding affecting real
-    // app context.
     const sandboxContext = {
       $: (path: string) => getContextValue(path, context),
       helpers: getJsHelperList(),

From deee5e4e05fcdb6f74a2d3812345ae5e051b87fa Mon Sep 17 00:00:00 2001
From: Sam Rose <hello@samwho.dev>
Date: Mon, 7 Oct 2024 12:35:05 +0100
Subject: [PATCH 4/4] Put the frontend back to how it was.

---
 .../src/helpers/javascript.ts                 | 32 +++++++++++--------
 .../string-templates/test/javascript.spec.ts  | 13 --------
 2 files changed, 19 insertions(+), 26 deletions(-)

diff --git a/packages/string-templates/src/helpers/javascript.ts b/packages/string-templates/src/helpers/javascript.ts
index 48db4b9809..4fb1329196 100644
--- a/packages/string-templates/src/helpers/javascript.ts
+++ b/packages/string-templates/src/helpers/javascript.ts
@@ -58,18 +58,7 @@ const getContextValue = (path: string, context: any) => {
     data = data[removeSquareBrackets(key)]
   })
 
-  // When data is returned from this function in the backend, isolated-vm copies
-  // it across the vm boundary. We need to replicate that behaviour on the
-  // frontend to ensure that JS runs the same in both environments.
-  if (isBackendService()) {
-    return data
-  } else {
-    if (typeof data === "object") {
-      return cloneDeep(data)
-    } else {
-      return data
-    }
-  }
+  return data
 }
 
 // Evaluates JS code against a certain context
@@ -90,8 +79,25 @@ export function processJS(handlebars: string, context: any) {
       snippetMap[snippet.name] = snippet.code
     }
 
+    let clonedContext: Record<string, any>
+    if (isBackendService()) {
+      // On the backned, values are copied across the isolated-vm boundary and
+      // so we don't need to do any cloning here. This does create a fundamental
+      // difference in how JS executes on the frontend vs the backend, e.g.
+      // consider this snippet:
+      //
+      //   $("array").push(2)
+      //   return $("array")[1]
+      //
+      // With the context of `{ array: [1] }`, the backend will return
+      // `undefined` whereas the frontend will return `2`. We should fix this.
+      clonedContext = context
+    } else {
+      clonedContext = cloneDeep(context)
+    }
+
     const sandboxContext = {
-      $: (path: string) => getContextValue(path, context),
+      $: (path: string) => getContextValue(path, clonedContext),
       helpers: getJsHelperList(),
 
       // Proxy to evaluate snippets when running in the browser
diff --git a/packages/string-templates/test/javascript.spec.ts b/packages/string-templates/test/javascript.spec.ts
index 9f2d493e23..9134005acb 100644
--- a/packages/string-templates/test/javascript.spec.ts
+++ b/packages/string-templates/test/javascript.spec.ts
@@ -176,19 +176,6 @@ describe("Javascript", () => {
       expect(result).toEqual(2)
       expect(context.array).toEqual([1])
     })
-
-    it("should copy values whenever returning them from $", async () => {
-      const context = { array: [1] }
-      const result = await processJS(
-        `
-        $("array").push(2);
-        return $("array")[1];
-      `,
-        context
-      )
-      expect(result).toEqual(undefined)
-      expect(context.array).toEqual([1])
-    })
   })
 
   describe("malice", () => {