-
-
Notifications
You must be signed in to change notification settings - Fork 188
fix: add support for log file using split state format #88
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
Conversation
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.
Pull request overview
This pull request adds support for extracting vault data from log files that use a split state format, specifically targeting Chrome on Windows (000004.log files). The PR enhances the existing vault extraction logic by adding a new extraction attempt (attempt 7) alongside the existing attempts, while also refactoring the code structure for better organization.
Changes:
- Added a new test fixture for Chrome Windows with split state format log file
- Implemented attempt 7 in the vault extraction logic to handle split state format
- Refactored attempt 6 to use block scoping and early returns for better code organization
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| test/fixtures/chrome-windows-2/000004.log | New test fixture containing vault data in split state format |
| app/lib.js | Added attempt 7 for split state format extraction and refactored attempt 6 with block scoping |
| bundle.js | Transpiled version of lib.js changes with appropriate variable renaming |
| app/lib.test.js | Added test case for the new Chrome Windows 2 fixture |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull request overview
Copilot reviewed 3 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (x == null) { | ||
| return | ||
| } |
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.
AI review caught this bug that could cause dedupe to throw.
| { | ||
| // attempt 6: chromium 000005.ldb on windows | ||
| const matchRegex = /Keyring[0-9][^\}]*(\{[^\{\}]*\\"\})/gu | ||
| const captureRegex = /Keyring[0-9][^\}]*(\{[^\{\}]*\\"\})/u | ||
| const ivRegex = /\\"iv.{1,4}[^A-Za-z0-9+\/]{1,10}([A-Za-z0-9+\/]{10,40}=*)/u | ||
| const dataRegex = /\\"[^":,is]*\\":\\"([A-Za-z0-9+\/]*=*)/u | ||
| const saltRegex = /,\\"salt.{1,4}[^A-Za-z0-9+\/]{1,10}([A-Za-z0-9+\/]{10,100}=*)/u | ||
| const vaults = dedupe(data.match(matchRegex)?.map(m => m.match(captureRegex)[1]) | ||
| .map(s => [dataRegex, ivRegex, saltRegex].map(r => s.match(r))) | ||
| .filter(([d,i,s]) => d&&d.length>1 && i&&i.length>1 && s&&s.length>1) | ||
| .map(([d,i,s]) => ({ | ||
| data: d[1], | ||
| iv: i[1], | ||
| salt: s[1], | ||
| }))) | ||
| if (vaults.length) { | ||
| /* istanbul ignore next */ | ||
| if (vaults.length > 1) { | ||
| console.log('Found multiple vaults!', vaults) | ||
| } | ||
| return vaults[0] | ||
| } |
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.
this hasn't changed, other than putting it in its own lexical block now, and it doesn't return null if there are no vaults found.
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.
Hide whitespace changes removes this from the diff
| console.log('Found multiple vaults!', vaults) | ||
| { | ||
| // attempt 7: log file using split state format, chromium 000004.log on windows-2 | ||
| const vaultRegex = /KeyringController[\s\S]*?"vault":"((?:[^"\\]|\\.)*)"/g |
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 vault is stored as a top-level key now, and the top-level keys aren't JSON serialized, so they aren't wrapped with quotes.
| function dedupe(arr) { | ||
| var result = []; | ||
| arr === null || arr === void 0 ? void 0 : arr.forEach(function (x) { | ||
| if (x == null) { |
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.
changes in the file are just from rebuilding and committing it.
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.
Pull request overview
Copilot reviewed 3 out of 5 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
app/lib.test.js:88
- This test is incorrectly placed inside the FIXTURES loop, causing it to run once for each fixture (6 times). Move this test outside the for loop to run it only once.
it(`returns null when vault not found`, async () => {
const encrypted = 'foobarblob';
const vaultData = extractVaultFromFile(encrypted)
expect(vaultData).toBe(null)
})
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
MajorLift
left a comment
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.
LGTM!
Note
Adds compatibility for another on-disk log variant and improves robustness.
"vault":"..."), JSON-decodes,dedupes, and returns the first vaultnulldedupeto ignorenull/undefinedentrieschrome-windows-2/000004.logfixture and a "returns null when vault not found" checkjest.config.js; updates compiledbundle.jsaccordinglyWritten by Cursor Bugbot for commit f8ad6fe. This will update automatically on new commits. Configure here.