chore: stricter wildcard variable validation

Author: kevinkim-ogp

packages/backend/src/helpers/__tests__/compute-parameters.test.ts

@@ -352,53 +352,48 @@ describe('compute parameters', () => {
 
   it.each([
     {
-      testDescription: 'wildcard on empty array',
-      params: `empty!! {{step.${randomStepID}.emptyArrayProp.*}}`,
-      expected: 'empty!! ',
-    },
-    {
-      testDescription: 'wildcard on array of values',
-      params: `{{step.${randomStepID}.arrayProp.*}}`,
-      expected: 'array value 1, hehe, array value 3',
-    },
-    {
-      testDescription: 'wildcard on array of objects to get strings',
+      testDescription: 'compute wildcard on array of objects to get strings',
       params: `wildcard get strings {{step.${randomStepID}.arrayOfObjectsProp.*.stringProp}}`,
       expected:
         'wildcard get strings string value 1 in array of objects, string value 2 in array of objects',
     },
     {
-      testDescription: 'wildcard on array of objects to get numbers',
+      testDescription: 'compute wildcard on array of objects to get numbers',
       params: `wildcard get numbers {{step.${randomStepID}.arrayOfObjectsProp.*.numberProp}}`,
       expected: 'wildcard get numbers 456, 789',
     },
     {
-      testDescription: 'wildcard on both strings and numbers',
+      testDescription: 'compute wildcard on both strings and numbers',
       params: `wildcard get strings and numbers {{step.${randomStepID}.arrayOfObjectsProp.*.stringProp}} {{step.${randomStepID}.arrayOfObjectsProp.*.numberProp}}`,
       expected:
         'wildcard get strings and numbers string value 1 in array of objects, string value 2 in array of objects 456, 789',
     },
     {
-      testDescription: 'wildcard on non-existent key',
-      params: `non-existent key! {{step.${randomStepID}.non-existent-key.*}}`,
+      testDescription: 'compute wildcard on non-existent key',
+      params: `non-existent key! {{step.${randomStepID}.non-existent-key.*.id}}`,
       expected: 'non-existent key! ',
     },
     {
-      testDescription: 'prop name with wildcard',
-      params: `prop with wildcard {{step.${randomStepID}.stringWith*}}`,
-      expected: 'prop with wildcard string value',
+      testDescription: 'compute wildcard on a string',
+      params: `wildcard on a string!! {{step.${randomStepID}.stringProp.*.id}}`,
+      expected: 'wildcard on a string!! string value',
     },
     {
-      testDescription: 'prop name with space and wildcard',
-      params: `prop with space and wildcard! {{step.${randomStepID}.string with *}}`,
-      expected: `prop with space and wildcard! string value 2`,
+      testDescription: 'ignore wildcard at end of variable',
+      params: `empty!! {{step.${randomStepID}.emptyArrayProp.*}}`,
+      expected: `empty!! {{step.${randomStepID}.emptyArrayProp.*}}`,
     },
     {
-      testDescription: 'wildcard on a string',
-      params: `wildcard on a string!! {{step.${randomStepID}.stringProp.*}}`,
-      expected: 'wildcard on a string!! string value',
+      testDescription: 'ignore wildcard inside the key',
+      params: `wildcard inside the key {{step.${randomStepID}.stringProp*}}`,
+      expected: `wildcard inside the key {{step.${randomStepID}.stringProp*}}`,
+    },
+    {
+      testDescription: 'ignore more than one wildcard inside the variable',
+      params: `two wildcards inside the key {{step.${randomStepID}.*.stringProp.*}}`,
+      expected: `two wildcards inside the key {{step.${randomStepID}.*.stringProp.*}}`,
     },
-  ])('should handle $testDescription', ({ params, expected }) => {
+  ])('should $testDescription', ({ params, expected }) => {
     const executionSteps = [
       {
         stepId: randomStepID,
@@ -418,8 +413,6 @@ describe('compute parameters', () => {
               numberProp: 789,
             },
           ],
-          'stringWith*': 'string value',
-          'string with *': 'string value 2',
         },
       } as unknown as ExecutionStep,
     ]

packages/backend/src/helpers/check-step-parameters.ts

@@ -1,7 +1,7 @@
 import { IJSONObject } from '@plumber/types'
 
-const VARIABLE_REGEX =
-  /({{step\.[\da-f]{8}-(?:[\da-f]{4}-){3}[\da-f]{12}(?:\.[\w* -]+)+}})/
+import { VARIABLE_REGEX } from './compute-parameters'
+
 const GLOBAL_VARIABLE_REGEX = new RegExp(VARIABLE_REGEX, 'g')
 
 /**

packages/backend/src/helpers/compute-parameters.ts

@@ -9,11 +9,31 @@ import Step from '../models/step'
 
 const GET_ALL_SEPARATOR = ','
 
-const variableRegExp =
-  /({{step\.[\da-f]{8}-(?:[\da-f]{4}-){3}[\da-f]{12}(?:\.[\w* -]+)+}})/g
+export const VARIABLE_REGEX =
+  /({{step\.[\da-f]{8}-(?:[\da-f]{4}-){3}[\da-f]{12}(?:\.(?:[\w -]+|\*))+}})/
+
+function isVariableValid(path: string) {
+  const rawPath = path.replace(/^{{step\.[^}]+?\./, '').replace(/}}$/, '')
+  const segments = rawPath.split('.')
+  const wildcardCount = segments.filter((s) => s === '*').length
+
+  if (
+    wildcardCount > 1 ||
+    segments[0] === '*' ||
+    segments[segments.length - 1] === '*'
+  ) {
+    return false
+  }
+
+  return true
+}
 
 function splitAndJoinAroundWildcard(arr: string[]) {
   const wildcardIndex = arr.indexOf('*')
+  if (wildcardIndex === -1) {
+    console.error("Wildcard '*' not found in array")
+    return { before: arr.join('.'), after: '' }
+  }
   return {
     before: arr.slice(0, wildcardIndex).join('.'),
     after: arr.slice(wildcardIndex + 1).join('.'),
@@ -59,11 +79,15 @@ function findAndSubstituteVariables(
     return rawValue
   }
 
-  const parts = rawValue.split(variableRegExp)
+  const parts = rawValue.split(VARIABLE_REGEX)
 
   return parts
     .map((part: string) => {
-      const isVariable = part.match(variableRegExp)
+      if (!isVariableValid(part)) {
+        return part
+      }
+
+      const isVariable = part.match(VARIABLE_REGEX)
       if (isVariable) {
         const stepIdAndKeyPath = part.replace(/{{step.|}}/g, '') as string
         const [stepId, ...keyPaths] = stepIdAndKeyPath.split('.')
@@ -80,6 +104,9 @@ function findAndSubstituteVariables(
           if (!base) {
             return ''
           }
+          if (typeof base === 'string') {
+            return base
+          }
 
           const values = after
             ? map(base as Record<string, unknown>, after)