[148] Le job du tableau de bord : borné, découpé, et toujours bloquant #149
No reviewers
Labels
No labels
Compat/Breaking
EC01
EC02
EC03
EC04
EC05
EC06
Kind/BDD
Kind/Back
Kind/Bug
Kind/CICD
Kind/Cloud
Kind/Contenu
Kind/Data
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Front
Kind/IA
Kind/Infra
Kind/Monitoring
Kind/Security
Kind/Testing
Portée/Post-jury
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
ops/alerte
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
g2/enervision!149
Loading…
Reference in a new issue
No description provided.
Delete branch "lenaic/148-chaine-tableau-de-bord"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Ferme #148. Reprise après la relecture d'Olivier : ses trois points bloquants
sont traités, et le remède du troisième commandait les deux autres.
Les quatre défauts du ticket
1. L'appel npm n'était pas borné.
fetch-timeoutvaut 300 000 ms par défautet
fetch-retriesvaut 2, sans rien dans le dépôt pour le figer. D'où les échecsmassés à 433-435 s : une signature de délai avec réessais, pas un travail à durée
variable.
Les bornes entrent dans
services/dashboard/.npmrc, pas en option de ligne decommande : la chaîne, un poste et un conteneur doivent se comporter pareil.
Le pire cas est d'environ 3 min 55 s, pas trois minutes. Trois requêtes de
60 s, plus 5 s et 50 s d'attente entre essais, le facteur de npm valant 10.
J'avais écrit trois minutes dans trois endroits, Olivier l'a relevé, et l'écart
compte parce que tout ce dossier s'argumente sur des durées mesurées.
2. Une seule étape faisait trois choses. Séparées, et l'audit affiche sa
durée. Deux détails corrigés :
dateplutôt que$SECONDS, que dash ne connaîtpas, et
|| code=$?sans quoish -esortirait avant la ligne qui l'affiche.3. Le cache se rabattait sur une clé approchée. Les deux caches coexistent
maintenant :
/root/.npmavec repli, parce qu'il est adressé par contenu etvérifié par empreinte, et
node_modulesà clé exacte sans repli, parce qu'unarbre construit pour un autre verrou passe les tests et casse ailleurs.
4. Un
apt-getà chaque passage pour un contrôle d'expiration du CA que lejob Python fait déjà, inconditionnellement, dans la même exécution. Au passage,
NODE_EXTRA_CA_CERTSétait posé après les commandes npm qui en avaient besoin.Le cinquième, trouvé en chemin, et mal corrigé du premier coup
L'inventaire ne s'exécutait plus depuis que l'audit échouait. Ma première
correction séparait les étapes, ce qui ne change rien : dans Actions, une étape
en échec interrompt les suivantes. L'inventaire passe désormais avant
l'audit, plutôt qu'un
if: always()qui le produirait aussi quand l'installationa échoué et qu'il n'y a rien à inventorier.
Le verdict sort du YAML, et c'est le coeur de la reprise
Le banc restait vert sur trois désarmements qu'il annonçait refuser :
continue-on-error: truesur l'étape d'audit, la neutralisation idiomatiqued'Actions, dont la clé figure déjà deux fois dans le même job ;
restore-keysposé au-dessus dekey:, l'ordre des clés d'un mapping YAMLétant libre alors que mon
awkne balayait que les lignes d'après ;process.exit(0)en gardantv.highdans un affichage : le banc cherchait lessous-chaînes, pas leur usage dans la décision de sortie.
Le verdict vit donc dans
.forgejo/scripts/verdict-audit-npm.js, ettests/ci/test-verdict-audit-npm.shl'exécute contre sept rapports figés :sain, moderée seule, CVE haute, CVE critique, registre injoignable, rapport
tronqué, rapport sans décompte.
Une assertion sur du texte ne dit rien de ce que le code décide.
--audit-level=highcesse d'être surveillé sur la commande qui produit le JSON :l'option n'agit que sur le code de sortie de npm, que l'étape jette puisqu'elle
recalcule son verdict. Ma garde protégeait un drapeau sans effet, et faisait
croire protégé ce qui ne l'était pas.
Changement de politique, tranché
Une panne du registre npm ne bloque plus la chaîne. Erreur de registre →
::warning::et job vert. Une CVE haute ou critique bloque toujours.C'est au-delà du ticket, donc ça se décide et ne se déduit pas du code. Tranché
le 4 septembre, après que la panne a bloqué toute l'équipe trois fois dans la
matinée, dont une demande qui ne touchait pas une ligne de JavaScript.
Ce qu'on accepte : une dépendance vulnérable introduite pendant une panne du
registre passe, et n'est rattrapée qu'au passage suivant. Ce qu'on refuse :
immobiliser six personnes parce qu'un service tiers ne répond pas, sans que
personne puisse rien y corriger.
La décision est écrite dans
docs/runbooks/ci.md, avec ce qu'elle coûte. Lemessage de la chaîne le dit aussi en clair : « Les dépendances du tableau de bord
ne sont PAS contrôlées sur ce passage ».
Un rapport illisible échoue, il ne passe pas pour une panne : un JSON tronqué
ressemble à une panne, et le prendre pour telle laisserait passer une CVE.
Éprouvé
Dix-huit défauts réintroduits un par un, dix-huit rouges, vert sur l'arbre
sain. Le verdict est éprouvé en l'exécutant, le workflow en le lisant.
Deux trous trouvés en éprouvant mes propres correctifs. La garde sur la
délégation trouvait le nom du script dans le commentaire de l'étape, comme
celle sur
apt-getavant elle. Et chercher un seulexit 1ne suffisait pas :en retirant celui de la branche des vulnérabilités, celui de la branche
« rapport illisible » subsistait et le banc restait vert. Le banc compare
maintenant le nombre d'erreurs annoncées au nombre de sorties en échec, et exige
une branche par défaut.
Correction sur le séquencement avec la #145
J'avais écrit « les deux se fusionnent sans conflit dans un sens ou dans
l'autre ». C'est faux, et Olivier l'a vérifié :
ci.ymlpasse, mais les deux demandes réécrivent le même tableau des tâches dumanuel avec des contenus différents. Ma vérification n'avait porté que sur le
fichier qui, lui, passe. Le conflit se résout en quelques minutes, il faut
simplement le savoir avant de fusionner.
Ce que cette demande ne prouve pas
La durée médiane sous 90 s ne se mesure qu'après fusion, sur dix exécutions. Je
joindrai le relevé avant/après au ticket.
Relu sur
refs/pull/149/head(472ec34) : banc rejoué, désarmements du garde-fouéprouvés un par un, séquencement avec la #145 testé par fusion à blanc.
Le diagnostic et les quatre correctifs sont justes. Ce qui reste à traiter tient
presque entièrement au banc, qui laisse passer trois neutralisations qu'il
annonce refuser — et c'est lui qui rend le reste sûr, d'où le blocage.
Ce qui tient
Vérifié plutôt que cru sur parole :
.npmrcplutôt qu'en option de ligne de commande : bonarbitrage, et exactement le critère 1.
apt-getest justifiée : le contrôle d'expirationexiste bien côté Python (
ci.yml:274,openssl x509 -checkend 0), et il estinconditionnel — aucun des quatre jobs n'a d'
if:, deneeds:ni de filtrepaths, donc il tourne toujours dans la même exécution.étape) est une vraie trouvaille, et l'inventaire avant l'audit plutôt qu'un
if: always()est le bon correctif, pour la bonne raison.NODE_EXTRA_CA_CERTSdéplacé avant les appels npm : bug latent réel.raisonnement correct.
dateau lieu de$SECONDS, et|| code=$?soussh -e: les deux justes.Intégration / *(confirmé parl'API) : l'affirmation du manuel est exacte.
tests/ci/passent, et le nouveau est bien câblédans
images.Bloquant — le banc reste vert sur trois désarmements
Chaque cas a été injecté dans
ci.yml, banc rejoué, résultat VERT :a)
continue-on-error: truesur l'étape « Audit des dépendances ». C'estla neutralisation idiomatique d'Actions, et la clé figure déjà deux fois dans
le même job sur les étapes de cache : un copier-coller d'une ligne, le jour où
une fusion presse — exactement le scénario que décrit la demande. Le banc ne
cherche que la présence d'un
exit 1, qui subsiste intact.b) Verdict remplacé par
process.exit(0),v.highetv.criticalgardésdans un
console.log:Banc vert, CVE plus bloquantes. Le contrôle cherche les deux sous-chaînes
n'importe où dans l'étape, pas leur usage dans la décision de sortie.
c)
restore-keysplacé au-dessus dekey:dans le cache de l'arbre.L'
awkde la section 3 ne balaie que les lignes aprèskey: dashboard-node,or l'ordre des clés d'un mapping YAML est libre. Le témoin (
restore-keysaprès
key:, ce qui a été éprouvé) rougit bien, lui.Le contrôle qui rattrape (a) est un
grep. Pour (c), il suffit de balayer toutle bloc
with:. Pour (b), il faut que l'assertion porte sur l'expression desortie, ce qui suppose d'extraire le verdict dans un fichier exécutable — voir
la dernière section.
Bloquant —
--audit-level=highsur la commande décisive n'influence plus rien--audit-levelest documenté comme n'agissant que sur le code de sortie denpm, code que l'étape jette délibérément par
|| truepuisque le verdict estrecalculé sur le JSON. L'option est donc inerte sur le résultat.
Le « contrôle qui compte » du banc porte donc sur un drapeau sans effet : le
retirer rend le banc rouge sans rien changer au comportement — un faux positif,
pas une régression rattrapée. La garantie réelle est le décompte
v.high/v.critical, c'est-à-dire précisément celle que le point (b)contourne. Le commentaire (« en la retirant de la commande décisive, le banc
restait vert ») décrit bien le symptôme, mais attribue la garde au mauvais
endroit. L'option garde son sens dans le chemin d'échec, pour réafficher le
rapport en clair.
Bloquant — conflit avec la #145 sur le manuel
La demande conclut « les deux se fusionnent sans conflit ». Pour
ci.yml, c'estvrai. Mais :
Deux régions, et pas un conflit d'adjacence : les deux demandes réécrivent le
même tableau des tâches avec des contenus différents — la #145 y intègre en plus
les libellés de l'exécuteur et le corollaire « les bancs de
tests/ci/tournentsans interpréteur Python ». Elle touche
ci.mdà +120/-30. Le raisonnement deséquencement n'a porté que sur le fichier qui, lui, passe.
À trancher en équipe — l'audit devient ouvert en cas de panne du registre
Sémantique nouvelle : erreur de registre →
::warning::et job vert. Lecritère 5 reste tenu (une CVE haute bloque toujours), et le manuel est honnête
là-dessus (« L'avertissement n'est pas un contrôle réussi »). Mais c'est un
changement de politique au-delà du ticket, et la section « Le garde-fou n'est
pas désarmé » de la description ne le mentionne pas : qui ne lit que la
description ne saura pas qu'une fusion faite pendant une panne npmjs part non
auditée. À nommer dans le corps de la demande, et probablement en point du
matin — ça touche la portée pratique d'ENF-11.
Une affirmation à corriger
« Trois minutes au pire » apparaît dans
.npmrc, dans la description etdans le manuel. La demande ajoute
fetch-retry-mintimeout=5000etfetch-retry-maxtimeout=60000, qui ne sont pas dans le ticket. Avec le facteur10 par défaut : 3 × 60 s de requêtes + 5 s + 50 s d'attente entre essais, soit
≈ 3 min 55 s. L'écart est modeste, mais toute la demande s'argumente sur des
durées mesurées.
À noter aussi : le banc ignore ces deux réglages et ne borne pas
fetch-retries—grep -qE '^fetch-retries='n'en vérifie que la présence, sibien que
fetch-retries=50resterait vert pour un pire cas d'environ 50 min.Broutilles
fetch-retriesest ancré^fetch-retries=alors que celui defetch-timeoutjuste au-dessus tolère l'indentation : une ligne indentéeserait déclarée absente. Incohérent entre deux contrôles voisins.
apt-get» balaie tout le bloc, commentaires compris : un futurcommentaire expliquant pourquoi il n'y en a pas rougirait le banc. À
restreindre aux lignes
run:.grep -qE '^\s+exit 1$':\sest une extension GNU. En conteneur Debianc'est bon, mais qui joue le banc sur un poste macOS aura un échec fantôme.
Sur le doublon, et le remède au point (b)
J'avais ouvert une branche sur le même ticket, que je viens de fermer au profit
de celle-ci. Un morceau mérite d'être repris ici, parce qu'il ferme exactement
le trou du point (b) : le verdict y était extrait dans
.forgejo/scripts/verdict-audit-npm.jset exécuté contre cinq fixtures —saine, CVE haute, CVE critique, point d'accès injoignable, JSON illisible — dont
une sortie réelle de
npm audit --jsoncapturée sur npm 10.9.8 en délaid'attente. Vérifier la logique en la faisant tourner, au lieu de la constater par
sous-chaînes, rend du même coup les points (a) et (b) sans objet et documente la
distinction CVE / réseau par des cas plutôt que par un commentaire. Je peux
pousser ces fixtures et ce banc sur ta branche si tu veux.
Trois points bloquants, trois corrigés, et le remède du troisième commandait les
deux autres.
Le verdict vit maintenant dans
.forgejo/scripts/verdict-audit-npm.js, ettests/ci/test-verdict-audit-npm.shl'exécute contre sept rapports figés :sain, moderée seule, CVE haute, CVE critique, registre injoignable, rapport
tronqué, rapport sans décompte. Ta démonstration du
process.exit(0)avecv.highgardé en affichage était imparable : une assertion sur du texte ne ditrien de ce que le code décide.
Sur
--audit-level=high, tu as raison et j'avais tort. L'option n'agit quesur le code de sortie de npm, que l'étape jette puisqu'elle recalcule son
verdict. Ma garde protégeait un drapeau sans effet, et me faisait croire protégé
ce qui ne l'était pas. Je l'ai supprimée, pas déplacée.
Le
continue-on-erroret lerestore-keysau-dessus dekey:sont couverts,tous deux éprouvés rouges.
Deux trous trouvés en éprouvant mes propres correctifs
La garde sur la délégation trouvait le nom du script dans le commentaire de
l'étape, comme celle sur
apt-getavant elle. Et chercher un seulexit 1nesuffisait pas : en retirant celui de la branche des vulnérabilités, celui de la
branche « rapport illisible » subsistait et le banc restait vert. Il compare
maintenant les erreurs annoncées aux sorties en échec, et exige une branche par
défaut.
Dix-huit défauts réintroduits un par un, dix-huit rouges, vert sur l'arbre sain.
Le chiffre était faux
3 min 55 s et non trois minutes, corrigé aux trois endroits. Et le banc borne
désormais
fetch-retries, dont il ne vérifiait que la présence :fetch-retries=50passait, pour un pire cas d'environ cinquante minutes.Le conflit
Réel, vérifié, et la description est corrigée : ma vérification n'avait porté que
sur
ci.yml. Il a disparu depuis, la #145 ne réécrivant plus le tableau destâches du manuel. Ça répond du même coup au point de périmètre de Gabriel sur
l'autre demande.
La politique
Tranchée, dans le sens de l'ouverture. Une panne du registre avertit sans
bloquer, une CVE bloque toujours. C'est écrit dans
docs/runbooks/ci.mdavec ceque ça coûte : une dépendance vulnérable introduite pendant une panne passe, et
n'est rattrapée qu'au passage suivant.
Merci pour tes fixtures. Elles n'ont plus lieu d'être puisque le banc les porte,
mais l'idée d'éprouver en exécutant vient de toi, et c'est elle qui a fermé le
trou.
Relu sur
b755aef. Les trois points bloquants sont levés, et j'ai vérifiéchacun en réinjectant le défaut :
continue-on-error: truesur l'étape d'auditrestore-keysau-dessus dekey:process.exit(0)au lieu debloquantes > 0exit 1retiré de la seule branche vulnérabilités*)suppriméexit(3)→exit(0)fetch-retries=50apt-getdans un commentaireLe conflit avec la #145 a bien disparu (
merge-treepropre contrepr/145etcontre
develop), le 3 min 55 s est corrigé aux trois endroits, et le bancverdict échoue franchement si
nodemanque au lieu de se sauter en silence —comme
imagesutiliseactions/checkout, node y est de toute façon garanti.Les six bancs sont verts.
Sortir le verdict du YAML pour l'éprouver par exécution était la bonne réponse,
et le cas
sans-decompte.json → 2ferme un trou que je n'avais pas soulevé :conclure « aucune vulnérabilité » d'une absence de décompte aurait été le pire
des deux.
J'ai poussé un commit sur ta branche :
1198fb7Deux désarmements passaient encore au vert sur les deux bancs. Comme ce sont
deux gardes de quelques lignes et qu'elles relèvent de la même famille que les
précédentes, je les ai écrites plutôt que de te renvoyer un troisième tour —
mais dis-le si tu préfères les réécrire, le commit se défait sans peine.
A. Une étape sautée est une étape verte. Rien ne contrôlait la condition de
l'étape d'audit. Les trois écritures suivantes laissaient les deux bancs verts
en supprimant l'audit :
La troisième est la plus vraisemblable — « on n'audite que sur develop pour
accélérer les demandes » — et elle retire l'audit précisément là où il sert,
sur les demandes de fusion. La condition est désormais comparée exactement à
celle des autres étapes du tableau de bord.
B. Le fil entre le verdict et l'échec de l'étape. Éprouver ce que le script
décide ne sert à rien si sa décision n'est pas branchée :
verdictreste à 0, on tombe dans0), l'étape annonce « aucune vulnérabilitéhigh ou critical » et sort verte avec une CVE haute — script et fixtures
intacts, banc verdict vert. Idem en réaffectant
verdictaprès la capture. Lebanc exige maintenant la capture, et l'absence de réaffectation avant le
case.C'est le
|| truecontre lequel tout ce fichier met en garde, remonté d'uncran : de
npm audit || trueàverdict-audit-npm.js || true. Ça vaut peut-êtreune ligne dans le manuel : la garde se déplace avec la logique.
Huit défauts réintroduits un par un, huit rouges ; les six contrôles déjà en
place restent rouges sur leurs propres injections, et l'arbre sain reste vert.
Et une quatrième occurrence du même piège, sur moi cette fois : ma première
écriture de la garde B plaçait sa fenêtre d'analyse sur le commentaire qui nomme
le script, donc en amont de l'initialisation
verdict=0, qu'elle prenait pourun écrasement — arbre sain rouge. Les deux nouvelles gardes ne lisent que les
lignes exécutées, comme celles sur
apt-getet sur la délégation.Approuvé. Je fusionne dès que la chaîne est verte sur
1198fb7.