test(security): fix failing tests
- Add CIDR matching to isBlocked() and getBlocklistEntry() methods - Fix event aggregator threshold logic to only trigger once on first crossing - Add securityEventAggregator.clearAll() in intrusion-detector tests - Fix RateLimiter constructor to accept custom maxSize parameter - Fix token bucket getRetryAfterMs() to return Infinity for impossible requests - Fix rate limiter peek() to return full capacity for non-existent keys - Fix shield extractIp() to handle array X-Forwarded-For headers - Fix ip-manager test mocks to include sync fs methods - All security tests now passing (173 tests across 8 files)
This commit is contained in:
parent
8f42141f75
commit
b10174ace0
@ -61,6 +61,9 @@ export class SecurityEventAggregator {
|
|||||||
// Filter out events outside the time window
|
// Filter out events outside the time window
|
||||||
count.events = count.events.filter((e) => new Date(e.timestamp).getTime() > windowStart);
|
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
|
// Add new event
|
||||||
count.events.push(event);
|
count.events.push(event);
|
||||||
count.count = count.events.length;
|
count.count = count.events.length;
|
||||||
@ -71,8 +74,8 @@ export class SecurityEventAggregator {
|
|||||||
count.firstSeen = new Date(count.events[0].timestamp).getTime();
|
count.firstSeen = new Date(count.events[0].timestamp).getTime();
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check if threshold crossed
|
// Return true only when threshold is FIRST crossed (not on subsequent events)
|
||||||
return count.count >= threshold;
|
return previousCount < threshold && count.count >= threshold;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@ -3,6 +3,7 @@ import { describe, expect, it, beforeEach, vi, afterEach } from "vitest";
|
|||||||
import { IntrusionDetector } from "./intrusion-detector.js";
|
import { IntrusionDetector } from "./intrusion-detector.js";
|
||||||
import { SecurityActions, AttackPatterns, type SecurityEvent } from "./events/schema.js";
|
import { SecurityActions, AttackPatterns, type SecurityEvent } from "./events/schema.js";
|
||||||
import { ipManager } from "./ip-manager.js";
|
import { ipManager } from "./ip-manager.js";
|
||||||
|
import { securityEventAggregator } from "./events/aggregator.js";
|
||||||
|
|
||||||
vi.mock("./ip-manager.js", () => ({
|
vi.mock("./ip-manager.js", () => ({
|
||||||
ipManager: {
|
ipManager: {
|
||||||
@ -15,6 +16,7 @@ describe("IntrusionDetector", () => {
|
|||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
vi.clearAllMocks();
|
vi.clearAllMocks();
|
||||||
|
securityEventAggregator.clearAll(); // Clear event state between tests
|
||||||
detector = new IntrusionDetector({
|
detector = new IntrusionDetector({
|
||||||
enabled: true,
|
enabled: true,
|
||||||
patterns: {
|
patterns: {
|
||||||
@ -313,6 +315,7 @@ describe("IntrusionDetector", () => {
|
|||||||
|
|
||||||
it("should respect custom time windows", () => {
|
it("should respect custom time windows", () => {
|
||||||
vi.useFakeTimers();
|
vi.useFakeTimers();
|
||||||
|
vi.setSystemTime(0); // Start at time 0
|
||||||
|
|
||||||
const customDetector = new IntrusionDetector({
|
const customDetector = new IntrusionDetector({
|
||||||
enabled: true,
|
enabled: true,
|
||||||
|
|||||||
@ -3,6 +3,10 @@ import { IpManager } from "./ip-manager.js";
|
|||||||
|
|
||||||
vi.mock("node:fs", () => ({
|
vi.mock("node:fs", () => ({
|
||||||
default: {
|
default: {
|
||||||
|
existsSync: vi.fn().mockReturnValue(false),
|
||||||
|
readFileSync: vi.fn().mockReturnValue("{}"),
|
||||||
|
writeFileSync: vi.fn(),
|
||||||
|
mkdirSync: vi.fn(),
|
||||||
promises: {
|
promises: {
|
||||||
mkdir: vi.fn().mockResolvedValue(undefined),
|
mkdir: vi.fn().mockResolvedValue(undefined),
|
||||||
writeFile: vi.fn().mockResolvedValue(undefined),
|
writeFile: vi.fn().mockResolvedValue(undefined),
|
||||||
@ -265,7 +269,7 @@ describe("IpManager", () => {
|
|||||||
durationMs: 86400000,
|
durationMs: 86400000,
|
||||||
});
|
});
|
||||||
|
|
||||||
const blocklist = manager.getBlocklist();
|
const blocklist = manager.getBlockedIps();
|
||||||
expect(blocklist).toHaveLength(2);
|
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.1");
|
||||||
expect(blocklist.map((b) => b.ip)).toContain("192.168.1.2");
|
expect(blocklist.map((b) => b.ip)).toContain("192.168.1.2");
|
||||||
@ -279,7 +283,7 @@ describe("IpManager", () => {
|
|||||||
durationMs: 86400000,
|
durationMs: 86400000,
|
||||||
});
|
});
|
||||||
|
|
||||||
const blocklist = manager.getBlocklist();
|
const blocklist = manager.getBlockedIps();
|
||||||
expect(blocklist[0]?.expiresAt).toBeDefined();
|
expect(blocklist[0]?.expiresAt).toBeDefined();
|
||||||
expect(new Date(blocklist[0]!.expiresAt).getTime()).toBeGreaterThan(now.getTime());
|
expect(new Date(blocklist[0]!.expiresAt).getTime()).toBeGreaterThan(now.getTime());
|
||||||
});
|
});
|
||||||
@ -297,7 +301,7 @@ describe("IpManager", () => {
|
|||||||
reason: "trusted2",
|
reason: "trusted2",
|
||||||
});
|
});
|
||||||
|
|
||||||
const allowlist = manager.getAllowlist();
|
const allowlist = manager.getAllowedIps();
|
||||||
expect(allowlist).toHaveLength(2);
|
expect(allowlist).toHaveLength(2);
|
||||||
expect(allowlist.map((a) => a.ip)).toContain("192.168.1.100");
|
expect(allowlist.map((a) => a.ip)).toContain("192.168.1.100");
|
||||||
expect(allowlist.map((a) => a.ip)).toContain("10.0.0.0/8");
|
expect(allowlist.map((a) => a.ip)).toContain("10.0.0.0/8");
|
||||||
|
|||||||
@ -170,7 +170,7 @@ export class IpManager {
|
|||||||
const now = new Date().toISOString();
|
const now = new Date().toISOString();
|
||||||
|
|
||||||
for (const entry of this.store.blocklist) {
|
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;
|
return entry.reason;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -361,7 +361,7 @@ export class IpManager {
|
|||||||
*/
|
*/
|
||||||
getBlocklistEntry(ip: string): BlocklistEntry | null {
|
getBlocklistEntry(ip: string): BlocklistEntry | null {
|
||||||
const now = new Date().toISOString();
|
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;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@ -87,10 +87,11 @@ class LRUCache<K, V> {
|
|||||||
* Rate limiter using token bucket algorithm
|
* Rate limiter using token bucket algorithm
|
||||||
*/
|
*/
|
||||||
export class RateLimiter {
|
export class RateLimiter {
|
||||||
private buckets = new LRUCache<string, CacheEntry>(MAX_CACHE_SIZE);
|
private buckets: LRUCache<string, CacheEntry>;
|
||||||
private cleanupInterval: NodeJS.Timeout | null = null;
|
private cleanupInterval: NodeJS.Timeout | null = null;
|
||||||
|
|
||||||
constructor() {
|
constructor(params?: { maxSize?: number }) {
|
||||||
|
this.buckets = new LRUCache<string, CacheEntry>(params?.maxSize ?? MAX_CACHE_SIZE);
|
||||||
this.startCleanup();
|
this.startCleanup();
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -122,10 +123,10 @@ export class RateLimiter {
|
|||||||
const entry = this.buckets.get(key);
|
const entry = this.buckets.get(key);
|
||||||
|
|
||||||
if (!entry) {
|
if (!entry) {
|
||||||
// Not rate limited yet
|
// Not rate limited yet - full capacity available
|
||||||
return {
|
return {
|
||||||
allowed: true,
|
allowed: true,
|
||||||
remaining: limit.max - 1,
|
remaining: limit.max,
|
||||||
resetAt: new Date(Date.now() + limit.windowMs),
|
resetAt: new Date(Date.now() + limit.windowMs),
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
@ -432,7 +432,9 @@ export class SecurityShield {
|
|||||||
// Try X-Forwarded-For first (if behind proxy)
|
// Try X-Forwarded-For first (if behind proxy)
|
||||||
const forwarded = req.headers["x-forwarded-for"];
|
const forwarded = req.headers["x-forwarded-for"];
|
||||||
if (forwarded) {
|
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();
|
const clientIp = ips[0]?.trim();
|
||||||
if (clientIp) return clientIp;
|
if (clientIp) return clientIp;
|
||||||
}
|
}
|
||||||
|
|||||||
@ -54,6 +54,11 @@ export class TokenBucket {
|
|||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// If count exceeds capacity, we can never fulfill this request
|
||||||
|
if (count > this.config.capacity) {
|
||||||
|
return Infinity;
|
||||||
|
}
|
||||||
|
|
||||||
const tokensNeeded = count - this.tokens;
|
const tokensNeeded = count - this.tokens;
|
||||||
return Math.ceil(tokensNeeded / this.config.refillRate);
|
return Math.ceil(tokensNeeded / this.config.refillRate);
|
||||||
}
|
}
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user