Merge pull request #14705 from Budibase/fix-processObjectSync

Make the new throwing behaviour of `processStringSync` opt-in.
Budibase · Oct 4, 2024 · a6d673f · a6d673f
2 parents c2358a6 + c10cdd3
commit a6d673f
Show file tree

Hide file tree

Showing 6 changed files with 33 additions and 24 deletions.
diff --git a/packages/builder/src/components/common/bindings/BindingPanel.svelte b/packages/builder/src/components/common/bindings/BindingPanel.svelte
@@ -145,10 +145,16 @@
   const debouncedEval = Utils.debounce((expression, context, snippets) => {
     try {
       expressionError = null
-      expressionResult = processStringSync(expression || "", {
-        ...context,
-        snippets,
-      })
+      expressionResult = processStringSync(
+        expression || "",
+        {
+          ...context,
+          snippets,
+        },
+        {
+          noThrow: false,
+        }
+      )
     } catch (err) {
       expressionResult = null
       expressionError = err

diff --git a/packages/server/src/jsRunner/tests/jsRunner.spec.ts b/packages/server/src/jsRunner/tests/jsRunner.spec.ts
@@ -41,11 +41,11 @@ describe("jsRunner (using isolated-vm)", () => {
   })
 
   it("should prevent sandbox escape", async () => {
-    await expect(
-      processJS(`return this.constructor.constructor("return process.env")()`)
-    ).rejects.toThrow(
-      "error while running user-supplied JavaScript: ReferenceError: process is not defined"
-    )
+    expect(
+      await processJS(
+        `return this.constructor.constructor("return process.env")()`
+      )
+    ).toEqual("ReferenceError: process is not defined")
   })
 
   describe("helpers", () => {

diff --git a/packages/server/src/utilities/rowProcessor/utils.ts b/packages/server/src/utilities/rowProcessor/utils.ts
@@ -1,5 +1,5 @@
 import { AutoFieldDefaultNames } from "../../constants"
-import { processStringSync, UserScriptError } from "@budibase/string-templates"
+import { processStringSync } from "@budibase/string-templates"
 import {
   AutoColumnFieldMetadata,
   FieldSchema,
@@ -81,14 +81,7 @@ export async function processFormulas<T extends Row | Row[]>(
             ...row,
             [column]: tracer.trace("processStringSync", {}, span => {
               span?.addTags({ table_id: table._id, column, static: isStatic })
-              try {
-                return processStringSync(formula, context)
-              } catch (err: any) {
-                if (err.code === UserScriptError.code) {
-                  return err.userScriptError.toString()
-                }
-                throw err
-              }
+              return processStringSync(formula, context)
             }),
           }
         }

diff --git a/packages/string-templates/src/helpers/javascript.ts b/packages/string-templates/src/helpers/javascript.ts
@@ -95,6 +95,8 @@ export function processJS(handlebars: string, context: any) {
   } catch (error: any) {
     onErrorLog && onErrorLog(error)
 
+    const { noThrow = true } = context.__opts || {}
+
     // The error handling below is quite messy, because it has fallen to
     // string-templates to handle a variety of types of error specific to usages
     // above it in the stack. It would be nice some day to refactor this to
@@ -123,11 +125,17 @@ export function processJS(handlebars: string, context: any) {
     // This is to catch the error that happens if a user-supplied JS script
     // throws for reasons introduced by the user.
     if (error.code === UserScriptError.code) {
+      if (noThrow) {
+        return error.userScriptError.toString()
+      }
       throw error
     }
 
     if (error.name === "SyntaxError") {
-      return error.toString()
+      if (noThrow) {
+        return error.toString()
+      }
+      throw error
     }
 
     return "Error while executing JS"

diff --git a/packages/string-templates/src/index.ts b/packages/string-templates/src/index.ts
@@ -123,7 +123,7 @@ function createTemplate(
 export async function processObject<T extends Record<string, any>>(
   object: T,
   context: object,
-  opts?: { noHelpers?: boolean; escapeNewlines?: boolean; onlyFound?: boolean }
+  opts?: ProcessOptions
 ): Promise<T> {
   testObject(object)
 
@@ -173,7 +173,7 @@ export async function processString(
 export function processObjectSync(
   object: { [x: string]: any },
   context: any,
-  opts: any
+  opts?: ProcessOptions
 ): object | Array<any> {
   testObject(object)
   for (let key of Object.keys(object || {})) {
@@ -231,10 +231,11 @@ export function processStringSync(
       return process(string)
     }
   } catch (err: any) {
-    if (err.code === UserScriptError.code) {
-      throw err
+    const { noThrow = true } = opts || {}
+    if (noThrow) {
+      return input
     }
-    return input
+    throw err
   }
 }
 

diff --git a/packages/string-templates/src/types.ts b/packages/string-templates/src/types.ts
@@ -3,6 +3,7 @@ export interface ProcessOptions {
   noEscaping?: boolean
   noHelpers?: boolean
   noFinalise?: boolean
+  noThrow?: boolean
   escapeNewlines?: boolean
   onlyFound?: boolean
   disabledHelpers?: string[]