ERP-core/docs/02-SECURITY.md

02 · SECURITY — Phase 1 Review Aid (Accepted Risks + Per-Feature Checklist)

What this is: a review aid, not a policy manual. Tick items during development and review. Before marking any feature [x] in Backend/PROGRESS.md, run its checklist here (Part C) plus the foundational controls (Part B). Navigation: you arrived from 00-CORE.md. Business rules are in 10-BACKEND-PHASE1.md, the API contract in 11-BACKEND-PHASE1.md, the frontend validation posture in 20-FRONTEND.md. Context: Phase 1 is single-tenant with RBAC deferred and approvals auto-on. Several exposures below are deliberate, accepted decisions — they are recorded in Part A so they are explicit, not implicit.

---

Part A — Accepted Risks Register

These are known, deliberately-accepted Phase-1 exposures. Each has a compensating control and a trigger to revisit. Do not "fix" them ad-hoc during Phase 1 — they are tracked here and closed in Part D order.

ID	Risk	Why accepted	Compensating control	Revisit trigger
AR-01	No authorization — any authenticated user can call any endpoint (post adjustments, confirm GRN, cancel PO, write off stock).	RBAC deferred (FR-X-01), Phase-1 scope.	Authentication required; immutable audit trail attributes every action.	First post-Phase-1 hardening → enable RBAC.
AR-02	Approvals auto-on — no value gate on PO or adjustment.	Phase-1 decision (FR-PROC-04, FR-STK-07).	Audit + mandatory reason codes.	Enable adjustment approval first, then PO approval.
AR-03	IDOR — sequential IDs + no RBAC → a user can access another's document by guessing the ID.	Consequence of AR-01.	Audit trail.	Closed with RBAC.
AR-04	GRN-without-PO — receive (and value) goods with no order; a fraud vector.	Emergency/direct receipt is useful (FR-GRN-02).	Permission-gated (when RBAC on); flagged for review; audit; cost entered here gets extra scrutiny (see C.3).	With RBAC / policy on direct receipts.
AR-05	In-transit loss window — dispatched-but-not-received stock is untracked shrinkage risk.	Inherent to in-transit transfers (FR-STK-05).	In-transit aging monitoring report.	Add stuck-transfer alert (Part D).
AR-06	Localhost dev secrets in appsettings.Development.json.	Local-dev convenience, current phase.	.gitignore + localhost only.	Before any shared/staging/prod → User Secrets / env vars; rotate.
AR-07	getUserDetails / LogoutUser callable without a bearer token — GET /api/v1/auth/users/{userId} and POST /api/v1/auth/logout resolve the target user from the URL/payload, not the caller's session, so any anonymous caller can fetch a profile or log out an arbitrary user's sessions by GUID.	Carried over verbatim from AuthHex's own dispatcher contract (API_REFERENCE.md §3) — ERPCore's AuthController proxies it as-is rather than silently tightening a contract it doesn't own.	GUIDs are not enumerable; every call is written to AuthEventLogs upstream in AuthHex.	Revisit once AuthHex exposes a token-scoped variant, or add ERPCore-side rate limiting / auth requirement ahead of AuthHex.
AR-08	No rate limiting on AuthController's anonymous endpoints (login, register, refresh, recovery, OTP send/verify) — brute-force and account-enumeration exposure.	Not built in this pass (docs/11 §2.0, added 2026-07-16); AuthHex may rate-limit server-side but ERPCore does not add its own layer yet.	AuthHex's own lockout/backoff (per docs/10 NFR-03), immutable audit trail.	Add ASP.NET Core rate limiting middleware to AuthController before any non-local deployment.
AR-09	HRM inherits AR-01 for salary/PII data — any door-admitted authenticated user can currently view/download any employee's salary figures, payslip, or uploaded documents (NIC scans, contracts). This is a explicit, flagged decision, not a silent inheritance: salary/PII is categorically more sensitive than Phase-1 inventory data, and this was called out to the business owner before HRM build started (see 12-BACKEND-HRM.md A.1).	RBAC still deferred repo-wide; a coarse HR-role door-gate was not made a blocking prerequisite for HRM go-live.	Immutable audit trail (as AR-01); sidebar-visibility hiding of Employees/Attendance/Payroll sections for non-HR roles via the existing NavItem/RolePermission mechanism (UI-level only, not a server-enforced gate).	HRM should be the forcing function that enables per-endpoint RBAC ahead of the rest of the system (see Part D) — salary-data exposure is a materially worse blast radius than inventory data.
AR-10	AuditLogsController exposes salary figures — a PayrollLine/EmployeeSalaryStructure mutation's ChangeSet contains salary amounts; AuditLogsController is not RBAC-gated, so any authenticated user can read another employee's salary history via GET /audit-logs?entityType=PayrollLine&entityId=X.	Consequence of AR-01/AR-09, specific enough to name on its own rather than leaving implicit.	None beyond authentication today.	Closed with RBAC (Part D), or an interim HRM-specific audit-log access filter.

