From d449b4e7bf28477a09864ed5d7ce6f94ba895e24 Mon Sep 17 00:00:00 2001 From: "Owen W. Taylor" Date: Mon, 6 Jul 2026 14:01:08 -0400 Subject: [PATCH] auth_policy: add tool exclusion support to policy rules Tools or toolsets prefixed with `-` in a rule's tools list now exclude that tool or toolset from matching. Exclusions take precedence over inclusions regardless of order. This makes the code match what has already been documented, except that the toolset exclusion is added compared to the documentation (which is updated for that.) Entries in the rule tool list are now syntactically validated (but not checked whether tools/toolsets actually exist.) Assisted-By: Claude Opus 4.6 --- docs/config-reference.md | 2 +- src/linux_mcp_server/auth_policy.py | 51 ++++++++++---- tests/test_auth_policy.py | 102 ++++++++++++++++++++++++++++ 3 files changed, 139 insertions(+), 16 deletions(-) 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):