Skip to content

fix(payment): harden EasyPay callbacks and refund parsing - #1989

Closed
18848298290-del wants to merge 3 commits into
TokenFlux:mainfrom
18848298290-del:fix/easypay-v1-callback-refund
Closed

18848298290-del wants to merge 3 commits into
TokenFlux:mainfrom
18848298290-del:fix/easypay-v1-callback-refund

Conversation

@18848298290-del

Copy link
Copy Markdown

Closes #1988

Summary

  • Accept common EasyPay V1 notification extension fields.
  • Continue rejecting checkout-only, unknown, duplicate, and ambiguous callback parameters.
  • Confirm successful callbacks through an upstream order query before local fulfillment.
  • Validate the upstream payment status, amount, order number, and trade_no.
  • Support JSON and plain-text EasyPay refund success responses.
  • Preserve the callback-forgery protections from 支付系统漏洞 Wei-Shaw/sub2api#7875 and sub2API新支付漏洞-已提交修复 Wei-Shaw/sub2api#8075.

Tests

  • go test ./internal/payment/provider
  • go test ./internal/payment ./internal/payment/httpapi
  • Relevant payment integration tests
  • git diff --check

@smartcmd smartcmd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes for two reproducible regressions: ambiguous refund text can finalize a failed refund, and upstream confirmation can prevent acknowledgement of callbacks for missing local orders. Reproduction details are attached inline.

I would also like to discuss the refund response policy before merging. Inferring financial success from prefixes such as 已退款 is too heuristic: that text can describe an earlier refund, a limit, or a rejected repeat request. Please clarify which gateway response formats this adapter supports and how each establishes that the current refund succeeded. JSON should follow the supported gateway's documented status codes. If plain-text compatibility is needed, it should recognize a narrowly defined complete success format and check any reported amount against the request. Adding more success/failure keywords would make this increasingly fragile.

Unrecognized or contradictory replies also need a defined recovery path. An unknown outcome does not justify either finalizing success or automatically restoring the deducted balance, because the external refund may already have happened. Please discuss how these cases should enter reconciliation or manual verification. Can the reported gateway provide a structured response, and if it cannot, what exact text formats does it guarantee?

Validation: existing payment provider, core, HTTP, PostgreSQL adapter, cross-module tests, and the PostgreSQL payment integration suite passed in an isolated snapshot. Scoped golangci-lint, declaration-order, formatter, and git diff checks passed. Additional regression probes for both findings fail on this head and pass against base f60e793; the refund probe also verifies committed PostgreSQL balance and order state. These probes use local HTTP fixtures and do not establish that a particular deployed gateway emits the constructed failure message. CI and Security Scan remain action_required at review time.

Please also complete the calling-rule and design comments for ConfirmNotification, NotificationConfirmer, and webhookVerifyNotificationWithProviders under the repository's code-comments requirements.

Comment on lines +859 to +861
return strings.HasPrefix(normalized, "退款成功") ||
strings.HasPrefix(normalized, "退款已成功") ||
strings.HasPrefix(normalized, "已退款")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Do not treat a previous-refund or failure diagnostic as success for this attempt

The unrestricted prefix match accepts both 已退款,请勿重复退款 and {"code":0,"msg":"已退款金额超过订单金额,本次退款失败"}. I reproduced this with a local HTTP fixture through EasyPay.Refund. In the PostgreSQL workflow test, a request to refund 50 from a balance of 100 returned Success: true, committed balance 50, and marked the order REFUNDED, despite the fixture reporting failure. With the base implementation, the same fixture restored balance 100 and kept the order COMPLETED.

Please use explicit success semantics for the supported JSON dialects and narrowly defined, full-message formats for supported plain-text responses, checking any returned amount against the requested refund. A prior refund or ambiguous message does not prove this attempt succeeded. Indeterminate outcomes need reconciliation or manual verification; adding failure-word exclusions to this prefix heuristic would leave the underlying ambiguity.

Comment on lines +204 to +206
if confirmer, ok := verifiedProvider.(payment.NotificationConfirmer); ok &&
notification.Status == payment.NotificationStatusSuccess {
if err := confirmer.ConfirmNotification(c.Request.Context(), notification); err != nil {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Handle a missing local order before requiring upstream confirmation

This confirmation runs before HandlePaymentNotification performs the local order lookup. If an accepted callback names a missing local order and the upstream lookup fails, the handler now returns 502 confirm failed before reaching the existing ErrOrderNotFound acknowledgement below. This can keep obsolete callbacks retrying even though there is no local order to fulfill.

A handler regression test with one enabled merchant, an accepted success notification, a classified missing local order, and a failing confirmer returns 502 on this head; the base returns 200 success.

After signature verification, resolve the local order and preserve the missing-order acknowledgement before querying upstream. Genuine database failures should remain retryable, and existing orders that need payment confirmation should retain the upstream check.

@smartcmd

Copy link
Copy Markdown
Contributor

这个问题我来处理吧

@smartcmd

Copy link
Copy Markdown
Contributor

已经解决了,请查收

@smartcmd smartcmd closed this Oct 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug/Security] 易支付 V1 扩展回调不到账及纯文本退款成功误判,修复需保留伪造回调防护

2 participants