diff --git a/extension/src/content/detector.ts b/extension/src/content/detector.ts index f369f0e..bfc19d8 100644 --- a/extension/src/content/detector.ts +++ b/extension/src/content/detector.ts @@ -93,9 +93,21 @@ setupFillListener(); scan(); // Watch for DOM changes (SPA navigation, dynamically loaded forms). -const observer = new MutationObserver(() => { - scan(); -}); +// Plan C Phase 5: SPA churn fires the MutationObserver many times per +// second. Trailing-edge debounce coalesces bursts so we run the full +// scan() at most once per quiet 200ms window. +const SCAN_DEBOUNCE_MS = 200; +let scanTimer: ReturnType | undefined; + +function scheduleScan(): void { + if (scanTimer !== undefined) clearTimeout(scanTimer); + scanTimer = setTimeout(() => { + scanTimer = undefined; + scan(); + }, SCAN_DEBOUNCE_MS); +} + +const observer = new MutationObserver(scheduleScan); observer.observe(document.body, { childList: true, diff --git a/extension/src/popup/components/__tests__/devices.test.ts b/extension/src/popup/components/__tests__/devices.test.ts index 60c31b7..2b0233c 100644 --- a/extension/src/popup/components/__tests__/devices.test.ts +++ b/extension/src/popup/components/__tests__/devices.test.ts @@ -95,6 +95,32 @@ describe('devices view', () => { expect(app.querySelector('#register-confirm-btn')).not.toBeNull(); }); + // Plan C Phase 5 — defensive Promise.allSettled: + // a rejected secondary feed (list_revoked) should not kill the whole render. + it('renders devices when revoked list fails (load-error slot shown)', async () => { + (sendMessage as ReturnType) + .mockResolvedValueOnce({ ok: true, data: { devices: [{ name: 'CLI', public_key: 'k', added_at: 1 }] } }) + .mockRejectedValueOnce(new Error('boom')); + + await renderDevices(app); + + // Primary list still rendered. + expect(app.innerHTML).toContain('CLI'); + // Inline fallback slot present. + expect(app.innerHTML).toContain("Couldn't load revoked devices"); + }); + + it('renders devices when revoked list returns {ok:false}', async () => { + (sendMessage as ReturnType) + .mockResolvedValueOnce({ ok: true, data: { devices: [{ name: 'CLI', public_key: 'k', added_at: 1 }] } }) + .mockResolvedValueOnce({ ok: false, error: 'list_revoked_failed' }); + + await renderDevices(app); + + expect(app.innerHTML).toContain('CLI'); + expect(app.innerHTML).toContain("Couldn't load revoked devices"); + }); + it('confirming register sends register_this_device with the entered name', async () => { (chrome.storage.local.get as ReturnType).mockResolvedValueOnce({ device_name: 'Unknown' }); // Initial render: list_devices + list_revoked. diff --git a/extension/src/popup/components/devices.ts b/extension/src/popup/components/devices.ts index 1c83394..8b85373 100644 --- a/extension/src/popup/components/devices.ts +++ b/extension/src/popup/components/devices.ts @@ -31,35 +31,64 @@ export function teardown(): void { // No cleanup needed } +/** + * DEV-C P2: defensive per-slot rendering. The active list is the primary + * feed — if it fails entirely, we still surface an error page. The + * revoked list is secondary — its failure renders an inline "couldn't + * load" slot but doesn't kill the page. + */ +function revokedLoadErrorHtml(): string { + return ` +
+ ▸ revoked devices +
+

Couldn't load revoked devices.

+
+
+ `; +} + export async function renderDevices(app: HTMLElement): Promise { // Get current device name from local storage const stored = await chrome.storage.local.get(['device_name']); const currentDeviceName: string | undefined = stored.device_name as string | undefined; - // Fetch active device list and revoked list in parallel - const [devicesResp, revokedResp] = await Promise.all([ + // Fetch active device list and revoked list in parallel. allSettled so a + // rejected secondary feed doesn't kill the whole render. + const [devicesSettled, revokedSettled] = await Promise.allSettled([ sendMessage({ type: 'list_devices' }), sendMessage({ type: 'list_revoked' }), ]); - if (!devicesResp.ok) { + if (devicesSettled.status === 'rejected' || !devicesSettled.value.ok) { app.innerHTML = `

Failed to load devices