---

Part B — Foundational Controls (cross-cutting — apply everywhere)

B.1 Authentication & JWT

• Strong signing key (≥ 256-bit) from env/secrets; the dev CHANGE_ME key never reaches non-local environments
• Algorithm pinned (HS256 or RS256); reject alg: none and algorithm-confusion
• Validate issuer, audience, lifetime, and signature on every request
• Short access-token lifetime; define refresh strategy if needed
• Passwords hashed with a strong KDF (ASP.NET Identity / PBKDF2 / bcrypt / argon2) — never plaintext/reversible
• /auth/login rate-limited + backoff/lockout (brute-force)
• Generic auth-failure messages (no account-enumeration signal)

B.2 Token storage & CSRF (httpOnly-cookie decision)

• Token in an httpOnly, Secure cookie (never localStorage) — removes XSS token theft (AuthCookieWriter.WriteSession, erp_at/erp_rt, 2026-07-16)
• SameSite=Strict on the auth cookie (AuthCookieWriter; assumes frontend + ERPCore share a registrable domain — revisit if deployed cross-domain)
• CSRF protection on every state-changing request (double-submit XSRF-TOKEN cookie + X-XSRF-TOKEN header, ValidateCsrfAttribute, applied to every mutating AuthController action; Bearer-header callers exempt since they aren't cookie-driven)
• CORS locked to the known frontend origin(s); credentials mode aligned with the cookie — not yet configured; required before any browser frontend can call these endpoints cross-origin (tracked with the frontend-wiring follow-up)
• Cookie scoped minimally (path/domain), Secure flag on (erp_rt scoped to /api/v1/auth/refresh-token; all three cookies Secure=true)

B.3 Audit integrity (this is the compensating control for AR-01 — it must hold)

• Audit log and stock ledger are append-only at the DB level (the app's DB role has no UPDATE/DELETE on those tables)
• Every mutation records who / when / old→new / reason (FR-X-02)
• No API path edits or deletes a posted transaction (FR-X-05); corrections are reversing entries
• Actor derived from token sub, never from the request body

B.4 Transport & headers

• HTTPS only; HSTS in production
• Security headers: CSP, X-Content-Type-Options: nosniff, frame-ancestors/X-Frame-Options, Referrer-Policy

B.5 Error & logging hygiene

• ProblemDetails.detail leaks no stack traces / connection strings / internals in production
• Serilog never logs tokens, passwords, cookies, or secret-bearing request bodies
• Correlation/trace id present for support without exposing sensitive data

B.6 Input & injection

• No raw or string-concatenated SQL; EF Core parameterizes — keep it that way
• DTOs are narrow — no mass-assignment/over-posting; client cannot set status, ids, createdBy, timestamps, or computed fields
• Server-side validation is authoritative (mirrors 20-FRONTEND §3); client validation is UX only
• Pagination bounds enforced (pageSize max) to prevent resource exhaustion

B.7 Concurrency as integrity

• FIFO layer consumption is row-locked inside the UoW transaction (no double-spend)
• ETag/If-Match on mutable resources; 412 on stale writes

---

Part C — Per-Feature Checklists

C.1 Master Data (Item / UOM / Category / Vendor / Warehouse / Bin)

• Create/update DTOs exclude server-controlled fields (status, ids, timestamps)
• Deactivate — not delete — referenced masters (FR-MD-08); hard delete blocked → MASTER_IN_USE
• Nested/reference writes validate the target exists and is active
• Item.salePrice is a legitimately client-supplied field (a deliberate exception to B.6's over-posting list) — validated >= 0 server-side, nullable. It is Sales-only (never enters GRN/FIFO/ledger), so unlike GRN unitCost it has no inventory-value or costing impact; the fixed/stock-value choice is frontend UX (docs/11 §2.1).

C.2 Procurement (Requisition / RFQ / PO / Purchase Return)

• PO totals computed server-side from lines (never trust client totals)
• PO edit-while-open: every change audit-logged old→new (this is the tampering surface)
• PO cancel blocked if any receipt exists
• Purchase-return quantity validated against received/available (no over-return)
• Note in review: AR-01/AR-02/AR-03 apply to these endpoints

C.3 GRN

• unitCost defaults to the PO line price; a per-line override is now permitted (decision revised 2026-07-20 — was "locked, client cost ignored"). When an override is entered it is used, and the PO price is snapshotted (poUnitPrice) so a priceVariance is recorded against it for review. Rationale: one PO legitimately spans batches received at different prices; the variance trail (plus the audit log) is the compensating control that replaces the old hard block.
• Derived figures stay server-computed — netUnitCost/receivedValue/vatAmount/lineTotal are never accepted from the client, so the client cannot inflate stock value except by an auditable unit-cost override. Discount reduces inventory cost; VAT is recoverable and never enters stock value.
• Direct GRN (no PO) remains the higher-scrutiny path where cost is entered with no PO to compare against → review flag + audit (AR-04)
• Off-PO lines on a PO-based GRN (poLineId: null, 2026-07-22) are the same exposure class as AR-04 — cost is entered with no PO line to compare against, and OVER_RECEIPT_TOLERANCE does not apply to them. Treat them with the direct-receipt scrutiny (review flag + audit); they do not touch PO balances.
• Over-receipt tolerance enforced server-side → OVER_RECEIPT_TOLERANCE (PO-linked lines only; off-PO lines have no PO qty to check)
• On-hold stock is not issuable (FR-WH-07); expired batch blocked

C.4 Stock Core (FIFO / Ledger)

• FIFO consumption row-locked (B.7) — integrity + no double-spend
• Negative-stock block enforced server-side → STOCK_NEGATIVE_BLOCKED
• Ledger append-only (B.3)
• Valuation computed from layers server-side; never from client input

C.5 Adjustments — HIGHEST-RISK FEATURE IN THE PHASE

Auto-post + no approval + direct write-off = the primary theft/fraud surface. Reason code + user stamp are the only live controls.

• Reason code mandatory, server-enforced → REASON_CODE_REQUIRED
• User stamp mandatory (from token)
• Decrease cannot drive available negative → STOCK_NEGATIVE_BLOCKED
• qtyDelta sanity bounds; large write-offs surface on a review report
• Review note: AR-02 — re-enabling adjustment approval is the first control to turn on post-Phase-1 (Part D)

C.6 Transfers

• Dispatch validates available ≥ qty → block otherwise
• Cost-preserving (dest inherits source cost) — no revaluation via transfer
• destWarehouseId != srcWarehouseId
• In-transit aging monitored (AR-05: stuck/never-received transfers)

C.7 Counts

• Variance posting is an adjustment in disguise → apply C.5 controls
• System-qty snapshot immutable once the count is opened
• Large variances flagged for review

C.8 HRM (Employee / Documents / Attendance / Leave / Payroll) — salary/PII data, see AR-09/AR-10

• Create/update DTOs exclude server-controlled fields (status, ids, createdBy, timestamps, computed payroll amounts)
• Employee is never hard-deleted (deactivate via EmployeeStatus only), matching FR-MD-08's deactivate-not-delete pattern
• Employee.UserId uniqueness (one User per Employee) enforced at the DB level (filtered unique index), not just service-level
• File uploads (EmployeeDocument, attendance spreadsheets): extension allowlist + content-type cross-check + size cap enforced server-side (client checks are UX only); magic-byte sniffing / antivirus scanning explicitly deferred, not silently skipped — treat as an extension of AR-08's "not built in this pass" posture
• EmployeeDocument download is never served via a static/guessable URL — authenticated controller action streaming through IFileStorageService only
• Attendance batch lock (Confirmed/UsedInPayroll) genuinely blocks record edits server-side (ATTENDANCE_BATCH_LOCKED), not just hidden in the UI
• Payroll figures (Gross/Net/Tax/EPF/ETF) are computed server-side only; the client never supplies or overrides them
• Payroll Unlock is the highest-risk action in this module (parallel to C.5's framing of Adjustments) — mandatory reason, heavy audit; treat as the first HRM candidate for real per-endpoint RBAC
• Untrusted spreadsheet parsing (ClosedXML/CsvHelper): packages pinned to current versions, no macro/external-entity execution path enabled
• Review note: AR-01/AR-09/AR-10 apply to every HRM endpoint until RBAC lands

---

Part D — Post-Phase-1 controls to enable (in order)

1. Adjustment approval (config flag already reserved) — closes the top fraud surface (AR-02, C.5).
2. RBAC enforcement (role→permission) — closes AR-01, AR-03, AR-04, and AR-09/AR-10 (HRM salary/PII). Given HRM's materially worse blast radius, consider bringing this forward ahead of item 3 once HRM ships (see AR-09's revisit trigger).
3. PO approval (value thresholds) — closes remaining AR-02.
4. Monitoring reports — stuck-transfer aging (AR-05) and large-variance/write-off review.

---

Part E — Locked security-relevant decisions

• GRN unitCost from the PO line (server-derived; not user-entered). Direct GRN is the audited exception.
• Frontend token in an httpOnly Secure cookie + CSRF protection (not localStorage).

---

End of 02-SECURITY.md. Run the relevant checklist before ticking a feature in Backend/PROGRESS.md.