fix(server): validate push-notification URLs before dispatch (SSRF hardening) by SashaMIT · Pull Request #1164 · a2aproject/a2a-python

SashaMIT · 2026-08-05T21:55:11Z

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

…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>

github-actions · 2026-08-05T21:58:00Z

🧪 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.45%	🟢 +14.03%
src/a2a/utils/push_url_validator.py (new)	—	87.80%	—
Total	92.97%	93.06%	🟢 +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):

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

@kuangmi-bit

…SSRF risks Per review from @kuangmi-bit: - Constructor now rejects an httpx.AsyncClient configured with follow_redirects=True. URL validation covers the initial URL only; with redirects enabled a validated public URL could 30x to an internal address and be dispatched unchecked. Failing fast at construction turns that misconfiguration into an explicit error. - push_url_validation_error docstring now documents the two residual risks: redirect targets are not re-validated (mitigated by the new guard) and DNS rebinding TOCTOU between validation and connection (documented as defense-in-depth; operators should keep network-level egress controls). - Notes that IPv4-mapped IPv6 forms are covered via ipaddress mapping. - Tests: setUp mocks pin follow_redirects=False explicitly; new test asserts the constructor guard raises on a redirect-following client. Full suite green: 1354 passed, 90 skipped, 3 xfailed. Signed-off-by: SashaMIT <sash.t.mitchell@gmail.com>

SashaMIT · 2026-08-06T17:45:50Z

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

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.
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 · 2026-08-09T11:56:16Z

Hi @SashaMIT,

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

#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.
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.
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.

Upstream's own push-notification e2e tests register loopback webhooks with real local receivers, which creation-time validation rejects by design. Add allow_private_push_urls (default False) to both request handlers and opt the test harness in, matching the dispatch-path sibling's shape (a2aproject#1164). Also: str() the getaddrinfo sockaddr host for ty, ruff-format the v1 handler test.

sokoliva

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

SashaMIT · 2026-08-27T17:55:30Z

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 · 2026-08-28T06:58:46Z

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.

## Summary The `on_create_task_push_notification_config` handlers (v1 and v2) stored client-supplied URLs without any validation. A caller who can create a push notification config can point the server at loopback, private-network, link-local, or cloud-metadata hosts, and the server will POST task events to that URL on every state change. ## Root cause `src/a2a/server/request_handlers/default_request_handler.py` and `default_request_handler_v2.py` call `push_config_store.set_info(task_id, params, context)` without checking `params.url`. The dispatch path (`BasePushNotificationSender._dispatch_notification`) is covered by #1164; this PR closes the **write path**. ## Fix - Added `push_url_validation_error` to `base_push_notification_sender.py` (same logic as #1164: blocks non-http(s) schemes, loopback, private, link-local, multicast, reserved, and unresolvable hosts). - Called it in both `on_create_task_push_notification_config` handlers before storing the config. - Raises `InvalidParamsError` with a descriptive message on rejection. ## Testing - `uv run pytest tests/server/request_handlers/ -k push_notification` — 55 passed. - New test: `test_on_create_task_push_notification_config_rejects_invalid_url` covers loopback and `file:` scheme rejection. - Updated existing tests that used unresolvable fixture URLs (`1.example.com`, `callback.com`) to use `example.com` (resolvable in CI). ## Relation to #1164 #1164 validates at dispatch time (read path). This PR validates at config creation time (write path). Both are needed: write-time validation fails fast and gives the client immediate feedback; dispatch-time validation is a defense-in-depth backstop. Made with [Cursor](https://cursor.com) --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Iva Sokolaj <102302011+sokoliva@users.noreply.github.com>

sokoliva

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

…dation Signed-off-by: SashaMIT <sash@ela.city> # Conflicts: # src/a2a/server/tasks/base_push_notification_sender.py

SashaMIT · 2026-08-31T09:29:32Z

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 · 2026-09-01T09:11:22Z

+__all__ = [
+    'BasePushNotificationSender',
+    'push_url_validation_error',
+]


Please remove. Since we moved push_url_validation_error to utils, let's also import from
from a2a.utils.push_url_validator

Alternatively, You can add push_url_validation_error to a2a/utils/__init__.py.

sokoliva · 2026-09-01T09:32:13Z

+    host = parsed.hostname
+    if not host:
+        return 'no hostname'
+    port = parsed.port or (443 if parsed.scheme == 'https' else 80)


It seems port also raises ValueError: port

Let's also put it in a try/except block to catch those errors, something like:

try: parsed = urllib.parse.urlparse(url) explicit_port = parsed.port except ValueError: return 'unparseable URL' (.... other code ...) port = explicit_port or (443 if parsed.scheme == 'https' else 80)

sokoliva · 2026-09-01T13:02:31Z

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

Drop the sender re-export so push_url_validation_error is imported from utils only.

SashaMIT · 2026-09-02T11:30:59Z

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.

sokoliva · 2026-09-02T12:22:15Z

+__all__ = [
+    'BasePushNotificationSender',
+]


Please remove this too. We export it here so there is no need to export it also in this file.

mykytanetipa

thank you for the contribution, couple comments from my side

mykytanetipa · 2026-09-02T12:32:16Z

+    )
+
+
+async def push_url_validation_error(url: str) -> str | None:


can we rename this to smth like validate_push_notification_url? the "error" prefix kinda distorts the purpose of the function which is validation.

mykytanetipa · 2026-09-02T12:34:00Z

