client: add RedactQueryParams to hide query string from logs and errors

client.go

@@ -432,6 +432,13 @@ type Client struct {
 	// PrepareRetry can prepare the request for retry operation, for example re-sign it
 	PrepareRetry PrepareRetry
 
+	// RedactQueryParams, when true, strips the query string from request URLs
+	// before they appear in logs and returned errors. This is useful because
+	// query parameters often carry credentials (tokens, signatures, etc.) and
+	// leaking them into error messages or log output can be a security hazard.
+	// Userinfo passwords are always redacted regardless of this setting.
+	RedactQueryParams bool
+
 	loggerInit sync.Once
 	clientInit sync.Once
 }
@@ -675,9 +682,9 @@ func (c *Client) Do(req *Request) (*http.Response, error) {
 	if logger != nil {
 		switch v := logger.(type) {
 		case LeveledLogger:
-			v.Debug("performing request", "method", req.Method, "url", redactURL(req.URL))
+			v.Debug("performing request", "method", req.Method, "url", c.loggableURL(req.URL))
 		case Logger:
-			v.Printf("[DEBUG] %s %s", req.Method, redactURL(req.URL))
+			v.Printf("[DEBUG] %s %s", req.Method, c.loggableURL(req.URL))
 		}
 	}
 
@@ -732,9 +739,9 @@ func (c *Client) Do(req *Request) (*http.Response, error) {
 		if err != nil {
 			switch v := logger.(type) {
 			case LeveledLogger:
-				v.Error("request failed", "error", err, "method", req.Method, "url", redactURL(req.URL))
+				v.Error("request failed", "error", err, "method", req.Method, "url", c.loggableURL(req.URL))
 			case Logger:
-				v.Printf("[ERR] %s %s request failed: %v", req.Method, redactURL(req.URL), err)
+				v.Printf("[ERR] %s %s request failed: %v", req.Method, c.loggableURL(req.URL), err)
 			}
 		} else {
 			// Call this here to maintain the behavior of logging all requests,
@@ -770,7 +777,7 @@ func (c *Client) Do(req *Request) (*http.Response, error) {
 
 		wait := c.Backoff(c.RetryWaitMin, c.RetryWaitMax, i, resp)
 		if logger != nil {
-			desc := fmt.Sprintf("%s %s", req.Method, redactURL(req.URL))
+			desc := fmt.Sprintf("%s %s", req.Method, c.loggableURL(req.URL))
 			if resp != nil {
 				desc = fmt.Sprintf("%s (status: %d)", desc, resp.StatusCode)
 			}
@@ -835,11 +842,11 @@ func (c *Client) Do(req *Request) (*http.Response, error) {
 	// communicate why
 	if err == nil {
 		return nil, fmt.Errorf("%s %s giving up after %d attempt(s)",
-			req.Method, redactURL(req.URL), attempt)
+			req.Method, c.loggableURL(req.URL), attempt)
 	}
 
 	return nil, fmt.Errorf("%s %s giving up after %d attempt(s): %w",
-		req.Method, redactURL(req.URL), attempt, err)
+		req.Method, c.loggableURL(req.URL), attempt, err)
 }
 
 // Try to read the response body so we can reuse this connection.
@@ -926,6 +933,15 @@ func (c *Client) StandardClient() *http.Client {
 // Taken from url.URL#Redacted() which was introduced in go 1.15.
 // We can switch to using it directly if we'll bump the minimum required go version.
 func redactURL(u *url.URL) string {
+	return redactURLFor(u, false)
+}
+
+// redactURLFor produces a log- and error-safe rendering of u. Userinfo
+// passwords are always redacted; when stripQuery is true the raw query and
+// fragment are also stripped so that secrets embedded in query parameters
+// (e.g. presigned-URL signatures, bearer tokens) don't leak through logs or
+// returned errors.
+func redactURLFor(u *url.URL, stripQuery bool) string {
 	if u == nil {
 		return ""
 	}
@@ -934,5 +950,16 @@ func redactURL(u *url.URL) string {
 	if _, has := ru.User.Password(); has {
 		ru.User = url.UserPassword(ru.User.Username(), "xxxxx")
 	}
+	if stripQuery {
+		ru.RawQuery = ""
+		ru.Fragment = ""
+		ru.RawFragment = ""
+	}
 	return ru.String()
 }
+
+// loggableURL returns the URL as it should appear in logs and error messages
+// for this client, honoring Client.RedactQueryParams.
+func (c *Client) loggableURL(u *url.URL) string {
+	return redactURLFor(u, c.RedactQueryParams)
+}

client_test.go

@@ -1403,3 +1403,96 @@ func TestClient_RedirectWithBody(t *testing.T) {
 		t.Fatalf("Expected the client to be redirected 2 times, got: %d", atomic.LoadInt32(&redirects))
 	}
 }
+
+func TestRedactURLFor(t *testing.T) {
+	tests := []struct {
+		name       string
+		raw        string
+		stripQuery bool
+		want       string
+	}{
+		{
+			name:       "plain_url_kept",
+			raw:        "https://example.com/a",
+			stripQuery: false,
+			want:       "https://example.com/a",
+		},
+		{
+			name:       "password_always_redacted",
+			raw:        "https://user:secret@example.com/a",
+			stripQuery: false,
+			want:       "https://user:xxxxx@example.com/a",
+		},
+		{
+			name:       "query_kept_when_strip_false",
+			raw:        "https://example.com/a?token=hunter2",
+			stripQuery: false,
+			want:       "https://example.com/a?token=hunter2",
+		},
+		{
+			name:       "query_stripped_when_strip_true",
+			raw:        "https://example.com/a?token=hunter2",
+			stripQuery: true,
+			want:       "https://example.com/a",
+		},
+		{
+			name:       "fragment_stripped_when_strip_true",
+			raw:        "https://example.com/a?k=v#frag",
+			stripQuery: true,
+			want:       "https://example.com/a",
+		},
+		{
+			name:       "password_and_query_both_redacted",
+			raw:        "https://user:secret@example.com/a?token=hunter2",
+			stripQuery: true,
+			want:       "https://user:xxxxx@example.com/a",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			u, err := url.Parse(tc.raw)
+			if err != nil {
+				t.Fatalf("parse: %v", err)
+			}
+			got := redactURLFor(u, tc.stripQuery)
+			if got != tc.want {
+				t.Fatalf("redactURLFor(%q, %v) = %q, want %q", tc.raw, tc.stripQuery, got, tc.want)
+			}
+		})
+	}
+
+	// Nil-safe path.
+	if got := redactURLFor(nil, true); got != "" {
+		t.Fatalf("redactURLFor(nil, true) = %q, want \"\"", got)
+	}
+}
+
+func TestClient_RedactQueryParams_InError(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusInternalServerError)
+	}))
+	defer ts.Close()
+
+	client := NewClient()
+	client.RetryWaitMin = 10 * time.Millisecond
+	client.RetryWaitMax = 10 * time.Millisecond
+	client.RetryMax = 1
+	client.RedactQueryParams = true
+
+	req, err := NewRequest("GET", ts.URL+"/api?token=supersecret", nil)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	_, err = client.Do(req)
+	if err == nil {
+		t.Fatalf("expected an error after retries are exhausted")
+	}
+	if strings.Contains(err.Error(), "supersecret") {
+		t.Fatalf("error message leaked query secret: %q", err.Error())
+	}
+	if strings.Contains(err.Error(), "token=") {
+		t.Fatalf("error message still contains query params: %q", err.Error())
+	}
+}