From e8ab0b641a1a362f5860ffd8030846ac9d03b8a9 Mon Sep 17 00:00:00 2001 From: MaksTinyWorkshop Date: Tue, 24 Mar 2026 10:31:58 +0100 Subject: [PATCH] capitalisation: guard admin NestJS silencieux + patterns Epic 4 app-alexandrie MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Anti-pattern: @UseGuards(AdminRoleGuard) sans @RequireAdminRole() → guard inefficace - Anti-pattern: code erreur générique sur statut HTTP sémantique (ALIAS_ALREADY_RESOLVED) - Anti-pattern: guard de rôle via return conditionnel dans render React Native - Pattern: tests e2e scénarios d'autorisation alternatifs avec buildApp isolé - Anti-pattern: méthodes store Zustand qui avalent les erreurs sans rethrow - Anti-pattern: regex globale singleton pour transformation de contenu --- 95_a_capitaliser.md | 135 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 135 insertions(+) diff --git a/95_a_capitaliser.md b/95_a_capitaliser.md index 289be25..946fcc0 100644 --- a/95_a_capitaliser.md +++ b/95_a_capitaliser.md @@ -100,3 +100,138 @@ Ce mécanisme permet : - d'éviter la pollution de la base de connaissance - de capitaliser progressivement l'expérience des projets - de garder `Lead_tech` cohérent et fiable. + +--- + +2026-03-24 — app-alexandrie + +FILE_UPDATE_PROPOSAL +Fichier cible : 10_backend_risques_et_vigilance.md + +Pourquoi : +Code review story 4.7 — pattern récurrent : utiliser `VALIDATION_ERROR` comme code d'erreur sur un 409 rend les erreurs indistinguables côté client et monitoring. Chaque scénario métier doit avoir son propre code. + +Proposition : +**Anti-pattern : code d'erreur générique sur statut HTTP sémantique** +Ne jamais utiliser `VALIDATION_ERROR` ou `INTERNAL_ERROR` sur un 409 CONFLICT. Chaque cas métier doit avoir un code dédié dans `error-code.ts` (ex: `ALIAS_ALREADY_RESOLVED`, `HANDLE_ALREADY_TAKEN`). Raison : les clients (mobile, monitoring, tests) ne peuvent pas distinguer les cas sans un code sémantique. Règle : 1 scénario métier distinct = 1 code d'erreur distinct. + +--- + +2026-03-24 — app-alexandrie + +FILE_UPDATE_PROPOSAL +Fichier cible : 10_frontend_risques_et_vigilance.md + +Pourquoi : +Code review story 4.7 — guard d'accès admin implémenté avec un return conditionnel inline dans le corps du composant React Native, causant un flash et une UX instable quand `user` est encore `null` au montage. + +Proposition : +**Anti-pattern : guard de rôle via return conditionnel dans le render** +Ne jamais faire `if (user?.role !== 'ADMIN') return ` directement dans le corps d'un composant. Pendant le chargement du store auth, `user` peut être `null`, ce qui cause un flash du contenu "Accès refusé" suivi d'un re-render. Pattern correct : `useEffect(() => { if (user !== null && user.role !== 'ADMIN') router.replace('/(tabs)'); }, [user, router])` + rendu vide (`return `) tant que `user === null || user.role !== 'ADMIN'`. + +--- + +2026-03-24 — app-alexandrie + +FILE_UPDATE_PROPOSAL +Fichier cible : 10_backend_risques_et_vigilance.md + +Pourquoi : +Code review story 4.7 — tests e2e manquants pour le scénario non-abonné 403 sur un endpoint protégé par entitlements. Le mock d'entitlements partagé dans le `beforeAll` du describe rendait ce test impossible sans un `buildApp` isolé. + +Proposition : +**Pattern : tests e2e de scénarios d'autorisation alternatifs avec buildApp isolé** +Quand un describe e2e partage un `buildApp` avec des entitlements actifs dans `beforeAll`, les tests de scénarios non-abonnés / inactifs doivent créer leur propre instance via `buildApp({ getEntitlementsForUser: jest.fn().mockResolvedValue({ subscription: { isActive: false, ... } }) })` avec `await app.close()` en fin de test. Ne pas tenter de surcharger le mock partagé (risque de pollution entre tests). + +--- + +2026-03-24 — app-alexandrie + +FILE_UPDATE_PROPOSAL +Fichier cible : 10_frontend_risques_et_vigilance.md + +Pourquoi : +Pattern récurrent dans les stores Zustand mobiles : les méthodes `update*` (updateThread, updateComment) omettent de relancer (`throw`) les erreurs catchées, ce qui avale silencieusement des erreurs métier (ex: UNSAFE_LINK) et empêche l'écran d'afficher un feedback à l'utilisateur. + +Proposition : +**Anti-pattern : méthodes store Zustand qui avalent les erreurs sans rethrow** + +```typescript +// ❌ MAUVAIS — l'erreur est avalée, l'écran ne sait pas que ça a échoué +async updateThread(forumSlug, threadId, body) { + await communityService.updateThread(accessToken, forumSlug, threadId, body); + // Si une erreur est lancée, elle disparaît silencieusement +}, + +// ✅ BON — l'erreur est propagée pour que l'écran puisse réagir +async updateThread(forumSlug, threadId, body) { + try { + await communityService.updateThread(accessToken, forumSlug, threadId, body); + } catch (e) { + const err = e as Error & { code?: string }; + throw err; // Le code d'erreur (ex: UNSAFE_LINK) est préservé sur l'objet + } +}, +``` + +Règle : toute méthode store qui appelle le service réseau DOIT soit (1) relancer l'erreur enrichie avec `throw err`, soit (2) la stocker dans le state (`set({ error: err.message })`). Jamais les deux à la fois sans rethrow si l'écran doit réagir au catch. + +--- + +FILE_UPDATE_PROPOSAL +Fichier cible : 10_frontend_risques_et_vigilance.md + +Pourquoi : +Les regex globales (`/g`) définies comme constantes au niveau module en TypeScript/Node.js maintiennent un état `lastIndex`. Bien que `String.prototype.replace()` réinitialise ce `lastIndex`, l'utilisation future avec `.test()` ou `.exec()` (ex: lors d'un refactor) introduira un bug stateful difficile à détecter. + +Proposition : +**Anti-pattern : regex globale singleton pour transformation de contenu** + +```typescript +// ❌ RISQUÉ — regex globale partagée entre tous les appels +const LINK_PATTERN = /https?:\/\/\S+/gi; +function processLinks(content: string) { + return content.replace(LINK_PATTERN, ...); // OK today + // Mais si quelqu'un ajoute LINK_PATTERN.test(x) ailleurs → bug lastIndex +} + +// ✅ SÛR — nouvelle instance à chaque appel, aucun état partagé +function makeLinkPattern(): RegExp { + return /https?:\/\/\S+/gi; +} +function processLinks(content: string) { + return content.replace(makeLinkPattern(), ...); +} +``` + +Règle : les regex avec flag `/g` ou `/y` utilisées pour la transformation de strings → toujours créer via une factory, jamais en singleton. + +--- + +2026-03-24 — app-alexandrie + +FILE_UPDATE_PROPOSAL +Fichier cible : 10_backend_risques_et_vigilance.md + +Pourquoi : +Story 4.5 Epic 4 — `@UseGuards(AdminRoleGuard)` sans `@RequireAdminRole()` laisse passer tous les utilisateurs silencieusement. Bug de sécurité non évident, détecté uniquement en review adversariale. + +Proposition : +**Anti-pattern : guard admin NestJS sans son décorateur couplé** + +`AdminRoleGuard.canActivate()` lit la metadata `REQUIRE_ADMIN_ROLE_KEY` posée par `@RequireAdminRole()`. Sans le décorateur, `requiresAdmin = false` → le guard approuve tout le monde. + +```typescript +// ❌ DANGEREUX — guard actif mais inefficace +@UseGuards(AdminRoleGuard) +@Get('admin/something') +handler() {} + +// ✅ CORRECT — toujours les deux ensemble +@RequireAdminRole() +@UseGuards(AdminRoleGuard) +@Get('admin/something') +handler() {} +``` + +Règle : tout endpoint admin = `@RequireAdminRole()` + `@UseGuards(AdminRoleGuard)` obligatoirement ensemble. S'applique à tout guard NestJS basé sur la reflection de metadata.