mirror of
https://github.com/AutoMaker-Org/automaker.git
synced 2026-01-30 06:12:03 +00:00
Compare commits
1 Commits
906f471521
...
security-s
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
c7ebdb1f80 |
@@ -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();
|
||||
}
|
||||
|
||||
|
||||
208
apps/server/src/lib/rate-limiter.ts
Normal file
208
apps/server/src/lib/rate-limiter.ts
Normal 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();
|
||||
};
|
||||
}
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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' });
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
@@ -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' });
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
@@ -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'));
|
||||
|
||||
@@ -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' });
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
@@ -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',
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
@@ -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' });
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
@@ -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:', {
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
249
apps/server/tests/unit/lib/rate-limiter.test.ts
Normal file
249
apps/server/tests/unit/lib/rate-limiter.test.ts
Normal 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);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -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);
|
||||
};
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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)}`;
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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';
|
||||
|
||||
Reference in New Issue
Block a user