From 000145af96f247aeb4c189b8fb951ec9308b0a16 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Thu, 5 Feb 2026 15:18:15 -0600 Subject: [PATCH] fix(SEC-ORCH-2): Add API key authentication to orchestrator API 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 --- .env.example | 10 ++ apps/orchestrator/.env.example | 7 + .../src/api/agents/agents.controller.ts | 6 + .../src/api/agents/agents.module.ts | 2 + .../src/common/guards/api-key.guard.spec.ts | 169 ++++++++++++++++++ .../src/common/guards/api-key.guard.ts | 82 +++++++++ 6 files changed, 276 insertions(+) create mode 100644 apps/orchestrator/src/common/guards/api-key.guard.spec.ts create mode 100644 apps/orchestrator/src/common/guards/api-key.guard.ts diff --git a/.env.example b/.env.example index 4f13421..5337042 100644 --- a/.env.example +++ b/.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 # ====================== diff --git a/apps/orchestrator/.env.example b/apps/orchestrator/.env.example index d87ede6..5c7eb68 100644 --- a/apps/orchestrator/.env.example +++ b/apps/orchestrator/.env.example @@ -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. diff --git a/apps/orchestrator/src/api/agents/agents.controller.ts b/apps/orchestrator/src/api/agents/agents.controller.ts index d8b74e5..69e4d90 100644 --- a/apps/orchestrator/src/api/agents/agents.controller.ts +++ b/apps/orchestrator/src/api/agents/agents.controller.ts @@ -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); diff --git a/apps/orchestrator/src/api/agents/agents.module.ts b/apps/orchestrator/src/api/agents/agents.module.ts index 8151b41..c6e071a 100644 --- a/apps/orchestrator/src/api/agents/agents.module.ts +++ b/apps/orchestrator/src/api/agents/agents.module.ts @@ -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 {} diff --git a/apps/orchestrator/src/common/guards/api-key.guard.spec.ts b/apps/orchestrator/src/common/guards/api-key.guard.spec.ts new file mode 100644 index 0000000..684a10e --- /dev/null +++ b/apps/orchestrator/src/common/guards/api-key.guard.spec.ts @@ -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): 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"); + }); + }); +}); diff --git a/apps/orchestrator/src/common/guards/api-key.guard.ts b/apps/orchestrator/src/common/guards/api-key.guard.ts new file mode 100644 index 0000000..6ee9d63 --- /dev/null +++ b/apps/orchestrator/src/common/guards/api-key.guard.ts @@ -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 }>(); + const providedKey = this.extractApiKeyFromHeader(request); + + if (!providedKey) { + throw new UnauthorizedException("No API key provided"); + } + + const configuredKey = this.configService.get("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 | 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; + } + } +}