Secure Code Review — Security

Secure Code Review Secure code review adalah proses memeriksa kode secara manual atau otomatis untuk menemukan vulnerability keamanan. Ini berbeda dari code…

Secure code review adalah membaca perubahan kode, biasanya sebuah pull request, dengan satu pertanyaan di kepala: "kalau input-nya jahat, apa yang terjadi?". Bayangin pengacara membaca kontrak. Orang biasa membaca apakah kalimatnya enak dan isinya masuk akal. Pengacara membaca tiap pasal sambil nanya "pasal ini bisa dipakai buat merugikan kita gak?". Code review biasa mengecek kodenya jalan dan rapi; secure code review mengecek kodenya gak bisa disalahgunakan.

🧪 Tebakan cepat: sebuah baris membatasi input jadi 100 karakter: String(req.query.q ?? '').slice(0, 100). Baris berikutnya menyambung q ke dalam query SQL. Apakah batas 100 karakter itu sudah mencegah SQL injection? Tebak dulu, jawabannya ada di bagian "Studi kasus: PR #42".

Ikuti datanya: source dan sink

Reviewer keamanan gak membaca kode dari atas ke bawah seperti cerita. Dia mengikuti data dari luar:

Begitu ketemu source, hal pertama yang dicek adalah validasi: apakah tipe, panjang, dan formatnya diperiksa di server? Lalu ikuti nilainya sampai ke sink, dan tanya: di sink ini, dia tetap jadi data, atau bisa berubah jadi perintah? Ini prinsip never trust user input dari pelajaran Security Checklist, dipakai sambil membaca kode.

Empat pola yang paling sering ketemu:

Pola Tandanya di kode Pelajarannya
Query dirakit dari string "... WHERE x = '" + input + "'" SQL Injection Prevention
Output tanpa escape res.send('<h2>' + input + '</h2>') XSS Prevention & Sanitization
Otorisasi hilang update atau delete berdasarkan :id tanpa cek pemilik OWASP Top 10 (Broken Access Control)
Secret di kode const KEY = 'sk_live_...' Environment Variables & Secrets

Studi kasus: PR #42

Ini diff yang sama dengan Lab. Developer menambah fitur cari order dan ubah catatan order di routes/orders.js:

const PAYMENT_KEY = 'sk_live_...'

router.get('/orders/search', auth, async (req, res) => {
  const q = String(req.query.q ?? '').slice(0, 100)
  const { rows } = await db.query("SELECT id, note FROM orders WHERE note LIKE '%" + q + "%'")
  res.send('<h2>Hasil untuk ' + q + '</h2>' + renderRows(rows))
})

router.put('/orders/:id', auth, async (req, res) => {
  const { rows } = await db.query('SELECT * FROM orders WHERE id = $1', [req.params.id])
  await db.query('UPDATE orders SET note = $1 WHERE id = $2', [req.body.note, req.params.id])
  res.json({ id: rows[0].id, note: req.body.note })
})

Hasil review-nya:

Baris Temuan Perbaikan
const PAYMENT_KEY = 'sk_live_...' secret di kode, ikut ter-commit process.env.PAYMENT_KEY, lalu rotate kunci lama
db.query("... LIKE '%" + q + "%'") query dirakit dari string parameter: LIKE $1 dengan ['%' + q + '%']
res.send('<h2>Hasil untuk ' + q ... output tanpa escape escapeHtml(q)
UPDATE orders ... WHERE id = $2 gak ada cek pemilik order cek rows[0]?.user_id sama dengan req.user.id dulu

Batas 100 karakter itu validasi, dan bagus, tapi belum mencegah SQL injection: 100 karakter sudah cukup untuk mengubah arti query. Yang mencegahnya adalah parameter. Itu jawaban tebakannya.

Perhatikan juga baris yang bukan temuan: SELECT * FROM orders WHERE id = $1 sudah pakai parameter, kedua route punya middleware auth, dan res.json cuma mengirim id dan note. Menandai baris yang aman bikin review berisik, dan developer berhenti membacanya. Satu lagi: baris UPDATE itu aman dari SQL injection, tapi tetap berbahaya. Parameter menjawab "apakah ini data?", bukan "apakah kamu boleh?".

Kode setelah review:

const PAYMENT_KEY = process.env.PAYMENT_KEY

router.get('/orders/search', auth, async (req, res) => {
  const q = String(req.query.q ?? '').slice(0, 100)
  const { rows } = await db.query('SELECT id, note FROM orders WHERE note LIKE $1', ['%' + q + '%'])
  res.send('<h2>Hasil untuk ' + escapeHtml(q) + '</h2>' + renderRows(rows))
})

router.put('/orders/:id', auth, async (req, res) => {
  const { rows } = await db.query('SELECT * FROM orders WHERE id = $1', [req.params.id])
  if (rows[0]?.user_id !== req.user.id) return res.sendStatus(403)
  await db.query('UPDATE orders SET note = $1 WHERE id = $2', [req.body.note, req.params.id])
  res.json({ id: rows[0].id, note: req.body.note })
})

SAST, DAST, dan review manual

Sebagian review bisa dibantu tool:

Cara Cara kerjanya Contoh tool Jago menemukan
SAST (Static Application Security Testing) membaca source code tanpa menjalankannya Semgrep, CodeQL, SonarQube, Psalm --taint-analysis query dari string, secret di kode
DAST (Dynamic Application Security Testing) mengirim request ke aplikasi yang sedang berjalan OWASP ZAP, Burp Suite header hilang, error yang bocor, masalah runtime
SCA (Software Composition Analysis) mencocokkan dependency dengan database CVE npm audit, Dependabot library versi rentan
Review manual manusia membaca dan memahami maksud kodenya kamu otorisasi dan logika bisnis

Tool jago menemukan pola, tapi gak tahu bahwa order cuma boleh diubah pemiliknya. Cek otorisasi seperti di PR #42 hampir selalu ditemukan manusia. Pelajaran berikutnya, Security Automation & Tools, memasang SAST, SCA, dan cek lain di CI supaya jalan di tiap pull request.

⚠️ Jebakan yang sering ketemu

TL;DR: Secure code review membaca pull request sambil mengikuti data dari source (req.query, req.body, req.params) ke sink (query, HTML, shell). Hal pertama yang dicek pada input user adalah validasi, tapi validasi saja gak mencegah injection. Empat pola utama: query dirakit dari string (perbaiki dengan parameter), output tanpa escape (escape), otorisasi hilang (cek pemilik di server), dan secret di kode (pindah ke env lalu rotate). SAST membaca kode tanpa menjalankannya, DAST menguji aplikasi yang sedang berjalan, SCA mengecek dependency, dan review manual menangkap otorisasi serta logika bisnis.

Yang akan kamu pelajari