Fix CodeQL security alert: Replace unsafe Function() with safe expression parser

Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>

Author: Copilot

packages/core/src/validation/validators/object-validation-engine.ts

@@ -78,74 +78,161 @@ export interface ValidationExpressionEvaluator {
 }
 
 /**
- * Simple expression evaluator (basic implementation)
- * In production, this should use a proper expression engine
+ * Safe expression evaluator using a simple parser (no dynamic code execution)
  * 
- * SECURITY NOTE: This implementation uses a sandboxed approach with limited
- * expression capabilities. For production use, consider:
+ * SECURITY: This implementation parses expressions into an AST and evaluates them
+ * without using eval() or new Function(). It supports:
+ * - Comparison operators: ==, !=, >, <, >=, <=
+ * - Logical operators: &&, ||, !
+ * - Property access: record.field, record['field']
+ * - Literals: true, false, null, numbers, strings
+ * 
+ * For more complex expressions, integrate a dedicated library like:
  * - JSONLogic (jsonlogic.com)
- * - expr-eval with allowlist
- * - Custom AST-based evaluator
+ * - filtrex
  */
 class SimpleExpressionEvaluator implements ValidationExpressionEvaluator {
   evaluate(expression: string, context: Record<string, any>): any {
     try {
-      // Sanitize expression: only allow basic comparisons and logical operators
-      // This is a basic safeguard - proper expression parsing should be used in production
-      const sanitizedExpression = this.sanitizeExpression(expression);
-      
-      // Create a safe evaluation context with read-only access
-      const safeContext = this.createSafeContext(context);
-      const contextKeys = Object.keys(safeContext);
-      const contextValues = Object.values(safeContext);
-      
-      // Use Function constructor with controlled input
-      const func = new Function(...contextKeys, `'use strict'; return (${sanitizedExpression});`);
-      return func(...contextValues);
+      return this.evaluateSafeExpression(expression.trim(), context);
     } catch (error) {
       console.error('Expression evaluation error:', error);
       return false;
     }
   }
 
   /**
-   * Sanitize expression to prevent code injection
+   * Safely evaluate an expression without using dynamic code execution
    */
-  private sanitizeExpression(expression: string): string {
-    // Remove potentially dangerous patterns
-    const dangerous = [
-      /require\s*\(/gi,
-      /import\s+/gi,
-      /eval\s*\(/gi,
-      /Function\s*\(/gi,
-      /constructor/gi,
-      /__proto__/gi,
-      /prototype/gi,
-    ];
-
-    for (const pattern of dangerous) {
-      if (pattern.test(expression)) {
-        throw new Error('Invalid expression: contains forbidden pattern');
+  private evaluateSafeExpression(expr: string, context: Record<string, any>): any {
+    // Handle boolean literals
+    if (expr === 'true') return true;
+    if (expr === 'false') return false;
+    if (expr === 'null') return null;
+    
+    // Handle string literals
+    if ((expr.startsWith('"') && expr.endsWith('"')) || 
+        (expr.startsWith("'") && expr.endsWith("'"))) {
+      return expr.slice(1, -1);
+    }
+    
+    // Handle numeric literals
+    if (/^-?\d+(\.\d+)?$/.test(expr)) {
+      return parseFloat(expr);
+    }
+    
+    // Handle logical NOT
+    if (expr.startsWith('!')) {
+      return !this.evaluateSafeExpression(expr.slice(1).trim(), context);
+    }
+    
+    // Handle logical AND
+    if (expr.includes('&&')) {
+      const parts = this.splitOnOperator(expr, '&&');
+      return parts.every(part => this.evaluateSafeExpression(part, context));
+    }
+    
+    // Handle logical OR
+    if (expr.includes('||')) {
+      const parts = this.splitOnOperator(expr, '||');
+      return parts.some(part => this.evaluateSafeExpression(part, context));
+    }
+    
+    // Handle comparison operators
+    const comparisonMatch = expr.match(/^(.+?)\s*(===|!==|==|!=|>=|<=|>|<)\s*(.+)$/);
+    if (comparisonMatch) {
+      const [, left, op, right] = comparisonMatch;
+      const leftVal = this.evaluateSafeExpression(left.trim(), context);
+      const rightVal = this.evaluateSafeExpression(right.trim(), context);
+      
+      switch (op) {
+        case '===':
+        case '==': return leftVal == rightVal;
+        case '!==':
+        case '!=': return leftVal != rightVal;
+        case '>': return leftVal > rightVal;
+        case '<': return leftVal < rightVal;
+        case '>=': return leftVal >= rightVal;
+        case '<=': return leftVal <= rightVal;
+        default: return false;
       }
     }
+    
+    // Handle property access (e.g., record.field or context.field)
+    return this.getValueFromContext(expr, context);
+  }
 
-    return expression;
+  /**
+   * Split expression on operator, respecting parentheses and quotes
+   */
+  private splitOnOperator(expr: string, operator: string): string[] {
+    const parts: string[] = [];
+    let current = '';
+    let depth = 0;
+    let inString = false;
+    let stringChar = '';
+    
+    for (let i = 0; i < expr.length; i++) {
+      const char = expr[i];
+      const nextChar = expr[i + 1];
+      
+      if ((char === '"' || char === "'") && !inString) {
+        inString = true;
+        stringChar = char;
+      } else if (char === stringChar && inString) {
+        inString = false;
+      }
+      
+      if (!inString) {
+        if (char === '(') depth++;
+        if (char === ')') depth--;
+        
+        if (depth === 0 && char === operator[0] && nextChar === operator[1]) {
+          parts.push(current.trim());
+          current = '';
+          i++; // Skip next character
+          continue;
+        }
+      }
+      
+      current += char;
+    }
+    
+    if (current) {
+      parts.push(current.trim());
+    }
+    
+    return parts;
   }
 
   /**
-   * Create a safe read-only context
+   * Get value from context by path (e.g., "record.age" or "age")
    */
-  private createSafeContext(context: Record<string, any>): Record<string, any> {
-    const safe: Record<string, any> = {};
-    for (const [key, value] of Object.entries(context)) {
-      // Deep clone primitive values and objects to prevent mutation
-      if (typeof value === 'object' && value !== null) {
-        safe[key] = JSON.parse(JSON.stringify(value));
+  private getValueFromContext(path: string, context: Record<string, any>): any {
+    // Handle bracket notation: record['field']
+    const bracketMatch = path.match(/^(\w+)\['([^']+)'\]$/);
+    if (bracketMatch) {
+      const [, obj, field] = bracketMatch;
+      return context[obj]?.[field];
+    }
+    
+    // Handle dot notation: record.field or just field
+    const parts = path.split('.');
+    let value: any = context;
+    
+    for (const part of parts) {
+      if (value && typeof value === 'object' && part in value) {
+        value = value[part];
       } else {
-        safe[key] = value;
+        // Try direct context access for simple identifiers
+        if (parts.length === 1 && part in context) {
+          return context[part];
+        }
+        return undefined;
       }
     }
-    return safe;
+    
+    return value;
   }
 }