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
↓ peach-browser-fed31ad0.tar.gzatlas at this save
files changed6 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 −0src/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 {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 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: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({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 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"