mirror of
https://github.com/Memo-2023/mana-monorepo.git
synced 2026-05-14 23:41:08 +02:00
fix(mana/web): handle vault-locked race + add guest plaintext fallback
Two related issues in the encryption pipeline that were both surfacing as silent failures when a user tried to log a mood / write to any encrypted field shortly after page load or while signed out: 1. Boot-time race The layout fires authStore.initialize() and vaultClient.unlock() in the same tick. The very first user mutation can land before the network round-trip that fetches the master key returns. encryptRecord then synchronously sees a null key and throws VaultLockedError — surfacing in the UI as "click does nothing" because nothing in the call chain catches it. Fix: KeyProvider gets a waitForKey(timeoutMs) method. MemoryKeyProvider implements it via the existing onChange listener, so callers resume as soon as setKey lands. encryptRecord now waits up to 2 s before throwing, which converts a near-miss race into a transparent millisecond delay. 2. Guest plaintext fallback (Option A in the chat thread) Guests have no auth token, so the server vault is unreachable by definition. Refusing every encrypted-field write would hide the bulk of the app behind a sign-up wall — undesirable for the try-before-you-buy local-first flow. Fix: encryptRecord now silently no-ops when getCurrentUserId() is null, writing plaintext to the local Dexie. guest-migration.ts waits for the vault (10 s budget) and then encrypts the registry fields per-table BEFORE the re-insert, so the on-disk state after sign-in matches "user signed up first, then typed everything". If the vault never opens (auth/network failure on /me/encryption-vault), migration aborts cleanly — guest data stays put rather than being re-inserted as plaintext under the real user id. UI side: cycles/ListView.svelte wraps every dayLogsStore.logDay call in a safeLogDay helper that catches VaultLockedError and surfaces a toast pointing the user at Settings → Sicherheit. Previously the unhandled rejection from a click handler vanished into the console. Tests: - record-helpers.test.ts now stamps a fake current user in beforeEach so the new guest-skip doesn't no-op the encryption asserts. The "throws when locked" test uses fake timers to flush the 2 s wait without sitting on it. - aes.test.ts: anonymous-class KeyProvider stub gains the new waitForKey method to satisfy the interface. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
1702caa4f7
commit
752f31bfad
6 changed files with 151 additions and 9 deletions
|
|
@ -192,6 +192,9 @@ describe('KeyProvider', () => {
|
|||
onChange() {
|
||||
return () => {};
|
||||
}
|
||||
async waitForKey() {
|
||||
return null;
|
||||
}
|
||||
})()
|
||||
);
|
||||
expect(getActiveKey()).toBe(null);
|
||||
|
|
|
|||
|
|
@ -34,6 +34,14 @@ export interface KeyProvider {
|
|||
/** Subscribe to lock/unlock transitions. Returns a dispose function.
|
||||
* Listeners fire only on STATE CHANGES, not on every getKey call. */
|
||||
onChange(listener: (unlocked: boolean) => void): () => void;
|
||||
|
||||
/** Resolves with the active key as soon as the vault is unlocked, or
|
||||
* with `null` if the timeout expires first. Used by encryptRecord to
|
||||
* ride out the boot-time race where the user clicks a mutation button
|
||||
* while the layout's `vaultClient.unlock()` round-trip is still in
|
||||
* flight. Implementations that can never unlock (NullKeyProvider)
|
||||
* resolve immediately with `null`. */
|
||||
waitForKey(timeoutMs: number): Promise<CryptoKey | null>;
|
||||
}
|
||||
|
||||
// ─── NullKeyProvider — default ─────────────────────────────────
|
||||
|
|
@ -54,6 +62,11 @@ class NullKeyProvider implements KeyProvider {
|
|||
onChange(): () => void {
|
||||
return () => {};
|
||||
}
|
||||
async waitForKey(): Promise<null> {
|
||||
// Null provider can never unlock — don't make callers wait the
|
||||
// full timeout for a guaranteed-null answer.
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
// ─── MemoryKeyProvider — Phase 3 production path ───────────────
|
||||
|
|
@ -107,6 +120,26 @@ export class MemoryKeyProvider implements KeyProvider {
|
|||
this.listeners.delete(listener);
|
||||
};
|
||||
}
|
||||
|
||||
waitForKey(timeoutMs: number): Promise<CryptoKey | null> {
|
||||
if (this.key) return Promise.resolve(this.key);
|
||||
return new Promise((resolve) => {
|
||||
let settled = false;
|
||||
const dispose = this.onChange((unlocked) => {
|
||||
if (settled || !unlocked) return;
|
||||
settled = true;
|
||||
clearTimeout(timer);
|
||||
dispose();
|
||||
resolve(this.key);
|
||||
});
|
||||
const timer = setTimeout(() => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
dispose();
|
||||
resolve(this.key); // null on miss
|
||||
}, timeoutMs);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// ─── Module-level active provider ──────────────────────────────
|
||||
|
|
@ -128,6 +161,13 @@ export function getActiveKey(): CryptoKey | null {
|
|||
return _activeProvider.getKey();
|
||||
}
|
||||
|
||||
/** Convenience: waits up to `timeoutMs` (default 2s) for the vault to
|
||||
* unlock. Used by encryptRecord to ride out the boot-time race between
|
||||
* a user click and the layout's async vault unlock round-trip. */
|
||||
export function waitForActiveKey(timeoutMs: number = 2000): Promise<CryptoKey | null> {
|
||||
return _activeProvider.waitForKey(timeoutMs);
|
||||
}
|
||||
|
||||
/** Convenience: synchronous lock check. */
|
||||
export function isVaultUnlocked(): boolean {
|
||||
return _activeProvider.isUnlocked();
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ import { encryptRecord, decryptRecord, decryptRecords, VaultLockedError } from '
|
|||
import { generateMasterKey, isEncrypted } from './aes';
|
||||
import { MemoryKeyProvider, setKeyProvider } from './key-provider';
|
||||
import * as registry from './registry';
|
||||
import { setCurrentUserId } from '../current-user';
|
||||
|
||||
let key: CryptoKey;
|
||||
let provider: MemoryKeyProvider;
|
||||
|
|
@ -24,6 +25,12 @@ beforeEach(async () => {
|
|||
provider.setKey(key);
|
||||
setKeyProvider(provider);
|
||||
|
||||
// encryptRecord silently skips when no user is signed in (guest
|
||||
// mode falls back to plaintext writes; guest-migration re-encrypts
|
||||
// on login). Stamp a fake user id so the encryption path actually
|
||||
// runs in these tests.
|
||||
setCurrentUserId('test-user');
|
||||
|
||||
// Pretend the notes table is enabled with title + body fields.
|
||||
vi.spyOn(registry, 'getEncryptedFields').mockImplementation((tableName: string) => {
|
||||
if (tableName === TEST_TABLE) return ['title', 'body'];
|
||||
|
|
@ -34,6 +41,7 @@ beforeEach(async () => {
|
|||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
provider.setKey(null);
|
||||
setCurrentUserId(null);
|
||||
});
|
||||
|
||||
describe('encryptRecord', () => {
|
||||
|
|
@ -83,12 +91,20 @@ describe('encryptRecord', () => {
|
|||
});
|
||||
|
||||
it('throws VaultLockedError when no key is available', async () => {
|
||||
// encryptRecord waits ~2s for the boot-time unlock race before
|
||||
// giving up. Use fake timers so this test doesn't actually idle
|
||||
// for two seconds — flush the timer manually after kicking off
|
||||
// the call.
|
||||
vi.useFakeTimers();
|
||||
provider.setKey(null);
|
||||
const record = { id: 'n', title: 'secret', body: 'also secret' };
|
||||
await expect(encryptRecord(TEST_TABLE, record)).rejects.toThrow(VaultLockedError);
|
||||
const promise = encryptRecord(TEST_TABLE, record);
|
||||
await vi.advanceTimersByTimeAsync(2500);
|
||||
await expect(promise).rejects.toThrow(VaultLockedError);
|
||||
// Record was not partially mutated
|
||||
expect(record.title).toBe('secret');
|
||||
expect(record.body).toBe('also secret');
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it('does not throw when the vault is locked but no fields need encryption', async () => {
|
||||
|
|
|
|||
|
|
@ -34,8 +34,9 @@
|
|||
*/
|
||||
|
||||
import { wrapValue, unwrapValue, isEncrypted } from './aes';
|
||||
import { getActiveKey, isVaultUnlocked } from './key-provider';
|
||||
import { getActiveKey, isVaultUnlocked, waitForActiveKey } from './key-provider';
|
||||
import { getEncryptedFields } from './registry';
|
||||
import { getCurrentUserId } from '../current-user';
|
||||
|
||||
/** Thrown by encryptRecord when no key is available. Module stores
|
||||
* catch this to surface "vault locked" UI. */
|
||||
|
|
@ -80,7 +81,23 @@ export async function encryptRecord<T extends object>(tableName: string, record:
|
|||
}
|
||||
if (todo.length === 0) return record;
|
||||
|
||||
const key = getActiveKey();
|
||||
// Guest mode: there is no auth token, so the server vault is
|
||||
// unreachable by definition. Falling back to plaintext keeps the
|
||||
// app usable for anonymous local-first writes — guestMigration.ts
|
||||
// will encrypt these records as part of the guest → user re-stamp
|
||||
// when the user eventually signs in. The compromise is documented
|
||||
// in the data-layer audit; the alternative (refusing the write)
|
||||
// hides the entire app behind a sign-up wall.
|
||||
if (getCurrentUserId() === null) return record;
|
||||
|
||||
// Boot-time race: the layout's `vaultClient.unlock()` runs in the
|
||||
// same tick as authStore.initialize(), so the very first user
|
||||
// mutation can land before the network round-trip finishes. Wait a
|
||||
// short window for the provider to flip before we give up — this
|
||||
// converts a near-miss race into a transparent ~ms delay instead of
|
||||
// a thrown VaultLockedError that the UI silently swallows.
|
||||
let key = getActiveKey();
|
||||
if (!key) key = await waitForActiveKey(2000);
|
||||
if (!key) throw new VaultLockedError(tableName);
|
||||
|
||||
for (const field of todo) {
|
||||
|
|
|
|||
|
|
@ -20,10 +20,24 @@
|
|||
* Deleting in guest mode is safe because nothing was ever pushed to the
|
||||
* server: `_pendingChanges` is cleared as part of the migration too, so the
|
||||
* delete is purely local and never reaches the sync layer.
|
||||
*
|
||||
* Encryption catch-up: while in guest mode, encryptRecord skips silently
|
||||
* (no master key available, no auth token to fetch one). Records on
|
||||
* encrypted tables therefore live as PLAINTEXT in IndexedDB until this
|
||||
* migration runs. After login + vault unlock we walk the same set of
|
||||
* records and re-encrypt the registry fields before re-inserting, so
|
||||
* the post-migration state is indistinguishable from "user signed up
|
||||
* first, then typed everything". The migration awaits the vault for up
|
||||
* to 10 s — if it never opens we abort the migration and leave the
|
||||
* guest data in place rather than re-inserting plaintext under the
|
||||
* real user id.
|
||||
*/
|
||||
|
||||
import { db, SYNC_APP_MAP, FIELD_TIMESTAMPS_KEY } from './database';
|
||||
import { GUEST_USER_ID } from './current-user';
|
||||
import { encryptRecord } from './crypto/record-helpers';
|
||||
import { waitForActiveKey } from './crypto/key-provider';
|
||||
import { getEncryptedFields } from './crypto/registry';
|
||||
|
||||
export interface GuestMigrationResult {
|
||||
migratedRecords: number;
|
||||
|
|
@ -41,6 +55,19 @@ export async function migrateGuestDataToUser(newUserId: string): Promise<GuestMi
|
|||
return { migratedRecords: 0, tablesTouched: 0 };
|
||||
}
|
||||
|
||||
// Wait for the vault to unlock before we touch anything. The layout
|
||||
// fires this migration and `vaultClient.unlock()` in the same
|
||||
// effect run, so they race; if we re-insert plaintext under the
|
||||
// real user id before the key arrives, those records would have to
|
||||
// be re-encrypted by a follow-up pass that doesn't exist. Bail out
|
||||
// instead — the guest data stays put and the user can retry by
|
||||
// signing out and back in.
|
||||
const key = await waitForActiveKey(10_000);
|
||||
if (!key) {
|
||||
console.warn('[mana] guest migration aborted: vault did not unlock in time');
|
||||
return { migratedRecords: 0, tablesTouched: 0 };
|
||||
}
|
||||
|
||||
// Drop any pending changes accumulated during guest mode — they were
|
||||
// never pushed (no auth token) and reference the old guest userId. The
|
||||
// re-inserts below will produce fresh pending changes that should NOT be
|
||||
|
|
@ -64,8 +91,17 @@ export async function migrateGuestDataToUser(newUserId: string): Promise<GuestMi
|
|||
if (guestRecords.length === 0) continue;
|
||||
tablesTouched++;
|
||||
|
||||
// Snapshot the encrypted-field allowlist once per table (null if
|
||||
// the table is not in the registry or currently disabled).
|
||||
const encryptedFields = getEncryptedFields(tableName);
|
||||
|
||||
// One transaction per table keeps the delete+add pair atomic and
|
||||
// avoids leaving the table half-migrated if Dexie throws partway.
|
||||
// Note: encryptRecord is async and uses Web Crypto, which is fine
|
||||
// inside a Dexie 'rw' transaction as long as we don't await
|
||||
// non-Dexie work between Dexie ops in a way that suspends the
|
||||
// transaction. Each iteration awaits the encrypt BEFORE touching
|
||||
// the table, then runs the delete+add pair back-to-back.
|
||||
await db.transaction('rw', table, async () => {
|
||||
for (const oldRecord of guestRecords) {
|
||||
const record = oldRecord as Record<string, unknown>;
|
||||
|
|
@ -78,6 +114,18 @@ export async function migrateGuestDataToUser(newUserId: string): Promise<GuestMi
|
|||
void _oldUser;
|
||||
void _oldFt;
|
||||
|
||||
// Catch-up encryption: guest writes left these fields as
|
||||
// plaintext because no key was available. Now that the
|
||||
// vault is unlocked, encrypt them in place before the
|
||||
// re-insert so the on-disk state matches a "logged in
|
||||
// from the start" user. encryptRecord is a no-op for
|
||||
// tables not in the registry, and idempotent for fields
|
||||
// that are somehow already encrypted (e.g. partial
|
||||
// migration retry after a crash).
|
||||
if (encryptedFields) {
|
||||
await encryptRecord(tableName, clean);
|
||||
}
|
||||
|
||||
await table.delete(id);
|
||||
await table.add({ ...clean, id });
|
||||
migratedRecords++;
|
||||
|
|
|
|||
|
|
@ -25,6 +25,8 @@
|
|||
import CycleCalendar from './components/CycleCalendar.svelte';
|
||||
import SymptomManager from './components/SymptomManager.svelte';
|
||||
import type { ViewProps } from '$lib/app-registry';
|
||||
import { toast } from '$lib/stores/toast.svelte';
|
||||
import { VaultLockedError } from '$lib/data/crypto';
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
const _props: ViewProps = $props();
|
||||
|
|
@ -97,31 +99,47 @@
|
|||
editingDate = todayIso;
|
||||
}
|
||||
|
||||
/** Wraps logDay calls so a locked vault becomes a visible toast instead
|
||||
* of a silent unhandled rejection. encryptRecord already waits up to
|
||||
* ~2s for the boot-time unlock race, so reaching this catch means the
|
||||
* vault is genuinely unavailable (auth/network failure on /me/encryption-vault). */
|
||||
async function safeLogDay(payload: Parameters<typeof dayLogsStore.logDay>[0]) {
|
||||
try {
|
||||
await dayLogsStore.logDay(payload);
|
||||
} catch (err) {
|
||||
if (err instanceof VaultLockedError) {
|
||||
toast.error('Vault gesperrt — bitte unter Einstellungen → Sicherheit entsperren.');
|
||||
return;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
async function setFlow(flow: Flow) {
|
||||
await dayLogsStore.logDay({ logDate: editingDate, flow });
|
||||
await safeLogDay({ logDate: editingDate, flow });
|
||||
}
|
||||
|
||||
async function setMood(mood: Mood) {
|
||||
const next = selectedMood === mood ? null : mood;
|
||||
await dayLogsStore.logDay({ logDate: editingDate, mood: next });
|
||||
await safeLogDay({ logDate: editingDate, mood: next });
|
||||
}
|
||||
|
||||
async function toggleSymptom(id: string) {
|
||||
const has = selectedSymptoms.includes(id);
|
||||
const next = has ? selectedSymptoms.filter((s) => s !== id) : [...selectedSymptoms, id];
|
||||
await dayLogsStore.logDay({ logDate: editingDate, symptoms: next });
|
||||
await safeLogDay({ logDate: editingDate, symptoms: next });
|
||||
}
|
||||
|
||||
async function saveTemperature() {
|
||||
const num = parseFloat(temperature);
|
||||
await dayLogsStore.logDay({
|
||||
await safeLogDay({
|
||||
logDate: editingDate,
|
||||
temperature: Number.isFinite(num) ? num : null,
|
||||
});
|
||||
}
|
||||
|
||||
async function saveNotes() {
|
||||
await dayLogsStore.logDay({ logDate: editingDate, notes: notesText.trim() || null });
|
||||
await safeLogDay({ logDate: editingDate, notes: notesText.trim() || null });
|
||||
}
|
||||
|
||||
async function deleteEditingLog() {
|
||||
|
|
@ -138,7 +156,7 @@
|
|||
|
||||
async function startPeriodToday() {
|
||||
await cyclesStore.createCycle({ startDate: todayIso });
|
||||
await dayLogsStore.logDay({ logDate: todayIso, flow: 'medium' });
|
||||
await safeLogDay({ logDate: todayIso, flow: 'medium' });
|
||||
backToToday();
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue