From 0a527d2a4e9093e76c070a51437bb84b982f5f39 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Tue, 3 Feb 2026 20:47:41 -0600 Subject: [PATCH] fix(#279): Validate orchestrator URL configuration (SSRF risk) Implemented comprehensive URL validation to prevent SSRF attacks: - Created URL validator utility with protocol whitelist (http/https only) - Blocked access to private IP ranges (10.x, 192.168.x, 172.16-31.x) - Blocked loopback addresses (127.x, localhost, 0.0.0.0) - Blocked link-local addresses (169.254.x) - Blocked IPv6 localhost (::1, ::) - Allow localhost in development/test environments only - Added structured audit logging for invalid URL attempts - Comprehensive test coverage (37 tests for URL validator) Security Impact: - Prevents attackers from redirecting agent spawn requests to internal services - Blocks data exfiltration via malicious orchestrator URL - All agent operations now validated against SSRF Files changed: - apps/api/src/federation/utils/url-validator.ts (new) - apps/api/src/federation/utils/url-validator.spec.ts (new) - apps/api/src/federation/federation-agent.service.ts (validation integration) - apps/api/src/federation/federation-agent.service.spec.ts (test updates) - apps/api/src/federation/audit.service.ts (audit logging) - apps/api/src/federation/federation.module.ts (service exports) Fixes #279 Co-Authored-By: Claude Sonnet 4.5 --- apps/api/src/federation/audit.service.ts | 14 ++ .../federation-agent.service.spec.ts | 9 + .../federation/federation-agent.service.ts | 22 +- apps/api/src/federation/federation.module.ts | 15 +- .../federation/utils/url-validator.spec.ts | 238 ++++++++++++++++++ .../api/src/federation/utils/url-validator.ts | 149 +++++++++++ .../279-orchestrator-url-validation.md | 46 ++++ 7 files changed, 489 insertions(+), 4 deletions(-) create mode 100644 apps/api/src/federation/utils/url-validator.spec.ts create mode 100644 apps/api/src/federation/utils/url-validator.ts create mode 100644 docs/scratchpads/279-orchestrator-url-validation.md diff --git a/apps/api/src/federation/audit.service.ts b/apps/api/src/federation/audit.service.ts index cdf569a..5713dfe 100644 --- a/apps/api/src/federation/audit.service.ts +++ b/apps/api/src/federation/audit.service.ts @@ -207,4 +207,18 @@ export class FederationAuditService { securityEvent: true, }); } + + /** + * Log invalid orchestrator URL configuration attempt + * Logged when orchestrator URL validation fails (SSRF prevention) + */ + logInvalidOrchestratorUrl(url: string, error: string): void { + this.logger.warn({ + event: "FEDERATION_INVALID_ORCHESTRATOR_URL", + url, + error, + timestamp: new Date().toISOString(), + securityEvent: true, + }); + } } diff --git a/apps/api/src/federation/federation-agent.service.spec.ts b/apps/api/src/federation/federation-agent.service.spec.ts index f1698ce..3f4ba62 100644 --- a/apps/api/src/federation/federation-agent.service.spec.ts +++ b/apps/api/src/federation/federation-agent.service.spec.ts @@ -8,6 +8,7 @@ import { HttpService } from "@nestjs/axios"; import { ConfigService } from "@nestjs/config"; import { FederationAgentService } from "./federation-agent.service"; import { CommandService } from "./command.service"; +import { FederationAuditService } from "./audit.service"; import { PrismaService } from "../prisma/prisma.service"; import { FederationConnectionStatus } from "@prisma/client"; import { of, throwError } from "rxjs"; @@ -55,10 +56,17 @@ describe("FederationAgentService", () => { if (key === "orchestrator.url") { return mockOrchestratorUrl; } + if (key === "NODE_ENV") { + return "test"; + } return undefined; }), }; + const mockAuditService = { + logInvalidOrchestratorUrl: vi.fn(), + }; + const module: TestingModule = await Test.createTestingModule({ providers: [ FederationAgentService, @@ -66,6 +74,7 @@ describe("FederationAgentService", () => { { provide: PrismaService, useValue: mockPrisma }, { provide: HttpService, useValue: mockHttpService }, { provide: ConfigService, useValue: mockConfigService }, + { provide: FederationAuditService, useValue: mockAuditService }, ], }).compile(); diff --git a/apps/api/src/federation/federation-agent.service.ts b/apps/api/src/federation/federation-agent.service.ts index b3cf59f..5f055f2 100644 --- a/apps/api/src/federation/federation-agent.service.ts +++ b/apps/api/src/federation/federation-agent.service.ts @@ -10,7 +10,9 @@ import { ConfigService } from "@nestjs/config"; import { firstValueFrom } from "rxjs"; import { PrismaService } from "../prisma/prisma.service"; import { CommandService } from "./command.service"; +import { FederationAuditService } from "./audit.service"; import { FederationConnectionStatus } from "@prisma/client"; +import { validateUrl } from "./utils/url-validator"; import type { CommandMessageDetails } from "./types/message.types"; import type { SpawnAgentCommandPayload, @@ -46,10 +48,24 @@ export class FederationAgentService { private readonly prisma: PrismaService, private readonly commandService: CommandService, private readonly httpService: HttpService, - private readonly configService: ConfigService + private readonly configService: ConfigService, + private readonly auditService: FederationAuditService ) { - this.orchestratorUrl = - this.configService.get("orchestrator.url") ?? "http://localhost:3001"; + const url = this.configService.get("orchestrator.url") ?? ""; + const nodeEnv = this.configService.get("NODE_ENV") ?? "production"; + const isDevelopment = nodeEnv === "development" || nodeEnv === "test"; + + // Validate orchestrator URL (SSRF prevention) + const validationResult = validateUrl(url, isDevelopment); + if (!validationResult.valid) { + const errorMessage = validationResult.error ?? "Unknown validation error"; + this.logger.error(`Invalid orchestrator URL: ${errorMessage}`); + // Log security event + this.auditService.logInvalidOrchestratorUrl(url, errorMessage); + throw new Error(errorMessage); + } + + this.orchestratorUrl = url; this.logger.log( `FederationAgentService initialized with orchestrator URL: ${this.orchestratorUrl}` ); diff --git a/apps/api/src/federation/federation.module.ts b/apps/api/src/federation/federation.module.ts index ebccd9f..8d785f4 100644 --- a/apps/api/src/federation/federation.module.ts +++ b/apps/api/src/federation/federation.module.ts @@ -17,6 +17,8 @@ import { FederationAuditService } from "./audit.service"; import { SignatureService } from "./signature.service"; import { ConnectionService } from "./connection.service"; import { OIDCService } from "./oidc.service"; +import { CommandService } from "./command.service"; +import { FederationAgentService } from "./federation-agent.service"; import { PrismaModule } from "../prisma/prisma.module"; @Module({ @@ -56,7 +58,18 @@ import { PrismaModule } from "../prisma/prisma.module"; SignatureService, ConnectionService, OIDCService, + CommandService, + FederationAgentService, + ], + exports: [ + FederationService, + CryptoService, + FederationAuditService, + SignatureService, + ConnectionService, + OIDCService, + CommandService, + FederationAgentService, ], - exports: [FederationService, CryptoService, SignatureService, ConnectionService, OIDCService], }) export class FederationModule {} diff --git a/apps/api/src/federation/utils/url-validator.spec.ts b/apps/api/src/federation/utils/url-validator.spec.ts new file mode 100644 index 0000000..c37924b --- /dev/null +++ b/apps/api/src/federation/utils/url-validator.spec.ts @@ -0,0 +1,238 @@ +/** + * Tests for URL Validator Utility + */ + +import { describe, it, expect } from "vitest"; +import { validateUrl } from "./url-validator"; + +describe("validateUrl", () => { + describe("valid URLs", () => { + it("should accept valid HTTP URL", () => { + const result = validateUrl("http://orchestrator.example.com:3001"); + expect(result.valid).toBe(true); + expect(result.error).toBeUndefined(); + }); + + it("should accept valid HTTPS URL", () => { + const result = validateUrl("https://orchestrator.example.com:3001"); + expect(result.valid).toBe(true); + expect(result.error).toBeUndefined(); + }); + + it("should accept URL without port", () => { + const result = validateUrl("https://orchestrator.example.com"); + expect(result.valid).toBe(true); + expect(result.error).toBeUndefined(); + }); + + it("should accept URL with path", () => { + const result = validateUrl("https://orchestrator.example.com:3001/api/v1"); + expect(result.valid).toBe(true); + expect(result.error).toBeUndefined(); + }); + + it("should accept localhost when allowLocalhost=true", () => { + const result = validateUrl("http://localhost:3001", true); + expect(result.valid).toBe(true); + expect(result.error).toBeUndefined(); + }); + + it("should accept 127.0.0.1 when allowLocalhost=true", () => { + const result = validateUrl("http://127.0.0.1:3001", true); + expect(result.valid).toBe(true); + expect(result.error).toBeUndefined(); + }); + + it("should accept public IP address", () => { + const result = validateUrl("http://8.8.8.8:3001"); + expect(result.valid).toBe(true); + expect(result.error).toBeUndefined(); + }); + }); + + describe("invalid protocols", () => { + it("should reject FTP protocol", () => { + const result = validateUrl("ftp://orchestrator.example.com:3001"); + expect(result.valid).toBe(false); + expect(result.error).toContain("Invalid orchestrator URL protocol"); + }); + + it("should reject file protocol", () => { + const result = validateUrl("file:///etc/passwd"); + expect(result.valid).toBe(false); + expect(result.error).toContain("Invalid orchestrator URL protocol"); + }); + + it("should reject javascript protocol", () => { + const result = validateUrl("javascript:alert(1)"); + expect(result.valid).toBe(false); + expect(result.error).toContain("Invalid orchestrator URL protocol"); + }); + + it("should reject data protocol", () => { + const result = validateUrl("data:text/html,

test

"); + expect(result.valid).toBe(false); + expect(result.error).toContain("Invalid orchestrator URL protocol"); + }); + + it("should reject gopher protocol", () => { + const result = validateUrl("gopher://example.com"); + expect(result.valid).toBe(false); + expect(result.error).toContain("Invalid orchestrator URL protocol"); + }); + }); + + describe("malformed URLs", () => { + it("should reject empty URL", () => { + const result = validateUrl(""); + expect(result.valid).toBe(false); + expect(result.error).toContain("Orchestrator URL is required"); + }); + + it("should reject whitespace-only URL", () => { + const result = validateUrl(" "); + expect(result.valid).toBe(false); + expect(result.error).toContain("Orchestrator URL is required"); + }); + + it("should reject URL without protocol", () => { + const result = validateUrl("orchestrator.example.com:3001"); + expect(result.valid).toBe(false); + expect(result.error).toContain("Invalid orchestrator URL"); + }); + + it("should reject invalid URL format", () => { + const result = validateUrl("not a url at all"); + expect(result.valid).toBe(false); + expect(result.error).toContain("Invalid orchestrator URL"); + }); + }); + + describe("private IP addresses", () => { + it("should reject localhost", () => { + const result = validateUrl("http://localhost:3001"); + expect(result.valid).toBe(false); + expect(result.error).toContain("private/internal addresses"); + }); + + it("should reject 127.0.0.1", () => { + const result = validateUrl("http://127.0.0.1:3001"); + expect(result.valid).toBe(false); + expect(result.error).toContain("private/internal addresses"); + }); + + it("should reject 127.x.x.x range", () => { + const result = validateUrl("http://127.1.1.1:3001"); + expect(result.valid).toBe(false); + expect(result.error).toContain("private/internal addresses"); + }); + + it("should reject 0.0.0.0", () => { + const result = validateUrl("http://0.0.0.0:3001"); + expect(result.valid).toBe(false); + expect(result.error).toContain("private/internal addresses"); + }); + + it("should reject 10.x.x.x range", () => { + const result = validateUrl("http://10.0.0.1:3001"); + expect(result.valid).toBe(false); + expect(result.error).toContain("private/internal addresses"); + }); + + it("should reject 10.255.255.255", () => { + const result = validateUrl("http://10.255.255.255:3001"); + expect(result.valid).toBe(false); + expect(result.error).toContain("private/internal addresses"); + }); + + it("should reject 192.168.x.x range", () => { + const result = validateUrl("http://192.168.1.1:3001"); + expect(result.valid).toBe(false); + expect(result.error).toContain("private/internal addresses"); + }); + + it("should reject 192.168.0.1", () => { + const result = validateUrl("http://192.168.0.1:3001"); + expect(result.valid).toBe(false); + expect(result.error).toContain("private/internal addresses"); + }); + + it("should reject 172.16.x.x range", () => { + const result = validateUrl("http://172.16.0.1:3001"); + expect(result.valid).toBe(false); + expect(result.error).toContain("private/internal addresses"); + }); + + it("should reject 172.31.x.x (end of range)", () => { + const result = validateUrl("http://172.31.255.255:3001"); + expect(result.valid).toBe(false); + expect(result.error).toContain("private/internal addresses"); + }); + + it("should accept 172.15.x.x (before private range)", () => { + const result = validateUrl("http://172.15.0.1:3001"); + expect(result.valid).toBe(true); + expect(result.error).toBeUndefined(); + }); + + it("should accept 172.32.x.x (after private range)", () => { + const result = validateUrl("http://172.32.0.1:3001"); + expect(result.valid).toBe(true); + expect(result.error).toBeUndefined(); + }); + + it("should reject 169.254.x.x (link-local)", () => { + const result = validateUrl("http://169.254.1.1:3001"); + expect(result.valid).toBe(false); + expect(result.error).toContain("private/internal addresses"); + }); + + it("should reject ::1 (IPv6 localhost)", () => { + const result = validateUrl("http://[::1]:3001"); + expect(result.valid).toBe(false); + expect(result.error).toContain("private/internal addresses"); + }); + + it("should reject :: (IPv6 any)", () => { + const result = validateUrl("http://[::]:3001"); + expect(result.valid).toBe(false); + expect(result.error).toContain("private/internal addresses"); + }); + + it("should reject .localhost domain", () => { + const result = validateUrl("http://app.localhost:3001"); + expect(result.valid).toBe(false); + expect(result.error).toContain("private/internal addresses"); + }); + + it("should reject .local domain", () => { + const result = validateUrl("http://orchestrator.local:3001"); + expect(result.valid).toBe(false); + expect(result.error).toContain("private/internal addresses"); + }); + }); + + describe("allowLocalhost parameter", () => { + it("should allow localhost when allowLocalhost=true", () => { + const result = validateUrl("http://localhost:3001", true); + expect(result.valid).toBe(true); + }); + + it("should allow 127.0.0.1 when allowLocalhost=true", () => { + const result = validateUrl("http://127.0.0.1:3001", true); + expect(result.valid).toBe(true); + }); + + it("should still reject 10.x.x.x even with allowLocalhost=true", () => { + const result = validateUrl("http://10.0.0.1:3001", true); + expect(result.valid).toBe(false); + expect(result.error).toContain("private/internal addresses"); + }); + + it("should still reject 192.168.x.x even with allowLocalhost=true", () => { + const result = validateUrl("http://192.168.1.1:3001", true); + expect(result.valid).toBe(false); + expect(result.error).toContain("private/internal addresses"); + }); + }); +}); diff --git a/apps/api/src/federation/utils/url-validator.ts b/apps/api/src/federation/utils/url-validator.ts new file mode 100644 index 0000000..1ab4495 --- /dev/null +++ b/apps/api/src/federation/utils/url-validator.ts @@ -0,0 +1,149 @@ +/** + * URL Validator Utility + * + * Validates URLs to prevent SSRF attacks by ensuring: + * - Valid URL format + * - Whitelisted protocols (http/https) + * - No access to private/internal IP addresses + */ + +/** + * Validation result + */ +export interface UrlValidationResult { + valid: boolean; + error?: string; +} + +/** + * Validate a URL for security concerns + * @param url URL to validate + * @param allowLocalhost Whether to allow localhost (for development) + * @returns Validation result + */ +export function validateUrl(url: string, allowLocalhost = false): UrlValidationResult { + // Check if URL is provided + if (!url || url.trim() === "") { + return { + valid: false, + error: "Orchestrator URL is required", + }; + } + + // Parse URL + let parsedUrl: URL; + try { + parsedUrl = new URL(url); + } catch { + return { + valid: false, + error: "Invalid orchestrator URL format", + }; + } + + // Validate protocol (only http and https allowed) + if (parsedUrl.protocol !== "http:" && parsedUrl.protocol !== "https:") { + return { + valid: false, + error: "Invalid orchestrator URL protocol (only http and https allowed)", + }; + } + + // Validate hostname (prevent SSRF to private addresses) + const hostname = parsedUrl.hostname.toLowerCase(); + + // Check for private/internal addresses + if (isPrivateOrInternalAddress(hostname, allowLocalhost)) { + return { + valid: false, + error: "Orchestrator URL cannot point to private/internal addresses", + }; + } + + return { valid: true }; +} + +/** + * Check if hostname is a private or internal address + * @param hostname Hostname to check + * @param allowLocalhost Whether localhost/loopback is allowed + * @returns True if private/internal + */ +function isPrivateOrInternalAddress(hostname: string, allowLocalhost = false): boolean { + const isLocalhost = + hostname === "localhost" || + hostname === "0.0.0.0" || + hostname.endsWith(".localhost") || + hostname.endsWith(".local"); + + // Check if it's an IP address + const ipv4Regex = /^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/; + const ipv4Match = ipv4Regex.exec(hostname); + + if (ipv4Match) { + const [, octet1, octet2, octet3, octet4] = ipv4Match; + const o1 = parseInt(octet1 ?? "0", 10); + const o2 = parseInt(octet2 ?? "0", 10); + const o3 = parseInt(octet3 ?? "0", 10); + const o4 = parseInt(octet4 ?? "0", 10); + + // Validate octets are in range + if (o1 > 255 || o2 > 255 || o3 > 255 || o4 > 255) { + return true; // Invalid IP, treat as suspicious + } + + // Check for loopback (127.0.0.0/8) and 0.0.0.0 + const isLoopback = o1 === 127 || (o1 === 0 && o2 === 0 && o3 === 0 && o4 === 0); + + if (isLoopback && allowLocalhost) { + return false; // Allow loopback in development + } + + if (isLoopback) { + return true; + } + + // Check for private IP ranges (always blocked) + // 10.0.0.0/8 + if (o1 === 10) { + return true; + } + + // 172.16.0.0/12 + if (o1 === 172 && o2 >= 16 && o2 <= 31) { + return true; + } + + // 192.168.0.0/16 + if (o1 === 192 && o2 === 168) { + return true; + } + + // 169.254.0.0/16 (link-local) + if (o1 === 169 && o2 === 254) { + return true; + } + } + + // Check for IPv6 localhost (URL class removes brackets) + const isIpv6Localhost = + hostname === "::1" || hostname === "::" || hostname === "[::1]" || hostname === "[::]"; + if (isIpv6Localhost && allowLocalhost) { + return false; + } + + if (isIpv6Localhost) { + return true; + } + + // Check for localhost domain names + if (isLocalhost && allowLocalhost) { + return false; + } + + if (isLocalhost) { + return true; + } + + return false; +} diff --git a/docs/scratchpads/279-orchestrator-url-validation.md b/docs/scratchpads/279-orchestrator-url-validation.md new file mode 100644 index 0000000..4ad444a --- /dev/null +++ b/docs/scratchpads/279-orchestrator-url-validation.md @@ -0,0 +1,46 @@ +# Issue #279: Validate orchestrator URL configuration (SSRF risk) + +## Objective + +Prevent SSRF vulnerability by validating orchestrator URL from environment variables. Ensure URL format is valid, protocol is whitelisted (http/https), and hostname is not malicious. + +## Security Impact + +- SSRF vulnerability - attacker could point URL to internal services +- Data exfiltration - agent spawn requests sent to attacker-controlled server +- All agent operations compromised + +## Location + +`apps/api/src/federation/federation-agent.service.ts:43-56` + +## Approach + +1. Create URL validation utility function +2. Whitelist protocols (http, https only) +3. Validate hostname (reject localhost, private IPs, loopback) +4. Add structured logging for validation failures via audit service +5. Write comprehensive tests + +## Implementation Plan + +- [ ] Write tests for URL validation (RED) +- [ ] Implement URL validation logic (GREEN) +- [ ] Integrate validation into FederationAgentService constructor +- [ ] Add audit logging for invalid URLs +- [ ] Refactor for clarity +- [ ] Run quality gates + +## Testing + +- Valid URLs (http://example.com:3001, https://orchestrator.example.com) +- Invalid protocols (ftp://, file://, javascript:) +- Internal/private IPs (127.0.0.1, 192.168.x.x, 10.x.x.x) +- Localhost variants (localhost, 0.0.0.0) +- Malformed URLs + +## Notes + +- Use Node's built-in URL class for parsing +- Consider environment-specific allowlists (dev can use localhost) +- Add security event logging via FederationAuditService