`; return; } - const devices = (devicesResp.data as { devices: Device[] }).devices; - const revokedDevices: RevokedEntry[] = revokedResp.ok - ? (revokedResp.data as { revoked: RevokedEntry[] }).revoked + // devicesSettled.value.ok is true here (guarded above), so .data is present. + const devicesData = (devicesSettled.value as { ok: true; data: unknown }).data; + const devices = (devicesData as { devices: Device[] }).devices; + const revokedOk = revokedSettled.status === 'fulfilled' && revokedSettled.value.ok; + const revokedDevices: RevokedEntry[] = revokedOk + ? ((revokedSettled.value as { ok: true; data: unknown }).data as { revoked: RevokedEntry[] }).revoked : []; const isRegistered = currentDeviceName && devices.some((d) => d.name === currentDeviceName); - // Precompute fingerprints for all active devices + // Precompute fingerprints for all active devices. allSettled so one bad + // public key doesn't kill the whole list — fall back to '(unknown)'. const fingerprints = new Map(); - await Promise.all(devices.map(async (d) => { - const fp = await sshFingerprint(d.public_key); - fingerprints.set(d.name, fp ?? '(unknown)'); - })); + const fpResults = await Promise.allSettled( + devices.map((d) => sshFingerprint(d.public_key).then((fp) => [d.name, fp] as const)), + ); + for (let i = 0; i < devices.length; i += 1) { + const r = fpResults[i]; + if (r.status === 'fulfilled' && r.value[1]) { + fingerprints.set(r.value[0], r.value[1]); + } else { + fingerprints.set(devices[i].name, '(unknown)'); + } + } const activeDevicesHtml = devices.length === 0 ? `

No devices registered

