From 3ccb076f0b645856d3f2871afc5fef4029af6769 Mon Sep 17 00:00:00 2001 From: zehjotkah <111518041+zehjotkah@users.noreply.github.com> Date: Tue, 23 Sep 2025 08:51:33 +0100 Subject: [PATCH 1/5] enabling safe functions in expression engine allowing toLowerCase(), replace(), and split() to be used in the expression editor. https://discord.com/channels/955905230107738152/1418928616648998992/1419623805541814393 --- packages/sdk/src/expression.test.ts | 90 +++++++++++++++++++++++++++++ packages/sdk/src/expression.ts | 15 ++++- 2 files changed, 104 insertions(+), 1 deletion(-) diff --git a/packages/sdk/src/expression.test.ts b/packages/sdk/src/expression.test.ts index 4c0d966031a2..5e5291780311 100644 --- a/packages/sdk/src/expression.test.ts +++ b/packages/sdk/src/expression.test.ts @@ -220,6 +220,60 @@ describe("lint expression", () => { error(1, 8, `"await" keyword is not supported`), ]); }); + + test("allow safe string methods", () => { + expect( + lintExpression({ + expression: `title.toLowerCase()`, + availableVariables: new Set(["title"]), + }) + ).toEqual([]); + expect( + lintExpression({ + expression: `title.replace(" ", "-")`, + availableVariables: new Set(["title"]), + }) + ).toEqual([]); + expect( + lintExpression({ + expression: `title.split("-")`, + availableVariables: new Set(["title"]), + }) + ).toEqual([]); + }); + + test("allow chained string methods", () => { + expect( + lintExpression({ + expression: `title.toLowerCase().replace(" ", "-").split("-")`, + availableVariables: new Set(["title"]), + }) + ).toEqual([]); + }); + + test("forbid unsafe method calls", () => { + expect( + lintExpression({ + expression: `arr.pop()`, + availableVariables: new Set(["arr"]), + }) + ).toEqual([error(0, 9, "Functions are not supported")]); + expect( + lintExpression({ + expression: `obj.push(1)`, + availableVariables: new Set(["obj"]), + }) + ).toEqual([error(0, 11, "Functions are not supported")]); + }); + + test("forbid standalone function calls", () => { + expect( + lintExpression({ + expression: `func()`, + availableVariables: new Set(["func"]), + }) + ).toEqual([error(0, 6, "Functions are not supported")]); + }); }); test("check simple literals", () => { @@ -346,6 +400,42 @@ describe("transpile expression", () => { } expect(errorString).toEqual(`Unexpected token (1:0) in ""`); }); + + test("transpile string methods with optional chaining", () => { + expect( + transpileExpression({ + expression: "title.toLowerCase()", + executable: true, + }) + ).toEqual("title?.toLowerCase()"); + expect( + transpileExpression({ + expression: "user.name.replace(' ', '-')", + executable: true, + }) + ).toEqual("user?.name?.replace(' ', '-')"); + expect( + transpileExpression({ + expression: "data.title.split('-')", + executable: true, + }) + ).toEqual("data?.title?.split('-')"); + }); + + test("transpile chained string methods with optional chaining", () => { + expect( + transpileExpression({ + expression: "title.toLowerCase().replace(/\\s+/g, '-')", + executable: true, + }) + ).toEqual("title?.toLowerCase()?.replace(/\\s+/g, '-')"); + expect( + transpileExpression({ + expression: "user.name.toLowerCase().replace(' ', '-').split('-')", + executable: true, + }) + ).toEqual("user?.name?.toLowerCase()?.replace(' ', '-')?.split('-')"); + }); }); describe("object expression transformations", () => { diff --git a/packages/sdk/src/expression.ts b/packages/sdk/src/expression.ts index f1c1e91108e5..e63111797255 100644 --- a/packages/sdk/src/expression.ts +++ b/packages/sdk/src/expression.ts @@ -112,7 +112,20 @@ export const lintExpression = ({ ThisExpression: addMessage(`"this" keyword is not supported`), FunctionExpression: addMessage("Functions are not supported"), UpdateExpression: addMessage("Increment and decrement are not supported"), - CallExpression: addMessage("Functions are not supported"), + CallExpression(node) { + // Check if this is a method call (MemberExpression) or standalone function call + if (node.callee.type === "MemberExpression") { + // Allow specific safe string methods + const allowedMethods = new Set(["toLowerCase", "replace", "split"]); + if ( + node.callee.property.type === "Identifier" && + allowedMethods.has(node.callee.property.name) + ) { + return; + } + } + addMessage("Functions are not supported")(node); + }, NewExpression: addMessage("Classes are not supported"), SequenceExpression: addMessage(`Only single expression is supported`), ArrowFunctionExpression: addMessage("Functions are not supported"), From ec5789f74fc7c00794f176e8d912125c26b8029b Mon Sep 17 00:00:00 2001 From: zehjotkah <111518041+zehjotkah@users.noreply.github.com> Date: Wed, 24 Sep 2025 18:43:40 +0100 Subject: [PATCH 2/5] feat(expressions): Add support for safe, non-mutating String and Array methods This pull request enhances the expression engine to support a curated list of safe, non-mutating methods on the String and Array prototypes. --- packages/sdk/src/expression.test.ts | 50 ++++++++++++++++++----------- packages/sdk/src/expression.ts | 31 +++++++++++++----- 2 files changed, 55 insertions(+), 26 deletions(-) diff --git a/packages/sdk/src/expression.test.ts b/packages/sdk/src/expression.test.ts index 5e5291780311..951eb6ec0b3a 100644 --- a/packages/sdk/src/expression.test.ts +++ b/packages/sdk/src/expression.test.ts @@ -222,24 +222,38 @@ describe("lint expression", () => { }); test("allow safe string methods", () => { - expect( - lintExpression({ - expression: `title.toLowerCase()`, - availableVariables: new Set(["title"]), - }) - ).toEqual([]); - expect( - lintExpression({ - expression: `title.replace(" ", "-")`, - availableVariables: new Set(["title"]), - }) - ).toEqual([]); - expect( - lintExpression({ - expression: `title.split("-")`, - availableVariables: new Set(["title"]), - }) - ).toEqual([]); + const stringMethods = [ + "toLowerCase", + "replace", + "split", + "at", + "endsWith", + "includes", + "startsWith", + "toUpperCase", + "toLocaleLowerCase", + "toLocaleUpperCase", + ]; + for (const method of stringMethods) { + expect( + lintExpression({ + expression: `title.${method}()`, + availableVariables: new Set(["title"]), + }) + ).toEqual([]); + } + }); + + test("allow safe array methods", () => { + const arrayMethods = ["at", "includes", "join", "slice"]; + for (const method of arrayMethods) { + expect( + lintExpression({ + expression: `arr.${method}()`, + availableVariables: new Set(["arr"]), + }) + ).toEqual([]); + } }); test("allow chained string methods", () => { diff --git a/packages/sdk/src/expression.ts b/packages/sdk/src/expression.ts index e63111797255..fb0337acbdb6 100644 --- a/packages/sdk/src/expression.ts +++ b/packages/sdk/src/expression.ts @@ -29,6 +29,21 @@ type ExpressionVisitor = { [K in Expression["type"]]: (node: Extract) => void; }; +const allowedStringMethods = new Set([ + "toLowerCase", + "replace", + "split", + "at", + "endsWith", + "includes", + "startsWith", + "toUpperCase", + "toLocaleLowerCase", + "toLocaleUpperCase", +]); + +const allowedArrayMethods = new Set(["at", "includes", "join", "slice"]); + export const lintExpression = ({ expression, availableVariables = new Set(), @@ -113,15 +128,15 @@ export const lintExpression = ({ FunctionExpression: addMessage("Functions are not supported"), UpdateExpression: addMessage("Increment and decrement are not supported"), CallExpression(node) { - // Check if this is a method call (MemberExpression) or standalone function call if (node.callee.type === "MemberExpression") { - // Allow specific safe string methods - const allowedMethods = new Set(["toLowerCase", "replace", "split"]); - if ( - node.callee.property.type === "Identifier" && - allowedMethods.has(node.callee.property.name) - ) { - return; + if (node.callee.property.type === "Identifier") { + const methodName = node.callee.property.name; + if ( + allowedStringMethods.has(methodName) || + allowedArrayMethods.has(methodName) + ) { + return; + } } } addMessage("Functions are not supported")(node); From 454b89ea6f1cfe309b0af134781cc11dc260b8f1 Mon Sep 17 00:00:00 2001 From: zehjotkah <111518041+zehjotkah@users.noreply.github.com> Date: Wed, 24 Sep 2025 18:59:42 +0100 Subject: [PATCH 3/5] update expression.test.ts to use test.each --- packages/sdk/src/expression.test.ts | 47 ++++++++++++++--------------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/packages/sdk/src/expression.test.ts b/packages/sdk/src/expression.test.ts index 951eb6ec0b3a..205fbfe82d33 100644 --- a/packages/sdk/src/expression.test.ts +++ b/packages/sdk/src/expression.test.ts @@ -221,32 +221,29 @@ describe("lint expression", () => { ]); }); - test("allow safe string methods", () => { - const stringMethods = [ - "toLowerCase", - "replace", - "split", - "at", - "endsWith", - "includes", - "startsWith", - "toUpperCase", - "toLocaleLowerCase", - "toLocaleUpperCase", - ]; - for (const method of stringMethods) { - expect( - lintExpression({ - expression: `title.${method}()`, - availableVariables: new Set(["title"]), - }) - ).toEqual([]); - } + test.each([ + "toLowerCase", + "replace", + "split", + "at", + "endsWith", + "includes", + "startsWith", + "toUpperCase", + "toLocaleLowerCase", + "toLocaleUpperCase", + ])("allow safe string method: %s", (method) => { + expect( + lintExpression({ + expression: `title.${method}()`, + availableVariables: new Set(["title"]), + }) + ).toEqual([]); }); - test("allow safe array methods", () => { - const arrayMethods = ["at", "includes", "join", "slice"]; - for (const method of arrayMethods) { + test.each(["at", "includes", "join", "slice"])( + "allow safe array method: %s", + (method) => { expect( lintExpression({ expression: `arr.${method}()`, @@ -254,7 +251,7 @@ describe("lint expression", () => { }) ).toEqual([]); } - }); + ); test("allow chained string methods", () => { expect( From fe2ded38fe84965d69bf4dbda339b32c54cc05a5 Mon Sep 17 00:00:00 2001 From: zehjotkah <111518041+zehjotkah@users.noreply.github.com> Date: Thu, 25 Sep 2025 10:07:13 +0100 Subject: [PATCH 4/5] Improved error messages to be specific --- packages/sdk/src/expression.test.ts | 8 ++++---- packages/sdk/src/expression.ts | 10 +++++++++- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/packages/sdk/src/expression.test.ts b/packages/sdk/src/expression.test.ts index 205fbfe82d33..90df85a27f1b 100644 --- a/packages/sdk/src/expression.test.ts +++ b/packages/sdk/src/expression.test.ts @@ -157,7 +157,7 @@ describe("lint expression", () => { ).toEqual([ error(1, 13, "Functions are not supported"), error(17, 25, "Functions are not supported"), - error(29, 33, "Functions are not supported"), + error(29, 33, `"fn" function is not supported`), ]); }); @@ -268,13 +268,13 @@ describe("lint expression", () => { expression: `arr.pop()`, availableVariables: new Set(["arr"]), }) - ).toEqual([error(0, 9, "Functions are not supported")]); + ).toEqual([error(0, 9, `"pop" function is not supported`)]); expect( lintExpression({ expression: `obj.push(1)`, availableVariables: new Set(["obj"]), }) - ).toEqual([error(0, 11, "Functions are not supported")]); + ).toEqual([error(0, 11, `"push" function is not supported`)]); }); test("forbid standalone function calls", () => { @@ -283,7 +283,7 @@ describe("lint expression", () => { expression: `func()`, availableVariables: new Set(["func"]), }) - ).toEqual([error(0, 6, "Functions are not supported")]); + ).toEqual([error(0, 6, `"func" function is not supported`)]); }); }); diff --git a/packages/sdk/src/expression.ts b/packages/sdk/src/expression.ts index fb0337acbdb6..c51f4a6458f1 100644 --- a/packages/sdk/src/expression.ts +++ b/packages/sdk/src/expression.ts @@ -128,6 +128,7 @@ export const lintExpression = ({ FunctionExpression: addMessage("Functions are not supported"), UpdateExpression: addMessage("Increment and decrement are not supported"), CallExpression(node) { + let calleeName; if (node.callee.type === "MemberExpression") { if (node.callee.property.type === "Identifier") { const methodName = node.callee.property.name; @@ -137,9 +138,16 @@ export const lintExpression = ({ ) { return; } + calleeName = methodName; } + } else if (node.callee.type === "Identifier") { + calleeName = node.callee.name; + } + if (calleeName) { + addMessage(`"${calleeName}" function is not supported`)(node); + } else { + addMessage("Functions are not supported")(node); } - addMessage("Functions are not supported")(node); }, NewExpression: addMessage("Classes are not supported"), SequenceExpression: addMessage(`Only single expression is supported`), From 64e5b73faf0a1043fac274e993b96d42c1f43cb5 Mon Sep 17 00:00:00 2001 From: zehjotkah <111518041+zehjotkah@users.noreply.github.com> Date: Sat, 27 Sep 2025 20:45:51 +0100 Subject: [PATCH 5/5] add optional chaining added optional chaining and additional tests. --- packages/sdk/src/expression.test.ts | 49 ++++++++++++++++++++++++++--- packages/sdk/src/expression.ts | 13 ++++++++ 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/packages/sdk/src/expression.test.ts b/packages/sdk/src/expression.test.ts index 90df85a27f1b..9a0abb2812d8 100644 --- a/packages/sdk/src/expression.test.ts +++ b/packages/sdk/src/expression.test.ts @@ -418,19 +418,19 @@ describe("transpile expression", () => { expression: "title.toLowerCase()", executable: true, }) - ).toEqual("title?.toLowerCase()"); + ).toEqual("title?.toLowerCase?.()"); expect( transpileExpression({ expression: "user.name.replace(' ', '-')", executable: true, }) - ).toEqual("user?.name?.replace(' ', '-')"); + ).toEqual("user?.name?.replace?.(' ', '-')"); expect( transpileExpression({ expression: "data.title.split('-')", executable: true, }) - ).toEqual("data?.title?.split('-')"); + ).toEqual("data?.title?.split?.('-')"); }); test("transpile chained string methods with optional chaining", () => { @@ -439,13 +439,52 @@ describe("transpile expression", () => { expression: "title.toLowerCase().replace(/\\s+/g, '-')", executable: true, }) - ).toEqual("title?.toLowerCase()?.replace(/\\s+/g, '-')"); + ).toEqual("title?.toLowerCase?.()?.replace?.(/\\s+/g, '-')"); expect( transpileExpression({ expression: "user.name.toLowerCase().replace(' ', '-').split('-')", executable: true, }) - ).toEqual("user?.name?.toLowerCase()?.replace(' ', '-')?.split('-')"); + ).toEqual("user?.name?.toLowerCase?.()?.replace?.(' ', '-')?.split?.('-')"); + }); + + test("transpile array methods with optional chaining", () => { + expect( + transpileExpression({ + expression: "items.map(item => item.id)", + executable: true, + }) + ).toEqual("items?.map?.(item => item?.id)"); + expect( + transpileExpression({ + expression: "data.list.filter(x => x > 0)", + executable: true, + }) + ).toEqual("data?.list?.filter?.(x => x > 0)"); + }); + + test("transpile nested method calls with optional chaining", () => { + expect( + transpileExpression({ + expression: "obj.method().prop.anotherMethod()", + executable: true, + }) + ).toEqual("obj?.method?.()?.prop?.anotherMethod?.()"); + }); + + test("preserve existing optional chaining", () => { + expect( + transpileExpression({ + expression: "obj?.method?.()", + executable: true, + }) + ).toEqual("obj?.method?.()"); + expect( + transpileExpression({ + expression: "obj?.prop?.method?.()", + executable: true, + }) + ).toEqual("obj?.prop?.method?.()"); }); }); diff --git a/packages/sdk/src/expression.ts b/packages/sdk/src/expression.ts index c51f4a6458f1..c4a066202a05 100644 --- a/packages/sdk/src/expression.ts +++ b/packages/sdk/src/expression.ts @@ -293,6 +293,19 @@ export const transpileExpression = ({ replacements.push([dotIndex, dotIndex, "?."]); } }, + CallExpression(node) { + if (executable === false || node.optional) { + return; + } + // Add optional chaining to method calls: obj.method() -> obj?.method?.() + if (node.callee.type === "MemberExpression") { + // Find the opening parenthesis after the method name + const openParenIndex = expression.indexOf("(", node.callee.end); + if (openParenIndex !== -1) { + replacements.push([openParenIndex, openParenIndex, "?."]); + } + } + }, }); // order from the latest to the first insertion to not break other positions replacements.sort(([leftStart], [rightStart]) => rightStart - leftStart);