fix(server): validate push-notification URLs before dispatch (SSRF hardening)

[SashaMIT]

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), localhost admin 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_notification used push_info.url exactly as supplied:

• no scheme restriction (ftp://, future handler schemes),
• no destination restriction (loopback / link-local / RFC1918 / reserved),
• and configs can be registered through multiple paths (tasks/pushNotificationConfig/create, inline on message/send), so write-time validation alone wouldn't cover all of them.

Fix

BasePushNotificationSender now validates each URL at dispatch time (one choke point covering every registration path):

• scheme must be http/https,
• host must resolve (unresolvable fails closed — the POST would fail anyway),
• every resolved address must be public unicast; loopback, link-local, private, reserved, multicast, and unspecified addresses are rejected.

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

• 7 new unit tests: metadata IP blocked, loopback blocked, private range blocked, non-HTTP scheme blocked, unresolvable host fails closed, public host allowed, opt-out allows private
• tests/server/tasks/ 185 pass (existing suites made DNS-hermetic)
• Push-notification e2e suite passes (test app opts out — its webhooks are real local servers)
• ruff check + ruff format clean on touched files

Made with Cursor

Made with Cursor

[github-actions]

🧪 Code Coverage (vs main)

⬇️ Download Full Report

	Base	PR	Delta
src/a2a/server/request_handlers/default_request_handler.py	97.90%	97.90%	🔴 -0.01%
src/a2a/server/request_handlers/default_request_handler_v2.py	92.05%	92.44%	🟢 +0.39%
src/a2a/server/tasks/base_push_notification_sender.py	81.43%	95.12%	🟢 +13.69%
src/a2a/utils/push_url_validator.py (new)	—	87.80%	—
Total	92.97%	93.05%	🟢 +0.09%

Generated by coverage-comment.yml

[kuangmi-bit]

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 -> reject loop requires ALL resolved addresses to be public, not just any — correct
• allow_private_push_urls escape 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):

1. Redirect targets are not re-validated. httpx.AsyncClient defaults to follow_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 the httpx_client parameter: "validation covers the initial URL only; keep follow_redirects=False (the default) or the client is exposed to redirect-based SSRF."

2. 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).

[SashaMIT]

Thanks @kuangmi-bit, both points addressed in 9747ab2:

1. Redirects: the constructor now rejects an httpx.AsyncClient with follow_redirects=True outright, with a ValueError explaining why. That turns the redirect-SSRF misconfiguration into a fail-fast at startup instead of a doc note people may miss. The residual-risk note is also in the httpx_client docstring.
2. DNS rebinding TOCTOU: documented as a known limitation in push_url_validation_error (validation and connection resolve separately; pinning the validated IP belongs in the transport layer), framed as defense-in-depth with network-level egress controls as the operator mitigation.

Also added the IPv4-mapped IPv6 note to the docstring as suggested, and a test asserting the constructor guard raises. Full suite green (1354 passed).

[kuangmi-bit]

Both points from my earlier review are addressed in 9747ab2 (fail-fast on follow_redirects=True, plus the second item). The SSRF hardening looks solid — approving. (cc: the duplicate #1169 covers the same ground; consider closing one.)

[ez-lbz]

Hi @SashaMIT,

Re: the overlap discussion on #1169 — a few points from our side:

1. #1169 is part of a batch of findings from a test framework we are building to exercise the A2A protocol across the SDKs; the whole batch was filed on the same day.
2. Between two PRs covering the same fix, the deciding factor should be the check results, not the filing order. As of now, #1164 has not passed all of its required checks — and per project rules, a PR that has not passed all checks cannot be accepted.
3. On that basis, review should treat #1169 as the primary candidate; if #1164 is to be considered at all, its failing checks need to be resolved first.

Happy to defer to the maintainers either way.

[sokoliva]

Thank you for this PR. Could you please resolve Lint and Check Spelling issues?

[SashaMIT]

Thanks. Lint and Check Spelling should be clear now (type-check on the resolved address, and the unrecognized words are gone from the docstring).

[ez-lbz]

Following up on my comment above: confirmed — Lint Code Base and Check Spelling now pass on the current head, so that objection is resolved.

The one remaining red, ITK, looks transient rather than code-related: the run logs show Error response from daemon: No such container: itk-service (the star-topology peers — go_v03/go_v10/python_v03/python_v10 — never came up), and ITK failed for two unrelated PRs at the same minute (17:55) while runs for other branches before and after that window succeed. A rerun should clear it.

With both #1164 and #1169 green, the overlap question is squarely the maintainers' call — happy to defer either way, as said.

[sokoliva]

Thank you for this PR! I left a few comments.

[SashaMIT]

Thanks Iva. I moved the validator to src/a2a/utils/push_url_validator.py, defaulted the sender hook to None like #1173 (the docstring points at push_url_validation_error), and dropped the follow_redirects constructor raise. This branch is now on current main so it sits on top of #1173.

[sokoliva]

Left a couple more NITs, please fix and then re-request review. :)

[SashaMIT]

Thanks Iva. I dropped the sender re-export so push_url_validation_error is imported from a2a.utils.push_url_validator only, and wrapped urlparse plus parsed.port so a bad port returns unparseable instead of raising. Re-requesting review.

[mykytanetipa]

thank you for the contribution, couple comments from my side

[SashaMIT]

Thanks Iva, and thanks Mykyta.

I dropped the sender __all__ so BasePushNotificationSender is exported only from a2a.server.tasks. The helper is now validate_push_notification_url and returns True when the URL is safe. Rejects log with logger.warning in the validator and no longer travel as raw strings. The handler hook is Callable[[str], Awaitable[bool]]. Restored the unrelated blank line in the in-memory dispatch test.

[SashaMIT]

Thanks Iva. Glad the dispatch-time check landed next to #1173. Appreciate the reviews from you and Mykyta.