feat: add script to reencrypt wallet with v2 encryption by bdesoky · Pull Request #9204 · BitGo/BitGoJS

bdesoky · 2026-07-08T14:46:34Z

This pull request introduces significant enhancements to the wallet encryption upgrade process and improves compatibility for PDF keycard generation in Node.js environments. The main changes include adding a new script to automate wallet keychain re-encryption and updating the keycard drawing utilities to support server-side (Node.js) usage.

Key changes:

1. New Wallet Encryption Upgrade Script

Added scripts/upgrade-wallet-encryption.ts, a comprehensive CLI tool to upgrade wallet keychains from v1 (SJCL/PBKDF2-SHA256 + AES-256-CCM) to v2 (Argon2id + AES-256-GCM) encryption, with support for regenerating the keycard PDF and handling various edge cases (e.g., changed passphrases, legacy backups).

2. Node.js Compatibility for Keycard PDF Generation

Updated drawKeycard and related functions in modules/key-card/src/drawKeycard.ts to detect Node.js environments and handle image and QR code rendering appropriately (e.g., using base64 data URLs instead of DOM elements). [1] [2] [3]
Modified QR code image handling to allow both HTMLCanvasElement and base64 string inputs, ensuring PDF generation works in both browser and Node.js contexts. [1] [2]

These changes ensure that wallet upgrades and keycard regeneration can be performed reliably in automated or server-side workflows, improving maintainability and operational flexibility.

linear-code · 2026-07-08T14:46:38Z

WCN-174

vmccarty

The instructions read well to me 👍 I just have one question about the "dry-run", but it doesn't seem too high priority.

pranavjain97

mpcv2 guard added, was able to re-encrypt with reduced mpcv2 keycard

pranavjain97 · 2026-07-10T18:31:34Z

@claude give approval rating

github-actions · 2026-07-10T18:31:54Z

Claude finished @pranavjain97's task —— View job

Code Review and Approval Rating

Tasks:

Review the git diff to understand all changes
Examine the new upgrade-wallet-encryption.ts script
Review modifications to drawKeycard.ts for Node.js compatibility
Check if previous reviewer feedback has been addressed
Assess code quality, security, and best practices
Provide overall approval rating

Overall Approval Rating: ⭐⭐⭐⭐⭐ STRONG APPROVE

This is a well-implemented, security-conscious feature that addresses a critical need for wallet encryption upgrades. The code quality is high and previous reviewer feedback has been properly addressed.

Strengths

🔒 Security Excellence

Proper encryption version detection (getEncryptionVersion() in scripts/upgrade-wallet-encryption.ts:53-67)
Safe passphrase handling with fallback to original passphrase for changed passwords (reencryptAsV2() in scripts/upgrade-wallet-encryption.ts:73-94)
MPCv2 validation preventing incomplete keycards (scripts/upgrade-wallet-encryption.ts:227-232)
Comprehensive error handling with informative messages throughout

🛠️ Robust Implementation

Yargs integration provides proper CLI argument parsing (scripts/upgrade-wallet-encryption.ts:152-183)
Dry-run functionality allows safe testing before making changes
Node.js compatibility for keycard generation via environment detection (modules/key-card/src/drawKeycard.ts:7)
Graceful image loading with fallbacks for missing coin logos (loadKeycardImage() function)

📋 Excellent Documentation

Comprehensive usage examples in the file header (scripts/upgrade-wallet-encryption.ts:5-29)
Clear parameter explanations for all CLI options
Inline comments explaining complex logic and edge cases

Previous Reviewer Feedback Resolution ✅

All feedback from @OttoAllmendinger and @pranavjain97 has been properly addressed:

✅ Yargs adoption - Replaced manual argument parsing with proper yargs library
✅ Error handling improvement - Image loading now logs warnings instead of silent suppression
✅ Type validation - getEncryptionVersion() now throws on unrecognized versions
✅ MPCv2 guard added - Prevents massive PDF generation without boxA/boxB values

Code Quality Assessment

Architecture & Design - Excellent

Clean separation of concerns with focused helper functions
Proper abstraction for Node.js vs browser environments in keycard drawing
Follows existing codebase patterns and conventions

Error Handling - Very Good

Comprehensive error messages guide users toward solutions
Graceful degradation for non-critical features (coin logos)
Proper validation of required parameters for different wallet types

Testing & Validation - Good

Dry-run mode enables safe validation before execution
Input sanitization (whitespace removal from box values)
Version detection prevents unnecessary re-encryption

Documentation - Outstanding

