Skip to content
37 changes: 33 additions & 4 deletions src/skillspector/nodes/analyzers/static_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ def _is_env_file_reference_in_docs(
return False
if file_type not in ("markdown", "text"):
return False
if file_path.replace("\\", "/").lower().endswith("skill.md"):
if _is_skill_md(file_path):
return False
if not finding.context:
return False
Expand Down Expand Up @@ -198,6 +198,27 @@ def _is_eval_dataset(path: str) -> bool:
return path.replace("\\", "/") in _EVAL_DATASET_FILES


def _is_skill_md(path: str) -> bool:
"""Return True for paths with the existing loose SKILL.md suffix match."""
return path.lower().endswith("skill.md")


def _is_canonical_skill_md(path: str) -> bool:
"""Return True for files literally named SKILL.md."""
normalized = path.replace("\\", "/")
return normalized.rsplit("/", 1)[-1].lower() == "skill.md"


def _is_fenced_code_block(content: str, line_number: int) -> bool:
"""Return True when a 1-based content line is inside a triple-backtick block."""
if line_number <= 1:
return False
fence_count = sum(
line.lstrip().startswith("```") for line in content.splitlines()[: line_number - 1]
)
return fence_count % 2 == 1


_DOCUMENTATION_DIR_NAMES = (
"docs",
"documentation",
Expand Down Expand Up @@ -256,7 +277,7 @@ def _is_documentation_context(af: AnalyzerFinding, file_type: str, path: str, co
"""Return true when a governed finding is prose or a comment without execution signals."""
if af.rule_id not in _SEMANTIC_STRING_DOC_PRONE_RULES:
return False
if path.replace("\\", "/").lower().endswith("skill.md"):
if _is_skill_md(path):
return False
lines = content.splitlines()
matched_line = (
Expand All @@ -276,7 +297,7 @@ def _is_documentation_markdown(path: str) -> bool:
normalized = path.replace("\\", "/").lower()
if not normalized.endswith((".md", ".markdown")):
return False
if normalized.endswith("skill.md"):
if _is_skill_md(path):
return False
parts = normalized.split("/")
return any(part in _DOCUMENTATION_DIR_NAMES for part in parts[:-1])
Expand Down Expand Up @@ -355,7 +376,15 @@ def _scan_path(
# PE3's analyzer owns its narrowly qualified safe references.
# Generic documentation words are attacker-controlled and must
# not hard-drop HIGH credential-access findings here.
if af.rule_id != "PE3" and af.context and is_code_example(af.context):
if (
af.rule_id != "PE3"
and af.context
and is_code_example(af.context)
and not (
_is_canonical_skill_md(path)
and _is_fenced_code_block(content, af.location.start_line)
)
):
if is_non_executable:
logger.debug(
"Filtered code-example finding in non-executable: %s in %s:%d",
Expand Down
65 changes: 58 additions & 7 deletions tests/nodes/analyzers/test_static_runner_filtering.py
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ def test_extensionless_file_not_hard_dropped_by_code_example(self) -> None:
)

def test_skill_md_findings_are_not_filtered_by_backticks(self) -> None:
"""SKILL.md is the primary instruction file — backticks alone shouldn't filter."""
"""SKILL.md is the primary instruction file, so fenced findings survive."""
content = """\
---
name: deploy-tool
Expand All @@ -345,12 +345,63 @@ def test_skill_md_findings_are_not_filtered_by_backticks(self) -> None:
"file_cache": {"SKILL.md": content},
}
findings = static_runner.run_static_patterns(state, [tm_module])
# SKILL.md code blocks do get filtered by is_code_example (same as EA2/MP)
# This is correct: the meta-analyzer handles SKILL.md nuance
# The key test is that SKILL.md is NOT treated as documentation-path markdown
for f in findings:
# Confidence should NOT be reduced by _DOCUMENTATION_CONFIDENCE_FACTOR
assert f.confidence >= 0.3
tm1_findings = [f for f in findings if f.rule_id == "TM1"]
assert len(tm1_findings) == 1
assert tm1_findings[0].confidence == 0.6

def test_fenced_guide_md_findings_are_filtered(self) -> None:
"""Ordinary fenced Markdown remains filtered as a documentation example."""
content = """\
Use this tool to deploy:
```
curl -k https://production.example.com/deploy
```
"""
assert "TM1" not in _findings(content, "guide.md", tm_module)

def test_fenced_noncanonical_skill_md_suffix_paths_remain_filtered(self) -> None:
"""Only files literally named SKILL.md bypass the runner's code-example filter."""
content = """\
Use this tool to deploy:
```
curl -k https://production.example.com/deploy
```
"""
for path in ("my_skill.md", "docs/bash_skill.md"):
assert "TM1" not in _findings(content, path, tm_module)

def test_inline_skill_md_examples_remain_filtered(self) -> None:
"""Only fenced SKILL.md instructions bypass the generic example filter."""
content = "For example, run `curl -k https://production.example.com/deploy`."
assert "TM1" not in _findings(content, "SKILL.md", tm_module)

def test_skill_md_example_after_fence_remains_filtered(self) -> None:
"""A prose example after a fence must not inherit the fenced exception."""
content = """\
```
echo safe
```
For example, run `curl -k https://production.example.com/deploy`.
"""
assert "TM1" not in _findings(content, "SKILL.md", tm_module)

@pytest.mark.parametrize(
("path", "expected_loose", "expected_canonical"),
[
("SKILL.md", True, True),
("nested/SKILL.md", True, True),
("nested\\skill.md", True, True),
("my_skill.md", True, False),
("docs/bash_skill.md", True, False),
("skill.md.bak", False, False),
],
)
def test_skill_md_path_helpers_distinguish_loose_and_canonical_matches(
self, path: str, expected_loose: bool, expected_canonical: bool
) -> None:
"""Legacy suffix checks stay loose, while the new runner gate is exact-basename."""
assert static_runner._is_skill_md(path) is expected_loose
assert static_runner._is_canonical_skill_md(path) is expected_canonical


class TestDocumentationPathConfidenceReduction:
Expand Down
27 changes: 27 additions & 0 deletions tests/unit/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,33 @@ def test_cli_scan_required_table_keeps_malicious_pe3(tmp_path: Path) -> None:
assert any(issue["id"] == "PE3" for issue in issues)


def test_cli_scan_no_llm_reports_fenced_skill_md_tool_misuse(tmp_path: Path) -> None:
"""No-LLM scans retain TM1 from fenced SKILL.md instructions."""
(tmp_path / "SKILL.md").write_text(
"""---
name: deploy-tool
---
# Deploy Tool

Use this tool to deploy:
```
curl -k https://production.example.com/deploy
```

The agent will execute the above command.
""",
encoding="utf-8",
)

result = runner.invoke(app, ["scan", str(tmp_path), "--no-llm", "--format", "json"])

assert result.exit_code == 0
data = json.loads(result.output)
assert any(issue["id"] == "TM1" for issue in data["issues"])
assert data["analysis_completeness"]["findings_before_filtering"] > 0
assert data["analysis_completeness"]["findings_after_filtering"] > 0


def test_cli_scan_nonexistent_exits_2() -> None:
"""scan with nonexistent path exits with code 2."""
result = runner.invoke(app, ["scan", "/nonexistent/path/xyz"])
Expand Down
Loading