diff --git a/knowledge/backend/patterns/auth.md b/knowledge/backend/patterns/auth.md index 86b089a..e9b2f69 100644 --- a/knowledge/backend/patterns/auth.md +++ b/knowledge/backend/patterns/auth.md @@ -239,3 +239,60 @@ try { - [ ] `P2025` absorbé silencieusement sur les suppressions de session - [ ] Effets de bord hors transaction documentés (cookie, email) - [ ] Tests couvrant le cas d'une session déjà expirée + +--- + + +## Pattern : Scope minimal du cookie refresh token + +- Objectif : limiter l'exposition du cookie refresh token au strict minimum. +- Contexte : migration d'un stockage localStorage vers cookies httpOnly pour les tokens d'authentification. +- Quand l'utiliser : dès qu'un refresh token est transmis via cookie. +- Quand l'éviter : jamais. +- Avantage : + - réduit la surface d'attaque — le cookie ne voyage qu'avec les requêtes de refresh + - évite l'envoi inutile du refresh token sur les endpoints auth (login, password, invitations) +- Limites / vigilance : + - vérifier que le path est compatible avec le routing réel de l'endpoint de refresh +- Validé le : 08-04-2026 +- Contexte technique : auth / cookies httpOnly — RL799_V2 + +### Règle + +- Le cookie `refresh_token` doit avoir `Path=/api/auth/refresh` (pas `/api/auth`). Seul l'endpoint de refresh a besoin de recevoir ce cookie. +- Plus le path est large, plus la surface d'attaque est grande (le cookie voyage avec chaque requête matchant le path). + +### Checklist + +- [ ] `Path=/api/auth/refresh` sur le cookie refresh token +- [ ] `Path=/` uniquement pour l'access token (nécessaire sur toutes les routes API) +- [ ] Vérifier que l'endpoint de refresh reçoit bien le cookie après changement de path + +--- + + +## Pattern : Distinction stricte 401 vs 403 + +- Objectif : permettre au frontend de réagir correctement aux erreurs d'authentification et d'autorisation. +- Contexte : helper centralisé `requireRoleAccess` ou équivalent. +- Quand l'utiliser : systématiquement sur tout helper d'authentification/autorisation. +- Quand l'éviter : jamais. +- Avantage : + - le frontend peut distinguer "session expirée → redirection login" de "permission manquante → message accès refusé" + - debug plus rapide en production +- Limites / vigilance : + - la distinction doit être cohérente sur tous les endpoints — ne pas retourner 403 pour un token absent sur certains services +- Validé le : 08-04-2026 +- Contexte technique : auth / RBAC — RL799_V2 + +### Règle + +Le helper `requireRoleAccess` doit retourner : +- **401 UNAUTHORIZED** : token absent, expiré, malformé, ou email manquant dans le payload +- **403 FORBIDDEN** : token valide mais rôle non autorisé pour la ressource + +### Checklist + +- [ ] 401 pour tout problème de token (absent, expiré, malformé) +- [ ] 403 uniquement quand le token est valide mais le rôle insuffisant +- [ ] Frontend redirige vers `/login` sur 401, affiche "accès refusé" sur 403 diff --git a/knowledge/backend/patterns/contracts.md b/knowledge/backend/patterns/contracts.md index 5abab57..4c2294e 100644 --- a/knowledge/backend/patterns/contracts.md +++ b/knowledge/backend/patterns/contracts.md @@ -167,3 +167,23 @@ Quand un repository utilise le pattern `{ ok: true; data } | { ok: false }` pour ### Signal review - `catch { return null }` dans un repository qui utilise `{ ok: false }` ailleurs + +--- + + +## Pattern : API crypto — variantes binaire et base64 + +- Objectif : éviter un round-trip base64 coûteux en mémoire quand les données sont déjà en binaire. +- Contexte : fonction crypto qui travaille en base64 pour la sérialisation (stockage DB, transport JSON) mais appelée aussi depuis des lectures fichier (binaire natif). +- Quand l'utiliser : dès qu'une fonction crypto accepte uniquement du base64 mais est appelée avec des données binaires. +- Quand l'éviter : si toutes les sources de données sont déjà en base64. +- Validé le : 08-04-2026 +- Contexte technique : Node.js crypto / fichiers — RL799_V2 story 13-7 + +### Règle + +Quand une fonction crypto travaille en base64 pour la sérialisation, prévoir une variante `*Buffer` qui accepte un `Buffer` natif pour les cas où les données sont déjà en binaire (lecture fichier, stream). Cela évite un double encodage (fichier → base64 → Buffer → déchiffrement). + +### Signal review + +- `buffer.toString('base64')` suivi immédiatement de `decrypt(base64String)` qui fait `Buffer.from(str, 'base64')` → round-trip inutile diff --git a/knowledge/backend/patterns/general.md b/knowledge/backend/patterns/general.md index b513205..ffe5a95 100644 --- a/knowledge/backend/patterns/general.md +++ b/knowledge/backend/patterns/general.md @@ -33,3 +33,29 @@ source_projects: [RL799_V2] ### Signal review - Tout service qui importe `verifyToken` directement ET fait son propre check RBAC est suspect de duplication. + +--- + + +## Pattern : Extension d'un service existant par type avec RBAC granulaire + +- Objectif : éviter la duplication de service quand un nouveau besoin est fonctionnellement identique à un service existant mais avec des restrictions d'accès différentes. +- Contexte : ajout d'un nouveau type de communication (ex: `vm`) avec restrictions RBAC spécifiques dans un service communications existant. +- Quand l'utiliser : quand le nouveau besoin utilise le même modèle de données et le même CRUD que le service existant, seules les restrictions d'accès changent. +- Quand l'éviter : quand le modèle de données diverge (champs spécifiques au nouveau type) — dans ce cas, un service dédié est préférable. +- Validé le : 08-04-2026 +- Contexte technique : backend / architecture — RL799_V2 story 18-7 + +### Règle + +Préférer étendre le service avec un nouveau type (enum/Set) et ajuster les restrictions RBAC par type, plutôt que dupliquer le service. + +### Avantages + +- Un seul repository, un seul endpoint, même format de réponse +- Tests existants couvrent la non-régression +- Moins de code à maintenir + +### Signal review + +- Nouveau service qui réplique le CRUD d'un service existant avec un filtre additionnel → candidat à la fusion par type diff --git a/knowledge/backend/risques/auth.md b/knowledge/backend/risques/auth.md index 3b6de86..4c38886 100644 --- a/knowledge/backend/risques/auth.md +++ b/knowledge/backend/risques/auth.md @@ -283,3 +283,50 @@ it('retourne 403 si subscription inactive', async () => { - **Signal review** : `email: user.email` dans le mapping d'un endpoint annuaire - Contexte technique : auth / annuaire — RL799_V2 02-04-2026 + +--- + + +## TOCTOU sur rotation de refresh token + +### Risques + +- Un pattern `findUnique` + `update` séparés sur la rotation de refresh token crée une fenêtre TOCTOU +- Deux requêtes concurrentes avec le même refresh token passent toutes les deux la vérification avant que l'une ne révoque → deux sessions valides émises, le vol de token passe inaperçu + +### Symptômes + +- Deux sessions actives issues du même refresh token +- Détection de vol impossible car les deux tokens sont valides + +### Bonnes pratiques / mitigations + +- Toujours utiliser un `updateMany` atomique avec condition `WHERE revokedAt IS NULL AND expiresAt > NOW()` et vérifier `count === 1` +- Si `count === 0`, le token a déjà été utilisé → révoquer **tous** les tokens du user (token family detection, RFC 6819) +- **Signal review** : `findUnique` suivi de `update` séparés dans un flux de rotation de refresh token + +- Contexte technique : auth / refresh token — RL799_V2 08-04-2026 + +--- + + +## Drift d'authentification par copier-coller de pattern auth + +### Risques + +- Quand un helper d'auth centralisé existe (`requireRoleAccess`), mais que de nouveaux services réimplémentent le même pattern manuellement (`extractAccessToken` + `verifyToken` + vérification locale), chaque service développe ses propres variantes (codes d'erreur différents, 401 vs 403, requestId ou non) +- La surface d'auth devient incohérente et indéfendable en audit + +### Symptômes + +- Un audit RBAC révèle qu'une part significative des routes ont un pattern d'auth "fait maison" au lieu du helper standard +- Codes d'erreur divergents entre services pour la même situation (token absent) + +### Bonnes pratiques / mitigations + +- Tout nouveau handler HTTP DOIT utiliser le helper centralisé pour l'authentification et l'autorisation +- Ne JAMAIS importer `extractAccessToken` + `verifyToken` directement dans un service métier +- Si le helper ne couvre pas un besoin (ex: besoin de `userId` en plus de `email`), étendre le helper plutôt que contourner +- **Signal review** : import de `verifyToken` dans un fichier service (hors `authHelpers.ts`) + +- Contexte technique : auth / architecture — RL799_V2 08-04-2026 diff --git a/knowledge/backend/risques/general.md b/knowledge/backend/risques/general.md index 4e2db4a..c90e299 100644 --- a/knowledge/backend/risques/general.md +++ b/knowledge/backend/risques/general.md @@ -438,3 +438,313 @@ try { - Permet la surcharge en test et le rechargement dynamique - Contexte technique : Node.js / tests — RL799_V2 07-04-2026 + +--- + + +## Rate limiting — couverture de test insuffisante + +### Risques + +- Tester uniquement l'objet `RateLimiter` en isolation (`.check()`, `.reset()`) donne une fausse confiance. Les bugs de câblage (limiter non appelé, mauvais limiter sur le mauvais endpoint, format de réponse 429 incorrect) passent au travers. + +### Symptômes + +- Tests unitaires verts mais réponse 429 absente ou mal formatée en intégration +- Header `retry-after` manquant dans la réponse HTTP + +### Bonnes pratiques / mitigations + +- Pour chaque rate limiter intégré dans un endpoint, ajouter au minimum : + 1. Un test d'intégration qui dépasse le seuil via le handler HTTP et vérifie status 429 + body + header `retry-after` + 2. Un test d'expiration de fenêtre (mock `Date.now` et avancer le temps) + 3. Vérifier que le logging sécurité est déclenché (au minimum visible dans la sortie test) + +- Contexte technique : backend / rate limiting — RL799_V2 07-04-2026 + +--- + + +## Nommage des métriques agrégées dans les DTO + +### Risques + +- Nommer un champ `xxxCount` ou `xxxThisYear` sans que le nom reflète exactement ce qui est compté +- Confusion en maintenance, bugs d'affichage, labels UI incohérents + +### Symptômes + +- `tenuesThisYear` qui compte en réalité les *présences* du membre, pas le nombre total de tenues +- Labels frontend incohérents avec le comportement réel du champ + +### Bonnes pratiques / mitigations + +- Le nom du champ DTO doit refléter exactement l'entité comptée ET le scope du comptage +- Préférer `presencesThisYear` à `tenuesThisYear` si on compte les attendances d'un user, réserver `tenuesThisYear` pour le COUNT de tenues elles-mêmes +- **Signal review** : tout champ `xxxCount` ou `xxxThisYear` dont le nom ne correspond pas à la requête sous-jacente + +- Contexte technique : backend / DTO — RL799_V2 07-04-2026 + +--- + + +## `.gitignore` — `.env*` wildcard capture `.env.example` + +### Risques + +- Utiliser `.env*` dans `.gitignore` sans exception exclut aussi `.env.example`, qui est le fichier standard de documentation des variables d'environnement et qui DOIT être versionné + +### Symptômes + +- Le fichier `.env.example` existe sur le disque mais n'apparaît pas dans `git status` +- Les développeurs ne savent pas quelles variables configurer + +### Bonnes pratiques / mitigations + +- Toujours ajouter `!.env.example` après le wildcard `.env*` dans chaque `.gitignore` (racine ET sous-projets du monorepo) +- Vérifier avec `git check-ignore ` que l'exception fonctionne +- **Signal review** : `.env*` dans `.gitignore` sans `!.env.example` + +- Contexte technique : git / configuration — RL799_V2 08-04-2026 + +--- + + +## Strip HTML regex — single-pass insuffisant + +### Risques + +- `input.replace(/<[^>]*>/g, '')` en un seul passage laisse des fragments exploitables sur des inputs malformés type `ipt>alert(1)` + +### Symptômes + +- Après strip, le résultat contient encore des fragments `>` ou des reconstitutions partielles de tags + +### Bonnes pratiques / mitigations + +- Toujours (1) boucler le strip jusqu'à stabilisation (`while (prev !== result)`) ET (2) supprimer les chevrons orphelins `<>` après la boucle +- Pour du plain text (pas de rich text), c'est suffisant. Pour du rich text, utiliser une lib dédiée (DOMPurify, sanitize-html) +- **Signal review** : `replace(/<[^>]*>/g, '')` sans boucle dans un code qui traite de l'input utilisateur + +- Contexte technique : sécurité / sanitisation — RL799_V2 08-04-2026 + +--- + + +## Incohérence source de vérité — filtrage vs affichage sur des tables différentes + +### Risques + +- Filtrer des entités sur un champ d'une table relationnelle (`profile.grade`) tout en affichant le résultat depuis une autre table (`directory.grade`). Si les deux tables ne sont pas synchronisées en permanence, les résultats de filtrage et d'affichage divergent silencieusement. + +### Symptômes + +- Un membre apparaît éligible mais avec un grade affiché incohérent, ou inversement un membre éligible est invisible parce que seule la table d'affichage a été mise à jour + +### Bonnes pratiques / mitigations + +- Toujours utiliser la même table comme source de vérité pour le filtrage ET l'affichage d'un même attribut +- Si deux tables portent la même information, choisir celle qui fait autorité et aligner le code dessus +- **Signal review** : `where` sur `tableA.field` avec `select` sur `tableB.field` pour le même attribut + +- Contexte technique : Prisma / relations — RL799_V2 08-04-2026 + +--- + + +## Check-then-create non atomique — catcher P2002 + +### Risques + +- Vérifier l'existence d'un enregistrement (`findFirst/findUnique`) puis créer dans un second appel est une race condition classique +- Deux requêtes concurrentes passent le check, l'une échoue sur la contrainte unique + +### Symptômes + +- Erreur 500 générique au lieu d'un conflit 409, intermittent et difficile à reproduire + +### Bonnes pratiques / mitigations + +- Toujours catcher l'erreur Prisma `P2002` (unique constraint violation) dans le bloc catch de la création et la transformer en réponse métier explicite (`USER_ALREADY_EXISTS`, `CONFLICT`, etc.) +- Le check préalable reste utile pour le cas nominal mais ne doit pas être la seule protection +- **Règle** : tout `prisma.create` sur une entité à contrainte unique doit avoir un catch `P2002` + +- Contexte technique : Prisma / concurrence — RL799_V2 08-04-2026 + +--- + + +## Double update non transactionnel sur la même entité + +### Risques + +- Enchaîner deux `prisma.update` séparés sur la même entité (ex: update status puis update metadata) sans transaction laisse l'entité dans un état partiel si le second échoue + +### Symptômes + +- L'entité est dans un état qui ne correspond à aucune transition définie +- Les flux aval (workflows, UI) se retrouvent bloqués + +### Bonnes pratiques / mitigations + +- Quand plusieurs champs d'une même entité doivent être modifiés ensemble pour représenter une transition d'état cohérente, les regrouper dans un seul appel `prisma.update` ou les envelopper dans une `$transaction` +- **Signal review** : deux `prisma.update` séquentiels sur le même `where: { id }` sans `$transaction` + +- Contexte technique : Prisma / atomicité — RL799_V2 08-04-2026 + +--- + + +## Fallback "legacy" qui bypass un nouveau workflow + +### Risques + +- Laisser un fallback de compatibilité dans une transition d'état (ex: accepter `'draft'` en plus de `'pending_vm_approval'` comme état source de la publication) crée un chemin de contournement du nouveau workflow +- Un appel direct à la fonction interne bypass le contrôle métier + +### Symptômes + +- Transition d'état possible depuis un état qui ne devrait plus être accepté +- `{ in: ['ancien', 'nouveau'] }` dans un filtre Prisma sur une transition d'état + +### Bonnes pratiques / mitigations + +- Quand un nouveau workflow remplace un flux direct, retirer le fallback vers l'ancien état dans la transition atomique +- Si la rétro-compatibilité est nécessaire, l'encapsuler dans un flag explicite ou une route dédiée, pas dans un `{ in: ['ancien', 'nouveau'] }` silencieux + +- Contexte technique : workflow / transitions d'état — RL799_V2 08-04-2026 + +--- + + +## Couverture incomplète des chemins d'écriture lors d'ajout de sécurité transverse + +### Risques + +- Quand on ajoute une mesure de sécurité (chiffrement, sanitisation, validation, audit) sur un chemin d'écriture (ex: upload), les chemins alternatifs vers la même ressource (ex: create-from-text, import CSV, seed) sont oubliés, laissant une faille + +### Symptômes + +- Un chemin d'écriture protégé, un autre non protégé, pour la même ressource +- Données non chiffrées ou non sanitisées créées par un chemin secondaire + +### Bonnes pratiques / mitigations + +- Avant de marquer une tâche sécurité comme terminée, lister TOUS les chemins d'écriture vers la ressource ciblée (grep `createDocument`, `writeFile`, `prisma.model.create`) et vérifier que chacun est couvert +- Documenter la liste dans les Dev Notes de la story + +- Contexte technique : sécurité / transverse — RL799_V2 08-04-2026 + +--- + + +## Anonymisation RGPD — pièges courants + +### Risques + +- **Données personnelles dans les audit logs** : stocker email ou nom dans les métadonnées d'un audit log d'anonymisation annule le droit à l'oubli (art. 17) +- **Password en clair après anonymisation** : remplacer le password par une string comme `'ANONYMIZED'` est un signal exploitable en base +- **Dernier admin anonymisable** : le système peut se retrouver sans administrateur +- **Sessions non révoquées** : le JWT existant reste valide jusqu'à expiration + +### Symptômes + +- Email en clair dans les métadonnées d'audit après anonymisation +- Password `'ANONYMIZED'` visible en base au lieu d'un hash bcrypt structurellement invalide +- Aucun admin actif restant après anonymisation +- Utilisateur anonymisé qui peut encore accéder à l'application + +### Bonnes pratiques / mitigations + +- Utiliser un hash tronqué (ex: `sha256(email).slice(0,12)`) pour la corrélation dans les audit logs, pas l'email brut +- Remplacer le password par un hash bcrypt structurellement invalide (ex: `$2b$10$INVALID...`) +- Vérifier qu'au moins un admin actif reste après anonymisation +- Révoquer tous les refresh tokens du user dans la même transaction + +- Contexte technique : RGPD / sécurité — RL799_V2 08-04-2026 + +--- + + +## Assertions de body d'erreur non mises à jour après migration de codes + +### Risques + +- Lors d'une migration de codes d'erreur (ex: distinguer `UNAUTHORIZED` de `FORBIDDEN`), les assertions de status HTTP sont mises à jour mais pas les assertions sur le body JSON + +### Symptômes + +- `assert.equal(response.status, 401)` passe, mais `assert.equal(body.error.code, 'FORBIDDEN')` échoue — le code réel est `UNAUTHORIZED` +- Détecté seulement quand on relance la suite complète + +### Bonnes pratiques / mitigations + +- Lors de toute modification d'un code d'erreur dans un helper centralisé, rechercher TOUTES les assertions qui testent l'ancien code (`grep -rn 'FORBIDDEN' __tests__/`) et les mettre à jour en cohérence +- Ne pas se fier au fait que "les tests passent" sans les exécuter réellement + +- Contexte technique : tests / migration de codes — RL799_V2 08-04-2026 + +--- + + +## Audit conditionnel sur un lookup DB secondaire + +### Risques + +- Le handler utilise `requireRoleAccess` qui ne retourne que `{ email, role }` mais pas `userId`. Pour logger l'audit, un second lookup (`getUserByEmail`) est nécessaire et peut échouer silencieusement + +### Symptômes + +- Une action sensible (création, promotion, suppression) réussit mais aucun log d'audit n'est écrit +- Le caller est pourtant authentifié + +### Bonnes pratiques / mitigations + +- Sur tout endpoint qui doit journaliser un audit, utiliser un helper qui retourne `{ userId, email, role }` directement depuis le JWT `sub` claim, plutôt qu'un helper + lookup DB +- Le userId est déjà dans le token — pas besoin d'aller le chercher en base + +- Contexte technique : audit / auth — RL799_V2 08-04-2026 + +--- + + +## Dérive du format d'erreur API entre services + +### Risques + +- Chaque service recrée sa propre fonction `errorResponse` locale au lieu de réutiliser un helper centralisé +- Le format standard `{ error: { code, message, requestId } }` n'est pas imposé par le typage + +### Symptômes + +- Certaines routes API retournent `{ error: { code, message } }` sans `requestId`, d'autres incluent le `requestId` +- Les erreurs sans requestId sont impossibles à tracer en production + +### Bonnes pratiques / mitigations + +- Tout nouveau service doit inclure `requestId: crypto.randomUUID()` dans ses réponses d'erreur +- Factoriser un helper `createApiErrorResponse` partagé dans `lib/` pour éviter la divergence +- **Signal review** : réponse d'erreur sans `requestId` dans un nouveau service + +- Contexte technique : observabilité / API — RL799_V2 08-04-2026 + +--- + + +## Unicité applicative sans contrainte DB + +### Risques + +- Un contrôle d'unicité uniquement applicatif sur des affectations "actives" reste vulnérable aux races concurrentes et peut créer deux titulaires actifs sur un même rôle + +### Symptômes + +- Deux enregistrements actifs pour un slot censé être unique (ex: deux titulaires pour un rôle d'officier) +- Bug intermittent sous charge concurrente, invisible en dev + +### Bonnes pratiques / mitigations + +- Pour tout agrégat avec invariant "un seul actif" (ex: mandat d'officier par rôle), imposer une contrainte d'unicité au niveau base (index unique partiel ou stratégie équivalente) en plus des checks service +- Les checks applicatifs seuls ne suffisent pas sous concurrence + +- Contexte technique : Prisma / contraintes — RL799_V2 08-04-2026 diff --git a/knowledge/backend/risques/nextjs.md b/knowledge/backend/risques/nextjs.md index 76587a3..8927c7b 100644 --- a/knowledge/backend/risques/nextjs.md +++ b/knowledge/backend/risques/nextjs.md @@ -120,3 +120,47 @@ return buildLocalizedPath(locale, "home"); - **Règle** : `os.tmpdir()` est interdit pour les fichiers qui seront renommés vers un volume Docker bind-mounté. - Contexte technique : Node.js / Docker / fichiers media — app-template-resto 31-03-2026 + +--- + + +## CSP strict casse les routes HTML dans une API Next.js + +### Risques + +- Appliquer un CSP strict (`default-src 'self'`) via `headers()` dans `next.config.ts` sur toutes les routes `/api/*` casse les routes qui servent du HTML avec des dépendances externes (Swagger UI, pages de documentation, etc.) + +### Symptômes + +- La page HTML se charge mais reste blanche — les scripts et styles externes sont bloqués silencieusement par le navigateur + +### Bonnes pratiques / mitigations + +- Quand on ajoute des headers de sécurité via un pattern global, vérifier s'il existe des routes qui servent du HTML (pas uniquement du JSON) +- Si oui, ajouter une règle dédiée plus spécifique **en premier** dans le tableau `headers()` (Next.js matche dans l'ordre de déclaration) avec un CSP adapté +- Factoriser les headers communs dans une constante partagée + +- Contexte technique : Next.js / CSP — RL799_V2 08-04-2026 + +--- + + +## Middleware Next.js — faux argument "Edge Runtime incompatible" pour éviter les imports + +### Risques + +- Dupliquer la logique métier dans `middleware.ts` au lieu d'importer depuis `src/lib/` sous prétexte que "Edge Runtime ne supporte pas les imports cross-boundary" +- C'est faux pour les modules purs (pas de `fs`, `child_process`, `crypto` Node-only, etc.) + +### Symptômes + +- Logique dupliquée entre middleware et utilitaires, dérive garantie à la prochaine modification +- Commentaire dans le code justifiant la duplication par une incompatibilité Edge non vérifiée + +### Bonnes pratiques / mitigations + +- Le middleware Next.js peut importer n'importe quel module local tant que celui-ci n'utilise que des API compatibles Edge (Web APIs, `process.env`, opérations JS pures) +- Factoriser la logique partagée dans un module commun et l'importer depuis le middleware +- **Signal review** : logique dupliquée dans `middleware.ts` avec un commentaire "Edge incompatible" + +- Contexte technique : Next.js / middleware — RL799_V2 08-04-2026 diff --git a/knowledge/backend/risques/prisma.md b/knowledge/backend/risques/prisma.md index 0bf2a02..e2bbf31 100644 --- a/knowledge/backend/risques/prisma.md +++ b/knowledge/backend/risques/prisma.md @@ -384,3 +384,26 @@ Checklist minimale après `prisma migrate resolve --applied` : - **Signal review** : si le code suppose l'unicité, la base doit l'imposer explicitement - Contexte technique : Prisma / contraintes DB — RL799_V2 06-04-2026 + +--- + + +## Catch-all silencieux dans les repositories Prisma + +### Risques + +- Un `try { prisma.update(...); return true } catch { return false }` dans un repository masque TOUTES les erreurs Prisma (connexion perdue, timeout, contrainte violée) derrière une réponse "not found" +- Le service appelant ne peut pas distinguer un échec technique d'une absence de données + +### Symptômes + +- Le service retourne 404 alors que la DB est down, ou 404 alors qu'une contrainte FK est violée +- Le monitoring ne voit aucune erreur 500 + +### Bonnes pratiques / mitigations + +- Dans les repositories Prisma, catcher uniquement `Prisma.PrismaClientKnownRequestError` avec le code spécifique attendu (`P2025` pour record not found, `P2002` pour unique constraint) +- Re-throw toute autre erreur pour qu'elle remonte en 500 dans le handler +- **Signal review** : `catch {` ou `catch (e) {` sans vérification de `e.code` dans un repository Prisma + +- Contexte technique : Prisma / error handling — RL799_V2 08-04-2026 diff --git a/knowledge/frontend/patterns/tests.md b/knowledge/frontend/patterns/tests.md index 93621c8..1d359b8 100644 --- a/knowledge/frontend/patterns/tests.md +++ b/knowledge/frontend/patterns/tests.md @@ -90,3 +90,45 @@ Si un test vérifie un *comportement* (ex: "le menu se ferme après clic"), il d - Validé le : 03-04-2026 - Contexte technique : Vue 3 / node:test — RL799_V2 story 6A.8 + +--- + + +## Pattern : Vérifier l'ordre DOM avec `compareDocumentPosition`, pas `boundingBox` + +### Synthèse + +- **Objectif** : valider l'ordre réel des éléments dans le DOM, indépendamment du rendu CSS. +- **Contexte** : tests E2E Playwright qui doivent vérifier l'ordre d'affichage de sections ou d'éléments. +- **Quand l'utiliser** : toute assertion d'ordre dans un test E2E. +- **Quand l'éviter** : si on veut explicitement tester la position visuelle CSS (rare). + +### Analyse + +- **Avantages** : + - vérifie l'ordre DOM réel, insensible aux propriétés CSS (`flex-order`, `position`, `transform`) + - pas de `null` en retour (contrairement à `boundingBox()` hors viewport) + - déterministe +- **Limites / vigilance** : + - ne vérifie pas la position visuelle — si le test doit valider un rendu CSS spécifique, `boundingBox` reste pertinent + +### Validation + +- Validé le : 08-04-2026 +- Contexte technique : Playwright / E2E — RL799_V2 story 17-5 + +### Implémentation + +```ts +const aBeforeB = await page.evaluate(() => { + const a = document.querySelector('[data-testid="section-a"]'); + const b = document.querySelector('[data-testid="section-b"]'); + if (!a || !b) return false; + return (a.compareDocumentPosition(b) & Node.DOCUMENT_POSITION_FOLLOWING) !== 0; +}); +expect(aBeforeB).toBe(true); +``` + +### Risque associé + +`boundingBox().y` vérifie la position visuelle rendue par CSS, pas l'ordre dans le DOM. De plus `boundingBox()` retourne `null` pour les éléments hors viewport → crash non déterministe. diff --git a/knowledge/frontend/risques/auth.md b/knowledge/frontend/risques/auth.md index 49c77ca..bea133c 100644 --- a/knowledge/frontend/risques/auth.md +++ b/knowledge/frontend/risques/auth.md @@ -112,3 +112,26 @@ if (user === null || user.role !== 'ADMIN') return ; - **Règle** : tout guard de rôle dans un composant React Native doit utiliser `useEffect` + redirect + rendu vide, jamais un return conditionnel direct - Contexte technique : React Native / Expo Router / Zustand auth — app-alexandrie 24-03-2026 + +--- + + +## Tests structurels obsolètes après migration du mécanisme d'auth + +### Risques + +- Lors de la migration d'un pattern `Authorization: Bearer` vers des cookies httpOnly, les tests structurels (qui mocquent `getSession()` et assertent sur les headers) deviennent tous faux sans que TypeScript ne détecte rien +- Les mocks ne matchent plus le code réel, les tests passent ou échouent silencieusement + +### Symptômes + +- Tests frontend qui mocquent `getSession()` et assertent `Authorization: Bearer` sur les appels `fetch`, alors que le code utilise désormais `apiFetch` avec cookies (`credentials: 'include'`) +- Faux positifs : les tests ne testent plus rien de réel + +### Bonnes pratiques / mitigations + +- Après toute migration d'un mécanisme d'auth frontend, auditer systématiquement les tests de services pour vérifier que les assertions portent sur le bon mécanisme (cookies vs headers) +- Adapter les mocks pour patcher `apiFetch` au lieu de `getSession` + `fetch` brut +- **Signal review** : `Authorization: Bearer` dans les tests alors que le code production utilise `credentials: 'include'` + +- Contexte technique : Vue 3 / auth migration — RL799_V2 08-04-2026 diff --git a/knowledge/frontend/risques/navigation.md b/knowledge/frontend/risques/navigation.md index f68f670..ea75bcd 100644 --- a/knowledge/frontend/risques/navigation.md +++ b/knowledge/frontend/risques/navigation.md @@ -254,3 +254,26 @@ const routes = [ - Ajouter un test qui valide la synchro sur changement de query param (même composant réutilisé, navigation intra-page) - Contexte technique : Vue 3 / Vue Router 4 — RL799_V2 02-04-2026 + +--- + + +## Vue Router — faux sentiment de sécurité avec tests textuels sur guards + +### Risques + +- Des assertions de type `content.includes("requiresRoles: ['admin']")` valident la présence de configuration mais pas le comportement réel du `beforeEach` +- Un changement dans le guard redirige mal (`login` vs `home`) ou ignore un cas RBAC sans casser les tests + +### Symptômes + +- Les tests passent alors qu'un changement dans le guard redirige vers la mauvaise page +- Régressions de guard runtime invisibles aux tests + +### Bonnes pratiques / mitigations + +- Ajouter au moins un test runtime qui exécute effectivement la logique du guard (session admin/non-admin) et vérifie la valeur retournée par le guard (`true` ou `{ name: 'home' }`) +- Les tests textuels (`includes`) sont acceptables comme smoke structurel mais ne doivent pas être la seule couverture des guards de navigation +- **Signal review** : guards de navigation couverts uniquement par des tests `includes()` sans test comportemental + +- Contexte technique : Vue 3 / Vue Router 4 / node:test — RL799_V2 08-04-2026 diff --git a/knowledge/frontend/risques/tests.md b/knowledge/frontend/risques/tests.md index 5005cdb..e540bd4 100644 --- a/knowledge/frontend/risques/tests.md +++ b/knowledge/frontend/risques/tests.md @@ -148,3 +148,26 @@ source_projects: [app-alexandrie, app-template-resto, RL799_V2] - **Règle** : si un test vérifie un *comportement* (ex: "le menu se ferme après clic"), il doit monter le composant, pas chercher une string dans le source - Contexte technique : Vue 3 / node:test — RL799_V2 02-04-2026 + +--- + + +## Anti-pattern : `.catch(() => false)` + `test.skip` dans les tests E2E + +### Risques + +- Le pattern `await locator.isVisible().catch(() => false)` suivi de `test.skip(true, ...)` masque les erreurs réelles (sélecteur cassé, timeout, changement de structure) derrière un skip silencieux +- Un AC peut rester perpétuellement non testé sans qu'aucun rapport ne le signale comme problème + +### Symptômes + +- Tests skippés en permanence dans les rapports CI +- AC marqués `[x]` dans la story mais jamais réellement validés + +### Bonnes pratiques / mitigations + +- Pour vérifier si un élément optionnel est présent (données dépendantes du seed), utiliser `await locator.count()` qui retourne 0 sans lancer d'exception, puis `test.skip` uniquement si count === 0 +- Réserver `.catch()` aux cas où une exception est réellement attendue et documentée +- **Signal review** : `.catch(() => false)` suivi de `test.skip` dans un test E2E + +- Contexte technique : Playwright / E2E — RL799_V2 08-04-2026 diff --git a/knowledge/workflow/risques/story-tracking.md b/knowledge/workflow/risques/story-tracking.md index 244c4df..42e4dba 100644 --- a/knowledge/workflow/risques/story-tracking.md +++ b/knowledge/workflow/risques/story-tracking.md @@ -227,3 +227,27 @@ source_projects: [app-alexandrie, app-template-resto, RL799_V2] - **Règle review** : tout type défini localement qui correspond à un DTO existant dans shared est une régression - Contexte technique : BMAD / agents autonomes — RL799_V2 07-04-2026 + +--- + + +## Incohérence entre Completion Notes et réalité du code + +### Risques + +- Les dev agents documentent des métriques dans les Completion Notes (nombre de tests, nombre de routes) sans les vérifier en fin d'implémentation +- Le reviewer passe du temps à réconcilier des chiffres qui ne matchent pas + +### Symptômes + +- "20 tests RBAC" revendiqués dans les Completion Notes alors que le fichier test n'en contient que 11 +- Route `DELETE /api/favorites` listée dans le rapport d'audit mais inexistante dans le code + +### Bonnes pratiques / mitigations + +- Avant de passer une story en `review`, le dev agent doit vérifier que les chiffres dans les Completion Notes correspondent exactement au code + - `grep -c 'test(' fichier.test.ts` pour le nombre de tests + - `grep -c 'export' fichier.routes.ts` pour le nombre de routes +- **Règle** : toute métrique dans les Completion Notes doit être vérifiable par une commande simple + +- Contexte technique : BMAD / workflow agent — RL799_V2 08-04-2026