Skip to content

Commit 58f040e

Browse files
baiyuqingclaude
andcommitted
Redact bearer tokens and cloud secrets from --debug HTTP dumps
DebugTransport in internal/service/cloud previously called httputil.DumpRequestOut(r, true) and httputil.DumpResponse(resp, true) whenever TICLOUD_DEBUG=1 (set by --debug). The dumped output included: * Authorization: Bearer <token> headers added by BearTokenTransport before the request reached DebugTransport. * Digest auth Authorization metadata from NewDigestTransport, which wraps DebugTransport. * Full JSON request bodies for S3/OSS/GCS/Azure import, export, and audit-log cloud-storage-config commands, including AWS/OSS secret access keys, GCS service account keys, and Azure SAS tokens. Anyone running `ticloud --debug <cmd>` or piping logs to a shared location could leak long-lived cloud credentials and short-lived bearer tokens. This is the "[High]" finding in the AKSK logging security review. Fix * New package internal/service/cloud/redact with DumpRequestOut and DumpResponse that mirror httputil's body=true behaviour but: - replace sensitive header values (Authorization / Proxy-Authorization / Cookie / Set-Cookie) with "[REDACTED]", preserving the Bearer/Digest/Basic scheme prefix so the auth method is still visible in logs; - parse JSON bodies and recursively replace values for case-insensitive sensitive keys (secret, secretAccessKey, accessKeySecret, serviceAccountKey, sasToken, private-key, oauth-client-secret, client_secret, access_token, refresh_token, password, token, and underscored variants); - replace non-JSON bodies entirely with a length-only placeholder, guarding pre-signed URLs and form-encoded credentials; - restore the original headers and a fresh body reader on the request/response so the wire-level HTTP exchange is unchanged. * DebugTransport.RoundTrip now calls redact.DumpRequestOut / redact.DumpResponse instead of the raw httputil helpers. No public API or CLI flag changes; only --debug stdout output is affected. Scope and out-of-scope Only the two "[High]" findings are addressed. The "[Medium]" issues (config describe / config set raw value printing, Resty SetDebug on auth client) and the "[Low/Latent]" generator-template issues remain to be handled in follow-up changes per their severity. Tests * internal/service/cloud/redact/redact_test.go (19 cases, 90.7% coverage): Bearer / Digest auth, Cookie, Proxy-Authorization, S3 / OSS / GCS / Azure / audit-log bodies, deeply nested JSON, arrays of secrets, case-insensitive key matching, non-JSON body redaction, empty body, body-still-readable round-trip, and assertions that non-sensitive identifiers (clusterId, displayName, access key id) remain visible. * internal/service/cloud/api_client_debug_test.go (4 cases): end-to- end through BearTokenTransport + DebugTransport against an httptest server with TICLOUD_DEBUG=1, capturing os.Stdout to assert that: - raw bearer tokens and response access_tokens never appear; - access key ids (identifiers, not secrets) remain visible; - debug off produces no stdout output; - the body delivered to the upstream server is byte-identical to the original payload (no behavioural drift on the wire). All tests pass under `make test` (-race -count=1). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 19aea6a commit 58f040e

4 files changed

Lines changed: 854 additions & 3 deletions

File tree

