-
-
Notifications
You must be signed in to change notification settings - Fork 3
tsk-b5fz5t [OPEN] A2A: recipient field + role resolution at delivery #228
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -101,11 +101,17 @@ | |
| ``GET /shelves?project=`` -> ``{"shelves": [...]}`` | ||
| ``GET /pending?agent=`` -> ``{"pending": [...]}`` | ||
| ``POST /pending/resolve`` ``{"id", "decision", "note"?}`` -> resolve result | ||
| ``POST /a2a/send`` ``{"from", "body", "thread"?, "reply_to"?, "refs"?, "blocks"?}`` -> send receipt | ||
| ``POST /a2a/send`` ``{"from", "body", "thread"?, "reply_to"?, "refs"?, "blocks"?, "recipient"?}`` -> send receipt | ||
| ``recipient``: optional addressee handle (agent ``@handle`` or role | ||
| ``@taOS-<name>``, e.g. ``@taOS-PA``); stored verbatim. A role recipient | ||
| is validated at send time against the configured resolver | ||
| (``a2a_role_resolver_url``): 400 if not resolvable to a single holder, | ||
| 503 if unreachable, 400 if no resolver configured. The resolved | ||
| identity is never stored -- binding is at delivery (taOS#2155). | ||
| ``refs``: optional list (<=8) of ``{"kind": doc|report|spec|log, "title", "uri", "sha256"?, "doc_id"?, "version"?, "for"?, "summary"?}`` | ||
| ``blocks``: optional list of arbitrary objects (no schema validation); when present, ``body`` must be non-empty | ||
| ``GET /a2a/messages`` ``?thread=&since=&limit=&fields=&format=`` -> ``{"messages": [...]}`` (``fields=id,sender,body`` projects keys; ``format=ndjson`` emits one message per line) | ||
| ``GET /a2a/stream`` ``?thread=&since=`` -> SSE stream (text/event-stream) | ||
| ``GET /a2a/messages`` ``?thread=&since=&limit=&recipient=&fields=&format=`` -> ``{"messages": [...]}`` (``fields=id,sender,body`` projects keys; ``format=ndjson`` emits one message per line; ``recipient=`` filters on the verbatim stored handle and, for a role ``@taOS-*``, annotates ``resolved_to`` with the current holder computed at read time) | ||
| ``GET /a2a/stream`` ``?thread=&since=`` -> SSE stream (text/event-stream; messages carry the verbatim ``recipient`` field when present, no ``resolved_to`` -- resolution is per-reader) | ||
| ``GET /a2a/channels`` -> ``{"channels": [...]}`` | ||
| ``GET /a2a/members`` ``?channel=<name>`` -> ``{"members": [...]}`` | ||
| ``POST /tasks`` ``{"title", "body"?, "project"?, "assignee"?, "priority"?, "depends_on"?: [...], "created_by"}`` -> task object | ||
|
|
@@ -174,6 +180,7 @@ | |
| from urllib.parse import parse_qs, urlsplit | ||
|
|
||
| from . import __version__, capabilities, config as _config, service | ||
| from .role_resolver import RoleResolver, RoleResolveError, is_role_handle | ||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # Static webui helpers | ||
|
|
@@ -578,7 +585,7 @@ def close(self) -> None: | |
|
|
||
|
|
||
| def _make_handler(data_dir, runner: _ServiceLoop, verifier=None, | ||
| grants_verifier=None): | ||
| grants_verifier=None, role_resolver=None): | ||
| """Build a handler class bound to a fixed ``data_dir``. | ||
|
|
||
| ThreadingHTTPServer instantiates the handler per request, so the data dir | ||
|
|
@@ -590,6 +597,13 @@ def _make_handler(data_dir, runner: _ServiceLoop, verifier=None, | |
| otherwise. ``GET /health``, ``GET /version``, ``GET /``, ``GET /ui``, and | ||
| static assets are always open so monitoring probes, capability/drift probes, | ||
| and the inspection UI keep working. | ||
|
|
||
| ``role_resolver`` is an injected seam (taOSmd #2155): an object exposing | ||
| ``resolve(role) -> canonical_id | None`` used to validate role recipients | ||
| at send time and annotate the current holder at delivery time. When None | ||
| it is built from the configured ``a2a_role_resolver_url``; when neither is | ||
| set, role recipients are rejected (400/503) as specified, and plain agent | ||
| handles pass through untouched. | ||
| """ | ||
| # Read the server-side expected token once at handler-class creation time. | ||
| # This is the token the *server* checks (not the client's outbound token). | ||
|
|
@@ -623,9 +637,19 @@ def _make_handler(data_dir, runner: _ServiceLoop, verifier=None, | |
| expected_iss=registry_auth.REGISTRY_ISS, | ||
| ) | ||
| _grants_verifier = registry_auth.grants_verifier_from_url( | ||
| _registry_url, | ||
| grants_token=_registry_admin_token, | ||
| ) | ||
| _registry_url, | ||
| grants_token=_registry_admin_token, | ||
| ) | ||
|
|
||
| # Role resolver (taOSmd #2155). Injected for tests; otherwise built from the | ||
| # configured resolver URL. When None, role recipients are rejected (the bus | ||
| # owns the message-side concepts, not the binding; a role cannot be | ||
| # validated or delivered without a resolver). | ||
| _role_resolver = role_resolver | ||
| if _role_resolver is None: | ||
| _role_resolver_url = _config.get_a2a_role_resolver_url(data_dir) | ||
| if _role_resolver_url: | ||
| _role_resolver = RoleResolver(_role_resolver_url) | ||
|
|
||
| # Paths that are always public regardless of the token setting. | ||
| _PUBLIC_PATHS = frozenset({"/", "/ui", "/health", "/version"}) | ||
|
|
@@ -1396,6 +1420,7 @@ def _handle_a2a_send(self) -> None: | |
| reply_to = body.get("reply_to") | ||
| refs = body.get("refs") | ||
| blocks = body.get("blocks") | ||
| recipient = body.get("recipient") | ||
| if not isinstance(from_, str) or not from_: | ||
| raise _BadRequest("'from' (non-empty string) is required") | ||
| # --- Envelope field validation (taOSmd #211) --- | ||
|
|
@@ -1440,6 +1465,33 @@ def _handle_a2a_send(self) -> None: | |
| raise _BadRequest( | ||
| "'body' (non-empty string) is required when 'blocks' is present" | ||
| ) | ||
| # --- Recipient validation (taOSmd #2155) --- | ||
| # recipient: optional addressee handle (agent @handle or role | ||
| # @taOS-<name>). Stored VERBATIM. A role recipient is validated at | ||
| # send time against the configured resolver -- the binding happens at | ||
| # delivery, so the resolved identity is never persisted and holder | ||
| # rotation needs no sender reconfiguration. | ||
| if recipient is not None: | ||
| if not isinstance(recipient, str) or not recipient.strip(): | ||
| raise _BadRequest("'recipient' must be a non-empty string when provided") | ||
| recipient = recipient.strip() | ||
| if is_role_handle(recipient): | ||
| if _role_resolver is None: | ||
| raise _BadRequest( | ||
| f"role recipient {recipient!r} requires a configured " | ||
| "role resolver (a2a_role_resolver_url)" | ||
| ) | ||
| try: | ||
| holder = _role_resolver.resolve(recipient) | ||
| except RoleResolveError as exc: | ||
| # Configured but unreachable: 503, never a silent drop. | ||
| self._send_json(503, {"error": f"role resolver unavailable: {exc}"}) | ||
| return | ||
|
Comment on lines
+1486
to
+1489
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1. /a2a/send resolver errors unarchived The new role-recipient validation path returns a 503 on RoleResolveError before calling service.a2a_send(), so no archive.record() occurs for that failed interaction. This violates the requirement to archive every interaction (including failures) via a centralized logger call. Agent Prompt
|
||
| # A None result means no single holder -> reject, nothing stored. | ||
| if holder is None: | ||
| raise _BadRequest( | ||
| f"role recipient {recipient!r} does not resolve to a single holder" | ||
| ) | ||
|
Comment on lines
+1468
to
+1494
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win Run the registry auth checks before the role resolution. This block resolves the role at line 1485. The registry identity and grant checks start at line 1501. An unauthenticated caller therefore reaches the resolver first. Two effects follow when
Move the recipient shape validation before the auth block if you want a cheap 400, and move the resolver call after the auth block. 🛡️ Proposed reordering if recipient is not None:
if not isinstance(recipient, str) or not recipient.strip():
raise _BadRequest("'recipient' must be a non-empty string when provided")
recipient = recipient.strip()
- if is_role_handle(recipient):
- if _role_resolver is None:
- raise _BadRequest(
- f"role recipient {recipient!r} requires a configured "
- "role resolver (a2a_role_resolver_url)"
- )
- try:
- holder = _role_resolver.resolve(recipient)
- except RoleResolveError as exc:
- # Configured but unreachable: 503, never a silent drop.
- self._send_json(503, {"error": f"role resolver unavailable: {exc}"})
- return
- # A None result means no single holder -> reject, nothing stored.
- if holder is None:
- raise _BadRequest(
- f"role recipient {recipient!r} does not resolve to a single holder"
- )
# Registry auth (opt-in): ...Then place the role-resolution block immediately after the registry auth block and before the 🤖 Prompt for AI Agents |
||
| # Registry auth (opt-in): when a verifier is configured, run the | ||
| # identity + grant checks and collect any failure reason. | ||
| # In enforce mode (a2a_auth_enforce=true) failures are rejected with | ||
|
|
@@ -1495,7 +1547,7 @@ def _handle_a2a_send(self) -> None: | |
| service.a2a_send( | ||
| sender=from_, body=body_text, | ||
| thread=thread, reply_to=reply_to, | ||
| refs=refs, blocks=blocks, | ||
| refs=refs, blocks=blocks, recipient=recipient, | ||
| data_dir=data_dir, | ||
| ) | ||
| ) | ||
|
|
@@ -1507,6 +1559,7 @@ def _handle_a2a_messages(self, qs: dict) -> None: | |
| limit_raw = (qs.get("limit") or [50])[0] | ||
| fields_raw = (qs.get("fields") or [None])[0] | ||
| fmt = (qs.get("format") or ["json"])[0] | ||
| recipient = (qs.get("recipient") or [None])[0] | ||
| try: | ||
| since = float(since_raw) if since_raw is not None else None | ||
| except (TypeError, ValueError) as exc: | ||
|
|
@@ -1517,9 +1570,42 @@ def _handle_a2a_messages(self, qs: dict) -> None: | |
| raise _BadRequest("'limit' must be an integer") from exc | ||
| if fmt not in ("json", "ndjson"): | ||
| raise _BadRequest("'format' must be 'json' or 'ndjson'") | ||
| # --- Delivery-time role resolution (taOSmd #2155) --- | ||
| # A role recipient (e.g. ?recipient=@taOS-PA) is matched against the | ||
| # verbatim stored handle by service.a2a_feed, then annotated with the | ||
| # CURRENT holder (resolved_to) computed here at read time. The stored | ||
| # envelope never carries resolved_to -- resolution is per-reader and | ||
| # per-read, so rotating the holder changes the answer without a | ||
| # re-send. Fail-loud: no resolver -> 400; unreachable -> 503; a | ||
| # reachable resolver that finds no holder is not an error (the role | ||
| # simply has no current holder), so resolved_to is omitted in that | ||
| # case. SSE does NOT annotate: role-addressed messages ride verbatim | ||
| # (recipient present, no resolved_to on the broadcast). | ||
| holder: str | None = None | ||
| annotate = False | ||
| if recipient is not None: | ||
| if not isinstance(recipient, str) or not recipient.strip(): | ||
| raise _BadRequest("'recipient' must be a non-empty string when provided") | ||
| recipient = recipient.strip() | ||
| if is_role_handle(recipient): | ||
| annotate = True | ||
| if _role_resolver is None: | ||
| self._send_json(400, {"error": f"role recipient {recipient!r} requires a configured role resolver (a2a_role_resolver_url)"}) | ||
| return | ||
| try: | ||
| holder = _role_resolver.resolve(recipient) | ||
| except RoleResolveError as exc: | ||
| self._send_json(503, {"error": f"role resolver unavailable: {exc}"}) | ||
| return | ||
| messages = runner.run( | ||
| service.a2a_feed(thread=thread, since=since, limit=limit_i, data_dir=data_dir) | ||
| service.a2a_feed( | ||
| thread=thread, since=since, limit=limit_i, | ||
| recipient=recipient, data_dir=data_dir, | ||
| ) | ||
| ) | ||
| if annotate and holder is not None: | ||
| for msg in messages: | ||
| msg["resolved_to"] = holder | ||
| # Compact mode: ?fields=id,sender,body projects each message down | ||
| # to the named keys so token-frugal consumers (LLM agents) skip | ||
| # framing they never read. Unknown names are ignored, never a 400, | ||
|
|
@@ -2062,7 +2148,7 @@ def _handle_admin_a2a_supersede_message(self) -> None: | |
|
|
||
|
|
||
| def make_server(host: str = DEFAULT_HOST, port: int = DEFAULT_PORT, data_dir=None, | ||
| verifier=None, grants_verifier=None): | ||
| verifier=None, grants_verifier=None, role_resolver=None): | ||
| """Create (but do not start) a :class:`ThreadingHTTPServer`. | ||
|
|
||
| Useful for tests that need to bind an ephemeral port (``port=0``) and read | ||
|
|
@@ -2072,6 +2158,9 @@ def make_server(host: str = DEFAULT_HOST, port: int = DEFAULT_PORT, data_dir=Non | |
| A :class:`_ServiceLoop` is started and attached as ``server.service_loop``; | ||
| closing it is handled by :func:`serve`. Tests that drive ``make_server`` | ||
| directly should call ``server.service_loop.close()`` during teardown. | ||
|
|
||
| ``role_resolver`` is an optional injectable seam (taOSmd #2155) for tests; | ||
| when omitted it is built from config by :func:`_make_handler`. | ||
| """ | ||
| if verifier is not None and grants_verifier is None: | ||
| # Identity without permission is half the locked auth contract | ||
|
|
@@ -2084,7 +2173,7 @@ def make_server(host: str = DEFAULT_HOST, port: int = DEFAULT_PORT, data_dir=Non | |
| runner = _ServiceLoop() | ||
| httpd = ThreadingHTTPServer( | ||
| (host, port), | ||
| _make_handler(data_dir, runner, verifier, grants_verifier), | ||
| _make_handler(data_dir, runner, verifier, grants_verifier, role_resolver), | ||
| ) | ||
| httpd.service_loop = runner | ||
| return httpd | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -212,12 +212,13 @@ async def a2a_send( | |||||||||||||||||||||||||||||||||||
| reply_to: str | None = None, | ||||||||||||||||||||||||||||||||||||
| refs: list | None = None, | ||||||||||||||||||||||||||||||||||||
| blocks: list | None = None, | ||||||||||||||||||||||||||||||||||||
| recipient: str | None = None, | ||||||||||||||||||||||||||||||||||||
| **_opts, | ||||||||||||||||||||||||||||||||||||
| ) -> dict: | ||||||||||||||||||||||||||||||||||||
| """POST /a2a/send: post a message to the remote A2A bus. | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Returns the send receipt ``{"id", "from", "thread", "reply_to"}`` | ||||||||||||||||||||||||||||||||||||
| plus ``refs``/``blocks`` when supplied (taOSmd #211). | ||||||||||||||||||||||||||||||||||||
| plus ``refs``/``blocks``/``recipient`` when supplied (taOSmd #211/#2155). | ||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||
| payload: dict = {"from": sender, "body": body, "thread": thread} | ||||||||||||||||||||||||||||||||||||
| if reply_to is not None: | ||||||||||||||||||||||||||||||||||||
|
|
@@ -226,6 +227,8 @@ async def a2a_send( | |||||||||||||||||||||||||||||||||||
| payload["refs"] = refs | ||||||||||||||||||||||||||||||||||||
| if blocks is not None: | ||||||||||||||||||||||||||||||||||||
| payload["blocks"] = blocks | ||||||||||||||||||||||||||||||||||||
| if recipient is not None: | ||||||||||||||||||||||||||||||||||||
| payload["recipient"] = recipient | ||||||||||||||||||||||||||||||||||||
| return await self._run("POST", "/a2a/send", payload) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| async def a2a_feed( | ||||||||||||||||||||||||||||||||||||
|
|
@@ -234,17 +237,23 @@ async def a2a_feed( | |||||||||||||||||||||||||||||||||||
| thread: str | None = None, | ||||||||||||||||||||||||||||||||||||
| since: float | None = None, | ||||||||||||||||||||||||||||||||||||
| limit: int = 50, | ||||||||||||||||||||||||||||||||||||
| recipient: str | None = None, | ||||||||||||||||||||||||||||||||||||
| **_opts, | ||||||||||||||||||||||||||||||||||||
| ) -> list[dict]: | ||||||||||||||||||||||||||||||||||||
| """GET /a2a/messages: return messages from the remote A2A bus, oldest-first. | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Returns the ``messages`` list from the server response. | ||||||||||||||||||||||||||||||||||||
| ``recipient`` is forwarded as ``?recipient=`` so the remote server can | ||||||||||||||||||||||||||||||||||||
| apply its verbatim recipient filter (taOSmd #2155). The remote does | ||||||||||||||||||||||||||||||||||||
| not annotate ``resolved_to``; callers reading a role recipient do their | ||||||||||||||||||||||||||||||||||||
| own delivery-time resolution. | ||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
243
to
249
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win The docstring contradicts the server behavior verified by the tests. Lines 246-248 state that the remote does not annotate 📝 Proposed docstring fix ``recipient`` is forwarded as ``?recipient=`` so the remote server can
- apply its verbatim recipient filter (taOSmd `#2155`). The remote does
- not annotate ``resolved_to``; callers reading a role recipient do their
- own delivery-time resolution.
+ apply its verbatim recipient filter (taOSmd `#2155`). For a role
+ recipient the server resolves the current holder at read time and adds
+ ``resolved_to`` to each message; this client returns that field
+ unchanged. The server returns 400 when no resolver is configured and
+ 503 when the resolver is unreachable, which surface as ``RuntimeError``.📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
| params: dict = {"limit": limit} | ||||||||||||||||||||||||||||||||||||
| if thread is not None: | ||||||||||||||||||||||||||||||||||||
| params["thread"] = thread | ||||||||||||||||||||||||||||||||||||
| if since is not None: | ||||||||||||||||||||||||||||||||||||
| params["since"] = since | ||||||||||||||||||||||||||||||||||||
| if recipient is not None: | ||||||||||||||||||||||||||||||||||||
| params["recipient"] = recipient | ||||||||||||||||||||||||||||||||||||
| resp = await self._run("GET", "/a2a/messages", params=params) | ||||||||||||||||||||||||||||||||||||
| return resp.get("messages", []) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3. Recipient size unchecked
🐞 Bug☼ ReliabilityPOST /a2a/send now accepts and persists recipient, but the existing 64KB size guard only covers {body, refs, blocks} and the new recipient validation enforces only “non-empty”. A client can bypass the envelope size guard with an arbitrarily large recipient, causing oversized archive rows and large echoed responses.Agent Prompt
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools