Skip to content

Commit b45f95f

Browse files
authored
Add support for suppressing the findings
1 parent 040327f commit b45f95f

15 files changed

+448
-127
lines changed

README.md

+27-1
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,8 @@ gosec -exclude-generated ./...
269269
### Annotating code
270270

271271
As with all automated detection tools, there will be cases of false positives. In cases where gosec reports a failure that has been manually verified as being safe,
272-
it is possible to annotate the code with a `#nosec` comment.
272+
it is possible to annotate the code with a comment that starts with `#nosec`.
273+
The `#nosec` comment should have the format `#nosec [RuleList] [-- Justification]`.
273274

274275
The annotation causes gosec to stop processing any further nodes within the
275276
AST so can apply to a whole block or more granularly to a single expression.
@@ -294,6 +295,10 @@ When a specific false positive has been identified and verified as safe, you may
294295
within a section of code, while continuing to scan for other problems. To do this, you can list the rule(s) to be suppressed within
295296
the `#nosec` annotation, e.g: `/* #nosec G401 */` or `// #nosec G201 G202 G203`
296297

298+
You could put the description or justification text for the annotation. The
299+
justification should be after the rule(s) to suppress and start with two or
300+
more dashes, e.g: `// #nosec G101 G102 -- This is a false positive`
301+
297302
In some cases you may also want to revisit places where `#nosec` annotations
298303
have been used. To run the scanner and ignore any `#nosec` annotations you
299304
can do the following:
@@ -302,6 +307,27 @@ can do the following:
302307
gosec -nosec=true ./...
303308
```
304309

310+
### Tracking suppressions
311+
312+
As described above, we could suppress violations externally (using `-include`/
313+
`-exclude`) or inline (using `#nosec` annotations) in gosec. This suppression
314+
inflammation can be used to generate corresponding signals for auditing
315+
purposes.
316+
317+
We could track suppressions by the `-track-suppressions` flag as follows:
318+
319+
```bash
320+
gosec -track-suppressions -exclude=G101 -fmt=sarif -out=results.sarif ./...
321+
```
322+
323+
- For external suppressions, gosec records suppression info where `kind` is
324+
`external` and `justification` is a certain sentence "Globally suppressed".
325+
- For inline suppressions, gosec records suppression info where `kind` is
326+
`inSource` and `justification` is the text after two or more dashes in the
327+
comment.
328+
329+
**Note:** Only SARIF and JSON formats support tracking suppressions.
330+
305331
### Build tags
306332

