Keep gateway error detail out of shopper-visible payment responses

CHANGELOG.md

@@ -21,6 +21,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - The test suite runs on PHPUnit 11/12 (`^11.5 || ^12.5`, was the EOL
   `^10.5`): PHP 8.3+ resolves PHPUnit 12, PHP 8.2 stays on PHPUnit 11.
   Dev-only - nothing changes for consumers of the plugin.
+- `doctrine/orm` 3.6.8 is excluded from the dev dependencies. It calls the
+  DBAL `Schema::edit()` API, which needs `doctrine/dbal` ^4.5, while Sylius 2.x
+  requires `doctrine/dbal` ^3.9 - so schema generation throws and every
+  functional test errors in `prepareDatabase()`. Dev-only, and a `require-dev`
+  entry rather than a `conflict` so consumers keep resolving their own graph;
+  drop it once a fixed 3.6.x is out.
 - Billing/shipping address fields in the one-off payment request are
   truncated to the character limits EveryPay enforces from 2026-10-01
   (city/street 50 characters, postcode 16), so an over-long address keeps
@@ -45,6 +51,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   payment completes its refund transition with the fresh snapshot. Any other
   rejection still rolls back and keeps the payment `completed`.
 
+### Security
+
+- EveryPay API error messages no longer carry the `api_username` - half of the
+  HTTP Basic credential pair, which rides in the query string of every GET call.
+  Messages now quote the endpoint without its query string, and credential values
+  echoed back in a gateway error body or quoted in a transport error are redacted.
+- A failed capture or status check stores a generic indicator in the payment
+  request `responseData` (`gateway_unavailable`, `invalid_gateway_response`)
+  rather than the raw exception message. Sylius serializes `responseData` to the
+  shopper, so gateway error bodies no longer reach it; the full detail stays in
+  the `everypay` log channel. An API consumer reading the `error` key now gets a
+  stable code instead of prose.
+- A transport failure is no longer chained to the `EveryPayApiException` it
+  raises. Log handlers and error trackers render a previous exception's message
+  verbatim, and the transport one quotes the full request URL - so the credential
+  the message itself had dropped came straight back through the chain. The
+  failing exception's class and reason are carried into the message instead.
+
 ## [0.5.0] - 2026-08-04
 
 ### Added

composer.json

@@ -44,6 +44,7 @@
     "require-dev": {
         "api-platform/json-schema": "^4.2",
         "behat/behat": "^3.14",
+        "doctrine/orm": "!=3.6.8",
         "friends-of-behat/symfony-extension": "^2.6",
         "knplabs/knp-menu-bundle": "^3.5",
         "phpstan/extension-installer": "^1.4",

src/Client/EveryPayApiClient.php

@@ -108,12 +108,27 @@ private function request(EveryPayCredentials $credentials, string $method, strin
             $options['json'] = $body;
         }
 
+        // Exception messages reach logs and responseData, so they quote the
+        // endpoint rather than $path, whose query string carries api_username.
+        $endpoint = $this->endpoint($path);
+
         try {
             $response = $this->httpClient->request($method, $credentials->baseUrl . $path, $options);
             $statusCode = $response->getStatusCode();
             $content = $response->getContent(false);
         } catch (HttpClientExceptionInterface $e) {
-            throw new EveryPayApiException(sprintf('EveryPay request %s %s failed: %s', $method, $path, $e->getMessage()), previous: $e);
+            // Transport messages quote the full request URL back at us.
+            $reason = str_replace($credentials->baseUrl . $path, $credentials->baseUrl . $endpoint, $e->getMessage());
+
+            // Not chained on purpose: log handlers render a previous exception's
+            // message verbatim, which would put the unredacted URL straight back.
+            throw new EveryPayApiException(sprintf(
+                'EveryPay request %s %s failed (%s): %s',
+                $method,
+                $endpoint,
+                $e::class,
+                $this->redactCredentials($reason, $credentials),
+            ));
         }
 
         $data = json_decode($content, true);
@@ -123,13 +138,18 @@ private function request(EveryPayCredentials $credentials, string $method, strin
                 'EveryPay responded HTTP %d to %s %s: %s',
                 $statusCode,
                 $method,
-                $path,
-                $this->extractErrorMessage($data, $content),
+                $endpoint,
+                $this->redactCredentials($this->extractErrorMessage($data, $content), $credentials),
             ), $statusCode);
         }
 
         if (!is_array($data)) {
-            throw new EveryPayApiException(sprintf('EveryPay returned a non-JSON body to %s %s: %s', $method, $path, $content), $statusCode);
+            throw new EveryPayApiException(sprintf(
+                'EveryPay returned a non-JSON body to %s %s: %s',
+                $method,
+                $endpoint,
+                $this->redactCredentials($content, $credentials),
+            ), $statusCode);
         }
 
         /** @var array<string, mixed> $decoded */
