Skip to content

Commit a7d39d9

Browse files
debidongdebidong
committed
fix: harden statuspage migration error handling
1 parent 2483a2f commit a7d39d9

2 files changed

Lines changed: 237 additions & 62 deletions

File tree

‎internal/cli/status_page_migrate.go‎

Lines changed: 109 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,33 @@ import (
1515
)
1616

1717
const migrationSourceAtlassian = "atlassian"
18+
const migrationErrorBodyLimit = 4096
19+
20+
var migrationSensitiveBodyKeys = map[string]struct{}{
21+
"apikey": {},
22+
"xapikey": {},
23+
"accesskey": {},
24+
"password": {},
25+
"passwd": {},
26+
"pwd": {},
27+
"token": {},
28+
"accesstoken": {},
29+
"refreshtoken": {},
30+
"idtoken": {},
31+
"sessiontoken": {},
32+
"authtoken": {},
33+
"oauthtoken": {},
34+
"bearertoken": {},
35+
"authorization": {},
36+
"auth": {},
37+
"secret": {},
38+
"clientsecret": {},
39+
"secretkey": {},
40+
"privatekey": {},
41+
"signingkey": {},
42+
"credential": {},
43+
"credentials": {},
44+
}
1845

1946
type statusPageMigrationService interface {
2047
StartStructure(ctx context.Context, sourceAPIKey, sourcePageID string) (*migrationStartResult, error)
@@ -173,16 +200,16 @@ func (a *statusPageMigrationAPI) do(ctx context.Context, method, path string, qu
173200

174201
resp, err := a.httpClient.Do(req)
175202
if err != nil {
176-
return fmt.Errorf("request failed: %w", err)
203+
return fmt.Errorf("request failed: %s", redactAppKey(err.Error(), a.appKey))
177204
}
178205
defer func() { _ = resp.Body.Close() }()
179206

180207
if resp.StatusCode != http.StatusOK {
181-
bodyBytes, readErr := io.ReadAll(resp.Body)
208+
bodyBytes, readErr := io.ReadAll(io.LimitReader(resp.Body, migrationErrorBodyLimit))
182209
if readErr != nil {
183210
return fmt.Errorf("API client error (HTTP %d)", resp.StatusCode)
184211
}
185-
return fmt.Errorf("API client error (HTTP %d): %s", resp.StatusCode, strings.TrimSpace(string(bodyBytes)))
212+
return fmt.Errorf("API client error (HTTP %d): %s", resp.StatusCode, sanitizeMigrationBody(string(bodyBytes)))
186213
}
187214

188215
if err := json.NewDecoder(resp.Body).Decode(out); err != nil {
@@ -207,6 +234,85 @@ func (a *statusPageMigrationAPI) do(ctx context.Context, method, path string, qu
207234
return nil
208235
}
209236

237+
func redactAppKey(message, appKey string) string {
238+
if appKey == "" {
239+
return message
240+
}
241+
242+
redacted := strings.ReplaceAll(message, appKey, "[redacted]")
243+
redacted = strings.ReplaceAll(redacted, url.QueryEscape(appKey), "[redacted]")
244+
return redacted
245+
}
246+
247+
func sanitizeMigrationBody(body string) string {
248+
if body == "" {
249+
return body
250+
}
251+
252+
var v any
253+
if err := json.Unmarshal([]byte(body), &v); err != nil {
254+
return body
255+
}
256+
257+
sanitized, redacted := sanitizeMigrationJSONValue(v)
258+
if !redacted {
259+
return body
260+
}
261+
262+
out, err := json.Marshal(sanitized)
263+
if err != nil {
264+
return body
265+
}
266+
return string(out)
267+
}
268+
269+
func sanitizeMigrationJSONValue(v any) (any, bool) {
270+
switch value := v.(type) {
271+
case map[string]any:
272+
sanitized := make(map[string]any, len(value))
273+
redacted := false
274+
for key, item := range value {
275+
if isMigrationSensitiveBodyKey(key) {
276+
sanitized[key] = "[REDACTED]"
277+
redacted = true
278+
continue
279+
}
280+
281+
sanitizedItem, itemRedacted := sanitizeMigrationJSONValue(item)
282+
sanitized[key] = sanitizedItem
283+
redacted = redacted || itemRedacted
284+
}
285+
return sanitized, redacted
286+
case []any:
287+
sanitized := make([]any, len(value))
288+
redacted := false
289+
for i, item := range value {
290+
sanitizedItem, itemRedacted := sanitizeMigrationJSONValue(item)
291+
sanitized[i] = sanitizedItem
292+
redacted = redacted || itemRedacted
293+
}
294+
return sanitized, redacted
295+
default:
296+
return v, false
297+
}
298+
}
299+
300+
func isMigrationSensitiveBodyKey(key string) bool {
301+
_, ok := migrationSensitiveBodyKeys[normalizeMigrationSensitiveBodyKey(key)]
302+
return ok
303+
}
304+
305+
func normalizeMigrationSensitiveBodyKey(key string) string {
306+
var b strings.Builder
307+
b.Grow(len(key))
308+
for _, r := range strings.ToLower(strings.TrimSpace(key)) {
309+
if (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') {
310+
b.WriteRune(r)
311+
}
312+
}
313+
return b.String()
314+
}
315+
210316
func newStatusPageMigrateCmd() *cobra.Command {
211317
cmd := &cobra.Command{
212318
Use: "migrate",

‎internal/cli/status_page_migrate_test.go‎

Lines changed: 128 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,27 @@ import (
44
"bytes"
55
"context"
66
"encoding/json"
7+
"fmt"
8+
"io"
79
"net/http"
8-
"net/http/httptest"
910
"strings"
1011
"testing"
1112
)
1213

14+
type roundTripperFunc func(*http.Request) (*http.Response, error)
15+
16+
func (f roundTripperFunc) RoundTrip(req *http.Request) (*http.Response, error) {
17+
return f(req)
18+
}
19+
20+
func jsonResponse(statusCode int, body string) *http.Response {
21+
return &http.Response{
22+
StatusCode: statusCode,
23+
Header: make(http.Header),
24+
Body: io.NopCloser(strings.NewReader(body)),
25+
}
26+
}
27+
1328
func TestStatusPageMigrationAPIStartStructure(t *testing.T) {
1429
t.Parallel()
1530

@@ -18,25 +33,21 @@ func TestStatusPageMigrationAPIStartStructure(t *testing.T) {
1833
var gotAppKey string
1934
var gotBody map[string]any
2035

21-
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
22-
gotMethod = r.Method
23-
gotPath = r.URL.Path
24-
gotAppKey = r.URL.Query().Get("app_key")
25-
if err := json.NewDecoder(r.Body).Decode(&gotBody); err != nil {
26-
t.Fatalf("decode body: %v", err)
27-
}
28-
w.Header().Set("Content-Type", "application/json")
29-
_ = json.NewEncoder(w).Encode(map[string]any{
30-
"data": map[string]any{"job_id": "job-1"},
31-
})
32-
}))
33-
defer ts.Close()
34-
3536
api := &statusPageMigrationAPI{
36-
httpClient: ts.Client(),
37-
baseURL: ts.URL,
38-
appKey: "fd-app-key",
39-
userAgent: "flashduty-cli/test",
37+
httpClient: &http.Client{
38+
Transport: roundTripperFunc(func(r *http.Request) (*http.Response, error) {
39+
gotMethod = r.Method
40+
gotPath = r.URL.Path
41+
gotAppKey = r.URL.Query().Get("app_key")
42+
if err := json.NewDecoder(r.Body).Decode(&gotBody); err != nil {
43+
t.Fatalf("decode body: %v", err)
44+
}
45+
return jsonResponse(http.StatusOK, `{"data":{"job_id":"job-1"}}`), nil
46+
}),
47+
},
48+
baseURL: "https://status.example.com",
49+
appKey: "fd-app-key",
50+
userAgent: "flashduty-cli/test",
4051
}
4152

4253
out, err := api.StartStructure(context.Background(), "atlassian-key", "page_123")
@@ -68,32 +79,18 @@ func TestStatusPageMigrationAPIGetStatus(t *testing.T) {
6879
var gotPath string
6980
var gotJobID string
7081

71-
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
72-
gotMethod = r.Method
73-
gotPath = r.URL.Path
74-
gotJobID = r.URL.Query().Get("job_id")
75-
w.Header().Set("Content-Type", "application/json")
76-
_ = json.NewEncoder(w).Encode(map[string]any{
77-
"data": map[string]any{
78-
"job_id": "job-2",
79-
"source_page_id": "src-1",
80-
"target_page_id": 1024,
81-
"phase": "history",
82-
"status": "running",
83-
"progress": map[string]any{
84-
"total_steps": 5,
85-
"completed_steps": 3,
86-
},
87-
},
88-
})
89-
}))
90-
defer ts.Close()
91-
9282
api := &statusPageMigrationAPI{
93-
httpClient: ts.Client(),
94-
baseURL: ts.URL,
95-
appKey: "fd-app-key",
96-
userAgent: "flashduty-cli/test",
83+
httpClient: &http.Client{
84+
Transport: roundTripperFunc(func(r *http.Request) (*http.Response, error) {
85+
gotMethod = r.Method
86+
gotPath = r.URL.Path
87+
gotJobID = r.URL.Query().Get("job_id")
88+
return jsonResponse(http.StatusOK, `{"data":{"job_id":"job-2","source_page_id":"src-1","target_page_id":1024,"phase":"history","status":"running","progress":{"total_steps":5,"completed_steps":3}}}`), nil
89+
}),
90+
},
91+
baseURL: "https://status.example.com",
92+
appKey: "fd-app-key",
93+
userAgent: "flashduty-cli/test",
9794
}
9895

9996
out, err := api.GetStatus(context.Background(), "job-2")
@@ -115,29 +112,101 @@ func TestStatusPageMigrationAPIGetStatus(t *testing.T) {
115112
}
116113
}
117114

115+
func TestStatusPageMigrationAPIRedactsAppKeyFromTransportError(t *testing.T) {
116+
t.Parallel()
117+
118+
api := &statusPageMigrationAPI{
119+
httpClient: &http.Client{
120+
Transport: roundTripperFunc(func(req *http.Request) (*http.Response, error) {
121+
return nil, fmt.Errorf("transport failed for %s", req.URL.String())
122+
}),
123+
},
124+
baseURL: "https://status.example.com",
125+
appKey: "secret-app-key",
126+
userAgent: "flashduty-cli/test",
127+
}
128+
129+
_, err := api.GetStatus(context.Background(), "job-4")
130+
if err == nil {
131+
t.Fatal("GetStatus() error = nil, want transport error")
132+
}
133+
if strings.Contains(err.Error(), "secret-app-key") {
134+
t.Fatalf("transport error leaked app key: %v", err)
135+
}
136+
}
137+
138+
func TestStatusPageMigrationAPICapsErrorBodyReads(t *testing.T) {
139+
t.Parallel()
140+
141+
largeBody := strings.Repeat("0123456789", 2000)
142+
143+
api := &statusPageMigrationAPI{
144+
httpClient: &http.Client{
145+
Transport: roundTripperFunc(func(r *http.Request) (*http.Response, error) {
146+
return jsonResponse(http.StatusBadGateway, largeBody), nil
147+
}),
148+
},
149+
baseURL: "https://status.example.com",
150+
appKey: "fd-app-key",
151+
userAgent: "flashduty-cli/test",
152+
}
153+
154+
_, err := api.GetStatus(context.Background(), "job-5")
155+
if err == nil {
156+
t.Fatal("GetStatus() error = nil, want HTTP error")
157+
}
158+
if got := len(err.Error()); got > 5000 {
159+
t.Fatalf("HTTP error too large: got %d chars, want <= 5000", got)
160+
}
161+
}
162+
163+
func TestStatusPageMigrationAPISanitizesErrorBodyFields(t *testing.T) {
164+
t.Parallel()
165+
166+
api := &statusPageMigrationAPI{
167+
httpClient: &http.Client{
168+
Transport: roundTripperFunc(func(r *http.Request) (*http.Response, error) {
169+
return jsonResponse(http.StatusBadGateway, `{"error":{"message":"upstream failed","details":{"ApiKey":"response-secret","nested":[{"ACCESS_TOKEN":"response-token"}]}}}`), nil
170+
}),
171+
},
172+
baseURL: "https://status.example.com",
173+
appKey: "fd-app-key",
174+
userAgent: "flashduty-cli/test",
175+
}
176+
177+
_, err := api.GetStatus(context.Background(), "job-6")
178+
if err == nil {
179+
t.Fatal("GetStatus() error = nil, want HTTP error")
180+
}
181+
if strings.Contains(err.Error(), "response-secret") || strings.Contains(err.Error(), "response-token") {
182+
t.Fatalf("HTTP error leaked secret body fields: %v", err)
183+
}
184+
if !strings.Contains(err.Error(), "[REDACTED]") {
185+
t.Fatalf("HTTP error missing redaction marker: %v", err)
186+
}
187+
}
188+
118189
func TestStatusPageMigrationAPICancel(t *testing.T) {
119190
t.Parallel()
120191

121192
var gotMethod string
122193
var gotPath string
123194
var gotBody map[string]any
124195

125-
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
126-
gotMethod = r.Method
127-
gotPath = r.URL.Path
128-
if err := json.NewDecoder(r.Body).Decode(&gotBody); err != nil {
129-
t.Fatalf("decode body: %v", err)
130-
}
131-
w.Header().Set("Content-Type", "application/json")
132-
_ = json.NewEncoder(w).Encode(map[string]any{"data": map[string]any{}})
133-
}))
134-
defer ts.Close()
135-
136196
api := &statusPageMigrationAPI{
137-
httpClient: ts.Client(),
138-
baseURL: ts.URL,
139-
appKey: "fd-app-key",
140-
userAgent: "flashduty-cli/test",
197+
httpClient: &http.Client{
198+
Transport: roundTripperFunc(func(r *http.Request) (*http.Response, error) {
199+
gotMethod = r.Method
200+
gotPath = r.URL.Path
201+
if err := json.NewDecoder(r.Body).Decode(&gotBody); err != nil {
202+
t.Fatalf("decode body: %v", err)
203+
}
204+
return jsonResponse(http.StatusOK, `{"data":{}}`), nil
205+
}),
206+
},
207+
baseURL: "https://status.example.com",
208+
appKey: "fd-app-key",
209+
userAgent: "flashduty-cli/test",
141210
}
142211

143212
if err := api.Cancel(context.Background(), "job-3"); err != nil {

0 commit comments

Comments
 (0)