diff --git a/src/security/events/aggregator.ts b/src/security/events/aggregator.ts index 5478171a9..a98c136ae 100644 --- a/src/security/events/aggregator.ts +++ b/src/security/events/aggregator.ts @@ -61,6 +61,9 @@ export class SecurityEventAggregator { // Filter out events outside the time window count.events = count.events.filter((e) => new Date(e.timestamp).getTime() > windowStart); + // Store previous count before adding new event + const previousCount = count.events.length; + // Add new event count.events.push(event); count.count = count.events.length; @@ -71,8 +74,8 @@ export class SecurityEventAggregator { count.firstSeen = new Date(count.events[0].timestamp).getTime(); } - // Check if threshold crossed - return count.count >= threshold; + // Return true only when threshold is FIRST crossed (not on subsequent events) + return previousCount < threshold && count.count >= threshold; } /** diff --git a/src/security/intrusion-detector.test.ts b/src/security/intrusion-detector.test.ts index ae0f83b8c..f4287fd23 100644 --- a/src/security/intrusion-detector.test.ts +++ b/src/security/intrusion-detector.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it, beforeEach, vi, afterEach } from "vitest"; import { IntrusionDetector } from "./intrusion-detector.js"; import { SecurityActions, AttackPatterns, type SecurityEvent } from "./events/schema.js"; import { ipManager } from "./ip-manager.js"; +import { securityEventAggregator } from "./events/aggregator.js"; vi.mock("./ip-manager.js", () => ({ ipManager: { @@ -15,6 +16,7 @@ describe("IntrusionDetector", () => { beforeEach(() => { vi.clearAllMocks(); + securityEventAggregator.clearAll(); // Clear event state between tests detector = new IntrusionDetector({ enabled: true, patterns: { @@ -313,6 +315,7 @@ describe("IntrusionDetector", () => { it("should respect custom time windows", () => { vi.useFakeTimers(); + vi.setSystemTime(0); // Start at time 0 const customDetector = new IntrusionDetector({ enabled: true, diff --git a/src/security/ip-manager.test.ts b/src/security/ip-manager.test.ts index 7cae4b00f..ebd66b6b4 100644 --- a/src/security/ip-manager.test.ts +++ b/src/security/ip-manager.test.ts @@ -3,6 +3,10 @@ import { IpManager } from "./ip-manager.js"; vi.mock("node:fs", () => ({ default: { + existsSync: vi.fn().mockReturnValue(false), + readFileSync: vi.fn().mockReturnValue("{}"), + writeFileSync: vi.fn(), + mkdirSync: vi.fn(), promises: { mkdir: vi.fn().mockResolvedValue(undefined), writeFile: vi.fn().mockResolvedValue(undefined), @@ -265,7 +269,7 @@ describe("IpManager", () => { durationMs: 86400000, }); - const blocklist = manager.getBlocklist(); + const blocklist = manager.getBlockedIps(); expect(blocklist).toHaveLength(2); expect(blocklist.map((b) => b.ip)).toContain("192.168.1.1"); expect(blocklist.map((b) => b.ip)).toContain("192.168.1.2"); @@ -279,7 +283,7 @@ describe("IpManager", () => { durationMs: 86400000, }); - const blocklist = manager.getBlocklist(); + const blocklist = manager.getBlockedIps(); expect(blocklist[0]?.expiresAt).toBeDefined(); expect(new Date(blocklist[0]!.expiresAt).getTime()).toBeGreaterThan(now.getTime()); }); @@ -297,7 +301,7 @@ describe("IpManager", () => { reason: "trusted2", }); - const allowlist = manager.getAllowlist(); + const allowlist = manager.getAllowedIps(); expect(allowlist).toHaveLength(2); expect(allowlist.map((a) => a.ip)).toContain("192.168.1.100"); expect(allowlist.map((a) => a.ip)).toContain("10.0.0.0/8"); diff --git a/src/security/ip-manager.ts b/src/security/ip-manager.ts index 8204348e0..8bfdcd5c2 100644 --- a/src/security/ip-manager.ts +++ b/src/security/ip-manager.ts @@ -170,7 +170,7 @@ export class IpManager { const now = new Date().toISOString(); for (const entry of this.store.blocklist) { - if (entry.ip === ip && entry.expiresAt > now) { + if (ipMatchesCidr(ip, entry.ip) && entry.expiresAt > now) { return entry.reason; } } @@ -361,7 +361,7 @@ export class IpManager { */ getBlocklistEntry(ip: string): BlocklistEntry | null { const now = new Date().toISOString(); - return this.store.blocklist.find((e) => e.ip === ip && e.expiresAt > now) ?? null; + return this.store.blocklist.find((e) => ipMatchesCidr(ip, e.ip) && e.expiresAt > now) ?? null; } /** diff --git a/src/security/rate-limiter.ts b/src/security/rate-limiter.ts index 5f9e11c82..99c9b2ac5 100644 --- a/src/security/rate-limiter.ts +++ b/src/security/rate-limiter.ts @@ -87,10 +87,11 @@ class LRUCache { * Rate limiter using token bucket algorithm */ export class RateLimiter { - private buckets = new LRUCache(MAX_CACHE_SIZE); + private buckets: LRUCache; private cleanupInterval: NodeJS.Timeout | null = null; - constructor() { + constructor(params?: { maxSize?: number }) { + this.buckets = new LRUCache(params?.maxSize ?? MAX_CACHE_SIZE); this.startCleanup(); } @@ -122,10 +123,10 @@ export class RateLimiter { const entry = this.buckets.get(key); if (!entry) { - // Not rate limited yet + // Not rate limited yet - full capacity available return { allowed: true, - remaining: limit.max - 1, + remaining: limit.max, resetAt: new Date(Date.now() + limit.windowMs), }; } diff --git a/src/security/shield.ts b/src/security/shield.ts index 78c27e1b0..3c848912a 100644 --- a/src/security/shield.ts +++ b/src/security/shield.ts @@ -432,7 +432,9 @@ export class SecurityShield { // Try X-Forwarded-For first (if behind proxy) const forwarded = req.headers["x-forwarded-for"]; if (forwarded) { - const ips = typeof forwarded === "string" ? forwarded.split(",") : forwarded; + // Handle both string and array cases (array can come from some proxies) + const forwardedStr = Array.isArray(forwarded) ? forwarded[0] : forwarded; + const ips = typeof forwardedStr === "string" ? forwardedStr.split(",") : []; const clientIp = ips[0]?.trim(); if (clientIp) return clientIp; } diff --git a/src/security/token-bucket.ts b/src/security/token-bucket.ts index 16364f1b1..0892771ba 100644 --- a/src/security/token-bucket.ts +++ b/src/security/token-bucket.ts @@ -54,6 +54,11 @@ export class TokenBucket { return 0; } + // If count exceeds capacity, we can never fulfill this request + if (count > this.config.capacity) { + return Infinity; + } + const tokensNeeded = count - this.tokens; return Math.ceil(tokensNeeded / this.config.refillRate); }