fix(ext): unswallow free() errors in SW session.clearCurrent + vitest
Phase 1 added impl Drop for SessionHandle on the Rust side so .free()
now actually removes the SESSIONS registry entry. The JS-side
try { current.free() } catch { /* already freed */ } swallow was
hiding the fact that .free() wasn't doing the cleanup at all;
post-Phase-1 it has to go so failures surface instead of being lost.
.free() callsite audit: exactly one match under extension/src/ — the
SW session.ts line this commit edits. Lifecycle audit: clearCurrent()
is reached via (a) popup lock → router popup-only.ts and (b)
session-timer expiry → service-worker/index.ts.
Refs: docs/superpowers/specs/2026-05-04-security-polish-design.md (Phase 2)
Refs: docs/superpowers/reviews/2026-05-04-architecture-review.md (P1.1, DEV-C P2 service-worker)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
37
extension/src/service-worker/__tests__/session.test.ts
Normal file
37
extension/src/service-worker/__tests__/session.test.ts
Normal file
@@ -0,0 +1,37 @@
|
||||
import { describe, expect, it, vi, beforeEach } from 'vitest';
|
||||
import type { SessionHandle } from '../../../wasm/relicario_wasm';
|
||||
import { clearCurrent, getCurrent, setCurrent } from '../session';
|
||||
|
||||
describe('session', () => {
|
||||
beforeEach(() => {
|
||||
// Reset module-scope `current` between tests. Overwrite first with a
|
||||
// benign no-throw fake so a prior test's throwing handle can't escape.
|
||||
setCurrent({ free: vi.fn(), value: 0 } as unknown as SessionHandle);
|
||||
clearCurrent();
|
||||
});
|
||||
|
||||
it('clearCurrent() is a no-op when no handle is set', () => {
|
||||
expect(() => clearCurrent()).not.toThrow();
|
||||
expect(getCurrent()).toBeNull();
|
||||
});
|
||||
|
||||
it('clearCurrent() calls free() exactly once and clears current', () => {
|
||||
const free = vi.fn();
|
||||
setCurrent({ free, value: 1 } as unknown as SessionHandle);
|
||||
|
||||
clearCurrent();
|
||||
expect(free).toHaveBeenCalledTimes(1);
|
||||
expect(getCurrent()).toBeNull();
|
||||
|
||||
clearCurrent();
|
||||
expect(free).toHaveBeenCalledTimes(1);
|
||||
expect(getCurrent()).toBeNull();
|
||||
});
|
||||
|
||||
it('clearCurrent() propagates exceptions from free()', () => {
|
||||
const free = vi.fn(() => { throw new Error('boom'); });
|
||||
setCurrent({ free, value: 2 } as unknown as SessionHandle);
|
||||
|
||||
expect(() => clearCurrent()).toThrow(/boom/);
|
||||
});
|
||||
});
|
||||
@@ -7,6 +7,12 @@
|
||||
/// Future multi-vault (β+) would replace `current` with
|
||||
/// `Map<vaultId, SessionHandle>` and thread `vaultId` through every
|
||||
/// handler. Deliberate α simplicity — not an oversight.
|
||||
///
|
||||
/// As of Phase 1 of the security-polish series, `impl Drop for SessionHandle`
|
||||
/// on the Rust side makes `.free()` the meaningful cleanup call: it removes
|
||||
/// the entry from the SESSIONS registry and zeroizes `master_key` and
|
||||
/// `image_secret`. Calling `wasm.lock(handle.value)` before `.free()` would
|
||||
/// be redundant belt-and-suspenders; this module intentionally does not.
|
||||
|
||||
import type { SessionHandle } from '../../wasm/relicario_wasm';
|
||||
|
||||
@@ -23,6 +29,6 @@ export function requireCurrent(): SessionHandle {
|
||||
|
||||
export function clearCurrent(): void {
|
||||
if (!current) return;
|
||||
try { current.free(); } catch { /* already freed */ }
|
||||
current.free();
|
||||
current = null;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user