From ed586233f6e837a07771f2cab7b227f44b687e3b Mon Sep 17 00:00:00 2001 From: GGThed Date: Fri, 7 Aug 2026 13:17:08 -0400 Subject: [PATCH] docs(audit): rebase sur le depot de reference et reverification complete Le miroir GitHub audite en premiere passe etait en retard de 16 commits sur git.immortal.host/clubesportsudes/team-tryouts. L'audit est rebase sur immortal/main @ bb0bc1c et l'ensemble des constats reverifie. Resolus par l'equipe (archives dans 00-provenance.md) : - seed automatique en production supprime - print du token Discord supprime - proxy_pass nginx corrige vers 127.0.0.1 - dossier supporting_scrits renomme Nouveaux constats : - SEC-20 flux OAuth2 Discord sans parametre state (CSRF de liaison) - SEC-21 le deploiement SFTP pousse .git/ sur le serveur - SEC-22 clear_db.py destructif sans garde-fou, admin/password en dur - MNT-16 discord_pending.json versionne Requalifies : - SEC-01 secrets retires du fichier mais toujours dans l'historique des deux depots, et dans le HEAD du miroir GitHub -> revocation requise - SEC-05 trusted_proxy='*' + bind 0.0.0.0 rend X-Forwarded-For usurpable, ce qui ouvre le rate limiting au lieu de le corriger - SEC-07 l'OAuth2 ajoute ne contraint pas l'identite : le discord_user_id transite par un champ cache du formulaire - MNT-05 psycopg[binary] non epingle, incompatible avec les URI postgresql:// que SQLAlchemy resout vers psycopg2 48 constats. Aucune modification du code applicatif. Co-Authored-By: Claude Opus 5 --- audit/00-provenance.md | 149 +++++++++ audit/01-securite.md | 607 ++++++++++++++++++++++-------------- audit/02-maintenabilite.md | 86 +++-- audit/03-standards-stack.md | 43 ++- audit/README.md | 65 ++-- audit/plan-remediation.md | 87 +++--- 6 files changed, 704 insertions(+), 333 deletions(-) create mode 100644 audit/00-provenance.md diff --git a/audit/00-provenance.md b/audit/00-provenance.md new file mode 100644 index 0000000..bd823ff --- /dev/null +++ b/audit/00-provenance.md @@ -0,0 +1,149 @@ +# 0 — Provenance et écart entre les dépôts + +## Deux dépôts, un seul fait autorité + +| Dépôt | Rôle | HEAD `main` | +|---|---|---| +| `git.immortal.host/clubesportsudes/team-tryouts` | **Référence** — c'est celui-ci qui est audité | `bb0bc1c` | +| `github.com/cedrick2711/team-tryouts` | Miroir, **en retard de 16 commits** | `08f02f7` | + +Les deux partagent un ancêtre commun (`d6fe505`). Le miroir GitHub porte en outre 2 commits qui ne sont pas sur le dépôt de référence (`0d5a408 fix registration` et son merge `08f02f7`) ; le correctif correspondant existe sur `immortal` sous la forme du commit `7d30aff Corriger erreur d'enregistrement`. + +**La première passe de cet audit a été menée sur le miroir GitHub**, avant que la bonne source ne soit connue. L'ensemble des constats a été revérifié contre `immortal/main`. Le présent document liste ce qui a changé. + +### Les 16 commits d'écart + +``` +bb0bc1c ajout de plateforme de base pour les url TRN +f89e4de régler problème avec mise a jour des status de message du discord bot +fcf10bc bug fix: Manager ne pouvait pas voir les tryouts. probleme avec discord bot +68e3da6 fix probleme avec dispos +d25b35c régler erreur 500 sur changement de role par admin +aeba4d7 régler problème de changement de rôle +7bb8022 added a clear_db to start fresh with only an admin +0dd4ecd Erreur de frappe dans un des dossiers +0f7788e Changement du layout de la page d'enregistrement +47d5ec4 added discord oauth2 to get basic user info to complete profile when registering +10af8c0 small change for prod +7d30aff Corriger erreur d'enregistrement +4097416 Update wsgi.py +056cea0 Update .gitea/workflows/git-to-ptero.yaml +d3b5700 Add .gitea/workflows/git-to-ptero.yaml +fd258de Update app/.env.exemple +``` + +--- + +## Constats de la première passe devenus caducs + +Quatre constats ont été résolus par l'équipe entre les deux points de l'historique. Ils sont conservés ici pour mémoire, et retirés du décompte des documents 01 à 03. + +### ✅ Seed automatique en production — résolu + +Le bloc de `app/app.py` qui déclenchait le seed quand la table `users` était vide a été supprimé (commit `7bb8022`), et `app/supporting_scrits/seed.py` (460 lignes, comptes `manager1`, `coach1`, `scout1`… tous avec le mot de passe `password`) a été supprimé. + +```diff + with app.app_context(): + import app.models as models +- from app.models import User + db.create_all() +- +- # Seed database if empty +- if User.query.count() == 0: +- from app.supporting_scrits.seed import seed_database +- seed_database() +``` + +Un déploiement neuf ne crée donc plus de comptes par défaut. + +> **Reste à traiter.** Le script de remplacement `clear_db.py` code toujours en dur `admin` / `password`, et il est bien plus dangereux que l'ancien seed sur un autre plan. Voir [SEC-22](01-securite.md#sec-22--). +> +> **À vérifier en production**, indépendamment du code : les comptes créés par l'ancien seed (`admin`, `manager1`, `manager2`, `coach1`, `coach2`, `coach3`, `scout1`) peuvent toujours exister en base avec le mot de passe `password`. La suppression du script ne supprime pas les comptes qu'il a créés. + +### ✅ Token Discord imprimé sur stdout — résolu + +La ligne `print(DISCORD_BOT_TOKEN or 'FAILED TO PRINT BOT TOKEN')` de `app/discord_bot.py:25` a été supprimée. Le module ne comporte plus aucun `print()`. + +### ✅ `proxy_pass` vers `0.0.0.0` — résolu + +`app/nginx.conf:122` : `proxy_pass http://0.0.0.0:5000;` → `proxy_pass http://127.0.0.1:5000;` + +> **Reste à traiter.** L'incohérence de ports subsiste : `wsgi.py:24` utilise toujours `PORT` par défaut à **10000** alors que nginx envoie vers **5000** et que le docstring du même fichier annonce 5000. Voir [STD-06](03-standards-stack.md#std-06--). + +### ✅ Faute de frappe `supporting_scrits` — résolu + +Le dossier a été renommé `app/supporting_scripts/` (commit `0dd4ecd`). + +> **Effet de bord non traité.** Le job CI « Security Scan » invoquait déjà un chemin erroné (`python security_scan.py` à la racine) ; le renommage l'éloigne encore. Voir [MNT-04](02-maintenabilite.md#mnt-04--). + +--- + +## Constats aggravés ou requalifiés + +### ⚠️ Secrets exposés — le nettoyage n'a pas révoqué + +`app/.env.exemple` a été nettoyé (commit `fd258de`) : les valeurs réelles ont été remplacées par des marqueurs (`flask_app_secret_key`, `my_discord_bot_token`, `URI_vers_db_posgres`). + +**Mais les secrets restent intégralement récupérables.** Vérification par recherche dans l'historique complet des deux dépôts : + +``` +$ git log --all --oneline -S "MTUyNzY3ODU3NjUyNTA1NDEyNQ" +fd258de Update app/.env.exemple ← retrait +2d3721b Ajout d'un .env.exemple pour simplifier la collaboration ← introduction +``` + +Le blob contenant le token Discord, le mot de passe PostgreSQL et la `SECRET_KEY` est toujours atteignable par `git show 2d3721b:app/.env.exemple` sur **les deux dépôts**. Il est de surcroît **toujours dans le HEAD du miroir GitHub**, donc visible par simple navigation dans l'interface web. + +Le constat reste donc critique et sa correction inchangée : **révoquer, puis purger**. Voir [SEC-01](01-securite.md#sec-01--). + +### ⚠️ En-têtes de proxy — le correctif a inversé le risque + +`wsgi.py` a reçu un bloc de configuration proxy (commit `4097416`) : + +```python +trusted_proxy='*', +trusted_proxy_count=1, +trusted_proxy_headers={'x-forwarded-for', 'x-forwarded-proto'}, +clear_untrusted_proxy_headers=True +``` + +L'intention est bonne, et `clear_untrusted_proxy_headers=True` est le bon réflexe. Mais `trusted_proxy='*'` signifie « faire confiance aux en-têtes de proxy quelle qu'en soit la provenance », et `HOST` vaut toujours `0.0.0.0` par défaut (`wsgi.py:26`). + +Avant ce changement, `request.remote_addr` valait toujours l'IP de nginx : le rate limiting était appliqué à un seau global — gênant, mais fermé. Désormais, un attaquant qui atteint directement le port applicatif contrôle `X-Forwarded-For` et peut donc **présenter une IP différente à chaque requête**, ce qui neutralise le rate limiting et le suivi de tentatives par IP. + +Le constat change de nature et reste élevé. Voir [SEC-05](01-securite.md#sec-05--). + +### ⚠️ Identifiant Discord — la vérification ajoutée n'en est pas une + +Un flux OAuth2 Discord complet a été ajouté à l'inscription (commit `47d5ec4`, `auth.py:326-456`). Il obtient de Discord un identifiant authentifié et le place en session. + +Mais cet identifiant est ensuite réinjecté dans le formulaire d'inscription **par un champ caché** : + +```html + + +``` + +et c'est cette valeur — passée par le client — qui est enregistrée (`auth.py:253`, `:290`). Un utilisateur peut la modifier avant envoi, ou poster directement le formulaire sans jamais passer par Discord. + +`RegisterSchema` valide désormais le format de `discord_user_id` (17-20 chiffres, `validators.py:211-215`), ce qui est un progrès, mais ne prouve rien sur la propriété du compte. + +La conséquence pratique est inchangée par rapport à la première passe — et le risque de fausse assurance est nouveau, puisque le flux *paraît* vérifié. Voir [SEC-07](01-securite.md#sec-07--). + +### ⚠️ Limite d'inscription desserrée + +`auth.py:190` : `@limiter.limit("3 per hour")` → `@limiter.limit("20 per hour")`. + +Le CAPTCHA arithmétique étant trivial ([SEC-13](01-securite.md#sec-13--)), le rate limiting était la protection réellement efficace contre la création automatisée de comptes. Passer à 20/heure la divise par un facteur ~7, et [SEC-05](01-securite.md#sec-05--) la rend contournable. + +--- + +## Éléments nouveaux ajoutés à l'audit + +| Élément | Constat | +|---|---| +| `auth.py:326-456` — flux OAuth2 Discord | [SEC-20](01-securite.md#sec-20--) absence de paramètre `state` · [SEC-07](01-securite.md#sec-07--) identité non contraignante | +| `.gitea/workflows/git-to-ptero.yaml` | [SEC-21](01-securite.md#sec-21--) déploiement de `.git/` par SFTP | +| `clear_db.py` | [SEC-22](01-securite.md#sec-22--) script destructif sans garde-fou | +| `migrations/add_tryout_coaches.py` | [STD-01](03-standards-stack.md#std-01--) migration manuelle hors outillage | +| `discord_pending.json` | [MNT-16](02-maintenabilite.md#mnt-16--) état d'exécution versionné | diff --git a/audit/01-securite.md b/audit/01-securite.md index 4750f60..2c7f272 100644 --- a/audit/01-securite.md +++ b/audit/01-securite.md @@ -1,24 +1,27 @@ # 1 — Sécurité -19 constats. Les références de lignes correspondent à `main` @ `08f02f7`. +22 constats. Références de lignes sur `immortal/main` @ `bb0bc1c`. + +Les constats résolus par l'équipe (ancien SEC-02 seed automatique, ancien SEC-04 token imprimé) sont archivés dans [`00-provenance.md`](00-provenance.md). Les identifiants des constats subsistants n'ont pas été renumérotés, pour préserver la traçabilité. | ID | Constat | Sévérité | |---|---|---| -| [SEC-01](#sec-01--) | Secrets de production réels committés dans le dépôt | 🔴 Critique | -| [SEC-02](#sec-02--) | Seed automatique en production avec mot de passe `password` | 🔴 Critique | +| [SEC-01](#sec-01--) | Secrets de production toujours récupérables dans l'historique | 🔴 Critique | | [SEC-03](#sec-03--) | CORS ouvert à toutes les origines avec credentials par défaut | 🔴 Critique | -| [SEC-04](#sec-04--) | Token du bot Discord imprimé sur stdout à l'import | 🔴 Critique | -| [SEC-05](#sec-05--) | En-têtes de proxy non validés → contournement HTTPS + rate limiting inopérant | 🟠 Élevé | +| [SEC-05](#sec-05--) | `trusted_proxy='*'` sur interface publique → en-têtes de proxy usurpables | 🟠 Élevé | | [SEC-06](#sec-06--) | Schémas de validation importés mais jamais appliqués sur les routes utilisateurs | 🟠 Élevé | -| [SEC-07](#sec-07--) | `discord_user_id` arbitraire → détournement des notifications privées | 🟠 Élevé | +| [SEC-07](#sec-07--) | Identifiant Discord modifiable malgré l'OAuth2 → détournement des notifications | 🟠 Élevé | | [SEC-08](#sec-08--) | Rate limiting en mémoire, non partagé et réinitialisé à chaque redémarrage | 🟠 Élevé | +| [SEC-20](#sec-20--) | Flux OAuth2 Discord sans paramètre `state` | 🟠 Élevé | +| [SEC-21](#sec-21--) | Le déploiement SFTP pousse le répertoire `.git/` sur le serveur | 🟠 Élevé | | [SEC-09](#sec-09--) | `nl2br` marque du HTML utilisateur non échappé comme sûr | 🟡 Moyen | | [SEC-10](#sec-10--) | CSP avec `'unsafe-inline'` sur `script-src` | 🟡 Moyen | | [SEC-11](#sec-11--) | Aucune validation de type sur l'upload de contrat signé | 🟡 Moyen | | [SEC-12](#sec-12--) | Énumération d'utilisateurs via les messages de login | 🟡 Moyen | -| [SEC-13](#sec-13--) | CAPTCHA arithmétique trivial | 🟡 Moyen | +| [SEC-13](#sec-13--) | CAPTCHA trivial, et limite d'inscription desserrée à 20/heure | 🟡 Moyen | | [SEC-14](#sec-14--) | `/health` expose l'erreur brute de la base de données | 🟡 Moyen | | [SEC-15](#sec-15--) | Profil complet de tout utilisateur visible par tout compte authentifié | 🟡 Moyen | +| [SEC-22](#sec-22--) | `clear_db.py` : destruction totale sans garde-fou, identifiants en dur | 🟡 Moyen | | [SEC-16](#sec-16--) | Conversions `int()` non protégées sur entrées utilisateur | 🔵 Faible | | [SEC-17](#sec-17--) | `add_to_team` ne vérifie pas la cohérence tryout/équipe/joueur | 🔵 Faible | | [SEC-18](#sec-18--) | Aucune réinitialisation de mot de passe ni MFA | 🔵 Faible | @@ -28,77 +31,53 @@ ## SEC-01 · 🔴 -**Secrets de production réels committés dans le dépôt** +**Secrets de production toujours récupérables dans l'historique** -`app/.env.exemple` — fichier **suivi par git** — ne contient pas des valeurs d'exemple mais des identifiants réels : +Le fichier `app/.env.exemple` a été nettoyé au commit `fd258de` : les valeurs réelles y sont désormais remplacées par des marqueurs. **Cela ne change rien au fait que les secrets sont compromis.** -| Ligne | Secret | +Recherche sur l'ensemble des références des deux dépôts : + +``` +$ git log --all --oneline -S "MTUyNzY3ODU3NjUyNTA1NDEyNQ" +fd258de Update app/.env.exemple +2d3721b Ajout d'un .env.exemple pour simplifier la collaboration + +$ git log --all --oneline -S "0YO038Od2QcQCsCTlNDAMOAOlcPGrIND" +fd258de Update app/.env.exemple +2d3721b Ajout d'un .env.exemple pour simplifier la collaboration +``` + +Le blob introduit en `2d3721b` reste atteignable par `git show 2d3721b:app/.env.exemple` sur **`immortal` comme sur GitHub**. Il contient : + +| Secret | Portée | |---|---| -| `:6` | `SECRET_KEY=65476749453935` — clé de signature des sessions Flask | -| `:18` | `DISCORD_BOT_TOKEN=MTUyNzY3ODU3NjUyNTA1NDEyNQ.G1gPNQ.LeFV…` — token du bot UdeS Esports | -| `:21` | `DATABASE_URL=postgresql://team_tryouts_db_user:0YO038Od2QcQ…@dpg-…render.com/team_tryouts_db` — base PostgreSQL Render, hôte public, avec mot de passe | +| `DISCORD_BOT_TOKEN` | Token du bot UdeS Esports — le commentaire du fichier le confirmait explicitement | +| `DATABASE_URL` | PostgreSQL Render, hôte public, **avec mot de passe** | +| `SECRET_KEY` | Clé de signature des sessions Flask | -Le commentaire ligne 16-17 confirme explicitement qu'il s'agit du token de production : *« This is the UdeS Esports BOT token »*. +Aggravant : **le miroir GitHub porte encore ces valeurs dans son HEAD**. Elles sont visibles par simple navigation dans l'interface web, sans même cloner. -**Impact.** Toute personne ayant accès au dépôt — y compris via un fork, un clone, ou si le dépôt devient public — obtient : -- un accès lecture/écriture complet à la base de production (l'hôte Render est joignable depuis Internet) : identités, courriels, téléphones, notes personnelles des joueurs, contrats ; -- le contrôle du bot Discord (envoi de DM en usurpant l'identité de l'organisation) ; -- la capacité de **forger des cookies de session Flask valides** grâce à la `SECRET_KEY`, donc de s'authentifier en tant que n'importe quel utilisateur, y compris `admin`, sans mot de passe. +**Impact.** Inchangé depuis la première passe : +- accès lecture/écriture complet à la base de production — identités, courriels, téléphones, notes personnelles, contrats ; +- contrôle du bot Discord, donc envoi de messages privés en usurpant l'identité de l'organisation ; +- avec la `SECRET_KEY`, **forge de cookies de session Flask valides** — authentification en tant que n'importe quel utilisateur, y compris `admin`, sans mot de passe. -Le `.gitignore` ignore bien `.env`, mais le fichier a été committé sous le nom `.env.exemple`, qui échappe à la règle. +**Correction.** Dans cet ordre : -**Présent dans l'historique** depuis le commit `2d3721b` (*« Ajout d'un .env.exemple pour simplifier la collaboration »*). Supprimer le fichier ne suffira pas. +1. **Révoquer**, avant toute manipulation de git — c'est la seule action qui rend les valeurs divulguées inoffensives : + - régénérer le token du bot (Discord Developer Portal → Bot → Reset Token) ; + - faire tourner le mot de passe PostgreSQL sur Render ; + - régénérer `SECRET_KEY` (`python -c "import secrets; print(secrets.token_hex(32))"`) — cela invalidera toutes les sessions en cours, ce qui est le comportement souhaité. +2. **Purger l'historique des deux dépôts** : + ```bash + git filter-repo --path app/.env.exemple --invert-paths + ``` + puis forcer la réécriture sur toutes les branches, et prévenir les collaborateurs qu'ils doivent recloner. +3. **Traiter le miroir GitHub** : soit le synchroniser après purge, soit le supprimer. Un miroir en retard de 16 commits qui expose des secrets dans son HEAD n'apporte rien et coûte beaucoup. +4. Ajouter `.env*` au `.gitignore` avec une exception explicite : `!.env.example`. +5. Ajouter un scan de secrets à la CI (`gitleaks`, ou `detect-secrets` en pre-commit). -**Correction.** -1. **Révoquer immédiatement, avant toute autre action** : régénérer le token du bot dans le Discord Developer Portal, faire tourner le mot de passe PostgreSQL sur Render, générer une nouvelle `SECRET_KEY` (`python -c "import secrets; print(secrets.token_hex(32))"`). La rotation de la `SECRET_KEY` invalidera toutes les sessions en cours, ce qui est le comportement souhaité ici. -2. Remplacer le contenu du fichier par des valeurs factices (`SECRET_KEY=`, `DATABASE_URL=postgresql://user:password@host:5432/dbname`). -3. Purger l'historique (`git filter-repo --path app/.env.exemple --invert-paths`, ou BFG), puis forcer la réécriture sur toutes les branches et prévenir les collaborateurs qu'ils doivent recloner. -4. Ajouter `.env*` (avec l'astérisque) au `.gitignore`, en gardant une exception explicite pour le modèle : `!.env.example`. -5. Ajouter un scan de secrets à la CI (`gitleaks`, ou `detect-secrets` en pre-commit) pour empêcher la récidive. - -> Renommer aussi le fichier en `.env.example` — l'orthographe actuelle est un francisme qui casse la détection automatique de la plupart des outils. - ---- - -## SEC-02 · 🔴 - -**Seed automatique en production avec mot de passe `password`** - -`app/app.py:348-356` - -```python -with app.app_context(): - import app.models as models - from app.models import User - db.create_all() - - if User.query.count() == 0: - from app.supporting_scrits.seed import seed_database - seed_database() -``` - -Ce bloc s'exécute **à chaque appel de `create_app()`**, sans distinction d'environnement — donc aussi via `wsgi.py`, c'est-à-dire en production. - -`app/supporting_scrits/seed.py` crée alors des comptes de démonstration dont le mot de passe est la chaîne littérale `password` : - -```python -username='admin', password_hash=hash_password('password'), # :42 -username='manager1', password_hash=hash_password('password'), # :48 -username='coach1', password_hash=hash_password('password'), # :60 -username='scout1', password_hash=hash_password('password'), # :79 -``` - -…et les affiche en clair au démarrage (`seed.py:449-453`). - -**Impact.** Tout déploiement neuf, toute restauration sur base vide, toute migration vers une nouvelle instance crée un compte `admin` / `password` accessible depuis Internet. C'est un contournement complet de l'authentification. Le compte `admin` a `can_manage_users() == True` : création, modification et suppression de tous les utilisateurs. - -Le mot de passe `password` ne respecte d'ailleurs pas la politique définie dans `validators.py:22-24` (8 caractères, majuscule, minuscule, chiffre) — ce qui montre que le seed contourne toute la couche de validation. - -**Correction.** -- Conditionner le seed : `if os.getenv('SEED_DEMO_DATA', 'false').lower() == 'true' and User.query.count() == 0:`. -- Mieux : sortir le seed du factory et en faire une commande CLI Flask (`flask seed-demo`), exécutée explicitement en développement. -- Faire générer les mots de passe de démo aléatoirement (`secrets.token_urlsafe(16)`) et les afficher une seule fois, plutôt que d'utiliser une constante. -- Vérifier immédiatement en production si les comptes `admin`, `manager1`, `manager2`, `coach1`, `coach2`, `coach3`, `scout1` existent avec ces mots de passe, et les désactiver le cas échéant. +> Renommer aussi le fichier en `.env.example` — `exemple` est un francisme qui échappe à la détection de la plupart des outils. --- @@ -106,7 +85,7 @@ Le mot de passe `password` ne respecte d'ailleurs pas la politique définie dans **CORS ouvert à toutes les origines avec credentials par défaut** -`app/app.py:76-95` +`app/app.py:76-95` — inchangé depuis la première passe. ```python allowed_origins = os.getenv('CORS_ALLOWED_ORIGINS', '').split(',') @@ -120,16 +99,16 @@ else: CORS(app, supports_credentials=True, methods=[…], max_age=3600) ``` -Le commentaire décrit une intention qui n'est pas implémentée : la branche `else` **autorise toutes les origines**, en développement comme en production. `flask-cors` avec `supports_credentials=True` et sans `origins` reflète l'en-tête `Origin` de la requête dans `Access-Control-Allow-Origin` et ajoute `Access-Control-Allow-Credentials: true`. +Le commentaire décrit une intention non implémentée : la branche `else` **autorise toutes les origines**, en développement comme en production. `flask-cors` avec `supports_credentials=True` et sans `origins` reflète l'en-tête `Origin` de la requête dans `Access-Control-Allow-Origin` et ajoute `Access-Control-Allow-Credentials: true`. -Le commentaire renvoie la responsabilité à nginx, mais `app/nginx.conf` **ne contient aucune directive CORS**. Et `app/.env.exemple` **ne définit pas `CORS_ALLOWED_ORIGINS`** : la configuration livrée aux équipes tombe donc systématiquement dans la branche permissive. +Le commentaire renvoie la responsabilité à nginx, mais `app/nginx.conf` **ne contient aucune directive CORS**. Et `app/.env.exemple` — y compris dans sa version nettoyée — **ne définit toujours pas `CORS_ALLOWED_ORIGINS`** : la configuration livrée aux équipes tombe donc systématiquement dans la branche permissive. -**Impact.** N'importe quel site tiers visité par un utilisateur connecté peut lire, avec ses cookies de session, le contenu de toutes les routes `GET` — notamment : -- `/users/disponibilities` : disponibilités de tous les joueurs actifs, avec noms d'utilisateur ; -- `/matches/api/events` : calendrier complet, participants, sessions 1:1 approuvées ; -- `/users/profile`, `/users//view` : données personnelles. +**Impact.** N'importe quel site tiers visité par un utilisateur connecté peut lire, avec ses cookies de session, le contenu de toutes les routes `GET` : +- `/users/disponibilities` — disponibilités de tous les joueurs actifs, avec noms d'utilisateur ; +- `/matches/api/events` — calendrier complet, participants, sessions 1:1 approuvées ; +- `/users/profile`, `/users//view` — données personnelles. -La protection CSRF (`CSRFProtect`) limite les écritures, mais n'empêche pas ces lectures. +La protection CSRF limite les écritures, mais n'empêche pas ces lectures. **Correction.** @@ -142,75 +121,61 @@ elif app.debug: # sinon : pas de CORS du tout — même origine uniquement ``` -L'application étant rendue côté serveur (Jinja2) et consommant ses propres API en même-origine, **le cas nominal est de ne pas activer CORS du tout**. Documenter `CORS_ALLOWED_ORIGINS` dans le fichier d'exemple. - ---- - -## SEC-04 · 🔴 - -**Token du bot Discord imprimé sur stdout à l'import** - -`app/discord_bot.py:24-25` - -```python -DISCORD_BOT_TOKEN = os.getenv('DISCORD_BOT_TOKEN') -print(DISCORD_BOT_TOKEN or 'FAILED TO PRINT BOT TOKEN') -``` - -Le token est écrit en clair sur la sortie standard **à chaque import du module**, donc à chaque démarrage de l'application. - -**Impact.** Le secret se retrouve dans les logs du superviseur de processus, les journaux de la plateforme d'hébergement, les logs de conteneur, et la sortie des jobs CI. Ces destinations sont typiquement conservées longtemps, indexées, et accessibles à un public plus large que les variables d'environnement elles-mêmes. - -À noter : le `SensitiveDataFilter` de `logging_config.py` ne peut rien ici — il filtre les enregistrements du module `logging`, pas les appels à `print()`. - -**Correction.** Supprimer la ligne. Si un diagnostic de configuration est nécessaire au démarrage : - -```python -logger.info('Discord bot token: %s', 'configuré' if DISCORD_BOT_TOKEN else 'ABSENT') -``` +L'application étant rendue côté serveur et consommant ses propres API en même-origine, **le cas nominal est de ne pas activer CORS du tout**. Documenter `CORS_ALLOWED_ORIGINS` dans le fichier d'exemple. --- ## SEC-05 · 🟠 -**En-têtes de proxy non validés → contournement HTTPS et rate limiting inopérant** +**`trusted_proxy='*'` sur interface publique → en-têtes de proxy usurpables** -L'application lit `X-Forwarded-Proto` pour décider d'appliquer HSTS et la redirection HTTPS : +`wsgi.py:29-42` -`app/app.py:163` — `is_https = request.is_secure or request.headers.get('X-Forwarded-Proto') == 'https'` -`app/app.py:182` — `if not request.is_secure and request.headers.get('X-Forwarded-Proto') != 'https':` +```python +host = os.getenv('HOST', '0.0.0.0') # Bind to localhost by default (Nginx reverse proxy) +... +serve( + app, host=host, port=port, threads=threads, + channel_timeout=30, cleanup_interval=30, + # Waitress Proxy Settings + trusted_proxy='*', + trusted_proxy_count=1, + trusted_proxy_headers={'x-forwarded-for', 'x-forwarded-proto'}, + clear_untrusted_proxy_headers=True +) +``` -Or **`ProxyFix` n'est jamais appliqué** et aucune liste de proxys de confiance n'est configurée. L'en-tête est accepté tel quel, quelle que soit sa provenance. +L'intention est correcte et `clear_untrusted_proxy_headers=True` est le bon réflexe. Deux réglages annulent le bénéfice : -Ce défaut est amplifié par la configuration réseau : +- **`trusted_proxy='*'`** demande à Waitress d'accepter `X-Forwarded-For` et `X-Forwarded-Proto` **de n'importe quel pair**, pas seulement de nginx ; +- **`HOST` vaut `0.0.0.0`** — le commentaire de la ligne 26 annonce l'inverse de ce que fait le code. Le port applicatif est donc joignable directement, en contournant nginx. -- `wsgi.py:26` — `host = os.getenv('HOST', '0.0.0.0')`, avec le commentaire trompeur *« Bind to localhost by default »*. Le serveur Waitress écoute en réalité sur **toutes les interfaces**. -- `app/nginx.conf:122` — `proxy_pass http://0.0.0.0:5000;` — `0.0.0.0` n'est pas une adresse de destination valide comme cible amont ; ce devrait être `127.0.0.1`. +**Impact.** La combinaison des deux rend les en-têtes de proxy entièrement contrôlables par un attaquant qui atteint le port applicatif : -**Impact.** -1. Le port de l'application est joignable directement, en contournant nginx — donc sans TLS, sans les en-têtes de sécurité ajoutés par nginx. -2. En envoyant `X-Forwarded-Proto: https` sur cette connexion en clair, on désactive la redirection HTTPS de `force_https()` et l'application se comporte comme si la connexion était sécurisée. -3. **Corollaire plus grave — le rate limiting est neutralisé.** `app/extensions.py:16-19` utilise `key_func=get_remote_address`, qui lit `request.remote_addr`. Sans `ProxyFix`, cette valeur est l'IP de nginx pour *toutes* les requêtes proxifiées. Conséquences : - - la limite de `10 per minute` sur `/auth/login` (`auth.py:79`) devient un **seau global partagé par tous les utilisateurs** — la protection anti-bruteforce ne fonctionne pas par attaquant ; - - inversement, un seul client peut consommer le quota global et **bloquer le login de toute l'organisation** (déni de service trivial) ; - - les limites par défaut `200/jour, 50/heure` s'appliquent à l'ensemble du trafic, ce qui rendra l'application inutilisable en usage normal dès quelques utilisateurs simultanés. +1. **Contournement du rate limiting et du suivi de tentatives.** `app/extensions.py:16-19` utilise `key_func=get_remote_address`, qui lit `request.remote_addr` — valeur que Waitress renseigne désormais depuis `X-Forwarded-For`. Un attaquant qui change cet en-tête à chaque requête obtient **un seau de rate limiting neuf à chaque fois** : la limite de `10 per minute` sur `/auth/login` (`auth.py:96`) ne s'applique plus, ni les limites par défaut `200/jour, 50/heure`. + + Il faut noter que ce changement **a inversé la nature du risque**. Avant, `remote_addr` valait toujours l'IP de nginx : la protection était mal calibrée (un seau global partagé par tous) mais fermée. Elle est maintenant ouverte. + + Le verrouillage de compte après 5 échecs (`auth.py:167-181`) reste opérant, puisqu'il est indexé sur l'utilisateur et non sur l'IP — c'est aujourd'hui la seule barrière anti-bruteforce réellement en place. + +2. **Contournement de la redirection HTTPS.** `app/app.py:182` teste `X-Forwarded-Proto`. En l'envoyant à `https` sur une connexion en clair, on désactive `force_https()` et l'application se comporte comme si la connexion était sécurisée. Même effet sur l'émission de HSTS (`app.py:163`). + +3. **Journalisation empoisonnée.** Toute IP écrite dans les logs devient une valeur fournie par le client — ce qui rend l'analyse post-incident peu fiable (et affaiblit d'avance la correction proposée en [SEC-19](#sec-19--)). **Correction.** ```python -from werkzeug.middleware.proxy_fix import ProxyFix - -# après la création de l'app, uniquement si l'on est réellement derrière un proxy -if os.getenv('BEHIND_PROXY', 'false').lower() == 'true': - app.wsgi_app = ProxyFix(app.wsgi_app, x_for=1, x_proto=1, x_host=1, x_port=1) +host = os.getenv('HOST', '127.0.0.1') +... + trusted_proxy=os.getenv('TRUSTED_PROXY', '127.0.0.1'), + trusted_proxy_count=1, + trusted_proxy_headers={'x-forwarded-for', 'x-forwarded-proto'}, + clear_untrusted_proxy_headers=True, ``` -`x_for=1` indique de ne faire confiance qu'au dernier saut — celui de nginx. Ne jamais activer ce middleware si l'application n'est pas derrière un proxy, sinon `X-Forwarded-For` devient falsifiable par le client. +Les deux corrections sont nécessaires : `trusted_proxy` restreint *qui* peut envoyer ces en-têtes, `HOST` restreint *qui peut se connecter*. Si l'hébergement Pterodactyl impose une écoute sur `0.0.0.0`, renseigner alors l'adresse réelle du proxy dans `TRUSTED_PROXY` et filtrer le port au pare-feu. -En complément : -- `wsgi.py` : passer le défaut de `HOST` à `127.0.0.1` (ce que le commentaire annonce déjà) ; -- `nginx.conf:122` : `proxy_pass http://127.0.0.1:5000;` ; -- filtrer au pare-feu le port applicatif. +Vérifier ensuite que `request.remote_addr` renvoie bien l'IP cliente, et non celle du proxy, avant de considérer le rate limiting comme fonctionnel. --- @@ -218,23 +183,23 @@ En complément : **Schémas de validation importés mais jamais appliqués sur les routes utilisateurs** -`app/routes/users.py:23-26` importe `CreateUserSchema`, `EditUserSchema` et `EditProfileSchema`. Vérification par comptage d'occurrences : **chacun de ces trois noms n'apparaît qu'une seule fois dans le fichier — sur la ligne d'import**. Ils ne sont jamais instanciés. +`app/routes/users.py:23-26` importe `CreateUserSchema`, `EditUserSchema` et `EditProfileSchema`. Chacun de ces trois noms n'apparaît **qu'une seule fois dans le fichier — sur la ligne d'import**. Ils ne sont jamais instanciés. Les trois routes concernées lisent le formulaire brut : -| Route | Lignes | Traitement | +| Route | Lignes | Traitement du mot de passe | |---|---|---| -| `create_user` | `:196-226` | `request.form.get('password')` → `hash_password(password)` directement | -| `edit_user` | `:98-133` | `password = request.form.get('password')` → `hash_password(password)` si non vide | -| `edit_profile` | `:255-295` | idem, sur son propre compte | +| `edit_user` | `:88-152` | `request.form.get('password')` → `hash_password()` si non vide | +| `create_user` | `:201-241` | `request.form.get('password')` → `hash_password()` sans contrôle | +| `edit_profile` | `:274-320` | idem, sur son propre compte | -Comparaison avec `auth.py:206-218`, où `RegisterSchema` **est** correctement chargé — l'inscription publique est donc validée, mais pas les trois autres chemins de création/modification de compte. +Comparaison avec `auth.py:228-230`, où `RegisterSchema` **est** correctement chargé — l'inscription publique est validée, mais pas les trois chemins de création/modification de compte. **Impact.** - **Aucune politique de mot de passe** sur ces routes : `a` est accepté. Un président créant les comptes de l'équipe peut leur attribuer des mots de passe d'un caractère, et n'importe quel utilisateur peut affaiblir le sien via `edit_profile`. - **Aucune validation de format** sur `email` (le champ n'est même pas vérifié comme étant une adresse), `username`, `phone`. -- `create_user` (`:216`) appelle `hash_password(password)` sans vérifier que `password` est non vide : un `password_hash` d'une chaîne vide est stocké, et le compte devient accessible avec un mot de passe vide. -- Dans `edit_user` (`:115-118`), `full_name` et `email` sont assignés sans contrôle de nullité, alors que les colonnes sont `nullable=False` (`models/user_model/user.py:20-21`) → `IntegrityError` non gérée → 500. +- `create_user` appelle `hash_password(password)` sans vérifier que `password` est non vide : le hachage d'une chaîne vide est stocké, et le compte devient accessible avec un mot de passe vide. +- Dans `edit_user`, `full_name` et `email` sont assignés sans contrôle de nullité alors que les colonnes sont `nullable=False` (`models/user_model/user.py:20-21`) → `IntegrityError` non gérée → 500. **Correction.** Appliquer les schémas déjà écrits, sur le modèle de `auth.py` : @@ -249,36 +214,69 @@ except ValidationError as err: return render_template('pages/edit_profile.html', ...) ``` -Attention : `EditUserSchema` et `EditProfileSchema` déclarent `password` avec `load_default=''` et `validate=validate_password` — un mot de passe vide échouera donc la validation. Il faut soit passer `validate=validate.And(...)` conditionnel, soit retirer le champ du payload quand il est vide avant le `load()`. +Attention : `EditUserSchema` et `EditProfileSchema` déclarent `password` avec `load_default=''` et `validate=validate_password` — un mot de passe vide échouera donc la validation. Retirer le champ du payload quand il est vide avant le `load()`. + +Point d'attention pour `edit_user` : le bloc de changement de rôle par SQL brut (`:114-126`) exécute `db.session.remove()`. Toute validation doit intervenir **avant** ce bloc, sinon l'objet `user` manipulé ensuite n'est plus celui qui a été validé. --- ## SEC-07 · 🟠 -**`discord_user_id` arbitraire → détournement des notifications privées** +**Identifiant Discord modifiable malgré l'OAuth2 → détournement des notifications** -`app/routes/users.py:262-263` et `:283-284` (route `edit_profile`, accessible à **tout utilisateur authentifié**) : +Un flux OAuth2 Discord complet a été ajouté (`auth.py:326-456`). Il obtient de Discord un identifiant authentifié et le stocke en session : ```python -discord_user_id = request.form.get('discord_user_id', '').strip() -... -current_user.discord_user_id = discord_user_id or None +session['discord_oauth'] = { + 'id': user_data.get('id'), # auth.py:448 — valeur authentifiée par Discord + 'username': user_data.get('username'), + ... +} ``` -Aucune validation (conséquence de SEC-06 : `validate_discord_user_id` existe dans `validators.py:83-97` mais n'est pas appelée), et **aucune vérification de propriété** : rien ne prouve que l'utilisateur contrôle réellement ce compte Discord. Aucune contrainte d'unicité sur la colonne non plus (`models/user_model/user.py:32`). +Mais cette valeur est ensuite **réinjectée dans le formulaire par un champ caché** : -**Impact.** Un joueur peut renseigner l'identifiant Discord d'une autre personne — un coach, un membre de la direction. Il reçoit alors à sa place les messages privés du bot. Selon les flux décrits dans le README, cela inclut : -- les demandes de sessions 1:1 avec leurs *« discussion points »*, souvent confidentiels ; -- les notifications de matchs et d'entraînements ; -- surtout, **la capacité de répondre à la place de la cible** : `discord_bot.py` traite les réactions ✅/❌ en DM pour accepter ou refuser une demande 1:1 (`on_reaction_add`, `:118`). L'attaquant obtient donc un pouvoir de décision qui ne lui appartient pas. +```html + + +``` + +et c'est la valeur **renvoyée par le client** qui est enregistrée (`auth.py:253` puis `:290`) : + +```python +discord_user_id = validated.get('discord_user_id') +... +user = Player(..., discord_user_id=discord_user_id, ...) +``` + +**La preuve d'identité obtenue de Discord est perdue au passage.** Un utilisateur peut modifier le champ caché avant envoi, ou poster directement le formulaire sans jamais avoir ouvert le flux OAuth2. `RegisterSchema` valide désormais le format (`validators.py:211-215` : 17-20 chiffres), ce qui est un progrès, mais ne prouve rien sur la propriété du compte. + +La route `edit_profile` (`users.py:284`, `:305`), accessible à **tout utilisateur authentifié**, permet de la même façon de définir un identifiant arbitraire — et n'applique même pas la validation de format ([SEC-06](#sec-06--)). Aucune contrainte d'unicité n'existe sur la colonne (`models/user_model/user.py:32`). + +**Impact.** Un joueur peut renseigner l'identifiant Discord d'une autre personne — un coach, un membre de la direction. Il reçoit alors à sa place les messages privés du bot : +- demandes de sessions 1:1 avec leurs *« discussion points »*, souvent confidentiels ; +- notifications de matchs et d'entraînements ; +- surtout, **la capacité de répondre à la place de la cible** : `discord_bot.py` traite les réactions ✅/❌ en message privé pour accepter ou refuser une demande 1:1. L'attaquant obtient un pouvoir de décision qui ne lui appartient pas. Deux utilisateurs peuvent en outre déclarer le même identifiant, ce qui rend le comportement non déterministe. -**Correction.** -1. Appliquer `validate_discord_user_id` (corrigé par SEC-06) — nécessaire mais très insuffisant : il ne vérifie que le format 17-20 chiffres. -2. Ajouter une contrainte d'unicité sur `User.discord_user_id`. -3. **Implémenter une vérification de possession** : à la saisie, envoyer un code à usage unique en DM sur l'identifiant déclaré et exiger sa saisie sur la plateforme avant d'activer le lien. C'est la seule correction qui traite réellement le problème. -4. En attendant, réserver la modification de ce champ aux administrateurs. +**Risque supplémentaire, nouveau :** le flux *paraît* vérifié. Un relecteur qui constate la présence d'un OAuth2 Discord conclura raisonnablement que l'identifiant est authentifié. Il ne l'est pas. + +**Correction.** La brique manquante est courte — la valeur authentifiée existe déjà en session, il suffit de ne plus la faire transiter par le client : + +```python +# auth.py, dans register() — ignorer ce que le formulaire prétend +discord_oauth = session.get('discord_oauth') or {} +discord_user_id = discord_oauth.get('id') # seule source de vérité +discord_username = discord_oauth.get('username') +``` + +et retirer le champ caché de `register.html:61` (l'afficher en lecture seule si l'on veut le montrer à l'utilisateur). + +En complément : +1. Ajouter une contrainte d'unicité sur `User.discord_user_id` (nécessite [STD-01](03-standards-stack.md#std-01--)). +2. Pour `edit_profile`, soit passer par le même flux OAuth2, soit réserver la modification de ce champ aux administrateurs. +3. Traiter [SEC-20](#sec-20--) : sans paramètre `state`, le contenu de `session['discord_oauth']` n'est lui-même pas digne de confiance. --- @@ -295,14 +293,14 @@ limiter = Limiter( ) ``` -Aucun `storage_uri` n'est fourni. Flask-Limiter bascule alors sur son backend `memory://`, qui est explicitement documenté comme non destiné à la production (la bibliothèque émet d'ailleurs un avertissement au démarrage). +Aucun `storage_uri` n'est fourni. Flask-Limiter bascule sur son backend `memory://`, explicitement documenté comme non destiné à la production. **Impact.** -- L'état est **par processus**. `wsgi.py:25` démarre Waitress avec `cpu_count() * 2 + 1` threads — cela reste un processus, donc le compteur est partagé ici ; mais toute évolution vers plusieurs workers ou plusieurs instances (montée en charge, déploiement bleu-vert) fragmente les compteurs et multiplie d'autant la limite effective. -- L'état est **perdu à chaque redémarrage** : un attaquant peut réinitialiser les compteurs si un redéploiement survient, et le verrouillage anti-bruteforce ne survit pas aux mises à jour. -- Combiné à SEC-05, la protection est de toute façon appliquée à la mauvaise clé. +- L'état est **par processus**. Toute évolution vers plusieurs workers ou plusieurs instances fragmente les compteurs et multiplie d'autant la limite effective. +- L'état est **perdu à chaque redémarrage** : les compteurs se réinitialisent à chaque redéploiement. +- Combiné à [SEC-05](#sec-05--), la protection est de toute façon indexée sur une clé que le client contrôle. -**Correction.** Adosser le limiteur à un stockage partagé — Redis de préférence, ou la base PostgreSQL déjà présente si l'on veut éviter une dépendance supplémentaire : +**Correction.** Adosser le limiteur à un stockage partagé — Redis de préférence, ou la base PostgreSQL déjà présente : ```python limiter = Limiter( @@ -312,7 +310,104 @@ limiter = Limiter( ) ``` -Réévaluer aussi les valeurs : `50 per hour` par IP est très bas pour une application web rendue côté serveur, où chaque page consomme plusieurs requêtes (`/matches/api/events`, `/users/disponibilities`…). Exclure les routes `/static` et `/health` du décompte. +Réévaluer aussi les valeurs : `50 per hour` par IP est très bas pour une application rendue côté serveur, où chaque page consomme plusieurs requêtes (`/matches/api/events`, `/users/disponibilities`…). Exclure `/static` et `/health` du décompte. + +Corriger [SEC-05](#sec-05--) d'abord : sans clé fiable, un stockage partagé ne sert à rien. + +--- + +## SEC-20 · 🟠 + +**Flux OAuth2 Discord sans paramètre `state`** + +`app/routes/auth.py:340-348` + +```python +params = { + 'client_id': DISCORD_CLIENT_ID, + 'redirect_uri': DISCORD_REDIRECT_URI, + 'response_type': 'code', + 'scope': 'identify connections', +} +query = '&'.join(f'{k}={requests.utils.quote(v)}' for k, v in params.items()) +auth_url = f'{DISCORD_API_BASE}/oauth2/authorize?{query}' +``` + +Aucun paramètre `state` n'est émis, et `discord_callback` (`:351-456`) n'en vérifie aucun : la requête de retour est acceptée sur la seule présence d'un `code` (`:363-366`). + +`state` est le mécanisme anti-CSRF prévu par la RFC 6749 (§10.12) : une valeur aléatoire liée à la session, émise à l'aller et vérifiée au retour. + +**Impact.** Un attaquant initie le flux avec **son propre** compte Discord, intercepte le `code` d'autorisation sans le consommer, puis amène la victime à visiter : + +``` +https:///auth/discord/callback?code= +``` + +Le serveur échange ce code, obtient le profil Discord **de l'attaquant**, et le place dans `session['discord_oauth']` — la session **de la victime**. Celle-ci voit alors le formulaire d'inscription pré-rempli avec l'identité Discord de l'attaquant, et l'enregistre sans raison de se méfier (le bandeau affiche *« Discord account connected! »*, `auth.py:455`). + +Combiné à [SEC-07](#sec-07--), c'est un chemin direct pour faire enregistrer à un tiers un `discord_user_id` contrôlé par l'attaquant — donc pour recevoir les notifications privées de ce compte. + +Le flux ne servant aujourd'hui qu'à pré-remplir un formulaire d'inscription, la portée est bornée. Elle s'étendrait immédiatement si l'OAuth2 était réutilisé pour l'authentification ou pour la liaison de compte a posteriori, ce qui est l'évolution naturelle de ce code. + +**Correction.** + +```python +# auth.py — discord_login() +state = secrets.token_urlsafe(32) +session['discord_oauth_state'] = state +params = {..., 'state': state} + +# auth.py — discord_callback() +expected = session.pop('discord_oauth_state', None) +if not expected or not secrets.compare_digest(request.args.get('state', ''), expected): + flash('Requête Discord invalide.', 'danger') + return redirect(url_for('auth.register')) +``` + +Deux points secondaires dans le même bloc : +- `requests.utils.quote(v)` (`:346`) lève un `TypeError` si `DISCORD_REDIRECT_URI` n'est pas défini — seul `DISCORD_CLIENT_ID` est vérifié (`:336`). Vérifier les trois variables, ou construire l'URL avec `urllib.parse.urlencode`. +- `:389` — `flash(f'Failed to connect to Discord. Please try again.', 'danger')` est une f-string sans substitution, et la variable `e` capturée à la ligne 388 n'est pas utilisée. Sans conséquence, mais signalé par Ruff (`F541`, `F841`). + +--- + +## SEC-21 · 🟠 + +**Le déploiement SFTP pousse le répertoire `.git/` sur le serveur** + +`.gitea/workflows/git-to-ptero.yaml:41` + +```yaml +mirror -R --verbose --parallel=4 ./ ./ +``` + +`mirror -R` téléverse le répertoire courant du runner vers la racine distante. Après un `actions/checkout`, ce répertoire contient **`.git/` en entier** — objets, packfiles, refs, historique complet. + +**Impact.** Si la racine du dépôt est aussi la racine servie par le serveur web — ce qui est le cas par défaut sur un hébergement de type Pterodactyl où l'application est déployée telle quelle — alors `.git/` devient téléchargeable. Les outils d'exploitation courants (`git-dumper` et équivalents) reconstruisent l'intégralité du dépôt à partir de `.git/config`, `.git/HEAD` et des objets, même sans listing de répertoire activé. + +L'attaquant obtient alors le code source complet **et tout l'historique** — donc le commit `2d3721b` et les secrets de production de [SEC-01](#sec-01--). Les deux constats se composent : la purge d'historique demandée en SEC-01 perd son intérêt si le déploiement republie l'historique à chaque exécution. + +Deux points secondaires du même workflow : + +- **`StrictHostKeyChecking=no`** (`:35`) désactive la vérification de l'empreinte du serveur SFTP : le premier échange est vulnérable à une interception, et la clé privée de déploiement peut être présentée à un serveur usurpateur. Utiliser un `known_hosts` épinglé, alimenté par `ssh-keyscan` une fois et stocké en secret de dépôt. +- **Aucune exclusion.** Outre `.git/`, le miroir pousse `__pycache__/`, les éventuels `logs/` et tout fichier résiduel du runner. + +**Correction.** + +```yaml +mirror -R --verbose --parallel=4 \ + --exclude-glob .git/ \ + --exclude-glob .github/ \ + --exclude-glob .gitea/ \ + --exclude-glob __pycache__/ \ + --exclude-glob '*.pyc' \ + --exclude-glob logs/ \ + --exclude-glob documents/ \ + ./ ./ +``` + +Plus robuste : construire un artefact de déploiement propre (`git archive HEAD | tar -x -C dist/`) et ne téléverser que `dist/`. `git archive` n'inclut par construction ni `.git/` ni les fichiers ignorés. + +Vérifier par ailleurs, sur le serveur actuel, si `.git/` est déjà présent et accessible — le workflow est déclenchable manuellement (`workflow_dispatch`, `:4`) et a pu être exécuté. Le cas échéant, supprimer le répertoire distant en plus d'appliquer le correctif. --- @@ -331,9 +426,9 @@ def nl2br(value): `markupsafe.Markup()` **désactive l'échappement automatique de Jinja2** pour la chaîne produite. Le contenu utilisateur est inséré tel quel, sans passer par `escape()`. -**Statut actuel : non exploitable.** Une recherche sur l'ensemble des templates ne trouve **aucune utilisation de `|nl2br`**. Le filtre est enregistré (`app.py:125`) mais mort. +**Statut actuel : non exploitable.** Aucune utilisation de `|nl2br` dans les templates. Le filtre est enregistré (`app.py:125`) mais mort. -**Risque.** C'est un piège en attente : le filtre porte un nom naturel, il est enregistré globalement, et la première personne qui écrira `{{ note.content|nl2br }}` — un usage évident sur `PersonalNote`, `TeamNote` ou les *discussion points* des 1:1 — introduira une XSS stockée sans s'en rendre compte. Ces contenus sont saisis par des coachs et joueurs et affichés à d'autres utilisateurs. +**Risque.** Piège en attente : le filtre porte un nom naturel, il est enregistré globalement, et la première personne qui écrira `{{ note.content|nl2br }}` — usage évident sur `PersonalNote`, `TeamNote` ou les *discussion points* des 1:1 — introduira une XSS stockée sans s'en rendre compte. Ces contenus sont saisis par des coachs et joueurs, et affichés à d'autres utilisateurs. **Correction.** Échapper avant de marquer : @@ -346,7 +441,7 @@ def nl2br(value): return Markup('
').join(escape(str(value)).splitlines()) ``` -`escape()` neutralise le HTML utilisateur ; seuls les `
` insérés par le filtre restent actifs. Alternative sans code : supprimer le filtre et utiliser `white-space: pre-line` en CSS. +Alternative sans code : supprimer le filtre et utiliser `white-space: pre-line` en CSS. --- @@ -359,20 +454,21 @@ def nl2br(value): ```python "script-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net; " "style-src 'self' 'unsafe-inline' https://cdnjs.cloudflare.com https://cdn.jsdelivr.net; " +"img-src 'self' data: https://cdn.discordapp.com; " ``` `'unsafe-inline'` sur `script-src` **annule l'essentiel du bénéfice de la CSP** : c'est précisément l'injection de `