` @@ -82,7 +111,9 @@ export async function renderDevices(app: HTMLElement): Promise { `; }).join(''); - const revokedSectionHtml = revokedDevices.length === 0 ? '' : ` + const revokedSectionHtml = !revokedOk + ? revokedLoadErrorHtml() + : revokedDevices.length === 0 ? '' : `
▸ show ${revokedDevices.length} revoked device${revokedDevices.length !== 1 ? 's' : ''}
@@ -117,7 +148,7 @@ export async function renderDevices(app: HTMLElement): Promise { ` : ''} ${devices.length > 0 ? `
ACTIVE · ${devices.length}
` : ''} ${activeDevicesHtml} - ${revokedDevices.length > 0 ? `
REVOKED · ${revokedDevices.length}
` : ''} + ${!revokedOk ? `
REVOKED · ?
` : (revokedDevices.length > 0 ? `
REVOKED · ${revokedDevices.length}
` : '')} ${revokedSectionHtml}
`; diff --git a/extension/src/popup/components/settings-vault.ts b/extension/src/popup/components/settings-vault.ts index cc5038e..36add5b 100644 --- a/extension/src/popup/components/settings-vault.ts +++ b/extension/src/popup/components/settings-vault.ts @@ -9,6 +9,7 @@ import type { import type { SessionTimeoutConfig } from '../../shared/messages'; import { relativeTime } from '../../shared/relative-time'; import { openGeneratorPanel, closeGeneratorPanel, isGeneratorPanelOpen } from './generator-panel'; +import { teardownSettingsCommon } from './settings'; import { GLYPH_NEXT } from '../../shared/glyphs'; let pendingSettings: VaultSettings | null = null; @@ -17,11 +18,7 @@ let pendingSession: SessionTimeoutConfig | null = null; let baseSession: SessionTimeoutConfig | null = null; export function teardown(): void { - closeGeneratorPanel(); - if (activeKeyHandler) { - document.removeEventListener('keydown', activeKeyHandler); - activeKeyHandler = null; - } + activeKeyHandler = teardownSettingsCommon(activeKeyHandler); pendingSettings = null; pendingSession = null; baseSession = null; diff --git a/extension/src/popup/components/settings.ts b/extension/src/popup/components/settings.ts index a4021ba..cacd7e6 100644 --- a/extension/src/popup/components/settings.ts +++ b/extension/src/popup/components/settings.ts @@ -53,13 +53,29 @@ export async function renderSettings(container: HTMLElement): Promise { await renderSection(activeSection); } -export function teardownSettings(): void { +/** + * Common cleanup invoked by both the device-settings teardown + * (settings.ts) and the vault-settings teardown (settings-vault.ts). + * Centralized to avoid the "regression class with known prior leaks" + * DEV-C P2 flagged. + * + * Closes the generator popover and detaches the supplied keydown + * handler from the document if present. Returns the new handler value + * (always null), so the caller can do `handler = teardownSettingsCommon(handler)`. + */ +export function teardownSettingsCommon( + keyHandler: ((e: KeyboardEvent) => void) | null, +): null { closeGeneratorPanel(); - teardownSecuritySection(); - if (activeKeyHandler) { - document.removeEventListener('keydown', activeKeyHandler); - activeKeyHandler = null; + if (keyHandler) { + document.removeEventListener('keydown', keyHandler); } + return null; +} + +export function teardownSettings(): void { + activeKeyHandler = teardownSettingsCommon(activeKeyHandler); + teardownSecuritySection(); pendingVaultSettings = null; sessionHandle = null; } diff --git a/extension/src/service-worker/__tests__/session-timer.test.ts b/extension/src/service-worker/__tests__/session-timer.test.ts index 39a56fc..93a04aa 100644 --- a/extension/src/service-worker/__tests__/session-timer.test.ts +++ b/extension/src/service-worker/__tests__/session-timer.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'; import * as timer from '../session-timer'; +import { READ_ONLY_CONTENT_CALLABLE } from '../session-timer'; describe('session-timer', () => { beforeEach(() => { @@ -97,3 +98,29 @@ describe('session-timer', () => { expect(cb).not.toHaveBeenCalled(); }); }); + +describe('READ_ONLY_CONTENT_CALLABLE — inversion exclusion set', () => { + // The SW handler invokes resetTimer() on every message whose type is NOT + // in this set. These cases encode the documented inversion contract from + // Plan C Phase 5: popup-only messages reset, content-callable writes + // reset, only passive content reads (currently just get_autofill_candidates) + // do NOT reset. + + it('popup-only message would reset the timer (not in exclusion set)', () => { + // e.g. list_items — popup interaction is unambiguously active use + expect(READ_ONLY_CONTENT_CALLABLE.has('list_items')).toBe(false); + }); + + it('content-callable get_autofill_candidates does NOT reset (in exclusion set)', () => { + expect(READ_ONLY_CONTENT_CALLABLE.has('get_autofill_candidates')).toBe(true); + }); + + it('content-callable capture_save_login DOES reset (write op = active use)', () => { + expect(READ_ONLY_CONTENT_CALLABLE.has('capture_save_login')).toBe(false); + }); + + it('content-callable check_credential DOES reset', () => { + // Asking "is this credential already saved" is user-initiated. + expect(READ_ONLY_CONTENT_CALLABLE.has('check_credential')).toBe(false); + }); +}); diff --git a/extension/src/service-worker/index.ts b/extension/src/service-worker/index.ts index e3dec89..2c68a3e 100644 --- a/extension/src/service-worker/index.ts +++ b/extension/src/service-worker/index.ts @@ -2,12 +2,12 @@ /// forwards every message into router/index.route(). import type { Request, Response, SessionTimeoutConfig } from '../shared/messages'; -import { CONTENT_CALLABLE_TYPES } from '../shared/messages'; import type { RouterState } from './router/index'; import { route } from './router/index'; import * as vault from './vault'; import { clearCurrent } from './session'; import * as sessionTimer from './session-timer'; +import { READ_ONLY_CONTENT_CALLABLE } from './session-timer'; // @ts-ignore TS2307 — resolved by webpack alias / copy import initDefault, { initSync } from '../../wasm/relicario_wasm.js'; @@ -53,6 +53,9 @@ sessionTimer.onExpired(() => { console.log('[relicario sw] session expired — locking vault'); clearCurrent(); state.manifest = null; + // Plan C Phase 5: don't leak the cached git-host client across a lock. + // The initializer rebuilds gitHost on demand, so clearing here is safe. + state.gitHost = null; // Best-effort broadcast — receiver may not exist yet. chrome.runtime.sendMessage({ type: 'session_expired' }).catch(() => {}); }); @@ -73,7 +76,10 @@ chrome.commands.onCommand.addListener((command) => { chrome.runtime.onMessage.addListener( (request: Request, sender: chrome.runtime.MessageSender, sendResponse: (r: Response) => void) => { (async () => { - if (!CONTENT_CALLABLE_TYPES.has(request.type as never)) { + // Plan C Phase 5: invert the reset rule. Reset on every message + // except a documented passive-read exclusion set, so an active + // autofiller / content-driven flow keeps the vault alive. + if (!READ_ONLY_CONTENT_CALLABLE.has(request.type)) { sessionTimer.resetTimer(); } if (!state.wasm) { diff --git a/extension/src/service-worker/session-timer.ts b/extension/src/service-worker/session-timer.ts index 9948586..2a7800a 100644 --- a/extension/src/service-worker/session-timer.ts +++ b/extension/src/service-worker/session-timer.ts @@ -48,3 +48,17 @@ export function stopTimer(): void { timerId = null; } } + +/** + * Content-callable message types that should NOT reset the inactivity timer. + * + * Rationale: a content script reading available autofill candidates is a + * passive query — it shouldn't keep the vault alive indefinitely while the + * user isn't actually interacting with it. + * + * Today this is the only known passive read; if a future content message + * is also passive, add it here with a one-line justification. + */ +export const READ_ONLY_CONTENT_CALLABLE: ReadonlySet = new Set([ + 'get_autofill_candidates', +]);