mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-09-24 16:02:55 +08:00
fix(core): ensure user permission overrides apply correctly (#9975)
This commit is contained in:
@@ -156,6 +156,27 @@ function askGuard(mcp: Record<string, "allow" | "ask" | "deny"> = {}) {
|
||||
})
|
||||
}
|
||||
|
||||
function denies(user: Permission.Ruleset) {
|
||||
return user.filter((rule) => rule.action === "deny")
|
||||
}
|
||||
|
||||
function askEditGuard() {
|
||||
return Permission.fromConfig({ edit: "deny" })
|
||||
}
|
||||
|
||||
function planEditRules() {
|
||||
return {
|
||||
"*": "deny" as const,
|
||||
[path.join(".kilo", "plans", "*.md")]: "allow" as const,
|
||||
[path.join(".opencode", "plans", "*.md")]: "allow" as const,
|
||||
[path.relative(Instance.worktree, path.join(Global.Path.data, path.join("plans", "*.md")))]: "allow" as const,
|
||||
}
|
||||
}
|
||||
|
||||
function planEditGuard() {
|
||||
return Permission.fromConfig({ edit: planEditRules() })
|
||||
}
|
||||
|
||||
function planGuard(mcp: Record<string, "allow" | "ask" | "deny"> = {}) {
|
||||
return Permission.fromConfig({
|
||||
"*": "deny",
|
||||
@@ -182,12 +203,7 @@ function planGuard(mcp: Record<string, "allow" | "ask" | "deny"> = {}) {
|
||||
[Truncate.GLOB]: "allow",
|
||||
[path.join(Global.Path.data, "plans", "*")]: "allow",
|
||||
},
|
||||
edit: {
|
||||
"*": "deny",
|
||||
[path.join(".kilo", "plans", "*.md")]: "allow",
|
||||
[path.join(".opencode", "plans", "*.md")]: "allow",
|
||||
[path.relative(Instance.worktree, path.join(Global.Path.data, path.join("plans", "*.md")))]: "allow",
|
||||
},
|
||||
edit: planEditRules(),
|
||||
...mcp,
|
||||
})
|
||||
}
|
||||
@@ -300,9 +316,10 @@ export function patchAgents(
|
||||
description: "Plan mode. Can only edit plan files; all other filesystem mutations are denied.",
|
||||
permission: Permission.merge(
|
||||
defaults,
|
||||
user,
|
||||
planGuard(kilo.mcpRules),
|
||||
user.filter((r: Permission.Rule) => r.action === "deny"),
|
||||
user,
|
||||
planEditGuard(),
|
||||
denies(user),
|
||||
),
|
||||
}
|
||||
}
|
||||
@@ -406,9 +423,10 @@ export function patchAgents(
|
||||
options: {},
|
||||
permission: Permission.merge(
|
||||
defaults,
|
||||
user, // user before ask-specific so ask's deny+allowlist wins
|
||||
askGuard(kilo.mcpRules),
|
||||
user.filter((r: Permission.Rule) => r.action === "deny"), // re-apply user denies so explicit MCP blocks win over mcpRules
|
||||
user,
|
||||
askEditGuard(),
|
||||
denies(user),
|
||||
),
|
||||
mode: "primary",
|
||||
native: true,
|
||||
|
||||
@@ -0,0 +1,75 @@
|
||||
import { afterEach, expect, test } from "bun:test"
|
||||
import { Effect } from "effect"
|
||||
import { Agent } from "../../src/agent/agent"
|
||||
import { Permission } from "../../src/permission"
|
||||
import { Instance } from "../../src/project/instance"
|
||||
import { provideInstance, tmpdir } from "../fixture/fixture"
|
||||
|
||||
function load<A>(dir: string, fn: (svc: Agent.Interface) => Effect.Effect<A>) {
|
||||
return Effect.runPromise(provideInstance(dir)(Agent.Service.use(fn)).pipe(Effect.provide(Agent.defaultLayer)))
|
||||
}
|
||||
|
||||
afterEach(async () => {
|
||||
await Instance.disposeAll()
|
||||
})
|
||||
|
||||
test("ask agent honors user MCP allow over generated ask rule", async () => {
|
||||
await using tmp = await tmpdir({
|
||||
config: {
|
||||
mcp: {
|
||||
context7: { type: "local", command: ["context7"] },
|
||||
},
|
||||
permission: {
|
||||
"context7_query-docs": { "*": "allow" },
|
||||
},
|
||||
},
|
||||
})
|
||||
|
||||
await Instance.provide({
|
||||
directory: tmp.path,
|
||||
fn: async () => {
|
||||
const ask = await load(tmp.path, (svc) => svc.get("ask"))
|
||||
expect(ask).toBeDefined()
|
||||
expect(Permission.evaluate("context7_query-docs", "*", ask!.permission).action).toBe("allow")
|
||||
},
|
||||
})
|
||||
})
|
||||
|
||||
test("plan agent honors user bash allow over read-only deny default", async () => {
|
||||
await using tmp = await tmpdir({
|
||||
config: {
|
||||
permission: {
|
||||
bash: { "cargo search *": "allow" },
|
||||
},
|
||||
},
|
||||
})
|
||||
|
||||
await Instance.provide({
|
||||
directory: tmp.path,
|
||||
fn: async () => {
|
||||
const plan = await load(tmp.path, (svc) => svc.get("plan"))
|
||||
expect(plan).toBeDefined()
|
||||
expect(Permission.evaluate("bash", "cargo search serde", plan!.permission).action).toBe("allow")
|
||||
},
|
||||
})
|
||||
})
|
||||
|
||||
test("plan agent still hard-denies non-plan edits after user edit allow", async () => {
|
||||
await using tmp = await tmpdir({
|
||||
config: {
|
||||
permission: {
|
||||
edit: { "src/output.log": "allow" },
|
||||
},
|
||||
},
|
||||
})
|
||||
|
||||
await Instance.provide({
|
||||
directory: tmp.path,
|
||||
fn: async () => {
|
||||
const plan = await load(tmp.path, (svc) => svc.get("plan"))
|
||||
expect(plan).toBeDefined()
|
||||
expect(Permission.evaluate("edit", "src/output.log", plan!.permission).action).toBe("deny")
|
||||
expect(Permission.evaluate("edit", ".kilo/plans/fix.md", plan!.permission).action).toBe("allow")
|
||||
},
|
||||
})
|
||||
})
|
||||
@@ -31,9 +31,8 @@ function askRulesetWithMcp(servers: string[], user: Permission.Ruleset = []) {
|
||||
const sanitized = key.replace(/[^a-zA-Z0-9_-]/g, "_")
|
||||
mcpRules[sanitized + "_*"] = "ask"
|
||||
}
|
||||
// Mirrors agent.ts merge order: user, ask-specific (with mcpRules), user denies last
|
||||
// Mirrors Ask agent merge order: defaults, ask-specific guard, user config, user denies last.
|
||||
return Permission.merge(
|
||||
user,
|
||||
Permission.fromConfig({
|
||||
"*": "deny",
|
||||
bash: readOnlyBash,
|
||||
@@ -53,6 +52,7 @@ function askRulesetWithMcp(servers: string[], user: Permission.Ruleset = []) {
|
||||
codebase_search: "allow",
|
||||
...mcpRules,
|
||||
}),
|
||||
user,
|
||||
user.filter((r) => r.action === "deny"),
|
||||
)
|
||||
}
|
||||
@@ -257,6 +257,13 @@ describe("Ask agent MCP permissions", () => {
|
||||
expect(result.action).toBe("ask")
|
||||
})
|
||||
|
||||
test("user config allow overrides MCP ask rules", () => {
|
||||
const allow = Permission.fromConfig({ "my-server_read_file": "allow" })
|
||||
const ruleset = askRulesetWithMcp(["my-server"], allow)
|
||||
const result = Permission.evaluate("my-server_read_file", "*", ruleset)
|
||||
expect(result.action).toBe("allow")
|
||||
})
|
||||
|
||||
test("MCP tools disabled without server config", () => {
|
||||
const ruleset = askRuleset()
|
||||
const result = Permission.disabled(["my-server_sometool"], ruleset)
|
||||
|
||||
Reference in New Issue
Block a user