From 8cc5566743429b0ff05a722a2200272cceaef202 Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Tue, 25 Aug 2026 06:40:11 -0400 Subject: [PATCH] =?UTF-8?q?[FIX]=20anonymisation=20:=20tirer=20les=20nombr?= =?UTF-8?q?es=20dans=20l'=C3=A9tendue=20mesur=C3=A9e?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `resource.calendar.attendance.hour_from` est un `float` qui porte une HEURE DE LA JOURNÉE : un tirage uniforme sur 0 à 1000 le sort de 0..23 et l'affichage de la fiche lève « ValueError: hour must be in 0..23 ». La borne vit dans `time(int(integral), ...)` de resource/models/utils.py, et aucune contrainte PostgreSQL ne la déclare : le schéma ne dit pas le SENS de la donnée. « 0 à 1000 » est donc faux pour tout nombre borné par un usage : heures, pourcentages, taux. On respecte désormais la seule borne que les DONNÉES déclarent, leur propre étendue — min et max mesurés par table, et 0 à 1000 quand elle est inconnue. Vérifié : les tirages tiennent dans l'étendue mesurée, et la fiche s'affiche sans lever. --- EN --- `resource.calendar.attendance.hour_from` is a `float` holding an HOUR OF DAY: a uniform draw over 0 to 1000 takes it out of 0..23 and displaying the record raises "ValueError: hour must be in 0..23". The bound lives in `time(int(integral), ...)` in resource/models/utils.py, and no PostgreSQL constraint declares it: the schema does not say what the data MEANS. So "0 to 1000" is wrong for any number bounded by usage: hours, percentages, rates. We now respect the only bound the DATA declares, its own range — min and max measured per table, and 0 to 1000 when it is unknown. Checked: draws stay inside the measured range, and the record displays without raising. Assisted-by: Claude Opus 5 (cherry picked from commit 8e0a19823714b1d18f05613d498f8136b682c047) --- script/analyse/anonymize.py | 124 ++++++++++++++++++++------ test/test_anonymize.py | 173 ++++++++++++++++++++++++++++++++++-- 2 files changed, 265 insertions(+), 32 deletions(-) diff --git a/script/analyse/anonymize.py b/script/analyse/anonymize.py index d34cee5..e9ab434 100755 --- a/script/analyse/anonymize.py +++ b/script/analyse/anonymize.py @@ -310,12 +310,40 @@ def expression_texte(champ, mots): def expression_nombre(champ): - """Le SQL qui remplace un nombre : au hasard, entre 0 et 1000.""" + """Le SQL qui remplace un nombre, DANS l'étendue de la colonne. + + 0 à 1000 était l'intention, et c'est faux pour tout nombre qui porte + un sens borné. Mesuré : `resource.calendar.attendance.hour_from` est + un `float` qui vaut une heure de la journée — 8,00 à 13,00 dans la + base d'origine. Un tirage à 957 fait lever Odoo : + + time(int(integral), ...) → ValueError: hour must be in 0..23 + resource/models/utils.py:45 + + Aucune contrainte PostgreSQL ne dit cela : la borne vit dans le code. + La seule que les DONNÉES déclarent est leur propre étendue, et c'est + celle qu'on respecte. Elle protège aussi les pourcentages, les taux et + les quantités, sans qu'il faille les nommer un par un. + + Sans étendue connue — colonne vide, ou sonde impossible — on retombe + sur 0 à 1000. + """ nom = ident(champ["name"]) - if champ["ttype"] == "integer": - tirage = "floor(random() * 1001)::integer" + bas, haut = champ.get("borne_min"), champ.get("borne_max") + entier = champ["ttype"] == "integer" + if bas is None or haut is None: + tirage = ( + "floor(random() * 1001)::integer" + if entier + else "round((random() * 1000)::numeric, 2)" + ) + elif entier: + # +1 pour que la borne haute soit atteignable ; si bas == haut, + # le tirage rend cette valeur, ce qui est sans risque : une + # colonne constante ne porte aucune information à masquer. + tirage = f"floor({bas} + random() * ({haut} - {bas} + 1))::integer" else: - tirage = "round((random() * 1000)::numeric, 2)" + tirage = f"round(({bas} + random() * ({haut} - {bas}))::numeric, 2)" return f"CASE WHEN {nom} IS NULL THEN NULL ELSE {tirage} END" @@ -448,29 +476,36 @@ def plan( return etapes -def colonnes_structurees(database, etapes, config_path=None): - """Les colonnes texte dont TOUT le contenu est un chemin d'identifiants. +def sonder_colonnes(database, etapes, config_path=None): + """Regarder ce que les colonnes CONTIENNENT. Rendre (écartées, bornes). - 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. + Deux mesures, une seule requête par table — sur une liste noire de 410 + modèles, la différence est de 410 allers-retours au lieu de 1 300. - 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. + ÉCARTÉES — 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 le sien sans lui donner ce + nom. Une seule valeur non conforme suffit à garder la colonne — 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. + BORNES — l'étendue réelle de chaque colonne numérique. C'est la seule + borne que les données déclarent, et elle vaut pour toutes celles que + le code d'Odoo impose sans que PostgreSQL en sache rien : heures de la + journée, pourcentages, taux. """ - ecartees = {} + ecartees, bornes = {}, {} 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: + nombres = [ + champ + for champ in etape["fields"] + if champ["ttype"] in TYPES_NOMBRE and champ["pg_type"] != "jsonb" + ] + if not textes and not nombres: continue morceaux = [] for champ in textes: @@ -479,6 +514,12 @@ def colonnes_structurees(database, etapes, config_path=None): f"count(*) FILTER (WHERE {nom} IS NOT NULL AND {nom} <> '')" f" || ':' || count(*) FILTER (WHERE {nom} ~ {litteral(MOTIF_CHEMIN)})" ) + for champ in nombres: + nom = ident(champ["name"]) + morceaux.append( + f"coalesce(min({nom})::text, '') || ':'" + f" || coalesce(max({nom})::text, '')" + ) sql = ( "SELECT " + " || '\x1f' || ".join(morceaux) @@ -490,24 +531,55 @@ def colonnes_structurees(database, etapes, config_path=None): ).strip() except Exception: # noqa: BLE001 - une table illisible ne bloque pas continue - for champ, mesure in zip(textes, brut.split(SEP)): + mesures = brut.split(SEP) + if len(mesures) != len(textes) + len(nombres): + continue + for champ, mesure in zip(textes, mesures): 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 + for champ, mesure in zip(nombres, mesures[len(textes) :]): + bas, _, haut = mesure.partition(":") + if nombre_valide(bas) and nombre_valide(haut): + bornes.setdefault(etape["model"], {})[champ["name"]] = ( + bas, + haut, + ) + return ecartees, bornes -def sans_structurees(etapes, ecartees, mots): - """Refaire le plan sans les colonnes que la sonde a écartées.""" - if not ecartees: - return etapes +def nombre_valide(texte): + """Un littéral numérique, et rien d'autre. + + Ces valeurs viennent de la base mais retournent dans du SQL : on ne + les recopie qu'après avoir vérifié qu'elles sont bien des nombres. + """ + try: + float(texte) + except (TypeError, ValueError): + return False + return True + + +def appliquer_sondes(etapes, ecartees, bornes, mots): + """Refaire le plan sans les écartées, et avec les bornes mesurées.""" propre = [] for etape in etapes: exclues = set(ecartees.get(etape["model"], ())) - gardes = [c for c in etape["fields"] if c["name"] not in exclues] + mesures = bornes.get(etape["model"], {}) + gardes = [] + for champ in etape["fields"]: + if champ["name"] in exclues: + continue + borne = mesures.get(champ["name"]) + gardes.append( + {**champ, "borne_min": borne[0], "borne_max": borne[1]} + if borne + else champ + ) # 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. @@ -697,8 +769,8 @@ def main(argv=None): ) # 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) + ecartees, bornes = sonder_colonnes(args.database, etapes, args.config) + etapes = appliquer_sondes(etapes, ecartees, bornes, mots) if ecartees: combien = sum(len(v) for v in ecartees.values()) print( diff --git a/test/test_anonymize.py b/test/test_anonymize.py index c845479..7c15ac2 100644 --- a/test/test_anonymize.py +++ b/test/test_anonymize.py @@ -677,7 +677,7 @@ class TestStructuredCharFieldsSurvive(unittest.TestCase): } ] try: - anon.colonnes_structurees("base", etapes) + anon.sonder_colonnes("base", etapes) finally: anon.lib_analyse.run_psql = vrai self.assertEqual(len(appels), 1) @@ -698,14 +698,16 @@ class TestStructuredCharFieldsSurvive(unittest.TestCase): {"model": "m", "fields": [self._champ("chemin")], "sql": "x"} ] try: - ecartees = anon.colonnes_structurees("base", etapes) + ecartees, _ = anon.sonder_colonnes("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" + # Une mesure PAR COLONNE : la sonde compte ce qu'elle + # reçoit et renonce si le compte ne tombe pas juste. + return "10:10\x1f10:10" vrai = anon.lib_analyse.run_psql anon.lib_analyse.run_psql = espion @@ -717,13 +719,13 @@ class TestStructuredCharFieldsSurvive(unittest.TestCase): } ] try: - ecartees = anon.colonnes_structurees("base", etapes) + ecartees, _ = anon.sonder_colonnes("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) + propre = anon.appliquer_sondes(etapes, {"m": ["chemin"]}, {}, None) self.assertEqual([c["name"] for c in propre[0]["fields"]], ["nom"]) self.assertNotIn('"chemin"', propre[0]["sql"]) @@ -732,5 +734,164 @@ class TestStructuredCharFieldsSurvive(unittest.TestCase): {"model": "m", "fields": [self._champ("chemin")], "sql": "x"} ] self.assertEqual( - anon.sans_structurees(etapes, {"m": ["chemin"]}, None), [] + anon.appliquer_sondes(etapes, {"m": ["chemin"]}, {}, None), [] ) + + +class TestANumberKeepsItsMeaningfulRange(unittest.TestCase): + """`ValueError: hour must be in 0..23`. + + `resource.calendar.attendance.hour_from` est un `float` qui vaut une + heure de la journée — 8,00 à 13,00 dans la base d'origine. Un tirage + à 957 fait lever Odoo au premier affichage d'un employé : + + time(int(integral), ...) resource/models/utils.py:45 + + Aucune contrainte PostgreSQL ne dit cela : la borne vit dans le code. + La seule que les DONNÉES déclarent est leur propre étendue, et c'est + la seule qu'on puisse respecter sans nommer les champs un par un. + """ + + def _champ(self, ttype="float", **kw): + base = { + "model": "resource.calendar.attendance", + "name": "hour_from", + "ttype": ttype, + "pg_type": "numeric", + "unique": False, + "checked": False, + "max_len": None, + } + base.update(kw) + return base + + def test_a_measured_range_bounds_the_draw(self): + sql = anon.expression_nombre( + self._champ(borne_min="8.0", borne_max="13.0") + ) + self.assertIn("8.0 + random() * (13.0 - 8.0)", sql) + self.assertNotIn("1000", sql) + + def test_an_integer_can_reach_its_upper_bound(self): + sql = anon.expression_nombre( + self._champ(ttype="integer", borne_min="0", borne_max="5") + ) + self.assertIn("(5 - 0 + 1)", sql) + + def test_without_a_range_it_falls_back_on_a_thousand(self): + self.assertIn("1000", anon.expression_nombre(self._champ())) + self.assertIn( + "1001", anon.expression_nombre(self._champ(ttype="integer")) + ) + + def test_a_half_known_range_is_not_used(self): + """Une borne sans l'autre ne borne rien.""" + for kw in ({"borne_min": "8.0"}, {"borne_max": "13.0"}): + self.assertIn("1000", anon.expression_nombre(self._champ(**kw))) + + def test_a_value_that_is_not_a_number_never_reaches_the_sql(self): + """Ces bornes viennent de la base et retournent dans du SQL.""" + self.assertTrue(anon.nombre_valide("8.0")) + self.assertTrue(anon.nombre_valide("-3")) + for mauvais in ("8.0); DROP TABLE x; --", "", None, "huit"): + self.assertFalse(anon.nombre_valide(mauvais), mauvais) + + def test_the_probe_reads_the_bounds(self): + recu = {} + + def espion(database, sql, config_path=None): + recu["sql"] = sql + return "8.0:13.0" + + vrai = anon.lib_analyse.run_psql + anon.lib_analyse.run_psql = espion + etapes = [{"model": "m", "fields": [self._champ()], "sql": "x"}] + try: + _, bornes = anon.sonder_colonnes("base", etapes) + finally: + anon.lib_analyse.run_psql = vrai + self.assertIn("min(", recu["sql"]) + self.assertIn("max(", recu["sql"]) + self.assertEqual(bornes["m"]["hour_from"], ("8.0", "13.0")) + + def test_an_empty_column_yields_no_bound(self): + def espion(database, sql, config_path=None): + return ":" + + vrai = anon.lib_analyse.run_psql + anon.lib_analyse.run_psql = espion + etapes = [{"model": "m", "fields": [self._champ()], "sql": "x"}] + try: + _, bornes = anon.sonder_colonnes("base", etapes) + finally: + anon.lib_analyse.run_psql = vrai + self.assertEqual(bornes, {}) + + def test_the_bounds_reach_the_generated_sql(self): + etapes = [{"model": "m", "fields": [self._champ()], "sql": "x"}] + propre = anon.appliquer_sondes( + etapes, {}, {"m": {"hour_from": ("8.0", "13.0")}}, None + ) + self.assertIn("8.0 + random()", propre[0]["sql"]) + + +class TestTheProbeDistrustsWhatItReads(unittest.TestCase): + """Ce que la sonde reçoit repart dans du SQL : elle le vérifie. + + Deux mutations ont survécu au premier tour, et les deux disaient la + même chose : j'éprouvais les fonctions de contrôle isolément sans + vérifier que la sonde s'en sert. Une garde qu'on n'exerce pas ne + garde rien. + """ + + def _champ(self, nom="hour_from", ttype="float"): + return { + "model": "m", + "name": nom, + "ttype": ttype, + "pg_type": "numeric", + "unique": False, + "checked": False, + "max_len": None, + } + + def _sonder(self, reponse, champs): + def espion(database, sql, config_path=None): + return reponse + + vrai = anon.lib_analyse.run_psql + anon.lib_analyse.run_psql = espion + etapes = [{"model": "m", "fields": champs, "sql": "x"}] + try: + return anon.sonder_colonnes("base", etapes) + finally: + anon.lib_analyse.run_psql = vrai + + def test_a_bound_that_is_not_a_number_is_refused(self): + """La base peut rendre autre chose qu'un nombre ; ces valeurs + retournent telles quelles dans un littéral SQL.""" + for reponse in ("abc:def", "8.0); DROP TABLE x; --:13", ":13"): + _, bornes = self._sonder(reponse, [self._champ()]) + self.assertEqual(bornes, {}, reponse) + + def test_a_valid_bound_still_passes(self): + _, bornes = self._sonder("8.0:13.0", [self._champ()]) + self.assertEqual(bornes["m"]["hour_from"], ("8.0", "13.0")) + + def test_a_short_answer_is_refused_whole(self): + """Moins de mesures que de colonnes : `zip` tronquerait en silence + et attribuerait la mesure d'une colonne à une autre.""" + champs = [self._champ("a"), self._champ("b"), self._champ("c")] + ecartees, bornes = self._sonder("8.0:13.0", champs) + self.assertEqual(bornes, {}) + self.assertEqual(ecartees, {}) + + def test_a_long_answer_is_refused_too(self): + champs = [self._champ("a")] + _, bornes = self._sonder("8.0:13.0\x1f1.0:2.0", champs) + self.assertEqual(bornes, {}) + + def test_the_exact_count_is_accepted(self): + champs = [self._champ("a"), self._champ("b")] + _, bornes = self._sonder("8.0:13.0\x1f1.0:2.0", champs) + self.assertEqual(len(bornes["m"]), 2)