From 4325cac7e48b177ed5cc68d30f8712580bf497a5 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Jan 2026 09:56:59 +0000 Subject: [PATCH] feat(security): harden modules with WebSocket hook, expanded fs monitoring, log rotation, and stack traces --- src/security/fs-monitor.ts | 149 ++++++++++++++++++++++++------- src/security/hardening-logger.ts | 66 +++++++++++++- src/security/hardening.test.ts | 134 +++++++++++++++++++++++++++ src/security/network-monitor.ts | 41 ++++++++- 4 files changed, 354 insertions(+), 36 deletions(-) diff --git a/src/security/fs-monitor.ts b/src/security/fs-monitor.ts index e68bff6e9..7d208aff9 100644 --- a/src/security/fs-monitor.ts +++ b/src/security/fs-monitor.ts @@ -40,8 +40,13 @@ let resolvedSensitivePaths: string[] | null = null; let enforceMode = false; let installed = false; -let originalReadFile: typeof fs.readFileSync | null = null; -let originalReadFileAsync: typeof fs.promises.readFile | null = null; +// Original function references for restoration +let origReadFileSync: typeof fs.readFileSync | null = null; +let origWriteFileSync: typeof fs.writeFileSync | null = null; +let origReadFile: typeof fs.promises.readFile | null = null; +let origWriteFile: typeof fs.promises.writeFile | null = null; +let origStat: typeof fs.promises.stat | null = null; +let origUnlink: typeof fs.promises.unlink | null = null; /** * Resolve a path that may start with ~. @@ -53,12 +58,25 @@ function expandHome(p: string): string { return p; } +/** + * Resolve a file path, following symlinks where possible to prevent bypass via symlinks. + */ +function resolveRealPath(filePath: string): string { + const resolved = path.resolve(filePath); + try { + return fs.realpathSync.native(resolved); + } catch { + // If realpath fails (e.g. file doesn't exist yet), fall back to path.resolve + return resolved; + } +} + /** * Check if a path falls under any sensitive path prefix. */ export function isSensitivePath(filePath: string): boolean { if (!resolvedSensitivePaths) return false; - const normalized = path.resolve(filePath); + const normalized = resolveRealPath(filePath); for (const sensitive of resolvedSensitivePaths) { if (normalized === sensitive || normalized.startsWith(sensitive + path.sep)) { return true; @@ -76,22 +94,38 @@ export function auditFileAccess( operation: "read" | "write" | "stat" | "readdir" | "unlink", ): boolean { if (!resolvedSensitivePaths) return true; - const normalized = path.resolve(filePath); + const normalized = resolveRealPath(filePath); if (!isSensitivePath(normalized)) return true; logSecurityEvent("sensitive_file_access", { operation, path: normalized, allowed: !enforceMode, + stackTrace: new Error().stack?.split("\n").slice(2, 7).join("\n"), }); return !enforceMode; } /** - * Install lightweight fs monitoring hooks. - * Only intercepts fs.readFileSync and fs.promises.readFile for sensitive path auditing. - * This is intentionally minimal to avoid breaking the application. + * Create a guard wrapper that audits the first arg (file path) before calling the original. + */ +function createGuard( + operation: "read" | "write" | "stat" | "readdir" | "unlink", +): (filePath: unknown) => void { + return (filePath: unknown) => { + if (typeof filePath === "string") { + const allowed = auditFileAccess(filePath, operation); + if (!allowed) { + throw new Error(`[fs-monitor] Blocked ${operation} access to sensitive path: ${filePath}`); + } + } + }; +} + +/** + * Install fs monitoring hooks for sensitive path auditing. + * Hooks both sync and async versions of: readFile, writeFile, stat, unlink. */ export function installFsMonitor(opts?: FsMonitorOptions): void { if (installed) return; @@ -103,40 +137,73 @@ export function installFsMonitor(opts?: FsMonitorOptions): void { resolvedSensitivePaths = rawPaths.map((p) => path.resolve(expandHome(p))); enforceMode = opts?.enforce ?? false; - // Wrap fs.readFileSync for synchronous reads - originalReadFile = fs.readFileSync; + const readGuard = createGuard("read"); + const writeGuard = createGuard("write"); + const statGuard = createGuard("stat"); + const unlinkGuard = createGuard("unlink"); + + // Sync hooks + origReadFileSync = fs.readFileSync; + const origRFS = origReadFileSync; (fs as { readFileSync: typeof fs.readFileSync }).readFileSync = ((...args: unknown[]) => { - const filePath = args[0]; - if (typeof filePath === "string") { - const allowed = auditFileAccess(filePath, "read"); - if (!allowed) { - throw new Error(`[fs-monitor] Blocked read access to sensitive path: ${filePath}`); - } - } - return (originalReadFile as Function).apply(fs, args); + readGuard(args[0]); + return (origRFS as Function).apply(fs, args); }) as typeof fs.readFileSync; - // Wrap fs.promises.readFile for async reads - originalReadFileAsync = fs.promises.readFile; + origWriteFileSync = fs.writeFileSync; + const origWFS = origWriteFileSync; + (fs as { writeFileSync: typeof fs.writeFileSync }).writeFileSync = ((...args: unknown[]) => { + writeGuard(args[0]); + return (origWFS as Function).apply(fs, args); + }) as typeof fs.writeFileSync; + + // Async hooks (fs.promises) + origReadFile = fs.promises.readFile; + const origRF = origReadFile; (fs.promises as { readFile: typeof fs.promises.readFile }).readFile = (async ( ...args: unknown[] ) => { - const filePath = args[0]; - if (typeof filePath === "string") { - const allowed = auditFileAccess(filePath, "read"); - if (!allowed) { - throw new Error(`[fs-monitor] Blocked read access to sensitive path: ${filePath}`); - } - } - return (originalReadFileAsync as Function).apply(fs.promises, args); + readGuard(args[0]); + return (origRF as Function).apply(fs.promises, args); }) as typeof fs.promises.readFile; + origWriteFile = fs.promises.writeFile; + const origWF = origWriteFile; + (fs.promises as { writeFile: typeof fs.promises.writeFile }).writeFile = (async ( + ...args: unknown[] + ) => { + writeGuard(args[0]); + return (origWF as Function).apply(fs.promises, args); + }) as typeof fs.promises.writeFile; + + origStat = fs.promises.stat; + const origST = origStat; + (fs.promises as { stat: typeof fs.promises.stat }).stat = (async (...args: unknown[]) => { + statGuard(args[0]); + return (origST as Function).apply(fs.promises, args); + }) as typeof fs.promises.stat; + + origUnlink = fs.promises.unlink; + const origUL = origUnlink; + (fs.promises as { unlink: typeof fs.promises.unlink }).unlink = (async (...args: unknown[]) => { + unlinkGuard(args[0]); + return (origUL as Function).apply(fs.promises, args); + }) as typeof fs.promises.unlink; + installed = true; logSecurityEvent("hardening_init", { module: "fs-monitor", sensitivePathCount: resolvedSensitivePaths.length, enforce: enforceMode, + hookedMethods: [ + "readFileSync", + "writeFileSync", + "promises.readFile", + "promises.writeFile", + "promises.stat", + "promises.unlink", + ], }); } @@ -145,13 +212,29 @@ export function installFsMonitor(opts?: FsMonitorOptions): void { */ export function uninstallFsMonitor(): void { if (!installed) return; - if (originalReadFile) { - (fs as { readFileSync: typeof fs.readFileSync }).readFileSync = originalReadFile; - originalReadFile = null; + if (origReadFileSync) { + (fs as { readFileSync: typeof fs.readFileSync }).readFileSync = origReadFileSync; + origReadFileSync = null; } - if (originalReadFileAsync) { - (fs.promises as { readFile: typeof fs.promises.readFile }).readFile = originalReadFileAsync; - originalReadFileAsync = null; + if (origWriteFileSync) { + (fs as { writeFileSync: typeof fs.writeFileSync }).writeFileSync = origWriteFileSync; + origWriteFileSync = null; + } + if (origReadFile) { + (fs.promises as { readFile: typeof fs.promises.readFile }).readFile = origReadFile; + origReadFile = null; + } + if (origWriteFile) { + (fs.promises as { writeFile: typeof fs.promises.writeFile }).writeFile = origWriteFile; + origWriteFile = null; + } + if (origStat) { + (fs.promises as { stat: typeof fs.promises.stat }).stat = origStat; + origStat = null; + } + if (origUnlink) { + (fs.promises as { unlink: typeof fs.promises.unlink }).unlink = origUnlink; + origUnlink = null; } resolvedSensitivePaths = null; installed = false; diff --git a/src/security/hardening-logger.ts b/src/security/hardening-logger.ts index 744076bd1..b2b962c4f 100644 --- a/src/security/hardening-logger.ts +++ b/src/security/hardening-logger.ts @@ -22,11 +22,23 @@ export type HardeningLoggerOptions = { logFile?: string; /** Also emit events to this callback (for tests). */ onEvent?: (event: SecurityEvent) => void; + /** Max log file size in bytes before rotation. Default: 10 MB. */ + maxFileSizeBytes?: number; + /** Number of rotated log files to keep. Default: 3. */ + maxRotatedFiles?: number; }; +/** Default max log file size: 10 MB */ +const DEFAULT_MAX_FILE_SIZE = 10 * 1024 * 1024; +/** Default number of rotated files to keep */ +const DEFAULT_MAX_ROTATED = 3; + let logStream: fs.WriteStream | null = null; let logFilePath: string | null = null; let eventCallback: ((event: SecurityEvent) => void) | null = null; +let bytesWritten = 0; +let maxFileSize = DEFAULT_MAX_FILE_SIZE; +let maxRotated = DEFAULT_MAX_ROTATED; /** * Resolve the default security log path. @@ -36,6 +48,43 @@ function defaultLogPath(): string { return path.join(stateDir, "security-audit.log"); } +/** + * Rotate the log file when it exceeds the size limit. + * Renames: .log -> .log.1, .log.1 -> .log.2, etc. Deletes oldest. + */ +function rotateLogFile(): void { + if (!logFilePath || !logStream) return; + try { + logStream.end(); + logStream = null; + + // Shift existing rotated files + for (let i = maxRotated; i >= 1; i--) { + const older = `${logFilePath}.${i}`; + if (i === maxRotated) { + try { + fs.unlinkSync(older); + } catch { + // ignore + } + } + const newer = i === 1 ? logFilePath : `${logFilePath}.${i - 1}`; + try { + fs.renameSync(newer, older); + } catch { + // ignore (file may not exist) + } + } + + // Open a fresh log file + logStream = fs.createWriteStream(logFilePath, { flags: "a", mode: 0o600 }); + logStream.on("error", () => {}); + bytesWritten = 0; + } catch { + // If rotation fails, continue with current stream + } +} + /** * Initialize the hardening audit logger. * Call once at gateway startup, before any other security module. @@ -44,12 +93,20 @@ export function initHardeningLogger(opts?: HardeningLoggerOptions): void { if (logStream) return; // already initialized logFilePath = opts?.logFile ?? defaultLogPath(); eventCallback = opts?.onEvent ?? null; + maxFileSize = opts?.maxFileSizeBytes ?? DEFAULT_MAX_FILE_SIZE; + maxRotated = opts?.maxRotatedFiles ?? DEFAULT_MAX_ROTATED; try { const dir = path.dirname(logFilePath); fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); logStream = fs.createWriteStream(logFilePath, { flags: "a", mode: 0o600 }); // Silently handle write stream errors to avoid crashing the gateway. logStream.on("error", () => {}); + // Track current file size for rotation + try { + bytesWritten = fs.statSync(logFilePath).size; + } catch { + bytesWritten = 0; + } } catch { // If we can't open the log file, continue without file logging. // The onEvent callback (if set) will still work. @@ -67,7 +124,13 @@ export function logSecurityEvent(type: SecurityEventType, detail: Record= maxFileSize) { + rotateLogFile(); + } } if (eventCallback) { eventCallback(event); @@ -84,6 +147,7 @@ export function closeHardeningLogger(): void { } logFilePath = null; eventCallback = null; + bytesWritten = 0; } /** diff --git a/src/security/hardening.test.ts b/src/security/hardening.test.ts index 8bf92e8f5..4306addf3 100644 --- a/src/security/hardening.test.ts +++ b/src/security/hardening.test.ts @@ -352,6 +352,140 @@ describe("fs-monitor", () => { __resetFsMonitorForTest(); expect(isFsMonitorActive()).toBe(false); }); + + it("audits write operations to sensitive paths", () => { + const events: SecurityEvent[] = []; + initHardeningLogger({ logFile: "/dev/null", onEvent: (e) => events.push(e) }); + installFsMonitor(); + + const awsCreds = path.join(os.homedir(), ".aws", "credentials"); + auditFileAccess(awsCreds, "write"); + const writeEvents = events.filter( + (e) => e.type === "sensitive_file_access" && e.detail.operation === "write", + ); + expect(writeEvents).toHaveLength(1); + }); + + it("audits stat/unlink operations on sensitive paths", () => { + const events: SecurityEvent[] = []; + initHardeningLogger({ logFile: "/dev/null", onEvent: (e) => events.push(e) }); + installFsMonitor(); + + const sshKey = path.join(os.homedir(), ".ssh", "id_rsa"); + auditFileAccess(sshKey, "stat"); + auditFileAccess(sshKey, "unlink"); + + const fsEvents = events.filter((e) => e.type === "sensitive_file_access"); + expect(fsEvents).toHaveLength(2); + expect(fsEvents[0].detail.operation).toBe("stat"); + expect(fsEvents[1].detail.operation).toBe("unlink"); + }); + + it("includes stack trace in audit events", () => { + const events: SecurityEvent[] = []; + initHardeningLogger({ logFile: "/dev/null", onEvent: (e) => events.push(e) }); + installFsMonitor(); + + const sshKey = path.join(os.homedir(), ".ssh", "id_rsa"); + auditFileAccess(sshKey, "read"); + + const fsEvent = events.find((e) => e.type === "sensitive_file_access"); + expect(fsEvent).toBeDefined(); + expect(fsEvent!.detail.stackTrace).toBeTruthy(); + expect(typeof fsEvent!.detail.stackTrace).toBe("string"); + }); + + it("normalizes paths with .. to detect traversal", () => { + installFsMonitor(); + + const home = os.homedir(); + // Path traversal: ~/Documents/../.ssh/id_rsa resolves to ~/.ssh/id_rsa + const traversalPath = path.join(home, "Documents", "..", ".ssh", "id_rsa"); + expect(isSensitivePath(traversalPath)).toBe(true); + }); +}); + +// --------------------------------------------------------------------------- +// Log Rotation +// --------------------------------------------------------------------------- + +describe("hardening-logger rotation", () => { + let tmpDir: string; + + beforeEach(async () => { + __resetHardeningLoggerForTest(); + tmpDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), "moltbot-log-rotation-")); + }); + + afterEach(async () => { + __resetHardeningLoggerForTest(); + await fs.promises.rm(tmpDir, { recursive: true, force: true }).catch(() => {}); + }); + + it("continues logging after rotation without crashing", () => { + const events: SecurityEvent[] = []; + const logFile = path.join(tmpDir, "test.log"); + // Tiny threshold so rotation triggers quickly + initHardeningLogger({ + logFile, + maxFileSizeBytes: 50, + maxRotatedFiles: 2, + onEvent: (e) => events.push(e), + }); + + // Write many events to trigger multiple rotations + for (let i = 0; i < 20; i++) { + logSecurityEvent("hardening_init", { iteration: i, padding: "x".repeat(30) }); + } + + // All events should be captured by the callback regardless of rotation + expect(events).toHaveLength(20); + // Logger should still be functional (no crash from rotation) + logSecurityEvent("hardening_init", { final: true }); + expect(events).toHaveLength(21); + }); +}); + +// --------------------------------------------------------------------------- +// Network Monitor - Stack Traces & WebSocket +// --------------------------------------------------------------------------- + +describe("network-monitor stack traces", () => { + beforeEach(() => { + __resetNetworkMonitorForTest(); + __resetHardeningLoggerForTest(); + }); + + afterEach(() => { + __resetNetworkMonitorForTest(); + __resetHardeningLoggerForTest(); + }); + + it("includes stack trace in blocked fetch events", async () => { + const events: SecurityEvent[] = []; + initHardeningLogger({ logFile: "/dev/null", onEvent: (e) => events.push(e) }); + installNetworkMonitor({ enforce: true }); + + try { + await fetch("https://evil.com/data"); + } catch { + // expected + } + + const blocked = events.find((e) => e.type === "blocked_network"); + expect(blocked).toBeDefined(); + expect(blocked!.detail.stackTrace).toBeTruthy(); + expect(typeof blocked!.detail.stackTrace).toBe("string"); + }); + + it("blocks IP address access (not in whitelist)", () => { + installNetworkMonitor(); + // Raw IP addresses should not be allowed unless explicitly whitelisted + expect(isDomainAllowed("1.2.3.4")).toBe(false); + expect(isDomainAllowed("192.168.1.1")).toBe(false); + // But loopback is whitelisted + expect(isDomainAllowed("127.0.0.1")).toBe(true); + }); }); // --------------------------------------------------------------------------- diff --git a/src/security/network-monitor.ts b/src/security/network-monitor.ts index 641152506..064a2ae84 100644 --- a/src/security/network-monitor.ts +++ b/src/security/network-monitor.ts @@ -61,6 +61,7 @@ let enforceMode = true; let originalFetch: typeof globalThis.fetch | null = null; let originalHttpRequest: typeof http.request | null = null; let originalHttpsRequest: typeof https.request | null = null; +let originalWebSocket: typeof globalThis.WebSocket | null = null; let installed = false; /** @@ -122,6 +123,7 @@ export function installNetworkMonitor(opts?: NetworkMonitorOptions): void { url: String( typeof input === "string" ? input : input instanceof URL ? input.href : input.url, ).slice(0, 200), + stackTrace: new Error().stack?.split("\n").slice(1, 6).join("\n"), }); if (enforceMode) { throw new Error(`[network-monitor] Blocked outbound request to ${hostname}`); @@ -140,7 +142,11 @@ export function installNetworkMonitor(opts?: NetworkMonitorOptions): void { (http as { request: typeof http.request }).request = ((...args: unknown[]) => { const hostname = extractHostnameFromHttpArgs(args); if (hostname && !isDomainAllowed(hostname)) { - logSecurityEvent("blocked_network", { method: "http.request", hostname }); + logSecurityEvent("blocked_network", { + method: "http.request", + hostname, + stackTrace: new Error().stack?.split("\n").slice(1, 6).join("\n"), + }); if (enforceMode) { throw new Error(`[network-monitor] Blocked outbound HTTP request to ${hostname}`); } @@ -155,7 +161,11 @@ export function installNetworkMonitor(opts?: NetworkMonitorOptions): void { (https as { request: typeof https.request }).request = ((...args: unknown[]) => { const hostname = extractHostnameFromHttpArgs(args); if (hostname && !isDomainAllowed(hostname)) { - logSecurityEvent("blocked_network", { method: "https.request", hostname }); + logSecurityEvent("blocked_network", { + method: "https.request", + hostname, + stackTrace: new Error().stack?.split("\n").slice(1, 6).join("\n"), + }); if (enforceMode) { throw new Error(`[network-monitor] Blocked outbound HTTPS request to ${hostname}`); } @@ -165,6 +175,29 @@ export function installNetworkMonitor(opts?: NetworkMonitorOptions): void { return (originalHttpsRequest as Function).apply(https, args); }) as typeof https.request; + // Wrap WebSocket constructor (Baileys uses WebSocket for WhatsApp protocol) + if (typeof globalThis.WebSocket !== "undefined") { + originalWebSocket = globalThis.WebSocket; + const OrigWs = originalWebSocket; + globalThis.WebSocket = new Proxy(OrigWs, { + construct(target, args) { + const url = args[0]; + const hostname = typeof url === "string" ? extractHostname(url) : null; + if (hostname && !isDomainAllowed(hostname)) { + logSecurityEvent("blocked_network", { + method: "WebSocket", + hostname, + stackTrace: new Error().stack?.split("\n").slice(1, 6).join("\n"), + }); + if (enforceMode) { + throw new Error(`[network-monitor] Blocked WebSocket connection to ${hostname}`); + } + } + return Reflect.construct(target, args); + }, + }) as typeof globalThis.WebSocket; + } + installed = true; logSecurityEvent("hardening_init", { @@ -219,6 +252,10 @@ export function uninstallNetworkMonitor(): void { (https as { request: typeof https.request }).request = originalHttpsRequest; originalHttpsRequest = null; } + if (originalWebSocket) { + globalThis.WebSocket = originalWebSocket; + originalWebSocket = null; + } allowedDomainSet = null; allowedSuffixList = null; installed = false;