# Laporan Code Review — Scuto Email Blast

- **Tanggal:** 2026-07-20
- **Nama Project:** scuto-email-blast (Laravel)
- **Scope:** Working tree bersih terhadap `main` → review menyeluruh atas kode aplikasi backend (Controllers, Middleware, Models, Exports, Routes)
- **Jumlah file dianalisis:** 16 file PHP (7 controller, 2 middleware, 5 model, 1 export, 3 route)

---

## Ringkasan Eksekutif

Secara umum kode sudah rapi: input divalidasi via Form Request validation, query memakai Eloquent (parameterized), password di-hash, dan ada proteksi reCAPTCHA + throttle di login. Namun ada beberapa temuan security yang perlu perhatian, terutama pada **endpoint API n8n** (mass assignment via `$guarded = []` + validasi status yang tidak lengkap) dan **enumerasi data lewat unsubscribe/bounce**. Sisanya adalah pelanggaran clean-code ringan (duplikasi filter, magic string status) yang tidak berbahaya.

| Severity | Jumlah |
|----------|--------|
| Critical | 0 |
| High     | 3 |
| Medium   | 4 |
| Low      | 3 |

---

## Temuan — Frontend

Tidak ditemukan masalah. Project ini adalah aplikasi Laravel dengan rendering server-side (Blade); tidak ada kode JavaScript/frontend aplikatif dalam scope yang direview (hanya scaffolding Vite/Tailwind).

---

## Temuan — Backend

### H-1. Mass assignment terbuka di semua model (`$guarded = []`)
- **Lokasi:** `app/Models/Campaign.php:9`, `app/Models/CampaignRecipient.php:7`, `app/Models/Contact.php:8`
- **Deskripsi:** Ketiga model memakai `protected $guarded = []` yang mematikan proteksi mass assignment sepenuhnya. Beberapa jalur `create()`/`update()` di-feed langsung dari input yang tidak sepenuhnya dibatasi `->only()`. Contoh paling nyata: `Campaign::create` di `DashboardController::store` hanya mengirim field tertentu, tapi tidak ada guard di model bila di masa depan ada endpoint yang meneruskan `$request->all()`. Ini menaikkan blast radius setiap bug validasi menjadi potensi mass-assignment. Rekomendasi: definisikan `$fillable` eksplisit per model.
- **Severity:** High

### H-2. Validasi `status` di `getRecipients` tidak dibatasi (raw query param)
- **Lokasi:** `app/Http/Controllers/Api/N8nController.php:39,45`
- **Deskripsi:** `$status = $request->query('status', ...)` dipakai langsung ke `->where('status', $status)` tanpa validasi terhadap enum `RecipientStatus`. Meski tetap parameterized (aman dari SQLi), nilai arbitrer memungkinkan caller mengintip data dengan filter status apa pun / status yang tidak seharusnya diekspos. Batasi dengan `Rule::in(RecipientStatus::values())` atau validasi enum.
- **Severity:** High

### H-3. Enumerasi kontak & tidak ada throttle di endpoint bounce/unsubscribe
- **Lokasi:** `app/Http/Controllers/Api/N8nController.php:103-126`, `app/Http/Controllers/Web/UnsubscribeController.php:11-33`
- **Deskripsi:** `updateBounce` mengembalikan `404 Contact not found` vs `200` — membocorkan apakah sebuah email terdaftar (email enumeration). Endpoint `updateBounce` juga bisa dipakai untuk menandai email SENT mana pun menjadi FAILED bagi siapa saja yang punya API key (tidak ada verifikasi bahwa bounce benar-benar berasal dari mailer). Untuk unsubscribe, route sudah dilindungi `signed` (baik), tapi bounce hanya bergantung pada satu API key statis tanpa rate limit. Pertimbangkan seragamkan response dan tambahkan throttle.
- **Severity:** High

### M-1. Duplikasi logic filter search/status (DRY)
- **Lokasi:** `app/Http/Controllers/Web/ContactController.php:20-32` vs `app/Exports/ContactsExport.php:26-40`
- **Deskripsi:** Blok filter `search` (name/email `like`) dan `status` (subscribed/unsubscribed) diduplikasi hampir identik di dua tempat. Bila aturan filter berubah, harus diedit dua kali dan rawan drift. Angkat ke query scope di model `Contact` (mis. `scopeFilter($search, $status)`) lalu pakai di keduanya.
- **Severity:** Medium

