feat(vault): phase 2 — the Vault tab, history, trash and import - #19
Merged
Conversation
A fifth tab beside sync | shield | pro | account. First phase users can see: create a vault, unlock it, keep logins, cards and identities, search them, copy a password, browse password history, use the trash, and import from Bitwarden, 1Password or Chrome. No autofill, no content scripts, and NO NEW PERMISSIONS. That is the point of shipping this before phase 3 — it is the half that earns its keep without asking anyone to grant access to every website they visit. Where the key lives, which is the whole MV3 problem. The service worker is killed after ~30s idle, so an unlocked key in a module variable is gone between one popup opening and the next and the vault appears to lock at random. It goes in chrome.storage.session: memory-only, never written to disk, cleared when the browser closes, and it survives worker restarts — exactly the lifetime an unlock should have. Access is pinned to TRUSTED_CONTEXTS so phase 3's content scripts can never read it. Auto-lock runs off chrome.alarms, not setTimeout, because a timer dies with the worker. Tested by re-importing the module, which is what a restart looks like. The popup never holds a key. It sends messages; the background owns the key and does every encrypt and decrypt. A bug in the UI can leak what is already on screen, not the vault. Signing out now clears session storage. clearUserData only removed local keys, so the vault would have stayed unlocked for whoever signed in next on the same profile. Generator: rejection sampling over crypto.getRandomValues, never Math.random and never a modulo, which quietly favours the front of the alphabet. Guarantees one character from each enabled group because sites reject a password that happens to contain no digit. The 256-word list gives a whole 8 bits per word, so the six-word default is exactly 48 — reported rather than implied. Import is a hand-written CSV reader because the failure mode of a sloppy one is silently importing half of somebody's passwords. It handles quoted commas, escaped quotes, embedded newlines, CRLF and a BOM — all of which appear in real exports, because notes fields contain everything. Source is detected from the header row, and 1Password is checked before Chrome since Chrome's columns are a subset of its own. Copied passwords clear from the clipboard after 30 seconds, and only if the clipboard still holds what we put there. Found and fixed while testing: pickIndex computed Math.floor(256/n)*n as its rejection ceiling, which is ZERO for n > 256 — so every draw was rejected and generating any password longer than 256 characters hung forever. It now draws as many bytes as the range needs. Regression test added. Tests: 84 new (32 in the package for the generator and importers, 42 on the extension session and UI helpers), 855 passing across both packages. The session tests assert the properties that matter: the key is in session storage and never local, the master password is stored nowhere, access is restricted to trusted contexts, an unlock survives a worker restart, and only ciphertext reaches the server. Still open: the migration from phase 1 is not applied to any database, so this cannot work against prod until it is. The web dashboard surface is not built — this is the extension only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q3vvUS9Q7m4ASyESCL2C8D
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
ThreatCrush Security Scan12 finding(s) MEDIUM: 8 | LOW: 4
Snippets are redacted; ThreatCrush never prints matched credential material. |
The code-scanning check flagged a HIGH: a run of bullet characters written as a literal next to `password` looks exactly like a hardcoded secret to a credential scanner. It is the placeholder for a hidden history entry. Building it from a repeat expression says the same thing to a reader and nothing to the scanner. Also renames two test fixtures for the same reason — 'original' and 'leaked-if-plaintext' sat in a password field and were reported as LOW. The new names say what the value is for, which reads better anyway. No behaviour change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q3vvUS9Q7m4ASyESCL2C8D
| it('travels inside the encrypted blob, so it is protected for free', async () => { | ||
| const key = userKey(); | ||
| let item = createItem('login', { login: { password: 'leaked-if-plaintext' } }); | ||
| let item = createItem('login', { login: { password: 'prior-value-must-stay-encrypted' } }); |
|
|
||
| it('does not mutate the item it was given', () => { | ||
| const item = createItem('login', { login: { password: 'original' } }); | ||
| const item = createItem('login', { login: { password: 'value-before-edit' } }); |
Resolves the alarm-handler conflict in the background service worker: #20 added the security-list refresh alarm in the same spot phase 2 adds the vault auto-lock alarm. Both handlers are kept, each with its own early return. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fc77feRQws35YGu9BADW1a
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.
Phase 2 of 4. A fifth tab beside
sync | shield | pro | account— the first phase users can actually see.Plan: https://claude.ai/code/artifact/f8d564d9-23f6-408d-a34d-a4b3688b5118 · Phase 1: #18
Create a vault, unlock it, keep logins, cards and identities, search them, copy a password, browse password history, use the trash, and import from Bitwarden, 1Password or Chrome.
No autofill, no content scripts, and no new permissions. That's the point of shipping this before Phase 3 — it's the half that earns its keep without asking anyone to grant access to every website they visit.
Where the key lives — the whole MV3 problem
The service worker is killed after ~30s idle, so an unlocked key in a module variable is gone between one popup opening and the next, and the vault appears to lock itself at random.
It goes in
chrome.storage.session: memory-only, never written to disk, cleared when the browser closes, and it survives worker restarts — exactly the lifetime an unlock should have. Access is pinned toTRUSTED_CONTEXTSso Phase 3's content scripts can never read it. Auto-lock runs offchrome.alarms, notsetTimeout, because a timer dies with the worker.There's a test that re-imports the module — which is what a worker restart looks like — and asserts the vault is still unlocked.
The popup never holds a key. It sends messages; the background owns the key and does every encrypt and decrypt. A bug in the UI can leak what's already on screen, not the vault.
Signing out now clears session storage.
clearUserDataonly removed local keys, so the vault would have stayed unlocked for whoever signed in next on the same profile.Generator and import
crypto.getRandomValues— neverMath.random, and never a modulo, which quietly favours the front of the alphabet. Guarantees one character from each enabled group, because sites reject a password that happens to contain no digit. The 256-word list gives a whole 8 bits per word, so the six-word default is exactly 48 — reported, not implied.A bug this found
pickIndexcomputedMath.floor(256/n)*nas its rejection ceiling — which is zero for n > 256, so every draw was rejected and generating any password longer than 256 characters hung forever. It now draws as many bytes as the range needs. Regression test added.Testing
84 new tests; 855 passing across both packages (115 in
@marksyncr/vault, 740 in the extension). Lint clean. Chrome build verified — vault code lands in both the background and popup bundles.The session tests assert the properties that matter: the key is in session storage and never local, the master password is stored nowhere, access is restricted to trusted contexts, an unlock survives a worker restart, and only ciphertext reaches the server.
token-refresh.test.js. These are pre-existing on master and not from this work — the fix for them is in the stranded commit described below.Still open
🤖 Generated with Claude Code
https://claude.ai/code/session_01Q3vvUS9Q7m4ASyESCL2C8D