Aller au contenu

PD-177 — Revue de Code

Resume

Critere Statut
Patterns NestJS 8/10
Qualite code 7/10
Gestion erreurs 6/10
Securite 7/10
Maintenabilite 7/10

Verdict : RESERVES

Points positifs

  • Bonne structuration par responsabilite (guard custody, guard exclusivite, facade wallet, recovery, validator preuve, interceptor securite).
  • Conventions TypeScript/NestJS globalement respectees: @Injectable(), DI par constructeur, nommage coherent, interfaces explicites.
  • Presence d'un mecanisme explicite de detection de fuite de secrets avec posture fail-closed.
  • WalletOperationalService applique un controle S2 en amont et trace les operations avec correlation ID (randomUUID).
  • AnchorProofValidator impose signer_address pour les nouveaux artefacts PD-177 et remonte les maillons manquants de facon exploitable.
  • BlockchainModule enregistre correctement les nouveaux composants PD-177 dans providers et exports.

Points a ameliorer

ID Description Fichier Gravite
R-01 getConfirmationPolicy() leve une Error generique au lieu d'une BlockchainError codee, ce qui casse la taxonomie d'erreurs et l'observabilite. src/modules/blockchain/constants/network-confirmation-policy.ts MAJEUR
R-02 SecretLeakInterceptor n'est que declare provider: il n'est pas branche globalement (APP_INTERCEPTOR) ni explicitement sur des routes visibles ici, risque d'ineffectivite du controle. src/modules/blockchain/blockchain.module.ts MAJEUR
R-03 AnchorExclusivityGuard renvoie INVALID_CUSTODY_MODE pour un acces non autorise: code d'erreur semantiquement incorrect pour un refus d'acces. src/modules/blockchain/wallet/anchor-exclusivity.guard.ts MAJEUR
R-04 Scan recursif sans protection contre references circulaires dans SecretLeakInterceptor (Object.entries sur objet arbitraire), risque de boucle infinie/stack overflow donc DoS applicatif. src/modules/blockchain/security/secret-leak.interceptor.ts MAJEUR
R-05 Regex base64 tres large (^[A-Za-z0-9+/]{43,}={0,2}$) peut provoquer des faux positifs eleves (hash/tokens non sensibles) et des blocages HTTP non necessaires. src/modules/blockchain/security/secret-leak.interceptor.ts MINEUR
R-06 WalletOperationalService.initialize() n'empeche pas explicitement une re-initialisation ou des appels concurrents; l'invariant "un seul wallet actif" repose sur l'ordre d'appel externe. src/modules/blockchain/wallet/wallet-operational.service.ts MAJEUR
R-07 network est introduit dans le DTO sans contrainte metier (enum/@IsIn) alors que la policy est strictement polygon/arbitrum; coherence contractuelle incomplete. src/modules/anchor/dto/proof-artifact.dto.ts MINEUR
R-08 La promesse "EIP-55 checksummed" n'est pas verifiee (regex hex simple), ce qui cree un ecart entre documentation et validation reelle. src/modules/anchor/dto/proof-artifact.dto.ts MINEUR

Detail par fichier

network-confirmation-policy.ts

  • Patterns/Nommage: propre, constantes UPPER_SNAKE_CASE, type Readonly<Record<...>> pertinent.
  • Types: interface ConfirmationPolicy claire, pas de any.
  • Point de vigilance: getConfirmationPolicy() fait un double if alors que l'acces direct NETWORK_CONFIRMATION_POLICY[network] suffit avec fallback explicite.
  • Ecart principal: exception generique Error au lieu de BlockchainError + BlockchainErrorCode structure.
  • Impact: homogenite de handling d'erreurs rompue (monitoring, mapping HTTP, testabilite).

custody-mode.guard.ts

  • Patterns NestJS: bon usage de @Injectable() et DI implicite.
  • Lisibilite: methode assertS2Mode explicite, comportement simple et testable.
  • Securite: posture fail-closed correcte (throw si mode != S2).
  • Logging: niveau error adapte en cas d'ecart, mais le log log de succes peut etre verbeux en prod (mineur).
  • Conclusion: composant globalement conforme.

secret-leak.interceptor.ts

  • Points forts: fail-closed explicite, aucune journalisation de valeur sensible, couverture des 3 familles (hex key, base64 key material, mnemonic).
  • Gestion d'erreurs: catchError rescane l'erreur puis relance, coherent avec la posture securite.
  • Ecart critique: pas de gestion des cycles d'objets (ex: erreur riche contenant references circulaires), risque de crash sur recursion.
  • Robustesse: manque de garde sur profondeur/taille d'objet; un payload volumineux peut impacter la latence.
  • Precision detection: regex base64 tres permissive -> faux positifs possibles; un ajustement contextuel (cles sensibles + longueur + entropie/nom de champ) ameliorerait le ratio signal/bruit.

wallet-operational.service.ts

  • Architecture: bonne facade operationnelle, responsabilite bien delimitee.
  • DI: usage correct de CustodyService + CustodyModeGuard.
  • Invariants: enforcement S2 effectif via guard.
  • Ecart majeur: absence de verrou logique/idempotence sur initialize() (double init, concurrence, ecrasement d'etat possibles).
  • Observabilite: correlation ID present; adresse wallet loggee (acceptable car publique).
  • Maintenabilite: mode: 'S2' hardcode dans getOperationalState() est coherent avec la garde, mais rend la methode moins evolutive si S2 n'est plus unique.

anchor-exclusivity.guard.ts

  • Scope: implementation compatible avec la contrainte MVP (whitelist statique, explicitement acceptee).
  • Lisibilite: simple, comprenable, facilement testable.
  • Ecart majeur: mauvais code d'erreur (INVALID_CUSTODY_MODE) pour un controle d'autorisation; devrait etre un code dedie type UNAUTHORIZED_WALLET_ACCESS/ANCHOR_EXCLUSIVITY_VIOLATION.
  • Securite: fail-closed correct (throw immediat).

wallet-recovery.service.ts

  • Conformite au stub: simulation de reprise correctement documentee (pas d'integration KMS reelle attendue ici).
  • Qualite: API claire (executeRecoveryExercise, getRecoveryProcedure), structures de rapport lisibles.
  • Testabilite: composant deterministe et simple a couvrir en unit tests.
  • Mineur: lastUpdated fixe en dur, risque d'obsolescence documentaire; preferable de lier a une version/config.

anchor-proof.validator.ts

  • Points forts: separation validate (non-throw) / assert (throw), bon pattern pour reutilisation.
  • Gestion d'erreur: BlockchainError avec code structure + contexte (lotId, missingLinks) correcte.
  • PD-177: verification signer_address obligatoire bien presente.
  • Reserve mineure: network n'est pas verifie alors qu'il est ajoute au DTO PD-177; a confirmer selon contrat d'export final.

proof-artifact.dto.ts (champs PD-177)

  • Patterns DTO: decorators class-validator corrects, messages explicites.
  • Compatibilite: @IsOptional() coherent avec retrocompatibilite indiquee.
  • Reserve: network accepte toute string; aligner sur un enum (polygon|arbitrum) renforcerait la robustesse.
  • Reserve: commentaire annonce EIP-55 checksummed mais la regex ne valide pas le checksum (simple format hex).

blockchain.module.ts

  • Structure NestJS: module propre, organisation claire des blocs PD-52/PD-177.
  • DI/Providers: tous les services PD-177 sont enregistres et exportes.
  • Ecart majeur: l'intercepteur de securite est seulement fourni, pas force globalement via APP_INTERCEPTOR (ni preuve d'usage local), donc garantie transversale possiblement non appliquee.