diff --git a/packages/opencode/src/cli/cmd/tui/routes/session/permission.tsx b/packages/opencode/src/cli/cmd/tui/routes/session/permission.tsx index 627ca11a8..a06bcabd9 100644 --- a/packages/opencode/src/cli/cmd/tui/routes/session/permission.tsx +++ b/packages/opencode/src/cli/cmd/tui/routes/session/permission.tsx @@ -1,43 +1,86 @@ import { createStore } from "solid-js/store" -import { For } from "solid-js" +import { For, Match, Switch } from "solid-js" import { useKeyboard } from "@opentui/solid" import { useTheme } from "../../context/theme" import type { PermissionRequest } from "@opencode-ai/sdk/v2" import { useSDK } from "../../context/sdk" -const OPTIONS = { - once: "Approve once", - always: "Approve always", - reject: "Reject", -} -const OPTION_LIST = Object.keys(OPTIONS) -type Option = keyof typeof OPTIONS - export function PermissionPrompt(props: { request: PermissionRequest }) { const sdk = useSDK() - const { theme } = useTheme() const [store, setStore] = createStore({ - reply: "once" as Option, + always: false, + }) + + return ( + + + { + if (option === "cancel") { + setStore("always", false) + return + } + sdk.client.permission.reply({ + reply: "always", + requestID: props.request.id, + }) + }} + /> + + + { + if (option === "always") { + setStore("always", true) + return + } + sdk.client.permission.reply({ + reply: option as "once" | "reject", + requestID: props.request.id, + }) + }} + /> + + + ) +} + +function Prompt>(props: { + title: string + body: string + options: T + onSelect: (option: keyof T) => void +}) { + const { theme } = useTheme() + const keys = Object.keys(props.options) as (keyof T)[] + const [store, setStore] = createStore({ + selected: keys[0], }) useKeyboard((evt) => { - if (evt.name === "left") { - const idx = OPTION_LIST.indexOf(store.reply) - const next = OPTION_LIST[(idx - 1 + OPTION_LIST.length) % OPTION_LIST.length] - setStore("reply", next as Option) + if (evt.name === "left" || evt.name == "h") { + evt.preventDefault() + const idx = keys.indexOf(store.selected) + const next = keys[(idx - 1 + keys.length) % keys.length] + setStore("selected", next) } - if (evt.name === "right") { - const idx = OPTION_LIST.indexOf(store.reply) - const next = OPTION_LIST[(idx + 1) % OPTION_LIST.length] - setStore("reply", next as Option) + if (evt.name === "right" || evt.name == "l") { + evt.preventDefault() + const idx = keys.indexOf(store.selected) + const next = keys[(idx + 1) % keys.length] + setStore("selected", next) } if (evt.name === "return") { - sdk.client.permission.reply({ - reply: store.reply, - requestID: props.request.id, - }) + evt.preventDefault() + props.onSelect(store.selected) } }) @@ -46,11 +89,13 @@ export function PermissionPrompt(props: { request: PermissionRequest }) { {"△"} - Permission required + {props.title} - {"→"} - {props.request.message} + + {"→"} + + {props.body} - + {(option) => ( - - {OPTIONS[option as Option]} + + {props.options[option]} )} diff --git a/packages/opencode/src/permission/next.ts b/packages/opencode/src/permission/next.ts index 81d55d157..d2365cfb6 100644 --- a/packages/opencode/src/permission/next.ts +++ b/packages/opencode/src/permission/next.ts @@ -3,6 +3,7 @@ import { BusEvent } from "@/bus/bus-event" import { Config } from "@/config/config" import { Identifier } from "@/id/id" import { Instance } from "@/project/instance" +import { Storage } from "@/storage/storage" import { fn } from "@/util/fn" import { Log } from "@/util/log" import { Wildcard } from "@/util/wildcard" @@ -88,7 +89,10 @@ export namespace PermissionNext { ), } - const state = Instance.state(() => { + const state = Instance.state(async () => { + const projectID = Instance.project.id + const stored = await Storage.read(["permission", projectID]).catch(() => [] as Ruleset) + const pending: Record< string, { @@ -98,13 +102,9 @@ export namespace PermissionNext { } > = {} - const approved: { - [projectID: string]: Set - } = {} - return { pending, - approved, + approved: stored, } }) @@ -113,15 +113,15 @@ export namespace PermissionNext { ruleset: Ruleset, }), async (input) => { + const s = await state() const { ruleset, ...request } = input for (const pattern of request.patterns ?? []) { - const action = evaluate(request.permission, pattern, ruleset) + const action = evaluate(request.permission, pattern, ruleset, s.approved) 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, @@ -145,7 +145,7 @@ export namespace PermissionNext { reply: Reply, }), async (input) => { - const s = state() + const s = await state() const existing = s.pending[input.requestID] if (!existing) return delete s.pending[input.requestID] @@ -173,28 +173,28 @@ export namespace PermissionNext { } if (input.reply === "once") { existing.resolve() - Bus.publish(Event.Replied, { - sessionID: existing.info.sessionID, - requestID: existing.info.id, - reply: input.reply, - }) return } if (input.reply === "always") { + const projectID = Instance.project.id + for (const pattern of existing.info.always) { + s.approved.push({ + permission: existing.info.permission, + pattern, + action: "allow", + }) + } + await Storage.write(["permission", projectID], s.approved) existing.resolve() - Bus.publish(Event.Replied, { - sessionID: existing.info.sessionID, - requestID: existing.info.id, - reply: input.reply, - }) return } }, ) - export function evaluate(permission: string, pattern: string, ruleset: Ruleset): Action { - log.info("evaluate", { permission, pattern, ruleset }) - const match = ruleset.findLast( + export function evaluate(permission: string, pattern: string, ...rulesets: Ruleset[]): Action { + const merged = merge(...rulesets) + log.info("evaluate", { permission, pattern, ruleset: merged }) + const match = merged.findLast( (rule) => Wildcard.match(permission, rule.permission) && Wildcard.match(pattern, rule.pattern), ) return match?.action ?? "ask" @@ -203,14 +203,14 @@ export namespace PermissionNext { const EDIT_TOOLS = ["edit", "write", "patch", "multiedit"] export function disabled(tools: string[], ruleset: Ruleset): Set { - const disabled = new Set() + const result = new Set() for (const tool of tools) { const permission = EDIT_TOOLS.includes(tool) ? "edit" : tool if (evaluate(permission, "*", ruleset) === "deny") { - disabled.add(tool) + result.add(tool) } } - return disabled + return result } export class RejectedError extends Error { diff --git a/packages/opencode/src/tool/bash.ts b/packages/opencode/src/tool/bash.ts index d2335ab4b..366d80617 100644 --- a/packages/opencode/src/tool/bash.ts +++ b/packages/opencode/src/tool/bash.ts @@ -13,6 +13,7 @@ import { fileURLToPath } from "url" import { Flag } from "@/flag/flag.ts" import { Shell } from "@/shell/shell" import { PermissionNext } from "@/permission/next" +import { BashArity } from "@/permission/arity" const MAX_OUTPUT_LENGTH = Flag.OPENCODE_EXPERIMENTAL_BASH_MAX_OUTPUT_LENGTH || 30_000 const DEFAULT_TIMEOUT = Flag.OPENCODE_EXPERIMENTAL_BASH_DEFAULT_TIMEOUT_MS || 2 * 60 * 1000 @@ -81,24 +82,11 @@ export const BashTool = Tool.define("bash", async () => { } const agent = await Agent.get(ctx.agent) - const checkExternalDirectory = async (dir: string) => { - if (Filesystem.contains(Instance.directory, dir)) return - await PermissionNext.ask({ - permission: "external_directory", - message: `This command references paths outside of ${Instance.directory}`, - patterns: [dir], - always: [dir + "*"], - sessionID: ctx.sessionID, - metadata: {}, - ruleset: agent.permission, - }) - } + const directories = new Set() + if (!Filesystem.contains(Instance.directory, cwd)) directories.add(cwd) + const patterns = new Set() + const always = new Set() - await checkExternalDirectory(cwd) - - const permissions = agent.permission.bash - - const askPatterns = new Set() for (const node of tree.rootNode.descendantsOfType("command")) { if (!node) continue const command = [] @@ -133,27 +121,39 @@ export const BashTool = Tool.define("bash", async () => { process.platform === "win32" && resolved.match(/^\/[a-z]\//) ? resolved.replace(/^\/([a-z])\//, (_, drive) => `${drive.toUpperCase()}:\\`).replace(/\//g, "\\") : resolved - - await checkExternalDirectory(normalized) + directories.add(normalized) } } } - // always allow cd if it passes above check + // cd covered by above check if (command.length && command[0] !== "cd") { - askPatterns.add(command.join(" ")) + patterns.add(command.join(" ")) + always.add(BashArity.prefix(command).join(" ") + "*") } } - if (askPatterns.size > 0) { - const patterns = Array.from(askPatterns) + if (directories.size > 0) { + const dirs = Array.from(directories) + await PermissionNext.ask({ + permission: "external_directory", + message: `Requesting access to external directories: ${dirs.join(", ")}`, + patterns: Array.from(directories), + always: Array.from(directories).map((x) => x + "*"), + sessionID: ctx.sessionID, + metadata: {}, + ruleset: agent.permission, + }) + } + + if (patterns.size > 0) { await PermissionNext.ask({ permission: "bash", - patterns, + patterns: Array.from(patterns), + always: Array.from(always), sessionID: ctx.sessionID, message: params.command, metadata: {}, - always: ["*"], ruleset: agent.permission, }) } diff --git a/packages/opencode/test/permission/next.test.ts b/packages/opencode/test/permission/next.test.ts index 11d29fe9a..ed7c2b130 100644 --- a/packages/opencode/test/permission/next.test.ts +++ b/packages/opencode/test/permission/next.test.ts @@ -1,5 +1,8 @@ import { test, expect } from "bun:test" import { PermissionNext } from "../../src/permission/next" +import { Instance } from "../../src/project/instance" +import { Storage } from "../../src/storage/storage" +import { tmpdir } from "../fixture/fixture" // fromConfig tests @@ -297,6 +300,14 @@ test("evaluate - permission patterns sorted by length regardless of object order expect(result).toBe("deny") }) +test("evaluate - merges multiple rulesets", () => { + const config: PermissionNext.Ruleset = [{ permission: "bash", pattern: "*", action: "allow" }] + const approved: PermissionNext.Ruleset = [{ permission: "bash", pattern: "rm", action: "deny" }] + // approved comes after config, so rm should be denied + const result = PermissionNext.evaluate("bash", "rm", config, approved) + expect(result).toBe("deny") +}) + // disabled tests test("disabled - returns empty set when all tools allowed", () => { @@ -405,3 +416,248 @@ test("disabled - specific allow overrides wildcard deny", () => { expect(result.has("edit")).toBe(true) expect(result.has("read")).toBe(true) }) + +// ask tests + +test("ask - resolves immediately when action is allow", async () => { + await using tmp = await tmpdir({ git: true }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const result = await PermissionNext.ask({ + sessionID: "session_test", + permission: "bash", + patterns: ["ls"], + message: "Run ls command", + metadata: {}, + always: [], + ruleset: [{ permission: "bash", pattern: "*", action: "allow" }], + }) + expect(result).toBeUndefined() + }, + }) +}) + +test("ask - throws RejectedError when action is deny", async () => { + await using tmp = await tmpdir({ git: true }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + await expect( + PermissionNext.ask({ + sessionID: "session_test", + permission: "bash", + patterns: ["rm -rf /"], + message: "Run dangerous command", + metadata: {}, + always: [], + ruleset: [{ permission: "bash", pattern: "*", action: "deny" }], + }), + ).rejects.toBeInstanceOf(PermissionNext.RejectedError) + }, + }) +}) + +test("ask - returns pending promise when action is ask", async () => { + await using tmp = await tmpdir({ git: true }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const promise = PermissionNext.ask({ + sessionID: "session_test", + permission: "bash", + patterns: ["ls"], + message: "Run ls command", + metadata: {}, + always: [], + ruleset: [{ permission: "bash", pattern: "*", action: "ask" }], + }) + // Promise should be pending, not resolved + expect(promise).toBeInstanceOf(Promise) + // Don't await - just verify it returns a promise + }, + }) +}) + +// reply tests + +test("reply - once resolves the pending ask", async () => { + await using tmp = await tmpdir({ git: true }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const askPromise = PermissionNext.ask({ + id: "permission_test1", + sessionID: "session_test", + permission: "bash", + patterns: ["ls"], + message: "Run ls command", + metadata: {}, + always: [], + ruleset: [], + }) + + await PermissionNext.reply({ + requestID: "permission_test1", + reply: "once", + }) + + await expect(askPromise).resolves.toBeUndefined() + }, + }) +}) + +test("reply - reject throws RejectedError", async () => { + await using tmp = await tmpdir({ git: true }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const askPromise = PermissionNext.ask({ + id: "permission_test2", + sessionID: "session_test", + permission: "bash", + patterns: ["ls"], + message: "Run ls command", + metadata: {}, + always: [], + ruleset: [], + }) + + await PermissionNext.reply({ + requestID: "permission_test2", + reply: "reject", + }) + + await expect(askPromise).rejects.toBeInstanceOf(PermissionNext.RejectedError) + }, + }) +}) + +test("reply - always persists approval and resolves", async () => { + await using tmp = await tmpdir({ git: true }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const askPromise = PermissionNext.ask({ + id: "permission_test3", + sessionID: "session_test", + permission: "bash", + patterns: ["ls"], + message: "Run ls command", + metadata: {}, + always: ["ls"], + ruleset: [], + }) + + await PermissionNext.reply({ + requestID: "permission_test3", + reply: "always", + }) + + await expect(askPromise).resolves.toBeUndefined() + }, + }) + // Re-provide to reload state with stored permissions + await Instance.provide({ + directory: tmp.path, + fn: async () => { + // Stored approval should allow without asking + const result = await PermissionNext.ask({ + sessionID: "session_test2", + permission: "bash", + patterns: ["ls"], + message: "Run ls command", + metadata: {}, + always: [], + ruleset: [], + }) + expect(result).toBeUndefined() + }, + }) +}) + +test("reply - reject cancels all pending for same session", async () => { + await using tmp = await tmpdir({ git: true }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const askPromise1 = PermissionNext.ask({ + id: "permission_test4a", + sessionID: "session_same", + permission: "bash", + patterns: ["ls"], + message: "Run ls", + metadata: {}, + always: [], + ruleset: [], + }) + + const askPromise2 = PermissionNext.ask({ + id: "permission_test4b", + sessionID: "session_same", + permission: "edit", + patterns: ["foo.ts"], + message: "Edit file", + metadata: {}, + always: [], + ruleset: [], + }) + + // Catch rejections before they become unhandled + const result1 = askPromise1.catch((e) => e) + const result2 = askPromise2.catch((e) => e) + + // Reject the first one + await PermissionNext.reply({ + requestID: "permission_test4a", + reply: "reject", + }) + + // Both should be rejected + expect(await result1).toBeInstanceOf(PermissionNext.RejectedError) + expect(await result2).toBeInstanceOf(PermissionNext.RejectedError) + }, + }) +}) + +test("ask - checks all patterns and stops on first deny", async () => { + await using tmp = await tmpdir({ git: true }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + await expect( + PermissionNext.ask({ + sessionID: "session_test", + permission: "bash", + patterns: ["echo hello", "rm -rf /"], + message: "Run commands", + metadata: {}, + always: [], + ruleset: [ + { permission: "bash", pattern: "*", action: "allow" }, + { permission: "bash", pattern: "rm *", action: "deny" }, + ], + }), + ).rejects.toBeInstanceOf(PermissionNext.RejectedError) + }, + }) +}) + +test("ask - allows all patterns when all match allow rules", async () => { + await using tmp = await tmpdir({ git: true }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const result = await PermissionNext.ask({ + sessionID: "session_test", + permission: "bash", + patterns: ["echo hello", "ls -la", "pwd"], + message: "Run safe commands", + metadata: {}, + always: [], + ruleset: [{ permission: "bash", pattern: "*", action: "allow" }], + }) + expect(result).toBeUndefined() + }, + }) +})