fix(security): SSRF via AgentCard URL and context ID Injection (A2A-SSRF-01, A2A-INJ-01)#895
Conversation
…SRF-01, A2A-INJ-01) - Add url_validation.py: validates AgentCard.url against loopback, RFP 1918, link-local (IMDS), and non-http(s) schemes before SDK uses it as RPC endpoint - Patch card_resolver.py: call validate_agent_card_url() after model_validate() for card url and all additional_interfaces urls - Patch default_request_handler.py: add optional get_caller_id hook to enforce cantext_id ownership; defaults to warn-and-allow for backword compatibility Fixes CWE-918 (SSRF) and CWE-639 (context injection)
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the security posture of the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces significant security enhancements by addressing two vulnerabilities: A2A-SSRF-01 and A2A-INJ-01. For A2A-SSRF-01, a new url_validation.py module is added to validate agent card URLs against private or reserved IP ranges, and this validation is integrated into card_resolver.py. For A2A-INJ-01, context-level ownership tracking is implemented in default_request_handler.py to prevent unauthorized message injection into existing contexts. Feedback includes a high-severity issue where the SSRF warning is not logged if get_caller_id is not configured, suggesting it be moved to __init__. There's also a medium-severity concern about the in-memory nature of _context_owners, which could lead to context hijacking after server restarts, suggesting persistence. Additionally, several low-severity comments recommend restoring removed docstrings for maintainability, addressing unreachable code in _check_context_ownership, and changing _BLOCKED_NETWORKS to a tuple for immutability.
- Move context ownership warning to __init__ (was unreachable) - Remove unreachable owner-is-None guard in _check_context_ownership - Change _BLOCKED_NETWORKS to tuple for immutability - Restore dropped docstrings in card_resolver and default_request_handler - Fix non-ASCII chars in comments (use -> and --) - Add tests/utils/test_url_validation.py with 26 SSRF validation tests
Mock(spec=AgentCard) with Pydantic v2 does not expose field attributes. Our SSRF patch accesses agent_card.url after model_validate(), add url and additional_interfaces to all Mock(spec=AgentCard) instances so the attribute access succeeds.
🧪 Code Coverage (vs
|
| Base | PR | Delta | |
|---|---|---|---|
| src/a2a/client/card_resolver.py | 100.00% | 95.00% | 🔴 -5.00% |
| src/a2a/server/request_handlers/default_request_handler.py | 97.34% | 90.70% | 🔴 -6.64% |
| src/a2a/utils/url_validation.py (new) | — | 89.19% | — |
| Total | 91.73% | 91.32% | 🔴 -0.41% |
Generated by coverage-comment.yml
- Consolidate two near-identical conftest files into tests/conftest.py to fix JSCPD copy-paste detection failure - Add ASSRF, canonname, gaierror, IMDS, INJ, sockaddr to spell-check allow list (all come from our url_validation.py patch)
…patch
- Remove module-level research docstrings from card_resolver.py and
default_request_handler.py (originals have none; D415 lint failure)
- Add missing docstrings to five push-notification handler methods
that existed in the original SDK (D102 lint failure)
- Fix import ordering in card_resolver.py and url_validation.py (I001)
- Convert double-quoted string literals to single quotes throughout
both patched source files (Q000; project uses quote-style = single)
- Restore parentheses in except clause:
except (asyncio.CancelledError, GeneratorExit):
Bare tuple syntax was added in Python 3.14; CI targets Python 3.10
where it raises SyntaxError (ruff had silently removed the parens)
Move pydantic import to correct position per project isort config
Fix import ordring and covert double quoted to single quotes
Security Fix: SSRF via AgentCard URL & Context ID Injection (A2A-SSRF-01, A2A-INJ-01)
Summary
This PR addresses two security vulnerabilities confirmed against
a2a-sdkv0.3.25:AgentCard.urlused as RPC endpoint with no validation — SSRF to internal networksmessage.contextIdaccepted without ownership check — cross-user context injectionChanges
Fix 1 — A2A-SSRF-01: URL validation for AgentCard (new file + patch)
New file:
src/a2a/utils/url_validation.pyAdds
validate_agent_card_url(url: str) -> Nonewhich:httporhttpsscheme (blocksfile://,gopher://, etc.)socket.getaddrinfoand rejects addresses within:127.0.0.0/8,::1)10/8,172.16/12,192.168/16)169.254.0.0/16,fe80::/10) — covers AWS/GCP/Azure metadata servicefc00::/7), shared address space (100.64.0.0/10), and other IANA reserved blocksA2ASSRFValidationError(subclass ofValueError) with a descriptive messagePatched:
src/a2a/client/card_resolver.pyCalls
validate_agent_card_url()afterAgentCard.model_validate()inget_agent_card(), before returning the card to the caller. Also validatesall
additional_interfaces[*].urlvalues (same attack surface).Why here: The card resolver is the single choke point where all remote
cards enter the SDK. Validating here covers
ClientFactory.connect(),direct
A2ACardResolverusage, and any future client code that callsget_agent_card().Fix 2 — A2A-INJ-01: Context ownership policy hook
Patched:
src/a2a/server/request_handlers/default_request_handler.pyAdds an optional
get_caller_idparameter toDefaultRequestHandler.__init__:In
_setup_message_execution, the first caller for a givencontext_idisrecorded in
_context_owners. Subsequent requests for the same context from adifferent identity are rejected with
ServerErrorbefore any task lookup:Default behaviour (backward compatible): When
get_caller_idis notprovided, the handler logs a
WARNINGand allows the request through.Existing deployments are unaffected.
Production use: Supply any callable that extracts a stable identity string
from
ServerCallContext(JWTsub, mTLS cert fingerprint, API key header, etc.).Why a hook rather than hard enforcement: The A2A spec does not yet define
a normative identity model. A hook lets each deployment enforce ownership
using whatever identity mechanism they have, without the SDK imposing a
specific auth scheme. The warning log ensures unenforced deployments are
visibly alerted.
Testing
SSRF validation (
tests/utils/test_url_validation.py)Parametrize
validate_agent_card_urlover blocked inputs (loopback127.0.0.1,localhost, RFC 1918 ranges, AWS IMDS169.254.169.254, IPv6::1,file://,gopher://) and assertA2ASSRFValidationErroris raised.Assert no exception for
https://agent.example.com/rpc.Context ownership integration test
Construct a
DefaultRequestHandlerwith aget_caller_idextractor.Send a first message as
"alice"to establish context ownership.Send a second message referencing the same
context_idwith caller"attacker"andassert a
ServerErroris raised. Verify the policy was invoked exactly once.Backward Compatibility
validate_agent_card_url(new module)get_agent_cardraisesA2AClientJSONErrorfor blocked URLsget_caller_idparameterReferences
Checklist
CONTRIBUTINGGuide.fix:which represents bug fixes, and correlates to a SemVer patch.feat:represents a new feature, and correlates to a SemVer minor.feat!:, orfix!:,refactor!:, etc., which represent a breaking change (indicated by the!) and will result in a SemVer major.bash scripts/format.shfrom the repository root to format)Fixes None 🦕