Tested against main (4abcf33).
Two findings split out of #44 (items 1 and 3). They are separate code paths but the same defect: a security control that does less than the UI says it does, in a place where the gap only matters to a user who is already worried.
Both are fixed in the PR that follows; #44 can have items 1 and 3 checked off when it lands.
1. secure_reset_vault promises containment it does not deliver
What happens
vault_commands.rs:74-121 does four things: it verifies the passphrase, deletes the vault-bound Keychain item, unlinks the vault file, and advances auth_generation while clearing the in-memory passphrase.
It does not touch state.sessions, state.sftp or state.local_terminals (state.rs:56-58). Nothing else does either — the frontend's publishDestroyedVaultState() (stores/vault.ts:24-33) flips three flags and empties the key-metadata store; it closes no panes and calls no backend command.
So after a reset:
- Every SSH session opened before it is still in
SessionManager's map, so session_write (commands.rs:490) and session_resize still route to it.
- Every SFTP channel is still in
SftpManager's map, so sftp_list_dir, sftp_write_file, sftp_remove_file and friends still work against it.
- Every local PTY is still in
state.local_terminals, still fed by its reader thread and still writable.
None of that depends on the vault, because none of those commands consults it after the connection is up. The key that authenticated the connection is gone; the connection is not.
What the user is told
VaultResetForm.vue:26-27:
Resetting the vault permanently deletes all stored SSH keys and credentials. This cannot be undone. Enter your master passphrase to confirm.
That is accurate about the keys and silent about the access. The person most likely to read it and press the button is someone who thinks something is wrong — the copy reads as "shut it all down", and the reset is otherwise built like a containment control: it is passphrase-gated, it removes the biometric credential before it removes the binding id, it invalidates authentication even when the directory fsync fails. Leaving the live connections out of that is the one place where the mechanism stops short of what the surrounding design implies.
Which way to fix it
Either drop the sessions, or say in the dialog that they survive.
I think dropping them is right. The reset is the only "make it stop" gesture the app has, and a reset that leaves a writable shell on the machine you were worried about is worse than no reset, because the user now believes they are done. The cost is small: local terminals are cheap to reopen and SSH sessions have to be re-established after a reset anyway, since the keys that opened them no longer exist.
The failure mode worth being careful about is that closing a connection is network work and network work can hang. SessionManager::close (session.rs:177-184) holds the session map's lock across handle.disconnect(...).await, so a stalled remote blocks every other caller of that map for as long as it stalls. Dropped naively into secure_reset_vault, that turns into a command that never returns after the vault is already destroyed — the renderer never gets its result, so publishDestroyedVaultState() never runs and the UI keeps showing an unlocked vault that no longer exists.
So whatever closes the sessions has to (a) run only after reset_crossed_destructive_boundary (vault_commands.rs:12-14) says the vault is gone, (b) sever addressability before it does any network work rather than after, and (c) be unable to change the value normalize_reset_result computed. A reset that destroyed the vault has to keep reporting that, including the [vault-reset-durability] marker path that the frontend keys off (stores/vault.ts:213-225).
2. An imported private key survives Cancel and failed imports, in plaintext
What happens
KeyManager.vue:33-38 keeps the add-key dialog's state in a component ref:
const addForm = ref({ label: "", import: false, privateKey: "", passphrase: "" });resetForm() (lines 70-73) is the only thing that clears it, and it runs on the success path only — line 48 after generate, line 63 after import. Three exits skip it:
- the failed-import
catch at lines 64-67, which only console.errors and alerts;
- the Cancel button at line 264,
@click="showAdd = false";
- the dialog's own dismissal at line 205,
@close="showAdd = false" (overlay click / Escape).
Dialog.vue:22-23 uses v-if, so the DOM node really is destroyed — but the ref is not, and the next time the dialog opens it is rendered straight back into an unmasked <Textarea> (lines 244-250).
browseForKeyFile() (lines 91-112) is what makes this more than a paste-buffer problem: it reads a whole private key file off disk into that same field, so the retained value is the complete contents of ~/.ssh/id_ed25519 rather than something the user typed and could reasonably expect to still be there.
Why the lock does not help
The obvious mitigation is not one. VaultUnlockModal is a sibling overlay in App.vue:105, rendered next to the view rather than in place of it, so locking the vault — including the 15-minute auto-lock — does not unmount KeyManager. Concretely: Browse in ~/.ssh/id_ed25519, the import fails on a wrong passphrase or an unsupported format, hit Cancel, walk away. The vault auto-locks, which reads as "safe now". Reopen Add Key and the key is there in clear text, with no vault authentication in between.
To be fair about the size of it: App.vue:94 mounts KeyManager with v-else-if, so navigating to any other view unmounts the component and drops the ref. The window is "user stays on the Keys view", and the material never leaves the renderer's heap. It is a retention bug, not a disclosure to anything remote.
The pattern to copy
The codebase already handles this correctly twice, and neither place does anything clever:
SshPasswordPrompt.vue:18-46 clears the reactive ref and the live input on every exit, with onBeforeUnmount(() => finish(null, false)) and a flush: "sync" watcher on the pane losing ownership.VaultUnlockModal.vue:18-27 clears synchronously at both the visibility boundary and the lock/unlock boundary, plus onUnmounted.
KeyManager should do the same: resetForm() from the failure branch and both cancel paths, plus a watcher on the dialog's visibility and an unmount hook so a fourth exit path added later cannot quietly reopen the window.
On masking the textarea
Worth mentioning because it looks like the obvious companion fix, and I do not think it is one. The passphrase field next to it is type="password", but a <textarea> has no equivalent — masking it means -webkit-text-security or a fake overlay, which breaks the one thing users legitimately need this field for, which is eyeballing that they pasted the right key. And it addresses the wrong half: the material being visible while the user is deliberately looking at it is fine. The defect is that it is still there afterwards.
Tested against
main(4abcf33).Two findings split out of #44 (items 1 and 3). They are separate code paths but the same defect: a security control that does less than the UI says it does, in a place where the gap only matters to a user who is already worried.
Both are fixed in the PR that follows; #44 can have items 1 and 3 checked off when it lands.
1.
secure_reset_vaultpromises containment it does not deliverWhat happens
vault_commands.rs:74-121does four things: it verifies the passphrase, deletes the vault-bound Keychain item, unlinks the vault file, and advancesauth_generationwhile clearing the in-memory passphrase.It does not touch
state.sessions,state.sftporstate.local_terminals(state.rs:56-58). Nothing else does either — the frontend'spublishDestroyedVaultState()(stores/vault.ts:24-33) flips three flags and empties the key-metadata store; it closes no panes and calls no backend command.So after a reset:
SessionManager's map, sosession_write(commands.rs:490) andsession_resizestill route to it.SftpManager's map, sosftp_list_dir,sftp_write_file,sftp_remove_fileand friends still work against it.state.local_terminals, still fed by its reader thread and still writable.None of that depends on the vault, because none of those commands consults it after the connection is up. The key that authenticated the connection is gone; the connection is not.
What the user is told
VaultResetForm.vue:26-27:That is accurate about the keys and silent about the access. The person most likely to read it and press the button is someone who thinks something is wrong — the copy reads as "shut it all down", and the reset is otherwise built like a containment control: it is passphrase-gated, it removes the biometric credential before it removes the binding id, it invalidates authentication even when the directory fsync fails. Leaving the live connections out of that is the one place where the mechanism stops short of what the surrounding design implies.
Which way to fix it
Either drop the sessions, or say in the dialog that they survive.
I think dropping them is right. The reset is the only "make it stop" gesture the app has, and a reset that leaves a writable shell on the machine you were worried about is worse than no reset, because the user now believes they are done. The cost is small: local terminals are cheap to reopen and SSH sessions have to be re-established after a reset anyway, since the keys that opened them no longer exist.
The failure mode worth being careful about is that closing a connection is network work and network work can hang.
SessionManager::close(session.rs:177-184) holds the session map's lock acrosshandle.disconnect(...).await, so a stalled remote blocks every other caller of that map for as long as it stalls. Dropped naively intosecure_reset_vault, that turns into a command that never returns after the vault is already destroyed — the renderer never gets its result, sopublishDestroyedVaultState()never runs and the UI keeps showing an unlocked vault that no longer exists.So whatever closes the sessions has to (a) run only after
reset_crossed_destructive_boundary(vault_commands.rs:12-14) says the vault is gone, (b) sever addressability before it does any network work rather than after, and (c) be unable to change the valuenormalize_reset_resultcomputed. A reset that destroyed the vault has to keep reporting that, including the[vault-reset-durability]marker path that the frontend keys off (stores/vault.ts:213-225).2. An imported private key survives Cancel and failed imports, in plaintext
What happens
KeyManager.vue:33-38keeps the add-key dialog's state in a component ref:resetForm()(lines 70-73) is the only thing that clears it, and it runs on the success path only — line 48 after generate, line 63 after import. Three exits skip it:catchat lines 64-67, which onlyconsole.errors andalerts;@click="showAdd = false";@close="showAdd = false"(overlay click / Escape).Dialog.vue:22-23usesv-if, so the DOM node really is destroyed — but the ref is not, and the next time the dialog opens it is rendered straight back into an unmasked<Textarea>(lines 244-250).browseForKeyFile()(lines 91-112) is what makes this more than a paste-buffer problem: it reads a whole private key file off disk into that same field, so the retained value is the complete contents of~/.ssh/id_ed25519rather than something the user typed and could reasonably expect to still be there.Why the lock does not help
The obvious mitigation is not one.
VaultUnlockModalis a sibling overlay inApp.vue:105, rendered next to the view rather than in place of it, so locking the vault — including the 15-minute auto-lock — does not unmountKeyManager. Concretely: Browse in~/.ssh/id_ed25519, the import fails on a wrong passphrase or an unsupported format, hit Cancel, walk away. The vault auto-locks, which reads as "safe now". Reopen Add Key and the key is there in clear text, with no vault authentication in between.To be fair about the size of it:
App.vue:94mountsKeyManagerwithv-else-if, so navigating to any other view unmounts the component and drops the ref. The window is "user stays on the Keys view", and the material never leaves the renderer's heap. It is a retention bug, not a disclosure to anything remote.The pattern to copy
The codebase already handles this correctly twice, and neither place does anything clever:
SshPasswordPrompt.vue:18-46clears the reactive ref and the live input on every exit, withonBeforeUnmount(() => finish(null, false))and aflush: "sync"watcher on the pane losing ownership.VaultUnlockModal.vue:18-27clears synchronously at both the visibility boundary and the lock/unlock boundary, plusonUnmounted.KeyManagershould do the same:resetForm()from the failure branch and both cancel paths, plus a watcher on the dialog's visibility and an unmount hook so a fourth exit path added later cannot quietly reopen the window.On masking the textarea
Worth mentioning because it looks like the obvious companion fix, and I do not think it is one. The passphrase field next to it is
type="password", but a<textarea>has no equivalent — masking it means-webkit-text-securityor a fake overlay, which breaks the one thing users legitimately need this field for, which is eyeballing that they pasted the right key. And it addresses the wrong half: the material being visible while the user is deliberately looking at it is fine. The defect is that it is still there afterwards.