> **I'm using the writing-plans skill to create the implementation plan.**

# Security Hardening Implementation Plan (Unguard + No Fillable)

> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.

**Goal:** Close all security findings from the 2026-07-04 audit while keeping global `Model::unguard()` and removing all `$fillable`/`$guarded` declarations. Compensate for disabled mass-assignment protection by whitelisting request fields with `$request->safe()->only([...])` before every Eloquent `create()`/`update()`/`fill()` call.

**Architecture:** Centralize security controls (rate limiting, account-status checks, LIKE escaping, upload rules, log redaction) and make request whitelisting the default pattern. No model-level mass-assignment guards.

**Tech Stack:** Laravel 13, PHP 8.4, Spatie Permission, Spatie Media Library, OwenIt Auditing, Sanctum, Pest.

**Worktree:** `C:/Users/hamma/Herd/turista/.worktrees/security-fixes` on branch `security-fixes`.

---

## Task 0: Establish clean baseline

**Files:**
- Worktree root

- [ ] **Step 1: Ensure dependencies and env are present**

`vendor/` and `node_modules/` are junctioned from the main worktree; `.env` is copied. Verify with:

```bash
export PATH="/c/Users/hamma/.config/herd/bin/php84:/c/Users/hamma/.config/herd/bin:$PATH"
php artisan --version
```

Expected: Laravel Framework 13.x

- [ ] **Step 2: Run the test suite**

```bash
php artisan test
```

Expected: PASS (note current failures if any and report them before proceeding).

- [ ] **Step 3: Commit baseline state if needed**

No changes yet.

---

## Task 1: Remove `$fillable`/`$guarded` from all models and confirm `Model::unguard()`

**Files:**
- Modify: `app/Providers/AppServiceProvider.php` (confirm/keep `Model::unguard();`)
- Modify: every file in `app/Models/` that declares `$fillable` or `$guarded`

- [ ] **Step 1: Confirm `Model::unguard()` in `AppServiceProvider::boot()`**

Ensure line ~79 contains:

```php
Model::unguard();
```

- [ ] **Step 2: Remove all `$fillable` and `$guarded` declarations from models**

Search:

```bash
grep -R "protected \$fillable\|protected array \$fillable\|protected \$guarded" app/Models
```

Delete those property declarations. Do NOT add `$guarded = []` as a replacement; the model should have no mass-assignment declarations at all.

- [ ] **Step 3: Run the test suite**

```bash
php artisan test
```

Expected: PASS (this step only removes declarations; tests should still pass because `unguard()` is active).

- [ ] **Step 4: Commit**

```bash
git add app/Models app/Providers/AppServiceProvider.php
git commit -m "security: remove fillable/guarded declarations; rely on global unguard"
```

---

## Task 2: Harden authentication responses and fix credential oracle

**Files:**
- Modify: `app/Http/Controllers/Api/AuthController.php`
- Modify: `app/Http/Requests/LoginRequest.php` if needed
- Test: `tests/Feature/Auth/LoginTest.php`, `tests/Feature/Auth/VerificationTest.php`, `tests/Feature/PendingCustomerFlowTest.php`, `tests/Feature/WalletPaymentRefundTest.php`

- [ ] **Step 1: Return generic 401 for all login failures**

In `AuthController::login`, ensure the same response is returned for:

- user not found
- password mismatch
- user not verified
- user not active/approved (if checked here)

Use exactly:

```php
return response()->json(['message' => 'Invalid credentials.'], 401);
```

Do NOT return `account_unverified`, the phone number, or any other account state.

- [ ] **Step 2: Remove OTP from all JSON responses**

Find helper methods such as `otpResponse()` and any branch that returns `otp` based on `app()->environment('local', 'testing')`. Remove the `$otp` parameter and the env branch. Return only `message` and optional `data`.

In `ReservationController`, remove any `otp` field from on-arrival responses.

- [ ] **Step 3: Fix email-only OTP verification**

In `AuthController::verifyAccount`, resolve the contact key using the user's stored `whatsapp_number` (via `AuthService::contactKeyForUser($user)` or equivalent) even when the request was submitted by email. Do not short-circuit when `whatsapp_number` is null.

- [ ] **Step 4: Fix password-reset user enumeration**

In `AuthController::resetPassword`, replace `firstOrFail()` with a pattern that does the same work for existing users but returns a generic response for missing users. Example:

