From ecfbfddc7ca14a30ff1bb6f790a09f509bf3437f Mon Sep 17 00:00:00 2001
From: Aaron Weiker <aweiker@forge.weiker.org>
Date: Mon, 11 May 2026 04:52:41 -0700
Subject: [PATCH] fix(gitea): redact query params from retry warning logs

Addresses security review finding: retry warnings were logging the full
request URL which could inadvertently leak sensitive query parameters
if future callers pass them.

Added redactURL() helper that:
- Strips query parameters from URLs before logging (replaces with [redacted])
- Returns [invalid URL] for unparseable URLs to avoid leaking any data
- Preserves the base path for debugging context

The error itself (lastErr) is kept as-is since APIError.Error() already
truncates response bodies to 200 chars, and network errors don't contain
user-controlled data.
---
 gitea/client.go      | 20 ++++++++++++++++++--
 gitea/client_test.go | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/gitea/client.go b/gitea/client.go
index 3ca101c..5826958 100644
--- a/gitea/client.go
+++ b/gitea/client.go
@@ -286,6 +286,22 @@ func isRetriableSyscallError(err error) bool {
 	return true
 }
 
+// redactURL strips query parameters from a URL for safe logging.
+// This prevents accidental exposure of sensitive data that future callers
+// might pass via query strings.
+func redactURL(rawURL string) string {
+	parsed, err := url.Parse(rawURL)
+	if err != nil {
+		// If we cannot parse it, return a safe placeholder rather than
+		// potentially logging something sensitive.
+		return "[invalid URL]"
+	}
+	if parsed.RawQuery != "" {
+		parsed.RawQuery = "[redacted]"
+	}
+	return parsed.String()
+}
+
 // doGet performs an HTTP GET request with retry on 5xx errors and temporary
 // network errors. Retries up to 3 times with exponential backoff (1s, 2s delays
 // by default; configurable via Client.RetryBackoff for testing).
@@ -316,7 +332,7 @@ func (c *Client) doGet(ctx context.Context, reqURL string) ([]byte, error) {
 			if delay > 0 {
 				slog.Warn("retrying request after error",
 					"attempt", attempt+1,
-					"url", reqURL,
+					"url", redactURL(reqURL),
 					"delay", delay.String(),
 					"lastError", lastErr)
 
@@ -343,7 +359,7 @@ func (c *Client) doGet(ctx context.Context, reqURL string) ([]byte, error) {
 			if attempt < maxAttempts-1 && isTemporaryNetError(err) {
 				slog.Warn("temporary network error, will retry",
 					"attempt", attempt+1,
-					"url", reqURL,
+					"url", redactURL(reqURL),
 					"error", err)
 				lastErr = err
 				continue
diff --git a/gitea/client_test.go b/gitea/client_test.go
index dc747fa..4578ad4 100644
--- a/gitea/client_test.go
+++ b/gitea/client_test.go
@@ -995,3 +995,45 @@ func TestIsRetriableSyscallError(t *testing.T) {
 		})
 	}
 }
+
+func TestRedactURL(t *testing.T) {
+	tests := []struct {
+		name  string
+		input string
+		want  string
+	}{
+		{
+			name:  "no query params",
+			input: "https://gitea.example.com/api/v1/repos/owner/repo/pulls/1",
+			want:  "https://gitea.example.com/api/v1/repos/owner/repo/pulls/1",
+		},
+		{
+			name:  "with query params - redacts",
+			input: "https://gitea.example.com/api/v1/repos/owner/repo/raw/file?ref=main",
+			want:  "https://gitea.example.com/api/v1/repos/owner/repo/raw/file?[redacted]",
+		},
+		{
+			name:  "multiple query params",
+			input: "https://example.com/path?token=secret&page=1",
+			want:  "https://example.com/path?[redacted]",
+		},
+		{
+			name:  "invalid URL",
+			input: "://invalid",
+			want:  "[invalid URL]",
+		},
+		{
+			name:  "empty string",
+			input: "",
+			want:  "",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := redactURL(tt.input)
+			if got != tt.want {
+				t.Errorf("redactURL(%q) = %q, want %q", tt.input, got, tt.want)
+			}
+		})
+	}
+}