fix(core): always deny tool calls for system agents - #10091
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge OverviewAll changes are correct and well-tested. The incremental diff introduces:
The double-call pattern (once inside Files Reviewed (4 files)
Reviewed by claude-4.6-sonnet-20260217 · 530,833 tokens Review guidance: REVIEW.md from base branch |
45221e2 to
1af3ecd
Compare
|
@shssoichiro my bot found this: High: the new deny is applied too early, so agent-specific config can still re-enable tools for title, summary, and compaction. patchAgents() resets them at packages/opencode/src/kilocode/agent/index.ts:440, but cfg.agent..permission is merged afterward at packages/opencode/src/agent/agent.ts:253 and packages/opencode/src/agent/agent.ts:286. Those system agents are explicitly configurable at packages/opencode/src/config/config.ts:214, and permission evaluation is last-match-wins at packages/opencode/src/permission/evaluate.ts:9. Example: agent.title.permission.bash = "allow" wins after the new "": "deny", so the PR does not yet satisfy “always deny tool calls.” True? |
1af3ecd to
0346863
Compare
|
Yes, it does seem to be valid. I added a test case for this, and refactored to resolve the issue. |
|
I found one P1 issue in the latest version: The hardening pass is still bypassable because it keys off the mutable agent Impact: a normal config such as this can keep tools enabled for a hidden system utility agent, preserving the tool-execution risk this PR is meant to remove: {
"agent": {
"title": {
"name": "custom-title",
"permission": { "bash": "allow" }
}
}
}Suggested fix: harden by the original agent identity/map key instead of only |
2bd296b to
be234fa
Compare
|
Thanks for submitting, this can still be bypassed.
Sessions can accept permission rules through the public create/update APIs, while prompt input accepts an arbitrary agent name. This means a later session-level allow overrides the wildcard deny installed on I reproduced this locally with a fake OpenAI-compatible provider that always requests the
SESSION_ID=$(
curl -fsS -X POST \
-H 'content-type: application/json' \
-H "x-kilo-directory: $PWD" \
--data '{
"title": "system agent permission repro",
"permission": [
{
"permission": "read",
"pattern": "*",
"action": "allow"
}
]
}' \
http://127.0.0.1:18081/session |
jq -r '.id'
)
curl -fsS -X POST \
-H 'content-type: application/json' \
-H "x-kilo-directory: $PWD" \
--data '{
"agent": "title",
"model": {
"providerID": "fake",
"modelID": "tool-test"
},
"parts": [
{
"type": "text",
"text": "Attempt the model response."
}
]
}' \
"http://127.0.0.1:18081/session/$SESSION_ID/prompt_async"
Without the session permission, the same provider receives no tools and Kilo reports the forced call as unavailable. A normal Can we enforce this as an invocation-time hard rule after session permissions are merged, rather than only replacing the configured agent rules? |
|
On another thought, this does not qualify as an edge case for normal usage. Let's try this. |
…-tools fix(core): always deny tool calls for system agents
Context
The Title agent would frequently attempt to make tool calls, even though its system prompt tells it not to do so. This resulted in a broken title generation. This was traced back to top-level user allow rules overriding the
deny: "*"rule for these agents.Implementation
Hardcode
deny: "*"for the Title, Summarize, and Compaction agents. Although this issue was most noticeable for the Title agent, it's possible it was impacting Summarize and Compaction as well, and it seems reasonable to deny tools to those agents as well, since they have no reason to be making tool calls in any situation.Get in Touch
ExpedientFalcon on Discord