Conversation
bdesoky
commented
May 19, 2026
pranavjain97
requested changes
May 20, 2026
Comment on lines
+149
to
+150
| * Update the password used to decrypt a single keychain. | ||
| * Handles v1 (SJCL) envelopes only. For v2 (Argon2id) support use {@link updateSingleKeychainPasswordAsync}. |
Contributor
There was a problem hiding this comment.
i would add a todo to deprecate this in the future
Comment on lines
+222
to
+224
| } catch (e) { | ||
| // catching an error here means that the password was incorrect or, less likely, the input to decrypt is corrupted | ||
| throw new Error('password used to decrypt keychain private key is incorrect'); |
Contributor
There was a problem hiding this comment.
cant the input to decrypt be corrupted even for v1?
Contributor
Author
There was a problem hiding this comment.
this is existing logic from the original updateSingleKeychainPassword
Comment on lines
+203
to
+226
| const oldEncryptedPrv = params.keychain.encryptedPrv; | ||
| try { | ||
| const decryptedPrv = await this.bitgo.decryptAsync({ input: oldEncryptedPrv, password: params.oldPassword }); | ||
| // Preserve the original envelope's encryption version so v2-encrypted keys stay v2 after the password change. | ||
| let encryptionVersion: EncryptionVersion | undefined; | ||
| try { | ||
| const parsed = JSON.parse(oldEncryptedPrv); | ||
| if (parsed.v === 2) { | ||
| encryptionVersion = 2; | ||
| } | ||
| } catch { | ||
| // non-JSON input — default to v1 (no encryptionVersion) | ||
| } | ||
| const newEncryptedPrv = await this.bitgo.encryptAsync({ | ||
| input: decryptedPrv, | ||
| password: params.newPassword, | ||
| encryptionVersion, | ||
| }); | ||
| return _.assign({}, params.keychain, { encryptedPrv: newEncryptedPrv }); | ||
| } catch (e) { | ||
| // catching an error here means that the password was incorrect or, less likely, the input to decrypt is corrupted | ||
| throw new Error('password used to decrypt keychain private key is incorrect'); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
This can be made bit cleaner like this with a helper for detecing the encryption version. Also narrowing down the the try/catch.
const oldEncryptedPrv = params.keychain.encryptedPrv;
let decryptedPrv: string;
try {
decryptedPrv = await this.bitgo.decryptAsync({ input: oldEncryptedPrv, password: params.oldPassword });
} catch (e) {
throw new Error('password used to decrypt keychain private key is incorrect');
}
const encryptionVersion = detectEncryptionVersion(oldEncryptedPrv);
const newEncryptedPrv = await this.bitgo.encryptAsync({
input: decryptedPrv,
password: params.newPassword,
encryptionVersion,
});
return _.assign({}, params.keychain, { encryptedPrv: newEncryptedPrv });
Contributor
Author
There was a problem hiding this comment.
sure can add this helper
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ticket: WCN-281
This pull request adds support for updating passwords on both v1 (SJCL) and v2 (Argon2id) encrypted keychains, ensuring that the encryption version is preserved during password changes. It introduces the new asynchronous method
updateSingleKeychainPasswordAsync, updates tests to cover mixed-version keychains, and refactors code to use the new async methods where appropriate.Keychain password update enhancements
updateSingleKeychainPasswordAsyncto theKeychainsclass, which supports both v1 and v2 (Argon2id) keychain envelopes and preserves the encryption version during password updates.updatePasswordmethod and related code paths to useupdateSingleKeychainPasswordAsyncfor asynchronous and version-aware password changes. [1] [2]Interface and documentation updates
IKeychainsinterface to include the newupdateSingleKeychainPasswordAsyncmethod and clarified documentation for both sync and async methods. [1] [2]Test improvements
keychains.tsto verify that password updates work with both v1 and v2 keychains, including cases with mixed keychain versions and ensuring the encryption version is preserved.Refactoring for async cryptography
decryptcalls withdecryptAsyncin keychain-related code to ensure compatibility with Argon2id and asynchronous cryptographic operations.Dependency updates
EncryptionVersionwhere needed to support version detection and preservation logic.This pull request introduces support for updating passwords on both v1 (SJCL) and v2 (Argon2id) encrypted keychains, ensuring that v2 keychains retain their encryption version after a password change. It adds an asynchronous method for updating a single keychain password, refactors code to use this method where appropriate, and enhances tests to cover mixed-version scenarios.
Keychain password update enhancements
updateSingleKeychainPasswordAsyncto theKeychainsclass, which supports updating passwords for both v1 and v2 (Argon2id) keychains while preserving the encryption version.updatePasswordand related code paths to use the new asynchronous method, ensuring compatibility with both keychain versions. [1] [2]Interface and API changes
IKeychainsinterface to include the newupdateSingleKeychainPasswordAsyncmethod.Test improvements
Internal refactoring
decryptAsyncmethod for consistency and improved error handling.