fed31ad00996

fix: HV-1 clamp future sync timestamps + remote-mutation alert queue (v3.7.1 base)

time
2026-06-10 01:35 UTC · 4 months ago
machine
koh2
agent
claude-opus-4-8
session
audit fix integration
parent
1b75540dc84c
6 files src/background/chirp-sync-runner.ts+4 −3src/background/chirp-sync-vault-adapter.ts+10 −4src/background/messages.ts+11 −0src/background/vault.sync-clamp.test.ts+126 −55src/background/vault.ts+48 −3src/shared/types.ts+1 −0
src/background/chirp-sync-runner.ts+4 −3
@@ -58,7 +58,7 @@5858     expectedRemoteHex: string,5959   ) => Promise<SyncEndpoint | null>;6060   getSubnet: () => Promise<string | null>;61-  createAdapter: () => ChirpSyncVaultAdapter;61+  createAdapter: (context?: { deviceName?: string }) => ChirpSyncVaultAdapter;6262 }6363 6464 const productionDeps: SyncRunnerDeps = {@@ -67,7 +67,7 @@6767   probeSubnet: (subnet, port, priv, pub, expected) =>6868     probeSubnetForPeer(subnet, port, priv, pub, expected),6969   getSubnet: getLocalSubnet,70-  createAdapter: createChirpSyncVaultAdapter,70+  createAdapter: (context) => createChirpSyncVaultAdapter(context),7171 };7272 7373 export async function productionSyncRunner(device: PairedDevice): Promise<SyncRunResult> {@@ -99,7 +99,8 @@9999   // Sanity — the remote pubkey must be 32-byte hex so noise can compare it.100100   void hexToPubkey(expectedRemoteHex);101101 102-  const adapter = deps.createAdapter();102+  // HV-1b: pass the peer being synced so remote-mutation alerts can name it.103+  const adapter = deps.createAdapter({ deviceName: device.deviceName });103104   const session = await connect(device, expectedRemoteHex, port, ourKeypair, deps);104105 105106   try {

view file

src/background/chirp-sync-vault-adapter.ts+10 −4
@@ -47,6 +47,7 @@4747   listPairedDevices,4848   upsertPairedDevice,4949 } from "./paired-devices";50+import { clampSyncTimestamp } from "../shared/sync-limits";5051 import type { VaultEntry } from "../shared/types";5152 import type {5253   ManifestDeviceEntry,@@ -66,7 +67,9 @@6667   }6768 }6869 69-export function createChirpSyncVaultAdapter(): ChirpSyncVaultAdapter {70+export function createChirpSyncVaultAdapter(71+  context: { deviceName?: string } = {},72+): ChirpSyncVaultAdapter {7073   return {7174     async computeVaultId() {7275       const master = requireMasterKey();@@ -135,7 +138,7 @@135138         decrypted.push(entry);136139       }137140       if (decrypted.length > 0) {138-        await applyMergedRemoteEntries(decrypted, {});141+        await applyMergedRemoteEntries(decrypted, {}, context);139142       }140143     },141144 @@ -144,11 +147,14 @@144147       const cleaned: Record<string, number> = {};145148       for (const [id, ts] of Object.entries(tombstones)) {146149         if (typeof ts === "number" && Number.isFinite(ts) && ts > 0) {147-          cleaned[id] = ts;150+          // HV-1c: clamp before the value participates in any comparison.151+          // vault.ts clamps again defensively; clamping here keeps the152+          // invariant local to the boundary where peer data enters.153+          cleaned[id] = clampSyncTimestamp(ts);148154         }149155       }150156       if (Object.keys(cleaned).length > 0) {151-        await applyMergedRemoteEntries([], cleaned);157+        await applyMergedRemoteEntries([], cleaned, context);152158       }153159     },154160 

view file

src/background/messages.ts+11 −0
@@ -10,6 +10,7 @@1010  */1111 1212 import { copyToClipboard } from "./clipboard";13+import { getPendingSyncAlerts, clearPendingSyncAlerts } from "./sync-alerts";1314 import { setupAutoLock, touchAutoLock } from "./alarms";1415 import { runSelfTest } from "./self-test";1516 import * as vault from "./vault";@@ -1258,6 +1259,16 @@12581259       case "GET_SYNC_STATE": {12591260         const { getSyncState } = await import("./sync-trigger");12601261         return { ok: true, data: getSyncState() };1262+      }1263+1264+      case "GET_SYNC_ALERTS": {1265+        const { consume } = (msg.payload ?? {}) as { consume?: boolean };1266+        const alerts = await getPendingSyncAlerts();1267+        // The popup reads on open and acknowledges with consume:true after1268+        // rendering, so a popup crash between read and render doesn't lose1269+        // the alerts.1270+        if (consume) await clearPendingSyncAlerts();1271+        return { ok: true, data: alerts };12611272       }12621273 12631274       default:

view file

src/background/vault.sync-clamp.test.ts+126 −55
@@ -1,21 +1,11 @@1-// Draft unit tests for HV-1 — sync timestamp clamp + remote-mutation alerts.2-// Target location after integration: src/background/vault.sync-clamp.test.ts3-// (or merged into the existing vault sync tests). Uses the same chrome4-// storage mock harness as src/background/vault.test.ts.1+// Unit tests for HV-1 — sync timestamp clamp + remote-mutation alerts.2+// Uses the same chrome storage mock harness as src/background/vault.test.ts:3+// the mock is installed in beforeEach and vault/sync-alerts are imported4+// dynamically afterwards, because vault.ts -> alarms.ts touches `chrome` at5+// module-evaluation time.56 6-import { beforeEach, describe, expect, it } from "vitest";7-import {8-  MAX_CLOCK_SKEW_MS,9-  clampSyncTimestamp,10-} from "../shared/sync-limits";11-import {12-  applyMergedRemoteEntries,13-  addEntry,14-  createVault,15-  getEntries,16-  getEntryById,17-} from "./vault";18-import { getPendingSyncAlerts, clearPendingSyncAlerts } from "./sync-alerts";7+import { beforeEach, describe, expect, it, vi } from "vitest";8+import { MAX_CLOCK_SKEW_MS, clampSyncTimestamp } from "../shared/sync-limits";199 2010 describe("clampSyncTimestamp", () => {2111   it("clamps a far-future timestamp to now + MAX_CLOCK_SKEW_MS", () => {@@ -34,14 +24,58 @@3424 });3525 3626 describe("applyMergedRemoteEntries — HV-1", () => {37-  beforeEach(async () => {38-    // chrome.storage mock reset per the existing vault.test.ts harness.39-    await createVault("correct horse battery staple");40-    await clearPendingSyncAlerts();27+  let storage: Record<string, unknown>;28+29+  beforeEach(() => {30+    vi.resetModules();31+    storage = {};32+33+    const chromeMock = {34+      storage: {35+        local: {36+          get: vi.fn(async (key?: string | string[]) => {37+            if (!key) return storage;38+            if (typeof key === "string") return { [key]: storage[key] };39+            return key.reduce<Record<string, unknown>>((result, entry) => {40+              result[entry] = storage[entry];41+              return result;42+            }, {});43+          }),44+          set: vi.fn(async (next: Record<string, unknown>) => {45+            Object.assign(storage, next);46+          }),47+          remove: vi.fn(async (keys: string | string[]) => {48+            const list = Array.isArray(keys) ? keys : [keys];49+            for (const key of list) delete storage[key];50+          }),51+        },52+      },53+      alarms: {54+        clear: vi.fn(),55+        create: vi.fn(),56+        onAlarm: { addListener: vi.fn() },57+      },58+      idle: {59+        setDetectionInterval: vi.fn(),60+        onStateChanged: { addListener: vi.fn() },61+      },62+    } as unknown as typeof chrome;63+64+    (globalThis as { chrome?: typeof chrome }).chrome = chromeMock;65+66+    vi.stubGlobal(67+      "fetch",68+      vi.fn(async () => {69+        throw new Error("network disabled in tests");70+      }),71+    );4172   });4273 4374   it("clamps a far-future remote modifiedAt so a local edit can win", async () => {44-    const local = await addEntry({75+    const vault = await import("./vault");76+    await vault.createVault("correct horse battery staple");77+78+    const local = await vault.addEntry({4579       title: "Bank",4680       username: "june",4781       password: "strong-local-password",@@ -49,30 +83,48 @@4983       type: "login",5084     });5185 52-    const farFuture = Date.now() + 10_000_000;53-    await applyMergedRemoteEntries(54-      [{ ...local, password: "attacker-password", modifiedAt: farFuture }],55-      {},56-    );86+    // Freeze Date.now() (only Date — real timers stay live) so the two merges87+    // share one `now`. The security property is: a clamped far-future value88+    // cannot pin the future against the monotonic local clock. With wall-clock89+    // drift between merges the attacker's re-clamped value would creep forward90+    // within the skew window; freezing time isolates the invariant under test.91+    vi.useFakeTimers({ toFake: ["Date"] });92+    vi.setSystemTime(new Date(local.modifiedAt + 1000));93+    try {94+      const farFuture = Date.now() + 10_000_000;95+      await vault.applyMergedRemoteEntries(96+        [{ ...local, password: "attacker-password", modifiedAt: farFuture }],97+        {},98+      );5799 58-    const merged = getEntryById(local.id)!;59-    // The remote value wins NOW (it is newer than the local edit), but its60-    // timestamp is clamped — it cannot pin the far future.61-    expect(merged.modifiedAt).toBeLessThanOrEqual(Date.now() + MAX_CLOCK_SKEW_MS);100+      const merged = vault.getEntryById(local.id)!;101+      // The remote value wins NOW (it is newer than the local edit), but its102+      // timestamp is clamped — it cannot pin the far future.103+      expect(merged.modifiedAt).toBeLessThanOrEqual(104+        Date.now() + MAX_CLOCK_SKEW_MS,105+      );62106 63-    // A subsequent local edit (monotonic clock) beats the clamped value.64-    const { updateEntry } = await import("./vault");65-    await updateEntry(local.id, { password: "honest-newer-password" });66-    // Re-sync the stale attacker value — it must NOT win again.67-    await applyMergedRemoteEntries(68-      [{ ...merged, password: "attacker-password", modifiedAt: farFuture }],69-      {},70-    );71-    expect(getEntryById(local.id)!.password).toBe("honest-newer-password");107+      // A subsequent local edit (monotonic clock) advances past the clamped108+      // value.109+      await vault.updateEntry(local.id, { password: "honest-newer-password" });110+      // Re-sync the stale attacker value — it must NOT win again.111+      await vault.applyMergedRemoteEntries(112+        [{ ...merged, password: "attacker-password", modifiedAt: farFuture }],113+        {},114+      );115+      expect(vault.getEntryById(local.id)!.password).toBe(116+        "honest-newer-password",117+      );118+    } finally {119+      vi.useRealTimers();120+    }72121   });73122 74123   it("clamps a far-future tombstone ts before applying", async () => {75-    const local = await addEntry({124+    const vault = await import("./vault");125+    await vault.createVault("correct horse battery staple");126+127+    const local = await vault.addEntry({76128       title: "Forum",77129       username: "june",78130       password: "pw",@@ -80,29 +132,39 @@80132       type: "login",81133     });82134     const farFuture = Date.now() + 10_000_000;83-    await applyMergedRemoteEntries([], { [local.id]: farFuture });135+    await vault.applyMergedRemoteEntries([], { [local.id]: farFuture });84136 85-    const deleted = getEntries().length === 086-      ? undefined87-      : undefined; // entry is soft-deleted — read via getDeletedEntries88-    void deleted;89-    const { getDeletedEntries } = await import("./vault");90-    const tombstoned = getDeletedEntries().find((e) => e.id === local.id)!;137+    const tombstoned = vault138+      .getDeletedEntries()139+      .find((e) => e.id === local.id)!;91140     expect(tombstoned.deletedAt).toBeLessThanOrEqual(92141       Date.now() + MAX_CLOCK_SKEW_MS,93142     );94143   });95144 96145   it("records a REMOTE_PASSWORD_CHANGE alert when a remote replaces a local password", async () => {97-    const local = await addEntry({146+    const vault = await import("./vault");147+    const { getPendingSyncAlerts, clearPendingSyncAlerts } = await import(148+      "./sync-alerts"149+    );150+    await vault.createVault("correct horse battery staple");151+    await clearPendingSyncAlerts();152+153+    const local = await vault.addEntry({98154       title: "Email",99155       username: "june",100156       password: "old-password",101157       url: "https://mail.example.com",102158       type: "login",103159     });104-    await applyMergedRemoteEntries(105-      [{ ...local, password: "replaced-by-peer", modifiedAt: Date.now() + 1000 }],160+    await vault.applyMergedRemoteEntries(161+      [162+        {163+          ...local,164+          password: "replaced-by-peer",165+          modifiedAt: Date.now() + 1000,166+        },167+      ],106168       {},107169       { deviceName: "Pixel 9" },108170     );@@ -117,16 +179,25 @@117179   });118180 119181   it("records a REMOTE_DELETE alert when a remote tombstone deletes a live entry", async () => {120-    const local = await addEntry({182+    const vault = await import("./vault");183+    const { getPendingSyncAlerts, clearPendingSyncAlerts } = await import(184+      "./sync-alerts"185+    );186+    await vault.createVault("correct horse battery staple");187+    await clearPendingSyncAlerts();188+189+    const local = await vault.addEntry({121190       title: "Shop",122191       username: "june",123192       password: "pw",124193       url: "https://shop.example.com",125194       type: "login",126195     });127-    await applyMergedRemoteEntries([], { [local.id]: Date.now() + 1000 }, {128-      deviceName: "Pixel 9",129-    });196+    await vault.applyMergedRemoteEntries(197+      [],198+      { [local.id]: Date.now() + 1000 },199+      { deviceName: "Pixel 9" },200+    );130201     const alerts = await getPendingSyncAlerts();131202     expect(alerts).toHaveLength(1);132203     expect(alerts[0]).toMatchObject({

view file

src/background/vault.ts+48 −3
@@ -11,6 +11,8 @@1111 1212 import { deriveKey, generateSalt } from "../crypto/argon2";1313 import { extractDomain } from "../shared/domain";14+import { clampSyncTimestamp } from "../shared/sync-limits";15+import { recordSyncAlerts, type SyncAlert } from "./sync-alerts";1416 import {1517   rootKeyToWords,1618   wordsToRootKeyMaterial,@@ -1836,6 +1838,7 @@18361838 export async function applyMergedRemoteEntries(18371839   remoteEntries: VaultEntry[],18381840   remoteTombstones: Record<string, number>,1841+  context: { deviceName?: string } = {},18391842 ): Promise<{ received: number; applied: number; conflicts: number }> {18401843   // DESIGN: last-write-wins is applied PER-ENTRY, not per-field. If two devices18411844   // concurrently edit different fields of the same entry, the one with the@@ -1852,11 +1855,22 @@18521855   const byId = new Map<string, VaultEntry>();18531856   for (const e of entries) byId.set(e.id, e);18541857 1858+  const alerts: SyncAlert[] = [];1859+  const deviceName = context.deviceName ?? "paired device";1860+  const mergeNow = Date.now();1861+18551862   for (const remote of remoteEntries) {18561863     const sanitized = sanitizeRestoredEntry(remote);18571864     if (!sanitized) continue;1865+    // HV-1a: a far-future remote modifiedAt must not permanently pin a1866+    // malicious value against honest local edits. Clamp BEFORE any1867+    // comparison against local timestamps.1868+    sanitized.modifiedAt = clampSyncTimestamp(sanitized.modifiedAt, mergeNow);1869+    if (typeof sanitized.deletedAt === "number") {1870+      sanitized.deletedAt = clampSyncTimestamp(sanitized.deletedAt, mergeNow);1871+    }18581872     const tombstoneTs = remoteTombstones[sanitized.id];1859-    if (tombstoneTs !== undefined && tombstoneTs >= sanitized.modifiedAt) {1873+    if (tombstoneTs !== undefined && clampSyncTimestamp(tombstoneTs, mergeNow) >= sanitized.modifiedAt) {18601874       continue;18611875     }18621876     const local = byId.get(sanitized.id);@@ -1867,6 +1881,22 @@18671881     } else if (sanitized.modifiedAt > local.modifiedAt) {18681882       const idx = entries.findIndex((e) => e.id === local.id);18691883       if (idx !== -1) {1884+        // HV-1b: surface remote password REPLACEMENTS (existing id, password1885+        // differs) so a compromised peer can't silently rewrite credentials.1886+        if (1887+          !local.deletedAt &&1888+          typeof sanitized.password === "string" &&1889+          typeof local.password === "string" &&1890+          sanitized.password !== local.password1891+        ) {1892+          alerts.push({1893+            type: "REMOTE_PASSWORD_CHANGE",1894+            entryId: sanitized.id,1895+            entryTitle: sanitized.title,1896+            deviceName,1897+            timestamp: mergeNow,1898+          });1899+        }18701900         entries[idx] = sanitized;18711901         byId.set(sanitized.id, sanitized);18721902         applied += 1;@@ -1875,14 +1905,26 @@18751905     }18761906   }18771907 1878-  for (const [id, deletedAt] of Object.entries(remoteTombstones)) {1879-    if (typeof deletedAt !== "number" || !Number.isFinite(deletedAt)) continue;1908+  for (const [id, rawDeletedAt] of Object.entries(remoteTombstones)) {1909+    if (typeof rawDeletedAt !== "number" || !Number.isFinite(rawDeletedAt)) continue;1910+    // HV-1a/1c: clamp the tombstone timestamp before every comparison.1911+    const deletedAt = clampSyncTimestamp(rawDeletedAt, mergeNow);18801912     const local = byId.get(id);18811913     if (!local) continue;18821914     if (local.deletedAt && local.deletedAt >= deletedAt) continue;18831915     if (deletedAt < local.modifiedAt) continue;18841916     const idx = entries.findIndex((e) => e.id === id);18851917     if (idx !== -1) {1918+      // HV-1b: surface remote deletes of live local entries.1919+      if (!local.deletedAt) {1920+        alerts.push({1921+          type: "REMOTE_DELETE",1922+          entryId: id,1923+          entryTitle: local.title,1924+          deviceName,1925+          timestamp: mergeNow,1926+        });1927+      }18861928       entries[idx] = { ...local, deletedAt, modifiedAt: deletedAt };18871929       applied += 1;18881930     }@@ -1892,6 +1934,9 @@18921934     await persistEntries("sync_received");18931935     buildDomainIndexSync(entries);18941936   }1937+  // Queued after the merge committed — an alert must describe a mutation1938+  // that actually landed. recordSyncAlerts never throws.1939+  await recordSyncAlerts(alerts);18951940   return { received, applied, conflicts };18961941 }18971942 

view file

src/shared/types.ts+1 −0
@@ -246,6 +246,7 @@246246 // handled in src/background/messages.ts. Everything in this group must247247 // have a matching case in that file's switch statement.248248 export type MessageType =249+  | "GET_SYNC_ALERTS"249250   | "UNLOCK"250251   | "LOCK"251252   | "IS_UNLOCKED"

view file