fix(#338): Warn when VALKEY_PASSWORD not set
- Log security warning when Valkey password not configured - Prominent warning in production environment - Tests verify warning behavior for SEC-ORCH-15 Refs #338 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -82,6 +82,89 @@ describe("ValkeyService", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("Security Warnings (SEC-ORCH-15)", () => {
|
||||||
|
it("should check NODE_ENV when VALKEY_PASSWORD not set in production", () => {
|
||||||
|
const configNoPassword = {
|
||||||
|
get: vi.fn((key: string, defaultValue?: unknown) => {
|
||||||
|
const config: Record<string, unknown> = {
|
||||||
|
"orchestrator.valkey.host": "localhost",
|
||||||
|
"orchestrator.valkey.port": 6379,
|
||||||
|
NODE_ENV: "production",
|
||||||
|
};
|
||||||
|
return config[key] ?? defaultValue;
|
||||||
|
}),
|
||||||
|
} as unknown as ConfigService;
|
||||||
|
|
||||||
|
// Create a service to trigger the warning
|
||||||
|
const testService = new ValkeyService(configNoPassword);
|
||||||
|
expect(testService).toBeDefined();
|
||||||
|
|
||||||
|
// Verify NODE_ENV was checked (warning path was taken)
|
||||||
|
expect(configNoPassword.get).toHaveBeenCalledWith("NODE_ENV", "development");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should check NODE_ENV when VALKEY_PASSWORD not set in development", () => {
|
||||||
|
const configNoPasswordDev = {
|
||||||
|
get: vi.fn((key: string, defaultValue?: unknown) => {
|
||||||
|
const config: Record<string, unknown> = {
|
||||||
|
"orchestrator.valkey.host": "localhost",
|
||||||
|
"orchestrator.valkey.port": 6379,
|
||||||
|
NODE_ENV: "development",
|
||||||
|
};
|
||||||
|
return config[key] ?? defaultValue;
|
||||||
|
}),
|
||||||
|
} as unknown as ConfigService;
|
||||||
|
|
||||||
|
const testService = new ValkeyService(configNoPasswordDev);
|
||||||
|
expect(testService).toBeDefined();
|
||||||
|
|
||||||
|
// Verify NODE_ENV was checked (warning path was taken)
|
||||||
|
expect(configNoPasswordDev.get).toHaveBeenCalledWith("NODE_ENV", "development");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should not check NODE_ENV when VALKEY_PASSWORD is configured", () => {
|
||||||
|
const configWithPassword = {
|
||||||
|
get: vi.fn((key: string, defaultValue?: unknown) => {
|
||||||
|
const config: Record<string, unknown> = {
|
||||||
|
"orchestrator.valkey.host": "localhost",
|
||||||
|
"orchestrator.valkey.port": 6379,
|
||||||
|
"orchestrator.valkey.password": "secure-password",
|
||||||
|
NODE_ENV: "production",
|
||||||
|
};
|
||||||
|
return config[key] ?? defaultValue;
|
||||||
|
}),
|
||||||
|
} as unknown as ConfigService;
|
||||||
|
|
||||||
|
const testService = new ValkeyService(configWithPassword);
|
||||||
|
expect(testService).toBeDefined();
|
||||||
|
|
||||||
|
// NODE_ENV should NOT be checked when password is set (warning path not taken)
|
||||||
|
expect(configWithPassword.get).not.toHaveBeenCalledWith("NODE_ENV", "development");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should default to development environment when NODE_ENV not set", () => {
|
||||||
|
const configNoEnv = {
|
||||||
|
get: vi.fn((key: string, defaultValue?: unknown) => {
|
||||||
|
const config: Record<string, unknown> = {
|
||||||
|
"orchestrator.valkey.host": "localhost",
|
||||||
|
"orchestrator.valkey.port": 6379,
|
||||||
|
};
|
||||||
|
// Return default value for NODE_ENV (simulating undefined env var)
|
||||||
|
if (key === "NODE_ENV") {
|
||||||
|
return defaultValue;
|
||||||
|
}
|
||||||
|
return config[key] ?? defaultValue;
|
||||||
|
}),
|
||||||
|
} as unknown as ConfigService;
|
||||||
|
|
||||||
|
const testService = new ValkeyService(configNoEnv);
|
||||||
|
expect(testService).toBeDefined();
|
||||||
|
|
||||||
|
// Should have checked NODE_ENV with default "development"
|
||||||
|
expect(configNoEnv.get).toHaveBeenCalledWith("NODE_ENV", "development");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("Lifecycle", () => {
|
describe("Lifecycle", () => {
|
||||||
it("should disconnect on module destroy", async () => {
|
it("should disconnect on module destroy", async () => {
|
||||||
mockClient.disconnect.mockResolvedValue(undefined);
|
mockClient.disconnect.mockResolvedValue(undefined);
|
||||||
|
|||||||
@@ -33,6 +33,23 @@ export class ValkeyService implements OnModuleDestroy {
|
|||||||
const password = this.configService.get<string>("orchestrator.valkey.password");
|
const password = this.configService.get<string>("orchestrator.valkey.password");
|
||||||
if (password) {
|
if (password) {
|
||||||
config.password = password;
|
config.password = password;
|
||||||
|
} else {
|
||||||
|
// SEC-ORCH-15: Warn when Valkey password is not configured
|
||||||
|
const nodeEnv = this.configService.get<string>("NODE_ENV", "development");
|
||||||
|
const isProduction = nodeEnv === "production";
|
||||||
|
|
||||||
|
if (isProduction) {
|
||||||
|
this.logger.warn(
|
||||||
|
"SECURITY WARNING: VALKEY_PASSWORD is not configured in production environment. " +
|
||||||
|
"Valkey connections without authentication are insecure. " +
|
||||||
|
"Set VALKEY_PASSWORD environment variable to secure your Valkey instance."
|
||||||
|
);
|
||||||
|
} else {
|
||||||
|
this.logger.warn(
|
||||||
|
"VALKEY_PASSWORD is not configured. " +
|
||||||
|
"Consider setting VALKEY_PASSWORD for secure Valkey connections."
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
this.client = new ValkeyClient(config);
|
this.client = new ValkeyClient(config);
|
||||||
|
|||||||
Reference in New Issue
Block a user