+    )
+
+
+async def push_url_validation_error(url: str) -> str | None:


since the purpose is validation I would change the return type to bool instead of "str | None", it's uncommon to use None for "safe" outcome

mykytanetipa · 2026-09-02T12:37:10Z

+        parsed = urllib.parse.urlparse(url)
+        explicit_port = parsed.port
+    except ValueError:
+        return 'unparseable URL'


please avoid propagation of errors as raw strings, if the intent is logging only please do it inplace with logger.warning

mykytanetipa · 2026-09-02T12:37:44Z

+    except ValueError:
+        return 'unparseable URL'
+    if parsed.scheme not in ('http', 'https'):
+        return f"scheme '{parsed.scheme}' is not http/https"


use logger.warning

mykytanetipa · 2026-09-02T12:37:51Z

+        return f"scheme '{parsed.scheme}' is not http/https"
+    host = parsed.hostname
+    if not host:
+        return 'no hostname'


use logger.warning

mykytanetipa · 2026-09-02T12:37:57Z

+        loop = asyncio.get_running_loop()
+        infos = await loop.getaddrinfo(host, port, type=socket.SOCK_STREAM)
+    except OSError:
+        return f"host '{host}' could not be resolved"


use logger.warning

mykytanetipa · 2026-09-02T12:38:03Z

+        return f"host '{host}' could not be resolved"
+    for info in infos:
+        if _ip_is_blocked(str(info[4][0])):
+            return f"host '{host}' resolves to a non-public address"


use logger.warning

mykytanetipa · 2026-09-02T12:39:48Z

        self.mock_httpx_client.post.return_value = mock_response

        self.config_store = InMemoryPushNotificationConfigStore()
-


minor: unrelated edit

Rename to validate_push_notification_url. Drop the sender __all__ re-export.

SashaMIT · 2026-09-02T19:01:58Z

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 requested a review from a team as a code owner August 5, 2026 21:55

kuangmi-bit reviewed Aug 6, 2026

View reviewed changes

kuangmi-bit approved these changes Aug 7, 2026

View reviewed changes

kuangmi-bit mentioned this pull request Aug 9, 2026

fix: validate push notification URLs before dispatch #1169

Open

SashaMIT mentioned this pull request Aug 9, 2026

fix(server): validate push-notification URLs at config creation #1173

Merged

sokoliva reviewed Aug 27, 2026

View reviewed changes

fix: clear spelling and type-check failures in URL validation

abbb878

sokoliva reviewed Aug 31, 2026

View reviewed changes

Comment thread src/a2a/server/tasks/base_push_notification_sender.py Outdated

sokoliva reviewed Aug 31, 2026

View reviewed changes

Comment thread src/a2a/server/tasks/base_push_notification_sender.py Outdated

sokoliva reviewed Aug 31, 2026

View reviewed changes

Comment thread src/a2a/server/tasks/base_push_notification_sender.py Outdated

sokoliva requested changes Aug 31, 2026

View reviewed changes

Merge branch 'upstream-main' into fix/push-notification-url-ssrf-vali…

cd55b1f

…dation Signed-off-by: SashaMIT <sash@ela.city> # Conflicts: # src/a2a/server/tasks/base_push_notification_sender.py

sokoliva reviewed Sep 1, 2026

View reviewed changes

sokoliva requested changes Sep 1, 2026

View reviewed changes

fix: treat invalid push URL ports as unparseable

3d67ca3

Drop the sender re-export so push_url_validation_error is imported from utils only.

sokoliva requested changes Sep 2, 2026

View reviewed changes

mykytanetipa reviewed Sep 2, 2026

View reviewed changes

SashaMIT added 2 commits September 2, 2026 20:01

fix: return bool from push URL validator and log rejects in place

d6e1415

Rename to validate_push_notification_url. Drop the sender __all__ re-export.

fix: flatten push URL validator guard for ruff SIM102

37f6a82

		)


		async def push_url_validation_error(url: str) -> str \| None:

		self.mock_httpx_client.post.return_value = mock_response

		self.config_store = InMemoryPushNotificationConfigStore()

Uh oh!

Conversation

SashaMIT commented Aug 5, 2026

Summary

Fix

Test plan

Uh oh!

github-actions Bot commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🧪 Code Coverage (vs main)

Uh oh!

kuangmi-bit left a comment

Choose a reason for hiding this comment

Uh oh!

SashaMIT commented Aug 6, 2026

Uh oh!

kuangmi-bit left a comment

Choose a reason for hiding this comment

Uh oh!

ez-lbz commented Aug 9, 2026

Uh oh!

sokoliva left a comment

Choose a reason for hiding this comment

Uh oh!

SashaMIT commented Aug 27, 2026

Uh oh!

ez-lbz commented Aug 28, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sokoliva left a comment

Choose a reason for hiding this comment

Uh oh!

SashaMIT commented Aug 31, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sokoliva Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sokoliva commented Sep 1, 2026

Uh oh!

SashaMIT commented Sep 2, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mykytanetipa left a comment

Choose a reason for hiding this comment

Uh oh!

mykytanetipa Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mykytanetipa Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SashaMIT commented Sep 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

github-actions Bot commented Aug 5, 2026 •

edited

Loading

🧪 Code Coverage (vs `main`)

sokoliva Sep 1, 2026 •

edited

Loading

mykytanetipa Sep 2, 2026 •

edited

Loading

mykytanetipa Sep 2, 2026 •

edited

Loading