diff --git a/.changeset/harden-planning-agent-edits.md b/.changeset/harden-planning-agent-edits.md new file mode 100644 index 00000000000..abd802a1e3c --- /dev/null +++ b/.changeset/harden-planning-agent-edits.md @@ -0,0 +1,5 @@ +--- +"@kilocode/cli": patch +--- + +Keep Plan and Architect mode source edits denied when agent-specific permissions request edit approval. diff --git a/packages/opencode/src/agent/agent.ts b/packages/opencode/src/agent/agent.ts index 085eca0637d..b3df2c7ae52 100644 --- a/packages/opencode/src/agent/agent.ts +++ b/packages/opencode/src/agent/agent.ts @@ -380,6 +380,7 @@ export const layer = Layer.effect( item.permission = Permission.merge(item.permission, Permission.fromConfig(value.permission ?? {})) // kilocode_change start KiloAgent.processConfigItem(item) + KiloAgent.hardenPlan(key, item, ctx.worktree, user, Permission.fromConfig(value.permission ?? {})) } function referencePrompt(reference: KiloReference.Resolved) { diff --git a/packages/opencode/src/kilocode/agent/index.ts b/packages/opencode/src/kilocode/agent/index.ts index 30e88a1aeb4..afaec2ca4cf 100644 --- a/packages/opencode/src/kilocode/agent/index.ts +++ b/packages/opencode/src/kilocode/agent/index.ts @@ -173,6 +173,24 @@ function denies(user: Permission.Ruleset) { return user.filter((rule) => rule.action === "deny") } +function editRestrictions(rules: Permission.Ruleset) { + const edit = rules.filter((rule) => rule.permission === "edit") + return edit.filter((rule, index) => { + if (rule.action !== "deny") return false + if (rule.pattern !== "*") return true + // A wildcard before a later edit exception is an allowlist baseline. The + // plan guard supplies the source catch-all, so do not append it alone. + return !edit.slice(index + 1).some((next) => next.action !== "deny") + }) +} + +function restrictions(user: Permission.Ruleset) { + return [ + ...user.filter((rule) => rule.action === "deny" && rule.permission !== "edit"), + ...editRestrictions(user), + ] +} + function askEditGuard() { return Permission.fromConfig({ edit: "deny" }) } @@ -195,6 +213,17 @@ function planEditGuard(worktree: string) { return Permission.fromConfig({ edit: planEditRules(worktree) }) } +export function hardenPlan( + key: string, + item: { permission: Permission.Ruleset }, + worktree: string, + ...explicit: Permission.Ruleset[] +) { + if (key !== "plan" && key !== "architect") return + const edit = explicit.map(editRestrictions) + item.permission = Permission.merge(item.permission, planEditGuard(worktree), ...edit) +} + function planGuard(worktree: string, mcp: Record = {}) { return Permission.fromConfig({ "*": "deny", @@ -402,7 +431,7 @@ export function patchAgents( planGuard(worktree, kilo.mcpRules), user, planEditGuard(worktree), - denies(user), + restrictions(user), ), } } diff --git a/packages/opencode/test/kilocode/agent-permission-overrides.test.ts b/packages/opencode/test/kilocode/agent-permission-overrides.test.ts index ff86bf03034..56fd8fb70c6 100644 --- a/packages/opencode/test/kilocode/agent-permission-overrides.test.ts +++ b/packages/opencode/test/kilocode/agent-permission-overrides.test.ts @@ -1,4 +1,5 @@ import { afterEach, expect, test } from "bun:test" +import type { ConfigV1 } from "@opencode-ai/core/v1/config/config" import { Effect } from "effect" import { Agent } from "../../src/agent/agent" import { Permission } from "../../src/permission" @@ -14,6 +15,21 @@ function load(dir: string, fn: (svc: Agent.Interface) => Effect.Effect) { ) } +async function get(config: Partial, name = "plan") { + await using tmp = await tmpdir({ config }) + const item = await provideTestInstance({ + directory: tmp.path, + fn: () => load(tmp.path, (svc) => svc.get(name)), + }) + return item +} + +function expectPlan(item: Agent.Info | undefined, action: Permission.Action = "allow") { + expect(item).toBeDefined() + expect(Permission.evaluate("edit", "src/output.log", item!.permission).action).toBe("deny") + expect(Permission.evaluate("edit", ".kilo/plans/fix.md", item!.permission).action).toBe(action) +} + afterEach(async () => { await disposeAllInstances() }) @@ -81,6 +97,194 @@ test("plan agent still hard-denies non-plan edits after user edit allow", async }) }) +test("plan agent still hard-denies non-plan edits after per-agent edit ask", async () => { + const plan = await get( + { + agent: { + plan: { + permission: { + edit: "ask", + }, + }, + }, + }, + ) + expectPlan(plan) +}) + +test("plan agent honors global and per-agent plan allows after wildcard edit deny", async () => { + const edit = { + "*": "deny" as const, + ".kilo/plans/*": "allow" as const, + } + for (const config of [ + { permission: { edit } }, + { + agent: { + plan: { + permission: { + edit, + }, + }, + }, + }, + ]) { + expectPlan(await get(config)) + } +}) + +test("plan agent preserves scalar edit deny", async () => { + const plan = await get( + { + agent: { + plan: { + permission: { + edit: "deny", + }, + }, + }, + }, + ) + expectPlan(plan, "deny") +}) + +test("plan agent preserves a terminal wildcard edit deny", async () => { + const plan = await get( + { + agent: { + plan: { + permission: { + edit: { + ".kilo/plans/*": "allow", + "*": "deny", + }, + }, + }, + }, + }, + ) + expectPlan(plan, "deny") +}) + +test("plan agent preserves explicit per-agent edit denies", async () => { + const plan = await get( + { + agent: { + plan: { + permission: { + edit: { + ".kilo/plans/private.md": "deny", + }, + }, + }, + }, + }, + ) + expectPlan(plan) + expect(Permission.evaluate("edit", ".kilo/plans/private.md", plan!.permission).action).toBe("deny") +}) + +test("plan agent preserves global edit denies after per-agent edit ask", async () => { + const plan = await get( + { + permission: { + edit: { + ".kilo/plans/private.md": "deny", + }, + }, + agent: { + plan: { + permission: { + edit: "ask", + }, + }, + }, + }, + ) + expectPlan(plan) + expect(Permission.evaluate("edit", ".kilo/plans/private.md", plan!.permission).action).toBe("deny") +}) + +test("plan agent preserves global non-edit denies before broader allows", async () => { + const plan = await get({ + permission: { + bash: { + "rm *": "deny", + "*": "allow", + }, + }, + }) + expect(Permission.evaluate("bash", "rm -rf x", plan!.permission).action).toBe("deny") + expect(Permission.evaluate("bash", "ls", plan!.permission).action).toBe("allow") +}) + +test("plan agent preserves per-agent tool allows with a wildcard deny", async () => { + const plan = await get( + { + agent: { + plan: { + permission: { + "*": "deny", + read: "allow", + glob: "allow", + edit: "ask", + }, + }, + }, + }, + ) + expectPlan(plan) + expect(Permission.evaluate("read", "src/output.log", plan!.permission).action).toBe("allow") + expect(Permission.evaluate("glob", "*", plan!.permission).action).toBe("allow") +}) + +test("marketplace architect honors plan allow after wildcard edit deny", async () => { + const architect = await get( + { + agent: { + architect: { + mode: "primary", + options: { + displayName: "Architect", + }, + permission: { + "*": "deny", + read: "allow", + glob: "allow", + edit: { + "*": "deny", + ".kilo/plans/*": "allow", + }, + }, + }, + }, + }, + "architect", + ) + expectPlan(architect) + expect(architect!.name).toBe("architect") + expect(architect!.displayName).toBe("Architect") + expect(Permission.evaluate("read", "src/output.log", architect!.permission).action).toBe("allow") + expect(Permission.evaluate("glob", "*", architect!.permission).action).toBe("allow") +}) + +test("non-planning agents retain per-agent edit permissions", async () => { + const code = await get( + { + agent: { + code: { + permission: { + edit: "ask", + }, + }, + }, + }, + "code", + ) + expect(code).toBeDefined() + expect(Permission.evaluate("edit", "src/output.log", code!.permission).action).toBe("ask") +}) + test("system utility agents ignore per-agent permission allows", async () => { await using tmp = await tmpdir({ config: {