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 {

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.