# Security Review

## Implemented

- **Authentication**: JWT access tokens (short-lived, held in memory on the
  client — never localStorage) + bcrypt-hashed passwords + an HTTP-only,
  `SameSite=lax` refresh cookie (`backend/src/controllers/authController.js`).
- **Authorization** — every protected route runs the 4-step check from spec
  §77, in this order:
  1. `middleware/auth.js` — valid JWT required.
  2. `middleware/authorize.js#requireCompany` — resolved `company_id` must
     match the caller's own company (Super Admin bypasses).
  3. `middleware/authorize.js#requireOutletAccess` — resolved `outlet_id`
     must be one of the user's assigned outlets.
  4. `middleware/authorize.js#requirePermission('slug')` — role must carry
     the specific permission.
  Verified live: a Cashier gets `403` on `/api/users` (missing permission)
  and `403` on `/api/orders?outlet_id=<foreign>` (outlet isolation).
- **Password hashing**: bcrypt, cost factor 10, never logged or returned by
  the API (`User` model's `defaultScope` excludes `password`/
  `refresh_token_hash`; a dedicated `withPassword` scope is used only
  internally by the auth controller).
- **Rate limiting**: global `express-rate-limit` on `/api/*`, plus a tighter
  limiter on `POST /api/auth/login`.
- **Input validation**: `express-validator` on auth routes; all other
  mutating endpoints validate via Sequelize model constraints
  (`allowNull`, `ENUM`, foreign keys) and explicit `422` checks in
  controllers for cross-field business rules.
- **SQL injection**: 100% Sequelize parameterized queries — no raw string
  concatenation into SQL anywhere in the codebase (the one place a full SQL
  file is executed is the `db:setup` script importing the trusted local
  `schema.sql`/`seed.sql`, not user input).
- **XSS**: React escapes all rendered content by default; no
  `dangerouslySetInnerHTML` anywhere in the frontend.
- **File upload safety**: Multer whitelists MIME type + extension, generates
  a random filename (never trusts the client-supplied name), enforces a size
  limit, and Sharp re-encodes every uploaded image (stripping embedded
  scripts/metadata) before it's persisted.
- **Guest document / media privacy**: uploaded files are **not** served via
  a public static directory. `GET /api/media/file/:folder/:filename` requires
  authentication + a permission check, and path traversal is blocked by
  verifying the resolved path stays under `backend/uploads/`.
- **Security headers**: Helmet applied globally in `app.js`.
- **CORS**: locked to `FRONTEND_URL`, credentials enabled only for that
  origin.
- **No stack traces in production**: `middleware/errorHandler.js` returns a
  generic "Internal server error" message for `500`s when
  `NODE_ENV=production`; full detail is logged server-side via Winston only.
- **Audit trail**: `middleware/auditLogger.js` records user/action/module/
  record id/IP/user-agent for every mutating request (`audit_logs` table);
  the financial flows (payments, refunds, checkout) additionally record
  domain-specific rows (`payments`, `refunds`, `hotel_folio_items`).
- **Financial integrity**: money is `DECIMAL(12,2)` everywhere in the schema;
  server-side arithmetic goes through `utils/money.js`, which converts to
  integer paise before summing/multiplying to avoid IEEE-754 drift, and all
  multi-step financial writes (order creation, KOT + stock deduction,
  payments, refunds, purchase receipt, stock transfer, check-in/checkout)
  run inside a Sequelize `transaction`.

## Checklist (spec §164)

| Item | Status |
|---|---|
| No hardcoded passwords | ✅ seed users share a documented demo password (`Admin@123`), clearly for local dev only; `create-admin` always prompts |
| No exposed secrets | ✅ `.env` files git-ignored; `.env.example` only |
| No SQL injection | ✅ Sequelize parameterized queries throughout |
| No unrestricted guest documents | ✅ authenticated `/api/media/file` route only |
| No unrestricted admin endpoints | ✅ `requirePermission` on every mutating route |
| No missing authorization | ✅ global 4-step chain in `routes/index.js` |
| No public access to private hotel data | ✅ all hotel routes behind auth+outlet scoping |
| No unsafe file upload | ✅ Multer MIME/ext/size validation + Sharp re-encode |
| No stack traces in production | ✅ generic message when `NODE_ENV=production` |

## Recommended before a real production launch

- Rotate `JWT_SECRET`/`JWT_REFRESH_SECRET` per environment; never reuse the
  `.env.example` placeholders.
- Put the API behind HTTPS (see `docs/DEPLOYMENT.md` for Certbot setup) —
  the refresh cookie should be marked `secure` in production, which the code
  already does based on `NODE_ENV`.
- Add automated security tests (e.g. a Jest+Supertest suite asserting the
  RBAC/outlet-isolation behavior demonstrated manually during this build).
- Consider adding CSRF protection if the app is ever embedded in a context
  where the SameSite=lax cookie isn't sufficient (e.g. cross-site iframes).