```php
$user = User::where('whatsapp_number', $whatsappNumber)->first();

if (! $user) {
    return response()->json(['message' => 'Invalid or expired reset code.'], 400);
}
```

- [ ] **Step 5: Update tests**

Change tests that expect `account_unverified` or OTP in JSON to expect generic `401 Invalid credentials.` and to read OTP from cache via `OtpService::cacheKey()` for test assertions.

- [ ] **Step 6: Run targeted tests**

```bash
php artisan test tests/Feature/Auth/LoginTest.php tests/Feature/Auth/VerificationTest.php tests/Feature/PendingCustomerFlowTest.php tests/Feature/WalletPaymentRefundTest.php tests/Feature/Auth/ForgotResetPasswordTest.php
```

Expected: PASS

- [ ] **Step 7: Commit**

```bash
git add app/Http/Controllers/Api/AuthController.php app/Http/Controllers/ReservationController.php tests/Feature/Auth tests/Feature/PendingCustomerFlowTest.php tests/Feature/WalletPaymentRefundTest.php
git commit -m "security: generic auth responses, remove OTP leaks, fix email verify"
```

---

## Task 3: Enforce active/approved account status in policies and controllers

**Files:**
- Modify: `app/Policies/BuildingPolicy.php`, `UnitPolicy.php`, `ReservationPolicy.php`, `EmployeePolicy.php`, `PromoCodePolicy.php`, `DiscountTemplatePolicy.php`, `InvoicePolicy.php`, `TransactionPolicy.php`, `ReceiptPolicy.php`, `OccasionPricePolicy.php`, `UnitAvailabilityPolicy.php`
- Modify: `app/Http/Controllers/BuildingController.php`, `UnitController.php`, `InvoiceController.php`, `TransactionController.php`, `CalendarController.php`, `ReservationController.php`
- Test: add tests in `tests/Feature/Auth/` and relevant feature tests

- [ ] **Step 1: Create or reuse account-status helper methods**

Ensure these methods exist and work on `User`:

```php
public function isApprovedOwner(): bool
public function isActiveEmployee(): bool
```

- [ ] **Step 2: Add a global middleware to block inactive accounts**

Create `app/Http/Middleware/EnsureAccountActive.php`:

```php
<?php

namespace App\Http\Middleware;

use Closure;
use Illuminate\Http\Request;
use Symfony\Component\HttpFoundation\Response;

class EnsureAccountActive
{
    public function handle(Request $request, Closure $next): Response
    {
        $user = $request->user();

        if (! $user) {
            return $next($request);
        }

        if ($user->hasRole('owner') && ! $user->isApprovedOwner()) {
            return response()->json(['message' => 'Account is not approved.'], 403);
        }

        if ($user->hasRole('employee') && ! $user->isActiveEmployee()) {
            return response()->json(['message' => 'Account is suspended.'], 403);
        }

        return $next($request);
    }
}
```

Register it in `bootstrap/app.php` inside `withMiddleware` so it runs **after** authentication (e.g., `appendToGroup('api', EnsureAccountActive::class)` or equivalent for the route groups).

- [ ] **Step 3: Harden policies with null-safe ownership checks**

In each policy, add active-status checks and use null-safe navigation. Example pattern for `view`:

```php
public function view(User $user, Building $building): bool
{
    if (! $user->isApprovedOwner() && ! $user->isActiveEmployee()) {
        return false;
    }

    return $building->owner_id === ($user->owner?->id ?? $user->employee?->owner_id);
}
```

For `ReceiptPolicy::ownsReceipt` and `OccasionPricePolicy::ownsUnit`, replace chained `->` with null-safe `?->`:

```php
$ownerId = $receipt->reservation?->unit?->building?->owner_id;
```

- [ ] **Step 4: Harden controller scopes**

In `ReservationController::index`, replace:

```php
$ownerId = ($user->owner ?? $user->employee->owner)->id;
```

with:

```php
$ownerId = ($user->owner ?? $user->employee?->owner)?->id;

if (! $ownerId) {
    abort(403, 'Invalid account scope.');
}
```

Apply the same null-safe pattern anywhere `$user->employee->owner` is accessed.

- [ ] **Step 5: Add regression tests**

Add tests that:
- create a suspended employee and assert `403` on building/unit/reservation endpoints
- create a pending owner and assert `403`
- verify a user with the `employee` role but no `employees` row gets `403`, not 500

- [ ] **Step 6: Run targeted tests**

