Compare commits

...

1 Commits

Author SHA1 Message Date
Test User
c7ebdb1f80 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.
2025-12-24 14:49:47 -05:00
22 changed files with 1439 additions and 99 deletions

View File

@@ -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();
}

View File

@@ -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();
};
}

View File

@@ -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) {
validatePath(value);
}
// Skip if not provided or empty
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;
}

View File

@@ -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' });
}
};
}

View File

@@ -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>;
if (!updates || typeof updates !== 'object') {
// Validate the request body before type assertion
const validationError = validateCredentialsUpdate(req.body);
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' });
}
};
}

View File

@@ -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'));

View File

@@ -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' });
}
};
}

View File

@@ -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',
});
}
};

View File

@@ -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' });
}
};
}

View File

@@ -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
const { authMethod } = req.body as { authMethod?: 'cli' | 'api_key' };
// Validate and extract the auth method from the request body
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:', {

View File

@@ -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
const token = (req.headers['x-terminal-token'] as string) || (req.query.token as string);
// Extract token from Authorization header only (Bearer token format)
// 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({

View File

@@ -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();

View File

@@ -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) {
deleteToken(token);
if (!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,
});

View File

@@ -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');
});
});
});

View File

@@ -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);
});
});
});

View File

@@ -59,8 +59,9 @@ export function useApiKeyManagement() {
hasGoogleKey: status.hasGoogleKey,
});
}
} catch (error) {
console.error('Failed to check API key status:', error);
} catch {
// 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)
// Full verification requires a backend endpoint
// Client-side validation cannot confirm key validity.
// 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);
};

View File

@@ -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(

View File

@@ -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)}`;

View File

@@ -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

View File

@@ -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);
const validatedDest = validatePath(dest);
// Source must exist, use symlink check
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);
const validatedNewPath = validatePath(newPath);
// Source must exist, use symlink check
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);
}

View File

@@ -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
// Note: DATA_DIR is checked above as an exception, but doesn't restrict other paths
// If no ALLOWED_ROOT_DIRECTORY restriction is configured:
// - 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

View File

@@ -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';