Bloc 100+101 — ⚙️ Suivi des visites configurable (URL + identifiant) + sections repliables - #127
Conversation
…tion) Numéroté 100 : le 99 est pris par le correctif du panneau joueur mergé juste avant. A. Nouveau champ « URL du script de suivi » dans l'onglet Configuration, volontairement générique — rien ne connaît l'outil derrière l'URL. Stocké dans une table `site_settings` (clé/valeur nommée) plutôt que dans `locale_settings`, qui est indexée par langue : les prochaines sections de l'onglet y écriront sans migration à chaque fois. La validation refuse tout ce qui n'est pas une URL http(s) analysable — y compris `javascript:`, `data:` et les URL portant des identifiants — puisque la valeur finit en <script src> sur toutes les pages du site. Champ vide = suivi désactivé, rien n'est chargé. B. Injection dans le <head> du layout racine, donc public ET admin, avec le nonce de la requête. C'est ce qui rend une URL modifiable à chaud possible : la CSP est en `'strict-dynamic'`, où les listes blanches de domaines sont ignorées et où seul le nonce autorise un script. Point que le brief n'avait pas vu : le nonce autorise le CHARGEMENT, pas les envois de mesures, qui relèvent de `connect-src 'self'`. Un tracker sur un autre domaine était donc chargé puis muet. Le middleware qui construit la CSP tourne sur l'Edge et ne peut pas lire la base ; l'origine vient donc de la variable TRACKING_ORIGIN, passée par `new URL()` pour n'en garder que le schéma/hôte/port — ce qui interdit aussi d'injecter des directives supplémentaires. Vérifié : la variable est bien lue à l'exécution, pas figée au build. C. Chaque section de l'onglet est un <details> à part : le pliage est une propriété du composant de section, pas un arbitrage entre deux panneaux connus. Indépendance structurelle, sémantique de divulgation et clavier gratuits, aucun composant client. Ouvertes par défaut, pour ne pas transformer l'onglet en pile d'en-têtes fermés. Tests : 17 unitaires sur la validation d'URL et l'origine CSP (dont l'injection de directive), 3 sur le pliage indépendant (clics réels, jsdom bascule bien les <details>), 3 sur le panneau, 3 sur la CSP du middleware, et 1 e2e qui fait le tour complet — sauvegarde admin, script présent avec son nonce sur une page publique ET une page admin, nonce autorisé par l'en-tête CSP servi, aucune violation signalée par le navigateur, puis effacement du champ et script absent. Contre-vérifié : sans nonce, le test rougit ; avec un nonce invalide, le navigateur remonte bien `script-src-elem https://…/script.js`, donc l'assertion « aucune violation » porte réellement. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HJgsDSfsCbbn8cochFGwe2
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e0fb4706c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| message: auditMessage( | ||
| session.user.name ?? session.user.id, | ||
| url ? "update" : "delete", | ||
| "l'URL de suivi des visites", |
There was a problem hiding this comment.
Saving this setting persists a French-only target into AuditLog.message, and /admin/logs renders that message verbatim, so an administrator using the English interface sees French tracking-history entries. Store a structured action/target or translation key and localize it when displaying the log rather than embedding fixed French text in the record.
AGENTS.md reference: AGENTS.md:L29-L29
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Le constat est juste, mais il ne décrit pas quelque chose que cette PR introduit — c'est la convention de tout le journal d'audit.
src/lib/audit-message.ts est français par construction : sa table de verbes l'est ("a créé", "a modifié", "a supprimé"…). Et les 12 appelants passent une cible française — "l'utilisateur X" (services/users.ts), "la langue DE" (la route locales juste à côté), "son compte X" (services/password.ts), etc. Ma route suit exactement ce que fait l'existant.
Corriger cela uniquement ici rendrait le journal incohérent : un administrateur anglophone verrait une entrée traduite sur le suivi des visites, et du français sur les onze autres types d'action. Le vrai correctif — stocker une clé d'action + une cible structurée et localiser à l'affichage dans /admin/logs — touche le modèle AuditLog, les 12 sites d'appel et la page du journal. C'est un bloc à part entière, que je signale au porteur du projet plutôt que de l'amorcer à moitié dans une PR sur la configuration.
Je laisse donc ce fil ouvert : il pointe un vrai manque, il n'est simplement pas du ressort de cette PR.
Generated by Claude Code
…mpose P1 (fondé, et sérieux). Le rôle `admin` a `configuration.write` mais se voit refuser `users.manage`, `logs.purge` et `content.*`. Or l'URL de suivi devient du code exécuté dans cette origine, avec un nonce valide, sur toutes les pages — y compris celles que charge un super admin. Un `admin` pouvait donc faire exécuter le script de son choix pendant la session d'un super admin et appeler l'API en son nom : exactement les capacités que la matrice de rôles lui refuse. Nouvelle capacité `configuration.scripts`, exclue du filtre `admin`, donc super_admin seul. La route est gardée dessus (et non plus sur `configuration.write`), et la section n'est pas rendue à un `admin` — lui montrer un champ dont l'enregistrement échoue serait un piège. Le reste de l'onglet Configuration ne change pas pour lui. P2 (fondé). La valeur précédente était lue hors transaction : deux sauvegardes concurrentes lisaient la même ancienne valeur et la seconde consignait un diff d'audit depuis une valeur qu'elle n'avait pas remplacée. Lecture déplacée dans la transaction. P1 sur l'audit en français : pas retenu ici. `auditMessage` est français par construction (sa table de verbes l'est) et les 12 appelants passent tous une cible française — c'est la convention de tout le journal d'audit, pas quelque chose que ce bloc introduit. Le corriger pour cette seule route rendrait le journal incohérent ; le corriger partout est un bloc à part. Demande utilisateur : TRACKING_ORIGIN est posée directement dans docker-compose.yml, sans passer par .env — ce n'est pas un secret, elle apparaît de toute façon dans l'en-tête CSP de chaque réponse. Elle reste surchargeable depuis l'environnement. Tests : la capacité est épinglée en unitaire (super_admin seul), et l'e2e crée un compte `admin` réel, vérifie qu'il reçoit 403 sur la route et que le champ ne lui est pas affiché. Contre-vérifié en ramenant la garde à `configuration.write` : le test rougit avec 200 au lieu de 403. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HJgsDSfsCbbn8cochFGwe2
Échec CI sur
|
Le vrai extrait fourni par Umami porte un `data-website-id` à côté du `src` ; le champ du Bloc 100 ne produisait que le `src`, donc l'intégration réelle ne mesurait rien. Second champ « Identifiant du site » dans la même section, optionnel — certains outils n'en demandent pas, et vide signifie « pas d'attribut » plutôt que « attribut vide » : `undefined` en JSX ne rend rien du tout. Même garde que l'URL (`configuration.scripts`, super admin seul), même route, même transaction. Validation : React échappe les valeurs d'attribut, donc rien ici ne peut injecter de balisage même sans contrôle. Ce que le contrôle refuse, c'est une valeur qui n'est pas un identifiant — guillemets, chevrons, accent grave, espaces, caractères de contrôle, au-delà de 200 caractères. Un admin qui colle la balise entière au lieu de l'identifiant l'apprend ici plutôt que par un tracker muet. Le message d'erreur distingue les deux champs, pour ne pas envoyer chercher au mauvais endroit. Les deux clés sont lues en une seule requête (`getTrackingSettings`), mémoïsée par requête HTTP : le layout racine s'exécute sur chaque page, c'est le coût de la fonctionnalité sur chacune d'elles. Tests : 13 unitaires sur la validation de l'identifiant, 2 sur le panneau (envoi conjoint, message propre à l'identifiant refusé), et l'e2e vérifie les deux attributs présents quand les deux champs le sont, `src` seul quand l'identifiant est vide, et 400 sur une valeur qui n'en est pas un. Contre-vérifié en retirant l'attribut du layout : l'e2e rougit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HJgsDSfsCbbn8cochFGwe2
magicgg91
changed the title
2 participants
A — Champs « URL du script de suivi » et « Identifiant du site »
Volontairement génériques : rien dans le site ne sait quel outil est derrière. Stockés dans une nouvelle table
site_settings(clé nommée / valeur) plutôt que danslocale_settings, indexée par langue — les prochaines sections de l'onglet y écriront sans migration à chaque fois.L'URL refuse tout ce qui n'est pas une URL http(s) analysable —
javascript:,data:, identifiants dans l'URL. Ce n'est pas du confort de saisie : la valeur finit en<script src>sur toutes les pages.L'identifiant (Bloc 101) est optionnel — certains outils n'en demandent pas — et vide signifie « pas d'attribut » plutôt que « attribut vide ». React échappe les valeurs d'attribut, donc rien ici ne peut injecter de balisage même sans contrôle ; ce que le contrôle refuse, c'est une valeur qui n'est pas un identifiant : guillemets, chevrons, accent grave, espaces, caractères de contrôle, au-delà de 200 caractères. Un admin qui colle la balise entière l'apprend ici, plutôt que par un tracker silencieusement muet — et le message distingue les deux champs, pour ne pas l'envoyer chercher au mauvais endroit.
La balise produite est exactement celle d'Umami :
Réservé au super admin (revue Codex, plus bas) : le rôle
admingarde le reste de l'onglet mais ne voit pas cette section.B — Injection dans le
<head>, et le point que le brief n'avait pas vuLe script est injecté dans le layout racine, donc public et admin d'un coup, avec le nonce de la requête. Ta lecture de la CSP était juste : sous
'strict-dynamic', les listes blanches de domaines sont ignorées et seul le nonce autorise un script — c'est ce qui rend une URL modifiable à chaud possible sans exception CSP par domaine.Mais le nonce autorise le chargement, pas les envois de mesures. Ceux-là relèvent de
connect-src, qui vaut'self': un tracker surstats.domopi.euaurait été chargé… puis muet, chaque envoi bloqué. Et le middleware qui construit la CSP tourne sur l'Edge — il ne peut pas lire la base. L'origine vient donc deTRACKING_ORIGIN, posée directement dansdocker-compose.yml(pas de.env: ce n'est pas un secret, elle apparaît de toute façon dans l'en-tête CSP de chaque réponse), et toujours surchargeable. Deux précautions :new URL()et seul le schéma/hôte/port est retenu, ce qui interdit structurellement d'injecter des directives supplémentaires (testé) ;next startavec — la CSP servie contenait bienconnect-src 'self' https://stats.domopi.eu.C — Sections repliables
Chaque section est un
<details>autonome, via un composantAdminConfigSectionque toute section future utilisera. L'indépendance est structurelle — aucun état partagé à arbitrer — et la sémantique de divulgation, le clavier et l'annonce lecteur d'écran viennent avec, sans composant client. Ouvertes par défaut.Revue Codex — 2 findings retenus sur 3
P1, escalade de privilège (retenu,
86f8610).adminaconfiguration.writemais se voit refuserusers.manage,logs.purgeetcontent.*. Une URL de script arbitraire lui rendait ces capacités par la bande : le script s'exécute dans cette origine, avec un nonce valide, sur les pages que charge un super admin, et peut appeler l'API en son nom. Nouvelle capacitéconfiguration.scripts, exclue du filtreadmin— route gardée dessus, section non rendue aux autres.P2, lecture hors transaction (retenu,
86f8610). Deux sauvegardes concurrentes consignaient un diff d'audit depuis une valeur jamais remplacée. Lecture déplacée danstx.P1, message d'audit en français (non retenu ici). Le constat est juste mais ne vient pas de cette PR :
auditMessageest français par construction et les 12 appelants passent une cible française — c'est la convention de tout le journal. Le corriger pour cette seule route le rendrait incohérent ; partout, cela touche le modèleAuditLog, les 12 sites d'appel et/admin/logs. Fil laissé ouvert, à traiter comme un bloc à part.Tests
<details>— une section pliée, l'autre reste ouverte, dans les deux sens.src,data-website-idet le nonce sur une page publique et une page admin → nonce autorisé par l'en-tête CSP servi → aucune violation signalée par le navigateur → identifiant vidé :srcseul, sans attribut → identifiant invalide : 400 → URL vidée : plus de script → et un vrai compteadminreçoit 403, sans voir le champ.Contre-vérifié dans les quatre sens : sans nonce le test rougit ; avec un nonce invalide le navigateur remonte
script-src-elem …(donc l'assertion « aucune violation » porte) ; en ramenant la garde àconfiguration.write, 200 au lieu de 403 ; en retirantdata-website-iddu layout, l'e2e rougit.Un bug trouvé et corrigé en cours de route
La première version de la route faisait un
upsertvia le client Prisma global à l'intérieur d'une transaction interactive. Sous SQLite, qui sérialise les écritures, la requête attend un verrou que la transaction détient elle-même : timeout à 5 s et 500 sur chaque sauvegarde. Attrapé par l'e2e avant le push.Validation
pnpm lint+ typecheck + prettier : proprespnpm test: 1340 tests vertspnpm build: succèspnpm test:e2e: 88/88 verts, base e2e recréée à neuf3e0fb47était tombée sur un test sans rapport (player-settings-panel), reproduit à l'identique surdevsans cette PR — détail et mesure dans ce commentaire.Pour mettre en service
site_settingss'applique au démarrage,TRACKING_ORIGINest déjà dans le compose).Il faut être super admin pour voir cette section.
🤖 Generated with Claude Code
https://claude.ai/code/session_01HJgsDSfsCbbn8cochFGwe2