internal/service/cloud/api_client.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,11 @@ import (
2020
"fmt"
2121
"io"
2222
"net/http"
23-
"net/http/httputil"
2423
"os"
2524

2625
"github.com/tidbcloud/tidbcloud-cli/internal/config"
2726
"github.com/tidbcloud/tidbcloud-cli/internal/prop"
27+
"github.com/tidbcloud/tidbcloud-cli/internal/service/cloud/redact"
2828
"github.com/tidbcloud/tidbcloud-cli/internal/version"
2929
"github.com/tidbcloud/tidbcloud-cli/pkg/tidbcloud/v1beta1/iam"
3030
"github.com/tidbcloud/tidbcloud-cli/pkg/tidbcloud/v1beta1/serverless/auditlog"
@@ -854,7 +854,7 @@ func (dt *DebugTransport) RoundTrip(r *http.Request) (*http.Response, error) {
854854
debug := os.Getenv(config.DebugEnv) == "true" || os.Getenv(config.DebugEnv) == "1"
855855

856856
if debug {
857-
dump, err := httputil.DumpRequestOut(r, true)
857+
dump, err := redact.DumpRequestOut(r)
858858
if err != nil {
859859
return nil, err
860860
}
@@ -867,7 +867,7 @@ func (dt *DebugTransport) RoundTrip(r *http.Request) (*http.Response, error) {
867867
}
868868

869869
if debug {
870-
dump, err := httputil.DumpResponse(resp, true)
870+
dump, err := redact.DumpResponse(resp)
871871
if err != nil {
872872
return resp, err
873873
}
Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
// Copyright 2026 PingCAP, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package cloud
16+
17+
import (
18+
"bytes"
19+
"context"
20+
"io"
21+
"net/http"
22+
"net/http/httptest"
23+
"os"
24+
"strings"
25+
"testing"
26+
27+
"github.com/tidbcloud/tidbcloud-cli/internal/config"
28+
)
29+
30+
// captureStdout swaps os.Stdout for a pipe while fn runs and returns the
31+
// captured bytes. Not safe for concurrent use; these tests must not run in
32+
// parallel.
33+
func captureStdout(t *testing.T, fn func()) string {
34+
t.Helper()
35+
old := os.Stdout
36+
r, w, err := os.Pipe()
37+
if err != nil {
38+
t.Fatalf("os.Pipe: %v", err)
39+
}
40+
os.Stdout = w
41+
42+
done := make(chan struct{})
43+
var buf bytes.Buffer
44+
go func() {
45+
_, _ = io.Copy(&buf, r)
46+
close(done)
47+
}()
48+
49+
fn()
50+
51+
_ = w.Close()
52+
<-done
53+
os.Stdout = old
54+
_ = r.Close()
55+
return buf.String()
56+
}
57+
58+
func TestDebugTransport_RedactsBearerToken(t *testing.T) {
59+
t.Setenv(config.DebugEnv, "1")
60+
61+
const token = "leakyBearerTokenABCDEF.payload.signature"
62+
63+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
64+
w.Header().Set("Content-Type", "application/json")
65+
w.Header().Set("Set-Cookie", "sid=responseLeakCookie; HttpOnly")
66+
w.WriteHeader(http.StatusOK)
67+
_, _ = w.Write([]byte(`{"access_token":"responseAccessTokenSecret","user":"alice"}`))
68+
}))
69+
defer srv.Close()
70+
71+
transport := NewBearTokenTransport(token)
72+
client := &http.Client{Transport: transport}
73+
74+
out := captureStdout(t, func() {
75+
req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, srv.URL+"/v1/x", nil)
76+
if err != nil {
77+
t.Fatalf("NewRequest: %v", err)
78+
}
79+
resp, err := client.Do(req)
80+
if err != nil {
81+
t.Fatalf("client.Do: %v", err)
82+
}
83+
_, _ = io.Copy(io.Discard, resp.Body)
84+
_ = resp.Body.Close()
85+
})
86+
87+
if strings.Contains(out, token) {
88+
t.Fatalf("raw bearer token leaked to stdout:\n%s", out)
89+
}
90+
if !strings.Contains(out, "Bearer [REDACTED]") {
91+
t.Fatalf("expected 'Bearer [REDACTED]' in stdout, got:\n%s", out)
92+
}
93+
if strings.Contains(out, "responseAccessTokenSecret") {
94+
t.Fatalf("response access_token leaked to stdout:\n%s", out)
95+
}
96+
if strings.Contains(out, "responseLeakCookie") {
97+
t.Fatalf("Set-Cookie value leaked to stdout:\n%s", out)
98+
}
99+
}
100+
101+
func TestDebugTransport_NoDebug_NoOutput(t *testing.T) {
102+
t.Setenv(config.DebugEnv, "")
103+
104+
const token = "shouldNotBePrintedToken"
105+
106+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
107+
w.WriteHeader(http.StatusOK)
108+
}))
109+
defer srv.Close()
110+
111+
client := &http.Client{Transport: NewBearTokenTransport(token)}
112+
113+
out := captureStdout(t, func() {
114+
req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, srv.URL, nil)
115+
resp, err := client.Do(req)
116+
if err != nil {
117+
t.Fatalf("client.Do: %v", err)
118+
}
119+
_ = resp.Body.Close()
120+
})
121+
122+
if out != "" {
123+
t.Fatalf("expected no stdout output when debug off, got %q", out)
124+
}
125+
}
126+
127+
func TestDebugTransport_RedactsS3ImportSecret(t *testing.T) {
128+
t.Setenv(config.DebugEnv, "1")
129+
130+
const (
131+
token = "bearerForS3Test"
132+
secretAccessValue = "wJalrXUtnFEMI_S3_SECRET_LEAK_CANARY"
133+
)
134+
135+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
136+
// drain the body so DumpRequestOut sees something to dump
137+
_, _ = io.Copy(io.Discard, r.Body)
138+
w.Header().Set("Content-Type", "application/json")
139+
w.WriteHeader(http.StatusOK)
140+
_, _ = w.Write([]byte(`{"id":"imp-1"}`))
141+
}))
142+
defer srv.Close()
143+
144+
body := bytes.NewBufferString(
145+
`{"source":{"type":"S3","s3":{"uri":"s3://b/k","accessKey":{"id":"AKIATEST","secret":"` + secretAccessValue + `"}}}}`,
146+
)
147+
148+
client := &http.Client{Transport: NewBearTokenTransport(token)}
149+
150+
out := captureStdout(t, func() {
151+
req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, srv.URL+"/v1/imports", body)
152+
if err != nil {
153+
t.Fatalf("NewRequest: %v", err)
154+
}
155+
req.Header.Set("Content-Type", "application/json")
156+
resp, err := client.Do(req)
157+
if err != nil {
158+
t.Fatalf("client.Do: %v", err)
159+
}
160+
_, _ = io.Copy(io.Discard, resp.Body)
161+
_ = resp.Body.Close()
162+
})
163+
164+
if strings.Contains(out, secretAccessValue) {
165+
t.Fatalf("S3 secret leaked to stdout:\n%s", out)
166+
}
167+
if !strings.Contains(out, "AKIATEST") {
168+
t.Fatalf("access key id should remain visible (it is an identifier, not a secret):\n%s", out)
169+
}
170+
if strings.Contains(out, token) {
171+
t.Fatalf("bearer token leaked to stdout:\n%s", out)
172+
}
173+
}
174+
175+
func TestDebugTransport_BodyForwardedToServer(t *testing.T) {
176+
t.Setenv(config.DebugEnv, "1")
177+
178+
const payload = `{"clusterId":"c-1","secret":"x"}`
179+
180+
var received []byte
181+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
182+
b, _ := io.ReadAll(r.Body)
183+
received = b
184+
w.WriteHeader(http.StatusOK)
185+
}))
186+
defer srv.Close()
187+
188+
client := &http.Client{Transport: NewBearTokenTransport("t")}
189+
190+
captureStdout(t, func() {
191+
req, _ := http.NewRequestWithContext(
192+
context.Background(),
193+
http.MethodPost,
194+
srv.URL+"/",
195+
bytes.NewBufferString(payload),
196+
)
197+
req.Header.Set("Content-Type", "application/json")
198+
resp, err := client.Do(req)
199+
if err != nil {
200+
t.Fatalf("client.Do: %v", err)
201+
}
202+
_ = resp.Body.Close()
203+
})
204+
205+
if string(received) != payload {
206+
t.Fatalf("server received %q, want %q (debug dump must not alter wire body)", string(received), payload)
207+
}
208+
}

0 commit comments

Comments
 (0)