Core Rule Set tests: inspect a request with a body only while bodies are read (closes #287)

A recorded request with a body or a Content-Type header is now inspected
only by the Core Rule Set that reads bodies, in the WordPress test as in
the other apps' tests. Reading bodies only adds rule matches, so passing
there means passing at the defaults. TestReadingBodiesOnlyAddsMatches
checks that on every 40th recorded request with a body of each app, as
recorded and with an SQL injection added to its query.

Model: opus-5-5

internal/coreruleset/coreruleset_test.go

@@ -625,11 +625,12 @@ const passwordField = "password="
 var hardValues = []string{"python3", "powershell", "ls%20-la", ".htaccess"}
 
 // wantRecordedRequestsPass checks that none of the recorded ordinary
-// requests in the file name in testdata is refused, at the defaults and
-// while bodies are read. A request with neither a body nor a Content-Type
-// header is checked only at the defaults: with neither, the set that reads
-// bodies has nothing more to check. Each request runs as its own parallel
-// subtest.
+// requests in the file name in testdata is refused. A request with a body
+// or a Content-Type header is checked while bodies are read, and any other
+// request at the defaults. Reading bodies only adds rule matches, so a
+// request that passes while bodies are read passes at the defaults too;
+// TestReadingBodiesOnlyAddsMatches checks that. Each request runs as its
+// own parallel subtest.
 func wantRecordedRequestsPass(t *testing.T, name string) {
 	t.Helper()
 
@@ -643,21 +644,21 @@ func wantRecordedRequestsPass(t *testing.T, name string) {
 
 			r := request{recorded.Method, recorded.Target, recorded.Headers}
 
-			got, _ := inspectBody(t, defaults, r, recorded.Body)
-			if got.Score >= threshold {
-				t.Errorf("%s %s refused: %+v", r.method, r.target, got)
-			}
-
 			typed := slices.ContainsFunc(recorded.Headers, func(header string) bool {
 				name, _, _ := strings.Cut(header, ":")
 
 				return strings.EqualFold(name, "Content-Type")
 			})
 			if recorded.Body == "" && !typed {
+				got, _ := inspectBody(t, defaults, r, "")
+				if got.Score >= threshold {
+					t.Errorf("%s %s refused: %+v", r.method, r.target, got)
+				}
+
 				return
 			}
 
-			got, _ = inspectBody(t, bodies, r, recorded.Body)
+			got, _ := inspectBody(t, bodies, r, recorded.Body)
 			if got.Score >= threshold {
 				t.Errorf("%s %s %.200q refused while bodies are read: %+v",
 					r.method, r.target, recorded.Body, got)
@@ -666,6 +667,78 @@ func wantRecordedRequestsPass(t *testing.T, name string) {
 	}
 }
 
+// TestReadingBodiesOnlyAddsMatches checks that every rule a recorded
+// request with a body matches at the defaults also matches while bodies
+// are read, with a score no lower. It checks every 40th such request of
+// each app, as recorded and with an SQL injection added to its query, which
+// the defaults must match.
+func TestReadingBodiesOnlyAddsMatches(t *testing.T) {
+	t.Parallel()
+
+	defaults, bodies := atDefaults(t), readingBodies(t)
+
+	for _, name := range []string{
+		"drupal-requests.txt",
+		"mastodon-requests.txt",
+		"mediawiki-requests.txt",
+		"nextcloud-requests.txt",
+		"vaultwarden-requests.txt",
+		"wordpress-requests.txt",
+	} {
+		withBody := 0
+
+		for _, recorded := range recordedRequests(t, name) {
+			if recorded.Body == "" {
+				continue
+			}
+
+			withBody++
+			if withBody%40 != 1 {
+				continue
+			}
+
+			separator := "?"
+			if strings.Contains(recorded.Target, "?") {
+				separator = "&"
+			}
+
+			r := request{recorded.Method, recorded.Target, recorded.Headers}
+			wantNoMatchLost(t, defaults, bodies, r, recorded.Body)
+
+			r.target += separator + "id=" + sqlInjection
+			if atDefault := wantNoMatchLost(t, defaults, bodies, r,
+				recorded.Body); len(atDefault.RuleIDs) == 0 {
+				t.Errorf("%s %s matched nothing at the defaults", r.method, r.target)
+			}
+		}
+	}
+}
+
+// wantNoMatchLost checks that every rule defaults matches in r with body,
+// bodies matches too, with a score no lower, and returns what defaults
+// found.
+func wantNoMatchLost(
+	t *testing.T, defaults, bodies *coreruleset.CoreRuleSet, r request, body string,
+) coreruleset.Result {
+	t.Helper()
+
+	atDefault, _ := inspectBody(t, defaults, r, body)
+	whileRead, _ := inspectBody(t, bodies, r, body)
+
+	for _, id := range atDefault.RuleIDs {
+		if !slices.Contains(whileRead.RuleIDs, id) {
+			t.Errorf("%s %s: %d matched at the defaults only", r.method, r.target, id)
+		}
+	}
+
+	if whileRead.Score < atDefault.Score {
+		t.Errorf("%s %s scored %d at the defaults and %d while bodies are read",
+			r.method, r.target, atDefault.Score, whileRead.Score)
+	}
+
+	return atDefault
+}
+
 func TestOrdinaryGiteaRequestsPassAndAttacksAreRefused(t *testing.T) {
 	t.Parallel()
 
@@ -849,16 +922,16 @@ func TestOrdinaryWordPressRequestsPassAndAttacksAreRefused(t *testing.T) {
 
 			r := request{recorded.Method, recorded.Target, recorded.Headers}
 
-			crsSets := []*coreruleset.CoreRuleSet{defaults}
+			// As in wantRecordedRequestsPass, a request with a body passes at
+			// the defaults if it passes while bodies are read.
+			crs := defaults
 			if recorded.Body != "" {
-				crsSets = append(crsSets, bodies)
+				crs = 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)
-				}
+			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)
 			}
 		})
 	}