Claude commited on
Commit
180bb96
·
unverified ·
1 Parent(s): 315a6b9

refactor(core): extraire safe_parse_xml en cercle 1 + appliquer aux 3 sites XXE résiduels

Browse files

Audit F1 — ``safe_parse_xml`` n'existait que dans ``picarones/web/corpus_utils.py``,
ce qui empêchait son réemploi depuis cercle 2 (``measurements/``) et cercle 3
(``extras/``). Trois sites parsaient encore du XML reçu d'une source externe via
``ET.fromstring`` direct :

- ``picarones/extras/importers/gallica.py`` (réponse SRU + métadonnées OAI),
- ``picarones/measurements/alto_metrics.py`` (extraction de texte d'un ALTO).

Création de ``picarones/core/xml_utils.py`` (cercle 1, abstraction pure, déléguée
à ``defusedxml`` exactement comme l'implémentation web). Les trois sites importent
désormais ``safe_parse_xml`` et délèguent leurs erreurs au logger à la place du
``except: pass`` ou du double-try imbriqué. ``picarones/web/corpus_utils.py``
réexporte ``safe_parse_xml`` dans son ``__all__`` pour rétrocompat.

Verrou levé : un module ``BaseModule`` tiers qui produit un ALTO depuis une
source non vérifiée (preuve de concept Sprint 33+) ne peut plus déclencher de
XXE, Billion Laughs ou DTD retrieval lors du calcul des métriques
``alto_text_*``.

3354 passed, 2 skipped, 0 failed. ``ruff check`` clean.

https://claude.ai/code/session_01Hsd7kL8yeCbXn1mA7GQK9L

picarones/core/xml_utils.py ADDED
@@ -0,0 +1,44 @@
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
1
+ """Parsing XML sécurisé — anti-XXE / Billion Laughs / DTD retrieval.
2
+
3
+ Helper transverse appliqué partout où Picarones parse du XML reçu
4
+ depuis une source externe (corpus uploadé via le web, manifeste
5
+ Gallica, ALTO produit par un module ``BaseModule`` tiers, etc.).
6
+
7
+ Délègue à :mod:`defusedxml` (dépendance dure du projet) qui durcit
8
+ le parser stdlib contre :
9
+
10
+ - **XXE** (``XML External Entity``) — résolution d'entités vers
11
+ des fichiers locaux ou des URL distantes ;
12
+ - **Billion Laughs** — expansion exponentielle d'entités ;
13
+ - **DTD retrieval** — fetch d'une DTD distante.
14
+
15
+ Discipline : tout module qui parse du XML doit utiliser
16
+ ``safe_parse_xml`` plutôt que ``xml.etree.ElementTree.fromstring``
17
+ directement. La règle est appliquée par un audit récurrent
18
+ (grep ``ET.fromstring`` dans ``picarones/``).
19
+ """
20
+
21
+ from __future__ import annotations
22
+
23
+ import xml.etree.ElementTree as ET
24
+ from typing import Optional
25
+
26
+ import defusedxml
27
+ import defusedxml.ElementTree as _SafeET
28
+
29
+
30
+ def safe_parse_xml(xml_bytes: bytes) -> Optional[ET.Element]:
31
+ """Parse du XML en bloquant les entités externes.
32
+
33
+ Retourne ``None`` si le payload n'est pas un XML valide ou si
34
+ ``defusedxml`` détecte une attaque
35
+ (``EntitiesForbidden``, ``ExternalReferenceForbidden``,
36
+ ``DTDForbidden``, ``NotSupportedError``).
37
+ """
38
+ try:
39
+ return _SafeET.fromstring(xml_bytes)
40
+ except (ET.ParseError, defusedxml.DefusedXmlException):
41
+ return None
42
+
43
+
44
+ __all__ = ["safe_parse_xml"]
picarones/extras/importers/gallica.py CHANGED
@@ -30,6 +30,8 @@ import urllib.error
30
  import urllib.parse
31
  import urllib.request
32
  import xml.etree.ElementTree as ET
 
 
33
  from dataclasses import dataclass
34
  from typing import Optional
35
 
@@ -261,10 +263,12 @@ class GallicaClient:
261
  def _parse_sru_response(self, xml_bytes: bytes, max_results: int) -> list[GallicaRecord]:
262
  """Parse la réponse SRU XML de Gallica."""
263
  records: list[GallicaRecord] = []
264
- try:
265
- root = ET.fromstring(xml_bytes)
266
- except ET.ParseError as exc:
267
- logger.error("Impossible de parser la réponse SRU: %s", exc)
 
 
268
  return records
269
 
270
  # Les enregistrements sont dans srw:records/srw:record/srw:recordData
@@ -446,9 +450,15 @@ class GallicaClient:
446
  url = f"{_GALLICA_BASE}/services/OAIRecord?ark=ark:/12148/{ark}"
447
  try:
448
  raw = self._fetch_url(url)
449
- root = ET.fromstring(raw)
450
- except (RuntimeError, ET.ParseError) as exc:
451
- logger.error("Erreur métadonnées OAI %s: %s", ark, exc)
 
 
 
 
 
 
452
  return {"ark": ark}
453
 
454
  def find_text(tag_suffix: str) -> str:
 
30
  import urllib.parse
31
  import urllib.request
32
  import xml.etree.ElementTree as ET
33
+
34
+ from picarones.core.xml_utils import safe_parse_xml
35
  from dataclasses import dataclass
36
  from typing import Optional
37
 
 
263
  def _parse_sru_response(self, xml_bytes: bytes, max_results: int) -> list[GallicaRecord]:
264
  """Parse la réponse SRU XML de Gallica."""
265
  records: list[GallicaRecord] = []
266
+ # ``safe_parse_xml`` neutralise XXE / Billion Laughs / DTD
267
+ # retrieval — Gallica est de confiance institutionnelle mais
268
+ # on ne baisse pas la garde sur du XML reçu via le réseau.
269
+ root = safe_parse_xml(xml_bytes)
270
+ if root is None:
271
+ logger.error("Impossible de parser la réponse SRU (XML invalide ou défense XXE déclenchée)")
272
  return records
273
 
274
  # Les enregistrements sont dans srw:records/srw:record/srw:recordData
 
450
  url = f"{_GALLICA_BASE}/services/OAIRecord?ark=ark:/12148/{ark}"
451
  try:
452
  raw = self._fetch_url(url)
453
+ except RuntimeError as exc:
454
+ logger.error("Erreur fetch métadonnées OAI %s: %s", ark, exc)
455
+ return {"ark": ark}
456
+ root = safe_parse_xml(raw)
457
+ if root is None:
458
+ logger.error(
459
+ "Erreur parse XML métadonnées OAI %s (XML invalide ou défense XXE déclenchée)",
460
+ ark,
461
+ )
462
  return {"ark": ark}
463
 
464
  def find_text(tag_suffix: str) -> str:
picarones/measurements/alto_metrics.py CHANGED
@@ -51,7 +51,8 @@ from __future__ import annotations
51
  import logging
52
  import re
53
  from typing import Any
54
- from xml.etree import ElementTree as ET
 
55
 
56
  from picarones.core.metric_registry import register_metric
57
  from picarones.core.modules import ArtifactType
@@ -125,12 +126,14 @@ def extract_text_from_alto(payload: Any) -> str:
125
  xml = _coerce_alto_to_str(payload).strip()
126
  if not xml:
127
  return ""
128
- try:
129
- root = ET.fromstring(xml)
130
- except ET.ParseError as exc:
 
 
131
  logger.warning(
132
- "[alto_metrics] ALTO non parsable (%s) texte extrait vide",
133
- exc,
134
  )
135
  return ""
136
 
 
51
  import logging
52
  import re
53
  from typing import Any
54
+
55
+ from picarones.core.xml_utils import safe_parse_xml
56
 
57
  from picarones.core.metric_registry import register_metric
58
  from picarones.core.modules import ArtifactType
 
126
  xml = _coerce_alto_to_str(payload).strip()
127
  if not xml:
128
  return ""
129
+ # ``safe_parse_xml`` neutralise XXE / Billion Laughs / DTD
130
+ # retrieval — l'ALTO peut venir d'un module ``BaseModule`` tiers
131
+ # qui n'a pas de garantie de provenance.
132
+ root = safe_parse_xml(xml.encode("utf-8") if isinstance(xml, str) else xml)
133
+ if root is None:
134
  logger.warning(
135
+ "[alto_metrics] ALTO non parsable (XML invalide ou défense XXE "
136
+ "déclenchée) — texte extrait vide",
137
  )
138
  return ""
139
 
picarones/web/corpus_utils.py CHANGED
@@ -1,21 +1,18 @@
1
  """Utilitaires de manipulation de corpus côté web.
2
 
3
- Parsing sécurisé d'XML (anti-XXE), détection ALTO/PAGE, extraction de
4
- texte GT, analyse de la structure d'un dossier corpus, extraction de
5
- ZIP avec garde-fous (taille décompressée, nombre de fichiers).
 
6
  """
7
 
8
  from __future__ import annotations
9
 
 
10
  import zipfile
11
  from pathlib import Path
12
- from typing import Optional
13
-
14
- import xml.etree.ElementTree as ET
15
-
16
- import defusedxml
17
- import defusedxml.ElementTree as _SafeET
18
 
 
19
  from picarones.web.state import IMAGE_EXTS
20
 
21
  # Garde-fous ZIP-bomb pour l'upload
@@ -27,30 +24,9 @@ MAX_ZIP_FILES = 2000
27
 
28
 
29
  # ──────────────────────────────────────────────────────────────────────────
30
- # Parsing XML sécurisé (ALTO / PAGE)
31
  # ──────────────────────────────────────────────────────────────────────────
32
 
33
- def safe_parse_xml(xml_bytes: bytes) -> Optional[ET.Element]:
34
- """Parse du XML en bloquant les entités externes (protection XXE).
35
-
36
- Délègue à :mod:`defusedxml` (dépendance dure du projet) qui durcit
37
- le parser stdlib contre :
38
-
39
- - **XXE** (``XML External Entity``) — résolution d'entités vers
40
- des fichiers locaux ou des URL distantes ;
41
- - **Billion Laughs** — expansion exponentielle d'entités ;
42
- - **DTD retrieval** — fetch d'une DTD distante.
43
-
44
- Retourne ``None`` si le payload n'est pas un XML valide ou si
45
- ``defusedxml`` détecte une attaque (``EntitiesForbidden``,
46
- ``ExternalReferenceForbidden``, etc.).
47
- """
48
- try:
49
- return _SafeET.fromstring(xml_bytes)
50
- except (ET.ParseError, defusedxml.DefusedXmlException):
51
- return None
52
-
53
-
54
  def detect_xml_gt(xml_bytes: bytes) -> tuple[str, str] | None:
55
  """Détecte si ``xml_bytes`` est un fichier ALTO ou PAGE XML.
56
 
 
1
  """Utilitaires de manipulation de corpus côté web.
2
 
3
+ Détection ALTO/PAGE, extraction de texte GT, analyse de la structure
4
+ d'un dossier corpus, extraction de ZIP avec garde-fous (taille
5
+ décompressée, nombre de fichiers). Le parsing XML sécurisé délègue
6
+ à :func:`picarones.core.xml_utils.safe_parse_xml`.
7
  """
8
 
9
  from __future__ import annotations
10
 
11
+ import xml.etree.ElementTree as ET
12
  import zipfile
13
  from pathlib import Path
 
 
 
 
 
 
14
 
15
+ from picarones.core.xml_utils import safe_parse_xml
16
  from picarones.web.state import IMAGE_EXTS
17
 
18
  # Garde-fous ZIP-bomb pour l'upload
 
24
 
25
 
26
  # ──────────────────────────────────────────────────────────────────────────
27
+ # Détection ALTO / PAGE depuis bytes XML
28
  # ──────────────────────────────────────────────────────────────────────────
29
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
30
  def detect_xml_gt(xml_bytes: bytes) -> tuple[str, str] | None:
31
  """Détecte si ``xml_bytes`` est un fichier ALTO ou PAGE XML.
32
 
tests/core/test_public_api.py CHANGED
@@ -406,6 +406,7 @@ class TestCercle1IsLean:
406
  EXPECTED_CERCLE1 = {
407
  "corpus.py", "facts.py", "metric_hooks.py", "metric_registry.py",
408
  "metrics.py", "modules.py", "pipeline.py", "results.py",
 
409
  }
410
 
411
  def test_cercle1_files_lean(self):
 
406
  EXPECTED_CERCLE1 = {
407
  "corpus.py", "facts.py", "metric_hooks.py", "metric_registry.py",
408
  "metrics.py", "modules.py", "pipeline.py", "results.py",
409
+ "xml_utils.py",
410
  }
411
 
412
  def test_cercle1_files_lean(self):