fix: share ACP owner-only approval classes (#201) (#59255)

Co-authored-by: OpenClaw Dummy Agent <octriage-dummy@example.invalid>
This commit is contained in:
Devin Robison
2026-04-02 10:45:41 -06:00
committed by GitHub
parent 176c059b05
commit 96b55821bc
6 changed files with 124 additions and 30 deletions

View File

@@ -226,6 +226,7 @@ Docs: https://docs.openclaw.ai
- Slack: stop retry-driven duplicate replies when draft-finalization edits fail ambiguously, and log configured allowlisted users/channels by readable name instead of raw IDs.
- Agents/OpenAI Responses: normalize raw bundled MCP tool schemas on the WebSocket/Responses path so bare-object, object-ish, and top-level union MCP tools no longer get rejected by OpenAI during tool registration. (#58299) Thanks @yelog.
- ACP/security: replace ACP's dangerous-tool name override with semantic approval classes, so only narrow readonly reads/searches can auto-approve while indirect exec-capable and control-plane tools always require explicit prompt approval. Thanks @vincentkoc.
- ACP: derive owner-only approval classes from the shared tool-policy fallback map so `cron`, `nodes`, and `whatsapp_login` cannot drift out of prompt-required coverage.
- ACP/sessions_spawn: register ACP child runs for completion tracking and lifecycle cleanup, and make registration-failure cleanup explicitly best-effort so callers do not assume an already-started ACP turn was fully aborted. (#40885) Thanks @xaeon2026 and @vincentkoc.
- ACP/tasks: mark cleanly exited ACP runs as blocked when they end on deterministic write or authorization blockers, and wake the parent session with a follow-up instead of falsely reporting success.
- ACPX/runtime: derive the bundled ACPX expected version from the extension package metadata instead of hardcoding a separate literal, so plugin-local ACPX installs stop drifting out of health-check parity after version bumps. (#49089) Thanks @jiejiesks and @vincentkoc.

View File

@@ -70,18 +70,40 @@ describe("classifyAcpToolApproval", () => {
});
});
it("classifies nodes as exec-capable even for list actions", () => {
expect(
classify({
title: "nodes: list",
rawInput: { name: "nodes", action: "list" },
}),
).toEqual({
toolName: "nodes",
approvalClass: "exec_capable",
autoApprove: false,
});
});
it.each([
{
title: "cron: status",
rawInput: { name: "cron", action: "status" },
expectedToolName: "cron",
expectedClass: "control_plane",
},
{
title: "nodes: list",
rawInput: { name: "nodes", action: "list" },
expectedToolName: "nodes",
expectedClass: "exec_capable",
},
{
title: "whatsapp_login: start",
rawInput: { name: "whatsapp_login" },
expectedToolName: "whatsapp_login",
expectedClass: "interactive",
},
] as const)(
"classifies shared owner-only ACP backstops for $expectedToolName",
({ title, rawInput, expectedToolName, expectedClass }) => {
expect(
classify({
title,
rawInput,
}),
).toEqual({
toolName: expectedToolName,
approvalClass: expectedClass,
autoApprove: false,
});
},
);
it("classifies gateway as control-plane", () => {
expect(

View File

@@ -2,6 +2,7 @@ import { homedir } from "node:os";
import path from "node:path";
import { isKnownCoreToolId } from "../agents/tool-catalog.js";
import { isMutatingToolCall } from "../agents/tool-mutation.js";
import { resolveOwnerOnlyToolApprovalClass } from "../agents/tool-policy.js";
const SAFE_SEARCH_TOOL_IDS = new Set(["search", "web_search", "memory_search"]);
const TRUSTED_SAFE_TOOL_ALIASES = new Set(["search"]);
@@ -11,17 +12,9 @@ const EXEC_CAPABLE_TOOL_IDS = new Set([
"shell",
"bash",
"process",
"nodes",
"code_execution",
]);
const CONTROL_PLANE_TOOL_IDS = new Set([
"gateway",
"cron",
"sessions_spawn",
"sessions_send",
"session_status",
]);
const INTERACTIVE_TOOL_IDS = new Set(["whatsapp_login"]);
const CONTROL_PLANE_TOOL_IDS = new Set(["sessions_spawn", "sessions_send", "session_status"]);
export type AcpApprovalClass =
| "readonly_scoped"
@@ -218,15 +211,16 @@ export function classifyAcpToolApproval(params: {
if (SAFE_SEARCH_TOOL_IDS.has(toolName) && isTrustedToolId) {
return { toolName, approvalClass: "readonly_search", autoApprove: true };
}
const ownerOnlyApprovalClass = resolveOwnerOnlyToolApprovalClass(toolName);
if (ownerOnlyApprovalClass) {
return { toolName, approvalClass: ownerOnlyApprovalClass, autoApprove: false };
}
if (EXEC_CAPABLE_TOOL_IDS.has(toolName)) {
return { toolName, approvalClass: "exec_capable", autoApprove: false };
}
if (CONTROL_PLANE_TOOL_IDS.has(toolName)) {
return { toolName, approvalClass: "control_plane", autoApprove: false };
}
if (INTERACTIVE_TOOL_IDS.has(toolName)) {
return { toolName, approvalClass: "interactive", autoApprove: false };
}
if (isMutatingToolCall(toolName, params.toolCall?.rawInput)) {
return { toolName, approvalClass: "mutating", autoApprove: false };
}

View File

@@ -400,6 +400,51 @@ describe("resolvePermissionRequest", () => {
expect(res).toEqual({ outcome: { outcome: "selected", optionId: "allow" } });
});
it.each([
{
toolName: "cron",
title: "cron: status",
rawInput: {
name: "cron",
action: "status",
},
},
{
toolName: "nodes",
title: "nodes: list",
rawInput: {
name: "nodes",
action: "list",
},
},
{
toolName: "whatsapp_login",
title: "whatsapp_login: start",
rawInput: {
name: "whatsapp_login",
},
},
] as const)(
"prompts for shared owner-only backstop tools: $toolName",
async ({ toolName, title, rawInput }) => {
const prompt = vi.fn(async () => true);
const res = await resolvePermissionRequest(
makePermissionRequest({
toolCall: {
toolCallId: `tool-${toolName}`,
title,
status: "pending",
rawInput,
},
}),
{ prompt, log: () => {} },
);
expect(prompt).toHaveBeenCalledTimes(1);
expect(prompt).toHaveBeenCalledWith(toolName, title);
expect(res).toEqual({ outcome: { outcome: "selected", optionId: "allow" } });
},
);
it("auto-approves search without prompting", async () => {
const prompt = vi.fn(async () => true);
const res = await resolvePermissionRequest(

View File

@@ -1,5 +1,6 @@
import { describe, expect, it } from "vitest";
import type { OpenClawConfig } from "../config/config.js";
import { DEFAULT_GATEWAY_HTTP_TOOL_DENY } from "../security/dangerous-tools.js";
import { isToolAllowed, resolveSandboxToolPolicyForAgent } from "./sandbox/tool-policy.js";
import type { SandboxToolPolicy } from "./sandbox/types.js";
import { TOOL_POLICY_CONFORMANCE } from "./tool-policy.conformance.js";
@@ -8,6 +9,7 @@ import {
expandToolGroups,
isOwnerOnlyToolName,
normalizeToolName,
resolveOwnerOnlyToolApprovalClass,
resolveToolProfilePolicy,
TOOL_GROUPS,
} from "./tool-policy.js";
@@ -84,6 +86,28 @@ describe("tool-policy", () => {
expect(isOwnerOnlyToolName("read")).toBe(false);
});
it("exposes stable approval classes for shared owner-only fallbacks", () => {
expect(resolveOwnerOnlyToolApprovalClass("whatsapp_login")).toBe("interactive");
expect(resolveOwnerOnlyToolApprovalClass("cron")).toBe("control_plane");
expect(resolveOwnerOnlyToolApprovalClass("gateway")).toBe("control_plane");
expect(resolveOwnerOnlyToolApprovalClass("nodes")).toBe("exec_capable");
expect(resolveOwnerOnlyToolApprovalClass("read")).toBeUndefined();
});
it("keeps ACP owner-only backstops aligned with the HTTP deny list", () => {
const sharedBackstops = DEFAULT_GATEWAY_HTTP_TOOL_DENY.flatMap((name) => {
const approvalClass = resolveOwnerOnlyToolApprovalClass(name);
return approvalClass ? ([[name, approvalClass]] as const) : [];
});
expect(Object.fromEntries(sharedBackstops)).toEqual({
cron: "control_plane",
gateway: "control_plane",
nodes: "exec_capable",
whatsapp_login: "interactive",
});
});
it("strips owner-only tools for non-owner senders", async () => {
const tools = createOwnerPolicyTools();
const filtered = applyOwnerOnlyToolPolicy(tools, false);

View File

@@ -15,6 +15,8 @@ export {
} from "./tool-policy-shared.js";
export type { ToolProfileId } from "./tool-policy-shared.js";
export type OwnerOnlyToolApprovalClass = "control_plane" | "exec_capable" | "interactive";
// Keep tool-policy browser-safe: do not import tools/common at runtime.
function wrapOwnerOnlyToolExecution(tool: AnyAgentTool, senderIsOwner: boolean): AnyAgentTool {
if (tool.ownerOnly !== true || senderIsOwner || !tool.execute) {
@@ -28,15 +30,21 @@ function wrapOwnerOnlyToolExecution(tool: AnyAgentTool, senderIsOwner: boolean):
};
}
const OWNER_ONLY_TOOL_NAME_FALLBACKS = new Set<string>([
"whatsapp_login",
"cron",
"gateway",
"nodes",
const OWNER_ONLY_TOOL_APPROVAL_CLASS_FALLBACKS = new Map<string, OwnerOnlyToolApprovalClass>([
["whatsapp_login", "interactive"],
["cron", "control_plane"],
["gateway", "control_plane"],
["nodes", "exec_capable"],
]);
export function resolveOwnerOnlyToolApprovalClass(
name: string,
): OwnerOnlyToolApprovalClass | undefined {
return OWNER_ONLY_TOOL_APPROVAL_CLASS_FALLBACKS.get(normalizeToolName(name));
}
export function isOwnerOnlyToolName(name: string) {
return OWNER_ONLY_TOOL_NAME_FALLBACKS.has(normalizeToolName(name));
return resolveOwnerOnlyToolApprovalClass(name) !== undefined;
}
function isOwnerOnlyTool(tool: AnyAgentTool) {