Skip to content

Commit 71ec709

Browse files
authored
fix(security): skip an invalid custom pattern regex instead of panicking (#1763)
## Bug `sensitive_data_detection.custom_patterns[].regex` comes straight from the user's config file and is not validated when the config loads. `buildCustomPattern` passes it to `PatternBuilder.WithRegex`, which uses `regexp.MustCompile`. One malformed regex therefore panics inside `security.NewDetector`. `NewDetector` runs at startup (`runtime.go`, `server.go`, `mcp.go`), on config reload (`ReloadConfig`), and when the credential input screen is built. A typo in the config means mcpproxy won't start at all. The validating loader `patterns.LoadCustomPatterns` exists, but it is not on this path. ## Trigger ```json "sensitive_data_detection": { "enabled": true, "custom_patterns": [{"name": "x", "regex": "("}] } ``` Result: `panic: regexp: Compile(`(`): error parsing regexp: missing closing ): `(`` ## Fix In `buildCustomPattern`, compile the regex with `regexp.Compile` first and drop the pattern if it is invalid. This matches how the function already drops a pattern with no name or no regex/keywords. The other custom patterns and all built-in patterns are still loaded. ## Test `TestNewDetector_InvalidCustomRegexIsSkipped` configures one broken and one valid custom pattern. It asserts that `NewDetector` does not panic and that the valid pattern still detects `TICKET-123456`. ## Commands run - `CGO_ENABLED=0 go test -count=1 ./internal/security`: ok - With the fix reverted (`git stash` of `detector.go`), the new test fails with `NewDetector panicked on an invalid custom regex: regexp: Compile(`(`): error parsing regexp: missing closing ): `(``. - `gofmt -l` on the edited files: no output
1 parent 569a266 commit 71ec709

2 files changed

Lines changed: 45 additions & 0 deletions

File tree

‎internal/security/detector.go‎

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

33
import (
4+
"regexp"
45
"strings"
56
"sync"
67
"time"
@@ -344,6 +345,11 @@ func buildCustomPattern(cp config.CustomPattern) *Pattern {
344345

345346
// Set pattern (regex or keywords)
346347
if cp.Regex != "" {
348+
// Config is user-edited and not validated on load; an invalid regex
349+
// must drop this one pattern, not panic in WithRegex's MustCompile.
350+
if _, err := regexp.Compile(cp.Regex); err != nil {
351+
return nil
352+
}
347353
builder.WithRegex(cp.Regex)
348354
} else if len(cp.Keywords) > 0 {
349355
builder.WithKeywords(cp.Keywords...)
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
package security
2+
3+
import (
4+
"testing"
5+
6+
"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
7+
)
8+
9+
// sensitive_data_detection.custom_patterns comes straight from the user's
10+
// config file; one bad regex must not take down NewDetector (startup, config
11+
// reload, credential screen) or the other custom patterns.
12+
func TestNewDetector_InvalidCustomRegexIsSkipped(t *testing.T) {
13+
cfg := config.DefaultSensitiveDataDetectionConfig()
14+
cfg.CustomPatterns = []config.CustomPattern{
15+
{Name: "broken", Regex: "("},
16+
{Name: "ticket", Regex: `TICKET-\d{6}`},
17+
}
18+
19+
var d *Detector
20+
func() {
21+
defer func() {
22+
if r := recover(); r != nil {
23+
t.Fatalf("NewDetector panicked on an invalid custom regex: %v", r)
24+
}
25+
}()
26+
d = NewDetector(cfg)
27+
}()
28+
29+
res := d.Scan(`{"note":"see TICKET-123456"}`, "")
30+
found := false
31+
for _, det := range res.Detections {
32+
if det.Type == "ticket" {
33+
found = true
34+
}
35+
}
36+
if !found {
37+
t.Errorf("valid custom pattern next to the broken one was not applied: %+v", res.Detections)
38+
}
39+
}

0 commit comments

Comments
 (0)