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.

vmccarty · 2026-07-09T17:40:32Z

+  console.log(`Wallet: ${wallet.label()} (${walletId})`);
+
+  // Fetch passcodeEncryptionCode — required to decrypt Box D and to generate Box D in the new keycard.
+  // Skipped in dry-run if not provided, since no keycard is produced anyway.


What's a dry run? Just like a test? Will this be something the client developers already have some frame of reference for, or does this need some kind of explanation somewhere?

I actually updated this so that it isnt skipped in dry-run when boxD is provided but forgot to push the change 😅
A dry run will attempt the decrypt + reencrypt but wont update anything in the backend. Just a way to test that the passphrase/boxD work properly

OttoAllmendinger · 2026-07-09T08:52:13Z

this belongs in a different PR

OttoAllmendinger · 2026-07-10T10:49:52Z

+    // SJCL envelopes have no "v" field or v=1
+    if (!envelope.v || envelope.v === 1) return 1;
+  } catch {
+    // not JSON


don't we want to about this condition?

updated to throw on unrecognized version

OttoAllmendinger · 2026-07-10T10:52:13Z

+      'passcoderecovery endpoint did not return a passcodeEncryptionCode — pass --passcodeEncryptionCode manually'
+    );
+  }
+  return recoveryInfo.passcodeEncryptionCode as string;


don't cast, validate

OttoAllmendinger · 2026-07-10T10:52:47Z

+ * jsPDF reads `.src` in Node.js; computeKeyCardImageDimensions reads `.width`/`.height`.
+ * This replicates what the BitGo UI does before calling drawKeycard.
+ */
+async function loadKeycardImage(url: string): Promise<HTMLImageElement | undefined> {


why are you silently suppressing all errors?

updated now to log a warning. the image isnt critical for the keycard so it wont throw

OttoAllmendinger · 2026-07-10T10:53:38Z

+  const args = process.argv.slice(2);
+  const get = (flag: string): string | undefined => {
+    const idx = args.indexOf(flag);
+    return idx !== -1 ? args[idx + 1] : undefined;
+  };
+
+  const env = (get('--env') ?? 'test') as EnvironmentName;
+  const coin = get('--coin');
+  const walletId = get('--walletId');
+  const passphrase = get('--passphrase');
+  const accessToken = get('--accessToken');
+  const otp = get('--otp');
+  const boxD = get('--boxD');
+  const boxA = get('--boxA');
+  const boxB = get('--boxB');
+  const passcodeEncryptionCode = get('--passcodeEncryptionCode');
+  const dryRun = args.includes('--dry-run');


use a real option parser, I believe we typically use yargs

updated to use yargs

OttoAllmendinger · 2026-07-10T10:54:40Z

+// Main
+// ---------------------------------------------------------------------------
+
+async function main() {


this is long and complicated enough that it deserves to be in modules/ properly, with a test

maybe even in modules/key-card/

hmm, i think this script is supposed to only be a temporary measure for users to opt in to v2 wallets until the UI defaults to v2 and provides a way to reencrypt wallets. Not sure if we need to move this to the module, @pranavjain97 what do you think?

we're going to need this script indefinitely for non-UI users with older wallets so we could potentially move it to the module. however, this can be done as a followup and shouldn't be blocking

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

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

…le (WCN-174) Extracts runUpgradeWalletEncryption and helpers from scripts/ into @bitgo/key-card so the upgrade flow is invokable via the SDK. The script becomes a thin CLI wrapper. Adds 61 unit tests covering all wallet types (standard, MPCv2, boxD passphrase recovery, dry-run). 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

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

feat: add script to reencrypt wallet with v2 encryption

6f43e3f

Ticket: WCN-174

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 from 06084fd to ff43ff7 Compare July 10, 2026 18:20

pranavjain97 force-pushed the WCN-174-reencrypt branch 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

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!

Choose a reason for hiding this comment

Uh oh!

bdesoky Jul 9, 2026 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bdesoky commented Jul 8, 2026 •

edited

Loading

vmccarty left a comment •

edited

Loading

bdesoky Jul 9, 2026 •

edited

Loading

github-actions Bot commented Jul 10, 2026 •

edited

Loading