Repository navigation
ci: a check of an absence in a step can fail: no command negated with ! (FRU-222) - #292
Conversation
There was a problem hiding this comment.
Score de confiance : 4/5 · if grep …; then exit 1; fi, et le test de garde couvre le cas. Le seul point restant est un trou du test, de sévérité mineure.
Niveau lite · anthropic/claude-haiku-5.5 · 0,005 $ · Le résumé, le diagramme et la liste des constats sont à la fin de la description de la PR.
|
Sakuga Review (lite, 4/5) on cb8b763, one P2 ( |
… ! (FRU-222) GitHub runs a step with bash -e, and bash does not stop on a command that ! inverts. The steps of the console looked for unsafe-inline and for the origin of the other worker with ! grep: with either in the policy, the step stayed green. They now exit by themselves, and workflows.test.ts fails on a negated command in any workflow. Co-Authored-By: Claude Opus 5.5 <[email protected]>
cb8b763 to
ae08b17
Compare
There was a problem hiding this comment.
Score de confiance : 4/5 · ! grep de deux étapes par des tests qui échouent réellement, et la garde de workflow est ajoutée. Le seul point à corriger est une garde qui laisse passer des formes proches de ! grep.
Niveau lite · anthropic/claude-haiku-5.5 · 0,005 $ · Le résumé, le diagramme et la liste des constats sont à la fin de la description de la PR.
For Mathieu to read: CI and the release workflow. Stack: #290 → this PR. Merge #290 first.
Refs FRU-222 (follow-up of #290: the one finding of its heavy review, P2, true)
The defect
GitHub runs a
runstep withbash -e, and bash does not applyerrexitto a command that!inverts. Three checks of the jobdocker imageand one ofcheck-consolewere written! grep -q …: with'unsafe-inline'in the policy of the console, or the origin of one worker in the policy of the other, the step stayed green. The step that #290 replaced onmainhad the same form.The fix
ci.yml: a functionabsent, which exits by itself;release-image.yml:if … grep -q …; then exit 1; fi.workflows.test.ts: a new case fails on any line of a workflow that negates a command with!, with a control on the form that was there.docs/decisions/image.md: the rule.Proof
'unsafe-inline'inscript-src, and the step « Boot the console for two workers… » extracted from each version ofci.ymland run against it as written. The step of feat(console): one image for every instance, published beside the worker, and a service of the compose file (FRU-222) #290 exits 0; the step of this PR exits 1 with « found « script-src[^;]*unsafe-inline » in /tmp/acme, where it must not be ». On the image as it is, the new step exits 0.workflows.test.tsfails on the workflows of feat(console): one image for every instance, published beside the worker, and a service of the compose file (FRU-222) #290 (it names the three lines ofci.ymland the one ofrelease-image.yml) and passes here.Not verified
check-consoleofrelease-image.ymlhas not run: no dispatch, no push to a registry.🤖 Generated with Claude Code
Sakuga Review · Score de confiance : 4/5
La PR peut être fusionnée : le correctif remplace les
! grepde deux étapes par des tests qui échouent réellement, et la garde de workflow est ajoutée. Le seul point à corriger est une garde qui laisse passer des formes proches de! grep.Constats
!négatifsapps/worker/src/workflows.test.ts:448La regex
^\s+! \S.*$ne voit que les lignes qui commencent par!. Une étape écriterun: ! grep -q x fichiersur une seule ligne, une liste- ! grep …, ou une chaînegrep a && ! grep bne sont pas détectées. Sousbash -e, ces formes ne font pas échouer l'étape, et le test reste vert. Correction : Élargir la regex pour attraper!aprèsrun:,-,&&,||,|ou;, et ajouter un témoin pour chacune de ces formes.absentréussit si le fichier est absent.github/workflows/ci.yml:405absentappellegrep -qpuis agit seulement si la sortie vaut 0. Si le fichier de contrôle n'existe pas, grep sort avec 2, la condition est fausse, et la fonction réussit sans rien vérifier. L'étape reste verte alors que la vérification n'a pas eu lieu. Le CLAUDE.md demande un contrôle qui échoue sur une violation planté, et aucun n'est ajouté ici. Correction : Dansabsent, tester le code de sortie de grep : sortie 0 échoue, sortie 2 échoue aussi avec un message, sortie 1 réussit. Ajouter un contrôle qui vérifie que la fonction échoue sur une politique contenant la chaîne.Résumé
La PR remplace les
! grepdans les étapes deci.yml(fonctionabsent) et derelease-image.ymlparif grep …; then exit 1; fi, car sousbash -eun!ne fait pas échouer l'étape. Elle ajoute un test dansworkflows.test.tsqui cherche les lignes commençant par!et une section dansimage.md. J'ai vérifié les deux nouvelles formes et le test : elles échouent bien quand la condition est présente. Je n'ai pas pu exécuter les workflows ni vérifier les numéros de ligne exacts deci.yml.Non relu
absentnon vérifiés, les lignes de la PR ont été comptées approximativementworkflows()n'est pas dans le diff, je n'ai pas pu confirmer qu'elle lit tous les workflowsRevues (2) · Dernier commit relu : ci: a check of an absence in a step can fail: n... · Niveau
lite·anthropic/claude-haiku-5.5· 0,005 $