nosec comment below another comment breaks the nosec annotation

[camporter]

Summary

When annotating with // #nosec, if there is a comment directly above the annotation, the annotation does not work as the two comments are treated as a multiline comment.

It appears that #735 changed whether a comment needs to only contain the annotation or whether the annotation needs to be at the beginning of a comment.

I don't necessarily think this should change now, but just wanted to highlight that this was a regression with v2.9.3 and below.

Steps to reproduce the behavior

// something
// #nosec
h := md5.New()

gosec version

v2.9.5

Go version (output of 'go version')

go version go1.16.11 linux/amd64

Operating system / Environment

Linux

Expected behavior

gosec ignores the comment from above, or docs are updated to avoid multiline comments.

Actual behavior

gosec does not handle the nosec annotation.

[ccojocar]

Thanks for pointing this out. Previously gosec used to ignore the entire AST under the #nosec comment. Maybe we can handle this now as a special case. What do you think @Yiwei-Ding?

[Yiwei-Ding]

My very first idea of putting #nosec as a prefix is to avoid something like // G101#nosecG102somethingelseG401, which looked strange but also worked.

I got 2 solutions here:

• Roll back to the previous behavior, i.e., if a comment (group) contains #nosec, gosec will trying to ignore violations.

  foundDefaultTag := strings.Contains(comment, noSecDefaultTag)
  foundAlternativeTag := strings.Contains(comment, noSecAlternativeTag)

• Discard the content before #nosec and consider the the reset of the comment as the #nosec annotation.

  if foundDefaultTag || foundAlternativeTag {
      if foundDefaultTag {
          comment = strings.SplitN(comment, noSecDefaultTag, 2)[1]
      } else {
          comment = strings.SplitN(comment, noSecAlternativeTag, 2)[1]
      }

Both are good to me. What's your idea @ccojocar ?

[ccojocar]

Discarding what's in front of nosec, it looks to me like a better option. The users should write the comment following the documented format, if they want their suppression message to show up in the report.