feat(mcp): add per-server tool allowlist policy module and allowedTools config field
This commit is contained in:
@@ -4790,6 +4790,49 @@ mod tests {
|
||||
let _ = fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn uninstall_mcp_preserves_allowed_tools_on_surviving_entries() {
|
||||
let dir = fresh_temp_dir("uninst-mcp-allowed-tools-");
|
||||
let mut store = BundleStore::load_from(dir.join("installed-bundles.yaml")).unwrap();
|
||||
store
|
||||
.upsert_bundle("omc", test_metadata("https://github.com/x/omc"))
|
||||
.unwrap();
|
||||
let mcp = dir.join("mcp.json");
|
||||
write_mcp(
|
||||
&mcp,
|
||||
r#"{"mcpServers": {
|
||||
"srv": {"type": "stdio", "command": "echo"},
|
||||
"user-srv": {"type": "stdio", "command": "mine", "allowedTools": ["get_*", "list_issues"]}
|
||||
}}"#,
|
||||
);
|
||||
let parsed: McpServersConfig =
|
||||
serde_json::from_str(&fs::read_to_string(&mcp).unwrap()).unwrap();
|
||||
let hash = hash_bytes(
|
||||
serde_json::to_string(parsed.mcp_servers.get("srv").unwrap())
|
||||
.unwrap()
|
||||
.as_bytes(),
|
||||
);
|
||||
store
|
||||
.record_mcp_servers(
|
||||
"omc",
|
||||
vec![mcp_server_record("srv", McpAction::Added, Some(hash))],
|
||||
)
|
||||
.unwrap();
|
||||
let servers = store.get("omc").unwrap().mcp_servers.clone();
|
||||
|
||||
let summary = uninstall_mcp_entries(&mut store, "omc", &servers, &mcp, true).unwrap();
|
||||
|
||||
assert_eq!(summary.removed, vec!["srv"]);
|
||||
let raw = fs::read_to_string(&mcp).unwrap();
|
||||
assert!(raw.contains("allowedTools"));
|
||||
let written: McpServersConfig = serde_json::from_str(&raw).unwrap();
|
||||
assert_eq!(
|
||||
written.mcp_servers.get("user-srv").unwrap().allowed_tools,
|
||||
Some(vec!["get_*".to_string(), "list_issues".to_string()])
|
||||
);
|
||||
let _ = fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn uninstall_mcp_reports_referenced_secrets_without_removing_them() {
|
||||
let dir = fresh_temp_dir("uninst-mcp-secrets-");
|
||||
|
||||
@@ -139,6 +139,7 @@ mod tests {
|
||||
url: None,
|
||||
headers: None,
|
||||
oauth: None,
|
||||
allowed_tools: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -156,6 +157,7 @@ mod tests {
|
||||
url: Some(url.to_string()),
|
||||
headers,
|
||||
oauth: None,
|
||||
allowed_tools: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,759 @@
|
||||
use crate::mcp::McpServersConfig;
|
||||
|
||||
use fancy_regex::Regex;
|
||||
use indexmap::IndexMap;
|
||||
use log::warn;
|
||||
use std::collections::HashMap;
|
||||
use std::fmt;
|
||||
|
||||
/// The configuration level that contributed a layer of tool patterns for an
|
||||
/// MCP server, as rendered in diagnostics.
|
||||
#[allow(dead_code)]
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub enum LayerSource {
|
||||
Global,
|
||||
AppConfig,
|
||||
Role(String),
|
||||
Agent(String),
|
||||
Session,
|
||||
Skill(String),
|
||||
Node(String),
|
||||
}
|
||||
|
||||
impl fmt::Display for LayerSource {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
match self {
|
||||
LayerSource::Global => write!(f, "global (mcp.json)"),
|
||||
LayerSource::AppConfig => write!(f, "config (config.yaml)"),
|
||||
LayerSource::Role(name) => write!(f, "role ({name})"),
|
||||
LayerSource::Agent(name) => write!(f, "agent ({name})"),
|
||||
LayerSource::Session => write!(f, "session (.set)"),
|
||||
LayerSource::Skill(name) => write!(f, "skill ({name})"),
|
||||
LayerSource::Node(id) => write!(f, "node ({id})"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
#[derive(Debug)]
|
||||
pub struct CompiledPatterns {
|
||||
source: LayerSource,
|
||||
raw: Vec<String>,
|
||||
regexes: Vec<Regex>,
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
#[derive(Debug, Default)]
|
||||
pub struct ToolFilter {
|
||||
layers: Vec<CompiledPatterns>,
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
impl ToolFilter {
|
||||
pub fn push_layer(&mut self, source: LayerSource, patterns: &[String]) {
|
||||
self.layers.push(CompiledPatterns {
|
||||
source,
|
||||
raw: patterns.to_vec(),
|
||||
regexes: patterns.iter().map(|p| compile_glob(p)).collect(),
|
||||
});
|
||||
}
|
||||
|
||||
/// A tool is allowed iff it matches at least one pattern in every layer.
|
||||
pub fn allows(&self, tool: &str) -> bool {
|
||||
self.layers.iter().all(|layer| {
|
||||
layer
|
||||
.regexes
|
||||
.iter()
|
||||
.any(|regex| regex.is_match(tool).unwrap_or(false))
|
||||
})
|
||||
}
|
||||
|
||||
/// The first matching raw pattern per layer, in layer order, or the
|
||||
/// source of the first layer with no match.
|
||||
pub fn allows_explain(&self, tool: &str) -> Result<Vec<(&LayerSource, &str)>, &LayerSource> {
|
||||
let mut matched = Vec::with_capacity(self.layers.len());
|
||||
for layer in &self.layers {
|
||||
// fancy_regex can fail at match time (backtracking limits);
|
||||
// treat that as a non-match rather than allowing the tool.
|
||||
match layer
|
||||
.regexes
|
||||
.iter()
|
||||
.position(|regex| regex.is_match(tool).unwrap_or(false))
|
||||
{
|
||||
Some(index) => matched.push((&layer.source, layer.raw[index].as_str())),
|
||||
None => return Err(&layer.source),
|
||||
}
|
||||
}
|
||||
Ok(matched)
|
||||
}
|
||||
}
|
||||
|
||||
/// Translates a glob pattern (`*` = any run of characters, `?` = exactly one)
|
||||
/// into an anchored regex. Patterns that fail to compile match nothing.
|
||||
#[allow(dead_code)]
|
||||
fn compile_glob(pattern: &str) -> Regex {
|
||||
let translated = format!(
|
||||
"^{}$",
|
||||
fancy_regex::escape(pattern)
|
||||
.replace("\\*", ".*")
|
||||
.replace("\\?", ".")
|
||||
);
|
||||
Regex::new(&translated).unwrap_or_else(|error| {
|
||||
warn!("Invalid MCP tool pattern '{pattern}': {error}. It will match nothing.");
|
||||
never_matching_regex()
|
||||
})
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
fn never_matching_regex() -> Regex {
|
||||
Regex::new("(?!)").expect("'(?!)' is a valid never-matching regex")
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
pub struct SkillMcpLayer {
|
||||
pub name: String,
|
||||
pub enabled_servers: Vec<String>,
|
||||
pub mcp_tools: IndexMap<String, Vec<String>>,
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
pub struct McpToolPolicy;
|
||||
|
||||
#[allow(dead_code)]
|
||||
impl McpToolPolicy {
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub fn effective(
|
||||
mcp_config: &McpServersConfig,
|
||||
session: Option<&IndexMap<String, Vec<String>>>,
|
||||
agent: Option<(&str, &IndexMap<String, Vec<String>>)>,
|
||||
role: Option<(&str, &IndexMap<String, Vec<String>>)>,
|
||||
global: Option<&IndexMap<String, Vec<String>>>,
|
||||
skills: &[SkillMcpLayer],
|
||||
node: Option<(&str, &IndexMap<String, Vec<String>>)>,
|
||||
aliases: &IndexMap<String, String>,
|
||||
) -> HashMap<String, ToolFilter> {
|
||||
let mut filters: HashMap<String, ToolFilter> = HashMap::new();
|
||||
|
||||
for (server, spec) in &mcp_config.mcp_servers {
|
||||
if let Some(patterns) = &spec.allowed_tools {
|
||||
filters
|
||||
.entry(server.clone())
|
||||
.or_default()
|
||||
.push_layer(LayerSource::Global, patterns);
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(map) = global {
|
||||
push_level(
|
||||
&mut filters,
|
||||
mcp_config,
|
||||
aliases,
|
||||
&LayerSource::AppConfig,
|
||||
map,
|
||||
None,
|
||||
);
|
||||
}
|
||||
if let Some((name, map)) = role {
|
||||
push_level(
|
||||
&mut filters,
|
||||
mcp_config,
|
||||
aliases,
|
||||
&LayerSource::Role(name.to_string()),
|
||||
map,
|
||||
None,
|
||||
);
|
||||
}
|
||||
if let Some((name, map)) = agent {
|
||||
push_level(
|
||||
&mut filters,
|
||||
mcp_config,
|
||||
aliases,
|
||||
&LayerSource::Agent(name.to_string()),
|
||||
map,
|
||||
None,
|
||||
);
|
||||
}
|
||||
if let Some(map) = session {
|
||||
push_level(
|
||||
&mut filters,
|
||||
mcp_config,
|
||||
aliases,
|
||||
&LayerSource::Session,
|
||||
map,
|
||||
None,
|
||||
);
|
||||
}
|
||||
for skill in skills {
|
||||
push_level(
|
||||
&mut filters,
|
||||
mcp_config,
|
||||
aliases,
|
||||
&LayerSource::Skill(skill.name.clone()),
|
||||
&skill.mcp_tools,
|
||||
Some(&skill.enabled_servers),
|
||||
);
|
||||
}
|
||||
if let Some((id, map)) = node {
|
||||
push_level(
|
||||
&mut filters,
|
||||
mcp_config,
|
||||
aliases,
|
||||
&LayerSource::Node(id.to_string()),
|
||||
map,
|
||||
None,
|
||||
);
|
||||
}
|
||||
|
||||
filters
|
||||
}
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
fn push_level(
|
||||
filters: &mut HashMap<String, ToolFilter>,
|
||||
mcp_config: &McpServersConfig,
|
||||
aliases: &IndexMap<String, String>,
|
||||
source: &LayerSource,
|
||||
map: &IndexMap<String, Vec<String>>,
|
||||
enabled_servers: Option<&[String]>,
|
||||
) {
|
||||
for (server, patterns) in expand_server_keys(mcp_config, aliases, map) {
|
||||
if let Some(enabled) = enabled_servers
|
||||
&& !enabled.iter().any(|id| id == &server)
|
||||
{
|
||||
continue;
|
||||
}
|
||||
filters
|
||||
.entry(server)
|
||||
.or_default()
|
||||
.push_layer(source.clone(), &patterns);
|
||||
}
|
||||
}
|
||||
|
||||
/// Expands map keys to server ids: a key that is a server id maps to itself;
|
||||
/// a key that is an alias expands to every configured id in its
|
||||
/// comma-separated value; anything else is dropped. Keys expanding to the
|
||||
/// same server merge their pattern lists.
|
||||
#[allow(dead_code)]
|
||||
fn expand_server_keys(
|
||||
mcp_config: &McpServersConfig,
|
||||
aliases: &IndexMap<String, String>,
|
||||
map: &IndexMap<String, Vec<String>>,
|
||||
) -> IndexMap<String, Vec<String>> {
|
||||
let mut expanded: IndexMap<String, Vec<String>> = IndexMap::new();
|
||||
for (key, patterns) in map {
|
||||
let key = key.trim();
|
||||
if mcp_config.mcp_servers.contains_key(key) {
|
||||
expanded
|
||||
.entry(key.to_string())
|
||||
.or_default()
|
||||
.extend(patterns.iter().cloned());
|
||||
} else if let Some(mapped) = aliases.get(key) {
|
||||
for mapped_id in mapped.split(',').map(str::trim) {
|
||||
if mcp_config.mcp_servers.contains_key(mapped_id) {
|
||||
expanded
|
||||
.entry(mapped_id.to_string())
|
||||
.or_default()
|
||||
.extend(patterns.iter().cloned());
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
expanded
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::mcp::{McpServer, McpServersConfig, McpTransportType};
|
||||
|
||||
fn spec(allowed_tools: Option<&[&str]>) -> McpServer {
|
||||
McpServer {
|
||||
transport_type: McpTransportType::Stdio,
|
||||
command: Some("echo".to_string()),
|
||||
args: None,
|
||||
env: None,
|
||||
cwd: None,
|
||||
url: None,
|
||||
headers: None,
|
||||
oauth: None,
|
||||
allowed_tools: allowed_tools.map(list),
|
||||
}
|
||||
}
|
||||
|
||||
fn config(servers: &[(&str, Option<&[&str]>)]) -> McpServersConfig {
|
||||
McpServersConfig {
|
||||
mcp_servers: servers
|
||||
.iter()
|
||||
.map(|(name, tools)| (name.to_string(), spec(*tools)))
|
||||
.collect(),
|
||||
}
|
||||
}
|
||||
|
||||
fn list(items: &[&str]) -> Vec<String> {
|
||||
items.iter().map(|s| s.to_string()).collect()
|
||||
}
|
||||
|
||||
fn tool_map(entries: &[(&str, &[&str])]) -> IndexMap<String, Vec<String>> {
|
||||
entries
|
||||
.iter()
|
||||
.map(|(server, patterns)| (server.to_string(), list(patterns)))
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn single_layer(patterns: &[&str]) -> ToolFilter {
|
||||
layered(&[(LayerSource::Global, patterns)])
|
||||
}
|
||||
|
||||
fn layered(layers: &[(LayerSource, &[&str])]) -> ToolFilter {
|
||||
let mut filter = ToolFilter::default();
|
||||
for (source, patterns) in layers {
|
||||
filter.push_layer(source.clone(), &list(patterns));
|
||||
}
|
||||
filter
|
||||
}
|
||||
|
||||
fn no_aliases() -> IndexMap<String, String> {
|
||||
IndexMap::new()
|
||||
}
|
||||
|
||||
fn aliases(entries: &[(&str, &str)]) -> IndexMap<String, String> {
|
||||
entries
|
||||
.iter()
|
||||
.map(|(key, value)| (key.to_string(), value.to_string()))
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn resolve(
|
||||
config: &McpServersConfig,
|
||||
session: Option<&IndexMap<String, Vec<String>>>,
|
||||
role: Option<(&str, &IndexMap<String, Vec<String>>)>,
|
||||
) -> HashMap<String, ToolFilter> {
|
||||
McpToolPolicy::effective(config, session, None, role, None, &[], None, &no_aliases())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn literal_pattern_matches_only_the_exact_name() {
|
||||
let filter = single_layer(&["get_issue"]);
|
||||
|
||||
assert!(filter.allows("get_issue"));
|
||||
assert!(!filter.allows("get_issues"));
|
||||
assert!(!filter.allows("get_issu"));
|
||||
assert!(!filter.allows("xget_issue"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn star_matches_any_run_of_characters() {
|
||||
let filter = single_layer(&["get_*"]);
|
||||
assert!(filter.allows("get_issue"));
|
||||
assert!(filter.allows("get_"));
|
||||
assert!(!filter.allows("set_issue"));
|
||||
|
||||
let filter = single_layer(&["*_issue"]);
|
||||
assert!(filter.allows("create_issue"));
|
||||
assert!(!filter.allows("create_pr"));
|
||||
|
||||
let filter = single_layer(&["get*sue"]);
|
||||
assert!(filter.allows("get_issue"));
|
||||
assert!(filter.allows("getsue"));
|
||||
|
||||
let filter = single_layer(&["*"]);
|
||||
assert!(filter.allows(""));
|
||||
assert!(filter.allows("anything_at_all"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn question_mark_matches_exactly_one_character() {
|
||||
let filter = single_layer(&["get_?"]);
|
||||
|
||||
assert!(filter.allows("get_a"));
|
||||
assert!(!filter.allows("get_"));
|
||||
assert!(!filter.allows("get_ab"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn regex_metacharacters_are_matched_literally() {
|
||||
let filter = single_layer(&["get.issue"]);
|
||||
assert!(filter.allows("get.issue"));
|
||||
assert!(!filter.allows("getXissue"));
|
||||
|
||||
for pattern in ["a(b", "a[b", "a+b", "a|b", "a$b"] {
|
||||
let filter = single_layer(&[pattern]);
|
||||
assert!(filter.allows(pattern), "'{pattern}' should match itself");
|
||||
assert!(!filter.allows("ab"), "'{pattern}' should not match 'ab'");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn backslash_is_literal_and_star_still_wildcards() {
|
||||
let filter = single_layer(&["a\\b"]);
|
||||
assert!(filter.allows("a\\b"));
|
||||
assert!(!filter.allows("ab"));
|
||||
|
||||
let filter = single_layer(&["a\\*b"]);
|
||||
assert!(filter.allows("a\\b"));
|
||||
assert!(filter.allows("a\\xyzb"));
|
||||
assert!(!filter.allows("ab"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_never_matching_placeholder_matches_nothing() {
|
||||
let regex = never_matching_regex();
|
||||
|
||||
assert!(!regex.is_match("").unwrap());
|
||||
assert!(!regex.is_match("anything").unwrap());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn within_a_layer_any_pattern_may_match() {
|
||||
let filter = single_layer(&["get_*", "set_*"]);
|
||||
|
||||
assert!(filter.allows("get_x"));
|
||||
assert!(filter.allows("set_x"));
|
||||
assert!(!filter.allows("delete_x"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn across_layers_every_layer_must_match() {
|
||||
let filter = layered(&[
|
||||
(LayerSource::Global, &["get_*"]),
|
||||
(LayerSource::Session, &["*_issue"]),
|
||||
]);
|
||||
|
||||
assert!(filter.allows("get_issue"));
|
||||
assert!(!filter.allows("get_pr"));
|
||||
assert!(!filter.allows("create_issue"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_empty_layer_blocks_everything() {
|
||||
let filter = layered(&[(LayerSource::Global, &["*"]), (LayerSource::Session, &[])]);
|
||||
|
||||
assert!(!filter.allows("anything"));
|
||||
assert_eq!(
|
||||
filter.allows_explain("anything"),
|
||||
Err(&LayerSource::Session)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn allows_explain_reports_the_first_matching_pattern_per_layer() {
|
||||
let filter = layered(&[
|
||||
(LayerSource::Global, &["x_*", "get_*"]),
|
||||
(LayerSource::Session, &["*"]),
|
||||
]);
|
||||
|
||||
assert_eq!(
|
||||
filter.allows_explain("get_issue").unwrap(),
|
||||
vec![
|
||||
(&LayerSource::Global, "get_*"),
|
||||
(&LayerSource::Session, "*")
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn allows_explain_reports_the_first_layer_without_a_match() {
|
||||
let filter = layered(&[
|
||||
(LayerSource::Global, &["get_*"]),
|
||||
(LayerSource::Session, &["*"]),
|
||||
]);
|
||||
assert_eq!(
|
||||
filter.allows_explain("delete_repo"),
|
||||
Err(&LayerSource::Global)
|
||||
);
|
||||
|
||||
let filter = layered(&[
|
||||
(LayerSource::Global, &["*"]),
|
||||
(LayerSource::Session, &["get_*"]),
|
||||
]);
|
||||
assert_eq!(
|
||||
filter.allows_explain("delete_repo"),
|
||||
Err(&LayerSource::Session)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn global_allowed_tools_from_mcp_json_is_the_first_layer() {
|
||||
let config = config(&[("gh", Some(&["get_*"]))]);
|
||||
let session_map = tool_map(&[("gh", &["*"])]);
|
||||
|
||||
let filters = resolve(&config, Some(&session_map), None);
|
||||
|
||||
assert_eq!(
|
||||
filters["gh"].allows_explain("get_issue").unwrap(),
|
||||
vec![
|
||||
(&LayerSource::Global, "get_*"),
|
||||
(&LayerSource::Session, "*")
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn servers_without_patterns_at_any_level_are_absent() {
|
||||
let config = config(&[("gh", None)]);
|
||||
|
||||
let filters = resolve(&config, None, None);
|
||||
|
||||
assert!(filters.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn server_absent_from_a_level_map_gets_no_layer_from_it() {
|
||||
let config = config(&[("gh", Some(&["get_*"])), ("gl", None)]);
|
||||
let role_map = tool_map(&[("gl", &["x_*"])]);
|
||||
|
||||
let filters = resolve(&config, None, Some(("dev", &role_map)));
|
||||
|
||||
assert!(filters["gh"].allows("get_issue"));
|
||||
assert!(!filters["gh"].allows("delete_repo"));
|
||||
assert_eq!(filters["gh"].allows_explain("get_issue").unwrap().len(), 1);
|
||||
assert!(filters["gl"].allows("x_1"));
|
||||
assert!(!filters["gl"].allows("y_1"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn empty_pattern_list_at_a_level_blocks_all_tools_for_that_server() {
|
||||
let config = config(&[("gh", Some(&["get_*"]))]);
|
||||
let session_map = tool_map(&[("gh", &[])]);
|
||||
|
||||
let filters = resolve(&config, Some(&session_map), None);
|
||||
|
||||
assert!(!filters["gh"].allows("get_issue"));
|
||||
assert_eq!(
|
||||
filters["gh"].allows_explain("get_issue"),
|
||||
Err(&LayerSource::Session)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn session_cannot_widen_a_role_restriction() {
|
||||
let config = config(&[("gh", None)]);
|
||||
let role_map = tool_map(&[("gh", &["get_*"])]);
|
||||
let session_map = tool_map(&[("gh", &["*"])]);
|
||||
|
||||
let filters = resolve(&config, Some(&session_map), Some(("dev", &role_map)));
|
||||
|
||||
assert!(filters["gh"].allows("get_issue"));
|
||||
assert!(!filters["gh"].allows("delete_repo"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn app_config_map_contributes_its_own_layer() {
|
||||
let config = config(&[("gh", None)]);
|
||||
let app_map = tool_map(&[("gh", &["get_*"])]);
|
||||
|
||||
let filters = McpToolPolicy::effective(
|
||||
&config,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
Some(&app_map),
|
||||
&[],
|
||||
None,
|
||||
&no_aliases(),
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
filters["gh"].allows_explain("get_issue").unwrap(),
|
||||
vec![(&LayerSource::AppConfig, "get_*")]
|
||||
);
|
||||
assert!(!filters["gh"].allows("delete_repo"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn skill_layer_applies_only_to_its_enabled_servers() {
|
||||
let config = config(&[("gh", None), ("gl", None)]);
|
||||
let skill = SkillMcpLayer {
|
||||
name: "reviewer".to_string(),
|
||||
enabled_servers: vec!["gh".to_string()],
|
||||
mcp_tools: tool_map(&[("gh", &["get_*"]), ("gl", &["*"])]),
|
||||
};
|
||||
|
||||
let filters = McpToolPolicy::effective(
|
||||
&config,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
&[skill],
|
||||
None,
|
||||
&no_aliases(),
|
||||
);
|
||||
|
||||
assert!(filters.contains_key("gh"));
|
||||
assert!(!filters.contains_key("gl"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn two_skills_naming_the_same_server_stack_independent_layers() {
|
||||
let config = config(&[("gh", None)]);
|
||||
let skills = vec![
|
||||
SkillMcpLayer {
|
||||
name: "a".to_string(),
|
||||
enabled_servers: vec!["gh".to_string()],
|
||||
mcp_tools: tool_map(&[("gh", &["get_*"])]),
|
||||
},
|
||||
SkillMcpLayer {
|
||||
name: "b".to_string(),
|
||||
enabled_servers: vec!["gh".to_string()],
|
||||
mcp_tools: tool_map(&[("gh", &["*_issue"])]),
|
||||
},
|
||||
];
|
||||
|
||||
let filters = McpToolPolicy::effective(
|
||||
&config,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
&skills,
|
||||
None,
|
||||
&no_aliases(),
|
||||
);
|
||||
|
||||
assert!(filters["gh"].allows("get_issue"));
|
||||
assert!(!filters["gh"].allows("get_pr"));
|
||||
assert!(!filters["gh"].allows("create_issue"));
|
||||
assert_eq!(filters["gh"].allows_explain("get_issue").unwrap().len(), 2);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn layers_stack_in_documented_order_with_node_last() {
|
||||
let config = config(&[("gh", Some(&["*"]))]);
|
||||
let app_map = tool_map(&[("gh", &["*"])]);
|
||||
let role_map = tool_map(&[("gh", &["*"])]);
|
||||
let agent_map = tool_map(&[("gh", &["*"])]);
|
||||
let session_map = tool_map(&[("gh", &["*"])]);
|
||||
let skills = vec![SkillMcpLayer {
|
||||
name: "reviewer".to_string(),
|
||||
enabled_servers: vec!["gh".to_string()],
|
||||
mcp_tools: tool_map(&[("gh", &["*"])]),
|
||||
}];
|
||||
let node_map = tool_map(&[("gh", &["*"])]);
|
||||
|
||||
let filters = McpToolPolicy::effective(
|
||||
&config,
|
||||
Some(&session_map),
|
||||
Some(("worker", &agent_map)),
|
||||
Some(("dev", &role_map)),
|
||||
Some(&app_map),
|
||||
&skills,
|
||||
Some(("n1", &node_map)),
|
||||
&no_aliases(),
|
||||
);
|
||||
|
||||
let sources: Vec<String> = filters["gh"]
|
||||
.allows_explain("anything")
|
||||
.unwrap()
|
||||
.iter()
|
||||
.map(|(source, _)| source.to_string())
|
||||
.collect();
|
||||
assert_eq!(
|
||||
sources,
|
||||
vec![
|
||||
"global (mcp.json)",
|
||||
"config (config.yaml)",
|
||||
"role (dev)",
|
||||
"agent (worker)",
|
||||
"session (.set)",
|
||||
"skill (reviewer)",
|
||||
"node (n1)",
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn alias_key_expands_to_all_mapped_servers() {
|
||||
let config = config(&[("github", None), ("gitlab", None)]);
|
||||
let role_map = tool_map(&[("gh", &["get_*"])]);
|
||||
|
||||
let filters = McpToolPolicy::effective(
|
||||
&config,
|
||||
None,
|
||||
None,
|
||||
Some(("dev", &role_map)),
|
||||
None,
|
||||
&[],
|
||||
None,
|
||||
&aliases(&[("gh", "github,gitlab")]),
|
||||
);
|
||||
|
||||
assert!(filters["github"].allows("get_issue"));
|
||||
assert!(!filters["github"].allows("delete_repo"));
|
||||
assert!(filters["gitlab"].allows("get_issue"));
|
||||
assert!(!filters["gitlab"].allows("delete_repo"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn alias_ids_missing_from_the_config_are_skipped() {
|
||||
let config = config(&[("github", None)]);
|
||||
let role_map = tool_map(&[("gh", &["get_*"])]);
|
||||
|
||||
let filters = McpToolPolicy::effective(
|
||||
&config,
|
||||
None,
|
||||
None,
|
||||
Some(("dev", &role_map)),
|
||||
None,
|
||||
&[],
|
||||
None,
|
||||
&aliases(&[("gh", "github,missing")]),
|
||||
);
|
||||
|
||||
assert_eq!(filters.len(), 1);
|
||||
assert!(filters.contains_key("github"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unknown_map_keys_are_dropped() {
|
||||
let config = config(&[("github", None)]);
|
||||
let role_map = tool_map(&[("nope", &["get_*"])]);
|
||||
|
||||
let filters = resolve(&config, None, Some(("dev", &role_map)));
|
||||
|
||||
assert!(filters.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn alias_and_direct_key_for_the_same_server_merge_into_one_layer() {
|
||||
let config = config(&[("github", None)]);
|
||||
let role_map = tool_map(&[("gh", &["get_*"]), ("github", &["set_*"])]);
|
||||
|
||||
let filters = McpToolPolicy::effective(
|
||||
&config,
|
||||
None,
|
||||
None,
|
||||
Some(("dev", &role_map)),
|
||||
None,
|
||||
&[],
|
||||
None,
|
||||
&aliases(&[("gh", "github")]),
|
||||
);
|
||||
|
||||
assert!(filters["github"].allows("get_issue"));
|
||||
assert!(filters["github"].allows("set_topic"));
|
||||
assert!(!filters["github"].allows("delete_repo"));
|
||||
assert_eq!(
|
||||
filters["github"].allows_explain("get_issue").unwrap().len(),
|
||||
1
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn layer_source_display() {
|
||||
assert_eq!(LayerSource::Global.to_string(), "global (mcp.json)");
|
||||
assert_eq!(LayerSource::AppConfig.to_string(), "config (config.yaml)");
|
||||
assert_eq!(LayerSource::Role("dev".into()).to_string(), "role (dev)");
|
||||
assert_eq!(
|
||||
LayerSource::Agent("worker".into()).to_string(),
|
||||
"agent (worker)"
|
||||
);
|
||||
assert_eq!(LayerSource::Session.to_string(), "session (.set)");
|
||||
assert_eq!(
|
||||
LayerSource::Skill("review".into()).to_string(),
|
||||
"skill (review)"
|
||||
);
|
||||
assert_eq!(LayerSource::Node("n1".into()).to_string(), "node (n1)");
|
||||
}
|
||||
}
|
||||
@@ -8,6 +8,7 @@ pub(crate) mod instructions;
|
||||
mod macro_policy;
|
||||
mod macros;
|
||||
mod mcp_factory;
|
||||
mod mcp_tool_policy;
|
||||
pub(crate) mod memory;
|
||||
pub(crate) mod paths;
|
||||
pub(crate) mod prompts;
|
||||
|
||||
@@ -5599,6 +5599,7 @@ mod tests {
|
||||
url: None,
|
||||
headers: None,
|
||||
oauth: None,
|
||||
allowed_tools: None,
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
@@ -242,6 +242,7 @@ fn build_stdio(cli: &Cli, has_url: bool) -> Result<McpServer> {
|
||||
url: None,
|
||||
headers: None,
|
||||
oauth: None,
|
||||
allowed_tools: None,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -300,6 +301,7 @@ fn build_remote(cli: &Cli, transport: McpTransportType, has_command: bool) -> Re
|
||||
url: Some(url),
|
||||
headers: (!headers.is_empty()).then_some(headers),
|
||||
oauth,
|
||||
allowed_tools: None,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
+23
-1
@@ -6,7 +6,7 @@ mod sse_transport;
|
||||
|
||||
use crate::config::AppConfig;
|
||||
use crate::config::paths;
|
||||
use crate::utils::{AbortSignal, abortable_run_with_spinner};
|
||||
use crate::utils::{AbortSignal, abortable_run_with_spinner, dimmed_text};
|
||||
use crate::vault::Vault;
|
||||
use crate::vault::interpolate_secrets;
|
||||
use anyhow::Error;
|
||||
@@ -166,6 +166,8 @@ pub(crate) struct McpServer {
|
||||
pub headers: Option<IndexMap<String, String>>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub oauth: Option<McpOAuthConfig>,
|
||||
#[serde(rename = "allowedTools", skip_serializing_if = "Option::is_none")]
|
||||
pub allowed_tools: Option<Vec<String>>,
|
||||
}
|
||||
|
||||
impl McpServer {
|
||||
@@ -177,6 +179,15 @@ impl McpServer {
|
||||
}
|
||||
|
||||
pub fn validate(&self, name: &str) -> Result<()> {
|
||||
if let Some(tools) = &self.allowed_tools
|
||||
&& tools.is_empty()
|
||||
{
|
||||
let message = format!(
|
||||
"MCP server '{name}' has an empty \"allowedTools\" list, so none of its tools will be callable"
|
||||
);
|
||||
warn!("{message}");
|
||||
eprintln!("{}", dimmed_text(&message));
|
||||
}
|
||||
if self.is_remote() {
|
||||
let type_label = match self.transport_type {
|
||||
McpTransportType::Http => "http",
|
||||
@@ -802,6 +813,7 @@ mod tests {
|
||||
url: None,
|
||||
headers: None,
|
||||
oauth: None,
|
||||
allowed_tools: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -815,6 +827,7 @@ mod tests {
|
||||
url: Some(url.to_string()),
|
||||
headers: None,
|
||||
oauth: None,
|
||||
allowed_tools: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -828,6 +841,7 @@ mod tests {
|
||||
url: Some(url.to_string()),
|
||||
headers: None,
|
||||
oauth: None,
|
||||
allowed_tools: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -860,6 +874,7 @@ mod tests {
|
||||
url: None,
|
||||
headers: None,
|
||||
oauth: None,
|
||||
allowed_tools: None,
|
||||
};
|
||||
|
||||
let err = spec.validate("test").unwrap_err();
|
||||
@@ -878,6 +893,7 @@ mod tests {
|
||||
url: Some("http://localhost".into()),
|
||||
headers: None,
|
||||
oauth: None,
|
||||
allowed_tools: None,
|
||||
};
|
||||
|
||||
let err = spec.validate("test").unwrap_err();
|
||||
@@ -898,6 +914,7 @@ mod tests {
|
||||
url: None,
|
||||
headers: Some(headers),
|
||||
oauth: None,
|
||||
allowed_tools: None,
|
||||
};
|
||||
|
||||
let err = spec.validate("test").unwrap_err();
|
||||
@@ -923,6 +940,7 @@ mod tests {
|
||||
url: None,
|
||||
headers: None,
|
||||
oauth: None,
|
||||
allowed_tools: None,
|
||||
};
|
||||
|
||||
let err = spec.validate("test").unwrap_err();
|
||||
@@ -941,6 +959,7 @@ mod tests {
|
||||
url: Some("http://localhost".into()),
|
||||
headers: None,
|
||||
oauth: None,
|
||||
allowed_tools: None,
|
||||
};
|
||||
|
||||
let err = spec.validate("test").unwrap_err();
|
||||
@@ -959,6 +978,7 @@ mod tests {
|
||||
url: Some("http://localhost".into()),
|
||||
headers: None,
|
||||
oauth: None,
|
||||
allowed_tools: None,
|
||||
};
|
||||
|
||||
let err = spec.validate("test").unwrap_err();
|
||||
@@ -977,6 +997,7 @@ mod tests {
|
||||
url: Some("http://localhost".into()),
|
||||
headers: None,
|
||||
oauth: None,
|
||||
allowed_tools: None,
|
||||
};
|
||||
|
||||
let err = spec.validate("test").unwrap_err();
|
||||
@@ -1002,6 +1023,7 @@ mod tests {
|
||||
url: None,
|
||||
headers: None,
|
||||
oauth: None,
|
||||
allowed_tools: None,
|
||||
};
|
||||
|
||||
let err = spec.validate("test").unwrap_err();
|
||||
|
||||
Reference in New Issue
Block a user