You cannot select more than 25 topics Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
sims-hq/docs/18-RED-TEAM-REVIEW.md

36 KiB

18 — Red-Team Security Review (2026-07-10)

Adversarial review of the SiMS Next build by five parallel reviewers, each reading the code and firing proof-of-concept requests at the live dev server (:5181). Every finding is labelled CONFIRMED (proven with a PoC) or THEORETICAL. Nothing was fixed — this is assessment; remediation is the founder's call (see plan at the end).

The one-sentence verdict

The store-server authenticates requests but does not authorize them and does not validate or anchor the business data in them — so any logged-in cashier (the lowest role) can, with curl, rewrite prices, evade GST, edit the item master, and forge the audit trail. The React app enforces all these rules; the server trusts the client to have done so. Nothing is shipped to a real store yet, so this is the right moment to find it.

Why "the server re-verifies every bill to the paisa" (R5) gave false confidence: it proves the client's total matches the client's own line prices — internal consistency, not authenticity against the item master. The supervisor gate we verified in the browser is real in the UI and unenforced on the server.

Remediation status — P0 CLOSED (2026-07-10)

The P0 workstreams below are REMEDIATED: the trust boundary now lives inside the store-server (item-master anchoring, per-route authorization, input validation, bound approval tokens). The dev DB was reset and every PoC re-fired against the fresh server; each that used to succeed now fails closed. A real-browser E2E confirms the honest counter flow still works. P1 and P2 remain open (see the plan at the end).

PoC Before (assessment) After (2026-07-10) Evidence
C1 cashier → POST /items (edit master) 200, item written 403 E-6105 authz middleware; a cashier may still POST /items a draft (200) — now with a server-forced tax class + price cap + DRAFT_SALE audit (see the 2026-07-13 follow-up below)
C1 cashier → GET /audit 200, full log 403 E-6105 AUDIT_VIEW gate
C1 cashier → GET /purchases/last-costs, /purchases 200, supplier costs 403 E-6105 PURCHASE_ENTRY/REPORT_VIEW gate
C2 cashier bills a ₹275 item at ₹1 committed, no audit 403 E-1302 (deviation needs approval) server anchors master price; nothing committed
C3 12% item sent as taxClassCode:ZERO committed, ₹0 GST committed at GST12 (CGST₹14.73+SGST₹14.73 on the ₹275 bill) tax class read from master, request ignored
C4 cashier POST /purchases w/ newSalePricePaise master price rewritten 403 E-6105 (route denied); a PURCHASE_ENTRY-only role receives stock but the price change is ignored (needs MASTER_EDIT) authz + MASTER_EDIT split
H4 bound token for item A reattached to item B audit falsely records the approver 403 E-6108 (bound mismatch); same token used correctly on item A → 200 token binds item/kind/before/after
H5 itemId:"DOES-NOT-EXIST" phantom bill + stock 400 E-1105 item verified vs (id, tenant)
H6 businessDate 2050-07-10 / not-a-date postdated bill / garbage series 400 E-1106 ISO + windowed validation
H7 owner/supervisor self-authorised override no audit row at all audited from the session identity (selfAuthorized:true) server auto-detects + audits every deviation
M6 /items?limit=-1 whole table clamped (≤ 1 row) limit clamped [1,MAX], offset ≥ 0
M7 wrong types 500 + stack + install path 400 {error,code} generic, no stack hand-rolled validators + 4-arg error handler
M8 negative/mismatched payment legs, fractional qty slipped through 400 (leg > 0, allowlisted mode, sums to payable; unit-decimals enforced) payment + qty guards
M12 giant line count unbounded ≤ 500 lines (400 over cap) assertArray cap
M14 X-Powered-By: Express banner present removed app.disable('x-powered-by')

Real-browser E2E (fresh DB, Chrome): Ramesh 4728 → normal Surf Excel bill ST1C2/26-000004 commits at ₹145; Ramesh F3 rate ₹145→₹140 → Suresh 8265bound-token commit …000005, audited approvedBy=Suresh; Thomas 9174 owner self-price-change → commit …000006, audited approvedBy=Thomas, selfAuthorized:true. Tests 207 green; typecheck + all three app builds clean.

HIGH follow-up — cashier draft quick-add path CLOSED (2026-07-13)

