Skip to content
This repository was archived by the owner on Apr 8, 2026. It is now read-only.

Commit bf59abc

Browse files
committed
Restore prompt-approved tool execution in CLI parity runs
ConversationRuntime already performs permission-policy checks and interactive approvals before dispatching a tool. The CLI tool executor was routing those same tool calls back through GlobalToolRegistry::execute, which re-ran the enforcer without a prompter and flipped approved bash calls back into denials. Add a preauthorized execution path for runtime-dispatched tools, keep registry enforcement for direct callers, and format the files that were already tripping rustfmt on main. Constraint: CI on main was failing both cargo fmt and the mock parity harness after permission enforcement landed Rejected: Remove registry enforcement globally | would reopen direct-dispatch permission gaps Confidence: high Scope-risk: narrow Reversibility: clean Directive: Use execute_preauthorized only after ConversationRuntime or an equivalent caller has already completed permission gating Tested: cargo fmt --all --check; cargo test -p rusty-claude-cli; cargo test -p tools Not-tested: Full workspace test matrix beyond the Rust CI workflow targets
1 parent 618a79a commit bf59abc

File tree

3 files changed

+60
-23
lines changed

3 files changed

+60
-23
lines changed

rust/crates/runtime/src/mcp_tool_bridge.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,10 @@ impl McpToolRegistry {
191191
let mut manager = manager
192192
.lock()
193193
.map_err(|_| "mcp server manager lock poisoned".to_string())?;
194-
manager.discover_tools().await.map_err(|error| error.to_string())?;
194+
manager
195+
.discover_tools()
196+
.await
197+
.map_err(|error| error.to_string())?;
195198
let response = manager
196199
.call_tool(&qualified_tool_name, arguments)
197200
.await
@@ -834,7 +837,9 @@ mod tests {
834837
None,
835838
);
836839
registry
837-
.set_manager(Arc::new(Mutex::new(McpServerManager::from_servers(&servers))))
840+
.set_manager(Arc::new(Mutex::new(McpServerManager::from_servers(
841+
&servers,
842+
))))
838843
.expect("manager should only be set once");
839844

840845
let result = registry

rust/crates/rusty-claude-cli/src/main.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,7 @@ use plugins::{PluginHooks, PluginManager, PluginManagerConfig, PluginRegistry};
4040
use render::{MarkdownStreamState, Spinner, TerminalRenderer};
4141
use runtime::{
4242
clear_oauth_credentials, generate_pkce_pair, generate_state, load_system_prompt,
43-
parse_oauth_callback_request_target,
44-
permission_enforcer::PermissionEnforcer,
43+
parse_oauth_callback_request_target, permission_enforcer::PermissionEnforcer,
4544
resolve_sandbox_status, save_oauth_credentials, ApiClient, ApiRequest, AssistantEvent,
4645
CompactionConfig, ConfigLoader, ConfigSource, ContentBlock, ConversationMessage,
4746
ConversationRuntime, MessageRole, OAuthAuthorizationRequest, OAuthConfig,
@@ -4976,7 +4975,7 @@ impl ToolExecutor for CliToolExecutor {
49764975
}
49774976
let value = serde_json::from_str(input)
49784977
.map_err(|error| ToolError::new(format!("invalid tool input JSON: {error}")))?;
4979-
match self.tool_registry.execute(tool_name, &value) {
4978+
match self.tool_registry.execute_preauthorized(tool_name, &value) {
49804979
Ok(output) => {
49814980
if self.emit_output {
49824981
let markdown = format_tool_result(tool_name, &output, false);

rust/crates/tools/src/lib.rs

Lines changed: 51 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,10 @@ impl GlobalToolRegistry {
127127
}
128128
}
129129

130-
Ok(Self { plugin_tools, enforcer: None })
130+
Ok(Self {
131+
plugin_tools,
132+
enforcer: None,
133+
})
131134
}
132135

133136
#[must_use]
@@ -242,8 +245,23 @@ impl GlobalToolRegistry {
242245
}
243246

244247
pub fn execute(&self, name: &str, input: &Value) -> Result<String, String> {
245-
if let Some(enforcer) = &self.enforcer {
246-
enforce_permission_check(enforcer, name, input)?;
248+
self.execute_inner(name, input, true)
249+
}
250+
251+
pub fn execute_preauthorized(&self, name: &str, input: &Value) -> Result<String, String> {
252+
self.execute_inner(name, input, false)
253+
}
254+
255+
fn execute_inner(
256+
&self,
257+
name: &str,
258+
input: &Value,
259+
enforce_permissions: bool,
260+
) -> Result<String, String> {
261+
if enforce_permissions {
262+
if let Some(enforcer) = &self.enforcer {
263+
enforce_permission_check(enforcer, name, input)?;
264+
}
247265
}
248266
if mvp_tool_specs().iter().any(|spec| spec.name == name) {
249267
return execute_tool(name, input);
@@ -2798,7 +2816,10 @@ struct SubagentToolExecutor {
27982816

27992817
impl SubagentToolExecutor {
28002818
fn new(allowed_tools: BTreeSet<String>) -> Self {
2801-
Self { allowed_tools, enforcer: None }
2819+
Self {
2820+
allowed_tools,
2821+
enforcer: None,
2822+
}
28022823
}
28032824

28042825
fn with_enforcer(mut self, enforcer: PermissionEnforcer) -> Self {
@@ -2817,8 +2838,7 @@ impl ToolExecutor for SubagentToolExecutor {
28172838
let value = serde_json::from_str(input)
28182839
.map_err(|error| ToolError::new(format!("invalid tool input JSON: {error}")))?;
28192840
if let Some(enforcer) = &self.enforcer {
2820-
enforce_permission_check(enforcer, tool_name, &value)
2821-
.map_err(ToolError::new)?;
2841+
enforce_permission_check(enforcer, tool_name, &value).map_err(ToolError::new)?;
28222842
}
28232843
execute_tool(tool_name, &value).map_err(ToolError::new)
28242844
}
@@ -4219,8 +4239,8 @@ mod tests {
42194239
use super::{
42204240
agent_permission_policy, allowed_tools_for_subagent, execute_agent_with_spawn,
42214241
execute_tool, final_assistant_text, mvp_tool_specs, permission_mode_from_plugin,
4222-
persist_agent_terminal_state, push_output_block, AgentInput, AgentJob,
4223-
GlobalToolRegistry, SubagentToolExecutor,
4242+
persist_agent_terminal_state, push_output_block, AgentInput, AgentJob, GlobalToolRegistry,
4243+
SubagentToolExecutor,
42244244
};
42254245
use api::OutputContentBlock;
42264246
use runtime::{
@@ -4243,10 +4263,11 @@ mod tests {
42434263
}
42444264

42454265
fn permission_policy_for_mode(mode: PermissionMode) -> PermissionPolicy {
4246-
mvp_tool_specs().into_iter().fold(
4247-
PermissionPolicy::new(mode),
4248-
|policy, spec| policy.with_tool_requirement(spec.name, spec.required_permission),
4249-
)
4266+
mvp_tool_specs()
4267+
.into_iter()
4268+
.fold(PermissionPolicy::new(mode), |policy, spec| {
4269+
policy.with_tool_requirement(spec.name, spec.required_permission)
4270+
})
42504271
}
42514272

42524273
#[test]
@@ -4321,7 +4342,9 @@ mod tests {
43214342
.expect_err("subagent write tool should be denied before dispatch");
43224343

43234344
// then
4324-
assert!(error.to_string().contains("requires workspace-write permission"));
4345+
assert!(error
4346+
.to_string()
4347+
.contains("requires workspace-write permission"));
43254348
}
43264349

43274350
#[test]
@@ -5813,7 +5836,10 @@ printf 'pwsh:%s' "$1"
58135836
fn given_read_only_enforcer_when_write_file_then_denied() {
58145837
let registry = read_only_registry();
58155838
let err = registry
5816-
.execute("write_file", &json!({ "path": "/tmp/x.txt", "content": "x" }))
5839+
.execute(
5840+
"write_file",
5841+
&json!({ "path": "/tmp/x.txt", "content": "x" }),
5842+
)
58175843
.expect_err("write_file should be denied in read-only mode");
58185844
assert!(
58195845
err.contains("current mode is read-only"),
@@ -5847,10 +5873,7 @@ printf 'pwsh:%s' "$1"
58475873
fs::write(&file, "content\n").expect("write test file");
58485874

58495875
let registry = read_only_registry();
5850-
let result = registry.execute(
5851-
"read_file",
5852-
&json!({ "path": file.display().to_string() }),
5853-
);
5876+
let result = registry.execute("read_file", &json!({ "path": file.display().to_string() }));
58545877
assert!(result.is_ok(), "read_file should be allowed: {result:?}");
58555878

58565879
let _ = fs::remove_dir_all(root);
@@ -5876,6 +5899,16 @@ printf 'pwsh:%s' "$1"
58765899
assert_eq!(output["stdout"], "ok");
58775900
}
58785901

5902+
#[test]
5903+
fn given_enforcer_when_execute_preauthorized_then_skips_redundant_permission_check() {
5904+
let registry = read_only_registry();
5905+
let result = registry
5906+
.execute_preauthorized("bash", &json!({ "command": "printf 'ok'" }))
5907+
.expect("preauthorized bash should skip registry enforcement");
5908+
let output: serde_json::Value = serde_json::from_str(&result).expect("json");
5909+
assert_eq!(output["stdout"], "ok");
5910+
}
5911+
58795912
struct TestServer {
58805913
addr: SocketAddr,
58815914
shutdown: Option<std::sync::mpsc::Sender<()>>,

0 commit comments

Comments
 (0)