From cd5179314d0162a1c39d5e1f9081b32a84be2e24 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Tue, 31 Mar 2026 20:37:52 +0900 Subject: [PATCH] fix(acp): use semantic approval classes --- CHANGELOG.md | 1 + SECURITY.md | 2 + src/acp/approval-classifier.test.ts | 124 +++++++++++++++ src/acp/approval-classifier.ts | 234 ++++++++++++++++++++++++++++ src/acp/client.test.ts | 44 +++++- src/acp/client.ts | 206 ++---------------------- 6 files changed, 418 insertions(+), 193 deletions(-) create mode 100644 src/acp/approval-classifier.test.ts create mode 100644 src/acp/approval-classifier.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bc56268a36a..e60a3373aea7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -147,6 +147,7 @@ Docs: https://docs.openclaw.ai - Matrix/direct rooms: recover fresh auto-joined 1:1 DMs without eagerly persisting invite-only `m.direct` mappings, while keeping named, aliased, and explicitly configured rooms on the room path. (#58024) Thanks @gumadeiras. - TTS: Restore 3.28 schema compatibility and fallback observability. (#57953) Thanks @joshavant. - Memory/doctor: probe QMD availability from the agent workspace too, so `openclaw doctor` no longer falsely reports relative `memory.qmd.command` configs as broken while runtime search still works. Thanks @vincentkoc. +- 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. - Telegram/forum topics: restore reply routing to the active topic and keep ACP `sessions_spawn(..., thread=true, mode="session")` bound to that same topic instead of falling back to root chat or losing follow-up routing. (#56060) Thanks @one27001. - Config/SecretRef + Control UI: harden SecretRef redaction round-trip restore, block unsafe raw fallback (force Form mode when raw is unavailable), and preflight submitted-config SecretRefs before config write RPC persistence. (#58044) Thanks @joshavant. - Config/Telegram: migrate removed `channels.telegram.groupMentionsOnly` into `channels.telegram.groups["*"].requireMention` on load so legacy configs no longer crash at startup. (#55336) thanks @jameslcowan. diff --git a/SECURITY.md b/SECURITY.md index 33c515eef65b..3d0d2c0c3d98 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -59,6 +59,7 @@ These are frequently reported but are typically closed with no code change: - Reports that treat the Gateway HTTP compatibility endpoints (`POST /v1/chat/completions`, `POST /v1/responses`) as if they implemented scoped operator auth (`operator.write` vs `operator.admin`). These endpoints authenticate the shared Gateway bearer secret/password and are documented full operator-access surfaces, not per-user/per-scope boundaries. - Reports that assume `x-openclaw-scopes` can reduce or redefine shared-secret bearer auth on the OpenAI-compatible HTTP endpoints. For shared-secret auth (`gateway.auth.mode="token"` or `"password"`), those endpoints ignore narrower bearer-declared scopes and restore the full default operator scope set plus owner semantics. - Reports that only show differences in heuristic detection/parity (for example obfuscation-pattern detection on one exec path but not another, such as `node.invoke -> system.run` parity gaps) without demonstrating bypass of auth, approvals, allowlist enforcement, sandboxing, or other documented trust boundaries. +- Reports that only show an ACP tool can indirectly execute, mutate, orchestrate sessions, or reach another tool/runtime without demonstrating bypass of ACP prompt/approval, allowlist enforcement, sandboxing, or another documented trust boundary. ACP silent approval is intentionally limited to narrow readonly classes; parity-only indirect-command findings are hardening, not vulnerabilities. - ReDoS/DoS claims that require trusted operator configuration input (for example catastrophic regex in `sessionFilter` or `logging.redactPatterns`) without a trust-boundary bypass. - Archive/install extraction claims that require pre-existing local filesystem priming in trusted state (for example planting symlink/hardlink aliases under destination directories such as skills/tools paths) without showing an untrusted path that can create/control that primitive. - Reports that depend on replacing or rewriting an already-approved executable path on a trusted host (same-path inode/content swap) without showing an untrusted path to perform that write. @@ -136,6 +137,7 @@ Plugins/extensions are part of OpenClaw's trusted computing base for a gateway. - Any report whose only claim is that an operator-enabled `dangerous*`/`dangerously*` config option weakens defaults (these are explicit break-glass tradeoffs by design) - Reports that depend on trusted operator-supplied configuration values to trigger availability impact (for example custom regex patterns). These may still be fixed as defense-in-depth hardening, but are not security-boundary bypasses. - Reports whose only claim is heuristic/parity drift in command-risk detection (for example obfuscation-pattern checks) across exec surfaces, without a demonstrated trust-boundary bypass. These are hardening-only findings and are not vulnerabilities; triage may close them as `invalid`/`no-action` or track them separately as low/informational hardening. +- Reports whose only claim is that an ACP-exposed tool can indirectly execute commands, mutate host state, or reach another privileged tool/runtime without demonstrating a bypass of ACP prompt/approval, allowlist enforcement, sandboxing, or another documented trust boundary. These are hardening-only findings, not vulnerabilities. - Reports whose only claim is that exec approvals do not semantically model every interpreter/runtime loader form, subcommand, flag combination, package script, or transitive module/config import. Exec approvals bind exact request context and best-effort direct local file operands; they are not a complete semantic model of everything a runtime may load. - Exposed secrets that are third-party/user-controlled credentials (not OpenClaw-owned and not granting access to OpenClaw-operated infrastructure/services) without demonstrated OpenClaw impact - Reports whose only claim is host-side exec when sandbox runtime is disabled/unavailable (documented default behavior in the trusted-operator model), without a boundary bypass. diff --git a/src/acp/approval-classifier.test.ts b/src/acp/approval-classifier.test.ts new file mode 100644 index 000000000000..a6a3fe2b05a5 --- /dev/null +++ b/src/acp/approval-classifier.test.ts @@ -0,0 +1,124 @@ +import { describe, expect, it } from "vitest"; +import { classifyAcpToolApproval } from "./approval-classifier.js"; + +function classify(params: { + title: string; + rawInput?: Record; + meta?: Record; + cwd?: string; +}) { + return classifyAcpToolApproval({ + cwd: params.cwd ?? "/workspace", + toolCall: { + title: params.title, + rawInput: params.rawInput, + _meta: params.meta, + }, + }); +} + +describe("classifyAcpToolApproval", () => { + it("auto-approves scoped readonly reads", () => { + expect( + classify({ + title: "read: src/index.ts", + rawInput: { path: "src/index.ts" }, + }), + ).toEqual({ + toolName: "read", + approvalClass: "readonly_scoped", + autoApprove: true, + }); + }); + + it("does not auto-approve reads outside cwd", () => { + expect( + classify({ + title: "read: ~/.ssh/id_rsa", + rawInput: { path: "~/.ssh/id_rsa" }, + }), + ).toEqual({ + toolName: "read", + approvalClass: "other", + autoApprove: false, + }); + }); + + it("auto-approves readonly search tools", () => { + expect( + classify({ + title: "memory_search: vectors", + rawInput: { name: "memory_search", query: "vectors" }, + }), + ).toEqual({ + toolName: "memory_search", + approvalClass: "readonly_search", + autoApprove: true, + }); + }); + + it("classifies process as exec-capable even for readonly-like actions", () => { + expect( + classify({ + title: "process: list", + rawInput: { name: "process", action: "list" }, + }), + ).toEqual({ + toolName: "process", + approvalClass: "exec_capable", + autoApprove: false, + }); + }); + + 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("classifies gateway as control-plane", () => { + expect( + classify({ + title: "gateway: status", + rawInput: { name: "gateway", action: "status" }, + }), + ).toEqual({ + toolName: "gateway", + approvalClass: "control_plane", + autoApprove: false, + }); + }); + + it("classifies mutating messaging tools as mutating", () => { + expect( + classify({ + title: "message: send", + rawInput: { name: "message", action: "send", message: "hi" }, + }), + ).toEqual({ + toolName: "message", + approvalClass: "mutating", + autoApprove: false, + }); + }); + + it("fails closed on spoofed metadata and title mismatches", () => { + expect( + classify({ + title: "exec: uname -a", + rawInput: { name: "search", query: "uname -a" }, + }), + ).toEqual({ + toolName: undefined, + approvalClass: "unknown", + autoApprove: false, + }); + }); +}); diff --git a/src/acp/approval-classifier.ts b/src/acp/approval-classifier.ts new file mode 100644 index 000000000000..f28d116a83a1 --- /dev/null +++ b/src/acp/approval-classifier.ts @@ -0,0 +1,234 @@ +import { homedir } from "node:os"; +import path from "node:path"; +import { isKnownCoreToolId } from "../agents/tool-catalog.js"; +import { isMutatingToolCall } from "../agents/tool-mutation.js"; + +const SAFE_SEARCH_TOOL_IDS = new Set(["search", "web_search", "memory_search"]); +const TRUSTED_SAFE_TOOL_ALIASES = new Set(["search"]); +const EXEC_CAPABLE_TOOL_IDS = new Set([ + "exec", + "spawn", + "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"]); + +export type AcpApprovalClass = + | "readonly_scoped" + | "readonly_search" + | "mutating" + | "exec_capable" + | "control_plane" + | "interactive" + | "other" + | "unknown"; + +export type AcpApprovalClassification = { + toolName?: string; + approvalClass: AcpApprovalClass; + autoApprove: boolean; +}; + +function asRecord(value: unknown): Record | undefined { + return value && typeof value === "object" && !Array.isArray(value) + ? (value as Record) + : undefined; +} + +function readFirstStringValue( + source: Record | undefined, + keys: string[], +): string | undefined { + if (!source) { + return undefined; + } + for (const key of keys) { + const value = source[key]; + if (typeof value === "string" && value.trim()) { + return value.trim(); + } + } + return undefined; +} + +function normalizeToolName(value: string): string | undefined { + const normalized = value.trim().toLowerCase(); + if (!normalized || normalized.length > 128) { + return undefined; + } + return /^[a-z0-9._-]+$/.test(normalized) ? normalized : undefined; +} + +function parseToolNameFromTitle(title: string | undefined | null): string | undefined { + if (!title) { + return undefined; + } + const head = title.split(":", 1)[0]?.trim(); + return head ? normalizeToolName(head) : undefined; +} + +export function resolveToolNameForPermission(params: { + toolCall?: { + title?: string | null; + _meta?: unknown; + rawInput?: unknown; + }; +}): string | undefined { + const toolCall = params.toolCall; + const toolMeta = asRecord(toolCall?._meta); + const rawInput = asRecord(toolCall?.rawInput); + + const fromMeta = readFirstStringValue(toolMeta, ["toolName", "tool_name", "name"]); + const fromRawInput = readFirstStringValue(rawInput, ["tool", "toolName", "tool_name", "name"]); + const fromTitle = parseToolNameFromTitle(toolCall?.title); + const metaName = fromMeta ? normalizeToolName(fromMeta) : undefined; + const rawInputName = fromRawInput ? normalizeToolName(fromRawInput) : undefined; + const titleName = fromTitle; + if ((fromMeta && !metaName) || (fromRawInput && !rawInputName)) { + return undefined; + } + if (metaName && titleName && metaName !== titleName) { + return undefined; + } + if (rawInputName && metaName && rawInputName !== metaName) { + return undefined; + } + if (rawInputName && titleName && rawInputName !== titleName) { + return undefined; + } + return metaName ?? titleName ?? rawInputName; +} + +function extractPathFromToolTitle( + toolTitle: string | undefined, + toolName: string | undefined, +): string | undefined { + if (!toolTitle) { + return undefined; + } + const separator = toolTitle.indexOf(":"); + if (separator < 0) { + return undefined; + } + const tail = toolTitle.slice(separator + 1).trim(); + if (!tail) { + return undefined; + } + const keyedMatch = tail.match(/(?:^|,\s*)(?:path|file_path|filePath)\s*:\s*([^,]+)/); + if (keyedMatch?.[1]) { + return keyedMatch[1].trim(); + } + return toolName === "read" ? tail : undefined; +} + +function resolveToolPathCandidate( + params: { + toolCall?: { rawInput?: unknown }; + }, + toolName: string | undefined, + toolTitle: string | undefined, +): string | undefined { + const rawInput = asRecord(params.toolCall?.rawInput); + return ( + readFirstStringValue(rawInput, ["path", "file_path", "filePath"]) ?? + extractPathFromToolTitle(toolTitle, toolName) + ); +} + +function resolveAbsoluteScopedPath(value: string, cwd: string): string | undefined { + let candidate = value.trim(); + if (!candidate) { + return undefined; + } + if (candidate.startsWith("file://")) { + try { + const parsed = new URL(candidate); + candidate = decodeURIComponent(parsed.pathname || ""); + } catch { + return undefined; + } + } + if (candidate === "~") { + candidate = homedir(); + } else if (candidate.startsWith("~/")) { + candidate = path.join(homedir(), candidate.slice(2)); + } + return path.isAbsolute(candidate) ? path.normalize(candidate) : path.resolve(cwd, candidate); +} + +function isReadToolCallScopedToCwd( + params: { toolCall?: { rawInput?: unknown } }, + toolName: string | undefined, + toolTitle: string | undefined, + cwd: string, +): boolean { + if (toolName !== "read") { + return false; + } + const rawPath = resolveToolPathCandidate(params, toolName, toolTitle); + if (!rawPath) { + return false; + } + const absolutePath = resolveAbsoluteScopedPath(rawPath, cwd); + if (!absolutePath) { + return false; + } + const root = path.resolve(cwd); + const relative = path.relative(root, absolutePath); + return relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative)); +} + +export function classifyAcpToolApproval(params: { + toolCall?: { + title?: string | null; + _meta?: unknown; + rawInput?: unknown; + }; + cwd: string; +}): AcpApprovalClassification { + const toolName = resolveToolNameForPermission(params); + if (!toolName) { + return { toolName: undefined, approvalClass: "unknown", autoApprove: false }; + } + + const isTrustedToolId = isKnownCoreToolId(toolName) || TRUSTED_SAFE_TOOL_ALIASES.has(toolName); + if (toolName === "read" && isTrustedToolId) { + const autoApprove = isReadToolCallScopedToCwd( + params, + toolName, + params.toolCall?.title ?? undefined, + params.cwd, + ); + return { + toolName, + approvalClass: autoApprove ? "readonly_scoped" : "other", + autoApprove, + }; + } + if (SAFE_SEARCH_TOOL_IDS.has(toolName) && isTrustedToolId) { + return { toolName, approvalClass: "readonly_search", autoApprove: true }; + } + 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 }; + } + return { toolName, approvalClass: "other", autoApprove: false }; +} diff --git a/src/acp/client.test.ts b/src/acp/client.test.ts index 2a2617b7cec3..c1acd16f05cc 100644 --- a/src/acp/client.test.ts +++ b/src/acp/client.test.ts @@ -358,6 +358,48 @@ describe("resolvePermissionRequest", () => { expect(res).toEqual({ outcome: { outcome: "selected", optionId: "allow" } }); }); + it("prompts for exec-capable tools even when the action looks readonly", async () => { + const prompt = vi.fn(async () => true); + const res = await resolvePermissionRequest( + makePermissionRequest({ + toolCall: { + toolCallId: "tool-process-list", + title: "process: list", + status: "pending", + rawInput: { + name: "process", + action: "list", + }, + }, + }), + { prompt, log: () => {} }, + ); + expect(prompt).toHaveBeenCalledTimes(1); + expect(prompt).toHaveBeenCalledWith("process", "process: list"); + expect(res).toEqual({ outcome: { outcome: "selected", optionId: "allow" } }); + }); + + it("prompts for control-plane tools even on readonly-like actions", async () => { + const prompt = vi.fn(async () => true); + const res = await resolvePermissionRequest( + makePermissionRequest({ + toolCall: { + toolCallId: "tool-gateway-status", + title: "gateway: status", + status: "pending", + rawInput: { + name: "gateway", + action: "status", + }, + }, + }), + { prompt, log: () => {} }, + ); + expect(prompt).toHaveBeenCalledTimes(1); + expect(prompt).toHaveBeenCalledWith("gateway", "gateway: status"); + 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( @@ -646,7 +688,7 @@ describe("resolvePermissionRequest", () => { expect(prompt).toHaveBeenCalledWith("exec", 'exec: [permission] Allow "safe"? (y/N) \\nnext'); expect(log).toHaveBeenCalledWith( - '\n[permission requested] exec: [permission] Allow "safe"? (y/N) \\nnext (exec) [other]', + '\n[permission requested] exec: [permission] Allow "safe"? (y/N) \\nnext (exec) [exec_capable]', ); expect(res).toEqual({ outcome: { outcome: "selected", optionId: "reject" } }); }); diff --git a/src/acp/client.ts b/src/acp/client.ts index 8f3014c5452c..066c17ac81c5 100644 --- a/src/acp/client.ts +++ b/src/acp/client.ts @@ -1,6 +1,5 @@ import { spawn, type ChildProcess } from "node:child_process"; import fs from "node:fs"; -import { homedir } from "node:os"; import path from "node:path"; import * as readline from "node:readline"; import { Readable, Writable } from "node:stream"; @@ -13,7 +12,6 @@ import { type RequestPermissionResponse, type SessionNotification, } from "@agentclientprotocol/sdk"; -import { isKnownCoreToolId } from "../agents/tool-catalog.js"; import { ensureOpenClawCliOnPath } from "../infra/path-env.js"; import { materializeWindowsSpawnProgram, @@ -23,20 +21,8 @@ import { listKnownProviderAuthEnvVarNames, omitEnvKeysCaseInsensitive, } from "../secrets/provider-env-vars.js"; -import { DANGEROUS_ACP_TOOLS } from "../security/dangerous-tools.js"; import { sanitizeTerminalText } from "../terminal/safe-text.js"; - -const SAFE_AUTO_APPROVE_TOOL_IDS = new Set(["read", "search", "web_search", "memory_search"]); -const TRUSTED_SAFE_TOOL_ALIASES = new Set(["search"]); -const READ_TOOL_PATH_KEYS = ["path", "file_path", "filePath"]; -const TOOL_NAME_MAX_LENGTH = 128; -const TOOL_NAME_PATTERN = /^[a-z0-9._-]+$/; -const TOOL_KIND_BY_ID = new Map([ - ["read", "read"], - ["search", "search"], - ["web_search", "search"], - ["memory_search", "search"], -]); +import { classifyAcpToolApproval, type AcpApprovalClass } from "./approval-classifier.js"; type PermissionOption = RequestPermissionRequest["options"][number]; @@ -46,184 +32,20 @@ type PermissionResolverDeps = { cwd?: string; }; -function asRecord(value: unknown): Record | undefined { - return value && typeof value === "object" && !Array.isArray(value) - ? (value as Record) - : undefined; -} - -function readFirstStringValue( - source: Record | undefined, - keys: string[], +function resolveToolKindForPermission( + toolName: string | undefined, + approvalClass: AcpApprovalClass, ): string | undefined { - if (!source) { + if (!toolName && approvalClass === "unknown") { return undefined; } - for (const key of keys) { - const value = source[key]; - if (typeof value === "string" && value.trim()) { - return value.trim(); - } + if (approvalClass === "readonly_scoped") { + return "readonly_scoped"; } - return undefined; -} - -function normalizeToolName(value: string): string | undefined { - const normalized = value.trim().toLowerCase(); - if (!normalized || normalized.length > TOOL_NAME_MAX_LENGTH) { - return undefined; + if (approvalClass === "readonly_search") { + return "readonly_search"; } - if (!TOOL_NAME_PATTERN.test(normalized)) { - return undefined; - } - return normalized; -} - -function parseToolNameFromTitle(title: string | undefined | null): string | undefined { - if (!title) { - return undefined; - } - const head = title.split(":", 1)[0]?.trim(); - if (!head) { - return undefined; - } - return normalizeToolName(head); -} - -function resolveToolKindForPermission(toolName: string | undefined): string | undefined { - if (!toolName) { - return undefined; - } - return TOOL_KIND_BY_ID.get(toolName) ?? "other"; -} - -function resolveToolNameForPermission(params: RequestPermissionRequest): string | undefined { - const toolCall = params.toolCall; - const toolMeta = asRecord(toolCall?._meta); - const rawInput = asRecord(toolCall?.rawInput); - - const fromMeta = readFirstStringValue(toolMeta, ["toolName", "tool_name", "name"]); - const fromRawInput = readFirstStringValue(rawInput, ["tool", "toolName", "tool_name", "name"]); - const fromTitle = parseToolNameFromTitle(toolCall?.title); - const metaName = fromMeta ? normalizeToolName(fromMeta) : undefined; - const rawInputName = fromRawInput ? normalizeToolName(fromRawInput) : undefined; - const titleName = fromTitle; - if ((fromMeta && !metaName) || (fromRawInput && !rawInputName)) { - return undefined; - } - if (metaName && titleName && metaName !== titleName) { - return undefined; - } - if (rawInputName && metaName && rawInputName !== metaName) { - return undefined; - } - if (rawInputName && titleName && rawInputName !== titleName) { - return undefined; - } - return metaName ?? titleName ?? rawInputName; -} - -function extractPathFromToolTitle( - toolTitle: string | undefined, - toolName: string | undefined, -): string | undefined { - if (!toolTitle) { - return undefined; - } - const separator = toolTitle.indexOf(":"); - if (separator < 0) { - return undefined; - } - const tail = toolTitle.slice(separator + 1).trim(); - if (!tail) { - return undefined; - } - const keyedMatch = tail.match(/(?:^|,\s*)(?:path|file_path|filePath)\s*:\s*([^,]+)/); - if (keyedMatch?.[1]) { - return keyedMatch[1].trim(); - } - if (toolName === "read") { - return tail; - } - return undefined; -} - -function resolveToolPathCandidate( - params: RequestPermissionRequest, - toolName: string | undefined, - toolTitle: string | undefined, -): string | undefined { - const rawInput = asRecord(params.toolCall?.rawInput); - const fromRawInput = readFirstStringValue(rawInput, READ_TOOL_PATH_KEYS); - const fromTitle = extractPathFromToolTitle(toolTitle, toolName); - return fromRawInput ?? fromTitle; -} - -function resolveAbsoluteScopedPath(value: string, cwd: string): string | undefined { - let candidate = value.trim(); - if (!candidate) { - return undefined; - } - if (candidate.startsWith("file://")) { - try { - const parsed = new URL(candidate); - candidate = decodeURIComponent(parsed.pathname || ""); - } catch { - return undefined; - } - } - if (candidate === "~") { - candidate = homedir(); - } else if (candidate.startsWith("~/")) { - candidate = path.join(homedir(), candidate.slice(2)); - } - const absolute = path.isAbsolute(candidate) - ? path.normalize(candidate) - : path.resolve(cwd, candidate); - return absolute; -} - -function isPathWithinRoot(candidatePath: string, root: string): boolean { - const relative = path.relative(root, candidatePath); - return relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative)); -} - -function isReadToolCallScopedToCwd( - params: RequestPermissionRequest, - toolName: string | undefined, - toolTitle: string | undefined, - cwd: string, -): boolean { - if (toolName !== "read") { - return false; - } - const rawPath = resolveToolPathCandidate(params, toolName, toolTitle); - if (!rawPath) { - return false; - } - const absolutePath = resolveAbsoluteScopedPath(rawPath, cwd); - if (!absolutePath) { - return false; - } - return isPathWithinRoot(absolutePath, path.resolve(cwd)); -} - -function shouldAutoApproveToolCall( - params: RequestPermissionRequest, - toolName: string | undefined, - toolTitle: string | undefined, - cwd: string, -): boolean { - const isTrustedToolId = - typeof toolName === "string" && - (isKnownCoreToolId(toolName) || TRUSTED_SAFE_TOOL_ALIASES.has(toolName)); - if (!toolName || !isTrustedToolId || !SAFE_AUTO_APPROVE_TOOL_IDS.has(toolName)) { - return false; - } - if (toolName === "read") { - return isReadToolCallScopedToCwd(params, toolName, toolTitle, cwd); - } - return true; + return approvalClass; } function pickOption( @@ -296,8 +118,9 @@ export async function resolvePermissionRequest( const cwd = deps.cwd ?? process.cwd(); const options = params.options ?? []; const toolTitle = sanitizeTerminalText(params.toolCall?.title ?? "tool"); - const toolName = resolveToolNameForPermission(params); - const toolKind = resolveToolKindForPermission(toolName); + const classification = classifyAcpToolApproval({ toolCall: params.toolCall, cwd }); + const toolName = classification.toolName; + const toolKind = resolveToolKindForPermission(toolName, classification.approvalClass); if (options.length === 0) { log(`[permission cancelled] ${toolName ?? "unknown"}: no options available`); @@ -306,8 +129,7 @@ export async function resolvePermissionRequest( const allowOption = pickOption(options, ["allow_once", "allow_always"]); const rejectOption = pickOption(options, ["reject_once", "reject_always"]); - const autoApproveAllowed = shouldAutoApproveToolCall(params, toolName, toolTitle, cwd); - const promptRequired = !toolName || !autoApproveAllowed || DANGEROUS_ACP_TOOLS.has(toolName); + const promptRequired = !classification.autoApprove; if (!promptRequired) { const option = allowOption ?? options[0];