From 77afde768c3a95b1acb51d59573c08e4c95b3232 Mon Sep 17 00:00:00 2001 From: Yeachan-Heo Date: Tue, 28 Apr 2026 05:44:14 +0000 Subject: [PATCH] Clarify allowed tool status handling Reject empty --allowedTools inputs instead of treating them as an empty restriction, and surface status JSON metadata that distinguishes default unrestricted tools from flag-provided allow lists. Confidence: high Scope-risk: narrow Tested: cargo test -p rusty-claude-cli rejects_empty_allowed_tools_flag -- --nocapture Tested: cargo test -p tools allowed_tools_rejects_empty_token_lists -- --nocapture Tested: cargo check -p rusty-claude-cli -p tools Tested: cargo test -p rusty-claude-cli -p tools Not-tested: full workspace cargo fmt --check is blocked by pre-existing unrelated formatting drift --- rust/crates/rusty-claude-cli/src/main.rs | 103 +++++++++++++++++++++-- rust/crates/tools/src/lib.rs | 22 +++++ 2 files changed, 118 insertions(+), 7 deletions(-) diff --git a/rust/crates/rusty-claude-cli/src/main.rs b/rust/crates/rusty-claude-cli/src/main.rs index c4ba812..64ba249 100644 --- a/rust/crates/rusty-claude-cli/src/main.rs +++ b/rust/crates/rusty-claude-cli/src/main.rs @@ -372,7 +372,14 @@ fn run() -> Result<(), Box> { model_flag_raw, permission_mode, output_format, - } => print_status_snapshot(&model, model_flag_raw.as_deref(), permission_mode, output_format)?, + allowed_tools, + } => print_status_snapshot( + &model, + model_flag_raw.as_deref(), + permission_mode, + output_format, + allowed_tools.as_ref(), + )?, CliAction::Sandbox { output_format } => print_sandbox_status_snapshot(output_format)?, CliAction::Prompt { prompt, @@ -510,6 +517,7 @@ enum CliAction { model_flag_raw: Option, permission_mode: PermissionMode, output_format: CliOutputFormat, + allowed_tools: Option, }, Sandbox { output_format: CliOutputFormat, @@ -844,9 +852,14 @@ fn parse_args(args: &[String]) -> Result { if let Some(action) = parse_local_help_action(&rest) { return action; } - if let Some(action) = - parse_single_word_command_alias(&rest, &model, model_flag_raw.as_deref(), permission_mode_override, output_format) - { + if let Some(action) = parse_single_word_command_alias( + &rest, + &model, + model_flag_raw.as_deref(), + permission_mode_override, + output_format, + allowed_tools.clone(), + ) { return action; } @@ -1051,6 +1064,7 @@ fn parse_single_word_command_alias( model_flag_raw: Option<&str>, permission_mode_override: Option, output_format: CliOutputFormat, + allowed_tools: Option, ) -> Option> { if rest.is_empty() { return None; @@ -1095,6 +1109,7 @@ fn parse_single_word_command_alias( model_flag_raw: model_flag_raw.map(str::to_string), // #148 permission_mode: permission_mode_override.unwrap_or_else(default_permission_mode), output_format, + allowed_tools, })), "sandbox" => Some(Ok(CliAction::Sandbox { output_format })), "doctor" => Some(Ok(CliAction::Doctor { output_format })), @@ -3226,6 +3241,7 @@ fn run_resume_command( default_permission_mode().as_str(), &context, None, // #148: resumed sessions don't have flag provenance + None, )), }) } @@ -5417,6 +5433,7 @@ fn print_status_snapshot( model_flag_raw: Option<&str>, permission_mode: PermissionMode, output_format: CliOutputFormat, + allowed_tools: Option<&AllowedToolSet>, ) -> Result<(), Box> { let usage = StatusUsage { message_count: 0, @@ -5450,6 +5467,7 @@ fn print_status_snapshot( permission_mode.as_str(), &context, Some(&provenance), + allowed_tools, ))? ), } @@ -5467,6 +5485,7 @@ fn status_json_value( // that don't have provenance (legacy resume paths) pass None, in which // case both new fields are omitted. provenance: Option<&ModelProvenance>, + allowed_tools: Option<&AllowedToolSet>, ) -> serde_json::Value { // #143: top-level `status` marker so claws can distinguish // a clean run from a degraded run (config parse failed but other fields @@ -5476,6 +5495,7 @@ fn status_json_value( let degraded = context.config_load_error.is_some(); let model_source = provenance.map(|p| p.source.as_str()); let model_raw = provenance.and_then(|p| p.raw.clone()); + let allowed_tool_entries = allowed_tools.map(|tools| tools.iter().cloned().collect::>()); json!({ "kind": "status", "status": if degraded { "degraded" } else { "ok" }, @@ -5484,6 +5504,11 @@ fn status_json_value( "model_source": model_source, "model_raw": model_raw, "permission_mode": permission_mode, + "allowed_tools": { + "source": if allowed_tools.is_some() { "flag" } else { "default" }, + "restricted": allowed_tools.is_some(), + "entries": allowed_tool_entries, + }, "usage": { "messages": usage.message_count, "turns": usage.turns, @@ -9807,6 +9832,18 @@ mod tests { assert!(error.contains("unsupported tool in --allowedTools: teleport")); } + #[test] + fn rejects_empty_allowed_tools_flag() { + for raw in ["", ",,"] { + let error = parse_args(&["--allowedTools".to_string(), raw.to_string()]) + .expect_err("empty allowedTools should be rejected"); + assert!( + error.contains("--allowedTools was provided with no usable tool names"), + "unexpected error for {raw:?}: {error}" + ); + } + } + #[test] fn parses_system_prompt_options() { let args = vec![ @@ -10261,7 +10298,14 @@ mod tests { cumulative: runtime::TokenUsage::default(), estimated_tokens: 0, }; - let json = super::status_json_value(Some("test-model"), usage, "workspace-write", &context, None); + let json = super::status_json_value( + Some("test-model"), + usage, + "workspace-write", + &context, + None, + None, + ); assert_eq!( json.get("status").and_then(|v| v.as_str()), Some("degraded"), @@ -10280,6 +10324,44 @@ mod tests { ); assert!(json.get("workspace").is_some(), "workspace field still reported"); assert!(json.get("sandbox").is_some(), "sandbox field still reported"); + assert_eq!( + json.pointer("/allowed_tools/source").and_then(|v| v.as_str()), + Some("default"), + "default status should expose unrestricted tool source: {json}" + ); + assert_eq!( + json.pointer("/allowed_tools/restricted").and_then(|v| v.as_bool()), + Some(false), + "default status should expose unrestricted tool state: {json}" + ); + + let allowed: super::AllowedToolSet = ["read_file", "grep_search"] + .into_iter() + .map(str::to_string) + .collect(); + let restricted_json = super::status_json_value( + Some("test-model"), + usage, + "workspace-write", + &context, + None, + Some(&allowed), + ); + assert_eq!( + restricted_json + .pointer("/allowed_tools/source") + .and_then(|v| v.as_str()), + Some("flag"), + "flag status should expose allow-list source: {restricted_json}" + ); + assert_eq!( + restricted_json + .pointer("/allowed_tools/entries") + .and_then(|v| v.as_array()) + .map(Vec::len), + Some(2), + "flag status should expose allow-list entries: {restricted_json}" + ); // Clean path: no config error → status: "ok", config_load_error: null. let clean_cwd = root.join("project-with-clean-config"); @@ -10288,8 +10370,14 @@ mod tests { super::status_context(None).expect("clean status_context should succeed") }); assert!(clean_context.config_load_error.is_none()); - let clean_json = - super::status_json_value(Some("test-model"), usage, "workspace-write", &clean_context, None); + let clean_json = super::status_json_value( + Some("test-model"), + usage, + "workspace-write", + &clean_context, + None, + None, + ); assert_eq!( clean_json.get("status").and_then(|v| v.as_str()), Some("ok"), @@ -10366,6 +10454,7 @@ mod tests { model_flag_raw: None, // #148: no --model flag passed permission_mode: PermissionMode::DangerFullAccess, output_format: CliOutputFormat::Text, + allowed_tools: None, } ); assert_eq!( diff --git a/rust/crates/tools/src/lib.rs b/rust/crates/tools/src/lib.rs index 1890190..f3d1849 100644 --- a/rust/crates/tools/src/lib.rs +++ b/rust/crates/tools/src/lib.rs @@ -240,6 +240,13 @@ impl GlobalToolRegistry { } } + if allowed.is_empty() { + return Err(format!( + "--allowedTools was provided with no usable tool names (got `{}`). Omit the flag to allow all tools.", + values.join(" ") + )); + } + Ok(Some(allowed)) } @@ -6883,6 +6890,21 @@ mod tests { assert!(empty_permission.contains("unsupported plugin permission: ")); } + #[test] + fn allowed_tools_rejects_empty_token_lists() { + let registry = GlobalToolRegistry::builtin(); + + for raw in ["", ",,", " "] { + let err = registry + .normalize_allowed_tools(&[raw.to_string()]) + .expect_err("empty allow-list input should be rejected"); + assert!( + err.contains("--allowedTools was provided with no usable tool names"), + "unexpected error for {raw:?}: {err}" + ); + } + } + #[test] fn runtime_tools_extend_registry_definitions_permissions_and_search() { let registry = GlobalToolRegistry::builtin()