Skip to content

Commit 3150b28

Browse files
authored
feat: add goanalysis package for nogo (#1449)
* feat: add goanalysis package for nogo Add goanalysis package providing a standard golang.org/x/tools/go/analysis.Analyzer for gosec. Enables integration with nogo, and go vet. - Implements analysis.Analyzer interface - Reuses SSA built by analysis framework for efficient caching - Configurable severity/confidence filtering via flags - Includes CWE IDs in diagnostics ([CWE-XXX] format) - Runs both AST rules and SSA analyzers - Respects #nosec and suppression directives Also exclude testdata from security scanning in Makefile to prevent false positives on intentionally vulnerable test files. * Also exclude testdata from github action
1 parent 7284e15 commit 3150b28

9 files changed

Lines changed: 386 additions & 3 deletions

File tree

.github/workflows/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ jobs:
3838
- name: Run Gosec Security Scanner
3939
uses: securego/gosec@master
4040
with:
41-
args: ./...
41+
args: '-exclude-dir=testdata ./...'
4242
- name: Run Tests
4343
run: make test
4444
- name: Perf Diff

.github/workflows/scan.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ jobs:
1818
uses: securego/gosec@master
1919
with:
2020
# we let the report trigger content trigger a failure using the GitHub Security features.
21-
args: '-no-fail -fmt sarif -out results.sarif ./...'
21+
args: '-no-fail -fmt sarif -out results.sarif -exclude-dir=testdata ./...'
2222
- name: Upload SARIF file
2323
uses: github/codeql-action/upload-sarif@v4
2424
with:

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ golangci:
4848

4949
sec:
5050
@echo "SECURITY SCANNING"
51-
./$(BIN) ./...
51+
./$(BIN) -exclude-dir=testdata ./...
5252

5353
govulncheck: install-govulncheck
5454
@echo "CHECKING VULNERABILITIES"

README.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,21 @@ jobs:
144144
sarif_file: results.sarif
145145
```
146146

147+
### Go Analysis
148+
149+
The `goanalysis` package provides a [`golang.org/x/tools/go/analysis.Analyzer`](https://pkg.go.dev/golang.org/x/tools/go/analysis) for integration with tools that support the standard Go analysis interface, such as Bazel's [nogo](https://github.com/bazelbuild/rules_go/blob/master/go/nogo.rst) framework:
150+
151+
```starlark
152+
nogo(
153+
name = "nogo",
154+
deps = [
155+
"@com_github_securego_gosec_v2//goanalysis",
156+
# add more analyzers as needed
157+
],
158+
visibility = ["//visibility:public"],
159+
)
160+
```
161+
147162
### Local Installation
148163

149164
```bash

analyzer.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,24 @@ func (gosec *Analyzer) CheckAnalyzers(pkg *packages.Package) {
428428
return
429429
}
430430

431+
gosec.CheckAnalyzersWithSSA(pkg, ssaResult)
432+
}
433+
434+
// CheckAnalyzersWithSSA runs analyzers on a given package using a pre-built SSA result.
435+
// This is useful when SSA has already been built by an external analysis framework
436+
// (e.g., nogo) and you want to avoid rebuilding it, which can be expensive for large
437+
// codebases and breaks caching strategies.
438+
func (gosec *Analyzer) CheckAnalyzersWithSSA(pkg *packages.Package, ssaResult *buildssa.SSA) {
439+
// significant performance improvement if no analyzers are loaded
440+
if len(gosec.analyzerSet.Analyzers) == 0 {
441+
return
442+
}
443+
444+
if ssaResult == nil {
445+
gosec.logger.Print("CheckAnalyzersWithSSA called with nil SSA result")
446+
return
447+
}
448+
431449
resultMap := map[*analysis.Analyzer]interface{}{
432450
buildssa.Analyzer: &analyzers.SSAAnalyzerResult{
433451
Config: gosec.Config(),

goanalysis/analyzer.go

Lines changed: 243 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,243 @@
1+
// (c) Copyright gosec's authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
// Package goanalysis provides a standard golang.org/x/tools/go/analysis.Analyzer for gosec.
16+
package goanalysis
17+
18+
import (
19+
"fmt"
20+
"go/token"
21+
"io"
22+
"log"
23+
"strconv"
24+
"strings"
25+
26+
"golang.org/x/tools/go/analysis"
27+
"golang.org/x/tools/go/analysis/passes/buildssa"
28+
"golang.org/x/tools/go/packages"
29+
30+
"github.com/securego/gosec/v2"
31+
"github.com/securego/gosec/v2/analyzers"
32+
"github.com/securego/gosec/v2/issue"
33+
"github.com/securego/gosec/v2/rules"
34+
)
35+
36+
const Doc = `gosec is a static analysis tool that scans Go code for security problems.`
37+
38+
// Analyzer is the standard go/analysis Analyzer for gosec.
39+
var Analyzer = &analysis.Analyzer{
40+
Name: "gosec",
41+
Doc: Doc,
42+
Run: run,
43+
Requires: []*analysis.Analyzer{buildssa.Analyzer},
44+
}
45+
46+
var (
47+
flagIncludeRules string
48+
flagExcludeRules string
49+
flagExcludeGenerated bool
50+
flagMinSeverity string
51+
flagMinConfidence string
52+
)
53+
54+
//nolint:gochecknoinits // Required for go/analysis Analyzer flag registration
55+
func init() {
56+
Analyzer.Flags.StringVar(&flagIncludeRules, "include", "", "Comma-separated list of rule IDs to include (e.g., G101,G102)")
57+
Analyzer.Flags.StringVar(&flagExcludeRules, "exclude", "", "Comma-separated list of rule IDs to exclude (e.g., G104)")
58+
Analyzer.Flags.BoolVar(&flagExcludeGenerated, "exclude-generated", true, "Exclude generated code from analysis")
59+
Analyzer.Flags.StringVar(&flagMinSeverity, "severity", "low", "Minimum severity: low, medium, or high")
60+
Analyzer.Flags.StringVar(&flagMinConfidence, "confidence", "low", "Minimum confidence: low, medium, or high")
61+
}
62+
63+
func run(pass *analysis.Pass) (any, error) {
64+
// Create gosec config and analyzer
65+
config := gosec.NewConfig()
66+
logger := log.New(io.Discard, "", 0) // Discard gosec's verbose logging
67+
gosecAnalyzer := gosec.NewAnalyzer(config, false, flagExcludeGenerated, false, 1, logger)
68+
69+
// Build filters from include/exclude flags
70+
ruleFilters := buildFilters(flagIncludeRules, flagExcludeRules, rules.NewRuleFilter)
71+
analyzerFilters := buildFilters(flagIncludeRules, flagExcludeRules, analyzers.NewAnalyzerFilter)
72+
73+
// Load rules and analyzers
74+
ruleList := rules.Generate(false, ruleFilters...)
75+
ruleBuilders, ruleSuppressed := ruleList.RulesInfo()
76+
gosecAnalyzer.LoadRules(ruleBuilders, ruleSuppressed)
77+
78+
analyzerList := analyzers.Generate(false, analyzerFilters...)
79+
analyzerDefs, analyzerSuppressed := analyzerList.AnalyzersInfo()
80+
gosecAnalyzer.LoadAnalyzers(analyzerDefs, analyzerSuppressed)
81+
82+
// Convert analysis.Pass to packages.Package
83+
pkg := convertPassToPackage(pass)
84+
85+
// Run gosec AST-based rules on the package
86+
// This populates context.Ignores with nosec suppressions from comments
87+
gosecAnalyzer.CheckRules(pkg)
88+
89+
// Run SSA-based analyzers using the cached SSA result provided by the analysis framework
90+
// This reuses the SSA already built, maintaining cache efficiency
91+
// Both AST and SSA issues will respect nosec comments via gosec's updateIssues()
92+
if ssaResult, ok := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA); ok && ssaResult != nil {
93+
gosecAnalyzer.CheckAnalyzersWithSSA(pkg, ssaResult)
94+
}
95+
96+
// Get all results (both AST and SSA, with nosec filtering already applied)
97+
issues, _, _ := gosecAnalyzer.Report()
98+
99+
// Report issues as diagnostics, filtering by severity and confidence
100+
minSev, err := parseScore(flagMinSeverity)
101+
if err != nil {
102+
return nil, fmt.Errorf("invalid severity %q: %w", flagMinSeverity, err)
103+
}
104+
minConf, err := parseScore(flagMinConfidence)
105+
if err != nil {
106+
return nil, fmt.Errorf("invalid confidence %q: %w", flagMinConfidence, err)
107+
}
108+
109+
for _, iss := range issues {
110+
if iss.Severity < minSev || iss.Confidence < minConf {
111+
continue
112+
}
113+
114+
pos := parsePosition(pass.Fset, iss)
115+
what := iss.What
116+
if iss.Cwe != nil && iss.Cwe.ID != "" {
117+
what = fmt.Sprintf("[%s] %s", iss.Cwe.SprintID(), iss.What)
118+
}
119+
msg := fmt.Sprintf("%s: %s (Severity: %s, Confidence: %s)", iss.RuleID, what, iss.Severity.String(), iss.Confidence.String())
120+
121+
// If we can't locate the issue, report it anyway but note the location problem
122+
if pos == token.NoPos {
123+
msg = fmt.Sprintf("%s [unable to locate %s:%s]", msg, iss.File, iss.Line)
124+
}
125+
126+
pass.Report(analysis.Diagnostic{
127+
Pos: pos,
128+
Category: iss.RuleID,
129+
Message: msg,
130+
})
131+
}
132+
133+
return nil, nil
134+
}
135+
136+
// convertPassToPackage converts an analysis.Pass to a packages.Package
137+
// that gosec expects. This allows us to reuse gosec's existing analysis logic.
138+
func convertPassToPackage(pass *analysis.Pass) *packages.Package {
139+
pkg := &packages.Package{
140+
Name: pass.Pkg.Name(),
141+
Fset: pass.Fset,
142+
Syntax: pass.Files,
143+
Types: pass.Pkg,
144+
TypesInfo: pass.TypesInfo,
145+
TypesSizes: pass.TypesSizes,
146+
}
147+
148+
// Populate file names for the package
149+
pkg.CompiledGoFiles = make([]string, len(pass.Files))
150+
for i, f := range pass.Files {
151+
pkg.CompiledGoFiles[i] = pass.Fset.File(f.Pos()).Name()
152+
}
153+
154+
return pkg
155+
}
156+
157+
// buildFilters creates include/exclude filters from comma-separated rule IDs
158+
func buildFilters[T any](include, exclude string, newFilter func(bool, ...string) T) []T {
159+
var filters []T
160+
if include != "" {
161+
if ids := parseRuleIDs(include); len(ids) > 0 {
162+
filters = append(filters, newFilter(false, ids...))
163+
}
164+
}
165+
if exclude != "" {
166+
if ids := parseRuleIDs(exclude); len(ids) > 0 {
167+
filters = append(filters, newFilter(true, ids...))
168+
}
169+
}
170+
return filters
171+
}
172+
173+
// parseRuleIDs parses a comma-separated list of rule IDs
174+
func parseRuleIDs(s string) []string {
175+
parts := strings.Split(s, ",")
176+
ids := make([]string, 0, len(parts))
177+
for _, p := range parts {
178+
if id := strings.TrimSpace(p); id != "" {
179+
ids = append(ids, id)
180+
}
181+
}
182+
return ids
183+
}
184+
185+
// parseScore converts a severity/confidence string to issue.Score
186+
func parseScore(s string) (issue.Score, error) {
187+
switch strings.ToLower(s) {
188+
case "high":
189+
return issue.High, nil
190+
case "medium":
191+
return issue.Medium, nil
192+
case "low":
193+
return issue.Low, nil
194+
default:
195+
return issue.Low, fmt.Errorf("must be low, medium, or high")
196+
}
197+
}
198+
199+
// parsePosition converts a gosec issue location to a token.Pos
200+
func parsePosition(fset *token.FileSet, iss *issue.Issue) token.Pos {
201+
var file *token.File
202+
fset.Iterate(func(f *token.File) bool {
203+
if f.Name() == iss.File {
204+
file = f
205+
return false
206+
}
207+
return true
208+
})
209+
210+
if file == nil {
211+
return token.NoPos
212+
}
213+
214+
// Handle line ranges (e.g., "28-34") by using the start line
215+
lineStr := iss.Line
216+
if idx := strings.Index(lineStr, "-"); idx > 0 {
217+
lineStr = lineStr[:idx]
218+
}
219+
220+
line, err := strconv.Atoi(lineStr)
221+
if err != nil || line < 1 || line > file.LineCount() {
222+
return token.NoPos
223+
}
224+
225+
lineStart := file.LineStart(line)
226+
227+
// Add column offset if available (column is 1-based)
228+
col, err := strconv.Atoi(iss.Col)
229+
if err != nil || col < 1 {
230+
return lineStart
231+
}
232+
233+
// Calculate position: lineStart is the position of the first character,
234+
// so we add (col - 1) to get the correct column position
235+
pos := lineStart + token.Pos(col-1)
236+
237+
// Ensure we don't exceed file bounds
238+
if int(pos) > file.Base()+file.Size() {
239+
return lineStart
240+
}
241+
242+
return pos
243+
}

goanalysis/analyzer_test.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// (c) Copyright gosec's authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package goanalysis_test
16+
17+
import (
18+
"testing"
19+
20+
"golang.org/x/tools/go/analysis/analysistest"
21+
22+
"github.com/securego/gosec/v2/goanalysis"
23+
)
24+
25+
func TestAnalyzer(t *testing.T) {
26+
analysistest.Run(t, analysistest.TestData(), goanalysis.Analyzer, "a")
27+
}
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
package a
2+
3+
import (
4+
"crypto/md5" // want `G501: \[CWE-327\] Blocklisted import crypto/md5: weak cryptographic primitive`
5+
"fmt"
6+
"os/exec"
7+
)
8+
9+
func VulnerableFunction() {
10+
// Test SQL injection - gosec doesn't catch simple string concatenation without database/sql
11+
query := "SELECT * FROM users WHERE name = '" + getUserInput() + "'"
12+
_ = query
13+
14+
// G204: Command injection (AST-based rule)
15+
cmd := exec.Command("sh", "-c", getUserInput()) // want `G204: \[CWE-78\] Subprocess launched with a potential tainted input or cmd arguments`
16+
_ = cmd
17+
18+
// G401: Weak crypto (AST-based rule)
19+
h := md5.New() // want `G401: \[CWE-328\] Use of weak cryptographic primitive`
20+
_ = h
21+
}
22+
23+
func getUserInput() string {
24+
return "test"
25+
}
26+
27+
func SecureFunction() {
28+
fmt.Println("This is secure")
29+
}
30+
31+
func IntegerOverflow() {
32+
// G115: Integer overflow in type conversion (SSA-based analyzer)
33+
var a uint32 = 0xFFFFFFFF
34+
b := int32(a) // want `G115`
35+
fmt.Println(b)
36+
}
37+
38+
func SliceBounds() {
39+
// G602: Slice bounds check (SSA-based analyzer)
40+
s := []int{1, 2, 3}
41+
idx := 10
42+
_ = s[:idx] // want `G602`
43+
}

0 commit comments

Comments
 (0)