fix(acp): prompt for non-read/search permissions

This commit is contained in:
Peter Steinberger
2026-02-14 12:53:27 +01:00
parent 9e24eee52c
commit bb1c3dfe10
2 changed files with 88 additions and 3 deletions

View File

@@ -48,6 +48,43 @@ describe("resolvePermissionRequest", () => {
expect(res).toEqual({ outcome: { outcome: "selected", optionId: "allow" } });
});
it("prompts for non-read/search tools (write)", async () => {
const prompt = vi.fn(async () => true);
const res = await resolvePermissionRequest(
makePermissionRequest({
toolCall: { toolCallId: "tool-w", title: "write: /tmp/pwn", status: "pending" },
}),
{ prompt, log: () => {} },
);
expect(prompt).toHaveBeenCalledTimes(1);
expect(prompt).toHaveBeenCalledWith("write", "write: /tmp/pwn");
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(
makePermissionRequest({
toolCall: { toolCallId: "tool-s", title: "search: foo", status: "pending" },
}),
{ prompt, log: () => {} },
);
expect(res).toEqual({ outcome: { outcome: "selected", optionId: "allow" } });
expect(prompt).not.toHaveBeenCalled();
});
it("prompts for fetch even when tool name is known", async () => {
const prompt = vi.fn(async () => false);
const res = await resolvePermissionRequest(
makePermissionRequest({
toolCall: { toolCallId: "tool-f", title: "fetch: https://example.com", status: "pending" },
}),
{ prompt, log: () => {} },
);
expect(prompt).toHaveBeenCalledTimes(1);
expect(res).toEqual({ outcome: { outcome: "selected", optionId: "reject" } });
});
it("uses allow_always and reject_always when once options are absent", async () => {
const options: RequestPermissionRequest["options"] = [
{ kind: "allow_always", name: "Always allow", optionId: "allow-always" },

View File

@@ -29,6 +29,8 @@ const DANGEROUS_ACP_TOOLS = new Set([
"apply_patch",
]);
const SAFE_AUTO_APPROVE_KINDS = new Set(["read", "search"]);
type PermissionOption = RequestPermissionRequest["options"][number];
type PermissionResolverDeps = {
@@ -77,6 +79,48 @@ function parseToolNameFromTitle(title: string | undefined | null): string | unde
return normalizeToolName(head);
}
function resolveToolKindForPermission(
params: RequestPermissionRequest,
toolName: string | undefined,
): string | undefined {
const toolCall = params.toolCall as unknown as { kind?: unknown; title?: unknown } | undefined;
const kindRaw = typeof toolCall?.kind === "string" ? toolCall.kind.trim().toLowerCase() : "";
if (kindRaw) {
return kindRaw;
}
const name =
toolName ??
parseToolNameFromTitle(typeof toolCall?.title === "string" ? toolCall.title : undefined);
if (!name) {
return undefined;
}
const normalized = name.toLowerCase();
// Prefer a conservative classifier: if in doubt, return "other" (prompt-required).
if (normalized === "read" || normalized.includes("read")) {
return "read";
}
if (normalized === "search" || normalized.includes("search") || normalized.includes("find")) {
return "search";
}
if (normalized.includes("fetch") || normalized.includes("http")) {
return "fetch";
}
if (normalized.includes("write") || normalized.includes("edit") || normalized.includes("patch")) {
return "edit";
}
if (normalized.includes("delete") || normalized.includes("remove")) {
return "delete";
}
if (normalized.includes("move") || normalized.includes("rename")) {
return "move";
}
if (normalized.includes("exec") || normalized.includes("run") || normalized.includes("bash")) {
return "execute";
}
return "other";
}
function resolveToolNameForPermission(params: RequestPermissionRequest): string | undefined {
const toolCall = params.toolCall;
const toolMeta = asRecord(toolCall?._meta);
@@ -158,6 +202,7 @@ export async function resolvePermissionRequest(
const options = params.options ?? [];
const toolTitle = params.toolCall?.title ?? "tool";
const toolName = resolveToolNameForPermission(params);
const toolKind = resolveToolKindForPermission(params, toolName);
if (options.length === 0) {
log(`[permission cancelled] ${toolName ?? "unknown"}: no options available`);
@@ -166,7 +211,8 @@ export async function resolvePermissionRequest(
const allowOption = pickOption(options, ["allow_once", "allow_always"]);
const rejectOption = pickOption(options, ["reject_once", "reject_always"]);
const promptRequired = !toolName || DANGEROUS_ACP_TOOLS.has(toolName);
const isSafeKind = Boolean(toolKind && SAFE_AUTO_APPROVE_KINDS.has(toolKind));
const promptRequired = !toolName || !isSafeKind || DANGEROUS_ACP_TOOLS.has(toolName);
if (!promptRequired) {
const option = allowOption ?? options[0];
@@ -174,11 +220,13 @@ export async function resolvePermissionRequest(
log(`[permission cancelled] ${toolName}: no selectable options`);
return cancelledPermission();
}
log(`[permission auto-approved] ${toolName}`);
log(`[permission auto-approved] ${toolName} (${toolKind ?? "unknown"})`);
return selectedPermission(option.optionId);
}
log(`\n[permission requested] ${toolTitle}${toolName ? ` (${toolName})` : ""}`);
log(
`\n[permission requested] ${toolTitle}${toolName ? ` (${toolName})` : ""}${toolKind ? ` [${toolKind}]` : ""}`,
);
const approved = await prompt(toolName, toolTitle);
if (approved && allowOption) {