@@ -138,6 +158,30 @@ private function request(EveryPayCredentials $credentials, string $method, strin
         return $decoded;
     }
 
+    /** The path without its query string, which carries api_username on every GET. */
+    private function endpoint(string $path): string
+    {
+        $queryPosition = strpos($path, '?');
+
+        return false === $queryPosition ? $path : substr($path, 0, $queryPosition);
+    }
+
+    /** Scrubs credentials from text we do not control: transport messages and gateway error bodies. */
+    private function redactCredentials(string $message, EveryPayCredentials $credentials): string
+    {
+        $secrets = array_values(array_unique(array_filter([
+            $credentials->apiUsername,
+            rawurlencode($credentials->apiUsername),
+            $credentials->apiSecret,
+        ], static fn (string $secret): bool => '' !== $secret)));
+
+        if ([] === $secrets) {
+            return $message;
+        }
+
+        return str_replace($secrets, '[redacted]', $message);
+    }
+
     private function extractErrorMessage(mixed $data, string $fallback): string
     {
         if (is_array($data)) {

src/CommandHandler/CaptureEveryPayPaymentHandler.php

@@ -94,7 +94,7 @@ public function __invoke(CaptureEveryPayPayment $command): void
                 'payment_id' => $payment->getId(),
                 'exception' => $e,
             ]);
-            $this->fail($paymentRequest, $payment, $e->getMessage());
+            $this->fail($paymentRequest, $payment, EveryPayGateway::ERROR_GATEWAY_UNAVAILABLE);
 
             return;
         }
@@ -106,7 +106,7 @@ public function __invoke(CaptureEveryPayPayment $command): void
                 'payment_id' => $payment->getId(),
                 'response' => $response,
             ]);
-            $this->fail($paymentRequest, $payment, 'EveryPay response is missing payment_reference or payment_link.');
+            $this->fail($paymentRequest, $payment, EveryPayGateway::ERROR_INVALID_GATEWAY_RESPONSE);
 
             return;
         }
@@ -142,9 +142,10 @@ public function __invoke(CaptureEveryPayPayment $command): void
         );
     }
 
-    private function fail(PaymentRequestInterface $paymentRequest, PaymentInterface $payment, string $reason): void
+    /** @param string $errorCode an EveryPayGateway::ERROR_* indicator, never raw exception or gateway text */
+    private function fail(PaymentRequestInterface $paymentRequest, PaymentInterface $payment, string $errorCode): void
     {
-        $paymentRequest->setResponseData(['error' => $reason]);
+        $paymentRequest->setResponseData(['error' => $errorCode]);
 
         $this->stateMachine->apply(
             $paymentRequest,

src/CommandHandler/StatusEveryPayPaymentHandler.php

@@ -46,7 +46,7 @@ public function __invoke(StatusEveryPayPayment $command): void
                 'payment_id' => $payment->getId(),
                 'exception' => $e,
             ]);
