From c1958403ad844f1fe607c64572757e8f5a772e60 Mon Sep 17 00:00:00 2001 From: Joseph Mearman Date: Wed, 21 Oct 2026 07:15:05 +0100 Subject: [PATCH] perf(core): recheck the shared trust file once per interval, not per read Every directory entry a node merges asks the trust boundary several questions, and each question stat()ed the shared trust file to see whether another process had replaced it. On a node serving a large local mesh that was thousands of stats a second. Reads now rest on the last look for TRUST_FILE_RECHECK_INTERVAL_MS, paced by an injected clock, which bounds how long another process's revocation can go unseen by a read. Writes still re-read first, since each rewrites the whole file and must not erase another process's change. GatewayTrust takes its location and clock as one options object. --- src/core/gateway-trust.ts | 52 +++++++-- src/core/mesh-store.ts | 2 +- src/test/gateway-trust-recheck.unit.test.ts | 112 ++++++++++++++++++++ src/test/gateway-trust.unit.test.ts | 75 ++++++++----- src/test/manual-clock.ts | 17 +++ 5 files changed, 221 insertions(+), 37 deletions(-) create mode 100644 src/test/gateway-trust-recheck.unit.test.ts create mode 100644 src/test/manual-clock.ts diff --git a/src/core/gateway-trust.ts b/src/core/gateway-trust.ts index 2ed0a832..72b69c0a 100644 --- a/src/core/gateway-trust.ts +++ b/src/core/gateway-trust.ts @@ -9,6 +9,8 @@ * * Principal-keyed trust (agent-comms#187): alongside the bare-device allowlist above, an operator may also trust a user-principal device-id (user-identity.ts) directly -- the root a remote peer's own dm:send-style delegation chain can terminate at, verified the same way device-membership-verification.ts already verifies a device's own group:member chain (`verifyCapabilityToken`'s chain-walk and its `rootIssuer` output). This is purely a second, parallel allowlist: `isTrusted`/`add`/`remove`/`list` keep checking only the bare-device set, unchanged, for a peer with no principal at all -- `isTrustedFor` is the additive entrypoint a caller who has already chain-verified a token uses to decide trust from that verified bearer and chain-root pair, accepting either a directly trusted device or a bearer rooted at a trusted principal. Persisted alongside the bare-device set (agent-comms#186's own persistence, extended here per that issue's own "agent-comms#187 covers what gets stored" framing): loadGatewayTrust/saveGatewayTrust now carry both sets. */ +import type { Clock } from "wire-mesh-core/ports/clock"; +import { createSystemClock } from "wire-mesh-core/adapters/system-clock"; import type { IdentitySlot, LoadedGatewayTrust } from "./identity-store.js"; import { gatewayTrustStamp, @@ -16,6 +18,17 @@ import { saveGatewayTrust, } from "./identity-store.js"; +const MS_PER_SECOND = 1000; +const TRUST_FILE_RECHECK_INTERVAL_SECONDS = 1; + +/** + * How long a read decision may rest on the last look at the trust file before the file is looked at again. + * + * Every directory entry a node merges asks the trust boundary several questions, so a hub node answers them thousands of times a second, and each used to cost a `stat`. This bound makes the cost one `stat` per interval. It is the longest another process's revocation can go unseen by a read, so it is chosen against the slowest thing a revocation already waits for: a device merged into the directory stays there until the next presence re-advertisement, many seconds on, so a one second bound adds no delay a user could observe to that path while leaving the check far below the rate at which it was being run. Writes never rely on it: each re-reads the file first, so one process cannot overwrite another's change. + */ +export const TRUST_FILE_RECHECK_INTERVAL_MS = + TRUST_FILE_RECHECK_INTERVAL_SECONDS * MS_PER_SECOND; + /** The read-only slice of GatewayTrust every consumer of the trust boundary actually needs (WireMeshTransport, HubSession, hub-forwarding.ts) -- named so call sites that only ever read trust decisions, never mutate them, don't repeat the same `Pick` inline at every field/parameter that takes one. */ export type GatewayTrustReader = Pick< GatewayTrust, @@ -44,6 +57,12 @@ export interface VerifiedMember extends Voucher { device: string; } +/** What a GatewayTrust is built from: where to persist (none for an in-memory boundary) and the clock its trust-file rechecks are paced by. */ +export interface GatewayTrustOptions { + location?: Readonly> | undefined; + clock?: Clock | undefined; +} + export class GatewayTrust { private readonly trusted = new Set(); private readonly trustedIssuers: Readonly>> = { @@ -59,16 +78,21 @@ export class GatewayTrust { Readonly> | undefined; /** gatewayTrustStamp as of the last load or write this instance made, so refresh() can tell when another process has replaced the file since. */ private stamp: string | undefined; + /** When the file's stamp was last compared, so reads can skip the comparison inside TRUST_FILE_RECHECK_INTERVAL_MS. */ + private checkedAt: number | undefined; + private readonly clock: Clock; /** Constructs the trust boundary, optionally bound to an identity location for persistence (agent-comms#186); any IdentitySlot serves as one, since only its directory matters. See this class's own doc comment for what a location does and doesn't change. Given a location, immediately loads whatever devices and principals are already trusted into the in-memory sets. */ - constructor(location?: Readonly>) { - this.location = location; + constructor(options: Readonly = {}) { + this.location = options.location; + this.clock = options.clock ?? createSystemClock(); this.load(); } /** Replaces the in-memory sets with whatever the file currently holds, recording the file's stamp. A no-op without a location. Verified members are left alone: they are in memory only and are dropped on their own when their principal stops being trusted. */ private load(): void { if (this.location === undefined) return; + this.checkedAt = this.clock.now(); this.stamp = gatewayTrustStamp(this.location); const loaded = loadGatewayTrust(this.location); this.trusted.clear(); @@ -85,22 +109,36 @@ export class GatewayTrust { } } - /** Reloads from disk when another process has replaced the file since this instance last looked. Called first by every method that reads the sets, so a trust decision is never older than the file. */ + /** Reloads from disk when another process has replaced the file since this instance last looked, looking at most once per TRUST_FILE_RECHECK_INTERVAL_MS. Called first by every method that only reads the sets, so a trust decision is never older than the file by more than that bound. */ private refresh(): void { if (this.location === undefined) return; + const now = this.clock.now(); + if ( + this.checkedAt !== undefined && + now - this.checkedAt < TRUST_FILE_RECHECK_INTERVAL_MS + ) { + return; + } + this.refreshNow(); + } + + /** Looks at the file now, whatever the last look was. Every method that changes the sets starts here, because it rewrites the whole file from them: starting from anything older would erase another process's change. */ + private refreshNow(): void { + if (this.location === undefined) return; + this.checkedAt = this.clock.now(); if (gatewayTrustStamp(this.location) !== this.stamp) this.load(); } /** Marks a remote device-id (hex, case-insensitive) as trusted: this side will merge its gossiped directory entries, dispatch its relayed requests, and route outbound hub requests to it. Idempotent. Persists the updated set when this instance was constructed with a location. */ add(deviceHex: string): void { - this.refresh(); + this.refreshNow(); this.trusted.add(deviceHex.toLowerCase()); this.persist(); } /** Withdraws a previously trusted device-id (hex, case-insensitive). A no-op if it was never trusted. Requests already in flight are unaffected; a directory entry merged from the hub because of this trust alone leaves the transport's view on its next read (KnownDevices, gossip-directory.ts). Persists the updated set when this instance was constructed with a location. */ remove(deviceHex: string): void { - this.refresh(); + this.refreshNow(); this.trusted.delete(deviceHex.toLowerCase()); this.persist(); } @@ -185,14 +223,14 @@ export class GatewayTrust { } private addIssuer(kind: GroupKind, deviceHex: string): void { - this.refresh(); + this.refreshNow(); this.trustedIssuers[kind].add(deviceHex.toLowerCase()); this.persist(); } /** Withdraws trust from one issuer and, in the same act, from every device reachable only through it; the transport's directory drops those devices on its next read (KnownDevices, gossip-directory.ts). */ private removeIssuer(kind: GroupKind, deviceHex: string): void { - this.refresh(); + this.refreshNow(); const issuer = deviceHex.toLowerCase(); this.trustedIssuers[kind].delete(issuer); for (const [key, member] of this.verifiedMembers) { diff --git a/src/core/mesh-store.ts b/src/core/mesh-store.ts index 8ff2aa82..9c23f68e 100644 --- a/src/core/mesh-store.ts +++ b/src/core/mesh-store.ts @@ -311,7 +311,7 @@ export class MeshStore implements CommsStore { this.coordinatorPort = coordinatorPort; this.hubUrl = hubUrl; this.roomJoinApprovalTimeoutMs = roomJoinApprovalTimeoutMs; - this.gatewayTrust = new GatewayTrust(slot); + this.gatewayTrust = new GatewayTrust({ location: slot }); this.naming = new Naming({ petnames: new Petnames(slot), issuerNames: async () => this.membership.issuerNames(), diff --git a/src/test/gateway-trust-recheck.unit.test.ts b/src/test/gateway-trust-recheck.unit.test.ts new file mode 100644 index 00000000..9d0a8890 --- /dev/null +++ b/src/test/gateway-trust-recheck.unit.test.ts @@ -0,0 +1,112 @@ +/** + * How often GatewayTrust looks at the shared trust file. A node answers trust questions for every directory entry it merges, so the look has to be rare by construction; these tests count the looks (never time them) through an injected clock and pin both halves of the bound: reads inside the interval cost none, and a change made by another process is seen once the interval has passed. + */ +import * as fs from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { + GatewayTrust, + TRUST_FILE_RECHECK_INTERVAL_MS, +} from "../core/gateway-trust.js"; +import type * as identityStore from "../core/identity-store.js"; +import type { IdentitySlot } from "../core/identity-store.js"; +import { manualClock } from "./manual-clock.js"; + +const stamps = vi.hoisted(() => ({ calls: 0 })); + +vi.mock("../core/identity-store.js", async (importOriginal) => { + const original = await importOriginal(); + + return { + ...original, + gatewayTrustStamp: ( + ...args: Parameters + ) => { + stamps.calls += 1; + + return original.gatewayTrustStamp(...args); + }, + }; +}); + +/** Questions asked of one trust boundary in the interval, standing in for the directory entries a busy hub merges. */ +const QUESTIONS = 500; + +function tempSlot(): IdentitySlot { + const dir = fs.mkdtempSync(path.join(tmpdir(), "agent-comms-trust-recheck-")); + + return { harness: "pi", cwd: "/tmp/project", dir }; +} + +describe("GatewayTrust trust-file rechecks", () => { + beforeEach(() => { + stamps.calls = 0; + }); + + it("looks at the file once for any number of reads inside one interval", () => { + const slot = tempSlot(); + const clock = manualClock(); + const trust = new GatewayTrust({ location: slot, clock: clock }); + const afterConstruction = stamps.calls; + + for (let question = 0; question < QUESTIONS; question += 1) { + trust.isTrusted("aabbcc"); + trust.isTrustedPrincipal("aabbcc"); + trust.hasAny(); + } + + expect(stamps.calls - afterConstruction).toBe(0); + }); + + it("looks again once the interval has passed, and only once per interval", () => { + const slot = tempSlot(); + const clock = manualClock(); + const trust = new GatewayTrust({ location: slot, clock: clock }); + const afterConstruction = stamps.calls; + + clock.advance(TRUST_FILE_RECHECK_INTERVAL_MS); + for (let question = 0; question < QUESTIONS; question += 1) { + trust.isTrusted("aabbcc"); + } + + expect(stamps.calls - afterConstruction).toBe(1); + }); + + it("sees another process's revocation within the interval's bound, and not before", () => { + const slot = tempSlot(); + const clock = manualClock(); + const reader = new GatewayTrust({ location: slot, clock: clock }); + const other = new GatewayTrust({ location: slot, clock: manualClock() }); + other.add("aabbcc"); + clock.advance(TRUST_FILE_RECHECK_INTERVAL_MS); + expect(reader.isTrusted("aabbcc")).toBe(true); + + other.remove("aabbcc"); + + expect( + reader.isTrusted("aabbcc"), + "a read inside the interval may still rest on the last look", + ).toBe(true); + clock.advance(TRUST_FILE_RECHECK_INTERVAL_MS); + expect( + reader.isTrusted("aabbcc"), + "once the interval has passed the revocation is seen", + ).toBe(false); + }); + + it("never lets a write rest on a stale look, so one process cannot erase another's change", () => { + const slot = tempSlot(); + const clock = manualClock(); + const first = new GatewayTrust({ location: slot, clock: clock }); + const second = new GatewayTrust({ location: slot, clock: manualClock() }); + second.add("ddeeff"); + + first.add("aabbcc"); + + expect( + new GatewayTrust({ location: slot }).list().sort(), + "the write re-read the file first, keeping the other process's entry", + ).toEqual(["aabbcc", "ddeeff"]); + }); +}); diff --git a/src/test/gateway-trust.unit.test.ts b/src/test/gateway-trust.unit.test.ts index e15b97a6..5534ec7c 100644 --- a/src/test/gateway-trust.unit.test.ts +++ b/src/test/gateway-trust.unit.test.ts @@ -5,8 +5,12 @@ import * as fs from "node:fs"; import { tmpdir } from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; -import { GatewayTrust } from "../core/gateway-trust.js"; +import { + GatewayTrust, + TRUST_FILE_RECHECK_INTERVAL_MS, +} from "../core/gateway-trust.js"; import type { IdentitySlot } from "../core/identity-store.js"; +import { manualClock } from "./manual-clock.js"; function tempSlot(harness: string): IdentitySlot { const dir = fs.mkdtempSync( @@ -87,16 +91,16 @@ describe("GatewayTrust persistence (agent-comms#186)", () => { it("constructed with a slot that has never saved a trusted set, starts empty", () => { const slot = tempSlot("pi"); - const trust = new GatewayTrust(slot); + const trust = new GatewayTrust({ location: slot }); expect(trust.hasAny()).toBe(false); expect(trust.list()).toEqual([]); }); it("loads a previously persisted trusted set on construction", () => { const slot = tempSlot("pi"); - new GatewayTrust(slot).add("aabbcc"); + new GatewayTrust({ location: slot }).add("aabbcc"); - const restarted = new GatewayTrust(slot); + const restarted = new GatewayTrust({ location: slot }); expect(restarted.isTrusted("aabbcc")).toBe(true); expect(restarted.list()).toEqual(["aabbcc"]); @@ -104,31 +108,31 @@ describe("GatewayTrust persistence (agent-comms#186)", () => { it("persists an add immediately, visible to a fresh instance for the same slot without either instance restarting", () => { const slot = tempSlot("pi"); - const trust = new GatewayTrust(slot); + const trust = new GatewayTrust({ location: slot }); trust.add("aabbcc"); - const other = new GatewayTrust(slot); + const other = new GatewayTrust({ location: slot }); expect(other.isTrusted("aabbcc")).toBe(true); }); it("persists a remove, so a restarted instance no longer trusts the removed device", () => { const slot = tempSlot("pi"); - const first = new GatewayTrust(slot); + const first = new GatewayTrust({ location: slot }); first.add("aabbcc"); first.add("ddeeff"); first.remove("aabbcc"); - const restarted = new GatewayTrust(slot); + const restarted = new GatewayTrust({ location: slot }); expect(restarted.list()).toEqual(["ddeeff"]); }); it("normalises hex case in the persisted set the same way the in-memory set is normalised", () => { const slot = tempSlot("pi"); - new GatewayTrust(slot).add("AaBbCc"); + new GatewayTrust({ location: slot }).add("AaBbCc"); - const restarted = new GatewayTrust(slot); + const restarted = new GatewayTrust({ location: slot }); expect(restarted.list()).toEqual(["aabbcc"]); }); @@ -136,25 +140,29 @@ describe("GatewayTrust persistence (agent-comms#186)", () => { it("two identity directories keep separate trust", () => { const slotA = tempSlot("pi"); const slotB = tempSlot("pi"); - new GatewayTrust(slotA).add("aabbcc"); + new GatewayTrust({ location: slotA }).add("aabbcc"); - expect(new GatewayTrust(slotB).list()).toEqual([]); + expect(new GatewayTrust({ location: slotB }).list()).toEqual([]); }); it("stores under different harnesses share one trusted set when they share an identity directory", () => { const pi = tempSlot("pi"); const claudeCode: IdentitySlot = { ...pi, harness: "claude-code" }; - new GatewayTrust(pi).add("aabbcc"); + new GatewayTrust({ location: pi }).add("aabbcc"); - expect(new GatewayTrust(claudeCode).isTrusted("aabbcc")).toBe(true); + expect(new GatewayTrust({ location: claudeCode }).isTrusted("aabbcc")).toBe( + true, + ); }); it("a running instance sees a device another instance trusted after it was constructed", () => { const slot = tempSlot("pi"); - const running = new GatewayTrust(slot); + const clock = manualClock(); + const running = new GatewayTrust({ location: slot, clock: clock }); expect(running.hasAny()).toBe(false); - new GatewayTrust(slot).add("aabbcc"); + new GatewayTrust({ location: slot }).add("aabbcc"); + clock.advance(TRUST_FILE_RECHECK_INTERVAL_MS); expect(running.hasAny()).toBe(true); expect(running.isTrusted("aabbcc")).toBe(true); @@ -163,10 +171,12 @@ describe("GatewayTrust persistence (agent-comms#186)", () => { it("a running instance stops trusting a device another instance removed", () => { const slot = tempSlot("pi"); - const running = new GatewayTrust(slot); + const clock = manualClock(); + const running = new GatewayTrust({ location: slot, clock: clock }); running.add("aabbcc"); - new GatewayTrust(slot).remove("aabbcc"); + new GatewayTrust({ location: slot }).remove("aabbcc"); + clock.advance(TRUST_FILE_RECHECK_INTERVAL_MS); expect(running.isTrusted("aabbcc")).toBe(false); expect(running.list()).toEqual([]); @@ -174,9 +184,11 @@ describe("GatewayTrust persistence (agent-comms#186)", () => { it("a running instance sees a principal another instance trusted", () => { const slot = tempSlot("pi"); - const running = new GatewayTrust(slot); + const clock = manualClock(); + const running = new GatewayTrust({ location: slot, clock: clock }); - new GatewayTrust(slot).addPrincipal("112233"); + new GatewayTrust({ location: slot }).addPrincipal("112233"); + clock.advance(TRUST_FILE_RECHECK_INTERVAL_MS); expect(running.isTrustedPrincipal("112233")).toBe(true); expect(running.listPrincipals()).toEqual(["112233"]); @@ -184,24 +196,29 @@ describe("GatewayTrust persistence (agent-comms#186)", () => { it("a change applies on top of another instance's earlier change instead of overwriting it", () => { const slot = tempSlot("pi"); - const first = new GatewayTrust(slot); - const second = new GatewayTrust(slot); + const clock = manualClock(); + const first = new GatewayTrust({ location: slot, clock: clock }); + const second = new GatewayTrust({ location: slot }); first.add("aabbcc"); second.add("ddeeff"); + clock.advance(TRUST_FILE_RECHECK_INTERVAL_MS); expect(first.list()).toEqual(["aabbcc", "ddeeff"]); - expect(new GatewayTrust(slot).list()).toEqual(["aabbcc", "ddeeff"]); + expect(new GatewayTrust({ location: slot }).list()).toEqual([ + "aabbcc", + "ddeeff", + ]); }); // Principal persistence (agent-comms#187) -- extends #186's own device-only persistence to the second, parallel principal allowlist, per #186's own issue text naming #187 as the one that decides what gets stored. it("persists a trusted principal, surviving a restart alongside the bare-device set", () => { const slot = tempSlot("pi"); - const first = new GatewayTrust(slot); + const first = new GatewayTrust({ location: slot }); first.add("aabbcc"); first.addPrincipal("112233"); - const restarted = new GatewayTrust(slot); + const restarted = new GatewayTrust({ location: slot }); expect(restarted.list()).toEqual(["aabbcc"]); expect(restarted.listPrincipals()).toEqual(["112233"]); @@ -209,12 +226,12 @@ describe("GatewayTrust persistence (agent-comms#186)", () => { it("persists a principal removal independently of the bare-device set", () => { const slot = tempSlot("pi"); - const first = new GatewayTrust(slot); + const first = new GatewayTrust({ location: slot }); first.addPrincipal("112233"); first.addPrincipal("445566"); first.removePrincipal("112233"); - const restarted = new GatewayTrust(slot); + const restarted = new GatewayTrust({ location: slot }); expect(restarted.listPrincipals()).toEqual(["445566"]); }); @@ -518,9 +535,9 @@ describe("GatewayTrust: machine-keyed trust (agent-comms#343)", () => { it("persists trusted machines for a fresh instance, and counts one as a reason to advertise", () => { const slot = tempSlot("pi"); - new GatewayTrust(slot).addMachine(MACHINE.toUpperCase()); + new GatewayTrust({ location: slot }).addMachine(MACHINE.toUpperCase()); - const restarted = new GatewayTrust(slot); + const restarted = new GatewayTrust({ location: slot }); expect(restarted.listMachines()).toEqual([MACHINE]); expect(restarted.listPrincipals()).toEqual([]); diff --git a/src/test/manual-clock.ts b/src/test/manual-clock.ts new file mode 100644 index 00000000..19888a2d --- /dev/null +++ b/src/test/manual-clock.ts @@ -0,0 +1,17 @@ +import type { Clock } from "wire-mesh-core/ports/clock"; + +/** A clock that only moves when a test advances it, so a bound stated in milliseconds is exercised without waiting. */ +export interface ManualClock extends Clock { + advance: (ms: number) => void; +} + +export function manualClock(startMs = 0): ManualClock { + let now = startMs; + + return { + now: () => now, + advance: (ms) => { + now += ms; + }, + }; +}