From ee3ef10ee132bcb97ed9b08f134e43508deddfc2 Mon Sep 17 00:00:00 2001 From: Conduction Release Bot Date: Mon, 31 Aug 2026 16:14:22 +0200 Subject: [PATCH] fix(vault): retire a secret's plaintext when the sidebar loads another PasswordField decrypts lazily and then caches the plaintext for its own lifetime: toggle() only resolves while `plain` is still null, and `revealed` persists too. That was harmless while the detail was a full page that remounted per secret. SecretDetailSidebar deliberately does NOT remount. Its own secretId watcher says so: clicking another row "swaps the :id segment without remounting this component". So the cache outlived the secret it belonged to, in two ways that both matter for a password manager: - Edit the open secret: load() refreshes the secret, the field kept showing the OLD plaintext. This is what the e2e caught, asserting the edited value and receiving the previous one. - Click another row while revealed: the panel showed secret B's name with secret A's plaintext, and Copy resolved A's value. The watcher already reset cardRevealed and bsnRevealed for exactly this reason. It could not reach inside PasswordField, so the password row was the one field that kept its cleartext across the swap. Binding :key to a token bumped on every successful load() remounts the field, which resets `plain` and `revealed` together. A changed secret is re-masked until the user asks for it again. Also: click the Move menuitem by role instead of dispatching el.click() at a node guessed from NcActionButton's markup. The old approach broke silently after the restyle. The trace shows the testid resolving in 0.1s and .move-form then timing out for 10s, with the menu still expanded and menuitem "Move" present in the snapshot: a click landing on the wrong node is indistinguishable from a dialog that refuses to open. The accessibility tree exposes the item whatever element renders it. Fixes two e2e failures on development introduced by the Stage-8 restyle, and two pre-existing lint errors in the same spec file. --- src/components/SecretDetailSidebar.vue | 31 +++++++++++++++++++++- tests/e2e/workflows/folder-sharing.spec.ts | 31 +++++++++++++++------- 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/src/components/SecretDetailSidebar.vue b/src/components/SecretDetailSidebar.vue index 6ae5447a..a931e1bf 100644 --- a/src/components/SecretDetailSidebar.vue +++ b/src/components/SecretDetailSidebar.vue @@ -193,7 +193,10 @@
- +
@@ -955,6 +958,29 @@ export default { cardRevealed: { number: false, cvv: false, pin: false }, /** Reveal state for the identity's BSN row (masked by default). */ bsnRevealed: false, + /** + * Bumped by every successful `load()`, and bound to + * `` so the field remounts whenever the + * loaded secret changes. + * + * PasswordField decrypts lazily and then CACHES the plaintext + * for its own lifetime (`plain` is only resolved while it is + * still `null`), and it keeps `revealed` across that lifetime + * too. That was harmless while the detail was a full page which + * remounted per secret. This sidebar deliberately does NOT + * remount — the `secretId` watcher above swaps the secret in + * place — so without a key the cache outlives the secret it + * belongs to, in two ways that both matter for a vault: + * + * • Edit the open secret: `load()` refreshes `this.secret`, + * the field keeps showing the OLD plaintext. + * • Click another row while revealed: the panel shows secret + * B's name with secret A's plaintext, and Copy copies A's. + * + * Remounting resets `plain` and `revealed` together, so a + * changed secret is re-masked until the user asks for it again. + */ + secretLoadToken: 0, } }, @@ -1340,6 +1366,9 @@ export default { this.error = '' try { this.secret = await useSecretStore().fetchSecret(this.secretId) + // Retire the previous secret's decrypted plaintext with the + // secret it came from. See `secretLoadToken` in data(). + this.secretLoadToken += 1 // Write-grade badge (folder-permission-grades §4.3) — a // copy the user may team-write shows the sync warning. try { diff --git a/tests/e2e/workflows/folder-sharing.spec.ts b/tests/e2e/workflows/folder-sharing.spec.ts index 939efeb5..c1e8611f 100644 --- a/tests/e2e/workflows/folder-sharing.spec.ts +++ b/tests/e2e/workflows/folder-sharing.spec.ts @@ -39,13 +39,13 @@ * were credited to zero scenarios. They are anchored per-test below, against the * `secrets-write-ui` scenarios they actually drive. */ -import { test, expect } from '@playwright/test' +import { expect, test } from '@playwright/test' import { clickOverflowAction, gotoLockSettled, openVault, unlockVault, -} from './_workflow-helpers' +} from './_workflow-helpers.ts' const REQ_TOKEN = `(() => { const head = document.querySelector('head[data-requesttoken]'); @@ -279,14 +279,25 @@ test.describe('Workflow: folders + sharing — folders/spec.md', () => { await page .getByRole('button', { name: /Secret actions/i }) .evaluate((el: HTMLElement) => el.click()) - // `data-testid` falls through to NcActionButton's ROOT, which is the - //
  • wrapper, not the