Skip to content

Commit 8116024

Browse files
Dumbrisclaude
andcommitted
fix(security): reduce false positives and deduplicate detections
- Add deduplication to AddDetection() to prevent duplicate type+location - AWS secret key pattern now requires keyword context (aws_secret_access_key=, AWS_SECRET_KEY:, secretAccessKey:) to avoid matching random base64 in RSA keys - Azure client secret pattern now requires keyword context (AZURE_CLIENT_SECRET=, client_secret:, clientSecret:) to avoid false positives - Update tests to reflect context-required behavior - Add TestResult_AddDetection_Deduplication test Before: id_rsa showed 9 detections (including aws_secret_key false positives) After: id_rsa shows 3 detections (rsa_private_key, private_key, high_entropy) Before: .env showed 29 detections (many duplicates) After: .env shows 9 unique detections Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 18a9b52 commit 8116024

4 files changed

Lines changed: 84 additions & 44 deletions

File tree

internal/security/detector_test.go

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package security
22

33
import (
4+
"fmt"
45
"testing"
56

67
"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
@@ -140,9 +141,10 @@ func TestResult_AddDetection(t *testing.T) {
140141
func TestResult_AddDetection_Multiple(t *testing.T) {
141142
result := NewResult()
142143

144+
// Add different types - all should be added
143145
for i := 0; i < 5; i++ {
144146
result.AddDetection(Detection{
145-
Type: "test_type",
147+
Type: fmt.Sprintf("test_type_%d", i),
146148
Category: "test_category",
147149
Severity: "medium",
148150
Location: "arguments",
@@ -153,6 +155,32 @@ func TestResult_AddDetection_Multiple(t *testing.T) {
153155
assert.Len(t, result.Detections, 5)
154156
}
155157

158+
func TestResult_AddDetection_Deduplication(t *testing.T) {
159+
result := NewResult()
160+
161+
// Add same type+location multiple times - should only store once
162+
for i := 0; i < 5; i++ {
163+
result.AddDetection(Detection{
164+
Type: "test_type",
165+
Category: "test_category",
166+
Severity: "medium",
167+
Location: "arguments",
168+
})
169+
}
170+
171+
assert.True(t, result.Detected)
172+
assert.Len(t, result.Detections, 1, "duplicate detections should be deduplicated")
173+
174+
// Add same type but different location - should add both
175+
result.AddDetection(Detection{
176+
Type: "test_type",
177+
Category: "test_category",
178+
Severity: "medium",
179+
Location: "response",
180+
})
181+
assert.Len(t, result.Detections, 2, "same type with different location should be added")
182+
}
183+
156184
// Integration tests for pattern detection
157185
func TestDetector_PatternDetection(t *testing.T) {
158186
tests := []struct {

internal/security/patterns/cloud.go

Lines changed: 8 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -38,33 +38,18 @@ func awsAccessKeyPattern() *Pattern {
3838
}
3939

4040
// AWS Secret Key pattern
41-
// Base64-encoded 40-character string
41+
// Requires keyword context to avoid false positives on random base64 strings
4242
func awsSecretKeyPattern() *Pattern {
43+
// Pattern requires keyword context like: aws_secret_access_key=, AWS_SECRET_KEY:, "secretAccessKey":
44+
// Handles formats: key=value, key: value, "key": "value"
4345
return NewPattern("aws_secret_key").
44-
WithRegex(`[A-Za-z0-9/+=]{40}`).
46+
WithRegex(`(?i)(?:aws[_-]?secret[_-]?(?:access[_-]?)?key|secret[_-]?access[_-]?key|secretAccessKey)["']?\s*[:=]\s*["']?([A-Za-z0-9/+=]{40})["']?`).
4547
WithCategory(CategoryCloudCredentials).
4648
WithSeverity(SeverityCritical).
4749
WithDescription("AWS secret access key").
4850
WithKnownExamples(
4951
"wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", // AWS documentation example
5052
).
51-
WithValidator(func(match string) bool {
52-
// Must contain at least one lowercase, one uppercase, and one digit/special
53-
hasLower := false
54-
hasUpper := false
55-
hasOther := false
56-
for _, c := range match {
57-
switch {
58-
case c >= 'a' && c <= 'z':
59-
hasLower = true
60-
case c >= 'A' && c <= 'Z':
61-
hasUpper = true
62-
case (c >= '0' && c <= '9') || c == '/' || c == '+' || c == '=':
63-
hasOther = true
64-
}
65-
}
66-
return hasLower && hasUpper && hasOther
67-
}).
6853
Build()
6954
}
7055

@@ -91,22 +76,15 @@ func gcpServiceAccountPattern() *Pattern {
9176
}
9277

9378
// Azure Client Secret pattern
94-
// Typically 34+ characters with special characters
79+
// Requires keyword context to avoid false positives
9580
func azureClientSecretPattern() *Pattern {
81+
// Pattern requires keyword context like: AZURE_CLIENT_SECRET=, client_secret:, "clientSecret":
82+
// Handles formats: key=value, key: value, "key": "value"
9683
return NewPattern("azure_client_secret").
97-
WithRegex(`[a-zA-Z0-9~._-]{34,}`).
84+
WithRegex(`(?i)(?:azure[_-]?client[_-]?secret|client[_-]?secret|clientSecret|AZURE_SECRET)["']?\s*[:=]\s*["']?([a-zA-Z0-9~._-]{34,})["']?`).
9885
WithCategory(CategoryCloudCredentials).
9986
WithSeverity(SeverityHigh).
10087
WithDescription("Azure client secret / app password").
101-
WithValidator(func(match string) bool {
102-
// Must contain at least one special character (~ . _ -)
103-
for _, c := range match {
104-
if c == '~' || c == '.' || c == '_' || c == '-' {
105-
return true
106-
}
107-
}
108-
return false
109-
}).
11088
Build()
11189
}
11290

internal/security/patterns/cloud_test.go

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -178,21 +178,39 @@ func TestAWSSecretKeyPattern(t *testing.T) {
178178
wantExample bool
179179
}{
180180
{
181-
name: "example secret key",
181+
name: "standalone secret key - no context, should not match",
182182
input: "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY",
183+
wantMatch: false, // Now requires context
184+
wantExample: false,
185+
},
186+
{
187+
name: "secret key with aws_secret_access_key context",
188+
input: `aws_secret_access_key = wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY`,
183189
wantMatch: true,
184190
wantExample: true,
185191
},
186192
{
187-
name: "secret key in context",
188-
input: `aws_secret_access_key = wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY`,
193+
name: "secret key with AWS_SECRET_KEY context",
194+
input: `AWS_SECRET_KEY="wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY"`,
189195
wantMatch: true,
190196
wantExample: true,
191197
},
192198
{
193-
name: "random 40 char base64-like",
194-
input: "abcdefghij1234567890ABCDEFGHIJ1234567890",
199+
name: "secret key with secretAccessKey JSON context",
200+
input: `"secretAccessKey": "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY"`,
195201
wantMatch: true,
202+
wantExample: true,
203+
},
204+
{
205+
name: "random 40 char base64-like without context - no match",
206+
input: "abcdefghij1234567890ABCDEFGHIJ1234567890",
207+
wantMatch: false, // Now requires context
208+
wantExample: false,
209+
},
210+
{
211+
name: "RSA private key content - should not match",
212+
input: "MIIEpAIBAAKCAQEA0Z3VS5JJcds3xfn/ygWyF8Pb",
213+
wantMatch: false, // No AWS context
196214
wantExample: false,
197215
},
198216
{
@@ -325,23 +343,33 @@ func TestAzureClientSecretPattern(t *testing.T) {
325343
wantMatch bool
326344
}{
327345
{
328-
name: "Azure client secret format 1",
346+
name: "standalone secret - no context, should not match",
329347
input: "7c9~abcdefghijklmnopqrstuvwxyz1234",
330-
wantMatch: true,
348+
wantMatch: false, // Now requires context
331349
},
332350
{
333-
name: "Azure client secret format 2",
351+
name: "standalone secret format 2 - no context, should not match",
334352
input: "abc.defghijklmnopqrstuvwxyz123456~",
335-
wantMatch: true,
353+
wantMatch: false, // Now requires context
336354
},
337355
{
338-
name: "Azure client secret in config",
356+
name: "Azure client secret with AZURE_CLIENT_SECRET context",
339357
input: `AZURE_CLIENT_SECRET=7c9~abcdefghijklmnopqrstuvwxyz1234`,
340358
wantMatch: true,
341359
},
342360
{
343-
name: "too short",
344-
input: "7c9~abc",
361+
name: "Azure client secret with client_secret context",
362+
input: `client_secret: "7c9~abcdefghijklmnopqrstuvwxyz1234"`,
363+
wantMatch: true,
364+
},
365+
{
366+
name: "Azure client secret with clientSecret JSON context",
367+
input: `"clientSecret": "abc.defghijklmnopqrstuvwxyz123456~"`,
368+
wantMatch: true,
369+
},
370+
{
371+
name: "too short even with context",
372+
input: `AZURE_CLIENT_SECRET=7c9~abc`,
345373
wantMatch: false,
346374
},
347375
}

internal/security/types.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,14 @@ func NewResult() *Result {
6969
}
7070
}
7171

72-
// AddDetection adds a detection to the result
72+
// AddDetection adds a detection to the result, avoiding duplicates
7373
func (r *Result) AddDetection(d Detection) {
74+
// Check for duplicate (same type + location)
75+
for _, existing := range r.Detections {
76+
if existing.Type == d.Type && existing.Location == d.Location {
77+
return // Already have this detection
78+
}
79+
}
7480
r.Detections = append(r.Detections, d)
7581
r.Detected = true
7682
}

0 commit comments

Comments
 (0)