Skip to content

Commit 72f1145

Browse files
authored
Fix #743 (#748)
* Check if nosec tag is in front of a line * Use \n instead of a whitespace in a test case
1 parent 63a8e78 commit 72f1145

File tree

2 files changed

+77
-2
lines changed

2 files changed

+77
-2
lines changed

analyzer.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -325,12 +325,19 @@ func (gosec *Analyzer) ignore(n ast.Node) map[string]SuppressionInfo {
325325

326326
for _, group := range groups {
327327
comment := strings.TrimSpace(group.Text())
328-
foundDefaultTag := strings.HasPrefix(comment, noSecDefaultTag)
329-
foundAlternativeTag := strings.HasPrefix(comment, noSecAlternativeTag)
328+
foundDefaultTag := strings.HasPrefix(comment, noSecDefaultTag) || regexp.MustCompile("\n *"+noSecDefaultTag).Match([]byte(comment))
329+
foundAlternativeTag := strings.HasPrefix(comment, noSecAlternativeTag) || regexp.MustCompile("\n *"+noSecAlternativeTag).Match([]byte(comment))
330330

331331
if foundDefaultTag || foundAlternativeTag {
332332
gosec.stats.NumNosec++
333333

334+
// Discard what's in front of the nosec tag.
335+
if foundDefaultTag {
336+
comment = strings.SplitN(comment, noSecDefaultTag, 2)[1]
337+
} else {
338+
comment = strings.SplitN(comment, noSecAlternativeTag, 2)[1]
339+
}
340+
334341
// Extract the directive and the justification.
335342
justification := ""
336343
commentParts := regexp.MustCompile(`-{2,}`).Split(comment, 2)

analyzer_test.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,74 @@ var _ = Describe("Analyzer", func() {
303303
Expect(metrics.NumNosec).Should(Equal(1))
304304
})
305305

306+
It("should not report errors when nosec tag is in front of a line", func() {
307+
sample := testutils.SampleCodeG401[0]
308+
source := sample.Code[0]
309+
analyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G401")).RulesInfo())
310+
311+
nosecPackage := testutils.NewTestPackage()
312+
defer nosecPackage.Close()
313+
nosecSource := strings.Replace(source, "h := md5.New()", "//Some description\n//#nosec G401\nh := md5.New()", 1)
314+
nosecPackage.AddFile("md5.go", nosecSource)
315+
err := nosecPackage.Build()
316+
Expect(err).ShouldNot(HaveOccurred())
317+
err = analyzer.Process(buildTags, nosecPackage.Path)
318+
Expect(err).ShouldNot(HaveOccurred())
319+
nosecIssues, _, _ := analyzer.Report()
320+
Expect(nosecIssues).Should(BeEmpty())
321+
})
322+
323+
It("should report errors when nosec tag is not in front of a line", func() {
324+
sample := testutils.SampleCodeG401[0]
325+
source := sample.Code[0]
326+
analyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G401")).RulesInfo())
327+
328+
nosecPackage := testutils.NewTestPackage()
329+
defer nosecPackage.Close()
330+
nosecSource := strings.Replace(source, "h := md5.New()", "//Some description\n//Another description #nosec G401\nh := md5.New()", 1)
331+
nosecPackage.AddFile("md5.go", nosecSource)
332+
err := nosecPackage.Build()
333+
Expect(err).ShouldNot(HaveOccurred())
334+
err = analyzer.Process(buildTags, nosecPackage.Path)
335+
Expect(err).ShouldNot(HaveOccurred())
336+
nosecIssues, _, _ := analyzer.Report()
337+
Expect(nosecIssues).Should(HaveLen(sample.Errors))
338+
})
339+
340+
It("should not report errors when rules are in front of nosec tag even rules are wrong", func() {
341+
sample := testutils.SampleCodeG401[0]
342+
source := sample.Code[0]
343+
analyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G401")).RulesInfo())
344+
345+
nosecPackage := testutils.NewTestPackage()
346+
defer nosecPackage.Close()
347+
nosecSource := strings.Replace(source, "h := md5.New()", "//G301\n//#nosec\nh := md5.New()", 1)
348+
nosecPackage.AddFile("md5.go", nosecSource)
349+
err := nosecPackage.Build()
350+
Expect(err).ShouldNot(HaveOccurred())
351+
err = analyzer.Process(buildTags, nosecPackage.Path)
352+
Expect(err).ShouldNot(HaveOccurred())
353+
nosecIssues, _, _ := analyzer.Report()
354+
Expect(nosecIssues).Should(BeEmpty())
355+
})
356+
357+
It("should report errors when there are nosec tags after a #nosec WrongRuleList annotation", func() {
358+
sample := testutils.SampleCodeG401[0]
359+
source := sample.Code[0]
360+
analyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G401")).RulesInfo())
361+
362+
nosecPackage := testutils.NewTestPackage()
363+
defer nosecPackage.Close()
364+
nosecSource := strings.Replace(source, "h := md5.New()", "//#nosec\n//G301\n//#nosec\nh := md5.New()", 1)
365+
nosecPackage.AddFile("md5.go", nosecSource)
366+
err := nosecPackage.Build()
367+
Expect(err).ShouldNot(HaveOccurred())
368+
err = analyzer.Process(buildTags, nosecPackage.Path)
369+
Expect(err).ShouldNot(HaveOccurred())
370+
nosecIssues, _, _ := analyzer.Report()
371+
Expect(nosecIssues).Should(HaveLen(sample.Errors))
372+
})
373+
306374
It("should be possible to use an alternative nosec tag", func() {
307375
// Rule for MD5 weak crypto usage
308376
sample := testutils.SampleCodeG401[0]

0 commit comments

Comments
 (0)