```bash
php artisan test tests/Feature/Auth tests/Feature/BuildingManagementTest.php tests/Feature/UnitTest.php tests/Feature/ReservationTest.php
```

Expected: PASS

- [ ] **Step 7: Commit**

```bash
git add app/Http/Middleware app/Policies app/Http/Controllers bootstrap/app.php tests/Feature
git commit -m "security: enforce active/approved status and null-safe ownership"
```

---

## Task 4: Add request whitelisting for every Eloquent create/update under unguard

**Files:**
- Modify: every controller and service that calls `Model::create($request->...)`, `$model->update($request->...)`, or `$model->fill($request->...)`

- [ ] **Step 1: Find all raw request-to-Eloquent passes**

Search:

```bash
grep -R "::create(\$request\|->update(\$request\|->fill(\$request" app/Http app/Services
```

- [ ] **Step 2: Apply whitelisting pattern**

For FormRequest-based controllers, use:

```php
$building = Building::create($request->safe()->only([
    'name', 'description', 'address', 'country_id', 'city_id', 'region_id',
    'currency_id', 'map_lat', 'map_lng', 'booking_conditions',
]));
```

For inline validation, use:

```php
$validated = $request->validate([...]);
$model->update(Arr::only($validated, ['field_a', 'field_b']));
```

Never pass `$request->all()`, `$request->validated()`, or `$request->safe()` directly to Eloquent without an explicit field list.

- [ ] **Step 3: Audit service-layer creates/updates**

In services such as `ReservationService`, `PaymentService`, `PromoCodeService`, ensure all arrays passed to Eloquent are built from explicitly allowed fields, not from raw request arrays.

- [ ] **Step 4: Run the test suite**

```bash
php artisan test
```

Expected: PASS

- [ ] **Step 5: Commit**

```bash
git add app/Http app/Services
git commit -m "security: whitelist request fields before Eloquent create/update"
```

---

## Task 5: Escape LIKE wildcards in all search inputs

**Files:**
- Create: `app/Helpers/SearchHelper.php` (or add method to existing helper)
- Modify: `app/Services/Filter/FilterService.php`
- Modify: `app/Http/Controllers/ReservationController.php`
- Modify: `app/Http/Controllers/OwnerController.php`
- Modify: `app/Http/Controllers/Api/Owner/EmployeeController.php`
- Modify: `app/Http/Controllers/Admin/UserController.php`

- [ ] **Step 1: Create a LIKE escape helper**

Create `app/Helpers/SearchHelper.php`:

```php
<?php

namespace App\Helpers;

class SearchHelper
{
    public static function likeEscape(string $term): string
    {
        return addcslashes($term, '%_');
    }
}
```

- [ ] **Step 2: Apply escaping before every `LIKE` interpolation**

Replace patterns like:

```php
$query->where('name', 'like', "%$search%");
```

with:

```php
$search = SearchHelper::likeEscape($search);
$query->where('name', 'like', "%{$search}%");
```

Apply across `FilterService`, `ReservationController`, `OwnerController`, `EmployeeController`, `UserController`.

- [ ] **Step 3: Add regression test**

Create `tests/Unit/Helpers/SearchHelperTest.php`:

```php
<?php

use App\Helpers\SearchHelper;

it('escapes LIKE wildcards', function () {
    expect(SearchHelper::likeEscape('100%'))->toBe('100\\%')
        ->and(SearchHelper::likeEscape('_test'))->toBe('\\_test');
});
```

- [ ] **Step 4: Run targeted tests**

```bash
php artisan test tests/Unit/Helpers/SearchHelperTest.php tests/Feature/BuildingUnitFiltersTest.php tests/Feature/AdminUserTest.php
```

Expected: PASS

- [ ] **Step 5: Commit**

```bash
git add app/Helpers app/Services app/Http/Controllers tests/Unit/Helpers
git commit -m "security: escape LIKE wildcards in search inputs"
```

---

## Task 6: Strengthen file upload validation and reject compressor failures

**Files:**
- Modify: `app/Rules/PhotoFileRules.php`
- Modify: `app/Http/Requests/BuildingRequest.php`
- Modify: `app/Http/Requests/UnitRequest.php`
- Modify: `app/Http/Requests/UnitUpdateRequest.php`
- Modify: `app/Http/Requests/Owner/UploadOwnerLogoRequest.php`
- Modify: `app/Services/ImageCompressor.php`
- Modify: `app/Http/Controllers/BuildingController.php`
- Modify: `app/Http/Controllers/UnitController.php`
- Test: `tests/Feature/BuildingManagementTest.php`, `tests/Feature/UnitUpdatePhotoTest.php`

