From 185f9940fd6365e639087498c132f38e58bd52f7 Mon Sep 17 00:00:00 2001 From: SpencersServer Date: Thu, 29 Jan 2026 13:28:42 +0200 Subject: [PATCH] security: add network isolation, rate limiting, and file permissions Step 7: Network Isolation - Sandbox already defaults to network=none (verified) - Add security validation for sandbox configs - Log warning when network is explicitly enabled - Add validateSandboxSecurity() to check for: - Network access enabled - Writable root filesystem - Capabilities not dropped - Browser host control enabled Step 8: Session Rate Limiting - Add src/security/rate-limit.ts with RateLimiter class - Pre-configured limiters for: - Session messages (60/min, 30s block) - Tool invocations (100/min) - Exec commands (30/min, 1min block) - Gateway API (120/min) - Auth attempts (5/min, 5min block) - Automatic cleanup of expired entries Step 9: File Permission Enforcement - Add src/security/file-permissions.ts - Check and enforce permissions on sensitive paths: - State directory (0700) - Config file (0600) - Credentials directory (0700) - OAuth and auth profile files (0600) - Audit logs and session data - Detect world-writable, group-writable, symlinks - validateSensitivePermissions() for auditing - enforceSensitivePermissions() for remediation Step 10: Security Module Index - Add src/security/index.ts exporting all security utilities - Provides centralized access to security features --- src/agents/sandbox/config.ts | 86 ++++++++++- src/security/file-permissions.ts | 252 +++++++++++++++++++++++++++++++ src/security/index.ts | 78 ++++++++++ src/security/rate-limit.ts | 185 +++++++++++++++++++++++ 4 files changed, 596 insertions(+), 5 deletions(-) create mode 100644 src/security/file-permissions.ts create mode 100644 src/security/index.ts create mode 100644 src/security/rate-limit.ts diff --git a/src/agents/sandbox/config.ts b/src/agents/sandbox/config.ts index aa848c54b..c54d34563 100644 --- a/src/agents/sandbox/config.ts +++ b/src/agents/sandbox/config.ts @@ -1,4 +1,5 @@ import type { MoltbotConfig } from "../../config/config.js"; +import { logInfo } from "../../logger.js"; import { resolveAgentConfig } from "../agent-scope.js"; import { DEFAULT_SANDBOX_BROWSER_AUTOSTART_TIMEOUT_MS, @@ -138,17 +139,27 @@ export function resolveSandboxConfigForAgent(cfg?: MoltbotConfig, agentId?: stri const toolPolicy = resolveSandboxToolPolicyForAgent(cfg, agentId); + const dockerConfig = resolveSandboxDockerConfig({ + scope, + globalDocker: agent?.docker, + agentDocker: agentSandbox?.docker, + }); + + // SECURITY: Log warning if network is explicitly enabled + if (dockerConfig.network && dockerConfig.network !== "none") { + logInfo( + `[security] Sandbox network enabled (${dockerConfig.network}) for agent ${agentId ?? "default"}. ` + + "Consider using network=none for better isolation." + ); + } + return { mode: agentSandbox?.mode ?? agent?.mode ?? "off", scope, workspaceAccess: agentSandbox?.workspaceAccess ?? agent?.workspaceAccess ?? "none", workspaceRoot: agentSandbox?.workspaceRoot ?? agent?.workspaceRoot ?? DEFAULT_SANDBOX_WORKSPACE_ROOT, - docker: resolveSandboxDockerConfig({ - scope, - globalDocker: agent?.docker, - agentDocker: agentSandbox?.docker, - }), + docker: dockerConfig, browser: resolveSandboxBrowserConfig({ scope, globalBrowser: agent?.browser, @@ -165,3 +176,68 @@ export function resolveSandboxConfigForAgent(cfg?: MoltbotConfig, agentId?: stri }), }; } + +/** + * SECURITY: Sandbox security validation types and functions. + */ +export type SandboxSecurityWarning = { + code: string; + message: string; + severity: "info" | "warn" | "critical"; +}; + +/** + * SECURITY: Validate sandbox configuration for potential security issues. + */ +export function validateSandboxSecurity(config: SandboxConfig): SandboxSecurityWarning[] { + const warnings: SandboxSecurityWarning[] = []; + + // Check network isolation + if (config.docker.network && config.docker.network !== "none") { + warnings.push({ + code: "sandbox.network_enabled", + message: `Sandbox has network access (${config.docker.network}). ` + + "This allows the sandboxed agent to make network requests.", + severity: "warn", + }); + } + + // Check if root filesystem is writable + if (config.docker.readOnlyRoot === false) { + warnings.push({ + code: "sandbox.writable_root", + message: "Sandbox root filesystem is writable. Consider enabling readOnlyRoot.", + severity: "warn", + }); + } + + // Check if all capabilities are dropped + if (!config.docker.capDrop || !config.docker.capDrop.includes("ALL")) { + warnings.push({ + code: "sandbox.capabilities_not_dropped", + message: "Sandbox may retain Linux capabilities. Consider capDrop: ['ALL'].", + severity: "warn", + }); + } + + // Check workspace access level + if (config.workspaceAccess === "rw") { + warnings.push({ + code: "sandbox.workspace_rw", + message: "Sandbox has read-write access to workspace. " + + "Consider 'ro' (read-only) if writes aren't needed.", + severity: "info", + }); + } + + // Check browser host control + if (config.browser.allowHostControl) { + warnings.push({ + code: "sandbox.browser_host_control", + message: "Sandbox can control host browser. This bypasses sandbox isolation for browser actions.", + severity: "warn", + }); + } + + return warnings; +} diff --git a/src/security/file-permissions.ts b/src/security/file-permissions.ts new file mode 100644 index 000000000..a110fd0ad --- /dev/null +++ b/src/security/file-permissions.ts @@ -0,0 +1,252 @@ +import fs from "node:fs"; +import path from "node:path"; +import os from "node:os"; + +/** + * SECURITY: File Permission Enforcement Module + * + * Ensures sensitive files and directories have appropriate permissions. + * Enforces restrictive permissions on configuration, credentials, and state files. + */ + +export interface PermissionCheck { + path: string; + exists: boolean; + mode?: number; + isDirectory?: boolean; + isSymlink?: boolean; + owner?: number; + group?: number; + worldReadable?: boolean; + worldWritable?: boolean; + groupWritable?: boolean; +} + +export interface PermissionViolation { + path: string; + issue: string; + severity: "warn" | "critical"; + remediation: string; +} + +/** + * Check permissions on a file or directory. + */ +export function checkPermissions(filePath: string): PermissionCheck { + const result: PermissionCheck = { + path: filePath, + exists: false, + }; + + try { + const stats = fs.lstatSync(filePath); + result.exists = true; + result.mode = stats.mode; + result.isDirectory = stats.isDirectory(); + result.isSymlink = stats.isSymbolicLink(); + result.owner = stats.uid; + result.group = stats.gid; + + // Check permission bits (Unix-style) + const perms = stats.mode & 0o777; + result.worldReadable = (perms & 0o004) !== 0; + result.worldWritable = (perms & 0o002) !== 0; + result.groupWritable = (perms & 0o020) !== 0; + } catch { + // File doesn't exist or can't be accessed + } + + return result; +} + +/** + * Enforce restrictive permissions on a file (0600). + */ +export function enforceFilePermissions( + filePath: string, + mode: number = 0o600 +): { changed: boolean; error?: string } { + try { + if (!fs.existsSync(filePath)) { + return { changed: false }; + } + + const stats = fs.statSync(filePath); + const currentMode = stats.mode & 0o777; + + if (currentMode !== mode) { + fs.chmodSync(filePath, mode); + return { changed: true }; + } + + return { changed: false }; + } catch (err) { + return { + changed: false, + error: err instanceof Error ? err.message : String(err), + }; + } +} + +/** + * Enforce restrictive permissions on a directory (0700). + */ +export function enforceDirectoryPermissions( + dirPath: string, + mode: number = 0o700 +): { changed: boolean; error?: string } { + try { + if (!fs.existsSync(dirPath)) { + fs.mkdirSync(dirPath, { recursive: true, mode }); + return { changed: true }; + } + + const stats = fs.statSync(dirPath); + if (!stats.isDirectory()) { + return { changed: false, error: "Path is not a directory" }; + } + + const currentMode = stats.mode & 0o777; + if (currentMode !== mode) { + fs.chmodSync(dirPath, mode); + return { changed: true }; + } + + return { changed: false }; + } catch (err) { + return { + changed: false, + error: err instanceof Error ? err.message : String(err), + }; + } +} + +/** + * Sensitive paths that should have restrictive permissions. + */ +export function getSensitivePaths(stateDir?: string): string[] { + const homeDir = os.homedir(); + const resolvedStateDir = stateDir || path.join(homeDir, ".clawdbot"); + + return [ + // State directory + resolvedStateDir, + // Config file + path.join(resolvedStateDir, "clawdbot.json"), + // Credentials directory + path.join(resolvedStateDir, "credentials"), + // OAuth file + path.join(resolvedStateDir, "credentials", "oauth.json"), + // Auth profiles + path.join(resolvedStateDir, "auth-profiles.json"), + // Exec approvals + path.join(resolvedStateDir, "exec-approvals.json"), + // Pairing files (wildcard - check directory) + path.join(resolvedStateDir, "credentials"), + // Audit logs + path.join(resolvedStateDir, "audit"), + // Session data + path.join(resolvedStateDir, "sessions"), + ]; +} + +/** + * Validate permissions on all sensitive paths. + */ +export function validateSensitivePermissions(stateDir?: string): PermissionViolation[] { + const violations: PermissionViolation[] = []; + const paths = getSensitivePaths(stateDir); + + for (const filePath of paths) { + const check = checkPermissions(filePath); + + if (!check.exists) continue; + + // Check for world-writable + if (check.worldWritable) { + violations.push({ + path: filePath, + issue: "World-writable permissions", + severity: "critical", + remediation: check.isDirectory + ? `chmod 700 "${filePath}"` + : `chmod 600 "${filePath}"`, + }); + } + + // Check for world-readable on sensitive files + if (check.worldReadable && !check.isDirectory) { + violations.push({ + path: filePath, + issue: "World-readable permissions on sensitive file", + severity: "warn", + remediation: `chmod 600 "${filePath}"`, + }); + } + + // Check for group-writable + if (check.groupWritable) { + violations.push({ + path: filePath, + issue: "Group-writable permissions", + severity: "warn", + remediation: check.isDirectory + ? `chmod 700 "${filePath}"` + : `chmod 600 "${filePath}"`, + }); + } + + // Check for symlinks (potential attack vector) + if (check.isSymlink) { + violations.push({ + path: filePath, + issue: "Sensitive path is a symlink (potential security risk)", + severity: "warn", + remediation: "Verify symlink target is trusted", + }); + } + } + + return violations; +} + +/** + * Enforce restrictive permissions on all sensitive paths. + * Returns the number of paths that were modified. + */ +export function enforceSensitivePermissions(stateDir?: string): { + modified: number; + errors: Array<{ path: string; error: string }>; +} { + const paths = getSensitivePaths(stateDir); + let modified = 0; + const errors: Array<{ path: string; error: string }> = []; + + for (const filePath of paths) { + if (!fs.existsSync(filePath)) continue; + + try { + const stats = fs.statSync(filePath); + const isDir = stats.isDirectory(); + const targetMode = isDir ? 0o700 : 0o600; + + const result = isDir + ? enforceDirectoryPermissions(filePath, targetMode) + : enforceFilePermissions(filePath, targetMode); + + if (result.changed) { + modified += 1; + } + if (result.error) { + errors.push({ path: filePath, error: result.error }); + } + } catch (err) { + errors.push({ + path: filePath, + error: err instanceof Error ? err.message : String(err), + }); + } + } + + return { modified, errors }; +} diff --git a/src/security/index.ts b/src/security/index.ts new file mode 100644 index 000000000..4d31f74e8 --- /dev/null +++ b/src/security/index.ts @@ -0,0 +1,78 @@ +/** + * SECURITY: Core security module exports + * + * This module provides centralized access to all security utilities. + */ + +// Audit logging +export { + type AuditEventType, + type AuditLogEntry, + configureAuditLog, + logSessionStart, + logSessionEnd, + logAuthFailure, + logToolInvoke, + logToolDenied, + logExecRun, + logDangerousCommandBlocked, + logPairingEvent, + logConfigChange, + logSecretDetected, + cleanupOldLogs, +} from "./audit-log.js"; + +// Dangerous command detection +export { + type DangerousCommandMatch, + detectDangerousCommand, + formatDangerousCommandError, +} from "./dangerous-commands.js"; + +// External content handling +export { + type ExternalContentSource, + type WrapExternalContentOptions, + type WebContentSource, + type WrapWebContentOptions, + detectSuspiciousPatterns, + wrapExternalContent, + buildSafeExternalPrompt, + isExternalHookSession, + getHookType, + wrapWebContent, + tagSearchSnippet, +} from "./external-content.js"; + +// File permissions +export { + type PermissionCheck, + type PermissionViolation, + checkPermissions, + enforceFilePermissions, + enforceDirectoryPermissions, + getSensitivePaths, + validateSensitivePermissions, + enforceSensitivePermissions, +} from "./file-permissions.js"; + +// Rate limiting +export { + type RateLimitConfig, + RateLimiter, + sessionMessageLimiter, + toolInvocationLimiter, + execCommandLimiter, + gatewayApiLimiter, + authAttemptLimiter, + cleanupAllRateLimiters, + startRateLimitCleanup, + stopRateLimitCleanup, +} from "./rate-limit.js"; + +// Secret guard +export { + ensureFilePermissions600, + assertNoSecretsInFile, + assertNoSecretsInConfig, +} from "./secret-guard.js"; diff --git a/src/security/rate-limit.ts b/src/security/rate-limit.ts new file mode 100644 index 000000000..a1693193b --- /dev/null +++ b/src/security/rate-limit.ts @@ -0,0 +1,185 @@ +/** + * SECURITY: Rate Limiting Module + * + * Provides rate limiting for various security-sensitive operations + * to prevent abuse and brute-force attacks. + */ + +export interface RateLimitConfig { + /** Maximum number of requests allowed in the window */ + maxRequests: number; + /** Time window in milliseconds */ + windowMs: number; + /** Optional: block duration after limit exceeded (ms) */ + blockDurationMs?: number; +} + +interface RateLimitEntry { + count: number; + windowStart: number; + blockedUntil?: number; +} + +/** + * In-memory rate limiter with configurable windows and blocking. + */ +export class RateLimiter { + private entries = new Map(); + private config: RateLimitConfig; + + constructor(config: RateLimitConfig) { + this.config = config; + } + + /** + * Check if a request is allowed for the given key. + * Returns true if allowed, false if rate limited. + */ + check(key: string): boolean { + const now = Date.now(); + const entry = this.entries.get(key); + + // Check if currently blocked + if (entry?.blockedUntil && now < entry.blockedUntil) { + return false; + } + + // No entry or window expired - start fresh + if (!entry || now - entry.windowStart > this.config.windowMs) { + this.entries.set(key, { + count: 1, + windowStart: now, + blockedUntil: undefined, + }); + return true; + } + + // Within window - check count + if (entry.count >= this.config.maxRequests) { + // Apply block if configured + if (this.config.blockDurationMs) { + entry.blockedUntil = now + this.config.blockDurationMs; + } + return false; + } + + // Increment and allow + entry.count += 1; + return true; + } + + /** + * Get remaining requests for a key in the current window. + */ + remaining(key: string): number { + const now = Date.now(); + const entry = this.entries.get(key); + + if (!entry || now - entry.windowStart > this.config.windowMs) { + return this.config.maxRequests; + } + + if (entry.blockedUntil && now < entry.blockedUntil) { + return 0; + } + + return Math.max(0, this.config.maxRequests - entry.count); + } + + /** + * Reset rate limit for a key. + */ + reset(key: string): void { + this.entries.delete(key); + } + + /** + * Clean up expired entries to prevent memory leaks. + */ + cleanup(): number { + const now = Date.now(); + let cleaned = 0; + + for (const [key, entry] of this.entries) { + const windowExpired = now - entry.windowStart > this.config.windowMs; + const blockExpired = !entry.blockedUntil || now >= entry.blockedUntil; + + if (windowExpired && blockExpired) { + this.entries.delete(key); + cleaned += 1; + } + } + + return cleaned; + } +} + +/** + * Pre-configured rate limiters for common use cases. + */ + +// Session message rate limiting (prevent spam) +export const sessionMessageLimiter = new RateLimiter({ + maxRequests: 60, // 60 messages + windowMs: 60 * 1000, // per minute + blockDurationMs: 30 * 1000, // 30 second block after limit +}); + +// Tool invocation rate limiting +export const toolInvocationLimiter = new RateLimiter({ + maxRequests: 100, // 100 tool calls + windowMs: 60 * 1000, // per minute +}); + +// Exec command rate limiting +export const execCommandLimiter = new RateLimiter({ + maxRequests: 30, // 30 commands + windowMs: 60 * 1000, // per minute + blockDurationMs: 60 * 1000, // 1 minute block +}); + +// Gateway API rate limiting (for external callers) +export const gatewayApiLimiter = new RateLimiter({ + maxRequests: 120, // 120 requests + windowMs: 60 * 1000, // per minute +}); + +// Auth attempt rate limiting +export const authAttemptLimiter = new RateLimiter({ + maxRequests: 5, // 5 attempts + windowMs: 60 * 1000, // per minute + blockDurationMs: 5 * 60 * 1000, // 5 minute block +}); + +/** + * Periodic cleanup for all rate limiters. + * Should be called periodically (e.g., every 5 minutes). + */ +export function cleanupAllRateLimiters(): number { + let total = 0; + total += sessionMessageLimiter.cleanup(); + total += toolInvocationLimiter.cleanup(); + total += execCommandLimiter.cleanup(); + total += gatewayApiLimiter.cleanup(); + total += authAttemptLimiter.cleanup(); + return total; +} + +// Start automatic cleanup every 5 minutes +let cleanupInterval: NodeJS.Timeout | null = null; + +export function startRateLimitCleanup(): void { + if (cleanupInterval) return; + cleanupInterval = setInterval(() => { + cleanupAllRateLimiters(); + }, 5 * 60 * 1000); + // Don't prevent process exit + cleanupInterval.unref(); +} + +export function stopRateLimitCleanup(): void { + if (cleanupInterval) { + clearInterval(cleanupInterval); + cleanupInterval = null; + } +}