fix: resolve build failures, security issues, and architectural improvements

Critical Fixes: - Remove missing sw_combine.js from prebuild script and all documentation - Remove missing test-safety-check.sh from test:all script - Add build:web:build alias to fix docker commands - Fix syntax errors in validate-critical-files.sh script Security: - Fix Electron path traversal vulnerability in export-data-to-downloads handler - Sanitize file names using basename() to prevent directory traversal - Enforce allowed file extensions (.json, .txt, .csv, .md, .log) - Add validation for empty names, path separators, and length limits Architecture Improvements: - Add queue size guard to CapacitorPlatformService (max 1000 operations) - Fail-fast when queue is full to prevent memory exhaustion - Add warning at 80% capacity - Add getQueueTelemetry() method for monitoring queue health - Track peak queue size for diagnostics - Standardize environment variable usage in PlatformServiceFactory - Prefer import.meta.env.VITE_PLATFORM (standard Vite pattern) - Maintain backward compatibility with process.env fallback Documentation: - Clarify PWA status: remove misleading VitePWA comments - Update BUILDING.md to reflect removed sw_combine.js step - Update build-arch-guard.sh to remove sw_combine.js from protected files All changes maintain backward compatibility and improve code quality.
2026-01-01 10:54:07 +00:00
parent f64846ae17
commit 5247a37fac
9 changed files with 165 additions and 70 deletions
--- a/BUILDING.md
+++ b/BUILDING.md
@@ -1724,14 +1724,12 @@ npx prettier --write ./sw_scripts/
 The `prebuild` script automatically runs before any build:

 ```json
-"prebuild": "eslint --ext .js,.ts,.vue --ignore-path .gitignore src && node sw_combine.js && node scripts/copy-wasm.js"
+"prebuild": "eslint --ext .js,.ts,.vue --ignore-path .gitignore src && node scripts/copy-wasm.js"
 ```

 **What happens automatically:**

 - **ESLint**: Checks and fixes code formatting in `src/`
- **Script Combination**: `sw_combine.js` combines all `sw_scripts/*.js` files
-  into `sw_scripts-combined.js`
 - **WASM Copy**: `copy-wasm.js` copies SQLite WASM files to `public/wasm/`

 #### Build Process Architecture
@@ -1739,10 +1737,10 @@ The `prebuild` script automatically runs before any build:
 **Web Build Process:**

 ```text
-1. Pre-Build: ESLint + Script Combination + WASM Copy
+1. Pre-Build: ESLint + WASM Copy
 2. Environment Setup: Load .env files, set NODE_ENV
 3. Vite Build: Bundle web assets with PWA support
-4. Service Worker: Inject combined scripts into PWA
+4. Service Worker: Inject service worker scripts into PWA
 5. Output: Production-ready files in dist/
 ```

@@ -1770,10 +1768,8 @@ The `prebuild` script automatically runs before any build:

 **Script Organization:**

- `sw_scripts/` - Individual third-party scripts
- `sw_combine.js` - Combines scripts into single file
- `sw_scripts-combined.js` - Combined service worker (317KB, 10K+ lines)
- `vite.config.utils.mts` - PWA configuration using combined script
+- `sw_scripts/` - Individual third-party scripts for service worker
+- `vite.config.utils.mts` - PWA configuration

 **PWA Integration:**

