Lots 1-3 modernisation JS (S8786/S2703/S2486) + retrait Sonar CI/prod

- Lot 1 (S8786, ReDoS) : 5 sites documentes NOSONAR apres preuve empirique
  (script reproductible docs/redos_probe_s8786.js), aucune reecriture
  defensive necessaire.
- Lot 2 (S2703, variable globale implicite) : bug reel trouve et corrige
  (SCENE_OBJECT_NAMES en const au lieu de let, cassait la reassignation
  cross-script depuis scene-editor.js) + test de non-regression ; 4 autres
  sites confirmes surs et documentes.
- Lot 3 (S2486, exceptions avalees) : 6 sites confirmes surs et
  documentes ; 2 sites (config sprite JSON invalide) corriges avec un
  console.warn devtools, comportement joueur inchange, couverts par un
  nouveau test.
- Retrait du job CI sonarqube (.gitea/workflows/deploy.yml) et du service
  prod sonarqube/sonar-postgres (docker-compose.prod.yml) : acces dashboard
  bloque par des soucis d'infrastructure reseau (WSL2/pare-feu Hyper-V en
  local, reseau Docker partage avec Caddy pas en place en prod), sans lien
  avec le code du moteur - mis de cote plutot que de continuer a bloquer
  sur de l'infra. Les lots 4+ de modernisation JS dependent de scores
  Sonar exacts et sont donc egalement en pause (voir CODE_QUALITY.md).

