chore(agents): auditer ce qui marche, pas seulement ce qui existe - #277
Merged
Conversation
Les 16 agents QA vérifiaient tous la même chose sous des angles différents :
la présence d'un mécanisme. Aucun ne demandait s'il fonctionne. Trois
incidents en trois mois sont sortis exactement par ce trou, et tous les
trois étaient verts pendant qu'ils échouaient :
- H3 : un client service_role dégradé en authenticated, toutes les
écritures d'audit refusées pendant 3 mois sans qu'une seule erreur
remonte (logAuditEvent avale ses propres échecs par conception)
- purge_audit_log_older_than_12_months() : SECURITY DEFINER sur une
table FORCE RLS, renvoie 0 qu'elle ait purgé ou été refusée, jamais
appelée depuis avril
- Playwright E2E : 214 passed, 173 skipped, tous les parcours connectés
dans les 173, `gh pr checks` vert du début à la fin
La preuve la plus gênante était dans gdpr-compliance-auditor lui-même,
dont la checklist affirmait « exportUserData() returns a complete JSON
bundle » (7 tables sur 14) et « executeDeletion() wipes » (aucun
appelant depuis avril). Il aurait validé les deux bugs qu'on a passé
deux jours à corriger.
Un agent créé, trois corrigés — plutôt qu'un agent par symptôme :
silent-failure-auditor (nouveau, Opus) — question unique : « si ça
s'arrêtait cette nuit, qu'est-ce qui serait différent demain matin ? ».
Écritures à zéro ligne prises pour des succès, chemins privilégiés
refusés en silence, catch qui mangent la preuve, tâches planifiées
jamais armées, gates qui passent à vide, mesures déclarées jamais
construites. Classe par durée d'invisibilité, pas par gravité.
rls-flow-tester — testait uniquement le sens attaquant. Ajoute le sens
privilégié : FORCE RLS s'applique aussi au propriétaire, un DEFINER
possédé par postgres peut écrire 0 ligne sans erreur, et le postgres
hébergé n'est pas celui de la stack locale. Exige des nombres de
lignes mesurés, pas « aucune erreur ».
security-auditor — les routes sous src/app/api/** échappent au matcher
du proxy : pas de session, donc le seul garde-fou est celui qu'on
écrit. Ajoute fail-closed, comparaison de digests (timingSafeEqual
lève sur des longueurs inégales), pas de secret en URL, sémantique
sans réessai, plafond qui doit être bruyant. Plus : la sous-permission
est une vulnérabilité, art. 32(1)(b) tombe sans perte de données.
gdpr-compliance-auditor — les deux affirmations fausses corrigées en
questions à poser au code, et un chapitre « déclaré vs implémenté » :
deux des trois constats les plus coûteux n'étaient pas des bugs, mais
des phrases publiées dans cinq locales que le code ne soutenait pas.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @thierryvm, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
status:review-needed
Contributor
Guide du relecteurCette PR affine plusieurs agents d’audit QA et en introduit un nouveau pour se concentrer sur les défaillances silencieuses, en les faisant passer d’assertions de type checklist à des procédures fondées sur des preuves qui mettent l’accent sur le comportement mesuré, les chemins privilégiés et les garanties déclarées vs implémentées, tout en mettant à jour la documentation globale de CLAUDE pour intégrer le nouvel agent et clarifier quand chaque auditeur doit être exécuté. Diagramme de flux pour la méthode silent-failure-auditor sur un mécanismeflowchart TD
A["Enumerate mechanism\n(e.g. audit log purge, cron, RLS path, CI gate)"] --> B["Ask: 'If this stopped tonight,\nwhat changes tomorrow?' "]
B --> C{"Failure would be observable?"}
C -- "Yes" --> D["Check implementation\nvia Read / Grep / Bash"]
D --> E["Label findings\nMEASURED / READ_IN_CODE / INFERRED"]
E --> F["Record mechanism + evidence\n+ visibility in findings table"]
C -- "No" --> G["Mark SILENT_FAILURE_POSSIBLE"]
G --> H["Describe observability gap:\nwhat signal would surface failure?"]
H --> F
F --> I["Rank by expected\nduration of invisibility"]
I --> J["Emit report with verdict\n& observability recommendations"]
Modifications au niveau des fichiers
Conseils et commandesInteragir avec Sourcery
Personnaliser votre expérienceAccédez à votre tableau de bord pour :
Obtenir de l’aide
Original review guide in EnglishReviewer's GuideThis PR refines multiple QA auditing agents and introduces a new one to focus on silent failures, shifting them from checklist-style assertions to evidence-based procedures that emphasize measured behavior, privileged paths, and declared-vs-implemented guarantees, while updating the global CLAUDE documentation to integrate the new agent and clarify when each auditor must run. Flow diagram for silent-failure-auditor method on a mechanismflowchart TD
A["Enumerate mechanism\n(e.g. audit log purge, cron, RLS path, CI gate)"] --> B["Ask: 'If this stopped tonight,\nwhat changes tomorrow?' "]
B --> C{"Failure would be observable?"}
C -- "Yes" --> D["Check implementation\nvia Read / Grep / Bash"]
D --> E["Label findings\nMEASURED / READ_IN_CODE / INFERRED"]
E --> F["Record mechanism + evidence\n+ visibility in findings table"]
C -- "No" --> G["Mark SILENT_FAILURE_POSSIBLE"]
G --> H["Describe observability gap:\nwhat signal would surface failure?"]
H --> F
F --> I["Rank by expected\nduration of invisibility"]
I --> J["Emit report with verdict\n& observability recommendations"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Le trou
Les 16 agents QA posaient tous, chacun à sa manière, la même question : est-ce présent ?
Aucun ne demandait : est-ce que ça marche, et le saurait-on si ça s'arrêtait ?
Trois incidents en trois mois sont sortis par ce trou. Les trois étaient verts pendant
qu'ils échouaient :
service_roledégradé enauthenticated; toutes les écritures d'audit refusées.logAuditEvent()avale ses erreurs par conception.purge_audit_log_older_than_12_months()SECURITY DEFINERsur une tableFORCE RLS: renvoie0qu'elle ait purgé ou été refusée.Playwright E2ELa preuve la plus gênante était dans
gdpr-compliance-auditorlui-même. Sa checklistaffirmait :
exportUserData()returns a complete JSON bundle » → 7 tables sur 14executeDeletion()wipes » → aucun appelant depuis avrilIl aurait validé les deux bugs qu'on a passé deux jours à corriger.
Ce que fait cette PR
Un agent créé, trois corrigés — plutôt qu'un agent par symptôme.
silent-failure-auditor(nouveau, Opus)Question unique : « si ça s'arrêtait cette nuit, qu'est-ce qui serait différent demain
matin ? ». Si la réponse honnête est « rien d'observable », c'est un constat — même si le
code est correct aujourd'hui.
Six terrains de chasse : écritures à zéro ligne prises pour des succès, chemins privilégiés
refusés en silence,
catchqui mangent la preuve, tâches planifiées jamais armées, gatesqui passent à vide, mesures déclarées jamais construites.
Deux règles de méthode : mesurer plutôt que raisonner (il a
Bashet la stack locale),et étiqueter chaque affirmation MEASURED / READ IN CODE / INFERRED / UNVERIFIABLE HERE.
Classement par durée d'invisibilité, pas par gravité — un bug critique qui hurle est
moins dangereux ici qu'un bug moyen qui ne hurlera jamais.
rls-flow-tester— le sens qui manquaitIl ne testait que la direction attaquant. Il teste maintenant aussi la direction
privilégiée :
FORCE RLSs'applique au propriétaire de la table, unSECURITY DEFINERpossédé par
postgrespeut écrire 0 ligne sans erreur, et lepostgreshébergé n'estpas celui de la stack locale. Il doit désormais rapporter des nombres de lignes mesurés,
pas « aucune erreur ».
security-auditor— endpoints non authentifiés + sous-permissionsrc/app/api/**est exclu du matcher du proxy (src/proxy.ts:139) : pas de session dutout. Six points bloquants ajoutés (fail-closed, comparaison de digests parce que
timingSafeEquallève sur des longueurs inégales, pas de secret en URL, sémantique sansréessai, plafond bruyant…), plus le rappel que la sous-permission est une vulnérabilité :
l'art. 32(1)(b) tombe sans qu'aucune donnée ne fuite.
gdpr-compliance-auditor— déclaré vs implémentéLes deux affirmations fausses deviennent des questions à poser au code. Nouveau
chapitre : deux des trois constats les plus coûteux n'étaient pas des bugs, mais des
phrases publiées dans cinq locales que le code ne soutenait pas.
Portée et risque
Markdown uniquement — aucun code applicatif, aucune migration, aucune dépendance. Zéro
impact runtime, donc aucun check CI ne peut valider le fond : la revue est humaine.
Deux réserves que je signale plutôt que de les taire :
plan-reviewern'a pas été invoqué. La règle vise le code > 50 lignes et les cheminssensibles (Server Actions,
proxy.ts, migrations…). Ce sont des prompts. Il tourne enparallèle sur le plan de l'étape 3b, autrement plus conséquent. Arbitrage assumé, à
contredire si tu préfères la règle littérale.
copie corrigée par son auteur. Les trois incidents cités sont mesurés et vérifiables,
mais le choix de ce qu'on cherche reste le mien.
🤖 Generated with Claude Code
Summary by Sourcery
Introduire un nouvel agent QA axé sur les défaillances silencieuses et renforcer les auditeurs existants afin de vérifier que les protections et les engagements RGPD fonctionnent réellement plutôt que de simplement exister.
Nouvelles fonctionnalités :
silent-failure-auditorpour détecter les mécanismes pouvant échouer sans impact observable, en couvrant les journaux d’audit, les écritures privilégiées, les tâches en arrière-plan, les garde-fous CI et les tâches de rétention.Améliorations :
gdpr-compliance-auditorafin de valider l’exhaustivité des exports de données utilisateur, le comportement d’effacement, ainsi que l’alignement entre les déclarations légales visibles par l’utilisateur et l’implémentation réelle à travers les différentes locales.rls-flow-testerpour couvrir les chemins RLS privilégiés, en garantissant que les opérationsservice_roleetSECURITY DEFINERne sont pas bloquées silencieusement et que les résultats sont mesurés via les comptes de lignes.security-auditorpour les endpoints non authentifiés et planifiés, en imposant un comportement fail-closed, une gestion sûre des secrets, des comparaisons en temps constant correctes, et des vérifications préalables explicites avant les opérations destructrices.CLAUDE.mdpour documenter le nouvel agent QA dédié aux défaillances silencieuses, les responsabilités élargies derls-flow-testeret le nombre accru d’agents QA.Original summary in English
Summary by Sourcery
Introduce a new QA agent focused on silent failures and strengthen existing auditors to verify that protections and GDPR promises actually work rather than just exist.
New Features:
Enhancements: