forked from EduCraft/curriculum-project-hub
fix(hub): stop unrestricted roles from loading multi-agent tools
Merge pull request #33 from fix/hub-no-background-agent-tools
This commit is contained in:
+24
-9
@@ -140,9 +140,8 @@ export async function runAgent(req: RunRequest): Promise<RunResult> {
|
|||||||
let cleanupSecurity = async (): Promise<void> => {};
|
let cleanupSecurity = async (): Promise<void> => {};
|
||||||
try {
|
try {
|
||||||
await persistAgentMessage(req, "user", req.prompt);
|
await persistAgentMessage(req, "user", req.prompt);
|
||||||
// Role tools JSON null means unrestricted (omit), not "deny all".
|
// Role tools JSON null means the default single-agent tool set, not "deny all".
|
||||||
const roleToolIds = req.tools === null ? undefined : req.tools;
|
const roleToolIds = req.tools === null ? undefined : req.tools;
|
||||||
const unrestricted = roleToolIds === undefined;
|
|
||||||
const toolConfig = claudeSdkToolConfigForRole(roleToolIds);
|
const toolConfig = claudeSdkToolConfigForRole(roleToolIds);
|
||||||
const workspaceRoot = req.project.workspaceRoot?.trim();
|
const workspaceRoot = req.project.workspaceRoot?.trim();
|
||||||
if (workspaceRoot === undefined || workspaceRoot === "") {
|
if (workspaceRoot === undefined || workspaceRoot === "") {
|
||||||
@@ -158,25 +157,41 @@ export async function runAgent(req: RunRequest): Promise<RunResult> {
|
|||||||
cleanupSecurity = security.cleanup;
|
cleanupSecurity = security.cleanup;
|
||||||
const hasSkills = security.skillIds.length > 0;
|
const hasSkills = security.skillIds.length > 0;
|
||||||
type QueryOptions = NonNullable<Parameters<typeof query>[0]["options"]>;
|
type QueryOptions = NonNullable<Parameters<typeof query>[0]["options"]>;
|
||||||
// When unrestricted, pass the SDK default toolset (`--tools default`) instead of
|
// Always use an explicit tool list — never the claude_code preset.
|
||||||
// an explicit subset. Native claude uses that path to register bundled tools like
|
// The preset registers Agent/SendMessage/Task multi-agent machinery.
|
||||||
// TodoWrite; listing names alone can omit them from the model's function list.
|
// Concurrent background agents abort with reason "background", and the
|
||||||
// allowedTools still carries explicit MCP names + TodoWrite for permission.
|
// Claude Agent SDK maps that to toolDenialKind "cancelled" with:
|
||||||
|
// "The user doesn't want to take this action right now..."
|
||||||
|
// which freezes Bash mid-run while Read/Glob continue to work.
|
||||||
|
// Hub "unrestricted" means the default single-agent built-ins + MCP, not
|
||||||
|
// the full interactive Claude product surface.
|
||||||
const skillExtras = hasSkills ? (["Skill"] as const) : ([] as const);
|
const skillExtras = hasSkills ? (["Skill"] as const) : ([] as const);
|
||||||
const toolsOption: QueryOptions["tools"] = unrestricted
|
const toolsOption: QueryOptions["tools"] = uniqueTools([
|
||||||
? { type: "preset", preset: "claude_code" }
|
...toolConfig.tools,
|
||||||
: uniqueTools([...toolConfig.tools, "TodoWrite", ...skillExtras]);
|
"TodoWrite",
|
||||||
|
...skillExtras,
|
||||||
|
]);
|
||||||
const allowedToolsOption = uniqueTools([
|
const allowedToolsOption = uniqueTools([
|
||||||
...toolConfig.allowedTools,
|
...toolConfig.allowedTools,
|
||||||
"TodoWrite",
|
"TodoWrite",
|
||||||
"mcp__cph_hub__todo_write",
|
"mcp__cph_hub__todo_write",
|
||||||
...skillExtras,
|
...skillExtras,
|
||||||
]);
|
]);
|
||||||
|
// Hard deny multi-agent orchestration even if a future preset/skills path
|
||||||
|
// reintroduces them — bypassPermissions would otherwise auto-allow them.
|
||||||
|
const disallowedToolsOption = [
|
||||||
|
"Agent",
|
||||||
|
"SendMessage",
|
||||||
|
"TeamCreate",
|
||||||
|
"Task",
|
||||||
|
"ScheduleWakeup",
|
||||||
|
] as const;
|
||||||
|
|
||||||
const options: QueryOptions = {
|
const options: QueryOptions = {
|
||||||
cwd: security.cwd,
|
cwd: security.cwd,
|
||||||
tools: toolsOption,
|
tools: toolsOption,
|
||||||
allowedTools: allowedToolsOption,
|
allowedTools: allowedToolsOption,
|
||||||
|
disallowedTools: [...disallowedToolsOption],
|
||||||
maxTurns: cap,
|
maxTurns: cap,
|
||||||
includePartialMessages: true,
|
includePartialMessages: true,
|
||||||
// ADR-0018: bypass interactive prompts (headless server); the sandbox
|
// ADR-0018: bypass interactive prompts (headless server); the sandbox
|
||||||
|
|||||||
@@ -114,10 +114,9 @@ describe("runAgent", () => {
|
|||||||
allowDangerouslySkipPermissions: true,
|
allowDangerouslySkipPermissions: true,
|
||||||
settingSources: [],
|
settingSources: [],
|
||||||
settings: { disableBundledSkills: true, todoFeatureEnabled: true },
|
settings: { disableBundledSkills: true, todoFeatureEnabled: true },
|
||||||
skills: [],
|
tools: expect.arrayContaining(["Read", "Write", "Bash", "Glob", "Grep", "TodoWrite"]),
|
||||||
tools: { type: "preset", preset: "claude_code" },
|
allowedTools: expect.arrayContaining(["TodoWrite", "mcp__cph_hub__todo_write"]),
|
||||||
allowedTools: expect.arrayContaining(["TodoWrite"]),
|
disallowedTools: expect.arrayContaining(["Agent", "SendMessage", "Task", "TeamCreate", "ScheduleWakeup"]),
|
||||||
strictMcpConfig: true,
|
|
||||||
sandbox: expect.objectContaining({
|
sandbox: expect.objectContaining({
|
||||||
enabled: true,
|
enabled: true,
|
||||||
failIfUnavailable: true,
|
failIfUnavailable: true,
|
||||||
@@ -132,6 +131,38 @@ describe("runAgent", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("never exposes multi-agent orchestration tools on unrestricted roles", async () => {
|
||||||
|
queryMock.mockReturnValue(messages(assistantMessage("ok"), resultMessage("sdk-session-1")));
|
||||||
|
|
||||||
|
await runAgent({
|
||||||
|
prompt: "继续",
|
||||||
|
model: undefined,
|
||||||
|
project: { projectId: "p", boundChatId: "c", workspaceRoot, workspaceDir: workspace },
|
||||||
|
systemPrompt: undefined,
|
||||||
|
tools: null,
|
||||||
|
runId: "run-1",
|
||||||
|
sessionId: "hub-session-1",
|
||||||
|
prisma: stubPrisma,
|
||||||
|
});
|
||||||
|
|
||||||
|
const call = queryMock.mock.calls[0]?.[0] as {
|
||||||
|
options?: { tools?: unknown; disallowedTools?: string[] };
|
||||||
|
} | undefined;
|
||||||
|
expect(call?.options?.tools).toEqual([
|
||||||
|
"Read",
|
||||||
|
"Write",
|
||||||
|
"Bash",
|
||||||
|
"Glob",
|
||||||
|
"Grep",
|
||||||
|
"WebFetch",
|
||||||
|
"WebSearch",
|
||||||
|
"TodoWrite",
|
||||||
|
]);
|
||||||
|
for (const blocked of ["Agent", "SendMessage", "Task", "TeamCreate", "ScheduleWakeup"]) {
|
||||||
|
expect(call?.options?.disallowedTools).toContain(blocked);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
it("does not send resume for a fresh Hub session", async () => {
|
it("does not send resume for a fresh Hub session", async () => {
|
||||||
queryMock.mockReturnValue(messages(assistantMessage("fresh"), resultMessage("sdk-session-1")));
|
queryMock.mockReturnValue(messages(assistantMessage("fresh"), resultMessage("sdk-session-1")));
|
||||||
|
|
||||||
@@ -194,6 +225,7 @@ describe("runAgent", () => {
|
|||||||
"mcp__cph_hub__todo_write",
|
"mcp__cph_hub__todo_write",
|
||||||
],
|
],
|
||||||
settings: expect.objectContaining({ todoFeatureEnabled: true }),
|
settings: expect.objectContaining({ todoFeatureEnabled: true }),
|
||||||
|
disallowedTools: expect.arrayContaining(["Agent", "SendMessage"]),
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
@@ -217,6 +249,7 @@ describe("runAgent", () => {
|
|||||||
tools: ["TodoWrite"],
|
tools: ["TodoWrite"],
|
||||||
allowedTools: ["TodoWrite", "mcp__cph_hub__todo_write"],
|
allowedTools: ["TodoWrite", "mcp__cph_hub__todo_write"],
|
||||||
settings: expect.objectContaining({ todoFeatureEnabled: true }),
|
settings: expect.objectContaining({ todoFeatureEnabled: true }),
|
||||||
|
disallowedTools: expect.arrayContaining(["Agent", "SendMessage"]),
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user