From b2e84e034f47425ee9a55012a3df8ba2b0f6a67e Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 9 Nov 2026 20:53:20 +0000 Subject: [PATCH] Core Rule Set tests: load each set of settings once (closes #252) Tests now share one Core Rule Set for each set of settings, loaded once through sync.OnceValues in shared(). Exclusions-file tests and TestCorazaWritesNoFile still load their own: loading is what they check. Loading was not most of the time: the WordPress and Drupal recorded requests, inspected one after another, were. Each recorded request now runs as its own parallel subtest, with the same request and outcome. Model: opus-5-5 --- internal/coreruleset/coreruleset_test.go | 109 ++++++++++++++++------- internal/coreruleset/drupal_test.go | 24 ++--- 2 files changed, 89 insertions(+), 44 deletions(-) diff --git a/internal/coreruleset/coreruleset_test.go b/internal/coreruleset/coreruleset_test.go index f695303..73298c3 100644 --- a/internal/coreruleset/coreruleset_test.go +++ b/internal/coreruleset/coreruleset_test.go @@ -25,14 +25,41 @@ import ( //nolint:gochecknoglobals // a constant cannot be a list var defaultDisabledRules = []int{920340, 920420, 920440, 920640, 930130, 930140} -// newCoreRuleSet returns the Core Rule Set at paranoia level level, with -// the rules in disabled switched off. -func newCoreRuleSet(t *testing.T, level int, disabled ...int) *coreruleset.CoreRuleSet { +// loaded holds, for each set of settings the tests have asked for, the +// function that loads the Core Rule Set with them once. +// +//nolint:gochecknoglobals // the tests share it +var loaded = struct { + sync.Mutex + + loaders map[string]func() (*coreruleset.CoreRuleSet, error) +}{loaders: map[string]func() (*coreruleset.CoreRuleSet, error){}} + +// loaderFor returns the function that loads the Core Rule Set with params +// once, making it the first time params are asked for. +func loaderFor(params coreruleset.Params) func() (*coreruleset.CoreRuleSet, error) { + loaded.Lock() + defer loaded.Unlock() + + settings := fmt.Sprintf("%+v", params) + + load, found := loaded.loaders[settings] + if !found { + load = sync.OnceValues(func() (*coreruleset.CoreRuleSet, error) { + return coreruleset.New(params) + }) + loaded.loaders[settings] = load + } + + return load +} + +// shared returns the Core Rule Set with params, loaded once for every +// test that asks for the same settings. +func shared(t *testing.T, params coreruleset.Params) *coreruleset.CoreRuleSet { t.Helper() - crs, err := coreruleset.New(coreruleset.Params{ - ParanoiaLevel: level, DisabledRules: disabled, - }) + crs, err := loaderFor(params)() if err != nil { t.Fatalf("load the Core Rule Set: %v", err) } @@ -40,6 +67,17 @@ func newCoreRuleSet(t *testing.T, level int, disabled ...int) *coreruleset.CoreR return crs } +// sharedCoreRuleSet returns the Core Rule Set at paranoia level level, +// with the rules in disabled switched off. The set is shared with every +// test that asks for the same settings. +func sharedCoreRuleSet( + t *testing.T, level int, disabled ...int, +) *coreruleset.CoreRuleSet { + t.Helper() + + return shared(t, coreruleset.Params{ParanoiaLevel: level, DisabledRules: disabled}) +} + // TestConcurrentRequestsKeepTheirOwnExclusions inspects a Drupal and a // WordPress request at once, many times over, on one Core Rule Set, as // sww's server does. Each passes only while its own exclusions apply. @@ -160,7 +198,7 @@ func matched(ids ...int) coreruleset.Result { func atDefaults(t *testing.T) *coreruleset.CoreRuleSet { t.Helper() - return newCoreRuleSet(t, 1, defaultDisabledRules...) + return sharedCoreRuleSet(t, 1, defaultDisabledRules...) } // wantChange checks that crs lets through passes, a request one of the @@ -395,8 +433,8 @@ func TestParanoiaLevel(t *testing.T) { // Accept-Charset is refused from paranoia level 2. r := get("/", "Accept-Charset: utf-8") - wantResult(t, newCoreRuleSet(t, 1), r, coreruleset.Result{}) - wantResult(t, newCoreRuleSet(t, 2), r, matched(920451)) + wantResult(t, sharedCoreRuleSet(t, 1), r, coreruleset.Result{}) + wantResult(t, sharedCoreRuleSet(t, 2), r, matched(920451)) } func TestEachDisabledRuleIsSwitchedOff(t *testing.T) { @@ -406,9 +444,9 @@ func TestEachDisabledRuleIsSwitchedOff(t *testing.T) { // which adds 3. r := request{http.MethodTrace, "/", []string{"Host: 192.0.2.1"}} - wantResult(t, newCoreRuleSet(t, 1), r, + wantResult(t, sharedCoreRuleSet(t, 1), r, coreruleset.Result{RuleIDs: []int{911100, 920350}, Score: 8}) - wantResult(t, newCoreRuleSet(t, 1, 920350, 911100), r, coreruleset.Result{}) + wantResult(t, sharedCoreRuleSet(t, 1, 920350, 911100), r, coreruleset.Result{}) } func TestImagePushThatReusesALayerIsNotCheckedForCommandNames(t *testing.T) { @@ -657,14 +695,9 @@ func recordedRequests(t *testing.T, name string) []recordedRequest { func readingBodiesUpTo64K(t *testing.T) *coreruleset.CoreRuleSet { t.Helper() - crs, err := coreruleset.New(coreruleset.Params{ + return shared(t, coreruleset.Params{ ParanoiaLevel: 1, DisabledRules: defaultDisabledRules, BodyLimit: 64<<10 - 1, }) - if err != nil { - t.Fatalf("load the Core Rule Set: %v", err) - } - - return crs } // wordpressAttackTargets returns attack in a query parameter no exclusion @@ -730,19 +763,23 @@ func TestOrdinaryWordPressRequestsPassAndAttacksAreRefused(t *testing.T) { defaults, bodies := atDefaults(t), readingBodiesUpTo64K(t) for _, recorded := range recordedRequests(t, "wordpress-requests.txt") { - r := request{recorded.Method, recorded.Target, recorded.Headers} + t.Run(recorded.Method+" "+recorded.Target, func(t *testing.T) { + t.Parallel() - crsSets := []*coreruleset.CoreRuleSet{defaults} - if recorded.Body != "" { - crsSets = append(crsSets, bodies) - } + r := request{recorded.Method, recorded.Target, recorded.Headers} - for _, crs := range crsSets { - got, _ := inspectBody(t, crs, r, recorded.Body) - if got.Score >= threshold { - t.Errorf("%s %s %.200q refused: %+v", r.method, r.target, recorded.Body, got) + crsSets := []*coreruleset.CoreRuleSet{defaults} + if recorded.Body != "" { + crsSets = append(crsSets, bodies) } - } + + for _, crs := range crsSets { + got, _ := inspectBody(t, crs, r, recorded.Body) + if got.Score >= threshold { + t.Errorf("%s %s %.200q refused: %+v", r.method, r.target, recorded.Body, got) + } + } + }) } for _, attack := range []string{ @@ -1289,14 +1326,9 @@ const injection = "1' OR '1'='1" func readingBodies(t *testing.T) *coreruleset.CoreRuleSet { t.Helper() - crs, err := coreruleset.New(coreruleset.Params{ + return shared(t, coreruleset.Params{ ParanoiaLevel: 1, DisabledRules: defaultDisabledRules, BodyLimit: bodyLimit, }) - if err != nil { - t.Fatalf("load the Core Rule Set: %v", err) - } - - return crs } // post is a POST request for / with a body of the type contentType, a @@ -1581,10 +1613,19 @@ func TestCorazaWritesNoFile(t *testing.T) { // refuses to load, and could not write a file of a multipart body. t.Setenv("TMPDIR", filepath.Join(t.TempDir(), "missing")) + // The test loads its own Core Rule Set, since loading is part of what + // it checks. + crs, err := coreruleset.New(coreruleset.Params{ + ParanoiaLevel: 1, DisabledRules: defaultDisabledRules, BodyLimit: bodyLimit, + }) + if err != nil { + t.Fatalf("load the Core Rule Set: %v", err) + } + body := "--b\r\nContent-Disposition: form-data; name=\"file\"; " + "filename=\"notes.txt\"\r\nContent-Type: text/plain\r\n\r\n" + strings.Repeat("a", 1000) + "\r\n" + field("q", injection) + end - wantBody(t, readingBodies(t), post(multipart), body, matched(942100), body) + wantBody(t, crs, post(multipart), body, matched(942100), body) } func TestBodyLimitOf1GLoads(t *testing.T) { diff --git a/internal/coreruleset/drupal_test.go b/internal/coreruleset/drupal_test.go index a3dfe9e..634c524 100644 --- a/internal/coreruleset/drupal_test.go +++ b/internal/coreruleset/drupal_test.go @@ -34,18 +34,22 @@ func TestOrdinaryDrupalRequestsPass(t *testing.T) { defaults, bodies := atDefaults(t), readingBodies(t) for _, recorded := range recordedRequests(t, "drupal-requests.txt") { - r := request{recorded.Method, recorded.Target, recorded.Headers} + t.Run(recorded.Method+" "+recorded.Target, func(t *testing.T) { + t.Parallel() - got, _ := inspectBody(t, defaults, r, recorded.Body) - if got.Score >= drupalThreshold { - t.Errorf("%s %s refused: %+v", r.method, r.target, got) - } + r := request{recorded.Method, recorded.Target, recorded.Headers} - got, _ = inspectBody(t, bodies, r, recorded.Body) - if got.Score >= drupalThreshold { - t.Errorf("%s %s %.200q refused while bodies are read: %+v", - r.method, r.target, recorded.Body, got) - } + got, _ := inspectBody(t, defaults, r, recorded.Body) + if got.Score >= drupalThreshold { + t.Errorf("%s %s refused: %+v", r.method, r.target, got) + } + + got, _ = inspectBody(t, bodies, r, recorded.Body) + if got.Score >= drupalThreshold { + t.Errorf("%s %s %.200q refused while bodies are read: %+v", + r.method, r.target, recorded.Body, got) + } + }) } }