Merge pull request #1228 from stripe/vcheung/redact-api-keys-in-logs

Redact client_secret from logs

error.go

@@ -294,6 +294,27 @@ func (e *RateLimitError) Error() string {
 	return e.stripeErr.Error()
 }
 
+// redact returns a copy of the error object with sensitive fields replaced with
+// a placeholder value.
+func (e *Error) redact() *Error {
+	// Fast path, since this applies to most cases
+	if e.PaymentIntent == nil && e.SetupIntent == nil {
+		return e
+	}
+	errCopy := *e
+	if e.PaymentIntent != nil {
+		pi := *e.PaymentIntent
+		errCopy.PaymentIntent = &pi
+		errCopy.PaymentIntent.ClientSecret = "REDACTED"
+	}
+	if e.SetupIntent != nil {
+		si := *e.SetupIntent
+		errCopy.SetupIntent = &si
+		errCopy.SetupIntent.ClientSecret = "REDACTED"
+	}
+	return &errCopy
+}
+
 // rawError deserializes the outer JSON object returned in an error response
 // from the API.
 type rawError struct {

error_test.go

@@ -38,3 +38,50 @@ func TestErrorResponse(t *testing.T) {
 	assert.Equal(t, "req_123", stripeErr.RequestID)
 	assert.Equal(t, 401, stripeErr.HTTPStatusCode)
 }
+
+func TestErrorRedact(t *testing.T) {
+	pi := &PaymentIntent{Amount: int64(400), ClientSecret: "foo"}
+	si := &SetupIntent{Description: "keepme", ClientSecret: "foo"}
+
+	t.Run("BothIntentObjects", func(t *testing.T) {
+		err := &Error{PaymentIntent: pi, SetupIntent: si}
+		redacted := err.redact()
+		assert.Equal(t, int64(400), err.PaymentIntent.Amount)
+		assert.Equal(t, int64(400), redacted.PaymentIntent.Amount)
+		assert.Equal(t, "keepme", err.SetupIntent.Description)
+		assert.Equal(t, "keepme", redacted.SetupIntent.Description)
+		assert.Equal(t, "foo", err.PaymentIntent.ClientSecret)
+		assert.Equal(t, "foo", err.SetupIntent.ClientSecret)
+		assert.Equal(t, "foo", pi.ClientSecret)
+		assert.Equal(t, "foo", si.ClientSecret)
+		assert.Equal(t, "REDACTED", redacted.PaymentIntent.ClientSecret)
+		assert.Equal(t, "REDACTED", redacted.SetupIntent.ClientSecret)
+	})
+
+	t.Run("NeitherIntentObject", func(t *testing.T) {
+		err := Error{PaymentIntent: nil, SetupIntent: nil}
+		redacted := err.redact()
+		assert.Nil(t, err.PaymentIntent)
+		assert.Nil(t, redacted.PaymentIntent)
+	})
+
+	t.Run("PaymentIntentAlone", func(t *testing.T) {
+		err := &Error{PaymentIntent: pi}
+		redacted := err.redact()
+		assert.Equal(t, int64(400), err.PaymentIntent.Amount)
+		assert.Equal(t, int64(400), redacted.PaymentIntent.Amount)
+		assert.Equal(t, "foo", err.PaymentIntent.ClientSecret)
+		assert.Equal(t, "foo", pi.ClientSecret)
+		assert.Equal(t, "REDACTED", redacted.PaymentIntent.ClientSecret)
+	})
+
+	t.Run("SetupIntentAlone", func(t *testing.T) {
+		err := &Error{SetupIntent: si}
+		redacted := err.redact()
+		assert.Equal(t, "keepme", err.SetupIntent.Description)
+		assert.Equal(t, "keepme", redacted.SetupIntent.Description)
+		assert.Equal(t, "foo", err.SetupIntent.ClientSecret)
+		assert.Equal(t, "foo", si.ClientSecret)
+		assert.Equal(t, "REDACTED", redacted.SetupIntent.ClientSecret)
+	})
+}

stripe.go

@@ -460,10 +460,10 @@ func (s *BackendImplementation) Do(req *http.Request, body *bytes.Buffer, v Last
 				// and uses it in a broader sense.
 				if res.StatusCode == 402 {
 					s.LeveledLogger.Infof("User-compelled request error from Stripe (status %v): %v",
-						res.StatusCode, stripeErr)
+						res.StatusCode, stripeErr.redact())
 				} else {
 					s.LeveledLogger.Errorf("Request error from Stripe (status %v): %v",
-						res.StatusCode, stripeErr)
+						res.StatusCode, stripeErr.redact())
 				}
 			} else {
 				s.LeveledLogger.Errorf("Error decoding error from Stripe: %v", err)

stripe_test.go

@@ -648,6 +648,53 @@ func TestDo_TelemetryEnabledNoDataRace(t *testing.T) {
 	assert.Equal(t, int32(times), requestNum)
 }
 
+func TestDo_Redaction(t *testing.T) {
+	type testServerResponse struct {
+		Error *Error `json:"error"`
+	}
+
+	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+
+		w.WriteHeader(402)
+		data, err := json.Marshal(testServerResponse{Error: &Error{PaymentIntent: &PaymentIntent{ClientSecret: "SHOULDBEREDACTED"}}})
+		assert.NoError(t, err)
+
+		_, err = w.Write(data)
+		assert.NoError(t, err)
+
+	}))
+	defer testServer.Close()
+
+	var logs bytes.Buffer
+	logger := &LeveledLogger{Level: LevelDebug, stderrOverride: &logs, stdoutOverride: &logs}
+
+	backend := GetBackendWithConfig(
+		APIBackend,
+		&BackendConfig{
+			EnableTelemetry:   Bool(true),
+			LeveledLogger:     logger,
+			MaxNetworkRetries: Int64(0),
+			URL:               String(testServer.URL),
+		},
+	).(*BackendImplementation)
+
+	request, err := backend.NewRequest(
+		http.MethodGet,
+		"/hello",
+		"sk_test_123",
+		"application/x-www-form-urlencoded",
+		nil,
+	)
+	assert.NoError(t, err)
+
+	var response Charge
+	err = backend.Do(request, nil, &response)
+	assert.Error(t, err)
+
+	assert.NotContains(t, logs.String(), "SHOULDBEREDACTED")
+	assert.Contains(t, logs.String(), "REDACTED")
+}
+
 func TestFormatURLPath(t *testing.T) {
 	assert.Equal(t, "/v1/resources/1/subresources/2",
 		FormatURLPath("/v1/resources/%s/subresources/%s", "1", "2"))