rodin/review-bot
2
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 6 Wiki Activity

fix: address NIT findings from sonnet review (#113)
PR Ready Gate / clear-labels (pull_request) Successful in 1s
Details
CI / test (pull_request) Successful in 17s
Details
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 35s
Details
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m7s
Details
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m32s
Details

Browse Source 
- Fix AllowInsecureHTTP doc comment: 'silently ignored' -> 'ignored' (it logs a warning)
- Remove unnecessary nil guard in redactURL (assigning nil to nil is a no-op)
- Add clarifying comment about acceptable double URL parse in doRequest
This commit is contained in:
claw
2026-05-13 12:48:02 -07:00
parent 4c032a3b53
commit 6b7f3f6924
1 changed files with 6 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
github/client.go
+5 -3
View File
@@ -152,7 +152,7 @@ type clientConfig struct {
// AllowInsecureHTTP permits sending credentials over plaintext HTTP connections. // AllowInsecureHTTP permits sending credentials over plaintext HTTP connections.
// In production, this option is gated by the REVIEW_BOT_ALLOW_INSECURE=1 // In production, this option is gated by the REVIEW_BOT_ALLOW_INSECURE=1
// environment variable. Without the env var set, the option is silently ignored // environment variable. Without the env var set, the option is ignored
// and a warning is logged. // and a warning is logged.
// //
// For tests, use AllowInsecureHTTPForTest (defined in export_test.go) which bypasses the env gate. // For tests, use AllowInsecureHTTPForTest (defined in export_test.go) which bypasses the env gate.
@@ -267,9 +267,8 @@ func redactURL(rawURL string) string {
if err != nil { if err != nil {
return "<unparseable URL>" return "<unparseable URL>"
} }
if u.User != nil {
u.User = nil u.User = nil
}
if u.RawQuery != "" { if u.RawQuery != "" {
u.RawQuery = "<redacted>" u.RawQuery = "<redacted>"
} }
@@ -281,6 +280,9 @@ func redactURL(rawURL string) string {
// It respects the Retry-After header when present, supporting both integer // It respects the Retry-After header when present, supporting both integer
// seconds and HTTP-date formats (capped at maxRetryAfter). // seconds and HTTP-date formats (capped at maxRetryAfter).
func (c *Client) doRequest(ctx context.Context, method, reqURL string, accept string) ([]byte, error) { func (c *Client) doRequest(ctx context.Context, method, reqURL string, accept string) ([]byte, error) {
// NOTE: This parses reqURL a second time (http.NewRequestWithContext parses it
// again internally). Acceptable cost: URL parsing is cheap and threading the
// parsed *url.URL through would complicate the interface for negligible gain.
if !c.allowInsecureHTTP { if !c.allowInsecureHTTP {
parsed, err := url.Parse(reqURL) parsed, err := url.Parse(reqURL)
if err != nil { if err != nil {