fix(server): validate push-notification URLs at config creation - #1173
fix(server): validate push-notification URLs at config creation#1173SashaMIT wants to merge 5 commits into
Conversation
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>
- 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.
🧪 Code Coverage (vs
|
| 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
|
Thanks. The library default is now an injectable async |
| push_url_validator: Callable[[str], Awaitable[str | None]] | ||
| | None = push_url_validation_error, |
There was a problem hiding this comment.
Let's actually default to None here to avoid making a breaking change. Also this would make more sense considering the spec states should.
This also fixes the ITK error:
ERROR - Test PR - star - push notification - jsonrpc failed with exception: JSON-RPC Error: {'code': -32602, 'message': "Invalid push notification URL: host '127.0.0.1' resolves to a non-public address", 'data': [{'@type': 'type.googleapis.com/google.rpc.ErrorInfo', 'reason': 'INVALID_PARAMS', 'domain': 'a2a-protocol.org', 'metadata': {}}]}
There was a problem hiding this comment.
Done. Default is now None so this is not a breaking change and the ITK loopback webhook keeps working. Pass push_url_validation_error to opt in.
| push_url_validator: Callable[[str], Awaitable[str | None]] | ||
| | None = push_url_validation_error, |
There was a problem hiding this comment.
Same as above: default to None.
There was a problem hiding this comment.
Same change here. Default is None.
|
Thanks, no inconvenience. Default is now None on both handlers so existing ITK loopback webhooks keep working. Pass |
2 participants
Summary
The
on_create_task_push_notification_confighandlers (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.pyanddefault_request_handler_v2.pycallpush_config_store.set_info(task_id, params, context)without checkingparams.url. The dispatch path (BasePushNotificationSender._dispatch_notification) is covered by #1164; this PR closes the write path.Fix
push_url_validation_errortobase_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).on_create_task_push_notification_confighandlers before storing the config.InvalidParamsErrorwith a descriptive message on rejection.Testing
uv run pytest tests/server/request_handlers/ -k push_notification— 55 passed.test_on_create_task_push_notification_config_rejects_invalid_urlcovers loopback andfile:scheme rejection.1.example.com,callback.com) to useexample.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