From 80931c9e64a4007410b1968aae4881f41b0e4c50 Mon Sep 17 00:00:00 2001 From: Sanket Saurav Date: Sat, 18 Apr 2026 23:20:39 -0700 Subject: [PATCH] fix(cli): execute YAML pattern analyzers during analysis RunCheckers loaded built-in and custom YAML pattern checkers into a patternCheckers map but never passed them to analysis.RunAnalyzers, so no YAML-defined rule ever produced a finding. Only Go-based analyzers were being executed end-to-end. Flatten patternCheckers into a slice of *Analyzer and run them through the same path used for Go analyzers. Preserve Severity and Category on the resulting issues by looking up the originating analyzer by name. Add a regression test that exercises RunCheckers with a custom YAML checker and asserts a finding is produced. --- pkg/cli/cli.go | 64 +++++++++++++++++++++++++++++++++++----- pkg/cli/cli_test.go | 71 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 128 insertions(+), 7 deletions(-) create mode 100644 pkg/cli/cli_test.go diff --git a/pkg/cli/cli.go b/pkg/cli/cli.go index eb54f98..bb5dd4c 100644 --- a/pkg/cli/cli.go +++ b/pkg/cli/cli.go @@ -431,17 +431,19 @@ func (c *Cli) RunCheckers(runBuiltinCheckers, runCustomCheckers bool) error { return err } + fileFilter := func(filename string) bool { + if c.CmpHash != "" { + _, isChanged := changedFileMap[filename] + return isChanged + } + return true + } + if len(goAnalyzers) > 0 { goIssues, err := analysis.RunAnalyzers( c.RootDirectory, goAnalyzers, - func(filename string) bool { - if c.CmpHash != "" { - _, isChanged := changedFileMap[filename] - return isChanged - } - return true - }, + fileFilter, ) if err != nil { return fmt.Errorf("failed to run Go-based analyzers: %w", err) @@ -461,6 +463,54 @@ func (c *Cli) RunCheckers(runBuiltinCheckers, runCustomCheckers bool) error { } } + // Flatten the per-language pattern checkers map into a slice and run them + // against the project. Without this step, YAML pattern checkers (both + // built-in and custom) are loaded but never executed. + var yamlAnalyzers []*analysis.Analyzer + yamlAnalyzerByName := make(map[string]*analysis.Analyzer) + for _, checkers := range patternCheckers { + for i := range checkers { + analyzer := &checkers[i] + yamlAnalyzers = append(yamlAnalyzers, analyzer) + yamlAnalyzerByName[analyzer.Name] = analyzer + } + } + + if len(yamlAnalyzers) > 0 { + yamlIssues, err := analysis.RunAnalyzers( + c.RootDirectory, + yamlAnalyzers, + fileFilter, + ) + if err != nil { + return fmt.Errorf("failed to run YAML pattern analyzers: %w", err) + } + for _, issue := range yamlIssues { + txt, _ := issue.AsText() + log.Error().Msg(string(txt)) + + // Look up the originating analyzer so we can preserve severity + // and category on the reported issue. + severity := analysis.Severity(issue.Severity) + category := analysis.Category(issue.Category) + if issue.Id != nil { + if a, ok := yamlAnalyzerByName[*issue.Id]; ok { + severity = a.Severity + category = a.Category + } + } + + result.issues = append(result.issues, &analysis.Issue{ + Filepath: issue.Filepath, + Message: issue.Message, + Severity: severity, + Category: category, + Node: issue.Node, + Id: issue.Id, + }) + } + } + if runCustomCheckers { customGoIssues, textIssues, err := c.runCustomGoAnalyzers() if err != nil { diff --git a/pkg/cli/cli_test.go b/pkg/cli/cli_test.go new file mode 100644 index 0000000..c285c08 --- /dev/null +++ b/pkg/cli/cli_test.go @@ -0,0 +1,71 @@ +package cli + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + "globstar.dev/pkg/config" +) + +// TestRunCheckers_ExecutesCustomYamlCheckers is a regression test ensuring +// that YAML pattern checkers loaded from a user's .globstar directory are +// actually executed against project files. Previously, RunCheckers loaded +// the checkers into a map but never passed them to the analyzer runner, so +// custom YAML rules silently produced zero findings. +func TestRunCheckers_ExecutesCustomYamlCheckers(t *testing.T) { + tmpDir := t.TempDir() + + checkerDir := filepath.Join(tmpDir, ".globstar") + if err := os.MkdirAll(checkerDir, 0o755); err != nil { + t.Fatalf("mkdir .globstar: %v", err) + } + + const yamlChecker = `language: go +name: go_filepath_clean_test +message: "Found filepath.Clean" +category: security +severity: critical +pattern: > + (call_expression + function: (selector_expression + operand: (identifier) @pkg + field: (field_identifier) @func + ) + (#eq? @pkg "filepath") + (#eq? @func "Clean") + ) @go_filepath_clean_test +` + if err := os.WriteFile(filepath.Join(checkerDir, "my_check.yml"), []byte(yamlChecker), 0o644); err != nil { + t.Fatalf("write yaml: %v", err) + } + + const goSource = `package main + +import "path/filepath" + +func main() { + _ = filepath.Clean("x") +} +` + if err := os.WriteFile(filepath.Join(tmpDir, "test.go"), []byte(goSource), 0o644); err != nil { + t.Fatalf("write go file: %v", err) + } + + conf := &config.Config{} + conf.PopulateDefaults() + conf.CheckerDir = checkerDir + + c := &Cli{ + RootDirectory: tmpDir, + Config: conf, + } + + err := c.RunCheckers(false, true) + // A critical issue should be raised, which by default causes RunCheckers + // to return a non-nil error ("found N issues") via FailWhen. If the YAML + // execution bug returns, no issues are produced and err would be nil. + require.Error(t, err, "expected RunCheckers to report a finding from the custom YAML checker") + require.Contains(t, err.Error(), "found 1 issues") +}