diff --git a/10_backend_risques_et_vigilance.md b/10_backend_risques_et_vigilance.md index 775ea15..a14c24f 100644 --- a/10_backend_risques_et_vigilance.md +++ b/10_backend_risques_et_vigilance.md @@ -8,7 +8,7 @@ Ce fichier recense des risques back-end susceptibles de provoquer : - régressions coûteuses, - incohérences de données. -Dernière mise à jour : 23-03-2026 +Dernière mise à jour : 24-03-2026 --- @@ -64,6 +64,8 @@ Dernière mise à jour : 23-03-2026 - [Redirect vers la page désactivée elle-même (boucle infinie feature flags)](#risque-redirect-boucle-infinie) - [Champ `tenantId` sans FK ni relation Prisma vers `Tenant`](#risque-tenantid-sans-fk-relation) - [NestJS `@UseGuards(AdminRoleGuard)` sans `@RequireAdminRole()` — silencieusement ouvert](#risque-adminroleguard-sans-decorateur) +- [Code d’erreur générique sur 409 (conflict)](#risque-code-erreur-generique-409) +- [Tests e2e d’autorisation avec buildApp isolé](#risque-e2e-autorisation-buildapp-isole) --- @@ -1093,3 +1095,51 @@ async showcaseThread(...): Promise { ... } ``` - Contexte technique : NestJS / Zod / contracts-first — app-alexandrie 23-03-2026 + +--- + + +## Code d’erreur générique sur 409 (conflict) + +### Risques + +- Erreurs indistinguables côté client et monitoring +- Tests/automatisations incapables de réagir à des cas métier distincts + +### Symptômes + +- Utilisation de codes génériques (ex: `VALIDATION_ERROR`, `INTERNAL_ERROR`) pour des 409 CONFLICT +- Impossibilité de distinguer “alias déjà pris” vs “autre conflit métier” côté client + +### Bonnes pratiques / mitigations + +- 1 scénario métier distinct = 1 code d’erreur dédié (ex: `ALIAS_ALREADY_RESOLVED`, `HANDLE_ALREADY_TAKEN`) +- Centraliser les codes dans `error-code.ts` et les mapper systématiquement + +--- + + +## Tests e2e d’autorisation avec buildApp isolé + +### Risques + +- Scénarios non‑abonné / droits inactifs impossibles à tester si le `buildApp` partagé active des entitlements en `beforeAll` +- Pollution croisée des tests e2e par partage d’instance + +### Symptômes + +- Impossible de reproduire un 403 “non abonné” dans un `describe` qui mocke des droits actifs globalement + +### Bonnes pratiques / mitigations + +- Créer une instance isolée pour les scénarios alternatifs: + +```ts +const app = await buildApp({ + getEntitlementsForUser: jest.fn().mockResolvedValue({ subscription: { isActive: false, ... } }) +}); +// ... test ... +await app.close(); +``` + +- Ne pas surcharger un mock global partagé; préférer un `buildApp` dédié par scénario diff --git a/10_frontend_risques_et_vigilance.md b/10_frontend_risques_et_vigilance.md index 27d9872..7eb34c0 100644 --- a/10_frontend_risques_et_vigilance.md +++ b/10_frontend_risques_et_vigilance.md @@ -7,7 +7,7 @@ Ce fichier recense des risques front-end susceptibles de provoquer : - dette technique rapide, - régressions UX/perf/a11y. -Dernière mise à jour : 23-03-2026 +Dernière mise à jour : 24-03-2026 --- @@ -60,6 +60,9 @@ Dernière mise à jour : 23-03-2026 - [`useCallback` inutile quand le callback est wrappé en inline au render](#risque-usecallback-inutile-inline) - [Formulaire React avec `defaultValue` sans `key` prop](#risque-formulaire-defaultvalue-sans-key) - [Zustand : optimistic update sur item absent de la liste principale](#risque-zustand-optimistic-update-sous-listes) +- [Guard de rôle via return conditionnel dans le render](#risque-guard-role-return-conditionnel) +- [Méthodes Zustand qui avalent les erreurs (sans rethrow)](#risque-zustand-sans-rethrow) +- [Regex globale singleton (/g) partagée au niveau module](#risque-regex-globale-singleton) --- @@ -1045,3 +1048,80 @@ const target = - Règle complémentaire : ne pas mettre à jour une sous-liste (ex: `pinnedThreads`) lors d'une action qui n'y a pas de rapport (ex: mise en vitrine ne touche pas `pinnedThreads`) - Contexte technique : React Native / Zustand — app-alexandrie 23-03-2026 + +--- + + +## Guard de rôle via return conditionnel dans le render + +### Risques + +- Flash “Accès refusé” pendant le chargement du store auth (`user === null` au montage) +- UX instable (re-render successif qui corrige le faux négatif) + +### Symptômes + +- `if (user?.role !== "ADMIN") return ` directement dans le composant + +### Bonnes pratiques / mitigations + +- Router côté client après détermination du rôle, et rendre un placeholder tant que l’état est inconnu + +```tsx +useEffect(() => { + if (user !== null && user.role !== 'ADMIN') router.replace('/(tabs)'); +}, [user, router]); + +return user === null || user.role !== 'ADMIN' ? : ; +``` + +--- + + +## Méthodes Zustand qui avalent les erreurs (sans rethrow) + +### Risques + +- Erreurs métier (ex: `UNSAFE_LINK`) invisibles pour l’écran → pas de feedback utilisateur + +### Symptômes + +- `try/catch` dans le store qui ne re‑lance pas l’erreur et ne met pas à jour un état d’erreur + +### Bonnes pratiques / mitigations + +- Relancer l’erreur pour que l’écran gère l’UX, ou stocker explicitement l’erreur dans le state (selon design) + +```ts +async updateThread(forumSlug, threadId, body) { + try { + await communityService.updateThread(accessToken, forumSlug, threadId, body); + } catch (e) { + const err = e as Error & { code?: string }; + throw err; // ou set({ error: err.message }) si l’UX l’exige + } +} +``` + +--- + + +## Regex globale singleton (/g) partagée au niveau module + +### Risques + +- État `lastIndex` partagé si la regex est réutilisée avec `.test()`/`.exec()` → bugs stateful difficiles à diagnostiquer + +### Symptômes + +- Regex définie en constante module: `const LINK_PATTERN = /https?:\/\/\S+/gi;` +- Fonction qui “marche” avec `replace` mais casse après un refactor qui ajoute `.test()` + +### Bonnes pratiques / mitigations + +- Créer une instance nouvelle par appel (factory), éviter le singleton module pour `/g` ou `/y` + +```ts +function makeLinkPattern(): RegExp { return /https?:\/\/\S+/gi; } +function processLinks(content: string) { return content.replace(makeLinkPattern(), ...); } +``` \ No newline at end of file diff --git a/95_a_capitaliser.md b/95_a_capitaliser.md index 946fcc0..932c853 100644 --- a/95_a_capitaliser.md +++ b/95_a_capitaliser.md @@ -102,136 +102,3 @@ Ce mécanisme permet : - 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.