Sensitive information shouldn't be included in error messages

[mwgalloway]

Hello,

The Adyen Client includes includes request parameters when raising errors. Request parameters often include credit card numbers and other personally identifying information. This poses a significant security risk as error messages are typically written to logs and also often sent to external monitoring services where you wouldn't want to have any sensitive information appearing. I'd like to suggest that this sort of information should not be included in error messages, or at the very least there should be the ability to configure not having it included.

Thanks,
Matt

[crrood]

Hi Matt,

Thanks for the suggestion - it's a very good point that we shouldn't expose any PCI sensitive data in error logging. Ideally we can find a way to include the rest of the request for debugging while masking card number and CVC.

Something like:

"Request": {
  "paymentMethod": {
    "type": "scheme",
    "number": "***",
    "expiryMonth": "10",
    "expiryYear": "2020",
    "holderName": "John Smith",
    "cvc": "***"
  },
  "amount": {
    "currency": "USD",
    "value": 1000
  },
  "reference": "Test transaction",
  "merchantAccount": "TestAccount"
}

If you want to try and make that happen I'll review the PR. Otherwise I'll take a look with the team once everyone is back from holidays.

Best,
Colin

[mwgalloway]

Thanks Colin,

I personally think it would be ideal if the request parameters weren't included in the error message at all. But, if they are going to be included, I would prefer that in addition to PCI data, all personally identifiable information, even non-sensitive information, such as billing address and account holder name was also obscured as well as payment instrument tokens. Let me know what you think, I've got a lot of other things to address but I might be able to squeeze in a PR over the next few days.

[crrood]

Hi Matt,

I created PR #50 to mask PAN and CVC data in the request attribute of the AdyenError class, which should be merged pending reviews. Since other fields can be useful in debugging, Adyen's policy is not to mask them by default. If you don't want to see any of the request data in your logging, please fork and modify the library locally.

Let us know if you have any other issues, and thanks for the contribution!

Best,
Colin