-            $paymentRequest->setResponseData(['error' => $e->getMessage()]);
+            $paymentRequest->setResponseData(['error' => EveryPayGateway::ERROR_GATEWAY_UNAVAILABLE]);
             $this->stateMachine->apply(
                 $paymentRequest,
                 PaymentRequestTransitions::GRAPH,

src/EveryPayGateway.php

@@ -54,6 +54,16 @@ final class EveryPayGateway
     /** Key inside Payment::getDetails() holding the EveryPay payment snapshot. */
     public const DETAILS_KEY = 'everypay';
 
+    /**
+     * Indicators for responseData['error']. Sylius serializes responseData to the
+     * shopper, so raw exception text must never go there - it carries the
+     * api_username and the gateway body. Detail belongs in the everypay log only.
+     */
+    public const ERROR_GATEWAY_UNAVAILABLE = 'gateway_unavailable';
+
+    /** EveryPay answered 2xx, but without the fields the flow needs. */
+    public const ERROR_INVALID_GATEWAY_RESPONSE = 'invalid_gateway_response';
+
     /** Sent as integration_details.integration (EveryPay merchant telemetry). */
     public const INTEGRATION_NAME = 'pkglt/sylius-everypay-plugin';
 

tests/Functional/CapturePaymentRequestTest.php

@@ -93,7 +93,12 @@ public function testApiFailureFailsPaymentRequestAndPaymentWithoutThrowing(): vo
         $payment = EveryPayGateway::corePaymentFrom($paymentRequest);
         self::assertSame(PaymentRequestInterface::STATE_FAILED, $paymentRequest->getState());
         self::assertSame(PaymentInterface::STATE_FAILED, $payment->getState());
-        self::assertArrayHasKey('error', $paymentRequest->getResponseData());
+        // Shopper-readable responseData carries a generic code, never the
+        // gateway text (which embeds the api_username).
+        self::assertSame(
+            ['error' => EveryPayGateway::ERROR_GATEWAY_UNAVAILABLE],
+            $paymentRequest->getResponseData(),
+        );
     }
 
     private function createCapturePaymentRequest(): PaymentRequestInterface

tests/Unit/Client/EveryPayApiClientTest.php

@@ -4,6 +4,7 @@
 
 namespace Tests\Pkg\SyliusEveryPayPlugin\Unit\Client;
 
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
 use Pkg\SyliusEveryPayPlugin\Client\EveryPayApiClient;
 use Pkg\SyliusEveryPayPlugin\Client\EveryPayApiException;
@@ -148,4 +149,59 @@ public function testTransportErrorIsWrappedInApiException(): void
 
         $client->getPayment($this->credentials(), 'abc123def456abc1');
     }
+
+    /**
+     * Failure messages are persisted into payment request responseData, which
+     * the shop API serializes for the customer - the api_username half of the
+     * Basic-auth pair must not ride along, neither from the request URL nor
+     * from a gateway response echoing it back.
+     *
+     * @param list<string> $expectedFragments
+     */
+    #[DataProvider('credentialLeakingFailures')]
+    public function testFailureMessagesKeepTheEndpointButNotTheApiUsername(MockResponse $response, array $expectedFragments): void
+    {
+        $client = new EveryPayApiClient(new MockHttpClient($response));
+
+        try {
+            $client->getPayment($this->credentials(), 'abc123def456abc1');
+            self::fail('The failing request did not throw.');
+        } catch (EveryPayApiException $exception) {
+            $message = $exception->getMessage();
+
+            self::assertStringNotContainsString('a04e7ce1060e7024', $message);
+            self::assertStringNotContainsString('api_username=', $message);
+
+            // Log handlers walk the chain, so a linked exception must stay clean too.
+            for ($linked = $exception->getPrevious(); $linked !== null; $linked = $linked->getPrevious()) {
+                self::assertStringNotContainsString('a04e7ce1060e7024', $linked->getMessage());
+            }
+
+            foreach ($expectedFragments as $fragment) {
+                self::assertStringContainsString($fragment, $message);
+            }
+        }
+    }
+
+    /** @return iterable<string, array{MockResponse, list<string>}> */
+    public static function credentialLeakingFailures(): iterable
+    {
+        yield 'transport error quoting the request URL' => [
+            new MockResponse('', ['error' => 'Could not resolve host for "https://igw-demo.every-pay.com/api/v4/payments/abc123def456abc1?api_username=a04e7ce1060e7024"']),
+            ['GET /v4/payments/abc123def456abc1 failed', 'Could not resolve host'],
+        ];
+
+        yield 'error body echoing the api_username' => [
+            new MockResponse(
+                json_encode(['error' => ['code' => 4013, 'message' => 'Unknown api_username a04e7ce1060e7024']], \JSON_THROW_ON_ERROR),
+                ['http_code' => 401],
+            ),
+            ['HTTP 401 to GET /v4/payments/abc123def456abc1', 'Unknown api_username [redacted]'],
+        ];
+
+        yield 'non-JSON body echoing the api_username' => [
+            new MockResponse('<html>api_username a04e7ce1060e7024 blocked</html>'),
+            ['non-JSON body to GET /v4/payments/abc123def456abc1', 'api_username [redacted] blocked'],
+        ];
+    }
 }