- [ ] **Step 1: Harden `PhotoFileRules::forSinglePhoto()`**

```php
public static function forSinglePhoto(string $key = 'photo'): array
{
    return [
        $key => ['required', 'image', 'mimes:jpg,jpeg,png,webp', 'max:10240'],
    ];
}
```

Remove HEIF/HEIC unless Imagick is guaranteed; if kept, ensure processing failures are rejected.

- [ ] **Step 2: Standardize MIME lists across requests**

Ensure `BuildingRequest`, `UnitRequest`, `UnitUpdateRequest`, and `UploadOwnerLogoRequest` use the same `image`, `mimes`, and `max` rules. Do not accept arbitrary MIME strings without `image` validation.

- [ ] **Step 3: Reject uploads when `ImageCompressor` fails**

In `ImageCompressor::compress()` / `handle()`, when compression fails, throw a `ValidationException` instead of returning the original `UploadedFile`:

```php
throw ValidationException::withMessages([
    $inputName => ['The image could not be processed. Please upload a valid JPG, PNG, or WebP image.'],
]);
```

- [ ] **Step 4: Add regression tests**

Add tests for:
- uploading a non-image file with image MIME rejected
- oversized image rejected
- compressor failure returns 422

- [ ] **Step 5: Run targeted tests**

```bash
php artisan test tests/Feature/BuildingManagementTest.php tests/Feature/UnitUpdatePhotoTest.php tests/Feature/BulkUnitTest.php
```

Expected: PASS

- [ ] **Step 6: Commit**

```bash
git add app/Rules app/Http/Requests app/Services app/Http/Controllers tests/Feature
git commit -m "security: harden image upload validation and compressor fallback"
```

---

## Task 7: Redact PII in OTP and WhatsApp logs

**Files:**
- Modify: `app/Services/OtpService.php`
- Modify: `app/Services/WhatsAppService.php`

- [ ] **Step 1: Create a log-redaction helper**

Create `app/Helpers/LogHelper.php`:

```php
<?php

namespace App\Helpers;

class LogHelper
{
    public static function redactPhone(string $phone): string
    {
        $digits = preg_replace('/\D/', '', $phone);

        if (strlen($digits) <= 4) {
            return str_repeat('*', strlen($digits));
        }

        return str_repeat('*', strlen($digits) - 4) . substr($digits, -4);
    }
}
```

- [ ] **Step 2: Reduce and redact OTP logs**

In `OtpService::send`, reduce to one success log and one failure log. Replace raw `original_key`/`normalized_key` with `LogHelper::redactPhone($key)`. Remove any log lines that include the actual OTP value.

- [ ] **Step 3: Redact WhatsApp service logs**

In `WhatsAppService`, redact `to` numbers and truncate/remove full response bodies. Log only correlation IDs, status codes, and error summaries.

- [ ] **Step 4: Run the test suite**

```bash
php artisan test
```

Expected: PASS

- [ ] **Step 5: Commit**

```bash
git add app/Helpers app/Services
git commit -m "security: redact phone numbers and OTPs from logs"
```

---

## Task 8: Harden session/cookie and CORS config defaults

**Files:**
- Modify: `config/session.php`
- Modify: `config/cors.php`
- Modify: `.env.example`
- Modify: `config/services.php`
- Modify: `routes/console.php`
- Modify: `config/request-docs.php`

- [ ] **Step 1: Secure session cookie defaults**

In `config/session.php`:

```php
'secure' => env('SESSION_SECURE_COOKIE', true),
'same_site' => env('SESSION_SAME_SITE', 'strict'),
'encrypt' => env('SESSION_ENCRYPT', true),
```

- [ ] **Step 2: Restrict CORS methods and headers**

In `config/cors.php`:

```php
'allowed_methods' => ['GET', 'POST', 'PUT', 'PATCH', 'DELETE'],
'allowed_headers' => ['Content-Type', 'Accept', 'Authorization', 'X-Requested-With'],
```

Keep `supports_credentials => false`.

- [ ] **Step 3: Update `.env.example`**

Set:

```dotenv
APP_DEBUG=false
LOG_LEVEL=error
SESSION_SECURE_COOKIE=true
SESSION_ENCRYPT=true
REQUEST_DOCS_ENABLED=false
```

- [ ] **Step 4: Disable CoreVerde by default**

