core/go-infra
8
0
Fork 0
Code Issues 4 Pull requests 1 Projects Releases Packages Wiki Activity Actions

fix(coderabbit): address review findings

Browse source 
- Truncate response body in error messages to 256 bytes to prevent
  sensitive data leakage from upstream APIs (client.go)
- Replace unchecked strings.Split index with safe repoShortName helper
  to prevent potential panic (cmd/monitor)
- Fix test assertions to match coreerr.E error format after prior refactor

Co-Authored-By: Virgil <virgil@lethean.io>
This commit is contained in:
Snider 2026-03-17 16:24:37 +00:00
parent 2217aad6f3
commit ec1a591b71
5 changed files with 36 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

19
client.go
View file

@ -144,7 +144,7 @@ func (a *APIClient) Do(req *http.Request, result any) error {
// Server errors are retryable.
if resp.StatusCode >= 500 {
lastErr = coreerr.E(a.prefix, fmt.Sprintf("HTTP %d: %s", resp.StatusCode, string(data)), nil)
lastErr = coreerr.E(a.prefix, fmt.Sprintf("HTTP %d: %s", resp.StatusCode, truncateBody(data, maxErrBodyLen)), nil)
if attempt < attempts-1 {
a.backoff(attempt, req)
}
@ -153,7 +153,7 @@ func (a *APIClient) Do(req *http.Request, result any) error {
// Client errors (4xx, except 429 handled above) are not retried.
if resp.StatusCode >= 400 {
return coreerr.E(a.prefix, fmt.Sprintf("HTTP %d: %s", resp.StatusCode, string(data)), nil)
return coreerr.E(a.prefix, fmt.Sprintf("HTTP %d: %s", resp.StatusCode, truncateBody(data, maxErrBodyLen)), nil)
}
// Success — decode if requested.
@ -228,7 +228,7 @@ func (a *APIClient) DoRaw(req *http.Request) ([]byte, error) {
}
if resp.StatusCode >= 500 {
lastErr = coreerr.E(a.prefix, fmt.Sprintf("HTTP %d: %s", resp.StatusCode, string(data)), nil)
lastErr = coreerr.E(a.prefix, fmt.Sprintf("HTTP %d: %s", resp.StatusCode, truncateBody(data, maxErrBodyLen)), nil)
if attempt < attempts-1 {
a.backoff(attempt, req)
}
@ -236,7 +236,7 @@ func (a *APIClient) DoRaw(req *http.Request) ([]byte, error) {
}
if resp.StatusCode >= 400 {
return nil, coreerr.E(a.prefix, fmt.Sprintf("HTTP %d: %s", resp.StatusCode, string(data)), nil)
return nil, coreerr.E(a.prefix, fmt.Sprintf("HTTP %d: %s", resp.StatusCode, truncateBody(data, maxErrBodyLen)), nil)
}
return data, nil
@ -261,6 +261,17 @@ func (a *APIClient) backoff(attempt int, req *http.Request) {
}
}
// maxErrBodyLen is the maximum number of bytes from a response body included in error messages.
const maxErrBodyLen = 256
// truncateBody limits response body length in error messages to prevent sensitive data leakage.
func truncateBody(data []byte, max int) string {
if len(data) <= max {
return string(data)
}
return string(data[:max]) + "...(truncated)"
}
// parseRetryAfter interprets the Retry-After header value.
// Supports seconds (integer) format. Falls back to 1 second.
func parseRetryAfter(val string) time.Duration {

6
client_test.go
View file

@ -137,7 +137,7 @@ func TestAPIClient_Do_Bad_ClientError(t *testing.T) {
err = c.Do(req, nil)
assert.Error(t, err)
assert.Contains(t, err.Error(), "test-api 404")
assert.Contains(t, err.Error(), "test-api: HTTP 404")
assert.Contains(t, err.Error(), "not found")
}
@ -226,7 +226,7 @@ func TestAPIClient_Do_Bad_ExhaustsRetries(t *testing.T) {
err = c.Do(req, nil)
assert.Error(t, err)
assert.Contains(t, err.Error(), "exhaust-test 500")
assert.Contains(t, err.Error(), "exhaust-test: HTTP 500")
// 1 initial + 2 retries = 3 attempts
assert.Equal(t, int32(3), attempts.Load())
}
@ -476,7 +476,7 @@ func TestAPIClient_DoRaw_Bad_ClientError(t *testing.T) {
_, err = c.DoRaw(req)
assert.Error(t, err)
assert.Contains(t, err.Error(), "raw-test 403")
assert.Contains(t, err.Error(), "raw-test: HTTP 403")
}
func TestAPIClient_DoRaw_Good_RetriesServerError(t *testing.T) {

4
cloudns_test.go
View file

@ -78,7 +78,7 @@ func TestCloudNSClient_DoRaw_Bad_HTTPError(t *testing.T) {
_, err = client.doRaw(req)
assert.Error(t, err)
assert.Contains(t, err.Error(), "cloudns API 403")
assert.Contains(t, err.Error(), "cloudns API: HTTP 403")
}
func TestCloudNSClient_DoRaw_Bad_ServerError(t *testing.T) {
@ -102,7 +102,7 @@ func TestCloudNSClient_DoRaw_Bad_ServerError(t *testing.T) {
_, err = client.doRaw(req)
assert.Error(t, err)
assert.Contains(t, err.Error(), "cloudns API 500")
assert.Contains(t, err.Error(), "cloudns API: HTTP 500")
}
// --- Zone JSON parsing ---

17
cmd/monitor/cmd_monitor.go
View file

@ -210,7 +210,7 @@ func resolveRepos() ([]string, error) {
func fetchRepoFindings(repoFullName string) ([]Finding, []string) {
var findings []Finding
var errs []string
repoName := strings.Split(repoFullName, "/")[1]
repoName := repoShortName(repoFullName)
// Fetch code scanning alerts
codeFindings, err := fetchCodeScanningAlerts(repoFullName)
@ -264,7 +264,7 @@ func fetchCodeScanningAlerts(repoFullName string) ([]Finding, error) {
return nil, log.E("monitor.fetchCodeScanning", "failed to parse response", err)
}
repoName := strings.Split(repoFullName, "/")[1]
repoName := repoShortName(repoFullName)
var findings []Finding
for _, alert := range alerts {
if alert.State != "open" {
@ -318,7 +318,7 @@ func fetchDependabotAlerts(repoFullName string) ([]Finding, error) {
return nil, log.E("monitor.fetchDependabot", "failed to parse response", err)
}
repoName := strings.Split(repoFullName, "/")[1]
repoName := repoShortName(repoFullName)
var findings []Finding
for _, alert := range alerts {
if alert.State != "open" {
@ -369,7 +369,7 @@ func fetchSecretScanningAlerts(repoFullName string) ([]Finding, error) {
return nil, log.E("monitor.fetchSecretScanning", "failed to parse response", err)
}
repoName := strings.Split(repoFullName, "/")[1]
repoName := repoShortName(repoFullName)
var findings []Finding
for _, alert := range alerts {
if alert.State != "open" {
@ -530,6 +530,15 @@ func outputTable(findings []Finding) error {
return nil
}
// repoShortName extracts the repo name from "org/repo" format.
// Returns the full string if no "/" is present.
func repoShortName(fullName string) string {
if i := strings.LastIndex(fullName, "/"); i >= 0 {
return fullName[i+1:]
}
return fullName
}
// truncate truncates a string to max runes (Unicode-safe)
func truncate(s string, max int) string {
runes := []rune(s)

6
hetzner_test.go
View file

@ -116,7 +116,7 @@ func TestHCloudClient_Do_Bad_APIError(t *testing.T) {
var result struct{}
err := client.get(context.Background(), "/servers", &result)
assert.Error(t, err)
assert.Contains(t, err.Error(), "hcloud API 403")
assert.Contains(t, err.Error(), "hcloud API: HTTP 403")
}
func TestHCloudClient_Do_Bad_APIErrorNoJSON(t *testing.T) {
@ -136,7 +136,7 @@ func TestHCloudClient_Do_Bad_APIErrorNoJSON(t *testing.T) {
err := client.get(context.Background(), "/servers", nil)
assert.Error(t, err)
assert.Contains(t, err.Error(), "hcloud API 500")
assert.Contains(t, err.Error(), "hcloud API: HTTP 500")
}
func TestHCloudClient_Do_Good_NilResult(t *testing.T) {
@ -218,7 +218,7 @@ func TestHRobotClient_Get_Bad_HTTPError(t *testing.T) {
err := client.get(context.Background(), "/server", nil)
assert.Error(t, err)
assert.Contains(t, err.Error(), "hrobot API 401")
assert.Contains(t, err.Error(), "hrobot API: HTTP 401")
}
// --- Type serialisation ---