The independent verifier found that the C1 remediation left one HIGH hole inside the quick-add convenience it deliberately kept open (an unknown-barcode item must not stop the queue): a cashier (BILL_CREATE, no MASTER_EDIT) could POST /items with status:'draft', choosing an arbitrary salePricePaise AND an arbitrary taxClassCode, then bill it. Because commitBill anchors each line to the just-authored master, this reproduced C2 (arbitrary price) and C3 (GST evasion — a taxable good self-added as ZERO, sold at ₹0 GST) with no deviation/override audit row — the only trace was ITEM_CREATE. Proven live: draft "Redteam Whiskey" ZERO tax @ ₹2000 → billed with ₹0 GST.

Remediated (2026-07-13) — the quick-add convenience is kept, the fraud is removed:

Vector Fix Evidence (re-fired live as cashier u1/4728, fresh DB :5181)
C3 via draft — self-add taxable good as ZERO Server forces the tax class on a non-MASTER_EDIT caller's draft to the store default (catalog.defaultTaxClass, resolved system→tenant, default GST18); the client's taxClassCode is ignored. A MASTER_EDIT role (manager/owner) still sets it. draft ZERO→server assigned GST18; billed the draft → CGST+SGST = 30508 paise (was ₹0)
C2 via draft — self-price an expensive unknown A non-MASTER_EDIT draft's salePricePaise is capped (catalog.draftMaxPricePaise, default ₹5,000 = 500000); over-cap is denied with a clear message. A MASTER_EDIT role may exceed it. draft @ ₹9,999 → 400 E-1108 "call a supervisor"; owner @ ₹9,999 → 200
Invisible draft sale — self-authored item sold with no trail Every bill line whose master row is still status:'draft' writes a DRAFT_SALE audit row (entity bill, {itemId, name, pricePaise, docNo}) in the commit transaction. Back office already shows draft status on Items. selling the two drafts wrote 2 DRAFT_SALE rows; an active-item sale writes none
Cross-counter bill read (residual LOW/MED) GET /bills for a caller without REPORT_VIEW is pinned server-side to the session user id — a cashier sees only their own bills; a REPORT_VIEW/manager keeps the full view. Ramesh → only u1 rows; Divya → only u4 rows; owner → all cashiers

Regression re-check (same run): C2 existing-item ₹275→₹1 no-approval → 403 E-1302; C3 existing-item ZERO@master → committed taxed at GST12 (CGST+SGST 2946 paise); C1 cashier POST /items status:'active'403 E-6105, cashier GET /audit403. Legit quick-add still works end-to-end (unknown barcode → draft @ ₹30 → 2× bill ST1C2/26-000004 ₹60). 220 tests green (9 new: draft tax forced, over-cap E-1108, DRAFT_SALE audit, cashier /bills scoped, owner exceptions); typecheck + all three app builds clean. Back office must complete each draft's real HSN/tax class later (the forced default is a safe placeholder, not a substitute for classification).

P1 edge & transport hardening — CLOSED (2026-07-13)

