From 7b739e04c888bcd9c08c2dfbe0ac59085ee0a315 Mon Sep 17 00:00:00 2001 From: Micheal Wilkinson Date: Sat, 21 Mar 2026 22:47:02 +0000 Subject: [PATCH] chore(go): harden coverage-gate file input handling --- coverage-gate/parse.go | 43 +++++++++++++++++++++++++++++++++---- coverage-gate/parse_test.go | 31 ++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 4 deletions(-) diff --git a/coverage-gate/parse.go b/coverage-gate/parse.go index da5a4cc..1ccb5b4 100644 --- a/coverage-gate/parse.go +++ b/coverage-gate/parse.go @@ -43,9 +43,9 @@ type PackageResult struct { // LoadPolicy reads policy JSON from disk. func LoadPolicy(path string) (Policy, error) { - f, err := os.Open(path) + f, err := openValidatedReadOnlyFile(path, ".json", "policy") if err != nil { - return Policy{}, fmt.Errorf("open policy: %w", err) + return Policy{}, err } defer f.Close() @@ -61,9 +61,9 @@ func LoadPolicy(path string) (Policy, error) { // ParseCoverProfile parses a Go coverprofile and aggregates package coverage. func ParseCoverProfile(profilePath string, policy Policy) (map[string]Coverage, error) { - f, err := os.Open(profilePath) + f, err := openValidatedReadOnlyFile(profilePath, "", "coverage profile") if err != nil { - return nil, fmt.Errorf("open coverage profile: %w", err) + return nil, err } defer f.Close() @@ -121,6 +121,41 @@ func ParseCoverProfile(profilePath string, policy Policy) (map[string]Coverage, return coverage, nil } +func openValidatedReadOnlyFile(path string, requiredExt string, label string) (*os.File, error) { + cleaned := filepath.Clean(strings.TrimSpace(path)) + if cleaned == "" || cleaned == "." { + return nil, fmt.Errorf("invalid %s path", label) + } + + if requiredExt != "" { + ext := strings.ToLower(filepath.Ext(cleaned)) + if ext != strings.ToLower(requiredExt) { + return nil, fmt.Errorf("invalid %s file extension: got %q, want %q", label, ext, requiredExt) + } + } + + absPath, err := filepath.Abs(cleaned) + if err != nil { + return nil, fmt.Errorf("resolve %s path: %w", label, err) + } + + info, err := os.Stat(absPath) + if err != nil { + return nil, fmt.Errorf("stat %s: %w", label, err) + } + if info.IsDir() { + return nil, fmt.Errorf("%s path must be a file, got directory", label) + } + + // #nosec G304 -- path is explicitly cleaned, normalized, and pre-validated as an existing file. + f, err := os.Open(absPath) + if err != nil { + return nil, fmt.Errorf("open %s: %w", label, err) + } + + return f, nil +} + // EvaluateCoverage evaluates package coverage against policy thresholds. func EvaluateCoverage(packages []string, byPackage map[string]Coverage, policy Policy) []PackageResult { results := make([]PackageResult, 0, len(packages)) diff --git a/coverage-gate/parse_test.go b/coverage-gate/parse_test.go index 037f747..75599ca 100644 --- a/coverage-gate/parse_test.go +++ b/coverage-gate/parse_test.go @@ -3,6 +3,7 @@ package main import ( "os" "path/filepath" + "strings" "testing" ) @@ -86,3 +87,33 @@ func TestEvaluateCoverage_NoStatementsPasses(t *testing.T) { t.Fatalf("expected pass for no-statement package, got %+v", results[0]) } } + +func TestLoadPolicy_RejectsNonJSONPath(t *testing.T) { + t.Parallel() + + tmp := t.TempDir() + policyPath := filepath.Join(tmp, "policy.yaml") + if err := os.WriteFile(policyPath, []byte("minimum_statement_coverage: 80\n"), 0600); err != nil { + t.Fatalf("write policy file: %v", err) + } + + _, err := LoadPolicy(policyPath) + if err == nil { + t.Fatal("expected LoadPolicy to fail for non-json extension") + } + if !strings.Contains(err.Error(), "invalid policy file extension") { + t.Fatalf("expected extension error, got: %v", err) + } +} + +func TestParseCoverProfile_RejectsDirectoryPath(t *testing.T) { + t.Parallel() + + _, err := ParseCoverProfile(t.TempDir(), Policy{MinimumStatementCoverage: 80}) + if err == nil { + t.Fatal("expected ParseCoverProfile to fail for directory path") + } + if !strings.Contains(err.Error(), "coverage profile path must be a file") { + t.Fatalf("expected directory path error, got: %v", err) + } +}