From 30c165033e0172637926f53bd6d5abbd73b25724 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 19 Jan 2026 04:50:48 +0000 Subject: [PATCH] Address all security review comments: sanitize remaining error messages and handle errors properly Co-authored-by: krau <71133316+krau@users.noreply.github.com> --- api/handlers.go | 16 ++++++++++------ api/middleware.go | 7 ++++++- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/api/handlers.go b/api/handlers.go index 17e82a3..017ac62 100644 --- a/api/handlers.go +++ b/api/handlers.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "encoding/json" - "fmt" "io" "net/http" "path" @@ -171,7 +170,7 @@ func handleCreateTask(w http.ResponseWriter, r *http.Request) { }) if err != nil { logger.Errorf("Failed to create task: %v", err) - respondError(w, fmt.Sprintf("failed to create task: %v", err), http.StatusInternalServerError) + respondError(w, "failed to create task", http.StatusInternalServerError) return } @@ -182,7 +181,7 @@ func handleCreateTask(w http.ResponseWriter, r *http.Request) { if err := core.AddTask(injectCtx, task); err != nil { logger.Errorf("Failed to add task: %v", err) updateTaskStatus(taskID, "failed", err.Error()) - respondError(w, fmt.Sprintf("failed to add task: %v", err), http.StatusInternalServerError) + respondError(w, "failed to add task to queue", http.StatusInternalServerError) return } @@ -242,7 +241,8 @@ func handleCancelTask(w http.ResponseWriter, r *http.Request) { } if err := core.CancelTask(r.Context(), taskID); err != nil { - respondError(w, fmt.Sprintf("failed to cancel task: %v", err), http.StatusInternalServerError) + log.FromContext(r.Context()).Errorf("Failed to cancel task %s: %v", taskID, err) + respondError(w, "failed to cancel task", http.StatusInternalServerError) return } @@ -365,7 +365,11 @@ func sendWebhook(taskID, status, errorMsg string) { defer resp.Body.Close() if resp.StatusCode >= 400 { - body, _ := io.ReadAll(resp.Body) - log.Errorf("Webhook returned error status %d: %s", resp.StatusCode, string(body)) + body, err := io.ReadAll(resp.Body) + if err != nil { + log.Errorf("Webhook returned error status %d, failed to read response body: %v", resp.StatusCode, err) + } else { + log.Errorf("Webhook returned error status %d: %s", resp.StatusCode, string(body)) + } } } diff --git a/api/middleware.go b/api/middleware.go index 3b3a92f..feeb533 100644 --- a/api/middleware.go +++ b/api/middleware.go @@ -91,7 +91,12 @@ func getClientIP(r *http.Request) string { } // Fall back to RemoteAddr - ip, _, _ := net.SplitHostPort(r.RemoteAddr) + ip, _, err := net.SplitHostPort(r.RemoteAddr) + if err != nil { + // If SplitHostPort fails, RemoteAddr might not have a port + // In this case, just return RemoteAddr as is + return r.RemoteAddr + } return ip }