From c084f415037466e80b5e6796bf1a6d368a6a729e Mon Sep 17 00:00:00 2001 From: coding-sub-agent Date: Thu, 13 Aug 2026 06:36:31 +0000 Subject: [PATCH 1/3] fix(cua-driver): hide policy-disabled MCP tools --- .../rust/crates/cua-driver-core/src/tool.rs | 6 +- .../tests/policy_tools_list_test.rs | 59 +++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 libs/cua-driver/rust/crates/cua-driver/tests/policy_tools_list_test.rs diff --git a/libs/cua-driver/rust/crates/cua-driver-core/src/tool.rs b/libs/cua-driver/rust/crates/cua-driver-core/src/tool.rs index 6e5c993520..32ebeaeb39 100644 --- a/libs/cua-driver/rust/crates/cua-driver-core/src/tool.rs +++ b/libs/cua-driver/rust/crates/cua-driver-core/src/tool.rs @@ -810,11 +810,13 @@ impl ToolRegistry { } pub fn tools_list(&self) -> Value { + let empty_args = Value::Object(serde_json::Map::new()); let list: Vec = self .order .iter() - .filter_map(|n| self.tools.get(n)) - .map(|t| t.def().to_list_entry()) + .filter(|name| crate::policy::authorize_tool_call(name, &empty_args).is_ok()) + .filter_map(|name| self.tools.get(name)) + .map(|tool| tool.def().to_list_entry()) .collect(); // `capability_version` is the contract version for the // capability tokens claimed by each tool entry. Bumped on diff --git a/libs/cua-driver/rust/crates/cua-driver/tests/policy_tools_list_test.rs b/libs/cua-driver/rust/crates/cua-driver/tests/policy_tools_list_test.rs new file mode 100644 index 0000000000..e01a9e3626 --- /dev/null +++ b/libs/cua-driver/rust/crates/cua-driver/tests/policy_tools_list_test.rs @@ -0,0 +1,59 @@ +//! Permission policy must shape the MCP tool roster as well as invocation. + +#![cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))] + +use std::collections::HashSet; + +use cua_driver_testkit::RawDriver; +use serde_json::json; + +#[test] +fn tools_list_hides_policy_denied_tools_and_calls_stay_denied() { + let directory = tempfile::tempdir().expect("temporary policy directory"); + let policy_path = directory.path().join("policy.yaml"); + std::fs::write( + &policy_path, + "allow:\n tools: [get_config]\ndeny:\n tools: [list_apps]\n", + ) + .expect("write permission policy"); + let policy = policy_path.display().to_string(); + + let Some(mut driver) = RawDriver::spawn_with_env(&[("CUA_DRIVER_POLICY_FILE", &policy)]) else { + return; + }; + + driver.send(&json!({"jsonrpc":"2.0","id":1,"method":"initialize","params":{}})); + driver.recv(); + + driver.send(&json!({"jsonrpc":"2.0","id":2,"method":"tools/list"})); + let response = driver.recv(); + let names: HashSet<&str> = response["result"]["tools"] + .as_array() + .expect("tools/list tools array") + .iter() + .filter_map(|tool| tool["name"].as_str()) + .collect(); + assert!( + names.contains("get_config"), + "allowed tool must remain listed" + ); + assert!( + !names.contains("list_apps"), + "policy-denied tool must not be advertised" + ); + + driver.send(&json!({ + "jsonrpc":"2.0", + "id":3, + "method":"tools/call", + "params":{"name":"list_apps","arguments":{}} + })); + let response = driver.recv(); + assert_eq!(response["result"]["isError"], true); + assert!( + response["result"]["content"][0]["text"] + .as_str() + .is_some_and(|message| message.contains("Permission denied")), + "policy-denied invocation must remain rejected: {response}" + ); +} From 34c2f6bfd42c88a8264882b766ca26fee09f660a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 13 Aug 2026 07:04:43 +0000 Subject: [PATCH 2/3] fix(cua-driver): use is_tool_listable for tools/list roster filtering Replace the tools_list filter that called authorize_tool_call with empty args with a new is_tool_listable function. The old approach incorrectly hid tools that are conditionally allowed via allow.rules, because arg constraints can never be satisfied by an empty argument object. is_tool_listable checks only whether a tool has any potential allow path: for YAML policies it returns true when the tool is not explicitly denied and appears in either allow.tools or allow.rules; for Rego it falls back to evaluating with empty args as before. Tools that pass this check remain in the roster so callers can attempt a constrained invocation. Also expand the policy_tools_list_test to include an allow.rules entry and assert that the rule-constrained tool still appears in tools/list. Co-authored-by: r33drichards <57335981+r33drichards@users.noreply.github.com> --- .../rust/crates/cua-driver-core/src/policy.rs | 60 +++++++++++++++++++ .../rust/crates/cua-driver-core/src/tool.rs | 3 +- .../tests/policy_tools_list_test.rs | 13 +++- 3 files changed, 72 insertions(+), 4 deletions(-) diff --git a/libs/cua-driver/rust/crates/cua-driver-core/src/policy.rs b/libs/cua-driver/rust/crates/cua-driver-core/src/policy.rs index b674f55613..91102ce20a 100644 --- a/libs/cua-driver/rust/crates/cua-driver-core/src/policy.rs +++ b/libs/cua-driver/rust/crates/cua-driver-core/src/policy.rs @@ -106,6 +106,28 @@ impl PolicyEngine { Self::Disabled => PolicyDecision::Error("policy support is not enabled".to_owned()), } } + + /// Returns `true` when the tool should appear in `tools/list`. + /// + /// For YAML policies a tool is potentially listable when it is not + /// explicitly denied and has at least one allow path (unconditional or + /// rule-constrained). For Rego, the policy is evaluated with empty + /// arguments as an approximation; Rego rules can implement their own + /// "no-args" sentinel if finer control is needed. + pub fn is_potentially_listable(&self, tool: &str) -> bool { + let tool = canonical_tool_name(tool); + match self { + #[cfg(feature = "yaml")] + Self::Yaml(policy) => policy.is_potentially_listable(tool), + #[cfg(feature = "rego")] + Self::Rego(policy) => { + let empty_args = Value::Object(serde_json::Map::new()); + matches!(policy.evaluate(tool, &empty_args), PolicyDecision::Allow) + } + #[cfg(not(any(feature = "yaml", feature = "rego")))] + Self::Disabled => false, + } + } } fn canonical_tool_name(tool: &str) -> &str { @@ -288,6 +310,30 @@ pub fn authorize_tool_call(tool: &str, args: &Value) -> Result<(), Authorization authorize_policy_layers(tool, args, layers) } +/// Returns `true` when the tool should be advertised in `tools/list`. +/// +/// Unlike [`authorize_tool_call`], this does not require a full argument set: +/// it checks only whether the tool has any potential allow path (is not +/// unconditionally denied). Tools that are conditionally allowed via +/// `allow.rules` are still listed so that callers can attempt a constrained +/// invocation. When no policy is configured, all tools are listable. +pub fn is_tool_listable(tool: &str) -> bool { + let managed = match configured_managed_policy() { + Ok(policy) => policy, + Err(_) => return false, + }; + let user = match configured_policy() { + Ok(policy) => policy, + Err(_) => return false, + }; + for policy in [managed, user].into_iter().flatten() { + if !policy.is_potentially_listable(tool) { + return false; + } + } + true +} + /// Eagerly validate the immutable process policy before any action endpoint is /// exposed. This prevents a configured typo from producing a listening daemon /// that only discovers the error after clients begin issuing calls. @@ -364,6 +410,20 @@ impl YamlPolicy { failures.join("; ") )) } + + /// Returns `true` when the tool is not explicitly denied and has at least + /// one potential allow path (an unconditional entry in `allow.tools` or an + /// entry in `allow.rules`). This is the correct predicate for advertising + /// a tool in `tools/list`: the tool may be conditionally allowed, so we + /// must not hide it just because a call with empty arguments would be + /// denied by unsatisfied constraints. + fn is_potentially_listable(&self, tool: &str) -> bool { + if self.denied_tools.iter().any(|denied| denied == tool) { + return false; + } + self.allowed_tools.iter().any(|allowed| allowed == tool) + || self.rules.iter().any(|rule| rule.tool == tool) + } } #[cfg(feature = "yaml")] diff --git a/libs/cua-driver/rust/crates/cua-driver-core/src/tool.rs b/libs/cua-driver/rust/crates/cua-driver-core/src/tool.rs index 32ebeaeb39..f5815a3eae 100644 --- a/libs/cua-driver/rust/crates/cua-driver-core/src/tool.rs +++ b/libs/cua-driver/rust/crates/cua-driver-core/src/tool.rs @@ -810,11 +810,10 @@ impl ToolRegistry { } pub fn tools_list(&self) -> Value { - let empty_args = Value::Object(serde_json::Map::new()); let list: Vec = self .order .iter() - .filter(|name| crate::policy::authorize_tool_call(name, &empty_args).is_ok()) + .filter(|name| crate::policy::is_tool_listable(name)) .filter_map(|name| self.tools.get(name)) .map(|tool| tool.def().to_list_entry()) .collect(); diff --git a/libs/cua-driver/rust/crates/cua-driver/tests/policy_tools_list_test.rs b/libs/cua-driver/rust/crates/cua-driver/tests/policy_tools_list_test.rs index e01a9e3626..68e85e1cd4 100644 --- a/libs/cua-driver/rust/crates/cua-driver/tests/policy_tools_list_test.rs +++ b/libs/cua-driver/rust/crates/cua-driver/tests/policy_tools_list_test.rs @@ -11,9 +11,14 @@ use serde_json::json; fn tools_list_hides_policy_denied_tools_and_calls_stay_denied() { let directory = tempfile::tempdir().expect("temporary policy directory"); let policy_path = directory.path().join("policy.yaml"); + // `get_config` is unconditionally allowed via `allow.tools`. + // `screenshot` is conditionally allowed via `allow.rules` with a + // constraint. Both must appear in `tools/list` because `tools/list` + // should not hide tools that are *potentially* allowed. + // `list_apps` is explicitly denied and must be absent. std::fs::write( &policy_path, - "allow:\n tools: [get_config]\ndeny:\n tools: [list_apps]\n", + "allow:\n tools: [get_config]\n rules:\n - tool: screenshot\n constraints:\n display_id: {\"const\": 0}\ndeny:\n tools: [list_apps]\n", ) .expect("write permission policy"); let policy = policy_path.display().to_string(); @@ -35,7 +40,11 @@ fn tools_list_hides_policy_denied_tools_and_calls_stay_denied() { .collect(); assert!( names.contains("get_config"), - "allowed tool must remain listed" + "unconditionally allowed tool must remain listed" + ); + assert!( + names.contains("screenshot"), + "rule-conditionally allowed tool must remain listed even though empty-arg evaluation would deny it" ); assert!( !names.contains("list_apps"), From 52444aabe71b492b8969d4fca605b2129c3c1f45 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 13 Aug 2026 07:05:44 +0000 Subject: [PATCH 3/3] Apply remaining changes Co-authored-by: r33drichards <57335981+r33drichards@users.noreply.github.com> --- libs/cua-driver/rust/crates/cua-driver-core/src/policy.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libs/cua-driver/rust/crates/cua-driver-core/src/policy.rs b/libs/cua-driver/rust/crates/cua-driver-core/src/policy.rs index 91102ce20a..5029d6503e 100644 --- a/libs/cua-driver/rust/crates/cua-driver-core/src/policy.rs +++ b/libs/cua-driver/rust/crates/cua-driver-core/src/policy.rs @@ -317,14 +317,18 @@ pub fn authorize_tool_call(tool: &str, args: &Value) -> Result<(), Authorization /// unconditionally denied). Tools that are conditionally allowed via /// `allow.rules` are still listed so that callers can attempt a constrained /// invocation. When no policy is configured, all tools are listable. +/// +/// On a policy loading error, this returns `true` (fail-open for listing). +/// The error will surface at invocation time through [`authorize_tool_call`], +/// and [`validate_configured_policy`] is expected to have caught it at startup. pub fn is_tool_listable(tool: &str) -> bool { let managed = match configured_managed_policy() { Ok(policy) => policy, - Err(_) => return false, + Err(_) => return true, }; let user = match configured_policy() { Ok(policy) => policy, - Err(_) => return false, + Err(_) => return true, }; for policy in [managed, user].into_iter().flatten() { if !policy.is_potentially_listable(tool) {