From d41b986cb04919cc5cca3f638bdd85e85ed70d2e Mon Sep 17 00:00:00 2001 From: LinYushen Date: Mon, 30 Mar 2026 13:12:59 +0800 Subject: [PATCH] feat(server): distinguish agent vs human CLI actions (#181) * feat(server): distinguish agent vs human CLI actions via X-Agent-ID/X-Task-ID headers Extract resolveActor helper in handler to centralize agent identity resolution from X-Agent-ID header with X-Task-ID cross-validation. Fix DeleteComment, DeleteIssue, and UpdateComment handlers that previously hardcoded "member" as actor type. Forward MULTICA_TASK_ID as X-Task-ID header from CLI client. Co-Authored-By: Claude Opus 4.6 (1M context) * fix(server): add debug logging and test coverage for resolveActor Add slog.Debug on agent/task validation failures for easier debugging. Add TestResolveActor with 5 cases covering member fallback, valid agent, non-existent agent, valid task, and mismatched task. Co-Authored-By: Claude Opus 4.6 (1M context) --------- Co-authored-by: Claude Opus 4.6 (1M context) --- server/cmd/multica/cmd_agent.go | 3 + server/cmd/server/router.go | 2 +- server/internal/cli/client.go | 4 + server/internal/handler/comment.go | 17 ++-- server/internal/handler/handler.go | 31 +++++++ server/internal/handler/handler_test.go | 111 ++++++++++++++++++++++++ server/internal/handler/issue.go | 21 +---- 7 files changed, 159 insertions(+), 30 deletions(-) diff --git a/server/cmd/multica/cmd_agent.go b/server/cmd/multica/cmd_agent.go index aad41210..8606cc43 100644 --- a/server/cmd/multica/cmd_agent.go +++ b/server/cmd/multica/cmd_agent.go @@ -44,6 +44,9 @@ func newAPIClient(cmd *cobra.Command) (*cli.APIClient, error) { if agentID := os.Getenv("MULTICA_AGENT_ID"); agentID != "" { client.AgentID = agentID } + if taskID := os.Getenv("MULTICA_TASK_ID"); taskID != "" { + client.TaskID = taskID + } return client, nil } diff --git a/server/cmd/server/router.go b/server/cmd/server/router.go index 07b9d170..fd34f8c5 100644 --- a/server/cmd/server/router.go +++ b/server/cmd/server/router.go @@ -58,7 +58,7 @@ func NewRouter(pool *pgxpool.Pool, hub *realtime.Hub, bus *events.Bus) chi.Route r.Use(cors.Handler(cors.Options{ AllowedOrigins: allowedOrigins(), AllowedMethods: []string{"GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"}, - AllowedHeaders: []string{"Accept", "Authorization", "Content-Type", "X-Workspace-ID", "X-Request-ID"}, + AllowedHeaders: []string{"Accept", "Authorization", "Content-Type", "X-Workspace-ID", "X-Request-ID", "X-Agent-ID", "X-Task-ID"}, AllowCredentials: true, MaxAge: 300, })) diff --git a/server/internal/cli/client.go b/server/internal/cli/client.go index 88734636..312276d0 100644 --- a/server/internal/cli/client.go +++ b/server/internal/cli/client.go @@ -22,6 +22,7 @@ type APIClient struct { WorkspaceID string Token string AgentID string // When set, requests are attributed to this agent instead of the user. + TaskID string // When set, sent as X-Task-ID for agent-task validation. HTTPClient *http.Client } @@ -45,6 +46,9 @@ func (c *APIClient) setHeaders(req *http.Request) { if c.AgentID != "" { req.Header.Set("X-Agent-ID", c.AgentID) } + if c.TaskID != "" { + req.Header.Set("X-Task-ID", c.TaskID) + } } // GetJSON performs a GET request and decodes the JSON response. diff --git a/server/internal/handler/comment.go b/server/internal/handler/comment.go index 59e9c34c..4c1a512f 100644 --- a/server/internal/handler/comment.go +++ b/server/internal/handler/comment.go @@ -102,16 +102,7 @@ func (h *Handler) CreateComment(w http.ResponseWriter, r *http.Request) { } // Determine author identity: agent (via X-Agent-ID header) or member. - authorType := "member" - authorID := userID - if agentID := r.Header.Get("X-Agent-ID"); agentID != "" { - // Validate the agent exists in this workspace. - agent, err := h.Queries.GetAgent(r.Context(), parseUUID(agentID)) - if err == nil && uuidToString(agent.WorkspaceID) == uuidToString(issue.WorkspaceID) { - authorType = "agent" - authorID = agentID - } - } + authorType, authorID := h.resolveActor(r, userID, uuidToString(issue.WorkspaceID)) comment, err := h.Queries.CreateComment(r.Context(), db.CreateCommentParams{ IssueID: issue.ID, @@ -205,8 +196,9 @@ func (h *Handler) UpdateComment(w http.ResponseWriter, r *http.Request) { } resp := commentToResponse(comment) + actorType, actorID := h.resolveActor(r, userID, uuidToString(issue.WorkspaceID)) slog.Info("comment updated", append(logger.RequestAttrs(r), "comment_id", commentId)...) - h.publish(protocol.EventCommentUpdated, uuidToString(issue.WorkspaceID), "member", userID, map[string]any{"comment": resp}) + h.publish(protocol.EventCommentUpdated, uuidToString(issue.WorkspaceID), actorType, actorID, map[string]any{"comment": resp}) writeJSON(w, http.StatusOK, resp) } @@ -250,8 +242,9 @@ func (h *Handler) DeleteComment(w http.ResponseWriter, r *http.Request) { return } + actorType, actorID := h.resolveActor(r, userID, uuidToString(issue.WorkspaceID)) slog.Info("comment deleted", append(logger.RequestAttrs(r), "comment_id", commentId, "issue_id", uuidToString(comment.IssueID))...) - h.publish(protocol.EventCommentDeleted, uuidToString(issue.WorkspaceID), "member", userID, map[string]any{ + h.publish(protocol.EventCommentDeleted, uuidToString(issue.WorkspaceID), actorType, actorID, map[string]any{ "comment_id": commentId, "issue_id": uuidToString(comment.IssueID), }) diff --git a/server/internal/handler/handler.go b/server/internal/handler/handler.go index aaee0cd5..c42d4f4d 100644 --- a/server/internal/handler/handler.go +++ b/server/internal/handler/handler.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "log/slog" "net/http" "github.com/jackc/pgx/v5" @@ -99,6 +100,36 @@ func requestUserID(r *http.Request) string { return r.Header.Get("X-User-ID") } +// resolveActor determines whether the request is from an agent or a human member. +// If X-Agent-ID and X-Task-ID headers are both set, validates that the task +// belongs to the claimed agent (defense-in-depth against manual header spoofing). +// If only X-Agent-ID is set, validates that the agent belongs to the workspace. +// Returns ("agent", agentID) on success, ("member", userID) otherwise. +func (h *Handler) resolveActor(r *http.Request, userID, workspaceID string) (actorType, actorID string) { + agentID := r.Header.Get("X-Agent-ID") + if agentID == "" { + return "member", userID + } + + // Validate the agent exists in the target workspace. + agent, err := h.Queries.GetAgent(r.Context(), parseUUID(agentID)) + if err != nil || uuidToString(agent.WorkspaceID) != workspaceID { + slog.Debug("resolveActor: X-Agent-ID rejected, agent not found or workspace mismatch", "agent_id", agentID, "workspace_id", workspaceID) + return "member", userID + } + + // When X-Task-ID is provided, cross-check that the task belongs to this agent. + if taskID := r.Header.Get("X-Task-ID"); taskID != "" { + task, err := h.Queries.GetAgentTask(r.Context(), parseUUID(taskID)) + if err != nil || uuidToString(task.AgentID) != agentID { + slog.Debug("resolveActor: X-Task-ID rejected, task not found or agent mismatch", "agent_id", agentID, "task_id", taskID) + return "member", userID + } + } + + return "agent", agentID +} + func requireUserID(w http.ResponseWriter, r *http.Request) (string, bool) { userID := requestUserID(r) if userID == "" { diff --git a/server/internal/handler/handler_test.go b/server/internal/handler/handler_test.go index e18630f6..daf68e0a 100644 --- a/server/internal/handler/handler_test.go +++ b/server/internal/handler/handler_test.go @@ -609,6 +609,117 @@ func TestVerifyCodeCreatesWorkspace(t *testing.T) { } } +func TestResolveActor(t *testing.T) { + ctx := context.Background() + + // Look up the agent created by the test fixture. + var agentID string + err := testPool.QueryRow(ctx, + `SELECT id FROM agent WHERE workspace_id = $1 AND name = $2`, + testWorkspaceID, "Handler Test Agent", + ).Scan(&agentID) + if err != nil { + t.Fatalf("failed to find test agent: %v", err) + } + + // Create a task for the agent so we can test X-Task-ID validation. + var issueID string + err = testPool.QueryRow(ctx, + `INSERT INTO issue (workspace_id, title, status, priority, creator_type, creator_id, number, position) + VALUES ($1, 'resolveActor test', 'todo', 'none', 'member', $2, 9999, 0) + RETURNING id`, testWorkspaceID, testUserID, + ).Scan(&issueID) + if err != nil { + t.Fatalf("failed to create test issue: %v", err) + } + + // Look up runtime_id for the agent. + var runtimeID string + err = testPool.QueryRow(ctx, `SELECT runtime_id FROM agent WHERE id = $1`, agentID).Scan(&runtimeID) + if err != nil { + t.Fatalf("failed to get agent runtime_id: %v", err) + } + + var taskID string + err = testPool.QueryRow(ctx, + `INSERT INTO agent_task_queue (agent_id, runtime_id, issue_id, status, priority) + VALUES ($1, $2, $3, 'queued', 0) + RETURNING id`, agentID, runtimeID, issueID, + ).Scan(&taskID) + if err != nil { + t.Fatalf("failed to create test task: %v", err) + } + + t.Cleanup(func() { + testPool.Exec(ctx, `DELETE FROM agent_task_queue WHERE id = $1`, taskID) + testPool.Exec(ctx, `DELETE FROM issue WHERE id = $1`, issueID) + }) + + tests := []struct { + name string + agentIDHeader string + taskIDHeader string + wantActorType string + wantIsAgent bool + }{ + { + name: "no headers returns member", + wantActorType: "member", + }, + { + name: "valid agent ID returns agent", + agentIDHeader: agentID, + wantActorType: "agent", + wantIsAgent: true, + }, + { + name: "non-existent agent ID returns member", + agentIDHeader: "00000000-0000-0000-0000-000000000099", + wantActorType: "member", + }, + { + name: "valid agent + valid task returns agent", + agentIDHeader: agentID, + taskIDHeader: taskID, + wantActorType: "agent", + wantIsAgent: true, + }, + { + name: "valid agent + wrong task returns member", + agentIDHeader: agentID, + taskIDHeader: "00000000-0000-0000-0000-000000000099", + wantActorType: "member", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := newRequest("GET", "/test", nil) + if tt.agentIDHeader != "" { + req.Header.Set("X-Agent-ID", tt.agentIDHeader) + } + if tt.taskIDHeader != "" { + req.Header.Set("X-Task-ID", tt.taskIDHeader) + } + + actorType, actorID := testHandler.resolveActor(req, testUserID, testWorkspaceID) + + if actorType != tt.wantActorType { + t.Errorf("actorType = %q, want %q", actorType, tt.wantActorType) + } + if tt.wantIsAgent { + if actorID != tt.agentIDHeader { + t.Errorf("actorID = %q, want agent %q", actorID, tt.agentIDHeader) + } + } else { + if actorID != testUserID { + t.Errorf("actorID = %q, want user %q", actorID, testUserID) + } + } + }) + } +} + func TestDaemonRegisterMissingWorkspaceReturns404(t *testing.T) { w := httptest.NewRecorder() req := httptest.NewRequest("POST", "/api/daemon/register", bytes.NewBufferString(`{ diff --git a/server/internal/handler/issue.go b/server/internal/handler/issue.go index 659717d4..efe78048 100644 --- a/server/internal/handler/issue.go +++ b/server/internal/handler/issue.go @@ -221,14 +221,7 @@ func (h *Handler) CreateIssue(w http.ResponseWriter, r *http.Request) { } // Determine creator identity: agent (via X-Agent-ID header) or member. - creatorType := "member" - actualCreatorID := creatorID - if agentID := r.Header.Get("X-Agent-ID"); agentID != "" { - if agent, err := h.Queries.GetAgent(r.Context(), parseUUID(agentID)); err == nil && uuidToString(agent.WorkspaceID) == workspaceID { - creatorType = "agent" - actualCreatorID = agentID - } - } + creatorType, actualCreatorID := h.resolveActor(r, creatorID, workspaceID) issue, err := qtx.CreateIssue(r.Context(), db.CreateIssueParams{ WorkspaceID: parseUUID(workspaceID), @@ -382,14 +375,7 @@ func (h *Handler) UpdateIssue(w http.ResponseWriter, r *http.Request) { (prevDueDate != nil && resp.DueDate != nil && *prevDueDate != *resp.DueDate) // Determine actor identity: agent (via X-Agent-ID header) or member. - actorType := "member" - actorID := userID - if agentID := r.Header.Get("X-Agent-ID"); agentID != "" { - if agent, err := h.Queries.GetAgent(r.Context(), parseUUID(agentID)); err == nil && uuidToString(agent.WorkspaceID) == workspaceID { - actorType = "agent" - actorID = agentID - } - } + actorType, actorID := h.resolveActor(r, userID, workspaceID) h.publish(protocol.EventIssueUpdated, workspaceID, actorType, actorID, map[string]any{ "issue": resp, @@ -495,7 +481,8 @@ func (h *Handler) DeleteIssue(w http.ResponseWriter, r *http.Request) { } userID := requestUserID(r) - h.publish(protocol.EventIssueDeleted, uuidToString(issue.WorkspaceID), "member", userID, map[string]any{"issue_id": id}) + actorType, actorID := h.resolveActor(r, userID, uuidToString(issue.WorkspaceID)) + h.publish(protocol.EventIssueDeleted, uuidToString(issue.WorkspaceID), actorType, actorID, map[string]any{"issue_id": id}) slog.Info("issue deleted", append(logger.RequestAttrs(r), "issue_id", id, "workspace_id", uuidToString(issue.WorkspaceID))...) w.WriteHeader(http.StatusNoContent) }