# Security Hardening Plan

This document records findings from a full-codebase security audit (Laravel 12 CRM API), prioritized by
severity. It is a **planning reference, not a changelog** — items are not yet fixed unless noted. When an
item is actioned, move it out of this doc and into the relevant `docs/` file per the maintenance table in
`CLAUDE.md`, or simply delete the entry here.

Severity follows OWASP ASVS-style reasoning: Critical = exploitable today with no auth barrier; High =
exploitable under realistic conditions or a systemic gap with no defense-in-depth; noted items are real
but intentionally out of scope for now.

---

## Implemented

The 1 Critical and 4 High items below have been fixed:

1. **Unauthenticated backfill routes.** Moved `/task/assignees/backfill` and
   `/sales/payment-status/backfill` into the `auth:sanctum` group in `routes/app.php`, changed GET to POST,
   and added `task_management.task.override` / `report_management.sales.override` permission checks inside
   `TaskController::backfill_task_assignees()` and `LeadSaleController::backfill_sale_payment_status()`.
2. **No throttle middleware.** Registered `api` (60/min, by user id or IP) and `register` (6/min by IP)
   named limiters in `AppServiceProvider::boot()`, applied `$middleware->throttleApi()` in
   `bootstrap/app.php`, and wrapped `/register`, `/register/direct`, `/register/verify`,
   `/app-version/create-direct`, `/login` in a `throttle:register` group in `routes/api.php`.
3. **Sanctum tokens never expire.** `config/sanctum.php` now reads
   `env('SANCTUM_TOKEN_EXPIRATION', 43200)` (30 days) instead of `null`; documented in `.env.example`.
4. **No catch-all exception renderer.** Added a `\Throwable` render callback in `bootstrap/app.php` that
   returns a generic sanitized JSON 500 for API requests when `app.debug` is off, while letting
   `ValidationException` and `HttpExceptionInterface` (404/403/405/etc.) fall through to Laravel's normal
   handling unchanged.
5. **`User` model missing `$hidden`.** Added `protected $hidden = ['otp', 'remember_token'];` to
   `app/Models/User.php` as a defense-in-depth backstop.

---

## Noted, not in scope

The following are real findings from the audit but are **explicitly not being actioned right now** per
product decision — recorded here for awareness only, no fix recommendation attached.

- **`permission` route middleware unused.** `CheckPermission` is aliased in `bootstrap/app.php:22` but
  never referenced as `'permission:...'` in any route file; all authorization is done via manual
  per-controller `$user->can(...)` calls (656+ occurrences). This is an intentional pattern the team is
  keeping — per-controller checks stay as-is.
- **IDOR risk on leads.** `LeadController.php` looks up `Lead::find($request->id / ->lead_id)` at ~15 call
  sites (lines 868, 1011, 1109, 1202, 1450, 1509, 1543, 1608, 2219, 2375, 2441, 2487, 2700, 2778, 2848)
  with only a permission-name check, never verifying the lead's `account_id`/`company_id` matches the
  actor's tenant. No global Eloquent scope enforces tenant isolation anywhere in the codebase.
- **Activity logging installed but unused.** `spatie/laravel-activitylog` has a read endpoint
  (`ActivityLogController`), but no model uses the `LogsActivity` trait and nothing calls
  `activity()->log()` — sensitive actions (user creation, role/permission changes, deletions) aren't
  actually being audited.
- **Inline validation instead of Form Requests.** Only 1 Form Request class exists across 63 controllers;
  most validation is inline `$request->validate()`. `LeadGroupController.php:42,157`,
  `CampaignController.php:50,101`, `RcsTemplateController.php:100` pass raw `$request->all()` into
  service/create layers without controller-level validation first.
- **`maatwebsite/excel` pinned to a dev branch.** `composer.json` has `"maatwebsite/excel":
  "4.x-dev as 4.0.0"` — not a tagged release.
- **Raw SQL string interpolation pattern (currently safe).** `MeetingController.php:777,792` interpolates
  server-generated `Carbon`/`DateHelper` values directly into `selectRaw`/`whereRaw` strings instead of
  using `?` bindings. Not exploitable today since the values aren't user input, but a risky pattern.
- **TLS verification disabled on outbound SMS call.** `ClientAuthController` uses
  `Http::withoutVerifying()` for its SMS API request.
- **Queue payloads stored unencrypted.** `QUEUE_CONNECTION=database` by default — any future queued job
  carrying OTP/PII would sit in plaintext in the `jobs`/`failed_jobs` tables. Currently fine since OTP send
  is synchronous, not queued.
- **Public webhook endpoints with no signature verification.** `routes/app.php:71-76`
  (Facebook/Instagram/IVR/JustDial/website lead-capture webhooks) are public with no signature check.
- **Full request payload logged on webhook errors.** `PhoneCallLogController.php` logs `$request->all()`
  on error paths — worth spot-checking no PII/secrets pass through.
