fix: resolve critical issues in reviewer, CWE matching, diff handling, and performance

- Fix go.mod version (1.25 → 1.23)
- Accumulate LLM errors instead of swallowing in reviewer
- Add regex field length validation (2000 char cap)
- Add git path traversal prevention with filepath.Rel()
- CWE matching with word boundaries instead of naive substring
- O(1) line lookup in comment mapping (pre-built map)
- Move regex compilation to package level (out of hot loops)
- Include concern in dedup key, skip empty paths, normalize with filepath.Clean()
- Add reflection output size validation
- Fix parseInt with strconv.Atoi and range validation
- Add binary file detection and line number bounds in diff parser

Author: Patel230

config.go

@@ -3,6 +3,7 @@ package sight
 import (
 	"os"
 	"path/filepath"
+	"strconv"
 	"strings"
 )
 
@@ -180,14 +181,28 @@ func parseTOMLArray(s string) []string {
 	return result
 }
 
+// parseInt parses a string as an integer with range validation.
+// It reads leading digits (ignoring trailing non-digit chars for TOML compat)
+// and caps the result at 1,000,000 to prevent unreasonable values.
 func parseInt(s string) int {
-	n := 0
-	for _, c := range s {
-		if c >= '0' && c <= '9' {
-			n = n*10 + int(c-'0')
-		} else {
-			break
-		}
+	s = strings.TrimSpace(s)
+	if s == "" {
+		return 0
+	}
+	// Extract leading digits
+	end := 0
+	for end < len(s) && s[end] >= '0' && s[end] <= '9' {
+		end++
+	}
+	if end == 0 {
+		return 0
+	}
+	n, err := strconv.Atoi(s[:end])
+	if err != nil {
+		return 0
+	}
+	if n < 0 || n > 1_000_000 {
+		return 0
 	}
 	return n
 }

coverage.out

@@ -1,3 +1,3 @@
 module github.com/GrayCodeAI/sight
 
-go 1.25.0
+go 1.23.0

go.mod

@@ -65,8 +65,16 @@ func MapToInlineFiltered(findings []FindingInput, files []diff.File, mode Filter
 	var comments []Inline
 	fileMap := buildFileMap(files)
 
+	// Pre-build line sets for O(1) lookup when using context or added modes
+	lineSetMap := make(map[string]diffLineSet)
+	if mode == FilterDiffContext || mode == FilterAdded {
+		for _, f := range files {
+			lineSetMap[f.Path] = buildDiffLineSet(f)
+		}
+	}
+
 	for _, f := range findings {
-		diffFile, exists := fileMap[f.File]
+		_, exists := fileMap[f.File]
 
 		switch mode {
 		case FilterNone:
@@ -81,15 +89,19 @@ func MapToInlineFiltered(findings []FindingInput, files []diff.File, mode Filter
 
 		case FilterDiffContext:
 			// Include if the line falls within any hunk's full range
-			// (added, removed, or context lines)
-			if exists && isInDiffContext(diffFile, f.Line) {
-				comments = append(comments, buildComment(f))
+			// (added, removed, or context lines) — O(1) lookup
+			if exists {
+				if ls, ok := lineSetMap[f.File]; ok && ls.contextLines[f.Line] {
+					comments = append(comments, buildComment(f))
+				}
 			}
 
 		default: // FilterAdded
-			// Include only if the line is within the diff's new-side range
-			if exists && isInDiff(diffFile, f.Line) {
-				comments = append(comments, buildComment(f))
+			// Include only if the line is within the diff's new-side range — O(1) lookup
+			if exists {
+				if ls, ok := lineSetMap[f.File]; ok && ls.newRangeLines[f.Line] {
+					comments = append(comments, buildComment(f))
+				}
 			}
 		}
 	}
@@ -105,6 +117,36 @@ func buildFileMap(files []diff.File) map[string]diff.File {
 	return m
 }
 
+// diffLineSet holds precomputed line number sets for O(1) lookup.
+type diffLineSet struct {
+	newRangeLines map[int]bool // lines within new-side hunk ranges
+	contextLines  map[int]bool // lines within context ranges or individual line refs
+}
+
+func buildDiffLineSet(file diff.File) diffLineSet {
+	s := diffLineSet{
+		newRangeLines: make(map[int]bool),
+		contextLines:  make(map[int]bool),
+	}
+	for _, hunk := range file.Hunks {
+		start := hunk.NewStart
+		end := hunk.NewStart + hunk.NewCount
+		for i := start; i <= end; i++ {
+			s.newRangeLines[i] = true
+			s.contextLines[i] = true
+		}
+		for _, l := range hunk.Lines {
+			if l.NewNum > 0 {
+				s.contextLines[l.NewNum] = true
+			}
+			if l.OldNum > 0 {
+				s.contextLines[l.OldNum] = true
+			}
+		}
+	}
+	return s
+}
+
 func isInDiff(file diff.File, line int) bool {
 	for _, hunk := range file.Hunks {
 		start := hunk.NewStart

internal/comment/comment.go

@@ -3,7 +3,9 @@ package context
 
 import (
 	"fmt"
+	"os"
 	"os/exec"
+	"path/filepath"
 	"strconv"
 	"strings"
 )
@@ -83,6 +85,10 @@ func ChangedFiles(base string) ([]string, error) {
 
 // Blame runs git blame on a specific line range and returns a summary.
 func Blame(file string, startLine, endLine int) (string, error) {
+	if err := validateFilePath(file); err != nil {
+		return "", fmt.Errorf("git blame: invalid path: %w", err)
+	}
+
 	args := []string{"blame", "--line-porcelain",
 		"-L", strconv.Itoa(startLine) + "," + strconv.Itoa(endLine),
 		"--", file}
@@ -95,7 +101,33 @@ func Blame(file string, startLine, endLine int) (string, error) {
 	return parseBlameAuthors(string(out)), nil
 }
 
+// validateFilePath ensures a file path does not traverse outside the working directory.
+func validateFilePath(file string) error {
+	if file == "" {
+		return fmt.Errorf("empty path")
+	}
+	wd, err := os.Getwd()
+	if err != nil {
+		return fmt.Errorf("cannot determine working directory: %w", err)
+	}
+	absPath := file
+	if !filepath.IsAbs(file) {
+		absPath = filepath.Join(wd, file)
+	}
+	rel, err := filepath.Rel(wd, absPath)
+	if err != nil {
+		return fmt.Errorf("cannot resolve relative path: %w", err)
+	}
+	if strings.HasPrefix(rel, "..") {
+		return fmt.Errorf("path %q traverses outside working directory", file)
+	}
+	return nil
+}
+
 func recentCommits(file string, n int) ([]string, error) {
+	if err := validateFilePath(file); err != nil {
+		return nil, fmt.Errorf("recentCommits: invalid path: %w", err)
+	}
 	out, err := exec.Command("git", "log", "--oneline", "-n", strconv.Itoa(n), "--", file).Output()
 	if err != nil {
 		return nil, err

internal/context/git.go

@@ -107,6 +107,24 @@ func parseFileChunk(chunk string) File {
 	lines := strings.Split(chunk, "\n")
 	file := File{}
 
+	// Detect binary files — git outputs "Binary files ... differ" for them
+	for _, line := range lines {
+		if strings.HasPrefix(line, "Binary files") && strings.Contains(line, "differ") {
+			// Binary file: extract path but skip hunk parsing
+			for _, l := range lines {
+				if strings.HasPrefix(l, "+++ b/") {
+					file.Path = strings.TrimPrefix(l, "+++ b/")
+				} else if strings.HasPrefix(l, "--- a/") {
+					file.OldPath = strings.TrimPrefix(l, "--- a/")
+				}
+			}
+			if file.Path == "" && file.OldPath != "" {
+				file.Path = file.OldPath
+			}
+			return file
+		}
+	}
+
 	for i, line := range lines {
 		switch {
 		case strings.HasPrefix(line, "--- a/"):
@@ -124,6 +142,10 @@ func parseFileChunk(chunk string) File {
 			file.Path = strings.TrimPrefix(line, "rename to ")
 		case strings.HasPrefix(line, "@@"):
 			hunk := parseHunkHeader(line)
+			// Validate line number bounds before parsing
+			if hunk.NewStart < 0 || hunk.OldStart < 0 || hunk.NewCount < 0 || hunk.OldCount < 0 {
+				continue
+			}
 			hunk.Lines = parseHunkLines(lines[i+1:])
 			file.Hunks = append(file.Hunks, hunk)
 		}

internal/diff/diff.go

@@ -90,18 +90,45 @@ var cweDatabase = []CWEMapping{
 
 // MatchCWE checks a finding's message (and fix) against the CWE database and
 // returns the CWE ID if a match is found. Returns empty string if no match.
+// Uses word boundary checks to avoid false positives from substring matching.
 func MatchCWE(message, fix string) string {
 	lower := strings.ToLower(message + " " + fix)
 	for _, cwe := range cweDatabase {
 		for _, keyword := range cwe.Keywords {
-			if strings.Contains(lower, keyword) {
+			if containsWordBoundary(lower, keyword) {
 				return cwe.ID
 			}
 		}
 	}
 	return ""
 }
 
+// containsWordBoundary checks if text contains keyword at a word boundary.
+// A word boundary is the start/end of string or a non-alphanumeric character.
+func containsWordBoundary(text, keyword string) bool {
+	idx := 0
+	for {
+		pos := strings.Index(text[idx:], keyword)
+		if pos < 0 {
+			return false
+		}
+		pos += idx
+		// Check left boundary
+		leftOK := pos == 0 || !isWordChar(text[pos-1])
+		// Check right boundary
+		end := pos + len(keyword)
+		rightOK := end >= len(text) || !isWordChar(text[end])
+		if leftOK && rightOK {
+			return true
+		}
+		idx = pos + 1
+	}
+}
+
+func isWordChar(c byte) bool {
+	return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9')
+}
+
 // LookupCWEName returns the human-readable name for a CWE ID.
 func LookupCWEName(id string) string {
 	for _, cwe := range cweDatabase {

internal/review/cwe.go

@@ -43,7 +43,13 @@ Adjust severity when:
 - A "critical" finding is actually just a code smell → downgrade
 - A "low" finding is actually exploitable → upgrade`
 
+// maxReflectPromptSize is the maximum allowed size for a reflection prompt.
+// This prevents oversized output from consuming excessive tokens.
+const maxReflectPromptSize = 100_000
+
 // BuildReflectPrompt constructs the prompt for self-reflection.
+// Returns an empty string if the resulting prompt exceeds maxReflectPromptSize,
+// indicating reflection should be skipped.
 func BuildReflectPrompt(findings []Finding, diffContext string) string {
 	var b strings.Builder
 	b.WriteString("Review these findings for accuracy. The original diff context is provided below.\n\n")
@@ -66,7 +72,13 @@ func BuildReflectPrompt(findings []Finding, diffContext string) string {
 	}
 	b.WriteString("```\n")
 
-	return b.String()
+	result := b.String()
+	// Reject if the prompt exceeds the maximum allowed size
+	if len(result) > maxReflectPromptSize {
+		return ""
+	}
+
+	return result
 }
 
 // ReflectResult holds the LLM's validation of a finding.