fix: prevent command injection in git clone arguments (closes #18)

- 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
This commit is contained in:
2026-02-15 21:33:02 -08:00
parent 97ee1e212f
commit 7c0278439d
2 changed files with 179 additions and 21 deletions

View File

@@ -10,6 +10,7 @@ import (
"log/slog"
"os"
"path/filepath"
"regexp"
"strconv"
"strings"
@@ -46,6 +47,18 @@ var ErrNotConnected = errors.New("docker client not connected")
// ErrGitCloneFailed is returned when git clone fails.
var ErrGitCloneFailed = errors.New("git clone failed")
// ErrInvalidBranch is returned when a branch name contains invalid characters.
var ErrInvalidBranch = errors.New("invalid branch name")
// ErrInvalidCommitSHA is returned when a commit SHA is not a valid hex string.
var ErrInvalidCommitSHA = errors.New("invalid commit SHA")
// validBranchRe matches safe git branch names.
var validBranchRe = regexp.MustCompile(`^[a-zA-Z0-9._/\-]+$`)
// validCommitSHARe matches a full-length hex commit SHA.
var validCommitSHARe = regexp.MustCompile(`^[0-9a-f]{40}$`)
// Params contains dependencies for Client.
type Params struct {
fx.In
@@ -430,6 +443,15 @@ func (c *Client) CloneRepo(
ctx context.Context,
repoURL, branch, commitSHA, sshPrivateKey, containerDir, hostDir string,
) (*CloneResult, error) {
// Validate inputs to prevent shell injection
if !validBranchRe.MatchString(branch) {
return nil, fmt.Errorf("%w: %q", ErrInvalidBranch, branch)
}
if commitSHA != "" && !validCommitSHARe.MatchString(commitSHA) {
return nil, fmt.Errorf("%w: %q", ErrInvalidCommitSHA, commitSHA)
}
if c.docker == nil {
return nil, ErrNotConnected
}
@@ -584,39 +606,36 @@ func (c *Client) createGitContainer(
) (string, error) {
gitSSHCmd := "ssh -i /keys/deploy_key -o StrictHostKeyChecking=no"
// Build the git command based on whether we have a specific commit SHA
var cmd []string
var entrypoint []string
// Build the git command using environment variables to avoid shell injection.
// Arguments are passed via env vars and quoted in the shell script.
var script string
if cfg.commitSHA != "" {
// Clone without depth limit so we can checkout any commit, then checkout specific SHA
// Using sh -c to run multiple commands - need to clear entrypoint
// Output "COMMIT:<sha>" marker at end for parsing
script := fmt.Sprintf(
"git clone --branch %s %s /repo && cd /repo && git checkout %s && echo COMMIT:$(git rev-parse HEAD)",
cfg.branch, cfg.repoURL, cfg.commitSHA,
)
entrypoint = []string{}
cmd = []string{"sh", "-c", script}
script = `git clone --branch "$CLONE_BRANCH" "$CLONE_URL" /repo && cd /repo && git checkout "$CLONE_SHA" && echo COMMIT:$(git rev-parse HEAD)`
} else {
// Shallow clone of branch HEAD, then output commit SHA
// Using sh -c to run multiple commands
script := fmt.Sprintf(
"git clone --depth 1 --branch %s %s /repo && cd /repo && echo COMMIT:$(git rev-parse HEAD)",
cfg.branch, cfg.repoURL,
)
entrypoint = []string{}
cmd = []string{"sh", "-c", script}
script = `git clone --depth 1 --branch "$CLONE_BRANCH" "$CLONE_URL" /repo && cd /repo && echo COMMIT:$(git rev-parse HEAD)`
}
env := []string{
"GIT_SSH_COMMAND=" + gitSSHCmd,
"CLONE_URL=" + cfg.repoURL,
"CLONE_BRANCH=" + cfg.branch,
}
if cfg.commitSHA != "" {
env = append(env, "CLONE_SHA="+cfg.commitSHA)
}
entrypoint := []string{}
cmd := []string{"sh", "-c", script}
// Use host paths for Docker bind mounts (Docker runs on the host, not in our container)
resp, err := c.docker.ContainerCreate(ctx,
&container.Config{
Image: gitImage,
Entrypoint: entrypoint,
Cmd: cmd,
Env: []string{"GIT_SSH_COMMAND=" + gitSSHCmd},
Env: env,
WorkingDir: "/",
},
&container.HostConfig{