Security: ReadOnly role can read customer PII + sales financials via ungated /customers and /sales reads #127

Summary

The /api/v1/customers and /api/v1/sales read endpoints are reachable by the ReadOnly role, exposing full customer PII and complete order financials. This contradicts the documented role model (ReadOnly = stock/production/dashboards only) and is inconsistent with the sibling money reads (/customers/balances, /payments), which are correctly gated to SalesAccess.

Severity: Medium — authenticated, same-tenant, read-only; reachable by an intentionally low-trust provisioned account (e.g. external bookkeeper/auditor).

Affected endpoints

Route groups mounted with bare .RequireAuthorization() — default policy admits any non-Denied principal, including ReadOnly (AuthPolicies.cs:34-37):

• GET /api/v1/customers — ListCustomers, web... CustomerEndpoints.cs:21
• GET /api/v1/customers/{id} — GetCustomer, CustomerEndpoints.cs:25
• GET /api/v1/sales — ListSalesOrders, SaleEndpoints.cs:63
• GET /api/v1/sales/{id} — GetSalesOrder, SaleEndpoints.cs:59

Group mounting: Program.cs:411-421.

Exposed data

• CustomerResponse (CustomerEndpoints.cs:82-83): Name, Phone, Email, Address, Note — customer PII.
• SalesOrderResponse (SaleEndpoints.cs:289-309): TotalMinorUnits + per-line UnitPriceMinorUnits — order financials.
• List endpoints page up to 500 rows per call.

Why this is a real gap (not a hardening opinion)

The repo defines an explicit role contract in three places and enforces it on the derived money views but not the underlying data:

• AuthPolicies.cs:15 — "ReadOnly → reads only: stock, production reports, dashboards"
• specs/product/specs.md:464 — "Read-only | View dashboards/reports only"
• specs/product/GLOSSARY.md:307-310 — "Read-only sees stock, history, and reports only … The SPA hides gated controls; the API returns 403 with a problem body regardless" ← false for these four reads.
• PaymentEndpoints.cs:21,51 — ListOrderPayments / ListCustomerBalances require SalesAccess; RoleMatrixTests.cs:202 asserts ReadOnly gets 403 on /customers/balances.
• SPA hides Customers/Sales nav from ReadOnly (web/src/routes/AppLayout.tsx:32-33) with a comment claiming the API enforces regardless.

Bare-RequireAuthorization() groups are described in AuthPolicies.cs:29 as "the open production READS" — customers/sales are not production reads; the pattern leaked past its intent.

Exploit scenario

1. Owner provisions a ReadOnly user for a low-trust reviewer, expecting stock/production visibility only.
2. That user, with their normal token, calls GET /api/v1/customers?limit=500 and GET /api/v1/sales?limit=500.
3. They enumerate the tenant's entire customer contact list (names, phones, emails, addresses) and every order's totals and line pricing — data explicitly outside their authorized scope.

Exposure is tenant-scoped (TenantContext), so no cross-tenant leak; the violation is the ReadOnly tier within a tenant.

Fix

Apply .RequireAuthorization(AuthPolicies.SalesAccess) to the customer and sales-order read endpoints (list + get), matching the gating on balances/payments:

• If Workers must retain these reads for the sell flow, use SalesFlow instead (still excludes ReadOnly).
• Extend RoleMatrixTests to assert ReadOnly receives 403 on GET /api/v1/customers and GET /api/v1/sales.

Docs to sync (per repo rule)

If the intended model changes, update GLOSSARY.md and the SPA Help page (#71) in the same PR.

Filed from a whole-repo security review of main (78d25b7).