Skip to Content
SécuritéPartie 16 — Cookbook & Annexes16.2 — Checklist revue de code

Chapitre 16.2 — Checklist revue de code

⏱️ TL;DR — Une checklist de revue de code sécurité : quoi chercher, systématiquement, dans chaque pull request. Organisée par thème (entrées, accès, injections, XSS, secrets, dépendances…), elle transforme les principes du cours en questions concrètes à se poser devant du code. L’idée n’est pas de tout vérifier à chaque fois, mais d’avoir les réflexes : pour la fonctionnalité touchée par la PR, se demander « comment j’attaquerais ça ? » (Partie 1.4) et passer les points pertinents. On peut aussi automatiser une partie (linters, scanners) pour que l’humain se concentre sur ce que les outils ne voient pas — surtout la logique métier (IDOR, autorisation).

🎯 Objectifs

  • Disposer d’une checklist de revue de code sécurité, par thème.
  • Savoir quels points sont automatisables et lesquels exigent l’œil humain.
  • Adopter le réflexe « chapeau d’attaquant » en revue (Partie 1.4).

Comment utiliser cette checklist

En revue, on ne passe pas 100 points sur chaque PR. On applique le jugement : pour la fonctionnalité modifiée, on identifie les risques pertinents (une PR qui touche une requête SQL → injections ; une qui touche un accès à une ressource → autorisation ; une qui ajoute un endpoint → auth + validation) et on passe les points correspondants. Le fil conducteur reste la question de la Partie 1.4 : « si je voulais casser ça, comment ? ».

Entrées & validation

  • Toute entrée (corps, params, en-têtes, uploads, réponses d’API tierces) est-elle traitée comme hostile ?
  • Les entrées sont-elles validées (type, format, longueur) par une allow-list / un schéma (Zod, PARAM_*, contraintes) ?
  • La validation est-elle bien côté serveur (pas seulement dans l’UI) ?
  • Y a-t-il un risque de mass assignment (le corps appliqué tel quel à un objet, avec role/ownerId) ?

Authentification & session

  • Les mots de passe sont-ils hachés (argon2id/bcrypt), jamais en clair/chiffrés/SHA nu ?
  • Les comparaisons de secrets sont-elles en temps constant (hash_equals, ===, pas ==) ?
  • La session est-elle en cookie HttpOnly/Secure/SameSite, régénérée à la connexion ?
  • Les endpoints sensibles ont-ils du rate limiting (login, reset) ? Les messages sont-ils génériques (pas d’énumération) ?

Autorisation (le point n°1)

  • Chaque accès à une ressource vérifie-t-il l’autorisation côté serveur (pas juste « connecté ») ?
  • Le contrôle est-il par objet (appartenance : owner_id/tenant_id), pas seulement par rôle ? → anti-IDOR
  • La requête est-elle portée par le propriétaire/tenant (WHERE owner_id = :moi) plutôt que « récupérer puis peut-être vérifier » ?
  • Les Server Actions / route handlers vérifient-ils auth + validation + autorisation (ce ne sont pas des appels locaux) ?
  • Le contrôle passe-t-il par un voter/capability/policy centralisé (pas un if isolé) ?

Injections

  • Les requêtes SQL sont-elles paramétrées (jamais de concaténation d’entrée, même « juste un entier ») ?
  • Pour un tri/colonne dynamique : allow-list (pas l’entrée brute) ?
  • Les commandes système passent-elles les arguments en tableau, sans shell (execFile/spawn) ?
  • Pas de unserialize/eval/include sur une entrée ?

XSS & sortie

  • Les sorties sont-elles encodées (auto-échappement React/Twig, s()/htmlspecialchars) selon le contexte ?
  • Chaque dangerouslySetInnerHTML/v-html/|raw/format_text sur de l’entrée est-il sanitizé (DOMPurify/HtmlSanitizer) et justifié ?
  • Les URL fournies par l’utilisateur (href/src) ont-elles un schéma validé (pas de javascript:) ?

CSRF / SSRF / redirections

  • Les actions modifiant l’état ont-elles une protection CSRF (jeton, sesskey) et pas de mutation via GET ?
  • Tout fetch serveur vers une URL utilisateur a-t-il une allow-list + rejet des IP internes (SSRF) ?
  • Les redirections ne pointent-elles que vers des cibles internes/allow-list (pas d’open redirect) ?
  • La config CORS est-elle stricte (allow-list, jamais * + credentials, pas de miroir d’Origin) ?

Secrets & données

  • Aucun secret dans le code/la PR (clé, token, mot de passe) ? (scan pre-commit actif ?)
  • Rien de sensible exposé au client (NEXT_PUBLIC_ sur un secret, prop/DTO trop riche) ?
  • Les logs ne fuitent-ils pas de secrets/PII (pas d’objet complet, mot de passe, token) ?
  • Minimisation : ne collecte/transmet-on que le nécessaire ?

Dépendances & config

  • Une nouvelle dépendance est-elle légitime (nom exact, mainteneur, popularité — anti-typosquatting) ? Le lockfile est-il à jour ?
  • Les erreurs sont-elles masquées en prod (pas de stacktrace au client) ?
  • Les en-têtes de sécurité et la config sont-ils cohérents ?

