fix: avoid logging full FiatOrder on deposit order failure#26735
Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
|
Can we add other non sensitive details to the order object in the error so that it becomes easy to debug like provider details and may be order link ? |
Only log the order ID instead of the full order object to prevent sensitive paymentDetails from being sent to Sentry.
Avoid logging full FiatOrder (and paymentDetails) to Sentry in: - RampsOrderDetails, Aggregator OrderDetails, DepositOrderDetails - orderProcessor unrecognized provider, AggregatorProcessor Update orderProcessor test to expect orderId in Logger.error.
Add provider, orderType, state, and network fields to Logger.error calls in all Ramp order processors to aid debugging without exposing PII.
04d1d1b to
f138be7
Compare
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection:
These are non-functional changes that:
While the risk is low, selecting SmokeRamps provides confidence that the ramp flows (buy/sell crypto) still work correctly after these changes. The changes don't warrant additional tags like SmokeWalletPlatform since they don't affect wallet home or actions entry points - they only affect internal error logging. Performance Test Selection: |
The committed fixture schema is out of date. To update, comment: |
Description
When a deposit order fails to process, the catch block in
processDepositOrderwas logging the fullFiatOrderobject to Sentry viaLogger.error. This object can contain sensitive payment details (e.g.paymentDetails).This fix replaces
orderwithorder.idin the log context so only the order identifier is sent to Sentry, not the full order payload.Changelog
CHANGELOG entry: null
Related issues
Fixes: https://consensyssoftware.atlassian.net/jira/software/c/projects/TRAM/boards/1568?assignee=712020%3Aff625c85-a003-41f4-9b73-e2af164f8214&selectedIssue=TRAM-3260
https://github.com/MetaMask/pm-security/issues/583
Manual testing steps
```gherkin
Feature: Deposit order error logging
Scenario: user triggers a deposit order processing failure
Given the user has an in-progress deposit order
When the SDK call to fetch the order fails
Then the error is logged to Sentry with only the order ID
And no sensitive paymentDetails are included in the log
```
Screenshots/Recordings
Before
Logger.error sends the full FiatOrder object (including paymentDetails) to Sentry.
After
Logger.error sends only
{ message, orderId }to Sentry.Pre-merge author checklist
Pre-merge reviewer checklist
Note
Low Risk
Low risk: changes only adjust
Logger.errorcontext fields and update a unit test, with no impact to order processing or UI behavior.Overview
Redacts potentially sensitive
FiatOrderdata from Sentry logs by replacing fullorderobjects with minimal metadata (e.g.,orderId,provider,orderType,state,network) in multiple ramp order processing and order-details error paths.Updates the
processOrderunit test to assert the new structured logging payload for unsupported providers.Written by Cursor Bugbot for commit f138be7. This will update automatically on new commits. Configure here.