fix(cli): harden ask and plan modes

.changeset/strict-ask-plan-guards.md

@@ -0,0 +1,5 @@
+---
+"@kilocode/cli": patch
+---
+
+Prevent Ask and Plan modes, including saved or allow-all approvals, from editing files before an explicit implementation step.

packages/opencode/src/kilocode/agent/index.ts

@@ -2,25 +2,22 @@
 import { Permission } from "@/permission"
 import { NamedError } from "@opencode-ai/shared/util/error"
 import { Glob } from "@opencode-ai/shared/util/glob"
-import { Truncate } from "../../tool"
+import * as Truncate from "../../tool/truncate"
 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 { Global } from "@/global"
 
 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",
@@ -41,7 +38,6 @@ export const bash: Record<string, "allow" | "ask" | "deny"> = {
   "whoami *": "allow",
   "printenv *": "allow",
   "man *": "allow",
-  // text processing
   "grep *": "allow",
   "rg *": "allow",
   "ag *": "allow",
@@ -50,26 +46,20 @@ export const bash: Record<string, "allow" | "ask" | "deny"> = {
   "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",
@@ -90,7 +80,6 @@ export const readOnlyBash: Record<string, "allow" | "ask" | "deny"> = {
   "whoami *": "allow",
   "printenv *": "allow",
   "man *": "allow",
-  // text processing (stdout only, no file modification)
   "grep *": "allow",
   "rg *": "allow",
   "ag *": "allow",
@@ -99,7 +88,6 @@ export const readOnlyBash: Record<string, "allow" | "ask" | "deny"> = {
   "cut *": "allow",
   "tr *": "allow",
   "jq *": "allow",
-  // git — allowlist of read-only subcommands, deny everything else
   "git *": "deny",
   "git log *": "allow",
   "git show *": "allow",
@@ -121,8 +109,86 @@ export const readOnlyBash: Record<string, "allow" | "ask" | "deny"> = {
   "git branch -a *": "allow",
   "git branch -r *": "allow",
   "git remote -v *": "allow",
-  // gh — require user approval since commands vary widely
   "gh *": "ask",
+  "*\n*": "deny",
+  "*<(*": "deny",
+  "*|*": "deny",
+  "*;*": "deny",
+  "*&&*": "deny",
+  "*&*": "deny",
+  "*$(*": "deny",
+  "*`*": "deny",
+  "*>*": "deny",
+  "* > *": "deny",
+  "*>>*": "deny",
+  "* >> *": "deny",
+  "*>|*": "deny",
+  "* >| *": "deny",
+  "sort -o *": "deny",
+  "sort * -o *": "deny",
+  "sort --output*": "deny",
+  "sort * --output*": "deny",
+}
+
+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",
+    semantic_search: "allow",
+    external_directory: {
+      [Truncate.GLOB]: "allow",
+    },
+    ...mcp,
+  })
+}
+
+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",
+    semantic_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,
+  })
 }
 
 // Generate per-server MCP wildcard rules that allow MCP tools with user approval.
@@ -232,27 +298,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",
-          },
-          semantic_search: "allow",
-        }),
         user,
+        planGuard(kilo.mcpRules),
+        user.filter((r: Permission.Rule) => r.action === "deny"),
       ),
     }
   }
@@ -355,29 +406,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",
-        semantic_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",

packages/opencode/src/kilocode/permission/drain.ts

@@ -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

packages/opencode/src/kilocode/session/prompt.ts

@@ -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,20 @@ 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"))
+  }
+
+  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.

packages/opencode/src/permission/index.ts

@@ -118,6 +118,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) })))
@@ -157,6 +158,7 @@ export interface Interface {
 interface PendingEntry {
   info: Request
   ruleset: Ruleset // kilocode_change
+  hardRuleset?: Ruleset // kilocode_change
   deferred: Deferred.Deferred<void, RejectedError | CorrectedError>
 }
 
@@ -171,6 +173,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(
@@ -203,7 +216,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
@@ -215,9 +228,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
@@ -242,7 +260,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),
@@ -300,9 +318,13 @@ 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
+        // kilocode_change start
+        const ok = item.info.patterns.every((pattern) => {
+          if (veto(item.info.permission, pattern, item.hardRuleset)) return false
+          return evaluate(item.info.permission, pattern, item.ruleset, approved).action === "allow"
+        })
+        // kilocode_change end
         if (!ok) continue
         pending.delete(id)
         yield* bus.publish(Event.Replied, {
@@ -390,7 +412,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 +429,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,