diff --git a/packages/core/src/permission.ts b/packages/core/src/permission.ts index 99743abd2f36..33cae70c839e 100644 --- a/packages/core/src/permission.ts +++ b/packages/core/src/permission.ts @@ -15,6 +15,7 @@ const PermissionEffect = Permission.Effect export { PermissionEffect as Effect } export { Rule, Ruleset } from "@opencode-ai/schema/permission" const missingAgentPermissions: Permission.Ruleset = [{ action: "*", resource: "*", effect: "deny" }] +const externalFollowupLimit = 256 export const ID = Permission.ID export type ID = typeof ID.Type @@ -108,9 +109,15 @@ export interface Interface { export class Service extends Context.Service()("@opencode/v2/Permission") {} +interface ExternalFollowup { + readonly action: "edit" | "read" + readonly resources: ReadonlyArray +} + interface Pending { readonly request: Request readonly agent?: AgentV2.ID + readonly externalFollowup?: ExternalFollowup readonly deferred: Deferred.Deferred } @@ -123,6 +130,7 @@ const layer = Layer.effect( const sessions = yield* SessionStore.Service const saved = yield* PermissionSaved.Service const pending = new Map() + const externalFollowups = new Map() yield* Effect.addFinalizer(() => Effect.forEach(pending.values(), (item) => Deferred.fail(item.deferred, new DeclinedError()), { @@ -131,6 +139,7 @@ const layer = Layer.effect( Effect.ensuring( Effect.sync(() => { pending.clear() + externalFollowups.clear() }), ), ), @@ -178,11 +187,52 @@ const layer = Layer.effect( } } - const create = (request: Request, agent?: AgentV2.ID) => + function sourceKey(input: Pick) { + if (input.source?.type !== "tool") return + return JSON.stringify([input.sessionID, input.source.messageID, input.source.callID]) + } + + function externalFollowup(input: AssertInput) { + if (input.action !== "external_directory") return + const value = input.metadata?.followup + if (!value || typeof value !== "object" || Array.isArray(value)) return + const action = Reflect.get(value, "action") + const resources = Reflect.get(value, "resources") + if (action !== "edit" && action !== "read") return + if (!Array.isArray(resources) || !resources.every((resource) => typeof resource === "string")) return + return { action, resources: [...resources] } satisfies ExternalFollowup + } + + function rememberExternalFollowup(input: Pending) { + if (!input.externalFollowup) return + const key = sourceKey(input.request) + if (!key) return + externalFollowups.set(key, input.externalFollowup) + if (externalFollowups.size > externalFollowupLimit) { + const oldest = externalFollowups.keys().next() + if (!oldest.done) externalFollowups.delete(oldest.value) + } + return key + } + + function consumeExternalFollowup(input: AssertInput) { + const key = sourceKey(input) + if (!key) return false + const followup = externalFollowups.get(key) + if (!followup) return false + externalFollowups.delete(key) + return ( + followup.action === input.action && + followup.resources.length === input.resources.length && + followup.resources.every((resource, index) => resource === input.resources[index]) + ) + } + + const create = (request: Request, agent?: AgentV2.ID, followup?: ExternalFollowup) => Effect.uninterruptible( Effect.gen(function* () { const deferred = yield* Deferred.make() - const item = { request, agent, deferred } + const item = { request, agent, externalFollowup: followup, deferred } if (pending.has(request.id)) return yield* Effect.die(new Error(`Duplicate pending permission ID: ${request.id}`)) pending.set(request.id, item) @@ -204,6 +254,7 @@ const layer = Layer.effect( Effect.uninterruptibleMask((restore) => Effect.gen(function* () { const result = yield* evaluateInput(input) + const approved = consumeExternalFollowup(input) if (result.effect === "deny") { return yield* new BlockedError({ rules: relevant(input, result.rules), @@ -211,8 +262,9 @@ const layer = Layer.effect( resources: input.resources, }) } + if (approved) return if (result.effect === "allow") return - const item = yield* create(request(input), input.agent) + const item = yield* create(request(input), input.agent, externalFollowup(input)) return yield* restore(Deferred.await(item.deferred)).pipe( Effect.catchTag("PermissionV2.DeclinedError", (error) => Effect.die(error)), Effect.ensuring( @@ -262,7 +314,9 @@ const layer = Layer.effect( resources: existing.request.save, }) } - yield* Deferred.succeed(existing.deferred, undefined) + const followup = rememberExternalFollowup(existing) + const resumed = yield* Deferred.succeed(existing.deferred, undefined) + if (!resumed && followup) externalFollowups.delete(followup) pending.delete(input.requestID) if (input.reply !== "always" || !existing.request.save?.length) return diff --git a/packages/core/src/tool/edit.ts b/packages/core/src/tool/edit.ts index 436a60e7cea0..92cbe0485a79 100644 --- a/packages/core/src/tool/edit.ts +++ b/packages/core/src/tool/edit.ts @@ -142,6 +142,7 @@ export const Plugin = { yield* unableToEdit( permission.assert({ ...LocationMutation.externalDirectoryPermission(external), + metadata: { followup: { action: "edit", resources: [target.resource] } }, sessionID: context.sessionID, agent: context.agent, source: permissionSource, diff --git a/packages/core/src/tool/patch.ts b/packages/core/src/tool/patch.ts index 4d7c3030b98a..86cf8b745eb7 100644 --- a/packages/core/src/tool/patch.ts +++ b/packages/core/src/tool/patch.ts @@ -105,9 +105,11 @@ export const Plugin = { const external = target.externalDirectory if (external) externalDirectories.set(external.resource, external) } + const resources = [...new Set(targets.map(({ target }) => target.resource))] for (const external of externalDirectories.values()) { yield* permission.assert({ ...LocationMutation.externalDirectoryPermission(external), + metadata: { followup: { action: "edit", resources } }, sessionID: context.sessionID, agent: context.agent, source, @@ -115,7 +117,7 @@ export const Plugin = { } yield* permission.assert({ action: "edit", - resources: [...new Set(targets.map(({ target }) => target.resource))], + resources, save: ["*"], sessionID: context.sessionID, agent: context.agent, diff --git a/packages/core/src/tool/read.ts b/packages/core/src/tool/read.ts index aa2e81b56457..2154a978291c 100644 --- a/packages/core/src/tool/read.ts +++ b/packages/core/src/tool/read.ts @@ -70,6 +70,7 @@ export const Plugin = { if (external) yield* permission.assert({ ...LocationMutation.externalDirectoryPermission(external), + metadata: { followup: { action: "read", resources: [target.resource] } }, sessionID: context.sessionID, agent: context.agent, source, diff --git a/packages/core/src/tool/write.ts b/packages/core/src/tool/write.ts index d7cd8c54f4cd..52426f239bcf 100644 --- a/packages/core/src/tool/write.ts +++ b/packages/core/src/tool/write.ts @@ -72,6 +72,7 @@ export const Plugin = { if (external) yield* permission.assert({ ...LocationMutation.externalDirectoryPermission(external), + metadata: { followup: { action: "edit", resources: [target.resource] } }, sessionID: context.sessionID, agent: context.agent, source, diff --git a/packages/core/test/permission.test.ts b/packages/core/test/permission.test.ts index 669fb4d9691e..3dcdb4855964 100644 --- a/packages/core/test/permission.test.ts +++ b/packages/core/test/permission.test.ts @@ -86,7 +86,7 @@ function assertion(input: Partial = {}) { } satisfies PermissionV2.AssertInput } -function waitForRequest() { +function waitForRequest(input = assertion()) { return Effect.gen(function* () { const service = yield* PermissionV2.Service const events = yield* EventV2.Service @@ -97,7 +97,7 @@ function waitForRequest() { : Effect.void, ) yield* Effect.addFinalizer(() => unsubscribe) - const fiber = yield* service.assert(assertion()).pipe(Effect.forkScoped) + const fiber = yield* service.assert(input).pipe(Effect.forkScoped) const request = yield* Deferred.await(asked) return { service, fiber, request } }) @@ -266,6 +266,88 @@ describe("PermissionV2", () => { }), ) + it.effect("uses external approval only for the exact blocking follow-up", () => + Effect.gen(function* () { + yield* setup() + const source = { type: "tool" as const, messageID: "msg_1", callID: "call_1" } + const followup = { action: "edit" as const, resources: ["/tmp/work/file.ts"] } + const external = yield* waitForRequest( + assertion({ + id: PermissionV2.ID.create("per_external"), + action: "external_directory", + resources: ["/tmp/work/*"], + metadata: { followup }, + source, + }), + ) + yield* external.service.reply({ requestID: external.request.id, reply: "once" }) + yield* Fiber.join(external.fiber) + + yield* external.service.assert(assertion({ ...followup, source })) + expect(yield* external.service.list()).toEqual([]) + + const repeated = yield* waitForRequest( + assertion({ id: PermissionV2.ID.create("per_repeated"), ...followup, source }), + ) + yield* repeated.service.reply({ requestID: repeated.request.id, reply: "reject" }) + yield* Fiber.await(repeated.fiber) + + const next = yield* waitForRequest( + assertion({ + id: PermissionV2.ID.create("per_external_2"), + action: "external_directory", + resources: ["/tmp/work/*"], + metadata: { followup }, + source, + }), + ) + yield* next.service.reply({ requestID: next.request.id, reply: "once" }) + yield* Fiber.join(next.fiber) + + const mismatch = yield* waitForRequest( + assertion({ + id: PermissionV2.ID.create("per_mismatch"), + action: "edit", + resources: ["/tmp/work/other.ts"], + source, + }), + ) + yield* mismatch.service.reply({ requestID: mismatch.request.id, reply: "reject" }) + yield* Fiber.await(mismatch.fiber) + }), + ) + + it.effect("does not retain an external follow-up after a configured denial", () => + Effect.gen(function* () { + yield* setup() + const source = { type: "tool" as const, messageID: "msg_2", callID: "call_2" } + const followup = { action: "edit" as const, resources: ["/tmp/work/file.ts"] } + const external = yield* waitForRequest( + assertion({ + id: PermissionV2.ID.create("per_external_deny"), + action: "external_directory", + resources: ["/tmp/work/*"], + metadata: { followup }, + source, + }), + ) + yield* external.service.reply({ requestID: external.request.id, reply: "once" }) + yield* Fiber.join(external.fiber) + + yield* setRules([{ action: "edit", resource: "*", effect: "deny" }]) + expect(yield* external.service.assert(assertion({ ...followup, source })).pipe(Effect.flip)).toBeInstanceOf( + PermissionV2.BlockedError, + ) + + yield* setRules([]) + const afterDeny = yield* waitForRequest( + assertion({ id: PermissionV2.ID.create("per_after_deny"), ...followup, source }), + ) + yield* afterDeny.service.reply({ requestID: afterDeny.request.id, reply: "reject" }) + yield* Fiber.await(afterDeny.fiber) + }), + ) + it.effect("defects when an asked permission is declined", () => Effect.gen(function* () { yield* setup() diff --git a/packages/docs/permissions.mdx b/packages/docs/permissions.mdx index 6b9659deb28f..7d6043164eb7 100644 --- a/packages/docs/permissions.mdx +++ b/packages/docs/permissions.mdx @@ -86,10 +86,17 @@ transitions. `doom_loop` and `lsp` are not current V2 Core permission actions. ## External directories -An external path requires a separate `external_directory` decision before the -tool's own `read` or `edit` decision. This applies to external paths used by -`read`, `edit`, `write`, and `patch`, and to an external `shell` working -directory. +An external path checks `external_directory` before the tool's own `read`, +`edit`, or `shell` action. This applies to external paths used by `read`, +`edit`, `write`, and `patch`, and to an external `shell` working directory. + +For the built-in file tools, approving an `external_directory` prompt also +approves the exact `read` or `edit` operation described by that prompt. This +avoids asking once for the directory boundary and again for the same operation. +A configured `deny` for that exact action and resource still blocks it. An +external `shell` working directory does not approve the command itself, and +when a directory boundary is already allowed by configuration, the tool's own +action is evaluated normally. ```jsonc { @@ -201,8 +208,8 @@ Saved approvals are durable and project-scoped. They are additional `allow` rules, but they can never override a configured `deny`. The proposed saved pattern may be broader than the displayed resource: several tools propose `*`, shell proposes the exact command text, and skills and subagents propose their -IDs. Review the confirmation carefully and remove saved approvals that are no -longer needed. +IDs. Review the displayed scope carefully and remove saved approvals that are +no longer needed. For non-interactive runs, `opencode2 run --auto` replies `once` to permission requests. It does not save approvals, and explicit `deny` rules remain enforced. diff --git a/packages/tui/src/routes/session/index.tsx b/packages/tui/src/routes/session/index.tsx index b16767046085..8e93d35b49e5 100644 --- a/packages/tui/src/routes/session/index.tsx +++ b/packages/tui/src/routes/session/index.tsx @@ -931,7 +931,14 @@ export function Session() { {null} 0}> - + + {(_) => { + const request = permissions()[0] + return request ? ( + + ) : null + }} + 0}> diff --git a/packages/tui/src/routes/session/permission.tsx b/packages/tui/src/routes/session/permission.tsx index a9382c85d3f0..92e91657e03a 100644 --- a/packages/tui/src/routes/session/permission.tsx +++ b/packages/tui/src/routes/session/permission.tsx @@ -134,26 +134,110 @@ function TextBody(props: { title: string; description?: string; icon?: string }) ) } +function preview(input?: string) { + const text = input?.trim() + if (!text) return "" + const lines = text.split("\n") + const first = lines[0]!.length > 60 ? `${lines[0]!.slice(0, 57)}...` : lines[0]! + return lines.length === 1 ? first : `${first} ... (+${lines.length - 1} lines)` +} + +function text(input: unknown) { + return typeof input === "string" ? input : undefined +} + +function ExternalBody(props: { + file?: string + dir?: string + preview?: string + content?: string + syntaxType?: string + expanded?: boolean + note?: string +}) { + const themeState = useTheme() + const theme = themeState.theme + const dimensions = useTerminalDimensions() + return ( + + + {"File: " + props.file} + + + {"Directory: " + props.dir} + + = 50)}> + + Preview + + {props.preview} + + } + > + + + + + + + + {props.note} + + + ) +} + export function PermissionPrompt(props: { request: PermissionV2Request; directory?: string }) { const sdk = useSDK() const data = useData() const [store, setStore] = createStore({ stage: "permission" as PermissionStage, + submitting: false, }) const pathFormatter = usePathFormatter() const session = createMemo(() => data.session.get(props.request.sessionID)) - const input = createMemo(() => { + const part = createMemo(() => { const tool = props.request.source - if (!tool) return {} + if (!tool) return const message = data.session.message.get(props.request.sessionID, tool.messageID) - if (message?.type !== "assistant") return {} - const part = message.content.find((part) => part.type === "tool" && part.id === tool.callID) - if (part?.type === "tool" && part.state.status !== "streaming") return part.state.input + if (message?.type !== "assistant") return + return message.content.find((part) => part.type === "tool" && part.id === tool.callID) + }) + + const input = createMemo(() => { + const current = part() + if (current?.type === "tool" && current.state.status !== "streaming") return current.state.input return {} }) + const toolName = createMemo(() => { + const current = part() + return current?.type === "tool" ? current.name : "" + }) const { theme } = useTheme() + const respond = (reply: "once" | "always" | "reject", message?: string) => { + if (store.submitting) return + setStore("submitting", true) + void sdk.api.permission + .reply({ + sessionID: props.request.sessionID, + reply, + requestID: props.request.id, + message: message || undefined, + }) + .catch(() => setStore("submitting", false)) + } return ( @@ -163,11 +247,11 @@ export function PermissionPrompt(props: { request: PermissionV2Request; director body={ - + - This will allow the following patterns until OpenCode is restarted + This will save the following scopes for this project {(pattern) => ( @@ -187,23 +271,14 @@ export function PermissionPrompt(props: { request: PermissionV2Request; director onSelect={(option) => { setStore("stage", "permission") if (option === "cancel") return - void sdk.api.permission.reply({ - sessionID: props.request.sessionID, - reply: "always", - requestID: props.request.id, - }) + respond("always") }} /> { - void sdk.api.permission.reply({ - sessionID: props.request.sessionID, - reply: "reject", - requestID: props.request.id, - message: message || undefined, - }) + respond("reject", message) }} onCancel={() => { setStore("stage", "permission") @@ -291,6 +366,8 @@ export function PermissionPrompt(props: { request: PermissionV2Request; director if (permission === "shell") { const command = typeof data.command === "string" ? data.command : "" return { + icon: "#", + title: "Run shell command", body: ( @@ -359,24 +436,86 @@ export function PermissionPrompt(props: { request: PermissionV2Request; director const pattern = props.request.resources[0] const derived = typeof pattern === "string" ? (pattern.includes("*") ? dirname(pattern) : pattern) : undefined - - const raw = parent ?? filepath ?? derived - const dir = pathFormatter.format(raw) - const patterns = props.request.resources.filter((p): p is string => typeof p === "string") - + const rawFile = text(data.path) ?? filepath + const file = pathFormatter.format(rawFile) + const dir = pathFormatter.format(parent ?? derived) + const saved = props.request.save?.[0] + const remembered = pathFormatter.format(saved && saved !== "*" ? dirname(saved) : (parent ?? derived)) + const note = remembered ? `Allow always remembers access to ${remembered} for this project.` : undefined + const tool = toolName() + + if (tool === "write") { + const content = text(data.content) + return { + icon: "→", + title: "Write file outside workspace", + body: (expanded: boolean) => ( + + ), + } + } + if (tool === "edit") { + const oldString = text(data.oldString) + const newString = text(data.newString) + const content = + oldString === undefined ? newString : ["Replace", oldString, "", "With", newString ?? ""].join("\n") + return { + icon: "→", + title: "Edit file outside workspace", + body: (expanded: boolean) => ( + + ), + } + } + if (tool === "patch") { + const content = text(data.patchText) + return { + icon: "→", + title: "Apply patch outside workspace", + body: (expanded: boolean) => ( + + ), + } + } + if (tool === "read") + return { + icon: "→", + title: "Read outside workspace", + body: , + } + if (tool === "shell") + return { + icon: "←", + title: "Access external working directory", + body: , + } return { icon: "←", title: `Access external directory ${dir}`, - body: ( - 0}> - - Patterns - - {(p) => {"- " + p}} - - - - ), + body: , } } @@ -433,7 +572,11 @@ export function PermissionPrompt(props: { request: PermissionV2Request; director : { once: "Allow once", reject: "Reject" } } escapeKey="reject" - fullscreen + fullscreen={ + props.request.action === "edit" || + props.request.action === "shell" || + (props.request.action === "external_directory" && ["write", "edit", "patch"].includes(toolName())) + } onSelect={(option) => { if (option === "always") { setStore("stage", "always") @@ -444,18 +587,10 @@ export function PermissionPrompt(props: { request: PermissionV2Request; director setStore("stage", "reject") return } - void sdk.api.permission.reply({ - sessionID: props.request.sessionID, - reply: "reject", - requestID: props.request.id, - }) + respond("reject") return } - void sdk.api.permission.reply({ - sessionID: props.request.sessionID, - reply: "once", - requestID: props.request.id, - }) + respond("once") }} /> ) @@ -551,7 +686,7 @@ function RejectPrompt(props: { onConfirm: (message: string) => void; onCancel: ( function Prompt>(props: { title: string header?: JSX.Element - body: JSX.Element + body: JSX.Element | ((expanded: boolean) => JSX.Element) options: T escapeKey?: keyof T fullscreen?: boolean @@ -566,7 +701,16 @@ function Prompt>(props: { expanded: false, }) const narrow = createMemo(() => dimensions().width < 80) + const compact = createMemo(() => dimensions().width < 50) const fullscreenHint = useCommandShortcut("permission.prompt.fullscreen") + const escapeHint = createMemo(() => + props.escapeKey ? String(props.options[props.escapeKey]).toLocaleLowerCase() : undefined, + ) + const body = () => (typeof props.body === "function" ? props.body(store.expanded) : props.body) + const shift = (direction: -1 | 1) => { + const idx = keys.indexOf(store.selected) + setStore("selected", keys[(idx + direction + keys.length) % keys.length]) + } useBindings(() => ({ mode: OPENCODE_BASE_MODE, @@ -595,42 +739,40 @@ function Prompt>(props: { key: "left", desc: "Previous permission option", group: "Permission", - cmd: () => { - const idx = keys.indexOf(store.selected) - const next = keys[(idx - 1 + keys.length) % keys.length] - setStore("selected", next) - }, + cmd: () => shift(-1), }, { key: "h", desc: "Previous permission option", group: "Permission", - cmd: () => { - const idx = keys.indexOf(store.selected) - const next = keys[(idx - 1 + keys.length) % keys.length] - setStore("selected", next) - }, + cmd: () => shift(-1), }, { key: "right", desc: "Next permission option", group: "Permission", - cmd: () => { - const idx = keys.indexOf(store.selected) - const next = keys[(idx + 1) % keys.length] - setStore("selected", next) - }, + cmd: () => shift(1), }, { key: "l", desc: "Next permission option", group: "Permission", - cmd: () => { - const idx = keys.indexOf(store.selected) - const next = keys[(idx + 1) % keys.length] - setStore("selected", next) - }, + cmd: () => shift(1), }, + ...(!props.fullscreen + ? [ + { key: "up", desc: "Previous permission option", group: "Permission", cmd: () => shift(-1) }, + { key: "k", desc: "Previous permission option", group: "Permission", cmd: () => shift(-1) }, + { key: "down", desc: "Next permission option", group: "Permission", cmd: () => shift(1) }, + { key: "j", desc: "Next permission option", group: "Permission", cmd: () => shift(1) }, + ] + : []), + ...keys.slice(0, 9).map((option, index) => ({ + key: String(index + 1), + desc: `Select ${props.options[option]}`, + group: "Permission", + cmd: () => props.onSelect(option), + })), { key: "return", desc: "Select permission option", @@ -665,7 +807,7 @@ function Prompt>(props: { ? { top: dimensions().height * -1 + 1, bottom: 1, left: 2, right: 2, position: "absolute" } : { top: 0, - maxHeight: 15, + maxHeight: narrow() ? Math.min(24, dimensions().height - 2) : 15, bottom: 0, left: 0, right: 0, @@ -686,12 +828,12 @@ function Prompt>(props: { {props.header} - {props.body} + {body()} >(props: { justifyContent={narrow() ? "flex-start" : "space-between"} alignItems={narrow() ? "flex-start" : "center"} > - + - {(option) => ( + {(option, index) => ( setStore("selected", option)} onMouseUp={() => { setStore("selected", option) @@ -714,13 +856,13 @@ function Prompt>(props: { }} > - {props.options[option]} + {`${index() + 1}. ${props.options[option]}`} )} - + {fullscreenHint()} {hint()} @@ -732,6 +874,11 @@ function Prompt>(props: { enter confirm + + + esc {escapeHint()} + +