Skip to content

Commit 1ca7687

Browse files
Janni TurunenJanni Turunen
authored andcommitted
fix: properly filter tools by agent permission rules
- Check permission array for tool-specific rules - Only filter out tools where ALL rules are deny - Support catch-all deny for tools without explicit rules - Add comprehensive test suite (12 tests) Fixes anomalyco#6527
1 parent f109c78 commit 1ca7687

2 files changed

Lines changed: 276 additions & 40 deletions

File tree

packages/opencode/src/tool/registry.ts

Lines changed: 46 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -116,44 +116,50 @@ export namespace ToolRegistry {
116116
return all().then((x) => x.map((t) => t.id))
117117
}
118118

119-
export async function tools(providerID: string, agent?: Agent.Info) {
120-
const tools = await all()
121-
const result = await Promise.all(
122-
tools
123-
.filter((t) => {
124-
// Enable websearch/codesearch for zen users OR via enable flag
125-
if (t.id === "codesearch" || t.id === "websearch") {
126-
return providerID === "opencode" || Flag.OPENCODE_ENABLE_EXA
127-
}
128-
129-
// Filter based on agent permissions if provided
130-
if (agent?.permission) {
131-
const toolPermission = (agent.permission as unknown as Record<string, unknown>)[t.id]
132-
133-
// If permission is an object (has command-level rules), allow the tool
134-
// Command restrictions are enforced at execution time
135-
if (toolPermission !== null && typeof toolPermission === "object") {
136-
return true
137-
}
138-
139-
// If permission is explicitly "deny" string, filter out the tool
140-
if (toolPermission === "deny") {
141-
return false
142-
}
143-
144-
// "allow", "ask", or undefined → allow the tool
145-
}
146-
147-
return true
148-
})
149-
.map(async (t) => {
150-
using _ = log.time(t.id)
151-
return {
152-
id: t.id,
153-
...(await t.init({ agent })),
154-
}
155-
}),
156-
)
157-
return result
158-
}
119+
export async function tools(providerID: string, agent?: Agent.Info) {
120+
const tools = await all()
121+
const result = await Promise.all(
122+
tools
123+
.filter((t) => {
124+
// Enable websearch/codesearch for zen users OR via enable flag
125+
if (t.id === "codesearch" || t.id === "websearch") {
126+
return providerID === "opencode" || Flag.OPENCODE_ENABLE_EXA
127+
}
128+
129+
// Filter based on agent permissions if provided
130+
if (agent?.permission && Array.isArray(agent.permission) && agent.permission.length > 0) {
131+
// Get all rules for this specific tool
132+
// Note: pattern field is intentionally ignored here - patterns are checked
133+
// at command execution time, not tool availability time
134+
const toolRules = agent.permission.filter(
135+
(rule: PermissionNext.Rule) => rule.permission === t.id
136+
)
137+
138+
if (toolRules.length > 0) {
139+
// Tool has explicit rules - only filter out if ALL rules are "deny"
140+
const allDeny = toolRules.every((rule: PermissionNext.Rule) => rule.action === "deny")
141+
return !allDeny
142+
}
143+
144+
// No specific rules for this tool - check for catch-all deny
145+
const hasCatchAllDeny = agent.permission.some(
146+
(rule: PermissionNext.Rule) => rule.permission === "*" && rule.action === "deny"
147+
)
148+
if (hasCatchAllDeny) {
149+
return false
150+
}
151+
}
152+
153+
return true
154+
})
155+
.map(async (t) => {
156+
using _ = log.time(t.id)
157+
return {
158+
id: t.id,
159+
...(await t.init({ agent })),
160+
}
161+
}),
162+
)
163+
return result
164+
}
159165
}
Lines changed: 230 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,230 @@
1+
import { describe, expect, test } from "bun:test"
2+
import path from "path"
3+
import { ToolRegistry } from "../../src/tool/registry"
4+
import { Instance } from "../../src/project/instance"
5+
6+
const projectRoot = path.join(__dirname, "../..")
7+
8+
describe("ToolRegistry.tools() permission filtering", () => {
9+
test("filters out tool with all-deny rules", async () => {
10+
await Instance.provide({
11+
directory: projectRoot,
12+
fn: async () => {
13+
const agent = {
14+
name: "test-agent",
15+
mode: "subagent" as const,
16+
permission: [
17+
{ permission: "bash", pattern: "*", action: "deny" as const }
18+
],
19+
options: {}
20+
}
21+
22+
const result = await ToolRegistry.tools("anthropic", agent as any)
23+
const toolIds = result.map(t => t.id)
24+
25+
expect(toolIds).not.toContain("bash")
26+
},
27+
})
28+
})
29+
30+
test("includes tool with mixed allow/deny rules", async () => {
31+
await Instance.provide({
32+
directory: projectRoot,
33+
fn: async () => {
34+
const agent = {
35+
name: "test-agent",
36+
mode: "subagent" as const,
37+
permission: [
38+
{ permission: "bash", pattern: "git*", action: "allow" as const },
39+
{ permission: "bash", pattern: "*", action: "deny" as const }
40+
],
41+
options: {}
42+
}
43+
44+
const result = await ToolRegistry.tools("anthropic", agent as any)
45+
const toolIds = result.map(t => t.id)
46+
47+
expect(toolIds).toContain("bash")
48+
},
49+
})
50+
})
51+
52+
test("catch-all deny filters tools without explicit rules", async () => {
53+
await Instance.provide({
54+
directory: projectRoot,
55+
fn: async () => {
56+
const agent = {
57+
name: "test-agent",
58+
mode: "subagent" as const,
59+
permission: [
60+
{ permission: "bash", pattern: "*", action: "allow" as const },
61+
{ permission: "*", pattern: "*", action: "deny" as const }
62+
],
63+
options: {}
64+
}
65+
66+
const result = await ToolRegistry.tools("anthropic", agent as any)
67+
const toolIds = result.map(t => t.id)
68+
69+
expect(toolIds).toContain("bash")
70+
expect(toolIds).not.toContain("edit")
71+
},
72+
})
73+
})
74+
75+
test("includes all tools when permission is empty array", async () => {
76+
await Instance.provide({
77+
directory: projectRoot,
78+
fn: async () => {
79+
const agent = {
80+
name: "test-agent",
81+
mode: "subagent" as const,
82+
permission: [],
83+
options: {}
84+
}
85+
86+
const result = await ToolRegistry.tools("anthropic", agent as any)
87+
88+
expect(result.length).toBeGreaterThan(0)
89+
},
90+
})
91+
})
92+
93+
test("includes all tools when agent has no permission field", async () => {
94+
await Instance.provide({
95+
directory: projectRoot,
96+
fn: async () => {
97+
// When no agent is passed, all tools should be included
98+
const result = await ToolRegistry.tools("anthropic", undefined)
99+
100+
expect(result.length).toBeGreaterThan(0)
101+
},
102+
})
103+
})
104+
105+
test("returns bash tool when no permissions set", async () => {
106+
await Instance.provide({
107+
directory: projectRoot,
108+
fn: async () => {
109+
const result = await ToolRegistry.tools("anthropic")
110+
111+
const bashTool = result.find(t => t.id === "bash")
112+
expect(bashTool).toBeDefined()
113+
},
114+
})
115+
})
116+
117+
test("returns edit tool when no permissions set", async () => {
118+
await Instance.provide({
119+
directory: projectRoot,
120+
fn: async () => {
121+
const result = await ToolRegistry.tools("anthropic")
122+
123+
const editTool = result.find(t => t.id === "edit")
124+
expect(editTool).toBeDefined()
125+
},
126+
})
127+
})
128+
129+
test("tool has proper structure with description and parameters", async () => {
130+
await Instance.provide({
131+
directory: projectRoot,
132+
fn: async () => {
133+
const result = await ToolRegistry.tools("anthropic")
134+
135+
const bashTool = result.find(t => t.id === "bash")
136+
expect(bashTool).toBeDefined()
137+
expect(bashTool?.description).toBeDefined()
138+
expect(typeof bashTool?.description).toBe("string")
139+
expect(bashTool?.parameters).toBeDefined()
140+
},
141+
})
142+
})
143+
144+
test("handles allow rule for specific tool", async () => {
145+
await Instance.provide({
146+
directory: projectRoot,
147+
fn: async () => {
148+
const agent = {
149+
name: "test-agent",
150+
mode: "subagent" as const,
151+
permission: [
152+
{ permission: "bash", pattern: "*", action: "allow" as const }
153+
],
154+
options: {}
155+
}
156+
157+
const result = await ToolRegistry.tools("anthropic", agent as any)
158+
const toolIds = result.map(t => t.id)
159+
160+
expect(toolIds).toContain("bash")
161+
},
162+
})
163+
})
164+
165+
test("ask action is treated as allow", async () => {
166+
await Instance.provide({
167+
directory: projectRoot,
168+
fn: async () => {
169+
const agent = {
170+
name: "test-agent",
171+
mode: "subagent" as const,
172+
permission: [
173+
{ permission: "bash", pattern: "*", action: "ask" as const }
174+
],
175+
options: {}
176+
}
177+
178+
const result = await ToolRegistry.tools("anthropic", agent as any)
179+
const toolIds = result.map(t => t.id)
180+
181+
expect(toolIds).toContain("bash")
182+
},
183+
})
184+
})
185+
186+
test("empty permission array bypasses filtering", async () => {
187+
await Instance.provide({
188+
directory: projectRoot,
189+
fn: async () => {
190+
const allToolsResult = await ToolRegistry.tools("anthropic")
191+
192+
const agent = {
193+
name: "test-agent",
194+
mode: "subagent" as const,
195+
permission: [],
196+
options: {}
197+
}
198+
199+
const filteredResult = await ToolRegistry.tools("anthropic", agent as any)
200+
201+
expect(filteredResult.length).toBe(allToolsResult.length)
202+
},
203+
})
204+
})
205+
206+
test("multiple specific allow rules work correctly", async () => {
207+
await Instance.provide({
208+
directory: projectRoot,
209+
fn: async () => {
210+
const agent = {
211+
name: "test-agent",
212+
mode: "subagent" as const,
213+
permission: [
214+
{ permission: "bash", pattern: "*", action: "allow" as const },
215+
{ permission: "read", pattern: "*", action: "allow" as const },
216+
{ permission: "edit", pattern: "*", action: "deny" as const }
217+
],
218+
options: {}
219+
}
220+
221+
const result = await ToolRegistry.tools("anthropic", agent as any)
222+
const toolIds = result.map(t => t.id)
223+
224+
expect(toolIds).toContain("bash")
225+
expect(toolIds).toContain("read")
226+
expect(toolIds).not.toContain("edit")
227+
},
228+
})
229+
})
230+
})

0 commit comments

Comments
 (0)