fix(channel): validate peer_id at envelope build — close path-traversal foothold - #2481
Merged
Merged
Conversation
…al foothold
Two trust-boundary leaks surfaced in code review of the channel-envelope
enrichment work:
1. _agent_card_url_for(peer_id) interpolated raw input into
${PLATFORM_URL}/registry/discover/<peer_id> with no UUID guard. An
upstream row with peer_id=`../../foo` produced an agent-visible URL
pointing at a sibling registry path. Same trust-boundary rationale
discover_peer's docstring already calls out: "never interpolate
path-traversal characters into the URL". Now gated by _validate_peer_id;
returns "" on validation failure.
2. _build_channel_notification echoed raw peer_id back into
meta["peer_id"], which on the push path renders inside the agent's
<channel peer_id="..." kind="..."> XML-attribute context. Attacker
bytes (control chars, embedded quotes) would land in agent-rendered
text wired into the next conversation turn. Now canonicalised through
_validate_peer_id before any meta write; on validation failure we
set "" rather than reflecting the raw bytes.
Defense-in-depth — both layers gate independently. Mutation-verified by
stashing both prod-side files and confirming both regression tests fail.
Tests:
- test_envelope_enrichment_invalid_peer_id_skips_lookup: updated to
pin the safe behavior (peer_id="" + agent_card_url absent), not the
prior leak shape.
- test_envelope_enrichment_strips_path_traversal_peer_id: NEW. Hard
regression for peer_id="../../foo" — pins both the URL-builder and
the meta echo against this specific exploit shape.
- Two existing tests updated to use UUID-shape placeholders instead
of "ws-peer-uuid" / "peer-ws-uuid" since those non-UUIDs now correctly
get stripped by the validator.
Resolves the Required-grade finding from the multi-axis review on PR #2471.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
requested a review
from hongmingwang-moleculeai
as a code owner
May 2, 2026 01:44
HongmingWang-Rabbit
enabled auto-merge
May 2, 2026 01:45
This was referenced May 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to PR #2471 (now merged at 9bbf32b). Multi-axis review of #2471 surfaced a Required-grade trust-boundary leak that landed before the fix could be applied — opening this targeted follow-up so staging closes the gap before any downstream rollout.
The two leaks
1. `_agent_card_url_for(peer_id)` interpolated raw input into the URL.
```python
def _agent_card_url_for(peer_id: str) -> str:
return f"{PLATFORM_URL}/registry/discover/{peer_id}"
```
An upstream inbox row with `peer_id="../../foo"` produced an agent-visible URL pointing at `${PLATFORM_URL}/registry/discover/../../foo`. Same trust-boundary rationale `discover_peer`'s docstring already documents (line 174): "never interpolate path-traversal characters into the URL." That guard was added on the tool surface but missed on the push-envelope surface. Now gated by `_validate_peer_id`; returns `""` on validation failure.
2. `_build_channel_notification` echoed raw peer_id back into `meta["peer_id"]`.
The push path renders the meta dict as XML attrs on a `<channel peer_id="..." kind="..." ...>` synthetic user turn in the agent's context. Attacker bytes (control chars, embedded quotes) would land in agent-rendered text wired into the next conversation turn — even if Claude Code's renderer escapes correctly today, "trust the renderer to escape" is the wrong default. Now canonicalised through `_validate_peer_id` before any meta write; on validation failure we set `""` rather than reflect raw bytes.
Defense-in-depth — both layers gate independently.
Tests
Mutation-verified. Stashed both prod-side files and confirmed both regression tests fail with the bug back in (`peer_id == "not-a-uuid"` is left intact, `agent_card_url == ".../registry/discover/not-a-uuid"`).
```
$ git stash push -- workspace/a2a_client.py workspace/a2a_mcp_server.py
$ pytest tests/test_a2a_mcp_server.py::test_envelope_enrichment_invalid_peer_id_skips_lookup
FAILED — assert meta["peer_id"] == "" # actual: "not-a-uuid"
```
Test plan
🤖 Generated with Claude Code