Extensive usage documentation with examples
Clear parameter explanations and requirements
Helpful inline comments explaining complex logic

Minor Observations (No Action Required)

Temporary Script Nature - As noted by @pranavjain97, this is intended as a temporary measure until UI provides v2 wallet defaults
Image Fallback Logging - Warnings for missing coin logos are appropriate and don't impact core functionality
Duck-typing Pattern - The HTMLImageElement compatibility object is a clever solution for Node.js/browser compatibility

Recommendation

✅ APPROVE - This PR is ready for merge. It addresses a critical operational need with a well-designed, secure implementation that properly handles edge cases and provides excellent user experience through comprehensive documentation and dry-run capabilities.

The code demonstrates BitGo's high security standards and will enable safe migration of wallet encryption for users requiring enhanced security provided by v2 encryption (Argon2id + AES-256-GCM).

Run Details:

Branch: WCN-174-reencrypt
Job Run Link

OttoAllmendinger

not going to ACK without module and tests, flushing

flushing review

OttoAllmendinger

see previous comment

pranavjain97 · 2026-07-13T16:20:46Z

not going to ACK without module and tests, flushing

I've moved the business logic to the key card module with tests. still have the script to invoke/e2e sample for clients. @OttoAllmendinger can you review?

zahin-mohammad

In addition to moving this out, have we done an e2e test with the keycard with

bitgo ui
wallet recovery wizard

zahin-mohammad

Logs can be moved in a follow up, along with OFC support. (OFC support can be done via key rotation instead of re-encryption)

OttoAllmendinger

looks much better, agree with zahin

zahin-mohammad

small nit

Ticket: WCN-174

WCN-174 - Fail early for MPCv2 wallets when --boxA/--boxB not provided to avoid generating incomplete keycards - Handle already-unlocked session gracefully instead of crashing - Strip whitespace from box values to handle PDF line-wrapped JSON TICKET: WCN-174

Adds a wallet encryption upgrade path from v1 (SJCL / PBKDF2-SHA256 + AES-256-CCM) to v2 (Argon2id + AES-256-GCM), invokable via the SDK and driven by a CLI script. sdk-core: - Keychains.getEncryptionVersion: now public, throws on unknown versions - Keychains.reencryptAsV2: new instance method for v1 to v2 upgrade - Wallet.upgradeEncryption: orchestrates the full flow with an injectable PDF generator for testability key-card: - Exports loadKeycardImage and createKeycardPdfGenerator factory (returns an UpgradeEncryptionPdfGenerator compatible with the wallet method) scripts/upgrade-wallet-encryption.ts: - Thin CLI wrapper around Wallet.upgradeEncryption - Handles MPCv2 (boxA/boxB), boxD-derived passphrase, dry-run, already-unlocked session, and PDF regeneration TICKET: WCN-174

…WCN-174) The retry-with-original-passphrase fallback is specific to keys encrypted at wallet creation (backup key, boxA, boxB — these are never re-encrypted on password change). It doesn't belong on the generic Keychains.reencryptAsV2 primitive, which was leaking backup-key context into its error message. - Keychains.reencryptAsV2 is now a pure primitive: decrypt with passphrase, re-encrypt as v2, surface errors directly - Fallback moves to a private Wallet.reencryptCreationTimeKey helper, used only for backup/boxA/boxB paths where the semantics apply - Error messages now name the specific key that failed to decrypt TICKET: WCN-174

bdesoky force-pushed the WCN-174-reencrypt branch 3 times, most recently from 00ed82a to 567bcdf Compare July 8, 2026 20:12

bdesoky changed the title ~~feat: add sdk method to reencrypt wallet with v2 encryption~~ feat: add script to reencrypt wallet with v2 encryption Jul 8, 2026

bdesoky force-pushed the WCN-174-reencrypt branch from 567bcdf to a30b584 Compare July 8, 2026 20:18

vmccarty previously approved these changes Jul 9, 2026

View reviewed changes

vmccarty reviewed Jul 9, 2026

View reviewed changes

Comment thread scripts/upgrade-wallet-encryption.ts Outdated

bdesoky dismissed vmccarty’s stale review via db796c3 July 9, 2026 17:45

bdesoky force-pushed the WCN-174-reencrypt branch from a30b584 to db796c3 Compare July 9, 2026 17:45

bdesoky marked this pull request as ready for review July 9, 2026 18:01

bdesoky requested review from a team as code owners July 9, 2026 18:01

bdesoky requested review from Marzooqa and s84krish July 9, 2026 18:01

