From 21030611231f4f51c636b43280d701b8a933eeb3 Mon Sep 17 00:00:00 2001 From: Nguyen Khac Trung Kien Date: Sun, 8 Feb 2026 09:52:49 +0700 Subject: [PATCH] fix(agent-teams): close latest review gaps for auth and race safety --- src/tools/agent-teams/messaging-tools.ts | 12 +- src/tools/agent-teams/team-task-update.ts | 11 +- src/tools/agent-teams/teammate-runtime.ts | 6 +- src/tools/agent-teams/teammate-tools.ts | 13 ++- .../agent-teams/tools.functional.test.ts | 106 ++++++++++++++++++ 5 files changed, 139 insertions(+), 9 deletions(-) diff --git a/src/tools/agent-teams/messaging-tools.ts b/src/tools/agent-teams/messaging-tools.ts index 5d382eed..8010b613 100644 --- a/src/tools/agent-teams/messaging-tools.ts +++ b/src/tools/agent-teams/messaging-tools.ts @@ -212,7 +212,7 @@ export function createReadInboxTool(): ToolDefinition { unread_only: tool.schema.boolean().optional().describe("Return only unread messages"), mark_as_read: tool.schema.boolean().optional().describe("Mark returned messages as read"), }, - execute: async (args: Record): Promise => { + execute: async (args: Record, context: TeamToolContext): Promise => { try { const input = TeamReadInboxInputSchema.parse(args) const teamError = validateTeamName(input.team_name) @@ -223,7 +223,15 @@ export function createReadInboxTool(): ToolDefinition { if (agentError) { return JSON.stringify({ error: agentError }) } - readTeamConfigOrThrow(input.team_name) + const config = readTeamConfigOrThrow(input.team_name) + const actor = resolveSenderFromContext(config, context) + if (!actor) { + return JSON.stringify({ error: "unauthorized_reader_session" }) + } + + if (actor !== "team-lead" && actor !== input.agent_name) { + return JSON.stringify({ error: "unauthorized_reader_session" }) + } const messages = readInbox( input.team_name, diff --git a/src/tools/agent-teams/team-task-update.ts b/src/tools/agent-teams/team-task-update.ts index 9916db7a..b979c965 100644 --- a/src/tools/agent-teams/team-task-update.ts +++ b/src/tools/agent-teams/team-task-update.ts @@ -118,8 +118,15 @@ export function updateTeamTask(teamName: string, taskId: string, patch: TeamTask if (patch.status && patch.status !== "deleted") { ensureForwardStatusTransition(currentTask.status, patch.status) - const effectiveBlockedBy = Array.from(new Set([...(currentTask.blockedBy ?? []), ...(patch.addBlockedBy ?? [])])) - ensureDependenciesCompleted(patch.status, effectiveBlockedBy, readTask) + } + + const effectiveStatus = patch.status ?? currentTask.status + const effectiveBlockedBy = Array.from(new Set([...(currentTask.blockedBy ?? []), ...(patch.addBlockedBy ?? [])])) + const shouldValidateDependencies = + (patch.status !== undefined || (patch.addBlockedBy?.length ?? 0) > 0) && effectiveStatus !== "deleted" + + if (shouldValidateDependencies) { + ensureDependenciesCompleted(effectiveStatus, effectiveBlockedBy, readTask) } let nextTask: TeamTask = { ...currentTask } diff --git a/src/tools/agent-teams/teammate-runtime.ts b/src/tools/agent-teams/teammate-runtime.ts index 90063c6b..8aabdf6a 100644 --- a/src/tools/agent-teams/teammate-runtime.ts +++ b/src/tools/agent-teams/teammate-runtime.ts @@ -90,10 +90,10 @@ export async function spawnTeammate(params: SpawnTeammateParams): Promise { + const refreshedMember = getTeamMember(current, input.agent_name) + if (!refreshedMember || !isTeammateMember(refreshedMember)) { + return current + } + + return removeTeammate(current, input.agent_name) + }) + resetOwnerTasks(input.team_name, input.agent_name) return JSON.stringify({ success: true, message: `${input.agent_name} removed` }) diff --git a/src/tools/agent-teams/tools.functional.test.ts b/src/tools/agent-teams/tools.functional.test.ts index 0620f534..d9ee7078 100644 --- a/src/tools/agent-teams/tools.functional.test.ts +++ b/src/tools/agent-teams/tools.functional.test.ts @@ -657,6 +657,112 @@ describe("agent-teams tools functional", () => { expect(result.error).toBe("team_not_found") }) + test("read_inbox denies cross-member access for non-lead sessions", async () => { + //#given + const { manager } = createMockManager() + const tools = createAgentTeamsTools(manager) + const leadContext = createContext() + + await executeJsonTool(tools, "team_create", { team_name: "core" }, leadContext) + await executeJsonTool( + tools, + "spawn_teammate", + { + team_name: "core", + name: "worker_1", + prompt: "Handle release prep", + }, + leadContext, + ) + + const teammateContext = createContext("ses-worker-1") + + //#when + const unauthorized = await executeJsonTool( + tools, + "read_inbox", + { + team_name: "core", + agent_name: "team-lead", + }, + teammateContext, + ) as { error?: string } + + //#then + expect(unauthorized.error).toBe("unauthorized_reader_session") + + //#when + const ownInbox = await executeJsonTool( + tools, + "read_inbox", + { + team_name: "core", + agent_name: "worker_1", + }, + teammateContext, + ) as unknown[] + + //#then + expect(Array.isArray(ownInbox)).toBe(true) + }) + + test("cannot add pending blockers to already in-progress task without status change", async () => { + //#given + const { manager } = createMockManager() + const tools = createAgentTeamsTools(manager) + const context = createContext() + + await executeJsonTool(tools, "team_create", { team_name: "core" }, context) + + const blocker = await executeJsonTool( + tools, + "team_task_create", + { + team_name: "core", + subject: "Blocker", + description: "Unfinished blocker", + }, + context, + ) as { id: string } + + const mainTask = await executeJsonTool( + tools, + "team_task_create", + { + team_name: "core", + subject: "Main", + description: "Main task", + }, + context, + ) as { id: string } + + await executeJsonTool( + tools, + "team_task_update", + { + team_name: "core", + task_id: mainTask.id, + status: "in_progress", + }, + context, + ) + + //#when + const result = await executeJsonTool( + tools, + "team_task_update", + { + team_name: "core", + task_id: mainTask.id, + add_blocked_by: [blocker.id], + }, + context, + ) as { error?: string } + + //#then + expect(result.error).toBe(`blocked_by_incomplete:${blocker.id}:pending`) + }) + test("binds sender to calling context and rejects sender spoofing", async () => { //#given const { manager } = createMockManager()