fix(SEC-ORCH-28+29): Add Valkey connection timeout + workItems MaxLength
Some checks failed
ci/woodpecker/push/woodpecker Pipeline failed
Some checks failed
ci/woodpecker/push/woodpecker Pipeline failed
SEC-ORCH-28: Add connectTimeout (5000ms default) and commandTimeout (3000ms default) to Valkey/Redis client to prevent indefinite connection hangs. Both are configurable via VALKEY_CONNECT_TIMEOUT_MS and VALKEY_COMMAND_TIMEOUT_MS environment variables. SEC-ORCH-29: Add @ArrayMaxSize(50) and @MaxLength(2000) to workItems in AgentContextDto to prevent memory exhaustion from unbounded input. Also adds @ArrayMaxSize(20) and @MaxLength(200) to skills array. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -230,6 +230,65 @@ describe("SpawnAgentDto validation", () => {
|
||||
const errors = await validate(dto);
|
||||
expect(errors.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
// ------ workItems MaxLength / ArrayMaxSize (SEC-ORCH-29) ------ //
|
||||
it("should reject workItems array exceeding max size of 50", async () => {
|
||||
const payload = validSpawnPayload();
|
||||
(payload.context as Record<string, unknown>).workItems = Array.from(
|
||||
{ length: 51 },
|
||||
(_, i) => `US-${String(i + 1).padStart(3, "0")}`
|
||||
);
|
||||
const dto = plainToInstance(SpawnAgentDto, payload);
|
||||
const errors = await validate(dto);
|
||||
expect(errors.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it("should accept workItems array at max size of 50", async () => {
|
||||
const payload = validSpawnPayload();
|
||||
(payload.context as Record<string, unknown>).workItems = Array.from(
|
||||
{ length: 50 },
|
||||
(_, i) => `US-${String(i + 1).padStart(3, "0")}`
|
||||
);
|
||||
const dto = plainToInstance(SpawnAgentDto, payload);
|
||||
const errors = await validate(dto);
|
||||
expect(errors).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("should reject a work item string exceeding 2000 characters", async () => {
|
||||
const payload = validSpawnPayload();
|
||||
(payload.context as Record<string, unknown>).workItems = ["x".repeat(2001)];
|
||||
const dto = plainToInstance(SpawnAgentDto, payload);
|
||||
const errors = await validate(dto);
|
||||
expect(errors.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it("should accept a work item string at exactly 2000 characters", async () => {
|
||||
const payload = validSpawnPayload();
|
||||
(payload.context as Record<string, unknown>).workItems = ["x".repeat(2000)];
|
||||
const dto = plainToInstance(SpawnAgentDto, payload);
|
||||
const errors = await validate(dto);
|
||||
expect(errors).toHaveLength(0);
|
||||
});
|
||||
|
||||
// ------ skills MaxLength / ArrayMaxSize (SEC-ORCH-29) ------ //
|
||||
it("should reject skills array exceeding max size of 20", async () => {
|
||||
const payload = validSpawnPayload();
|
||||
(payload.context as Record<string, unknown>).skills = Array.from(
|
||||
{ length: 21 },
|
||||
(_, i) => `skill-${i}`
|
||||
);
|
||||
const dto = plainToInstance(SpawnAgentDto, payload);
|
||||
const errors = await validate(dto);
|
||||
expect(errors.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it("should reject a skill string exceeding 200 characters", async () => {
|
||||
const payload = validSpawnPayload();
|
||||
(payload.context as Record<string, unknown>).skills = ["s".repeat(201)];
|
||||
const dto = plainToInstance(SpawnAgentDto, payload);
|
||||
const errors = await validate(dto);
|
||||
expect(errors.length).toBeGreaterThan(0);
|
||||
});
|
||||
});
|
||||
|
||||
// ------------------------------------------------------------------ //
|
||||
|
||||
@@ -6,6 +6,8 @@ import {
|
||||
IsArray,
|
||||
IsOptional,
|
||||
ArrayNotEmpty,
|
||||
ArrayMaxSize,
|
||||
MaxLength,
|
||||
IsIn,
|
||||
Validate,
|
||||
ValidatorConstraint,
|
||||
@@ -83,12 +85,16 @@ export class AgentContextDto {
|
||||
|
||||
@IsArray()
|
||||
@ArrayNotEmpty()
|
||||
@ArrayMaxSize(50, { message: "workItems must contain at most 50 items" })
|
||||
@IsString({ each: true })
|
||||
@MaxLength(2000, { each: true, message: "Each work item must be at most 2000 characters" })
|
||||
workItems!: string[];
|
||||
|
||||
@IsArray()
|
||||
@IsOptional()
|
||||
@ArrayMaxSize(20, { message: "skills must contain at most 20 items" })
|
||||
@IsString({ each: true })
|
||||
@MaxLength(200, { each: true, message: "Each skill must be at most 200 characters" })
|
||||
skills?: string[];
|
||||
}
|
||||
|
||||
|
||||
@@ -122,6 +122,40 @@ describe("orchestratorConfig", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("valkey timeout config (SEC-ORCH-28)", () => {
|
||||
it("should use default connectTimeout of 5000 when not set", () => {
|
||||
delete process.env.VALKEY_CONNECT_TIMEOUT_MS;
|
||||
|
||||
const config = orchestratorConfig();
|
||||
|
||||
expect(config.valkey.connectTimeout).toBe(5000);
|
||||
});
|
||||
|
||||
it("should use provided connectTimeout when VALKEY_CONNECT_TIMEOUT_MS is set", () => {
|
||||
process.env.VALKEY_CONNECT_TIMEOUT_MS = "10000";
|
||||
|
||||
const config = orchestratorConfig();
|
||||
|
||||
expect(config.valkey.connectTimeout).toBe(10000);
|
||||
});
|
||||
|
||||
it("should use default commandTimeout of 3000 when not set", () => {
|
||||
delete process.env.VALKEY_COMMAND_TIMEOUT_MS;
|
||||
|
||||
const config = orchestratorConfig();
|
||||
|
||||
expect(config.valkey.commandTimeout).toBe(3000);
|
||||
});
|
||||
|
||||
it("should use provided commandTimeout when VALKEY_COMMAND_TIMEOUT_MS is set", () => {
|
||||
process.env.VALKEY_COMMAND_TIMEOUT_MS = "8000";
|
||||
|
||||
const config = orchestratorConfig();
|
||||
|
||||
expect(config.valkey.commandTimeout).toBe(8000);
|
||||
});
|
||||
});
|
||||
|
||||
describe("spawner config", () => {
|
||||
it("should use default maxConcurrentAgents of 20 when not set", () => {
|
||||
delete process.env.MAX_CONCURRENT_AGENTS;
|
||||
|
||||
@@ -8,6 +8,8 @@ export const orchestratorConfig = registerAs("orchestrator", () => ({
|
||||
port: parseInt(process.env.VALKEY_PORT ?? "6379", 10),
|
||||
password: process.env.VALKEY_PASSWORD,
|
||||
url: process.env.VALKEY_URL ?? "redis://localhost:6379",
|
||||
connectTimeout: parseInt(process.env.VALKEY_CONNECT_TIMEOUT_MS ?? "5000", 10),
|
||||
commandTimeout: parseInt(process.env.VALKEY_COMMAND_TIMEOUT_MS ?? "3000", 10),
|
||||
},
|
||||
claude: {
|
||||
apiKey: process.env.CLAUDE_API_KEY,
|
||||
|
||||
@@ -16,11 +16,15 @@ const mockRedisInstance = {
|
||||
mget: vi.fn(),
|
||||
};
|
||||
|
||||
// Capture constructor arguments for verification
|
||||
let lastRedisConstructorArgs: unknown[] = [];
|
||||
|
||||
// Mock ioredis
|
||||
vi.mock("ioredis", () => {
|
||||
return {
|
||||
default: class {
|
||||
constructor() {
|
||||
constructor(...args: unknown[]) {
|
||||
lastRedisConstructorArgs = args;
|
||||
return mockRedisInstance;
|
||||
}
|
||||
},
|
||||
@@ -53,6 +57,25 @@ describe("ValkeyClient", () => {
|
||||
});
|
||||
|
||||
describe("Connection Management", () => {
|
||||
it("should pass default timeout options to Redis when not configured", () => {
|
||||
new ValkeyClient({ host: "localhost", port: 6379 });
|
||||
const options = lastRedisConstructorArgs[0] as Record<string, unknown>;
|
||||
expect(options.connectTimeout).toBe(5000);
|
||||
expect(options.commandTimeout).toBe(3000);
|
||||
});
|
||||
|
||||
it("should pass custom timeout options to Redis when configured", () => {
|
||||
new ValkeyClient({
|
||||
host: "localhost",
|
||||
port: 6379,
|
||||
connectTimeout: 10000,
|
||||
commandTimeout: 8000,
|
||||
});
|
||||
const options = lastRedisConstructorArgs[0] as Record<string, unknown>;
|
||||
expect(options.connectTimeout).toBe(10000);
|
||||
expect(options.commandTimeout).toBe(8000);
|
||||
});
|
||||
|
||||
it("should disconnect on close", async () => {
|
||||
mockRedis.quit.mockResolvedValue("OK");
|
||||
|
||||
|
||||
@@ -16,6 +16,10 @@ export interface ValkeyClientConfig {
|
||||
port: number;
|
||||
password?: string;
|
||||
db?: number;
|
||||
/** Connection timeout in milliseconds (default: 5000) */
|
||||
connectTimeout?: number;
|
||||
/** Command timeout in milliseconds (default: 3000) */
|
||||
commandTimeout?: number;
|
||||
logger?: {
|
||||
error: (message: string, error?: unknown) => void;
|
||||
};
|
||||
@@ -57,6 +61,8 @@ export class ValkeyClient {
|
||||
port: config.port,
|
||||
password: config.password,
|
||||
db: config.db,
|
||||
connectTimeout: config.connectTimeout ?? 5000,
|
||||
commandTimeout: config.commandTimeout ?? 3000,
|
||||
});
|
||||
this.logger = config.logger;
|
||||
}
|
||||
|
||||
@@ -23,6 +23,8 @@ export class ValkeyService implements OnModuleDestroy {
|
||||
const config: ValkeyClientConfig = {
|
||||
host: this.configService.get<string>("orchestrator.valkey.host", "localhost"),
|
||||
port: this.configService.get<number>("orchestrator.valkey.port", 6379),
|
||||
connectTimeout: this.configService.get<number>("orchestrator.valkey.connectTimeout", 5000),
|
||||
commandTimeout: this.configService.get<number>("orchestrator.valkey.commandTimeout", 3000),
|
||||
logger: {
|
||||
error: (message: string, error?: unknown) => {
|
||||
this.logger.error(message, error instanceof Error ? error.stack : String(error));
|
||||
|
||||
Reference in New Issue
Block a user