The P1 "lock down the edges + TLS" workstream is REMEDIATED: H1, M1, M2, M4, M13, M14 are closed and M15 is noted (closed once prod is HTTPS). Server changes stayed in server.ts + api.ts plus three pure, unit-tested modules (print-guard.ts, rate-limit.ts, session-policy.ts); client edits were limited to logout wiring + the POS post-login bootstrap re-fetch. No new runtime dep except helmet (justified). All PoCs were re-fired live against :5181 (the HTTP dev instance, unchanged); the session-TTL and rate-limit PoCs ran against a short-lived isolated instance (:5182, own data dir) so the dev DB and :5181 stayed clean. (M5 — tenant-scoping commitBill's store/counter lookup — is owned by the money-logic workstream and is NOT part of this pass.)

# Before After (2026-07-13) Evidence (live)
H1 SSRF via POST /api/print authed cashier opened a socket to ANY host:port + bytes (internal port-scan / egress) destination constrained: LAN-only IP (127/8,10/8,172.16/12,192.168/16; link-local/public rejected), port allowlist {9100,9101,9102} (+SIMS_PRINTER_PORTS), ≤64 KB payload; the host is resolved and every resolved IP checked, then we connect to the resolved IP (no DNS-rebind). Now an authenticated route inside apiRouter authed → 8.8.8.8:80 400 E-2202, 8.8.8.8:9100 400 E-2203 (no socket opened), 127.0.0.1:9100 502 ECONNREFUSED (guard passed → socket attempted); unauth → 401
M1 /bootstrap leaks roster roles + GSTIN pre-auth the unauthenticated payload carried each user's role + the tenant GSTIN (targeting data) the pre-auth payload drops role + gstin (keeps id+name for PIN login); the SAME endpoint restores both when called WITH a session — the POS re-fetches after login, back office is always authed unauth /api/bootstrapstore has no gstin, users are {id,name} only (grep for gstin/role: none); authed → gstin 32ABCDE1234F1Z5 + roles present
M2 verify-pin unauth PIN oracle / token mint callable with NO session — a raw PIN-guessing + token-minting endpoint now behind requireAuth (a logged-in caller) and rate-limited; the approver PIN still rides in the body unauth POST /auth/verify-pin401 E-6103; authed cashier caller + Suresh/8265 → 200 + bound approvalId; wrong approver PIN → still 401
M4 no session expiry / logout / revocation a token was valid until the process restarted sessions carry created + last-seen; requireAuth evicts past a 12 h absolute / 30 m idle TTL (→ 401 E-6110); POST /api/auth/logout deletes the session (POS "Lock" + back-office "Logout" call it); global + per-IP /auth/* rate limit (20/min/IP, 200/min global, env-tunable) layered on the per-user lockout logout: /items 200 → logout 200 → /items 401; idle (1 s-TTL instance): after 1.4 s → 401 E-6110; rate limit (cap 3): attempts 1-3 = 401, 4-5 = 429
M13 plaintext HTTP bearer token rides the LAN in cleartext HTTPS available: SIMS_TLS_CERT+SIMS_TLS_KEY set ⇒ https.createServer; unset ⇒ HTTP dev (unchanged). Dev-cert one-liner + "prod MUST supply certs" documented in server.ts. HSTS + upgrade-insecure-requests emitted only under TLS (so dev HTTP is never told to upgrade to a port that serves no HTTPS) dev :5181 stays HTTP (verification/browser flow intact); the TLS path is code-reviewed, not forced on dev
M14 no security headers / X-Powered-By no CSP, no nosniff/frame/referrer headers helmet on with a CSP tuned for the SPAs ('self' scripts + 'unsafe-inline' styles for React inline styles + blob:/data: for the invoice/label tabs + data: fonts; the blob tabs' only inline script — window.print() — is allowed by a single 'unsafe-hashes' sha256, NOT by opening script-src to 'unsafe-inline'); X-Frame-Options: DENY (helmet only ships SAMEORIGIN) + frame-ancestors 'none'; nosniff; Referrer-Policy: no-referrer; x-powered-by already gone curl -i /api/health shows the full header set, no X-Powered-By, no HSTS on HTTP; back office + POS both load in a real browser with ZERO CSP console violations, and the window.print() inline handler is not blocked
M15 SW cache-first over HTTP → MITM poisoning THEORETICAL, needs LAN MITM noted — closes automatically once prod serves HTTPS (M13); no code change needed for the dev slice n/a (documentation)

Tests: two new files — edge-guards.test.ts (pure: LAN/port/byte print validator, sliding-window rate limiter, session-TTL check) and edge.test.ts (HTTP against a mounted apiRouter: bootstrap minimize, verify-pin auth, TTL eviction, logout, rate-limit 429, print allowlist). 245 tests green (was 220), typecheck clean on root + all three apps, all three apps build, store-server rebuilt + restarted on :5181 (/api/health ok). Browser check: back office login + POS login screens render with no CSP violations; the POS login tiles now show no role subtitle (the pre-auth roster no longer carries roles — the expected M1 effect). Residue: the live PoCs added one benign, unused override_approval row to the :5181 dev DB (a verify-pin mint); no bills/items/stock were written. No git commits.

P2 data-integrity & crypto — CLOSED (2026-07-13)

The P2 "harden-before-cloud" data-integrity slice is REMEDIATED: H2, M3, M5, M9, M10 are closed (M11 client-PII and M14 helmet closed earlier). Only H3 (encryption at rest) remains, deliberately owned by a separate agent next. Changes were confined to apps/store-server/src/{db.ts, repos.ts, api.ts, server.ts}, packages/auth/src/pin.ts, packages/domain/src/doc-series.ts, plus dev-start.mjs + .env.example; the transport/helmet/TLS and requireAuth/rate-limit/session code from the P1 pass was left intact (only writeAudit and the commit/series paths were touched).

# What changed Evidence (live on the reset :5181, SIMS_SEED_DEMO=1)
H2 audit hash chain audit_log gained prev_hash+entry_hash (CREATE TABLE + PRAGMA-guarded migrate). writeAudit links each row into the tenant's chain inside the existing commit transactions (entry_hash = sha256(canonical(prev_hash, tenant_id, at_wall, user_id, action, entity, entity_id, before_json, after_json)), genesis '', insertion order = SQLite rowid under the single-writer model). verifyAuditChain(db, tenant) recomputes and returns {ok, brokenAtId?}; GET /api/audit/verify (AUDIT_VIEW) exposes it. Tamper-EVIDENCE, not prevention — the file is still writable (H3). Owner (thomas/9174) GET /api/audit/verify after a real bill → {ok:true}; then a direct better-sqlite3 UPDATE audit_log SET after_json=… on one row → verify → {ok:false, brokenAtId:<that row>} (brokenAtId matched the tampered row exactly).
M3 PIN pepper pin.ts HMAC-SHA256s the secret with SIMS_PIN_PEPPER before scrypt in both hashPin/verifyPin; unset ⇒ raw-secret fallback (byte-identical to pre-M3) + one-time warning. .env.example documents it; enabling requires PIN re-enrolment. Unit tests: round-trip with a pepper; a peppered hash fails to verify once the pepper is gone (a DB-only theft is uncrackable) and again with a different pepper (rotation invalidates); unset path still verifies. Dev logins on :5181 unchanged (pepper unset).
M5 tenant-scope commit store AND counter selected WHERE id=? AND tenant_id=?; unknown-for-tenant → 400 E-1110 before any money logic (kills the undefined-deref 500). Both /bills and /bills/offline covered. POST /api/bills with storeId:"no-such-store"400 {"code":"E-1110"} (not 500). Unit: a store/counter belonging to another tenant is rejected; honest same-tenant commit still bills.
M9 4-digit FY seriesPrefix embeds the 4-digit FY start year; the counter↔FY / is dropped so the doc no fits the GST 16-char rule at 6-digit sequences (ST1C22026-000123). Old ST1C2/26-… collided a century apart. Fresh bill on :5181 → doc no ST1C22026-000001 (length 16, matches ^ST1C22026-\d{6}$). Unit: seriesPrefix('ST1','C2','2026-27')…'2126-27', both ≤16 at seq 999999.
M10 demo-seed gate seedIfEmpty/ensureBatchTrackingSeed run only when SIMS_SEED_DEMO=1; dev npm startdev-start.mjs sets it. Isolated instance, flag unset → healthy but 0 tenants, thomas login 401. :5181 with the flag set → seeds MegaMart + 4 users as before.

Tests: 260 green (was 245): new audit-chain.test.ts (5: chain continuity + UPDATE/DELETE/INSERT detection + per-tenant scope), tenant-scope.test.ts (4: nonexistent/cross-tenant store/counter reject, honest commit), a /audit/verify HTTP case in edge.test.ts, 4 PIN-pepper cases in auth.test.ts, and a 4-digit-FY collision + 16-char case in domain.test.ts. Typecheck clean on root + all three apps; all three apps build; the dev DB was reset and the store-server restarted on :5181 with SIMS_SEED_DEMO=1 (/api/health ok, seeded, audit/verify ok on the clean chain). H3 (encryption at rest) is the only open P2 item. No git commits.

H3 — encryption at rest — CLOSED (2026-07-13)

The last open finding is REMEDIATED: the store DB is now full-DB encrypted at rest with SQLCipher, which closes H3, the F5 at-rest concern, and the M11 residual (the offline queue's lines/amounts sitting in plaintext until drained — the server tier they drain into is now encrypted). better-sqlite3 was swapped for the API-compatible drop-in better-sqlite3-multiple-ciphers (same API + pragma('key=…'); it ships its own DefinitelyTyped-derived declarations, so DB and every call type unchanged — no shim, no loosened strictness). The change was confined to apps/store-server/src/db.ts, package.json, build-server.mjs and .env.example; none of the earlier hardening (pepper, hash chain, tenant-scope, seed gate, TLS/helmet, print allowlist) was touched.

Path taken: full-DB encryption. The native package installed from a prebuilt binary (no node-gyp build) on this Windows / Node 22 box, so the app-level field-encryption fallback was not needed.

Vector Fix Evidence (live on the reset, keyed :5181)
Whole file readable if copied — a stolen/snooped sims.db (+ WAL) yielded phones/GSTINs (DPDP), costs & margins, full sales history, party ledgers, PIN salts/hashes In openDb(), when SIMS_DB_KEY is set the DB is keyed (cipher='sqlcipher', key=…) before any read, so every page — main db and the WAL/SHM sidecars — is ciphertext. The key lives ONLY in the server env (a machine-scoped DPAPI/keychain secret), never in the repo or the DB; single quotes in it are escaped so an arbitrary passphrase is a safe SQL literal. head -c 16 data/sims.db28 85 83 94 c9 b5 4f 93 … (random), not 53 51 4c 69 … ("SQLite format 3\0"). Grep of sims.db / -wal / -shm for MegaMart, Lakshmi, the phone 9840012345, and the tenant GSTIN 32ABCDE1234F1Z5none found in cleartext.
A stolen copy must be unreadable without the key Wrong/absent key fails closed Opening the live data/sims.db without the key → file is not a database; with a wrong key → file is not a database; with the correct key → opens (tenant count 1, parties ["Lakshmi","Joseph",…] decrypt).
The app must still fully work through the cipher The keyed DB is transparent to the app Cashier Ramesh (u1/4728) PIN login 200; GET /items?all=1 200 (11 items decrypt); POST /bills 200 → ST1C22026-000001 (payable ₹5.00); owner Thomas (u3/9174) GET /audit/verify{ok:true} on a chain written through the encrypted DB. /api/health ok.
Dev fallback unchanged Key UNSET ⇒ open unencrypted exactly as before + a one-time warning that prod MUST set it (mirrors the SIMS_PIN_PEPPER pattern) Unit test: key unset ⇒ file is plaintext (SQLite format 3\0) and the phone is present in cleartext — a positive control proving the byte-scan genuinely detects plaintext. The one-time boot warning fires only when unset; with the dev key set on :5181 it does not.

Tests: 265 green (was 260; +5 in new encryption.test.ts: a keyed file has no SQLite magic + no cleartext PII across db/WAL, the keyed data round-trips, opening without / with the wrong key throws, and the plaintext-fallback positive control). Typecheck clean on root + store-server; all three apps build (esbuild externals the ciphers native module; vite unaffected). The dev DB was reset (data/ deleted — it held red-team junk + a deliberately-tampered audit row) and the store-server restarted on :5181 with a dev SIMS_DB_KEY, so encryption is live and proven end-to-end while the :5181 flow still works.

A production deploy still MUST: (1) supply SIMS_DB_KEY as a machine-scoped secret (Windows DPAPI or an OS keychain) — never a plaintext file beside the DB, never in the repo; (2) migrate any existing plaintext sims.db once, offline (ATTACH … KEY …; SELECT sqlcipher_export('enc'); DETACH — documented in openDb() and .env.example) before the first encrypted boot; a fresh/empty DB is created encrypted automatically; (3) treat key rotation as a re-key (PRAGMA rekey) or an export/reimport, and back up the key with the DB (an unrecoverable key = an unrecoverable DB); (4) still tighten the data-dir ACLs as defence-in-depth. No git commits.

Clean bill of health (verified defended — do not "fix")

Area Verdict
SQL injection None — every query is a bound better-sqlite3 prepared statement; DROP TABLE payloads inert
XSS / HTML injection None — every invoice/label/receipt sink escapes; React auto-escaping intact; zero dangerouslySetInnerHTML
Dependency supply chain npm audit 0 vulnerabilities across 542 deps; lockfile committed
CSRF / CORS Not vulnerable — bearer-token-in-header design (not cookies) defends both
doc-series under load Crash-safe & gap-free by transaction design (genuine strength, S7-proven)
Approval token entropy/TTL/single-use Correct — 5-min TTL + atomic single-use enforced server-side
Engine numeric guards Rejects negative/zero/NaN qty, negative price, >100% discount, payment mismatch, >1 MB body

Findings — ranked

CRITICAL — exploitable now by any cashier session

# Finding Evidence Fix
C1 REMEDIATED No server-side authorization. RBAC (permissions.ts can()/gateFor()) is called only in client code; api.ts never imports it. Every route past requireAuth checks only that a token exists. Cashier token created items, read full /audit, dumped supplier costs, listed all counters' bills — all 200. api.ts:138 is the lone gate. Per-route authorization middleware mapping route→ActionCode, can(session, action) server-side, deny by default.
C2 REMEDIATED Bill line prices trusted from client; the override/discount gate is client-only and curl-bypassable with no audit. commitBill recomputes from input.lines and never reads item.sale_price_paise/mrp. Cashier sold a ₹270 item at ₹1 — committed, no override, no PIN, no audit row (repos.ts:223, compute.ts:41-61). Load each line's master price; reject any deviation unless a bound override token is redeemed; hard-block price > MRP.
C3 REMEDIATED Client-chosen tax class → GST evasion. taxClassCode comes from the request; resolveTaxClass trusts it. A 12% item sent as ZERO committed with ₹0 GST (compute.ts:81). Scales to the whole ledger. Resolve tax class from the item master, never the request.
C4 REMEDIATED Purchase posting rewrites master price/MRP with no role check. Cashier posted a purchase with newSalePricePaise:99999; the item master changed (api.ts purchase route → repos-purchase.ts:99-106). Require PURCHASE_ENTRY; separate "receive stock" from "change master price".

HIGH

# Finding Evidence Fix
H1 REMEDIATED (2026-07-13) Authenticated SSRF via POST /api/print — arbitrary host:port + bytes; clean internal port-scan oracle. (Correctly auth-gated: unauth → 401.) Cashier token opened sockets to chosen host:port; closed→502, open→504 (server.ts:37-58). DONE: LAN-only IP + port allowlist ({9100,9101,9102}, SIMS_PRINTER_PORTS) + ≤64 KB cap; host resolved and each resolved IP LAN-checked, connect to the resolved IP (no rebind). print-guard.ts + the /print route in api.ts. See the P1 section.
H2 REMEDIATED (2026-07-13) Audit log & "immutable" bills are tamper-evident by convention only — no hash chain, no signing; plain SQLite file. Any file-level UPDATE/DELETE rewrites bills, deletes override evidence, forges approvals (repos.ts:431-443, schema). Undercuts the GST/MCA audit-trail claim. DONE: per-tenant hash chain on audit_log (prev_hash/entry_hash; entry_hash = sha256(canonical(prev_hash, tenant_id, at_wall, user_id, action, entity, entity_id, before_json, after_json))), written in-transaction by writeAudit; verifyAuditChain + GET /api/audit/verify (AUDIT_VIEW) recompute it. Any out-of-band UPDATE/DELETE/INSERT breaks the chain visibly. Tamper-EVIDENCE, not prevention — sealing the writable file is H3. See the P2 section.
H3 REMEDIATED (2026-07-13) No encryption at rest — PIN hashes, phone numbers (DPDP), GSTINs, costs, full sales history in a plain sims.db (+ WAL). A copied file yields the whole business + every staff PIN hash (db.ts new Database(file)). DONE: full-DB SQLCipher via the drop-in better-sqlite3-multiple-ciphers; openDb() keys the DB from SIMS_DB_KEY (env-only, machine-scoped) before any read, so sims.db + WAL/SHM are ciphertext; key unset ⇒ plaintext dev fallback + one-time warning. Proven live: header not SQLite format 3, no cleartext PII in db/WAL, open-without-key fails, app (login+bill+audit/verify) works with the key. Prod must supply the key via DPAPI/keychain + migrate an existing plaintext DB once (sqlcipher_export). See the H3 section.
H4 REMEDIATED Approval token not bound to the override it authorizes — redirectable. Token stores only approver+time+used, not item/kind/amount/cashier. A real "Milk ₹50→45" approval was reattached to "Whiskey ₹2000→200"; audit falsely records the supervisor (repos.ts:184-190, 303-337). Audited before/after are client-supplied too. Bind item/kind/before/after (+cashier) into the token at mint; re-verify at redeem.
H5 REMEDIATED Item never validated → phantom / cross-tenant lines. No SELECT item, no FK on stock_movement.item_id. itemId:"DOES-NOT-EXIST" committed a bill + phantom stock movement (repos.ts:192-343). Verify itemId belongs to tenant+store; add FK.
H6 REMEDIATED Unvalidated businessDate → antedated/postdated GST invoices + garbage series. 2050-07-10 postdated a bill; "not-a-date" produced doc_no ST1C2/N--000001 (repos.ts:239). Validate ISO date, reject NaN FY, bound to the open business day.
H7 REMEDIATED Self-authorized overrides (owner/supervisor at POS) are never audited end-to-end. gateFor==='allow' applies the edit with no override entry, so nothing reaches the server or the log (BillingScreen.tsx:431). The least-supervised roles leave no trail. Emit an audited (server-recomputed) override for every price/discount change, including self-authorized.

MEDIUM

# Finding Fix
M1 REMEDIATED (2026-07-13) Unauthenticated /api/bootstrap leaks the full user roster (ids, names, roles, who has a PIN) + store GSTIN → targeted PIN attack. DONE: pre-auth payload drops role + gstin (keeps id+name for PIN login); both restored on the SAME endpoint when called WITH a session (POS re-fetches post-login). See the P1 section.
M2 REMEDIATED (2026-07-13) /auth/verify-pin is an unauthenticated PIN oracle that also mints approval tokens and writes DB rows on each call. DONE: now behind requireAuth (a logged-in caller) + the /auth/* rate limiter; approver PIN still in the body.
M3 REMEDIATED (2026-07-13, pepper) Weak PIN throttle — in-memory, per-user, no global/IP limit, resets on restart; 4-digit space; scrypt cost immaterial vs the tiny keyspace; no server-side pepper. DONE (pepper): pin.ts HMAC-SHA256s the secret with a server-held pepper (SIMS_PIN_PEPPER) BEFORE scrypt, in both hashPin/verifyPin, so a stolen DB can't be brute-forced against the 10k PIN keyspace without the env-only pepper. Unset ⇒ raw-secret fallback (dev unchanged) + one-time warning. .env.example added; enabling requires re-enrolling PINs (dev re-seed does it). The IP/global /auth/* limiter already landed in M2/M4; longer approver PINs deferred (policy). See the P2 section.
M4 REMEDIATED (2026-07-13) No session expiry, logout, or revocation — a leaked token is valid until the process restarts; a fired employee can't be cut off. DONE: 12 h absolute + 30 m idle TTL evicted in requireAuth (→ 401 E-6110); POST /api/auth/logout deletes the session (POS Lock + BO Logout wired); global + per-IP /auth/* rate limit (rate-limit.ts, session-policy.ts).
M5 REMEDIATED (2026-07-13) commitBill store/counter lookup not tenant-scoped → cross-tenant reference (latent, single-tenant dev DB) + a bad storeId 500s the handler. DONE: commitBill selects store AND counter WHERE id=? AND tenant_id=? and throws 400 E-1110 ("Unknown store or counter for this tenant") BEFORE any money logic — killing the old undefined-deref 500 and any cross-tenant reference. Both /bills and /bills/offline funnel through it. (E-1110, not the task-suggested E-1109, because E-1109 is already the payments-≠-payable code.) See the P2 section.
M6 Pagination bypasslimit=-1 / huge / NaN returns whole tables → unbounded stall/OOM on /items,/bills,/audit. Clamp limit to [1,MAX], offset ≥ 0, reject NaN.
M7 No request schema validationas casts; wrong types 500 with full stack traces + absolute install paths. zod/valibot per route + a generic error handler that never leaks the stack; NODE_ENV=production.
M8 Numeric edges slip through: fractional qty on integer units, qty > 2^53 (precision loss / ledger corruption), negative payment legs when the sum still matches. Enforce unit-decimals; clamp qty/total < 2^53; require every payment leg > 0 with an allowlisted mode.
M9 REMEDIATED (2026-07-13) doc-series fyShort 2-digit truncation → FYs 100 years apart collide, forcing UNIQUE violations that can block billing on a prefix. DONE: seriesPrefix now embeds the full 4-digit FY start year ("2026" vs "2126" no longer collide). To keep the GST 16-char rule at 6-digit sequences the counter↔FY / is dropped (the sequence - stays): ST1C22026-000123 (16 chars). doc_no format changes → dev reseed handles it; prod starts the new format at cutover. Caveat: store+counter codes must total ≤ 5 chars (the same tightness the old format had at C10). See the P2 section.
M10 REMEDIATED (2026-07-13) Hardcoded demo owner credentials (thomas/PIN 9174/pw sims) auto-seed on an empty DB. DONE: seedIfEmpty/ensureBatchTrackingSeed run ONLY when SIMS_SEED_DEMO=1; a prod empty DB never auto-creates the known-credential owner (stays empty for onboarding). Dev keeps seeding via npm startdev-start.mjs (sets the flag). Proven live: flag unset ⇒ 0 tenants, thomas login 401. See the P2 section.
M11 REMEDIATED (client PII) Unencrypted customer PII in the offline IndexedDB (names, GSTINs, line items) on shared counter PCs. POS client fixed: the queued bill now persists only customerId — no customerName/buyerGstin — and the server re-resolves the buyer's name/GSTIN/place-of-supply from that id at drain, exactly as for an online bill (repos.ts:296-298). Catalog + staff-roster cache is wiped on Lock/logout (clearCatalogCache), while un-drained queued bills are preserved. Verified in a real browser IndexedDB (queued record carries customerId only; injected PII dropped; cache cleared on lock; queue survived). Files: apps/pos/src/offline.ts, App.tsx onLock, BillingScreen.tsx queueOffline; test apps/pos/test/offline.test.ts. Residual (now closed): the queued lines/amounts sat in a plain IndexedDB until drained; the server tier they drain into is now SQLCipher-encrypted at rest (H3, closed 2026-07-13), so the standing at-rest exposure is removed end-to-end.
M12 (line cap) Single-threaded synchronous server: one 5,200-line bill blocked /api/health 155 ms; no per-bill line cap. Cap lines.length; clamp result sizes (M6).
M13 REMEDIATED (2026-07-13) Plaintext HTTP — bearer token rides the LAN cleartext; a sniffer unlocks H1 + all authed reads. The single highest-leverage fix. DONE (available, not forced on dev): SIMS_TLS_CERT+SIMS_TLS_KEY ⇒ HTTPS (https.createServer); unset ⇒ HTTP dev. Dev-cert one-liner + "prod MUST supply certs" in server.ts. Prod deploy must set the certs.
M14 REMEDIATED (2026-07-13) No helmet / security headers; X-Powered-By: Express banner. DONE: helmet on (SPA-compatible CSP; nosniff; Referrer-Policy: no-referrer) + explicit X-Frame-Options: DENY + frame-ancestors 'none'; x-powered-by already disabled (P0). Both apps load with zero CSP violations.
M15 NOTED (2026-07-13) Service-worker cache-first over HTTP → MITM-injected bundle cached persistently (THEORETICAL; needs LAN MITM). Closed once prod is HTTPS (M13). No dev-slice code change.

LOW

  • Bearer tokens + purchase-entry draft in web storage (readable by same-origin JS; not exploitable today since XSS is refuted). Move session to an httpOnly cookie eventually.

Root cause & remediation shape

Almost every Critical/High traces to one architectural fact: the store-server was built as a trusted-client data layer (correct while the only client was our own verified React app in the browser) but it exposes a REST API that trusts the client for identity, business data, and input shape. The fix is one coherent theme, not 30 scattered patches:

Move the trust boundary into the server.

Priority Workstream Closes
P0 DONE Anchor every bill line to the item master (price, tax class, MRP, existence, unit) inside commitBill; a deviation requires a redeemed, bound override token C2, C3, H4, H5, H7, M8
P0 DONE Server-side authorization: per-route ActionCode middleware, deny-by-default C1, C4
P0 DONE Input validation layer (hand-rolled guards, no new deps) + generic error handler; clamp limit/offset; validate businessDate; cap lines H6, M6, M7, M8, M12
P1 DONE Lock down the edges: /api/print allowlist, /bootstrap minimize, /auth/verify-pin behind auth + rate limit, session TTL/logout, tenant-scope store/counter (M5 closed with the P2 pass) H1, M1, M2, M4, M5
P1 DONE TLS on the store LAN (available via env; prod must supply certs) M13, M15, amplifies all
P2 DONE (except H3) Harden-before-cloud (D14): PIN pepper, audit hash chain, demo-seed gate, 4-digit-FY series, tenant-scope commit — plus offline-cache PII (M11) + helmet (M14) already closed H2, M3, M5, M9, M10, M11, M14
P2 DONE (2026-07-13) Encryption at rest — full-DB SQLCipher via better-sqlite3-multiple-ciphers, keyed from a machine-scoped SIMS_DB_KEY; prod must supply the key (DPAPI/keychain) + tighten data-dir ACLs H3

Reviewer note on test artifacts

The PoCs left benign junk in the dev DB (test bills 000017000023, two garbage-FY bills, a corrupted Carry Bag stock balance, a self-clearing u4 lockout). Reset the dev DB (delete apps/store-server/data/, restart) before any demo. No source was modified.