fix(cli): keep ask and plan modes read-only under broad permission rules

[johnnyeric]

Issue

Fixes #12053

Context

The reporter had never edited a permission and had no catch-all rule. Their whole relevant config is bash: { "*": "ask", "*bevy_app*": null, "ls *": "allow" } — the shape the settings UI writes.

That one rule is enough. Permission rules are positional (Permission.evaluate is a findLast), and patchAgents merged the global config after the read-only guards, so "*": "ask" shadowed Ask's entire readOnlyBash allowlist. Ask never lost its edit deny — it didn't need it. Every shell command became a prompt instead of a refusal, the user approved one, and Ask edited the codebase.

Their config verbatim through the real Agent.Service:

command	Ask on main	Ask here	Code
echo 'pwned' > src/main.rs	ask	deny	ask
python - <<'PY' … PY	ask	deny	ask
rm -rf src	ask	deny	ask
ls -la	allow	allow	allow

On main, Ask and Code are identical: "read-only mode" had degraded to "mode that asks first."

Two automatic paths make this worse, which is why the fix goes past bash: "Always allow" persists <command> * rules into the global config (tool/shell.ts → config.updateGlobal), so approving python - * once in Code silently grants it to Ask forever; and "Allow everything" writes a bare {"*": {"*": "allow"}}, which reaches every tool.

Implementation

baseline() reapplies the guard after the user layer for ask and plan: the catch-all deny plus the read-only allowlist, the user's rules re-expanded onto safe permissions by exact name, then the bash, MCP and guarded-deny ceilings. Config can no longer widen bash, task, write, notebook_edit, notebook_execute, agent_manager, repo_clone or interactive_terminal for these modes.

Worth reviewer attention:

• No pattern-shape classifier. Shape does not carry intent, and a narrow python - * is untrustworthy precisely because "Always allow" writes it automatically. The boundary is provenance: a top-level rule never widens a guarded tool, and agent.<name>.permission is the only opt-in.
• Layer 1 reapplies the allowlist with the deny. The deny alone strands read/grep/plan_exit and Ask cannot read a file. Every other test still passed in that state, hence the explicit "keep their safe tools" cases.
• Blanket vs targeted denies. A * deny is an absolute ceiling; one aimed at a single target — plan's task: { general: "deny" } — is a default that an exact-named user rule may lift, preserving upstream's per-subagent opt-in (test/agent/agent.test.ts). No wildcard qualifies.
• Delegation carries the same set. Plan delegates to the built-in explore, which builds its ruleset from the same config. KiloTask.inherited now derives its mutation set from the exported guarded list, so the two cannot drift.

Behavior change: two existing tests moved from the global form to the per-agent form, because the global form is the vulnerability.

Out of scope, verified rather than assumed: Permission.resolve promotes ask → allow under a saved approval, so the MCP ask is not a runtime floor (needs hardRuleset); and orchestrator has the same bug class, tracked separately as it is deprecated.

Screenshots / Video

Ask

Before

### After

Plan

After

How to Test

Manual/local verification

Executed by the agent against the real Agent.Service (no mocks): the table above, from the issue's verbatim config; plus Plan → explore under a catch-all, where agent_manager, repo_clone and write went allow before and deny after. Code mode's ruleset is identical before and after.

331 pass / 0 fail across test/agent/agent.test.ts, agent-permission-overrides, ask-agent-permissions, plan-mode-subagent-bypass, permission-task, permission/next, tool/task, agent-skill-permissions, agent-routing. bun turbo typecheck clean (29/29). check-opencode-annotations.ts --worktree: no shared upstream files touched.

Reviewer test steps

1. Put the issue's permission block into ~/.config/kilo/kilo.jsonc, with no agent.* overrides.
2. In Ask mode, ask the model to change a file. On main you get a prompt and it succeeds; here it is denied outright.
3. Confirm Ask can still read, grep, glob and run ls -la.
4. Add agent.ask.permission.bash: { "cargo search *": "allow" } and confirm that one command works again and Plan does not inherit it.

Blocked checks and substitute verification

• session-export/e2e fails in CI, but also fails on main (run 31814004052) — pre-existing.
• session-prompt-permission-refresh fails locally both here and with main's agent/index.ts restored, including standalone — pre-existing.

Checklist

• Issue linked above, or exception explained
• Tests/verification described
• Screenshots/video included for visual changes, or marked N/A
• Changeset considered for user-facing changes
• I personally reviewed the diff and can explain the changes, including any AI-assisted work.

Get in Touch

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 727bf37bb2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Incremental review of commit 3f9b0b1 (changes since 727bf37). The new commit resolves the previously flagged finding: guarded is now exported and KiloTask.inherited builds its ceiling set from it (["edit", ...guarded]), so Plan-delegated subagent sessions inherit denies for write, agent_manager, repo_clone, interactive_terminal, and task in addition to the existing edit/bash/notebook set. Verified against Permission.evaluate findLast semantics: the blanket-push loop only fires when the caller's ruleset evaluates the guarded tool to deny at pattern *, so Plan's task: { "*": "allow" } correctly avoids a blanket task deny while its bash/write ceilings propagate, and the targeted task: { general: "deny" } is carried verbatim by the deny filter. The session: Pick<Session.Info, "permission"> narrowing is compatible with the real caller (src/tool/task.ts), and the new guarded import is only dereferenced at call time. The new test composes the real Agent.Service-loaded plan/explore agents exactly as the task tool does, with no mocks.

Files Reviewed (3 files)

• packages/opencode/src/kilocode/agent/index.ts
• packages/opencode/src/kilocode/tool/task.ts
• packages/opencode/test/kilocode/agent-permission-overrides.test.ts

Previous Review Summary (commit 727bf37)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 727bf37)

Status: No Issues Found | Recommendation: Merge

lgtm — the baseline() layering was verified against Permission.evaluate's findLast semantics: the reapplied catch-all deny plus read-only allowlist, exact-name re-expansion of user rules onto safe permissions only, and the bash/MCP/guarded-deny ceilings compose correctly, and user denies still land last. The MCP server-name collision fix (emitting guardedDenies after the ...mcp spread) is sound, the blanket-vs-targeted deny distinction for plan's task: { general: "deny" } cannot be lifted by wildcards, and the agent.<name>.permission opt-in path is confirmed to merge after patchAgents. Tests exercise the real Agent.Service with no mocks, changeset is present and user-facing, and all changes are confined to kilo-owned paths.

Files Reviewed (3 files)

• .changeset/ask-mode-permission-boundary.md
• packages/opencode/src/kilocode/agent/index.ts
• packages/opencode/test/kilocode/agent-permission-overrides.test.ts

---

_{Reviewed by kimi-k3 · Input: 209.1K · Output: 10.5K · Cached: 413.2K}

_{Review guidance: REVIEW.md from base branch main}