feat: Implement API key authentication with rate limiting and secure comparison

- Added rate limiting to the authentication middleware to prevent brute-force attacks.
- Introduced a secure comparison function to mitigate timing attacks during API key validation.
- Created a new rate limiter class to track failed authentication attempts and block requests after exceeding the maximum allowed failures.
- Updated the authentication middleware to handle rate limiting and secure key comparison.
- Enhanced error handling for rate-limited requests, providing appropriate responses to clients.

apps/server/src/lib/auth.ts

@@ -2,9 +2,30 @@
  * Authentication middleware for API security
  *
  * Supports API key authentication via header or environment variable.
+ * Includes rate limiting to prevent brute-force attacks.
  */
 
+import * as crypto from 'crypto';
 import type { Request, Response, NextFunction } from 'express';
+import { apiKeyRateLimiter } from './rate-limiter.js';
+
+/**
+ * Performs a constant-time string comparison to prevent timing attacks.
+ * Uses crypto.timingSafeEqual with proper buffer handling.
+ */
+function secureCompare(a: string, b: string): boolean {
+  const bufferA = Buffer.from(a, 'utf8');
+  const bufferB = Buffer.from(b, 'utf8');
+
+  // If lengths differ, we still need to do a constant-time comparison
+  // to avoid leaking length information. We compare against bufferA twice.
+  if (bufferA.length !== bufferB.length) {
+    crypto.timingSafeEqual(bufferA, bufferA);
+    return false;
+  }
+
+  return crypto.timingSafeEqual(bufferA, bufferB);
+}
 
 // API key from environment (optional - if not set, auth is disabled)
 const API_KEY = process.env.AUTOMAKER_API_KEY;
@@ -14,6 +35,7 @@ const API_KEY = process.env.AUTOMAKER_API_KEY;
  *
  * If AUTOMAKER_API_KEY is set, requires matching key in X-API-Key header.
  * If not set, allows all requests (development mode).
+ * Includes rate limiting to prevent brute-force attacks.
  */
 export function authMiddleware(req: Request, res: Response, next: NextFunction): void {
   // If no API key is configured, allow all requests
@@ -22,6 +44,22 @@ export function authMiddleware(req: Request, res: Response, next: NextFunction):
     return;
   }
 
+  const clientIp = apiKeyRateLimiter.getClientIp(req);
+
+  // Check if client is rate limited
+  if (apiKeyRateLimiter.isBlocked(clientIp)) {
+    const retryAfterMs = apiKeyRateLimiter.getBlockTimeRemaining(clientIp);
+    const retryAfterSeconds = Math.ceil(retryAfterMs / 1000);
+
+    res.setHeader('Retry-After', retryAfterSeconds.toString());
+    res.status(429).json({
+      success: false,
+      error: 'Too many failed authentication attempts. Please try again later.',
+      retryAfter: retryAfterSeconds,
+    });
+    return;
+  }
+
   // Check for API key in header
   const providedKey = req.headers['x-api-key'] as string | undefined;
 
@@ -33,7 +71,10 @@ export function authMiddleware(req: Request, res: Response, next: NextFunction):
     return;
   }
 
