diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 00000000000..8361e83c11c --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,4 @@ +## 2025-06-25 - [Git Argument Injection] +**Vulnerability:** Argument injection (flag injection) in `git` commands when passing user-controlled strings (like branch names, refs, or repository URLs) as positional arguments. +**Learning:** Even when using `exec.Command` (which avoids shell injection), some commands like `git` can interpret positional arguments starting with `-` as flags, potentially leading to unauthorized configuration changes or other security risks. +**Prevention:** Use the `--` separator to explicitly delineate options from positional arguments in `git` commands (e.g., `git checkout --`). Always place `--` before any argument that could potentially start with a hyphen. diff --git a/pkg/cli/git.go b/pkg/cli/git.go index 7493dfabc12..9b9fc6d7312 100644 --- a/pkg/cli/git.go +++ b/pkg/cli/git.go @@ -465,7 +465,7 @@ func getCurrentBranch() (string, error) { func createAndSwitchBranch(branchName string, verbose bool) error { console.LogVerbose(verbose, "Creating and switching to branch: "+branchName) - cmd := exec.Command("git", "checkout", "-b", branchName) + cmd := exec.Command("git", "checkout", "-b", branchName, "--") if err := cmd.Run(); err != nil { return fmt.Errorf("failed to create and switch to branch %s: %w", branchName, err) } @@ -477,7 +477,7 @@ func createAndSwitchBranch(branchName string, verbose bool) error { func switchBranch(branchName string, verbose bool) error { console.LogVerbose(verbose, "Switching to branch: "+branchName) - cmd := exec.Command("git", "checkout", branchName) + cmd := exec.Command("git", "checkout", branchName, "--") if err := cmd.Run(); err != nil { return fmt.Errorf("failed to switch to branch %s: %w", branchName, err) } @@ -503,7 +503,7 @@ func commitChanges(message string, verbose bool) error { func pushBranch(branchName string, verbose bool) error { console.LogVerbose(verbose, "Pushing branch: "+branchName) - cmd := exec.Command("git", "push", "-u", "origin", branchName) + cmd := exec.Command("git", "push", "-u", "origin", "--", branchName) if err := cmd.Run(); err != nil { return fmt.Errorf("failed to push branch %s: %w", branchName, err) } diff --git a/pkg/parser/remote_fetch.go b/pkg/parser/remote_fetch.go index d317a4fb979..4c5f3292152 100644 --- a/pkg/parser/remote_fetch.go +++ b/pkg/parser/remote_fetch.go @@ -73,7 +73,7 @@ func getOrCreateListRepoClone(owner, repo, ref, host string) (string, error) { return "", fmt.Errorf("failed to create temp directory: %w", err) } - cloneCmd := exec.Command("git", "clone", "--depth", "1", "--branch", ref, "--single-branch", "--filter=blob:none", "--no-checkout", repoURL, tmpDir) + cloneCmd := exec.Command("git", "clone", "--depth", "1", "--branch", ref, "--single-branch", "--filter=blob:none", "--no-checkout", "--", repoURL, tmpDir) cloneOutput, err := cloneCmd.CombinedOutput() if err != nil { if cleanupErr := os.RemoveAll(tmpDir); cleanupErr != nil { @@ -458,12 +458,12 @@ func resolveRefToSHAViaGit(owner, repo, ref, host string) (string, error) { // Try to resolve the ref using git ls-remote // Format: git ls-remote - cmd := exec.Command("git", "ls-remote", repoURL, ref) + cmd := exec.Command("git", "ls-remote", "--", repoURL, ref) output, err := cmd.Output() if err != nil { // If exact ref doesn't work, try with refs/heads/ and refs/tags/ prefixes for _, prefix := range []string{"refs/heads/", "refs/tags/"} { - cmd = exec.Command("git", "ls-remote", repoURL, prefix+ref) + cmd = exec.Command("git", "ls-remote", "--", repoURL, prefix+ref) output, err = cmd.Output() if err == nil && len(output) > 0 { break @@ -625,7 +625,7 @@ func downloadFileViaGit(ctx context.Context, owner, repo, path, ref, host string // git archive command: git archive --remote= // #nosec G204 -- repoURL, ref, and path are from workflow import configuration authored by the // developer; exec.Command with separate args (not shell execution) prevents shell injection. - cmd := exec.Command("git", "archive", "--remote="+repoURL, ref, path) + cmd := exec.Command("git", "archive", "--remote="+repoURL, ref, "--", path) archiveOutput, err := cmd.Output() if err != nil { // If git archive fails, try with git clone + git show as a fallback @@ -703,24 +703,24 @@ func downloadFileViaGitClone(owner, repo, path, ref, host string) ([]byte, error if isSHA { // For SHA refs, we need to clone without --branch and then checkout the specific commit // Clone with minimal depth and no branch specified - cloneCmd = exec.Command("git", "clone", "--depth", "1", "--no-single-branch", repoURL, tmpDir) + cloneCmd = exec.Command("git", "clone", "--depth", "1", "--no-single-branch", "--", repoURL, tmpDir) if output, err := cloneCmd.CombinedOutput(); err != nil { // Try without --no-single-branch if the first attempt fails remoteLog.Printf("Clone with --no-single-branch failed, trying full clone: %s", string(output)) - cloneCmd = exec.Command("git", "clone", repoURL, tmpDir) + cloneCmd = exec.Command("git", "clone", "--", repoURL, tmpDir) if output, err := cloneCmd.CombinedOutput(); err != nil { return nil, fmt.Errorf("failed to clone repository: %w\nOutput: %s", err, string(output)) } } // Now checkout the specific commit - checkoutCmd := exec.Command("git", "-C", tmpDir, "checkout", ref) + checkoutCmd := exec.Command("git", "-C", tmpDir, "checkout", ref, "--") if output, err := checkoutCmd.CombinedOutput(); err != nil { return nil, fmt.Errorf("failed to checkout commit %s: %w\nOutput: %s", ref, err, string(output)) } } else { // For branch/tag refs, use --branch flag - cloneCmd = exec.Command("git", "clone", "--depth", "1", "--branch", ref, repoURL, tmpDir) + cloneCmd = exec.Command("git", "clone", "--depth", "1", "--branch", ref, "--", repoURL, tmpDir) if output, err := cloneCmd.CombinedOutput(); err != nil { return nil, fmt.Errorf("failed to clone repository: %w\nOutput: %s", err, string(output)) } @@ -1144,7 +1144,7 @@ func listDirAllFilesViaGitForHost(owner, repo, ref, dirPath, host string) ([]str return nil, err } - lsTreeCmd := exec.Command("git", "-C", tmpDir, "ls-tree", "-r", "--name-only", "HEAD", dirPath+"/") + lsTreeCmd := exec.Command("git", "-C", tmpDir, "ls-tree", "-r", "--name-only", "HEAD", "--", dirPath+"/") lsTreeOutput, err := lsTreeCmd.CombinedOutput() if err != nil { remoteLog.Printf("Failed to list dir files: %s", string(lsTreeOutput)) @@ -1283,7 +1283,7 @@ func listDirAllFilesRecursivelyViaGitForHost(owner, repo, ref, dirPath, host str // Normalise dirPath so it never has a trailing slash before we append one. cleanDirPath := strings.TrimRight(dirPath, "/") - lsTreeCmd := exec.Command("git", "-C", tmpDir, "ls-tree", "-r", "--name-only", "HEAD", cleanDirPath+"/") + lsTreeCmd := exec.Command("git", "-C", tmpDir, "ls-tree", "-r", "--name-only", "HEAD", "--", cleanDirPath+"/") lsTreeOutput, err := lsTreeCmd.CombinedOutput() if err != nil { remoteLog.Printf("Failed to list dir files recursively: %s", string(lsTreeOutput)) @@ -1404,7 +1404,7 @@ func listDirSubdirsViaGitForHost(owner, repo, ref, dirPath, host string) ([]stri } // Use ls-tree -d to list only direct subdirectory entries. - lsTreeDirsCmd := exec.Command("git", "-C", tmpDir, "ls-tree", "--name-only", "-d", "HEAD", dirPath+"/") + lsTreeDirsCmd := exec.Command("git", "-C", tmpDir, "ls-tree", "--name-only", "-d", "HEAD", "--", dirPath+"/") lsTreeDirsOutput, err := lsTreeDirsCmd.CombinedOutput() if err != nil { remoteLog.Printf("Failed to list tree subdirs: %s", string(lsTreeDirsOutput)) @@ -1475,7 +1475,7 @@ func listWorkflowFilesViaGitForHost(owner, repo, ref, workflowPath, host string) // Do a minimal clone using filter=blob:none for faster cloning (metadata only, no blobs) // Use --depth=1 for shallow clone and --no-checkout to skip checkout initially - cloneCmd := exec.Command("git", "clone", "--depth", "1", "--branch", ref, "--single-branch", "--filter=blob:none", "--no-checkout", repoURL, tmpDir) + cloneCmd := exec.Command("git", "clone", "--depth", "1", "--branch", ref, "--single-branch", "--filter=blob:none", "--no-checkout", "--", repoURL, tmpDir) cloneOutput, err := cloneCmd.CombinedOutput() if err != nil { remoteLog.Printf("Failed to clone repository: %s", string(cloneOutput)) @@ -1483,7 +1483,7 @@ func listWorkflowFilesViaGitForHost(owner, repo, ref, workflowPath, host string) } // Use git ls-tree to list files in the specified workflows directory - lsTreeCmd := exec.Command("git", "-C", tmpDir, "ls-tree", "-r", "--name-only", "HEAD", workflowPath+"/") + lsTreeCmd := exec.Command("git", "-C", tmpDir, "ls-tree", "-r", "--name-only", "HEAD", "--", workflowPath+"/") lsTreeOutput, err := lsTreeCmd.CombinedOutput() if err != nil { remoteLog.Printf("Failed to list files: %s", string(lsTreeOutput))