docs(security): SEC-C6 — override approval is client-asserted, audit forgery hole

Automated review flagged POST /api/bills trusting overrides[].approvedByUserId
from the client; verify-pin result never bound to the commit. Server-side
approval-token fix specified; must land before any pilot (Phase 1).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
feat/client-detail-redesign
Thomas Joise 2 weeks ago
parent 406839659d
commit 507eec67f3

@ -292,6 +292,9 @@ An append-only audit log without built-in analytics has zero deterrent value —
| Bills per hour vs counter peer median | no-bill sales | < 60% of median sustained |
| Khata credit to same phone-number cluster | fake-credit fraud | > threshold |
**SEC-C6 · Override approval is client-asserted — audit forgery hole (found by automated review, 2026-07-10)** *(fix in Phase 1 before any pilot)*
`POST /api/bills``commitBill` trusts `overrides[].approvedByUserId` straight from the request: `/auth/verify-pin` (the E-6105 supervisor gate) verifies a PIN but its result is never bound to the commit, so any client on the LAN can stamp "approved by <owner>" into the audit log with no PIN entered at all — the SEC-C2/POS-9 discount gate becomes decorative and the audit trail is forgeable exactly where fraud analytics rely on it (the "supervisor approvals while approver off-shift" counter above audits forged names, not real approvals). **Impact:** high — sweethearting with fabricated supervisor cover; violates invariant I7 in spirit. **Mitigation (all server-side, in the commit transaction):** (a) recompute the gate — canonical `sale_price_paise`/MRP and discount caps checked in the txn; any deviation *requires* a matching override entry or the commit rejects; (b) `/auth/verify-pin` returns a **short-lived, single-use approval token** (server-stored `{approverUserId, cashierId, itemId, kind, before, after, exp}`); the commit verifies and consumes the token and derives `approvedByUserId` from the server record — never from the request body; (c) approver role membership (`APPROVER_ROLES`) re-read from `app_user` inside the txn; display-name lookup tenant-scoped. Flagged twice by automated review on the codebase's first commit (api.ts route + repos.ts commitBill lenses).
**SEC-D · Licensing, offline enforcement & piracy** *(state machine Phase 1; KILL/dealer program Phase 2)*
Licensing is cloud-authoritative but the POS is offline-tolerant *indefinitely* — an infinitely-offline counter can never re-verify its subscription, yet genuine tier-3 customers go offline for weeks and pirates go offline forever, and setting the PC clock back is the first thing everyone tries. **Impact:** too strict bricks a paying customer's counter (unforgivable, and the dealer channel screams); too loose makes subscriptions optional. **Mitigation:** the license token is a **signed snapshot of `plan_feature` rows verified locally with an embedded public key only** (no secrets in an Electron bundle — anything shipped is public), refreshed on every successful sync (no separate licensing call to fail independently). Enforcement is a **degradation ladder, never a cliff — billing and printing survive every state except a human-approved, dual-control KILL**, and even KILL leaves the app read-only with data export, because their data is theirs and holding it hostage is the story that kills us in the channel. The ladder timings are a founder-sign-off decision (they encode a commercial stance on how gently we treat non-payers) and each is a `setting` row so support can extend a flooded town's stores in one action:

Loading…
Cancel
Save