-  if (providedKey !== API_KEY) {
+  if (!secureCompare(providedKey, API_KEY)) {
+    // Record failed attempt
+    apiKeyRateLimiter.recordFailure(clientIp);
+
     res.status(403).json({
       success: false,
       error: 'Invalid API key.',
@@ -41,6 +82,9 @@ export function authMiddleware(req: Request, res: Response, next: NextFunction):
     return;
   }
 
+  // Successful authentication - reset rate limiter for this IP
+  apiKeyRateLimiter.reset(clientIp);
+
   next();
 }
 

apps/server/src/lib/rate-limiter.ts

@@ -0,0 +1,208 @@
+/**
+ * In-memory rate limiter for authentication endpoints
+ *
+ * Provides brute-force protection by tracking failed attempts per IP address.
+ * Blocks requests after exceeding the maximum number of failures within a time window.
+ */
+
+import type { Request, Response, NextFunction } from 'express';
+
+interface AttemptRecord {
+  count: number;
+  firstAttempt: number;
+  blockedUntil: number | null;
+}
+
+interface RateLimiterConfig {
+  maxAttempts: number;
+  windowMs: number;
+  blockDurationMs: number;
+}
+
+const DEFAULT_CONFIG: RateLimiterConfig = {
+  maxAttempts: 5,
+  windowMs: 15 * 60 * 1000, // 15 minutes
+  blockDurationMs: 15 * 60 * 1000, // 15 minutes
+};
+
+/**
+ * Rate limiter instance that tracks attempts by a key (typically IP address)
+ */
+export class RateLimiter {
+  private attempts: Map<string, AttemptRecord> = new Map();
+  private config: RateLimiterConfig;
+
+  constructor(config: Partial<RateLimiterConfig> = {}) {
+    this.config = { ...DEFAULT_CONFIG, ...config };
+  }
+
+  /**
+   * Extract client IP address from request
+   * Handles proxied requests via X-Forwarded-For header
+   */
+  getClientIp(req: Request): string {
+    const forwarded = req.headers['x-forwarded-for'];
+    if (forwarded) {
+      const forwardedIp = Array.isArray(forwarded) ? forwarded[0] : forwarded.split(',')[0];
+      return forwardedIp.trim();
+    }
+    return req.socket.remoteAddress || 'unknown';
+  }
+
+  /**
+   * Check if a key is currently rate limited
+   */
+  isBlocked(key: string): boolean {
+    const record = this.attempts.get(key);
+    if (!record) return false;
+
+    const now = Date.now();
+
+    // Check if currently blocked
+    if (record.blockedUntil && now < record.blockedUntil) {
+      return true;
+    }
+
+    // Clear expired block
+    if (record.blockedUntil && now >= record.blockedUntil) {
+      this.attempts.delete(key);
+      return false;
+    }
+
+    return false;
+  }
+
+  /**
+   * Get remaining time until block expires (in milliseconds)
+   */
+  getBlockTimeRemaining(key: string): number {
+    const record = this.attempts.get(key);
+    if (!record?.blockedUntil) return 0;
+
+    const remaining = record.blockedUntil - Date.now();
+    return remaining > 0 ? remaining : 0;
+  }
+
+  /**
+   * Record a failed authentication attempt
+   * Returns true if the key is now blocked
+   */
+  recordFailure(key: string): boolean {
+    const now = Date.now();
+    const record = this.attempts.get(key);
+
+    if (!record) {
+      this.attempts.set(key, {
+        count: 1,
+        firstAttempt: now,
+        blockedUntil: null,
+      });
+      return false;
+    }
+
+    // If window has expired, reset the counter
+    if (now - record.firstAttempt > this.config.windowMs) {
+      this.attempts.set(key, {
+        count: 1,
+        firstAttempt: now,
+        blockedUntil: null,
+      });
+      return false;
+    }
+
+    // Increment counter
+    record.count += 1;
+
+    // Check if should be blocked
+    if (record.count >= this.config.maxAttempts) {
+      record.blockedUntil = now + this.config.blockDurationMs;
+      return true;
+    }
+
+    return false;
+  }
+
+  /**
+   * Clear a key's record (e.g., on successful authentication)
+   */
+  reset(key: string): void {
+    this.attempts.delete(key);
+  }
+
+  /**
+   * Get the number of attempts remaining before block
+   */
+  getAttemptsRemaining(key: string): number {
+    const record = this.attempts.get(key);
+    if (!record) return this.config.maxAttempts;
+
+    const now = Date.now();
+
+    // If window expired, full attempts available
+    if (now - record.firstAttempt > this.config.windowMs) {
+      return this.config.maxAttempts;
+    }
+
+    return Math.max(0, this.config.maxAttempts - record.count);
+  }
+
+  /**
+   * Clean up expired records to prevent memory leaks
+   */
+  cleanup(): void {
+    const now = Date.now();
+    const keysToDelete: string[] = [];
+
+    this.attempts.forEach((record, key) => {
+      // Mark for deletion if block has expired
+      if (record.blockedUntil && now >= record.blockedUntil) {
+        keysToDelete.push(key);
+        return;
+      }
+      // Mark for deletion if window has expired and not blocked
+      if (!record.blockedUntil && now - record.firstAttempt > this.config.windowMs) {
+        keysToDelete.push(key);
+      }
+    });
+
+    keysToDelete.forEach((key) => this.attempts.delete(key));
+  }
+}
+
+// Shared rate limiter instances for authentication endpoints
+export const apiKeyRateLimiter = new RateLimiter();
+export const terminalAuthRateLimiter = new RateLimiter();
+
+// Clean up expired records periodically (every 5 minutes)
+setInterval(
+  () => {
+    apiKeyRateLimiter.cleanup();
+    terminalAuthRateLimiter.cleanup();
+  },
+  5 * 60 * 1000
+);
+
+/**
+ * Create rate limiting middleware for authentication endpoints
+ * This middleware checks if the request is rate limited before processing
+ */
+export function createRateLimitMiddleware(rateLimiter: RateLimiter) {
+  return (req: Request, res: Response, next: NextFunction): void => {
+    const clientIp = rateLimiter.getClientIp(req);
+
+    if (rateLimiter.isBlocked(clientIp)) {
+      const retryAfterMs = rateLimiter.getBlockTimeRemaining(clientIp);
+      const retryAfterSeconds = Math.ceil(retryAfterMs / 1000);
+
+      res.setHeader('Retry-After', retryAfterSeconds.toString());
+      res.status(429).json({
+        success: false,
+        error: 'Too many failed authentication attempts. Please try again later.',
+        retryAfter: retryAfterSeconds,
+      });
+      return;
+    }
+
+    next();
+  };
+}

apps/server/src/middleware/validate-paths.ts

@@ -7,6 +7,29 @@
 import type { Request, Response, NextFunction } from 'express';
 import { validatePath, PathNotAllowedError } from '@automaker/platform';
 
+/**
+ * Custom error for invalid path type
+ */
+class InvalidPathTypeError extends Error {
+  constructor(paramName: string, expectedType: string, actualType: string) {
+    super(`Invalid type for '${paramName}': expected ${expectedType}, got ${actualType}`);
+    this.name = 'InvalidPathTypeError';
+  }
+}
+
+/**
+ * Validates that a value is a non-empty string suitable for path validation
+ *
+ * @param value - The value to check
+ * @param paramName - The parameter name for error messages
+ * @throws InvalidPathTypeError if value is not a valid string
+ */
+function assertValidPathString(value: unknown, paramName: string): asserts value is string {
+  if (typeof value !== 'string') {
+    throw new InvalidPathTypeError(paramName, 'string', typeof value);
+  }
+}
+
 /**
  * Creates a middleware that validates specified path parameters in req.body
  * @param paramNames - Names of parameters to validate (e.g., 'projectPath', 'worktreePath')
@@ -27,7 +50,8 @@ export function validatePathParams(...paramNames: string[]) {
         if (paramName.endsWith('?')) {
           const actualName = paramName.slice(0, -1);
           const value = req.body[actualName];
-          if (value) {
+          if (value !== undefined && value !== null) {
+            assertValidPathString(value, actualName);
             validatePath(value);
           }
           continue;
@@ -37,17 +61,30 @@ export function validatePathParams(...paramNames: string[]) {
         if (paramName.endsWith('[]')) {
           const actualName = paramName.slice(0, -2);
           const values = req.body[actualName];
-          if (Array.isArray(values) && values.length > 0) {
+
-            for (const value of values) {
+          // Skip if not provided or empty
-              validatePath(value);
+          if (values === undefined || values === null) {
-            }
+            continue;
+          }
+
+          // Validate that it's actually an array
+          if (!Array.isArray(values)) {
+            throw new InvalidPathTypeError(actualName, 'array', typeof values);
+          }
+
+          // Validate each element in the array
+          for (let i = 0; i < values.length; i++) {
+            const value = values[i];
+            assertValidPathString(value, `${actualName}[${i}]`);
+            validatePath(value);
           }
           continue;
         }
 
         // Handle regular parameters
         const value = req.body[paramName];
-        if (value) {
+        if (value !== undefined && value !== null) {
+          assertValidPathString(value, paramName);
           validatePath(value);
         }
       }
@@ -62,6 +99,14 @@ export function validatePathParams(...paramNames: string[]) {
         return;
       }
 
+      if (error instanceof InvalidPathTypeError) {
+        res.status(400).json({
+          success: false,
+          error: error.message,
+        });
+        return;
+      }
+
       // Re-throw unexpected errors
       throw error;
     }

apps/server/src/routes/settings/routes/get-credentials.ts

@@ -10,7 +10,7 @@
 
 import type { Request, Response } from 'express';
 import type { SettingsService } from '../../../services/settings-service.js';
-import { getErrorMessage, logError } from '../common.js';
+import { logError } from '../common.js';
 
 /**
  * Create handler factory for GET /api/settings/credentials
@@ -29,7 +29,7 @@ export function createGetCredentialsHandler(settingsService: SettingsService) {
       });
     } catch (error) {
       logError(error, 'Get credentials failed');
-      res.status(500).json({ success: false, error: getErrorMessage(error) });
+      res.status(500).json({ success: false, error: 'Failed to retrieve credentials' });
     }
   };
 }

apps/server/src/routes/settings/routes/update-credentials.ts

@@ -11,7 +11,71 @@
 import type { Request, Response } from 'express';
 import type { SettingsService } from '../../../services/settings-service.js';
 import type { Credentials } from '../../../types/settings.js';
-import { getErrorMessage, logError } from '../common.js';
+import { logError } from '../common.js';
+
+/** Maximum allowed length for API keys to prevent abuse */
+const MAX_API_KEY_LENGTH = 512;
+
+/** Known API key provider names that are valid */
+const VALID_API_KEY_PROVIDERS = ['anthropic', 'google', 'openai'] as const;
+
+/**
+ * Validates that the provided updates object has the correct structure
+ * and all apiKeys values are strings within acceptable length limits.
+ *
+ * @param updates - The partial credentials update object to validate
+ * @returns An error message if validation fails, or null if valid
+ */
+function validateCredentialsUpdate(updates: unknown): string | null {
+  if (!updates || typeof updates !== 'object' || Array.isArray(updates)) {
+    return 'Invalid request body - expected credentials object';
+  }
+
+  const obj = updates as Record<string, unknown>;
+
+  // If apiKeys is provided, validate its structure
+  if ('apiKeys' in obj) {
+    const apiKeys = obj.apiKeys;
+
+    if (apiKeys === null || apiKeys === undefined) {
+      // Allow null/undefined to clear
+      return null;
+    }
+
+    if (typeof apiKeys !== 'object' || Array.isArray(apiKeys)) {
+      return 'Invalid apiKeys - expected object';
+    }
+
+    const keysObj = apiKeys as Record<string, unknown>;
+
+    // Validate each provided API key
+    for (const [provider, value] of Object.entries(keysObj)) {
+      // Check provider name is valid
+      if (!VALID_API_KEY_PROVIDERS.includes(provider as (typeof VALID_API_KEY_PROVIDERS)[number])) {
+        return `Invalid API key provider: ${provider}. Valid providers: ${VALID_API_KEY_PROVIDERS.join(', ')}`;
+      }
+
+      // Check value is a string
+      if (typeof value !== 'string') {
+        return `Invalid API key for ${provider} - expected string`;
+      }
+
+      // Check length limit
+      if (value.length > MAX_API_KEY_LENGTH) {
+        return `API key for ${provider} exceeds maximum length of ${MAX_API_KEY_LENGTH} characters`;
+      }
+    }
+  }
+
+  // Validate version if provided
+  if ('version' in obj && obj.version !== undefined) {
+    if (typeof obj.version !== 'number' || !Number.isInteger(obj.version) || obj.version < 0) {
+      return 'Invalid version - expected non-negative integer';
+    }
+  }
+
+  return null;
+}
 
 /**
  * Create handler factory for PUT /api/settings/credentials
@@ -22,16 +86,19 @@ import { getErrorMessage, logError } from '../common.js';
 export function createUpdateCredentialsHandler(settingsService: SettingsService) {
   return async (req: Request, res: Response): Promise<void> => {
     try {
-      const updates = req.body as Partial<Credentials>;
+      // Validate the request body before type assertion
-
+      const validationError = validateCredentialsUpdate(req.body);
-      if (!updates || typeof updates !== 'object') {
+      if (validationError) {
         res.status(400).json({
           success: false,
-          error: 'Invalid request body - expected credentials object',
+          error: validationError,
         });
         return;
       }
 
+      // Safe to cast after validation
+      const updates = req.body as Partial<Credentials>;
+
       await settingsService.updateCredentials(updates);
 
       // Return masked credentials for confirmation
@@ -43,7 +110,7 @@ export function createUpdateCredentialsHandler(settingsService: SettingsService)
       });
     } catch (error) {
       logError(error, 'Update credentials failed');
-      res.status(500).json({ success: false, error: getErrorMessage(error) });
+      res.status(500).json({ success: false, error: 'Failed to update credentials' });
     }
   };
 }

apps/server/src/routes/setup/common.ts

@@ -9,6 +9,17 @@ import { getErrorMessage as getErrorMessageShared, createLogError } from '../com
 
 const logger = createLogger('Setup');
 
+/**
+ * Escapes special regex characters in a string to prevent regex injection.
+ * This ensures user input can be safely used in RegExp constructors.
+ *
+ * @param str - The string to escape
+ * @returns The escaped string safe for use in RegExp
+ */
+export function escapeRegExp(str: string): string {
+  return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+}
+
 // Storage for API keys (in-memory cache) - private
 const apiKeys: Record<string, string> = {};
 
@@ -33,6 +44,32 @@ export function getAllApiKeys(): Record<string, string> {
   return { ...apiKeys };
 }
 
+/**
+ * Escape a value for safe inclusion in a .env file.
+ * Handles special characters like quotes, newlines, dollar signs, and backslashes.
+ * Returns a properly quoted string if needed.
+ */
+function escapeEnvValue(value: string): string {
+  // Check if the value contains any characters that require quoting
+  const requiresQuoting = /[\s"'$`\\#\n\r]/.test(value) || value.includes('=');
+
+  if (!requiresQuoting) {
+    return value;
+  }
+
+  // Use double quotes and escape special characters within
+  // Escape backslashes first to avoid double-escaping
+  let escaped = value
+    .replace(/\\/g, '\\\\') // Escape backslashes
+    .replace(/"/g, '\\"') // Escape double quotes
+    .replace(/\$/g, '\\$') // Escape dollar signs (prevents variable expansion)
+    .replace(/`/g, '\\`') // Escape backticks
+    .replace(/\n/g, '\\n') // Escape newlines
+    .replace(/\r/g, '\\r'); // Escape carriage returns
+
+  return `"${escaped}"`;
+}
+
 /**
  * Helper to persist API keys to .env file
  */
@@ -47,21 +84,24 @@ export async function persistApiKeyToEnv(key: string, value: string): Promise<vo
       // .env file doesn't exist, we'll create it
     }
 
-    // Parse existing env content
+    // Escape the value for safe .env file storage
+    const escapedValue = escapeEnvValue(value);
+
+    // Parse existing env content - match key with optional quoted values
     const lines = envContent.split('\n');
-    const keyRegex = new RegExp(`^${key}=`);
+    const keyRegex = new RegExp(`^${escapeRegExp(key)}=`);
     let found = false;
     const newLines = lines.map((line) => {
       if (keyRegex.test(line)) {
         found = true;
-        return `${key}=${value}`;
+        return `${key}=${escapedValue}`;
       }
       return line;
     });
 
     if (!found) {
       // Add the key at the end
-      newLines.push(`${key}=${value}`);
+      newLines.push(`${key}=${escapedValue}`);
     }
 
     await fs.writeFile(envPath, newLines.join('\n'));

apps/server/src/routes/setup/routes/api-keys.ts

@@ -3,7 +3,7 @@
  */
 
 import type { Request, Response } from 'express';
-import { getApiKey, getErrorMessage, logError } from '../common.js';
+import { getApiKey, logError } from '../common.js';
 
 export function createApiKeysHandler() {
   return async (_req: Request, res: Response): Promise<void> => {
@@ -14,7 +14,7 @@ export function createApiKeysHandler() {
       });
     } catch (error) {
       logError(error, 'Get API keys failed');
-      res.status(500).json({ success: false, error: getErrorMessage(error) });
+      res.status(500).json({ success: false, error: 'Failed to retrieve API keys status' });
     }
   };
 }

apps/server/src/routes/setup/routes/delete-api-key.ts

@@ -11,7 +11,7 @@ const logger = createLogger('Setup');
 
 // In-memory storage reference (imported from common.ts pattern)
 // We need to modify common.ts to export a deleteApiKey function
-import { setApiKey } from '../common.js';
+import { setApiKey, escapeRegExp } from '../common.js';
 
 /**
  * Remove an API key from the .env file
@@ -30,7 +30,7 @@ async function removeApiKeyFromEnv(key: string): Promise<void> {
 
     // Parse existing env content and remove the key
     const lines = envContent.split('\n');
-    const keyRegex = new RegExp(`^${key}=`);
+    const keyRegex = new RegExp(`^${escapeRegExp(key)}=`);
     const newLines = lines.filter((line) => !keyRegex.test(line));
 
     // Remove empty lines at the end
@@ -68,9 +68,10 @@ export function createDeleteApiKeyHandler() {
 
       const envKey = envKeyMap[provider];
       if (!envKey) {
+        logger.warn(`[Setup] Unknown provider requested for deletion: ${provider}`);
         res.status(400).json({
           success: false,
-          error: `Unknown provider: ${provider}. Only anthropic is supported.`,
+          error: 'Unknown provider. Only anthropic is supported.',
         });
         return;
       }
@@ -94,7 +95,7 @@ export function createDeleteApiKeyHandler() {
       logger.error('[Setup] Delete API key error:', error);
       res.status(500).json({
         success: false,
-        error: error instanceof Error ? error.message : 'Failed to delete API key',
+        error: 'Failed to delete API key',
       });
     }
   };

apps/server/src/routes/setup/routes/store-api-key.ts

@@ -3,7 +3,7 @@
  */
 
 import type { Request, Response } from 'express';
-import { setApiKey, persistApiKeyToEnv, getErrorMessage, logError } from '../common.js';
+import { setApiKey, persistApiKeyToEnv, logError } from '../common.js';
 import { createLogger } from '@automaker/utils';
 
 const logger = createLogger('Setup');
@@ -30,9 +30,10 @@ export function createStoreApiKeyHandler() {
         await persistApiKeyToEnv('ANTHROPIC_API_KEY', apiKey);
         logger.info('[Setup] Stored API key as ANTHROPIC_API_KEY');
       } else {
+        logger.warn(`[Setup] Unsupported provider requested: ${provider}`);
         res.status(400).json({
           success: false,
-          error: `Unsupported provider: ${provider}. Only anthropic is supported.`,
+          error: 'Unsupported provider. Only anthropic is supported.',
         });
         return;
       }
@@ -40,7 +41,7 @@ export function createStoreApiKeyHandler() {
       res.json({ success: true });
     } catch (error) {
       logError(error, 'Store API key failed');
-      res.status(500).json({ success: false, error: getErrorMessage(error) });
+      res.status(500).json({ success: false, error: 'Failed to store API key' });
     }
   };
 }

