diff --git a/packages/opencode/src/permission/index.ts b/packages/opencode/src/permission/index.ts index 5fbd621ae0..f1cd43fdbe 100644 --- a/packages/opencode/src/permission/index.ts +++ b/packages/opencode/src/permission/index.ts @@ -6,7 +6,6 @@ import { Identifier } from "../id/id" import { Plugin } from "../plugin" import { Instance } from "../project/instance" import { Wildcard } from "../util/wildcard" -import { Config } from "@/config/config" export namespace Permission { const log = Log.create({ service: "permission" }) diff --git a/packages/opencode/src/permission/next.ts b/packages/opencode/src/permission/next.ts index 5102a22fdc..83e98a70fa 100644 --- a/packages/opencode/src/permission/next.ts +++ b/packages/opencode/src/permission/next.ts @@ -246,4 +246,8 @@ export namespace PermissionNext { ) } } + + export async function list() { + return state().then((x) => Object.values(x.pending).map((x) => x.info)) + } } diff --git a/packages/opencode/src/server/server.ts b/packages/opencode/src/server/server.ts index 35ebcda643..9d75308c1c 100644 --- a/packages/opencode/src/server/server.ts +++ b/packages/opencode/src/server/server.ts @@ -1525,6 +1525,7 @@ export namespace Server { "/session/:sessionID/permissions/:permissionID", describeRoute({ summary: "Respond to permission", + deprecated: true, description: "Approve or deny a permission request from the AI assistant.", operationId: "permission.respond", responses: { @@ -1546,15 +1547,12 @@ export namespace Server { permissionID: z.string(), }), ), - validator("json", z.object({ response: Permission.Response })), + validator("json", z.object({ response: PermissionNext.Reply })), async (c) => { const params = c.req.valid("param") - const sessionID = params.sessionID - const permissionID = params.permissionID - Permission.respond({ - sessionID, - permissionID, - response: c.req.valid("json").response, + PermissionNext.reply({ + requestID: params.permissionID, + reply: c.req.valid("json").response, }) return c.json(true) }, @@ -1605,14 +1603,14 @@ export namespace Server { description: "List of pending permissions", content: { "application/json": { - schema: resolver(Permission.Info.array()), + schema: resolver(PermissionNext.Request.array()), }, }, }, }, }), async (c) => { - const permissions = Permission.list() + const permissions = await PermissionNext.list() return c.json(permissions) }, ) diff --git a/packages/opencode/src/session/processor.ts b/packages/opencode/src/session/processor.ts index c8771345bf..227ca64bb9 100644 --- a/packages/opencode/src/session/processor.ts +++ b/packages/opencode/src/session/processor.ts @@ -3,7 +3,6 @@ import { Log } from "@/util/log" import { Identifier } from "@/id/id" import { Session } from "." import { Agent } from "@/agent/agent" -import { Permission } from "@/permission" import { Snapshot } from "@/snapshot" import { SessionSummary } from "./summary" import { Bus } from "@/bus" @@ -202,11 +201,6 @@ export namespace SessionProcessor { status: "error", input: value.input, error: (value.error as any).toString(), - metadata: - value.error instanceof Permission.RejectedError || - value.error instanceof Permission.RejectedError - ? value.error.metadata - : undefined, time: { start: match.state.time.start, end: Date.now(), @@ -214,10 +208,7 @@ export namespace SessionProcessor { }, }) - if ( - value.error instanceof Permission.RejectedError || - value.error instanceof PermissionNext.RejectedError - ) { + if (value.error instanceof PermissionNext.RejectedError) { blocked = shouldBreak } delete toolcalls[value.toolCallId] diff --git a/packages/opencode/test/config/config.test.ts b/packages/opencode/test/config/config.test.ts index 9288bbbdaa..c2ed3abe8f 100644 --- a/packages/opencode/test/config/config.test.ts +++ b/packages/opencode/test/config/config.test.ts @@ -478,7 +478,7 @@ Helper subagent prompt`, directory: tmp.path, fn: async () => { const config = await Config.get() - expect(config.agent?.["helper"]).toEqual({ + expect(config.agent?.["helper"]).toMatchObject({ name: "helper", model: "test/model", mode: "subagent", diff --git a/packages/opencode/test/tool/bash.test.ts b/packages/opencode/test/tool/bash.test.ts index 31875d556b..ee82813fb5 100644 --- a/packages/opencode/test/tool/bash.test.ts +++ b/packages/opencode/test/tool/bash.test.ts @@ -2,8 +2,8 @@ import { describe, expect, test } from "bun:test" import path from "path" import { BashTool } from "../../src/tool/bash" import { Instance } from "../../src/project/instance" -import { Permission } from "../../src/permission" import { tmpdir } from "../fixture/fixture" +import type { PermissionNext } from "../../src/permission/next" const ctx = { sessionID: "test", @@ -38,397 +38,164 @@ describe("tool.bash", () => { }) describe("tool.bash permissions", () => { - test("allows command matching allow pattern", async () => { - await using tmp = await tmpdir({ - init: async (dir) => { - await Bun.write( - path.join(dir, "opencode.json"), - JSON.stringify({ - permission: { - bash: { - "echo *": "allow", - "*": "deny", - }, - }, - }), - ) - }, - }) + test("asks for bash permission with correct pattern", async () => { + await using tmp = await tmpdir({ git: true }) await Instance.provide({ directory: tmp.path, fn: async () => { const bash = await BashTool.init() - const result = await bash.execute( + const requests: Array> = [] + const testCtx = { + ...ctx, + ask: async (req: Omit) => { + requests.push(req) + }, + } + await bash.execute( { command: "echo hello", description: "Echo hello", }, - ctx, + testCtx, ) - expect(result.metadata.exit).toBe(0) - expect(result.metadata.output).toContain("hello") + expect(requests.length).toBe(1) + expect(requests[0].permission).toBe("bash") + expect(requests[0].patterns).toContain("echo hello") }, }) }) - test("denies command matching deny pattern", async () => { - await using tmp = await tmpdir({ - init: async (dir) => { - await Bun.write( - path.join(dir, "opencode.json"), - JSON.stringify({ - permission: { - bash: { - "curl *": "deny", - "*": "allow", - }, - }, - }), - ) - }, - }) + test("asks for bash permission with multiple commands", async () => { + await using tmp = await tmpdir({ git: true }) await Instance.provide({ directory: tmp.path, fn: async () => { const bash = await BashTool.init() - await expect( - bash.execute( - { - command: "curl https://example.com", - description: "Fetch URL", - }, - ctx, - ), - ).rejects.toThrow("restricted") - }, - }) - }) - - test("denies all commands with wildcard deny", async () => { - await using tmp = await tmpdir({ - init: async (dir) => { - await Bun.write( - path.join(dir, "opencode.json"), - JSON.stringify({ - permission: { - bash: { - "*": "deny", - }, - }, - }), - ) - }, - }) - await Instance.provide({ - directory: tmp.path, - fn: async () => { - const bash = await BashTool.init() - await expect( - bash.execute( - { - command: "ls", - description: "List files", - }, - ctx, - ), - ).rejects.toThrow("restricted") - }, - }) - }) - - test("more specific pattern overrides general pattern", async () => { - await using tmp = await tmpdir({ - init: async (dir) => { - await Bun.write( - path.join(dir, "opencode.json"), - JSON.stringify({ - permission: { - bash: { - "*": "deny", - "ls *": "allow", - "pwd*": "allow", - }, - }, - }), - ) - }, - }) - await Instance.provide({ - directory: tmp.path, - fn: async () => { - const bash = await BashTool.init() - // ls should be allowed - const result = await bash.execute( - { - command: "ls -la", - description: "List files", + const requests: Array> = [] + const testCtx = { + ...ctx, + ask: async (req: Omit) => { + requests.push(req) }, - ctx, - ) - expect(result.metadata.exit).toBe(0) - - // pwd should be allowed - const pwd = await bash.execute( - { - command: "pwd", - description: "Print working directory", - }, - ctx, - ) - expect(pwd.metadata.exit).toBe(0) - - // cat should be denied - await expect( - bash.execute( - { - command: "cat /etc/passwd", - description: "Read file", - }, - ctx, - ), - ).rejects.toThrow("restricted") - }, - }) - }) - - test("denies dangerous subcommands while allowing safe ones", async () => { - await using tmp = await tmpdir({ - init: async (dir) => { - await Bun.write( - path.join(dir, "opencode.json"), - JSON.stringify({ - permission: { - bash: { - "find *": "allow", - "find * -delete*": "deny", - "find * -exec*": "deny", - "*": "deny", - }, - }, - }), - ) - }, - }) - await Instance.provide({ - directory: tmp.path, - fn: async () => { - const bash = await BashTool.init() - // Basic find should work - const result = await bash.execute( - { - command: "find . -name '*.ts'", - description: "Find typescript files", - }, - ctx, - ) - expect(result.metadata.exit).toBe(0) - - // find -delete should be denied - await expect( - bash.execute( - { - command: "find . -name '*.tmp' -delete", - description: "Delete temp files", - }, - ctx, - ), - ).rejects.toThrow("restricted") - - // find -exec should be denied - await expect( - bash.execute( - { - command: "find . -name '*.ts' -exec cat {} \\;", - description: "Find and cat files", - }, - ctx, - ), - ).rejects.toThrow("restricted") - }, - }) - }) - - test("allows git read commands while denying writes", async () => { - await using tmp = await tmpdir({ - git: true, - init: async (dir) => { - await Bun.write( - path.join(dir, "opencode.json"), - JSON.stringify({ - permission: { - bash: { - "git status*": "allow", - "git log*": "allow", - "git diff*": "allow", - "git branch": "allow", - "git commit *": "deny", - "git push *": "deny", - "*": "deny", - }, - }, - }), - ) - }, - }) - await Instance.provide({ - directory: tmp.path, - fn: async () => { - const bash = await BashTool.init() - // git status should work - const status = await bash.execute( - { - command: "git status", - description: "Git status", - }, - ctx, - ) - expect(status.metadata.exit).toBe(0) - - // git log should work - const log = await bash.execute( - { - command: "git log --oneline -5", - description: "Git log", - }, - ctx, - ) - expect(log.metadata.exit).toBe(0) - - // git commit should be denied - await expect( - bash.execute( - { - command: "git commit -m 'test'", - description: "Git commit", - }, - ctx, - ), - ).rejects.toThrow("restricted") - - // git push should be denied - await expect( - bash.execute( - { - command: "git push origin main", - description: "Git push", - }, - ctx, - ), - ).rejects.toThrow("restricted") - }, - }) - }) - - test("denies external directory access when permission is deny", async () => { - await using tmp = await tmpdir({ - init: async (dir) => { - await Bun.write( - path.join(dir, "opencode.json"), - JSON.stringify({ - permission: { - external_directory: "deny", - bash: { - "*": "allow", - }, - }, - }), - ) - }, - }) - await Instance.provide({ - directory: tmp.path, - fn: async () => { - const bash = await BashTool.init() - // Should deny cd to parent directory (cd is checked for external paths) - await expect( - bash.execute( - { - command: "cd ../", - description: "Change to parent directory", - }, - ctx, - ), - ).rejects.toThrow() - }, - }) - }) - - test("denies workdir outside project when external_directory is deny", async () => { - await using tmp = await tmpdir({ - init: async (dir) => { - await Bun.write( - path.join(dir, "opencode.json"), - JSON.stringify({ - permission: { - external_directory: "deny", - bash: { - "*": "allow", - }, - }, - }), - ) - }, - }) - await Instance.provide({ - directory: tmp.path, - fn: async () => { - const bash = await BashTool.init() - await expect( - bash.execute( - { - command: "ls", - workdir: "/tmp", - description: "List /tmp", - }, - ctx, - ), - ).rejects.toThrow() - }, - }) - }) - - test("handles multiple commands in sequence", async () => { - await using tmp = await tmpdir({ - init: async (dir) => { - await Bun.write( - path.join(dir, "opencode.json"), - JSON.stringify({ - permission: { - bash: { - "echo *": "allow", - "curl *": "deny", - "*": "deny", - }, - }, - }), - ) - }, - }) - await Instance.provide({ - directory: tmp.path, - fn: async () => { - const bash = await BashTool.init() - // echo && echo should work - const result = await bash.execute( + } + await bash.execute( { command: "echo foo && echo bar", description: "Echo twice", }, - ctx, + testCtx, ) - expect(result.metadata.output).toContain("foo") - expect(result.metadata.output).toContain("bar") + expect(requests.length).toBe(1) + expect(requests[0].permission).toBe("bash") + expect(requests[0].patterns).toContain("echo foo") + expect(requests[0].patterns).toContain("echo bar") + }, + }) + }) - // echo && curl should fail (curl is denied) - await expect( - bash.execute( - { - command: "echo hi && curl https://example.com", - description: "Echo then curl", - }, - ctx, - ), - ).rejects.toThrow("restricted") + test("asks for external_directory permission when cd to parent", async () => { + await using tmp = await tmpdir({ git: true }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const bash = await BashTool.init() + const requests: Array> = [] + const testCtx = { + ...ctx, + ask: async (req: Omit) => { + requests.push(req) + }, + } + await bash.execute( + { + command: "cd ../", + description: "Change to parent directory", + }, + testCtx, + ) + const extDirReq = requests.find((r) => r.permission === "external_directory") + expect(extDirReq).toBeDefined() + }, + }) + }) + + test("asks for external_directory permission when workdir is outside project", async () => { + await using tmp = await tmpdir({ git: true }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const bash = await BashTool.init() + const requests: Array> = [] + const testCtx = { + ...ctx, + ask: async (req: Omit) => { + requests.push(req) + }, + } + await bash.execute( + { + command: "ls", + workdir: "/tmp", + description: "List /tmp", + }, + testCtx, + ) + const extDirReq = requests.find((r) => r.permission === "external_directory") + expect(extDirReq).toBeDefined() + expect(extDirReq!.patterns).toContain("/tmp") + }, + }) + }) + + test("includes always patterns for auto-approval", async () => { + await using tmp = await tmpdir({ git: true }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const bash = await BashTool.init() + const requests: Array> = [] + const testCtx = { + ...ctx, + ask: async (req: Omit) => { + requests.push(req) + }, + } + await bash.execute( + { + command: "git log --oneline -5", + description: "Git log", + }, + testCtx, + ) + expect(requests.length).toBe(1) + expect(requests[0].always.length).toBeGreaterThan(0) + expect(requests[0].always.some((p) => p.endsWith("*"))).toBe(true) + }, + }) + }) + + test("does not ask for bash permission when command is cd only", async () => { + await using tmp = await tmpdir({ git: true }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const bash = await BashTool.init() + const requests: Array> = [] + const testCtx = { + ...ctx, + ask: async (req: Omit) => { + requests.push(req) + }, + } + await bash.execute( + { + command: "cd .", + description: "Stay in current directory", + }, + testCtx, + ) + const bashReq = requests.find((r) => r.permission === "bash") + expect(bashReq).toBeUndefined() }, }) }) diff --git a/packages/opencode/test/tool/patch.test.ts b/packages/opencode/test/tool/patch.test.ts index 2a0df48f98..3d3ec574e6 100644 --- a/packages/opencode/test/tool/patch.test.ts +++ b/packages/opencode/test/tool/patch.test.ts @@ -3,7 +3,7 @@ import path from "path" import { PatchTool } from "../../src/tool/patch" import { Instance } from "../../src/project/instance" import { tmpdir } from "../fixture/fixture" -import { Permission } from "../../src/permission" +import { PermissionNext } from "../../src/permission/next" import * as fs from "fs/promises" const ctx = { @@ -60,7 +60,8 @@ describe("tool.patch", () => { patchTool.execute({ patchText: maliciousPatch }, ctx) // TODO: this sucks await new Promise((resolve) => setTimeout(resolve, 1000)) - expect(Permission.pending()[ctx.sessionID]).toBeDefined() + const pending = await PermissionNext.list() + expect(pending.find((p) => p.sessionID === ctx.sessionID)).toBeDefined() }, }) }) diff --git a/packages/opencode/test/tool/read.test.ts b/packages/opencode/test/tool/read.test.ts index a808070a6d..1093a17fea 100644 --- a/packages/opencode/test/tool/read.test.ts +++ b/packages/opencode/test/tool/read.test.ts @@ -3,6 +3,7 @@ import path from "path" import { ReadTool } from "../../src/tool/read" import { Instance } from "../../src/project/instance" import { tmpdir } from "../fixture/fixture" +import type { PermissionNext } from "../../src/permission/next" const ctx = { sessionID: "test", @@ -19,14 +20,6 @@ describe("tool.read external_directory permission", () => { await using tmp = await tmpdir({ init: async (dir) => { await Bun.write(path.join(dir, "test.txt"), "hello world") - await Bun.write( - path.join(dir, "opencode.json"), - JSON.stringify({ - permission: { - external_directory: "deny", - }, - }), - ) }, }) await Instance.provide({ @@ -43,14 +36,6 @@ describe("tool.read external_directory permission", () => { await using tmp = await tmpdir({ init: async (dir) => { await Bun.write(path.join(dir, "subdir", "test.txt"), "nested content") - await Bun.write( - path.join(dir, "opencode.json"), - JSON.stringify({ - permission: { - external_directory: "deny", - }, - }), - ) }, }) await Instance.provide({ @@ -63,83 +48,74 @@ describe("tool.read external_directory permission", () => { }) }) - test("denies reading absolute path outside project directory", async () => { + test("asks for external_directory permission when reading absolute path outside project", async () => { await using outerTmp = await tmpdir({ init: async (dir) => { await Bun.write(path.join(dir, "secret.txt"), "secret data") }, }) - await using tmp = await tmpdir({ - init: async (dir) => { - await Bun.write( - path.join(dir, "opencode.json"), - JSON.stringify({ - permission: { - external_directory: "deny", - }, - }), - ) - }, - }) + await using tmp = await tmpdir({ git: true }) await Instance.provide({ directory: tmp.path, fn: async () => { const read = await ReadTool.init() - await expect(read.execute({ filePath: path.join(outerTmp.path, "secret.txt") }, ctx)).rejects.toThrow( - "not in the current working directory", - ) + const requests: Array> = [] + const testCtx = { + ...ctx, + ask: async (req: Omit) => { + requests.push(req) + }, + } + await read.execute({ filePath: path.join(outerTmp.path, "secret.txt") }, testCtx) + const extDirReq = requests.find((r) => r.permission === "external_directory") + expect(extDirReq).toBeDefined() + expect(extDirReq!.patterns.some((p) => p.includes(outerTmp.path))).toBe(true) }, }) }) - test("denies reading relative path that traverses outside project directory", async () => { - await using tmp = await tmpdir({ - init: async (dir) => { - await Bun.write( - path.join(dir, "opencode.json"), - JSON.stringify({ - permission: { - external_directory: "deny", - }, - }), - ) - }, - }) + test("asks for external_directory permission when reading relative path outside project", async () => { + await using tmp = await tmpdir({ git: true }) await Instance.provide({ directory: tmp.path, fn: async () => { const read = await ReadTool.init() - await expect(read.execute({ filePath: "../../../etc/passwd" }, ctx)).rejects.toThrow( - "not in the current working directory", - ) + const requests: Array> = [] + const testCtx = { + ...ctx, + ask: async (req: Omit) => { + requests.push(req) + }, + } + // This will fail because file doesn't exist, but we can check if permission was asked + await read.execute({ filePath: "../outside.txt" }, testCtx).catch(() => {}) + const extDirReq = requests.find((r) => r.permission === "external_directory") + expect(extDirReq).toBeDefined() }, }) }) - test("allows reading outside project directory when external_directory is allow", async () => { - await using outerTmp = await tmpdir({ - init: async (dir) => { - await Bun.write(path.join(dir, "external.txt"), "external content") - }, - }) + test("does not ask for external_directory permission when reading inside project", async () => { await using tmp = await tmpdir({ + git: true, init: async (dir) => { - await Bun.write( - path.join(dir, "opencode.json"), - JSON.stringify({ - permission: { - external_directory: "allow", - }, - }), - ) + await Bun.write(path.join(dir, "internal.txt"), "internal content") }, }) await Instance.provide({ directory: tmp.path, fn: async () => { const read = await ReadTool.init() - const result = await read.execute({ filePath: path.join(outerTmp.path, "external.txt") }, ctx) - expect(result.output).toContain("external content") + const requests: Array> = [] + const testCtx = { + ...ctx, + ask: async (req: Omit) => { + requests.push(req) + }, + } + await read.execute({ filePath: path.join(tmp.path, "internal.txt") }, testCtx) + const extDirReq = requests.find((r) => r.permission === "external_directory") + expect(extDirReq).toBeUndefined() }, }) })