diff --git a/src/skillspector/nodes/analyzers/static_runner.py b/src/skillspector/nodes/analyzers/static_runner.py index f97ef8a4..f9c50068 100644 --- a/src/skillspector/nodes/analyzers/static_runner.py +++ b/src/skillspector/nodes/analyzers/static_runner.py @@ -140,7 +140,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 @@ -165,6 +165,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", @@ -192,7 +203,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 = ( @@ -212,7 +223,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]) @@ -286,6 +297,7 @@ def run_static_patterns( file_type = _infer_file_type(path) is_doc_markdown = _is_documentation_markdown(path) is_non_executable = file_type in _NON_EXECUTABLE_FILE_TYPES + is_canonical_skill_md = _is_canonical_skill_md(path) for module in pattern_modules: raw = module.analyze(content=content, file_path=path, file_type=file_type) for af in raw: @@ -297,7 +309,7 @@ def run_static_patterns( af.location.start_line, ) continue - if af.context and is_code_example(af.context): + if af.context and is_code_example(af.context) and not is_canonical_skill_md: if is_non_executable: logger.debug( "Filtered code-example finding in non-executable: %s in %s:%d", diff --git a/tests/nodes/analyzers/test_static_runner_filtering.py b/tests/nodes/analyzers/test_static_runner_filtering.py index a69d7870..96c0d2b2 100644 --- a/tests/nodes/analyzers/test_static_runner_filtering.py +++ b/tests/nodes/analyzers/test_static_runner_filtering.py @@ -210,7 +210,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 @@ -229,12 +229,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: diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index e340ccd8..5c526392 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -68,6 +68,33 @@ def test_cli_scan_no_llm(tmp_path: Path) -> None: assert result.exit_code == 0 +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"])