fix(SEC-ORCH-2): Add API key authentication to orchestrator API
Some checks failed
ci/woodpecker/push/woodpecker Pipeline failed
Some checks failed
ci/woodpecker/push/woodpecker Pipeline failed
Add OrchestratorApiKeyGuard to protect agent management endpoints (spawn, kill, kill-all, status) from unauthorized access. Uses X-API-Key header with constant-time comparison to prevent timing attacks. - Create apps/orchestrator/src/common/guards/api-key.guard.ts - Add comprehensive tests for all guard scenarios - Apply guard to AgentsController (controller-level protection) - Document ORCHESTRATOR_API_KEY in .env.example files - Health endpoints remain unauthenticated for monitoring Security: Prevents unauthorized users from draining API credits or killing all agents via unprotected endpoints. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
10
.env.example
10
.env.example
@@ -224,6 +224,16 @@ RATE_LIMIT_STORAGE=redis
|
||||
# multi-tenant isolation. Each Discord bot instance should be configured for
|
||||
# a single workspace.
|
||||
|
||||
# ======================
|
||||
# Orchestrator Configuration
|
||||
# ======================
|
||||
# API Key for orchestrator agent management endpoints
|
||||
# CRITICAL: Generate a random API key with at least 32 characters
|
||||
# Example: openssl rand -base64 32
|
||||
# Required for all /agents/* endpoints (spawn, kill, kill-all, status)
|
||||
# Health endpoints (/health/*) remain unauthenticated
|
||||
ORCHESTRATOR_API_KEY=REPLACE_WITH_RANDOM_API_KEY_MINIMUM_32_CHARS
|
||||
|
||||
# ======================
|
||||
# Logging & Debugging
|
||||
# ======================
|
||||
|
||||
@@ -21,6 +21,13 @@ GIT_USER_EMAIL="orchestrator@mosaicstack.dev"
|
||||
KILLSWITCH_ENABLED=true
|
||||
SANDBOX_ENABLED=true
|
||||
|
||||
# API Authentication
|
||||
# CRITICAL: Generate a random API key with at least 32 characters
|
||||
# Example: openssl rand -base64 32
|
||||
# Required for all /agents/* endpoints (spawn, kill, kill-all, status)
|
||||
# Health endpoints (/health/*) remain unauthenticated
|
||||
ORCHESTRATOR_API_KEY=REPLACE_WITH_RANDOM_API_KEY_MINIMUM_32_CHARS
|
||||
|
||||
# Quality Gates
|
||||
# YOLO mode bypasses all quality gates (default: false)
|
||||
# WARNING: Only enable for development/testing. Not recommended for production.
|
||||
|
||||
@@ -10,17 +10,23 @@ import {
|
||||
UsePipes,
|
||||
ValidationPipe,
|
||||
HttpCode,
|
||||
UseGuards,
|
||||
} from "@nestjs/common";
|
||||
import { QueueService } from "../../queue/queue.service";
|
||||
import { AgentSpawnerService } from "../../spawner/agent-spawner.service";
|
||||
import { AgentLifecycleService } from "../../spawner/agent-lifecycle.service";
|
||||
import { KillswitchService } from "../../killswitch/killswitch.service";
|
||||
import { SpawnAgentDto, SpawnAgentResponseDto } from "./dto/spawn-agent.dto";
|
||||
import { OrchestratorApiKeyGuard } from "../../common/guards/api-key.guard";
|
||||
|
||||
/**
|
||||
* Controller for agent management endpoints
|
||||
*
|
||||
* All endpoints require API key authentication via X-API-Key header.
|
||||
* Set ORCHESTRATOR_API_KEY environment variable to configure the expected key.
|
||||
*/
|
||||
@Controller("agents")
|
||||
@UseGuards(OrchestratorApiKeyGuard)
|
||||
export class AgentsController {
|
||||
private readonly logger = new Logger(AgentsController.name);
|
||||
|
||||
|
||||
@@ -4,9 +4,11 @@ import { QueueModule } from "../../queue/queue.module";
|
||||
import { SpawnerModule } from "../../spawner/spawner.module";
|
||||
import { KillswitchModule } from "../../killswitch/killswitch.module";
|
||||
import { ValkeyModule } from "../../valkey/valkey.module";
|
||||
import { OrchestratorApiKeyGuard } from "../../common/guards/api-key.guard";
|
||||
|
||||
@Module({
|
||||
imports: [QueueModule, SpawnerModule, KillswitchModule, ValkeyModule],
|
||||
controllers: [AgentsController],
|
||||
providers: [OrchestratorApiKeyGuard],
|
||||
})
|
||||
export class AgentsModule {}
|
||||
|
||||
169
apps/orchestrator/src/common/guards/api-key.guard.spec.ts
Normal file
169
apps/orchestrator/src/common/guards/api-key.guard.spec.ts
Normal file
@@ -0,0 +1,169 @@
|
||||
import { describe, it, expect, beforeEach, vi } from "vitest";
|
||||
import { ExecutionContext, UnauthorizedException } from "@nestjs/common";
|
||||
import { ConfigService } from "@nestjs/config";
|
||||
import { OrchestratorApiKeyGuard } from "./api-key.guard";
|
||||
|
||||
describe("OrchestratorApiKeyGuard", () => {
|
||||
let guard: OrchestratorApiKeyGuard;
|
||||
let mockConfigService: ConfigService;
|
||||
|
||||
beforeEach(() => {
|
||||
mockConfigService = {
|
||||
get: vi.fn(),
|
||||
} as unknown as ConfigService;
|
||||
|
||||
guard = new OrchestratorApiKeyGuard(mockConfigService);
|
||||
});
|
||||
|
||||
const createMockExecutionContext = (headers: Record<string, string>): ExecutionContext => {
|
||||
return {
|
||||
switchToHttp: () => ({
|
||||
getRequest: () => ({
|
||||
headers,
|
||||
}),
|
||||
}),
|
||||
} as ExecutionContext;
|
||||
};
|
||||
|
||||
describe("canActivate", () => {
|
||||
it("should return true when valid API key is provided", () => {
|
||||
const validApiKey = "test-orchestrator-api-key-12345";
|
||||
vi.mocked(mockConfigService.get).mockReturnValue(validApiKey);
|
||||
|
||||
const context = createMockExecutionContext({
|
||||
"x-api-key": validApiKey,
|
||||
});
|
||||
|
||||
const result = guard.canActivate(context);
|
||||
|
||||
expect(result).toBe(true);
|
||||
expect(mockConfigService.get).toHaveBeenCalledWith("ORCHESTRATOR_API_KEY");
|
||||
});
|
||||
|
||||
it("should throw UnauthorizedException when no API key is provided", () => {
|
||||
const context = createMockExecutionContext({});
|
||||
|
||||
expect(() => guard.canActivate(context)).toThrow(UnauthorizedException);
|
||||
expect(() => guard.canActivate(context)).toThrow("No API key provided");
|
||||
});
|
||||
|
||||
it("should throw UnauthorizedException when API key is invalid", () => {
|
||||
const validApiKey = "correct-orchestrator-api-key";
|
||||
const invalidApiKey = "wrong-api-key";
|
||||
|
||||
vi.mocked(mockConfigService.get).mockReturnValue(validApiKey);
|
||||
|
||||
const context = createMockExecutionContext({
|
||||
"x-api-key": invalidApiKey,
|
||||
});
|
||||
|
||||
expect(() => guard.canActivate(context)).toThrow(UnauthorizedException);
|
||||
expect(() => guard.canActivate(context)).toThrow("Invalid API key");
|
||||
});
|
||||
|
||||
it("should throw UnauthorizedException when ORCHESTRATOR_API_KEY is not configured", () => {
|
||||
vi.mocked(mockConfigService.get).mockReturnValue(undefined);
|
||||
|
||||
const context = createMockExecutionContext({
|
||||
"x-api-key": "some-key",
|
||||
});
|
||||
|
||||
expect(() => guard.canActivate(context)).toThrow(UnauthorizedException);
|
||||
expect(() => guard.canActivate(context)).toThrow("API key authentication not configured");
|
||||
});
|
||||
|
||||
it("should handle uppercase header name (X-API-Key)", () => {
|
||||
const validApiKey = "test-orchestrator-api-key-12345";
|
||||
vi.mocked(mockConfigService.get).mockReturnValue(validApiKey);
|
||||
|
||||
const context = createMockExecutionContext({
|
||||
"X-API-Key": validApiKey,
|
||||
});
|
||||
|
||||
const result = guard.canActivate(context);
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it("should handle mixed case header name (X-Api-Key)", () => {
|
||||
const validApiKey = "test-orchestrator-api-key-12345";
|
||||
vi.mocked(mockConfigService.get).mockReturnValue(validApiKey);
|
||||
|
||||
const context = createMockExecutionContext({
|
||||
"X-Api-Key": validApiKey,
|
||||
});
|
||||
|
||||
const result = guard.canActivate(context);
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it("should reject empty string API key", () => {
|
||||
vi.mocked(mockConfigService.get).mockReturnValue("valid-key");
|
||||
|
||||
const context = createMockExecutionContext({
|
||||
"x-api-key": "",
|
||||
});
|
||||
|
||||
expect(() => guard.canActivate(context)).toThrow(UnauthorizedException);
|
||||
expect(() => guard.canActivate(context)).toThrow("No API key provided");
|
||||
});
|
||||
|
||||
it("should reject whitespace-only API key", () => {
|
||||
vi.mocked(mockConfigService.get).mockReturnValue("valid-key");
|
||||
|
||||
const context = createMockExecutionContext({
|
||||
"x-api-key": " ",
|
||||
});
|
||||
|
||||
expect(() => guard.canActivate(context)).toThrow(UnauthorizedException);
|
||||
expect(() => guard.canActivate(context)).toThrow("No API key provided");
|
||||
});
|
||||
|
||||
it("should use constant-time comparison to prevent timing attacks", () => {
|
||||
const validApiKey = "test-api-key-12345";
|
||||
vi.mocked(mockConfigService.get).mockReturnValue(validApiKey);
|
||||
|
||||
const startTime = Date.now();
|
||||
const context1 = createMockExecutionContext({
|
||||
"x-api-key": "wrong-key-short",
|
||||
});
|
||||
|
||||
try {
|
||||
guard.canActivate(context1);
|
||||
} catch {
|
||||
// Expected to fail
|
||||
}
|
||||
const shortKeyTime = Date.now() - startTime;
|
||||
|
||||
const startTime2 = Date.now();
|
||||
const context2 = createMockExecutionContext({
|
||||
"x-api-key": "test-api-key-12344", // Very close to correct key
|
||||
});
|
||||
|
||||
try {
|
||||
guard.canActivate(context2);
|
||||
} catch {
|
||||
// Expected to fail
|
||||
}
|
||||
const longKeyTime = Date.now() - startTime2;
|
||||
|
||||
// Times should be similar (within 10ms) to prevent timing attacks
|
||||
// Note: This is a simplified test; real timing attack prevention
|
||||
// is handled by crypto.timingSafeEqual
|
||||
expect(Math.abs(shortKeyTime - longKeyTime)).toBeLessThan(10);
|
||||
});
|
||||
|
||||
it("should reject keys with different lengths even if prefix matches", () => {
|
||||
const validApiKey = "orchestrator-secret-key-abc123";
|
||||
vi.mocked(mockConfigService.get).mockReturnValue(validApiKey);
|
||||
|
||||
const context = createMockExecutionContext({
|
||||
"x-api-key": "orchestrator-secret-key-abc123-extra",
|
||||
});
|
||||
|
||||
expect(() => guard.canActivate(context)).toThrow(UnauthorizedException);
|
||||
expect(() => guard.canActivate(context)).toThrow("Invalid API key");
|
||||
});
|
||||
});
|
||||
});
|
||||
82
apps/orchestrator/src/common/guards/api-key.guard.ts
Normal file
82
apps/orchestrator/src/common/guards/api-key.guard.ts
Normal file
@@ -0,0 +1,82 @@
|
||||
import { Injectable, CanActivate, ExecutionContext, UnauthorizedException } from "@nestjs/common";
|
||||
import { ConfigService } from "@nestjs/config";
|
||||
import { timingSafeEqual } from "crypto";
|
||||
|
||||
/**
|
||||
* OrchestratorApiKeyGuard - Authentication guard for orchestrator API endpoints
|
||||
*
|
||||
* Validates the X-API-Key header against the ORCHESTRATOR_API_KEY environment variable.
|
||||
* Uses constant-time comparison to prevent timing attacks.
|
||||
*
|
||||
* Usage:
|
||||
* @UseGuards(OrchestratorApiKeyGuard)
|
||||
* @Controller('agents')
|
||||
* export class AgentsController { ... }
|
||||
*/
|
||||
@Injectable()
|
||||
export class OrchestratorApiKeyGuard implements CanActivate {
|
||||
constructor(private readonly configService: ConfigService) {}
|
||||
|
||||
canActivate(context: ExecutionContext): boolean {
|
||||
const request = context.switchToHttp().getRequest<{ headers: Record<string, string> }>();
|
||||
const providedKey = this.extractApiKeyFromHeader(request);
|
||||
|
||||
if (!providedKey) {
|
||||
throw new UnauthorizedException("No API key provided");
|
||||
}
|
||||
|
||||
const configuredKey = this.configService.get<string>("ORCHESTRATOR_API_KEY");
|
||||
|
||||
if (!configuredKey) {
|
||||
throw new UnauthorizedException("API key authentication not configured");
|
||||
}
|
||||
|
||||
if (!this.isValidApiKey(providedKey, configuredKey)) {
|
||||
throw new UnauthorizedException("Invalid API key");
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract API key from X-API-Key header (case-insensitive)
|
||||
*/
|
||||
private extractApiKeyFromHeader(request: {
|
||||
headers: Record<string, string>;
|
||||
}): string | undefined {
|
||||
const headers = request.headers;
|
||||
|
||||
// Check common variations (lowercase, uppercase, mixed case)
|
||||
// HTTP headers are typically normalized to lowercase, but we check common variations for safety
|
||||
const apiKey =
|
||||
headers["x-api-key"] || headers["X-API-Key"] || headers["X-Api-Key"] || undefined;
|
||||
|
||||
// Return undefined if key is empty string
|
||||
if (typeof apiKey === "string" && apiKey.trim() === "") {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
return apiKey;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate API key using constant-time comparison to prevent timing attacks
|
||||
*/
|
||||
private isValidApiKey(providedKey: string, configuredKey: string): boolean {
|
||||
try {
|
||||
// Convert strings to buffers for constant-time comparison
|
||||
const providedBuffer = Buffer.from(providedKey, "utf8");
|
||||
const configuredBuffer = Buffer.from(configuredKey, "utf8");
|
||||
|
||||
// Keys must be same length for timingSafeEqual
|
||||
if (providedBuffer.length !== configuredBuffer.length) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return timingSafeEqual(providedBuffer, configuredBuffer);
|
||||
} catch {
|
||||
// If comparison fails for any reason, reject
|
||||
return false;
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user