Jason Woltje
7a84d96d72
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>
81 lines
2.1 KiB
Markdown
81 lines
2.1 KiB
Markdown
# 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
|