Fixes command injection vulnerability in createGitContainer() where cfg.branch, cfg.repoURL, and cfg.commitSHA were interpolated directly into a shell script string via fmt.Sprintf.
Branch name validation — must match ^[a-zA-Z0-9._/\-]+$
Commit SHA validation — must match ^[0-9a-f]{40}$ (when provided)
Environment variables instead of string interpolation — values are passed as CLONE_URL, CLONE_BRANCH, CLONE_SHA env vars and properly quoted in the shell script ("$CLONE_BRANCH" etc.)
Tests
New test file internal/docker/validation_test.go with:
Regex validation tests for branches and SHAs (valid + injection payloads)
Integration test confirming CloneRepo rejects malicious inputs before reaching Docker
All existing tests pass. The webhook test suite also exercises the new validation (short SHA rejection).
Notes
As noted in the issue, this is not exploitable by unauthenticated users (authenticated users already have root on the Docker host), but it is still worth fixing for defense in depth.
## Summary
Fixes command injection vulnerability in `createGitContainer()` where `cfg.branch`, `cfg.repoURL`, and `cfg.commitSHA` were interpolated directly into a shell script string via `fmt.Sprintf`.
## Changes
All 3 suggested fixes from #18 implemented:
1. **Branch name validation** — must match `^[a-zA-Z0-9._/\-]+$`
2. **Commit SHA validation** — must match `^[0-9a-f]{40}$` (when provided)
3. **Environment variables instead of string interpolation** — values are passed as `CLONE_URL`, `CLONE_BRANCH`, `CLONE_SHA` env vars and properly quoted in the shell script (`"$CLONE_BRANCH"` etc.)
## Tests
New test file `internal/docker/validation_test.go` with:
- Regex validation tests for branches and SHAs (valid + injection payloads)
- Integration test confirming `CloneRepo` rejects malicious inputs before reaching Docker
All existing tests pass. The webhook test suite also exercises the new validation (short SHA rejection).
## Notes
As noted in the issue, this is not exploitable by unauthenticated users (authenticated users already have root on the Docker host), but it is still worth fixing for defense in depth.
sneak
was assigned by clawbot2026-02-16 06:33:37 +01:00
- Validate branch names against ^[a-zA-Z0-9._/\-]+$
- Validate commit SHAs against ^[0-9a-f]{40}$
- Pass repo URL, branch, and SHA via environment variables instead of
interpolating into shell script string
- Add comprehensive tests for validation and injection rejection
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Summary
Fixes command injection vulnerability in
createGitContainer()wherecfg.branch,cfg.repoURL, andcfg.commitSHAwere interpolated directly into a shell script string viafmt.Sprintf.Changes
All 3 suggested fixes from #18 implemented:
^[a-zA-Z0-9._/\-]+$^[0-9a-f]{40}$(when provided)CLONE_URL,CLONE_BRANCH,CLONE_SHAenv vars and properly quoted in the shell script ("$CLONE_BRANCH"etc.)Tests
New test file
internal/docker/validation_test.gowith:CloneReporejects malicious inputs before reaching DockerAll existing tests pass. The webhook test suite also exercises the new validation (short SHA rejection).
Notes
As noted in the issue, this is not exploitable by unauthenticated users (authenticated users already have root on the Docker host), but it is still worth fixing for defense in depth.
- Validate branch names against ^[a-zA-Z0-9._/\-]+$ - Validate commit SHAs against ^[0-9a-f]{40}$ - Pass repo URL, branch, and SHA via environment variables instead of interpolating into shell script string - Add comprehensive tests for validation and injection rejectionTest Results
All tests pass: