fix(acp): harden permission tool-name validation

This commit is contained in:
Peter Steinberger
2026-02-24 01:11:24 +00:00
parent f97c0922e1
commit 63dcd28ae0
2 changed files with 129 additions and 2 deletions

View File

@@ -87,6 +87,75 @@ describe("resolvePermissionRequest", () => {
expect(res).toEqual({ outcome: { outcome: "selected", optionId: "reject" } });
});
it("auto-approves read when rawInput path resolves inside cwd", async () => {
const prompt = vi.fn(async () => true);
const res = await resolvePermissionRequest(
makePermissionRequest({
toolCall: {
toolCallId: "tool-read-inside-cwd",
title: "read: ignored-by-raw-input",
status: "pending",
rawInput: { path: "docs/security.md" },
},
}),
{ prompt, log: () => {}, cwd: "/tmp/openclaw-acp-cwd" },
);
expect(prompt).not.toHaveBeenCalled();
expect(res).toEqual({ outcome: { outcome: "selected", optionId: "allow" } });
});
it("auto-approves read when rawInput file URL resolves inside cwd", async () => {
const prompt = vi.fn(async () => true);
const res = await resolvePermissionRequest(
makePermissionRequest({
toolCall: {
toolCallId: "tool-read-inside-cwd-file-url",
title: "read: ignored-by-raw-input",
status: "pending",
rawInput: { path: "file:///tmp/openclaw-acp-cwd/docs/security.md" },
},
}),
{ prompt, log: () => {}, cwd: "/tmp/openclaw-acp-cwd" },
);
expect(prompt).not.toHaveBeenCalled();
expect(res).toEqual({ outcome: { outcome: "selected", optionId: "allow" } });
});
it("prompts for read when rawInput path escapes cwd via traversal", async () => {
const prompt = vi.fn(async () => false);
const res = await resolvePermissionRequest(
makePermissionRequest({
toolCall: {
toolCallId: "tool-read-escape-cwd",
title: "read: ignored-by-raw-input",
status: "pending",
rawInput: { path: "../.ssh/id_rsa" },
},
}),
{ prompt, log: () => {}, cwd: "/tmp/openclaw-acp-cwd/workspace" },
);
expect(prompt).toHaveBeenCalledTimes(1);
expect(prompt).toHaveBeenCalledWith("read", "read: ignored-by-raw-input");
expect(res).toEqual({ outcome: { outcome: "selected", optionId: "reject" } });
});
it("prompts for read when scoped path is missing", async () => {
const prompt = vi.fn(async () => false);
const res = await resolvePermissionRequest(
makePermissionRequest({
toolCall: {
toolCallId: "tool-read-no-path",
title: "read",
status: "pending",
},
}),
{ prompt, log: () => {} },
);
expect(prompt).toHaveBeenCalledTimes(1);
expect(prompt).toHaveBeenCalledWith("read", "read");
expect(res).toEqual({ outcome: { outcome: "selected", optionId: "reject" } });
});
it("prompts for non-core read-like tool names", async () => {
const prompt = vi.fn(async () => false);
const res = await resolvePermissionRequest(
@@ -176,6 +245,59 @@ describe("resolvePermissionRequest", () => {
expect(res).toEqual({ outcome: { outcome: "selected", optionId: "allow" } });
});
it("prompts when metadata tool name contains invalid characters", async () => {
const prompt = vi.fn(async () => false);
const res = await resolvePermissionRequest(
makePermissionRequest({
toolCall: {
toolCallId: "tool-invalid-meta",
title: "read: src/index.ts",
status: "pending",
_meta: { toolName: "read.*" },
},
}),
{ prompt, log: () => {} },
);
expect(prompt).toHaveBeenCalledTimes(1);
expect(prompt).toHaveBeenCalledWith(undefined, "read: src/index.ts");
expect(res).toEqual({ outcome: { outcome: "selected", optionId: "reject" } });
});
it("prompts when raw input tool name exceeds max length", async () => {
const prompt = vi.fn(async () => false);
const res = await resolvePermissionRequest(
makePermissionRequest({
toolCall: {
toolCallId: "tool-long-raw",
title: "read: src/index.ts",
status: "pending",
rawInput: { toolName: "r".repeat(129) },
},
}),
{ prompt, log: () => {} },
);
expect(prompt).toHaveBeenCalledTimes(1);
expect(prompt).toHaveBeenCalledWith(undefined, "read: src/index.ts");
expect(res).toEqual({ outcome: { outcome: "selected", optionId: "reject" } });
});
it("prompts when title tool name contains non-allowed characters", async () => {
const prompt = vi.fn(async () => false);
const res = await resolvePermissionRequest(
makePermissionRequest({
toolCall: {
toolCallId: "tool-bad-title-name",
title: "read🚀: src/index.ts",
status: "pending",
},
}),
{ prompt, log: () => {} },
);
expect(prompt).toHaveBeenCalledTimes(1);
expect(prompt).toHaveBeenCalledWith(undefined, "read🚀: src/index.ts");
expect(res).toEqual({ outcome: { outcome: "selected", optionId: "reject" } });
});
it("returns cancelled when no permission options are present", async () => {
const prompt = vi.fn(async () => true);
const res = await resolvePermissionRequest(makePermissionRequest({ options: [] }), {

View File

@@ -20,6 +20,8 @@ import { DANGEROUS_ACP_TOOLS } from "../security/dangerous-tools.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<string, string>([
["read", "read"],
["search", "search"],
@@ -59,7 +61,10 @@ function readFirstStringValue(
function normalizeToolName(value: string): string | undefined {
const normalized = value.trim().toLowerCase();
if (!normalized) {
if (!normalized || normalized.length > TOOL_NAME_MAX_LENGTH) {
return undefined;
}
if (!TOOL_NAME_PATTERN.test(normalized)) {
return undefined;
}
return normalized;
@@ -70,7 +75,7 @@ function parseToolNameFromTitle(title: string | undefined | null): string | unde
return undefined;
}
const head = title.split(":", 1)[0]?.trim();
if (!head || !/^[a-zA-Z0-9._-]+$/.test(head)) {
if (!head) {
return undefined;
}
return normalizeToolName(head);