fix(cli): prevent ask plan permission bypass

This commit is contained in:
Alex Alecu
2026-04-27 14:21:22 +03:00
parent eae081a0c7
commit 2277e3d9f6
7 changed files with 288 additions and 15 deletions
+1 -1
View File
@@ -2,4 +2,4 @@
"@kilocode/cli": patch
---
Prevent Ask and Plan modes from editing files before an explicit implementation step.
Prevent Ask and Plan modes, including saved or allow-all approvals, from editing files before an explicit implementation step.
@@ -6,6 +6,7 @@ import { ConfigProtection } from "@/kilocode/permission/config-paths"
interface PendingEntry {
info: Permission.Request
ruleset: Permission.Ruleset
hardRuleset?: Permission.Ruleset
deferred: Deferred.Deferred<void, Permission.RejectedError | Permission.CorrectedError>
}
@@ -25,9 +26,12 @@ export function drainCovered(
if (id === exclude) continue
// Never auto-resolve config file edit permissions
if (ConfigProtection.isRequest(entry.info)) continue
const actions = entry.info.patterns.map((pattern: string) =>
Permission.evaluate(entry.info.permission, pattern, entry.ruleset, approved),
)
const actions = entry.info.patterns.map((pattern: string) => {
const rule = Permission.evaluate(entry.info.permission, pattern, entry.ruleset, approved)
const hard = entry.hardRuleset ? Permission.evaluate(entry.info.permission, pattern, entry.hardRuleset) : undefined
if (hard?.action === "deny") return hard
return rule
})
const denied = actions.some((r: Permission.Rule) => r.action === "deny")
const allowed = !denied && actions.every((r: Permission.Rule) => r.action === "allow")
if (!denied && !allowed) continue
@@ -64,6 +64,11 @@ export namespace KiloSessionPrompt {
return Permission.merge(rules, input.agent.permission, rules.filter((rule) => rule.action === "deny"))
}
export function hardPermissions(input: { agent: { name: string; permission: Permission.Ruleset } }) {
if (!["ask", "plan"].includes(input.agent.name)) return
return input.agent.permission
}
/**
* Mutable cache for environment details, keyed by user message ID
* so it recomputes when a new user message arrives.
+31 -7
View File
@@ -120,6 +120,7 @@ export const AskInput = Schema.Struct({
...Request.fields,
id: Schema.optional(PermissionID),
ruleset: Ruleset,
hardRuleset: Schema.optional(Ruleset), // kilocode_change
})
.annotate({ identifier: "PermissionAskInput" })
.pipe(withStatics((s) => ({ zod: zod(s) })))
@@ -159,6 +160,7 @@ export interface Interface {
interface PendingEntry {
info: Request
ruleset: Ruleset // kilocode_change
hardRuleset?: Ruleset // kilocode_change
deferred: Deferred.Deferred<void, RejectedError | CorrectedError>
}
@@ -173,6 +175,17 @@ export function evaluate(permission: string, pattern: string, ...rulesets: Rules
return evalRule(permission, pattern, ...rulesets)
}
// kilocode_change start
function veto(permission: string, pattern: string, ruleset?: Ruleset) {
if (!ruleset) return false
return evaluate(permission, pattern, ruleset).action === "deny"
}
function subset(permission: string, ruleset: Ruleset) {
return ruleset.filter((rule) => Wildcard.match(permission, rule.permission))
}
// kilocode_change end
export class Service extends Context.Service<Service, Interface>()("@opencode/Permission") {}
export const layer = Layer.effect(
@@ -205,7 +218,7 @@ export const layer = Layer.effect(
const ask = Effect.fn("Permission.ask")(function* (input: AskInput) {
const { approved, pending } = yield* InstanceState.get(state)
const { ruleset, ...request } = input
const { ruleset, hardRuleset, ...request } = input // kilocode_change
const s = yield* InstanceState.get(state) // kilocode_change
const local = s.session[request.sessionID] ?? [] // kilocode_change
let needsAsk = false
@@ -217,9 +230,14 @@ export const layer = Layer.effect(
for (const pattern of request.patterns) {
const rule = evaluate(request.permission, pattern, ruleset, approved, local) // kilocode_change — include session-scoped rules
log.info("evaluated", { permission: request.permission, pattern, action: rule })
// kilocode_change start — saved/session approvals cannot override hard Ask/Plan denials
if (veto(request.permission, pattern, hardRuleset)) {
return yield* new DeniedError({ ruleset: subset(request.permission, hardRuleset ?? []) })
}
// kilocode_change end
if (rule.action === "deny") {
return yield* new DeniedError({
ruleset: ruleset.filter((rule) => Wildcard.match(request.permission, rule.permission)),
ruleset: subset(request.permission, ruleset), // kilocode_change
})
}
// kilocode_change start — override "allow" to "ask" for config paths
@@ -244,7 +262,7 @@ export const layer = Layer.effect(
log.info("asking", { id, permission: info.permission, patterns: info.patterns })
const deferred = yield* Deferred.make<void, RejectedError | CorrectedError>()
pending.set(id, { info, ruleset, deferred }) // kilocode_change
pending.set(id, { info, ruleset, hardRuleset, deferred }) // kilocode_change
yield* bus.publish(Event.Asked, info)
return yield* Effect.ensuring(
Deferred.await(deferred),
@@ -302,9 +320,11 @@ export const layer = Layer.effect(
for (const [id, item] of pending.entries()) {
if (item.info.sessionID !== existing.info.sessionID) continue
const ok = item.info.patterns.every(
(pattern) => evaluate(item.info.permission, pattern, item.ruleset, approved).action === "allow", // kilocode_change — include original ruleset
)
if (ConfigProtection.isRequest(item.info)) continue // kilocode_change
const ok = item.info.patterns.every((pattern) => {
if (veto(item.info.permission, pattern, item.hardRuleset)) return false // kilocode_change
return evaluate(item.info.permission, pattern, item.ruleset, approved).action === "allow" // kilocode_change — include original ruleset
})
if (!ok) continue
pending.delete(id)
yield* bus.publish(Event.Replied, {
@@ -390,7 +410,10 @@ export const layer = Layer.effect(
if (input.requestID) {
const entry = s.pending.get(PermissionID.make(input.requestID))
if (entry && (!input.sessionID || entry.info.sessionID === input.sessionID)) {
const ok = entry
? entry.info.patterns.every((pattern) => !veto(entry.info.permission, pattern, entry.hardRuleset))
: false // kilocode_change
if (entry && ok && (!input.sessionID || entry.info.sessionID === input.sessionID)) {
s.pending.delete(PermissionID.make(input.requestID))
yield* bus.publish(Event.Replied, {
sessionID: entry.info.sessionID,
@@ -404,6 +427,7 @@ export const layer = Layer.effect(
for (const [id, entry] of s.pending) {
if (input.sessionID && entry.info.sessionID !== input.sessionID) continue
if (ConfigProtection.isRequest(entry.info)) continue
if (entry.info.patterns.some((pattern) => veto(entry.info.permission, pattern, entry.hardRuleset))) continue // kilocode_change
s.pending.delete(id)
yield* bus.publish(Event.Replied, {
sessionID: entry.info.sessionID,
+8 -4
View File
@@ -405,6 +405,7 @@ NOTE: At any point in time through this workflow you should feel free to ask the
input.agent.permission,
KiloSessionPrompt.guardPermissions({ agent: input.agent, session: input.session }),
),
hardRuleset: KiloSessionPrompt.hardPermissions({ agent: input.agent }),
// kilocode_change end
})
.pipe(Effect.orDie),
@@ -624,12 +625,13 @@ NOTE: At any point in time through this workflow you should feel free to ask the
permission
.ask({
...req,
sessionID,
// kilocode_change start - reapply Ask/Plan subagent guards after session permissions
sessionID,
ruleset: Permission.merge(
taskAgent.permission,
KiloSessionPrompt.guardPermissions({ agent: taskAgent, session }),
),
hardRuleset: KiloSessionPrompt.hardPermissions({ agent: taskAgent }),
// kilocode_change end
})
.pipe(Effect.orDie),
@@ -1391,12 +1393,14 @@ NOTE: At any point in time through this workflow you should feel free to ask the
const lastAssistantMsg = msgs.findLast(
(msg) => msg.info.role === "assistant" && msg.info.id === lastAssistant?.id,
)
// kilocode_change start - keep provider-executed tools from forcing a re-loop
// Some providers return "stop" even when the assistant message contains tool calls.
// Keep the loop running so tool results can be sent back to the model.
// Skip provider-executed tool parts — those were fully handled within the
// provider's stream (e.g. DWS Agent Platform) and don't need a re-loop.
const hasToolCalls =
lastAssistantMsg?.parts.some((part) => part.type === "tool" && !part.metadata?.providerExecuted) ?? false
// kilocode_change end
// kilocode_change start - plan_exit is a hard stop before another model call
if (
@@ -1583,11 +1587,11 @@ NOTE: At any point in time through this workflow you should feel free to ask the
])
const system = [...env, ...(skills ? [skills] : []), ...instructions]
const format = lastUser.format ?? { type: "text" as const }
if (format.type === "json_schema") system.push(STRUCTURED_OUTPUT_SYSTEM_PROMPT)
const result = yield* handle.process({
if (format.type === "json_schema") system.push(STRUCTURED_OUTPUT_SYSTEM_PROMPT) // kilocode_change
const result = yield* handle.process({ // kilocode_change
// kilocode_change start - keep Ask/Plan tool filtering hardened against session allows
user: lastUser,
agent,
// kilocode_change start - keep Ask/Plan tool filtering hardened against session allows
permission: KiloSessionPrompt.guardPermissions({ agent, session }),
// kilocode_change end
sessionID,
@@ -239,6 +239,76 @@ describe("saveAlwaysRules", () => {
),
)
it.live("saved always approval does not override hard deny ruleset", () =>
withDir({ git: true }, () =>
Effect.gen(function* () {
const asking = yield* ask({
id: PermissionID.make("permission_hard_deny_seed"),
sessionID: SessionID.make("session_test"),
permission: "bash",
patterns: ["printf seed"],
metadata: {},
always: ["printf *"],
ruleset: [{ permission: "bash", pattern: "*", action: "ask" }],
}).pipe(Effect.forkScoped)
yield* waitForPending(1)
yield* reply({ requestID: PermissionID.make("permission_hard_deny_seed"), reply: "always" })
yield* Fiber.join(asking)
const exit = yield* ask({
sessionID: SessionID.make("session_test"),
permission: "bash",
patterns: ["printf bypass > ask-saved-bypass.txt"],
metadata: {},
always: [],
ruleset: [{ permission: "bash", pattern: "*", action: "ask" }],
hardRuleset: [
{ permission: "bash", pattern: "*", action: "deny" },
{ permission: "bash", pattern: "printf *", action: "allow" },
{ permission: "bash", pattern: "*>*", action: "deny" },
{ permission: "bash", pattern: "* > *", action: "deny" },
],
}).pipe(Effect.exit)
expectFailure(exit, Permission.DeniedError)
}),
),
)
it.live("saved always approval still works when hard ruleset does not deny", () =>
withDir({ git: true }, () =>
Effect.gen(function* () {
const asking = yield* ask({
id: PermissionID.make("permission_hard_ask_seed"),
sessionID: SessionID.make("session_test"),
permission: "bash",
patterns: ["gh issue list"],
metadata: {},
always: ["gh *"],
ruleset: [{ permission: "bash", pattern: "*", action: "ask" }],
}).pipe(Effect.forkScoped)
yield* waitForPending(1)
yield* reply({ requestID: PermissionID.make("permission_hard_ask_seed"), reply: "always" })
yield* Fiber.join(asking)
const result = yield* ask({
sessionID: SessionID.make("session_test"),
permission: "bash",
patterns: ["gh pr list"],
metadata: {},
always: [],
ruleset: [{ permission: "bash", pattern: "*", action: "ask" }],
hardRuleset: [
{ permission: "bash", pattern: "*", action: "deny" },
{ permission: "bash", pattern: "gh *", action: "ask" },
],
})
expect(result).toBeUndefined()
}),
),
)
it.live("accepts hierarchy patterns from metadata.rules", () =>
withDir({ git: true }, () =>
Effect.gen(function* () {
@@ -105,6 +105,7 @@ function errorTool(parts: MessageV2.Part[]) {
return part?.state.status === "error" ? (part as ErrorToolPart) : undefined
}
// kilocode_change start - helpers for Ask/Plan guard regression tests
function names(input: Record<string, unknown> | undefined) {
const tools = input?.tools
if (!Array.isArray(tools)) return []
@@ -129,6 +130,7 @@ const waitQuestion = Effect.fn("test.waitQuestion")(function* (sessionID: Sessio
yield* Effect.sleep("10 millis")
}
})
// kilocode_change end
const mcp = Layer.succeed(
MCP.Service,
@@ -538,6 +540,7 @@ it.live("glob tool keeps instance context during prompt runs", () =>
),
)
// kilocode_change start - assistant tool parts should continue loop despite stop finish
it.live("loop continues when finish is stop but assistant has tool parts", () =>
provideTmpdirServer(
Effect.fnUntraced(function* ({ llm }) {
@@ -567,7 +570,9 @@ it.live("loop continues when finish is stop but assistant has tool parts", () =>
{ git: true, config: providerCfg },
),
)
// kilocode_change end
// kilocode_change start - Ask mode guard coverage
unix("ask agent denies bash redirection during prompt loop", () =>
provideTmpdirServer(
({ dir, llm }) =>
@@ -631,6 +636,167 @@ it.live("ask agent hides edit tools despite session allow-all", () =>
{ git: true, config: providerCfg },
),
)
// kilocode_change end
// kilocode_change start - saved permissions must not bypass Ask/Plan guards
unix("ask agent denies bash redirection despite global allow-everything", () =>
provideTmpdirServer(
({ dir, llm }) =>
withSh(() =>
Effect.gen(function* () {
const permission = yield* Permission.Service
const prompt = yield* SessionPrompt.Service
const sessions = yield* Session.Service
const session = yield* sessions.create({ title: "Ask global guard" })
const file = path.join(dir, "ask-global-bug.txt")
yield* permission.allowEverything({ enable: true })
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-global-bug.txt", description: "write ask bug" })
yield* llm.text("done")
yield* prompt.loop({ sessionID: session.id })
yield* permission.allowEverything({ enable: false })
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 },
),
)
unix("ask agent denies bash redirection despite session allow-everything", () =>
provideTmpdirServer(
({ dir, llm }) =>
withSh(() =>
Effect.gen(function* () {
const permission = yield* Permission.Service
const prompt = yield* SessionPrompt.Service
const sessions = yield* Session.Service
const session = yield* sessions.create({
title: "Ask local guard",
permission: [{ permission: "*", pattern: "*", action: "allow" }],
})
const file = path.join(dir, "ask-session-bug.txt")
yield* permission.allowEverything({ enable: true, sessionID: session.id })
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-session-bug.txt", description: "write ask bug" })
yield* llm.text("done")
yield* prompt.loop({ sessionID: session.id })
yield* permission.allowEverything({ enable: false, 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("plan agent denies non-plan write despite global allow-everything", () =>
provideTmpdirServer(
({ dir, llm }) =>
Effect.gen(function* () {
const permission = yield* Permission.Service
const prompt = yield* SessionPrompt.Service
const sessions = yield* Session.Service
const session = yield* sessions.create({ title: "Plan global guard" })
const plan = Session.plan(session)
const file = path.join(dir, "plan-global-bug.txt")
yield* permission.allowEverything({ enable: true })
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- Stay in plan mode.\n" })
yield* llm.tool("write", { filePath: file, content: "bug\n" })
yield* llm.text("done")
yield* prompt.loop({ sessionID: session.id })
yield* permission.allowEverything({ enable: false })
expect(yield* Effect.promise(() => Bun.file(plan).exists())).toBe(true)
expect(yield* Effect.promise(() => Bun.file(file).exists())).toBe(false)
const msgs = yield* MessageV2.filterCompactedEffect(session.id)
const tools = msgs
.flatMap((msg) => msg.parts)
.filter((part): part is MessageV2.ToolPart => part.type === "tool" && part.tool === "write")
const good = tools.find((part) => (part.state.input as { filePath?: string } | undefined)?.filePath === plan)
const bad = tools.find((part) => (part.state.input as { filePath?: string } | undefined)?.filePath === file)
expect(good?.state.status).toBe("completed")
expect(bad?.state.status).toBe("error")
}),
{ git: true, config: providerCfg },
),
)
it.live("plan agent denies non-plan write despite session allow-everything", () =>
provideTmpdirServer(
({ dir, llm }) =>
Effect.gen(function* () {
const permission = yield* Permission.Service
const prompt = yield* SessionPrompt.Service
const sessions = yield* Session.Service
const session = yield* sessions.create({
title: "Plan local guard",
permission: [{ permission: "*", pattern: "*", action: "allow" }],
})
const plan = Session.plan(session)
const file = path.join(dir, "plan-session-bug.txt")
yield* permission.allowEverything({ enable: true, sessionID: session.id })
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- Stay in plan mode.\n" })
yield* llm.tool("write", { filePath: file, content: "bug\n" })
yield* llm.text("done")
yield* prompt.loop({ sessionID: session.id })
yield* permission.allowEverything({ enable: false, sessionID: session.id })
expect(yield* Effect.promise(() => Bun.file(plan).exists())).toBe(true)
expect(yield* Effect.promise(() => Bun.file(file).exists())).toBe(false)
const msgs = yield* MessageV2.filterCompactedEffect(session.id)
const tools = msgs
.flatMap((msg) => msg.parts)
.filter((part): part is MessageV2.ToolPart => part.type === "tool" && part.tool === "write")
const good = tools.find((part) => (part.state.input as { filePath?: string } | undefined)?.filePath === plan)
const bad = tools.find((part) => (part.state.input as { filePath?: string } | undefined)?.filePath === file)
expect(good?.state.status).toBe("completed")
expect(bad?.state.status).toBe("error")
}),
{ git: true, config: providerCfg },
),
)
// kilocode_change end
unix("plan follow-up stops before queued mutations after plan_exit", () =>
provideTmpdirServer(