Aller au contenu

PD-81 -- Revue de Code

Resume

Critere Statut
Patterns NestJS OK/RESERVES
Qualite code OK/RESERVES
Gestion erreurs OK/RESERVES
Maintenabilite OK/RESERVES
Conformite contracts RESERVES/KO

Verdict : RESERVES

Points positifs

  • Bonne separation controller/service/adapter/manager, avec des controllers globalement "thin" et une orchestration metier centralisee.
  • Patterns NestJS globalement respectes (DI explicite, guards dedies, services specialises, DTOs cotes controller).
  • Tracabilite probatoire bien structuree dans services/legal-audit-trail.service.ts (hash -> HSM -> TSA -> persistence) avec fail-closed sur HSM/TSA.
  • Verification des invariants critiques bien presente sur plusieurs flux (contextId, scopeDocumentIds, TTL, statut ACTIVE, out-of-scope).
  • Absence de any, typage TypeScript globalement propre et lisible, nommage de classes/methodes coherent.
  • Bon decoupage fonctionnel autour de LegalValidationAdapterService et LegalReKeyManagerService conforme a l'intention d'architecture.

Points a ameliorer

ID Description Fichier Gravite
R-01 Violation de pattern contractuel: injection directe @InjectRepository(LegalReKey) interdite par code contract (doit passer par LegalReKeyRepository). services/legal-composite-proof.service.ts MAJEUR
R-02 Endpoint GET /access/:legalReKeyId/status delegue vers consultDocument, provoque effets de bord (audit "document accessed"), semantique incorrecte et type de retour incoherent. controllers/legal-pre.controller.ts MAJEUR
R-03 Mutations d'etat (expireReKey, closeByEndOfConsultation) executees hors transaction SERIALIZABLE, en tension avec la regle "aucune mutation d'etat hors SERIALIZABLE". services/legal-rekey-manager.service.ts MAJEUR
R-04 Les transactions "SERIALIZABLE" utilisent queryRunner, mais les lectures passent par rekeyRepository (hors queryRunner.manager) : garantie transactionnelle potentiellement incomplete. services/legal-rekey-manager.service.ts, services/legal-destruction.service.ts MAJEUR
R-05 Contrat "Retry-After header" non respecte: la garde renvoie retryAfter dans le body uniquement. guards/legal-rate-limit.guard.ts MINEUR
R-06 Canonicalisation JSON partielle (tri de cles seulement au niveau racine) alors que le commentaire annonce RFC8785; risque de non-determinisme si payload imbrique. services/legal-audit-trail.service.ts MINEUR
R-07 Dependance injectee non utilisee (hashService) -> bruit technique et baisse de lisibilite. services/legal-rekey-manager.service.ts MINEUR

Analyse detaillee par fichier

  • services/legal-pre-orchestrator.service.ts Orchestration claire et lisible, bonne delegation vers services intermediaires. Les controles de contexte et d'etat sont explicites. Bonne emit des evenements probatoires aux moments cles. Point de vigilance: le service est un peu "large", mais reste maintenable vu la nature orchestrateur.

  • services/legal-rekey-manager.service.ts Bonne encapsulation du cycle de vie ReKey et conformite generale de nommage. Controle des invariants critiques present. En revanche, coherence transactionnelle a renforcer (mutations hors SERIALIZABLE + usage mixte queryRunner/repository). Qualite globale bonne mais ecarts contractuels structurants.

  • services/legal-destruction.service.ts Design propre, flux de destruction lisible, idempotence bien geree, zeroization explicite. Logging utile (debug/warn/log/error). Meme reserve transactionnelle que ci-dessus: la lecture via repository peut contourner la session du queryRunner.

  • services/mandate-validator.service.ts Service net, sequence de validation bien ordonnee, erreurs metier explicites et typage propre. Gestion d'erreur TSP non silencieuse et fail-closed. Tres bonne testabilite (methode auxiliaire validateTspFields).

  • services/legal-validation-adapter.service.ts Bon role d'adaptation entre contextes Legal PRE et PD-82, mapping explicite, controle de l'identite du second validateur avant delegation. Code lisible et localement coherent. Maintenabilite correcte grace au decouplage.

  • services/legal-audit-trail.service.ts Pipeline probatoire solide et bien structure, distinction probatif/informatif pertinente, fail-closed bien applique la ou requis. Logging d'erreurs present. Point technique a ajuster: canonicalisation annoncee plus forte que l'implementation reelle.

  • services/legal-composite-proof.service.ts Structure claire des 5 sections de preuve, precondition "pas de ACTIVE ReKey" bien appliquee. Bonne separation des builders prouvant une bonne maintenabilite. Non-conformite contractuelle majeure sur l'acces LegalReKey via repository direct.

  • controllers/legal-pre.controller.ts Controller globalement thin et propre, guards et decorateurs NestJS bien utilises. Les routes metier principales sont bien deleguees. Defaut important sur getAccessStatus (delegation a la mauvaise methode, contrat API non respecte).

  • guards/legal-pre-access.guard.ts Guard simple, lisible, robuste pour son scope, messages d'erreur explicites. Pattern NestJS correct, pas de logique parasite. Maintenable.

  • guards/legal-write-block.guard.ts Bonne defense-in-depth, comportement de blocage clair, emission informative non bloquante coherente. Le catch vide est intentionnel ici et acceptable fonctionnellement. Qualite globale bonne.

  • guards/legal-rate-limit.guard.ts Logique metier claire et fail-closed sur indisponibilite Redis. Bonne observabilite via event informatif sur depassement de quota. Ajustement necessaire pour respecter strictement le contrat de reponse HTTP (header Retry-After).

Recommandations

  1. Corriger en priorite controllers/legal-pre.controller.ts pour que GET .../status appelle une methode read-only dediee (getLegalAccessStatus) sans effet de bord.
  2. Aligner strictement les mutations d'etat sur des transactions SERIALIZABLE coherentes de bout en bout (lectures/ecritures via queryRunner.manager).
  3. Remplacer @InjectRepository(LegalReKey) par LegalReKeyRepository dans services/legal-composite-proof.service.ts pour respecter le contrat.
  4. Completer LegalRateLimitGuard avec ajout du header Retry-After en plus du body.
  5. Durcir la canonicalisation JSON (deep sort RFC8785) ou ajuster le commentaire pour eviter un contrat implicite trompeur.