fix: keep explicit denies working for read-only first-party tools
decision_for returned Allow for any read-only tool at its first line, before consulting the settings.toml deny rules or per-tool overrides. Once the web-search and repo-tabs work reclassified mcp__sigit__{list_*,get_*, search_code,web_search} as read-only, a user's explicit deny of one of those tools stopped being enforced: it had been honored in v1.4.1, when those names still counted as mutating. decision_for now checks deny rules for every tool, read-only included, and also honors an explicit per-tool deny mode for a read-only tool. It reads the explicit override rather than the effective mode, so the default ask or deny mode never starts prompting on ordinary read_file calls. Read-only tools that aren't explicitly denied still run without a prompt.
paydii committed
Jul 11, 2026 at 15:10 UTC
2b547a984017eba79a51b20d30b872123ce068fe
2 files changed
+74
-14
src/permissions.rs
+66
-14
@@ -242,11 +242,42 @@ fn matchable_argument(tool_name: &str, arguments: &str) -> Option<String> {
242
/// string, consulted by rule patterns and granular session grants. See the
243
/// module docs for the layering.
244
pub fn decision_for(session: &str, tool_name: &str, arguments: &str) -> Decision {
245
- if classify(tool_name) == ToolRisk::ReadOnly {
245
+ let read_only = classify(tool_name) == ToolRisk::ReadOnly;
246
+ let argument = matchable_argument(tool_name, arguments);
247
+ let rules = settings::permission_rules();
248
+
249
+ // An explicit deny applies to every tool, read-only ones included. This is
250
+ // what lets a user still switch off a first-party query tool that
251
+ // `classify` now treats as read-only (`web_search`, `list_issues`, …);
252
+ // without it, the reclassification would silently stop honoring a deny that
253
+ // was enforced when those names counted as mutating.
254
+ if let Some(rule) = rules
255
+ .deny
256
+ .iter()
257
+ .find(|rule| rule_matches(rule, tool_name, argument.as_deref()))
258
+ {
259
+ return Decision::Deny(format!(
260
+ "`{tool_name}` is denied by the permission rule `{rule}` in settings.toml. \
261
+ Do not retry it; work without this tool or ask the user to change \
262
+ the policy."
263
+ ));
264
+ }
265
+
266
+ if read_only {
267
+ // Read-only tools never prompt and are never plan-mode blocked, but an
268
+ // explicit per-tool `deny` still turns one off. Crucially we consult
269
+ // the *explicit* override, not `permission_mode_for`, so the default
270
+ // `ask`/`deny` mode never starts prompting on every read_file.
271
+ if settings::permission_mode_explicit(tool_name) == Some(PermissionMode::Deny) {
272
+ return Decision::Deny(format!(
273
+ "`{tool_name}` is denied by the permission policy in settings.toml. \
274
+ Do not retry it; work without this tool or ask the user to change \
275
+ the policy."
276
+ ));
277
+ }
278
return Decision::Allow;
279
}
280
249
- let argument = matchable_argument(tool_name, arguments);
281
let (plan_mode, granted) = with_session(session, |s| {
282
(
283
s.plan_mode,
@@ -263,18 +294,6 @@ pub fn decision_for(session: &str, tool_name: &str, arguments: &str) -> Decision
294
return Decision::Allow;
295
}
296
266
- let rules = settings::permission_rules();
267
- if let Some(rule) = rules
268
- .deny
269
- .iter()
270
- .find(|rule| rule_matches(rule, tool_name, argument.as_deref()))
271
- {
272
- return Decision::Deny(format!(
273
- "`{tool_name}` is denied by the permission rule `{rule}` in settings.toml. \
274
- Do not retry it; work without this tool or ask the user to change \
275
- the policy."
276
- ));
277
- }
297
if rules
298
.allow
299
.iter()
@@ -486,6 +505,39 @@ mod tests {
505
}
506
}
507
508
+ #[test]
509
+ fn read_only_tool_still_honors_an_explicit_deny() {
510
+ let _guard = env_guard();
511
+ // A deny rule listing a read-only first-party query tool is enforced,
512
+ // even though the tool never prompts otherwise.
513
+ store_rules(&[], &["mcp__sigit__web_search"]);
514
+ assert!(matches!(
515
+ decision_for("t", "mcp__sigit__web_search", "{}"),
516
+ Decision::Deny(_)
517
+ ));
518
+ // A tool with no deny rule is unaffected.
519
+ assert_eq!(
520
+ decision_for("t", "mcp__sigit__list_issues", "{}"),
521
+ Decision::Allow
522
+ );
523
+
524
+ // An explicit per-tool `deny` mode also turns a read-only tool off,
525
+ // while the default mode never makes read-only tools prompt.
526
+ let mut settings = settings::load();
527
+ settings.permissions.rules.deny.clear();
528
+ settings.permissions.default = PermissionMode::Ask;
529
+ settings
530
+ .permissions
531
+ .tools
532
+ .insert("mcp__sigit__web_search".to_string(), PermissionMode::Deny);
533
+ settings::store(&settings).unwrap();
534
+ assert!(matches!(
535
+ decision_for("t", "mcp__sigit__web_search", "{}"),
536
+ Decision::Deny(_)
537
+ ));
538
+ assert_eq!(decision_for("t", "read_file", "{}"), Decision::Allow);
539
+ }
540
+
541
#[test]
542
fn official_mcp_mutating_and_foreign_servers_stay_gated() {
543
for tool in [
src/settings.rs
+8
@@ -229,6 +229,14 @@ pub fn permission_rules() -> PermissionRules {
229
load().permissions.rules
230
}
231
232
+/// The tool's explicit `[permissions.tools]` override, or `None` when it has
233
+/// no entry. Unlike [`permission_mode_for`] this never falls back to the
234
+/// default or the env override, so callers can tell "the user set a mode for
235
+/// this exact tool" apart from "the tool inherits the default".
236
+pub fn permission_mode_explicit(tool_name: &str) -> Option<PermissionMode> {
237
+ load().permissions.tools.get(tool_name).copied()
238
+}
239
+
240
/// The effective permission mode for one tool: its `[permissions.tools]`
241
/// override when present, else the default (see [`permission_default`]).
242
pub fn permission_mode_for(tool_name: &str) -> PermissionMode {