Skip to content

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

Merged
sokoliva merged 6 commits into
a2aproject:mainfrom
SashaMIT:fix/push-notification-config-url-validation
Aug 31, 2026
Merged

fix(server): validate push-notification URLs at config creation#1173
sokoliva merged 6 commits into
a2aproject:mainfrom
SashaMIT:fix/push-notification-config-url-validation

Conversation

@SashaMIT

@SashaMIT SashaMIT commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

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 fix(server): validate push-notification URLs before dispatch (SSRF hardening) #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

The on_create_task_push_notification_config handlers (v1 and v2) stored
client-supplied URLs without validation. A malicious client could register
a push notification config pointing at loopback, private-network, or
cloud-metadata hosts, and the server would POST task events to that URL
on every state change.

Add push_url_validation_error checks to both create handlers, rejecting
non-http(s) schemes and hosts that resolve to non-public addresses.
This complements a2aproject#1164 (dispatch-time validation) by closing the write
path.

Test: test_on_create_task_push_notification_config_rejects_invalid_url
Co-authored-by: Cursor <cursoragent@cursor.com>
@SashaMIT
SashaMIT requested a review from a team as a code owner August 9, 2026 14:37
- default_request_handler_v2: restore a2a.server.tasks imports under
  TYPE_CHECKING (patch dedented them, tripping TC001) and sort import block
- base_push_notification_sender: catch OSError instead of naming
  socket.gaierror (check-spelling rejects the token)
- cross-version client_1_0 lifecycle test uses example.com placeholder:
  creation now validates resolvability/public reachability, and the CRUD
  lifecycle never dials the URL
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.
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown

🧪 Code Coverage (vs main)

⬇️ Download Full Report

Base PR Delta
src/a2a/server/request_handlers/default_request_handler.py 98.13% 97.90% 🔴 -0.23%
src/a2a/server/request_handlers/default_request_handler_v2.py 94.25% 92.05% 🔴 -2.20%
src/a2a/server/tasks/base_push_notification_sender.py 94.44% 81.43% 🔴 -13.02%
Total 93.12% 92.96% 🔴 -0.16%

Generated by coverage-comment.yml

Comment thread src/a2a/server/request_handlers/default_request_handler.py Outdated
Comment thread src/a2a/server/request_handlers/default_request_handler_v2.py Outdated
Comment thread src/a2a/server/request_handlers/default_request_handler_v2.py
Comment thread src/a2a/server/request_handlers/default_request_handler.py
Comment thread src/a2a/server/tasks/base_push_notification_sender.py Outdated
@SashaMIT

Copy link
Copy Markdown
Contributor Author

Thanks. The library default is now an injectable async push_url_validator (default push_url_validation_error, None skips). Validation runs before both set_info sites, including the send-message path. Resolver uses the running loop's getaddrinfo.

Comment thread src/a2a/server/request_handlers/default_request_handler.py Outdated
Comment thread src/a2a/server/request_handlers/default_request_handler_v2.py Outdated

@sokoliva sokoliva left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for making the changes. I left 2 equivalent comments. Sorry for the inconvenience and thank you for the patience!

@SashaMIT

Copy link
Copy Markdown
Contributor Author

Thanks, no inconvenience. Default is now None on both handlers so existing ITK loopback webhooks keep working. Pass push_url_validation_error to opt in.

@sokoliva
sokoliva merged commit 3eb88e2 into a2aproject:main Aug 31, 2026
17 checks passed
@SashaMIT

Copy link
Copy Markdown
Contributor Author

Thanks Iva, glad this landed. The None default keeps existing ITK loopback webhooks working.

mykytanetipa pushed a commit that referenced this pull request Sep 8, 2026
🤖 I have created a release *beep* *boop*
---


##
[1.1.4](v1.1.3...v1.1.4)
(2026-09-07)


### Features

* **itk:** register itk-python-v10-agent as a uv workspace member and
update dependency version markers
([#1203](#1203))
([6eee895](6eee895))
* **itk:** use shared scenarios
([#1201](#1201))
([b4a0b21](b4a0b21))


### Bug Fixes

* make event queue sink removal idempotent
([#1134](#1134))
([58c72c6](58c72c6))
* omit artifacts from list tasks responses
([#1212](#1212))
([35ef52e](35ef52e))
* owner-scope cancel/subscribe and write terminal state on cancel
([#1159](#1159),
[#1170](#1170))
([#1172](#1172))
([ddbf853](ddbf853))
* prevent first-owner write loss in in-memory stores
([#1194](#1194))
([bcc489c](bcc489c))
* **server:** let subscriber taps evict on full instead of wedging
dispatch ([#1137](#1137))
([0c2126f](0c2126f))
* **server:** surface producer errors after failed tasks
([#1229](#1229))
([bc32d7e](bc32d7e))
* **server:** validate push-notification URLs at config creation
([#1173](#1173))
([3eb88e2](3eb88e2))
* **server:** validate push-notification URLs before dispatch (SSRF
hardening)
([#1164](#1164))
([57a9df3](57a9df3))
* **server:** warn when queue_manager is ignored in
DefaultRequestHandlerV2
([#1153](#1153))
([08fd223](08fd223))


### Documentation

* **server:** say what the evict-on-full check actually tests
([#1209](#1209))
([4b7b242](4b7b242))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants