fix: harden XML parsing with defusedxml across gateway, skills, and tests

[lanzhi-lee]

What does this PR do?

Hardens all XML parsing call sites across the codebase by replacing xml.etree.ElementTree.fromstring / parse with their defusedxml equivalents. The stdlib parser resolves external entities by default, which could allow an attacker to read local files, perform SSRF, or trigger denial-of-service via entity expansion (XXE).

defusedxml is the Python-recommended drop-in replacement that blocks external entity resolution, DTD fetching, and entity expansion attacks by default.

Related Issue

Type of Change

• 🔒 Security fix

Changes Made

• gateway/platforms/wecom_callback.py — replace ET.fromstring with defusedxml.ElementTree.fromstring in _decrypt_request and _build_event (WeCom callback payloads)
• optional-skills/devops/watchers/scripts/watch_rss.py — replace ET.fromstring with defusedxml.fromstring; widen except to also catch DefusedXmlException
• skills/productivity/powerpoint/scripts/office/helpers/simplify_redlines.py — replace ET.parse with defusedxml.parse; widen both except ET.ParseError clauses to also catch DefusedXmlException
• skills/research/arxiv/scripts/search_arxiv.py — replace ET.fromstring with defusedxml.fromstring (arXiv API responses)
• tests/acp/test_registry_manifest.py — replace ET.fromstring with defusedxml.fromstring
• tests/gateway/test_wecom_callback.py — replace ET.fromstring with defusedxml.fromstring
• pyproject.toml + uv.lock — promote defusedxml==0.7.1 to a core dependency (zero transitive deps), since it is now used across gateway, skills, optional-skills, and tests that have no relation to any single optional-dependency group

How to Test

1. Run pytest tests/ -q — all existing tests should pass
2. Confirm defusedxml.fromstring raises EntitiesForbidden when given XML with <!DOCTYPE ...> declarations:

   from defusedxml.ElementTree import fromstring
   fromstring('<!DOCTYPE foo [<!ENTITY xxe SYSTEM "file:///etc/passwd">]><root>&xxe;</root>')
   # raises EntitiesForbidden

3. For watch_rss.py: verify a malicious feed XML with DTD triggers the graceful sys.exit(2) path instead of an uncaught traceback

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run pytest tests/ -q and all tests pass
• I've tested on my platform: macOS 15.3

Documentation & Housekeeping

• I've updated relevant documentation — or N/A
• I've updated cli-config.yaml.example if I added/changed config keys — or N/A
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
• I've considered cross-platform impact (Windows, macOS) — or N/A
• I've updated tool descriptions/schemas if I changed tool behavior — or N/A

🤖 Generated with Claude Code

[austinpickett]

Please use PULL_REQUEST_TEMPLATE.md

[Copilot]

Pull request overview

Hardens XML parsing across three call sites by replacing xml.etree.ElementTree parsing with defusedxml equivalents, to defend against XXE/DTD/entity-expansion attacks on potentially attacker-controlled XML inputs (WeCom callbacks, RSS feeds, and .docx tracked-change extraction). Adds defusedxml==0.7.1 to the [messaging] extra.

Changes:

• Replace ET.fromstring with defusedxml.ElementTree.fromstring in gateway/platforms/wecom_callback.py (decrypt + event build paths).
• Apply the same hardening to optional-skills/devops/watchers/scripts/watch_rss.py and skills/productivity/powerpoint/scripts/office/helpers/simplify_redlines.py (via _safe_parse).
• Declare defusedxml==0.7.1 in the [messaging] optional-dependency group (and corresponding uv.lock update).

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
gateway/platforms/wecom_callback.py	Swap ET.fromstring for safe defusedxml variant in WeCom decrypt + event paths.
optional-skills/devops/watchers/scripts/watch_rss.py	Parse RSS/Atom feeds via defusedxml.
skills/productivity/powerpoint/scripts/office/helpers/simplify_redlines.py	Parse word/document.xml via defusedxml parse.
pyproject.toml	Add defusedxml==0.7.1 to [messaging] extra.
uv.lock	Lockfile update for defusedxml.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[lanzhi-lee]

Done — updated the PR description to use the template. Thanks!

[lanzhi-lee]

@austinpickett Updated — the PR description now uses the template and covers all changed files. Thanks for the review!

[austinpickett]

Code Review — fix: harden XML parsing with defusedxml

