diff --git a/docs/config-reference.md b/docs/config-reference.md index d98f1b6b..d9f0ed7c 100644 --- a/docs/config-reference.md +++ b/docs/config-reference.md @@ -148,7 +148,7 @@ and when a matching rule is found, the action from that rule is used, and proces Each rule has the following properties that are used for matching: - **`host`** (required, string): either `localhost` for local execution or a pattern (with * and ? wildcards) that matches a remote host. `localhost` must be specified literally - `host: *` will *not* match `localhost`. - - **`tools`** (required, list of strings) - a list of tool names or toolsets to match. Use `*` to match all tools. A toolset (as for `LINUX_MCP_TOOLSET`) is represented by a `@` prefix. If a tool name is preceded by `-`, that excludes the tool. (Exclusions take precedence, order doesn't matter. Toolsets cannot be excluded.) + - **`tools`** (required, list of strings) - a list of tool names or toolsets to match. Use `*` to match all tools. A toolset (as for `LINUX_MCP_TOOLSET`) is represented by a `@` prefix. If a tool name or toolset name is preceded by `-`, that excludes the tool or toolset. (Exclusions take precedence, order doesn't matter.) - **`claims`** (object) - claims from the OAuth2 token to match on. Each item in here is of the form ``: `` with the following match rules: - if `` is a string, and the value from the token is a string, they must match exactly - if `` is a string, and the value from the token is a list, `` must be in the list (example: `groups: app-rhel-mcp-server-users`) diff --git a/src/linux_mcp_server/auth_policy.py b/src/linux_mcp_server/auth_policy.py index c83501f5..5791b257 100644 --- a/src/linux_mcp_server/auth_policy.py +++ b/src/linux_mcp_server/auth_policy.py @@ -1,5 +1,6 @@ import fnmatch import logging +import re from enum import Enum from functools import cache @@ -63,6 +64,14 @@ def validate_host_action(self): raise ValueError(f"Rule with host: '{self.host}' cannot be use action 'local'") return self + # Check that tool entries are syntactically valid + @model_validator(mode="after") + def validate_tools(self): + for pattern in self.tools: + if not re.match(r"^(?:-?@?[a-z][a-z0-9_]*|\*)$", pattern): + raise ValueError("Entry in tools list must be '*', '[-]', or '[-]@'") + return self + # Validate that SSH_KEY action has ssh_key configuration @model_validator(mode="after") def validate_ssh_key_config(self): @@ -77,23 +86,35 @@ def matches_host(self, target_host: str | None) -> bool: return fnmatch.fnmatch(target_host, self.host) # Check if the rule matches the policy tool name - # Supports exact tool name: "run_script_readonly", prefixes: "@fixed" and wildcard: "*" + # Supports exact tool name: "run_script_readonly", prefixes: "@fixed", wildcard: "*", + # and exclusions: "-tool_name" (exclusions take precedence over inclusions) def matches_tool(self, tool_name: str, tool_tags: set[str]) -> bool: - for allowed_tool in self.tools: - if allowed_tool == "*": - return True - elif allowed_tool.startswith("@"): - # Toolset prefix check if tool belongs to this toolset - toolset_name = allowed_tool[1:] # Remove @ prefix - toolset = get_toolset(toolset_name) - if toolset is None: - logger.warning(f"Unknown toolset: {toolset_name}") - return False - - if toolset.includes_tool(tool_tags): + def matches(patterns: list[str]): + for pattern in patterns: + if pattern == "*": return True - elif allowed_tool == tool_name: - return True + elif pattern.startswith("@"): + # Toolset prefix check if tool belongs to this toolset + toolset_name = pattern[1:] # Remove @ prefix + toolset = get_toolset(toolset_name) + if toolset is None: + logger.warning(f"Unknown toolset: {toolset_name}") + return False + + if toolset.includes_tool(tool_tags): + return True + elif pattern == tool_name: + return True + return False + + exclusions = [t[1:] for t in self.tools if t.startswith("-")] + if matches(exclusions): + return False + + inclusions = [t for t in self.tools if not t.startswith("-")] + if matches(inclusions): + return True + return False # Check if the tokens claims satisfy the policy rules claim requirements diff --git a/tests/test_auth_policy.py b/tests/test_auth_policy.py index f9dfbe20..7b64c7b4 100644 --- a/tests/test_auth_policy.py +++ b/tests/test_auth_policy.py @@ -4,6 +4,8 @@ import pytest +from pydantic import ValidationError + from linux_mcp_server.auth_policy import AuthPolicy from linux_mcp_server.auth_policy import evaluate_policy from linux_mcp_server.auth_policy import get_policy @@ -45,6 +47,36 @@ def test_localhost_normalization(self): assert rule.matches_host("localhost") +class TestPolicyRuleValidation: + @pytest.mark.parametrize( + "tools,is_error", + [ + [["*"], False], + [["get_service_status"], False], + [["get_service_status_42"], False], + [["-get_service_status"], False], + [["@run_script"], False], + [["-@run_script"], False], + [["get_*"], True], + [["_get_service_status"], True], + [["42_get_service_status"], True], + [["-@run_Script"], True], + ], + ) + def test_validate_tools(self, tools: list[str], is_error: bool): + def make_rule(): + return PolicyRule(host="localhost", tools=tools, claims={}, action=PolicyAction.LOCAL, all_users=True) + + if is_error: + with pytest.raises( + ValidationError, match=r"Entry in tools list must be '\*', '\[-\]', or '\[-\]@'" + ): + make_rule() + else: + rule = make_rule() + assert rule.tools == tools + + class TestPolicyRuleToolMatching: def test_wildcard_match(self): rule = PolicyRule( @@ -80,6 +112,76 @@ def test_toolset_prefix_match(self): assert rule.matches_tool("validate_script", {"run_script"}) assert not rule.matches_tool("get_service_status", set()) + def test_exclusion_with_wildcard(self): + rule = PolicyRule( + host="*", + tools=["*", "-run_script"], + claims={}, + action=PolicyAction.SSH_DEFAULT, + all_users=True, + ) + assert rule.matches_tool("other_tool", set()) + assert not rule.matches_tool("run_script", set()) + + def test_exclusion_with_toolset(self): + rule = PolicyRule( + host="*", + tools=["@run_script", "-validate_script"], + claims={}, + action=PolicyAction.SSH_DEFAULT, + all_users=True, + ) + # validate_script is in the toolset but explicitly excluded + assert not rule.matches_tool("validate_script", {"run_script"}) + # run_script tool itself is still included via the toolset + assert rule.matches_tool("run_script", {"run_script"}) + + def test_exclusion_of_toolset(self): + rule = PolicyRule( + host="*", + tools=["*", "-@run_script"], + claims={}, + action=PolicyAction.SSH_DEFAULT, + all_users=True, + ) + # validate_script is in excluded via toolset + assert not rule.matches_tool("validate_script", {"run_script"}) + # get_service_status is not excluded + assert rule.matches_tool("get_service_status", set()) + + def test_exclusion_takes_precedence_over_explicit_include(self): + rule = PolicyRule( + host="*", + tools=["run_script", "-run_script"], + claims={}, + action=PolicyAction.SSH_DEFAULT, + all_users=True, + ) + assert not rule.matches_tool("run_script", set()) + + def test_exclusion_order_independent(self): + # Exclusion listed before the wildcard — should still exclude + rule = PolicyRule( + host="*", + tools=["-run_script", "*"], + claims={}, + action=PolicyAction.SSH_DEFAULT, + all_users=True, + ) + assert not rule.matches_tool("run_script", set()) + assert rule.matches_tool("other_tool", set()) + + def test_exclusion_does_not_match_other_tools(self): + rule = PolicyRule( + host="*", + tools=["*", "-run_script"], + claims={}, + action=PolicyAction.SSH_DEFAULT, + all_users=True, + ) + assert rule.matches_tool("run_script_readonly", set()) + assert rule.matches_tool("validate_script", set()) + class TestPolicyRuleClaimMatching: def test_empty_claims(self):