From e739a9a59f8f022f5a3782b85f6c818b0073d5be Mon Sep 17 00:00:00 2001 From: Varshith Puli Date: Sat, 7 Nov 2026 01:53:01 +0000 Subject: [PATCH 1/3] fix(tp4): anchor findings to the assessed file, align severity, and drop absence-only mismatches TP4 reported every finding at SKILL.md:1 even when the assessed fence lived in another file, escalated severity to HIGH at confidence 0.7 while the README pattern table lists TP4 as MEDIUM, and flagged partial workflow snippets for omitting the rest of the declared workflow. Anchor each TP4 finding to the batch file and lines, which the evidence already carried. TP4 severity is now always MEDIUM, matching the README. Add a partial-snippet criterion to the TP4 prompt, and suppress an assessment deterministically when every mismatched_capabilities entry is phrased as an absence, since that carries no affirmative evidence of a mismatch. Mixed and affirmative assessments still emit. Fixes NVIDIA/SkillSpector#1066 Signed-off-by: Varshith Puli --- .../nodes/analyzers/mcp_tool_poisoning.py | 60 +++++++- tests/test_mcp_tool_poisoning.py | 138 +++++++++++++++++- tests/unit/test_tp4_openai_http.py | 5 +- 3 files changed, 197 insertions(+), 6 deletions(-) diff --git a/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py b/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py index fd0d4f41d..2fed50eca 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,47 @@ 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. +_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", + ) +) + + +def _tp4_mismatched_item_is_absence(item: str) -> bool: + """Return whether one mismatched capability is phrased as an absence.""" + normalized = " ".join(item.strip().casefold().split()) + return any(pattern.search(normalized) for pattern in _TP4_ABSENCE_PATTERNS) + + +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 +1198,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 +1218,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..19ee4586a 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,138 @@ 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 + + @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), + ], + ) + 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 +2327,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 From dd3b48ad6106d2828edc62aef0ee496464b51d2b Mon Sep 17 00:00:00 2001 From: Varshith Puli Date: Mon, 9 Nov 2026 19:23:49 +0000 Subject: [PATCH 2/3] fix(tp4): keep affirmative capabilities containing absence words (#1069) An absence marker anywhere in a mismatched capability (without, missing, lacks) suppressed the whole assessment, even when the item described an affirmative undeclared capability such as uploading source files to an undeclared endpoint without user consent. Make suppression conservative: an item counts as an absence only when no clause of it describes affirmative behavior, with capability verbs under a marker treated as governed by it. Add regressions for an affirmative capability containing without and for an item holding both an affirmative action and a negated clause. Signed-off-by: Varshith Puli --- .../nodes/analyzers/mcp_tool_poisoning.py | 58 ++++++++++++++++++- tests/test_mcp_tool_poisoning.py | 53 +++++++++++++++++ 2 files changed, 108 insertions(+), 3 deletions(-) diff --git a/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py b/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py index 2fed50eca..8bc2d4bd3 100644 --- a/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py +++ b/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py @@ -1153,7 +1153,12 @@ def _tp4_partial_event( # 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. +# 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 ( @@ -1179,10 +1184,57 @@ def _tp4_partial_event( ) +_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.""" + """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()) - return any(pattern.search(normalized) for pattern in _TP4_ABSENCE_PATTERNS) + 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: diff --git a/tests/test_mcp_tool_poisoning.py b/tests/test_mcp_tool_poisoning.py index 19ee4586a..bf0a3b6f4 100644 --- a/tests/test_mcp_tool_poisoning.py +++ b/tests/test_mcp_tool_poisoning.py @@ -1296,6 +1296,49 @@ def test_affirmative_mismatched_capability_emitted(self): 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", [ @@ -1307,6 +1350,16 @@ def test_affirmative_mismatched_capability_emitted(self): ("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): From 017b3a92f56609557069e3cadeab781c87daa373 Mon Sep 17 00:00:00 2001 From: Varshith Puli Date: Mon, 9 Nov 2026 22:05:33 +0000 Subject: [PATCH 3/3] style: apply ruff format to TP4 analyzer and tests (#1069) --- src/skillspector/nodes/analyzers/mcp_tool_poisoning.py | 4 +--- tests/test_mcp_tool_poisoning.py | 6 ++---- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py b/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py index 8bc2d4bd3..c759c1c26 100644 --- a/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py +++ b/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py @@ -1227,9 +1227,7 @@ def _tp4_mismatched_item_is_absence(item: str) -> bool: if not clause: continue marker_positions = [ - match.start() - for pattern in _TP4_ABSENCE_PATTERNS - for match in pattern.finditer(clause) + 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): diff --git a/tests/test_mcp_tool_poisoning.py b/tests/test_mcp_tool_poisoning.py index bf0a3b6f4..11be51e13 100644 --- a/tests/test_mcp_tool_poisoning.py +++ b/tests/test_mcp_tool_poisoning.py @@ -1327,10 +1327,8 @@ def test_affirmative_action_with_negated_clause_emitted(self): 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", + "Uploads source files to an external endpoint but does not log the transfer", + "Sends credentials to a remote host without validating the certificate", ], ), batch,