Remove secrets from presigned URL when logging

Author: adi-benz

client.go

@@ -658,7 +658,7 @@ 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", redactedURL{req.URL})
 		case Logger:
 			v.Printf("[DEBUG] %s %s", req.Method, redactURL(req.URL))
 		}
@@ -715,7 +715,7 @@ 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", redactedURL{req.URL})
 			case Logger:
 				v.Printf("[ERR] %s %s request failed: %v", req.Method, redactURL(req.URL), err)
 			}
@@ -910,16 +910,3 @@ 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 {
-	if u == nil {
-		return ""
-	}
-
-	ru := *u
-	if _, has := ru.User.Password(); has {
-		ru.User = url.UserPassword(ru.User.Username(), "xxxxx")
-	}
-	return ru.String()
-}

redact_url.go

@@ -0,0 +1,46 @@
+package retryablehttp
+
+import (
+	"log/slog"
+	"net/url"
+)
+
+type redactedURL struct{ url *url.URL }
+
+func (r redactedURL) LogValue() slog.Value {
+	return slog.StringValue(r.String())
+}
+
+func (r *redactedURL) String() string {
+	return redactURL(r.url)
+}
+
+var sensitiveParams = []string{
+	"X-Amz-Signature",
+	"X-Amz-Security-Token",
+	"X-Amz-Credential",
+}
+
+func redactURL(u *url.URL) string {
+	if u == nil {
+		return ""
+	}
+
+	ru := *u
+
+	// 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.
+	if _, has := ru.User.Password(); has {
+		ru.User = url.UserPassword(ru.User.Username(), "xxxxx")
+	}
+
+	params := ru.Query()
+	for _, param := range sensitiveParams {
+		if params.Has(param) {
+			params.Set(param, "REDACTED")
+		}
+	}
+	ru.RawQuery = params.Encode()
+
+	return ru.String()
+}

redact_url_test.go

@@ -0,0 +1,53 @@
+package retryablehttp
+
+import (
+	"bytes"
+	"log/slog"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"sync/atomic"
+	"testing"
+)
+
+func TestPresignedURLRedacted(t *testing.T) {
+	ts := serveFailOnceServer()
+	defer ts.Close()
+
+	logBuffer, logger := createBuffLogger()
+	client := NewClient()
+	client.Logger = logger
+
+	resp, err := client.Get(ts.URL + "?X-Amz-Credential=SECRET&X-Amz-Signature=SECRET&X-Amz-Security-Token=SECRET")
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	resp.Body.Close()
+
+	actualLogs := string(logBuffer.Bytes())
+	if strings.Contains(actualLogs, "SECRET") {
+		t.Fatalf("log contains SECRET that should have been redacted: %s", actualLogs)
+	}
+}
+
+func serveFailOnceServer() *httptest.Server {
+	var retries int32 = 0
+	return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if atomic.LoadInt32(&retries) == 0 {
+			w.WriteHeader(500)
+		} else {
+			w.WriteHeader(200)
+		}
+
+		atomic.AddInt32(&retries, 1)
+	}))
+}
+
+func createBuffLogger() (bytes.Buffer, *slog.Logger) {
+	logBuffer := bytes.Buffer{}
+	textHandler := slog.NewTextHandler(&logBuffer, &slog.HandlerOptions{
+		Level: slog.LevelDebug,
+	})
+	logger := slog.New(textHandler)
+	return logBuffer, logger
+}