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) => 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) => { + 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) + }) + }) })