Security and Code Quality Remediation (M6-Fixes) #343

Merged
jason.woltje merged 57 commits from fix/security into develop 2026-02-06 17:49:14 +00:00
2 changed files with 302 additions and 2 deletions
Showing only changes of commit 7ae92f3e1c - Show all commits

View File

@@ -0,0 +1,257 @@
import { describe, it, expect, beforeEach, vi, afterEach, Mock } from "vitest";
import { ThrottlerValkeyStorageService } from "./throttler-storage.service";
// Create a mock Redis class
const createMockRedis = (
options: {
shouldFailConnect?: boolean;
error?: Error;
} = {}
): Record<string, Mock> => ({
connect: vi.fn().mockImplementation(() => {
if (options.shouldFailConnect) {
return Promise.reject(options.error ?? new Error("Connection refused"));
}
return Promise.resolve();
}),
ping: vi.fn().mockResolvedValue("PONG"),
quit: vi.fn().mockResolvedValue("OK"),
multi: vi.fn().mockReturnThis(),
incr: vi.fn().mockReturnThis(),
pexpire: vi.fn().mockReturnThis(),
exec: vi.fn().mockResolvedValue([
[null, 1],
[null, 1],
]),
get: vi.fn().mockResolvedValue("5"),
});
// Mock ioredis module
vi.mock("ioredis", () => {
return {
default: vi.fn().mockImplementation(() => createMockRedis({ shouldFailConnect: true })),
};
});
describe("ThrottlerValkeyStorageService", () => {
let service: ThrottlerValkeyStorageService;
let loggerErrorSpy: ReturnType<typeof vi.spyOn>;
beforeEach(() => {
vi.clearAllMocks();
service = new ThrottlerValkeyStorageService();
// Spy on logger methods - access the private logger
const logger = (
service as unknown as { logger: { error: () => void; log: () => void; warn: () => void } }
).logger;
loggerErrorSpy = vi.spyOn(logger, "error").mockImplementation(() => undefined);
vi.spyOn(logger, "log").mockImplementation(() => undefined);
vi.spyOn(logger, "warn").mockImplementation(() => undefined);
});
afterEach(() => {
vi.clearAllMocks();
});
describe("initialization and fallback behavior", () => {
it("should start in fallback mode before initialization", () => {
// Before onModuleInit is called, useRedis is false by default
expect(service.isUsingFallback()).toBe(true);
});
it("should log ERROR when Redis connection fails", async () => {
const newService = new ThrottlerValkeyStorageService();
const newLogger = (
newService as unknown as { logger: { error: () => void; log: () => void } }
).logger;
const newErrorSpy = vi.spyOn(newLogger, "error").mockImplementation(() => undefined);
vi.spyOn(newLogger, "log").mockImplementation(() => undefined);
await newService.onModuleInit();
// Verify ERROR was logged (not WARN)
expect(newErrorSpy).toHaveBeenCalledWith(
expect.stringContaining("Failed to connect to Valkey for rate limiting")
);
expect(newErrorSpy).toHaveBeenCalledWith(
expect.stringContaining("DEGRADED MODE: Falling back to in-memory rate limiting storage")
);
});
it("should log message indicating rate limits will not be shared", async () => {
const newService = new ThrottlerValkeyStorageService();
const newLogger = (
newService as unknown as { logger: { error: () => void; log: () => void } }
).logger;
const newErrorSpy = vi.spyOn(newLogger, "error").mockImplementation(() => undefined);
vi.spyOn(newLogger, "log").mockImplementation(() => undefined);
await newService.onModuleInit();
expect(newErrorSpy).toHaveBeenCalledWith(
expect.stringContaining("Rate limits will not be shared across API instances")
);
});
it("should be in fallback mode when Redis connection fails", async () => {
const newService = new ThrottlerValkeyStorageService();
const newLogger = (
newService as unknown as { logger: { error: () => void; log: () => void } }
).logger;
vi.spyOn(newLogger, "error").mockImplementation(() => undefined);
vi.spyOn(newLogger, "log").mockImplementation(() => undefined);
await newService.onModuleInit();
expect(newService.isUsingFallback()).toBe(true);
});
});
describe("isUsingFallback()", () => {
it("should return true when in memory fallback mode", () => {
// Default state is fallback mode
expect(service.isUsingFallback()).toBe(true);
});
it("should return boolean type", () => {
const result = service.isUsingFallback();
expect(typeof result).toBe("boolean");
});
});
describe("getHealthStatus()", () => {
it("should return degraded status when in fallback mode", () => {
// Default state is fallback mode
const status = service.getHealthStatus();
expect(status).toEqual({
healthy: true,
mode: "memory",
degraded: true,
message: expect.stringContaining("in-memory fallback"),
});
});
it("should indicate degraded mode message includes lack of sharing", () => {
const status = service.getHealthStatus();
expect(status.message).toContain("not shared across instances");
});
it("should always report healthy even in degraded mode", () => {
// In degraded mode, the service is still functional
const status = service.getHealthStatus();
expect(status.healthy).toBe(true);
});
it("should have correct structure for health checks", () => {
const status = service.getHealthStatus();
expect(status).toHaveProperty("healthy");
expect(status).toHaveProperty("mode");
expect(status).toHaveProperty("degraded");
expect(status).toHaveProperty("message");
});
it("should report mode as memory when in fallback", () => {
const status = service.getHealthStatus();
expect(status.mode).toBe("memory");
});
it("should report degraded as true when in fallback", () => {
const status = service.getHealthStatus();
expect(status.degraded).toBe(true);
});
});
describe("getHealthStatus() with Redis (unit test via internal state)", () => {
it("should return non-degraded status when Redis is available", () => {
// Manually set the internal state to simulate Redis being available
// This tests the method logic without requiring actual Redis connection
const testService = new ThrottlerValkeyStorageService();
// Access private property for testing (this is acceptable for unit testing)
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(testService as any).useRedis = true;
const status = testService.getHealthStatus();
expect(status).toEqual({
healthy: true,
mode: "redis",
degraded: false,
message: expect.stringContaining("Redis storage"),
});
});
it("should report distributed mode message when Redis is available", () => {
const testService = new ThrottlerValkeyStorageService();
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(testService as any).useRedis = true;
const status = testService.getHealthStatus();
expect(status.message).toContain("distributed mode");
});
it("should report isUsingFallback as false when Redis is available", () => {
const testService = new ThrottlerValkeyStorageService();
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(testService as any).useRedis = true;
expect(testService.isUsingFallback()).toBe(false);
});
});
describe("in-memory fallback operations", () => {
it("should increment correctly in fallback mode", async () => {
const result = await service.increment("test-key", 60000, 10, 0, "default");
expect(result.totalHits).toBe(1);
expect(result.isBlocked).toBe(false);
});
it("should accumulate hits in fallback mode", async () => {
await service.increment("test-key", 60000, 10, 0, "default");
await service.increment("test-key", 60000, 10, 0, "default");
const result = await service.increment("test-key", 60000, 10, 0, "default");
expect(result.totalHits).toBe(3);
});
it("should return correct blocked status when limit exceeded", async () => {
// Make 3 requests with limit of 2
await service.increment("test-key", 60000, 2, 1000, "default");
await service.increment("test-key", 60000, 2, 1000, "default");
const result = await service.increment("test-key", 60000, 2, 1000, "default");
expect(result.totalHits).toBe(3);
expect(result.isBlocked).toBe(true);
expect(result.timeToBlockExpire).toBe(1000);
});
it("should return 0 for get on non-existent key in fallback mode", async () => {
const result = await service.get("non-existent-key");
expect(result).toBe(0);
});
it("should return correct timeToExpire in response", async () => {
const ttl = 30000;
const result = await service.increment("test-key", ttl, 10, 0, "default");
expect(result.timeToExpire).toBe(ttl);
});
it("should isolate different keys in fallback mode", async () => {
await service.increment("key-1", 60000, 10, 0, "default");
await service.increment("key-1", 60000, 10, 0, "default");
const result1 = await service.increment("key-1", 60000, 10, 0, "default");
const result2 = await service.increment("key-2", 60000, 10, 0, "default");
expect(result1.totalHits).toBe(3);
expect(result2.totalHits).toBe(1);
});
});
});

