From 6f620f6731287fa8b1594831194fe127f534b3b4 Mon Sep 17 00:00:00 2001 From: Quentin Rousseau Date: Wed, 15 Jul 2026 11:06:04 -0700 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=94=92=EF=B8=8F=20fix:=20redact=20Aut?= =?UTF-8?q?horization=20header=20in=20debug=20transport=20output?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Debug mode (`--debug` / `ROOTLY_DEBUG`) was dumping full HTTP requests including the raw Authorization header (API key or OAuth token) to stderr. In CI pipelines and log aggregation systems this exposes credentials in plaintext. Clones the request before dumping and replaces the Authorization value with `Bearer [REDACTED]`. The original request with the real token is still sent to the server unchanged. Adds 3 regression tests: - Redaction appears in debug output - Real token still reaches the server - No spurious redaction when no auth header present 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- internal/api/client.go | 7 ++- internal/api/client_test.go | 113 ++++++++++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 1 deletion(-) diff --git a/internal/api/client.go b/internal/api/client.go index 7cc5a4b..6957804 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -35,7 +35,12 @@ type debugTransport struct { } func (dt *debugTransport) RoundTrip(req *http.Request) (*http.Response, error) { - dump, _ := httputil.DumpRequestOut(req, true) + safeReq := req.Clone(req.Context()) + if auth := safeReq.Header.Get("Authorization"); auth != "" { + safeReq.Header.Set("Authorization", "Bearer [REDACTED]") + } + safeReq.Body = req.Body + dump, _ := httputil.DumpRequestOut(safeReq, true) fmt.Fprintf(os.Stderr, "\n--- DEBUG REQUEST ---\n%s\n", dump) resp, err := dt.transport.RoundTrip(req) diff --git a/internal/api/client_test.go b/internal/api/client_test.go index e46a526..4eb7fdb 100644 --- a/internal/api/client_test.go +++ b/internal/api/client_test.go @@ -1,10 +1,14 @@ package api import ( + "bytes" "context" "encoding/json" + "io" "net/http" "net/http/httptest" + "os" + "strings" "testing" "github.com/rootlyhq/rootly-cli/internal/config" @@ -221,3 +225,112 @@ func TestParseTimePtr(t *testing.T) { func strPtr(s string) *string { return &s } + +func TestDebugTransportRedactsAuthorization(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + var buf bytes.Buffer + dt := &debugTransport{transport: http.DefaultTransport} + + req, err := http.NewRequest("GET", server.URL+"/v1/incidents", nil) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Authorization", "Bearer super-secret-token-12345") + + oldStderr := os.Stderr + r, w, _ := os.Pipe() + os.Stderr = w + + _, err = dt.RoundTrip(req) + if err != nil { + t.Fatalf("RoundTrip failed: %v", err) + } + + w.Close() + io.Copy(&buf, r) + os.Stderr = oldStderr + + output := buf.String() + if strings.Contains(output, "super-secret-token-12345") { + t.Error("debug output contains raw API token — Authorization header must be redacted") + } + if !strings.Contains(output, "Bearer [REDACTED]") { + t.Error("debug output should contain 'Bearer [REDACTED]'") + } + if !strings.Contains(output, "DEBUG REQUEST") { + t.Error("debug output should contain request dump marker") + } +} + +func TestDebugTransportPreservesActualAuthHeader(t *testing.T) { + var receivedAuth string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + receivedAuth = r.Header.Get("Authorization") + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + dt := &debugTransport{transport: http.DefaultTransport} + + req, err := http.NewRequest("GET", server.URL+"/v1/incidents", nil) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Authorization", "Bearer real-token") + + oldStderr := os.Stderr + _, w, _ := os.Pipe() + os.Stderr = w + + _, err = dt.RoundTrip(req) + w.Close() + os.Stderr = oldStderr + + if err != nil { + t.Fatalf("RoundTrip failed: %v", err) + } + + if receivedAuth != "Bearer real-token" { + t.Errorf("server received Authorization = %q, want %q", receivedAuth, "Bearer real-token") + } +} + +func TestDebugTransportNoAuthHeader(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + var buf bytes.Buffer + dt := &debugTransport{transport: http.DefaultTransport} + + req, err := http.NewRequest("GET", server.URL+"/v1/incidents", nil) + if err != nil { + t.Fatal(err) + } + + oldStderr := os.Stderr + r, w, _ := os.Pipe() + os.Stderr = w + + _, err = dt.RoundTrip(req) + if err != nil { + t.Fatalf("RoundTrip failed: %v", err) + } + + w.Close() + io.Copy(&buf, r) + os.Stderr = oldStderr + + output := buf.String() + if strings.Contains(output, "[REDACTED]") { + t.Error("should not contain [REDACTED] when no Authorization header is set") + } + if !strings.Contains(output, "DEBUG REQUEST") { + t.Error("debug output should still contain request dump marker") + } +} From d47bd9d208501b961fa2b6691f55347c975e16b3 Mon Sep 17 00:00:00 2001 From: Quentin Rousseau Date: Wed, 15 Jul 2026 11:10:56 -0700 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=90=9B=20fix:=20prevent=20body=20cons?= =?UTF-8?q?umption=20and=20fix=20lint=20in=20debug=20transport?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address Greptile review feedback: DumpRequestOut with body=true was consuming the shared request body via Clone's shallow copy, causing POST/PUT/PATCH requests to arrive empty at the server. Now dumps headers only (body=false) with http.NoBody on the clone. Also fix lint: close response bodies, use http.NoBody, use NewRequestWithContext in tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- internal/api/client.go | 4 ++-- internal/api/client_test.go | 15 +++++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/internal/api/client.go b/internal/api/client.go index 6957804..82ecd84 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -39,8 +39,8 @@ func (dt *debugTransport) RoundTrip(req *http.Request) (*http.Response, error) { if auth := safeReq.Header.Get("Authorization"); auth != "" { safeReq.Header.Set("Authorization", "Bearer [REDACTED]") } - safeReq.Body = req.Body - dump, _ := httputil.DumpRequestOut(safeReq, true) + safeReq.Body = http.NoBody + dump, _ := httputil.DumpRequestOut(safeReq, false) fmt.Fprintf(os.Stderr, "\n--- DEBUG REQUEST ---\n%s\n", dump) resp, err := dt.transport.RoundTrip(req) diff --git a/internal/api/client_test.go b/internal/api/client_test.go index 4eb7fdb..db50d74 100644 --- a/internal/api/client_test.go +++ b/internal/api/client_test.go @@ -235,7 +235,7 @@ func TestDebugTransportRedactsAuthorization(t *testing.T) { var buf bytes.Buffer dt := &debugTransport{transport: http.DefaultTransport} - req, err := http.NewRequest("GET", server.URL+"/v1/incidents", nil) + req, err := http.NewRequestWithContext(context.Background(), "GET", server.URL+"/v1/incidents", http.NoBody) if err != nil { t.Fatal(err) } @@ -245,10 +245,11 @@ func TestDebugTransportRedactsAuthorization(t *testing.T) { r, w, _ := os.Pipe() os.Stderr = w - _, err = dt.RoundTrip(req) + resp, err := dt.RoundTrip(req) if err != nil { t.Fatalf("RoundTrip failed: %v", err) } + resp.Body.Close() w.Close() io.Copy(&buf, r) @@ -276,7 +277,7 @@ func TestDebugTransportPreservesActualAuthHeader(t *testing.T) { dt := &debugTransport{transport: http.DefaultTransport} - req, err := http.NewRequest("GET", server.URL+"/v1/incidents", nil) + req, err := http.NewRequestWithContext(context.Background(), "GET", server.URL+"/v1/incidents", http.NoBody) if err != nil { t.Fatal(err) } @@ -286,13 +287,14 @@ func TestDebugTransportPreservesActualAuthHeader(t *testing.T) { _, w, _ := os.Pipe() os.Stderr = w - _, err = dt.RoundTrip(req) + resp, err := dt.RoundTrip(req) w.Close() os.Stderr = oldStderr if err != nil { t.Fatalf("RoundTrip failed: %v", err) } + resp.Body.Close() if receivedAuth != "Bearer real-token" { t.Errorf("server received Authorization = %q, want %q", receivedAuth, "Bearer real-token") @@ -308,7 +310,7 @@ func TestDebugTransportNoAuthHeader(t *testing.T) { var buf bytes.Buffer dt := &debugTransport{transport: http.DefaultTransport} - req, err := http.NewRequest("GET", server.URL+"/v1/incidents", nil) + req, err := http.NewRequestWithContext(context.Background(), "GET", server.URL+"/v1/incidents", http.NoBody) if err != nil { t.Fatal(err) } @@ -317,10 +319,11 @@ func TestDebugTransportNoAuthHeader(t *testing.T) { r, w, _ := os.Pipe() os.Stderr = w - _, err = dt.RoundTrip(req) + resp, err := dt.RoundTrip(req) if err != nil { t.Fatalf("RoundTrip failed: %v", err) } + resp.Body.Close() w.Close() io.Copy(&buf, r)