From 46fe7baed04eba67dc14b893102d35857c138ec3 Mon Sep 17 00:00:00 2001
From: clawbot <35+clawbot@noreply.example.org>
Date: Sun, 4 Oct 2026 03:31:37 +0200
Subject: [PATCH] Mask a target URL's query string when the URL has no path
 (closes #500)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`urlSecrets` treated a target URL's request URI as a secret only when the URL had a path, so for a target URL such as `https://example.com/?token=…` a response echoing the request line showed the token in the event log and on the event's page. It now also treats the query string, and the request URI that carries it, as secrets whenever the URL has one, whatever its path. With "Pass the query string on to this target" on, only the target's own part is masked, not the event's. A URL with a path is masked as before. As for paths and userinfo, no length floor applies.

Model: opus-5-5
---
 internal/delivery/target_redact.go      | 35 +++++++++++----------
 internal/delivery/target_redact_test.go | 42 +++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 16 deletions(-)

diff --git a/internal/delivery/target_redact.go b/internal/delivery/target_redact.go
index a9050db..267e584 100644
--- a/internal/delivery/target_redact.go
+++ b/internal/delivery/target_redact.go
@@ -162,18 +162,22 @@ func targetSecrets(t *database.Target) []string {
 }
 
 // urlSecrets returns the substrings of a destination URL that
-// must not survive into a rendered page: the whole URL, the
-// parts of it MaskURL elides, and any userinfo.
+// must not survive into a rendered page: the whole URL; its
+// path, unless that is empty or "/"; its query string, and the
+// request URI that carries it, which a remote echoing the
+// request line shows even when the URL has no path; and its
+// userinfo and password.
 //
-// No length floor is applied to the path, and none to the
-// userinfo. A short path or a four-byte username is treated as
-// a credential exactly like a long one, because the field takes
-// an arbitrary URL and no part of it can be assumed non-secret —
-// the same rule MaskURL applies. headerSecrets does carry a
-// floor, and the difference is deliberate: a header is picked
-// out by a name-shaped guess and its value may be ordinary
-// text, whereas a URL's path and userinfo are credential
-// material by position.
+// No length floor is applied to the path, the query string or
+// the userinfo. A short path or a four-byte username is
+// treated as a credential exactly like a long one, because the
+// field takes an arbitrary URL and no part of it can be
+// assumed non-secret — the same rule MaskURL applies.
+// headerSecrets does carry a floor, and the difference is
+// deliberate: a header is picked out by a name-shaped guess
+// and its value may be ordinary text, whereas a URL's path,
+// query string and userinfo are credential material by
+// position.
 func urlSecrets(raw string) []string {
 	raw = strings.TrimSpace(raw)
 	if raw == "" {
@@ -188,12 +192,11 @@ func urlSecrets(raw string) []string {
 	}
 
 	if parsed.Path != "" && parsed.Path != "/" {
-		requestURI := parsed.RequestURI()
-		secrets = append(secrets, requestURI)
+		secrets = append(secrets, parsed.EscapedPath())
+	}
 
-		if escaped := parsed.EscapedPath(); escaped != requestURI {
-			secrets = append(secrets, escaped)
-		}
+	if parsed.RawQuery != "" {
+		secrets = append(secrets, parsed.RequestURI(), parsed.RawQuery)
 	}
 
 	if parsed.User != nil {
diff --git a/internal/delivery/target_redact_test.go b/internal/delivery/target_redact_test.go
index e28e302..52613de 100644
--- a/internal/delivery/target_redact_test.go
+++ b/internal/delivery/target_redact_test.go
@@ -202,6 +202,48 @@ func TestRedactor_RemovesHTTPURLQueryAndUserinfo(t *testing.T) {
 	}
 }
 
+// TestRedactor_RemovesEchoedQueryOfURLWithoutPath covers an
+// HTTP target URL whose credential is all in its query string.
+// Written with or without the "/", the request line sends it
+// as "/?token=…", and a target passing the event's query string
+// on sends that after an "&". The event's part stays visible:
+// the event's page shows it anyway.
+func TestRedactor_RemovesEchoedQueryOfURLWithoutPath(t *testing.T) {
+	t.Parallel()
+
+	const secret = "s3cr3t"
+
+	marker := delivery.RedactionMarker
+
+	// An echoed request line, and what the event log shows of it.
+	echoes := map[string]string{
+		"POST /?token=" + secret + " HTTP/1.1": "POST " + marker +
+			" HTTP/1.1",
+		"POST ?token=" + secret + " HTTP/1.1": "POST ?" + marker +
+			" HTTP/1.1",
+		"POST /?token=" + secret + "&a=1&b=2 HTTP/1.1": "POST " +
+			marker + "&a=1&b=2 HTTP/1.1",
+		"POST ?token=" + secret + "&a=1&b=2 HTTP/1.1": "POST ?" +
+			marker + "&a=1&b=2 HTTP/1.1",
+	}
+
+	for _, dest := range []string{
+		"https://example.com/?token=" + secret,
+		"https://example.com?token=" + secret,
+	} {
+		r := delivery.NewRedactor(&database.Target{
+			Type:   database.TargetTypeHTTP,
+			Config: `{"url":"` + dest + `"}`,
+		})
+
+		for echoed, want := range echoes {
+			assert.Equal(
+				t, want, r.Redact(echoed), "%s: %s", dest, echoed,
+			)
+		}
+	}
+}
+
 // TestRedactor_LeavesUnrelatedTextAlone pins that the
 // redactor matches literally: it does not guess at what a
 // secret looks like, so ordinary response content survives.