mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-09-24 16:02:55 +08:00
fix(cli): harden plan edit permissions (#12458)
This commit is contained in:
@@ -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) {
|
||||
|
||||
@@ -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<string, "allow" | "ask" | "deny"> = {}) {
|
||||
return Permission.fromConfig({
|
||||
"*": "deny",
|
||||
@@ -402,7 +431,7 @@ export function patchAgents(
|
||||
planGuard(worktree, kilo.mcpRules),
|
||||
user,
|
||||
planEditGuard(worktree),
|
||||
denies(user),
|
||||
restrictions(user),
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<A>(dir: string, fn: (svc: Agent.Interface) => Effect.Effect<A>) {
|
||||
)
|
||||
}
|
||||
|
||||
async function get(config: Partial<ConfigV1.Info>, 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: {
|
||||
|
||||
Reference in New Issue
Block a user