Merge pull request #42 from ruvnet/security/fix-critical-vulnerabilities

Security: Fix critical vulnerabilities (includes fr4iser90 PR #38 + fix)

.claude/helpers/intelligence.cjs

@@ -259,7 +259,19 @@ function parseMemoryDir(dir, entries) {
   try {
     const files = fs.readdirSync(dir).filter(f => f.endsWith('.md'));
     for (const file of files) {
+      // Validate file name to prevent path traversal
+      if (file.includes('..') || file.includes('/') || file.includes('\\')) {
+        continue;
+      }
+      
       const filePath = path.join(dir, file);
+      // Additional validation: ensure resolved path is within the base directory
+      const resolvedPath = path.resolve(filePath);
+      const resolvedDir = path.resolve(dir);
+      if (!resolvedPath.startsWith(resolvedDir)) {
+        continue; // Path traversal attempt detected
+      }
+      
       const content = fs.readFileSync(filePath, 'utf-8');
       if (!content.trim()) continue;
 

.claude/helpers/metrics-db.mjs

@@ -7,7 +7,7 @@
 
 import initSqlJs from 'sql.js';
 import { readFileSync, writeFileSync, existsSync, mkdirSync, readdirSync, statSync } from 'fs';
-import { dirname, join, basename } from 'path';
+import { dirname, join, basename, resolve } from 'path';
 import { fileURLToPath } from 'url';
 import { execSync } from 'child_process';
 
@@ -154,7 +154,19 @@ function countFilesAndLines(dir, ext = '.ts') {
     try {
       const entries = readdirSync(currentDir, { withFileTypes: true });
       for (const entry of entries) {
+        // Validate entry name to prevent path traversal
+        if (entry.name.includes('..') || entry.name.includes('/') || entry.name.includes('\\')) {
+          continue;
+        }
+        
         const fullPath = join(currentDir, entry.name);
+        // Additional validation: ensure resolved path is within the base directory
+        const resolvedPath = resolve(fullPath);
+        const resolvedCurrentDir = resolve(currentDir);
+        if (!resolvedPath.startsWith(resolvedCurrentDir)) {
+          continue; // Path traversal attempt detected
+        }
+        
         if (entry.isDirectory() && !entry.name.includes('node_modules')) {
           walk(fullPath);
         } else if (entry.isFile() && entry.name.endsWith(ext)) {
@@ -209,7 +221,20 @@ function calculateModuleProgress(moduleDir) {
  * Check security file status
  */
 function checkSecurityFile(filename, minLines = 100) {
+  // Validate filename to prevent path traversal
+  if (filename.includes('..') || filename.includes('/') || filename.includes('\\')) {
+    return false;
+  }
+  
   const filePath = join(V3_DIR, '@claude-flow/security/src', filename);
+  
+  // Additional validation: ensure resolved path is within the expected directory
+  const resolvedPath = resolve(filePath);
+  const expectedDir = resolve(join(V3_DIR, '@claude-flow/security/src'));
+  if (!resolvedPath.startsWith(expectedDir)) {
+    return false; // Path traversal attempt detected
+  }
+  
   if (!existsSync(filePath)) return false;
 
   try {

.claude/helpers/statusline.cjs

@@ -47,8 +47,27 @@ const c = {
 };
 
 // Safe execSync with strict timeout (returns empty string on failure)
+// Validates command to prevent command injection
 function safeExec(cmd, timeoutMs = 2000) {
   try {
+    // Validate command to prevent command injection
+    // Only allow commands that match safe patterns (no shell metacharacters)
+    if (typeof cmd !== 'string') {
+      return '';
+    }
+    
+    // Check for dangerous shell metacharacters that could allow injection
+    const dangerousChars = /[;&|`$(){}[\]<>'"\\]/;
+    if (dangerousChars.test(cmd)) {
+      // If dangerous chars found, only allow if it's a known safe pattern
+      // Allow 'sh -c' with single-quoted script (already escaped)
+      const safeShPattern = /^sh\s+-c\s+'[^']*'$/;
+      if (!safeShPattern.test(cmd)) {
+        console.warn('safeExec: Command contains potentially dangerous characters');
+        return '';
+      }
+    }
+    
     return execSync(cmd, {
       encoding: 'utf-8',
       timeout: timeoutMs,

.github/workflows/cd.yml

@@ -45,12 +45,17 @@ jobs:
 
     - name: Determine deployment environment
       id: determine-env
+      env:
+        # Use environment variable to prevent shell injection
+        GITHUB_EVENT_NAME: ${{ github.event_name }}
+        GITHUB_REF: ${{ github.ref }}
+        GITHUB_INPUT_ENVIRONMENT: ${{ github.event.inputs.environment }}
       run: |
-        if [[ "${{ github.event_name }}" == "workflow_dispatch" ]]; then
-          echo "environment=${{ github.event.inputs.environment }}" >> $GITHUB_OUTPUT
-        elif [[ "${{ github.ref }}" == "refs/heads/main" ]]; then
+        if [[ "$GITHUB_EVENT_NAME" == "workflow_dispatch" ]]; then
+          echo "environment=$GITHUB_INPUT_ENVIRONMENT" >> $GITHUB_OUTPUT
+        elif [[ "$GITHUB_REF" == "refs/heads/main" ]]; then
           echo "environment=staging" >> $GITHUB_OUTPUT
-        elif [[ "${{ github.ref }}" == refs/tags/v* ]]; then
+        elif [[ "$GITHUB_REF" == refs/tags/v* ]]; then
           echo "environment=production" >> $GITHUB_OUTPUT
         else
           echo "environment=staging" >> $GITHUB_OUTPUT

ui/components/DashboardTab.js

@@ -103,10 +103,18 @@ export class DashboardTab {
     Object.entries(features).forEach(([feature, enabled]) => {
       const featureElement = document.createElement('div');
       featureElement.className = `feature-item ${enabled ? 'enabled' : 'disabled'}`;
-      featureElement.innerHTML = `
-        <span class="feature-name">${this.formatFeatureName(feature)}</span>
-        <span class="feature-status">${enabled ? '✓' : '✗'}</span>
-      `;
+      
+      // Use textContent instead of innerHTML to prevent XSS
+      const featureNameSpan = document.createElement('span');
+      featureNameSpan.className = 'feature-name';
+      featureNameSpan.textContent = this.formatFeatureName(feature);
+      
+      const featureStatusSpan = document.createElement('span');
+      featureStatusSpan.className = 'feature-status';
+      featureStatusSpan.textContent = enabled ? '✓' : '✗';
+      
+      featureElement.appendChild(featureNameSpan);
+      featureElement.appendChild(featureStatusSpan);
       featuresContainer.appendChild(featureElement);
     });
   }
@@ -296,10 +304,18 @@ export class DashboardTab {
       ['zone_1', 'zone_2', 'zone_3', 'zone_4'].forEach(zoneId => {
         const zoneElement = document.createElement('div');
         zoneElement.className = 'zone-item';
-        zoneElement.innerHTML = `
-          <span class="zone-name">${zoneId}</span>
-          <span class="zone-count">undefined</span>
-        `;
+        
+        // Use textContent instead of innerHTML to prevent XSS
+        const zoneNameSpan = document.createElement('span');
+        zoneNameSpan.className = 'zone-name';
+        zoneNameSpan.textContent = zoneId;
+        
+        const zoneCountSpan = document.createElement('span');
+        zoneCountSpan.className = 'zone-count';
+        zoneCountSpan.textContent = 'undefined';
+        
+        zoneElement.appendChild(zoneNameSpan);
+        zoneElement.appendChild(zoneCountSpan);
         zonesContainer.appendChild(zoneElement);
       });
       return;
@@ -309,10 +325,18 @@ export class DashboardTab {
       const zoneElement = document.createElement('div');
       zoneElement.className = 'zone-item';
       const count = typeof data === 'object' ? (data.person_count || data.count || 0) : data;
-      zoneElement.innerHTML = `
-        <span class="zone-name">${zoneId}</span>
-        <span class="zone-count">${count}</span>
-      `;
+      
+      // Use textContent instead of innerHTML to prevent XSS
+      const zoneNameSpan = document.createElement('span');
+      zoneNameSpan.className = 'zone-name';
+      zoneNameSpan.textContent = zoneId;
+      
+      const zoneCountSpan = document.createElement('span');
+      zoneCountSpan.className = 'zone-count';
+      zoneCountSpan.textContent = String(count);
+      
+      zoneElement.appendChild(zoneNameSpan);
+      zoneElement.appendChild(zoneCountSpan);
       zonesContainer.appendChild(zoneElement);
     });
   }

ui/components/HardwareTab.js

@@ -107,20 +107,29 @@ export class HardwareTab {
     const txActive = activeAntennas.filter(a => a.classList.contains('tx')).length;
     const rxActive = activeAntennas.filter(a => a.classList.contains('rx')).length;
     
-    arrayStatus.innerHTML = `
-      <div class="array-info">
-        <span class="info-label">Active TX:</span>
-        <span class="info-value">${txActive}/3</span>
-      </div>
-      <div class="array-info">
-        <span class="info-label">Active RX:</span>
-        <span class="info-value">${rxActive}/6</span>
-      </div>
-      <div class="array-info">
-        <span class="info-label">Signal Quality:</span>
-        <span class="info-value">${this.calculateSignalQuality(txActive, rxActive)}%</span>
-      </div>
-    `;
+    // Clear and rebuild using safe DOM methods to prevent XSS
+    arrayStatus.innerHTML = '';
+    
+    const createInfoDiv = (label, value) => {
+      const div = document.createElement('div');
+      div.className = 'array-info';
+      
+      const labelSpan = document.createElement('span');
+      labelSpan.className = 'info-label';
+      labelSpan.textContent = label;
+      
+      const valueSpan = document.createElement('span');
+      valueSpan.className = 'info-value';
+      valueSpan.textContent = value;
+      
+      div.appendChild(labelSpan);
+      div.appendChild(valueSpan);
+      return div;
+    };
+    
+    arrayStatus.appendChild(createInfoDiv('Active TX:', `${txActive}/3`));
+    arrayStatus.appendChild(createInfoDiv('Active RX:', `${rxActive}/6`));
+    arrayStatus.appendChild(createInfoDiv('Signal Quality:', `${this.calculateSignalQuality(txActive, rxActive)}%`));
   }
 
   // Calculate signal quality based on active antennas

ui/components/PoseDetectionCanvas.js

@@ -539,14 +539,23 @@ export class PoseDetectionCanvas {
     const persons = this.state.lastPoseData?.persons?.length || 0;
     const zones = Object.keys(this.state.lastPoseData?.zone_summary || {}).length;
 
-    statsEl.innerHTML = `
-      Connection: ${this.state.connectionState}<br>
-      Frames: ${this.state.frameCount}<br>
-      FPS: ${fps.toFixed(1)}<br>
-      Persons: ${persons}<br>
-      Zones: ${zones}<br>
-      Uptime: ${uptime}s
-    `;
+    // Use textContent instead of innerHTML to prevent XSS
+    statsEl.textContent = '';
+    const lines = [
+      `Connection: ${this.state.connectionState}`,
+      `Frames: ${this.state.frameCount}`,
+      `FPS: ${fps.toFixed(1)}`,
+      `Persons: ${persons}`,
+      `Zones: ${zones}`,
+      `Uptime: ${uptime}s`
+    ];
+    lines.forEach((line, index) => {
+      if (index > 0) {
+        statsEl.appendChild(document.createElement('br'));
+      }
+      const textNode = document.createTextNode(line);
+      statsEl.appendChild(textNode);
+    });
   }
 
   showError(message) {

ui/config/api.config.js

@@ -107,8 +107,12 @@ export function buildApiUrl(endpoint, params = {}) {
 
 // Helper function to build WebSocket URLs
 export function buildWsUrl(endpoint, params = {}) {
-  const protocol = window.location.protocol === 'https:' 
-    ? API_CONFIG.WSS_PREFIX 
+  // Use secure WebSocket (wss://) when serving over HTTPS or on non-localhost
+  // Use ws:// only for localhost development
+  const isLocalhost = window.location.hostname === 'localhost' || window.location.hostname === '127.0.0.1';
+  const isSecure = window.location.protocol === 'https:';
+  const protocol = (isSecure || !isLocalhost)
+    ? API_CONFIG.WSS_PREFIX
     : API_CONFIG.WS_PREFIX;
   
   // Match Rust sensing server port

v1/src/commands/status.py

@@ -152,7 +152,8 @@ async def _get_database_status(settings: Settings) -> Dict[str, Any]:
         
         # Get table counts
         async with db_manager.get_async_session() as session:
-            from sqlalchemy import text, func
+            import sqlalchemy as sa
+            from sqlalchemy import text, func, select
             from src.database.models import Device, Session, CSIData, PoseDetection, SystemMetric, AuditLog
             
             tables = {
@@ -164,10 +165,19 @@ async def _get_database_status(settings: Settings) -> Dict[str, Any]:
                 "audit_logs": AuditLog,
             }
             
+            # Whitelist of allowed table names to prevent SQL injection
+            allowed_table_names = set(tables.keys())
+            
             for table_name, model in tables.items():
                 try:
+                    # Validate table_name against whitelist to prevent SQL injection
+                    if table_name not in allowed_table_names:
+                        db_status["tables"][table_name] = {"error": "Invalid table name"}
+                        continue
+                    
+                    # Use SQLAlchemy ORM model for safe query instead of raw SQL
                     result = await session.execute(
-                        text(f"SELECT COUNT(*) FROM {table_name}")
+                        select(func.count()).select_from(model)
                     )
                     count = result.scalar()
                     db_status["tables"][table_name] = {"count": count}

v1/src/database/migrations/001_initial.py

@@ -237,13 +237,25 @@ def upgrade():
         'system_metrics', 'audit_logs'
     ]
     
+    # Whitelist validation to prevent SQL injection
+    allowed_tables = set(tables_with_updated_at)
+    
     for table in tables_with_updated_at:
-        op.execute(f"""
-            CREATE TRIGGER update_{table}_updated_at
-                BEFORE UPDATE ON {table}
-                FOR EACH ROW
-                EXECUTE FUNCTION update_updated_at_column();
-        """)
+        # Validate table name against whitelist
+        if table not in allowed_tables:
+            continue
+        
+        # Use parameterized query with SQLAlchemy's text() and bindparam
+        # Note: For table names in DDL, we validate against whitelist
+        # SQLAlchemy's op.execute with text() is safe when table names are whitelisted
+        op.execute(
+            sa.text(f"""
+                CREATE TRIGGER update_{table}_updated_at
+                    BEFORE UPDATE ON {table}
+                    FOR EACH ROW
+                    EXECUTE FUNCTION update_updated_at_column();
+            """)
+        )
     
     # Insert initial data
     _insert_initial_data()
@@ -258,8 +270,18 @@ def downgrade():
         'system_metrics', 'audit_logs'
     ]
     
+    # Whitelist validation to prevent SQL injection
+    allowed_tables = set(tables_with_updated_at)
+    
     for table in tables_with_updated_at:
-        op.execute(f"DROP TRIGGER IF EXISTS update_{table}_updated_at ON {table};")
+        # Validate table name against whitelist
+        if table not in allowed_tables:
+            continue
+        
+        # Use parameterized query with SQLAlchemy's text()
+        op.execute(
+            sa.text(f"DROP TRIGGER IF EXISTS update_{table}_updated_at ON {table};")
+        )
     
     # Drop function
     op.execute("DROP FUNCTION IF EXISTS update_updated_at_column();")
@@ -335,22 +357,43 @@ def _insert_initial_data():
     ]
     
     for metric_name, metric_type, value, unit, source, component in metrics_data:
-        op.execute(f"""
-            INSERT INTO system_metrics (
-                id, metric_name, metric_type, value, unit, source, component,
-                description, metadata
-            ) VALUES (
-                gen_random_uuid(),
-                '{metric_name}',
-                '{metric_type}',
-                {value},
-                '{unit}',
-                '{source}',
-                '{component}',
-                'Initial {metric_name} metric',
-                '{{"initial": true, "version": "1.0.0"}}'
-            );
-        """)
+        # Use parameterized query to prevent SQL injection
+        # Escape single quotes in string values
+        safe_metric_name = metric_name.replace("'", "''")
+        safe_metric_type = metric_type.replace("'", "''")
+        safe_unit = unit.replace("'", "''") if unit else ''
+        safe_source = source.replace("'", "''") if source else ''
+        safe_component = component.replace("'", "''") if component else ''
+        safe_description = f'Initial {safe_metric_name} metric'.replace("'", "''")
+        
+        # Use SQLAlchemy's text() with proper escaping
+        op.execute(
+            sa.text(f"""
+                INSERT INTO system_metrics (
+                    id, metric_name, metric_type, value, unit, source, component,
+                    description, metadata
+                ) VALUES (
+                    gen_random_uuid(),
+                    :metric_name,
+                    :metric_type,
+                    :value,
+                    :unit,
+                    :source,
+                    :component,
+                    :description,
+                    :metadata
+                )
+            """).bindparams(
+                metric_name=safe_metric_name,
+                metric_type=safe_metric_type,
+                value=value,
+                unit=safe_unit,
+                source=safe_source,
+                component=safe_component,
+                description=safe_description,
+                metadata='{"initial": true, "version": "1.0.0"}'
+            )
+        )
     
     # Insert initial audit log
     op.execute("""