tests/Unit/CommandHandler/CaptureEveryPayPaymentHandlerTest.php

@@ -73,7 +73,12 @@ public function testApiFailureFailsPaymentRequestAndPaymentWithoutThrowing(): vo
 
         $handler(new CaptureEveryPayPayment('hash'));
 
-        self::assertArrayHasKey('error', $paymentRequest->getResponseData());
+        // A generic code only - the exception message carries the api_username
+        // and the gateway body, and responseData reaches the shopper.
+        self::assertSame(
+            ['error' => EveryPayGateway::ERROR_GATEWAY_UNAVAILABLE],
+            $paymentRequest->getResponseData(),
+        );
         self::assertSame(
             [
                 [$paymentRequest, PaymentRequestTransitions::GRAPH, PaymentRequestTransitions::TRANSITION_FAIL],
@@ -107,7 +112,10 @@ public function testResponseWithoutReferenceOrLinkFailsTheAttempt(): void
 
         // A 2xx response without the hosted page link is as unusable as an
         // API error: fail the attempt so the customer gets a fresh payment.
-        self::assertArrayHasKey('error', $paymentRequest->getResponseData());
+        self::assertSame(
+            ['error' => EveryPayGateway::ERROR_INVALID_GATEWAY_RESPONSE],
+            $paymentRequest->getResponseData(),
+        );
         self::assertSame(
             [
                 [$paymentRequest, PaymentRequestTransitions::GRAPH, PaymentRequestTransitions::TRANSITION_FAIL],

tests/Unit/CommandHandler/StatusEveryPayPaymentHandlerTest.php

@@ -11,6 +11,7 @@
 use Pkg\SyliusEveryPayPlugin\EveryPayGateway;
 use Pkg\SyliusEveryPayPlugin\Processor\EveryPayPaymentSynchronizer;
 use Pkg\SyliusEveryPayPlugin\Processor\EveryPayStateMapper;
+use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
 use Sylius\Abstraction\StateMachine\StateMachineInterface;
 use Sylius\Bundle\PaymentBundle\Provider\PaymentRequestProviderInterface;
@@ -23,11 +24,15 @@
 use Sylius\Component\Payment\PaymentTransitions;
 use Symfony\Component\HttpClient\MockHttpClient;
 use Symfony\Component\HttpClient\Response\MockResponse;
+use Tests\Pkg\SyliusEveryPayPlugin\Support\RecordingLogger;
 
 final class StatusEveryPayPaymentHandlerTest extends TestCase
 {
     private const PAYMENT_REFERENCE = 'abc123def456abc123def456abc123def456abc123def456abc123def456abcd';
 
+    /** Half of the HTTP Basic credential pair - it must never reach the shopper. */
+    private const API_USERNAME = 'a04e7ce1060e7024';
+
     /** @var array<array{object, string, string}> */
     private array $appliedTransitions = [];
 
@@ -61,7 +66,10 @@ public function testApiFailureFailsOnlyTheRequestSoCallbacksSettleThePaymentLate
         // The documented invariant: a temporary API failure on customer return
         // is swallowed - the payment stays processing and the server callback
         // redeliveries settle it later.
-        self::assertArrayHasKey('error', $paymentRequest->getResponseData());
+        self::assertSame(
+            ['error' => EveryPayGateway::ERROR_GATEWAY_UNAVAILABLE],
+            $paymentRequest->getResponseData(),
+        );
         self::assertSame(
             [[$paymentRequest, PaymentRequestTransitions::GRAPH, PaymentRequestTransitions::TRANSITION_FAIL]],
             $this->appliedTransitions,
@@ -72,11 +80,38 @@ public function testApiFailureFailsOnlyTheRequestSoCallbacksSettleThePaymentLate
         self::assertSame(PaymentInterface::STATE_PROCESSING, $payment->getState());
     }
 
+    public function testApiFailureKeepsTheCredentialAndGatewayTextOutOfTheShopperVisibleResponse(): void
+    {
+        $paymentRequest = $this->paymentRequest();
+        $logger = new RecordingLogger();
+        // The client folds the request path (which carries api_username) and the
+        // raw gateway body into the exception message. responseData is served to
+        // the shopper by the Sylius shop API, so neither may be persisted there.
+        $handler = $this->handler(
+            $paymentRequest,
+            new MockResponse('gateway maintenance in progress', ['http_code' => 500]),
+            $logger,
+        );
+
+        $handler(new StatusEveryPayPayment('hash'));
+
+        $responseData = $paymentRequest->getResponseData();
+        self::assertSame(['error' => EveryPayGateway::ERROR_GATEWAY_UNAVAILABLE], $responseData);
+
+        $serialized = json_encode($responseData, \JSON_THROW_ON_ERROR);
+        self::assertStringNotContainsString(self::API_USERNAME, $serialized);
+        self::assertStringNotContainsString('maintenance', $serialized);
+
+        // The detail stays operator-facing: it is logged (with the exception in
+        // the context) on the everypay channel.
+        self::assertSame(['EveryPay status check on customer return failed.'], $logger->messages('error'));
+    }
+
     private function paymentRequest(): PaymentRequest
     {
         $gatewayConfig = $this->createStub(GatewayConfigInterface::class);
         $gatewayConfig->method('getConfig')->willReturn([
-            EveryPayGateway::CONFIG_API_USERNAME => 'a04e7ce1060e7024',
+            EveryPayGateway::CONFIG_API_USERNAME => self::API_USERNAME,
             EveryPayGateway::CONFIG_API_SECRET => 'secret',
             EveryPayGateway::CONFIG_ACCOUNT_NAME => 'EUR3D1',
             EveryPayGateway::CONFIG_ENVIRONMENT => EveryPayGateway::ENVIRONMENT_DEMO,
@@ -95,7 +130,7 @@ private function paymentRequest(): PaymentRequest
         return new PaymentRequest($payment, $method);
     }
 
-    private function handler(PaymentRequest $paymentRequest, MockResponse $apiResponse): StatusEveryPayPaymentHandler
+    private function handler(PaymentRequest $paymentRequest, MockResponse $apiResponse, ?LoggerInterface $logger = null): StatusEveryPayPaymentHandler
     {
         $paymentRequestProvider = $this->createStub(PaymentRequestProviderInterface::class);
         $paymentRequestProvider->method('provide')->willReturn($paymentRequest);
@@ -120,7 +155,7 @@ function (object $subject, string $graph, string $transition): void {
             $paymentRequestProvider,
             $synchronizer,
             $stateMachine,
-            new NullLogger(),
+            $logger ?? new NullLogger(),
         );
     }
 }