### M-2. Magic string status ('subscribed'/'unsubscribed'/'admin'/'user')
- **Lokasi:** `ContactController.php:27,29`, `ContactsExport.php:34,36`, `UserController.php:25`, `User.php:isAdmin()`
- **Deskripsi:** Status subscribe dan role user dipakai sebagai string literal tersebar di banyak file. RecipientStatus & CampaignStatus sudah pakai Enum — konsistensikan dengan mengangkat role dan subscribe-status menjadi konstanta/Enum bernama juga.
- **Severity:** Medium

### M-3. Fungsi `getCampaign` tidak dilindungi terhadap campaign tidak eksis dengan pesan jelas + render view tiap request
- **Lokasi:** `app/Http/Controllers/Api/N8nController.php:20-30`
- **Deskripsi:** `findOrFail` melempar 404 generic (ok), tapi `view(...)->render()` dieksekusi untuk setiap panggilan tanpa caching. Karena n8n memanggil `getCampaign` dan `getRecipients` berulang (pagination), rendering HTML body berulang untuk data yang tidak berubah adalah pemborosan. Pertimbangkan pisahkan: recipients tak perlu membawa body, dan body cukup diambil sekali. Tidak berbahaya, tapi mengganggu efisiensi.
- **Severity:** Medium

### M-4. Error handling n8n webhook di `store` tidak logging kegagalan
- **Lokasi:** `app/Http/Controllers/Web/DashboardController.php:76-81`
- **Deskripsi:** Saat trigger n8n gagal (server error / exception), user diberi flash message tapi kegagalan tidak di-`Log`. Untuk operasi async yang penting seperti blast email, jejak error sebaiknya dicatat agar bisa ditindaklanjuti. Silent selain flash message menyulitkan debugging produksi. (Catatan: jangan log payload sensitif — cukup campaign_id + status/exception.)
- **Severity:** Medium

### L-1. Default `limit` recipients tanpa upper bound
- **Lokasi:** `app/Http/Controllers/Api/N8nController.php:38,46`
- **Deskripsi:** `$limit = $request->query('limit', 50)` dipakai langsung ke `->limit($limit)`. Caller bisa meminta limit sangat besar (mis. 1.000.000) dan memicu beban query/memory. Tambahkan clamp maksimum (mis. `min($limit, 200)`) dan cast integer.
- **Severity:** Low

### L-2. `map` pada export mengasumsikan `created_at` selalu ada
- **Lokasi:** `app/Exports/ContactsExport.php:61`
- **Deskripsi:** `$contact->created_at->format(...)` akan error bila `created_at` null (mis. data hasil import upsert tanpa timestamp). Gunakan optional chaining `$contact->created_at?->format(...)` agar tidak fatal saat export.
- **Severity:** Low

### L-3. Whitespace/dead code minor
- **Lokasi:** `app/Http/Controllers/Api/N8nController.php:48-49` (baris kosong ganda), `app/Http/Controllers/Web/ReportController.php:48` (branch `if` yang tidak melakukan apa-apa untuk status selain SENT/FAILED)
- **Deskripsi:** Baris kosong berlebih dan cabang kondisi kosong. Nitpick style; sebaiknya dirapikan agar niat kode jelas (mis. abaikan status invalid secara eksplisit atau validasi lebih dulu).
- **Severity:** Low

---

## Kesimpulan & Prioritas

Urutan perbaikan yang disarankan:

1. **H-2** — Validasi `status` param di `getRecipients` dengan `Rule::in(RecipientStatus)` (cepat, tutup celah exposure).
2. **H-3** — Seragamkan response bounce (hindari email enumeration) + tambahkan throttle di endpoint publik/semi-publik.
3. **H-1** — Ganti `$guarded = []` menjadi `$fillable` eksplisit di Campaign, CampaignRecipient, Contact.
4. **M-1 & M-2** — Angkat filter kontak ke query scope + konsolidasi magic string status/role ke Enum/konstanta.
5. **M-3 & M-4** — Hindari render view berulang untuk n8n + tambahkan logging kegagalan webhook.
6. **L-1 → L-3** — Clamp limit, guard `created_at` null, rapikan dead code.

Tidak ada temuan Critical. Fondasi keamanan (hash password, throttle login, reCAPTCHA, signed URL unsubscribe, sanitasi HTML via `clean()`) sudah baik.