Compare commits

..

3 commits

Author SHA1 Message Date
504003c47e Merge pull request 'Fix CodeRabbit findings' (#8) from agent/fix-coderabbit-findings--verify-each-aga into dev
Reviewed-on: #8
2026-03-24 11:43:34 +00:00
Snider
1cecf00148 Merge origin/dev into agent/fix-coderabbit-findings--verify-each-aga
Resolve conflict in hetzner_test.go: keep PR's CodeRabbit fixes
alongside dev's new test functions (load balancer, snapshot, GetServer).

Co-Authored-By: Virgil <virgil@lethean.io>
2026-03-24 11:41:09 +00:00
Snider
ec1a591b71 fix(coderabbit): address review findings
- 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>
2026-03-17 16:24:37 +00:00
2 changed files with 28 additions and 8 deletions

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 {

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)