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]>
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]>
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]>
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]>
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-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]>
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]>
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]>
GGThed