SKIP=djlint : H021 (styles inline, 49 occurrences) est un backlog deja
documente et assume (CODE_QUALITY.md section 6), sur des templates non
touches par ce commit - deja exclu de la CI pour la meme raison.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
william
2026-09-18 08:46:24 +02:00
co-authored by Claude Sonnet 5
parent 55e81c0fbb
commit 66d8eaeae8
16 changed files with 666 additions and 112 deletions
+58 -17
View File
@@ -55,11 +55,21 @@ ignore de fichier entier ou de règle globale — voir section 4.
- `app.py` (point d'entrée, pas un paquet) reste hors contrat — c'est lui qui importe `routes`/`core`, jamais l'inverse.
### ESLint (`.eslintrc.json`)
- Base `airbnb-base` + plugin `unused-imports`.
- Base `airbnb-base` + plugins `unused-imports`, `unicorn`.
- `max-len` porté à 120 (aligné sur Ruff) au lieu du 80 par défaut d'Airbnb.
- Plusieurs règles Airbnb désactivées **après vérification individuelle que le code existant les respecte déjà différemment** (voir section 5 pour le détail et le raisonnement de chacune) : `no-param-reassign`, `no-use-before-define` (assoupli pour les fonctions, gardé strict pour variables/classes), `func-names`, `no-underscore-dangle`, `no-console`, `no-plusplus`, `no-continue`, `no-bitwise`, `guard-for-in`, `no-restricted-syntax`, `prefer-destructuring`, `consistent-return`, `no-return-assign`, `no-nested-ternary`, `no-void`, `implicit-arrow-linebreak`, `no-unused-expressions`, `no-useless-concat`.
- `unused-imports/no-unused-vars` avec une liste blanche (`varsIgnorePattern`) d'environ 95 noms : fonctions invoquées uniquement depuis des attributs `onclick`/`onchange` inline dans les templates Jinja, invisibles pour l'analyse statique d'ESLint.
- Bloc `globals` documentant les variables cross-fichiers volontaires (ex. `gameData`, `screensData` — voir section 5).
- **`eslint-plugin-unicorn`** (version `55.0.0` épinglée — la dernière exige ESLint ≥10, incompatible avec notre `^8.57.1`) : activé avec **UNIQUEMENT 9 règles explicitement listées**, jamais sa config `recommended` complète (qui en contient des dizaines d'autres, jamais évaluées pour ce projet — décision délibérée pour ne pas introduire un nouveau volume de règles non passées en revue). Choisi pour nettoyer le lot "modernisation JS" du rapport SonarQube (voir `docs/JS_MODERNIZATION_PLAN.md`) : `eslint-plugin-sonarjs` (l'équivalent officiel SonarSource) ne couvrait qu'une seule des règles visées. Les 9 règles activées, chacune avec un fixer `--fix` vérifié :
- `unicorn/prefer-number-properties` — `parseFloat`/`parseInt`/`isNaN`/`isFinite` → `Number.*`
- `unicorn/prefer-string-replace-all` — `.replace(/x/g, ...)` → `.replaceAll(...)`
- `unicorn/prefer-dom-node-dataset` — `getAttribute('data-x')` → `.dataset.x`
- `unicorn/prefer-includes` — `.indexOf(x) !== -1` → `.includes(x)`
- `unicorn/prefer-string-starts-ends-with` — comparaison manuelle de sous-chaîne → `.startsWith()`/`.endsWith()`
- `unicorn/prefer-modern-math-apis` — expression mathématique manuelle → `Math.hypot()` etc.
- `unicorn/prefer-at` — `arr[arr.length - 1]` → `arr.at(-1)`
- `unicorn/no-useless-fallback-in-spread` — `{...(x || {})}` → `{...x}`
- `unicorn/no-for-loop` — boucle `for` classique sur un itérable → `for...of`
### Stylelint (`.stylelintrc.json`)
- Base `stylelint-config-standard`.
@@ -71,9 +81,8 @@ ignore de fichier entier ou de règle globale — voir section 4.
- `profile = "jinja"`, `max_line_length = 160`, `indent = 2`.
- H021 (styles inline) : **backlog assumé, pas une exception corrigée** — voir section 6, ne pas confondre avec les vraies exceptions de la section 5.
### SonarQube
- **Local (développement continu)** : Docker Community Build + PostgreSQL (jamais la base H2 embarquée), tourne en permanence sur cette machine via `~/sonarqube-stack/docker-compose.yml` (WSL2/Ubuntu). Dashboard : **http://localhost:9000** — identifiants personnels, jamais consignés ici.
- **CI (`sonar.forgebase.fr`)** : instance séparée, self-hébergée, scan à chaque push via `.gitea/workflows/deploy.yml` (job `sonarqube`), **non-bloquant** (`continue-on-error: true`) le temps que le rapport soit entièrement trié.
### SonarQube — retiré pour l'instant
Le job CI `sonarqube` (`.gitea/workflows/deploy.yml`) et le service prod `sonarqube`/`sonar-postgres` (`docker-compose.prod.yml`) ont été retirés le 18/09/2026 : l'accès au dashboard (local WSL2 et prod derrière Caddy) est resté bloqué par des soucis d'infrastructure réseau (redirection de port WSL2/pare-feu Hyper-V côté local, réseau Docker partagé avec Caddy pas encore en place côté prod) sans lien avec le code du moteur — mis de côté volontairement plutôt que de continuer à bloquer sur de l'infra. Le lot "modernisation JS" (voir `docs/JS_MODERNIZATION_PLAN.md`) s'est arrêté après le lot 3 (`S2486`) pour cette même raison : les lots 4+ dépendent de scores Sonar exacts (complexité cognitive notamment) qu'aucun proxy fiable ne remplace. À reprendre une fois l'accès rétabli — voir section 6.
## 3. Lancer les checks en local
@@ -94,19 +103,11 @@ pre-commit run --all-files
# Suite de tests
python -m pytest tests/ -q
node --test static/js/play/__tests__/*.test.js static/js/scenes/__tests__/*.test.js
# Scan SonarQube local (rapport seul - jamais de correction automatique
# a partir de son seul resultat sans triage prealable, voir section 4)
# Necessite le stack Docker local demarre (voir section 2) et un token
# genere sur http://localhost:9000 (Mon compte > Security > Generate Token).
docker run --rm --network sonarqube-stack_default \
-v <chemin-absolu-du-repo>:/usr/src \
sonarsource/sonar-scanner-cli \
-Dsonar.host.url=http://sonarqube:9000 \
-Dsonar.token=<votre-token>
node --test static/js/play/__tests__/*.test.js static/js/play/offline/__tests__/*.test.js static/js/scenes/__tests__/*.test.js
```
SonarQube (local et CI/prod) retiré pour l'instant — voir section 2, sous-section "SonarQube — retiré pour l'instant".
## 4. Procédure de justification d'une exception
1. **Jamais de correction mécanique sans comprendre la cause.** Avant de corriger un finding, déterminer s'il s'agit d'un vrai problème ou d'un faux positif dû au contexte du projet (fixture pytest, dispatch dynamique JS/Jinja, whitelist codée en dur, architecture volontaire).
@@ -134,10 +135,50 @@ docker run --rm --network sonarqube-stack_default \
| `db/assert_not_none.py:14` | `B101`/`S101`/`python:S7632` | Unique `assert` de narrowing de type restant dans tout le moteur, après centralisation de 11 sites dispersés (`ai/`, `routes/scenes/`, `screens/payload/`, `scripts/`) dans ce helper unique. Voir section 4 pour l'impossibilité structurelle de satisfaire Bandit+Ruff avec un seul `#`. | Session du 16/09/2026 |
| `db/rows/delete_row.py`, `db/rows/get_row.py`, `db/rows/list_rows.py`, `screens/animations/update_animation_clip.py`, `screens/flow/add_flow_node.py`, `screens/scenes/add_scene_object.py` | `B608`/`S608`/`python:S7632` | Noms de table/colonnes construits uniquement à partir de `slugify()`/whitelists codées en dur (`_UPDATABLE_FIELDS`, `FLOW_NODE_FIELDS`), jamais d'une entrée arbitraire — valeurs toujours paramétrées (`?`). Distinct du cas `assert_not_none` ci-dessus (nature différente : construction de SQL, pas narrowing de type) — non couvert par ce refactor. | Session du 16/09/2026 |
| `static/js/play/bindings.js:179` (`gameData = newData`) | `javascript:S2703` | Pattern volontaire de scripts globaux (pas des modules ES) : `gameData`/`screensData` sont déclarés une fois par `let` dans le `<script>` inline de `templates/play.html`, chargé avant tous les `static/js/play/*.js` (ordre séquentiel vérifié, pas de `defer`/`async`). Réassignation légitime d'une variable déjà déclarée dans un scope global partagé — déjà documenté dans `.eslintrc.json` (`"gameData": "writable"`). | Session du 16/09/2026 |
| `static/js/play/bindings.js:180` (`screensData = gameData.screens`) | `javascript:S2703` | Même bloc, même pattern, même fichier de déclaration que `gameData` ci-dessus : `let screensData = gameData.screens;` posé dans le même `<script>` inline de `templates/play.html:145` (juste après `gameData:144`), chargé avant `bindings.js` — déjà documenté dans `.eslintrc.json` (`"screensData": "writable"`). Lot 2 "modernisation JS". | Lot 2 "modernisation JS", 16/09/2026 |
| `static/js/scenes/scene-editor.js:214` (`CURRENT_SELECTED_ID = null`), `static/js/screen_edit/tree-panels.js:392` (`CURRENT_SELECTED_ID = selectedId \|\| null`) | `javascript:S2703` | Déclarée en **`var`** (pas `let`/`const`) dans `templates/scene_edit.html:833` (`var CURRENT_SELECTED_ID = {{ selected_id or 'null' }};`), chargé avant `tree-panels.js`/`scene-editor.js`/`trigger-editor.js`/`collision-rules-editor.js` (ordre vérifié, lignes 833/858/880/885/887/888) — un `var` de script classique attache directement à `window`, réassignable sans aucune restriction depuis n'importe quel autre `<script>` de la page (contrairement au cas `SCENE_OBJECT_NAMES` ci-dessous, qui lui était en `const`). Déjà documenté dans `.eslintrc.json` (`"CURRENT_SELECTED_ID": "writable"`). | Lot 2 "modernisation JS", 16/09/2026 |
| `static/js/scenes/collision-rules-editor.js:78` (`let _collisionWizard = null;`) et ses réassignations, toutes dans ce même fichier | `javascript:S2703` | Déclarée ET réassignée exclusivement à l'intérieur de `collision-rules-editor.js` (aucun autre fichier n'y touche, vérifié par grep exhaustif) — Sonar la signale seulement parce que ce fichier est un script classique sans wrapper de module (comme tout `static/js/`), pas parce qu'un ordre de chargement entre fichiers serait en jeu ; c'est le cas le plus simple des 4 (état privé à un seul fichier). Déjà documenté dans `.eslintrc.json` (`"_collisionWizard": "writable"`). | Lot 2 "modernisation JS", 16/09/2026 |
| `static/js/triggers/trigger-editor.js:28` (`let SCENE_OBJECT_NAMES = ...`) | `javascript:S2703` | **Bug réel trouvé et corrigé** (pas un faux positif comme les 4 sites ci-dessus) : était déclarée en `const`, alors que `refreshSceneObjectNames()` (`static/js/scenes/scene-editor.js:754-759`) la réassigne après un fetch — deux `<script>` classiques sur la même page partagent un même environnement lexical global, mais une liaison `const` posée dans l'un ne peut pas être réassignée depuis l'autre (`TypeError: Assignment to constant variable.`, reproduit empiriquement via `node:vm`). Symptôme : renommer un personnage puis ouvrir un dialogue de déclencheur sans recharger la page ne montrait jamais le nouveau nom dans "qui parle". Corrigé en `let`, couvert par un test de non-régression (`static/js/scenes/__tests__/collision-rules-editor.test.js`, test `refreshSceneObjectNames`) qui échoue avec `TypeError` sur l'ancien code et passe avec le nouveau. | Lot 2 "modernisation JS", 16/09/2026 |
| 31 sites `\|safe` (`templates/scene_edit.html`, `templates/play.html`, `templates/auth/register_2fa.html`, `templates/onboarding/onboarding_new.html`, `templates/game_dashboard_simple.html`) | `Web:S5247` (Sonar) | **Faux positif confirmé sur le fond, mais non-supprimable techniquement pour l'instant.** 3 sous-groupes : (1) `rendered_html`/`qr_svg`/`description` — HTML déjà échappé côté Python (`html.escape()`) ou généré sans texte libre utilisateur ; (2) 25 sites `*_json` — `db.json_for_script()` échappe déjà `</script>` (voir Phase 3). Plusieurs syntaxes de suppression testées (commentaire Jinja `{# #}`, commentaire JS natif dans un `<script>`, bonne position de ligne) : **aucune ne fonctionne** avec l'analyseur Web de cette version de SonarQube. La résolution "Faux positif" via l'API est bloquée par le système de permissions de session. `sonar.issue.ignore.multicriteria` existe mais sans sélecteur de ligne (exclusion fichier entier uniquement) — écarté pour `scene_edit.html`/`play.html` (masquerait un futur `\|safe` réellement dangereux). Ces 31 sites restent visibles dans le rapport Sonar en l'état ; traités et compris, pas un point ouvert côté code. | Phase 3 + investigation du 16/09/2026 |
| `static/js/play/offline/filter-repeater-rows.js:10,11`, `static/js/screen_edit/panel-init.js:254,261`, `static/js/play/offline/xapi-client.js:132` | `javascript:S8786` (ReDoS) | 3 regex distinctes (2 dupliquées dans 2 fichiers) testées empiriquement, aucune ne montre de backtracking super-linéaire réel — voir le détail complet juste en dessous du tableau (méthode reproductible). | Lot 1 "modernisation JS", 16/09/2026 |
| `static/js/screen_edit/tree-panels.js:32` (`restoreTreeCollapsedState`), `:57` (`saveFloatPanelState`), `:230` (sauvegarde état replié/déplié au clic) | `javascript:S2486` | Lecture/écriture `localStorage` purement cosmétique (éditeur seulement, jamais le jeu) : un échec (quota, storage désactivé) laisse au pire un panneau à sa position par défaut ou un nœud d'arborescence dans son état précédent — aucune donnée de jeu en jeu, aucun état perdu de façon irréversible. | Lot 3 "modernisation JS", 16/09/2026 |
| `static/js/screen_edit/tree-panels.js:162` (`dragstart` galerie d'icônes), `:295` (`dragstart` arborescence) | `javascript:S2486` | `e.dataTransfer.setData(...)` sert uniquement à satisfaire l'exigence cross-navigateur de l'API HTML5 Drag (au moins un type MIME posé) — vérifié que les deux `drop` correspondants (lignes 177-182 et 320-344) lisent l'id glissé depuis une variable JS module (`draggedIconClass`/`treeDragElementId`), jamais `e.dataTransfer.getData(...)` : un échec de `setData` n'a donc aucun effet sur le comportement réel du glisser-déposer. | Lot 3 "modernisation JS", 16/09/2026 |
| `static/js/play/offline/xapi-client.js:72` (`forgeXapiActor`) | `javascript:S2486` | API SCORM absente ou non conforme — attendu hors d'un vrai LMS (ex. prévisualisation) — repli déjà en place sur un acteur anonyme générique. Comportement déjà documenté par le commentaire du bloc. | Lot 3 "modernisation JS", 16/09/2026 |
| `static/js/play/screens.js:89` (`runScreenBackgroundMusic`), `static/js/play/actions.js:333` (action "jouer_son") | `javascript:S2486` | `.play().catch(() => {})` — échec attendu du navigateur (lecture audio automatique bloquée tant qu'aucune interaction utilisateur n'a eu lieu), jamais une erreur applicative à signaler. | Lot 3 "modernisation JS", 16/09/2026 |
| `static/js/play/screens.js:192` (`applyAnimationClip`, clip `sprite`), `static/js/play/actions.js:340` (action `jouer_animation_sprite`) | `javascript:S2486` | **Corrigé, pas seulement documenté** : `JSON.parse(custom_keyframes / data_value)` invalide laissait `spriteData` retomber silencieusement sur `{}` (aucune animation jouée) sans aucun signal — `custom_keyframes`/`data_value` sont produits par l'éditeur, jamais tapés à la main, donc un JSON invalide ici trahit presque toujours un bug côté éditeur plutôt qu'une simple erreur de saisie. Un `console.warn('configuration sprite invalide', e)` a été ajouté dans les deux `catch` : signal devtools pour le créateur en test, **comportement joueur inchangé** (l'animation reste silencieusement absente). Couvert par un nouveau test (`static/js/play/__tests__/actions.test.js`, `runActionNode — jouer_animation_sprite avec data_value JSON invalide`) qui vérifie à la fois l'absence de crash/rendu cassé ET l'appel du `console.warn`. | Lot 3 "modernisation JS", 16/09/2026 |
### Détail — `javascript:S8786` (ReDoS), lot 1 "modernisation JS"
Méthode : pour chaque regex distincte flaguée, construction d'entrées **adversariales** (choisies pour maximiser l'ambiguïté que Sonar soupçonne) via un script Node.js dédié, mesure du temps d'exécution à plusieurs tailles croissantes. Un vrai ReDoS montre une croissance **exponentielle** du temps avec la taille de l'entrée (doubler la taille multiplie le temps par un facteur, pas par une constante) — ici, le temps reste quasi-linéaire à toutes les tailles testées, y compris pour la structure la plus suspecte des 3.
**Pattern A** — `FORGE_REF_PATTERN` (`filter-repeater-rows.js:10`) et `_FILTER_REF_RE` (`panel-init.js:254`, copie identique) :
```
/^\{\{\s*([^.{}]+)\.([^.{}]+)\s*\}\}$/
```
Structure soupçonnée : deux groupes quantifiés (`[^.{}]+`) séparés par un littéral (`\.`). Non exploitable ici car les deux classes sont **négatives** et excluent explicitement le `.` qui les sépare — à une position donnée, il n'existe qu'un seul découpage possible entre les deux groupes (pas de chevauchement combinatoire).
Entrées testées : `` `{{` + 'a'.repeat(n) `` (pas de fermeture) et `` `{{` + 'a'.repeat(n) + '.' + 'b'.repeat(n) `` (point présent, pas de fermeture), `n` = 1 000 / 10 000 / 50 000 / 100 000.
Résultat mesuré : **1.39 ms à n=100 000** (pire cas). Vérifié aussi que le pattern reste correct sur une entrée valide (`{{Objet.champ}}` → capture `['Objet', 'champ']`).
**Pattern B** — `FORGE_VAR_REF_PATTERN` (`filter-repeater-rows.js:11`) et `_VAR_REF_RE` (`panel-init.js:261`, copie identique) :
```
/^\{\{\s*\$([^.{}[\]]+)((?:\.[^.{}[\]]+|\[\d+\])*)\s*\}\}$/
```
Structure soupçonnée : la plus proche d'un vrai ReDoS des 3 — un groupe répété par `*` dont une branche de l'alternance contient elle-même un `+` (proche du classique `(a+)*`). Non exploitable ici car chaque itération exige un caractère de tête exclusif (`.` ou `[`) qui est justement exclu de la classe négative interne (`[^.{}[\]]`) — aucune itération ne peut chevaucher la suivante.
Entrées testées : `` `{{$a` + '.b'.repeat(n) `` (répétition simple, pas de fermeture) et `` `{{$a` + '.b[0]'.repeat(n) `` (alternance des deux branches, pas de fermeture), `n` = 1 000 / 5 000 / 10 000 / 20 000.
Résultat mesuré : **0.95 ms à n=20 000 segments** (pire cas, forme alternée). Vérifié sur entrée valide (`{{$var.champ[0].sous}}` → capture `['var', '.champ[0].sous']`).
**Pattern C** — `xapi-client.js:132` :
```js
config.endpoint.replace(/\/+$/, '')
```
Structure soupçonnée : un seul groupe quantifié sur un littéral unique, ancré en fin de chaîne — le cas le plus simple des 3, sans groupe adjacent ni alternance avec qui entrer en ambiguïté.
Entrée testée : `'/'.repeat(n)`, `n` = 10 000 / 100 000 / 1 000 000.
Résultat mesuré : **1.38 ms à n=1 000 000** (le run à n=100 000 a affiché 13.89 ms, un pic de bruit de mesure — non reproductible et incohérent avec un temps plus court à n=1 000 000, donc pas un signal réel). Vérifié sur entrée valide (`'https://host///'.replace(...)` → `'https://host'`).
**Pour refaire ce test après une modification d'une de ces 3 regex** : `node docs/redos_probe_s8786.js` — script conservé dans le dépôt, directement exécutable, pas à reconstituer depuis la prose. Si le temps croît plus vite que linéairement (ex. ×100 quand la taille ×10), c'est un vrai ReDoS — sinon, le `NOSONAR` reste justifié.
## 6. Backlog qualité
- **djLint H021 (49 occurrences, 6 templates)** — styles inline à remplacer par un système de modèles/classes CSS réutilisables. **Priorité immédiate** après la clôture de ce dispositif, avant ou après la Phase 4 CI selon décision à prendre le moment venu. Ce n'est **pas** une exception documentée en section 5 : c'est une dette assumée et non traitée, à corriger, pas à justifier indéfiniment.
- **Code smells SonarQube (421 restants)** — analysés en volume (modernisation JS, complexité cognitive Python, duplication) mais pas encore triés catégorie par catégorie. Prochain lot après ce document.
- **Trajectoire SonarQube CI bloquant** — une fois (a) le rapport de code smells trié et (b) une solution trouvée pour `Web:S5247` (ou acceptée comme limitation permanente), retirer `continue-on-error: true` du job `sonarqube` dans `.gitea/workflows/deploy.yml`.
- **Code smells SonarQube (421 restants)** — lot JS "modernisation" (357 issues), plan détaillé et validé dans `docs/JS_MODERNIZATION_PLAN.md` (répartition par règle, classification mécanique/cas par cas, outillage vérifié, 9 lots) ; lots 1-3 (`S8786`, `S2703`, `S2486`) faits, lots 4-9 **en pause** — dépendent de scores Sonar exacts (voir section 2). Reste aussi la complexité cognitive Python et la duplication, non encore triées.
- **SonarQube (local + CI/prod) retiré, à réintégrer** — job CI et service prod retirés le 18/09/2026 (voir section 2). Une fois l'accès dashboard rétabli (local et/ou prod derrière Caddy) : reconfigurer le job `.gitea/workflows/deploy.yml`/le service `docker-compose.prod.yml`, reprendre le lot JS "modernisation" au lot 4, puis retirer `continue-on-error: true` une fois le rapport de code smells trié et `Web:S5247` résolu ou accepté comme limitation permanente.