diff --git a/go/core/internal/a2a/passthrough_handler.go b/go/core/internal/a2a/passthrough_handler.go index 7c92a658e..49bbdc0df 100644 --- a/go/core/internal/a2a/passthrough_handler.go +++ b/go/core/internal/a2a/passthrough_handler.go @@ -35,6 +35,16 @@ func validateShareContext(ctx context.Context, contextID string) error { return nil } +// requireWritableShare rejects a mutating operation issued under a read-only +// share token. A read-only share grants viewing the owner's session, not +// changing it. Requests without a share token are unaffected. +func requireWritableShare(ctx context.Context) error { + if sc, ok := pkgauth.ShareContextFrom(ctx); ok && sc.ReadOnly { + return fmt.Errorf("share token is read-only") + } + return nil +} + // injectInitiatedBy records the caller's identity in message metadata when the // request carries a share token, so the agent can attribute the interaction. func injectInitiatedBy(ctx context.Context, msg *a2atype.Message) { @@ -71,10 +81,16 @@ func (h *PassthroughRequestHandler) ListTasks(ctx context.Context, req *a2atype. } func (h *PassthroughRequestHandler) CancelTask(ctx context.Context, req *a2atype.CancelTaskRequest) (*a2atype.Task, error) { + if err := requireWritableShare(ctx); err != nil { + return nil, err + } return h.client.CancelTask(ctx, req) } func (h *PassthroughRequestHandler) SendMessage(ctx context.Context, req *a2atype.SendMessageRequest) (a2atype.SendMessageResult, error) { + if err := requireWritableShare(ctx); err != nil { + return nil, err + } if req.Message != nil { if err := validateShareContext(ctx, req.Message.ContextID); err != nil { return nil, err @@ -89,6 +105,12 @@ func (h *PassthroughRequestHandler) SubscribeToTask(ctx context.Context, req *a2 } func (h *PassthroughRequestHandler) SendStreamingMessage(ctx context.Context, req *a2atype.SendMessageRequest) iter.Seq2[a2atype.Event, error] { + if err := requireWritableShare(ctx); err != nil { + return func(yield func(a2atype.Event, error) bool) { + var zero a2atype.Event + yield(zero, err) + } + } if req.Message != nil { if err := validateShareContext(ctx, req.Message.ContextID); err != nil { return func(yield func(a2atype.Event, error) bool) { @@ -114,10 +136,16 @@ func (h *PassthroughRequestHandler) ListTaskPushConfigs(ctx context.Context, req } func (h *PassthroughRequestHandler) CreateTaskPushConfig(ctx context.Context, req *a2atype.PushConfig) (*a2atype.PushConfig, error) { + if err := requireWritableShare(ctx); err != nil { + return nil, err + } return h.client.CreateTaskPushConfig(ctx, req) } func (h *PassthroughRequestHandler) DeleteTaskPushConfig(ctx context.Context, req *a2atype.DeleteTaskPushConfigRequest) error { + if err := requireWritableShare(ctx); err != nil { + return err + } return h.client.DeleteTaskPushConfig(ctx, req) } diff --git a/go/core/internal/a2a/readonly_share_test.go b/go/core/internal/a2a/readonly_share_test.go new file mode 100644 index 000000000..8afd846de --- /dev/null +++ b/go/core/internal/a2a/readonly_share_test.go @@ -0,0 +1,52 @@ +package a2a + +import ( + "context" + "testing" + + a2atype "github.com/a2aproject/a2a-go/v2/a2a" + pkgauth "github.com/kagent-dev/kagent/go/core/pkg/auth" + "github.com/stretchr/testify/require" +) + +func ctxWithShare(userID string, readOnly bool) context.Context { + return pkgauth.ShareContextTo(ctxWithUser(userID), &pkgauth.ShareContext{ + Token: "tok", + SessionID: "sess-1", + UserID: "owner-id", + ReadOnly: readOnly, + }) +} + +func TestRequireWritableShare(t *testing.T) { + require.NoError(t, requireWritableShare(context.Background()), "no share token: allowed") + require.NoError(t, requireWritableShare(ctxWithShare("caller", false)), "read-write share: allowed") + require.Error(t, requireWritableShare(ctxWithShare("caller", true)), "read-only share: rejected") +} + +// A read-only share must not reach the upstream client on any mutating method. +// The handler is built with a nil client on purpose: the guard has to return +// before any client call, so a nil dereference would mean the guard is missing. +func TestReadOnlyShareRejectsWrites(t *testing.T) { + h := &PassthroughRequestHandler{} + ctx := ctxWithShare("caller", true) + + _, err := h.CancelTask(ctx, &a2atype.CancelTaskRequest{}) + require.Error(t, err, "CancelTask") + + _, err = h.SendMessage(ctx, &a2atype.SendMessageRequest{}) + require.Error(t, err, "SendMessage") + + _, err = h.CreateTaskPushConfig(ctx, &a2atype.PushConfig{}) + require.Error(t, err, "CreateTaskPushConfig") + + err = h.DeleteTaskPushConfig(ctx, &a2atype.DeleteTaskPushConfigRequest{}) + require.Error(t, err, "DeleteTaskPushConfig") + + var streamErr error + for _, e := range h.SendStreamingMessage(ctx, &a2atype.SendMessageRequest{}) { + streamErr = e + break + } + require.Error(t, streamErr, "SendStreamingMessage") +} diff --git a/go/core/internal/httpserver/middleware.go b/go/core/internal/httpserver/middleware.go index fb190da38..67eb60007 100644 --- a/go/core/internal/httpserver/middleware.go +++ b/go/core/internal/httpserver/middleware.go @@ -119,14 +119,15 @@ func (s *HTTPServer) shareTokenMiddleware(next http.Handler) http.Handler { return } - // Enforce read-only on session and A2A paths only. Visitors retain full - // authenticated access to all other endpoints (creating their own sessions, - // submitting feedback, etc.) — the share token should not restrict unrelated operations. + // Enforce read-only on the session REST path by HTTP verb. A2A traffic is + // JSON-RPC over POST, so the verb does not distinguish reads from writes; + // its read-only enforcement is per-method in the A2A request handler + // (requireWritableShare), which lets a read-only share list and get tasks + // while still rejecting message sends, cancels, and push-config writes. + // Visitors retain full authenticated access to all other endpoints + // (creating their own sessions, submitting feedback, etc.). if share.ReadOnly && r.Method != http.MethodGet && r.Method != http.MethodHead { - path := r.URL.Path - if strings.HasPrefix(path, APIPathSessions+"/") || - strings.HasPrefix(path, APIPathA2A+"/") || - strings.HasPrefix(path, APIPathA2ASandboxes+"/") { + if strings.HasPrefix(r.URL.Path, APIPathSessions+"/") { http.Error(w, "This share link is read-only", http.StatusForbidden) return } diff --git a/go/core/internal/httpserver/server_share_middleware_test.go b/go/core/internal/httpserver/server_share_middleware_test.go index ba4d0b27a..5b8b5f84a 100644 --- a/go/core/internal/httpserver/server_share_middleware_test.go +++ b/go/core/internal/httpserver/server_share_middleware_test.go @@ -168,7 +168,10 @@ func TestShareTokenMiddleware(t *testing.T) { wantReadOnly: false, }, { - name: "valid read-only token with POST to A2A path returns 403", + // A2A is JSON-RPC over POST, so read-only enforcement can't be done by + // verb here; the middleware passes the request through with a ShareContext + // and the A2A handler rejects mutating methods per-method. + name: "valid read-only token with POST to A2A path passes through with ShareContext", getShare: func(_ context.Context, _ string) (*dbpkg.SessionShare, error) { return okShare, nil }, @@ -177,8 +180,9 @@ func TestShareTokenMiddleware(t *testing.T) { r.Header.Set("X-Share-Token", "valid-token") return withUser(r, "visitor-id") }, - wantStatus: http.StatusForbidden, - wantShareCtx: false, + wantStatus: http.StatusOK, + wantShareCtx: true, + wantReadOnly: true, }, { name: "valid read-write token with POST to A2A path passes through",