fix(security): le client service_role fuitait la session utilisateur (H3, #192)#273
Merged
Conversation
…ssion `createAdminClient()` paired the service_role key with a cookie adapter, so @supabase/ssr found the caller session and sent that JWT as `Authorization` instead of the key. Every query silently ran as `authenticated` — a role `audit_log` denies. Measured: same key, same table, same database, INSERTED without cookies and 42501 permission denied with them. Known since 2026-05-28 as H3 (#192) and never measured until now. On an uncorrected build, a successful login plus an account rename left NOTHING in audit_log: only events emitted before a session cookie exists were ever written, and logAuditEvent swallows its error, so nothing ever reported it. Replaced by createServiceRoleClient() in a new cookie-free module. Not a GRANT on audit_log — that would open the audit trail to every signed-in user, the exact opposite of migration 20260417000003. Two defects this fix would otherwise ACTIVATE are closed here: - executeDeletion logged GDPR_DELETION_COMPLETED with resource_id set to the UUID it had just erased, re-identifying the account one row after deletion - the audit pseudonymisation result was never checked, so a refusal left identifying rows behind while the deletion carried on Tests: network-contract assertion on the outgoing identity, a static import ban, the first tests src/lib/gdpr/ has ever had, and an authenticated e2e spec proven red on the uncorrected build and green after. Authenticated floor measured 24 -> 25; public floor unchanged at 215 (the spec skips there). Closes #192 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.
|
…tion
Both defects were dormant only because audit writes were being refused. This
commit makes them land, so it has to fix them too.
Export truncation was silent and arbitrary. `audit_log` is capped at 1000 rows
with no `order`, so PostgREST returned rows in physical order — a user past the
cap would have received an unpredictable subset of their own audit trail inside
a file the UI presents as a complete art. 20 export. Now ordered newest-first,
and the cap is asserted.
Deletion requests were logged twice. `requestAccountDeletionAction` already
emits GDPR_DELETION_REQUESTED with the request IP and user agent;
`requestDeletion` emitted a second, poorer row without them. The library keeps
the write, the caller keeps the audit line.
Test coverage on the paths this PR repointed but never exercised:
- requestDeletion and cancelDeletion were 0% covered and are both reachable
from settings.ts; cancelDeletion's two-filter scope is now pinned, since
dropping the status filter would rewrite already-processed requests
- export.ts was 0% covered with no test file at all. Every one of the seven
queries now has its scoping column asserted by name — with a real
service_role client, an unscoped one would hand a user someone else's
financial data, and RLS no longer backstops that
- the "never a module-level singleton" invariant admin.ts documents is now
tested rather than merely stated
The fake Supabase client caught the export change on its own, which is the
point of building it to the real chain shape.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tach `persistSession: false` stops a session being STORED, not HELD. A later `.auth.setSession()` or `.auth.signInWithPassword()` on the returned client would have re-opened H3 without anyone editing admin.ts — the guarantee was behavioural, and neither the static import test nor the header test could see it. The security audit measured exactly that: the service_role key overwritten by a user JWT, on a client this module had built. `accessToken` makes the SDK ask a callback for the request identity instead of consulting its own auth state, and it makes the entire `.auth` namespace throw. Measured here: `Authorization: Bearer <service_role>` still on every request, `.auth.getSession()` and `.auth.admin` both refusing. Convention becomes an SDK guarantee. Sealing also hides `auth.admin.deleteUser`, which the erasure flow needs — hence `createServiceRoleAdminClient()`, named the long way round so that reaching for it takes a decision. One legitimate caller: executeDeletion. Also corrects a comment that would have misled the next reader: `user_consents` and `deletion_requests` cascade from `users`, not from `workspaces`. And records on the module the two erasure limits the audit established — audit rows written with a null user_id keep an IP the pseudonymisation can never reach, and the three destructive statements are not one transaction. Both belong to the step that wires executeDeletion to a cron; it still has no caller. Verified end to end: the authenticated e2e spec still passes against a rebuilt server, so audit writes authenticate correctly through the sealed path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds the audit outcomes, the six findings that entered the PR, and the eight tracked out-of-scope items — including the three public texts that become inaccurate at merge and need @Thierry's call. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…n the browser @Thierry approved server-only, so it is installed. Measuring it then contradicted what I claimed for it: a 'use client' page importing the service-role module still BUILDS on Next 16 with Turbopack. Next aliases the package to its own copy, server-side to empty, client-side to the throwing one — so the failure lands when the chunk runs in the browser, not at build. The guard lost with next/headers is not restored by the package. What restores it is a rule in scripts/lint-use-server.mjs, which CI already runs, so no workflow change: any 'use client' module importing the service-role client, the audit logger, or the GDPR modules fails the lint. Falsified — exit 1 with a probe page in place, exit 0 without it. server-only stays: canonical marker, zero cost, a named error instead of an undefined key. It is now documented for what it does rather than what it was supposed to do. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
It said "every row the user owns". It returns 7 of 14 tables. i18n-auditor found it while checking the public copy for the same claim — and a comment that keeps asserting completeness is how the corrected UI text grows back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ction The production table holds four event types and all four are written before a session cookie exists: auth.rate_limited, admin.access.rate_limited (both from unauthenticated requests), auth.signup and auth.password_reset. auth.login has zero rows while the project shows 47 monthly active users. No financial mutation, no GDPR event, no successful admin access ever reached it. So the trail has been blind from 2026-04-16 to 2026-07-27 — the same hole measured locally, not a test artefact. The register entry argues the qualification rather than asserting it: the failure is fail-closed, a degraded client gets FEWER rights, so nothing was exposed or altered and art. 33/34 do not apply. What does apply is art. 32(1)(b) and (d) — the privacy policy declares an append-only audit log as a security measure, and a declared measure that silently did not work for three months is a breach of the duty to test its effectiveness. Art. 5(2) too: there is no way to show when an export was served over that period. Consent proof (art. 7(1)) never travelled through this client and is intact. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Supabase counts distinct users who made an auth request during the billing cycle. npm run e2e:auth exercises the authenticated journeys AGAINST PRODUCTION, seeding throwaway ankora-e2e+<hex>@ankora.test accounts; deleting them at the end does not remove them from the cycle's count. So 47 is mostly test users, and citing it as evidence of real usage was wrong — in a compliance document, which is where it matters most. The argument never needed it. auth.signup and auth.rate_limited rows exist, which proves people signed up and attempted to sign in, while not one successful login was ever recorded. That stands on its own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…lure 5 accounts in production, none of them test users, one signed in within the last 30 days. Three belong to third parties — people close to @Thierry, but data subjects with full rights all the same. I had told him the deletion queue could wait for "the first third-party user"; that user already exists, and existed before this session. Corrected here and in the step-3b notes. Also records an independent cross-check: auth.users.last_sign_in_at is populated for all five accounts, so real logins happened between April and July, while audit_log holds zero auth.login rows. Two sources, one conclusion — the evidence no longer rests on reading the absence of rows alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
thierryvm
added a commit
that referenced
this pull request
Jul 26, 2026
…t », base légale) (#274) ## Trois affirmations publiques devenues fausses Relevées par l'audit RGPD de la PR #273, corrigées ici dans les **cinq locales**. **1. L'export RGPD n'est pas complet.** Le texte annonce « un JSON complet de tout ce qu'Ankora détient sur toi ». Il interroge **7 tables sur 14**. Manquent `commitments` et `commitment_payments` (les dettes) et `accounts` (les soldes) — pas un arrondi, les deux catégories les plus sensibles de l'app. Remplacé par ce que le fichier contient réellement. L'affirmation devient plus problématique aujourd'hui qu'hier : le journal d'activité qu'elle embarque était vide jusqu'au correctif #273, donc le fichier a maintenant l'air assez fourni pour qu'on le croie exhaustif. **2. La base légale du journal d'audit était fausse.** Déclarée « obligation légale ». Aucune loi belge n'impose un journal d'audit à un outil budgétaire — c'est de l'**intérêt légitime** (art. 6(1)(f)), qui suppose une mise en balance et non un devoir statutaire. Se tromper de base n'est pas une approximation de traduction, c'est une déclaration de conformité indéfendable. **3. Même correction** sur la description du bouton d'export dans les Paramètres. ## Ce que l'audit i18n a rattrapé Deux formulations que je ne pouvais pas relire moi-même : - **nl-BE** mélangeait `veiligheid` et `beveiligingslogs` dans la même liste → `behoud van de beveiliging` - **es-ES** écrivait « derecho de portabilidad » là où le règlement dit « derecho a la portabilidad de los datos » Les termes statutaires des bases légales sont désormais **verrouillés au glossaire** (§2bis, v1.4) pour les quatre bases de l'art. 6 — un relecteur natif serait tenté de les « améliorer » en synonymes que le règlement n'emploie pas (`legitiem belang`, `interés justificado`). ## Vérifications Parité des clés intacte (aucune clé ajoutée ni supprimée), balises `<b>` cohérentes sur les 5 locales, 1702 tests verts, typecheck et lint propres. JSON validé fichier par fichier. ## Hors périmètre, signalé L'audit a confirmé une dette connue : les sections `landing.pricing` de nl/de/es sont encore intégralement en français. Sans rapport avec ce correctif, mais c'est la section voisine. Ne rend PAS l'export complet — c'est une PR à part. Celle-ci arrête seulement de promettre ce qui n'est pas livré. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…ontext @Thierry confirmed the facts: five accounts, three of them third parties, and no alert ever reached him during the period. The third parties are family, two of them his children, and none has used the app since April or May — so the practical risk of harm over the blind window is nil. Recorded with the limit stated rather than implied: the household exemption (art. 2(2)(c)) does NOT apply, because Ankora ships publicly with a marketing site, a pricing page and a privacy policy. Family ties lower the risk, they do not remove the obligations — art. 17 in particular, which is why the deletion queue is the next task. Also states the reservation plainly: neither of us is a lawyer. The no-notification conclusion rests on the fail-closed analysis and on measured facts, not on professional advice, and should be revisited if the number of third-party users grows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
thierryvm
added a commit
that referenced
this pull request
Jul 27, 2026
## 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 : | Incident | Déguisement | Durée d'invisibilité | | --- | --- | --- | | H3 (#273) | Client `service_role` dégradé en `authenticated` ; toutes les écritures d'audit refusées. `logAuditEvent()` avale ses erreurs par conception. | **3 mois** | | `purge_audit_log_older_than_12_months()` | `SECURITY DEFINER` sur une table `FORCE RLS` : renvoie `0` qu'elle ait purgé ou été refusée. | Depuis avril — **jamais appelée** | | `Playwright E2E` | 214 passed, **173 skipped** — tous les parcours connectés dans les 173. | Jusqu'à ce que quelqu'un lise la ligne du reporter | La preuve la plus gênante était dans `gdpr-compliance-auditor` lui-même. Sa checklist affirmait : - « `exportUserData()` returns a **complete** JSON bundle » → 7 tables sur 14 - « `executeDeletion()` **wipes** » → aucun appelant depuis avril **Il 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, `catch` qui mangent la preuve, tâches planifiées jamais armées, gates qui passent à vide, mesures déclarées jamais construites. Deux règles de méthode : **mesurer plutôt que raisonner** (il a `Bash` et 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 manquait Il ne testait que la direction attaquant. Il teste maintenant aussi la direction privilégiée : `FORCE RLS` s'applique **au propriétaire de la table**, un `SECURITY DEFINER` possédé par `postgres` peut écrire **0 ligne sans erreur**, et le `postgres` hébergé n'est pas 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-permission `src/app/api/**` est **exclu du matcher du proxy** (`src/proxy.ts:139`) : pas de session du tout. Six points bloquants ajoutés (fail-closed, comparaison de digests parce que `timingSafeEqual` lève sur des longueurs inégales, pas de secret en URL, sémantique sans ré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 :** 1. **`plan-reviewer` n'a pas été invoqué.** La règle vise le code > 50 lignes et les chemins sensibles (Server Actions, `proxy.ts`, migrations…). Ce sont des prompts. Il tourne en parallè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. 2. **J'écris les agents qui auditent mon propre travail.** C'est structurellement une 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](https://claude.com/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 : - Ajouter un agent `silent-failure-auditor` pour 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 : - Renforcer `gdpr-compliance-auditor` afin 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. - Étendre `rls-flow-tester` pour couvrir les chemins RLS privilégiés, en garantissant que les opérations `service_role` et `SECURITY DEFINER` ne sont pas bloquées silencieusement et que les résultats sont mesurés via les comptes de lignes. - Étendre les recommandations de `security-auditor` pour 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. - Mettre à jour `CLAUDE.md` pour documenter le nouvel agent QA dédié aux défaillances silencieuses, les responsabilités élargies de `rls-flow-tester` et le nombre accru d’agents QA. <details> <summary>Original summary in English</summary> ## 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: - Add a silent-failure-auditor agent to detect mechanisms that can fail without observable impact, covering audit logs, privileged writes, background jobs, CI gates, and retention tasks. Enhancements: - Tighten the gdpr-compliance-auditor to validate completeness of user data exports, erasure behavior, and alignment between user-facing legal claims and actual implementation across locales. - Expand rls-flow-tester to cover privileged RLS paths, ensuring service_role and SECURITY DEFINER operations are not silently blocked and that results are measured via row counts. - Extend security-auditor guidance for unauthenticated and scheduled endpoints, enforcing fail-closed behavior, safe secret handling, correct constant-time comparisons, and explicit preflight checks before destructive operations. - Update CLAUDE.md to document the new silent-failure QA agent, the expanded responsibilities of rls-flow-tester, and the increased QA agent count. </details> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
thierryvm
added a commit
that referenced
this pull request
Jul 27, 2026
…ne (ADR-024) (#279) ## Pourquoi cet ADR existe L'étape 3b doit brancher un exécuteur sur une file de suppression que **rien ne vide**, pendant que `/app/settings/deletion-status` affiche à l'utilisateur une date et un décompte de jours restants. Ce n'est pas une omission : c'est une **affirmation inexacte faite à la personne concernée** (art. 12(1)) sur un droit garanti par l'art. 17. Brancher cet exécuteur impose des choix de schéma qui ne se défont pas : statut supplémentaire, colonne de verrou, index unique partiel, fonction SQL, réécriture de deux politiques RLS. ADR-023 ne couvrait que le délai de grâce. Doctrine projet : **décision en session N, implémentation en session N+1** — donc zéro ligne de code applicatif ici. ## Deux conceptions mortes, consignées exprès Rien n'invite plus sûrement à réécrire une mauvaise idée que son absence du dossier. **Conception 1 — fonction `SECURITY DEFINER` atomique. Rejetée.** `FORCE ROW LEVEL SECURITY` s'applique **au propriétaire de la table**. Possédée par `postgres`, la fonction écrirait **zéro ligne sans lever d'erreur** si le `postgres` hébergé n'a pas `BYPASSRLS` — ce qu'on ne peut pas mesurer. Compte supprimé, IP et user-agent conservés, plus aucune clé de jointure pour les retrouver. **Conception 2 — la même en `SECURITY INVOKER` appelée par `service_role`. Rejetée aussi.** Mesuré : `service_role` n'a **aucun privilège** sur `auth.users` ni sur `auth.audit_log_entries`. **Conséquence** : l'atomicité SQL de bout en bout est **impossible par toute conception**. Ce n'est pas une limite de notre code, c'est la frontière entre PostgREST et GoTrue. ## La décision **La garantie devient l'idempotence et la reprise, pas l'atomicité.** Il n'y a qu'une instruction irréversible ; la précéder d'un nettoyage rejouable suffit. Et surtout : **aucun privilège nouveau n'est demandé** — c'est ce qui rend cette conception défendable là où les deux autres reposaient sur une hypothèse invérifiable. Trois corollaires contre-intuitifs, écrits pour ne pas être redécouverts : - `completed` / `completed_at` sont **inatteignables** — la ligne cascade avec le compte. - Un `deleteUser` « user not found » compte comme un **succès**, sinon la ligne devient une pilule empoisonnée réclamée et échouée chaque jour, pour toujours. - Une pseudonymisation touchant **0 ligne est un succès** aussi. L'inverse gèlerait la file pour tout compte sans événement d'audit — exactement la panne muette qu'on corrige. **Le verrou anti-double-suppression** repose sur une nouvelle colonne `claimed_at`, jamais sur `requested_at` : une ligne n'étant réclamable que 14 jours après sa demande, un test sur `requested_at` serait **toujours vrai** et remettrait en file les lignes qu'une exécution concurrente est en train de traiter. **`deletion_self_insert` est supprimée plutôt que durcie.** Vérifié : aucune insertion client n'existe dans `src/`. La politique accorde une capacité que le produit n'utilise pas — et c'est la capacité, pas sa date, qui est le vecteur de vol de session. **Livraison en deux PR** : A inerte (rien ne peut détruire), B armement. La revue de B ne portera alors que sur une question : *est-ce que ça peut partir quand il ne faut pas ?* ## Contenu - `docs/adr/ADR-024-…` — la décision, ce qui est écarté, et **ce qui restera non prouvé** - `docs/plans/step-3b-deletion-queue.md` — le plan d'exécution pour la session suivante ## Ce qui reste à faire côté @Thierry Trois lectures SQL en production, **aucune écriture** (détail dans le plan). La troisième est un **NO-GO de PR-B** : toute la conception repose sur un chemin PostgREST `service_role` jamais re-vérifié en production depuis le correctif #273. ## Portée Markdown uniquement, aucun code applicatif. `plan-reviewer` a fait **trois tours** sur ce dossier et a bloqué à chaque fois quelque chose de réel — dont, au dernier tour, le fait que cette décision de schéma ne pouvait pas partir dans la même session que son implémentation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Résumé par Sourcery Documenter la conception de la file d’attente de suppression de compte et le plan de mise en œuvre, en séparant les modifications de schéma et d’orchestration (PR-A) du câblage du cron et de la modification de la période de grâce (PR-B). Améliorations : - Clarifier les implications RGPD et produit du flux de suppression, y compris les sémantiques de suppression idempotente, la gestion du journal d’audit et le périmètre de destruction de l’espace de travail. - Consigner les vérifications requises en production, la Definition of Done et les responsabilités QA pour activer en toute sécurité le cron de suppression et la période de grâce de 14 jours. Documentation : - Ajouter ADR-024 documentant la conception choisie de la file d’attente de suppression de compte, les approches rejetées, les invariants et les risques ouverts. - Ajouter un plan d’exécution détaillé pour l’étape 3b décrivant comment introduire la file d’attente de suppression, scindé en une PR-A non destructive et une PR-B armant le cron, incluant les tests, les vérifications de déploiement et le rollback. <details> <summary>Original summary in English</summary> ## Summary by Sourcery Document the account deletion queue design and implementation plan, separating schema and orchestration changes (PR-A) from cron wiring and grace-period change (PR-B). Enhancements: - Clarify GDPR and product implications of the deletion flow, including idempotent deletion semantics, audit log handling, and workspace destruction radius. - Record required production checks, Definition of Done, and QA responsibilities for safely enabling the deletion cron and 14-day grace period. Documentation: - Add ADR-024 documenting the chosen account deletion queue design, rejected approaches, invariants, and open risks. - Add a detailed execution plan for step 3b describing how to introduce the deletion queue, split into a non-destructive PR-A and a cron-arming PR-B, including tests, rollout checks, and rollback. </details> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
thierryvm
added a commit
that referenced
this pull request
Jul 27, 2026
#280) ## Pourquoi maintenant Règle durable du projet : **le ROADMAP se remet à jour avant d'ouvrir la branche suivante, pas après.** La branche suivante est PR-A de l'étape 3b. Deux écarts à solder d'abord. ## Écart 1 — l'étape 3 était annoncée « suivante » Elle est en cours depuis deux jours, et son détail vaut d'être lisible. L'exécution des spécifications a montré que le préalable n'était pas la file de suppression, mais **ce sur quoi elle repose** : le journal d'audit n'enregistrait rien depuis avril, et trois affirmations publiques étaient inexactes. Six lots livrés (#273 → #279), deux restants (PR-A inerte, PR-B armement), et un écart art. 17 mesuré qui part en session dédiée (#278). Le **verrou de PR-B** est désormais écrit dans le ROADMAP : trois lectures production, dont une NO-GO — toute la conception repose sur un chemin PostgREST `service_role` jamais re-vérifié en production depuis #273. ## Écart 2 — une dette réglée encore listée comme ouverte « Angle mort du préflight comptes » figurait dans les dettes ouvertes alors que #276 l'a comblé ce matin. Un ROADMAP qui garde une dette réglée est aussi trompeur qu'un qui en oublie une : il fait perdre du temps à qui la reprend. ## Portée Un seul fichier, markdown. Aucun code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Summary by Sourcery Mettre à jour le ROADMAP pour refléter l’état actuel et la répartition détaillée de l’étape 3, ainsi que la résolution récente de la dette. Documentation : - Ajouter une sous-section détaillée documentant l’avancement de l’étape 3, les lots livrés, les PR restantes et les contraintes associées en matière d’audit/journalisation. - Marquer la dette « Angle mort du préflight comptes » comme résolue, avec une explication mise à jour du nouveau comportement de vérification pour les comptes Supabase et Vercel. <details> <summary>Original summary in English</summary> ## Summary by Sourcery Update the ROADMAP to reflect the current status and detailed breakdown of step 3 and recent debt resolution. Documentation: - Add a detailed sub-section documenting step 3 progress, delivered lots, remaining PRs, and associated audit/logging constraints. - Mark the 'Angle mort du préflight comptes' debt as resolved with an updated explanation of the new verification behavior for Supabase and Vercel accounts. </details> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
thierryvm
added a commit
that referenced
this pull request
Jul 27, 2026
…erge pour une preuve Rapport PR-3B-A : les trois lectures production, les 10 mutations de falsification, les planchers e2e mesurés deux fois, et l'écart art. 17 de l'issue #278 énoncé sans détour — une personne qui exerce son droit à l'effacement verra son adresse e-mail rester en base, en clair, tant que `auth.audit_log_entries` n'est pas traitée. Registre de conformité (§6.5) : la vérification d'efficacité du correctif #273 en production est faite, datée, et racontée telle qu'elle s'est passée. Le premier relevé rendait des chiffres identiques à ceux de la veille — il ne distinguait pas « le correctif est cassé » de « personne ne s'est connecté ». C'est la reconnexion réelle qui a produit la preuve. Sa clôture affirmait « contresigné par le merge de la PR #273 ». Un merge ne démontre que le départ d'un code, jamais qu'une écriture atterrit. Corrigé. de-DE : le pronom `Sie` reprenait `die Löschung` et était correct, mais en tête de phrase il ressemble à une fuite de vouvoiement pour un relecteur ou un grep de registre. Reformulé. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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 défaut, mesuré
createAdminClient()associait la clé service_role à un adaptateur de cookies.@supabase/ssry trouvait la session de l'appelant et envoyait son JWT enAuthorizationà la place de la clé. Chaque requête tournait donc en rôleauthenticated— à quiaudit_logest révoqué.Deux inserts, même clé, même base, même table, seul le bocal à cookies change :
C'est H3, issue #192, ouverte le 28 mai,
type:security, sortie du scope de la PR security-hardening. Elle y était « piste forte ». Elle n'avait jamais été mesurée.Ce que ça cassait réellement
logAuditEvent()est appelé depuis toutes les Server Actions, qui tournent toutes avec un cookie de session. Sur un build non corrigé, après une connexion réussie et un renommage de compte, le journal local contenait :auth.rate_limitedauth.password_resetauth.loginaccount.renamedSeul ce qui précédait l'existence d'un cookie était écrit.
logAuditEventavalant son erreur, rien n'a protesté pendant trois mois.revokes'y applique, l'inférence est forte, mais mes deux tentatives de lecture d'agrégats ont été bloquées. Requête à passer côté @Thierry, détail dans le rapport.Le correctif
createServiceRoleClient()dans un module neuf, sans cookies, sans storage, avec refus explicite d'exécution navigateur.createAdminClientsupprimé — laisser les deux côte à côte, c'est garantir qu'on reprenne le mauvais.Surtout pas un
GRANTsuraudit_log: ça aurait ouvert le journal d'audit en écriture à tout utilisateur connecté, l'inverse exact de ce que verrouille20260417000003.Deux défauts que ce correctif ACTIVAIT
executeDeletionjournalisaitGDPR_DELETION_COMPLETEDavecresource_id= l'UUID qu'il venait d'effacer → compte ré-identifiable une ligne après sa suppression (le jsonb n'est pas cascadé paron delete set null). Invisible tant que l'insert échouait.Preuve
Rouge avant, vert après, même spec, même serveur :
Garde-fous falsifiés : module saboté volontairement → 2 échecs sur 3, puis restauration et retour au vert.
npm run testTests ajoutés
admin.test.ts— assertion sur le contrat réseau (quelle identité part réellement), refus navigateur, interdiction statique des imports@supabase/ssr/next/headers.deletion.test.ts— les premiers tests desrc/lib/gdpr/, qui n'en avait aucun.e2e/audit-log.spec.ts— parcours connecté réel, assertions avant la suppression de l'utilisateur semé (la supprimer d'abord effacerait la preuve).Décision demandée
Dépendance
server-onlynon ajoutée, volontairement. L'ancien client importaitnext/headers, ce qui faisait échouer le build en cas d'import depuis un composant client ; le nouveau module perd ce garde-fou machine.server-only(Vercel, ~1 Ko, gratuit) le restituerait, mais la doctrine exige une validation explicite pour toute dépendance. Deux garde-fous sans dépendance tiennent en attendant. Le risque n'est pas une fuite de clé — Next n'inline pas les variables nonNEXT_PUBLIC_*.Hors périmètre
Export RGPD aux 14 tables, Zod consent, file de suppression + cron, purge
audit_log, THI-206.executeDeletionn'a toujours aucun appelant : cette PR rend correct un code pas encore branché, elle ne fait pas marcher la suppression de compte.Rapport complet :
docs/prs/PR-h3-service-role-client-report.mdCloses #192