Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 37 additions & 6 deletions a2a_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -617,14 +617,47 @@ def _parse_push_config(configuration: dict) -> PushNotificationConfig | None:
if not _is_safe_webhook_url(url):
logger.warning("[a2a] rejected unsafe webhook url: %s", url)
return None
auth = cfg.get("authentication") or {}
return PushNotificationConfig(
url=url,
token=auth.get("credentials"),
token=_extract_push_token(cfg),
id=cfg.get("id", str(uuid4())),
)


def _extract_push_token(cfg: dict) -> str | None:
"""Pull the bearer token from a PushNotificationConfig.

The A2A spec's PushNotificationConfig allows two shapes for the
authentication secret, and clients choose between them at will:

1. **Top-level ``token``** — the simple form every SDK produces by
default. ``@a2a-js/sdk`` serialises ``{url, token}`` directly
and Workstacean's ``SkillDispatcherPlugin`` takes this path
(``a2a-executor.ts:329`` → ``setTaskPushNotificationConfig({
pushNotificationConfig: { url, token } })``).
2. **Structured ``authentication.credentials``** — the RFC-8821
``AuthenticationInfo`` form with ``schemes`` + ``credentials``.

Before this helper Quinn only read (2), so Workstacean's top-level
token fell on the floor → Quinn sent callbacks with no
``Authorization`` header → Workstacean returned 401 "Invalid
notification token" → TaskTracker fell back to polling every
dispatch (quinn#61 follow-up).

Preference: top-level ``token`` first (what most callers send), fall
back to ``authentication.credentials``. Either one null-safely
resolves to ``None`` which disables the header entirely — the same
behaviour as the SSRF guard letting a public-IP callback through
unauthenticated.
"""
top_level = cfg.get("token")
if isinstance(top_level, str) and top_level:
return top_level
auth = cfg.get("authentication") or {}
creds = auth.get("credentials")
return creds if isinstance(creds, str) and creds else None
Comment on lines +653 to +658
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Handle non-dict authentication to avoid 500s on malformed config.

Line 656 assumes authentication is a dict. If a client sends a truthy non-object value, auth.get(...) raises AttributeError and the request fails with 500 instead of safely resolving to None.

🔧 Proposed fix
 def _extract_push_token(cfg: dict) -> str | None:
@@
-    auth = cfg.get("authentication") or {}
-    creds = auth.get("credentials")
+    auth = cfg.get("authentication")
+    if not isinstance(auth, dict):
+        return None
+    creds = auth.get("credentials")
     return creds if isinstance(creds, str) and creds else None
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@a2a_handler.py` around lines 653 - 658, The code assumes
cfg.get("authentication") returns a dict and calls auth.get(...), which throws
AttributeError for truthy non-dict values; change the auth handling to validate
its type before using .get (e.g., retrieve auth = cfg.get("authentication"); if
not isinstance(auth, dict): auth = {}), then read creds =
auth.get("credentials") and return creds only if it's a non-empty string (as
with the existing creds check). This targets the auth variable and creds
extraction in the shown function to avoid 500s on malformed config.



# ── Webhook delivery ──────────────────────────────────────────────────────────


Expand Down Expand Up @@ -1199,10 +1232,9 @@ def _rpc_result(result):
"webhook url rejected: must be http/https, public IP, "
"not loopback/private/link-local/multicast/reserved",
)
auth = cfg_in.get("authentication") or {}
cfg = PushNotificationConfig(
url=url,
token=auth.get("credentials"),
token=_extract_push_token(cfg_in),
id=cfg_in.get("id", str(uuid4())),
)
async with _store._lock:
Expand Down Expand Up @@ -1407,10 +1439,9 @@ async def _create_push_config(task_id: str, request: Request, body: dict):
"not loopback/private/link-local/multicast/reserved",
)

auth = body.get("authentication") or {}
cfg = PushNotificationConfig(
url=url,
token=auth.get("credentials"),
token=_extract_push_token(body),
id=body.get("id", str(uuid4())),
)

Expand Down
41 changes: 41 additions & 0 deletions tests/test_a2a_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,47 @@ def test_parse_push_config_rejects_unsafe_url():
assert cfg is None


def test_extract_push_token_top_level():
"""A2A spec PushNotificationConfig accepts two token shapes:
top-level `token` (what @a2a-js/sdk serialises by default and
Workstacean's SkillDispatcherPlugin uses) OR structured
`authentication.credentials`. Before quinn only read the latter —
Workstacean's top-level token landed in `cfg.token` as None,
Quinn's `_deliver_webhook` skipped the Authorization header, and
Workstacean's callback endpoint returned 401 on every delivery.
quinn#61 follow-up."""
from a2a_handler import _extract_push_token
assert _extract_push_token({"token": "ws-token-xyz"}) == "ws-token-xyz"


def test_extract_push_token_structured_authentication():
"""The RFC-8821 AuthenticationInfo shape still works."""
from a2a_handler import _extract_push_token
assert _extract_push_token({
"authentication": {"schemes": ["Bearer"], "credentials": "rfc-token"},
}) == "rfc-token"


def test_extract_push_token_prefers_top_level_when_both_present():
"""If a caller sends both (unlikely but spec-permitted), top-level
wins — it's the common case and structured is the fallback."""
from a2a_handler import _extract_push_token
assert _extract_push_token({
"token": "preferred",
"authentication": {"credentials": "fallback"},
}) == "preferred"


def test_extract_push_token_none_when_absent():
"""Neither shape present → None. Delivery still fires, but without
an Authorization header (for public or unauthenticated callbacks)."""
from a2a_handler import _extract_push_token
assert _extract_push_token({}) is None
assert _extract_push_token({"token": ""}) is None
assert _extract_push_token({"authentication": {}}) is None
assert _extract_push_token({"token": None}) is None


# ── SSRF allowlist (trusted docker-network hosts) ────────────────────────────


Expand Down
Loading