Ce qu’on automatise vs l’œil humain

  • Automatisable (linters, scanners — à mettre en CI) : npm/composer audit (dépendances), scan de secrets (gitleaks), règles ESLint (react/no-danger), analyse statique (SAST) qui repère des motifs (concaténation SQL, eval, dangerouslySetInnerHTML), scan d’images (Trivy).
  • Œil humain indispensable : la logique métier — surtout l’autorisation par objet (IDOR), les failles de logique, la pertinence d’une exception de sécurité. Aucun outil ne sait qu’un formateur ne devrait pas voir la classe d’un autre : toi oui.

💡 Réflexe — Fais des points automatisables une affaire d’outils (en CI, pour qu’ils ne dépendent pas de la vigilance) et concentre ton attention humaine sur ce que les outils ratent : l’autorisation par objet et la logique métier. En revue, le réflexe le plus rentable est de prendre chaque endpoint/accès touché par la PR et de te demander « et si j’y mettais l’id d’un autre ? » — c’est là que se cachent les failles les plus graves et les moins détectables automatiquement.

🧭 Sur FormaCampus — FormaCampus a intégré une version de cette checklist à son template de PR (cases à cocher selon le type de changement) et automatisé le maximum en CI (audit, scan de secrets, ESLint no-danger, SAST, scan d’images — build bloquant). En revue humaine, le relecteur enfile le « chapeau d’attaquant » (Partie 1.4) et se concentre sur l’autorisation par objet et la logique métier — ce que les outils ne voient pas. C’est ce process léger qui a permis d’attraper l’IDOR d’export (Partie 5) en revue, pas en prod.

✏️ Exercices

Exercice 1 — Priorise la revue. Une PR ajoute un endpoint GET /api/documents/:id/download. Quels trois points de la checklist regardes-tu en priorité, et pourquoi ?

✅ Solution

(1) Autorisation par objet (anti-IDOR) — priorité absolue : le :id vient du client ; l’endpoint vérifie-t-il que le document appartient au demandeur (requête portée par le propriétaire/tenant), et pas seulement que l’utilisateur est connecté ? C’est le risque n°1 (Broken Access Control) et le plus probable ici. (2) Traversée de répertoire / chemin de fichier — si le téléchargement lit un fichier depuis un chemin, ce chemin est-il contraint (pas de ../, allow-list, basename) et le fichier servi via un mécanisme sûr (pas d’exécution) ? (3) Fuite de données / DTO — l’endpoint ne renvoie-t-il que le fichier autorisé, sans exposer de métadonnées sensibles, et logue-t-il l’accès sans fuiter de PII ? Bonus : rate limiting (abus de téléchargement). L’autorisation par objet est la plus critique et la moins détectable automatiquement — c’est là que l’œil humain compte le plus.

Exercice 2 — Automatiser ou revoir ? Classe ces vérifications en « automatisable (CI) » ou « œil humain » : (a) un secret commité, (b) un formateur peut-il voir la classe d’un autre, (c) une dépendance a une CVE, (d) un dangerouslySetInnerHTML non sanitizé.

✅ Solution

(a) Automatisable : un scan de secrets (gitleaks) en pre-commit/CI détecte les motifs de secrets. (b) Œil humain : c’est de la logique métier / autorisation par objet — aucun outil ne connaît la règle « un formateur ne voit que ses classes » ; il faut vérifier que le code applique bien ce contrôle (voter, requête portée par le propriétaire). (c) Automatisable : npm/composer audit (+ Dependabot) détecte les CVE des dépendances. (d) Automatisable (partiellement) : une règle ESLint (react/no-danger) repère les dangerouslySetInnerHTML, mais l’œil humain doit juger si la donnée est sanitizée et si le HTML est justifié. Règle générale : outils pour les motifs (secrets, CVE, sinks), humain pour la logique et le jugement (autorisation, pertinence).

🧠 Quiz de révision

1. Quel est le point de revue le plus critique et le moins automatisable ?

L’autorisation par objet (anti-IDOR) et la logique métier : aucun outil ne connaît tes règles métier (« un formateur ne voit que ses classes »). C’est là que l’œil humain est indispensable — le réflexe « et si j’y mettais l’id d’un autre ? ».

2. Quels contrôles a-t-on intérêt à automatiser en CI ?

Ceux basés sur des motifs : npm/composer audit (CVE), scan de secrets (gitleaks), règles ESLint (react/no-danger), analyse statique (SAST : concaténation SQL, eval, sinks), scan d’images (Trivy). Ils libèrent l’attention humaine pour la logique.

3. Devant un nouvel endpoint, quelles vérifications de base ?

Auth (session requise ?), validation des entrées (schéma/allow-list, côté serveur), et autorisation par objet (l’objet visé appartient-il au demandeur ?). Plus : pas de mutation via GET, protection CSRF si état modifié, rate limiting si sensible.

4. Comment utiliser efficacement une checklist de revue ?

Avec jugement : pour la fonctionnalité modifiée, identifier les risques pertinents et passer les points correspondants (pas tout à chaque fois), guidé par « si je voulais casser ça, comment ? ». Idéalement intégrée au template de PR et doublée d’automatisations en CI.

5. Pourquoi un dangerouslySetInnerHTML mérite-t-il un coup d’œil en revue ?

Parce que c’est un sink XSS : il injecte du HTML brut. En revue, on vérifie que la donnée est sanitizée (DOMPurify) et que le HTML est vraiment nécessaire (sinon, interpolation normale échappée). Une règle de linter le signale, mais le jugement reste humain.

Chapitre suivant : Checklist mise en prod — le tour du propriétaire avant de livrer en production.

Last updated on
16.1 — OWASP Top 10 & ASVS16.3 — Checklist mise en prod