Skip to content

fix(sendMany): sanitize sensitive transaction prebuild and keychain logging (BG-F02) - #268

Open
mertcano wants to merge 1 commit into
BitGo:masterfrom
mertcano:mertcano-patch-1
Open

mertcano wants to merge 1 commit into
BitGo:masterfrom
mertcano:mertcano-patch-1

Conversation

@mertcano

@mertcano mertcano commented Sep 17, 2026 •

Copy link
Copy Markdown

Description

This pull request resolves a medium-severity sensitive data logging vulnerability in advanced-wallets (F-02), identified during the workspace security audit.

Previously, handleSendMany.ts serialized full transaction prebuild objects (txPrebuilt) and raw signing keychain payloads to the application logs. Keychains may contain sensitive cryptographic fields such as encryptedPrv, and serialized transaction prebuilds expose raw PSBT hexes, public keys, and full recipient metadata in log aggregators.

Summary of Changes

  • Sanitized Event Logging: Removed object-level logging of txPrebuilt and signingKeychain[cite: 24]. Replaced inline public-key interpolation with generic audit messages (logger.debug('Transaction prebuild verified') and logger.info('Signing with <source> keychain'))[cite: 24].
  • Preserved Diagnostic Error Tracing: Retained exception message reporting (err.message) to support operational troubleshooting without dumping underlying sensitive payloads to logger.error[cite: 24].
  • Defensive Test Assertions: Added unit test assertions in src/_tests/api/master/sendMany.test.ts to inspect all logger stubs (error, warn, info, http, debug) and verify that public keys (xpub_user), key IDs, PSBT hex strings, and raw transaction components are never emitted during successful flows or validation failures[cite: 24].

Issue Number

BG-F02-SENSITIVE-LOGGING[cite: 24]

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Tested locally using Supertest, Mocha, Sinon, and Nock against the Master BitGo Express endpoint test suite[cite: 24].

Reproduction Instructions

Run targeted unit tests for sendMany:

npm run test -- src/_tests/api/master/sendMany.test.ts

Verified Test Cases
Clean Success Logging: Asserted that successful sendMany transactions emit generic informational logs without leaking xpub_user, user-key-id, or prebuild hex strings[cite: 24].

Local Validation Failure: Verified that when verifyTransaction returns false, only the sanitized validation error message is logged[cite: 24].

Validation Exception: Verified that when verifyTransaction throws an unexpected error, only the error message is logged without raw transaction objects[cite: 24].

Checklist:
[x] My code follows the style guidelines of this project

[x] I have performed a self-review of my own code

[x] My code compiles correctly for both Node and Browser environments

[x] I have commented my code, particularly in hard-to-understand areas

[x] My commits follow [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) and I have properly described any BREAKING CHANGES

[x] The ticket or github issue was included in the commit message as a reference

[x] I have made corresponding changes to the documentation and on any new/updated functions and/or methods - [jsdoc](https://jsdoc.app/)

[x] I have added tests that prove my fix is effective or that my feature works

[x] New and existing unit tests pass locally with my changes

DescriptionThis pull request resolves a medium-severity sensitive data logging vulnerability in advanced-wallets (F-02), identified during the workspace security audit.  Previously, handleSendMany.ts serialized full transaction prebuild objects (txPrebuilt) and raw signing keychain payloads to the application logs. Keychains may contain sensitive cryptographic fields such as encryptedPrv, and serialized transaction prebuilds expose raw PSBT hexes, public keys, and full recipient metadata in log aggregators.  Summary of ChangesSanitized Event Logging: Removed object-level logging of txPrebuilt and signingKeychain. Replaced inline public-key interpolation with generic audit messages (logger.debug('Transaction prebuild verified') and logger.info('Signing with <source> keychain')).  Preserved Diagnostic Error Tracing: Retained exception message reporting (err.message) to support operational troubleshooting without dumping underlying sensitive payloads to logger.error.  Defensive Test Assertions: Added unit test assertions in src/_tests/api/master/sendMany.test.ts to inspect all logger stubs (error, warn, info, http, debug) and verify that public keys (xpub_user), key IDs, PSBT hex strings, and raw transaction components are never emitted during successful flows or validation failures.  Issue NumberBG-F02-SENSITIVE-LOGGING  Type of change[x] Bug fix (non-breaking change which fixes an issue)[ ] New feature (non-breaking change which adds functionality)[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)[ ] This change requires a documentation updateHow Has This Been Tested?Tested locally using Supertest, Mocha, Sinon, and Nock against the Master BitGo Express endpoint test suite.  Reproduction InstructionsRun targeted unit tests for sendMany:Bashnpm run test -- src/_tests/api/master/sendMany.test.ts
Verified Test CasesClean Success Logging: Asserted that successful sendMany transactions emit generic informational logs without leaking xpub_user, user-key-id, or prebuild hex strings.  Local Validation Failure: Verified that when verifyTransaction returns false, only the sanitized validation error message is logged.  Validation Exception: Verified that when verifyTransaction throws an unexpected error, only the error message is logged without raw transaction objects.  Checklist:  [x] My code follows the style guidelines of this project  [x] I have performed a self-review of my own code  [x] My code compiles correctly for both Node and Browser environments  [x] I have commented my code, particularly in hard-to-understand areas  [x] My commits follow Conventional Commits and I have properly described any BREAKING CHANGES  [x] The ticket or github issue was included in the commit message as a reference  [x] I have made corresponding changes to the documentation and on any new/updated functions and/or methods - jsdoc[x] I have added tests that prove my fix is effective or that my feature works[x] New and existing unit tests pass locally with my changes
@mertcano
mertcano requested review from a team as code owners September 17, 2026 21:15

This branch has not been deployed

No deployments
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.

3 participants