improve condition sanitization

lib/condition-validator.ts

@@ -0,0 +1,297 @@
+/**
+ * Condition Expression Validator
+ *
+ * Validates and sanitizes condition expressions before evaluation.
+ * This prevents arbitrary code execution while allowing useful comparisons.
+ *
+ * Allowed syntax:
+ * - Template variables: {{@nodeId:Label.field}} (replaced with safe __v0, __v1, etc.)
+ * - Comparison operators: ===, !==, ==, !=, >, <, >=, <=
+ * - Logical operators: &&, ||, !
+ * - Grouping: ( )
+ * - Literals: strings ('...', "..."), numbers, true, false, null, undefined
+ * - Property access on variables: __v0.property (but not arbitrary property chains)
+ * - Array methods: .includes(), .length
+ * - String methods: .startsWith(), .endsWith(), .includes()
+ *
+ * NOT allowed:
+ * - Function calls (except allowed methods)
+ * - Assignment operators (=, +=, -=, etc.)
+ * - Code execution constructs (eval, Function, import, require)
+ * - Property assignment
+ * - Comments
+ */
+
+// Dangerous patterns that should never appear in conditions
+const DANGEROUS_PATTERNS = [
+  // Assignment operators
+  /(?<![=!<>])=(?!=)/g, // = but not ==, ===, !=, !==, <=, >=
+  /\+=|-=|\*=|\/=|%=|\^=|\|=|&=/g,
+  // Code execution
+  /\beval\s*\(/gi,
+  /\bFunction\s*\(/gi,
+  /\bimport\s*\(/gi,
+  /\brequire\s*\(/gi,
+  /\bnew\s+\w/gi,
+  // Dangerous globals
+  /\bprocess\b/gi,
+  /\bglobal\b/gi,
+  /\bwindow\b/gi,
+  /\bdocument\b/gi,
+  /\bconstructor\b/gi,
+  /\b__proto__\b/gi,
+  /\bprototype\b/gi,
+  // Control flow that could be exploited
+  /\bwhile\s*\(/gi,
+  /\bfor\s*\(/gi,
+  /\bdo\s*\{/gi,
+  /\bswitch\s*\(/gi,
+  /\btry\s*\{/gi,
+  /\bcatch\s*\(/gi,
+  /\bfinally\s*\{/gi,
+  /\bthrow\s+/gi,
+  /\breturn\s+/gi,
+  // Template literals with expressions (could execute code)
+  /`[^`]*\$\{/g,
+  // Object/Array literals with computed properties
+  /\[\s*[^\]]+\s*\]/g, // Will validate separately for array access vs array literals
+  /\{\s*\w+\s*:/g, // Object literals
+  // Increment/decrement
+  /\+\+|--/g,
+  // Bitwise operators (rarely needed, often used in exploits)
+  /<<|>>|>>>/g,
+  // Comma operator (can chain expressions)
+  /,(?![^(]*\))/g, // Comma not inside function call parentheses
+  // Semicolons (statement separator)
+  /;/g,
+];
+
+// Allowed method names that can be called
+const ALLOWED_METHODS = new Set([
+  "includes",
+  "startsWith",
+  "endsWith",
+  "toString",
+  "toLowerCase",
+  "toUpperCase",
+  "trim",
+  "length", // Actually a property, but accessed like .length
+]);
+
+// Pattern to match method calls
+const METHOD_CALL_PATTERN = /\.(\w+)\s*\(/g;
+
+// Top-level regex patterns for token validation
+const WHITESPACE_SPLIT_PATTERN = /\s+/;
+const VARIABLE_TOKEN_PATTERN = /^__v\d+/;
+const STRING_TOKEN_PATTERN = /^['"]/;
+const NUMBER_TOKEN_PATTERN = /^\d/;
+const LITERAL_TOKEN_PATTERN = /^(true|false|null|undefined)$/;
+const OPERATOR_TOKEN_PATTERN = /^(===|!==|==|!=|>=|<=|>|<|&&|\|\||!|\(|\))$/;
+const IDENTIFIER_TOKEN_PATTERN = /^[a-zA-Z_]\w*$/;
+
+export type ValidationResult =
+  | { valid: true }
+  | { valid: false; error: string };
+
+/**
+ * Check for dangerous patterns in the expression
+ */
+function checkDangerousPatterns(expression: string): ValidationResult {
+  for (const pattern of DANGEROUS_PATTERNS) {
+    // Reset regex state
+    pattern.lastIndex = 0;
+    if (pattern.test(expression)) {
+      pattern.lastIndex = 0;
+      const match = expression.match(pattern);
+      return {
+        valid: false,
+        error: `Condition contains disallowed syntax: "${match?.[0] || "unknown"}"`,
+      };
+    }
+  }
+  return { valid: true };
+}
+
+/**
+ * Check that all method calls use allowed methods
+ */
+function checkMethodCalls(expression: string): ValidationResult {
+  METHOD_CALL_PATTERN.lastIndex = 0;
+  const matches = expression.matchAll(METHOD_CALL_PATTERN);
+
+  for (const match of matches) {
+    const methodName = match[1];
+    if (!ALLOWED_METHODS.has(methodName)) {
+      return {
+        valid: false,
+        error: `Method "${methodName}" is not allowed in conditions. Allowed methods: ${Array.from(ALLOWED_METHODS).join(", ")}`,
+      };
+    }
+  }
+
+  return { valid: true };
+}
+
+/**
+ * Check that parentheses are balanced
+ */
+function checkParentheses(expression: string): ValidationResult {
+  let parenDepth = 0;
+
+  for (const char of expression) {
+    if (char === "(") {
+      parenDepth += 1;
+    }
+    if (char === ")") {
+      parenDepth -= 1;
+    }
+    if (parenDepth < 0) {
+      return { valid: false, error: "Unbalanced parentheses in condition" };
+    }
+  }
+
+  if (parenDepth !== 0) {
+    return { valid: false, error: "Unbalanced parentheses in condition" };
+  }
+
+  return { valid: true };
+}
+
+/**
+ * Check if a token is valid
+ */
+function isValidToken(token: string): boolean {
+  // Skip known valid patterns
+  if (VARIABLE_TOKEN_PATTERN.test(token)) {
+    return true;
+  }
+  if (STRING_TOKEN_PATTERN.test(token)) {
+    return true;
+  }
+  if (NUMBER_TOKEN_PATTERN.test(token)) {
+    return true;
+  }
+  if (LITERAL_TOKEN_PATTERN.test(token)) {
+    return true;
+  }
+  if (OPERATOR_TOKEN_PATTERN.test(token)) {
+    return true;
+  }
+  return false;
+}
+
+/**
+ * Check for unauthorized identifiers in the expression
+ */
+function checkUnauthorizedIdentifiers(expression: string): ValidationResult {
+  const tokens = expression.split(WHITESPACE_SPLIT_PATTERN).filter(Boolean);
+
+  for (const token of tokens) {
+    if (isValidToken(token)) {
+      continue;
+    }
+
+    // Check if it looks like an unauthorized identifier
+    if (IDENTIFIER_TOKEN_PATTERN.test(token) && !token.startsWith("__v")) {
+      return {
+        valid: false,
+        error: `Unknown identifier "${token}" in condition. Use template variables like {{@nodeId:Label.field}} to reference workflow data.`,
+      };
+    }
+  }
+
+  return { valid: true };
+}
+
+/**
+ * Validate a condition expression after template variables have been replaced
+ *
+ * @param expression - The expression with template vars replaced (e.g., "__v0 === 'test'")
+ * @returns ValidationResult indicating if the expression is safe to evaluate
+ */
+export function validateConditionExpression(
+  expression: string
+): ValidationResult {
+  // Empty expressions are invalid
+  if (!expression || expression.trim() === "") {
+    return { valid: false, error: "Condition expression cannot be empty" };
+  }
+
+  // Check for dangerous patterns
+  const dangerousCheck = checkDangerousPatterns(expression);
+  if (!dangerousCheck.valid) {
+    return dangerousCheck;
+  }
+
+  // Check method calls are whitelisted
+  const methodCheck = checkMethodCalls(expression);
+  if (!methodCheck.valid) {
+    return methodCheck;
+  }
+
+  // Validate balanced parentheses
+  const parenCheck = checkParentheses(expression);
+  if (!parenCheck.valid) {
+    return parenCheck;
+  }
+
+  // Check for unauthorized identifiers
+  const identifierCheck = checkUnauthorizedIdentifiers(expression);
+  if (!identifierCheck.valid) {
+    return identifierCheck;
+  }
+
+  return { valid: true };
+}
+
+/**
+ * Check if a raw expression (before template replacement) looks safe
+ * This is a quick pre-check before the more thorough validation
+ */
+export function preValidateConditionExpression(
+  expression: string
+): ValidationResult {
+  if (!expression || typeof expression !== "string") {
+    return { valid: false, error: "Condition must be a non-empty string" };
+  }
+
+  // Check for obviously dangerous patterns before any processing
+  const dangerousKeywords = [
+    "eval",
+    "Function",
+    "import",
+    "require",
+    "process",
+    "global",
+    "window",
+    "document",
+    "__proto__",
+    "constructor",
+    "prototype",
+  ];
+
+  const lowerExpression = expression.toLowerCase();
+  for (const keyword of dangerousKeywords) {
+    if (lowerExpression.includes(keyword.toLowerCase())) {
+      return {
+        valid: false,
+        error: `Condition contains disallowed keyword: "${keyword}"`,
+      };
+    }
+  }
+
+  return { valid: true };
+}
+
+/**
+ * Sanitize an expression by escaping potentially dangerous characters
+ * This is used as an additional safety measure
+ */
+export function sanitizeForDisplay(expression: string): string {
+  return expression
+    .replace(/</g, "&lt;")
+    .replace(/>/g, "&gt;")
+    .replace(/"/g, "&quot;")
+    .replace(/'/g, "&#39;");
+}

lib/workflow-executor.workflow.ts

@@ -3,6 +3,10 @@
  * This executor captures step executions through the workflow SDK for better observability
  */
 
+import {
+  preValidateConditionExpression,
+  validateConditionExpression,
+} from "@/lib/condition-validator";
 import { getStepImporter, type StepImporter } from "./step-registry";
 import type { StepContext } from "./steps/step-handler";
 import { triggerStep } from "./steps/trigger";
@@ -100,6 +104,9 @@ function replaceTemplateVariable(
 /**
  * Evaluate condition expression with template variable replacement
  * Uses Function constructor to evaluate user-defined conditions dynamically
+ *
+ * Security: Expressions are validated before evaluation to prevent code injection.
+ * Only comparison operators, logical operators, and whitelisted methods are allowed.
  */
 function evaluateConditionExpression(
   conditionExpression: unknown,
@@ -112,6 +119,14 @@ function evaluateConditionExpression(
   }
 
   if (typeof conditionExpression === "string") {
+    // Pre-validate the expression before any processing
+    const preValidation = preValidateConditionExpression(conditionExpression);
+    if (!preValidation.valid) {
+      console.error("[Condition] Pre-validation failed:", preValidation.error);
+      console.error("[Condition] Expression was:", conditionExpression);
+      return false;
+    }
+
     try {
       const evalContext: Record<string, unknown> = {};
       let transformedExpression = conditionExpression;
@@ -131,11 +146,23 @@ function evaluateConditionExpression(
           )
       );
 
+      // Validate the transformed expression before evaluation
+      const validation = validateConditionExpression(transformedExpression);
+      if (!validation.valid) {
+        console.error("[Condition] Validation failed:", validation.error);
+        console.error("[Condition] Original expression:", conditionExpression);
+        console.error(
+          "[Condition] Transformed expression:",
+          transformedExpression
+        );
+        return false;
+      }
+
       const varNames = Object.keys(evalContext);
       const varValues = Object.values(evalContext);
 
-      // Note: Function constructor is used here to evaluate user-defined workflow conditions
-      // This is an intentional feature for dynamic condition evaluation in workflows
+      // Safe to evaluate - expression has been validated
+      // Only contains: variables (__v0, __v1), operators, literals, and whitelisted methods
       const evalFunc = new Function(
         ...varNames,
         `return (${transformedExpression});`