apps/server/src/routes/setup/routes/verify-claude-auth.ts

@@ -10,6 +10,41 @@ import { getApiKey } from '../common.js';
 
 const logger = createLogger('Setup');
 
+/**
+ * Simple mutex implementation to prevent race conditions when
+ * modifying process.env during concurrent verification requests.
+ *
+ * The Claude Agent SDK reads ANTHROPIC_API_KEY from process.env,
+ * so we must temporarily modify it for verification. This mutex
+ * ensures only one verification runs at a time.
+ */
+class VerificationMutex {
+  private locked = false;
+  private queue: Array<() => void> = [];
+
+  async acquire(): Promise<void> {
+    return new Promise((resolve) => {
+      if (!this.locked) {
+        this.locked = true;
+        resolve();
+      } else {
+        this.queue.push(resolve);
+      }
+    });
+  }
+
+  release(): void {
+    if (this.queue.length > 0) {
+      const next = this.queue.shift();
+      if (next) next();
+    } else {
+      this.locked = false;
+    }
+  }
+}
+
+const verificationMutex = new VerificationMutex();
+
 // Known error patterns that indicate auth failure
 const AUTH_ERROR_PATTERNS = [
   'OAuth token revoked',
@@ -68,14 +103,79 @@ function containsAuthError(text: string): boolean {
   return AUTH_ERROR_PATTERNS.some((pattern) => lowerText.includes(pattern.toLowerCase()));
 }
 
+/** Valid authentication method values */
+const VALID_AUTH_METHODS = ['cli', 'api_key'] as const;
+type AuthMethod = (typeof VALID_AUTH_METHODS)[number];
+
+/**
+ * Validates and extracts the authMethod from the request body.
+ *
+ * @param body - The request body to validate
+ * @returns The validated authMethod or undefined if not provided
+ * @throws Error if authMethod is provided but invalid
+ */
+function validateAuthMethod(body: unknown): AuthMethod | undefined {
+  if (!body || typeof body !== 'object') {
+    return undefined;
+  }
+
+  const obj = body as Record<string, unknown>;
+
+  if (!('authMethod' in obj) || obj.authMethod === undefined || obj.authMethod === null) {
+    return undefined;
+  }
+
+  const authMethod = obj.authMethod;
+
+  if (typeof authMethod !== 'string') {
+    throw new Error(`Invalid authMethod type: expected string, got ${typeof authMethod}`);
+  }
+
+  if (!VALID_AUTH_METHODS.includes(authMethod as AuthMethod)) {
+    throw new Error(
+      `Invalid authMethod value: '${authMethod}'. Valid values: ${VALID_AUTH_METHODS.join(', ')}`
+    );
+  }
+
+  return authMethod as AuthMethod;
+}
+
 export function createVerifyClaudeAuthHandler() {
   return async (req: Request, res: Response): Promise<void> => {
     try {
-      // Get the auth method from the request body
+      // Validate and extract the auth method from the request body
-      const { authMethod } = req.body as { authMethod?: 'cli' | 'api_key' };
+      let authMethod: AuthMethod | undefined;
+      try {
+        authMethod = validateAuthMethod(req.body);
+      } catch (validationError) {
+        res.status(400).json({
+          success: false,
+          authenticated: false,
+          error: validationError instanceof Error ? validationError.message : 'Invalid request',
+        });
+        return;
+      }
 
       logger.info(`[Setup] Verifying Claude authentication using method: ${authMethod || 'auto'}`);
 
+      // Early validation before acquiring mutex - check if API key is needed but missing
+      if (authMethod === 'api_key') {
+        const storedApiKey = getApiKey('anthropic');
+        if (!storedApiKey && !process.env.ANTHROPIC_API_KEY) {
+          res.json({
+            success: true,
+            authenticated: false,
+            error: 'No API key configured. Please enter an API key first.',
+          });
+          return;
+        }
+      }
+
+      // Acquire mutex to prevent race conditions when modifying process.env
+      // The SDK reads ANTHROPIC_API_KEY from environment, so concurrent requests
+      // could interfere with each other without this lock
+      await verificationMutex.acquire();
+
       // Create an AbortController with a 30-second timeout
       const abortController = new AbortController();
       const timeoutId = setTimeout(() => abortController.abort(), 30000);
@@ -84,7 +184,7 @@ export function createVerifyClaudeAuthHandler() {
       let errorMessage = '';
       let receivedAnyContent = false;
 
-      // Save original env values
+      // Save original env values (inside mutex to ensure consistency)
       const originalAnthropicKey = process.env.ANTHROPIC_API_KEY;
 
       try {
@@ -99,17 +199,8 @@ export function createVerifyClaudeAuthHandler() {
           if (storedApiKey) {
             process.env.ANTHROPIC_API_KEY = storedApiKey;
             logger.info('[Setup] Using stored API key for verification');
-          } else {
-            // Check env var
-            if (!process.env.ANTHROPIC_API_KEY) {
-              res.json({
-                success: true,
-                authenticated: false,
-                error: 'No API key configured. Please enter an API key first.',
-              });
-              return;
-            }
           }
+          // Note: if no stored key, we use the existing env var (already validated above)
         }
 
         // Run a minimal query to verify authentication
@@ -129,7 +220,8 @@ export function createVerifyClaudeAuthHandler() {
         for await (const msg of stream) {
           const msgStr = JSON.stringify(msg);
           allMessages.push(msgStr);
-          logger.info('[Setup] Stream message:', msgStr.substring(0, 500));
+          // Debug log only message type to avoid leaking sensitive data
+          logger.debug('[Setup] Stream message type:', msg.type);
 
           // Check for billing errors FIRST - these should fail verification
           if (isBillingError(msgStr)) {
@@ -221,7 +313,8 @@ export function createVerifyClaudeAuthHandler() {
         } else {
           // No content received - might be an issue
           logger.warn('[Setup] No content received from stream');
-          logger.warn('[Setup] All messages:', allMessages.join('\n'));
+          // Log only message count to avoid leaking sensitive data
+          logger.warn('[Setup] Total messages received:', allMessages.length);
           errorMessage = 'No response received from Claude. Please check your authentication.';
         }
       } catch (error: unknown) {
@@ -277,6 +370,8 @@ export function createVerifyClaudeAuthHandler() {
           // If we cleared it and there was no original, keep it cleared
           delete process.env.ANTHROPIC_API_KEY;
         }
+        // Release the mutex so other verification requests can proceed
+        verificationMutex.release();
       }
 
       logger.info('[Setup] Verification result:', {

apps/server/src/routes/terminal/common.ts

@@ -65,6 +65,18 @@ export function cleanupExpiredTokens(): void {
 // Clean up expired tokens every 5 minutes
 setInterval(cleanupExpiredTokens, 5 * 60 * 1000);
 
+/**
+ * Extract Bearer token from Authorization header
+ * Returns undefined if header is missing or malformed
+ */
+export function extractBearerToken(req: Request): string | undefined {
+  const authHeader = req.headers.authorization;
+  if (!authHeader || !authHeader.startsWith('Bearer ')) {
+    return undefined;
+  }
+  return authHeader.slice(7); // Remove 'Bearer ' prefix
+}
+
 /**
  * Validate a terminal session token
  */
@@ -116,8 +128,9 @@ export function terminalAuthMiddleware(req: Request, res: Response, next: NextFu
     return;
   }
 
-  // Check for session token
+  // Extract token from Authorization header only (Bearer token format)
-  const token = (req.headers['x-terminal-token'] as string) || (req.query.token as string);
+  // Query string tokens are not supported due to security risks (URL logging, referrer leakage)
+  const token = extractBearerToken(req);
 
   if (!validateTerminalToken(token)) {
     res.status(401).json({

apps/server/src/routes/terminal/routes/auth.ts

@@ -1,7 +1,9 @@
 /**
  * POST /auth endpoint - Authenticate with password to get a session token
+ * Includes rate limiting to prevent brute-force attacks.
  */
 
+import * as crypto from 'crypto';
 import type { Request, Response } from 'express';
 import {
   getTerminalEnabledConfigValue,
@@ -11,6 +13,25 @@ import {
   getTokenExpiryMs,
   getErrorMessage,
 } from '../common.js';
+import { terminalAuthRateLimiter } from '../../../lib/rate-limiter.js';
+
+/**
+ * Performs a constant-time string comparison to prevent timing attacks.
+ * Uses crypto.timingSafeEqual with proper buffer handling.
+ */
+function secureCompare(a: string, b: string): boolean {
+  const bufferA = Buffer.from(a, 'utf8');
+  const bufferB = Buffer.from(b, 'utf8');
+
+  // If lengths differ, we still need to do a constant-time comparison
+  // to avoid leaking length information. We compare against bufferA twice.
+  if (bufferA.length !== bufferB.length) {
+    crypto.timingSafeEqual(bufferA, bufferA);
+    return false;
+  }
+
+  return crypto.timingSafeEqual(bufferA, bufferB);
+}
 
 export function createAuthHandler() {
   return (req: Request, res: Response): void => {
@@ -36,9 +57,28 @@ export function createAuthHandler() {
       return;
     }
 
+    const clientIp = terminalAuthRateLimiter.getClientIp(req);
+
+    // Check if client is rate limited
+    if (terminalAuthRateLimiter.isBlocked(clientIp)) {
+      const retryAfterMs = terminalAuthRateLimiter.getBlockTimeRemaining(clientIp);
+      const retryAfterSeconds = Math.ceil(retryAfterMs / 1000);
+
+      res.setHeader('Retry-After', retryAfterSeconds.toString());
+      res.status(429).json({
+        success: false,
+        error: 'Too many failed authentication attempts. Please try again later.',
+        retryAfter: retryAfterSeconds,
+      });
+      return;
+    }
+
     const { password } = req.body;
 
-    if (!password || password !== terminalPassword) {
+    if (!password || !secureCompare(password, terminalPassword)) {
+      // Record failed attempt
+      terminalAuthRateLimiter.recordFailure(clientIp);
+
       res.status(401).json({
         success: false,
         error: 'Invalid password',
@@ -46,6 +86,9 @@ export function createAuthHandler() {
       return;
     }
 
+    // Successful authentication - reset rate limiter for this IP
+    terminalAuthRateLimiter.reset(clientIp);
+
     // Generate session token
     const token = generateToken();
     const now = new Date();

apps/server/src/routes/terminal/routes/logout.ts

@@ -1,18 +1,36 @@
 /**
  * POST /logout endpoint - Invalidate a session token
+ *
+ * Security: Only allows invalidating the token used for authentication.
+ * This ensures users can only log out their own sessions.
  */
 
 import type { Request, Response } from 'express';
-import { deleteToken } from '../common.js';
+import { deleteToken, extractBearerToken, validateTerminalToken } from '../common.js';
 
 export function createLogoutHandler() {
   return (req: Request, res: Response): void => {
-    const token = (req.headers['x-terminal-token'] as string) || req.body.token;
+    const token = extractBearerToken(req);
 
-    if (token) {
+    if (!token) {
-      deleteToken(token);
+      res.status(401).json({
+        success: false,
+        error: 'Authorization header with Bearer token is required',
+      });
+      return;
     }
 
+    if (!validateTerminalToken(token)) {
+      res.status(401).json({
+        success: false,
+        error: 'Invalid or expired token',
+      });
+      return;
+    }
+
+    // Token is valid and belongs to the requester - safe to invalidate
+    deleteToken(token);
+
     res.json({
       success: true,
     });

apps/server/tests/unit/lib/auth.test.ts

@@ -1,6 +1,16 @@
 import { describe, it, expect, beforeEach, vi } from 'vitest';
 import { createMockExpressContext } from '../../utils/mocks.js';
 
+/**
+ * Creates a mock Express context with socket properties for rate limiter support
+ */
+function createMockExpressContextWithSocket() {
+  const ctx = createMockExpressContext();
+  ctx.req.socket = { remoteAddress: '127.0.0.1' } as any;
+  ctx.res.setHeader = vi.fn().mockReturnThis();
+  return ctx;
+}
+
 /**
  * Note: auth.ts reads AUTOMAKER_API_KEY at module load time.
  * We need to reset modules and reimport for each test to get fresh state.
@@ -29,7 +39,7 @@ describe('auth.ts', () => {
       process.env.AUTOMAKER_API_KEY = 'test-secret-key';
 
       const { authMiddleware } = await import('@/lib/auth.js');
-      const { req, res, next } = createMockExpressContext();
+      const { req, res, next } = createMockExpressContextWithSocket();
 
       authMiddleware(req, res, next);
 
@@ -45,7 +55,7 @@ describe('auth.ts', () => {
       process.env.AUTOMAKER_API_KEY = 'test-secret-key';
 
       const { authMiddleware } = await import('@/lib/auth.js');
-      const { req, res, next } = createMockExpressContext();
+      const { req, res, next } = createMockExpressContextWithSocket();
       req.headers['x-api-key'] = 'wrong-key';
 
       authMiddleware(req, res, next);
@@ -62,7 +72,7 @@ describe('auth.ts', () => {
       process.env.AUTOMAKER_API_KEY = 'test-secret-key';
 
       const { authMiddleware } = await import('@/lib/auth.js');
-      const { req, res, next } = createMockExpressContext();
+      const { req, res, next } = createMockExpressContextWithSocket();
       req.headers['x-api-key'] = 'test-secret-key';
 
       authMiddleware(req, res, next);
@@ -113,4 +123,197 @@ describe('auth.ts', () => {
       });
     });
   });
+
+  describe('security - AUTOMAKER_API_KEY not set', () => {
+    it('should allow requests without any authentication when API key is not configured', async () => {
+      delete process.env.AUTOMAKER_API_KEY;
+
+      const { authMiddleware } = await import('@/lib/auth.js');
+      const { req, res, next } = createMockExpressContext();
+
+      authMiddleware(req, res, next);
+
+      expect(next).toHaveBeenCalled();
+      expect(res.status).not.toHaveBeenCalled();
+      expect(res.json).not.toHaveBeenCalled();
+    });
+
+    it('should allow requests even with invalid key header when API key is not configured', async () => {
+      delete process.env.AUTOMAKER_API_KEY;
+
+      const { authMiddleware } = await import('@/lib/auth.js');
+      const { req, res, next } = createMockExpressContext();
+      req.headers['x-api-key'] = 'some-random-key';
+
+      authMiddleware(req, res, next);
+
+      expect(next).toHaveBeenCalled();
+      expect(res.status).not.toHaveBeenCalled();
+    });
+
+    it('should report auth as disabled when no API key is configured', async () => {
+      delete process.env.AUTOMAKER_API_KEY;
+
+      const { isAuthEnabled, getAuthStatus } = await import('@/lib/auth.js');
+
+      expect(isAuthEnabled()).toBe(false);
+      expect(getAuthStatus()).toEqual({
+        enabled: false,
+        method: 'none',
+      });
+    });
+  });
+
+  describe('security - authentication correctness', () => {
+    it('should correctly authenticate with matching API key', async () => {
+      const testKey = 'correct-secret-key-12345';
+      process.env.AUTOMAKER_API_KEY = testKey;
+
+      const { authMiddleware } = await import('@/lib/auth.js');
+      const { req, res, next } = createMockExpressContextWithSocket();
+      req.headers['x-api-key'] = testKey;
+
+      authMiddleware(req, res, next);
+
+      expect(next).toHaveBeenCalled();
+      expect(res.status).not.toHaveBeenCalled();
+    });
+
+    it('should reject keys that differ by a single character', async () => {
+      process.env.AUTOMAKER_API_KEY = 'correct-secret-key';
+
+      const { authMiddleware } = await import('@/lib/auth.js');
+      const { req, res, next } = createMockExpressContextWithSocket();
+      req.headers['x-api-key'] = 'correct-secret-keY'; // Last char uppercase
+
+      authMiddleware(req, res, next);
+
+      expect(res.status).toHaveBeenCalledWith(403);
+      expect(next).not.toHaveBeenCalled();
+    });
+
+    it('should reject keys with extra characters', async () => {
+      process.env.AUTOMAKER_API_KEY = 'secret-key';
+
+      const { authMiddleware } = await import('@/lib/auth.js');
+      const { req, res, next } = createMockExpressContextWithSocket();
+      req.headers['x-api-key'] = 'secret-key-extra';
+
+      authMiddleware(req, res, next);
+
+      expect(res.status).toHaveBeenCalledWith(403);
+      expect(next).not.toHaveBeenCalled();
+    });
+
+    it('should reject keys that are a prefix of the actual key', async () => {
+      process.env.AUTOMAKER_API_KEY = 'full-secret-key';
+
+      const { authMiddleware } = await import('@/lib/auth.js');
+      const { req, res, next } = createMockExpressContextWithSocket();
+      req.headers['x-api-key'] = 'full-secret';
+
+      authMiddleware(req, res, next);
+
+      expect(res.status).toHaveBeenCalledWith(403);
+      expect(next).not.toHaveBeenCalled();
+    });
+
+    it('should reject empty string API key header', async () => {
+      process.env.AUTOMAKER_API_KEY = 'secret-key';
+
+      const { authMiddleware } = await import('@/lib/auth.js');
+      const { req, res, next } = createMockExpressContextWithSocket();
+      req.headers['x-api-key'] = '';
+
+      authMiddleware(req, res, next);
+
+      // Empty string is falsy, so should get 401 (no key provided)
+      expect(res.status).toHaveBeenCalledWith(401);
+      expect(next).not.toHaveBeenCalled();
+    });
+
+    it('should handle keys with special characters correctly', async () => {
+      const specialKey = 'key-with-$pecial!@#chars_123';
+      process.env.AUTOMAKER_API_KEY = specialKey;
+
+      const { authMiddleware } = await import('@/lib/auth.js');
+      const { req, res, next } = createMockExpressContextWithSocket();
+      req.headers['x-api-key'] = specialKey;
+
+      authMiddleware(req, res, next);
+
+      expect(next).toHaveBeenCalled();
+      expect(res.status).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('security - rate limiting', () => {
+    it('should block requests after multiple failed attempts', async () => {
+      process.env.AUTOMAKER_API_KEY = 'correct-key';
+
+      const { authMiddleware } = await import('@/lib/auth.js');
+      const { apiKeyRateLimiter } = await import('@/lib/rate-limiter.js');
+
+      // Reset the rate limiter for this test
+      apiKeyRateLimiter.reset('192.168.1.100');
+
+      // Simulate multiple failed attempts
+      for (let i = 0; i < 5; i++) {
+        const { req, res, next } = createMockExpressContextWithSocket();
+        req.socket.remoteAddress = '192.168.1.100';
+        req.headers['x-api-key'] = 'wrong-key';
+        authMiddleware(req, res, next);
+      }
+
+      // Next request should be rate limited
+      const { req, res, next } = createMockExpressContextWithSocket();
+      req.socket.remoteAddress = '192.168.1.100';
+      req.headers['x-api-key'] = 'correct-key'; // Even with correct key
+
+      authMiddleware(req, res, next);
+
+      expect(res.status).toHaveBeenCalledWith(429);
+      expect(next).not.toHaveBeenCalled();
+
+      // Cleanup
+      apiKeyRateLimiter.reset('192.168.1.100');
+    });
+
+    it('should reset rate limit on successful authentication', async () => {
+      process.env.AUTOMAKER_API_KEY = 'correct-key';
+
+      const { authMiddleware } = await import('@/lib/auth.js');
+      const { apiKeyRateLimiter } = await import('@/lib/rate-limiter.js');
+
+      // Reset the rate limiter for this test
+      apiKeyRateLimiter.reset('192.168.1.101');
+
+      // Simulate a few failed attempts (not enough to trigger block)
+      for (let i = 0; i < 3; i++) {
+        const { req, res, next } = createMockExpressContextWithSocket();
+        req.socket.remoteAddress = '192.168.1.101';
+        req.headers['x-api-key'] = 'wrong-key';
+        authMiddleware(req, res, next);
+      }
+
+      // Successful authentication should reset the counter
+      const {
+        req: successReq,
+        res: successRes,
+        next: successNext,
+      } = createMockExpressContextWithSocket();
+      successReq.socket.remoteAddress = '192.168.1.101';
+      successReq.headers['x-api-key'] = 'correct-key';
+
+      authMiddleware(successReq, successRes, successNext);
+
+      expect(successNext).toHaveBeenCalled();
+
+      // After reset, we should have full attempts available again
+      expect(apiKeyRateLimiter.getAttemptsRemaining('192.168.1.101')).toBe(5);
+
+      // Cleanup
+      apiKeyRateLimiter.reset('192.168.1.101');
+    });
+  });
 });

apps/server/tests/unit/lib/rate-limiter.test.ts

@@ -0,0 +1,249 @@
+import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest';
+import { RateLimiter } from '../../../src/lib/rate-limiter.js';
+import type { Request } from 'express';
+
+describe('RateLimiter', () => {
+  let rateLimiter: RateLimiter;
+
+  beforeEach(() => {
+    rateLimiter = new RateLimiter({
+      maxAttempts: 3,
+      windowMs: 60000, // 1 minute
+      blockDurationMs: 60000, // 1 minute
+    });
+    vi.useFakeTimers();
+  });
+
+  afterEach(() => {
+    vi.useRealTimers();
+  });
+
+  describe('getClientIp', () => {
+    it('should extract IP from x-forwarded-for header', () => {
+      const req = {
+        headers: { 'x-forwarded-for': '192.168.1.100' },
+        socket: { remoteAddress: '127.0.0.1' },
+      } as unknown as Request;
+
+      expect(rateLimiter.getClientIp(req)).toBe('192.168.1.100');
+    });
+
+    it('should use first IP from x-forwarded-for with multiple IPs', () => {
+      const req = {
+        headers: { 'x-forwarded-for': '192.168.1.100, 10.0.0.1, 172.16.0.1' },
+        socket: { remoteAddress: '127.0.0.1' },
+      } as unknown as Request;
+
+      expect(rateLimiter.getClientIp(req)).toBe('192.168.1.100');
+    });
+
+    it('should fall back to socket remoteAddress when no x-forwarded-for', () => {
+      const req = {
+        headers: {},
+        socket: { remoteAddress: '127.0.0.1' },
+      } as unknown as Request;
+
+      expect(rateLimiter.getClientIp(req)).toBe('127.0.0.1');
+    });
+
+    it('should return "unknown" when no IP can be determined', () => {
+      const req = {
+        headers: {},
+        socket: { remoteAddress: undefined },
+      } as unknown as Request;
+
+      expect(rateLimiter.getClientIp(req)).toBe('unknown');
+    });
+  });
+
+  describe('isBlocked', () => {
+    it('should return false for unknown keys', () => {
+      expect(rateLimiter.isBlocked('192.168.1.1')).toBe(false);
+    });
+
+    it('should return false after recording fewer failures than max', () => {
+      rateLimiter.recordFailure('192.168.1.1');
+      rateLimiter.recordFailure('192.168.1.1');
+
+      expect(rateLimiter.isBlocked('192.168.1.1')).toBe(false);
+    });
+
+    it('should return true after reaching max failures', () => {
+      rateLimiter.recordFailure('192.168.1.1');
+      rateLimiter.recordFailure('192.168.1.1');
+      rateLimiter.recordFailure('192.168.1.1');
+
+      expect(rateLimiter.isBlocked('192.168.1.1')).toBe(true);
+    });
+
+    it('should return false after block expires', () => {
+      rateLimiter.recordFailure('192.168.1.1');
+      rateLimiter.recordFailure('192.168.1.1');
+      rateLimiter.recordFailure('192.168.1.1');
+
+      expect(rateLimiter.isBlocked('192.168.1.1')).toBe(true);
+
+      // Advance time past block duration
+      vi.advanceTimersByTime(60001);
+
+      expect(rateLimiter.isBlocked('192.168.1.1')).toBe(false);
+    });
+  });
+
+  describe('recordFailure', () => {
+    it('should return false when not yet blocked', () => {
+      expect(rateLimiter.recordFailure('192.168.1.1')).toBe(false);
+      expect(rateLimiter.recordFailure('192.168.1.1')).toBe(false);
+    });
+
+    it('should return true when threshold is reached', () => {
+      rateLimiter.recordFailure('192.168.1.1');
+      rateLimiter.recordFailure('192.168.1.1');
+      expect(rateLimiter.recordFailure('192.168.1.1')).toBe(true);
+    });
+
+    it('should reset counter after window expires', () => {
+      rateLimiter.recordFailure('192.168.1.1');
+      rateLimiter.recordFailure('192.168.1.1');
+
+      // Advance time past window
+      vi.advanceTimersByTime(60001);
+
+      // Should start fresh
+      expect(rateLimiter.recordFailure('192.168.1.1')).toBe(false);
+      expect(rateLimiter.getAttemptsRemaining('192.168.1.1')).toBe(2);
+    });
+
+    it('should track different IPs independently', () => {
+      rateLimiter.recordFailure('192.168.1.1');
+      rateLimiter.recordFailure('192.168.1.1');
+      rateLimiter.recordFailure('192.168.1.1');
+
+      rateLimiter.recordFailure('192.168.1.2');
+
+      expect(rateLimiter.isBlocked('192.168.1.1')).toBe(true);
+      expect(rateLimiter.isBlocked('192.168.1.2')).toBe(false);
+    });
+  });
+
+  describe('reset', () => {
+    it('should clear record for a key', () => {
+      rateLimiter.recordFailure('192.168.1.1');
+      rateLimiter.recordFailure('192.168.1.1');
+
+      rateLimiter.reset('192.168.1.1');
+
+      expect(rateLimiter.getAttemptsRemaining('192.168.1.1')).toBe(3);
+    });
+
+    it('should clear blocked status', () => {
+      rateLimiter.recordFailure('192.168.1.1');
+      rateLimiter.recordFailure('192.168.1.1');
+      rateLimiter.recordFailure('192.168.1.1');
+
+      expect(rateLimiter.isBlocked('192.168.1.1')).toBe(true);
+
+      rateLimiter.reset('192.168.1.1');
+
+      expect(rateLimiter.isBlocked('192.168.1.1')).toBe(false);
+    });
+  });
+
+  describe('getAttemptsRemaining', () => {
+    it('should return max attempts for unknown key', () => {
+      expect(rateLimiter.getAttemptsRemaining('192.168.1.1')).toBe(3);
+    });
+
+    it('should decrease as failures are recorded', () => {
+      rateLimiter.recordFailure('192.168.1.1');
+      expect(rateLimiter.getAttemptsRemaining('192.168.1.1')).toBe(2);
+
+      rateLimiter.recordFailure('192.168.1.1');
+      expect(rateLimiter.getAttemptsRemaining('192.168.1.1')).toBe(1);
+
+      rateLimiter.recordFailure('192.168.1.1');
+      expect(rateLimiter.getAttemptsRemaining('192.168.1.1')).toBe(0);
+    });
+
+    it('should return max attempts after window expires', () => {
+      rateLimiter.recordFailure('192.168.1.1');
+      rateLimiter.recordFailure('192.168.1.1');
+
+      vi.advanceTimersByTime(60001);
+
+      expect(rateLimiter.getAttemptsRemaining('192.168.1.1')).toBe(3);
+    });
+  });
+
+  describe('getBlockTimeRemaining', () => {
+    it('should return 0 for non-blocked key', () => {
+      expect(rateLimiter.getBlockTimeRemaining('192.168.1.1')).toBe(0);
+    });
+
+    it('should return remaining block time for blocked key', () => {
+      rateLimiter.recordFailure('192.168.1.1');
+      rateLimiter.recordFailure('192.168.1.1');
+      rateLimiter.recordFailure('192.168.1.1');
+
+      vi.advanceTimersByTime(30000); // Advance 30 seconds
+
+      const remaining = rateLimiter.getBlockTimeRemaining('192.168.1.1');
+      expect(remaining).toBeGreaterThan(29000);
+      expect(remaining).toBeLessThanOrEqual(30000);
+    });
+
+    it('should return 0 after block expires', () => {
+      rateLimiter.recordFailure('192.168.1.1');
+      rateLimiter.recordFailure('192.168.1.1');
+      rateLimiter.recordFailure('192.168.1.1');
+
+      vi.advanceTimersByTime(60001);
+
+      expect(rateLimiter.getBlockTimeRemaining('192.168.1.1')).toBe(0);
+    });
+  });
+
+  describe('cleanup', () => {
+    it('should remove expired blocks', () => {
+      rateLimiter.recordFailure('192.168.1.1');
+      rateLimiter.recordFailure('192.168.1.1');
+      rateLimiter.recordFailure('192.168.1.1');
+
+      vi.advanceTimersByTime(60001);
+
+      rateLimiter.cleanup();
+
+      // After cleanup, the record should be gone
+      expect(rateLimiter.getAttemptsRemaining('192.168.1.1')).toBe(3);
+    });
+
+    it('should remove expired windows', () => {
+      rateLimiter.recordFailure('192.168.1.1');
+
+      vi.advanceTimersByTime(60001);
+
+      rateLimiter.cleanup();
+
+      expect(rateLimiter.getAttemptsRemaining('192.168.1.1')).toBe(3);
+    });
+
+    it('should preserve active records', () => {
+      rateLimiter.recordFailure('192.168.1.1');
+
+      vi.advanceTimersByTime(30000); // Half the window
+
+      rateLimiter.cleanup();
+
+      expect(rateLimiter.getAttemptsRemaining('192.168.1.1')).toBe(2);
+    });
+  });
+
+  describe('default configuration', () => {
+    it('should use sensible defaults', () => {
+      const defaultLimiter = new RateLimiter();
+
+      // Should have 5 max attempts by default
+      expect(defaultLimiter.getAttemptsRemaining('test')).toBe(5);
+    });
+  });
+});

apps/ui/src/components/views/settings-view/api-keys/hooks/use-api-key-management.ts

@@ -59,8 +59,9 @@ export function useApiKeyManagement() {
               hasGoogleKey: status.hasGoogleKey,
             });
           }
-        } catch (error) {
+        } catch {
-          console.error('Failed to check API key status:', error);
+          // Silently handle API key status check failures to avoid exposing
+          // sensitive error details in the console
         }
       }
     };
@@ -98,26 +99,29 @@ export function useApiKeyManagement() {
   };
 
   // Test Google/Gemini connection
-  // TODO: Add backend endpoint for Gemini API key verification
+  // NOTE: Full API key validation requires a backend call to verify the key
+  // against Google's API. The current client-side validation only checks
+  // basic format requirements and cannot confirm the key is actually valid.
   const handleTestGeminiConnection = async () => {
     setTestingGeminiConnection(true);
     setGeminiTestResult(null);
 
-    // Basic validation - check key format
+    // Basic client-side format validation only
+    // This does NOT verify the key is valid with Google's API
     if (!googleKey || googleKey.trim().length < 10) {
       setGeminiTestResult({
         success: false,
-        message: 'Please enter a valid API key.',
+        message: 'Please enter an API key with at least 10 characters.',
       });
       setTestingGeminiConnection(false);
       return;
     }
 
-    // For now, just validate the key format (starts with expected prefix)
+    // Client-side validation cannot confirm key validity.
-    // Full verification requires a backend endpoint
+    // The key will be verified when first used with the Gemini API.
     setGeminiTestResult({
       success: true,
-      message: 'API key saved. Connection test not yet available.',
+      message: 'API key format accepted. Key will be validated on first use with Gemini API.',
     });
     setTestingGeminiConnection(false);
   };

apps/ui/src/components/views/terminal-view.tsx

@@ -296,7 +296,7 @@ export function TerminalView() {
 
     const headers: Record<string, string> = {};
     if (terminalState.authToken) {
-      headers['X-Terminal-Token'] = terminalState.authToken;
+      headers['Authorization'] = `Bearer ${terminalState.authToken}`;
     }
 
     console.log(`[Terminal] Killing ${sessionIds.length} sessions on server`);
@@ -459,7 +459,7 @@ export function TerminalView() {
     try {
       const headers: Record<string, string> = {};
       if (terminalState.authToken) {
-        headers['X-Terminal-Token'] = terminalState.authToken;
+        headers['Authorization'] = `Bearer ${terminalState.authToken}`;
       }
       const response = await fetch(`${serverUrl}/api/terminal/settings`, { headers });
       const data = await response.json();
@@ -488,7 +488,7 @@ export function TerminalView() {
         'Content-Type': 'application/json',
       };
       if (terminalState.authToken) {
-        headers['X-Terminal-Token'] = terminalState.authToken;
+        headers['Authorization'] = `Bearer ${terminalState.authToken}`;
       }
 
       // Try to use the bulk delete endpoint if available, otherwise delete individually
@@ -501,7 +501,7 @@ export function TerminalView() {
           const xhr = new XMLHttpRequest();
           xhr.open('DELETE', url, false); // synchronous
           if (terminalState.authToken) {
-            xhr.setRequestHeader('X-Terminal-Token', terminalState.authToken);
+            xhr.setRequestHeader('Authorization', `Bearer ${terminalState.authToken}`);
           }
           xhr.send();
         } catch {
@@ -595,7 +595,7 @@ export function TerminalView() {
         // Get fresh auth token from store
         const authToken = useAppStore.getState().terminalState.authToken;
         if (authToken) {
-          headers['X-Terminal-Token'] = authToken;
+          headers['Authorization'] = `Bearer ${authToken}`;
         }
 
         // Helper to check if a session still exists on server
@@ -833,7 +833,7 @@ export function TerminalView() {
         'Content-Type': 'application/json',
       };
       if (terminalState.authToken) {
-        headers['X-Terminal-Token'] = terminalState.authToken;
+        headers['Authorization'] = `Bearer ${terminalState.authToken}`;
       }
 
       const response = await fetch(`${serverUrl}/api/terminal/sessions`, {
@@ -892,7 +892,7 @@ export function TerminalView() {
         'Content-Type': 'application/json',
       };
       if (terminalState.authToken) {
-        headers['X-Terminal-Token'] = terminalState.authToken;
+        headers['Authorization'] = `Bearer ${terminalState.authToken}`;
       }
 
       const response = await fetch(`${serverUrl}/api/terminal/sessions`, {
@@ -952,7 +952,7 @@ export function TerminalView() {
     try {
       const headers: Record<string, string> = {};
       if (terminalState.authToken) {
-        headers['X-Terminal-Token'] = terminalState.authToken;
+        headers['Authorization'] = `Bearer ${terminalState.authToken}`;
       }
 
       const response = await fetch(`${serverUrl}/api/terminal/sessions/${sessionId}`, {
@@ -998,7 +998,7 @@ export function TerminalView() {
     // Kill all sessions on the server
     const headers: Record<string, string> = {};
     if (terminalState.authToken) {
-      headers['X-Terminal-Token'] = terminalState.authToken;
+      headers['Authorization'] = `Bearer ${terminalState.authToken}`;
     }
 
     await Promise.all(

apps/ui/src/components/views/terminal-view/terminal-panel.tsx

@@ -940,7 +940,12 @@ export function TerminalPanel({
     if (!terminal) return;
 
     const connect = () => {
-      // Build WebSocket URL with token
+      // Build WebSocket URL with token in query string
+      // Note: WebSocket API in browsers does not support custom headers during the upgrade handshake,
+      // so we must pass the token via query string. This is acceptable because:
+      // 1. WebSocket URLs are not exposed in HTTP Referer headers
+      // 2. The connection is upgraded to a secure WebSocket protocol immediately
+      // 3. Server-side logging should not log query parameters containing tokens
       let url = `${wsUrl}/api/terminal/ws?sessionId=${sessionId}`;
       if (authToken) {
         url += `&token=${encodeURIComponent(authToken)}`;

apps/ui/src/store/app-store.ts

@@ -2676,7 +2676,10 @@ export const useAppStore = create<AppState & AppActions>()(
           kanbanCardDetailLevel: state.kanbanCardDetailLevel,
           boardViewMode: state.boardViewMode,
           // Settings
-          apiKeys: state.apiKeys,
+          // NOTE: apiKeys are intentionally NOT persisted to localStorage for security.
+          // API keys are stored server-side only via the storeApiKey API to prevent
+          // exposure through XSS attacks. The apiKeys state is populated on app load
+          // from the secure server-side storage.
           maxConcurrency: state.maxConcurrency,
           // Note: autoModeByProject is intentionally NOT persisted
           // Auto-mode should always default to OFF on app refresh

libs/platform/src/secure-fs.ts

@@ -4,29 +4,46 @@
  * All file I/O operations must go through this adapter to enforce
  * ALLOWED_ROOT_DIRECTORY restrictions at the actual access point,
  * not just at the API layer. This provides defense-in-depth security.
+ *
+ * Security features:
+ * - Path validation: All paths are validated against allowed directories
+ * - Symlink protection: Operations on existing files resolve symlinks before validation
+ *   to prevent directory escape attacks via symbolic links
+ *
+ * TOCTOU (Time-of-check to time-of-use) note:
+ * There is an inherent race condition between path validation and the actual file
+ * operation. To mitigate this, we use the validated realpath (symlinks resolved)
+ * for the actual operation wherever possible. However, this cannot fully prevent
+ * race conditions in a multi-process environment. For maximum security in
+ * high-risk scenarios, consider using file descriptor-based operations or
+ * additional locking mechanisms.
  */
 
 import fs from 'fs/promises';
 import type { Dirent } from 'fs';
 import path from 'path';
-import { validatePath } from './security.js';
+import { validatePath, validatePathWithSymlinkCheck } from './security.js';
 
 /**
  * Wrapper around fs.access that validates path first
+ * Uses symlink-aware validation to prevent directory escape attacks
  */
 export async function access(filePath: string, mode?: number): Promise<void> {
-  const validatedPath = validatePath(filePath);
+  // Use symlink check since we're checking an existing path
+  const validatedPath = validatePathWithSymlinkCheck(filePath);
   return fs.access(validatedPath, mode);
 }
 
 /**
  * Wrapper around fs.readFile that validates path first
+ * Uses symlink-aware validation to prevent reading files outside allowed directories
  */
 export async function readFile(
   filePath: string,
   encoding?: BufferEncoding
 ): Promise<string | Buffer> {
-  const validatedPath = validatePath(filePath);
+  // Use symlink check since we're reading an existing file
+  const validatedPath = validatePathWithSymlinkCheck(filePath);
   if (encoding) {
     return fs.readFile(validatedPath, encoding);
   }
@@ -35,29 +52,34 @@ export async function readFile(
 
 /**
  * Wrapper around fs.writeFile that validates path first
+ * Uses symlink-aware validation for existing files, or validates parent for new files
  */
 export async function writeFile(
   filePath: string,
   data: string | Buffer,
   encoding?: BufferEncoding
 ): Promise<void> {
-  const validatedPath = validatePath(filePath);
+  // Use symlink check with requireExists=false to handle both new and existing files
+  const validatedPath = validatePathWithSymlinkCheck(filePath, { requireExists: false });
   return fs.writeFile(validatedPath, data, encoding);
 }
 
 /**
  * Wrapper around fs.mkdir that validates path first
+ * Uses symlink-aware validation for parent directory to prevent creating dirs via symlink escape
  */
 export async function mkdir(
   dirPath: string,
   options?: { recursive?: boolean; mode?: number }
 ): Promise<string | undefined> {
-  const validatedPath = validatePath(dirPath);
+  // Use symlink check with requireExists=false since directory may not exist yet
+  const validatedPath = validatePathWithSymlinkCheck(dirPath, { requireExists: false });
   return fs.mkdir(validatedPath, options);
 }
 
 /**
  * Wrapper around fs.readdir that validates path first
+ * Uses symlink-aware validation to prevent listing directories outside allowed paths
  */
 export async function readdir(
   dirPath: string,
@@ -71,7 +93,8 @@ export async function readdir(
   dirPath: string,
   options?: { withFileTypes?: boolean; encoding?: BufferEncoding }
 ): Promise<string[] | Dirent[]> {
-  const validatedPath = validatePath(dirPath);
+  // Use symlink check since we're reading an existing directory
+  const validatedPath = validatePathWithSymlinkCheck(dirPath);
   if (options?.withFileTypes === true) {
     return fs.readdir(validatedPath, { withFileTypes: true });
   }
@@ -80,66 +103,85 @@ export async function readdir(
 
 /**
  * Wrapper around fs.stat that validates path first
+ * Uses symlink-aware validation to prevent stat on files outside allowed paths
  */
 export async function stat(filePath: string): Promise<any> {
-  const validatedPath = validatePath(filePath);
+  // Use symlink check since we're getting info about an existing file
+  const validatedPath = validatePathWithSymlinkCheck(filePath);
   return fs.stat(validatedPath);
 }
 
 /**
  * Wrapper around fs.rm that validates path first
+ * Uses symlink-aware validation to prevent deleting files/directories outside allowed paths
  */
 export async function rm(
   filePath: string,
   options?: { recursive?: boolean; force?: boolean }
 ): Promise<void> {
-  const validatedPath = validatePath(filePath);
+  // Use symlink check since we're removing an existing file/directory
+  const validatedPath = validatePathWithSymlinkCheck(filePath);
   return fs.rm(validatedPath, options);
 }
 
 /**
  * Wrapper around fs.unlink that validates path first
+ * Uses symlink-aware validation to prevent unlinking files outside allowed paths
  */
 export async function unlink(filePath: string): Promise<void> {
-  const validatedPath = validatePath(filePath);
+  // Use symlink check since we're unlinking an existing file
+  const validatedPath = validatePathWithSymlinkCheck(filePath);
   return fs.unlink(validatedPath);
 }
 
 /**
  * Wrapper around fs.copyFile that validates both paths first
+ * Uses symlink-aware validation for source, and parent validation for destination
  */
 export async function copyFile(src: string, dest: string, mode?: number): Promise<void> {
-  const validatedSrc = validatePath(src);
+  // Source must exist, use symlink check
-  const validatedDest = validatePath(dest);
+  const validatedSrc = validatePathWithSymlinkCheck(src);
+  // Destination may not exist, validate with parent fallback
+  const validatedDest = validatePathWithSymlinkCheck(dest, { requireExists: false });
   return fs.copyFile(validatedSrc, validatedDest, mode);
 }
 
 /**
  * Wrapper around fs.appendFile that validates path first
+ * Uses symlink-aware validation for existing files, or validates parent for new files
  */
 export async function appendFile(
   filePath: string,
   data: string | Buffer,
   encoding?: BufferEncoding
 ): Promise<void> {
-  const validatedPath = validatePath(filePath);
+  // File may or may not exist, use symlink check with parent fallback
+  const validatedPath = validatePathWithSymlinkCheck(filePath, { requireExists: false });
   return fs.appendFile(validatedPath, data, encoding);
 }
 
 /**
  * Wrapper around fs.rename that validates both paths first
+ * Uses symlink-aware validation for source, and parent validation for destination
  */
 export async function rename(oldPath: string, newPath: string): Promise<void> {
-  const validatedOldPath = validatePath(oldPath);
+  // Source must exist, use symlink check
-  const validatedNewPath = validatePath(newPath);
+  const validatedOldPath = validatePathWithSymlinkCheck(oldPath);
+  // Destination may not exist, validate with parent fallback
+  const validatedNewPath = validatePathWithSymlinkCheck(newPath, { requireExists: false });
   return fs.rename(validatedOldPath, validatedNewPath);
 }
 
 /**
  * Wrapper around fs.lstat that validates path first
  * Returns file stats without following symbolic links
+ *
+ * Note: This intentionally uses validatePath (not validatePathWithSymlinkCheck)
+ * because lstat is used to inspect symlinks themselves. Using realpathSync
+ * would defeat the purpose of lstat.
  */
 export async function lstat(filePath: string): Promise<any> {
+  // Use basic validation since lstat is for inspecting symlinks
   const validatedPath = validatePath(filePath);
   return fs.lstat(validatedPath);
 }

libs/platform/src/security.ts

@@ -1,9 +1,22 @@
 /**
  * Security utilities for path validation
  * Enforces ALLOWED_ROOT_DIRECTORY constraint with appData exception
+ *
+ * Security considerations:
+ * - Symlink resolution: validatePathWithSymlinkCheck() resolves symlinks to prevent
+ *   escaping the allowed directory via symbolic links
+ * - TOCTOU: There is an inherent race condition between path validation and file
+ *   operation. Callers should use the resolved realpath for operations when possible.
  */
 
 import path from 'path';
+import fs from 'fs';
+
+/**
+ * Security mode: 'strict' fails closed when ALLOWED_ROOT_DIRECTORY is not set,
+ * 'permissive' allows all paths (legacy behavior, not recommended for production)
+ */
+let securityMode: 'strict' | 'permissive' = 'strict';
 
 /**
  * Error thrown when a path is not allowed by security policy
@@ -25,22 +38,43 @@ let dataDirectory: string | null = null;
  * Initialize security settings from environment variables
  * - ALLOWED_ROOT_DIRECTORY: main security boundary
  * - DATA_DIR: appData exception, always allowed
+ * - SECURITY_MODE: 'strict' (default, fail-closed) or 'permissive' (legacy, fail-open)
  */
 export function initAllowedPaths(): void {
+  // Load security mode
+  const mode = process.env.SECURITY_MODE?.toLowerCase();
+  if (mode === 'permissive') {
+    securityMode = 'permissive';
+    console.warn(
+      '[Security] WARNING: Running in PERMISSIVE mode - all paths allowed when ALLOWED_ROOT_DIRECTORY is not set. ' +
+        'This is not recommended for production environments.'
+    );
+  } else {
+    securityMode = 'strict';
+  }
+
   // Load ALLOWED_ROOT_DIRECTORY
   const rootDir = process.env.ALLOWED_ROOT_DIRECTORY;
   if (rootDir) {
     allowedRootDirectory = path.resolve(rootDir);
-    console.log(`[Security] ✓ ALLOWED_ROOT_DIRECTORY configured: ${allowedRootDirectory}`);
+    console.log(`[Security] ALLOWED_ROOT_DIRECTORY configured: ${allowedRootDirectory}`);
+  } else if (securityMode === 'strict') {
+    console.error(
+      '[Security] CRITICAL: ALLOWED_ROOT_DIRECTORY not set in strict mode. ' +
+        'All file operations outside DATA_DIR will be denied. ' +
+        'Set ALLOWED_ROOT_DIRECTORY or use SECURITY_MODE=permissive to allow all paths.'
+    );
   } else {
-    console.log('[Security] ⚠️  ALLOWED_ROOT_DIRECTORY not set - allowing access to all paths');
+    console.warn(
+      '[Security] WARNING: ALLOWED_ROOT_DIRECTORY not set - allowing access to all paths'
+    );
   }
 
   // Load DATA_DIR (appData exception - always allowed)
   const dataDir = process.env.DATA_DIR;
   if (dataDir) {
     dataDirectory = path.resolve(dataDir);
-    console.log(`[Security] ✓ DATA_DIR configured: ${dataDirectory}`);
+    console.log(`[Security] DATA_DIR configured: ${dataDirectory}`);
   }
 }
 
@@ -49,7 +83,10 @@ export function initAllowedPaths(): void {
  * Returns true if:
  * - Path is within ALLOWED_ROOT_DIRECTORY, OR
  * - Path is within DATA_DIR (appData exception), OR
- * - No restrictions are configured (backward compatibility)
+ * - No restrictions are configured AND security mode is 'permissive'
+ *
+ * In strict mode (default), paths are denied if ALLOWED_ROOT_DIRECTORY is not set,
+ * unless they are within DATA_DIR.
  */
 export function isPathAllowed(filePath: string): boolean {
   const resolvedPath = path.resolve(filePath);
@@ -59,24 +96,29 @@ export function isPathAllowed(filePath: string): boolean {
     return true;
   }
 
-  // If no ALLOWED_ROOT_DIRECTORY restriction is configured, allow all paths
+  // If no ALLOWED_ROOT_DIRECTORY restriction is configured:
-  // Note: DATA_DIR is checked above as an exception, but doesn't restrict other paths
+  // - In strict mode: deny (fail-closed)
+  // - In permissive mode: allow all paths (legacy behavior)
   if (!allowedRootDirectory) {
-    return true;
+    return securityMode === 'permissive';
   }
 
   // Allow if within ALLOWED_ROOT_DIRECTORY
-  if (allowedRootDirectory && isPathWithinDirectory(resolvedPath, allowedRootDirectory)) {
+  if (isPathWithinDirectory(resolvedPath, allowedRootDirectory)) {
     return true;
   }
 
-  // If restrictions are configured but path doesn't match, deny
+  // Path doesn't match any allowed directory, deny
   return false;
 }
 
 /**
  * Validate a path - resolves it and checks permissions
  * Throws PathNotAllowedError if path is not allowed
+ *
+ * NOTE: This function uses path.resolve() which does NOT resolve symbolic links.
+ * For operations on existing files where symlink attacks are a concern, use
+ * validatePathWithSymlinkCheck() instead.
  */
 export function validatePath(filePath: string): string {
   const resolvedPath = path.resolve(filePath);
@@ -88,6 +130,74 @@ export function validatePath(filePath: string): string {
   return resolvedPath;
 }
 
+/**
+ * Validate a path with symlink resolution for existing files
+ * This prevents symlink-based directory escape attacks by resolving the
+ * actual filesystem path before validation.
+ *
+ * @param filePath - The path to validate
+ * @param options.requireExists - If true (default), throws if path doesn't exist.
+ *                                If false, falls back to validatePath for non-existent paths.
+ * @returns The real path (symlinks resolved) if file exists, or resolved path if not
+ * @throws PathNotAllowedError if the real path escapes allowed directories
+ *
+ * Security note: There is still a TOCTOU race between this check and the actual
+ * file operation. For maximum security, callers should use the returned realpath
+ * for the subsequent operation, not the original path.
+ */
+export function validatePathWithSymlinkCheck(
+  filePath: string,
+  options: { requireExists?: boolean } = {}
+): string {
+  const { requireExists = true } = options;
+  const resolvedPath = path.resolve(filePath);
+
+  try {
+    // Check if path exists and get info without following symlinks
+    const lstats = fs.lstatSync(resolvedPath);
+
+    // Get the real path (resolves all symlinks)
+    const realPath = fs.realpathSync(resolvedPath);
+
+    // Validate the real path, not the symlink path
+    if (!isPathAllowed(realPath)) {
+      throw new PathNotAllowedError(`${filePath} (resolves to ${realPath} via symlink)`);
+    }
+
+    // If it's a symlink, log for security auditing
+    if (lstats.isSymbolicLink()) {
+      console.log(`[Security] Symlink detected: ${resolvedPath} -> ${realPath}`);
+    }
+
+    return realPath;
+  } catch (error) {
+    // Handle file not found
+    if ((error as NodeJS.ErrnoException).code === 'ENOENT') {
+      if (requireExists) {
+        throw error;
+      }
+      // For new files, validate the parent directory with symlink check if it exists
+      const parentDir = path.dirname(resolvedPath);
+      try {
+        const realParentPath = fs.realpathSync(parentDir);
+        if (!isPathAllowed(realParentPath)) {
+          throw new PathNotAllowedError(`${filePath} (parent resolves to ${realParentPath})`);
+        }
+        // Return the path within the real parent
+        return path.join(realParentPath, path.basename(resolvedPath));
+      } catch (parentError) {
+        // Parent doesn't exist either, fall back to basic validation
+        if ((parentError as NodeJS.ErrnoException).code === 'ENOENT') {
+          return validatePath(filePath);
+        }
+        throw parentError;
+      }
+    }
+    // Re-throw PathNotAllowedError and other errors
+    throw error;
+  }
+}
+
 /**
  * Check if a path is within a directory, with protection against path traversal
  * Returns true only if resolvedPath is within directoryPath

libs/platform/tests/security.test.ts

@@ -95,27 +95,57 @@ describe('security.ts', () => {
       expect(isPathAllowed('/app/data/credentials.json')).toBe(true);
     });
 
-    it('should allow all paths when no restrictions configured', async () => {
+    it('should deny all paths in strict mode when no restrictions configured', async () => {
       delete process.env.ALLOWED_ROOT_DIRECTORY;
       delete process.env.DATA_DIR;
+      delete process.env.SECURITY_MODE; // Default to strict
 
       const { initAllowedPaths, isPathAllowed } = await import('../src/security');
       initAllowedPaths();
 
+      // In strict mode, paths are denied when no ALLOWED_ROOT_DIRECTORY is set
+      expect(isPathAllowed('/any/path')).toBe(false);
+      expect(isPathAllowed('/etc/passwd')).toBe(false);
+    });
+
+    it('should allow all paths in permissive mode when no restrictions configured', async () => {
+      delete process.env.ALLOWED_ROOT_DIRECTORY;
+      delete process.env.DATA_DIR;
+      process.env.SECURITY_MODE = 'permissive';
+
+      const { initAllowedPaths, isPathAllowed } = await import('../src/security');
+      initAllowedPaths();
+
+      // In permissive mode, all paths are allowed when no restrictions configured
       expect(isPathAllowed('/any/path')).toBe(true);
       expect(isPathAllowed('/etc/passwd')).toBe(true);
     });
 
-    it('should allow all paths when only DATA_DIR is configured', async () => {
+    it('should deny non-DATA_DIR paths in strict mode when only DATA_DIR is configured', async () => {
       delete process.env.ALLOWED_ROOT_DIRECTORY;
       process.env.DATA_DIR = '/data';
+      delete process.env.SECURITY_MODE; // Default to strict
 
       const { initAllowedPaths, isPathAllowed } = await import('../src/security');
       initAllowedPaths();
 
       // DATA_DIR should be allowed
       expect(isPathAllowed('/data/file.txt')).toBe(true);
-      // And all other paths should be allowed since no ALLOWED_ROOT_DIRECTORY restriction
+      // Other paths should be denied in strict mode
+      expect(isPathAllowed('/any/path')).toBe(false);
+    });
+
+    it('should allow all paths in permissive mode when only DATA_DIR is configured', async () => {
+      delete process.env.ALLOWED_ROOT_DIRECTORY;
+      process.env.DATA_DIR = '/data';
+      process.env.SECURITY_MODE = 'permissive';
+
+      const { initAllowedPaths, isPathAllowed } = await import('../src/security');
+      initAllowedPaths();
+
+      // DATA_DIR should be allowed
+      expect(isPathAllowed('/data/file.txt')).toBe(true);
+      // Other paths should also be allowed in permissive mode
       expect(isPathAllowed('/any/path')).toBe(true);
     });
   });
@@ -155,13 +185,28 @@ describe('security.ts', () => {
       expect(result).toBe(path.resolve(cwd, './file.txt'));
     });
 
-    it('should not throw when no restrictions configured', async () => {
+    it('should throw in strict mode when no restrictions configured', async () => {
       delete process.env.ALLOWED_ROOT_DIRECTORY;
       delete process.env.DATA_DIR;
+      delete process.env.SECURITY_MODE; // Default to strict
+
+      const { initAllowedPaths, validatePath, PathNotAllowedError } =
+        await import('../src/security');
+      initAllowedPaths();
+
+      // In strict mode, paths are denied when no ALLOWED_ROOT_DIRECTORY is set
+      expect(() => validatePath('/any/path')).toThrow(PathNotAllowedError);
+    });
+
+    it('should not throw in permissive mode when no restrictions configured', async () => {
+      delete process.env.ALLOWED_ROOT_DIRECTORY;
+      delete process.env.DATA_DIR;
+      process.env.SECURITY_MODE = 'permissive';
 
       const { initAllowedPaths, validatePath } = await import('../src/security');
       initAllowedPaths();
 
+      // In permissive mode, all paths are allowed when no restrictions configured
       expect(() => validatePath('/any/path')).not.toThrow();
     });
   });
@@ -212,6 +257,110 @@ describe('security.ts', () => {
     });
   });
 
+  describe('path traversal attack prevention', () => {
+    it('should block basic path traversal with ../', async () => {
+      process.env.ALLOWED_ROOT_DIRECTORY = '/allowed';
+      delete process.env.DATA_DIR;
+
+      const { initAllowedPaths, isPathAllowed } = await import('../src/security');
+      initAllowedPaths();
+
+      expect(isPathAllowed('/allowed/../etc/passwd')).toBe(false);
+      expect(isPathAllowed('/allowed/subdir/../../etc/passwd')).toBe(false);
+    });
+
+    it('should block path traversal with multiple ../ sequences', async () => {
+      process.env.ALLOWED_ROOT_DIRECTORY = '/allowed/deep/nested';
+      delete process.env.DATA_DIR;
+
+      const { initAllowedPaths, isPathAllowed } = await import('../src/security');
+      initAllowedPaths();
+
+      expect(isPathAllowed('/allowed/deep/nested/../../../etc/passwd')).toBe(false);
+      expect(isPathAllowed('/allowed/deep/nested/../../../../root')).toBe(false);
+    });
+
+    it('should block standalone .. in path components', async () => {
+      process.env.ALLOWED_ROOT_DIRECTORY = '/allowed';
+      delete process.env.DATA_DIR;
+
+      const { initAllowedPaths, isPathAllowed } = await import('../src/security');
+      initAllowedPaths();
+
+      expect(isPathAllowed('/allowed/foo/..bar')).toBe(true); // This is a valid filename, not traversal
+      expect(isPathAllowed('/allowed/foo/../bar')).toBe(true); // Resolves within allowed
+      expect(isPathAllowed('/allowed/../notallowed')).toBe(false);
+    });
+
+    it('should handle edge case of path ending with /..', async () => {
+      process.env.ALLOWED_ROOT_DIRECTORY = '/allowed/subdir';
+      delete process.env.DATA_DIR;
+
+      const { initAllowedPaths, isPathAllowed } = await import('../src/security');
+      initAllowedPaths();
+
+      expect(isPathAllowed('/allowed/subdir/..')).toBe(false);
+      expect(isPathAllowed('/allowed/subdir/../..')).toBe(false);
+    });
+
+    it('should properly resolve and block complex traversal attempts', async () => {
+      process.env.ALLOWED_ROOT_DIRECTORY = '/home/user/projects';
+      delete process.env.DATA_DIR;
+
+      const { initAllowedPaths, isPathAllowed } = await import('../src/security');
+      initAllowedPaths();
+
+      // Attempt to escape via complex path
+      expect(isPathAllowed('/home/user/projects/app/../../../etc/shadow')).toBe(false);
+
+      // Valid path that uses .. but stays within allowed
+      expect(isPathAllowed('/home/user/projects/app/../lib/file.ts')).toBe(true);
+    });
+
+    it('should validate path throws on traversal attacks', async () => {
+      process.env.ALLOWED_ROOT_DIRECTORY = '/allowed';
+      delete process.env.DATA_DIR;
+
+      const { initAllowedPaths, validatePath, PathNotAllowedError } =
+        await import('../src/security');
+      initAllowedPaths();
+
+      expect(() => validatePath('/allowed/../etc/passwd')).toThrow(PathNotAllowedError);
+      expect(() => validatePath('/allowed/../../root/.ssh/id_rsa')).toThrow(PathNotAllowedError);
+    });
+
+    it('should handle paths with mixed separators (cross-platform)', async () => {
+      process.env.ALLOWED_ROOT_DIRECTORY = '/allowed';
+      delete process.env.DATA_DIR;
+
+      const { initAllowedPaths, isPathAllowed } = await import('../src/security');
+      initAllowedPaths();
+
+      // Node's path.resolve handles these correctly on each platform
+      const maliciousPath = path.resolve('/allowed', '..', 'etc', 'passwd');
+      expect(isPathAllowed(maliciousPath)).toBe(false);
+    });
+
+    it('should correctly identify paths at the boundary', async () => {
+      process.env.ALLOWED_ROOT_DIRECTORY = '/allowed';
+      delete process.env.DATA_DIR;
+
+      const { initAllowedPaths, isPathAllowed } = await import('../src/security');
+      initAllowedPaths();
+
+      // The allowed directory itself should be allowed
+      expect(isPathAllowed('/allowed')).toBe(true);
+      expect(isPathAllowed('/allowed/')).toBe(true);
+
+      // Parent of allowed should not be allowed
+      expect(isPathAllowed('/')).toBe(false);
+
+      // Sibling directories should not be allowed
+      expect(isPathAllowed('/allowed2')).toBe(false);
+      expect(isPathAllowed('/allowedextra')).toBe(false);
+    });
+  });
+
   describe('getDataDirectory', () => {
     it('should return the configured data directory', async () => {
       process.env.DATA_DIR = '/data';