From 1898bc6574ddcc06a8f8535fd7817682e7849c62 Mon Sep 17 00:00:00 2001 From: Dax Raad Date: Thu, 18 Dec 2025 00:09:42 -0500 Subject: [PATCH] sync --- .opencode/opencode.jsonc | 3 + .../cmd/tui/component/dialog-permission.tsx | 6 +- packages/opencode/src/config/config.ts | 12 +- packages/opencode/src/permission/next.ts | 90 ++++++++----- packages/opencode/src/session/llm.ts | 6 +- packages/opencode/src/tool/registry.ts | 19 ++- packages/opencode/src/tool/write.ts | 28 ++-- .../opencode/test/permission/next.test.ts | 112 +++++++++++++++- packages/opencode/test/tool/registry.test.ts | 120 ++++++++++++++++++ 9 files changed, 318 insertions(+), 78 deletions(-) create mode 100644 packages/opencode/test/tool/registry.test.ts diff --git a/.opencode/opencode.jsonc b/.opencode/opencode.jsonc index cbcbb0c65..a4fe2ae2e 100644 --- a/.opencode/opencode.jsonc +++ b/.opencode/opencode.jsonc @@ -5,6 +5,9 @@ // "url": "https://enterprise.dev.opencode.ai", // }, "instructions": ["STYLE_GUIDE.md"], + "permission": { + "*": "ask", + }, "provider": { "opencode": { "options": {}, diff --git a/packages/opencode/src/cli/cmd/tui/component/dialog-permission.tsx b/packages/opencode/src/cli/cmd/tui/component/dialog-permission.tsx index 91be5c093..51ce27b77 100644 --- a/packages/opencode/src/cli/cmd/tui/component/dialog-permission.tsx +++ b/packages/opencode/src/cli/cmd/tui/component/dialog-permission.tsx @@ -5,11 +5,7 @@ import { useTheme } from "../context/theme" export function Permission() { const dialog = useDialog() - onMount(() => { - setTimeout(() => { - dialog.replace(() => ) - }, 2000) - }) + onMount(() => {}) return null } diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index 9f8c312a7..e4ecba155 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -404,6 +404,7 @@ export namespace Config { external_directory: PermissionRule.optional(), todowrite: PermissionAction.optional(), todoread: PermissionAction.optional(), + webfetch: PermissionAction.optional(), websearch: PermissionAction.optional(), codesearch: PermissionAction.optional(), doom_loop: PermissionAction.optional(), @@ -451,7 +452,6 @@ export namespace Config { .catchall(z.any()) .transform((agent) => { const knownKeys = new Set([ - "name", "model", "prompt", "description", @@ -485,9 +485,13 @@ export namespace Config { } } - return { ...agent, options, permission } as typeof agent & { - options: Record - permission: Permission + // Convert legacy maxSteps to steps + const steps = agent.steps ?? agent.maxSteps + + return { ...agent, options, permission, steps } as typeof agent & { + options?: Record + permission?: Permission + steps?: number } }) .meta({ diff --git a/packages/opencode/src/permission/next.ts b/packages/opencode/src/permission/next.ts index d9ed02c5b..84e349356 100644 --- a/packages/opencode/src/permission/next.ts +++ b/packages/opencode/src/permission/next.ts @@ -4,10 +4,14 @@ import { Config } from "@/config/config" import { Identifier } from "@/id/id" import { Instance } from "@/project/instance" import { fn } from "@/util/fn" +import { Log } from "@/util/log" import { Wildcard } from "@/util/wildcard" +import { sortBy } from "remeda" import z from "zod" export namespace PermissionNext { + const log = Log.create({ service: "permission" }) + export const Rule = Config.PermissionObject.meta({ ref: "PermissionRule", }) @@ -36,13 +40,19 @@ export namespace PermissionNext { const result: Ruleset = {} for (const ruleset of rulesets) { for (const [permission, rule] of Object.entries(ruleset)) { - result[permission] ??= {} - for (const [pattern, action] of Object.entries(rule)) { - for (const existing of Object.keys(result[permission])) { - if (Wildcard.match(existing, pattern)) { - delete result[permission][existing] + for (const existingPerm of Object.keys(result)) { + if (Wildcard.match(existingPerm, permission)) { + for (const [pattern, action] of Object.entries(rule)) { + for (const existingPattern of Object.keys(result[existingPerm])) { + if (Wildcard.match(existingPattern, pattern)) { + result[existingPerm][existingPattern] = action + } + } } } + } + result[permission] ??= {} + for (const [pattern, action] of Object.entries(rule)) { result[permission][pattern] = action } } @@ -54,11 +64,12 @@ export namespace PermissionNext { .object({ id: Identifier.schema("permission"), sessionID: Identifier.schema("session"), - type: z.string(), + patterns: z.string().array(), title: z.string(), description: z.string(), - keys: z.string().array(), - patterns: z.string().array().optional(), + metadata: z.record(z.string(), z.any()), + always: z.string().array(), + permission: z.string(), }) .meta({ ref: "PermissionRequest", @@ -75,7 +86,7 @@ export namespace PermissionNext { }) export const Event = { - Updated: BusEvent.define("permission.request", Request), + Requested: BusEvent.define("permission.requested", Request), } const state = Instance.state(() => { @@ -98,22 +109,36 @@ export namespace PermissionNext { } }) - export const ask = fn(Request.partial({ id: true }), async (input) => { - const id = input.id ?? Identifier.ascending("permission") - return new Promise((resolve, reject) => { - const s = state() - const info: Request = { - id, - ...input, + export const request = fn( + Request.partial({ id: true }).extend({ + ruleset: Ruleset, + }), + async (input) => { + const { ruleset, ...request } = input + for (const pattern of request.patterns ?? []) { + const action = evaluate(request.permission, pattern, ruleset) + log.info("evaluated", { permission: request.permission, pattern, action }) + if (action === "deny") throw new RejectedError() + if (action === "ask") { + const id = input.id ?? Identifier.ascending("permission") + return new Promise((resolve, reject) => { + const s = state() + const info: Request = { + id, + ...request, + } + s.pending[id] = { + info, + resolve, + reject, + } + Bus.publish(Event.Requested, info) + }) + } + if (action === "allow") continue } - s.pending[id] = { - info, - resolve, - reject, - } - Bus.publish(Event.Updated, info) - }) - }) + }, + ) export const respond = fn( z.object({ @@ -140,18 +165,15 @@ export namespace PermissionNext { export type Action = z.infer export function evaluate(permission: string, pattern: string, ruleset: Ruleset): Action { - const rule = ruleset[permission] - if (!rule) return "ask" - - let best: { length: number; action: Action } | undefined - for (const [p, action] of Object.entries(rule)) { - if (!Wildcard.match(pattern, p)) continue - if (!best || p.length > best.length) { - best = { length: p.length, action } + log.info("evaluate", { permission, pattern, ruleset }) + for (const [permissionPattern, rule] of sortBy(Object.entries(ruleset), [([k]) => k.length, "desc"])) { + if (!Wildcard.match(permission, permissionPattern)) continue + for (const [p, action] of sortBy(Object.entries(rule), [([k]) => k.length, "desc"])) { + if (!Wildcard.match(pattern, p)) continue + return action } } - - return best?.action ?? "ask" + return "ask" } export class RejectedError extends Error { diff --git a/packages/opencode/src/session/llm.ts b/packages/opencode/src/session/llm.ts index a81aa7db2..64a80655e 100644 --- a/packages/opencode/src/session/llm.ts +++ b/packages/opencode/src/session/llm.ts @@ -188,11 +188,7 @@ export namespace LLM { } async function resolveTools(input: Pick) { - const enabled = pipe( - input.agent.tools, - mergeDeep(await ToolRegistry.enabled(input.agent)), - mergeDeep(input.user.tools ?? {}), - ) + const enabled = pipe({}, mergeDeep(await ToolRegistry.enabled(input.agent)), mergeDeep(input.user.tools ?? {})) for (const [key, value] of Object.entries(enabled)) { if (value === false) delete input.tools[key] } diff --git a/packages/opencode/src/tool/registry.ts b/packages/opencode/src/tool/registry.ts index 647c74267..143267d10 100644 --- a/packages/opencode/src/tool/registry.ts +++ b/packages/opencode/src/tool/registry.ts @@ -136,17 +136,14 @@ export namespace ToolRegistry { export async function enabled(agent: Agent.Info): Promise> { const result: Record = {} - if (agent.permission.edit === "deny") { - result["edit"] = false - result["write"] = false - } - if (agent.permission.bash["*"] === "deny" && Object.keys(agent.permission.bash).length === 1) { - result["bash"] = false - } - if (agent.permission.webfetch === "deny") { - result["webfetch"] = false - result["codesearch"] = false - result["websearch"] = false + for (const [tool, action] of Object.entries(agent.permission)) { + if (!Bun.deepEquals(action, { "*": "deny" })) continue + result[tool] = false + if (tool === "edit") { + result["write"] = false + result["patch"] = false + result["multiedit"] = false + } } return result diff --git a/packages/opencode/src/tool/write.ts b/packages/opencode/src/tool/write.ts index 6b8fd3dd1..190a5883d 100644 --- a/packages/opencode/src/tool/write.ts +++ b/packages/opencode/src/tool/write.ts @@ -10,6 +10,7 @@ import { FileTime } from "../file/time" import { Filesystem } from "../util/filesystem" import { Instance } from "../project/instance" import { Agent } from "../agent/agent" +import { PermissionNext } from "@/permission/next" const MAX_DIAGNOSTICS_PER_FILE = 20 const MAX_PROJECT_DIAGNOSTICS_FILES = 5 @@ -24,6 +25,7 @@ export const WriteTool = Tool.define("write", { const agent = await Agent.get(ctx.agent) const filepath = path.isAbsolute(params.filePath) ? params.filePath : path.join(Instance.directory, params.filePath) + /* TODO if (!Filesystem.contains(Instance.directory, filepath)) { const parentDir = path.dirname(filepath) if (agent.permission.external_directory === "ask") { @@ -52,24 +54,24 @@ export const WriteTool = Tool.define("write", { ) } } + */ const file = Bun.file(filepath) const exists = await file.exists() if (exists) await FileTime.assert(ctx.sessionID, filepath) - if (agent.permission.edit === "ask") - await Permission.ask({ - type: "write", - sessionID: ctx.sessionID, - messageID: ctx.messageID, - callID: ctx.callID, - title: exists ? "Overwrite this file: " + filepath : "Create new file: " + filepath, - metadata: { - filePath: filepath, - content: params.content, - exists, - }, - }) + await PermissionNext.request({ + permission: "edit", + title: "tbd", + patterns: [path.relative(Instance.worktree, filepath)], + always: ["*"], + sessionID: ctx.sessionID, + metadata: {}, + description: exists + ? "Overwrite this file: " + path.relative(Instance.directory, filepath) + : "Create new file: " + path.relative(Instance.directory, filepath), + ruleset: agent.permission, + }) await Bun.write(filepath, params.content) await Bus.publish(File.Event.Edited, { diff --git a/packages/opencode/test/permission/next.test.ts b/packages/opencode/test/permission/next.test.ts index 9cb2b36db..9d81d7eb3 100644 --- a/packages/opencode/test/permission/next.test.ts +++ b/packages/opencode/test/permission/next.test.ts @@ -46,9 +46,9 @@ test("merge - adds new permission", () => { }) }) -test("merge - wildcard wipes specific patterns", () => { +test("merge - wildcard overwrites specific patterns", () => { const result = PermissionNext.merge({ bash: { foo: "ask", bar: "allow" } }, { bash: { "*": "deny" } }) - expect(result).toEqual({ bash: { "*": "deny" } }) + expect(result).toEqual({ bash: { foo: "deny", bar: "deny", "*": "deny" } }) }) test("merge - specific pattern after wildcard", () => { @@ -56,12 +56,12 @@ test("merge - specific pattern after wildcard", () => { expect(result).toEqual({ bash: { "*": "deny", foo: "allow" } }) }) -test("merge - glob pattern wipes matching patterns", () => { +test("merge - glob pattern overwrites matching patterns", () => { const result = PermissionNext.merge( { bash: { "foo/bar": "ask", "foo/baz": "allow", other: "deny" } }, { bash: { "foo/*": "deny" } }, ) - expect(result).toEqual({ bash: { "foo/*": "deny", other: "deny" } }) + expect(result).toEqual({ bash: { "foo/bar": "deny", "foo/baz": "deny", other: "deny", "foo/*": "deny" } }) }) test("merge - multiple rulesets", () => { @@ -77,15 +77,115 @@ test("merge - empty ruleset does nothing", () => { expect(result).toEqual({ bash: { "*": "allow" } }) }) -test("merge - nested glob patterns", () => { +test("merge - nested glob patterns overwrites matching", () => { const result = PermissionNext.merge( { edit: { "src/components/Button.tsx": "allow", "src/components/Input.tsx": "allow" } }, { edit: { "src/components/*": "deny" } }, ) - expect(result).toEqual({ edit: { "src/components/*": "deny" } }) + expect(result).toEqual({ + edit: { "src/components/Button.tsx": "deny", "src/components/Input.tsx": "deny", "src/components/*": "deny" }, + }) }) test("merge - non-matching glob preserves existing", () => { const result = PermissionNext.merge({ edit: { "src/foo.ts": "allow" } }, { edit: { "test/*": "deny" } }) expect(result).toEqual({ edit: { "src/foo.ts": "allow", "test/*": "deny" } }) }) + +test("merge - wildcard permission overwrites all other permissions", () => { + const result = PermissionNext.merge( + { bash: { "/bin/ls": "allow" }, edit: { "src/*": "allow" } }, + { "*": { "*": "ask" } }, + ) + // The wildcard permission should overwrite existing permissions' values + expect(result).toEqual({ + bash: { "/bin/ls": "ask" }, + edit: { "src/*": "ask" }, + "*": { "*": "ask" }, + }) +}) + +// evaluate tests + +test("evaluate - exact permission and pattern match", () => { + const result = PermissionNext.evaluate("bash", "rm", { bash: { rm: "deny" } }) + expect(result).toBe("deny") +}) + +test("evaluate - wildcard pattern match", () => { + const result = PermissionNext.evaluate("bash", "rm", { bash: { "*": "allow" } }) + expect(result).toBe("allow") +}) + +test("evaluate - specific pattern takes precedence over wildcard", () => { + const result = PermissionNext.evaluate("bash", "rm", { bash: { "*": "allow", rm: "deny" } }) + expect(result).toBe("deny") +}) + +test("evaluate - glob pattern match", () => { + const result = PermissionNext.evaluate("edit", "src/foo.ts", { edit: { "src/*": "allow" } }) + expect(result).toBe("allow") +}) + +test("evaluate - more specific glob takes precedence", () => { + const result = PermissionNext.evaluate("edit", "src/components/Button.tsx", { + edit: { "src/*": "deny", "src/components/*": "allow" }, + }) + expect(result).toBe("allow") +}) + +test("evaluate - wildcard permission match", () => { + const result = PermissionNext.evaluate("bash", "rm", { "*": { "*": "deny" } }) + expect(result).toBe("deny") +}) + +test("evaluate - specific permission takes precedence over wildcard permission", () => { + const result = PermissionNext.evaluate("bash", "rm", { + "*": { "*": "deny" }, + bash: { "*": "allow" }, + }) + expect(result).toBe("allow") +}) + +test("evaluate - unknown permission with wildcard fallback", () => { + const result = PermissionNext.evaluate("unknown_tool", "anything", { "*": { "*": "ask" } }) + expect(result).toBe("ask") +}) + +test("evaluate - unknown permission without wildcard returns ask", () => { + const result = PermissionNext.evaluate("unknown_tool", "anything", { bash: { "*": "allow" } }) + expect(result).toBe("ask") +}) + +test("evaluate - empty ruleset returns ask", () => { + const result = PermissionNext.evaluate("bash", "rm", {}) + expect(result).toBe("ask") +}) + +test("evaluate - no matching pattern returns ask", () => { + const result = PermissionNext.evaluate("edit", "etc/passwd", { edit: { "src/*": "allow" } }) + expect(result).toBe("ask") +}) + +test("evaluate - glob permission pattern", () => { + const result = PermissionNext.evaluate("mcp_server_tool", "anything", { + "mcp_*": { "*": "allow" }, + }) + expect(result).toBe("allow") +}) + +test("evaluate - specific permission over glob permission", () => { + const result = PermissionNext.evaluate("mcp_dangerous", "anything", { + "mcp_*": { "*": "allow" }, + mcp_dangerous: { "*": "deny" }, + }) + expect(result).toBe("deny") +}) + +test("evaluate - combined permission and pattern specificity", () => { + const result = PermissionNext.evaluate("edit", "src/secret.ts", { + "*": { "*": "ask" }, + edit: { "*": "allow", "src/secret.ts": "deny" }, + }) + expect(result).toBe("deny") +}) diff --git a/packages/opencode/test/tool/registry.test.ts b/packages/opencode/test/tool/registry.test.ts new file mode 100644 index 000000000..262831dbc --- /dev/null +++ b/packages/opencode/test/tool/registry.test.ts @@ -0,0 +1,120 @@ +import { describe, expect, test } from "bun:test" +import { ToolRegistry } from "../../src/tool/registry" +import type { Agent } from "../../src/agent/agent" + +describe("ToolRegistry.enabled", () => { + test("returns empty object when all tools allowed", async () => { + const agent: Agent.Info = { + name: "test", + mode: "primary", + permission: { + edit: { "*": "allow" }, + bash: { "*": "allow" }, + }, + options: {}, + } + const result = await ToolRegistry.enabled(agent) + expect(result).toEqual({}) + }) + + test("disables edit tools when edit is denied", async () => { + const agent: Agent.Info = { + name: "test", + mode: "primary", + permission: { + edit: { "*": "deny" }, + bash: { "*": "allow" }, + }, + options: {}, + } + const result = await ToolRegistry.enabled(agent) + expect(result.edit).toBe(false) + expect(result.write).toBe(false) + expect(result.patch).toBe(false) + expect(result.multiedit).toBe(false) + }) + + test("disables specific tool when denied with wildcard", async () => { + const agent: Agent.Info = { + name: "test", + mode: "primary", + permission: { + bash: { "*": "deny" }, + edit: { "*": "allow" }, + }, + options: {}, + } + const result = await ToolRegistry.enabled(agent) + expect(result.bash).toBe(false) + }) + + test("does not disable tool when partially denied", async () => { + const agent: Agent.Info = { + name: "test", + mode: "primary", + permission: { + bash: { + "rm *": "deny", + "*": "allow", + }, + edit: { "*": "allow" }, + }, + options: {}, + } + const result = await ToolRegistry.enabled(agent) + expect(result.bash).toBeUndefined() + }) + + test("disables multiple tools when multiple denied", async () => { + const agent: Agent.Info = { + name: "test", + mode: "primary", + permission: { + edit: { "*": "deny" }, + bash: { "*": "deny" }, + webfetch: { "*": "deny" }, + }, + options: {}, + } + const result = await ToolRegistry.enabled(agent) + expect(result.edit).toBe(false) + expect(result.write).toBe(false) + expect(result.patch).toBe(false) + expect(result.multiedit).toBe(false) + expect(result.bash).toBe(false) + expect(result.webfetch).toBe(false) + }) + + test("does not disable tool when action is ask", async () => { + const agent: Agent.Info = { + name: "test", + mode: "primary", + permission: { + edit: { "*": "ask" }, + bash: { "*": "ask" }, + }, + options: {}, + } + const result = await ToolRegistry.enabled(agent) + expect(result.edit).toBeUndefined() + expect(result.bash).toBeUndefined() + }) + + test("does not disable tool when wildcard deny has additional allow rules", async () => { + const agent: Agent.Info = { + name: "test", + mode: "primary", + permission: { + bash: { + "*": "deny", + "echo *": "allow", + }, + edit: { "*": "allow" }, + }, + options: {}, + } + const result = await ToolRegistry.enabled(agent) + // bash should NOT be disabled because there's an allow rule for "echo *" + expect(result.bash).toBeUndefined() + }) +})