From 2437a9b5eae74941f2b98e424e5cbd69419e0203 Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Tue, 25 Aug 2026 05:06:58 -0400 Subject: [PATCH] =?UTF-8?q?[FIX]=20analyse:=20ne=20pas=20=C3=A9crire=20de?= =?UTF-8?q?=20mots=20dans=20les=20champs=20qui=20portent=20des=20ids?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Odoo déclare `parent_path` en `char` mais y range un chemin d'identifiants — « 1/7/12/ » — reparsé aussitôt par int() : un mot écrit là casse le premier chargement de page. Sept modèles `_parent_store` en portent un dans une base ordinaire, et account_payment_term passe de même `days_next_month` à int(). L'anonymisation écarte les deux noms connus, puis sonde le contenu : une colonne dont chaque valeur est un chemin reste intacte, même dans un module maison. Les barres obliques sont exigées : un numéro tout en chiffres y échapperait. Vérifié sur une base de production : les sept modèles chargent, les partenaires sont anonymisés. --- EN --- Odoo declares `parent_path` as `char` but stores an identifier path in it — « 1/7/12/ » — parsed right back with int(): a word written there breaks the first page load. Seven `_parent_store` models carry one in an ordinary database, and account_payment_term likewise passes `days_next_month` to int(). Anonymisation skips the two known names, then probes the content: a column whose every value is a path is left alone, even in an in-house module. The slashes are required: a number of pure digits would escape it. Verified on a production database: the seven models load, partners are anonymised. Assisted-by: Claude Opus 5 (cherry picked from commit b47c94524d75172e7afce96e62f3f15b7ad6e225) --- script/analyse/anonymize.py | 103 +++++++++++++++++++++++++++++ script/todo/todo_i18n.py | 8 +++ test/test_anonymize.py | 128 ++++++++++++++++++++++++++++++++++++ 3 files changed, 239 insertions(+) diff --git a/script/analyse/anonymize.py b/script/analyse/anonymize.py index f3fbc97..d34cee5 100755 --- a/script/analyse/anonymize.py +++ b/script/analyse/anonymize.py @@ -106,6 +106,24 @@ CHAMPS_INTERDITS = frozenset( ) CHAMPS_CONNEXION = frozenset({"login", "password"}) +# Des `char` d'Odoo qui portent une STRUCTURE, pas du texte. Odoo les +# reparse, et un mot y fait lever le serveur entier : +# parent_path → `int(id) for id in parent_path.split('/')` +# (base/models/res_company.py:117, models.py:203) +# days_next_month → `int(self.days_next_month)` +# (account/models/account_payment_term.py:321) +# `parent_path` existe sur tout modèle `_parent_store` — sept dans une +# base ordinaire. La sonde de contenu ci-dessous les retrouverait, mais +# les nommer coûte moins cher qu'une requête et ne dépend pas des données +# présentes le jour où l'on passe. +CHAMPS_STRUCTURES = frozenset({"parent_path", "days_next_month"}) + +# Un chemin d'identifiants : « 1/ », « 1/7/12/ ». Le motif exige les +# barres obliques — sans elles, un numéro de téléphone tout en chiffres +# serait pris pour une structure et échapperait à l'anonymisation, ce +# qui serait un défaut de confidentialité, pas de robustesse. +MOTIF_CHEMIN = r"^[0-9]+(/[0-9]+)*/$" + # Le point de départ du mode hybride : ce qui porte des données # personnelles dans une base Odoo ordinaire. MODELES_PAR_DEFAUT = ( @@ -197,6 +215,8 @@ def champ_retenu(champ, inclure_connexion=False): if champ["name"].endswith("_id") or champ["name"].endswith("_ids"): # Une relation qui aurait échappé au filtre de ttype. return False + if champ["name"] in CHAMPS_STRUCTURES: + return False if champ.get("checked"): # Une contrainte CHECK hors de portée. Mesuré, et la distinction # compte : sur un NOMBRE toute contrainte borne la valeur — @@ -428,6 +448,75 @@ def plan( return etapes +def colonnes_structurees(database, etapes, config_path=None): + """Les colonnes texte dont TOUT le contenu est un chemin d'identifiants. + + Le filet général, là où `CHAMPS_STRUCTURES` ne nomme que le connu : + un module maison peut poser son propre champ de chemin, et il ne + portera pas ce nom-là. On regarde donc ce que la colonne CONTIENT. + + Une colonne est écartée seulement si elle est NON VIDE et que toutes + ses valeurs sont des chemins. Un seul contre-exemple suffit à la + garder : mieux vaut anonymiser une colonne douteuse que taire une + donnée personnelle. + + Une requête par TABLE, pas par colonne : sur une liste noire de 410 + modèles, la différence est de 410 allers-retours au lieu de 863. + """ + ecartees = {} + for etape in etapes: + textes = [ + champ + for champ in etape["fields"] + if champ["ttype"] in TYPES_TEXTE and champ["pg_type"] != "jsonb" + ] + if not textes: + continue + morceaux = [] + for champ in textes: + nom = ident(champ["name"]) + morceaux.append( + f"count(*) FILTER (WHERE {nom} IS NOT NULL AND {nom} <> '')" + f" || ':' || count(*) FILTER (WHERE {nom} ~ {litteral(MOTIF_CHEMIN)})" + ) + sql = ( + "SELECT " + + " || '\x1f' || ".join(morceaux) + + f" FROM {ident(table_de(etape['model']))}" + ) + try: + brut = lib_analyse.run_psql( + database, sql, config_path=config_path + ).strip() + except Exception: # noqa: BLE001 - une table illisible ne bloque pas + continue + for champ, mesure in zip(textes, brut.split(SEP)): + try: + remplies, chemins = (int(x) for x in mesure.split(":")) + except ValueError: + continue + if remplies and remplies == chemins: + ecartees.setdefault(etape["model"], []).append(champ["name"]) + return ecartees + + +def sans_structurees(etapes, ecartees, mots): + """Refaire le plan sans les colonnes que la sonde a écartées.""" + if not ecartees: + return etapes + propre = [] + for etape in etapes: + exclues = set(ecartees.get(etape["model"], ())) + gardes = [c for c in etape["fields"] if c["name"] not in exclues] + # Pas de garde sur une liste vide : `sql_pour_table` rend None, et + # le `if sql` ci-dessous l'écarte. Deux vérifications pour la même + # chose se contredisent un jour. + sql = sql_pour_table(table_de(etape["model"]), gardes, mots) + if sql: + propre.append({**etape, "fields": gardes, "sql": sql}) + return propre + + def render(etapes, applique=False, verbeux=False): """Le rapport. Il dit ce qui est ÉCARTÉ autant que ce qui est pris.""" if not etapes: @@ -606,6 +695,20 @@ def main(argv=None): args.include_logins, mots, ) + # La sonde AVANT le rendu : la marche à blanc doit montrer ce que + # `--apply` ferait, pas une approximation plus large. + ecartees = colonnes_structurees(args.database, etapes, args.config) + etapes = sans_structurees(etapes, ecartees, mots) + if ecartees: + combien = sum(len(v) for v in ecartees.values()) + print( + f"🧭 {combien} {t('column(s) hold identifier paths and are left')}" + f" {t('alone:')}" + ) + for modele in sorted(ecartees): + print(f" {modele} : {', '.join(sorted(ecartees[modele]))}") + print() + if not args.apply: print(render(etapes, applique=False, verbeux=args.verbose)) return 1 if etapes else 0 diff --git a/script/todo/todo_i18n.py b/script/todo/todo_i18n.py index 8f86596..f7feb41 100644 --- a/script/todo/todo_i18n.py +++ b/script/todo/todo_i18n.py @@ -10108,6 +10108,14 @@ TRANSLATIONS = { "fr": "désactive pas les crons, et laisse les clés de paiement en place.", "en": "not disable crons, and leaves payment keys in place.", }, + "column(s) hold identifier paths and are left": { + "fr": "colonne(s) portent des chemins d'identifiants et sont laissées", + "en": "column(s) hold identifier paths and are left", + }, + "alone:": { + "fr": "intactes :", + "en": "alone:", + }, } diff --git a/test/test_anonymize.py b/test/test_anonymize.py index 6da204d..c845479 100644 --- a/test/test_anonymize.py +++ b/test/test_anonymize.py @@ -606,3 +606,131 @@ class TestACheckDoesNotSilenceTheMainField(unittest.TestCase): "max_len": None, } self.assertFalse(anon.champ_retenu(champ)) + + +class TestStructuredCharFieldsSurvive(unittest.TestCase): + """`ValueError: invalid literal for int() with base 10: 'bruyere'`. + + Odoo déclare `parent_path` en `char`, mais y range un CHEMIN + D'IDENTIFIANTS — « 1/7/12/ » — qu'il reparse : + + int(id) for id in company.parent_path.split('/') + base/models/res_company.py:117, models.py:203 et :221 + + Y écrire un mot fait lever le serveur au premier chargement de page. + Mesuré : sept modèles `_parent_store` dans une base ordinaire — + res.company, product.category, stock.location, hr.department, + website.menu, account.analytic.plan, helpdesk.ticket.category. + + Même chose pour `days_next_month`, qu'Odoo passe à `int()` + (account/models/account_payment_term.py:321). + """ + + def _champ(self, nom): + return { + "model": "res.company", + "name": nom, + "ttype": "char", + "pg_type": "character varying", + "unique": False, + "checked": False, + "max_len": None, + } + + def test_the_known_parsers_are_named(self): + self.assertIn("parent_path", anon.CHAMPS_STRUCTURES) + self.assertIn("days_next_month", anon.CHAMPS_STRUCTURES) + + def test_they_are_never_touched_whatever_the_model(self): + for nom in anon.CHAMPS_STRUCTURES: + self.assertFalse(anon.champ_retenu(self._champ(nom)), nom) + + def test_a_phone_number_is_not_mistaken_for_a_structure(self): + """Le motif EXIGE les barres obliques. Sans elles, un numéro tout + en chiffres passerait pour une structure et échapperait à + l'anonymisation — un défaut de confidentialité, pas de robustesse. + """ + import re + + motif = re.compile(anon.MOTIF_CHEMIN) + for valeur in ("5141234567", "0", "42", "1234-5678"): + self.assertIsNone(motif.match(valeur), valeur) + for valeur in ("1/", "1/7/12/", "3/4/"): + self.assertIsNotNone(motif.match(valeur), valeur) + + def test_the_probe_asks_once_per_table_not_once_per_column(self): + """Sur 410 modèles, la différence est de 410 allers-retours au + lieu de 863.""" + appels = [] + + def espion(database, sql, config_path=None): + appels.append(sql) + return "0:0\x1f0:0" + + vrai = anon.lib_analyse.run_psql + anon.lib_analyse.run_psql = espion + etapes = [ + { + "model": "res.company", + "fields": [self._champ("name"), self._champ("street")], + "sql": "x", + } + ] + try: + anon.colonnes_structurees("base", etapes) + finally: + anon.lib_analyse.run_psql = vrai + self.assertEqual(len(appels), 1) + self.assertIn('"name"', appels[0]) + self.assertIn('"street"', appels[0]) + + def test_one_counter_example_is_enough_to_keep_a_column(self): + """Mieux vaut anonymiser une colonne douteuse que taire une + donnée personnelle.""" + + def espion(database, sql, config_path=None): + # 10 valeurs remplies, 9 seulement sont des chemins. + return "10:9" + + vrai = anon.lib_analyse.run_psql + anon.lib_analyse.run_psql = espion + etapes = [ + {"model": "m", "fields": [self._champ("chemin")], "sql": "x"} + ] + try: + ecartees = anon.colonnes_structurees("base", etapes) + finally: + anon.lib_analyse.run_psql = vrai + self.assertEqual(ecartees, {}) + + def test_an_all_paths_column_is_dropped_from_the_plan(self): + def espion(database, sql, config_path=None): + return "10:10" + + vrai = anon.lib_analyse.run_psql + anon.lib_analyse.run_psql = espion + etapes = [ + { + "model": "m", + "fields": [self._champ("chemin"), self._champ("nom")], + "sql": "x", + } + ] + try: + ecartees = anon.colonnes_structurees("base", etapes) + finally: + anon.lib_analyse.run_psql = vrai + # Les deux colonnes rendent le même compte ici : c'est le principe + # qu'on vérifie, pas la ligne exacte. + self.assertIn("m", ecartees) + propre = anon.sans_structurees(etapes, {"m": ["chemin"]}, None) + self.assertEqual([c["name"] for c in propre[0]["fields"]], ["nom"]) + self.assertNotIn('"chemin"', propre[0]["sql"]) + + def test_a_model_entirely_dropped_leaves_no_empty_statement(self): + etapes = [ + {"model": "m", "fields": [self._champ("chemin")], "sql": "x"} + ] + self.assertEqual( + anon.sans_structurees(etapes, {"m": ["chemin"]}, None), [] + )