From 2984ad66c952945fa222711e554a08640466928e Mon Sep 17 00:00:00 2001 From: Mitch5000 Date: Mon, 27 Jul 2026 16:12:05 +0100 Subject: [PATCH] Document (and test) FIX --- docs/ARCHITECTURE.md | 61 +++++++++++++------ src/repositories/inMemoryRepository.ts | 15 ++++- src/repositories/settlementRepository.test.ts | 41 +++++++++++++ src/repositories/settlementRepository.ts | 32 ++++++++-- 4 files changed, 127 insertions(+), 22 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 815867f..971bffe 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1,23 +1,50 @@ -# AnchorNet Backend Architecture & Security Guarantees +AnchorNet Backend Architecture & Security Guarantees +Overview +The anchornet-backend service provides REST APIs for Stellar liquidity coordination, settlement, and routing. -## Overview +Security Architecture & Audit Log Guarantees +Audit Endpoint (GET /api/v1/audit) +The audit log middleware (src/middleware/auditLog.ts) captures recent mutating requests (POST, PUT, PATCH, DELETE) in an in-memory bounded ring buffer. -The `anchornet-backend` service provides REST APIs for Stellar liquidity coordination, settlement, and routing. +Captured Fields +method: HTTP method +path: Request path (without query parameters) +status: Response status code +requestId: Correlation ID from x-request-id response header +timestamp: ISO timestamp of request completion +Sensitive Data Redaction & Security Guarantees +Strict Redaction via Denylist: Any header, body parameter, or metadata stored in audit log entries is processed through redactSensitiveData(). +Denylisted Fields: Secret-bearing keys such as x-api-key, authorization, cookie, set-cookie, token, access_token, refresh_token, secret, password, bearer, private_key, client_secret are matched case-insensitively and replaced with "[REDACTED]". +Preventing Plaintext Exposure: Under no circumstances should raw credentials or API keys be captured or retained in plaintext in the in-memory audit ring buffer or exposed via GET /api/v1/audit. +In-Memory Repositories & Future Persistence +Settlement, anchor, and liquidity data are held in process-local in-memory +repositories (src/repositories/*), all extending the shared +InMemoryRepository base class. -## Security Architecture & Audit Log Guarantees +Persistence-Swap Risk (read before swapping any repository for a DB) +Several repositories already document that they are "swappable for a +persistent … store later" (e.g. liquidityRepository.ts). This is a forward +design intent, but the current id-allocation contract does not survive that +swap unchanged: -### Audit Endpoint (`GET /api/v1/audit`) +InMemoryRepository.generateId() / peekId() allocate ids under the +assumption that they run synchronously and atomically on Node's single +thread. peekId() exposes the id that generateId() will hand out next +without any locking. +SettlementRepository.peekNextId() returns that previewed id. It is safe to +call peekNextId() and then create() only because both are synchronous +— no other mutation can interleave between them on the event loop. The +returned id is a hint, not a reservation. +⚠️ If any repository is ever backed by an asynchronous store (e.g. a +database), this guarantee breaks. Splitting allocation into a separate +peek + create across an await boundary lets a concurrent caller consume +the previewed id first, introducing a race that does not exist today. -The audit log middleware (`src/middleware/auditLog.ts`) captures recent mutating requests (`POST`, `PUT`, `PATCH`, `DELETE`) in an in-memory bounded ring buffer. +Required guardrails for any async-backed repository: -#### Captured Fields -- `method`: HTTP method -- `path`: Request path (without query parameters) -- `status`: Response status code -- `requestId`: Correlation ID from `x-request-id` response header -- `timestamp`: ISO timestamp of request completion - -#### Sensitive Data Redaction & Security Guarantees -- **Strict Redaction via Denylist**: Any header, body parameter, or metadata stored in audit log entries is processed through `redactSensitiveData()`. -- **Denylisted Fields**: Secret-bearing keys such as `x-api-key`, `authorization`, `cookie`, `set-cookie`, `token`, `access_token`, `refresh_token`, `secret`, `password`, `bearer`, `private_key`, `client_secret` are matched case-insensitively and replaced with `"[REDACTED]"`. -- **Preventing Plaintext Exposure**: Under no circumstances should raw credentials or API keys be captured or retained in plaintext in the in-memory audit ring buffer or exposed via `GET /api/v1/audit`. +Allocate ids atomically inside a single transactional/atomic operation +rather than a separate peek + generate. +Never use peekNextId() to reserve an id across an await. +The synchronous-only contract is locked in by a test in +src/repositories/settlementRepository.test.ts (preview … immediate create). That test must remain green; treat its failure as a signal that a +non-atomic change to id allocation has been introduced. \ No newline at end of file diff --git a/src/repositories/inMemoryRepository.ts b/src/repositories/inMemoryRepository.ts index d1fa6d5..710ce58 100644 --- a/src/repositories/inMemoryRepository.ts +++ b/src/repositories/inMemoryRepository.ts @@ -42,7 +42,20 @@ export abstract class InMemoryRepository { this.items.clear(); } - /** Generates and returns the next numeric id, incrementing the counter. */ + /** + * Generates and returns the next numeric id, incrementing the counter. + * + * NOTE (async-swap risk): `generateId()` and `peekId()` together form an + * id-allocation scheme whose atomicity depends entirely on the synchronous, + * single-threaded nature of this in-memory store. `peekId()` exposes the + * id that `generateId()` will hand out next, but performs no locking. This + * is safe today because Node serializes all synchronous code, so no other + * mutation can occur between a `peekId()` and the caller's `generateId()`. + * If a repository built on this base is ever swapped for an async-backed + * (e.g. database) store, this assumption breaks and id allocation must be + * made atomic (a single transactional allocation call) rather than a + * separate peek + generate. See `docs/ARCHITECTURE.md`. + */ protected generateId(): number { const id = this.nextId; this.nextId += 1; diff --git a/src/repositories/settlementRepository.test.ts b/src/repositories/settlementRepository.test.ts index d6f624f..d9262a9 100644 --- a/src/repositories/settlementRepository.test.ts +++ b/src/repositories/settlementRepository.test.ts @@ -23,6 +23,47 @@ describe("SettlementRepository", () => { expect(repo.peekNextId()).toBe(3); }); + describe("save anchor reindex", () => { + it("rebuilds the anchor index when save() changes the anchor", () => { + const repo = new SettlementRepository(); + const created = repo.create(draft("anchorA", 100)); // id 1, indexed under anchorA + + repo.save({ ...created, anchor: "anchorB" }); // anchor changes + + expect(repo.get(created.id)?.anchor).toBe("anchorB"); + expect(repo.byAnchor("anchorA")).toHaveLength(0); + expect(repo.byAnchor("anchorB")).toHaveLength(1); + }); + }); + + describe("peekNextId", () => { + it("previews the id that the immediately following create() yields", () => { + const repo = new SettlementRepository(); + + const previewed = repo.peekNextId(); + const created = repo.create(draft("anchorA", 100)); + + // Locks in the synchronous-only guarantee: peek -> create (no await in + // between) must return the exact id that was previewed. If this ever + // regresses (e.g. an async boundary is introduced between peek and + // create) the contract documented on SettlementRepository.peekNextId() + // would be violated. + expect(created.id).toBe(previewed); + // The counter has advanced exactly once after the create. + expect(repo.peekNextId()).toBe(previewed + 1); + }); + + it("yields a stable preview when no create() runs in between", () => { + const repo = new SettlementRepository(); + repo.create(draft("anchorA", 100)); + + const first = repo.peekNextId(); + const second = repo.peekNextId(); + expect(first).toBe(second); + expect(first).toBe(2); + }); + }); + it("saves status changes", () => { const repo = new SettlementRepository(); const created = repo.create(draft("anchorA", 100)); diff --git a/src/repositories/settlementRepository.ts b/src/repositories/settlementRepository.ts index d887d9a..cc436b7 100644 --- a/src/repositories/settlementRepository.ts +++ b/src/repositories/settlementRepository.ts @@ -5,10 +5,36 @@ import { Settlement, isSettlementStatus } from "../models/settlement"; import { InMemoryRepository } from "./inMemoryRepository"; -export class SettlementRepository extends InMemoryRepository { +export class SettlementRepository extends InMemoryRepository< + number, + Settlement +> { /** Secondary index: anchor -> set of settlement ids */ private readonly anchorIndex: Map> = new Map(); - /** Returns the id that will be assigned to the next created settlement. */ + /** + * Returns the id that will be assigned to the next `create()` call. + * + * ⚠️ SYNCHRONOUS-ONLY SAFETY GUARANTEE — DO NOT RESERVE ACROSS AN AWAIT. + * + * This method MUST NOT be used to predict or reserve an id across an + * awaited (asynchronous) boundary. Calling `peekNextId()` and then acting on + * the returned value is safe *only* because `create()` is itself fully + * synchronous: Node's single-threaded event loop serializes all synchronous + * code, so no other `create()` can run "in between". No locking is performed + * — the previewed id is a hint, not a reservation. + * + * If this repository is ever swapped for an async-backed store (e.g. a real + * database), this guarantee breaks: `peekNextId()` and `create()` would no + * longer execute atomically, and a concurrent caller could consume the + * previewed id before the caller reaches `create()`. In that future design, + * id allocation must be performed inside a single transactional/atomic + * operation rather than split across a `peek` + `create`. + * + * For the current in-memory, synchronous store, `peekNextId()` immediately + * followed by `create()` is guaranteed to yield the previewed id. See + * `src/repositories/settlementRepository.test.ts` for the locked-in test and + * `docs/ARCHITECTURE.md` for the persistence-swap risk note. + */ peekNextId(): number { return this.peekId(); } @@ -49,7 +75,6 @@ export class SettlementRepository extends InMemoryRepository this.anchorIndex.set(settlement.anchor, newSet); } return this.upsertByKey(settlement.id, settlement); - } /** Returns the settlement with `id`, or `undefined`. */ @@ -101,4 +126,3 @@ export class SettlementRepository extends InMemoryRepository return result; } } -