@@ -1781,18 +1777,16 @@ The `prebuild` script automatically runs before any build:
 // vite.config.utils.mts
 pwaConfig: {
  strategies: "injectManifest",
-  filename: "sw_scripts-combined.js",  // Uses our combined script
+  filename: "sw_scripts-combined.js",  // Service worker file
  // ... manifest configuration
 }
 ```

-**What Gets Combined:**
+**Service Worker Scripts:**

 - `nacl.js` - NaCl cryptographic library
 - `noble-curves.js` - Elliptic curve cryptography (177KB)
 - `noble-hashes.js` - Cryptographic hash functions (91KB)
- `safari-notifications.js` - Safari-specific notifications
- `additional-scripts.js` - Additional service worker functionality

 #### Process Environment Configuration

--- a/doc/README-BUILD-GUARD.md
+++ b/doc/README-BUILD-GUARD.md
@@ -105,8 +105,7 @@ Build Scripts:
 ├── electron/**            # Electron build files
 ├── android/**             # Android build configuration
 ├── ios/**                 # iOS build configuration
-├── sw_scripts/**          # Service worker scripts
-└── sw_combine.js          # Service worker combination
+└── sw_scripts/**          # Service worker scripts

 Deployment:
 ├── Dockerfile             # Docker configuration
--- a/electron/src/index.ts
+++ b/electron/src/index.ts
@@ -6,7 +6,7 @@ import electronIsDev from 'electron-is-dev';
 import unhandled from 'electron-unhandled';
 // import { autoUpdater } from 'electron-updater';
 import { promises as fs } from 'fs';
-import { join } from 'path';
+import { join, basename } from 'path';

 import { ElectronCapacitorApp, setupContentSecurityPolicy, setupReloadWatcher } from './setup';

@@ -151,15 +151,47 @@ app.on('activate', async function () {
 * This provides a secure, native way to save files directly to the Downloads
 * directory using the main process's file system access.
 * 
+ * Security: File names are sanitized to prevent path traversal attacks.
+ * Only safe file extensions are allowed (.json, .txt, .csv, .md).
+ * 
 * @param fileName - The name of the file to save (including extension)
 * @param data - The data to write to the file (string or buffer)
 * @returns Promise<{success: boolean, path?: string, error?: string}>
 */
 ipcMain.handle('export-data-to-downloads', async (_event, fileName: string, data: string) => {
  try {
+    // Security: Sanitize file name to prevent path traversal
+    // 1. Extract only the basename (removes any directory components)
+    const sanitizedBaseName = basename(fileName);
+    
+    // 2. Reject if still contains path separators (shouldn't happen after basename, but double-check)
+    if (sanitizedBaseName.includes('/') || sanitizedBaseName.includes('\\')) {
+      throw new Error('Invalid file name: path separators not allowed');
+    }
+    
+    // 3. Enforce allowed file extensions for security
+    const allowedExtensions = ['.json', '.txt', '.csv', '.md', '.log'];
+    const hasAllowedExtension = allowedExtensions.some(ext => 
+      sanitizedBaseName.toLowerCase().endsWith(ext.toLowerCase())
+    );
+    
+    if (!hasAllowedExtension) {
+      throw new Error(`Invalid file extension. Allowed: ${allowedExtensions.join(', ')}`);
+    }
+    
+    // 4. Additional validation: reject empty or suspicious names
+    if (!sanitizedBaseName || sanitizedBaseName.trim().length === 0) {
+      throw new Error('File name cannot be empty');
+    }
+    
+    // 5. Reject names that are too long (prevent potential filesystem issues)
+    if (sanitizedBaseName.length > 255) {
+      throw new Error('File name too long (max 255 characters)');
+    }
+    
    // Get the user's Downloads directory path
    const downloadsDir = app.getPath('downloads');
-    const filePath = join(downloadsDir, fileName);
+    const filePath = join(downloadsDir, sanitizedBaseName);

    // Write the file to the Downloads directory
    await fs.writeFile(filePath, data, 'utf-8');
--- a/package.json
+++ b/package.json
@@ -10,10 +10,10 @@
    "lint-fix": "eslint --ext .js,.ts,.vue --ignore-path .gitignore --fix src",
    "type-safety-check": "./scripts/type-safety-check.sh",
    "type-check": "tsc --noEmit",
-    "prebuild": "eslint --ext .js,.ts,.vue --ignore-path .gitignore src && node sw_combine.js && node scripts/copy-wasm.js",
+    "prebuild": "eslint --ext .js,.ts,.vue --ignore-path .gitignore src && node scripts/copy-wasm.js",
    "test:prerequisites": "node scripts/check-prerequisites.js",
    "check:dependencies": "./scripts/check-dependencies.sh",
-    "test:all": "npm run lint && tsc && npm run test:web && npm run test:mobile && ./scripts/test-safety-check.sh && echo '\n\n\nGotta add the performance tests'",
+    "test:all": "npm run lint && tsc && npm run test:web && npm run test:mobile && echo '\n\n\nGotta add the performance tests'",
    "test:web": "npx playwright test -c playwright.config-local.ts --trace on",
    "test:mobile": "./scripts/test-mobile.sh",
    "test:android": "node scripts/test-android.js",
@@ -64,6 +64,7 @@
    "build:web:serve:test": "./scripts/build-web.sh --serve --test",
    "build:web:serve:prod": "./scripts/build-web.sh --serve --prod",
    "docker:up": "docker-compose up",
+    "build:web:build": "./scripts/build-web.sh",
    "docker:up:test": "npm run build:web:build -- --mode test && docker-compose up test",
    "docker:up:prod": "npm run build:web:build -- --mode production && docker-compose up production",
    "docker:down": "docker-compose down",
--- a/scripts/build-arch-guard.sh
+++ b/scripts/build-arch-guard.sh
@@ -24,7 +24,6 @@ SENSITIVE=(
  "android/**" 
  "ios/**"
  "sw_scripts/**" 
-  "sw_combine.js" 
  "Dockerfile" 
  "docker/**"
  "capacitor.config.ts"
--- a/scripts/validate-critical-files.sh
+++ b/scripts/validate-critical-files.sh
@@ -1,6 +1,11 @@
 #!/bin/bash
+#
+# Critical Files Migration Validator
+# Author: Matthew Raymer
+# Description: Validates migration status of critical files
+#

-echo 🔍 Critical Files Migration Validator"
+echo "🔍 Critical Files Migration Validator"
 echo "====================================="

 # Function to check actual usage (not comments)
@@ -10,77 +15,87 @@ check_actual_usage() {
    local description="$3"
    
    # Remove comments and check for actual usage
-    local count=$(grep -v ^[[:space:]]*//\|^[[:space:]]*\*\|^[[:space:]]*<!--" "$file" | \
-                  grep -v TODO.*migration\|FIXME.*migration" | \
+    local count=$(grep -v "^[[:space:]]*//\|^[[:space:]]*\*\|^[[:space:]]*<!--" "$file" | \
+                  grep -v "TODO.*migration\|FIXME.*migration" | \
                  grep -v "Migration.*replaced\|migrated.*from" | \
-                  grep -c $pattern" || echo 0)
+                  grep -c "$pattern" || echo 0)
    
-    if [$count" -gt0 then
-        echo  ❌ $description: $count instances
-        return 1 else
-        echo   ✅$description: None found
+    if [ "$count" -gt 0 ]; then
+        echo "  ❌ $description: $count instances"
+        return 1
+    else
+        echo "  ✅ $description: None found"
        return 0
    fi
 }

 # Function to check notification migration
 check_notifications() {
-    local file="$1  
+    local file="$1"
    # Check for notification helpers
-    local has_helpers=$(grep -c "createNotifyHelpers" $file" || echo "0")
+    local has_helpers=$(grep -c "createNotifyHelpers" "$file" || echo "0")
    
    # Check for direct $notify calls (excluding helper setup)
    local direct_notify=$(grep -v "createNotifyHelpers" "$file" | \
-                         grep -v this\.notify\." | \
+                         grep -v "this\.notify\." | \
                         grep -c "this\.\$notify" || echo 0)
    
-    if $has_helpers" -gt0 && $direct_notify" -eq0 then
-        echo "  ✅ Complete notification migration
+    if [ "$has_helpers" -gt 0 ] && [ "$direct_notify" -eq 0 ]; then
+        echo "  ✅ Complete notification migration"
        return 0
-    elif $has_helpers" -gt0 && $direct_notify" -gt0 then
-        echo "  ⚠️  Mixed pattern: $direct_notify direct calls
-        return 1 else
-        echo "  ❌ No notification migration
+    elif [ "$has_helpers" -gt 0 ] && [ "$direct_notify" -gt 0 ]; then
+        echo "  ⚠️  Mixed pattern: $direct_notify direct calls"
+        return 1
+    else
+        echo "  ❌ No notification migration"
        return 1
    fi
 }

 # Function to analyze a file
 analyze_file() {
-    local file="$1  echo ""
+    local file="$1"
+    echo ""
    echo "📄 Analyzing: $file"
    echo "----------------------------------------"
    
-    local issues=0    # Check legacy patterns
-    echo "🔍 Legacy Patterns:
-    check_actual_usage$file aseUtil" "databaseUtil usage || ((issues++))
-    check_actual_usage "$filelogConsoleAndDb ConsoleAndDb usage || ((issues++))
-    check_actual_usage$file formServiceFactory\.getInstance ct PlatformService usage ||((issues++))
+    local issues=0
+    # Check legacy patterns
+    echo "🔍 Legacy Patterns:"
+    check_actual_usage "$file" "databaseUtil" "databaseUtil usage" || ((issues++))
+    check_actual_usage "$file" "logConsoleAndDb" "ConsoleAndDb usage" || ((issues++))
+    check_actual_usage "$file" "formServiceFactory\.getInstance" "PlatformService usage" || ((issues++))
    
    # Check notifications
-    echo 🔔 Notifications:"
-    check_notifications "$file ||((issues++))
+    echo "🔔 Notifications:"
+    check_notifications "$file" || ((issues++))
    
    # Check PlatformServiceMixin
    echo "🔧 PlatformServiceMixin:"
-    local has_mixin=$(grep -cPlatformServiceMixin" $file || echo 0)
-    local has_mixins=$(grep -cmixins.*PlatformServiceMixin\|mixins.*\[PlatformServiceMixin" $file" || echo 0)
+    local has_mixin=$(grep -c "PlatformServiceMixin" "$file" || echo 0)
+    local has_mixins=$(grep -c "mixins.*PlatformServiceMixin\|mixins.*\[PlatformServiceMixin" "$file" || echo 0)
    
-    if  $has_mixin" -gt 0 && $has_mixins" -gt0 then
-        echo "  ✅ PlatformServiceMixin properly integrated    elif  $has_mixin" -gt 0 && $has_mixins" -eq0 then
-        echo "  ⚠️  Imported but not used as mixin        ((issues++))
+    if [ "$has_mixin" -gt 0 ] && [ "$has_mixins" -gt 0 ]; then
+        echo "  ✅ PlatformServiceMixin properly integrated"
+    elif [ "$has_mixin" -gt 0 ] && [ "$has_mixins" -eq 0 ]; then
+        echo "  ⚠️  Imported but not used as mixin"
+        ((issues++))
    else
-        echo "  ❌ No PlatformServiceMixin usage        ((issues++))
+        echo "  ❌ No PlatformServiceMixin usage"
+        ((issues++))
    fi
    
    # Check TODO comments
-    local todo_count=$(grep -c TODO.*migration\|FIXME.*migration" $file || echo "0) if $todo_count" -gt0 then
-        echo  ⚠️  TODO/FIXME comments: $todo_count        ((issues++))
+    local todo_count=$(grep -c "TODO.*migration\|FIXME.*migration" "$file" || echo "0")
+    if [ "$todo_count" -gt 0 ]; then
+        echo "  ⚠️  TODO/FIXME comments: $todo_count"
+        ((issues++))
    fi
    
-    if$issues" -eq0 then
-        echo "✅ File is fully migrated else
-        echo❌ $issues issues found"
+    if [ "$issues" -eq 0 ]; then
+        echo "✅ File is fully migrated"
+    else
+        echo "❌ $issues issues found"
    fi
    
    return $issues
@@ -88,35 +103,39 @@ analyze_file() {

 # Main analysis
 echo ""
-echo 📊 Critical Files Analysis"
+echo "📊 Critical Files Analysis"
 echo "=========================="

 # Critical files from our assessment
 files=(
-   src/components/MembersList.vue"
+    "src/components/MembersList.vue"
    "src/views/ContactsView.vue"
-    src/views/OnboardMeetingSetupView.vue"
-  src/db/databaseUtil.ts"
-    src/db/index.ts
+    "src/views/OnboardMeetingSetupView.vue"
+    "src/db/databaseUtil.ts"
+    "src/db/index.ts"
 )

 total_issues=0
-for file in ${files[@]}"; do
+for file in "${files[@]}"; do
    if [ -f "$file" ]; then
        analyze_file "$file"
        total_issues=$((total_issues + $?))
    else
-        echo ❌ File not found: $file"
+        echo "❌ File not found: $file"
    fi
 done

 # Summary
-echo "echo📋 Summary"
-echo=========="
+echo ""
+echo "📋 Summary"
+echo "=========="
 echo "Files analyzed: ${#files[@]}"
 echo "Total issues found: $total_issues"

-if$total_issues" -eq 0]; then
-    echo "✅ All critical files are properly migrated   exit 0 echo "❌ Migration issues require attention"
+if [ "$total_issues" -eq 0 ]; then
+    echo "✅ All critical files are properly migrated"
+    exit 0
+else
+    echo "❌ Migration issues require attention"
    exit 1
-fi 
+fi
--- a/src/main.web.ts
+++ b/src/main.web.ts
@@ -3,7 +3,8 @@ import { logger } from "./utils/logger";

 const platform = process.env.VITE_PLATFORM;

-// PWA service worker is automatically registered by VitePWA plugin
+// Note: PWA functionality is currently not implemented.
+// Service worker registration would be handled here when PWA support is added.

 const app = initializeApp();

--- a/src/services/PlatformServiceFactory.ts
+++ b/src/services/PlatformServiceFactory.ts
@@ -39,7 +39,9 @@ export class PlatformServiceFactory {
    }

    // Only log when actually creating the instance
-    const platform = process.env.VITE_PLATFORM || "web";
+    // Use import.meta.env for Vite environment variables (standard Vite pattern)
+    // process.env.VITE_PLATFORM is defined via Vite's define config, but import.meta.env is preferred
+    const platform = (import.meta.env?.VITE_PLATFORM || process.env.VITE_PLATFORM || "web") as string;

    if (!PlatformServiceFactory.creationLogged) {
      // Use console for critical startup message to avoid circular dependency
--- a/src/services/platforms/CapacitorPlatformService.ts
+++ b/src/services/platforms/CapacitorPlatformService.ts
@@ -54,6 +54,8 @@ export class CapacitorPlatformService
  private initializationPromise: Promise<void> | null = null;
  private operationQueue: Array<QueuedOperation> = [];
  private isProcessingQueue: boolean = false;
+  private readonly MAX_QUEUE_SIZE = 1000; // Maximum queue size before failing
+  private maxQueueSizeReached = 0; // Track peak queue size for telemetry

  constructor() {
    super();
@@ -371,6 +373,31 @@ export class CapacitorPlatformService
    });

    return new Promise<R>((resolve, reject) => {
+      // Queue size guard: prevent memory exhaustion from unbounded queue growth
+      if (this.operationQueue.length >= this.MAX_QUEUE_SIZE) {
+        const error = new Error(
+          `Database operation queue is full (${this.MAX_QUEUE_SIZE} operations). ` +
+          `This usually indicates the database is not initializing properly or operations are being queued too quickly.`
+        );
+        logger.error(
+          `[CapacitorPlatformService] Queue size limit reached: ${this.operationQueue.length}/${this.MAX_QUEUE_SIZE}`,
+        );
+        reject(error);
+        return;
+      }
+
+      // Track peak queue size for telemetry
+      if (this.operationQueue.length > this.maxQueueSizeReached) {
+        this.maxQueueSizeReached = this.operationQueue.length;
+      }
+
+      // Log warning if queue is getting large (but not at limit yet)
+      if (this.operationQueue.length > this.MAX_QUEUE_SIZE * 0.8) {
+        logger.warn(
+          `[CapacitorPlatformService] Queue size is high: ${this.operationQueue.length}/${this.MAX_QUEUE_SIZE}`,
+        );
+      }
+
      // Create completely plain objects that Vue cannot make reactive
      // Step 1: Deep clone the converted params to ensure they're plain objects
      const plainParams = JSON.parse(JSON.stringify(convertedParams));
@@ -865,6 +892,27 @@ export class CapacitorPlatformService
    };
  }

+  /**
+   * Gets telemetry information about the database operation queue.
+   * Useful for debugging and monitoring queue health.
+   * @returns Queue telemetry data
+   */
+  getQueueTelemetry(): {
+    currentSize: number;
+    maxSize: number;
+    peakSize: number;
+    isProcessing: boolean;
+    initialized: boolean;
+  } {
+    return {
+      currentSize: this.operationQueue.length,
+      maxSize: this.MAX_QUEUE_SIZE,
+      peakSize: this.maxQueueSizeReached,
+      isProcessing: this.isProcessingQueue,
+      initialized: this.initialized,
+    };
+  }
+
  /**
   * Checks and requests storage permissions if needed
   * @returns Promise that resolves when permissions are granted