From 558a82274f0638e336afb929c75ada77dd0d1148 Mon Sep 17 00:00:00 2001 From: Yiftach Cohen Date: Wed, 18 Feb 2026 14:05:28 +0200 Subject: [PATCH 1/9] [PPSC-437] feat(output): add defense-in-depth secret masking for human and JSON output - Add maskSecretInMultiLineString masking to human-readable output before display - Implement maskFixForDisplay to mask secrets in Fix patches and proposed fixes - Add maskScanResultForOutput for JSON formatter to mask secrets before serialization - Expand secret pattern detection with 22 regex patterns for well-known API prefixes - Add Bearer token and dict literal detection for improved secret catching - Comprehensive test coverage for all masking scenarios (existing content, multi-line, all fix fields) - Defense-in-depth approach: mask at output layer even if upstream already masked --- internal/output/human.go | 33 +++ internal/output/human_test.go | 129 +++++++++++ internal/output/json.go | 73 +++++- internal/output/json_test.go | 269 ++++++++++++++++++++++ internal/util/mask.go | 41 +++- internal/util/mask_test.go | 404 ++++++++++++++++++++++++++++++++++ 6 files changed, 945 insertions(+), 4 deletions(-) diff --git a/internal/output/human.go b/internal/output/human.go index 2475c08..00024dd 100644 --- a/internal/output/human.go +++ b/internal/output/human.go @@ -889,6 +889,12 @@ func renderFinding(w io.Writer, finding model.Finding, opts FormatOptions) { } } + // Defense-in-depth: always mask secrets in code snippets before display, + // even if upstream already masked (masking is idempotent on masked content) + if finding.CodeSnippet != "" { + finding.CodeSnippet = util.MaskSecretInMultiLineString(finding.CodeSnippet) + } + // Code snippet with framed box if finding.CodeSnippet != "" { _, _ = fmt.Fprintf(w, "\n") @@ -1180,12 +1186,39 @@ func wrapTitle(title string, maxWidth, indent int) string { return result.String() } +// maskFixForDisplay creates a copy of Fix with secrets masked in code fields. +// This provides defense-in-depth against secret leakage through proposed fixes and patches. +func maskFixForDisplay(fix *model.Fix) *model.Fix { + fixCopy := *fix + + // Mask Patch (unified diff, multi-line) + if fixCopy.Patch != nil && *fixCopy.Patch != "" { + masked := util.MaskSecretInMultiLineString(*fixCopy.Patch) + fixCopy.Patch = &masked + } + + // Mask ProposedFixes content + if len(fixCopy.ProposedFixes) > 0 { + maskedFixes := make([]model.CodeSnippetFix, len(fixCopy.ProposedFixes)) + for i, pf := range fixCopy.ProposedFixes { + maskedFixes[i] = pf + maskedFixes[i].Content = util.MaskSecretInMultiLineString(pf.Content) + } + fixCopy.ProposedFixes = maskedFixes + } + + return &fixCopy +} + // formatFixSection formats the proposed fix section for display. func formatFixSection(fix *model.Fix) string { if fix == nil { return "" } + // Defense-in-depth: mask secrets in code-containing fields before display + fix = maskFixForDisplay(fix) + s := GetStyles() var sb strings.Builder diff --git a/internal/output/human_test.go b/internal/output/human_test.go index a8b4d36..349c243 100644 --- a/internal/output/human_test.go +++ b/internal/output/human_test.go @@ -881,3 +881,132 @@ func TestFormatCodeSnippetWithFrame_RedactedSnippet(t *testing.T) { }) } } + +func TestRenderFinding_MasksSecrets(t *testing.T) { + cli.InitColors(cli.ColorModeNever) + SyncColors() + + tests := []struct { + name string + codeSnippet string + wantContains string + wantNotContain string + }{ + { + name: "masks AWS key in snippet", + codeSnippet: `aws_secret_access_key = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY"`, + wantContains: "********", + wantNotContain: "wJalrXUtnFEMI", + }, + { + name: "masks password in snippet", + codeSnippet: `password = "SuperSecretPassword123!"`, + wantContains: "********", + wantNotContain: "SuperSecretPassword123!", + }, + { + name: "already masked content preserved", + codeSnippet: `password = ********[20-40]`, + wantContains: "********", + }, + { + name: "normal code without secrets unchanged", + codeSnippet: `fmt.Println("hello world")`, + wantContains: `fmt.Println("hello world")`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buf strings.Builder + finding := model.Finding{ + ID: "test-1", + Severity: model.SeverityHigh, + Title: "Test Finding", + CodeSnippet: tt.codeSnippet, + SnippetStartLine: 1, + StartLine: 1, + EndLine: 1, + } + renderFinding(&buf, finding, FormatOptions{}) + output := buf.String() + + if tt.wantContains != "" && !strings.Contains(output, tt.wantContains) { + t.Errorf("expected output to contain %q, got:\n%s", tt.wantContains, output) + } + if tt.wantNotContain != "" && strings.Contains(output, tt.wantNotContain) { + t.Errorf("expected output NOT to contain %q, got:\n%s", tt.wantNotContain, output) + } + }) + } +} + +func TestMaskFixForDisplay(t *testing.T) { + tests := []struct { + name string + fix *model.Fix + wantNotContain string + wantContains string + }{ + { + name: "masks patch secrets", + fix: &model.Fix{ + Patch: ptrString(`- password = "OldSecret123"`), + Explanation: "Remove hardcoded password", + }, + wantNotContain: "OldSecret123", + wantContains: "********", + }, + { + name: "masks proposed fix content", + fix: &model.Fix{ + ProposedFixes: []model.CodeSnippetFix{ + { + FilePath: "config.go", + Content: `api_key = "sk-1234567890abcdefghij"`, + }, + }, + }, + wantNotContain: "sk-1234567890abcdefghij", + wantContains: "********", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + masked := maskFixForDisplay(tt.fix) + + // Check patch + if masked.Patch != nil { + if tt.wantNotContain != "" && strings.Contains(*masked.Patch, tt.wantNotContain) { + t.Errorf("expected masked patch NOT to contain %q", tt.wantNotContain) + } + if tt.wantContains != "" && !strings.Contains(*masked.Patch, tt.wantContains) { + t.Errorf("expected masked patch to contain %q", tt.wantContains) + } + } + + // Check proposed fixes + for _, pf := range masked.ProposedFixes { + if tt.wantNotContain != "" && strings.Contains(pf.Content, tt.wantNotContain) { + t.Errorf("expected masked proposed fix NOT to contain %q", tt.wantNotContain) + } + if tt.wantContains != "" && !strings.Contains(pf.Content, tt.wantContains) { + t.Errorf("expected masked proposed fix to contain %q", tt.wantContains) + } + } + + // Verify original is not modified + if tt.fix.Patch != nil && strings.Contains(*tt.fix.Patch, "********") { + // This would indicate the original was modified + if tt.wantNotContain != "" && !strings.Contains(*tt.fix.Patch, tt.wantNotContain) { + t.Error("original fix should not be modified") + } + } + }) + } +} + +func ptrString(s string) *string { + return &s +} diff --git a/internal/output/json.go b/internal/output/json.go index e3aed34..329fd25 100644 --- a/internal/output/json.go +++ b/internal/output/json.go @@ -5,6 +5,7 @@ import ( "io" "github.com/ArmisSecurity/armis-cli/internal/model" + "github.com/ArmisSecurity/armis-cli/internal/util" ) // JSONFormatter formats scan results as JSON. @@ -12,9 +13,11 @@ type JSONFormatter struct{} // Format formats the scan result as JSON. func (f *JSONFormatter) Format(result *model.ScanResult, w io.Writer) error { + // Defense-in-depth: mask secrets before JSON serialization + masked := maskScanResultForOutput(result) encoder := json.NewEncoder(w) encoder.SetIndent("", " ") - return encoder.Encode(result) + return encoder.Encode(masked) } // FormatWithOptions formats the scan result as JSON with custom options. @@ -27,6 +30,9 @@ func (f *JSONFormatter) FormatWithOptions(result *model.ScanResult, w io.Writer, // formatWithDebug outputs JSON with additional debug metadata. func (f *JSONFormatter) formatWithDebug(result *model.ScanResult, w io.Writer, opts FormatOptions) error { + // Defense-in-depth: mask secrets before JSON serialization + masked := maskScanResultForOutput(result) + type debugOutput struct { *model.ScanResult FormatOptions struct { @@ -36,7 +42,7 @@ func (f *JSONFormatter) formatWithDebug(result *model.ScanResult, w io.Writer, o } `json:"_formatOptions"` } - out := debugOutput{ScanResult: result} + out := debugOutput{ScanResult: masked} out.FormatOptions.GroupBy = opts.GroupBy out.FormatOptions.RepoPath = opts.RepoPath out.FormatOptions.Debug = opts.Debug @@ -45,3 +51,66 @@ func (f *JSONFormatter) formatWithDebug(result *model.ScanResult, w io.Writer, o encoder.SetIndent("", " ") return encoder.Encode(out) } + +// maskScanResultForOutput creates a shallow copy of ScanResult with secrets +// masked in code-containing fields. This provides defense-in-depth against +// secret leakage through JSON output. +func maskScanResultForOutput(result *model.ScanResult) *model.ScanResult { + if result == nil { + return nil + } + + // Shallow copy the ScanResult + masked := *result + + // Deep copy and mask the findings slice + if len(result.Findings) > 0 { + masked.Findings = make([]model.Finding, len(result.Findings)) + for i, f := range result.Findings { + masked.Findings[i] = maskFindingSecrets(f) + } + } + + return &masked +} + +// maskFindingSecrets returns a copy of the Finding with secrets masked in code fields. +func maskFindingSecrets(f model.Finding) model.Finding { + // Mask CodeSnippet + if f.CodeSnippet != "" { + f.CodeSnippet = util.MaskSecretInMultiLineString(f.CodeSnippet) + } + + // Mask Fix data + if f.Fix != nil { + fixCopy := *f.Fix + + if fixCopy.Patch != nil && *fixCopy.Patch != "" { + masked := util.MaskSecretInMultiLineString(*fixCopy.Patch) + fixCopy.Patch = &masked + } + + if fixCopy.VulnerableCode != nil { + vcCopy := *fixCopy.VulnerableCode + vcCopy.Content = util.MaskSecretInMultiLineString(vcCopy.Content) + fixCopy.VulnerableCode = &vcCopy + } + + if len(fixCopy.ProposedFixes) > 0 { + maskedFixes := make([]model.CodeSnippetFix, len(fixCopy.ProposedFixes)) + for i, pf := range fixCopy.ProposedFixes { + maskedFixes[i] = pf + maskedFixes[i].Content = util.MaskSecretInMultiLineString(pf.Content) + } + fixCopy.ProposedFixes = maskedFixes + } + + if len(fixCopy.PatchFiles) > 0 { + fixCopy.PatchFiles = util.MaskSecretsInStringMap(fixCopy.PatchFiles) + } + + f.Fix = &fixCopy + } + + return f +} diff --git a/internal/output/json_test.go b/internal/output/json_test.go index f3498c4..cdc621c 100644 --- a/internal/output/json_test.go +++ b/internal/output/json_test.go @@ -3,6 +3,7 @@ package output import ( "bytes" "encoding/json" + "strings" "testing" "github.com/ArmisSecurity/armis-cli/internal/model" @@ -123,3 +124,271 @@ func TestJSONFormatter_EmptyFindings(t *testing.T) { t.Errorf("Expected 0 findings, got %d", len(decoded.Findings)) } } + +func TestJSONFormatter_MasksSecrets(t *testing.T) { + formatter := &JSONFormatter{} + secretSnippet := `password = "SuperSecretPassword123!"` + + result := &model.ScanResult{ + ScanID: "test-mask", + Status: "completed", + Findings: []model.Finding{ + { + ID: "secret-1", + Type: model.FindingTypeSecret, + Severity: model.SeverityCritical, + Title: "Exposed Secret", + CodeSnippet: secretSnippet, + }, + }, + Summary: model.Summary{Total: 1}, + } + + var buf bytes.Buffer + err := formatter.Format(result, &buf) + if err != nil { + t.Fatalf("Format failed: %v", err) + } + + output := buf.String() + if strings.Contains(output, "SuperSecretPassword123!") { + t.Error("expected secret to be masked in JSON output") + } + if !strings.Contains(output, "********") { + t.Error("expected masked placeholder in JSON output") + } + + // Verify original struct is not modified + if result.Findings[0].CodeSnippet != secretSnippet { + t.Error("original ScanResult should not be modified by formatter") + } +} + +func TestJSONFormatter_MasksFixSecrets(t *testing.T) { + formatter := &JSONFormatter{} + secretPatch := `- password = "OldSecret123" ++ password = "NewSecret456"` // #nosec G101 - test data for secret masking + + patch := secretPatch + result := &model.ScanResult{ + ScanID: "test-fix-mask", + Status: "completed", + Findings: []model.Finding{ + { + ID: "fix-secret-1", + Type: model.FindingTypeSecret, + Severity: model.SeverityHigh, + Title: "Secret with Fix", + Fix: &model.Fix{ + Patch: &patch, + Explanation: "Remove hardcoded password", + }, + }, + }, + Summary: model.Summary{Total: 1}, + } + + var buf bytes.Buffer + err := formatter.Format(result, &buf) + if err != nil { + t.Fatalf("Format failed: %v", err) + } + + output := buf.String() + if strings.Contains(output, "OldSecret123") || strings.Contains(output, "NewSecret456") { + t.Error("expected secrets in Fix.Patch to be masked in JSON output") + } +} + +func TestJSONFormatter_MaskedContentPreserved(t *testing.T) { + formatter := &JSONFormatter{} + // Already masked content - masking preserves the masked markers + alreadyMasked := `password = ********[20-40]` + + result := &model.ScanResult{ + ScanID: "test-masked", + Status: "completed", + Findings: []model.Finding{ + { + ID: "masked-1", + Type: model.FindingTypeSecret, + Severity: model.SeverityMedium, + Title: "Already Masked", + CodeSnippet: alreadyMasked, + }, + }, + Summary: model.Summary{Total: 1}, + } + + var buf bytes.Buffer + err := formatter.Format(result, &buf) + if err != nil { + t.Fatalf("Format failed: %v", err) + } + + // Should still contain masked markers (may be re-masked but not corrupted) + output := buf.String() + if !strings.Contains(output, "********") { + t.Error("masked content should be preserved with masked markers") + } + // Should not contain raw secret values + if strings.Contains(output, "SuperSecret") || strings.Contains(output, "password123") { + t.Error("should not contain any raw secrets") + } +} + +func TestJSONFormatter_MasksAllFixFields(t *testing.T) { + formatter := &JSONFormatter{} + + startLine := 10 + endLine := 15 + result := &model.ScanResult{ + ScanID: "test-all-fix-fields", + Status: "completed", + Findings: []model.Finding{ + { + ID: "fix-all-1", + Type: model.FindingTypeSecret, + Severity: model.SeverityHigh, + Title: "Secret with all fix fields", + Fix: &model.Fix{ + VulnerableCode: &model.CodeSnippetFix{ + Content: `api_key = "sk-vulnerable-key-12345"`, // #nosec G101 + }, + ProposedFixes: []model.CodeSnippetFix{ + { + FilePath: "config.go", + StartLine: &startLine, + EndLine: &endLine, + Content: `secret_key = "sk-proposed-fix-secret-value"`, // #nosec G101 + }, + }, + PatchFiles: map[string]string{ + "config.go": `- api_key = "sk-patchfile-secret-789"`, // #nosec G101 + }, + }, + }, + }, + Summary: model.Summary{Total: 1}, + } + + var buf bytes.Buffer + err := formatter.Format(result, &buf) + if err != nil { + t.Fatalf("Format failed: %v", err) + } + + output := buf.String() + + // Verify all secret values are masked + secrets := []string{ + "sk-vulnerable-key-12345", + "sk-proposed-fix-secret-value", + "sk-patchfile-secret-789", + } + for _, secret := range secrets { + if strings.Contains(output, secret) { + t.Errorf("expected %q to be masked in JSON output", secret) + } + } + + // Verify masked placeholders are present + if !strings.Contains(output, "********") { + t.Error("expected masked placeholders in JSON output") + } +} + +func TestJSONFormatter_MultiLineSecrets(t *testing.T) { + formatter := &JSONFormatter{} + multiLineSnippet := `config := struct { + Password string + APIKey string +}{ + Password: "SuperSecretPass123", + APIKey: "sk-multi-line-secret-key", +}` // #nosec G101 + + result := &model.ScanResult{ + ScanID: "test-multiline", + Status: "completed", + Findings: []model.Finding{ + { + ID: "multiline-1", + Type: model.FindingTypeSecret, + Severity: model.SeverityHigh, + Title: "Multi-line secret", + CodeSnippet: multiLineSnippet, + }, + }, + Summary: model.Summary{Total: 1}, + } + + var buf bytes.Buffer + err := formatter.Format(result, &buf) + if err != nil { + t.Fatalf("Format failed: %v", err) + } + + output := buf.String() + + // Both secrets on different lines should be masked + if strings.Contains(output, "SuperSecretPass123") { + t.Error("expected Password secret to be masked") + } + if strings.Contains(output, "sk-multi-line-secret-key") { + t.Error("expected APIKey secret to be masked") + } +} + +func TestJSONFormatter_NilAndEmptyHandling(t *testing.T) { + formatter := &JSONFormatter{} + + t.Run("nil result", func(t *testing.T) { + var buf bytes.Buffer + err := formatter.Format(nil, &buf) + if err != nil { + t.Fatalf("Format failed on nil result: %v", err) + } + // Should encode nil as "null" + if !strings.Contains(buf.String(), "null") { + t.Error("expected null in output for nil result") + } + }) + + t.Run("empty findings", func(t *testing.T) { + result := &model.ScanResult{ + ScanID: "empty", + Status: "completed", + Findings: []model.Finding{}, + } + var buf bytes.Buffer + err := formatter.Format(result, &buf) + if err != nil { + t.Fatalf("Format failed: %v", err) + } + // Should succeed without panic + }) + + t.Run("finding with nil fix", func(t *testing.T) { + result := &model.ScanResult{ + ScanID: "nil-fix", + Status: "completed", + Findings: []model.Finding{ + { + ID: "no-fix", + CodeSnippet: `password = "test123456789"`, + Fix: nil, + }, + }, + } + var buf bytes.Buffer + err := formatter.Format(result, &buf) + if err != nil { + t.Fatalf("Format failed: %v", err) + } + // Should mask snippet even with nil fix + if strings.Contains(buf.String(), "test123456789") { + t.Error("expected secret to be masked even with nil fix") + } + }) +} diff --git a/internal/util/mask.go b/internal/util/mask.go index 346242a..532fb55 100644 --- a/internal/util/mask.go +++ b/internal/util/mask.go @@ -15,6 +15,29 @@ import ( // Assignment operators supported: =, :, :=, => // The regex (?::=|[:=]>?) matches := first, then falls back to :, =, =>, or :> var secretPatterns = []*regexp.Regexp{ + // Well-known secret prefixes (standalone, no assignment needed) + // These catch secrets by their identifying prefix patterns + regexp.MustCompile(`['"]?(sk[-_](?:live|test|proj)[-_][A-Za-z0-9]{20,})['"]?`), // Stripe/OpenAI secret keys (specific) + regexp.MustCompile(`['"]?(sk-[A-Za-z0-9]{20,})['"]?`), // Generic sk- tokens (OpenAI, etc.) + regexp.MustCompile(`['"]?(pk[-_](?:live|test)[-_][A-Za-z0-9]{20,})['"]?`), // Stripe publishable keys + regexp.MustCompile(`['"]?(AIzaSy[A-Za-z0-9_-]{33})['"]?`), // Google/Firebase API keys + regexp.MustCompile(`['"]?(SG\.[A-Za-z0-9_-]{22}\.[A-Za-z0-9_-]{43})['"]?`), // SendGrid API keys + regexp.MustCompile(`['"]?(xox[baprs]-[A-Za-z0-9-]{10,})['"]?`), // Slack tokens + regexp.MustCompile(`['"]?(ghp_[A-Za-z0-9]{36,})['"]?`), // GitHub PATs (new format) + regexp.MustCompile(`['"]?(gho_[A-Za-z0-9]{36,})['"]?`), // GitHub OAuth tokens + regexp.MustCompile(`['"]?(glpat-[A-Za-z0-9_-]{20,})['"]?`), // GitLab PATs + regexp.MustCompile(`['"]?(AKIA[A-Z0-9]{16})['"]?`), // AWS access key IDs + regexp.MustCompile(`['"]?(AC[a-zA-Z0-9]{32})['"]?`), // Twilio Account SIDs + regexp.MustCompile(`['"]?(token_[A-Za-z0-9]{20,})['"]?`), // token_ prefix values (e.g., token_1234567890...) + regexp.MustCompile(`['"]?(key-[a-f0-9]{32})['"]?`), // Mailgun API keys + regexp.MustCompile(`['"]?(DefaultEndpointsProtocol=https;[^'"]{20,})['"]?`), // Azure connection strings + regexp.MustCompile(`['"]?(mongodb(?:\+srv)?://[^'"]{10,})['"]?`), // MongoDB connection strings + regexp.MustCompile(`['"]?(postgresql://[^'"]{10,})['"]?`), // PostgreSQL connection strings + regexp.MustCompile(`['"]?(mysql://[^'"]{10,})['"]?`), // MySQL connection strings + regexp.MustCompile(`['"]?(https://hooks\.slack\.com/services/[A-Za-z0-9/]+)['"]?`), // Slack webhooks + regexp.MustCompile(`['"]?(https://[A-Za-z0-9]+@[A-Za-z0-9]+\.ingest\.sentry\.io/[0-9]+)['"]?`), // Sentry DSN URLs + regexp.MustCompile(`['"]?(-----BEGIN (?:RSA |EC |DSA |OPENSSH |PGP )?PRIVATE KEY-----[^'"]+-----END[^'"]+-----)['"]?`), // Private keys + // AWS credentials (most specific - matches aws_secret_access_key before generic "secret") regexp.MustCompile(`(?i)(aws[-_]?access[-_]?key[-_]?id|aws[-_]?secret[-_]?access[-_]?key)\s*(?::=|[:=]>?)\s*['"]?([A-Za-z0-9/+=]{16,})['"]?`), // Private keys (detect key content; require 16+ chars to reduce false positives) @@ -25,14 +48,18 @@ var secretPatterns = []*regexp.Regexp{ regexp.MustCompile(`(?i)(database[-_]?url|db[-_]?password|db[-_]?pass|database[-_]?password)\s*(?::=|[:=]>?)\s*['"]?([^\s'"]{8,})['"]?`), // Connection strings (require 10+ chars to reduce false positives) regexp.MustCompile(`(?i)(connection[-_]?string|conn[-_]?str)\s*(?::=|[:=]>?)\s*['"]?([^\s'"]{10,})['"]?`), - // Service-specific tokens (Slack, Discord, Stripe, etc.) - regexp.MustCompile(`(?i)(slack[-_]?token|discord[-_]?token|stripe[-_]?key|stripe[-_]?secret|twilio[-_]?auth|npm[-_]?token|pypi[-_]?token|github[-_]?token|gitlab[-_]?token)\s*(?::=|[:=]>?)\s*['"]?([A-Za-z0-9_./+=-]{10,})['"]?`), + + // Service-specific tokens - expanded list + regexp.MustCompile(`(?i)(slack[-_]?token|discord[-_]?token|stripe[-_]?(?:key|secret|api[-_]?key)|twilio[-_]?(?:auth|token|sid)|npm[-_]?token|pypi[-_]?token|github[-_]?token|gitlab[-_]?token|openai[-_]?(?:key|api[-_]?key)|azure[-_]?(?:key|connection[-_]?string)|sendgrid[-_]?(?:key|api[-_]?key)|firebase[-_]?(?:key|api[-_]?key)|mailchimp[-_]?(?:key|api[-_]?key)|mailgun[-_]?(?:key|api[-_]?key)|algolia[-_]?(?:key|api[-_]?key|app[-_]?id)|mapbox[-_]?(?:token|key)|datadog[-_]?(?:key|api[-_]?key)|pagerduty[-_]?(?:key|api[-_]?key)|newrelic[-_]?(?:key|license[-_]?key)|paypal[-_]?(?:secret|client[-_]?id)|square[-_]?token)\s*(?::=|[:=]>?)\s*['"]?([A-Za-z0-9_./+=-]{10,})['"]?`), + // API keys and tokens (require 10+ chars to avoid short non-secrets) regexp.MustCompile(`(?i)(api[-_]?key|apikey|api_token|access[-_]?token|auth[-_]?token|bearer|token)\s*(?::=|[:=]>?)\s*['"]?([A-Za-z0-9_./+=-]{10,})['"]?`), // Signing and encryption keys regexp.MustCompile(`(?i)(signing[-_]?key|encryption[-_]?key|secret[-_]?key)\s*(?::=|[:=]>?)\s*['"]?([^\s'"]{16,})['"]?`), // JWT tokens - header starts with eyJ (base64 of '{"'), payload and signature are any base64url regexp.MustCompile(`(eyJ[A-Za-z0-9_-]*\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]*)`), + // Bearer tokens in Authorization headers - "Bearer " without assignment operator + regexp.MustCompile(`['"]?Bearer\s+([A-Za-z0-9_./+=-]{20,})['"]?`), // Password patterns (require 8+ chars to reduce false positives) regexp.MustCompile(`(?i)(password|passwd|pwd|secret)\s*(?::=|[:=]>?)\s*['"]?([^\s'"]{8,})['"]?`), // Hex strings that look like secrets (32+ chars) @@ -40,6 +67,16 @@ var secretPatterns = []*regexp.Regexp{ // Generic credentials (least specific) - uses word boundaries and 10-char minimum // to reduce false positives on common variable names like 'authService' or 'credType' regexp.MustCompile(`(?i)\b(credential|cred|auth)\b\s*(?::=|[:=]>?)\s*['"]?([^\s'"]{10,})['"]?`), + + // Generic catch-all for any identifier ending in _key, _secret, _token, _auth, _password + // with object prefix (self., this., obj.) - handles self.openai_key = "..." + // Requires quoted string value to avoid matching os.Getenv() calls + regexp.MustCompile(`(?i)(?:self\.|this\.)?([a-z_][a-z0-9_]*[-_](?:key|secret|token|auth|password))\s*(?::=|[:=]>?)\s*['"]([^'"]{10,})['"]`), + + // Dict/JSON literals with quoted keys - handles {"api_key": "value"}, "password": "value", etc. + // Matches: bare keywords (password, secret, token) OR compound keys (api_key, auth_token) OR private_key_id + // Two capture groups: (1) key name, (2) value - so masking preserves the key + regexp.MustCompile(`(?i)['"](password|secret|token|auth|credential|private[-_]?key[-_]?id|[a-z_][a-z0-9_]*[-_](?:key|secret|token|auth|password|credential))['"]\s*:\s*['"]([^'"]{10,})['"]`), } // commonLiterals contains values that should not be masked even if they match a pattern. diff --git a/internal/util/mask_test.go b/internal/util/mask_test.go index cffdaa7..b97399f 100644 --- a/internal/util/mask_test.go +++ b/internal/util/mask_test.go @@ -354,3 +354,407 @@ func TestMaskSecretsInStringMap_DoesNotModifyOriginal(t *testing.T) { t.Error("MaskSecretsInStringMap modified the original map") } } + +func TestMaskSecretInLine_WellKnownPrefixes(t *testing.T) { + tests := []struct { + name string + input string + wantNotContain string + }{ + { + name: "OpenAI key prefix sk-proj", + input: `self.openai_key = "sk-proj-1234567890abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"`, + wantNotContain: "sk-proj-1234567890", + }, + { + name: "Google/Firebase API key AIzaSy", + input: `firebase_key = "AIzaSyDaGmWKa4JsXZ-HjGw7ISLn_3namBGewQe"`, + wantNotContain: "AIzaSyDaGmWKa4JsXZ", + }, + { + name: "SendGrid key SG.", + input: `SENDGRID_KEY = "SG.1234567890abcdefghijklmnopqr.stuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"`, + wantNotContain: "SG.1234567890", + }, + { + name: "Stripe publishable key pk_live", + input: `stripe_public = "pk_live_51H9X8YF2eZvKYlo2C8RzQ9mN4P3vT5aU6gW7hE8iF9jG0kH1lI2mJ3nK4oL5pM6"`, + wantNotContain: "pk_live_51H9X8YF", + }, + { + name: "Mailgun key-prefix", + input: `MAILGUN_KEY = "key-1234567890abcdefghijklmnopqrstuv"`, + wantNotContain: "key-1234567890", + }, + { + name: "Twilio Account SID AC prefix", + input: `TWILIO_SID = "AC1234567890abcdefghijklmnopqrstuv"`, + wantNotContain: "AC1234567890", + }, + { + name: "Azure connection string", + input: `azure_key = "DefaultEndpointsProtocol=https;AccountName=myaccount;AccountKey=abcdefghij"`, + wantNotContain: "DefaultEndpointsProtocol=https", + }, + { + name: "Slack webhook URL", + input: `SLACK_WEBHOOK = "https://example-slack-webhook.invalid/services/T00000000/B00000000/XXXXXXXXXXXXXXXXXXXXXXXX"`, + wantNotContain: "example-slack-webhook", + }, + { + name: "MongoDB connection string", + input: `connection_string = "mongodb://admin:password123@production-db.example.com:27017/mydb"`, + wantNotContain: "mongodb://admin:password", + }, + { + name: "PostgreSQL connection string", + input: `postgres_conn = "postgresql://dbuser:SuperSecret123@db.example.com:5432/production_db"`, + wantNotContain: "postgresql://dbuser:SuperSecret", + }, + { + name: "Sentry DSN URL", + input: `"dsn": "https://1234567890abcdef@o123456.ingest.sentry.io/1234567"`, // #nosec G101 + wantNotContain: "1234567890abcdef@", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := util.MaskSecretInLine(tt.input) + if strings.Contains(result, tt.wantNotContain) { + t.Errorf("MaskSecretInLine() = %q, should NOT contain %q", result, tt.wantNotContain) + } + // Verify masked placeholder is present + if !strings.Contains(result, "********") { + t.Errorf("MaskSecretInLine() = %q, expected masked placeholder", result) + } + }) + } +} + +func TestMaskSecretInLine_ServiceSpecificPatterns(t *testing.T) { + tests := []struct { + name string + input string + wantNotContain string + }{ + { + name: "self.openai_key with object prefix", + input: `self.openai_key = "sk-secret-1234567890abcdef"`, + wantNotContain: "sk-secret-1234", + }, + { + name: "sendgrid_key assignment", + input: `sendgrid_key = "SG.abcdefghijklmnopqrstuvwx.1234567890ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456"`, + wantNotContain: "SG.abcdefgh", + }, + { + name: "firebase_api_key", + input: `firebase_api_key = "AIzaSyDaGmWKa4JsXZ-HjGw7ISLn_3namBGewQe"`, + wantNotContain: "AIzaSyDaGmW", + }, + { + name: "mailchimp_key", + input: `mailchimp_key = "1234567890abcdefghijklmnopqrstuv-us1"`, + wantNotContain: "1234567890abcdef", + }, + { + name: "algolia_api_key", + input: `algolia_api_key = "1234567890abcdefghijklmnopqrstuv"`, + wantNotContain: "1234567890abc", + }, + { + name: "datadog_api_key", + input: `datadog_api_key = "1234567890abcdefghijklmnopqrstuv"`, + wantNotContain: "1234567890abc", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := util.MaskSecretInLine(tt.input) + if strings.Contains(result, tt.wantNotContain) { + t.Errorf("MaskSecretInLine() = %q, should NOT contain %q", result, tt.wantNotContain) + } + }) + } +} + +func TestMaskSecretInLine_DictLiterals(t *testing.T) { + tests := []struct { + name string + input string + wantNotContain string + wantContains string + }{ + { + name: "quoted key dict literal", + input: `"api_key": "sk-1234567890abcdefghijklmnopqrstuvwxyz"`, + wantNotContain: "sk-1234567890abc", + wantContains: `"api_key"`, + }, + { + name: "JSON-style secret token", + input: `"auth_token": "ghp_1234567890abcdefghijklmnopqrstuvwxyzAB"`, + wantNotContain: "ghp_1234567890", + wantContains: `"auth_token"`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := util.MaskSecretInLine(tt.input) + if strings.Contains(result, tt.wantNotContain) { + t.Errorf("MaskSecretInLine() = %q, should NOT contain %q", result, tt.wantNotContain) + } + if tt.wantContains != "" && !strings.Contains(result, tt.wantContains) { + t.Errorf("MaskSecretInLine() = %q, should contain %q", result, tt.wantContains) + } + }) + } +} + +func TestMaskSecretInLine_DoesNotMaskEnvVarCalls(t *testing.T) { + // Note: The 10+ character minimum for values means short function names + // like os.Getenv (9 chars) won't be masked, but longer ones like + // os.environ.get (14 chars) may be masked as false positives. + // This is an acceptable trade-off for catching unquoted secrets. + tests := []struct { + name string + input string + wantContain string + }{ + { + name: "os.Getenv call preserved (under 10 chars)", + input: `api_key = os.Getenv("API_KEY")`, + wantContain: `os.Getenv("API_KEY")`, + }, + { + name: "getenv call preserved", + input: `key = getenv("SECRET")`, + wantContain: `getenv("SECRET")`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := util.MaskSecretInLine(tt.input) + if !strings.Contains(result, tt.wantContain) { + t.Errorf("MaskSecretInLine() = %q, should contain %q (env var call should not be masked)", result, tt.wantContain) + } + }) + } +} + +// TestMaskSecretInLine_TwilioSIDAlphanumeric verifies that Twilio Account SIDs +// with alphanumeric characters (not just hex) are properly masked. +func TestMaskSecretInLine_TwilioSIDAlphanumeric(t *testing.T) { + tests := []struct { + name string + input string + wantNotContain string + }{ + { + name: "Twilio SID with non-hex letters (g-z)", + input: `account_sid = "AC1234567890abcdefghijklmnopqrstuv"`, // #nosec G101 + wantNotContain: "AC1234567890", + }, + { + name: "Twilio SID uppercase letters beyond F", + input: `TWILIO_SID = "ACabcdefGHIJKLMNOPQRSTUVWXYZ012345"`, // #nosec G101 + wantNotContain: "ACabcdefGHIJKL", + }, + { + name: "Twilio SID in JSON", + input: `"accountSid": "AC0123456789ABCDEFghijklmnopqrstuv"`, // #nosec G101 + wantNotContain: "AC0123456789", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := util.MaskSecretInLine(tt.input) + if strings.Contains(result, tt.wantNotContain) { + t.Errorf("MaskSecretInLine() = %q, should NOT contain %q", result, tt.wantNotContain) + } + if !strings.Contains(result, "********") { + t.Errorf("MaskSecretInLine() = %q, expected masked placeholder", result) + } + }) + } +} + +// TestMaskSecretInLine_GenericSkToken verifies that sk- tokens without +// the specific live/test/proj suffix are still masked. +func TestMaskSecretInLine_GenericSkToken(t *testing.T) { + tests := []struct { + name string + input string + wantNotContain string + }{ + { + name: "generic sk- token (no suffix)", + input: `key = "sk-1234567890abcdefghijklmnopqrstuvwxyz"`, // #nosec G101 + wantNotContain: "sk-1234567890", + }, + { + name: "OpenAI token without proj/live/test", + input: `OPENAI_KEY = "sk-abcdefghijklmnopqrstuvwxyz012345"`, // #nosec G101 + wantNotContain: "sk-abcdefghij", + }, + { + name: "sk- token in quoted value", + input: `"apiKey": "sk-ABCDEFGHIJKLMNOPQRSTUVWXYZ012345"`, // #nosec G101 + wantNotContain: "sk-ABCDEFGHIJ", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := util.MaskSecretInLine(tt.input) + if strings.Contains(result, tt.wantNotContain) { + t.Errorf("MaskSecretInLine() = %q, should NOT contain %q", result, tt.wantNotContain) + } + if !strings.Contains(result, "********") { + t.Errorf("MaskSecretInLine() = %q, expected masked placeholder", result) + } + }) + } +} + +// TestMaskSecretInLine_BearerToken verifies that Bearer tokens in +// Authorization headers are masked (without assignment operator). +func TestMaskSecretInLine_BearerToken(t *testing.T) { + tests := []struct { + name string + input string + wantNotContain string + wantContains string + }{ + { + name: "Bearer token in header string", + input: `"Bearer sk-1234567890abcdefghijklmnopqrstuvwxyz"`, // #nosec G101 + wantNotContain: "sk-1234567890", + }, + { + name: "Bearer with single quotes", + input: `'Bearer ghp_1234567890abcdefghijklmnopqrstuvwxyzABCD'`, // #nosec G101 + wantNotContain: "ghp_1234567890", + }, + { + name: "Authorization header value", + input: `Authorization: "Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.test"`, // #nosec G101 + wantNotContain: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9", + wantContains: "Authorization", + }, + { + name: "Bearer without quotes", + input: `Bearer sk-1234567890abcdefghijklmnopqrstuvwxyz`, // #nosec G101 + wantNotContain: "sk-1234567890", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := util.MaskSecretInLine(tt.input) + if strings.Contains(result, tt.wantNotContain) { + t.Errorf("MaskSecretInLine() = %q, should NOT contain %q", result, tt.wantNotContain) + } + if tt.wantContains != "" && !strings.Contains(result, tt.wantContains) { + t.Errorf("MaskSecretInLine() = %q, should contain %q", result, tt.wantContains) + } + if !strings.Contains(result, "********") { + t.Errorf("MaskSecretInLine() = %q, expected masked placeholder", result) + } + }) + } +} + +// TestMaskSecretInLine_TokenPrefixValues verifies that values starting with +// token_ followed by alphanumeric are masked. +func TestMaskSecretInLine_TokenPrefixValues(t *testing.T) { + tests := []struct { + name string + input string + wantNotContain string + }{ + { + name: "X-Auth-Token header value", + input: `"X-Auth-Token": "token_1234567890abcdefghijklmnopqrstuvwxyz"`, // #nosec G101 + wantNotContain: "token_1234567890", + }, + { + name: "token_ value without quotes", + input: `auth = token_abcdefghijklmnopqrstuvwxyz012345`, // #nosec G101 + wantNotContain: "token_abcdefghij", + }, + { + name: "token_ in assignment", + input: `my_token = "token_ABCDEFGHIJKLMNOPQRSTUVWXYZ"`, // #nosec G101 + wantNotContain: "token_ABCDEFGHIJ", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := util.MaskSecretInLine(tt.input) + if strings.Contains(result, tt.wantNotContain) { + t.Errorf("MaskSecretInLine() = %q, should NOT contain %q", result, tt.wantNotContain) + } + if !strings.Contains(result, "********") { + t.Errorf("MaskSecretInLine() = %q, expected masked placeholder", result) + } + }) + } +} + +// TestMaskSecretInLine_BareKeywordDictLiterals verifies that dict literals with +// bare keywords like "password" (not "db_password") are masked. +func TestMaskSecretInLine_BareKeywordDictLiterals(t *testing.T) { + tests := []struct { + name string + input string + wantNotContain string + wantContains string + }{ + { + name: "bare password key", + input: `"password": "SuperSecretPassword123!"`, // #nosec G101 + wantNotContain: "SuperSecretPassword", + wantContains: `"password"`, + }, + { + name: "bare secret key", + input: `"secret": "my_super_secret_value_here"`, // #nosec G101 + wantNotContain: "my_super_secret", + wantContains: `"secret"`, + }, + { + name: "bare token key", + input: `"token": "abcdefghijklmnopqrstuvwxyz"`, // #nosec G101 + wantNotContain: "abcdefghijklmnop", + wantContains: `"token"`, + }, + { + name: "private_key_id field", + input: `"private_key_id": "1234567890abcdef1234567890abcdef12345678"`, // #nosec G101 + wantNotContain: "1234567890abcdef", + wantContains: `"private_key_id"`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := util.MaskSecretInLine(tt.input) + if strings.Contains(result, tt.wantNotContain) { + t.Errorf("MaskSecretInLine() = %q, should NOT contain %q", result, tt.wantNotContain) + } + if tt.wantContains != "" && !strings.Contains(result, tt.wantContains) { + t.Errorf("MaskSecretInLine() = %q, should contain %q", result, tt.wantContains) + } + if !strings.Contains(result, "********") { + t.Errorf("MaskSecretInLine() = %q, expected masked placeholder", result) + } + }) + } +} From 7a5fa1cb9f07b31496efc80b67bf7b98a52a29f0 Mon Sep 17 00:00:00 2001 From: Yiftach Cohen Date: Wed, 18 Feb 2026 14:31:07 +0200 Subject: [PATCH 2/9] [PPSC-437] fix(util): preserve quote structure in secret masking and fix CI failures - Fix well-known prefix patterns to preserve surrounding quotes when masking by using submatches[1] replacement instead of masking entire match - Fix Slack webhook test URL to use correct hooks.slack.com domain - Add #nosec G101 comments to test fixtures (false positive warnings) - Replace WriteString(fmt.Sprintf(...)) with fmt.Fprintf for efficiency - Add regression test for quote preservation --- internal/output/human.go | 6 +-- internal/util/mask.go | 13 +++++-- internal/util/mask_test.go | 80 +++++++++++++++++++++++++++++++------- 3 files changed, 79 insertions(+), 20 deletions(-) diff --git a/internal/output/human.go b/internal/output/human.go index 00024dd..4bff47c 100644 --- a/internal/output/human.go +++ b/internal/output/human.go @@ -728,11 +728,11 @@ func renderSummaryDashboard(w io.Writer, result *model.ScanResult) error { content.WriteString("SCAN COMPLETE\n") // Total findings - simple and prominent - content.WriteString(fmt.Sprintf("%d findings", result.Summary.Total)) + fmt.Fprintf(&content, "%d findings", result.Summary.Total) // Duration if available (inline) if duration := scanDuration(result); duration != "" { - content.WriteString(fmt.Sprintf(" • %s", duration)) + fmt.Fprintf(&content, " • %s", duration) } content.WriteString("\n") @@ -789,7 +789,7 @@ func renderSummaryDashboard(w io.Writer, result *model.ScanResult) error { catParts = append(catParts, fmt.Sprintf("%s (%d)", util.FormatCategory(cc.category), cc.count)) } catLabel := s.MutedText.Render("Categories:") - content.WriteString(fmt.Sprintf("%s %s\n", catLabel, strings.Join(catParts, ", "))) + fmt.Fprintf(&content, "%s %s\n", catLabel, strings.Join(catParts, ", ")) } // Render the summary box using predefined style diff --git a/internal/util/mask.go b/internal/util/mask.go index 532fb55..f7c8f17 100644 --- a/internal/util/mask.go +++ b/internal/util/mask.go @@ -103,7 +103,7 @@ func MaskSecretInLine(line string) string { return match } - // For patterns with prefix + value, mask just the value + // For patterns with prefix + value (3+ capture groups), mask just the value (submatches[2]) if len(submatches) >= 3 { value := submatches[2] // Skip common literals that aren't secrets @@ -114,8 +114,15 @@ func MaskSecretInLine(line string) string { return strings.Replace(match, value, masked, 1) } - // For patterns without prefix (like JWT), mask the whole match - return maskValue(match) + // For patterns with a single capture group (well-known prefixes, JWT, Bearer, etc.), + // mask the captured value and replace it in the match to preserve surrounding structure + // (quotes, "Bearer " prefix, etc.) + value := submatches[1] + if commonLiterals[strings.ToLower(value)] { + return match + } + masked := maskValue(value) + return strings.Replace(match, value, masked, 1) }) } diff --git a/internal/util/mask_test.go b/internal/util/mask_test.go index b97399f..a092f29 100644 --- a/internal/util/mask_test.go +++ b/internal/util/mask_test.go @@ -28,13 +28,13 @@ func TestMaskSecretInLine(t *testing.T) { }, { name: "AWS access key", - input: `AWS_ACCESS_KEY_ID=AKIAIOSFODNN7EXAMPLE`, + input: `AWS_ACCESS_KEY_ID=AKIAIOSFODNN7EXAMPLE`, // #nosec G101 wantContains: "AWS_ACCESS_KEY_ID", wantNotContain: "IOSFODNN7", }, { name: "token in config", - input: `auth_token: "ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"`, + input: `auth_token: "ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"`, // #nosec G101 wantContains: "auth_token", wantNotContain: "xxxxxxxxxxx", }, @@ -64,7 +64,7 @@ func TestMaskSecretInLine(t *testing.T) { }, { name: "secret hex hash", - input: `secret_hash = "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2"`, + input: `secret_hash = "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2"`, // #nosec G101 wantContains: "secret_hash", wantNotContain: "e5f6a1b2c3d4e5f6a1b2c3d4", }, @@ -179,12 +179,12 @@ func TestMaskSecretInLine_NoPrefixLeakage(t *testing.T) { }, { name: "GitHub token prefix ghp_ must not leak", - input: `auth_token = "ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"`, + input: `auth_token = "ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"`, // #nosec G101 forbiddenPrefixes: []string{"ghp_"}, }, { name: "AWS key prefix AKIA must not leak", - input: `AWS_ACCESS_KEY_ID=AKIAIOSFODNN7EXAMPLE`, + input: `AWS_ACCESS_KEY_ID=AKIAIOSFODNN7EXAMPLE`, // #nosec G101 forbiddenPrefixes: []string{"AKIA"}, }, { @@ -368,7 +368,7 @@ func TestMaskSecretInLine_WellKnownPrefixes(t *testing.T) { }, { name: "Google/Firebase API key AIzaSy", - input: `firebase_key = "AIzaSyDaGmWKa4JsXZ-HjGw7ISLn_3namBGewQe"`, + input: `firebase_key = "AIzaSyDaGmWKa4JsXZ-HjGw7ISLn_3namBGewQe"`, // #nosec G101 wantNotContain: "AIzaSyDaGmWKa4JsXZ", }, { @@ -383,7 +383,7 @@ func TestMaskSecretInLine_WellKnownPrefixes(t *testing.T) { }, { name: "Mailgun key-prefix", - input: `MAILGUN_KEY = "key-1234567890abcdefghijklmnopqrstuv"`, + input: `MAILGUN_KEY = "key-1234567890abcdefghijklmnopqrstuv"`, // #nosec G101 wantNotContain: "key-1234567890", }, { @@ -398,8 +398,8 @@ func TestMaskSecretInLine_WellKnownPrefixes(t *testing.T) { }, { name: "Slack webhook URL", - input: `SLACK_WEBHOOK = "https://example-slack-webhook.invalid/services/T00000000/B00000000/XXXXXXXXXXXXXXXXXXXXXXXX"`, - wantNotContain: "example-slack-webhook", + input: `SLACK_WEBHOOK = "https://hooks.slack.com/services/T00000000/B00000000/XXXXXXXXXXXXXXXXXXXXXXXX"`, + wantNotContain: "hooks.slack.com", }, { name: "MongoDB connection string", @@ -408,7 +408,7 @@ func TestMaskSecretInLine_WellKnownPrefixes(t *testing.T) { }, { name: "PostgreSQL connection string", - input: `postgres_conn = "postgresql://dbuser:SuperSecret123@db.example.com:5432/production_db"`, + input: `postgres_conn = "postgresql://dbuser:SuperSecret123@db.example.com:5432/production_db"`, // #nosec G101 wantNotContain: "postgresql://dbuser:SuperSecret", }, { @@ -450,7 +450,7 @@ func TestMaskSecretInLine_ServiceSpecificPatterns(t *testing.T) { }, { name: "firebase_api_key", - input: `firebase_api_key = "AIzaSyDaGmWKa4JsXZ-HjGw7ISLn_3namBGewQe"`, + input: `firebase_api_key = "AIzaSyDaGmWKa4JsXZ-HjGw7ISLn_3namBGewQe"`, // #nosec G101 wantNotContain: "AIzaSyDaGmW", }, { @@ -460,12 +460,12 @@ func TestMaskSecretInLine_ServiceSpecificPatterns(t *testing.T) { }, { name: "algolia_api_key", - input: `algolia_api_key = "1234567890abcdefghijklmnopqrstuv"`, + input: `algolia_api_key = "1234567890abcdefghijklmnopqrstuv"`, // #nosec G101 wantNotContain: "1234567890abc", }, { name: "datadog_api_key", - input: `datadog_api_key = "1234567890abcdefghijklmnopqrstuv"`, + input: `datadog_api_key = "1234567890abcdefghijklmnopqrstuv"`, // #nosec G101 wantNotContain: "1234567890abc", }, } @@ -495,7 +495,7 @@ func TestMaskSecretInLine_DictLiterals(t *testing.T) { }, { name: "JSON-style secret token", - input: `"auth_token": "ghp_1234567890abcdefghijklmnopqrstuvwxyzAB"`, + input: `"auth_token": "ghp_1234567890abcdefghijklmnopqrstuvwxyzAB"`, // #nosec G101 wantNotContain: "ghp_1234567890", wantContains: `"auth_token"`, }, @@ -758,3 +758,55 @@ func TestMaskSecretInLine_BareKeywordDictLiterals(t *testing.T) { }) } } + +// TestMaskSecretInLine_PreservesQuotes verifies that well-known prefix patterns +// preserve surrounding quote structure when masking. This is a regression test +// for a bug where patterns like ['"]?(sk-...)['"]? replaced the entire match +// (including quotes) instead of just the captured value. +func TestMaskSecretInLine_PreservesQuotes(t *testing.T) { + tests := []struct { + name string + input string + wantContain string // substring that MUST be present (to verify structure) + }{ + { + name: "sk- token preserves double quotes", + input: `api_key = "sk-1234567890abcdefghijklmnopqrstuvwxyz"`, + wantContain: `= "********`, + }, + { + name: "sk- token preserves single quotes", + input: `api_key = 'sk-1234567890abcdefghijklmnopqrstuvwxyz'`, + wantContain: `= '********`, + }, + { + name: "GitHub PAT preserves quotes", + input: `token = "ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"`, + wantContain: `= "********`, + }, + { + name: "Slack webhook preserves quotes", + input: `WEBHOOK = "https://hooks.slack.com/services/T00000000/B00000000/XXXXXXXXXXXXXXXXXXXXXXXX"`, + wantContain: `= "********`, + }, + { + name: "Bearer token preserves quotes and prefix", + input: `"Bearer sk-1234567890abcdefghijklmnopqrstuvwxyz"`, + wantContain: `"Bearer ********`, + }, + { + name: "MongoDB connection string preserves quotes", + input: `conn = "mongodb://admin:password123@db.example.com:27017/mydb"`, + wantContain: `= "********`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := util.MaskSecretInLine(tt.input) + if !strings.Contains(result, tt.wantContain) { + t.Errorf("MaskSecretInLine() = %q, should contain %q (quotes not preserved)", result, tt.wantContain) + } + }) + } +} From 98be7652fc715de647a681930cc63fb85d0257e1 Mon Sep 17 00:00:00 2001 From: Yiftach Cohen Date: Wed, 18 Feb 2026 14:34:04 +0200 Subject: [PATCH 3/9] [PPSC-437] fix(lint): add missing #nosec G101 comments and fix remaining QF1012 - Add #nosec G101 to Slack webhook and MongoDB test cases - Add #nosec G101 to quote preservation regression tests - Fix remaining WriteString(fmt.Sprintf(...)) in formatProposedSnippet --- internal/output/human.go | 6 +++--- internal/util/mask_test.go | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/output/human.go b/internal/output/human.go index 4bff47c..a1f75d1 100644 --- a/internal/output/human.go +++ b/internal/output/human.go @@ -1280,11 +1280,11 @@ func formatProposedSnippet(snippet model.CodeSnippetFix) string { s := GetStyles() var sb strings.Builder - sb.WriteString(fmt.Sprintf(" File: %s", snippet.FilePath)) + fmt.Fprintf(&sb, " File: %s", snippet.FilePath) if snippet.StartLine != nil && snippet.EndLine != nil { - sb.WriteString(fmt.Sprintf(" (lines %d-%d)", *snippet.StartLine, *snippet.EndLine)) + fmt.Fprintf(&sb, " (lines %d-%d)", *snippet.StartLine, *snippet.EndLine) } else if snippet.StartLine != nil { - sb.WriteString(fmt.Sprintf(" (line %d)", *snippet.StartLine)) + fmt.Fprintf(&sb, " (line %d)", *snippet.StartLine) } sb.WriteString("\n") diff --git a/internal/util/mask_test.go b/internal/util/mask_test.go index a092f29..bed1b53 100644 --- a/internal/util/mask_test.go +++ b/internal/util/mask_test.go @@ -398,7 +398,7 @@ func TestMaskSecretInLine_WellKnownPrefixes(t *testing.T) { }, { name: "Slack webhook URL", - input: `SLACK_WEBHOOK = "https://hooks.slack.com/services/T00000000/B00000000/XXXXXXXXXXXXXXXXXXXXXXXX"`, + input: `SLACK_WEBHOOK = "https://hooks.slack.com/services/T00000000/B00000000/XXXXXXXXXXXXXXXXXXXXXXXX"`, // #nosec G101 wantNotContain: "hooks.slack.com", }, { @@ -781,12 +781,12 @@ func TestMaskSecretInLine_PreservesQuotes(t *testing.T) { }, { name: "GitHub PAT preserves quotes", - input: `token = "ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"`, + input: `token = "ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"`, // #nosec G101 wantContain: `= "********`, }, { name: "Slack webhook preserves quotes", - input: `WEBHOOK = "https://hooks.slack.com/services/T00000000/B00000000/XXXXXXXXXXXXXXXXXXXXXXXX"`, + input: `WEBHOOK = "https://hooks.slack.com/services/T00000000/B00000000/XXXXXXXXXXXXXXXXXXXXXXXX"`, // #nosec G101 wantContain: `= "********`, }, { @@ -796,7 +796,7 @@ func TestMaskSecretInLine_PreservesQuotes(t *testing.T) { }, { name: "MongoDB connection string preserves quotes", - input: `conn = "mongodb://admin:password123@db.example.com:27017/mydb"`, + input: `conn = "mongodb://admin:password123@db.example.com:27017/mydb"`, // #nosec G101 wantContain: `= "********`, }, } From 2bba9d4f8d678d2fc51f49aac5f998ded8f1de50 Mon Sep 17 00:00:00 2001 From: Yiftach Cohen Date: Wed, 18 Feb 2026 14:47:14 +0200 Subject: [PATCH 4/9] [PPSC-437] fix: address code review comments for secret masking - Fix idempotency comment wording (already-masked content remains masked) - Add documentation about intentional pattern ordering for sk- patterns - Add VulnerableCode and PatchFiles masking to maskFixForDisplay for consistency with JSON formatter (defense-in-depth) - Use minimum lengths instead of exact for SendGrid/Google API patterns - Fix sk- pattern to make second delimiter optional (sk-proj vs sk-proj-) - Fix private key regex to use (?s) flag for multiline matching - Add TestMaskSecretInLine_PrivateKeys for RSA/EC/DSA key coverage - Add benchmark tests for performance measurement --- internal/output/human.go | 14 ++++++- internal/util/mask.go | 44 +++++++++++--------- internal/util/mask_test.go | 83 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 21 deletions(-) diff --git a/internal/output/human.go b/internal/output/human.go index a1f75d1..490acae 100644 --- a/internal/output/human.go +++ b/internal/output/human.go @@ -890,7 +890,7 @@ func renderFinding(w io.Writer, finding model.Finding, opts FormatOptions) { } // Defense-in-depth: always mask secrets in code snippets before display, - // even if upstream already masked (masking is idempotent on masked content) + // even if upstream already masked (already-masked content remains masked) if finding.CodeSnippet != "" { finding.CodeSnippet = util.MaskSecretInMultiLineString(finding.CodeSnippet) } @@ -1207,6 +1207,18 @@ func maskFixForDisplay(fix *model.Fix) *model.Fix { fixCopy.ProposedFixes = maskedFixes } + // Mask VulnerableCode content (defense-in-depth for consistency with JSON formatter) + if fixCopy.VulnerableCode != nil && fixCopy.VulnerableCode.Content != "" { + maskedVuln := *fixCopy.VulnerableCode + maskedVuln.Content = util.MaskSecretInMultiLineString(maskedVuln.Content) + fixCopy.VulnerableCode = &maskedVuln + } + + // Mask PatchFiles content (map of filename -> patch content) + if len(fixCopy.PatchFiles) > 0 { + fixCopy.PatchFiles = util.MaskSecretsInStringMap(fixCopy.PatchFiles) + } + return &fixCopy } diff --git a/internal/util/mask.go b/internal/util/mask.go index f7c8f17..7f18c1b 100644 --- a/internal/util/mask.go +++ b/internal/util/mask.go @@ -17,26 +17,30 @@ import ( var secretPatterns = []*regexp.Regexp{ // Well-known secret prefixes (standalone, no assignment needed) // These catch secrets by their identifying prefix patterns - regexp.MustCompile(`['"]?(sk[-_](?:live|test|proj)[-_][A-Za-z0-9]{20,})['"]?`), // Stripe/OpenAI secret keys (specific) - regexp.MustCompile(`['"]?(sk-[A-Za-z0-9]{20,})['"]?`), // Generic sk- tokens (OpenAI, etc.) - regexp.MustCompile(`['"]?(pk[-_](?:live|test)[-_][A-Za-z0-9]{20,})['"]?`), // Stripe publishable keys - regexp.MustCompile(`['"]?(AIzaSy[A-Za-z0-9_-]{33})['"]?`), // Google/Firebase API keys - regexp.MustCompile(`['"]?(SG\.[A-Za-z0-9_-]{22}\.[A-Za-z0-9_-]{43})['"]?`), // SendGrid API keys - regexp.MustCompile(`['"]?(xox[baprs]-[A-Za-z0-9-]{10,})['"]?`), // Slack tokens - regexp.MustCompile(`['"]?(ghp_[A-Za-z0-9]{36,})['"]?`), // GitHub PATs (new format) - regexp.MustCompile(`['"]?(gho_[A-Za-z0-9]{36,})['"]?`), // GitHub OAuth tokens - regexp.MustCompile(`['"]?(glpat-[A-Za-z0-9_-]{20,})['"]?`), // GitLab PATs - regexp.MustCompile(`['"]?(AKIA[A-Z0-9]{16})['"]?`), // AWS access key IDs - regexp.MustCompile(`['"]?(AC[a-zA-Z0-9]{32})['"]?`), // Twilio Account SIDs - regexp.MustCompile(`['"]?(token_[A-Za-z0-9]{20,})['"]?`), // token_ prefix values (e.g., token_1234567890...) - regexp.MustCompile(`['"]?(key-[a-f0-9]{32})['"]?`), // Mailgun API keys - regexp.MustCompile(`['"]?(DefaultEndpointsProtocol=https;[^'"]{20,})['"]?`), // Azure connection strings - regexp.MustCompile(`['"]?(mongodb(?:\+srv)?://[^'"]{10,})['"]?`), // MongoDB connection strings - regexp.MustCompile(`['"]?(postgresql://[^'"]{10,})['"]?`), // PostgreSQL connection strings - regexp.MustCompile(`['"]?(mysql://[^'"]{10,})['"]?`), // MySQL connection strings - regexp.MustCompile(`['"]?(https://hooks\.slack\.com/services/[A-Za-z0-9/]+)['"]?`), // Slack webhooks - regexp.MustCompile(`['"]?(https://[A-Za-z0-9]+@[A-Za-z0-9]+\.ingest\.sentry\.io/[0-9]+)['"]?`), // Sentry DSN URLs - regexp.MustCompile(`['"]?(-----BEGIN (?:RSA |EC |DSA |OPENSSH |PGP )?PRIVATE KEY-----[^'"]+-----END[^'"]+-----)['"]?`), // Private keys + // + // NOTE: Pattern ordering is intentional - more specific patterns MUST come before + // generic ones to ensure proper matching. For example, "sk[-_](?:live|test|proj)..." + // must precede "sk-..." so Stripe/OpenAI keys with suffixes are matched first. + regexp.MustCompile(`['"]?(sk[-_](?:live|test|proj)[-_]?[A-Za-z0-9]{20,})['"]?`), // Stripe/OpenAI secret keys (specific) + regexp.MustCompile(`['"]?(sk-[A-Za-z0-9]{20,})['"]?`), // Generic sk- tokens (OpenAI, etc.) + regexp.MustCompile(`['"]?(pk[-_](?:live|test)[-_][A-Za-z0-9]{20,})['"]?`), // Stripe publishable keys + regexp.MustCompile(`['"]?(AIzaSy[A-Za-z0-9_-]{30,})['"]?`), // Google/Firebase API keys (min 30 chars after prefix) + regexp.MustCompile(`['"]?(SG\.[A-Za-z0-9_-]{20,}\.[A-Za-z0-9_-]{40,})['"]?`), // SendGrid API keys (flexible lengths) + regexp.MustCompile(`['"]?(xox[baprs]-[A-Za-z0-9-]{10,})['"]?`), // Slack tokens + regexp.MustCompile(`['"]?(ghp_[A-Za-z0-9]{36,})['"]?`), // GitHub PATs (new format) + regexp.MustCompile(`['"]?(gho_[A-Za-z0-9]{36,})['"]?`), // GitHub OAuth tokens + regexp.MustCompile(`['"]?(glpat-[A-Za-z0-9_-]{20,})['"]?`), // GitLab PATs + regexp.MustCompile(`['"]?(AKIA[A-Z0-9]{16})['"]?`), // AWS access key IDs + regexp.MustCompile(`['"]?(AC[a-zA-Z0-9]{32})['"]?`), // Twilio Account SIDs + regexp.MustCompile(`['"]?(token_[A-Za-z0-9]{20,})['"]?`), // token_ prefix values (e.g., token_1234567890...) + regexp.MustCompile(`['"]?(key-[a-f0-9]{32})['"]?`), // Mailgun API keys + regexp.MustCompile(`['"]?(DefaultEndpointsProtocol=https;[^'"]{20,})['"]?`), // Azure connection strings + regexp.MustCompile(`['"]?(mongodb(?:\+srv)?://[^'"]{10,})['"]?`), // MongoDB connection strings + regexp.MustCompile(`['"]?(postgresql://[^'"]{10,})['"]?`), // PostgreSQL connection strings + regexp.MustCompile(`['"]?(mysql://[^'"]{10,})['"]?`), // MySQL connection strings + regexp.MustCompile(`['"]?(https://hooks\.slack\.com/services/[A-Za-z0-9/]+)['"]?`), // Slack webhooks + regexp.MustCompile(`['"]?(https://[A-Za-z0-9]+@[A-Za-z0-9]+\.ingest\.sentry\.io/[0-9]+)['"]?`), // Sentry DSN URLs + regexp.MustCompile(`(?s)['"]?(-----BEGIN (?:RSA |EC |DSA |OPENSSH |PGP )?PRIVATE KEY-----.*?-----END (?:RSA |EC |DSA |OPENSSH |PGP )?PRIVATE KEY-----)['"]?`), // Private keys (multiline) // AWS credentials (most specific - matches aws_secret_access_key before generic "secret") regexp.MustCompile(`(?i)(aws[-_]?access[-_]?key[-_]?id|aws[-_]?secret[-_]?access[-_]?key)\s*(?::=|[:=]>?)\s*['"]?([A-Za-z0-9/+=]{16,})['"]?`), diff --git a/internal/util/mask_test.go b/internal/util/mask_test.go index bed1b53..1050ef1 100644 --- a/internal/util/mask_test.go +++ b/internal/util/mask_test.go @@ -810,3 +810,86 @@ func TestMaskSecretInLine_PreservesQuotes(t *testing.T) { }) } } + +// TestMaskSecretInLine_PrivateKeys verifies that PEM-formatted private keys +// are properly masked, including RSA, EC, DSA, OPENSSH, and PGP variants. +func TestMaskSecretInLine_PrivateKeys(t *testing.T) { + tests := []struct { + name string + input string + wantNotContain string + }{ + { + name: "RSA private key", + input: `-----BEGIN RSA PRIVATE KEY----- +MIIEowIBAAKCAQEA0Z3VS5JJcds3xfn/ygWyf8DgnX5X9g5yjW5tNk+PNqp +-----END RSA PRIVATE KEY-----`, + wantNotContain: "MIIEowIBAAKCAQEA", + }, + { + name: "EC private key inline", + input: `key = "-----BEGIN EC PRIVATE KEY-----MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEH-----END EC PRIVATE KEY-----"`, // #nosec G101 + wantNotContain: "MIGHAgEAMBMG", + }, + { + name: "Generic private key", + input: `-----BEGIN PRIVATE KEY-----MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSk-----END PRIVATE KEY-----`, + wantNotContain: "MIIEvgIBADAN", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := util.MaskSecretInLine(tt.input) + if strings.Contains(result, tt.wantNotContain) { + t.Errorf("MaskSecretInLine() = %q, should NOT contain %q", result, tt.wantNotContain) + } + if !strings.Contains(result, "********") { + t.Errorf("MaskSecretInLine() = %q, expected masked placeholder", result) + } + }) + } +} + +// Benchmark tests to measure performance impact of secret masking patterns. +// Run with: go test -bench=. -benchmem ./internal/util/... + +func BenchmarkMaskSecretInLine_NoSecrets(b *testing.B) { + line := `func main() { fmt.Println("hello world") }` + b.ResetTimer() + for i := 0; i < b.N; i++ { + util.MaskSecretInLine(line) + } +} + +func BenchmarkMaskSecretInLine_WithSecret(b *testing.B) { + line := `api_key = "sk-1234567890abcdefghijklmnopqrstuvwxyz"` // #nosec G101 + b.ResetTimer() + for i := 0; i < b.N; i++ { + util.MaskSecretInLine(line) + } +} + +func BenchmarkMaskSecretInLine_MultipleSecrets(b *testing.B) { + line := `config = { "api_key": "sk-1234567890abcdef", "token": "ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" }` // #nosec G101 + b.ResetTimer() + for i := 0; i < b.N; i++ { + util.MaskSecretInLine(line) + } +} + +func BenchmarkMaskSecretInMultiLineString(b *testing.B) { + content := `package main + +func main() { + apiKey := "sk-1234567890abcdefghijklmnopqrstuvwxyz" + password := "supersecretpassword123" + token := "ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + fmt.Println("Hello, World!") +} +` // #nosec G101 + b.ResetTimer() + for i := 0; i < b.N; i++ { + util.MaskSecretInMultiLineString(content) + } +} From b40459c8b27c14d221bcbdb435d62b0a883b7f49 Mon Sep 17 00:00:00 2001 From: Yiftach Cohen Date: Wed, 18 Feb 2026 14:54:09 +0200 Subject: [PATCH 5/9] [PPSC-437] fix(lint): add missing #nosec G101 and fix QF1012 issues - Add #nosec G101 to RSA private key test fixture - Convert WriteString(fmt.Sprintf(...)) to fmt.Fprintf for efficiency --- internal/output/human.go | 4 ++-- internal/util/mask_test.go | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/output/human.go b/internal/output/human.go index 490acae..9676e74 100644 --- a/internal/output/human.go +++ b/internal/output/human.go @@ -1309,7 +1309,7 @@ func formatProposedSnippet(snippet model.CodeSnippetFix) string { } for i, line := range highlightedLines { lineNum := s.ProposedLineNumber.Render(fmt.Sprintf("%4d", startLine+i)) - sb.WriteString(fmt.Sprintf(" %s %s\n", lineNum, line)) + fmt.Fprintf(&sb, " %s %s\n", lineNum, line) } return sb.String() @@ -1761,7 +1761,7 @@ func formatDiffWithColorsStyled(patch string) string { case "context": // Handle ellipsis marker (omitted lines indicator) if op.Line.OldNum == -1 && op.Line.NewNum == -1 { - sb.WriteString(fmt.Sprintf(" %s\n", s.MutedText.Render(op.Line.Content))) + fmt.Fprintf(&sb, " %s\n", s.MutedText.Render(op.Line.Content)) afterHunk = false emptyCount = 0 continue diff --git a/internal/util/mask_test.go b/internal/util/mask_test.go index 1050ef1..83d3f21 100644 --- a/internal/util/mask_test.go +++ b/internal/util/mask_test.go @@ -819,6 +819,7 @@ func TestMaskSecretInLine_PrivateKeys(t *testing.T) { input string wantNotContain string }{ + // #nosec G101 -- test fixture, not a real credential { name: "RSA private key", input: `-----BEGIN RSA PRIVATE KEY----- From f5a0174eec4638515c3f0ef14b410ee27c9f86a4 Mon Sep 17 00:00:00 2001 From: Yiftach Cohen Date: Wed, 18 Feb 2026 15:24:51 +0200 Subject: [PATCH 6/9] [PPSC-437] fix(security): address PR review comments and CWE-770 findings - Add maxLineLength (10KB) limit in MaskSecretInLine to prevent ReDoS - Add maxTokenizeLength (10KB) limit in tokenizeLine to prevent unbounded memory - Update DB connection string patterns to require credentials (user:pass@) - Split service-specific tokens regex into 8 smaller patterns for performance - Change strings.Replace to strings.ReplaceAll for complete secret masking - Use specific char class for PEM private keys to prevent backtracking - Use local copy in renderFinding to avoid struct mutation - Add documentation comments for pattern limitations (quotes, false positives) - Add test for maxLineLength ReDoS protection --- internal/output/human.go | 25 ++++++++++++--- internal/util/mask.go | 44 ++++++++++++++++++++------- internal/util/mask_test.go | 62 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 16 deletions(-) diff --git a/internal/output/human.go b/internal/output/human.go index 9676e74..0f079e6 100644 --- a/internal/output/human.go +++ b/internal/output/human.go @@ -890,15 +890,19 @@ func renderFinding(w io.Writer, finding model.Finding, opts FormatOptions) { } // Defense-in-depth: always mask secrets in code snippets before display, - // even if upstream already masked (already-masked content remains masked) - if finding.CodeSnippet != "" { - finding.CodeSnippet = util.MaskSecretInMultiLineString(finding.CodeSnippet) + // even if upstream already masked. Already-masked content (e.g., "********[20-40]") + // remains safely masked, though the exact format may change on re-processing. + // Create a local copy of the finding to avoid modifying the caller's struct, + // since formatCodeSnippetWithFrame reads from finding.CodeSnippet directly. + maskedFinding := finding + if maskedFinding.CodeSnippet != "" { + maskedFinding.CodeSnippet = util.MaskSecretInMultiLineString(maskedFinding.CodeSnippet) } // Code snippet with framed box - if finding.CodeSnippet != "" { + if maskedFinding.CodeSnippet != "" { _, _ = fmt.Fprintf(w, "\n") - _, _ = fmt.Fprintf(w, "%s\n", formatCodeSnippetWithFrame(finding)) + _, _ = fmt.Fprintf(w, "%s\n", formatCodeSnippetWithFrame(maskedFinding)) } // Display proposed fix if available @@ -1694,7 +1698,18 @@ func buildTokenPositions(tokens []string) []int { } // tokenizeLine splits a line into word-like tokens preserving positions +// maxTokenizeLength is the maximum line length that will be tokenized. +// Lines exceeding this limit return a single-element slice to prevent +// unbounded memory allocation (CWE-770) from attacker-controlled input. +const maxTokenizeLength = 10 * 1024 + func tokenizeLine(s string) []string { + // Defense against unbounded memory allocation (CWE-770): + // return early for extremely long lines to prevent slice growth + if len(s) > maxTokenizeLength { + return []string{s} + } + var tokens []string var current strings.Builder diff --git a/internal/util/mask.go b/internal/util/mask.go index 7f18c1b..df6473b 100644 --- a/internal/util/mask.go +++ b/internal/util/mask.go @@ -31,16 +31,16 @@ var secretPatterns = []*regexp.Regexp{ regexp.MustCompile(`['"]?(gho_[A-Za-z0-9]{36,})['"]?`), // GitHub OAuth tokens regexp.MustCompile(`['"]?(glpat-[A-Za-z0-9_-]{20,})['"]?`), // GitLab PATs regexp.MustCompile(`['"]?(AKIA[A-Z0-9]{16})['"]?`), // AWS access key IDs - regexp.MustCompile(`['"]?(AC[a-zA-Z0-9]{32})['"]?`), // Twilio Account SIDs + regexp.MustCompile(`['"]?(AC[a-zA-Z0-9]{32})['"]?`), // Twilio Account SIDs (AC prefix + 32 chars; may match non-secrets in rare cases) regexp.MustCompile(`['"]?(token_[A-Za-z0-9]{20,})['"]?`), // token_ prefix values (e.g., token_1234567890...) regexp.MustCompile(`['"]?(key-[a-f0-9]{32})['"]?`), // Mailgun API keys - regexp.MustCompile(`['"]?(DefaultEndpointsProtocol=https;[^'"]{20,})['"]?`), // Azure connection strings - regexp.MustCompile(`['"]?(mongodb(?:\+srv)?://[^'"]{10,})['"]?`), // MongoDB connection strings - regexp.MustCompile(`['"]?(postgresql://[^'"]{10,})['"]?`), // PostgreSQL connection strings - regexp.MustCompile(`['"]?(mysql://[^'"]{10,})['"]?`), // MySQL connection strings + regexp.MustCompile(`['"]?(DefaultEndpointsProtocol=https;[^'"]{20,})['"]?`), // Azure connection strings (note: stops at quotes; passwords with quotes are partially matched) + regexp.MustCompile(`['"]?(mongodb(?:\+srv)?://[^:'"@]+:[^@'"]+@[^'"]{5,})['"]?`), // MongoDB connection strings with credentials (stops at quotes) + regexp.MustCompile(`['"]?(postgresql://[^:'"@]+:[^@'"]+@[^'"]{5,})['"]?`), // PostgreSQL connection strings with credentials (stops at quotes) + regexp.MustCompile(`['"]?(mysql://[^:'"@]+:[^@'"]+@[^'"]{5,})['"]?`), // MySQL connection strings with credentials (stops at quotes) regexp.MustCompile(`['"]?(https://hooks\.slack\.com/services/[A-Za-z0-9/]+)['"]?`), // Slack webhooks regexp.MustCompile(`['"]?(https://[A-Za-z0-9]+@[A-Za-z0-9]+\.ingest\.sentry\.io/[0-9]+)['"]?`), // Sentry DSN URLs - regexp.MustCompile(`(?s)['"]?(-----BEGIN (?:RSA |EC |DSA |OPENSSH |PGP )?PRIVATE KEY-----.*?-----END (?:RSA |EC |DSA |OPENSSH |PGP )?PRIVATE KEY-----)['"]?`), // Private keys (multiline) + regexp.MustCompile(`['"]?(-----BEGIN (?:RSA |EC |DSA |OPENSSH |PGP )?PRIVATE KEY-----[\s\nA-Za-z0-9+/=]+-----END (?:RSA |EC |DSA |OPENSSH |PGP )?PRIVATE KEY-----)['"]?`), // Private keys (multiline; uses specific char class for PEM base64 content to prevent backtracking) // AWS credentials (most specific - matches aws_secret_access_key before generic "secret") regexp.MustCompile(`(?i)(aws[-_]?access[-_]?key[-_]?id|aws[-_]?secret[-_]?access[-_]?key)\s*(?::=|[:=]>?)\s*['"]?([A-Za-z0-9/+=]{16,})['"]?`), @@ -53,8 +53,15 @@ var secretPatterns = []*regexp.Regexp{ // Connection strings (require 10+ chars to reduce false positives) regexp.MustCompile(`(?i)(connection[-_]?string|conn[-_]?str)\s*(?::=|[:=]>?)\s*['"]?([^\s'"]{10,})['"]?`), - // Service-specific tokens - expanded list - regexp.MustCompile(`(?i)(slack[-_]?token|discord[-_]?token|stripe[-_]?(?:key|secret|api[-_]?key)|twilio[-_]?(?:auth|token|sid)|npm[-_]?token|pypi[-_]?token|github[-_]?token|gitlab[-_]?token|openai[-_]?(?:key|api[-_]?key)|azure[-_]?(?:key|connection[-_]?string)|sendgrid[-_]?(?:key|api[-_]?key)|firebase[-_]?(?:key|api[-_]?key)|mailchimp[-_]?(?:key|api[-_]?key)|mailgun[-_]?(?:key|api[-_]?key)|algolia[-_]?(?:key|api[-_]?key|app[-_]?id)|mapbox[-_]?(?:token|key)|datadog[-_]?(?:key|api[-_]?key)|pagerduty[-_]?(?:key|api[-_]?key)|newrelic[-_]?(?:key|license[-_]?key)|paypal[-_]?(?:secret|client[-_]?id)|square[-_]?token)\s*(?::=|[:=]>?)\s*['"]?([A-Za-z0-9_./+=-]{10,})['"]?`), + // Service-specific tokens - split into smaller patterns for better performance and maintainability + regexp.MustCompile(`(?i)(slack[-_]?token|discord[-_]?token)\s*(?::=|[:=]>?)\s*['"]?([A-Za-z0-9_./+=-]{10,})['"]?`), + regexp.MustCompile(`(?i)(stripe[-_]?(?:key|secret|api[-_]?key)|twilio[-_]?(?:auth|token|sid))\s*(?::=|[:=]>?)\s*['"]?([A-Za-z0-9_./+=-]{10,})['"]?`), + regexp.MustCompile(`(?i)(npm[-_]?token|pypi[-_]?token)\s*(?::=|[:=]>?)\s*['"]?([A-Za-z0-9_./+=-]{10,})['"]?`), + regexp.MustCompile(`(?i)(github[-_]?token|gitlab[-_]?token)\s*(?::=|[:=]>?)\s*['"]?([A-Za-z0-9_./+=-]{10,})['"]?`), + regexp.MustCompile(`(?i)(openai[-_]?(?:key|api[-_]?key)|azure[-_]?(?:key|connection[-_]?string))\s*(?::=|[:=]>?)\s*['"]?([A-Za-z0-9_./+=-]{10,})['"]?`), + regexp.MustCompile(`(?i)(sendgrid[-_]?(?:key|api[-_]?key)|firebase[-_]?(?:key|api[-_]?key)|mailchimp[-_]?(?:key|api[-_]?key)|mailgun[-_]?(?:key|api[-_]?key))\s*(?::=|[:=]>?)\s*['"]?([A-Za-z0-9_./+=-]{10,})['"]?`), + regexp.MustCompile(`(?i)(algolia[-_]?(?:key|api[-_]?key|app[-_]?id)|mapbox[-_]?(?:token|key)|datadog[-_]?(?:key|api[-_]?key)|pagerduty[-_]?(?:key|api[-_]?key)|newrelic[-_]?(?:key|license[-_]?key))\s*(?::=|[:=]>?)\s*['"]?([A-Za-z0-9_./+=-]{10,})['"]?`), + regexp.MustCompile(`(?i)(paypal[-_]?(?:secret|client[-_]?id)|square[-_]?token)\s*(?::=|[:=]>?)\s*['"]?([A-Za-z0-9_./+=-]{10,})['"]?`), // API keys and tokens (require 10+ chars to avoid short non-secrets) regexp.MustCompile(`(?i)(api[-_]?key|apikey|api_token|access[-_]?token|auth[-_]?token|bearer|token)\s*(?::=|[:=]>?)\s*['"]?([A-Za-z0-9_./+=-]{10,})['"]?`), @@ -62,7 +69,8 @@ var secretPatterns = []*regexp.Regexp{ regexp.MustCompile(`(?i)(signing[-_]?key|encryption[-_]?key|secret[-_]?key)\s*(?::=|[:=]>?)\s*['"]?([^\s'"]{16,})['"]?`), // JWT tokens - header starts with eyJ (base64 of '{"'), payload and signature are any base64url regexp.MustCompile(`(eyJ[A-Za-z0-9_-]*\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]*)`), - // Bearer tokens in Authorization headers - "Bearer " without assignment operator + // Bearer tokens in Authorization headers - masks entire "Bearer " string + // Single capture group intentionally includes the full token for complete masking regexp.MustCompile(`['"]?Bearer\s+([A-Za-z0-9_./+=-]{20,})['"]?`), // Password patterns (require 8+ chars to reduce false positives) regexp.MustCompile(`(?i)(password|passwd|pwd|secret)\s*(?::=|[:=]>?)\s*['"]?([^\s'"]{8,})['"]?`), @@ -90,13 +98,24 @@ var commonLiterals = map[string]bool{ "undefined": true, "none": true, "empty": true, } +// maxLineLength is the maximum line length that will be processed for secret masking. +// Lines exceeding this limit are returned unmodified to prevent ReDoS attacks (CWE-770). +// 10KB is sufficient for any reasonable code line while preventing resource exhaustion. +const maxLineLength = 10 * 1024 + // MaskSecretInLine replaces secret values in a line with asterisks while // preserving the code structure. Returns the masked line. +// Lines exceeding maxLineLength (10KB) are returned unmodified to prevent ReDoS. func MaskSecretInLine(line string) string { if line == "" { return line } + // Defense against ReDoS (CWE-770): skip regex processing for extremely long lines + if len(line) > maxLineLength { + return line + } + result := line for _, pattern := range secretPatterns { @@ -115,7 +134,9 @@ func MaskSecretInLine(line string) string { return match } masked := maskValue(value) - return strings.Replace(match, value, masked, 1) + // Use ReplaceAll to mask all occurrences of the secret value within the match, + // though typically each match contains the value only once + return strings.ReplaceAll(match, value, masked) } // For patterns with a single capture group (well-known prefixes, JWT, Bearer, etc.), @@ -126,7 +147,8 @@ func MaskSecretInLine(line string) string { return match } masked := maskValue(value) - return strings.Replace(match, value, masked, 1) + // Use ReplaceAll to mask all occurrences of the secret value within the match + return strings.ReplaceAll(match, value, masked) }) } diff --git a/internal/util/mask_test.go b/internal/util/mask_test.go index 83d3f21..b855671 100644 --- a/internal/util/mask_test.go +++ b/internal/util/mask_test.go @@ -480,6 +480,50 @@ func TestMaskSecretInLine_ServiceSpecificPatterns(t *testing.T) { } } +// TestMaskSecretInLine_NonCredentialDBConnectionStrings verifies that database connection +// strings WITHOUT credentials (like localhost development configs) are NOT masked. +// This prevents false positives on non-secret development configurations. +func TestMaskSecretInLine_NonCredentialDBConnectionStrings(t *testing.T) { + tests := []struct { + name string + input string + }{ + { + name: "MongoDB localhost without credentials", + input: `mongodb://localhost:27017`, + }, + { + name: "MongoDB localhost with database", + input: `mongodb://localhost:27017/mydb`, + }, + { + name: "PostgreSQL localhost without credentials", + input: `postgresql://localhost:5432/mydb`, + }, + { + name: "MySQL localhost without credentials", + input: `mysql://localhost:3306/test`, + }, + { + name: "MongoDB with only host", + input: `mongodb://db.example.com:27017`, + }, + { + name: "MongoDB+srv without credentials", + input: `mongodb+srv://cluster0.example.mongodb.net/mydb`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := util.MaskSecretInLine(tt.input) + if result != tt.input { + t.Errorf("MaskSecretInLine() = %q, want %q (should NOT mask non-credential connection strings)", result, tt.input) + } + }) + } +} + func TestMaskSecretInLine_DictLiterals(t *testing.T) { tests := []struct { name string @@ -852,6 +896,24 @@ MIIEowIBAAKCAQEA0Z3VS5JJcds3xfn/ygWyf8DgnX5X9g5yjW5tNk+PNqp } } +// TestMaskSecretInLine_MaxLineLength verifies that extremely long lines are +// returned unmodified to prevent ReDoS attacks (CWE-770). +func TestMaskSecretInLine_MaxLineLength(t *testing.T) { + // Create a line just under the limit with a secret - should be masked + shortLine := `api_key = "` + strings.Repeat("x", 100) + `"` // #nosec G101 + result := util.MaskSecretInLine(shortLine) + if !strings.Contains(result, "********") { + t.Errorf("Lines under limit should be masked, got: %s", result) + } + + // Create a line over the limit (10KB+) with a secret - should NOT be masked + longLine := `api_key = "` + strings.Repeat("x", 11*1024) + `"` // #nosec G101 + result = util.MaskSecretInLine(longLine) + if result != longLine { + t.Errorf("Lines over limit should be returned unmodified") + } +} + // Benchmark tests to measure performance impact of secret masking patterns. // Run with: go test -bench=. -benchmem ./internal/util/... From 3536c3646217942e7a58d13edb29fe0f6aabd76b Mon Sep 17 00:00:00 2001 From: Yiftach Cohen Date: Wed, 18 Feb 2026 15:30:33 +0200 Subject: [PATCH 7/9] [PPSC-437] fix(lint): add nosec comments for gosec v2.10 false positives - G117: ClientSecret field names in auth config structs (not secret values) - G704: SSRF false positives for validated/constant URLs (auth, API, GitHub) - G705: XSS false positives for stderr output (not HTML rendering) - G115: Integer overflow for uintptr->int conversion (safe on all platforms) - G204: Command injection false positives for validated image names --- internal/auth/auth.go | 2 +- internal/auth/client.go | 6 +++--- internal/cli/color.go | 2 +- internal/httpclient/client.go | 2 +- internal/output/styles.go | 2 +- internal/progress/progress.go | 2 +- internal/scan/image/image.go | 12 ++++++------ internal/update/update.go | 2 +- 8 files changed, 15 insertions(+), 15 deletions(-) diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 4b35abf..adfa1e4 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -17,7 +17,7 @@ import ( type AuthConfig struct { // JWT auth credentials ClientID string - ClientSecret string + ClientSecret string //nolint:gosec // G117: This is a config field name, not a secret value AuthEndpoint string // Full URL to the authentication service // Legacy Basic auth diff --git a/internal/auth/client.go b/internal/auth/client.go index ce9f16c..79ef887 100644 --- a/internal/auth/client.go +++ b/internal/auth/client.go @@ -58,7 +58,7 @@ func NewAuthClient(endpoint string, debug bool) (*AuthClient, error) { // authRequest is the request body for the authenticate endpoint. type authRequest struct { ClientID string `json:"client_id"` - ClientSecret string `json:"client_secret"` + ClientSecret string `json:"client_secret"` //nolint:gosec // G117: This is a JSON field name for auth request, not a secret value } // authResponse is the response from the authenticate endpoint. @@ -88,7 +88,7 @@ func (c *AuthClient) Authenticate(ctx context.Context, clientID, clientSecret st req.Header.Set("Content-Type", "application/json") - resp, err := c.httpClient.Do(req) + resp, err := c.httpClient.Do(req) //nolint:gosec // G704: authEndpoint is constructed from validated config, not user input if err != nil { return "", fmt.Errorf("authentication request failed: %w", err) } @@ -107,7 +107,7 @@ func (c *AuthClient) Authenticate(ctx context.Context, clientID, clientSecret st if resp.StatusCode != http.StatusOK { // Log detailed error info when debug mode is enabled if c.debug { - fmt.Fprintf(os.Stderr, "DEBUG: Auth failed with status %d, body: %s\n", resp.StatusCode, string(body)) + fmt.Fprintf(os.Stderr, "DEBUG: Auth failed with status %d, body: %s\n", resp.StatusCode, string(body)) //nolint:gosec // G705: Debug output to stderr only, not rendered in HTML } // Don't include raw response body in error to prevent potential info leakage return "", fmt.Errorf("authentication failed (status %d)", resp.StatusCode) diff --git a/internal/cli/color.go b/internal/cli/color.go index 55bd5f0..b111861 100644 --- a/internal/cli/color.go +++ b/internal/cli/color.go @@ -70,7 +70,7 @@ func InitColors(mode ColorMode) { disableColors() return } - if !term.IsTerminal(int(os.Stderr.Fd())) { + if !term.IsTerminal(int(os.Stderr.Fd())) { //nolint:gosec // G115: Fd() returns uintptr which fits in int on all supported platforms disableColors() return } diff --git a/internal/httpclient/client.go b/internal/httpclient/client.go index 515f407..e3fa0e7 100644 --- a/internal/httpclient/client.go +++ b/internal/httpclient/client.go @@ -68,7 +68,7 @@ func (c *Client) Do(req *http.Request) (*http.Response, error) { req.Body = newBody } - resp, err = c.httpClient.Do(req) + resp, err = c.httpClient.Do(req) //nolint:gosec // G704: URL is from API client, validated before use if err != nil { // Close response body if present to prevent resource leaks // (some HTTP errors may return both a response and an error) diff --git a/internal/output/styles.go b/internal/output/styles.go index 23916c8..47050ae 100644 --- a/internal/output/styles.go +++ b/internal/output/styles.go @@ -412,7 +412,7 @@ const ( // TerminalWidth detects the current terminal width with fallbacks. // Returns BoxWidth if detection fails (non-TTY, pipe, etc.) func TerminalWidth() int { - w, _, err := term.GetSize(int(os.Stderr.Fd())) + w, _, err := term.GetSize(int(os.Stderr.Fd())) //nolint:gosec // G115: Fd() returns uintptr which fits in int on all supported platforms if err != nil || w <= 0 { return BoxWidth } diff --git a/internal/progress/progress.go b/internal/progress/progress.go index 55a7aa6..6a1c8b1 100644 --- a/internal/progress/progress.go +++ b/internal/progress/progress.go @@ -60,7 +60,7 @@ type fdWriter interface { // isTerminalWriter reports whether the given writer is connected to a terminal. func isTerminalWriter(w io.Writer) bool { if f, ok := w.(fdWriter); ok { - return term.IsTerminal(int(f.Fd())) + return term.IsTerminal(int(f.Fd())) //nolint:gosec // G115: Fd() returns uintptr which fits in int on all supported platforms } return false } diff --git a/internal/scan/image/image.go b/internal/scan/image/image.go index 58d72f2..61d1774 100644 --- a/internal/scan/image/image.go +++ b/internal/scan/image/image.go @@ -146,7 +146,7 @@ func (s *Scanner) ScanTarball(ctx context.Context, tarballPath string) (*model.S uploadSpinner.Stop() styles := output.GetStyles() - fmt.Fprintf(os.Stderr, "%s %s\n\n", + fmt.Fprintf(os.Stderr, "%s %s\n\n", //nolint:gosec // G705: Output to stderr with lipgloss styles, not HTML styles.MutedText.Render("Scan initiated with ID:"), styles.ScanID.Render(scanID)) @@ -164,7 +164,7 @@ func (s *Scanner) ScanTarball(ctx context.Context, tarballPath string) (*model.S } spinner.Stop() - fmt.Fprintf(os.Stderr, "%s %s\n\n", + fmt.Fprintf(os.Stderr, "%s %s\n\n", //nolint:gosec // G705: Output to stderr with lipgloss styles, not HTML styles.MutedText.Render("Scan completed in"), styles.Duration.Render(scan.FormatElapsed(elapsed))) @@ -208,11 +208,11 @@ func (s *Scanner) exportImage(ctx context.Context, imageName, outputPath string) var pullCmd, saveCmd *exec.Cmd switch dockerCmd { case dockerBinary: - pullCmd = exec.CommandContext(ctx, "docker", "pull", imageName) - saveCmd = exec.CommandContext(ctx, "docker", "save", "-o", outputPath, imageName) + pullCmd = exec.CommandContext(ctx, "docker", "pull", imageName) //nolint:gosec // G204: imageName is validated by validateImageName() + saveCmd = exec.CommandContext(ctx, "docker", "save", "-o", outputPath, imageName) //nolint:gosec // G204: imageName is validated, outputPath is controlled case podmanBinary: - pullCmd = exec.CommandContext(ctx, "podman", "pull", imageName) - saveCmd = exec.CommandContext(ctx, "podman", "save", "-o", outputPath, imageName) + pullCmd = exec.CommandContext(ctx, "podman", "pull", imageName) //nolint:gosec // G204: imageName is validated by validateImageName() + saveCmd = exec.CommandContext(ctx, "podman", "save", "-o", outputPath, imageName) //nolint:gosec // G204: imageName is validated, outputPath is controlled default: return fmt.Errorf("unsupported container CLI: %q", dockerCmd) } diff --git a/internal/update/update.go b/internal/update/update.go index ec9355f..b7a5a02 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -149,7 +149,7 @@ func (c *Checker) fetchLatestVersion(ctx context.Context) (string, error) { req.Header.Set("Accept", "application/vnd.github.v3+json") req.Header.Set("User-Agent", "armis-cli-update-check") - resp, err := c.httpClient.Do(req) + resp, err := c.httpClient.Do(req) //nolint:gosec // G704: URL is constant GitHub API endpoint if err != nil { return "", err } From 5f6da5bae3b573053a35c309c9088962550ffbdd Mon Sep 17 00:00:00 2001 From: Yiftach Cohen Date: Thu, 19 Feb 2026 12:29:05 +0200 Subject: [PATCH 8/9] [PPSC-437] refactor: DRY fix masking and add edge case tests - Extract shared maskFixForDisplay function in json.go to avoid duplicating masking logic across formatters - Fix private key regex pattern to use specific char class [\n\r] instead of \s to limit potential backtracking - Add worst-case benchmark for regex pattern performance testing - Add tests for nested/escaped quote edge cases in secret masking --- internal/output/json.go | 30 ++------------------ internal/util/mask.go | 8 +++--- internal/util/mask_test.go | 57 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 32 deletions(-) diff --git a/internal/output/json.go b/internal/output/json.go index 329fd25..8ed3cc5 100644 --- a/internal/output/json.go +++ b/internal/output/json.go @@ -81,35 +81,9 @@ func maskFindingSecrets(f model.Finding) model.Finding { f.CodeSnippet = util.MaskSecretInMultiLineString(f.CodeSnippet) } - // Mask Fix data + // Mask Fix data using shared function (DRY - same logic as human formatter) if f.Fix != nil { - fixCopy := *f.Fix - - if fixCopy.Patch != nil && *fixCopy.Patch != "" { - masked := util.MaskSecretInMultiLineString(*fixCopy.Patch) - fixCopy.Patch = &masked - } - - if fixCopy.VulnerableCode != nil { - vcCopy := *fixCopy.VulnerableCode - vcCopy.Content = util.MaskSecretInMultiLineString(vcCopy.Content) - fixCopy.VulnerableCode = &vcCopy - } - - if len(fixCopy.ProposedFixes) > 0 { - maskedFixes := make([]model.CodeSnippetFix, len(fixCopy.ProposedFixes)) - for i, pf := range fixCopy.ProposedFixes { - maskedFixes[i] = pf - maskedFixes[i].Content = util.MaskSecretInMultiLineString(pf.Content) - } - fixCopy.ProposedFixes = maskedFixes - } - - if len(fixCopy.PatchFiles) > 0 { - fixCopy.PatchFiles = util.MaskSecretsInStringMap(fixCopy.PatchFiles) - } - - f.Fix = &fixCopy + f.Fix = maskFixForDisplay(f.Fix) } return f diff --git a/internal/util/mask.go b/internal/util/mask.go index df6473b..0b312ab 100644 --- a/internal/util/mask.go +++ b/internal/util/mask.go @@ -35,12 +35,12 @@ var secretPatterns = []*regexp.Regexp{ regexp.MustCompile(`['"]?(token_[A-Za-z0-9]{20,})['"]?`), // token_ prefix values (e.g., token_1234567890...) regexp.MustCompile(`['"]?(key-[a-f0-9]{32})['"]?`), // Mailgun API keys regexp.MustCompile(`['"]?(DefaultEndpointsProtocol=https;[^'"]{20,})['"]?`), // Azure connection strings (note: stops at quotes; passwords with quotes are partially matched) - regexp.MustCompile(`['"]?(mongodb(?:\+srv)?://[^:'"@]+:[^@'"]+@[^'"]{5,})['"]?`), // MongoDB connection strings with credentials (stops at quotes) - regexp.MustCompile(`['"]?(postgresql://[^:'"@]+:[^@'"]+@[^'"]{5,})['"]?`), // PostgreSQL connection strings with credentials (stops at quotes) - regexp.MustCompile(`['"]?(mysql://[^:'"@]+:[^@'"]+@[^'"]{5,})['"]?`), // MySQL connection strings with credentials (stops at quotes) + regexp.MustCompile(`['"]?(mongodb(?:\+srv)?://[^:'"@]+:[^@'"]+@[^'"]{5,})['"]?`), // MongoDB connection strings with credentials (note: stops at quotes; passwords with quotes are partially matched) + regexp.MustCompile(`['"]?(postgresql://[^:'"@]+:[^@'"]+@[^'"]{5,})['"]?`), // PostgreSQL connection strings with credentials (note: stops at quotes; passwords with quotes are partially matched) + regexp.MustCompile(`['"]?(mysql://[^:'"@]+:[^@'"]+@[^'"]{5,})['"]?`), // MySQL connection strings with credentials (note: stops at quotes; passwords with quotes are partially matched) regexp.MustCompile(`['"]?(https://hooks\.slack\.com/services/[A-Za-z0-9/]+)['"]?`), // Slack webhooks regexp.MustCompile(`['"]?(https://[A-Za-z0-9]+@[A-Za-z0-9]+\.ingest\.sentry\.io/[0-9]+)['"]?`), // Sentry DSN URLs - regexp.MustCompile(`['"]?(-----BEGIN (?:RSA |EC |DSA |OPENSSH |PGP )?PRIVATE KEY-----[\s\nA-Za-z0-9+/=]+-----END (?:RSA |EC |DSA |OPENSSH |PGP )?PRIVATE KEY-----)['"]?`), // Private keys (multiline; uses specific char class for PEM base64 content to prevent backtracking) + regexp.MustCompile(`['"]?(-----BEGIN (?:RSA |EC |DSA |OPENSSH |PGP )?PRIVATE KEY-----[\n\rA-Za-z0-9+/= ]+-----END (?:RSA |EC |DSA |OPENSSH |PGP )?PRIVATE KEY-----)['"]?`), // Private keys (multiline; uses specific char class without \s to limit backtracking) // AWS credentials (most specific - matches aws_secret_access_key before generic "secret") regexp.MustCompile(`(?i)(aws[-_]?access[-_]?key[-_]?id|aws[-_]?secret[-_]?access[-_]?key)\s*(?::=|[:=]>?)\s*['"]?([A-Za-z0-9/+=]{16,})['"]?`), diff --git a/internal/util/mask_test.go b/internal/util/mask_test.go index b855671..507562b 100644 --- a/internal/util/mask_test.go +++ b/internal/util/mask_test.go @@ -956,3 +956,60 @@ func main() { util.MaskSecretInMultiLineString(content) } } + +// BenchmarkMaskSecretInLine_WorstCase tests performance with input that triggers +// near-matches across multiple patterns without actually matching. This helps detect +// potential backtracking issues when patterns partially match but fail. +func BenchmarkMaskSecretInLine_WorstCase(b *testing.B) { + // Input designed to trigger partial matches across many patterns: + // - Looks like assignment but short values + // - Contains secret-like keywords + // - Has quotes and special chars that patterns must traverse + line := `config = { "api": "short", "key": "val", "sk-": "x", "password": "", "token": "12345" }` + b.ResetTimer() + for i := 0; i < b.N; i++ { + util.MaskSecretInLine(line) + } +} + +// TestMaskSecretInLine_NestedQuotes verifies behavior with JSON-style escaped quotes. +// Note: The regex patterns stop at quote boundaries, so escaped quotes within values +// will cause partial matching. This is documented behavior - the value up to the +// escaped quote will be masked, which is acceptable from a security standpoint +// (partial masking is safe, leaking secrets is not). +func TestMaskSecretInLine_NestedQuotes(t *testing.T) { + tests := []struct { + name string + input string + wantNotContain string + description string + }{ + { + name: "escaped quotes in value - partial mask expected", + input: `"api_key": "value_with_\"escaped\"_content_1234567890"`, // #nosec G101 + wantNotContain: "value_with_", + description: "Content before escaped quote should be masked", + }, + { + name: "simple quoted value without escapes", + input: `"password": "SuperSecretPassword123!"`, // #nosec G101 + wantNotContain: "SuperSecretPassword", + description: "Standard case without escapes should mask fully", + }, + { + name: "value ending with escaped quote", + input: `"secret": "ends_with_quote\""`, // #nosec G101 + wantNotContain: "ends_with_quote", + description: "Value before trailing escaped quote should be masked", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := util.MaskSecretInLine(tt.input) + if strings.Contains(result, tt.wantNotContain) { + t.Errorf("MaskSecretInLine() = %q, should NOT contain %q (%s)", result, tt.wantNotContain, tt.description) + } + }) + } +} From e20d85447e416014092a1e341c9b3109bca1d736 Mon Sep 17 00:00:00 2001 From: Yiftach Cohen Date: Thu, 19 Feb 2026 12:39:12 +0200 Subject: [PATCH 9/9] [PPSC-437] fix(lint): add nosec G705 for debug output false positives gosec v2.10's taint analysis flags CLI debug output as XSS risk (G705). This is a false positive - stderr output in a CLI tool has no browser context for XSS attacks. Add #nosec G705 comments with explanation. --- internal/scan/image/image.go | 1 + internal/scan/repo/repo.go | 1 + 2 files changed, 2 insertions(+) diff --git a/internal/scan/image/image.go b/internal/scan/image/image.go index 61d1774..e406ea2 100644 --- a/internal/scan/image/image.go +++ b/internal/scan/image/image.go @@ -321,6 +321,7 @@ func convertNormalizedFindings(normalizedFindings []model.NormalizedFinding, deb if err != nil { fmt.Fprintf(os.Stderr, "\n=== DEBUG: Finding #%d JSON Marshal Error: %v ===\n\n", i+1, err) } else { + // #nosec G705 -- Debug output to stderr; no XSS risk in CLI context fmt.Fprintf(os.Stderr, "\n=== DEBUG: Finding #%d Raw JSON ===\n%s\n=== END DEBUG ===\n\n", i+1, string(rawJSON)) } } diff --git a/internal/scan/repo/repo.go b/internal/scan/repo/repo.go index 57101d1..9852bb7 100644 --- a/internal/scan/repo/repo.go +++ b/internal/scan/repo/repo.go @@ -611,6 +611,7 @@ func convertNormalizedFindings(normalizedFindings []model.NormalizedFinding, deb if err != nil { fmt.Fprintf(os.Stderr, "\n=== DEBUG: Finding #%d JSON Marshal Error: %v ===\n\n", i+1, err) } else { + // #nosec G705 -- Debug output to stderr; no XSS risk in CLI context fmt.Fprintf(os.Stderr, "\n=== DEBUG: Finding #%d Raw JSON ===\n%s\n=== END DEBUG ===\n\n", i+1, string(rawJSON)) } }