diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a13d502..656486b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -143,12 +143,11 @@ jobs: run: git clone --depth=1 https://github.com/GrayCodeAI/hawk.git ../hawk - name: govulncheck run: | - go install golang.org/x/vuln/cmd/govulncheck@latest + go install golang.org/x/vuln/cmd/govulncheck@v1.1.4 govulncheck ./... - name: gosec (advisory) - continue-on-error: true run: | - go install github.com/securego/gosec/v2/cmd/gosec@latest + go install github.com/securego/gosec/v2/cmd/gosec@v2.22.4 gosec -exclude=G104,G301,G302,G304,G306 ./... # ------------------------------------------------------------------------- diff --git a/incremental.go b/incremental.go index 37c077c..e4c4ace 100644 --- a/incremental.go +++ b/incremental.go @@ -230,6 +230,20 @@ func gitRoot(ctx context.Context) (string, error) { return strings.TrimSpace(string(out)), nil } +// validateGitRef ensures a git ref contains no dangerous characters. +func validateGitRef(ref string) error { + if ref == "" { + return fmt.Errorf("empty git ref") + } + if ref[0] == '-' { + return fmt.Errorf("git ref %q starts with dash", ref) + } + if strings.ContainsAny(ref, ";&|$`(){}[]<>!#*?\n\r\x00\\ ") { + return fmt.Errorf("git ref %q contains forbidden characters", ref) + } + return nil +} + // gitDiffRange runs `git diff base...head` with a context timeout. func gitDiffRange(ctx context.Context, base, head string) (string, error) { // Default 30s timeout if context has no deadline @@ -239,13 +253,22 @@ func gitDiffRange(ctx context.Context, base, head string) (string, error) { defer cancel() } + if err := validateGitRef(base); err != nil { + return "", fmt.Errorf("invalid base ref: %w", err) + } + if err := validateGitRef(head); err != nil { + return "", fmt.Errorf("invalid head ref: %w", err) + } + // Try three-dot syntax first (merge-base diff) + // #nosec G204 — base and head validated by validateGitRef above out, err := exec.CommandContext(ctx, "git", "diff", base+"..."+head).Output() if err == nil { return string(out), nil } // Fall back to two-dot syntax + // #nosec G204 — base and head validated by validateGitRef above out, err = exec.CommandContext(ctx, "git", "diff", base, head).Output() if err != nil { return "", fmt.Errorf("git diff %s %s failed: %w", base, head, err) diff --git a/internal/context/git.go b/internal/context/git.go index c9c6936..a8b7ef0 100644 --- a/internal/context/git.go +++ b/internal/context/git.go @@ -75,8 +75,13 @@ func FormatContext(contexts []FileContext) string { // DiffBase returns the diff of the current branch against a base branch. func DiffBase(base string) (string, error) { + if err := validateGitRef(base); err != nil { + return "", err + } + // #nosec G204 — base validated by validateGitRef out, err := exec.Command("git", "diff", base+"...HEAD").Output() if err != nil { + // #nosec G204 — base validated by validateGitRef out2, err2 := exec.Command("git", "diff", base).Output() if err2 != nil { return "", fmt.Errorf("git diff failed: %w", err) @@ -88,6 +93,10 @@ func DiffBase(base string) (string, error) { // ChangedFiles returns the list of files changed relative to a base. func ChangedFiles(base string) ([]string, error) { + if err := validateGitRef(base); err != nil { + return nil, err + } + // #nosec G204 — base validated by validateGitRef out, err := exec.Command("git", "diff", "--name-only", base+"...HEAD").Output() if err != nil { return nil, fmt.Errorf("git diff --name-only failed: %w", err) @@ -111,6 +120,7 @@ func Blame(file string, startLine, endLine int) (string, error) { "--", file, } + // #nosec G204 — file validated by validateFilePath above, startLine/endLine are ints out, err := exec.Command("git", args...).Output() if err != nil { return "", fmt.Errorf("git blame failed: %w", err) @@ -119,6 +129,20 @@ func Blame(file string, startLine, endLine int) (string, error) { return parseBlameAuthors(string(out)), nil } +// validateGitRef ensures a git ref (branch, tag, SHA) contains no dangerous characters. +func validateGitRef(ref string) error { + if ref == "" { + return fmt.Errorf("empty git ref") + } + if ref[0] == '-' { + return fmt.Errorf("git ref %q starts with dash", ref) + } + if strings.ContainsAny(ref, ";&|$`(){}[]<>!#*?\n\r\x00\\ ") { + return fmt.Errorf("git ref %q contains forbidden characters", ref) + } + return nil +} + // validateFilePath ensures a file path does not traverse outside the working directory. func validateFilePath(file string) error { if file == "" { @@ -146,6 +170,7 @@ func recentCommits(file string, n int) ([]string, error) { if err := validateFilePath(file); err != nil { return nil, fmt.Errorf("recentCommits: invalid path: %w", err) } + // #nosec G204 — file validated by validateFilePath above out, err := exec.Command("git", "log", "--oneline", "-n", strconv.Itoa(n), "--", file).Output() if err != nil { return nil, err diff --git a/testdata/crossfunc/vuln.go b/testdata/crossfunc/vuln.go index d01b73d..8878d21 100644 --- a/testdata/crossfunc/vuln.go +++ b/testdata/crossfunc/vuln.go @@ -33,6 +33,7 @@ func handler(w http.ResponseWriter, r *http.Request) { // runQuery builds a SQL statement from the tainted parameter (sink in a // different function than the source). func runQuery(q string) { + // #nosec G202 — intentional test data for taint analysis query := "SELECT * FROM users WHERE name = '" + q + "'" _, _ = db.Query(query) }