Belajar Application Security Engineer - Security Code Review

#Pendahuluan

Setelah di episode 11 kita membangun pipeline yang mengotomasi SAST, SCA, dan secret scanning, muncul pertanyaan wajar: kalau semua itu berjalan otomatis, untuk apa masih review manual? Jawabannya ada di kategori temuan yang konsisten mendominasi insiden serius namun mustahil ditemukan tool: broken authorization pada alur bisnis, race condition, logika refund ganda, bypass workflow approval. Tool tidak tahu bahwa "diskon hanya boleh sekali per user" — manusia yang membaca konteks, yang tahu.

Episode ini memberi metodologi review yang bisa dieksekusi dalam waktu terbatas: cara memilih PR mana yang layak waktu mendalam, apa yang dicari per kategori, dan bagaimana menyampaikannya agar developer mau mengikuti.

#Triase: Tidak Semua PR Layak Waktu Kalian

AppSec engineer tidak bisa membaca setiap baris dari ratusan PR mingguan — dan tidak harus. Prioritaskan dengan skor risiko:

PR refaktor nama variabel atau update copywriting? Lewatkan. PR yang menambah endpoint pembayaran? Baca baris demi baris.

Automasi triasinya murah dan berdampak besar:

gh pr diff 142 --name-only
# cocokkan pola: auth/, middleware/, api/, Dockerfile, .github/workflows/

Banyak tim membangun routing otomatis: label security-review menempel bila path cocok pola sensitif — AppSec hanya dipanggil untuk yang relevan.

#Metodologi Review Lima Langkah

Untuk PR prioritas tinggi, jalankan urutan ini:

1. Pahami intent sebelum membaca kode. Baca deskripsi + tiket: fitur ini melakukan apa secara bisnis? Ancaman naturalnya apa? (Fitur upload file → ancaman: RCE via path, DoS ukuran, malware hosting.)

2. Telusuri input sampai sink. Untuk tiap parameter baru, ikuti perjalanannya: masuk dari mana, divalidasi siapa, berakhir di mana? Ini versi manual dari taint analysis episode 7 — dan kalian jauh lebih pintar dari tool karena paham konteks.

3. Cek otorisasi pada level objek. Setiap query yang memakai ID dari request: apakah kepemilikan diverifikasi? Pola pertanyaan sederhana: "kalau saya ganti angka di URL ini, apa yang saya lihat?"

4. Bandingkan dengan sibling code. Endpoint baru harus konsisten dengan endpoint lama yang sudah diaudit — kalau handler lama punya rate limit dan yang baru tidak, itu regresi halus yang scanner tak kenal.

5. Pikirkan state & concurrency. Dua request bersamaan, retry, timeout — apa yang rusak? (Kupon dipakai dua kali via race; saldo minus via double-submit.)

#Checklist Per Kategori

Checklist ringkas saat membaca — turunan langsung OWASP Top 10 episode 3 tapi dalam bentuk pertanyaan kode:

Simpan sebagai template PR comment agar konsisten antar reviewer.

#Bug Business Logic: Wilayah Eksklusif Manusia

Contoh nyata pola yang hanya ketemu review manual:

app.post("/api/cart/apply-coupon", requireAuth, async (req, res) => {
  const coupon = await db.coupon.findValid(req.body.code);
  if (!coupon) return res.status(404).json({ error: "invalid coupon" });
 
  await db.cart.addDiscount(req.user.cartId, coupon.discountPct);
  await db.coupon.markUsed(coupon.id, req.user.id);
  return res.json({ ok: true });
});

Kode ini lolos SAST (tidak ada injection), lolos SCA (tidak ada dependency issue), dan lolos pengecekan auth (middleware jalan). Tapi:

1. Tidak ada cek "sudah pernah pakai?" sebelum apply — user bisa apply kupon one-time berkali-kali sebelum markUsed dieksekusi.
2. Race condition: dua request paralel keduanya melihat used=false — butuh transaksi atomik atau unique constraint.
3. Diskon tidak divalidasi ulang di endpoint checkout — harga final harus direkalkulasi server-side saat bayar, bukan percaya state cart.

Temuan seperti inilah yang membenarkan posisi AppSec engineer di organisasi. Latihannya: selalu minta deskripsi flow bisnis, lalu tanyakan apa yang terjadi jika langkah ini dilakukan dua kali / dilewati / dibalik urutannya.

#Menyampaikan Temuan agar Dieksekusi

Review teknis terbaik gagal nilainya bila developer defensif. Format komentar yang terbukti bekerja:

1. Baris spesifik — bukan generalisasi tentang "kode ini".
2. Dampak satu kalimat dalam bahasa bisnis ("semua user bisa membaca order pengguna lain").
3. Diff perbaikan konkret — siap-copy, bukan "pertimbangkan parameterized query".

[Must-fix] routes/orders.ts:42 — orderId dari params langsung dipakai tanpa
cek ownership. User A dapat membaca/mengubah order User B (IDOR).
 
Saran fix:
  const order = await db.order.findOne({ id: req.params.id, userId: req.user.id });
  if (!order) return res.status(404).end();

Dan kelola proporsi: jika semua temuan kalian berlabel [Must-fix], label itu kehilangan makna. Skala severity kalian harus konsisten dengan SLA remediasi yang kita bangun di episode 13.

#Common Pitfalls

• Review semua PR dangkal-dangkal: hasilnya nol temuan bernilai plus waktu habis; lebih baik triase ketat + mendalam selektif.
• Menjadi gatekeeper tunggal: kalau hanya kalian yang bisa menilai security, organisasi mati saat kalian cuti — itulah alasan security champions (episode 24).
• Nitinpick gaya di komentar security: campur aduk style comment dengan must-fix membuat must-fix tenggelam.
• Mengandalkan ingatan untuk checklist: review fatigue itu nyata; checklist tertulis menjaga kualitas di PR ke-15 hari itu.

#Penutup

Inti yang harus dibawa pulang:

• Nilai review manual ada di business logic, otorisasi objek, dan concurrency — area buta tool.
• Triase dengan skor risiko; baca mendalam hanya PR yang menyentuh auth, sink, data sensitif, trust boundary.
• Metodologi lima langkah: intent → taint manual → object-level authz → banding sibling → state/race.
• Komentar efektif = baris + dampak bisnis + diff siap pakai.

Di episode 13 kita lanjut siklus hidup temuan: Vulnerability Remediation — CVSS dan EPSS untuk memprioritaskan, menyusun fix guidance yang developer terima, SLA per severity, dan proses risk acceptance untuk yang tidak bisa diperbaiki. Sampai jumpa di episode 13!