8 files, +29/-16. Replaces stdlib xml.etree.ElementTree parsing with defusedxml to prevent XXE attacks. Clean, well-scoped security hardening.

---

⚠️ Minor fix needed

(item 1) Dead import in wecom_callback.py — after replacing both ET.fromstring() calls with _safe_fromstring, the from xml.etree import ElementTree as ET import (line 21) is completely unused. No remaining ET. references in the file. Remove it.

---

💡 Suggestions

(item 2) Four documentation files still show xml.etree.ElementTree in code examples (research-arxiv.md and SKILL.md). While not production code, they contradict the security posture. Could be a follow-up PR.

---

✅ Looks Good

• All parsing call sites migrated: 8 ET.fromstring()/ET.parse() calls across 5 production files and 2 test files correctly replaced with defusedxml equivalents
• Exception handling updated: DefusedXmlException correctly added alongside ET.ParseError in except clauses
• XML construction left alone: wecom_crypto.py correctly keeps ET.Element, ET.SubElement, ET.tostring on stdlib — these are generation, not parsing
• defusedxml==0.7.1 properly pinned — latest release, zero transitive deps
• Protections: XXE, billion laughs, DTD retrieval, processing instructions all blocked by default

Verdict: Approve with (item 1) dead-import cleanup. The core security changes are correct and complete. LGTM.

Reviewed by Hermes Agent

[lanzhi-lee]

@austinpickett All review items addressed:

• item 1: Removed the unused import from
• item 2: Updated all 4 code examples in and to use

Ready for another look when you get a chance. Thanks!

[teknium1]

Thanks for the security hardening. I would keep this open for salvage rather than close it as already merged.

Problems

• Against current main, the WeCom slice is already implemented: 5744b17 switched gateway/platforms/wecom_callback.py:335 and :341 to defusedxml-backed parsing, and 31c8d5f added the [wecom] dependency/lazy-dep path. GitHub reports this PR as DIRTY, so salvage should drop or adjust those hunks.
• The broader XML parser class is still incomplete: optional-skills/devops/watchers/scripts/watch_rss.py:38, skills/productivity/powerpoint/scripts/office/helpers/simplify_redlines.py:131/:155, and skills/research/arxiv/scripts/search_arxiv.py:50 still use stdlib parsing on current main. A sibling user-document parser surface also remains in tools/read_extract.py:100/:170/:191/:213.
• pyproject.toml:39-44 says core deps are only for packages used by every Hermes session; this PR's core defusedxml promotion conflicts with current main's [wecom]/lazy-dep placement.

Suggested changes

• Salvage the remaining call sites, cover tools/read_extract.py or explain why it is out of scope, add malicious-DTD/entity tests, and update the zh-Hans arXiv docs if the English docs stay in scope.

Automated hermes-sweeper review.

[teknium1]

This core dependency placement conflicts with the current pyproject.toml scope rule that base dependencies are only for packages used by every Hermes session. Current main puts defusedxml behind the WeCom extra/lazy-dep path, so a salvage should either keep it gated or add a concrete justification for making every install carry it.

[lanzhi-lee]

@teknium1 Updated this PR for another review.

Changes made:

• Rebased/salvaged onto current upstream/main and dropped the stale WeCom-only hunks already covered on main.
• Hardened the remaining XML parser surfaces called out in the review: RSS watcher, PowerPoint redline helper, arXiv search, and tools/read_extract.py DOCX/XLSX parsing.
• Moved defusedxml to core with an explicit justification because core read_file document extraction now parses untrusted DOCX/XLSX XML; kept wecom as an empty compatibility extra.
• Added malicious DTD/XXE regression coverage for DOCX/XLSX extraction and updated English + zh-Hans arXiv docs examples.

Validation run locally:

• python -m py_compile optional-skills/devops/watchers/scripts/watch_rss.py skills/productivity/powerpoint/scripts/office/helpers/simplify_redlines.py skills/research/arxiv/scripts/search_arxiv.py tools/read_extract.py
• python -m pytest tests/tools/test_read_extract.py -q → 20 passed
• python -m ruff check optional-skills/devops/watchers/scripts/watch_rss.py skills/productivity/powerpoint/scripts/office/helpers/simplify_redlines.py skills/research/arxiv/scripts/search_arxiv.py tools/read_extract.py tests/tools/test_read_extract.py
• git diff --check

Could you please take another look when you have a chance? Thanks!