Aller au contenu

PD-280 — Revue de Code

Résumé

Critère Statut
Patterns NestJS ⚠️
Qualité code ⚠️
Gestion erreurs ⚠️
Maintenabilité ⚠️

Verdict : ❌ NON_CONFORME

Motif principal: l’axe obligatoire “preuve contractuelle des guards d’accès” n’est pas couvert (MAJEUR).

Points positifs

  • Bonne séparation des responsabilités: ProofVerificationController / ProofVerificationService / ProofVerificationMapperService / VerificationContractInterceptor.
  • DI NestJS proprement appliquée (@InjectRepository, providers module, config injectable).
  • Contrat métier PD-280 globalement bien traduit côté mapping (PENDING -> null, calcul verificationStatus, clamp nextCheckAfterSeconds).
  • Interceptor riche en validations de cohérence de sortie et tests négatifs nombreux.
  • Structuration des codes d’erreur centralisée via PROOF_VERIFICATION_ERROR_CODES.

Points à améliorer

ID Description Fichier Gravité
R-01 Preuve contractuelle guards absente pour endpoint protégé (@UseGuards + @Roles): aucun test n’asserte 403 exact, body vide, appel audit trail. src/modules/integrity/controllers/proof-verification.controller.ts:24, src/modules/integrity/controllers/__tests__/proof-verification.controller.spec.ts:52 MAJEUR
R-02 Les tests controller contournent les guards (overrideGuard(...).useValue({ canActivate: () => true })), donc aucune vérification réelle du contrat d’accès. src/modules/integrity/controllers/__tests__/proof-verification.controller.spec.ts:52 MAJEUR
R-03 Trace corrélable incomplète dans l’interceptor: invariant ERR-280-03 demande requestId + proofId, mais seul proofId est loggé et renvoyé. src/modules/integrity/interceptors/verification-contract.interceptor.ts:57, src/modules/integrity/interceptors/verification-contract.interceptor.ts:193 MAJEUR
R-04 Migration non alignée avec le contrat “extension enum PostgreSQL check_result à 4 valeurs”: commentaire explicite “No ALTER TYPE needed”. Si l’invariant INV-280-01 est DB-level, la preuve manque. src/database/migrations/1741400000000-PD-280-AddProofVerificationJobs.ts:8 MAJEUR
R-05 L’interceptor ne valide pas explicitement le domaine de verificationStatus (PENDING|DONE), une valeur inconnue peut passer certains contrôles. src/modules/integrity/interceptors/verification-contract.interceptor.ts:121 MAJEUR
R-06 mapLinkValue retourne null en fallback sur valeur inconnue: risque de masquer une corruption de données au lieu d’échouer fail-closed. src/modules/integrity/services/proof-verification-mapper.service.ts:153 MAJEUR
R-07 DTO peu strict: pendingReason?: string au lieu d’un enum typé + validation runtime DTO (lisibilité contrat API diminuée). src/modules/integrity/dto/proof-verification-response.dto.ts:35 MINEUR

Détail par fichier

proof-verification.controller.ts

  • Bon usage NestJS: @UseGuards(JwtAuthGuard, AuthorizationGuard) + @Roles('user'), ParseUUIDPipe v4 avec exceptionFactory propre.
  • Le endpoint est correctement en GET read-only.
  • Point de vigilance: pas de mécanisme visible d’audit d’accès refusé au niveau de ce périmètre (peut être géré ailleurs, non prouvé ici).

proof-verification.controller.spec.ts

  • Bonne couverture nominale/erreurs fonctionnelles (400/404, propagation, payload).
  • Manque bloquant (axe obligatoire): pas de tests d’accès refusé.
  • Les guards sont mockés en “always allow”, donc impossible de prouver le contrat sécurité.
  • À ajouter: tests d’intégration/e2e pour 403, body rejet vide, audit trail appelé.

verification-contract.interceptor.ts

  • Très bon volume de validations contractuelles (clés linkResults, valeurs autorisées, interdiction string PENDING, bornes nextCheckAfterSeconds).
  • Écart MAJEUR: pas de requestId dans logs/erreur corrélable (ERR-280-03).
  • Écart MAJEUR: verificationStatus non validé explicitement contre enum autorisé.
  • Suggestion: fail-closed direct si statut hors {PENDING,DONE}.

verification-contract.interceptor.spec.ts

  • Bon niveau de tests négatifs et cas limites.
  • Ne couvre pas la présence d’un requestId corrélable dans les erreurs/logs (lié à R-03).

proof-verification.service.ts

  • Structure claire, lisible, testable.
  • Bon guard métier proof_finalized => no PENDING.
  • Erreurs structurées correctes (PROOF_NOT_FOUND, VERIFICATION_CONTRACT_BROKEN).
  • Pas de mutation: conforme à l’intention read-only.

proof-verification-mapper.service.ts

  • Mapping global cohérent avec la spec (PENDING interne -> null API, statut dérivé proprement).
  • Clamp conforme CA-10.
  • Écart MAJEUR: fallback silencieux vers null sur valeur invalide (mapLinkValue) au lieu d’exception.
  • Maintenabilité correcte, code simple à relire.

1741400000000-PD-280-AddProofVerificationJobs.ts

  • Migration lisible, indexation pertinente, garde en down migration utile.
  • Écart MAJEUR potentiel de conformité contractuelle: pas de preuve d’extension enum DB check_result à 4 valeurs alors que le contrat l’énonce explicitement.
  • Si la stratégie officielle est “varchar au lieu d’enum DB”, il faut documenter ce delta contractuel explicitement dans la story (sinon non-conformité formelle).

Axe obligatoire — preuve contractuelle des guards d’accès

Endpoint protégé identifié: - GET /proofs/:proofId/verification via @UseGuards(JwtAuthGuard, AuthorizationGuard) + @Roles('user') dans src/modules/integrity/controllers/proof-verification.controller.ts:24

Vérifications requises: 1. Assertion HTTP exact (403) → ABSENTE
2. Assertion body de rejet vide → ABSENTE
3. Assertion appel audit trail → ABSENTE

Conclusion axe obligatoire: MAJEUR (non respecté).