In `config/services.php`:

```php
'enabled' => env('COREVERDE_ENABLED', false),
```

- [ ] **Step 5: Prevent scheduled command overlap**

In `routes/console.php`:

```php
Schedule::command(...)->everyFiveMinutes()->withoutOverlapping();
```

- [ ] **Step 6: Hide request-docs metadata**

In `config/request-docs.php`:

```php
'hide_sql_data' => true,
'hide_logs_data' => true,
'hide_models_data' => true,
```

- [ ] **Step 7: Run the test suite**

```bash
php artisan test
```

Expected: PASS

- [ ] **Step 8: Commit**

```bash
git add config .env.example routes/console.php
git commit -m "security: harden session, CORS, and config defaults"
```

---

## Task 9: Add rate limiting to authenticated and public routes

**Files:**
- Modify: `app/Providers/AppServiceProvider.php` (or `bootstrap/app.php` if rate limits are defined there)
- Modify: `routes/api.php`

- [ ] **Step 1: Define named rate limiters**

In `AppServiceProvider::boot()` (or route service provider):

```php
RateLimiter::for('authenticated', function (Request $request) {
    return Limit::perMinute(120)->by($request->user()?->id ?: $request->ip());
});

RateLimiter::for('public', function (Request $request) {
    return Limit::perMinute(60)->by($request->ip());
});

RateLimiter::for('sensitive', function (Request $request) {
    return Limit::perMinute(10)->by($request->user()?->id ?: $request->ip());
});
```

- [ ] **Step 2: Apply to route groups**

In `routes/api.php`:

```php
Route::middleware(['auth:api', 'verified', 'password_changed', 'throttle:authenticated'])
    ->group(function () {
        // authenticated owner/employee/admin routes
    });

Route::middleware('throttle:public')->group(function () {
    // public catalog routes
});
```

Apply `throttle:sensitive` to transaction create/refund, employee create, and bulk operations.

- [ ] **Step 3: Add tests**

Add `tests/Feature/RateLimitTest.php` with simple assertions that too many requests to `/api/v1/countries` and a protected endpoint return `429`.

- [ ] **Step 4: Run targeted tests**

```bash
php artisan test tests/Feature/RateLimitTest.php
```

Expected: PASS

- [ ] **Step 5: Commit**

```bash
git add app/Providers routes/api.php tests/Feature/RateLimitTest.php
git commit -m "security: add rate limiting to authenticated and public routes"
```

---

## Task 10: Final verification and cleanup

**Files:**
- Worktree root

- [ ] **Step 1: Run the full test suite**

```bash
php artisan test
```

Expected: PASS

- [ ] **Step 2: Run static analysis/formatting checks**

```bash
vendor/bin/pint --test
php artisan insights  # if installed
```

Expected: no style issues introduced

- [ ] **Step 3: Run composer audit**

```bash
php "C:/Users/hamma/.config/herd/bin/composer.phar" audit --format=plain
```

Expected: no critical/high vulnerabilities (fix if Guzzle or other deps have CVEs).

- [ ] **Step 4: Final review**

Dispatch a final code-review subagent to review the whole branch for security correctness, completeness against the audit findings, and consistency with the unguard/no-fillable constraint.

- [ ] **Step 5: Merge or hand off**

Use superpowers:finishing-a-development-branch to decide how to integrate `security-fixes` into `dev`.

---

## Spec Coverage Checklist

- [ ] Global `Model::unguard()` retained and all `$fillable`/`$guarded` removed
- [ ] Request whitelisting applied to every Eloquent create/update
- [ ] Auth login returns generic 401 for all failure cases
- [ ] OTPs never returned in HTTP responses
- [ ] Email-only OTP verification works
- [ ] Password reset does not leak non-existent users
- [ ] Inactive owners/employees blocked globally and in policies
- [ ] Null-safe ownership checks in policies/controllers
- [ ] LIKE wildcards escaped in search
- [ ] File uploads validate `image`, `mimes`, `max`
- [ ] Image compressor rejects failures instead of storing originals
- [ ] PII redacted in OTP/WhatsApp logs
- [ ] Session secure/encrypt/same_site hardened
- [ ] CORS methods/headers restricted
- [ ] `.env.example` production-safe defaults
- [ ] CoreVerde disabled by default
- [ ] Scheduled commands use `withoutOverlapping`
- [ ] Request docs metadata hidden
- [ ] Rate limiting on authenticated and public routes