View File

@@ -53,8 +53,11 @@ export class ThrottlerValkeyStorageService implements ThrottlerStorage, OnModule
this.logger.log("Valkey connected successfully for rate limiting");
} catch (error) {
const errorMessage = error instanceof Error ? error.message : String(error);
this.logger.warn(`Failed to connect to Valkey for rate limiting: ${errorMessage}`);
this.logger.warn("Falling back to in-memory rate limiting storage");
this.logger.error(`Failed to connect to Valkey for rate limiting: ${errorMessage}`);
this.logger.error(
"DEGRADED MODE: Falling back to in-memory rate limiting storage. " +
"Rate limits will not be shared across API instances."
);
this.useRedis = false;
this.client = undefined;
}
@@ -168,6 +171,46 @@ export class ThrottlerValkeyStorageService implements ThrottlerStorage, OnModule
return `${this.THROTTLER_PREFIX}${key}`;
}
/**
* Check if the service is using fallback in-memory storage
*
* This indicates a degraded state where rate limits are not shared
* across API instances. Use this for health checks.
*
* @returns true if using in-memory fallback, false if using Redis
*/
isUsingFallback(): boolean {
return !this.useRedis;
}
/**
* Get rate limiter health status for health check endpoints
*
* @returns Health status object with storage mode and details
*/
getHealthStatus(): {
healthy: boolean;
mode: "redis" | "memory";
degraded: boolean;
message: string;
} {
if (this.useRedis) {
return {
healthy: true,
mode: "redis",
degraded: false,
message: "Rate limiter using Redis storage (distributed mode)",
};
}
return {
healthy: true, // Service is functional, but degraded
mode: "memory",
degraded: true,
message:
"Rate limiter using in-memory fallback (degraded mode - limits not shared across instances)",
};
}
/**
* Clean up on module destroy
*/