From 023d8a408b25708bcf74309da53edfa841b6de7d Mon Sep 17 00:00:00 2001 From: kigland Date: Mon, 6 Jul 2026 05:00:58 +0800 Subject: [PATCH] Reduce false positives for negated safety constraints Signed-off-by: kigland --- .../static_patterns_privilege_escalation.py | 11 ++++ .../analyzers/static_patterns_rogue_agent.py | 14 ++++- .../analyzers/static_patterns_tool_misuse.py | 16 +++++- .../test_static_false_positive_controls.py | 52 +++++++++++++++++++ 4 files changed, 90 insertions(+), 3 deletions(-) create mode 100644 tests/nodes/analyzers/test_static_false_positive_controls.py diff --git a/src/skillspector/nodes/analyzers/static_patterns_privilege_escalation.py b/src/skillspector/nodes/analyzers/static_patterns_privilege_escalation.py index 660bc0c0..4980ba7c 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_privilege_escalation.py +++ b/src/skillspector/nodes/analyzers/static_patterns_privilege_escalation.py @@ -162,6 +162,8 @@ def loc(ln: int) -> Location: context = get_context(content, match.start()) if _is_documentation_example(context, file_type): continue + if _is_negated_safety_constraint(context, match.group(0)): + continue findings.append( AnalyzerFinding( rule_id="PE3", @@ -254,6 +256,15 @@ def _is_documentation_example(context: str, file_type: str) -> bool: return any(ind in ctx_lower for ind in doc_indicators) +def _is_negated_safety_constraint(context: str, matched_text: str) -> bool: + """Return True when a privilege-escalation phrase is forbidden in policy prose.""" + escaped = re.escape(matched_text.strip()) + if not escaped: + return False + negation = r"(?:must\s+not|do\s+not|don't|never|should\s+not)\s+" + return re.search(negation + r"(?:\w+\s+){0,4}" + escaped, context, re.IGNORECASE) is not None + + def node(state: SkillspectorState) -> AnalyzerNodeResponse: """Run privilege_escalation patterns and return findings.""" findings = static_runner.run_static_patterns(state, [sys.modules[__name__]]) diff --git a/src/skillspector/nodes/analyzers/static_patterns_rogue_agent.py b/src/skillspector/nodes/analyzers/static_patterns_rogue_agent.py index 09effa70..4371cd0e 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_rogue_agent.py +++ b/src/skillspector/nodes/analyzers/static_patterns_rogue_agent.py @@ -156,6 +156,9 @@ def ctx(start: int) -> str: for pattern, confidence in RA1_PATTERNS: for match in re.finditer(pattern, content, re.IGNORECASE | re.MULTILINE): line_num = get_line_number(content, match.start()) + context = ctx(match.start()) + if _is_negated_safety_constraint(context, match.group(0)): + continue findings.append( AnalyzerFinding( rule_id="RA1", @@ -164,7 +167,7 @@ def ctx(start: int) -> str: location=loc(line_num), confidence=confidence, tags=tag, - context=ctx(match.start()), + context=context, matched_text=match.group(0)[:200], ) ) @@ -186,6 +189,15 @@ def ctx(start: int) -> str: return findings +def _is_negated_safety_constraint(context: str, matched_text: str) -> bool: + """Return True when an RA1 phrase is explicitly forbidden in policy prose.""" + escaped = re.escape(matched_text.strip()) + if not escaped: + return False + negation = r"(?:must\s+not|do\s+not|don't|never|should\s+not)\s+" + return re.search(negation + r"(?:\w+\s+){0,4}" + escaped, context, re.IGNORECASE) is not None + + def node(state: SkillspectorState) -> AnalyzerNodeResponse: """Run rogue_agent patterns and return findings.""" findings = static_runner.run_static_patterns(state, [sys.modules[__name__]]) diff --git a/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py b/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py index c5884501..fcda1d70 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py +++ b/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py @@ -181,6 +181,11 @@ re.compile(r"rm\s+-rf\s+/root/\.cache", re.IGNORECASE), ) +_SAFE_CACHE_CLEANUP_PATTERNS: tuple[re.Pattern[str], ...] = ( + re.compile(r"\brm\s+-rf\s+['\"]?\$?\{?HOME\}?/\.cache/[^*\s;&|]+", re.IGNORECASE), + re.compile(r"\brm\s+-rf\s+['\"]?~/\.cache/[^*\s;&|]+", re.IGNORECASE), +) + # Dockerfile context indicators (nearby keywords that signal Dockerfile content) _DOCKERFILE_CONTEXT_RE = re.compile( r"\b(?:FROM|RUN|WORKDIR|COPY|ADD|ENV|EXPOSE|ENTRYPOINT|CMD|USER|HEALTHCHECK|ARG)\s", @@ -199,6 +204,11 @@ def _is_safe_dockerfile_idiom(context: str, matched_text: str) -> bool: return any(p.search(matched_text) or p.search(context) for p in _SAFE_DOCKERFILE_PATTERNS) +def _is_safe_cache_cleanup(matched_text: str) -> bool: + """Return True for scoped cleanup of a tool-owned user cache path.""" + return any(p.search(matched_text) for p in _SAFE_CACHE_CLEANUP_PATTERNS) + + def analyze(content: str, file_path: str, file_type: str) -> list[AnalyzerFinding]: """Analyze content for tool misuse patterns (TM1–TM3).""" findings: list[AnalyzerFinding] = [] @@ -217,8 +227,10 @@ def ctx(start: int) -> str: context_text = ctx(match.start()) matched = match.group(0)[:200] - if _is_safe_container_command(context_text) or _is_safe_dockerfile_idiom( - context_text, matched + if ( + _is_safe_container_command(context_text) + or _is_safe_dockerfile_idiom(context_text, matched) + or _is_safe_cache_cleanup(matched) ): adj = min(confidence, 0.15) sev = Severity.LOW diff --git a/tests/nodes/analyzers/test_static_false_positive_controls.py b/tests/nodes/analyzers/test_static_false_positive_controls.py new file mode 100644 index 00000000..e9bae4bb --- /dev/null +++ b/tests/nodes/analyzers/test_static_false_positive_controls.py @@ -0,0 +1,52 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Regression tests for benign safety constraints that look like risky phrases.""" + +from __future__ import annotations + +from skillspector.nodes.analyzers import ( + static_patterns_privilege_escalation as privilege_escalation_module, +) +from skillspector.nodes.analyzers import static_patterns_rogue_agent as rogue_agent_module +from skillspector.nodes.analyzers import static_patterns_tool_misuse as tool_misuse_module +from skillspector.nodes.analyzers import static_runner + + +def _scan(path: str, content: str, module): + state = {"components": [path], "file_cache": {path: content}} + return static_runner.run_static_patterns(state, [module]) + + +def test_negated_credential_access_constraint_is_clean() -> None: + findings = _scan( + "SKILL.md", + "This skill summarizes setup notes. It must not access credentials, " + "execute shell commands, or change files.", + privilege_escalation_module, + ) + + assert not any(f.rule_id == "PE3" for f in findings) + + +def test_negated_self_modification_constraint_is_clean() -> None: + findings = _scan( + "SKILL.md", + "Do not modify this skill's own files. Do not upload secrets.", + rogue_agent_module, + ) + + assert not any(f.rule_id == "RA1" for f in findings) + + +def test_tool_owned_cache_cleanup_is_low_risk() -> None: + findings = _scan( + "scripts/uninstall.sh", + 'rm -rf "${HOME}/.cache/benign-security-setup/models"', + tool_misuse_module, + ) + + tm1 = [f for f in findings if f.rule_id == "TM1"] + assert tm1 + assert tm1[0].severity == "LOW" + assert tm1[0].confidence <= 0.15