Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/strict-ask-plan-guards.md
Original file line number Diff line number Diff line change
@@ -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.
16 changes: 8 additions & 8 deletions .kilo/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 8 additions & 8 deletions .kilocode/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

139 changes: 83 additions & 56 deletions packages/opencode/src/kilocode/agent/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -121,8 +109,84 @@ 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",
"*|*": "deny",
Comment thread
alex-alecu marked this conversation as resolved.
"*;*": "deny",
"*&&*": "deny",
"*&*": "deny",
"*$(*": "deny",
"*`*": "deny",
"*>*": "deny",
Comment thread
alex-alecu marked this conversation as resolved.
"* > *": "deny",
"*>>*": "deny",
"* >> *": "deny",
"*>|*": "deny",
"* >| *": "deny",
"sort -o *": "deny",
Comment thread
alex-alecu marked this conversation as resolved.
"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.
Expand Down Expand Up @@ -232,27 +296,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"),
),
}
}
Expand Down Expand Up @@ -355,29 +404,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",
Expand Down
10 changes: 7 additions & 3 deletions packages/opencode/src/kilocode/permission/drain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>
}

Expand All @@ -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
Expand Down
15 changes: 15 additions & 0 deletions packages/opencode/src/kilocode/session/prompt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider moving this to a const at the top, and use it here and in hardPermissions()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in f57c65a.

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.
Expand Down
Loading
Loading