Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 20 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,17 @@ 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"


_DOCUMENTATION_DIR_NAMES = (
"docs",
"documentation",
Expand Down Expand Up @@ -256,7 +267,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 +287,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 +366,12 @@ 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)
):
if is_non_executable:
logger.debug(
"Filtered code-example finding in non-executable: %s in %s:%d",
Expand Down
50 changes: 43 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,48 @@ 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)

@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 @@ -208,6 +208,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