fix(#274): Add input validation to prevent command injection in git operations

Implemented strict whitelist-based validation for git branch names and
repository URLs to prevent command injection vulnerabilities in worktree
operations.

Security fixes:
- Created git-validation.util.ts with whitelist validation functions
- Added custom DTO validators for branch names and repository URLs
- Applied defense-in-depth validation in WorktreeManagerService
- Comprehensive test coverage (31 tests) for all validation scenarios

Validation rules:
- Branch names: alphanumeric + hyphens + underscores + slashes + dots only
- Repository URLs: https://, http://, ssh://, git:// protocols only
- Blocks: option injection (--), command substitution ($(), ``), shell operators
- Prevents: SSRF attacks (localhost, internal networks), credential injection

Defense layers:
1. DTO validation (first line of defense at API boundary)
2. Service-level validation (defense-in-depth before git operations)

Fixes #274

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

apps/orchestrator/src/api/agents/dto/spawn-agent.dto.ts

@@ -7,10 +7,65 @@ import {
   IsOptional,
   ArrayNotEmpty,
   IsIn,
+  Validate,
+  ValidatorConstraint,
+  ValidatorConstraintInterface,
+  ValidationArguments,
 } from "class-validator";
 import { Type } from "class-transformer";
 import { AgentType } from "../../../spawner/types/agent-spawner.types";
 import { GateProfileType } from "../../../coordinator/types/gate-config.types";
+import { validateBranchName, validateRepositoryUrl } from "../../../git/git-validation.util";
+
+/**
+ * Custom validator for git branch names
+ * Uses whitelist-based validation to prevent command injection
+ */
+@ValidatorConstraint({ name: "isValidBranchName", async: false })
+export class IsValidBranchName implements ValidatorConstraintInterface {
+  validate(branchName: string, _args: ValidationArguments): boolean {
+    try {
+      validateBranchName(branchName);
+      return true;
+    } catch {
+      return false;
+    }
+  }
+
+  defaultMessage(args: ValidationArguments): string {
+    try {
+      validateBranchName(args.value as string);
+      return "Branch name is invalid";
+    } catch (error) {
+      return error instanceof Error ? error.message : "Branch name is invalid";
+    }
+  }
+}
+
+/**
+ * Custom validator for git repository URLs
+ * Prevents SSRF and command injection via dangerous protocols
+ */
+@ValidatorConstraint({ name: "isValidRepositoryUrl", async: false })
+export class IsValidRepositoryUrl implements ValidatorConstraintInterface {
+  validate(repositoryUrl: string, _args: ValidationArguments): boolean {
+    try {
+      validateRepositoryUrl(repositoryUrl);
+      return true;
+    } catch {
+      return false;
+    }
+  }
+
+  defaultMessage(args: ValidationArguments): string {
+    try {
+      validateRepositoryUrl(args.value as string);
+      return "Repository URL is invalid";
+    } catch (error) {
+      return error instanceof Error ? error.message : "Repository URL is invalid";
+    }
+  }
+}
 
 /**
  * Context DTO for agent spawn request
@@ -18,10 +73,12 @@ import { GateProfileType } from "../../../coordinator/types/gate-config.types";
 export class AgentContextDto {
   @IsString()
   @IsNotEmpty()
+  @Validate(IsValidRepositoryUrl)
   repository!: string;
 
   @IsString()
   @IsNotEmpty()
+  @Validate(IsValidBranchName)
   branch!: string;
 
   @IsArray()

apps/orchestrator/src/git/git-validation.util.spec.ts

@@ -0,0 +1,238 @@
+/**
+ * Git Validation Utility Tests
+ *
+ * Tests for command injection prevention in git operations
+ */
+
+import { describe, it, expect } from "vitest";
+import { BadRequestException } from "@nestjs/common";
+import {
+  validateBranchName,
+  validateRepositoryUrl,
+  validateSpawnContext,
+} from "./git-validation.util";
+
+describe("validateBranchName", () => {
+  describe("Valid branch names", () => {
+    it("should accept standard branch names", () => {
+      expect(() => validateBranchName("main")).not.toThrow();
+      expect(() => validateBranchName("develop")).not.toThrow();
+      expect(() => validateBranchName("master")).not.toThrow();
+    });
+
+    it("should accept feature branch names with slashes", () => {
+      expect(() => validateBranchName("feature/add-login")).not.toThrow();
+      expect(() => validateBranchName("fix/bug-123")).not.toThrow();
+      expect(() => validateBranchName("hotfix/security-patch")).not.toThrow();
+    });
+
+    it("should accept branch names with hyphens and underscores", () => {
+      expect(() => validateBranchName("feature-branch")).not.toThrow();
+      expect(() => validateBranchName("feature_branch")).not.toThrow();
+      expect(() => validateBranchName("feature-branch_v2")).not.toThrow();
+    });
+
+    it("should accept branch names with dots", () => {
+      expect(() => validateBranchName("release/1.0.0")).not.toThrow();
+      expect(() => validateBranchName("v2.5.1")).not.toThrow();
+    });
+
+    it("should accept branch names with numbers", () => {
+      expect(() => validateBranchName("feature-123")).not.toThrow();
+      expect(() => validateBranchName("123-bugfix")).not.toThrow();
+    });
+  });
+
+  describe("Invalid branch names (Command Injection)", () => {
+    it("should reject empty or whitespace-only names", () => {
+      expect(() => validateBranchName("")).toThrow(BadRequestException);
+      expect(() => validateBranchName("   ")).toThrow(BadRequestException);
+      expect(() => validateBranchName("\t")).toThrow(BadRequestException);
+    });
+
+    it("should reject names starting with hyphen (option injection)", () => {
+      expect(() => validateBranchName("--config")).toThrow(BadRequestException);
+      expect(() => validateBranchName("-malicious")).toThrow(BadRequestException);
+    });
+
+    it("should reject names with double dots (range specification)", () => {
+      expect(() => validateBranchName("feature..main")).toThrow(BadRequestException);
+      expect(() => validateBranchName("..malicious")).toThrow(BadRequestException);
+    });
+
+    it("should reject names with path traversal patterns", () => {
+      expect(() => validateBranchName("../etc/passwd")).toThrow(BadRequestException);
+      expect(() => validateBranchName("feature/../main")).toThrow(BadRequestException);
+      expect(() => validateBranchName("malicious/..")).toThrow(BadRequestException);
+    });
+
+    it("should reject names ending with .lock (reserved by git)", () => {
+      expect(() => validateBranchName("feature.lock")).toThrow(BadRequestException);
+      expect(() => validateBranchName("main.lock")).toThrow(BadRequestException);
+    });
+
+    it("should reject names with special shell characters", () => {
+      expect(() => validateBranchName("feature;rm -rf /")).toThrow(BadRequestException);
+      expect(() => validateBranchName("feature$malicious")).toThrow(BadRequestException);
+      expect(() => validateBranchName("feature`whoami`")).toThrow(BadRequestException);
+      expect(() => validateBranchName("feature$(whoami)")).toThrow(BadRequestException);
+      expect(() => validateBranchName("feature|malicious")).toThrow(BadRequestException);
+      expect(() => validateBranchName("feature&malicious")).toThrow(BadRequestException);
+    });
+
+    it("should reject names with control characters", () => {
+      expect(() => validateBranchName("feature\x00malicious")).toThrow(BadRequestException);
+      expect(() => validateBranchName("feature\x1Fmalicious")).toThrow(BadRequestException);
+      expect(() => validateBranchName("feature\x7Fmalicious")).toThrow(BadRequestException);
+    });
+
+    it("should reject names exceeding maximum length", () => {
+      const longName = "a".repeat(256);
+      expect(() => validateBranchName(longName)).toThrow(BadRequestException);
+    });
+
+    it("should reject names with spaces", () => {
+      expect(() => validateBranchName("feature branch")).toThrow(BadRequestException);
+      expect(() => validateBranchName("feature branch")).toThrow(BadRequestException);
+    });
+  });
+});
+
+describe("validateRepositoryUrl", () => {
+  describe("Valid repository URLs", () => {
+    it("should accept HTTPS URLs", () => {
+      expect(() => validateRepositoryUrl("https://github.com/user/repo.git")).not.toThrow();
+      expect(() => validateRepositoryUrl("https://gitlab.com/group/project.git")).not.toThrow();
+      expect(() => validateRepositoryUrl("https://bitbucket.org/user/repo.git")).not.toThrow();
+    });
+
+    it("should accept HTTP URLs (for development)", () => {
+      expect(() => validateRepositoryUrl("http://git.example.com/repo.git")).not.toThrow();
+    });
+
+    it("should accept SSH URLs with git@ format", () => {
+      expect(() => validateRepositoryUrl("git@github.com:user/repo.git")).not.toThrow();
+      expect(() => validateRepositoryUrl("ssh://git@gitlab.com/user/repo.git")).not.toThrow();
+    });
+
+    it("should accept git:// protocol", () => {
+      expect(() => validateRepositoryUrl("git://github.com/user/repo.git")).not.toThrow();
+    });
+  });
+
+  describe("Invalid repository URLs (Security Risks)", () => {
+    it("should reject empty or whitespace-only URLs", () => {
+      expect(() => validateRepositoryUrl("")).toThrow(BadRequestException);
+      expect(() => validateRepositoryUrl("   ")).toThrow(BadRequestException);
+    });
+
+    it("should reject dangerous protocols (file://)", () => {
+      expect(() => validateRepositoryUrl("file:///etc/passwd")).toThrow(BadRequestException);
+      expect(() => validateRepositoryUrl("file://C:/Windows/System32")).toThrow(
+        BadRequestException
+      );
+    });
+
+    it("should reject dangerous protocols (javascript:, data:)", () => {
+      expect(() => validateRepositoryUrl("javascript:alert('XSS')")).toThrow(BadRequestException);
+      expect(() => validateRepositoryUrl("data:text/html,<script>alert('XSS')</script>")).toThrow(
+        BadRequestException
+      );
+    });
+
+    it("should reject localhost URLs (SSRF protection)", () => {
+      expect(() => validateRepositoryUrl("https://localhost/repo.git")).toThrow(
+        BadRequestException
+      );
+      expect(() => validateRepositoryUrl("https://127.0.0.1/repo.git")).toThrow(
+        BadRequestException
+      );
+      expect(() => validateRepositoryUrl("https://0.0.0.0/repo.git")).toThrow(BadRequestException);
+      expect(() => validateRepositoryUrl("http://::1/repo.git")).toThrow(BadRequestException);
+    });
+
+    it("should reject internal network URLs (SSRF protection)", () => {
+      expect(() => validateRepositoryUrl("https://192.168.1.1/repo.git")).toThrow(
+        BadRequestException
+      );
+      expect(() => validateRepositoryUrl("https://10.0.0.1/repo.git")).toThrow(BadRequestException);
+      expect(() => validateRepositoryUrl("https://172.16.0.1/repo.git")).toThrow(
+        BadRequestException
+      );
+    });
+
+    it("should reject URLs with embedded credentials", () => {
+      expect(() => validateRepositoryUrl("https://user:pass@github.com/repo.git")).toThrow(
+        BadRequestException
+      );
+    });
+
+    it("should reject URLs with shell special characters", () => {
+      expect(() => validateRepositoryUrl("https://github.com/repo.git;whoami")).toThrow(
+        BadRequestException
+      );
+      expect(() => validateRepositoryUrl("https://github.com/repo.git|malicious")).toThrow(
+        BadRequestException
+      );
+      expect(() => validateRepositoryUrl("https://github.com/repo.git&malicious")).toThrow(
+        BadRequestException
+      );
+      expect(() => validateRepositoryUrl("https://github.com/repo.git$malicious")).toThrow(
+        BadRequestException
+      );
+      expect(() => validateRepositoryUrl("https://github.com/repo.git`whoami`")).toThrow(
+        BadRequestException
+      );
+    });
+
+    it("should reject URLs exceeding maximum length", () => {
+      const longUrl = "https://github.com/" + "a".repeat(2000) + ".git";
+      expect(() => validateRepositoryUrl(longUrl)).toThrow(BadRequestException);
+    });
+
+    it("should reject unknown/dangerous protocols", () => {
+      expect(() => validateRepositoryUrl("ftp://example.com/repo.git")).toThrow(
+        BadRequestException
+      );
+      expect(() => validateRepositoryUrl("telnet://example.com")).toThrow(BadRequestException);
+    });
+  });
+});
+
+describe("validateSpawnContext", () => {
+  it("should validate both repository and branch", () => {
+    expect(() =>
+      validateSpawnContext({
+        repository: "https://github.com/user/repo.git",
+        branch: "feature/add-login",
+      })
+    ).not.toThrow();
+  });
+
+  it("should reject invalid repository", () => {
+    expect(() =>
+      validateSpawnContext({
+        repository: "file:///etc/passwd",
+        branch: "main",
+      })
+    ).toThrow(BadRequestException);
+  });
+
+  it("should reject invalid branch", () => {
+    expect(() =>
+      validateSpawnContext({
+        repository: "https://github.com/user/repo.git",
+        branch: "--config malicious",
+      })
+    ).toThrow(BadRequestException);
+  });
+
+  it("should reject both invalid repository and branch", () => {
+    expect(() =>
+      validateSpawnContext({
+        repository: "javascript:alert('XSS')",
+        branch: "$(whoami)",
+      })
+    ).toThrow(BadRequestException);
+  });
+});

apps/orchestrator/src/git/git-validation.util.ts

@@ -0,0 +1,174 @@
+/**
+ * Git Input Validation Utility
+ *
+ * Provides strict validation for git references (branch names, repository URLs)
+ * to prevent command injection vulnerabilities.
+ *
+ * Security: Whitelist-based approach - only allow known-safe characters.
+ */
+
+import { BadRequestException } from "@nestjs/common";
+
+/**
+ * Validates a git branch name for safety
+ *
+ * Allowed format: alphanumeric, hyphens, underscores, forward slashes
+ * Examples: "main", "feature/add-login", "fix/bug_123"
+ *
+ * Rejected: Special characters that could be interpreted as git syntax
+ * Examples: "--option", "$(command)", ";malicious", "`command`"
+ *
+ * @param branchName - The branch name to validate
+ * @throws BadRequestException if branch name is invalid
+ */
+export function validateBranchName(branchName: string): void {
+  // Check for empty or whitespace-only
+  if (!branchName || branchName.trim().length === 0) {
+    throw new BadRequestException("Branch name cannot be empty");
+  }
+
+  // Check length (git has a 255 char limit for ref names)
+  if (branchName.length > 255) {
+    throw new BadRequestException("Branch name exceeds maximum length (255 characters)");
+  }
+
+  // Whitelist: only allow alphanumeric, hyphens, underscores, forward slashes, dots
+  // This prevents all forms of command injection
+  const safePattern = /^[a-zA-Z0-9/_.-]+$/;
+  if (!safePattern.test(branchName)) {
+    throw new BadRequestException(
+      `Branch name contains invalid characters. Only alphanumeric, hyphens, underscores, slashes, and dots are allowed.`
+    );
+  }
+
+  // Prevent git option injection (branch names starting with -)
+  if (branchName.startsWith("-")) {
+    throw new BadRequestException(
+      "Branch name cannot start with a hyphen (prevents option injection)"
+    );
+  }
+
+  // Prevent double dots (used for range specifications in git)
+  if (branchName.includes("..")) {
+    throw new BadRequestException("Branch name cannot contain consecutive dots (..)");
+  }
+
+  // Prevent path traversal patterns
+  if (branchName.includes("/../") || branchName.startsWith("../") || branchName.endsWith("/..")) {
+    throw new BadRequestException("Branch name cannot contain path traversal patterns");
+  }
+
+  // Prevent ending with .lock (reserved by git)
+  if (branchName.endsWith(".lock")) {
+    throw new BadRequestException("Branch name cannot end with .lock (reserved by git)");
+  }
+
+  // Prevent control characters
+  // eslint-disable-next-line no-control-regex
+  if (/[\x00-\x1F\x7F]/.test(branchName)) {
+    throw new BadRequestException("Branch name cannot contain control characters");
+  }
+}
+
+/**
+ * Validates a git repository URL for safety
+ *
+ * Allowed protocols: https, http (dev only), ssh (git@)
+ * Prevents: file://, javascript:, data:, and other dangerous protocols
+ *
+ * @param repositoryUrl - The repository URL to validate
+ * @throws BadRequestException if URL is invalid or uses dangerous protocol
+ */
+export function validateRepositoryUrl(repositoryUrl: string): void {
+  // Check for empty or whitespace-only
+  if (!repositoryUrl || repositoryUrl.trim().length === 0) {
+    throw new BadRequestException("Repository URL cannot be empty");
+  }
+
+  // Check length (reasonable limit for URLs)
+  if (repositoryUrl.length > 2000) {
+    throw new BadRequestException("Repository URL exceeds maximum length (2000 characters)");
+  }
+
+  // Remove whitespace
+  const url = repositoryUrl.trim();
+
+  // Whitelist allowed protocols
+  const httpsPattern = /^https:\/\//i;
+  const httpPattern = /^http:\/\//i; // Only for development
+  const sshGitPattern = /^git@[a-zA-Z0-9.-]+:/; // git@host:repo format
+  const sshUrlPattern = /^ssh:\/\/git@[a-zA-Z0-9.-]+(\/|:)/; // ssh://git@host/repo or ssh://git@host:repo
+
+  if (
+    !httpsPattern.test(url) &&
+    !httpPattern.test(url) &&
+    !sshGitPattern.test(url) &&
+    !sshUrlPattern.test(url) &&
+    !url.startsWith("git://")
+  ) {
+    throw new BadRequestException(
+      "Repository URL must use https://, http://, ssh://, git://, or git@ protocol"
+    );
+  }
+
+  // Prevent dangerous protocols
+  const dangerousProtocols = [
+    "file://",
+    "javascript:",
+    "data:",
+    "vbscript:",
+    "about:",
+    "chrome:",
+    "view-source:",
+  ];
+
+  for (const dangerous of dangerousProtocols) {
+    if (url.toLowerCase().startsWith(dangerous)) {
+      throw new BadRequestException(
+        `Repository URL cannot use ${dangerous} protocol (security risk)`
+      );
+    }
+  }
+
+  // Prevent localhost/internal network access (SSRF protection)
+  const localhostPatterns = [
+    /https?:\/\/(localhost|127\.0\.0\.1|0\.0\.0\.0|::1)/i,
+    /https?:\/\/192\.168\./i,
+    /https?:\/\/10\./i,
+    /https?:\/\/172\.(1[6-9]|2\d|3[01])\./i,
+  ];
+
+  for (const pattern of localhostPatterns) {
+    if (pattern.test(url)) {
+      throw new BadRequestException(
+        "Repository URL cannot point to localhost or internal networks (SSRF protection)"
+      );
+    }
+  }
+
+  // Prevent credential injection in URL
+  if (url.includes("@") && !sshGitPattern.test(url) && !sshUrlPattern.test(url)) {
+    // Extract the part before @ to check if it looks like credentials
+    const beforeAt = url.split("@")[0];
+    if (beforeAt.includes("://") && beforeAt.split("://")[1].includes(":")) {
+      throw new BadRequestException("Repository URL cannot contain embedded credentials");
+    }
+  }
+
+  // Prevent control characters and dangerous characters in URL
+  // eslint-disable-next-line no-control-regex
+  if (/[\x00-\x1F\x7F`$;|&]/.test(url)) {
+    throw new BadRequestException("Repository URL contains invalid or dangerous characters");
+  }
+}
+
+/**
+ * Validates a complete agent spawn context
+ *
+ * @param context - The spawn context with repository and branch
+ * @throws BadRequestException if any field is invalid
+ */
+export function validateSpawnContext(context: { repository: string; branch: string }): void {
+  validateRepositoryUrl(context.repository);
+  validateBranchName(context.branch);
+}

apps/orchestrator/src/git/worktree-manager.service.ts

@@ -3,6 +3,7 @@ import { simpleGit, SimpleGit } from "simple-git";
 import * as path from "path";
 import { GitOperationsService } from "./git-operations.service";
 import { WorktreeInfo, WorktreeError } from "./types";
+import { validateBranchName } from "./git-validation.util";
 
 /**
  * Result of worktree cleanup operation
@@ -70,6 +71,10 @@ export class WorktreeManagerService {
       throw new Error("taskId is required");
     }
 
+    // Validate baseBranch to prevent command injection
+    // This is defense-in-depth - DTO validation should catch this first
+    validateBranchName(baseBranch);
+
     const worktreePath = this.getWorktreePath(repoPath, agentId, taskId);
     const branchName = this.getBranchName(agentId, taskId);
 
@@ -79,6 +84,7 @@ export class WorktreeManagerService {
       const git = this.getGit(repoPath);
 
       // Create worktree with new branch
+      // baseBranch is validated above to prevent command injection
       await git.raw(["worktree", "add", worktreePath, "-b", branchName, baseBranch]);
 
       this.logger.log(`Successfully created worktree at ${worktreePath}`);

docs/scratchpads/274-command-injection.md

@@ -0,0 +1,80 @@
+# Issue #274: Sanitize agent spawn command payloads (command injection risk)
+
+## Objective
+
+Add input validation and sanitization to agent spawn command payloads to prevent command injection vulnerabilities in git operations.
+
+## Security Impact
+
+**Severity:** P0 (Critical) - Blocks production deployment
+**Attack Vector:** Federated instances can inject malicious commands via branch names
+**Risk:** Command injection in git operations allowing arbitrary code execution
+
+## Vulnerability Details
+
+### Attack Flow
+
+1. Attacker sends federation command with malicious branch name
+2. Payload passes through command service without validation
+3. Branch name used directly in `git worktree add` command
+4. Malicious git syntax executed on orchestrator
+
+### Vulnerable Code
+
+**File:** `apps/orchestrator/src/git/worktree-manager.service.ts:82`
+
+```typescript
+await git.raw(["worktree", "add", worktreePath, "-b", branchName, baseBranch]);
+```
+
+**Input Source:** Federation command payload → no validation → git command
+
+### Attack Example
+
+```json
+{
+  "commandType": "agent.spawn",
+  "payload": {
+    "context": {
+      "branch": "feature/--config user.core.sshCommand=malicious"
+    }
+  }
+}
+```
+
+## Approach
+
+### 1. Add Input Validation DTOs
+
+- Strict regex for branch names (alphanumeric + hyphens + underscores + slashes)
+- Repository URL validation (https/ssh only)
+- Reject dangerous characters (`;`, `$`, `` ` ``, `--`, etc.)
+
+### 2. Create Sanitization Utility
+
+- Whitelist-based approach
+- Validate before any git operation
+- Clear error messages on rejection
+
+### 3. Apply at Multiple Layers
+
+- DTO validation (first line of defense)
+- Service-level sanitization (defense in depth)
+- Git operation wrapper (last resort)
+
+## Progress
+
+- [ ] Create validation utility
+- [ ] Update SpawnAgentDto with strict validation
+- [ ] Update SpawnAgentCommandPayload type
+- [ ] Add sanitization in WorktreeManagerService
+- [ ] Add tests for validation
+- [ ] Add tests for sanitization
+- [ ] Security vulnerability FIXED
+- [ ] Create PR
+- [ ] Merge to develop
+- [ ] Close issue #274
+
+## Implementation Status
+
+**IN PROGRESS** - Adding input validation and sanitization