feat: allow read-only shares to read A2A tasks#2191
Conversation
Read-only share tokens were blocked from every A2A method because the share middleware rejects all non-GET requests on the A2A path, and A2A is JSON-RPC over POST. That left ListTasks and GetTask unreachable for a read-only share, which is the default share mode. Move read-only enforcement for A2A off the transport verb and into the request handler: reads (ListTasks, GetTask, push-config gets, SubscribeToTask) pass through, and mutating methods (SendMessage, SendStreamingMessage, CancelTask, CreateTaskPushConfig, DeleteTaskPushConfig) are rejected via requireWritableShare. The session REST path keeps its verb-based read-only block. Read-only writes over A2A now return a JSON-RPC error instead of HTTP 403. Signed-off-by: QuentinBisson <quentin@giantswarm.io>
|
@onematchfox would love your thoughts as well |
I'm on PTO at the moment so haven't looked at the code/implementation itself but the general ask/fix makes sense to me. My original implementation focussed on the controller API endpoints used to read state from the Kagent DB rather than the semantics of similar functionality that could be achieved directly via A2A methods. |
|
Out of draft and ready for review |
There was a problem hiding this comment.
Pull request overview
This PR updates share-token authorization so that read-only share links can successfully perform A2A task reads (A2A is JSON-RPC over POST), while still preventing A2A writes by enforcing read-only at the A2A method level rather than via HTTP verb blocking in middleware.
Changes:
- Stop blanket-blocking non-GET/HEAD A2A requests for read-only shares in
shareTokenMiddleware(session REST path remains verb-blocked). - Add
requireWritableSharechecks to mutating A2A methods inPassthroughRequestHandler. - Update/add tests to cover the new behavior at both the HTTP middleware layer and the A2A handler layer.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| go/core/internal/httpserver/server_share_middleware_test.go | Updates middleware expectations so read-only shares can POST to A2A paths and still receive a ShareContext. |
| go/core/internal/httpserver/middleware.go | Removes A2A/A2A-sandbox verb-based blocking for read-only shares; keeps session REST verb-based blocking. |
| go/core/internal/a2a/readonly_share_test.go | Adds unit tests for requireWritableShare and ensures mutating A2A methods are rejected under read-only share context. |
| go/core/internal/a2a/passthrough_handler.go | Introduces requireWritableShare and applies it to mutating A2A methods (SendMessage, streaming send, cancel, push-config writes). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if err := requireWritableShare(ctx); err != nil { | ||
| return func(yield func(a2atype.Event, error) bool) { | ||
| var zero a2atype.Event | ||
| yield(zero, err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Why not return an error here like the other functions?
There was a problem hiding this comment.
SendStreamingMessage returns iter.Seq2[a2atype.Event, error], not (result, error), so there's no error return to hand back here.
With this interface the only way to surface the error to the caller is to yield it from the sequence, which is what the closure does.
What
Read-only share tokens can now read tasks over A2A (
ListTasks,GetTask). They were blocked from every A2A method becauseshareTokenMiddlewarerejects all non-GET requests on the A2A path, and A2A is JSON-RPC over POST. Read-only is the default share mode (session_shares.godefaultsread_onlytotrue), so the share-scoped task read path added in #2187 was unreachable for the common case.How
Read-only enforcement for A2A moves from the transport verb to the request handler:
ListTasks,GetTask, push-config get/list,SubscribeToTask.requireWritableShare:SendMessage,SendStreamingMessage,CancelTask,CreateTaskPushConfig,DeleteTaskPushConfig.The middleware no longer blanket-blocks non-GET A2A requests for read-only shares, so the per-method guards are what stop a read-only share from mutating a session. Guarding at the handler covers both wires (v0 and v1 route the same methods to it).
Behavior change
A read-only share attempting a mutating A2A method now returns a JSON-RPC error result instead of an HTTP 403.
Follows #2187 (store-backed
ListTasks); branched frommain, no code dependency on it.