fix(cli): harden ask and plan permissions

This commit is contained in:
Alex Alecu
2026-04-27 13:05:46 +03:00
parent ede13b7ec5
commit eae081a0c7
8 changed files with 398 additions and 218 deletions
+5
View File
@@ -0,0 +1,5 @@
---
"@kilocode/cli": patch
---
Prevent Ask and Plan modes from editing files before an explicit implementation step.
+6 -148
View File
@@ -6,124 +6,17 @@ import { Truncate } from "../../tool"
import { Config } from "../../config"
import { Instance } from "../../project/instance"
import { makeRuntime } from "@/effect/run-service"
import { Global } from "@/global"
import { Telemetry } from "@kilocode/kilo-telemetry"
import z from "zod"
import path from "path"
import { askGuard, bash, planGuard } from "./permissions"
export { bash, readOnlyBash } from "./permissions"
import PROMPT_DEBUG from "../../agent/prompt/debug.txt"
import PROMPT_ORCHESTRATOR from "../../agent/prompt/orchestrator.txt"
import PROMPT_ASK from "../../agent/prompt/ask.txt"
import PROMPT_EXPLORE from "../../agent/prompt/explore.txt"
// Safe bash commands that don't need user approval.
// Only commands that cannot execute arbitrary code or subprocesses.
export const bash: Record<string, "allow" | "ask" | "deny"> = {
"*": "ask",
// read-only / informational
"cat *": "allow",
"head *": "allow",
"tail *": "allow",
"less *": "allow",
"ls *": "allow",
"tree *": "allow",
"pwd *": "allow",
"echo *": "allow",
"wc *": "allow",
"which *": "allow",
"type *": "allow",
"file *": "allow",
"diff *": "allow",
"du *": "allow",
"df *": "allow",
"date *": "allow",
"uname *": "allow",
"whoami *": "allow",
"printenv *": "allow",
"man *": "allow",
// text processing
"grep *": "allow",
"rg *": "allow",
"ag *": "allow",
"sort *": "allow",
"uniq *": "allow",
"cut *": "allow",
"tr *": "allow",
"jq *": "allow",
// file operations
"touch *": "allow",
"mkdir *": "allow",
"cp *": "allow",
"mv *": "allow",
// compilers (no script execution)
"tsc *": "allow",
"tsgo *": "allow",
// archive
"tar *": "allow",
"unzip *": "allow",
"gzip *": "allow",
"gunzip *": "allow",
}
// Read-only bash commands for ask/plan agents.
// Unknown commands are DENIED (not "ask") because these agents must never modify the filesystem.
export const readOnlyBash: Record<string, "allow" | "ask" | "deny"> = {
"*": "deny",
// read-only / informational
"cat *": "allow",
"head *": "allow",
"tail *": "allow",
"less *": "allow",
"ls *": "allow",
"tree *": "allow",
"pwd *": "allow",
"echo *": "allow",
"wc *": "allow",
"which *": "allow",
"type *": "allow",
"file *": "allow",
"diff *": "allow",
"du *": "allow",
"df *": "allow",
"date *": "allow",
"uname *": "allow",
"whoami *": "allow",
"printenv *": "allow",
"man *": "allow",
// text processing (stdout only, no file modification)
"grep *": "allow",
"rg *": "allow",
"ag *": "allow",
"sort *": "allow",
"uniq *": "allow",
"cut *": "allow",
"tr *": "allow",
"jq *": "allow",
// git — allowlist of read-only subcommands, deny everything else
"git *": "deny",
"git log *": "allow",
"git show *": "allow",
"git diff *": "allow",
"git status *": "allow",
"git blame *": "allow",
"git rev-parse *": "allow",
"git rev-list *": "allow",
"git ls-files *": "allow",
"git ls-tree *": "allow",
"git ls-remote *": "allow",
"git shortlog *": "allow",
"git describe *": "allow",
"git cat-file *": "allow",
"git name-rev *": "allow",
"git stash list *": "allow",
"git tag -l *": "allow",
"git branch --list *": "allow",
"git branch -a *": "allow",
"git branch -r *": "allow",
"git remote -v *": "allow",
// gh — require user approval since commands vary widely
"gh *": "ask",
}
// Generate per-server MCP wildcard rules that allow MCP tools with user approval.
export function getMcpRules(cfg: Config.Info): Record<string, "allow" | "ask" | "deny"> {
@@ -227,26 +120,12 @@ export function patchAgents(
if (agents.plan) {
agents.plan = {
...agents.plan,
description: "Plan mode. Only allows editing plan files; asks before editing anything else.",
description: "Plan mode. Can only edit plan files; all other filesystem mutations are denied.",
permission: Permission.merge(
defaults,
Permission.fromConfig({
question: "allow",
suggest: "allow", // kilocode_change
plan_exit: "allow",
bash: readOnlyBash,
...kilo.mcpRules,
external_directory: {
[path.join(Global.Path.data, "plans", "*")]: "allow",
},
edit: {
"*": "ask",
[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",
},
}),
user,
planGuard(kilo.mcpRules),
user.filter((r: Permission.Rule) => r.action === "deny"),
),
}
}
@@ -347,28 +226,7 @@ export function patchAgents(
permission: Permission.merge(
defaults,
user, // user before ask-specific so ask's deny+allowlist wins
Permission.fromConfig({
"*": "deny",
bash: readOnlyBash,
read: {
"*": "allow",
"*.env": "ask",
"*.env.*": "ask",
"*.env.example": "allow",
},
grep: "allow",
glob: "allow",
list: "allow",
question: "allow",
webfetch: "allow",
websearch: "allow",
codesearch: "allow",
codebase_search: "allow",
external_directory: {
[Truncate.GLOB]: "allow",
},
...kilo.mcpRules,
}),
askGuard(kilo.mcpRules),
user.filter((r: Permission.Rule) => r.action === "deny"), // re-apply user denies so explicit MCP blocks win over mcpRules
),
mode: "primary",
@@ -0,0 +1,168 @@
import path from "path"
import { Global } from "@/global"
import { Instance } from "@/project/instance"
import { Permission } from "@/permission"
import * as Truncate from "@/tool/truncate"
export const bash: Record<string, "allow" | "ask" | "deny"> = {
"*": "ask",
"cat *": "allow",
"head *": "allow",
"tail *": "allow",
"less *": "allow",
"ls *": "allow",
"tree *": "allow",
"pwd *": "allow",
"echo *": "allow",
"wc *": "allow",
"which *": "allow",
"type *": "allow",
"file *": "allow",
"diff *": "allow",
"du *": "allow",
"df *": "allow",
"date *": "allow",
"uname *": "allow",
"whoami *": "allow",
"printenv *": "allow",
"man *": "allow",
"grep *": "allow",
"rg *": "allow",
"ag *": "allow",
"sort *": "allow",
"uniq *": "allow",
"cut *": "allow",
"tr *": "allow",
"jq *": "allow",
"touch *": "allow",
"mkdir *": "allow",
"cp *": "allow",
"mv *": "allow",
"tsc *": "allow",
"tsgo *": "allow",
"tar *": "allow",
"unzip *": "allow",
"gzip *": "allow",
"gunzip *": "allow",
}
export const readOnlyBash: Record<string, "allow" | "ask" | "deny"> = {
"*": "deny",
"cat *": "allow",
"head *": "allow",
"tail *": "allow",
"less *": "allow",
"ls *": "allow",
"tree *": "allow",
"pwd *": "allow",
"echo *": "allow",
"wc *": "allow",
"which *": "allow",
"type *": "allow",
"file *": "allow",
"diff *": "allow",
"du *": "allow",
"df *": "allow",
"date *": "allow",
"uname *": "allow",
"whoami *": "allow",
"printenv *": "allow",
"man *": "allow",
"grep *": "allow",
"rg *": "allow",
"ag *": "allow",
"sort *": "allow",
"uniq *": "allow",
"cut *": "allow",
"tr *": "allow",
"jq *": "allow",
"git *": "deny",
"git log *": "allow",
"git show *": "allow",
"git diff *": "allow",
"git status *": "allow",
"git blame *": "allow",
"git rev-parse *": "allow",
"git rev-list *": "allow",
"git ls-files *": "allow",
"git ls-tree *": "allow",
"git ls-remote *": "allow",
"git shortlog *": "allow",
"git describe *": "allow",
"git cat-file *": "allow",
"git name-rev *": "allow",
"git stash list *": "allow",
"git tag -l *": "allow",
"git branch --list *": "allow",
"git branch -a *": "allow",
"git branch -r *": "allow",
"git remote -v *": "allow",
"gh *": "ask",
"*>*": "deny",
"* > *": "deny",
"*>>*": "deny",
"* >> *": "deny",
"*>|*": "deny",
"* >| *": "deny",
"sort -o *": "deny",
"sort * -o *": "deny",
}
export function askGuard(mcp: Record<string, "allow" | "ask" | "deny"> = {}) {
return Permission.fromConfig({
"*": "deny",
bash: readOnlyBash,
read: {
"*": "allow",
"*.env": "ask",
"*.env.*": "ask",
"*.env.example": "allow",
},
grep: "allow",
glob: "allow",
list: "allow",
question: "allow",
webfetch: "allow",
websearch: "allow",
codesearch: "allow",
codebase_search: "allow",
external_directory: {
[Truncate.GLOB]: "allow",
},
...mcp,
})
}
export function planGuard(mcp: Record<string, "allow" | "ask" | "deny"> = {}) {
return Permission.fromConfig({
"*": "deny",
question: "allow",
suggest: "allow",
plan_exit: "allow",
bash: readOnlyBash,
read: {
"*": "allow",
"*.env": "ask",
"*.env.*": "ask",
"*.env.example": "allow",
},
grep: "allow",
glob: "allow",
list: "allow",
webfetch: "allow",
websearch: "allow",
codesearch: "allow",
codebase_search: "allow",
external_directory: {
[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",
},
...mcp,
})
}
@@ -9,6 +9,7 @@ import { Session } from "@/session"
import { Flag } from "@/flag/flag"
import { PlanFollowup } from "@/kilocode/plan-followup"
import { KiloSession } from "@/kilocode/session"
import { Permission } from "@/permission"
import { environmentDetails, type EditorContext } from "@/kilocode/editor-context"
import { Identifier } from "@/id/id"
import { Filesystem } from "@/util"
@@ -54,6 +55,15 @@ export namespace KiloSessionPrompt {
return PlanFollowup.abort(sessionID)
}
export function guardPermissions(input: {
agent: { name: string; permission: Permission.Ruleset }
session: Pick<Session.Info, "permission">
}) {
const rules = input.session.permission ?? []
if (!["ask", "plan"].includes(input.agent.name)) return rules
return Permission.merge(rules, input.agent.permission, rules.filter((rule) => rule.action === "deny"))
}
/**
* Mutable cache for environment details, keyed by user message ID
* so it recomputes when a new user message arrives.
+32 -3
View File
@@ -400,7 +400,12 @@ NOTE: At any point in time through this workflow you should feel free to ask the
...req,
sessionID: input.session.id,
tool: { messageID: input.processor.message.id, callID: options.toolCallId },
ruleset: Permission.merge(input.agent.permission, input.session.permission ?? []),
// kilocode_change start - reapply Ask/Plan mode guards after session permissions
ruleset: Permission.merge(
input.agent.permission,
KiloSessionPrompt.guardPermissions({ agent: input.agent, session: input.session }),
),
// kilocode_change end
})
.pipe(Effect.orDie),
})
@@ -620,7 +625,12 @@ NOTE: At any point in time through this workflow you should feel free to ask the
.ask({
...req,
sessionID,
ruleset: Permission.merge(taskAgent.permission, session.permission ?? []),
// kilocode_change start - reapply Ask/Plan subagent guards after session permissions
ruleset: Permission.merge(
taskAgent.permission,
KiloSessionPrompt.guardPermissions({ agent: taskAgent, session }),
),
// kilocode_change end
})
.pipe(Effect.orDie),
})
@@ -1388,6 +1398,23 @@ NOTE: At any point in time through this workflow you should feel free to ask the
const hasToolCalls =
lastAssistantMsg?.parts.some((part) => part.type === "tool" && !part.metadata?.providerExecuted) ?? false
// kilocode_change start - plan_exit is a hard stop before another model call
if (
lastAssistant?.finish &&
hasToolCalls &&
lastAssistant.parentID === lastUser.id &&
lastUser.id < lastAssistant.id &&
KiloSessionPrompt.shouldAskPlanFollowup({ messages: msgs, abort: AbortSignal.any([]) })
) {
const action = yield* Effect.promise((signal) =>
KiloSessionPrompt.askPlanFollowup({ sessionID, messages: msgs, abort: signal }),
)
if (action === "continue") continue
yield* slog.info("exiting loop")
break
}
// kilocode_change end
if (
lastAssistant?.finish &&
!["tool-calls"].includes(lastAssistant.finish) &&
@@ -1560,7 +1587,9 @@ NOTE: At any point in time through this workflow you should feel free to ask the
const result = yield* handle.process({
user: lastUser,
agent,
permission: session.permission,
// kilocode_change start - keep Ask/Plan tool filtering hardened against session allows
permission: KiloSessionPrompt.guardPermissions({ agent, session }),
// kilocode_change end
sessionID,
parentSessionID: session.parentID,
system,
+23 -7
View File
@@ -119,20 +119,36 @@ test("ask agent denies edit/write/bash even when user config adds a specific edi
// kilocode_change end
// kilocode_change start
test("plan agent asks before edits except .kilo/plans/* and .opencode/plans/*", async () => {
test("plan agent denies edits except .kilo/plans/* and .opencode/plans/*", async () => {
await using tmp = await tmpdir()
await Instance.provide({
directory: tmp.path,
fn: async () => {
const plan = await load(tmp.path, (svc) => svc.get("plan"))
expect(plan).toBeDefined()
// Wildcard requires permission
expect(evalPerm(plan, "edit")).toBe("ask")
// kilocode_change start
// .kilo/plans/ is the primary allowed path
expect(evalPerm(plan, "edit")).toBe("deny")
expect(Permission.evaluate("edit", "src/index.ts", plan!.permission).action).toBe("deny")
expect(Permission.evaluate("edit", ".kilo/plans/foo.md", plan!.permission).action).toBe("allow")
expect(Permission.evaluate("edit", ".opencode/plans/foo.md", plan!.permission).action).toBe("allow")
},
})
})
test("plan agent user config allows cannot re-enable non-plan edits", 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/foo.md", plan!.permission).action).toBe("allow")
// kilocode_change end
// .opencode/plans/ is also allowed as backward compat fallback
expect(Permission.evaluate("edit", ".opencode/plans/foo.md", plan!.permission).action).toBe("allow")
},
})
@@ -1,65 +1,6 @@
import { test, expect, describe } from "bun:test"
import { Permission } from "../../src/permission"
// Reconstruct the Ask agent's readOnlyBash allowlist (mirrors kilocode/agent/index.ts)
// Uses an allow-list approach for git: deny by default, allow specific read-only subcommands.
const readOnlyBash: Record<string, "allow" | "ask" | "deny"> = {
"*": "deny",
// read-only / informational
"cat *": "allow",
"head *": "allow",
"tail *": "allow",
"less *": "allow",
"ls *": "allow",
"tree *": "allow",
"pwd *": "allow",
"echo *": "allow",
"wc *": "allow",
"which *": "allow",
"type *": "allow",
"file *": "allow",
"diff *": "allow",
"du *": "allow",
"df *": "allow",
"date *": "allow",
"uname *": "allow",
"whoami *": "allow",
"printenv *": "allow",
"man *": "allow",
// text processing (stdout only, no file modification)
"grep *": "allow",
"rg *": "allow",
"ag *": "allow",
"sort *": "allow",
"uniq *": "allow",
"cut *": "allow",
"tr *": "allow",
"jq *": "allow",
// git — allowlist of read-only subcommands, deny everything else
"git *": "deny",
"git log *": "allow",
"git show *": "allow",
"git diff *": "allow",
"git status *": "allow",
"git blame *": "allow",
"git rev-parse *": "allow",
"git rev-list *": "allow",
"git ls-files *": "allow",
"git ls-tree *": "allow",
"git ls-remote *": "allow",
"git shortlog *": "allow",
"git describe *": "allow",
"git cat-file *": "allow",
"git name-rev *": "allow",
"git stash list *": "allow",
"git tag -l *": "allow",
"git branch --list *": "allow",
"git branch -a *": "allow",
"git branch -r *": "allow",
"git remote -v *": "allow",
// gh — require user approval since commands vary widely
"gh *": "ask",
}
import { readOnlyBash } from "../../src/kilocode/agent/permissions"
/** Build the Ask agent ruleset without MCP servers */
function askRuleset() {
@@ -166,6 +107,23 @@ describe("Ask agent bash permissions", () => {
}
})
describe("denied output redirection and writer flags", () => {
const denied = [
"echo hi > file",
"echo hi >> file",
"cat a > b",
"jq . a.json > b.json",
"sort names.txt -o names.txt",
]
for (const cmd of denied) {
test(`"${cmd}" → deny`, () => {
const result = Permission.evaluate("bash", cmd, ruleset)
expect(result.action).toBe("deny")
})
}
})
describe("denied git write commands", () => {
const denied = [
"git commit -m 'test'",
@@ -105,6 +105,31 @@ function errorTool(parts: MessageV2.Part[]) {
return part?.state.status === "error" ? (part as ErrorToolPart) : undefined
}
function names(input: Record<string, unknown> | undefined) {
const tools = input?.tools
if (!Array.isArray(tools)) return []
return tools.flatMap((item) => {
if (!item || typeof item !== "object") return []
const obj = item as Record<string, unknown>
const fn = obj.function
if (fn && typeof fn === "object") {
const name = (fn as Record<string, unknown>).name
if (typeof name === "string") return [name]
}
if (typeof obj.name === "string") return [obj.name]
return []
})
}
const waitQuestion = Effect.fn("test.waitQuestion")(function* (sessionID: SessionID) {
for (let i = 0; i < 100; i++) {
const list = yield* Effect.promise(() => Question.list())
const item = list.find((q) => q.sessionID === sessionID)
if (item) return item
yield* Effect.sleep("10 millis")
}
})
const mcp = Layer.succeed(
MCP.Service,
MCP.Service.of({
@@ -543,6 +568,117 @@ it.live("loop continues when finish is stop but assistant has tool parts", () =>
),
)
unix("ask agent denies bash redirection during prompt loop", () =>
provideTmpdirServer(
({ dir, llm }) =>
withSh(() =>
Effect.gen(function* () {
const prompt = yield* SessionPrompt.Service
const sessions = yield* Session.Service
const session = yield* sessions.create({ title: "Ask bash guard" })
const file = path.join(dir, "ask-bug.txt")
yield* prompt.prompt({
sessionID: session.id,
agent: "ask",
noReply: true,
parts: [{ type: "text", text: "Explain the repo" }],
})
yield* llm.tool("bash", { command: "echo bug > ask-bug.txt", description: "write ask bug" })
yield* llm.text("done")
yield* prompt.loop({ sessionID: session.id })
expect(yield* Effect.promise(() => Bun.file(file).exists())).toBe(false)
const msgs = yield* MessageV2.filterCompactedEffect(session.id)
const tool = msgs
.flatMap((msg) => msg.parts)
.find((part): part is MessageV2.ToolPart => part.type === "tool" && part.tool === "bash")
expect(tool?.state.status).toBe("error")
}),
),
{ git: true, config: providerCfg },
),
)
it.live("ask agent hides edit tools despite session allow-all", () =>
provideTmpdirServer(
({ dir, llm }) =>
Effect.gen(function* () {
const prompt = yield* SessionPrompt.Service
const sessions = yield* Session.Service
const session = yield* sessions.create({
title: "Ask session guard",
permission: [{ permission: "*", pattern: "*", action: "allow" }],
})
const file = path.join(dir, "ask-write-bug.txt")
yield* prompt.prompt({
sessionID: session.id,
agent: "ask",
noReply: true,
parts: [{ type: "text", text: "Explain the repo" }],
})
yield* llm.tool("write", { filePath: file, content: "bug\n" })
yield* llm.text("done")
yield* prompt.loop({ sessionID: session.id })
const inputs = yield* llm.inputs
expect(names(inputs[0])).not.toContain("write")
expect(names(inputs[0])).not.toContain("edit")
expect(yield* Effect.promise(() => Bun.file(file).exists())).toBe(false)
}),
{ git: true, config: providerCfg },
),
)
unix("plan follow-up stops before queued mutations after plan_exit", () =>
provideTmpdirServer(
({ dir, llm }) =>
withSh(() =>
Effect.gen(function* () {
const prompt = yield* SessionPrompt.Service
const sessions = yield* Session.Service
const session = yield* sessions.create({ title: "Plan hard stop" })
const plan = Session.plan(session)
const file = path.join(dir, "plan-after-exit.txt")
yield* prompt.prompt({
sessionID: session.id,
agent: "plan",
noReply: true,
parts: [{ type: "text", text: "Create a plan only" }],
})
yield* llm.tool("write", { filePath: plan, content: "# Plan\n\n- Stop after planning.\n" })
yield* llm.tool("plan_exit", {})
yield* llm.tool("bash", { command: "echo bug > plan-after-exit.txt", description: "write plan bug" })
const fiber = yield* prompt.loop({ sessionID: session.id }).pipe(Effect.forkChild)
const question = yield* waitQuestion(session.id)
if (!question) {
yield* Fiber.interrupt(fiber)
expect(question).toBeDefined()
return
}
const calls = yield* llm.calls
const pending = yield* llm.pending
const mutated = yield* Effect.promise(() => Bun.file(file).exists())
const planned = yield* Effect.promise(() => Bun.file(plan).exists())
yield* Effect.promise(() => Question.reject(question.id))
const exit = yield* Fiber.await(fiber)
expect(Exit.isSuccess(exit)).toBe(true)
expect(calls).toBe(2)
expect(pending).toBe(1)
expect(mutated).toBe(false)
expect(planned).toBe(true)
}),
),
{ git: true, config: providerCfg },
),
)
it.live("failed subtask preserves metadata on error tool state", () =>
provideTmpdirServer(
Effect.fnUntraced(function* ({ llm }) {