From 235f710853b3e44b5b2fa236cfefd0b9d0c2a044 Mon Sep 17 00:00:00 2001 From: erio Date: Sun, 19 Apr 2026 01:46:50 +0800 Subject: [PATCH] feat(payment): redact provider secrets in admin config API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Admin GET /api/v1/admin/payment/providers previously returned every config value — including privateKey / apiV3Key / secretKey etc. — verbatim. Any future XSS on the admin UI would hand attackers the full set of production payment credentials, and the plaintext values sat unnecessarily in browser memory for every operator. Treat those fields as write-only from the admin surface: - decryptAndMaskConfig() strips sensitive keys from the GET response. The authoritative list is an explicit per-provider registry that mirrors the frontend's PROVIDER_CONFIG_FIELDS sensitive flag: alipay → privateKey, publicKey, alipayPublicKey wxpay → privateKey, apiV3Key, publicKey stripe → secretKey, webhookSecret (publishableKey stays plain) easypay → pkey Payment runtime still reads the full config via decryptConfig, so nothing at the gateway changes. - mergeConfig() treats an empty value for a sensitive key as "leave unchanged" — the admin UI omits unchanged secrets so operators can tweak non-sensitive settings without re-entering credentials. - Admin dialog (PaymentProviderDialog.vue): * secret inputs get autocomplete="new-password", data-1p-ignore, data-lpignore and data-bwignore so password managers do not offer to save provider credentials * in edit mode the required-field check skips sensitive fields (empty is the "keep existing" signal) and the placeholder shows "leave empty to keep" instead of the default example value * create mode still requires every non-optional field, including secrets, since there is nothing to preserve - Unit test renamed to TestIsSensitiveProviderConfigField, covers the per-provider registry and specifically asserts that Stripe's publishableKey is NOT treated as a secret. --- .../service/payment_config_providers.go | 78 +++++++++++++++---- .../service/payment_config_providers_test.go | 61 +++++++++------ .../payment/PaymentProviderDialog.vue | 23 ++++-- 3 files changed, 118 insertions(+), 44 deletions(-) diff --git a/backend/internal/service/payment_config_providers.go b/backend/internal/service/payment_config_providers.go index 8e470525..3d1e4dc4 100644 --- a/backend/internal/service/payment_config_providers.go +++ b/backend/internal/service/payment_config_providers.go @@ -52,7 +52,7 @@ func (s *PaymentConfigService) ListProviderInstancesWithConfig(ctx context.Conte AllowUserRefund: inst.AllowUserRefund, SortOrder: inst.SortOrder, PaymentMode: inst.PaymentMode, } - resp.Config, err = s.decryptAndMaskConfig(inst.Config) + resp.Config, err = s.decryptAndMaskConfig(inst.ProviderKey, inst.Config) if err != nil { return nil, fmt.Errorf("decrypt config for instance %d: %w", inst.ID, err) } @@ -61,8 +61,26 @@ func (s *PaymentConfigService) ListProviderInstancesWithConfig(ctx context.Conte return result, nil } -func (s *PaymentConfigService) decryptAndMaskConfig(encrypted string) (map[string]string, error) { - return s.decryptConfig(encrypted) +// decryptAndMaskConfig returns the stored config with sensitive fields omitted. +// Admin UIs display masked placeholders for these; the raw values never leave +// the server. Callers that need the full config (e.g. payment runtime) must +// use decryptConfig directly. +func (s *PaymentConfigService) decryptAndMaskConfig(providerKey, encrypted string) (map[string]string, error) { + cfg, err := s.decryptConfig(encrypted) + if err != nil { + return nil, err + } + if cfg == nil { + return nil, nil + } + masked := make(map[string]string, len(cfg)) + for k, v := range cfg { + if isSensitiveProviderConfigField(providerKey, k) { + continue + } + masked[k] = v + } + return masked, nil } // pendingOrderStatuses are order statuses considered "in progress". @@ -72,16 +90,27 @@ var pendingOrderStatuses = []string{ payment.OrderStatusRecharging, } -var sensitiveConfigPatterns = []string{"key", "pkey", "secret", "private", "password"} +// providerSensitiveConfigFields is the authoritative list of config keys that +// are treated as secrets per provider. Must stay in sync with the frontend +// definition at frontend/src/components/payment/providerConfig.ts +// (PROVIDER_CONFIG_FIELDS, fields with sensitive: true). +// +// Key matching is case-insensitive. Non-listed keys (e.g. appId, notifyUrl, +// stripe publishableKey) are returned in plaintext by the admin GET API. +var providerSensitiveConfigFields = map[string]map[string]struct{}{ + payment.TypeEasyPay: {"pkey": {}}, + payment.TypeAlipay: {"privatekey": {}, "publickey": {}, "alipaypublickey": {}}, + payment.TypeWxpay: {"privatekey": {}, "apiv3key": {}, "publickey": {}}, + payment.TypeStripe: {"secretkey": {}, "webhooksecret": {}}, +} -func isSensitiveConfigField(fieldName string) bool { - lower := strings.ToLower(fieldName) - for _, p := range sensitiveConfigPatterns { - if strings.Contains(lower, p) { - return true - } +func isSensitiveProviderConfigField(providerKey, fieldName string) bool { + fields, ok := providerSensitiveConfigFields[providerKey] + if !ok { + return false } - return false + _, found := fields[strings.ToLower(fieldName)] + return found } func (s *PaymentConfigService) countPendingOrders(ctx context.Context, providerInstanceID int64) (int, error) { @@ -137,10 +166,26 @@ func validateProviderRequest(providerKey, name, supportedTypes string) error { // NOTE: This function exceeds 30 lines due to per-field nil-check patch update // boilerplate and pending-order safety checks. func (s *PaymentConfigService) UpdateProviderInstance(ctx context.Context, id int64, req UpdateProviderInstanceRequest) (*dbent.PaymentProviderInstance, error) { + var cachedInst *dbent.PaymentProviderInstance + loadInst := func() (*dbent.PaymentProviderInstance, error) { + if cachedInst != nil { + return cachedInst, nil + } + inst, err := s.entClient.PaymentProviderInstance.Get(ctx, id) + if err != nil { + return nil, fmt.Errorf("load provider instance: %w", err) + } + cachedInst = inst + return inst, nil + } if req.Config != nil { + inst, err := loadInst() + if err != nil { + return nil, err + } hasSensitive := false - for k := range req.Config { - if isSensitiveConfigField(k) && req.Config[k] != "" { + for k, v := range req.Config { + if v != "" && isSensitiveProviderConfigField(inst.ProviderKey, k) { hasSensitive = true break } @@ -283,9 +328,14 @@ func (s *PaymentConfigService) mergeConfig(ctx context.Context, id int64, newCon return nil, fmt.Errorf("decrypt existing config for instance %d: %w", id, err) } if existing == nil { - return newConfig, nil + existing = map[string]string{} } for k, v := range newConfig { + // Preserve existing secrets when the client submits an empty value + // (admin UI omits the value to indicate "leave unchanged"). + if v == "" && isSensitiveProviderConfigField(inst.ProviderKey, k) { + continue + } existing[k] = v } return existing, nil diff --git a/backend/internal/service/payment_config_providers_test.go b/backend/internal/service/payment_config_providers_test.go index 2aaa874f..bc2a9b18 100644 --- a/backend/internal/service/payment_config_providers_test.go +++ b/backend/internal/service/payment_config_providers_test.go @@ -97,41 +97,52 @@ func TestValidateProviderRequest(t *testing.T) { } } -func TestIsSensitiveConfigField(t *testing.T) { +func TestIsSensitiveProviderConfigField(t *testing.T) { t.Parallel() tests := []struct { - field string - wantSen bool + providerKey string + field string + wantSen bool }{ - // Sensitive fields (contain key/secret/private/password/pkey patterns) - {"secretKey", true}, - {"apiSecret", true}, - {"pkey", true}, - {"privateKey", true}, - {"apiPassword", true}, - {"appKey", true}, - {"SECRET_TOKEN", true}, - {"PrivateData", true}, - {"PASSWORD", true}, - {"mySecretValue", true}, + // Stripe: publishableKey is public, only secretKey/webhookSecret are secrets + {"stripe", "secretKey", true}, + {"stripe", "webhookSecret", true}, + {"stripe", "SecretKey", true}, // case-insensitive + {"stripe", "publishableKey", false}, + {"stripe", "appId", false}, - // Non-sensitive fields - {"appId", false}, - {"mchId", false}, - {"apiBase", false}, - {"endpoint", false}, - {"merchantNo", false}, - {"paymentMode", false}, - {"notifyUrl", false}, + // Alipay + {"alipay", "privateKey", true}, + {"alipay", "publicKey", true}, + {"alipay", "alipayPublicKey", true}, + {"alipay", "appId", false}, + {"alipay", "notifyUrl", false}, + + // Wxpay + {"wxpay", "privateKey", true}, + {"wxpay", "apiV3Key", true}, + {"wxpay", "publicKey", true}, + {"wxpay", "publicKeyId", false}, + {"wxpay", "certSerial", false}, + {"wxpay", "mchId", false}, + + // EasyPay + {"easypay", "pkey", true}, + {"easypay", "pid", false}, + {"easypay", "apiBase", false}, + + // Unknown provider: never sensitive + {"unknown", "secretKey", false}, } for _, tc := range tests { - t.Run(tc.field, func(t *testing.T) { + tc := tc + t.Run(tc.providerKey+"/"+tc.field, func(t *testing.T) { t.Parallel() - got := isSensitiveConfigField(tc.field) - assert.Equal(t, tc.wantSen, got, "isSensitiveConfigField(%q)", tc.field) + got := isSensitiveProviderConfigField(tc.providerKey, tc.field) + assert.Equal(t, tc.wantSen, got, "isSensitiveProviderConfigField(%q, %q)", tc.providerKey, tc.field) }) } } diff --git a/frontend/src/components/payment/PaymentProviderDialog.vue b/frontend/src/components/payment/PaymentProviderDialog.vue index 10c1bfea..624ddcdd 100644 --- a/frontend/src/components/payment/PaymentProviderDialog.vue +++ b/frontend/src/components/payment/PaymentProviderDialog.vue @@ -88,13 +88,24 @@ v-model="config[field.key]" rows="3" class="input font-mono text-xs" + autocomplete="new-password" + data-1p-ignore + data-lpignore="true" + data-bwignore="true" + spellcheck="false" + :placeholder="editing ? t('admin.accounts.leaveEmptyToKeep') : ''" />