From d3b3abacff3fa32240fa7050d95962ba4143edfb Mon Sep 17 00:00:00 2001
From: Jannis Braun <151788261+TheZwiss@users.noreply.github.com>
Date: Sun, 3 May 2026 04:06:43 +0200
Subject: [PATCH] harden(desktop): RecoveryStateStore listener safety + frozen
 state

- Snapshot listener set before notifying so subscribers can subscribe/
  unsubscribe during notification without breaking the pass
- Per-callback try/catch so one throwing subscriber does not silence others
- Object.freeze on each state object so the live reference returned by
  get() cannot be accidentally mutated externally (compile-time
  Readonly<> is hint only)
- 3 new tests pinning these invariants
---
 packages/desktop/src/recovery.test.ts | 32 +++++++++++++++++++++++++++
 packages/desktop/src/recovery.ts      | 15 ++++++++++---
 2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/packages/desktop/src/recovery.test.ts b/packages/desktop/src/recovery.test.ts
index 5a94896c..97b540e0 100644
--- a/packages/desktop/src/recovery.test.ts
+++ b/packages/desktop/src/recovery.test.ts
@@ -71,4 +71,36 @@ describe('RecoveryStateStore', () => {
     store.markRecoveryExited();
     expect(store.isInRecoveryMode()).toBe(false);
   });
+
+  it('a throwing listener does not stop other listeners', () => {
+    const store = new RecoveryStateStore();
+    const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
+    const a = vi.fn(() => { throw new Error('boom'); });
+    const b = vi.fn();
+    store.subscribe(a);
+    store.subscribe(b);
+    expect(() => store.update({ updateState: 'checking' })).not.toThrow();
+    expect(a).toHaveBeenCalledTimes(1);
+    expect(b).toHaveBeenCalledTimes(1);
+    errSpy.mockRestore();
+  });
+
+  it('a listener can unsubscribe itself during notification without breaking the pass', () => {
+    const store = new RecoveryStateStore();
+    const b = vi.fn();
+    let unsubA: (() => void) | null = null;
+    unsubA = store.subscribe(() => { unsubA?.(); });
+    store.subscribe(b);
+    expect(() => store.update({ updateState: 'checking' })).not.toThrow();
+    // Second update: the self-unsubscribed listener should be gone, b still fires
+    store.update({ updateState: 'idle' });
+    expect(b).toHaveBeenCalledTimes(2);
+  });
+
+  it('returned state is frozen — accidental external mutation throws in strict mode', () => {
+    const store = new RecoveryStateStore();
+    store.update({ updateState: 'checking' });
+    const s = store.get();
+    expect(Object.isFrozen(s)).toBe(true);
+  });
 });
diff --git a/packages/desktop/src/recovery.ts b/packages/desktop/src/recovery.ts
index 002bc60c..edde5074 100644
--- a/packages/desktop/src/recovery.ts
+++ b/packages/desktop/src/recovery.ts
@@ -30,7 +30,7 @@ const INITIAL_STATE: RecoveryState = {
 };
 
 export class RecoveryStateStore {
-  private state: RecoveryState = { ...INITIAL_STATE };
+  private state: RecoveryState = Object.freeze({ ...INITIAL_STATE }) as RecoveryState;
   private listeners = new Set<(s: RecoveryState) => void>();
   private inRecoveryMode = false;
 
@@ -39,8 +39,17 @@ export class RecoveryStateStore {
   }
 
   update(partial: Partial<RecoveryState>): void {
-    this.state = { ...this.state, ...partial };
-    for (const cb of this.listeners) cb(this.state);
+    this.state = Object.freeze({ ...this.state, ...partial }) as RecoveryState;
+    // Snapshot before iterating: a listener can subscribe/unsubscribe others
+    // (or itself) during notification without affecting the current notify pass.
+    const snapshot = Array.from(this.listeners);
+    for (const cb of snapshot) {
+      try {
+        cb(this.state);
+      } catch (err) {
+        console.error('[recovery] listener threw:', err);
+      }
+    }
   }
 
   subscribe(cb: (s: RecoveryState) => void): () => void {