307333
gosec is able to pass your [Go build tags](https://golang.org/pkg/go/build/) to the analyzer.

analyzer.go

+85-47
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ const LoadMode = packages.NeedName |
4343
packages.NeedTypesInfo |
4444
packages.NeedSyntax
4545

46+
const externalSuppressionJustification = "Globally suppressed."
47+
48+
const aliasOfAllRules = "*"
49+
4650
var generatedCodePattern = regexp.MustCompile(`^// Code generated .* DO NOT EDIT\.$`)
4751

4852
// The Context is populated with data parsed from the source code as it is scanned.
@@ -57,7 +61,7 @@ type Context struct {
5761
Root *ast.File
5862
Config Config
5963
Imports *ImportTracker
60-
Ignores []map[string]bool
64+
Ignores []map[string][]SuppressionInfo
6165
PassedValues map[string]interface{}
6266
}
6367

@@ -72,21 +76,29 @@ type Metrics struct {
7276
// Analyzer object is the main object of gosec. It has methods traverse an AST
7377
// and invoke the correct checking rules as on each node as required.
7478
type Analyzer struct {
75-
ignoreNosec bool
76-
ruleset RuleSet
77-
context *Context
78-
config Config
79-
logger *log.Logger
80-
issues []*Issue
81-
stats *Metrics
82-
errors map[string][]Error // keys are file paths; values are the golang errors in those files
83-
tests bool
84-
excludeGenerated bool
85-
showIgnored bool
79+
ignoreNosec bool
80+
ruleset RuleSet
81+
context *Context
82+
config Config
83+
logger *log.Logger
84+
issues []*Issue
85+
stats *Metrics
86+
errors map[string][]Error // keys are file paths; values are the golang errors in those files
87+
tests bool
88+
excludeGenerated bool
89+
showIgnored bool
90+
trackSuppressions bool
91+
}
92+
93+
// SuppressionInfo object is to record the kind and the justification that used
94+
// to suppress violations.
95+
type SuppressionInfo struct {
96+
Kind string `json:"kind"`
97+
Justification string `json:"justification"`
8698
}
8799

88100
// NewAnalyzer builds a new analyzer.
89-
func NewAnalyzer(conf Config, tests bool, excludeGenerated bool, logger *log.Logger) *Analyzer {
101+
func NewAnalyzer(conf Config, tests bool, excludeGenerated bool, trackSuppressions bool, logger *log.Logger) *Analyzer {
90102
ignoreNoSec := false
91103
if enabled, err := conf.IsGlobalEnabled(Nosec); err == nil {
92104
ignoreNoSec = enabled
@@ -99,17 +111,18 @@ func NewAnalyzer(conf Config, tests bool, excludeGenerated bool, logger *log.Log
99111
logger = log.New(os.Stderr, "[gosec]", log.LstdFlags)
100112
}
101113
return &Analyzer{
102-
ignoreNosec: ignoreNoSec,
103-
showIgnored: showIgnored,
104-
ruleset: make(RuleSet),
105-
context: &Context{},
106-
config: conf,
107-
logger: logger,
108-
issues: make([]*Issue, 0, 16),
109-
stats: &Metrics{},
110-
errors: make(map[string][]Error),
111-
tests: tests,
112-
excludeGenerated: excludeGenerated,
114+
ignoreNosec: ignoreNoSec,
115+
showIgnored: showIgnored,
116+
ruleset: NewRuleSet(),
117+
context: &Context{},
118+
config: conf,
119+
logger: logger,
120+
issues: make([]*Issue, 0, 16),
121+
stats: &Metrics{},
122+
errors: make(map[string][]Error),
123+
tests: tests,
124+
excludeGenerated: excludeGenerated,
125+
trackSuppressions: trackSuppressions,
113126
}
114127
}
115128

@@ -125,10 +138,10 @@ func (gosec *Analyzer) Config() Config {
125138

126139
// LoadRules instantiates all the rules to be used when analyzing source
127140
// packages
128-
func (gosec *Analyzer) LoadRules(ruleDefinitions map[string]RuleBuilder) {
141+
func (gosec *Analyzer) LoadRules(ruleDefinitions map[string]RuleBuilder, ruleSuppressed map[string]bool) {
129142
for id, def := range ruleDefinitions {
130143
r, nodes := def(id, gosec.config)
131-
gosec.ruleset.Register(r, nodes...)
144+
gosec.ruleset.Register(r, ruleSuppressed[id], nodes...)
132145
}
133146
}
134147

@@ -295,7 +308,7 @@ func (gosec *Analyzer) AppendError(file string, err error) {
295308
}
296309

297310
// ignore a node (and sub-tree) if it is tagged with a nosec tag comment
298-
func (gosec *Analyzer) ignore(n ast.Node) ([]string, bool) {
311+
func (gosec *Analyzer) ignore(n ast.Node) map[string]SuppressionInfo {
299312
if groups, ok := gosec.context.Comments[n]; ok && !gosec.ignoreNosec {
300313

301314
// Checks if an alternative for #nosec is set and, if not, uses the default.
@@ -307,31 +320,44 @@ func (gosec *Analyzer) ignore(n ast.Node) ([]string, bool) {
307320

308321
for _, group := range groups {
309322

310-
foundDefaultTag := strings.Contains(group.Text(), noSecDefaultTag)
311-
foundAlternativeTag := strings.Contains(group.Text(), noSecAlternativeTag)
323+
foundDefaultTag := strings.HasPrefix(group.Text(), noSecDefaultTag)
324+
foundAlternativeTag := strings.HasPrefix(group.Text(), noSecAlternativeTag)
312325

313326
if foundDefaultTag || foundAlternativeTag {
314327
gosec.stats.NumNosec++
315328

329+
// Extract the directive and the justification.
330+
justification := ""
331+
commentParts := regexp.MustCompile(`-{2,}`).Split(group.Text(), 2)
332+
directive := commentParts[0]
333+
if len(commentParts) > 1 {
334+
justification = strings.TrimSpace(strings.TrimRight(commentParts[1], "\n"))
335+
}
336+
316337
// Pull out the specific rules that are listed to be ignored.
317338
re := regexp.MustCompile(`(G\d{3})`)
318-
matches := re.FindAllStringSubmatch(group.Text(), -1)
339+
matches := re.FindAllStringSubmatch(directive, -1)
319340

320-
// If no specific rules were given, ignore everything.
321-
if len(matches) == 0 {
322-
return nil, true
341+
suppression := SuppressionInfo{
342+
Kind: "inSource",
343+
Justification: justification,
323344
}
324345

325346
// Find the rule IDs to ignore.
326-
var ignores []string
347+
ignores := make(map[string]SuppressionInfo)
327348
for _, v := range matches {
328-
ignores = append(ignores, v[1])
349+
ignores[v[1]] = suppression
350+
}
351+
352+
// If no specific rules were given, ignore everything.
353+
if len(matches) == 0 {
354+
ignores[aliasOfAllRules] = suppression
329355
}
330-
return ignores, false
356+
return ignores
331357
}
332358
}
333359
}
334-
return nil, false
360+
return nil
335361
}
336362

337363
// Visit runs the gosec visitor logic over an AST created by parsing go code.
@@ -346,31 +372,40 @@ func (gosec *Analyzer) Visit(n ast.Node) ast.Visitor {
346372
}
347373

348374
// Get any new rule exclusions.
349-
ignoredRules, ignoreAll := gosec.ignore(n)
350-
if ignoreAll {
351-
return nil
352-
}
375+
ignoredRules := gosec.ignore(n)
353376

354377
// Now create the union of exclusions.
355-
ignores := map[string]bool{}
378+
ignores := map[string][]SuppressionInfo{}
356379
if len(gosec.context.Ignores) > 0 {
357380
for k, v := range gosec.context.Ignores[0] {
358381
ignores[k] = v
359382
}
360383
}
361384

362-
for _, v := range ignoredRules {
363-
ignores[v] = true
385+
for ruleID, suppression := range ignoredRules {
386+
ignores[ruleID] = append(ignores[ruleID], suppression)
364387
}
365388

366389
// Push the new set onto the stack.
367-
gosec.context.Ignores = append([]map[string]bool{ignores}, gosec.context.Ignores...)
390+
gosec.context.Ignores = append([]map[string][]SuppressionInfo{ignores}, gosec.context.Ignores...)
368391

369392
// Track aliased and initialization imports
370393
gosec.context.Imports.TrackImport(n)
371394

372395
for _, rule := range gosec.ruleset.RegisteredFor(n) {
373-
_, ignored := ignores[rule.ID()]
396+
// Check if all rules are ignored.
397+
suppressions, ignored := ignores[aliasOfAllRules]
398+
if !ignored {
399+
suppressions, ignored = ignores[rule.ID()]
400+
}
401+
// Track external suppressions.
402+
if gosec.ruleset.IsRuleSuppressed(rule.ID()) {
403+
ignored = true
404+
suppressions = append(suppressions, SuppressionInfo{
405+
Kind: "external",
406+
Justification: externalSuppressionJustification,
407+
})
408+
}
374409

375410
issue, err := rule.Match(n, gosec.context)
376411
if err != nil {
@@ -385,7 +420,10 @@ func (gosec *Analyzer) Visit(n ast.Node) ast.Visitor {
385420
if !ignored || !gosec.showIgnored {
386421
gosec.stats.NumFound++
387422
}
388-
if !ignored || gosec.showIgnored || gosec.ignoreNosec {
423+
if ignored && gosec.trackSuppressions {
424+
issue.WithSuppressions(suppressions)
425+
gosec.issues = append(gosec.issues, issue)
426+
} else if !ignored || gosec.showIgnored || gosec.ignoreNosec {
389427
gosec.issues = append(gosec.issues, issue)
390428
}
391429
}

0 commit comments

Comments
 (0)