-
-
Notifications
You must be signed in to change notification settings - Fork 172
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix updated vault decryption #63
Conversation
@mikesposito Your changes makes sense, but is there a way I can test this PR manually considering there aren't any tests for this repo? |
The repo does have tests as of #47. I think ideally another fixture should be added here (or replace the existing added in #62) to also cover the missing cases mentioned. As for testing manually: Fire up the built frontend in a browser and upload / paste data into the form alongside your passphrase as extracted from filesystem directory following the linked user guide. |
Good point! I added a new test fixture with the |
@@ -64,11 +64,25 @@ function extractVaultFromFile (data) { | |||
// attempt 4: chromium 000006.log on MacOS | |||
// this variant also contains a 'keyMetadata' key in the vault, which should be | |||
// a nested object. | |||
const matches = data.match(/KeyringController":(\{"vault":".*=\\"\}"\})/); | |||
const matches = data.match(/KeyringController":(\{"vault":".*?=\\"\}"\})/); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lazy (?
) operator here is important, because there are instances of .log
files that present multiple occurrences of this, and in these cases if we don't use the lazy operator we'll match the entire text between the start of the first occurrence and the end of the last one. We are instead only interested in the first occurrence.
@legobeat Ah sorry. You're right — thanks for adding those tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
#62 introduced an additional attempt to encrypt the vault, but:
bundle.js
was not properly updatedThis PR fixes the problem by recovering the vault part by part, to ensure capturing only the needed parts