Le commit precedent finissait teams.py en notant que le defaut venait d'une
correction appliquee a un seul endroit. Balayer les autres modules
immediatement, plutot que d'attendre qu'une passe d'audit les retrouve, a
sorti les deux derniers.
tryouts.register_player lisait int(request.form.get('player_id')) -- 500 sur
une valeur non numerique -- et verifiait le role sans regarder
is_active_account. Un compte desactive pouvait donc etre inscrit a une
selection.
users/contracts._selectable_players ne filtrait pas non plus les comptes
desactives dans sa branche non-coach : la liste de depot de contrat proposait
encore des gens partis du club. Un contrat est un document nominatif signe.
PlayerSelectionSchema porte desormais le champ, et TeamPlayerSchema en herite
en ajoutant son statut. Un schema partage est ce qui empeche le prochain
appelant d'etre oublie -- c'est precisement parce que chaque route avait le
sien, ecrit a la main, que la correction a du etre faite trois fois.
Verifie par mutation.
Co-Authored-By: Claude Opus 5 <[email protected]>
La vague K a pose un schema sur create_team et edit_team et a laisse cinq
routes soeurs du meme fichier lire int(request.form.get(...)) : add_coach,
add_manager, remove_coach, remove_manager et add_player. Un identifiant non
numerique y etait un 500 dans chacune.
C'est exactement la lecon que ce projet repete depuis la vague D -- corriger
un motif fautif dans une seule couche le laisse dans les autres -- et cette
fois c'est le correctif lui-meme qui l'a commise. Notee comme telle.
Deux defauts de plus, trouves en finissant.
add_player ecrivait status tel quel dans une colonne NOT NULL String(20). Et
toggle_player_status lit "substitute si status == starter, sinon starter" :
une valeur inconnue devenait donc starter a la premiere bascule, c'est-a-dire
promouvait son porteur. Liste blanche dans TEAM_PLAYER_STATUSES.
Et aucune de ces routes ne regardait is_active_account. La requete qui
alimente la liste deroulante des joueurs ne le filtrait pas non plus, alors
que les deux requetes juste au-dessus, coachs et gerants, le posaient -- deux
lignes d'ecart, meme fichier. Un compte desactive etait donc propose et
accepte, alors que is_active_account est precisement ce qui dit que la
personne a quitte le club. Meme oubli dans tryouts.py.
_staff_member delegue desormais a _assignable au lieu de repeter isinstance :
deux fonctions du meme fichier repondant differemment a "ce compte peut-il
prendre ce role" est la forme de tous les defauts qu'a eus ce module.
Verifie par mutation : retirer le controle d'activite ou la liste blanche
fait tomber trois tests.
Co-Authored-By: Claude Opus 5 <[email protected]>
MNT-14. Chaque vue de liste faisait .all() sur sa table. L'audit evaluait
l'impact a nul -- justement, a l'echelle d'une association etudiante -- et
recommandait de choisir le motif maintenant plutot que de le retro-adapter
plus tard. C'est ce que ceci est.
Applique a list_users, list_evaluations et team_matches.list_matches. Pour
cette derniere, la pagination borne aussi la boucle sur les participants,
qui est le N+1 que le constat designait comme le premier a se degrader.
Trois decisions, parce que ce sont celles qui se prennent deux fois
differemment sinon.
error_out=False : les numeros de page arrivent par l'URL, donc ?page=999 est
une chose qu'on tape ou qu'un signet perime contient. Le defaut de
Flask-SQLAlchemy y repond par un 404, ce qui est deroutant pour quelqu'un qui
est simplement alle une page trop loin.
Un plafond sur per_page : c'est aussi un parametre d'URL, et sans plafond
?per_page=100000 redonne a la main exactement la requete non bornee que la
pagination existe pour empecher.
page_url est un global Jinja plutot qu'une valeur que chaque vue passe. Ce
qui se rate avec des liens de pagination, c'est le reste de la chaine de
requete : la liste d'evaluations porte sort et order, celle des matchs
d'equipe porte team_id, et un lien qui les perd reinitialise silencieusement
la vue que la personne regardait. Les deux tests qui l'epinglent tombent si
page_url cesse de les recopier -- verifie par mutation.
Les tris sont completes par une cle unique : une requete paginee sans ORDER
BY stable peut montrer la meme ligne deux fois et jamais une autre.
Au passage, huit entrees fuzzy corrigees dans les catalogues, dont deux
laissees par le commit SEC-16 : une entree fuzzy est ignoree a l'execution,
donc ces messages retombaient en anglais.
Co-Authored-By: Claude Opus 5 <[email protected]>
MNT-12. Le meme bloc de parsing date/heure vivait dans quatre modules avec
trois reponses differentes a la meme saisie invalide : signaler et rediriger,
mettre la valeur a None et annoncer la reussite, ou passer au suivant en
silence. Les vagues E et G ont ferme le cote matchs avec des schemas ; il
restait la disponibilite, les creneaux de coach et les demandes individuelles.
Deux choses trouvees en appliquant, aucune dans le constat.
OneOnOneRequestSchema et DisponibilityAddSchema etaient definis dans
validators.py et appeles NULLE PART : aucun import, aucun test. C'est le
motif SEC-AUTHZ-001 -- une politique de validation ecrite et non appliquee --
qui survivait dans un coin que personne n'avait rouvert. Les deux passaient
d'ailleurs par des fields.String + Regexp, qui verifient la forme et laissent
l'appelant convertir ; fields.Date et fields.Time font les deux.
Et manage_coach_availability supprimait tous les creneaux existants avant de
reajouter ceux qu'il savait lire, ignorant les autres en silence et repondant
{'success': true}. Un envoi malforme effacait donc les heures reservables
d'un coach en annoncant la reussite -- et les demandes individuelles sont
refusees contre exactement cette table, donc le coach devenait injoignable
sans que rien ne le dise. Une operation de remplacement doit tout valider
avant de rien supprimer : le lot est refuse en entier.
Trouve aussi : le controle de disponibilite comparait les chaines du
formulaire aux chaines serialisees, ce qui ne marchait que parce que les deux
cotes etaient en HH:MM a zero non significatif. La comparaison porte
desormais sur des objets time.
Et le lint a rattrape une regression que la relecture avait manquee --
sixieme fois : datetime retire de one_on_one.py alors que deux fonctions non
touchees l'utilisaient encore.
Co-Authored-By: Claude Opus 5 <[email protected]>
SEC-16, de l'audit anterieur. La vague G a pose un schema a la frontiere de
matches, team_matches, tryouts et evaluations, et le HANDOFF en a tire une
regle : tout champ de formulaire passe par un schema de app/validators.py,
pas par request.form.get. teams.py ne l'avait jamais appliquee.
Deux defauts, pas un.
int(request.form.get('coach_id')) leve sur une valeur non numerique : une
soumission fabriquee etait un 500.
Et l'identifiant obtenu etait ensuite resolu sans verifier le role du compte,
dans deux des trois endroits qui le faisaient. La branche sync_staff
d'edit_team testait isinstance(user, Coach) ; son autre branche non, et
create_team non plus. Le meme fichier en desaccord avec lui-meme, sur
exactement le defaut que la vague G avait corrige dans tryouts.py -- une
soumission fabriquee pouvait nommer un joueur parmi les coachs d'une equipe.
L'identifiant vient d'un <select> rendu par le navigateur : c'est une valeur
que le client choisit.
_staff_member est la reponse unique, et OrgTeamSchema garantit que les
identifiants arrivent en entiers. Les deux tests qui epinglent le role
tombent si la verification saute : verifie par mutation.
Co-Authored-By: Claude Opus 5 <[email protected]>
STD-09, trouve en recroisant l'audit anterieur -- celui mene sur le miroir
GitHub, jamais repasse depuis qu'on a decouvert que ce n'etait pas la bonne
source.
Sept gestionnaires d'erreur portaient chacun leur copie d'une liste de
prefixes d'URL decidant "JSON ou page HTML". Les copies avaient derive --
trois testaient /users/coach-availability, quatre non -- et toutes
manquaient les memes points. Un fetch() qui recoit une page d'erreur HTML
leve en la parsant : sur le calendrier, les listes de selections et
d'equipes restaient vides, sans message dans la page et sans rien dans le
journal.
Deux choses apprises en ecrivant le test, aucune n'etait dans le constat.
L'approche par prefixe ne pouvait pas etre reparee. Trois des seize vues
JSON sont a des chemins qu'aucun prefixe ne distingue des pages HTML
voisines -- /matches/<id>/toggle-presence/<id> et ses deux cousins, que les
gabarits appellent justement en fetch(). Les vues se declarent donc
elles-memes (@json_endpoint, app/api.py), et un test parcourt la carte des
URL pour verifier qu'aucune vue appelant jsonify n'a ete oubliee.
Et surtout : @login_required n'atteint jamais le gestionnaire 401.
Flask-Login intercepte avant et redirige. Les seize points JSON repondaient
donc a une session expiree par une 302 vers un formulaire HTML, quoi que
dise la liste de prefixes. Reecrire la liste seule aurait eu l'air d'un
correctif sans rien changer.
Au passage, le message flash de ce gestionnaire etait la seule chaine de
l'application qui n'avait jamais ete traduite.
Co-Authored-By: Claude Opus 5 <[email protected]>
SEC-AUTHZ-006. delete_team etait la seule des dix operations d'equipe
gardee par la capacite globale can_manage_teams() ; les neuf autres passent
par can_manage_this_org_team(team). Le constat avait raison sur
l'incoherence et tort sur le correctif.
Appliquer sa recommandation telle quelle -- remplacer par la verification
par objet -- ELARGIT l'acces. Coach repond False a la capacite globale et
True pour ses propres equipes : la substitution donnait donc a chaque coach
le pouvoir de supprimer l'equipe qu'il entraine, avec ses notes d'equipe et
son historique de matchs. Le constat raisonnait sur Manager, ou les deux
repondent True, et a manque le role ou elles divergent.
Les deux sont donc exigees. Le comportement d'aujourd'hui est preserve a
l'identique (administrateurs et gerants oui, coachs non) et la dette que le
constat visait est bien fermee : le jour ou Manager.can_manage_this_org_team
sera resserre -- ce qui est souhaitable -- la suppression se resserrera avec
lui au lieu de rester la seule porte ouverte.
C'est la quatrieme recommandation d'audit qu'il faut corriger avant de
l'appliquer. Le test qui l'epingle echoue si quelqu'un refait la
simplification : verifie par mutation.
Co-Authored-By: Claude Opus 5 <[email protected]>
DATA-012. delete_user supprimait les lignes Contract et laissait les PDF.
Des contrats nominatifs signes restaient donc sur le serveur apres la
suppression du compte, sans plus aucune reference en base : invisibles pour
l application, ingerables par elle, et toujours des donnees personnelles.
Les chemins sont lus **avant** que les lignes partent — apres, plus rien ne
dit ou sont les fichiers — et les fichiers sont retires **apres** le commit.
L ordre compte dans ce sens et pas dans l autre : un echec entre les deux
doit laisser un fichier sans ligne, ce qui est recuperable et correspond
exactement a l etat precedent, plutot qu une ligne sans fichier, qui est un
telechargement en 500 pour toujours.
Un fichier deja absent est journalise en info et ignore ; un fichier
impossible a retirer est journalise en erreur avec ce que ca implique — il
devient orphelin, donc plus rien dans l application ne proposera jamais de
le supprimer. Rien ici ne peut faire echouer la suppression du compte : le
compte est la partie que quelqu un a demandee.
Le nombre de fichiers retires part dans le journal d authentification, a
cote de account.deleted.
554 tests.
OPS-011, en partie. Ce que le workflow garantit maintenant :
- rien ne part d un arbre casse. La suite, ruff check et ruff format
tournent sur le runner de deploiement avant tout envoi. Une CI verte sur
GitHub ne prouve rien ici : le deploiement se declenche a la main, sur ce
que la branche contient a cet instant ;
- seuls les fichiers nommes partent. La charge est une liste blanche —
app/, wsgi.py, requirements.txt — et non l arbre de travail moins neuf
exclusions. C est par cette porte que clear_db.py, la suite de tests et
les definitions de CI se sont retrouves sur le noeud de production ;
- le deploiement est verifie. /health est interroge pendant deux minutes
apres l envoi et le job echoue s il ne repond jamais « healthy ». Avant,
un arbre a moitie televerse etait un deploiement vert.
Ce qui n est pas garanti, et c est ecrit dans le fichier : la bascule n est
pas atomique. Le miroir se fait sur place, donc pendant le transfert la
production execute un melange de deux versions.
En cherchant a fermer ce point, un defaut a part entiere est apparu. Les
contrats etaient ranges a os.getcwd()/documents et leur chemin absolu
ecrit en base. La racine de stockage suivait donc le repertoire depuis
lequel le processus avait ete lance : redemarrer le serveur ailleurs
envoie les nouveaux contrats dans un nouvel arbre et rend les anciens
illisibles — la base continuant d affirmer qu ils sont la, la panne se
manifeste par un 500 au telechargement, pas par quelque chose
d actionnable.
app/storage.py fixe la racine et DOCUMENTS_ROOT la deplace. Les nouvelles
lignes gardent un chemin relatif, les anciennes gardent leur chemin absolu
et continuent de resoudre : aucune migration de donnees n est necessaire,
donc ce changement n attend pas Alembic.
C etait aussi le troisieme pre-requis de la bascule par repertoires de
version. Les deux autres sont hors d atteinte d ici — la commande de
demarrage Pterodactyl doit pointer sur current/, et les repertoires
partages doivent etre installes sur le noeud. Les deux sont decrits dans
docs/deployment.md, avec la procedure de retour arriere qui manquait.
511 tests.
ARCH-005, seconde moitie. Meme forme que pour les matchs : des champs lus
a la main sur request.form, deux verifies et le reste cru sur parole.
Cote tryout :
- game pilote la liste des postes et les champs de gamertag montres au
joueur qui s inscrit. Il etait accepte tel quel : une faute de frappe
produisait une selection pour laquelle personne ne pouvait etre evalue ;
- max_players etait int(x) if x else None — un 500 sur « twelve », et un
-3 accepte sans broncher ;
- coach_ids etait charge par User.id.in_(...) sans filtre de role. Une
soumission fabriquee a la main pouvait donc nommer un joueur coach d une
selection, ce qui est une attribution de droits : gerer la selection et
evaluer ses joueurs. Ce n est pas un formulaire que l interface propose,
et ca marchait.
Cote evaluation, validate_score transformait tout ce qui sortait de 1..10
— 11, 0, « bien » — en None. Le critere disparaissait de la moyenne et la
page annoncait l evaluation enregistree. Rien ne distinguait « non
evalue » de « evalue, refuse, et oublie ».
Le calcul de la moyenne remonte sur le modele, en Evaluation.overall_from
et apply_scores. Il vivait dans la route, additionnant neuf variables
locales, et ne pouvait pas etre exerce sans requete HTTP, session
authentifiee et base — c est TEST-002, et c est pourquoi le calcul des
scores n avait aucun test. Il en a maintenant six, sans rien monter.
Une precision qui compte : aucun critere rempli donne None, pas 0. La
grille commence a 1, donc un zero serait une note qu aucun joueur ne peut
recevoir, et qui le classerait sous tout le monde dans la liste.
Les neuf criteres sont ecrits en toutes lettres dans le schema plutot que
generes depuis le modele — un schema se lit — et un test verifie que les
deux listes coincident. C est la garde qui empeche la derive, pas
l astuce.
Douze chaines traduites, dont trois que pybabel avait devinees en fuzzy :
une entree fuzzy est ignoree a l execution, le piege est consigne dans
docs/translations.md.
30 tests neufs. 477 au total.
ARCH-005, premiere moitie. matches.py et team_matches.py lisaient une
quarantaine de champs sur request.form a la main et les croyaient tous.
Ce que ca produisait n etait pas bruyant :
- edit_match attrapait une heure invalide et faisait start_time = None,
puis annoncait que le match etait mis a jour. Le match perdait son
heure et le calendrier l affichait a minuit ;
- match_type etait accepte tel quel. Une valeur inconnue creait un match
auquel aucun joueur n etait rattache, sans un mot ;
- une fin avant le debut etait enregistree telle quelle ;
- title est NOT NULL dans le modele et n etait pas verifie dans la route,
donc un titre vide etait un 500 ;
- 'a,b' dans la selection de joueurs arrivait sur int() sans garde.
app/forms.py rassemble les deux fonctions de frontiere, qui vivaient dans
users/_shared.py parce que c est la qu elles avaient d abord servi. Elles
y restent re-exportees, donc aucun des trente appels n a bouge.
Le mixin des schemas lit desormais un champ vide comme un champ absent.
C est ce qui rendait ces formulaires invalidables : un formulaire HTML
envoie tout ce qu il affiche, donc une date optionnelle non remplie
arrive comme '' et non comme rien. Seuls les champs declares optionnels
sont concernes ; un champ requis laisse vide doit toujours echouer.
Deux duplications absorbees au passage, toutes deux nommees par l audit :
la boucle de creation des participants, ecrite deux fois et deja divergee
— la copie de edit_match gardait ses identifiants en chaines et appelait
int() une ligne plus loin — et le contexte de re-affichage du formulaire,
dont les versions courtes faisaient mourir un refus dans tojson sur un
Undefined : un message de validation devenait un 500.
Limite connue et consignee : le formulaire revient rempli avec les
valeurs enregistrees, pas avec la saisie refusee. Reafficher la
soumission demande de toucher aux gabarits, c est un autre changement.
19 tests neufs sur ces routes, qui n en avaient aucun. 447 au total.
SEC-AUTH-008. L addition a deux operandes entre 1 et 10 avait dix-neuf
reponses possibles et se resolvait en lisant la question comme une chaine.
Elle n arretait aucune inscription automatisee. Elle ajoutait en revanche
une etape a chaque personne, lecteur d ecran compris, contre une apparence
de protection — ce qui est pire que rien, puisque ca se compte comme une
protection.
L autre branche proposee par l audit etait un vrai service de CAPTCHA :
un tiers, une cle d API, une requete a chaque affichage, et un script
etranger remis dans script-src, defaisant le travail qui a ferme
SEC-WEB-001. Disproportionne pour le site d un club.
A la place, deux verifications invisibles pour un visiteur : un champ
piege, cache par la feuille de style et hors du parcours clavier, qu un
robot remplisseur complete et qu une personne ne voit jamais ; et un delai
minimal entre la remise du formulaire et son retour, l horodatage etant
dans la session signee et non dans un champ.
Le plafond est dit dans le code plutot que sous-entendu : ceci arrete le
pourriel de masse, pas quelqu un qui lit la page. Ce qui filtrerait
vraiment les inscriptions serait l activation des comptes par le staff —
is_active_account vaut True par defaut. C est une decision de produit.
L angle « session forgeable » du constat tombe : avec SECRET_KEY
compromise (SEC-001), on forge une session connectee sur n importe quel
compte et on n a aucune raison de s inscrire.
Au passage, ARCH-005 en partie : le bloc « regenerer, purger les mots de
passe, re-rendre » etait recopie quatre fois. Un seul helper, et la purge
des mots de passe ne peut plus etre oubliee dans la cinquieme copie.
Les refus sont journalises avec leur motif — c est le seul endroit ou un
abus du formulaire devient visible — mais restent indistinguables pour
l expediteur : nommer la regle indique comment la contourner.
429 tests.
Active la regle isort (I) de ruff. 45 fichiers reordonnes, aucun
changement de comportement : la suite passe avant comme apres.
app/models/__init__.py en est exclu. Ses imports sont ranges en onze
couches commentees qui decrivent le graphe de dependances ; trier par
ordre alphabetique laisse chaque titre au-dessus d un import qu il ne
decrit pas, et ce fichier n a qu un role, etre lu.
Commit isole, comme le formatage : un diff de brassage ne doit pas
servir de couverture a un changement de comportement.
ARCH-006, avec une requalification du constat.
**Le chiffre de l audit surestimait le probleme.** « 58 commit() en routes
pour un seul rollback() » lisait un ratio comme un defaut. Mesure plutot
que suppose :
- Flask-SQLAlchemy demonte la session a la fin de chaque requete, ce qui
annule tout ce qui n a pas ete commite ;
- l unique rollback est dans le gestionnaire 500, c est-a-dire au bon
endroit ;
- depuis la vague D, aucun module de routes ne contient `except
Exception` : les 34 releves sont dans discord_bot.py, les scripts et le
service de notification, ou avaler l erreur est le comportement voulu
et documente.
Ce que le decompte ne pouvait pas voir, c est le vrai defaut : une fonction
qui commite DEUX fois, ou un echec apres le premier commit laisse une
demi-operation persistee. Il y en avait deux dans tout le depot -- une
analyse AST le confirme. edit_user etait la grave, corrigee avec ARCH-008.
register est la seconde : le compte etait commite, puis les gamertags dans
une seconde transaction. Un echec entre les deux laissait un compte dont
les jeux declares etaient absents, l inscription etant annoncee reussie.
Le premier commit devient un flush -- l identifiant est necessaire pour les
lignes suivantes, pas la durabilite.
Les deux commit() de login ne sont pas concernes : ils sont dans des
branches mutuellement exclusives, succes et echec.
tests/test_transactions.py enonce la garantie plutot que de la supposer :
un echec en cours de requete ne laisse aucune ligne, et l inscription est
tout ou rien. Le second echoue sur le code d avant.
Co-Authored-By: Claude Opus 5 <[email protected]>
PERF-001, la page la plus consultee de l application. Quatre boucles
posaient une requete par ligne :
User.query.get() par inscription
Evaluation.query par joueur inscrit, pour savoir si ce coach
l avait deja evalue
TeamMember.query par equipe
User.query.get() par membre d equipe
Plus, sur chaque match de type player_vs_player, deux interrogations
supplementaires de la relation dynamique `participants` pour trier par
camp -- alors que la liste complete venait d etre chargee douze lignes plus
haut.
Toutes remplacees par un chargement groupe. Les evaluations de ce coach
sont deduites de la liste `evaluations` deja en memoire, pas redemandees.
Mesure, sur un tryout de 10 inscrits, 2 equipes et 1 match :
34 requetes avant, 12 apres. Le test fixe un budget de 25, volontairement
large -- il ne peut que baisser, et il echoue sur le code d avant.
_users_by_id() est le helper partage par les trois chargements ; une ligne
absente est simplement absente du dictionnaire, ce que faisait deja un
get() renvoyant None.
Un second test verifie que les dix joueurs apparaissent toujours sur la
page : une requete groupee qui perd des lignes est le risque reel ici, pas
l erreur bruyante.
Co-Authored-By: Claude Opus 5 <[email protected]>
PERF-002, PERF-003, PERF-004. Aucun changement de comportement : chaque
reecriture est accompagnee de tests qui enoncent la reponse attendue, pas
la methode.
PERF-002 -- /matches/api/events
Le flux parcourait `tryout.matches` pour chaque tryout visible -- pour un
president, tout l historique du club -- puis posait une requete
MatchParticipant PAR match pour savoir si la personne qui regarde y figure.
Le cout du calendrier croissait avec l historique, a chaque navigation.
FullCalendar envoie deja `start` et `end` sur une source d evenements de
type URL. Personne ne les lisait. La requete est desormais bornee, et les
participants de tous les matchs de la fenetre sont charges en une fois,
joueur compris. Des bornes illisibles sont ignorees plutot que refusees :
un calendrier qui en montre trop est un probleme de performance, un
calendrier qui renvoie 400 est une page blanche.
PERF-003 -- get_players_available_at_time
Chargeait tous les joueurs actifs, puis une requete PlayerDisponibility par
joueur, sur une colonne non indexee. Deux requetes desormais, quelle que
soit la taille du club. Mesure dans le test : 7 requetes pour 6 joueurs
avant, 2 apres.
PERF-004 -- decompte des evaluations en attente
Chargeait toutes les inscriptions du club et toutes les evaluations du
coach, construisait deux ensembles Python et les soustrayait -- deux
lectures de table entiere pour produire un entier. Un COUNT DISTINCT avec
NOT EXISTS.
Les tests couvrent ce que la reecriture aurait pu changer sans bruit : fin
de creneau exclusive, compte desactive exclu, joueur a cheval sur deux
creneaux compte une fois, evaluation d un autre coach qui ne libere pas la
ligne, double inscription comptee une fois (DB-006 n a pas encore atterri,
donc le cas existe).
PERF-001 (view_tryout) n est pas fait : c est le plus gros des quatre, il
touche la page la plus consultee et merite son propre lot.
23 tests ajoutes, 371 au total.
Co-Authored-By: Claude Opus 5 <[email protected]>
Seconde moitie d ARCH-003.
Les memes vingt lignes vivaient dans trois routes -- matches.create_match,
matches.edit_match et team_matches.create_match : formater la date et
l heure, puis parcourir deux listes paralleles pour apparier un joueur avec
la ligne de participation qu une reaction Discord doit pouvoir retrouver.
Trois copies, donc trois occasions de diverger. Elles avaient deja diverge :
create_match lisait les heures des variables qu il venait d analyser, et
affichait 'TBD' des qu une des deux manquait ;
edit_match les relisait depuis la ligne enregistree et substituait
l heure de debut a une heure de fin absente.
Un match avec une heure de debut et pas de fin annoncait donc une heure
dans une route et 'TBD' dans l autre, pour la meme donnee.
app/services/scheduling.py retient la regle la plus soigneuse des deux.
zip_participants() isole l appariement par index, qui n est correct que
tant que les deux listes sont construites en phase -- desormais un seul
endroit a relire au lieu de trois, et une liste plus courte donne None,
converti en reference vers le match lui-meme.
matches.py 711 -> 687 lignes, team_matches.py 347 -> 337.
9 tests sur le service seul, sans base ni requete.
Co-Authored-By: Claude Opus 5 <[email protected]>
ARCH-004 et la moitie NotificationService d ARCH-003.
app/routes/users.py faisait 1 699 lignes et couvrait six sujets qui ne
partageaient rien d autre qu un prefixe d URL. Il devient un paquet :
blueprint.py l objet Blueprint, seul
_shared.py helpers de formulaire, gamertags, validation de PDF
accounts.py 348 l. liste, creation, edition, suppression, fiche
availability.py 233 l. disponibilites joueur et creneaux coach
contracts.py 206 l. depot, signature, telechargement
notes.py 376 l. notes d equipe et notes nominatives
one_on_one.py 259 l. demandes de seance individuelle
profile.py 132 l. profil de la personne connectee
Aucun fichier ne depasse 400 lignes -- le critere d acceptation de l audit.
**Un seul blueprint, pas six.** Les endpoints restent `users.*`. Les
renommer aurait touche 137 appels `url_for` dans les gabarits, pour un
benefice nul : l objectif est un fichier qu on peut lire, pas une carte
d URL a reapprendre. Les 30 endpoints sont identiques avant et apres,
verifie sur url_map.
send_discord_notification part dans app/services/notifications.py. Elle
tirait `requests`, `logging` et le bot Discord dans un module dont le sujet
est le traitement HTTP, et se trouvait coincee entre deux definitions de
route. Son `except Exception` est conserve et documente : une notification
qui n arrive pas ne doit pas annuler la transaction qu elle annoncait.
tests/test_route_map.py, nouveau : il parcourt gabarits et code, releve
tout endpoint nomme litteralement dans un url_for, et verifie qu il existe
dans la carte. C est le mode de defaillance de ce genre de decoupage --
pas une erreur a l import, mais un BuildError chez la premiere personne qui
ouvre la page concernee. Un test de garde verifie aussi que le scan trouve
quelque chose, sinon le reste serait vide de sens.
Piege rencontre, et corrige : test_role_change patchait
`app.routes.users.update_user_gamertags`. Apres le decoupage ce nom est un
reexport, pas celui qu accounts.py resout -- le patch aurait pu laisser la
route appeler la vraie fonction et le test passer sans rien verifier. Ici
monkeypatch a echoue bruyamment, mais la cible est desormais explicite et
le test enregistre que la doublure a bien ete appelee.
347 tests passent (263 + 84, dont 82 parametres par la carte des routes).
Co-Authored-By: Claude Opus 5 <[email protected]>
QUA-002, seconde moitie. Le depot etant formate, l elargissement porte sur
des defauts et non sur du brassage.
Ajoute a la selection : B (bugbear), C4, RET, SIM, UP. Le lot entier n a
produit que 24 signalements sur 76 fichiers -- le code etait plus propre
que l audit ne le craignait. Neuf corriges automatiquement, quinze a la
main.
SIM108 est ignore : forcer un ternaire se lit moins bien que le if/else
qu il remplace, au seul endroit ou il se declenche.
isort (I) n est PAS active. Il reordonnerait les imports de 48 fichiers,
soit une seconde passe de pur brassage juste apres le commit de formatage.
A faire, mais seul.
Deux vrais defauts trouves par les nouvelles regles
- team_matches.edit_match faisait `except ValueError: pass` sur l heure de
debut et l heure de fin, trois lignes sous un champ date qui, lui,
signale et redirige. Une heure mal saisie etait donc acceptee par le
formulaire, jetee, l ancienne valeur conservee -- et la page annoncait
la reussite. Meme traitement que la date desormais.
- backup.py levait BackupError depuis deux blocs `except` sans `from`,
ce qui perdait la cause d origine dans la trace.
Ainsi que : un `return` explicite dans force_https, `%`-formatage remplace
dans log_auth_event (operations de chaine avant journalisation, pas des
gabarits de logger -- la redaction n est pas affectee), une compréhension
inutile, un `set(...)` en compréhension d ensemble, `open(..., 'r')`, une
variable de boucle inutilisee, et `contextlib.suppress` dans conftest.
CI : `ruff format --check` remplace le commentaire qui expliquait pourquoi
il etait absent.
263 tests passent. Les deux nouveaux messages sont traduits ; attention,
pybabel les avait apparies en `fuzzy` avec des entrees « date » existantes,
et une entree fuzzy est ignoree a l execution.
Co-Authored-By: Claude Opus 5 <[email protected]>
QUA-002, premiere moitie. **Ce commit ne fait que reformater** : aucun
changement de comportement, aucune ligne de logique touchee. 72 fichiers,
4 restaient deja conformes. Il est isole exprès, pour que `git log -p` sur
les commits voisins reste lisible.
`quote-style = "preserve"` etait deja pose dans pyproject.toml, ce qui
evite le brassage guillemets simples / doubles : le diff porte sur les
retours a la ligne, l indentation des appels longs et les virgules
finales, pas sur le style de chaine.
Verification : 263 tests passent avant et apres, ruff check propre.
L activation en CI arrive dans le commit suivant, separement, pour que ce
diff-ci ne contienne rien d autre.
Co-Authored-By: Claude Opus 5 <[email protected]>
ARCH-007.
Trois points d entree, trois configurations differentes
python app/app.py 0.0.0.0:10000, debogueur desactive par defaut
python run.py 127.0.0.2:5000, debogueur ACTIVE par defaut
python wsgi.py Waitress, production
Le bloc __main__ de app/app.py disparait : ce module expose la fabrique.
run.py reste le point d entree de developpement, wsgi.py celui de
production, et c est tout.
run.py passait FLASK_DEBUG a 'true' par defaut. Le debogueur Werkzeug
execute du code soumis depuis le navigateur ; un processus lance ainsi et
laisse joignable est un shell distant. Le defaut passe a 'false', avec
l avertissement reecrit pour dire ce que le mode implique reellement.
L hote devient 127.0.0.1 -- 127.0.0.2 est une boucle locale valide mais
inhabituelle -- et hote comme port sont surchargeables par DEV_HOST et
DEV_PORT.
Code mort retire
- discord_bot.py, notify_player_about_one_on_one : jamais appelee, seule
la variante _direct l est.
- evaluations.py, branche else de list_evaluations : elle listait les
evaluations recues, une vue de joueur, alors que les joueurs sont
rediriges au debut de la fonction et que can_evaluate() est vrai pour
les quatre roles restants. Inatteignable.
Les autres elements du constat sont deja resorbes : get_auth_logger est
appelee depuis log_auth_event (OBS-001), et ALLOWED_CONTRACT_EXTENSIONS /
ALLOWED_SIGNED_EXTENSIONS sont lues par pdf_upload_error (SEC-021).
wsgi.py n est pas touche : trusted_proxy et HOST attendent la reponse du
developpeur sur la topologie reelle (nginx sur la meme machine ou non).
Co-Authored-By: Claude Opus 5 <[email protected]>
SEC-021 -- rien ne validait le contrat signe
upload_signed_contract se contentait d un nom de fichier non vide.
ALLOWED_SIGNED_EXTENSIONS etait declaree juste a cote et jamais lue. Le
fichier atterrissait sur le disque sous un nom que download_signed_contract
sert ensuite : ce qu un joueur televerse est ce qu un gerant ouvre.
Le nom seul ne suffisait pas non plus cote upload_contract, qui verifiait
`.pdf` en fin de chaine -- payload.pdf ne dit rien des octets.
pdf_upload_error() couvre les deux routes : extension dans la liste, puis
signature %PDF- en tete de flux. Le flux est rembobine, l appelant
enregistre toujours le fichier entier.
OPS-014 -- intent Discord privilegie inutile
Le bot demandait GUILD_MEMBERS et ne s en servait pas : rien n enumere ni
ne recherche de membre de serveur, les personnes sont jointes par le
discord_user_id enregistre sur leur compte. Retire.
message_content reste : on_raw_reaction_add lit le texte de la reponse d un
coach pour consigner un motif de refus.
CI-003 et CI-005 sont deja appliques (permissions: contents: read,
checkout@v4, exclusions de deploiement). L epinglage par SHA des actions
n est pas fait : ce sont des actions GitHub de premiere partie, et
l epingler sans Dependabot echange une exposition contre une autre.
11 tests, dont deux verifient que signer un contrat marche toujours et
qu un autre joueur ne peut pas le faire.
Co-Authored-By: Claude Opus 5 <[email protected]>
Quatre constats de la liste des gains rapides, tous sur auth.py.
SEC-017 -- enumeration de comptes
Le formulaire repondait "il vous reste 3 tentative(s)" a un compte connu et
"verifiez le nom d utilisateur et le mot de passe" a un inconnu. Le decompte
lui-meme etait la fuite : la meme information, etalee sur cinq requetes.
S y ajoutait un ecart de temps de reponse, check_password n etant appele que
si la ligne existait -- scrypt est cher, l ecart est mesurable.
Un seul message pour tous les echecs, et la verification s execute
desormais sur les deux branches : contre un hachage aleatoire tire une fois
par processus quand l identifiant n existe pas.
SEC-018 -- verrou de compte declenchable par un tiers
Cinq mauvaises reponses mettaient un compte connu hors service pendant
quinze minutes, indefiniment renouvelables. Sur un compte president, c est
toute l administration, et aucun ecran ne permettait de defaire.
Le compteur et la fenetre restent -- ce sont la trace qu un administrateur
lit quand un compte est pilonne, et la fenetre double jusqu a un plafond.
Ce qui change : de bons identifiants passent, fenetre ouverte ou non, et
remettent le compteur a zero. Le proprietaire du compte ne peut plus etre
bloque par un tiers.
Ce que cela coute, dit franchement : un verrou dur n arretait de toute
facon pas un attaquant ayant trouve le mot de passe -- il lui suffisait
d attendre. Le debit de tentatives reste borne par la limite de 10/minute
par IP. Une limite par couple (compte, IP) demanderait un stockage dedie ;
elle attend Alembic.
L evenement account.locked devient account.throttled : "locked" affirmait
plus que ce qui se passe.
SEC-019 -- deconnexion en GET
/auth/logout n avait pas de methods, donc GET, donc hors protection CSRF :
n importe quelle page pouvait deconnecter un visiteur avec une balise img.
La route passe en POST et l entree de navigation devient un formulaire avec
jeton. Le style suit -- les regles .nav-links visaient les liens seuls.
SEC-020 -- validation de redirection
is_safe_url interrogeait urlparse().netloc. urlparse lit /\evil.com comme
un chemin, sans netloc ; plusieurs navigateurs normalisent l antislash en
barre oblique avant de resoudre, ce qui en fait //evil.com. La fonction
refuse maintenant antislash et caracteres de controle, exige un chemin
enracine, et compare l origine explicitement.
Le xfail(strict) qui documentait SEC-017 est leve. 40 tests dans
test_auth_session.py, dont la table des cibles refusees.
Co-Authored-By: Claude Opus 5 <[email protected]>
ARCH-008. `role` est le discriminateur polymorphe, et SQLAlchemy fixe la
classe d une instance au chargement. edit_user ecrivait donc la colonne par
un UPDATE de niveau instruction puis relisait la ligne -- c est correct.
La suite ne l etait pas.
db.session.commit()
db.session.remove() # discard stale session entirely
user = User.query.get(user_id_local)
Deux defauts dans ces trois lignes.
1. remove() jette la session entiere. Tout ce que la requete tenait encore
se retrouvait detache, current_user compris ; le moindre acces a un
attribut ensuite levait DetachedInstanceError. La route ne survivait
qu en ayant recopie le nom et l id de l acteur dans des variables
locales avant -- un contournement, pas la correction. Un expunge de la
seule instance perimee suffit.
2. Le commit intermediaire coupait l edition en deux. Le role etait acquis
avant que le reste du formulaire soit applique : une erreur ensuite
laissait un compte promu et le reste perdu, sans qu aucune interface ne
le signale. Sans ce commit, l UPDATE reste dans la transaction, la
relecture le voit, et l ensemble part en un seul commit.
Les evenements d audit passent apres le commit. account.role_changed etait
journalise avant l UPDATE : le journal affirmait un changement que la
transaction pouvait encore annuler.
tests/test_role_change.py, 6 tests. Un seul echoue sur le code d avant --
celui de l atomicite ; les cinq autres fixent le comportement qui marchait
deja, pour que la suite de la vague D ne le casse pas.
Co-Authored-By: Claude Opus 5 <[email protected]>
ARCH-002. La question "a quelles equipes ce coach est-il rattache ?" etait
posee a huit endroits, de cinq facons differentes, et trois d entre elles
donnaient une mauvaise reponse en production.
Le motif fautif, present tel quel dans six routes :
OrgTeam.query.filter_by(coach_id=user.id).first()
Il repond au plus une equipe, et seulement par la colonne heritee. Deux
pannes en decoulaient, silencieuses -- les pages s affichaient, vides :
- un coach rattache uniquement par la relation many-to-many n avait
aucune equipe, donc aucun joueur, aucun contrat, aucune note d equipe,
aucun match a venir sur son tableau de bord ;
- un coach de deux equipes n en voyait qu une. Le formulaire de contrat
lui proposait la moitie de son effectif, alors que la route POST
acceptait l autre moitie.
app/permissions.py devient le module ou la question se pose une fois :
coach_org_teams, manager_org_teams, attached_org_teams, visible_org_teams,
can_manage_org_team, org_team_player_ids, coach_player_ids,
can_manage_player_contract, coach_tryouts, coach_manages_tryout. Toutes
lisent les deux rattachements et toutes les equipes.
Le meme ecart existait dans le modele : Coach.get_visible_tryouts ne lisait
que la relation m2m -- calendrier vide pour un coach rattache par la
colonne -- et can_manage_this_tryout ignorait la colonne pour l equipe
cible. Les deux delegent desormais.
Corrections de portee, au passage
- one_on_one lisait org_team.coach_id : un joueur dont l equipe declare
ses coachs par la relation etait informe qu il n avait pas de coach, et
le formulaire de demande restait ferme. Passe par get_coaches(), qui
retombe deja sur la colonne heritee.
- notes_dashboard conditionnait les notes personnelles du coach a
l existence d une equipe : un coach sans equipe ne voyait pas ses
propres notes.
- can_manage_team_match reformulait can_manage_this_org_team ; la
reformulation avait derive. Elle appelle maintenant la regle.
Limite assumee : le panneau de notes d equipe reste ecrit pour une seule
equipe et affiche donc la premiere. La resolution est corrigee, la mise en
page multi-equipes ne l est pas -- c est un choix produit, pas un bug.
14 tests ajoutes. Cinq echouent sur le code d avant, verifie en remettant
les routes et le modele a leur etat precedent.
ARCH-001 fera disparaitre la colonne heritee ; cela demande une migration
de donnees, donc Alembic. D ici la, ce module est ce qui rend la
duplication inoffensive.
Co-Authored-By: Claude Opus 5 <[email protected]>
198 appels flash dans les sept modules de routes, plus les 23 messages de
validation de app/validators.py. Le catalogue compte desormais 631 chaines,
aucune non traduite.
validators.py utilise lazy_gettext : les champs de schema sont construits a
l'import, donc avant qu'une requete existe. Un gettext ordinaire s'y
resoudrait une seule fois, dans la langue active au demarrage.
Un bug introduit par la conversion, puis corrige
Le convertisseur automatique ne voyait que le premier litteral d'un appel
flash, ce qui a casse deux chaines concatenees sur plusieurs lignes dans
users.py -- le resultat n'etait meme pas du Python valide. Ma premiere
verification ne l'a pas vu : elle enchainait py_compile sur head, or head
reussit toujours, donc le "OK" s'affichait quoi qu'il arrive. Les deux
appels sont reecrits et la verification refaite correctement.
Un bug plus interessant, revele par le test de fumee
La langue choisie ne survivait pas a la connexion. login() et logout()
appellent tous deux session.clear() -- l'un contre la fixation de session,
l'autre pour terminer la session -- et le choix de langue partait avec le
reste. Concretement : quelqu'un qui lisait la page de connexion en anglais
se retrouvait en francais des qu'il se connectait.
La langue est une preference d'affichage, pas un etat appartenant au
compte. Les deux endroits la reportent maintenant explicitement, a cote du
jeton CSRF. Quatre tests couvrent le cas, dont un qui verifie que corriger
une cle preservee n'a pas fait tomber l'autre.
Detail de nommage : le convertisseur avait genere %(value)s pour une
expression conditionnelle, ce qui n'aide pas un traducteur. Renomme en
%(player)s.
Les 14 traductions ecrites avec une apostrophe droite sont normalisees en
apostrophe typographique. Sans consequence en HTML, ou ' s'affiche
correctement -- mais les blocs <script> ne decodent pas les entites, et
autant que le catalogue soit homogene.
200 tests.
Co-Authored-By: Claude Opus 5 <[email protected]>
DATA-004, DATA-005, DATA-006. L'audit les classait "forte probabilite" faute
de pouvoir les executer. Les tests les confirment : ce sont des bugs averes,
declenchables par tout manager ou administrateur depuis l'interface.
Erreurs reellement obtenues avant correction :
NOT NULL constraint failed: match_participants.match_id
NOT NULL constraint failed: team_matches.org_team_id
Supprimer un match (DATA-004)
Match.participants n'avait pas de cascade. SQLAlchemy tentait donc de
detacher les participants en mettant match_id a NULL, ce que la colonne
refuse. Tout match ayant eu des participants etait indestructible.
TeamMatch.participants declarait deja delete-orphan ; Match non.
Supprimer un tryout (DATA-006)
Les PersonalNote pointant vers ses matchs, equipes ou vers lui-meme
n'etaient pas traitees.
Supprimer une equipe (DATA-005)
TeamNote.org_team_id et TeamMatch.org_team_id sont NOT NULL et n'etaient
pas traites du tout. De plus la fonction validait trois fois : un echec au
troisieme temps laissait les tryouts detaches et les joueurs retires sans
que l'equipe soit supprimee -- un etat incoherent que rien ne rattrapait.
Une seule transaction desormais.
Regle appliquee, uniforme
Ce qui n'a de sens que dans le parent est supprime avec lui : participants,
membres, notes d'equipe, matchs de saison.
Ce qui lui survit est seulement detache : les notes personnelles sont les
observations d'un coach sur un joueur, pas des donnees de tryout. Les
supprimer avec le tryout detruirait du contenu sans rapport. Idem pour les
contrats et les demandes de rencontre individuelle.
Fidelite des tests
conftest.py active PRAGMA foreign_keys=ON. SQLite ignore les cles
etrangeres par defaut ; PostgreSQL les applique toujours. Sans ce reglage,
la suite pouvait valider une suppression qui echoue en production --
precisement la classe de bug corrigee ici. Les 146 tests passent avec les
contraintes actives.
9 tests, dont trois qui verifient que les entites survivantes survivent
vraiment : une note garde son contenu et perd son contexte, un tryout
survit a l'equipe qu'il visait.
Co-Authored-By: Claude Opus 5 <[email protected]>
Le site s'affiche desormais en francais par defaut, avec un selecteur de
langue permettant de basculer vers l'anglais.
Choix de conception : les chaines sources restent en anglais
Elles servent d'identifiants gettext, et le francais est fourni par
catalogue avec BABEL_DEFAULT_LOCALE = 'fr'. Le code reste ainsi dans une
seule langue -- la meme que ses commentaires et docstrings -- tandis que
ce qu'un membre voit est du francais.
Consequence qui rend la migration praticable : une chaine non encore
traduite retombe en anglais, pas sur un identifiant brut. Les gabarits
peuvent donc etre migres un par un sans jamais laisser le site a moitie
casse.
Selection de la langue (app/i18n.py)
1. choix explicite via le selecteur, garde en session
2. sinon en-tete Accept-Language du navigateur, restreint a fr et en
3. sinon francais
Un choix explicite prime toujours, y compris sur un navigateur anglophone.
Selecteur
Extrait en partiel et inclus dans les deux branches de la mise en page :
barre laterale une fois connecte, ET page d'authentification. Quelqu'un
qui ne lit pas la langue courante doit pouvoir en changer AVANT de se
connecter -- le laisser derriere l'authentification aurait ete un defaut
d'accessibilite. Chaque langue est ecrite dans sa propre langue.
La route /lang/<locale> valide le Referer avant de rediriger : sans ce
controle, elle constituait une redirection ouverte.
Migre dans cette passe
navigation complete, page de connexion, les cinq pages d'erreur, et
l'integralite des messages flash de routes/auth.py. 64 chaines, dont
aucune non traduite.
Verification
25 tests, dont deux garde-fous d'integrite : un catalogue .mo manquant
ou une entree non traduite font echouer la suite. Sans cela, une
compilation oubliee servirait de l'anglais partout, en silence et sans
rien dans les journaux.
Un test existant a du etre corrige, et c'est instructif
test_login_failure_message_does_not_reveal_account_existence cherchait la
sous-chaine anglaise 'attempt(s) remaining'. La page etant desormais en
francais, elle etait absente des deux cotes, l'assertion passait, et le
mode strict a signale le faux succes. Le test comparait donc l'anglais,
pas le comportement. Il compare desormais les messages flash rendus,
quelle que soit la langue. La faille SEC-AUTH-006 reste ouverte, et le
test la documente toujours.
Les catalogues .po ET .mo sont versionnes : le deploiement est un simple
miroir de fichiers, sans etape de compilation. messages.pot, regenerable,
ne l'est pas.
docs/translations.md documente le processus, les deux pieges (concatenation
de phrases, traduction a l'import), et l'etat de la migration. A noter pour
la suite : les chaines dans les blocs <script> ne peuvent pas etre balisees
telles quelles, il faudra les passer par des attributs data- -- ce qui
rejoint le chantier de sortie de unsafe-inline (OPS-010).
Co-Authored-By: Claude Opus 5 <[email protected]>
SEC-AUTHZ-004 et SEC-AUTHZ-005. La meme question -- ce coach peut-il agir
sur ce joueur ? -- recevait cinq reponses differentes selon la route :
teams.py:add_player_note verifiait l'appartenance via TeamPlayer
users.py, 4 routes de notes ne verifiaient rien au-dela d'isinstance
contract.py:can_view interrogeait la colonne heritee coach_id, et
traitait un team_id nul comme un joker
Consequences levees
- tout coach pouvait ecrire une note nominative sur tout joueur du club.
Ces notes sont visibles par le joueur concerne.
- tout coach figurant dans OrgTeam.coach_id pouvait lire n'importe quel
contrat sans equipe rattachee. Or upload_contract laisse team_id nul des
que le joueur n'appartient a aucune equipe : la condition
`not self.team_id or ...` ouvrait donc largement.
- symetriquement, un coach rattache uniquement par la relation
many-to-many ne voyait aucun contrat.
app/permissions.py
Premier pas concret vers ARCH-002, sans refonte : un module unique, pas
une couche de services. coach_org_team_ids() lit la relation m2m ET la
colonne heritee, donc le deuxieme coach d'une equipe cesse d'etre
invisible. coach_can_access_player() accorde l'acces si le joueur est sur
une equipe du coach, ou inscrit a un tryout qu'il gere, ou participant a
un match de ce tryout.
14 tests, dont deux verifient que les chemins legitimes fonctionnent
toujours : un coach note bien son propre joueur, et voit bien son contrat.
Note : la regle metier retenue -- equipe OU tryout -- est une lecture du
comportement existant, pas une decision produit. Si le club attend autre
chose, c'est desormais un seul endroit a changer.
Co-Authored-By: Claude Opus 5 <[email protected]>
SEC-AUTHZ-007 - auto-verrouillage de l'administration
Le changement de role n'excluait ni l'utilisateur courant, ni le dernier
compte admin actif. Une seule manipulation suffisait a transformer le seul
president en joueur, et plus aucune interface ne permettait de revenir en
arriere : il fallait une intervention directe en base.
Deux gardes distinctes, parce que ce sont deux erreurs differentes :
- changer son propre role est refuse, meme s'il reste d'autres admins.
Un president qui veut se retrograder doit le faire faire par un autre.
- retrograder le dernier admin actif est refuse.
Le decompte exclut les comptes desactives : trois admins dont deux
desactives, cela fait un seul administrateur reel.
SEC-WEB-003 - CORS ouvert par defaut
Sans CORS_ALLOWED_ORIGINS, la branche else appelait
CORS(app, supports_credentials=True) sans argument origins. flask-cors
retient alors '*' et, les identifiants etant autorises, renvoie en echo
l'Origin de l'appelant avec Access-Control-Allow-Credentials: true --
l'inverse exact de ce qu'annonçait le commentaire.
L'exploitation etait bloquee par SESSION_COOKIE_SAMESITE = 'Lax', qui
empeche le navigateur de joindre le cookie de session a une requete
fetch inter-site. Toute la protection tenait donc a ce seul reglage.
Cette application rend du HTML en meme origine : elle n'a besoin
d'aucune politique CORS. La branche par defaut est supprimee, la
configuration explicite reste possible.
5 tests ajoutes, dont deux verifient que les chemins legitimes continuent
de fonctionner : un autre administrateur reste retrogradable, et une
origine explicitement configuree est toujours honoree.
Co-Authored-By: Claude Opus 5 <[email protected]>
OBS-001. logging_config.py configurait un fichier auth.log avec rotation,
un logger nomme 'team_tryouts.auth' et un filtre de redaction. Mais
get_auth_logger n'etait importe nulle part : le fichier etait cree et
restait vide. Aucune connexion, aucun echec, aucun verrouillage, aucun
changement de role, aucune suppression de compte ne laissait de trace.
En cas de suspicion de compromission, il n'y avait rien a consulter.
Ajout de log_auth_event(event, **fields), qui emet des paires cle=valeur
ordonnees -- greppable sans dependance de journalisation JSON.
Evenements couverts
authentification login.success, login.failure,
login.failure.unknown_user, login.rejected.locked,
login.rejected.deactivated, account.locked, logout,
account.registered
administration account.created_by_admin, account.updated,
account.role_changed (avec ancien et nouveau role),
account.deleted, account.password_reset_by_admin
libre-service account.password_changed
Le champ ip vient de request.remote_addr, donc de X-Forwarded-For. Tant que
Waitress tourne avec trusted_proxy='*' (SEC-WEB-002), cette valeur est
choisie par l'appelant : c'est une indication, pas une preuve. Le point est
documente dans la docstring.
Un test a fait remonter ARCH-003, jusqu'ici classe comme fragilite latente
Journaliser en fin de edit_user levait DetachedInstanceError : le
changement de role appelle db.session.remove() en plein cycle de requete,
ce qui detache current_user de la session. Le code s'en tirait parce qu'il
redirigeait immediatement sans plus y toucher. L'identite de l'acteur est
desormais capturee en debut de traitement. Le constat est donc confirme
comme reel, et non plus seulement probable -- sa correction de fond reste
au programme.
15 tests, dont 4 sur le filtre de redaction lui-meme : c'est un controle de
securite, il doit etre verifie.
Co-Authored-By: Claude Opus 5 <[email protected]>
SEC-AUTH-004. L'URL d'autorisation ne portait que client_id, redirect_uri,
response_type et scope. Sans state, le callback acceptait n'importe quel
code d'autorisation qu'on lui presentait.
Scenario ferme : un attaquant obtient un code pour SON compte Discord, puis
fait charger l'URL de callback par le navigateur de la victime. Le
formulaire d'inscription de la victime se retrouve pre-rempli avec
l'identite Discord de l'attaquant. C'est le login CSRF decrit par la
RFC 6749 §10.12.
Mise en oeuvre
secrets.token_urlsafe(32) genere le jeton, stocke en session avant la
redirection. Le callback le compare en temps constant avec
secrets.compare_digest, et le consomme systematiquement -- valide ou non --
pour qu'il ne puisse pas etre rejoue. Le controle intervient avant
l'echange du code : un callback rejete ne declenche aucun appel reseau.
Deux corrections accessoires sur le meme chemin
- DISCORD_REDIRECT_URI est desormais verifie au meme titre que
DISCORD_CLIENT_ID. Non defini, il faisait lever requests.utils.quote(None)
au lieu de signaler un probleme de configuration.
- la construction de la chaine de requete passe a urlencode() plutot qu'a
une concatenation manuelle.
9 tests : presence du state, stockage en session, unicite entre deux
demandes, rejet sans state, avec un state forge, sans demande prealable,
et non-rejouabilite.
Reste ouvert : l'identite Discord obtenue reste ensuite reinjectee par un
champ cache du formulaire (SEC-AUTH-005). Le state protege la liaison, pas
encore la valeur elle-meme.
Co-Authored-By: Claude Opus 5 <[email protected]>
SEC-XSS-001. Chaine complete : un nom d'utilisateur libre arrivait dans le
DOM d'un coach ou d'un administrateur, en meme origine, avec sa session.
La CSP autorisant 'unsafe-inline', rien ne l'arretait.
Cote serveur - la cause
/matches/api/events construisait de la presentation dans un champ JSON :
match_desc = participants_str + f"<br>{match.description}"
Le navigateur deposait cette valeur telle quelle dans innerHTML. Les noms
de joueurs y transitaient sans echappement -- et il ne pouvait pas y en
avoir : c'est du JSON, pas du HTML.
Les deux valeurs etaient deja des cles distinctes du payload. La
concatenation faisait donc aussi afficher les participants deux fois dans
le modal : une fois dans "Teams", une fois en tete de "Description".
Corriger la faille corrige l'affichage.
Cote navigateur - le sink
showEventModal assemblait une chaine HTML puis l'affectait a innerHTML.
Remplace par une construction de noeuds : makeEl / detailItem /
multilineNode / teamNode passent tout texte par textContent. Les retours
a la ligne d'une description restent rendus, via des <br> crees en dur.
Les deux listes deroulantes concatenaient egalement titres de tryout et
noms d'equipe dans innerHTML. Remplacees par new Option(), dont le
premier argument est pose en texte.
Verification
5 tests sur le contrat de l'API, dont un avec un nom d'utilisateur
hostile ecrit directement en base -- ce que la validation refuse
desormais, mais que des lignes anterieures peuvent contenir.
Le JavaScript inline extrait passe `node --check`.
Reste ouvert : la CSP autorise toujours 'unsafe-inline' (SEC-WEB-001), donc
la defense en profondeur manque encore. Suivi en OPS-010.
Co-Authored-By: Claude Opus 5 <[email protected]>
Aucun changement de comportement. Les chaines concernees ne contenaient
aucune substitution.
A noter pour plus tard : run_https.py conserve une banniere en caracteres
semi-graphiques, du meme type que celle qui faisait planter security_scan.py
sur une console Windows en cp1252. Le script n'etant lance qu'en
developpement et de facon explicite, le point est signale sans etre corrige
ici.
Co-Authored-By: Claude Opus 5 <[email protected]>
SEC-AUTHZ-002. add_to_team recevait tryout_id et team_id independamment
dans l'URL, controlait l'autorisation sur le tryout, puis operait sur
l'equipe sans jamais etablir de lien entre les deux. Un gestionnaire du
tryout A pouvait donc modifier une equipe du tryout B.
Le lint pointait exactement dessus : `team` etait charge ligne 463 puis
jamais utilise. La correction automatique proposee etait de supprimer la
variable, ce qui aurait fait taire l'avertissement en cimentant la faille.
Elle est desormais utilisee pour ce a quoi elle servait.
Trois defauts sur la meme route, corriges ensemble :
- team.tryout_id != tryout_id repond maintenant 404
- seuls les joueurs inscrits au tryout peuvent rejoindre ses equipes
- int(player_id) sur une entree de formulaire brute levait ValueError,
donc une erreur 500, sur toute valeur non numerique
Nettoyage automatique par ruff : 34 imports et variables morts retires
sur l'ensemble du paquet. La suite de tests a servi de filet, elle passe
a l'identique avant et apres. Aucun changement de comportement.
A noter, OneOnOneRequestSchema figurait aussi parmi les imports morts :
c'est un quatrieme schema jamais appele, la route one_on_one validant ses
dates a la main. Unifier la validation reste a faire (ARCH-005).
Co-Authored-By: Claude Opus 5 <[email protected]>
SEC-AUTHZ-001. CreateUserSchema, EditUserSchema et EditProfileSchema
etaient importes dans users.py et jamais appeles : chaque nom
n'apparaissait qu'une fois dans le fichier, sur sa ligne d'import. Les
trois routes lisaient request.form directement.
Consequences levees :
- aucune politique de mot de passe sur create_user, edit_user et
edit_profile. Un mot de passe d'un caractere etait accepte pour un
compte administrateur.
- aucune validation de format sur username, email, phone,
discord_user_id.
- edit_user ne verifiait pas l'unicite du courriel avant affectation :
la contrainte unique remontait en IntegrityError, donc en erreur 500.
Un controle explicite excluant l'utilisateur courant est ajoute.
C'est aussi le point d'injection de la chaine de XSS stocke SEC-XSS-001 :
edit_profile acceptait n'importe quel nom d'utilisateur, charge HTML
comprise, qui ressortait ensuite en JSON via /matches/api/events et
etait injectee par innerHTML dans le calendrier.
Deux details de formulaire imposaient un adaptateur, _form_payload :
- request.form.to_dict() ne conserve que la premiere valeur d'une cle
repetee, donc games doit etre relu avec getlist().
- une case a cocher non cochee est absente de la soumission, ce qui
n'est pas la meme chose qu'un load_default. Sans injection explicite,
decocher is_active_account aurait cesse de desactiver le compte.
- un mot de passe vide signifie "conserver l'actuel" et non "definir le
mot de passe vide" : le champ est retire avant validation.
Le controle manuel du role devient redondant, le schema le contraint deja
par OneOf(USER_TYPES).
Verifie par quatre tests qui echouaient avant ce changement.
Co-Authored-By: Claude Opus 5 <[email protected]>
Deux protections etaient configurees sans avoir d'effet.
Expiration de session
app.py:73 definit PERMANENT_SESSION_LIFETIME = 3600, mais Flask
n'applique cette duree qu'aux sessions marquees permanentes. Aucune
occurrence de session.permanent n'existait dans app/. Le cookie emis
etait donc un cookie de session navigateur, sans expiration, et le
serveur ne verifiait aucune anciennete. Ajout de session.permanent
juste avant login_user, apres la rotation anti-fixation.
Desactivation de compte
is_active_account n'etait consulte qu'au moment du login (auth.py:141).
User n'ayant pas surcharge is_active, UserMixin renvoyait True en
permanence. Desactiver un compte empechait donc la reconnexion mais
laissait vivre la session en cours.
La propriete is_active seule ne suffit pas : Flask-Login ne la consulte
qu'a l'appel de login_user, jamais lors de la restauration d'une session
depuis le cookie. Le verrou effectif est donc dans load_user, qui renvoie
desormais None pour un compte desactive. La propriete est ajoutee malgre
tout pour que login_user soit coherent avec le chargeur.
load_user passe au passage de Query.get() (API heritee, avertie en
SQLAlchemy 2.0) a db.session.get(), et tolere un identifiant non entier
sans lever.
Co-Authored-By: Claude Opus 5 <[email protected]>
cedrick2711