bdesoky force-pushed the WCN-174-reencrypt branch 2 times, most recently from 4d55f08 to ab75384 Compare July 9, 2026 21:59

OttoAllmendinger previously requested changes Jul 10, 2026

View reviewed changes

bdesoky force-pushed the WCN-174-reencrypt branch from ab75384 to 6f43e3f Compare July 10, 2026 14:46

pranavjain97 requested changes Jul 10, 2026

View reviewed changes

Comment thread scripts/upgrade-wallet-encryption.ts Outdated

pranavjain97 force-pushed the WCN-174-reencrypt branch 2 times, most recently from ff43ff7 to 455f6c1 Compare July 10, 2026 18:28

pranavjain97 previously approved these changes Jul 10, 2026

View reviewed changes

pranavjain97 requested a review from OttoAllmendinger July 10, 2026 18:31

OttoAllmendinger reviewed Jul 10, 2026

View reviewed changes

OttoAllmendinger requested review from OttoAllmendinger and vmccarty July 10, 2026 19:46

OttoAllmendinger reviewed Jul 10, 2026

View reviewed changes

pranavjain97 dismissed their stale review via 5051e01 July 10, 2026 20:55

pranavjain97 requested review from OttoAllmendinger and zahin-mohammad July 13, 2026 16:20

zahin-mohammad requested changes Jul 13, 2026

View reviewed changes

Comment thread modules/key-card/src/upgradeWalletEncryption.ts

pranavjain97 force-pushed the WCN-174-reencrypt branch from f87bf33 to 364e027 Compare July 13, 2026 20:58

pranavjain97 requested a review from a team as a code owner July 13, 2026 20:58

pranavjain97 requested a review from zahin-mohammad July 13, 2026 21:17

zahin-mohammad previously approved these changes Jul 13, 2026

View reviewed changes

Comment thread modules/sdk-core/src/bitgo/keychain/keychains.ts Outdated

Comment thread modules/sdk-core/src/bitgo/wallet/wallet.ts

Comment thread modules/sdk-core/src/bitgo/wallet/wallet.ts

OttoAllmendinger reviewed Jul 14, 2026

View reviewed changes

Comment thread scripts/upgrade-wallet-encryption.ts

OttoAllmendinger previously approved these changes Jul 14, 2026

View reviewed changes

pranavjain97 dismissed stale reviews from OttoAllmendinger and zahin-mohammad via ea6144e July 14, 2026 18:31

pranavjain97 force-pushed the WCN-174-reencrypt branch from ea6144e to 8b2afaa Compare July 14, 2026 18:33

zahin-mohammad approved these changes Jul 14, 2026

View reviewed changes

Comment thread modules/sdk-core/src/bitgo/wallet/wallet.ts

bdesoky and others added 4 commits July 14, 2026 17:41

feat: add script to reencrypt wallet with v2 encryption

3dbc8aa

Ticket: WCN-174

pranavjain97 force-pushed the WCN-174-reencrypt branch from 8b2afaa to 6c83cf1 Compare July 14, 2026 21:43

pranavjain97 merged commit 45a8f59 into master Jul 14, 2026
25 checks passed

Uh oh!

Conversation

bdesoky commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. New Wallet Encryption Upgrade Script

2. Node.js Compatibility for Keycard PDF Generation

Uh oh!

linear-code Bot commented Jul 8, 2026

Uh oh!

vmccarty left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pranavjain97 left a comment

Choose a reason for hiding this comment

Uh oh!

pranavjain97 commented Jul 10, 2026

Uh oh!

github-actions Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review and Approval Rating

Tasks:

Overall Approval Rating: ⭐⭐⭐⭐⭐ STRONG APPROVE

Strengths

🔒 Security Excellence

🛠️ Robust Implementation

📋 Excellent Documentation

Previous Reviewer Feedback Resolution ✅

Code Quality Assessment

Architecture & Design - Excellent

Error Handling - Very Good

Testing & Validation - Good

Documentation - Outstanding

Minor Observations (No Action Required)

Recommendation

Uh oh!

OttoAllmendinger left a comment

Choose a reason for hiding this comment

Uh oh!

OttoAllmendinger left a comment

Choose a reason for hiding this comment

Uh oh!

pranavjain97 commented Jul 13, 2026

Uh oh!

zahin-mohammad left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zahin-mohammad left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

OttoAllmendinger left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zahin-mohammad left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bdesoky commented Jul 8, 2026 •

edited

Loading

vmccarty left a comment •

edited

Loading

github-actions Bot commented Jul 10, 2026 •

edited

Loading