Keep gateway error detail out of shopper-visible payment responses - #11
Merged
Conversation
The api_username rides in the query string of every GET call, so folding the request path into an exception message published half of the HTTP Basic credential pair into anything that reads it - the everypay log channel, and payment request responseData, which Sylius serializes to the shopper. Messages now quote the endpoint with the query string stripped. Text the client does not control is scrubbed as well: a transport exception quotes the full request URL back, and the gateway can echo the submitted username in an error body. The wire request is unchanged - the API still requires api_username in the URL.
Sylius serializes responseData to the shopper through the shop API, so the raw EveryPay exception message the capture and status handlers stored there published gateway error bodies - and, until the previous commit, the api_username with them - to whoever held the payment request hash. A single transient outage parked it there for good. Both handlers now store an EveryPayGateway::ERROR_* code, and fail() takes an $errorCode rather than free text so the sink cannot accept prose again. The detail is unchanged in the everypay log channel. Nothing in the plugin reads the error key, and neither the response provider nor any template renders it.
Redacting the wrapper's message was only half the job: log handlers and error trackers walk the chain and render a previous exception's message verbatim, and Symfony's transport exceptions quote the full request URL - api_username included. The credential kept reaching the everypay log through the link, after the message had dropped it. The transport exception's class and its (redacted) reason go into the message instead, so a timeout still reads differently from a refused connection. Nothing in the plugin called getPrevious().
3.6.8 was released today and breaks every functional test: its GenerateSchemaEventArgs::setSchema() needs the DBAL Schema::edit() API from doctrine/dbal ^4.5, symfony/doctrine-bridge calls it while building the messenger transport schema, and sylius/sylius requires doctrine/dbal ^3.9 - so the combination cannot be satisfied and prepareDatabase() throws before any test body runs. The highest-deps jobs picked it up as soon as it landed; 3.6.7 is fine. The constraint carries no lower bound on purpose. sylius/sylius allows doctrine/orm ^2.18 || ^3.5, and the lowest-deps jobs resolve the 2.x branch; adding a ^3.x floor here would force 3.x on them and change a graph that works. This entry only removes the one broken release. A require-dev entry rather than a conflict: nothing about the breakage is specific to this plugin, so consumers should keep resolving their own graph instead of inheriting our opinion. Drop the exclusion once a fixed 3.6.x ships.
acirtautas
force-pushed
the
harden-gateway-error-disclosure
branch
from
August 5, 2026 23:03
4c5bb40 to
964c2ad
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three related changes, one per commit, that stop EveryPay error detail — and the
api_usernamewith it — from reaching places it should not.Keep EveryPay credentials out of API client error messages
api_usernameis half of the HTTP Basic credential pair and rides in the query string of every GET call, so folding the request path into an exception message published it to anything that read the message.Exception messages now quote the endpoint with the query string stripped. Text the client does not control is scrubbed too: a transport exception quotes the full request URL back, and the gateway can echo the submitted username in an error body. The wire request is untouched — the API still requires
api_usernamein the URL, and the existing URL-building test still asserts it.Store a generic error indicator in payment request responseData
Sylius serializes
responseDatato the shopper through the shop API, so the raw exception message the capture and status handlers stored undererrorexposed gateway error bodies to whoever held the payment request hash — and it persists, so one transient outage parked it there for good.Both handlers now store an
EveryPayGateway::ERROR_*code (gateway_unavailable,invalid_gateway_response), andfail()takes an$errorCoderather than free text so the sink cannot accept prose again. Full detail is unchanged in theeverypaylog channel.Stop chaining the transport exception
Redacting the wrapper's message turned out to be only half the job. Log handlers and error trackers walk the exception chain and render a previous exception's message verbatim, and Symfony's transport exceptions quote the full request URL — so the credential came straight back through the link after the message had dropped it.
The transport exception is no longer chained; its class and redacted reason go into the message instead, so a timeout still reads differently from a refused connection. Nothing in the plugin called
getPrevious().Behaviour
The
errorkey ofresponseDatachanges from prose to a stable code. Nothing in the plugin reads it — the response provider looks only atpayment_linkandpayment_methods, and no template, feature or translation references it — but an API consumer parsing the text would now see a code, which is why the values are constants.EveryPayApiExceptionraised from a transport failure no longer has a previous exception. ItsstatusCodeis unchanged, and bothValidEveryPayCredentialsValidatorand the refund reconciliation branch on that, not on the chain.Payment states, transitions, retry semantics and log output are otherwise unchanged.
Checks
vendor/bin/phpunit(94 unit, 21 functional),vendor/bin/behat --strict(15/15),vendor/bin/phpstan analyse(level 9),vendor/bin/ecs check— all green. The one functional deprecation is the pre-existingpayum/coreSplObjectStorage::attach()notice on PHP 8.5.The redaction test asserts across the whole exception chain, and was checked against the chained version to confirm it fails there rather than passing vacuously.