From 0447508891d2eb25e4430988fe3fca71291efafd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Efe=20G=C3=B6kdemir?= Date: Tue, 29 Sep 2026 18:36:48 +0300 Subject: [PATCH 1/3] fix: avoid broadening allowed tool preapproval MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Efe Gökdemir --- src/skillspector/nodes/analyzers/mcp_least_privilege.py | 7 ++++--- tests/test_mcp_least_privilege.py | 2 ++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/skillspector/nodes/analyzers/mcp_least_privilege.py b/src/skillspector/nodes/analyzers/mcp_least_privilege.py index f05a6777e..8455bea16 100644 --- a/src/skillspector/nodes/analyzers/mcp_least_privilege.py +++ b/src/skillspector/nodes/analyzers/mcp_least_privilege.py @@ -618,9 +618,10 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: break confidence = _clamp(0.55 if capability in test_only_caps else 0.75) remediation = ( - f"Add a tool that covers the '{capability}' capability to the " - "'allowed-tools' frontmatter field in SKILL.md, or remove " - "the code that requires it." + f"Review whether the '{capability}' capability is required and " + "whether the host runtime policy permits it. Remove code that " + "does not need the capability; do not broaden 'allowed-tools' " + "preapproval solely to silence this finding." if allowed_tools else f"Add the '{capability}' capability to the MCP server manifest's " "'permissions' list, or remove the code that requires it." diff --git a/tests/test_mcp_least_privilege.py b/tests/test_mcp_least_privilege.py index e606e4698..7cae7a1cd 100644 --- a/tests/test_mcp_least_privilege.py +++ b/tests/test_mcp_least_privilege.py @@ -331,6 +331,8 @@ def test_allowed_tools_underdeclared_fires_lp1(self): assert lp1.remediation is not None assert "'allowed-tools'" in lp1.remediation assert "'permissions'" not in lp1.remediation + assert "Add a tool" not in lp1.remediation + assert "preapproval" in lp1.remediation def test_allowed_tools_fully_covered_no_lp1(self): """allowed-tools: [Bash] + only shell code → no LP1 (capability is covered).""" From bd8837ee2dc89befaeb240d24726d242d17382cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Efe=20G=C3=B6kdemir?= Date: Fri, 2 Oct 2026 21:55:47 +0300 Subject: [PATCH 2/3] fix: make LP1 and LP3 remediation fail-safe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Efe Gökdemir --- docs/B.3.1-mcp-least-privilege.md | 22 +++++++++------- .../nodes/analyzers/mcp_least_privilege.py | 26 ++++++++++++------- .../nodes/analyzers/pattern_defaults.py | 4 +-- tests/test_mcp_least_privilege.py | 9 +++++++ 4 files changed, 41 insertions(+), 20 deletions(-) diff --git a/docs/B.3.1-mcp-least-privilege.md b/docs/B.3.1-mcp-least-privilege.md index 774e9c0cc..afd447e09 100644 --- a/docs/B.3.1-mcp-least-privilege.md +++ b/docs/B.3.1-mcp-least-privilege.md @@ -121,10 +121,13 @@ to need. code contains `httpx.post(...)`. LP1 fires for the undeclared `network` capability. -**Remediation:** For Agent Skills `SKILL.md`, add a tool that covers the missing -capability to the `allowed-tools` frontmatter field. For MCP server manifests, -add the missing capability to the `permissions` list. Otherwise, remove the code -that requires it. +**Remediation:** Review whether the capability is required and how the host +runtime enforces it. Remove code that does not need the capability. For Agent +Skills `SKILL.md`, `allowed-tools` records tools preapproved for invocation; it +is not a permission ceiling. Add a tool only when it is independently approved +as necessary, not to silence this finding. For MCP server manifests, add the +required capability to the `permissions` list only after reviewing the actual +policy. --- @@ -176,11 +179,12 @@ rather than deception) but still a significant transparency gap. `allowed-tools:` key, but the code calls `os.environ["API_KEY"]` and `subprocess.run(...)`. -**Remediation:** Declare the skill's tool scope in the manifest type you are -authoring. For Claude Code / Agent Skills `SKILL.md`, list the tools the skill -may invoke in the `allowed-tools` frontmatter field (`permissions` is not part -of the SKILL.md schema and is ignored). For MCP server manifests, add a -`permissions` list naming the required capabilities. +**Remediation:** Clarify the skill's intended tool scope and review how the host +runtime enforces it. For Claude Code / Agent Skills `SKILL.md`, `allowed-tools` +records tools preapproved for invocation, not a permission ceiling; add tools +only when independently approved as necessary, not to silence this finding +(`permissions` is not part of the `SKILL.md` schema and is ignored). For MCP +server manifests, use a `permissions` list naming the required capabilities. --- diff --git a/src/skillspector/nodes/analyzers/mcp_least_privilege.py b/src/skillspector/nodes/analyzers/mcp_least_privilege.py index 8455bea16..352242cb2 100644 --- a/src/skillspector/nodes/analyzers/mcp_least_privilege.py +++ b/src/skillspector/nodes/analyzers/mcp_least_privilege.py @@ -579,10 +579,12 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: "Without declared permissions the skill's intent is opaque and cannot be validated." ), remediation=( - "Declare the skill's tool scope: for Claude Code / Agent Skills " - "SKILL.md, list the tools the skill may invoke in the " - "'allowed-tools' frontmatter field; for MCP server manifests, " - "add a 'permissions' list naming the required capabilities." + "Clarify the skill's intended tool scope and review how the host " + "runtime enforces it. For Claude Code / Agent Skills SKILL.md, " + "'allowed-tools' records preapproved tools, not a permission ceiling; " + "add a tool only when it is independently approved as necessary, " + "not to silence this finding. For MCP server manifests, use a " + "'permissions' list naming the required capabilities." ), ) ) @@ -626,6 +628,16 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: else f"Add the '{capability}' capability to the MCP server manifest's " "'permissions' list, or remove the code that requires it." ) + explanation = ( + f"The skill uses '{capability}' capability that is not covered by " + "the declared 'allowed-tools' preapproval. Review the host runtime's " + "actual enforcement policy; this declaration alone does not establish " + "a permission ceiling." + if allowed_tools + else f"The skill uses '{capability}' capability that is not covered by " + "the declared MCP 'permissions' list. Review the server's actual " + "authorization policy." + ) budget.emit( Finding( rule_id="LP1", @@ -638,11 +650,7 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: file=primary_file, category=_CATEGORY, tags=list(_TAGS), - explanation=( - f"The skill uses '{capability}' capability that is not listed in " - "its permissions. This may indicate deceptive intent or missing " - "permission declarations." - ), + explanation=explanation, remediation=remediation, ) ) diff --git a/src/skillspector/nodes/analyzers/pattern_defaults.py b/src/skillspector/nodes/analyzers/pattern_defaults.py index ee8059593..f040a3b65 100644 --- a/src/skillspector/nodes/analyzers/pattern_defaults.py +++ b/src/skillspector/nodes/analyzers/pattern_defaults.py @@ -418,9 +418,9 @@ class PatternCategory(StrEnum): "YR3": "Remove all cryptocurrency mining code, pool references, and miner binaries. Mining in agent skills is unauthorized resource abuse. Report the skill as malicious.", "YR4": "Remove offensive tool references and exploit code. Legitimate agent skills should not contain penetration testing tools, exploit frameworks, or reconnaissance utilities.", # MCP Least Privilege (B.3.1) - "LP1": "Declare the missing capability in the manifest type being scanned: for Agent Skills SKILL.md, add a covering tool to the 'allowed-tools' frontmatter field; for MCP server manifests, add the capability to the 'permissions' list. Otherwise, remove the code that requires it.", + "LP1": "Review whether the capability is required and how the host runtime enforces it. Remove code that does not need it. For Agent Skills, do not broaden 'allowed-tools' preapproval solely to silence this finding; change preapproval only after reviewing the actual host policy. For MCP server manifests, add the required capability to 'permissions' only after that review.", "LP2": "Replace wildcard permissions ('*', 'all', 'full', 'any') with an explicit list of required permissions.", - "LP3": "Declare the skill's tool scope: for Claude Code / Agent Skills SKILL.md, list the tools the skill may invoke in the 'allowed-tools' frontmatter field; for MCP server manifests, add a 'permissions' list naming the required capabilities.", + "LP3": "Clarify the skill's intended tool scope and review how the host runtime enforces it. For Claude Code / Agent Skills SKILL.md, 'allowed-tools' records preapproved tools, not a permission ceiling; add tools only when independently approved as necessary, not to silence this finding. For MCP server manifests, use a 'permissions' list naming the required capabilities.", "LP4": "Remove the declared permission if the corresponding capability is no longer used.", # MCP Tool Poisoning (B.3.2) "TP1": "Remove hidden content (HTML comments, markdown comments, zero-width characters, base64 blobs) from metadata fields. Metadata should contain plain, visible text only.", diff --git a/tests/test_mcp_least_privilege.py b/tests/test_mcp_least_privilege.py index 7cae7a1cd..ac7393ad7 100644 --- a/tests/test_mcp_least_privilege.py +++ b/tests/test_mcp_least_privilege.py @@ -24,6 +24,7 @@ import yaml from skillspector.nodes.analyzers import mcp_least_privilege +from skillspector.nodes.analyzers.pattern_defaults import get_remediation from skillspector.nodes.report import report # --------------------------------------------------------------------------- @@ -266,6 +267,10 @@ def test_no_permissions_field(self): )["issues"][0] assert issue["pattern"] == lp3.message assert issue["finding"] == lp3.message + for remediation in (lp3.remediation, get_remediation("LP3")): + assert remediation is not None + assert "independently approved as necessary" in remediation + assert "not to silence this finding" in remediation class TestLP3AllowedTools: @@ -333,6 +338,10 @@ def test_allowed_tools_underdeclared_fires_lp1(self): assert "'permissions'" not in lp1.remediation assert "Add a tool" not in lp1.remediation assert "preapproval" in lp1.remediation + assert "do not broaden" in lp1.remediation.lower() + fallback = get_remediation("LP1") + assert "do not broaden" in fallback.lower() + assert "solely to silence" in fallback def test_allowed_tools_fully_covered_no_lp1(self): """allowed-tools: [Bash] + only shell code → no LP1 (capability is covered).""" From d8f7d7971991b05298d7291b050c62fff4d30015 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Efe=20G=C3=B6kdemir?= Date: Fri, 2 Oct 2026 22:01:40 +0300 Subject: [PATCH 3/3] docs: state runtime permissions may be unknown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Efe Gökdemir --- docs/B.3.1-mcp-least-privilege.md | 6 ++++-- .../nodes/analyzers/mcp_least_privilege.py | 12 ++++++++---- src/skillspector/nodes/analyzers/pattern_defaults.py | 4 ++-- tests/test_mcp_least_privilege.py | 3 +++ 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/docs/B.3.1-mcp-least-privilege.md b/docs/B.3.1-mcp-least-privilege.md index afd447e09..7e487e1b8 100644 --- a/docs/B.3.1-mcp-least-privilege.md +++ b/docs/B.3.1-mcp-least-privilege.md @@ -122,7 +122,8 @@ code contains `httpx.post(...)`. LP1 fires for the undeclared `network` capability. **Remediation:** Review whether the capability is required and how the host -runtime enforces it. Remove code that does not need the capability. For Agent +runtime enforces it; runtime permissions are unknown unless separately +verified. Remove code that does not need the capability. For Agent Skills `SKILL.md`, `allowed-tools` records tools preapproved for invocation; it is not a permission ceiling. Add a tool only when it is independently approved as necessary, not to silence this finding. For MCP server manifests, add the @@ -180,7 +181,8 @@ rather than deception) but still a significant transparency gap. `subprocess.run(...)`. **Remediation:** Clarify the skill's intended tool scope and review how the host -runtime enforces it. For Claude Code / Agent Skills `SKILL.md`, `allowed-tools` +runtime enforces it; runtime permissions are unknown unless separately +verified. For Claude Code / Agent Skills `SKILL.md`, `allowed-tools` records tools preapproved for invocation, not a permission ceiling; add tools only when independently approved as necessary, not to silence this finding (`permissions` is not part of the `SKILL.md` schema and is ignored). For MCP diff --git a/src/skillspector/nodes/analyzers/mcp_least_privilege.py b/src/skillspector/nodes/analyzers/mcp_least_privilege.py index 352242cb2..af94309ac 100644 --- a/src/skillspector/nodes/analyzers/mcp_least_privilege.py +++ b/src/skillspector/nodes/analyzers/mcp_least_privilege.py @@ -580,7 +580,8 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: ), remediation=( "Clarify the skill's intended tool scope and review how the host " - "runtime enforces it. For Claude Code / Agent Skills SKILL.md, " + "runtime enforces it; runtime permissions are unknown unless " + "separately verified. For Claude Code / Agent Skills SKILL.md, " "'allowed-tools' records preapproved tools, not a permission ceiling; " "add a tool only when it is independently approved as necessary, " "not to silence this finding. For MCP server manifests, use a " @@ -621,7 +622,8 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: confidence = _clamp(0.55 if capability in test_only_caps else 0.75) remediation = ( f"Review whether the '{capability}' capability is required and " - "whether the host runtime policy permits it. Remove code that " + "whether the host runtime policy permits it; runtime permissions " + "are unknown unless separately verified. Remove code that " "does not need the capability; do not broaden 'allowed-tools' " "preapproval solely to silence this finding." if allowed_tools @@ -631,12 +633,14 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: explanation = ( f"The skill uses '{capability}' capability that is not covered by " "the declared 'allowed-tools' preapproval. Review the host runtime's " - "actual enforcement policy; this declaration alone does not establish " + "actual enforcement policy; runtime permissions are unknown unless " + "separately verified, and this declaration alone does not establish " "a permission ceiling." if allowed_tools else f"The skill uses '{capability}' capability that is not covered by " "the declared MCP 'permissions' list. Review the server's actual " - "authorization policy." + "authorization policy; runtime permissions are unknown unless " + "separately verified." ) budget.emit( Finding( diff --git a/src/skillspector/nodes/analyzers/pattern_defaults.py b/src/skillspector/nodes/analyzers/pattern_defaults.py index f040a3b65..f28957b13 100644 --- a/src/skillspector/nodes/analyzers/pattern_defaults.py +++ b/src/skillspector/nodes/analyzers/pattern_defaults.py @@ -418,9 +418,9 @@ class PatternCategory(StrEnum): "YR3": "Remove all cryptocurrency mining code, pool references, and miner binaries. Mining in agent skills is unauthorized resource abuse. Report the skill as malicious.", "YR4": "Remove offensive tool references and exploit code. Legitimate agent skills should not contain penetration testing tools, exploit frameworks, or reconnaissance utilities.", # MCP Least Privilege (B.3.1) - "LP1": "Review whether the capability is required and how the host runtime enforces it. Remove code that does not need it. For Agent Skills, do not broaden 'allowed-tools' preapproval solely to silence this finding; change preapproval only after reviewing the actual host policy. For MCP server manifests, add the required capability to 'permissions' only after that review.", + "LP1": "Review whether the capability is required and how the host runtime enforces it; runtime permissions are unknown unless separately verified. Remove code that does not need it. For Agent Skills, do not broaden 'allowed-tools' preapproval solely to silence this finding; change preapproval only after reviewing the actual host policy. For MCP server manifests, add the required capability to 'permissions' only after that review.", "LP2": "Replace wildcard permissions ('*', 'all', 'full', 'any') with an explicit list of required permissions.", - "LP3": "Clarify the skill's intended tool scope and review how the host runtime enforces it. For Claude Code / Agent Skills SKILL.md, 'allowed-tools' records preapproved tools, not a permission ceiling; add tools only when independently approved as necessary, not to silence this finding. For MCP server manifests, use a 'permissions' list naming the required capabilities.", + "LP3": "Clarify the skill's intended tool scope and review how the host runtime enforces it; runtime permissions are unknown unless separately verified. For Claude Code / Agent Skills SKILL.md, 'allowed-tools' records preapproved tools, not a permission ceiling; add tools only when independently approved as necessary, not to silence this finding. For MCP server manifests, use a 'permissions' list naming the required capabilities.", "LP4": "Remove the declared permission if the corresponding capability is no longer used.", # MCP Tool Poisoning (B.3.2) "TP1": "Remove hidden content (HTML comments, markdown comments, zero-width characters, base64 blobs) from metadata fields. Metadata should contain plain, visible text only.", diff --git a/tests/test_mcp_least_privilege.py b/tests/test_mcp_least_privilege.py index ac7393ad7..80ba94d88 100644 --- a/tests/test_mcp_least_privilege.py +++ b/tests/test_mcp_least_privilege.py @@ -271,6 +271,7 @@ def test_no_permissions_field(self): assert remediation is not None assert "independently approved as necessary" in remediation assert "not to silence this finding" in remediation + assert "runtime permissions are unknown unless separately verified" in remediation class TestLP3AllowedTools: @@ -339,9 +340,11 @@ def test_allowed_tools_underdeclared_fires_lp1(self): assert "Add a tool" not in lp1.remediation assert "preapproval" in lp1.remediation assert "do not broaden" in lp1.remediation.lower() + assert "runtime permissions are unknown unless separately verified" in lp1.remediation fallback = get_remediation("LP1") assert "do not broaden" in fallback.lower() assert "solely to silence" in fallback + assert "runtime permissions are unknown unless separately verified" in fallback def test_allowed_tools_fully_covered_no_lp1(self): """allowed-tools: [Bash] + only shell code → no LP1 (capability is covered)."""