fix: harden XML parsing with defusedxml across gateway, skills, and tests by lanzhi-lee · Pull Request #28585 · NousResearch/hermes-agent

lanzhi-lee · 2026-05-19T07:07:36Z

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

Run pytest tests/ -q — all existing tests should pass

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

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 · 2026-05-19T10:34:53Z

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.

…ests Replace xml.etree.ElementTree.fromstring/parse with defusedxml equivalents to prevent XXE injection in all code paths that parse external or untrusted XML: - gateway/platforms/wecom_callback.py: WeCom callback payloads - optional-skills/devops/watchers/scripts/watch_rss.py: RSS/Atom feeds - skills/productivity/powerpoint/.../simplify_redlines.py: .docx XML - skills/research/arxiv/scripts/search_arxiv.py: arXiv API responses - tests/acp/test_registry_manifest.py: SVG icon parsing - tests/gateway/test_wecom_callback.py: test XML parsing Promote defusedxml to a core dependency (zero transitive deps) since it is used across gateway, skills, optional-skills, and tests that have no relation to any single optional-dependency group. Catch DefusedXmlException alongside ET.ParseError in watch_rss.py and simplify_redlines.py so hostile XML triggers the existing graceful fallback instead of an uncaught traceback. Co-Authored-By: MiMo-V2.5-Pro <noreply@xiaomi.com>

lanzhi-lee · 2026-05-19T11:27:11Z

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

lanzhi-lee · 2026-05-19T11:48:18Z

@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

Co-Authored-By: MiMo-V2.5-Pro <noreply@xiaomi.com>

lanzhi-lee · 2026-05-21T02:03:30Z

@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!

lanzhi-lee requested a review from a team May 19, 2026 07:07

alt-glitch added type/security Security vulnerability or hardening P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/wecom WeCom / WeChat Work adapter labels May 19, 2026

austinpickett requested a review from Copilot May 19, 2026 10:34

Copilot started reviewing on behalf of austinpickett May 19, 2026 10:34 View session

Copilot AI reviewed May 19, 2026

View reviewed changes

Comment thread optional-skills/devops/watchers/scripts/watch_rss.py

Comment thread optional-skills/devops/watchers/scripts/watch_rss.py

Comment thread skills/productivity/powerpoint/scripts/office/helpers/simplify_redlines.py

Comment thread pyproject.toml Outdated

lanzhi-lee force-pushed the fix/wecom-callback-xxe branch from 402ad11 to 2c3e7d7 Compare May 19, 2026 11:19

lanzhi-lee changed the title ~~fix(gateway): use defusedxml to prevent XXE in WeCom callback~~ fix: harden XML parsing with defusedxml across gateway, skills, and tests May 19, 2026

austinpickett previously approved these changes May 20, 2026

View reviewed changes

lanzhi-lee and others added 2 commits May 21, 2026 09:46

fix: remove unused xml.etree.ElementTree import in wecom_callback.py

bc6e36e

Co-Authored-By: MiMo-V2.5-Pro <noreply@xiaomi.com>

chore: update arxiv docs to use defusedxml in code examples

cb273d1

Co-Authored-By: MiMo-V2.5-Pro <noreply@xiaomi.com>

alt-glitch mentioned this pull request May 25, 2026

harden: restrict markdown link schemes; parse untrusted XML with defusedxml #32155

Closed

lanzhi-lee dismissed austinpickett’s stale review via cb273d1 May 28, 2026 22:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

fix: harden XML parsing with defusedxml across gateway, skills, and tests#28585
lanzhi-lee wants to merge 3 commits into
NousResearch:mainfrom
lanzhi-lee:fix/wecom-callback-xxe

lanzhi-lee commented May 19, 2026 •

edited

Loading

Uh oh!

austinpickett commented May 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lanzhi-lee commented May 19, 2026

Uh oh!

lanzhi-lee commented May 19, 2026

Uh oh!

austinpickett left a comment

Uh oh!

lanzhi-lee commented May 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

lanzhi-lee commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Related Issue

Type of Change

Changes Made

How to Test

Checklist

Code

Documentation & Housekeeping

Uh oh!

austinpickett commented May 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lanzhi-lee commented May 19, 2026

Uh oh!

lanzhi-lee commented May 19, 2026

Uh oh!

austinpickett left a comment

Choose a reason for hiding this comment

Code Review — fix: harden XML parsing with defusedxml

⚠️ Minor fix needed

💡 Suggestions

✅ Looks Good

Uh oh!

lanzhi-lee commented May 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lanzhi-lee commented May 19, 2026 •

edited

Loading