Fix #1240: nosec comments now work with trailing open brackets (#1475)

Add line-based fallback for nosec comment detection when ast.CommentMap
fails to associate comments correctly. This fixes the case where #nosec
comments at the end of lines with open brackets were being ignored.

Refactor duplicate parsing logic into parseNoSecDirective helper function
to reduce code complexity and improve maintainability.

Author: ravisastryk

analyzer.go

@@ -144,6 +144,7 @@ func (i ignores) get(file string, line string) map[string][]issue.SuppressionInf
 type Context struct {
 	FileSet      *token.FileSet
 	Comments     ast.CommentMap
+	AllComments  []*ast.CommentGroup // All comments in the file for line-based nosec lookup (fix for #1240)
 	Info         *types.Info
 	Pkg          *types.Package
 	PkgFiles     []*ast.File
@@ -475,6 +476,7 @@ func (gosec *Analyzer) checkRules(pkg *packages.Package) ([]*issue.Issue, *Metri
 			FileSet:      pkg.Fset,
 			Config:       gosec.config,
 			Comments:     ast.NewCommentMap(pkg.Fset, file, file.Comments),
+			AllComments:  file.Comments, // Store all comments for line-based lookup (fix for #1240)
 			Root:         file,
 			Info:         pkg.TypesInfo,
 			Pkg:          pkg.Types,
@@ -800,15 +802,39 @@ func (v *astVisitor) updateIgnoredRulesForNode(n ast.Node) {
 	}
 }
 
+// parseNoSecDirective extracts rules and justification from nosec directive arguments
+func parseNoSecDirective(args string) map[string]issue.SuppressionInfo {
+	justification := ""
+	commentParts := regexp.MustCompile(`-{2,}`).Split(args, 2)
+	directive := commentParts[0]
+	if len(commentParts) > 1 {
+		justification = strings.TrimSpace(strings.TrimRight(commentParts[1], "\n"))
+	}
+
+	re := regexp.MustCompile(`(G\d{3})`)
+	matches := re.FindAllStringSubmatch(directive, -1)
+
+	suppression := issue.SuppressionInfo{
+		Kind:          "inSource",
+		Justification: justification,
+	}
+
+	ignores := make(map[string]issue.SuppressionInfo)
+	if len(matches) == 0 {
+		ignores[aliasOfAllRules] = suppression
+	} else {
+		for _, v := range matches {
+			ignores[v[1]] = suppression
+		}
+	}
+	return ignores
+}
+
 // ignore checks if a node is tagged with a nosec comment and returns the suppressed rules.
 func (v *astVisitor) ignore(n ast.Node) map[string]issue.SuppressionInfo {
 	if v.ignoreNosec {
 		return nil
 	}
-	groups, ok := v.context.Comments[n]
-	if !ok {
-		return nil
-	}
 
 	noSecDefaultTag, err := v.gosec.config.GetGlobal(Nosec)
 	if err != nil {
@@ -823,38 +849,56 @@ func (v *astVisitor) ignore(n ast.Node) map[string]issue.SuppressionInfo {
 		noSecAlternativeTag = NoSecTag(noSecAlternativeTag)
 	}
 
-	for _, group := range groups {
-		found, args := findNoSecDirective(group, noSecDefaultTag, noSecAlternativeTag)
-		if !found {
-			continue
-		}
-		v.stats.NumNosec++
-
-		justification := ""
-		commentParts := regexp.MustCompile(`-{2,}`).Split(args, 2)
-		directive := commentParts[0]
-		if len(commentParts) > 1 {
-			justification = strings.TrimSpace(strings.TrimRight(commentParts[1], "\n"))
-		}
-
-		re := regexp.MustCompile(`(G\d{3})`)
-		matches := re.FindAllStringSubmatch(directive, -1)
-
-		suppression := issue.SuppressionInfo{
-			Kind:          "inSource",
-			Justification: justification,
-		}
+	// First, try the existing AST CommentMap lookup
+	groups, ok := v.context.Comments[n]
+	if ok {
+		for _, group := range groups {
+			found, args := findNoSecDirective(group, noSecDefaultTag, noSecAlternativeTag)
+			if !found {
+				continue
+			}
+			v.stats.NumNosec++
+			return parseNoSecDirective(args)
+		}
+	}
+
+	// FIX FOR ISSUE #1240:
+	// If not found in CommentMap, check all comments on the same line.
+	// This handles the case where the comment is associated with a parent node
+	// (like IfStmt or BlockStmt) instead of the specific expression that
+	// triggers the rule (like a type conversion CallExpr).
+	//
+	// Go's ast.CommentMap associates comments with the "largest" node on the
+	// same line. For example:
+	//     if len(x) <= int(y) { // #nosec G115
+	// The comment gets associated with the IfStmt, not the int(y) CallExpr.
+	// This fallback ensures the nosec still applies to the conversion.
+	if v.context.FileSet != nil && n != nil && v.context.AllComments != nil {
+		nodePos := v.context.FileSet.Position(n.Pos())
+		nodeEndPos := v.context.FileSet.Position(n.End())
+
+		for _, group := range v.context.AllComments {
+			if group == nil {
+				continue
+			}
 
-		ignores := make(map[string]issue.SuppressionInfo)
-		for _, v := range matches {
-			ignores[v[1]] = suppression
-		}
+			for _, comment := range group.List {
+				commentPos := v.context.FileSet.Position(comment.Pos())
 
-		if len(matches) == 0 {
-			ignores[aliasOfAllRules] = suppression
+				// Check if the comment is on the same line as the node
+				// (either where it starts or ends, to handle multi-line nodes)
+				if commentPos.Line == nodePos.Line || commentPos.Line == nodeEndPos.Line {
+					found, args := findNoSecDirective(group, noSecDefaultTag, noSecAlternativeTag)
+					if !found {
+						continue
+					}
+					v.stats.NumNosec++
+					return parseNoSecDirective(args)
+				}
+			}
 		}
-		return ignores
 	}
+
 	return nil
 }
 

analyzer_test.go

@@ -2303,4 +2303,134 @@ func main() {
 			Expect(issues[0].Suppressions[0].Justification).To(Equal("false positive, this is not a private data"))
 		})
 	})
+
+	Context("when fixing issue #1240 - nosec with open bracket", func() {
+		It("should suppress G115 when #nosec is at the end of an if line with bracket", func() {
+			source := `
+package main
+
+import "fmt"
+
+func main() {
+	ten := 10
+	uintTen := uint(10)
+	configVal := uint(ten) // #nosec G115 -- this works
+	inputSlice := []int{1, 2, 3, 4, 5}
+
+	if len(inputSlice) <= int(uintTen) { // #nosec G115 -- this works
+		fmt.Println("hello world!")
+	}
+
+	if len(inputSlice) <= int(configVal) { // #nosec G115 -- this should work now (fix for #1240)
+		fmt.Println("hello world!")
+	}
+}
+`
+			analyzer.LoadAnalyzers(analyzers.Generate(false, analyzers.NewAnalyzerFilter(false, "G115")).AnalyzersInfo())
+			pkg := testutils.NewTestPackage()
+			defer pkg.Close()
+			pkg.AddFile("main.go", source)
+			err := pkg.Build()
+			Expect(err).ShouldNot(HaveOccurred())
+			err = analyzer.Process(buildTags, pkg.Path)
+			Expect(err).ShouldNot(HaveOccurred())
+			issues, metrics, _ := analyzer.Report()
+			// No G115 issues should be reported as all conversions are suppressed
+			for _, issue := range issues {
+				if issue.RuleID == "G115" {
+					Fail(fmt.Sprintf("G115 should be suppressed but was reported at line %s", issue.Line))
+				}
+			}
+			Expect(metrics.NumNosec).Should(BeNumerically(">=", 3)) // At least 3 nosec comments
+		})
+
+		It("should suppress G115 when #nosec is used with block comment before bracket", func() {
+			source := `
+package main
+
+import "fmt"
+
+func main() {
+	configVal := uint(10)
+	inputSlice := []int{1, 2, 3, 4, 5}
+
+	if len(inputSlice) <= int(configVal) /* #nosec G115 */ {
+		fmt.Println("hello world!")
+	}
+}
+`
+			analyzer.LoadAnalyzers(analyzers.Generate(false, analyzers.NewAnalyzerFilter(false, "G115")).AnalyzersInfo())
+			pkg := testutils.NewTestPackage()
+			defer pkg.Close()
+			pkg.AddFile("main.go", source)
+			err := pkg.Build()
+			Expect(err).ShouldNot(HaveOccurred())
+			err = analyzer.Process(buildTags, pkg.Path)
+			Expect(err).ShouldNot(HaveOccurred())
+			issues, _, _ := analyzer.Report()
+			// No G115 issues should be reported
+			for _, issue := range issues {
+				if issue.RuleID == "G115" {
+					Fail(fmt.Sprintf("G115 should be suppressed but was reported at line %s", issue.Line))
+				}
+			}
+		})
+
+		It("should suppress G115 in for loop with bracket and trailing comment", func() {
+			source := `
+package main
+
+func main() {
+	x := uint(10)
+	for i := 0; i < int(x); i++ { // #nosec G115
+		println(i)
+	}
+}
+`
+			analyzer.LoadAnalyzers(analyzers.Generate(false, analyzers.NewAnalyzerFilter(false, "G115")).AnalyzersInfo())
+			pkg := testutils.NewTestPackage()
+			defer pkg.Close()
+			pkg.AddFile("main.go", source)
+			err := pkg.Build()
+			Expect(err).ShouldNot(HaveOccurred())
+			err = analyzer.Process(buildTags, pkg.Path)
+			Expect(err).ShouldNot(HaveOccurred())
+			issues, _, _ := analyzer.Report()
+			// No G115 issues should be reported
+			for _, issue := range issues {
+				if issue.RuleID == "G115" {
+					Fail(fmt.Sprintf("G115 should be suppressed but was reported at line %s", issue.Line))
+				}
+			}
+		})
+
+		It("should suppress G115 in switch statement with bracket and trailing comment", func() {
+			source := `
+package main
+
+func main() {
+	x := uint(10)
+	switch int(x) { // #nosec G115
+	case 10:
+		println("ten")
+	}
+}
+`
+			analyzer.LoadAnalyzers(analyzers.Generate(false, analyzers.NewAnalyzerFilter(false, "G115")).AnalyzersInfo())
+			pkg := testutils.NewTestPackage()
+			defer pkg.Close()
+			pkg.AddFile("main.go", source)
+			err := pkg.Build()
+			Expect(err).ShouldNot(HaveOccurred())
+			err = analyzer.Process(buildTags, pkg.Path)
+			Expect(err).ShouldNot(HaveOccurred())
+			issues, _, _ := analyzer.Report()
+			// No G115 issues should be reported
+			for _, issue := range issues {
+				if issue.RuleID == "G115" {
+					Fail(fmt.Sprintf("G115 should be suppressed but was reported at line %s", issue.Line))
+				}
+			}
+		})
+	})
 })