diff --git a/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py b/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py index fd0d4f41d..c759c1c26 100644 --- a/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py +++ b/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py @@ -1063,7 +1063,9 @@ class _TP4CheckOutcome: Flag a mismatch when code performs an undeclared capability, has a materially different primary purpose, accesses inconsistent resources, or has unrelated triggers. Do not flag supporting implementation details or over-declared -permissions. Return the assessment using the structured output schema. +permissions. A chunk may implement only one declared workflow step; do not +flag omitted capabilities. Each entry must name what the code DOES. Return +the assessment using the structured output schema. """ @@ -1147,6 +1149,97 @@ def _tp4_partial_event( ) +# Phrasings that describe an absence of functionality rather than a +# capability the code performs. A TP4 assessment whose every +# mismatched_capabilities entry matches one of these carries no affirmative +# evidence of a mismatch (issue #1066): a partial snippet cannot be faulted +# for omitting the rest of the declared workflow. The match alone is not +# enough: an absence marker inside an item that also describes affirmative +# behavior (for example "Uploads source files to an undeclared endpoint +# without user consent") does not make the item an absence, so +# _tp4_mismatched_item_is_absence also requires that no clause of the item +# contain an affirmative capability verb outside the scope of a marker. +_TP4_ABSENCE_PATTERNS = tuple( + re.compile(pattern) + for pattern in ( + r"^no\b", + r"^not\b", + r"^cannot\b", + r"^can'?t\b", + r"\bdoes not\b", + r"\bdoesn'?t\b", + r"\bdo not\b", + r"\bdid not\b", + r"\bno evidence\b", + r"\bno implementation\b", + r"\bno support\b", + r"\bnot implement", + r"\blacks?\b", + r"\bmissing\b", + r"\bwithout\b", + r"\bfails? to\b", + r"\babsence of\b", + r"\bomits?\b", + ) +) + + +_TP4_CLAUSE_SPLIT_PATTERN = re.compile( + r"\s*;\s*|\s+but\s+|\s+however\s+|\s+although\s+|\s+though\s+|\s+while\s+|\s+whereas\s+" +) + +_TP4_AFFIRMATIVE_VERB_PATTERN = re.compile( + r"\b(?:" + r"access|archive|authenticate|backup|browse|call|collect|compress|" + r"connect|cop(?:y|ies|ied|ying)|crawl|create|decrypt|delete|download|" + r"encode|encrypt|enumerate|execute|export|exfiltrate|extract|fetch|" + r"fingerprint|gather|generate|hash|import|install|invoke|issue|leak|" + r"load|log|modify|monitor|move|navigate|notify|open|parse|perform|" + r"post|process|profile|publish|pull|push|quer(?:y|ies|ied|ying)|read|" + r"record|remove|render|request|respond|restore|retrieve|run|save|" + r"scan|scrape|search|send|share|sign|spawn|store|submit|subscribe|" + r"sync|track|transmit|type|update|upload|validate|verify|write|click" + r")(?:s|es|ed|ing|d|ning)?\b" +) + + +def _tp4_mismatched_item_is_absence(item: str) -> bool: + """Return whether one mismatched capability is phrased as an absence. + + Conservative: an absence marker only counts when no clause of the item + describes affirmative behavior. A capability verb that precedes the + first absence marker in its clause, or sits in a clause with no marker, + is affirmative evidence. So "Uploads source files to an undeclared + endpoint without user consent", "Uploads files but does not encrypt + them", and "No support for TLS, but sends credentials in plaintext" + are not absences, while "No evidence of querying videos" and "The code + does not implement search" still are: their verbs sit under the marker. + Clauses split only on contrast boundaries, so enumerations joined by + "and" or commas stay under one leading marker. + """ + normalized = " ".join(item.strip().casefold().split()) + if not normalized: + return False + if not any(pattern.search(normalized) for pattern in _TP4_ABSENCE_PATTERNS): + return False + for clause in _TP4_CLAUSE_SPLIT_PATTERN.split(normalized): + clause = clause.strip() + if not clause: + continue + marker_positions = [ + match.start() for pattern in _TP4_ABSENCE_PATTERNS for match in pattern.finditer(clause) + ] + for verb in _TP4_AFFIRMATIVE_VERB_PATTERN.finditer(clause): + if not marker_positions or verb.start() < min(marker_positions): + return False + return True + + +def _tp4_all_mismatched_are_absences(items: list[str]) -> bool: + """Return whether every listed capability is an absence, not a behavior.""" + return bool(items) and all(_tp4_mismatched_item_is_absence(item) for item in items) + + def _tp4_finding( result: _TP4AnalysisResult, batch: Batch, @@ -1155,6 +1248,13 @@ def _tp4_finding( """Convert one bounded batch assessment without dropping source evidence.""" if not result.is_mismatch or result.confidence < 0.5: return None + if _tp4_all_mismatched_are_absences(result.mismatched_capabilities): + logger.debug( + "%s: TP4 assessment suppressed: every mismatched capability is " + "phrased as an absence, so there is no affirmative mismatch evidence", + ANALYZER_ID, + ) + return None declared = (result.declared_purpose_summary or description[:512])[:512] actual = result.actual_behavior_summary[:1024] mismatched = [str(item)[:256] for item in result.mismatched_capabilities[:16]] @@ -1168,9 +1268,13 @@ def _tp4_finding( return Finding( rule_id="TP4", message=message, - severity="HIGH" if result.confidence >= 0.7 else "MEDIUM", + # TP4 is documented as MEDIUM in the README pattern table; the + # confidence-based escalation to HIGH was never documented (issue #1066). + severity="MEDIUM", confidence=result.confidence, - file="SKILL.md", + file=batch.file_path, + start_line=batch.start_line, + end_line=batch.end_line, category=_CATEGORY, pattern=get_pattern_name("TP4"), finding=message, diff --git a/tests/test_mcp_tool_poisoning.py b/tests/test_mcp_tool_poisoning.py index 67b8d7ad1..11be51e13 100644 --- a/tests/test_mcp_tool_poisoning.py +++ b/tests/test_mcp_tool_poisoning.py @@ -1139,7 +1139,7 @@ def test_mismatch_detected(self, monkeypatch: pytest.MonkeyPatch): result = node(state) tp4 = [f for f in result["findings"] if f.rule_id == "TP4"] assert len(tp4) >= 1 - assert tp4[0].severity in {"HIGH", "MEDIUM"} + assert tp4[0].severity == "MEDIUM" assert tp4[0].pattern == "Description-Behavior Mismatch" assert tp4[0].finding == tp4[0].message @@ -1181,6 +1181,189 @@ def test_finding_maps_result_metadata_for_unit_ci(self): assert finding.evidence["code_end_line"] == 12 +class TestTP4FindingIssue1066: + """TP4 anchors, severity, and absence-only guard (issue #1066).""" + + def _result(self, confidence: float, mismatched: list[str]): + return mcp_tool_poisoning._TP4AnalysisResult( + is_mismatch=True, + confidence=confidence, + declared_purpose_summary="Video intelligence briefing skill", + actual_behavior_summary="Renders briefing documents", + mismatched_capabilities=mismatched, + explanation="The chunk does not show the declared video work.", + ) + + def test_finding_anchors_to_real_file_and_lines(self): + batch = mcp_tool_poisoning.Batch( + file_path="references/briefing-authoring.md", + content="unzip -q -o out.epub -d _chk", + start_line=27, + end_line=28, + ) + + finding = mcp_tool_poisoning._tp4_finding( + self._result(0.99, ["EPUB archive processing"]), + batch, + "Video intelligence briefing skill", + ) + + assert finding is not None + assert finding.file == "references/briefing-authoring.md" + assert finding.start_line == 27 + assert finding.end_line == 28 + assert finding.evidence["code_path"] == "references/briefing-authoring.md" + assert finding.evidence["code_start_line"] == 27 + assert finding.evidence["code_end_line"] == 28 + + def test_finding_severity_is_medium_at_high_confidence(self): + batch = mcp_tool_poisoning.Batch( + file_path="SKILL.md", + content="python scripts/markdown_pdf.py", + start_line=697, + end_line=697, + ) + + finding = mcp_tool_poisoning._tp4_finding( + self._result(0.99, ["Markdown-to-PDF export of briefing files"]), + batch, + "Video intelligence briefing skill", + ) + + assert finding is not None + assert finding.severity == "MEDIUM" + + def test_absence_only_mismatched_capabilities_suppressed(self): + batch = mcp_tool_poisoning.Batch( + file_path="SKILL.md", + content="pytest tests/evals/test_search_quality.py -v -s", + start_line=314, + end_line=314, + ) + + finding = mcp_tool_poisoning._tp4_finding( + self._result( + 0.99, + [ + "No evidence of querying videos, transcripts, concepts, " + "quotes, corpus status, or profile data", + "No implementation of the described nugget synthesis/save behavior", + ], + ), + batch, + "Video corpus search skill", + ) + + assert finding is None + + def test_mixed_mismatched_capabilities_still_emitted(self): + batch = mcp_tool_poisoning.Batch( + file_path="SKILL.md", + content="pytest tests/evals/test_search_quality.py -v -s", + start_line=314, + end_line=314, + ) + + finding = mcp_tool_poisoning._tp4_finding( + self._result( + 0.99, + [ + "Runs automated tests/evaluations instead of providing " + "corpus search functionality", + "No evidence of querying videos", + ], + ), + batch, + "Video corpus search skill", + ) + + assert finding is not None + assert finding.severity == "MEDIUM" + + def test_affirmative_mismatched_capability_emitted(self): + batch = mcp_tool_poisoning.Batch( + file_path="scripts/tool.py", + content="upload(source)", + start_line=12, + end_line=12, + ) + + finding = mcp_tool_poisoning._tp4_finding( + self._result(0.9, ["Sends source data to a remote endpoint"]), + batch, + "A local text utility", + ) + + assert finding is not None + + def test_affirmative_capability_containing_without_emitted(self): + batch = mcp_tool_poisoning.Batch( + file_path="scripts/tool.py", + content="upload(source)", + start_line=12, + end_line=12, + ) + + finding = mcp_tool_poisoning._tp4_finding( + self._result( + 0.9, + ["Uploads source files to an undeclared endpoint without user consent"], + ), + batch, + "A local text utility", + ) + + assert finding is not None + + def test_affirmative_action_with_negated_clause_emitted(self): + batch = mcp_tool_poisoning.Batch( + file_path="scripts/tool.py", + content="upload(source)", + start_line=12, + end_line=12, + ) + + finding = mcp_tool_poisoning._tp4_finding( + self._result( + 0.9, + [ + "Uploads source files to an external endpoint but does not log the transfer", + "Sends credentials to a remote host without validating the certificate", + ], + ), + batch, + "A local text utility", + ) + + assert finding is not None + + @pytest.mark.parametrize( + "item, expected", + [ + ("No evidence of querying videos", True), + ("No implementation of the described behavior", True), + ("The code does not implement search", True), + ("Lacks any corpus query", True), + ("Missing authentication", True), + ("Runs automated tests instead of searching", False), + ("Sends source data to a remote endpoint", False), + ("Markdown-to-PDF export of briefing files", False), + ( + "Uploads source files to an undeclared endpoint without user consent", + False, + ), + ("Uploads files but does not encrypt them", False), + ("No support for TLS, but sends credentials in plaintext", False), + ( + "Sends credentials to a remote host without validating the certificate", + False, + ), + ], + ) + def test_absence_phrasing_detection(self, item: str, expected: bool): + assert mcp_tool_poisoning._tp4_mismatched_item_is_absence(item) is expected + + class TestTP4MarkdownFences: def test_markdown_only_fenced_python_reaches_tp4(self, monkeypatch: pytest.MonkeyPatch): structured = _mock_tp4_structured_llm( @@ -2195,7 +2378,9 @@ async def arun_batches_detailed(self, batches): if confidence is not None: finding = result.findings[0] assert finding.rule_id == "TP4" - assert finding.severity == ("HIGH" if confidence >= 0.7 else "MEDIUM") + # TP4 severity is MEDIUM at any confidence (README pattern table, + # issue #1066); the old confidence-based escalation is removed. + assert finding.severity == "MEDIUM" assert finding.evidence["assessment_complete"] is False assert "llm-format-recovery" in finding.tags producers = [ diff --git a/tests/unit/test_tp4_openai_http.py b/tests/unit/test_tp4_openai_http.py index 2ee5711fe..f47a04bd9 100644 --- a/tests/unit/test_tp4_openai_http.py +++ b/tests/unit/test_tp4_openai_http.py @@ -341,7 +341,10 @@ def test_full_scanner_graph_reports_tp4_http_results( findings = [f for f in result["findings"] if f.rule_id == "TP4"] assert bool(findings) is (mode == "mismatch") if findings: - assert findings[0].file == "SKILL.md" + # TP4 findings anchor to the assessed file, not SKILL.md (issue #1066). + assert findings[0].file == "format.py" + assert findings[0].start_line == 1 + assert findings[0].end_line == 2 assert findings[0].evidence["code_path"] == "format.py" assert findings[0].evidence["code_start_line"] == 1 assert findings[0].evidence["code_end_line"] == 2