Skip to content

Keep gateway error detail out of shopper-visible payment responses - #11

Merged
acirtautas merged 4 commits into
mainfrom
harden-gateway-error-disclosure
Aug 5, 2026
Merged

Keep gateway error detail out of shopper-visible payment responses#11
acirtautas merged 4 commits into
mainfrom
harden-gateway-error-disclosure

Conversation

@acirtautas

@acirtautas acirtautas commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Three related changes, one per commit, that stop EveryPay error detail — and the api_username with it — from reaching places it should not.

Keep EveryPay credentials out of API client error messages

api_username is 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_username in the URL, and the existing URL-building test still asserts it.

Store a generic error indicator in payment request responseData

Sylius serializes responseData to the shopper through the shop API, so the raw exception message the capture and status handlers stored under error exposed 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), and fail() takes an $errorCode rather than free text so the sink cannot accept prose again. Full detail is unchanged in the everypay log 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 error key of responseData changes from prose to a stable code. Nothing in the plugin reads it — the response provider looks only at payment_link and payment_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.

EveryPayApiException raised from a transport failure no longer has a previous exception. Its statusCode is unchanged, and both ValidEveryPayCredentialsValidator and 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-existing payum/core SplObjectStorage::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.

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
acirtautas force-pushed the harden-gateway-error-disclosure branch from 4c5bb40 to 964c2ad Compare August 5, 2026 23:03
@acirtautas
acirtautas merged commit 92aa308 into main Aug 5, 2026
8 checks passed
@acirtautas
acirtautas deleted the harden-gateway-error-disclosure branch August 5, 2026 23:15
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.

1 participant