fix(server): validate push-notification URLs before dispatch (SSRF hardening) - #1164
fix(server): validate push-notification URLs before dispatch (SSRF hardening)#1164SashaMIT wants to merge 1 commit into
Conversation
…rdening) A client sets its push-notification webhook URL via tasks/pushNotificationConfig (or inline on message/send), and the server then POSTs task events to that URL. The URL was used exactly as supplied - no scheme check, no destination check - so every deployment of the reference sender exposed a blind server-side request forgery primitive: point a task's push config at http://169.254.169.254/... (cloud metadata), http://localhost:PORT/admin, or any internal service and the agent server POSTs there on every task event. BasePushNotificationSender now validates each URL at dispatch time: scheme must be http/https, the host must resolve, and every resolved address must be public unicast (loopback, link-local, private, reserved, multicast, and unspecified addresses are rejected; unresolvable hosts fail closed since the POST would fail anyway). Operators whose legitimate webhooks live on private networks can opt out with allow_private_push_urls=True. Validation happens at dispatch rather than at config-write so configs registered through any path (create, inline on send, future stores) are covered by the same choke point. Residual risk, documented in the constructor docstring: DNS rebinding between validation and the POST itself remains possible for attacker-controlled domains; static internal targets are fully blocked. Tests: 7 new unit tests (metadata IP, loopback, private range, non-http scheme, unresolvable host fail-closed, public allowed, opt-out); existing suites made DNS-hermetic; push-notification e2e app opts out since its webhooks are real local servers. Signed-off-by: SashaMIT <sash@ela.city> Co-authored-by: Cursor <cursoragent@cursor.com>
🧪 Code Coverage (vs
|
| Base | PR | Delta | |
|---|---|---|---|
| src/a2a/server/events/event_queue_v2.py | 91.79% | 91.28% | 🔴 -0.51% |
| src/a2a/server/tasks/base_push_notification_sender.py | 94.44% | 89.87% | 🔴 -4.57% |
| src/a2a/utils/telemetry.py | 91.47% | 90.70% | 🔴 -0.78% |
| Total | 93.00% | 92.94% | 🔴 -0.06% |
Generated by coverage-comment.yml
kuangmi-bit
left a comment
kuangmi-bit
left a comment
There was a problem hiding this comment.
Reviewed the SSRF hardening — solid implementation. A few notes from having done the same analysis on a sibling A2A-ecosystem project (a registry service) recently:
What's done well:
- Validation happens after DNS resolution (
getaddrinfo→ check every returned address) — this catches IP literals in integer/hex forms and IPv4-mapped IPv6 (::ffff:127.0.0.1), not just dotted-quad strings - Fail-closed on unresolvable hosts (the POST would fail anyway; treating it as a pass would be a bypass)
- The
for info in infos: if blocked -> rejectloop requires ALL resolved addresses to be public, not just any — correct allow_private_push_urlsescape hatch keeps legitimate private-network webhooks working without weakening the default- Test coverage is thorough (metadata endpoint, loopback, private range, scheme, fail-closed, public allow, opt-out)
Two residual risks worth documenting (not blocking):
-
Redirect targets are not re-validated.
httpx.AsyncClientdefaults tofollow_redirects=False, but a caller can enable it — in that case the initial URL passes validation and a redirect to an internal address (e.g.https://public.example→http://169.254.169.254/) is dispatched without re-checking. Worth a doc note on thehttpx_clientparameter: "validation covers the initial URL only; keepfollow_redirects=False(the default) or the client is exposed to redirect-based SSRF." -
DNS rebinding TOCTOU window. Validation and connection are two separate resolutions; a hostile DNS server can return a public IP for the validation lookup and a private IP for the connection lookup. Hard to close fully at this layer (would require pinning the validated IP in the transport), but worth documenting as a known limitation so operators can mitigate with network controls.
Minor: consider a short docstring note in push_url_validation_error that IPv4-mapped IPv6 is covered (the is_private/is_loopback checks on mapped addresses already handle it, but the comment would save future readers a double-take).
2 participants
Summary
In plain terms: when a client tells an A2A agent "send my task updates to this webhook", the agent server POSTs to whatever URL the client supplied — no checks at all. That means any client can make the agent server send requests to internal network addresses: cloud metadata endpoints (
169.254.169.254),localhostadmin panels, or unauthenticated internal services. This is the classic server-side request forgery (SSRF) pattern, and it fires on every task event.Concretely,
BasePushNotificationSender._dispatch_notificationusedpush_info.urlexactly as supplied:ftp://, future handler schemes),tasks/pushNotificationConfig/create, inline onmessage/send), so write-time validation alone wouldn't cover all of them.Fix
BasePushNotificationSendernow validates each URL at dispatch time (one choke point covering every registration path):http/https,Operators whose legitimate webhooks live on private networks opt out explicitly:
BasePushNotificationSender(..., allow_private_push_urls=True).Residual risk, stated honestly: DNS rebinding between validation and the POST itself remains possible for attacker-controlled domains (validation and the actual connection resolve the name separately). Static internal targets — the realistic SSRF cases here — are fully blocked. Noted in the constructor docstring.
Test plan
tests/server/tasks/185 pass (existing suites made DNS-hermetic)ruff check+ruff formatclean on touched filesMade with Cursor
Made with Cursor