fix(gitea): redact query params from retry warning logs
PR Ready Gate / clear-labels (pull_request) Successful in 1s
CI / test (pull_request) Successful in 17s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 34s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m14s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m17s
PR Ready Gate / clear-labels (pull_request) Successful in 1s
CI / test (pull_request) Successful in 17s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 34s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m14s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m17s
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.
This commit is contained in:
+18
-2
@@ -286,6 +286,22 @@ func isRetriableSyscallError(err error) bool {
|
|||||||
return true
|
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
|
// 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
|
// network errors. Retries up to 3 times with exponential backoff (1s, 2s delays
|
||||||
// by default; configurable via Client.RetryBackoff for testing).
|
// 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 {
|
if delay > 0 {
|
||||||
slog.Warn("retrying request after error",
|
slog.Warn("retrying request after error",
|
||||||
"attempt", attempt+1,
|
"attempt", attempt+1,
|
||||||
"url", reqURL,
|
"url", redactURL(reqURL),
|
||||||
"delay", delay.String(),
|
"delay", delay.String(),
|
||||||
"lastError", lastErr)
|
"lastError", lastErr)
|
||||||
|
|
||||||
@@ -343,7 +359,7 @@ func (c *Client) doGet(ctx context.Context, reqURL string) ([]byte, error) {
|
|||||||
if attempt < maxAttempts-1 && isTemporaryNetError(err) {
|
if attempt < maxAttempts-1 && isTemporaryNetError(err) {
|
||||||
slog.Warn("temporary network error, will retry",
|
slog.Warn("temporary network error, will retry",
|
||||||
"attempt", attempt+1,
|
"attempt", attempt+1,
|
||||||
"url", reqURL,
|
"url", redactURL(reqURL),
|
||||||
"error", err)
|
"error", err)
|
||||||
lastErr = err
|
lastErr = err
|
||||||
continue
|
continue
|
||||||
|
|||||||
@@ -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)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user