diff --git a/CLAUDE.md b/CLAUDE.md index 910e1c5..761e04a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -43,7 +43,7 @@ distilled to what applies to HQ. A change that breaks one does not land. | Append-only audit | Every mutation calls `writeAudit(db, userId, action, entity, id, before, after)` in the **same transaction** as the change. The audit log is append-only — never update or delete rows. Previews and the public share route write nothing. | `audit.ts` | | Client id is the future tenant id | `client.id` is a client-generated UUIDv7 that becomes `tenant_id` when the cloud tier stands up — same rows, promoted once. Never re-key clients. | `repos-clients.ts` | | No silent caps | Every list that can grow paginates or warns; never silently truncate. | reports, directory queries | -| Strict TS, green before landing | Strict TypeScript everywhere; `npm run typecheck` and `npm test` both clean before anything lands. Money math is locked by tests — a rounding change without a test change does not land. | `tsc`, vitest (377 tests) | +| Strict TS, green before landing | Strict TypeScript everywhere; `npm run typecheck` and `npm test` both clean before anything lands. Money math is locked by tests — a rounding change without a test change does not land. | `tsc`, vitest (389 tests) | ## Running it @@ -66,7 +66,7 @@ cd apps/hq-web && npm run dev - Postgres integration tests: `HQ_PG_TEST_URL=postgres://hq:hq_dev@localhost:5432/hq_test npm test` (wipes that database each run — never point it at real data). - Health check: `GET /api/health` → `{ ok: true }`. -- Verified state: `npm install` + typecheck clean, 377 tests pass. Exercise real flows end +- Verified state: `npm install` + typecheck clean, 389 tests pass. Exercise real flows end to end before claiming done. ## Docs (kept in `docs/`) diff --git a/README.md b/README.md index 4347987..d2c2d2c 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,7 @@ document and conversation we've had with them. ## Build state (start here) -Working codebase, not a plan. `npm install` + typecheck are clean and **377 tests pass**. +Working codebase, not a plan. `npm install` + typecheck are clean and **389 tests pass**. Current state and run-vs-pending detail live in **[STATUS.md](STATUS.md)**; the go-live runbook (server, HTTPS, backups, Gmail/AWS/import wiring, the Postgres switch) is in **[docs/DEPLOY-HQ.md](docs/DEPLOY-HQ.md)**. diff --git a/STATUS.md b/STATUS.md index b6063f1..3d60c62 100644 --- a/STATUS.md +++ b/STATUS.md @@ -25,7 +25,7 @@ section-per-module quotation PDF — then closed all 8 findings from its re | --- | --- | | `npm install` | clean | | `npm run typecheck` (root + workspaces) | clean, no errors | -| `npm test` (`vitest run`) | **377 tests pass across 71 files** | +| `npm test` (`vitest run`) | **389 tests pass across 74 files** | The 346 figure is the live `vitest run` count measured 2026-07-17, after the D18 go-live cluster **and its post-delivery review pass** (8 reviewer findings diff --git a/apps/hq/src/db.ts b/apps/hq/src/db.ts index e506212..7039bd2 100644 --- a/apps/hq/src/db.ts +++ b/apps/hq/src/db.ts @@ -354,7 +354,7 @@ CREATE TABLE IF NOT EXISTS aws_usage ( updated_at TEXT NOT NULL, UNIQUE (client_id, month) -- one row per client per month; the pull upserts on this key ); -${INDEX_DDL}` +` /** Open the SQLite engine (dev/tests). Deliberately sync — schema + migrate run * on the raw handle before it is wrapped, so test fixtures stay one-liners. */ @@ -370,6 +370,9 @@ export function openDb(dataDir?: string): DB { } raw.exec(SCHEMA) migrate(raw) + // Indexes run AFTER migrate() so every column they reference exists on an older DB + // that migrate() has just patched (e.g. client.owner_id, the D24 lockout columns). + raw.exec(INDEX_DDL) return new SqliteDb(raw) } diff --git a/apps/hq/src/repos-payments.ts b/apps/hq/src/repos-payments.ts index 180b471..46a18b6 100644 --- a/apps/hq/src/repos-payments.ts +++ b/apps/hq/src/repos-payments.ts @@ -164,6 +164,13 @@ export async function recordPayment(db: DB, userId: string, input: RecordPayment allocated.push({ documentId, amountPaise }) await refreshSettlementStatus(db, userId, documentId) } + // D24 (red-team): serialize concurrent allocations to the SAME invoice so two payments + // cannot both read the old outstanding and over-allocate. On Postgres, take a row lock + // before reading outstanding — held to COMMIT. On SQLite the connection mutex already + // serializes whole transactions, and FOR UPDATE is a syntax error there, so it's a no-op. + const lockInvoice = async (docId: string): Promise => { + if (db.engine === 'postgres') await db.run(`SELECT id FROM document WHERE id=? FOR UPDATE`, docId) + } if (input.allocations !== undefined) { // Explicit allocations bypass the oldest-first loop, but never exceed @@ -178,6 +185,7 @@ export async function recordPayment(db: DB, userId: string, input: RecordPayment if (doc.docNo === null) throw new Error(`Invoice is not issued: ${a.documentId}`) if (doc.status === 'cancelled') throw new Error(`Invoice is cancelled: ${doc.docNo}`) if (doc.clientId !== input.clientId) throw new Error(`Invoice ${doc.docNo} belongs to another client`) + await lockInvoice(doc.id) const outstanding = await outstandingOf(db, doc) if (a.amountPaise > outstanding) { throw new Error(`Allocation ${a.amountPaise} exceeds outstanding ${outstanding} on ${doc.docNo}`) @@ -193,6 +201,7 @@ export async function recordPayment(db: DB, userId: string, input: RecordPayment // stays unallocated (an advance). for (const doc of await unsettledInvoices(db, input.clientId)) { if (pool <= 0) break + await lockInvoice(doc.id) const take = Math.min(pool, await outstandingOf(db, doc)) if (take <= 0) continue pool -= take diff --git a/docs/06-DECISIONS.md b/docs/06-DECISIONS.md index c3d2fa3..274e3b3 100644 --- a/docs/06-DECISIONS.md +++ b/docs/06-DECISIONS.md @@ -382,3 +382,41 @@ so `ORDER BY id` is genuine creation order (pagination + the sync cursor rely on sub-millisecond tie no longer falls back to the random tail. Suite at 377 tests / 71 files after D22. + +## D24 — Red-team audit remediation ✅ (2026-07-18) +A 14-surface multi-agent red-team + quality audit (report artifact + `redteam-findings`) +returned 96 verified findings — **0 critical, 16 high, 43 medium, 37 low**. All 16 highs and +the load-bearing mediums were fixed in seven batches, each committed green with tests: +- **Auth (A):** `staff_user.failed_count`/`locked_until` — 8-fail/15-min account lockout; + `login()` constant-time (dummy scrypt on missing email — no enumeration oracle) returning a + typed `{ok,reason}`; per-IP (20/min) + per-email (8/15min) limiters on `/auth/login`; + per-user reveal limiter (20/min) on the three credential-reveal routes; `trust proxy` so + `req.ip` is the real client behind nginx. `makeRateLimiter` → `rate-limit.ts` (no cycle). +- **Authorization (B):** `requireManagerial` gate on `cancel` / `credit-note` (destructive) + and `POST /documents/:id/share` (the unauthenticated-public-link IDOR); `PATCH + /interactions/:id` restricted to author-or-managerial. **Deliberate:** general client + read/edit stays open to staff — a shared support-team book is the intended model; the + sensitive credential reveals are managerial + rate-limited. +- **Money (C):** credit-note capped to the invoice's remaining creditable value (read+insert + one txn); `nextDocNo` atomic `UPDATE … RETURNING` (row lock, no duplicate/burned numbers); + `markStatus` rejects accepted/lost on non-QUOTATION docs; `computeBill` bounds qty + magnitude (fractional kept for weighed Store goods) + HQ `buildLines` requires whole-unit + integer qty; `recordPayment` rejects TDS > cash received. +- **Concurrency (D):** `SqliteDb.transaction` serialized by an async mutex + AsyncLocalStorage + re-entrancy (nested → savepoints) — the shared `txnDepth` counter could interleave + concurrent BEGIN/COMMIT and lose writes; scheduler isolates each recurring plan (one bad + plan no longer aborts the whole scan + auto-send); notify-broadcast IIFE gets a trailing catch. +- **Rendering (E):** strict CSP meta in both letterhead templates + puppeteer aborts every + non-`data:` request — an `esc()` miss degrades to a rendering glitch, not SSRF/RCE. Preview + ↔ PDF byte-fidelity preserved. +- **Backend perf (D24):** 30 indexes on the hot FK/status/date columns, both engines + (`INDEX_DDL` + pg migration 007); client search + district/sector filters `LOWER(…) LIKE + LOWER(?)` — bare LIKE was case-SENSITIVE on Postgres and silently broke prod search. +- **Frontend (F):** a11y (keyboard rows, `
`+skip link, aria-labels, form Login, alert + toasts), UX safety (confirm on mass-email + reminder send, in-flight disable to stop + double-submit, debounced search, visible inline errors, ErrorBoundary), and shared + component extraction. +**Explicitly accepted / deferred (documented, not fixed):** async scrypt for `verifyPin` +(large shared-package refactor; the event-loop-DoS vector is mitigated by the login rate +limiter now); rendered-PDF caching on `/share`; api.ts file split; broad naming-consistency +renames (`staff`/`user`/`employee`, `at_wall`, schema.sql drift) — too risky pre-go-live.