diff --git a/action.yml b/action.yml index 9469965..86ab2a5 100644 --- a/action.yml +++ b/action.yml @@ -198,7 +198,15 @@ runs: # tool_result reads a tool's own [PASS]/[FAIL] line (bulwark scan's own report() # output) — the ground truth for whether that specific tool failed, since a nonzero # SCAN_EXIT_CODE only says "at least one check failed", not which one. - tool_result() { grep -oE "^\[(PASS|FAIL)\] $2\$" "$1" | tail -1 | sed -E 's/^\[([A-Z]+)\].*/\1/'; } + # The optional ": " prefix matches how rust.crateLabel and + # golang.moduleLabel qualify a tool name once more than one crate or + # module is discovered ("sdk/wardnet-go: gosec"). Without it the + # anchored pattern silently matches nothing in exactly the multi-module + # repos that need attribution, leaving the comment unable to say which + # tool failed. FAIL anywhere wins: "FAIL" sorts before "PASS", so + # `sort | head -1` reports FAIL if any crate or module failed; taking + # the last line instead would let a passing module mask a failing one. + tool_result() { grep -oE "^\[(PASS|FAIL)\] (.*: )?$2\$" "$1" | sed -E 's/^\[([A-Z]+)\].*/\1/' | sort | head -1; } # Max bytes of raw tool output to inline per section. GitHub caps a PR # comment at 65536 chars, and the full logs (multi-thousand-line cargo / diff --git a/cmd/bulwark/scan.go b/cmd/bulwark/scan.go index 67107a2..867fabe 100644 --- a/cmd/bulwark/scan.go +++ b/cmd/bulwark/scan.go @@ -64,7 +64,11 @@ func newScanCmd() *cobra.Command { if !cfg.Go.Enabled { continue } - results = append(results, golang.Check(ctx, dir)...) + goResults, err := golang.Check(ctx, dir, cfg.Go.Exclude) + if err != nil { + return err + } + results = append(results, goResults...) } } if cfg.Semgrep.Enabled { diff --git a/internal/detect/detect.go b/internal/detect/detect.go index abf277b..76ba162 100644 --- a/internal/detect/detect.go +++ b/internal/detect/detect.go @@ -131,6 +131,58 @@ func TSPackageDirs(root string, exclude []string) ([]string, error) { return dirs, nil } +// GoModuleDirs returns every directory under root containing a go.mod, so +// each Go module can be scanned independently. Directories named in exclude +// (in addition to the built-in defaults) are skipped, matching TSPackageDirs. +// +// Unlike RustCrateDirs, there is no ancestor/member relationship to resolve: +// a nested go.mod starts a genuinely separate module and is *excluded* from +// its parent's package graph, so `./...` at an ancestor would never reach it. +// Every go.mod is therefore its own scan root. A `go.work` file does not +// change this — it affects local resolution, not module boundaries, and +// gosec/govulncheck still need to run inside each module. +func GoModuleDirs(root string, exclude []string) ([]string, error) { + skip := skipSet(exclude) + var dirs []string + var visit func(dir string) error + visit = func(dir string) error { + entries, err := os.ReadDir(dir) + if err != nil { + return err + } + for _, e := range entries { + if !e.IsDir() && e.Name() == "go.mod" { + dirs = append(dirs, dir) + break + } + } + for _, e := range entries { + if e.IsDir() && !skip[e.Name()] && !goIgnoredDir(e.Name()) { + if err := visit(filepath.Join(dir, e.Name())); err != nil { + return err + } + } + } + return nil + } + if err := visit(root); err != nil { + return nil, err + } + return dirs, nil +} + +// goIgnoredDir reports whether the Go toolchain itself ignores a directory +// when resolving packages: "testdata", and any name beginning with "." or "_". +// +// Honouring this matters because a go.mod under testdata/ is a fixture, not a +// project. Such modules are routinely unbuildable on purpose — no go.sum, or +// deliberately pinned to a vulnerable dependency to exercise a scanner. The +// parent's `./...` never compiles them, so treating one as a real scan root +// would fail the whole run on code that is not shipped. +func goIgnoredDir(name string) bool { + return name == "testdata" || strings.HasPrefix(name, ".") || strings.HasPrefix(name, "_") +} + // RustCrateDirs returns every directory under root that is the effective // root of an independent Cargo invocation: each directory containing a // Cargo.toml, except a nested Cargo.toml whose nearest Cargo.toml ancestor diff --git a/internal/detect/detect_test.go b/internal/detect/detect_test.go index 025875c..cbc273d 100644 --- a/internal/detect/detect_test.go +++ b/internal/detect/detect_test.go @@ -108,3 +108,108 @@ func TestRustCrateDirsDefaultSkipDirsHonored(t *testing.T) { t.Fatalf("got %v, want none", dirs) } } + +func TestGoModuleDirsSingleModuleAtRoot(t *testing.T) { + root := t.TempDir() + writeFile(t, filepath.Join(root, "go.mod"), "module example.com/foo\n") + + dirs, err := GoModuleDirs(root, nil) + if err != nil { + t.Fatal(err) + } + if got := sorted(dirs); len(got) != 1 || got[0] != root { + t.Fatalf("got %v, want [%s]", got, root) + } +} + +// The regression this whole function exists for: a scan root that holds no +// go.mod itself, with the real modules in subdirectories. Running the tools +// at the root made govulncheck exit "no go.mod file". +func TestGoModuleDirsModulesInSubdirectories(t *testing.T) { + root := t.TempDir() + sdk := filepath.Join(root, "sdk", "wardnet-go") + cli := filepath.Join(root, "wctl") + writeFile(t, filepath.Join(sdk, "go.mod"), "module wardnet.network/go\n") + writeFile(t, filepath.Join(cli, "go.mod"), "module example.com/wctl\n") + writeFile(t, filepath.Join(root, "daemon", "src", "main.rs"), "fn main() {}\n") + + dirs, err := GoModuleDirs(root, nil) + if err != nil { + t.Fatal(err) + } + got := sorted(dirs) + want := sorted([]string{sdk, cli}) + if len(got) != len(want) || got[0] != want[0] || got[1] != want[1] { + t.Fatalf("got %v, want %v", got, want) + } +} + +// A nested go.mod is a separate module excluded from its parent's package +// graph, so `./...` at the parent never reaches it — both must be returned. +// This is the point of difference from RustCrateDirs, where a member crate is +// deliberately folded into its workspace root. +func TestGoModuleDirsNestedModuleIsIndependent(t *testing.T) { + root := t.TempDir() + nested := filepath.Join(root, "tools", "gen") + writeFile(t, filepath.Join(root, "go.mod"), "module example.com/outer\n") + writeFile(t, filepath.Join(nested, "go.mod"), "module example.com/outer/tools/gen\n") + + dirs, err := GoModuleDirs(root, nil) + if err != nil { + t.Fatal(err) + } + got := sorted(dirs) + want := sorted([]string{root, nested}) + if len(got) != len(want) || got[0] != want[0] || got[1] != want[1] { + t.Fatalf("got %v, want %v", got, want) + } +} + +func TestGoModuleDirsRespectsExclude(t *testing.T) { + root := t.TempDir() + keep := filepath.Join(root, "keep") + writeFile(t, filepath.Join(keep, "go.mod"), "module example.com/keep\n") + writeFile(t, filepath.Join(root, "legacy", "go.mod"), "module example.com/legacy\n") + + dirs, err := GoModuleDirs(root, []string{"legacy"}) + if err != nil { + t.Fatal(err) + } + if got := sorted(dirs); len(got) != 1 || got[0] != keep { + t.Fatalf("got %v, want [%s]", got, keep) + } +} + +func TestGoModuleDirsDefaultSkipDirsHonored(t *testing.T) { + root := t.TempDir() + writeFile(t, filepath.Join(root, "go.mod"), "module example.com/foo\n") + writeFile(t, filepath.Join(root, "vendor", "dep", "go.mod"), "module example.com/dep\n") + + dirs, err := GoModuleDirs(root, nil) + if err != nil { + t.Fatal(err) + } + if got := sorted(dirs); len(got) != 1 || got[0] != root { + t.Fatalf("got %v, want [%s]", got, root) + } +} + +// A go.mod under testdata/ is a fixture, not a project — often deliberately +// unbuildable or pinned to a vulnerable dependency to exercise a scanner. Go's +// own package loading ignores testdata, "_"- and "."-prefixed directories, and +// discovery has to agree or the scan fails on code that never ships. +func TestGoModuleDirsSkipsGoIgnoredDirs(t *testing.T) { + root := t.TempDir() + writeFile(t, filepath.Join(root, "go.mod"), "module example.com/foo\n") + writeFile(t, filepath.Join(root, "testdata", "broken", "go.mod"), "module example.com/fixture\n") + writeFile(t, filepath.Join(root, "_scratch", "go.mod"), "module example.com/scratch\n") + writeFile(t, filepath.Join(root, ".tools", "go.mod"), "module example.com/tools\n") + + dirs, err := GoModuleDirs(root, nil) + if err != nil { + t.Fatal(err) + } + if got := sorted(dirs); len(got) != 1 || got[0] != root { + t.Fatalf("got %v, want [%s]", got, root) + } +} diff --git a/internal/golang/golang.go b/internal/golang/golang.go index a8d62d9..d476ae2 100644 --- a/internal/golang/golang.go +++ b/internal/golang/golang.go @@ -1,4 +1,5 @@ -// Package golang runs gosec and govulncheck against a Go module. Both are +// Package golang runs gosec and govulncheck against every Go module found +// under the scan root. Both are // installed via `go install` into a bulwark-managed, version-keyed bin // directory (never trusting whatever gosec/govulncheck might already be on // PATH) — the same "pin the exact toolchain, don't reuse ambient installs" @@ -11,6 +12,7 @@ import ( "os" "path/filepath" + "wardnet/bulwark/internal/detect" "wardnet/bulwark/internal/executil" ) @@ -24,23 +26,71 @@ const ( govulncheckPkg = "golang.org/x/vuln/cmd/govulncheck@" + govulncheckVersion ) -// Check runs gosec and govulncheck against the Go module rooted at dir. -func Check(ctx context.Context, dir string) []executil.Result { +// Check runs gosec and govulncheck against every Go module under root, +// skipping any directory named in exclude. +// +// Both tools are module-scoped: govulncheck exits with "no go.mod file" when +// run anywhere but a module root, so running once at the scan root only ever +// worked when the module happened to sit exactly there. A monorepo that keeps +// its Go modules in subdirectories got a hard failure from govulncheck and a +// misleading pass from gosec, which walks a tree happily without a module. +func Check(ctx context.Context, root string, exclude []string) ([]executil.Result, error) { + modDirs, err := detect.GoModuleDirs(root, exclude) + if err != nil { + return nil, err + } + if len(modDirs) == 0 { + return nil, nil + } + + multi := len(modDirs) > 1 + var results []executil.Result + for _, dir := range modDirs { + results = append(results, checkModule(ctx, dir, moduleLabel(root, dir, multi))...) + } + return results, nil +} + +// moduleLabel mirrors rust.crateLabel: a ": " prefix, applied +// only when more than one module was discovered, so single-module output keeps +// the bare tool names. Relative to root rather than absolute, so results do not +// carry a machine-specific path, and prefixed rather than suffixed so the +// action's `^\[(PASS|FAIL)\]