fix(#274): Add input validation to prevent command injection in git operations
Some checks failed
ci/woodpecker/push/woodpecker Pipeline failed
Some checks failed
ci/woodpecker/push/woodpecker Pipeline failed
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>
This commit is contained in:
@@ -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}`);
|
||||
|
||||
Reference in New Issue
Block a user