fix(golang): scan each Go module, not just the scan root - #27
Open
pedromvgomes wants to merge 3 commits into
Open
fix(golang): scan each Go module, not just the scan root#27pedromvgomes wants to merge 3 commits into
pedromvgomes wants to merge 3 commits into
Conversation
Ecosystem detection walks the tree for go.mod, but the Go check then ran gosec and govulncheck once at the scan root. Both tools are module-scoped, so that only ever worked when a module happened to sit exactly at the root. Pointed at a monorepo whose Go modules live in subdirectories, govulncheck exits "no go.mod file" -- a hard failure that looks like a scanner crash -- while gosec quietly passes, because it walks a tree happily without a module and so reports on nothing. The pass is the worse half: it reads as "no findings" when the truth is "nothing was scanned". Adds detect.GoModuleDirs, mirroring TSPackageDirs, and runs both tools per module with the directory in the result name (gosec(dir), govulncheck(dir)) exactly as eslint(dir) already does. Unlike RustCrateDirs there is no ancestor to fold into: a nested go.mod is a separate module excluded from its parent's package graph, so every one is its own scan root. Also wires up cfg.Go.Exclude, which the config has always accepted, documented, and had a passing test for -- while nothing ever read it. gosec now runs with -exclude-generated. Findings in generated files are not actionable: the only fix is changing the generator or its input, and a `#nosec` annotation is erased by the next regeneration. This matches how generated code is already treated by golangci-lint's `exclusions: generated` and semgrep's own skip. Verified against the layout that exposed this -- two modules under a scan root holding no go.mod. Before: govulncheck FAIL "no go.mod file", gosec a hollow PASS. After: both tools run in both modules and report per module. Claude-Session: https://claude.ai/code/session_014BTXdocUzxnMytvB1qoQcG
pedromvgomes
added a commit
to wardnet/wardnet
that referenced
this pull request
Aug 5, 2026
Moving the Go modules under source/ put them inside bulwark's scan root for
the first time, so gosec now actually analyses them. It had been reporting
[PASS] on a tree it never entered.
Three findings, all false positives, annotated on the line with where the
value comes from -- the convention source/.bulwark.yml sets out:
G304 tunnels.go -- `conf` is the --conf flag. Reading the file the
operator named is the command's entire purpose.
G304 backup.go -- `passphraseFile` is --passphrase-file, read with the
operator's own privileges, and exists precisely so a
passphrase need not land in shell history.
G302 selfupdate.go -- the replacement wctl binary must be executable;
0600 would install a file that cannot run.
The two G117 findings in the SDK's generated rest.gen.go are handled in
wardnet/bulwark#27 by passing -exclude-generated, since an annotation there
would be erased by the next `go generate`.
Note this needs bulwark/pull/27 merged and released to go green: on the
current release the Go check fails before it reaches any of these.
Claude-Session: https://claude.ai/code/session_014BTXdocUzxnMytvB1qoQcG
Result naming now follows rust.crateLabel exactly -- a "<relative dir>: " prefix applied only when more than one module is found -- instead of a "tool(dir)" suffix carrying an absolute path. The suffix form was a real bug, not a style point. action.yml's tool_result greps `^\[(PASS|FAIL)\] <tool>$`, anchored, so "[FAIL] gosec(/abs/wctl)" matched nothing: the scan failed while the PR comment reported no gosec or govulncheck findings at all, in exactly the multi-module repos this change exists to support. The pattern now tolerates the optional label, which also fixes the same latent hole for multi-crate Rust. It resolves FAIL over PASS, so one passing module cannot mask a failing one. Discovery now honours the directories Go's own package loading ignores: testdata, and names starting with "." or "_". A go.mod under testdata/ is a fixture, routinely unbuildable on purpose or pinned to a vulnerable dependency to exercise a scanner. The parent's ./... never compiles it, so treating it as a scan root would fail the run on code that is not shipped. Discovery finding nothing is now an explicit no-op rather than an accident of the loop, matching rust.Check. Adds internal/golang's first test file, covering the label logic -- the part that is unit-testable without shelling out to the real tools. Left as-is: GoModuleDirs propagates a ReadDir error and aborts the scan. That is a new failure mode for Go, but it is what RustCrateDirs and TSPackageDirs already do, and diverging here would be the surprise. Claude-Session: https://claude.ai/code/session_014BTXdocUzxnMytvB1qoQcG
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found while moving wardnet's Go SDK and
wctlundersource/(wardnet/wardnet#1113), which made bulwark scan Go code for the first time.The bug
detect.Ecosystemswalks the tree forgo.mod, so Go is detected correctly. Butgolang.Checkwas then handed the scan root and ran both tools there once. Both are module-scoped, so this only ever worked when a module sat exactly at the root.Against a monorepo whose modules live in subdirectories (
--dir source, modules atsource/sdk/wardnet-goandsource/wctl):The failure is loud and looks like a broken scanner. The
[PASS]is the more dangerous half: gosec walks a directory tree happily without a module, finds nothing to analyse, and reports success. It reads as "no findings" when it means "nothing was scanned".The fix
detect.GoModuleDirsmirrorsTSPackageDirs, and both tools run per module with the directory in the result name — exactly aseslint(<dir>)already does:Unlike
RustCrateDirsthere is no ancestor relationship to resolve: a nestedgo.modstarts a genuinely separate module, excluded from its parent's package graph, so./...at an ancestor would never reach it. Everygo.modis its own scan root. Ago.workdoesn't change that — it affects local resolution, not module boundaries.Two things came with it:
cfg.Go.Excludeis now actually used. The config has always accepted it, documented it, and had a passing test for it, while nothing ever read it.-exclude-generated. Findings in generated files aren't actionable — the only fix is changing the generator or its input, and a#nosecannotation is wiped by the next regeneration. This matches golangci-lint'sexclusions: generatedand semgrep's own generated-file skip. In practice it was flaggingG117on aPasswordfield of a generated login-request DTO.Testing
Five table-style tests alongside the
RustCrateDirsones, including the exact regression: a scan root with nogo.modand modules in subdirectories.go test ./...andgolangci-lintclean.Verified end-to-end by building both binaries and running them against a faithful copy of the failing tree — before:
govulncheckFAIL plus a hollow gosec PASS; after: four real per-module results.Once this is merged and released, wardnet/wardnet#1113 picks it up automatically — the action installs
latest.Merge Commit Message
fix(golang): scan each Go module, not just the scan root