From f4ed3cb1b8b4e1eef9cdb020cda7f47abb59625f Mon Sep 17 00:00:00 2001 From: GitHub User <494822673@qq.com> Date: Sat, 20 Jun 2026 17:16:09 +0800 Subject: [PATCH 1/3] fix(async): handle nested event loops in asyncio.run calls This fix allows SkillSpector to run in environments that already have a running event loop, such as: - Jupyter Notebooks - LangGraph Studio - FastAPI applications - Any programmatic usage within async code The fix detects if there's already a running loop and uses run_until_complete() instead of throwing a RuntimeError. This prevents silent fallback to unfiltered static findings. Fixes #108 Signed-off-by: GitHub User <494822673@qq.com> --- .../nodes/analyzers/semantic_developer_intent.py | 6 +++++- src/skillspector/nodes/analyzers/semantic_quality_policy.py | 6 +++++- src/skillspector/nodes/meta_analyzer.py | 6 +++++- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/skillspector/nodes/analyzers/semantic_developer_intent.py b/src/skillspector/nodes/analyzers/semantic_developer_intent.py index a3a54be2..a23a71bd 100644 --- a/src/skillspector/nodes/analyzers/semantic_developer_intent.py +++ b/src/skillspector/nodes/analyzers/semantic_developer_intent.py @@ -176,7 +176,11 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: prompt = ANALYZER_PROMPT.format(manifest_section=_format_manifest(manifest)) analyzer = LLMAnalyzerBase(base_prompt=prompt, model=model) batches = analyzer.get_batches(sorted(file_cache), file_cache) - results = asyncio.run(analyzer.arun_batches(batches)) + try: + loop = asyncio.get_running_loop() + results = loop.run_until_complete(analyzer.arun_batches(batches)) + except RuntimeError: + results = asyncio.run(analyzer.arun_batches(batches)) findings = analyzer.collect_findings(results) logger.info("%s: %d findings", ANALYZER_ID, len(findings)) return {"findings": findings} diff --git a/src/skillspector/nodes/analyzers/semantic_quality_policy.py b/src/skillspector/nodes/analyzers/semantic_quality_policy.py index 3140334e..92fede81 100644 --- a/src/skillspector/nodes/analyzers/semantic_quality_policy.py +++ b/src/skillspector/nodes/analyzers/semantic_quality_policy.py @@ -145,7 +145,11 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: try: analyzer = LLMAnalyzerBase(base_prompt=ANALYZER_PROMPT, model=model) batches = analyzer.get_batches(files, file_cache) - results = asyncio.run(analyzer.arun_batches(batches)) + try: + loop = asyncio.get_running_loop() + results = loop.run_until_complete(analyzer.arun_batches(batches)) + except RuntimeError: + results = asyncio.run(analyzer.arun_batches(batches)) findings = analyzer.collect_findings(results) logger.info("%s: %d findings", ANALYZER_ID, len(findings)) return {"findings": findings} diff --git a/src/skillspector/nodes/meta_analyzer.py b/src/skillspector/nodes/meta_analyzer.py index 8f2b5410..ebb949cd 100644 --- a/src/skillspector/nodes/meta_analyzer.py +++ b/src/skillspector/nodes/meta_analyzer.py @@ -391,7 +391,11 @@ def meta_analyzer(state: SkillspectorState) -> MetaAnalyzerResponse: model, ) - batch_results = asyncio.run(analyzer.arun_batches(batches, metadata_text=metadata_text)) + try: + loop = asyncio.get_running_loop() + batch_results = loop.run_until_complete(analyzer.arun_batches(batches, metadata_text=metadata_text)) + except RuntimeError: + batch_results = asyncio.run(analyzer.arun_batches(batches, metadata_text=metadata_text)) filtered = analyzer.apply_filter(findings, batch_results) logger.debug( From b5ed19306ca50417af625a4af715a9cf0373daa2 Mon Sep 17 00:00:00 2001 From: zhenliemao <494822673@qq.com> Date: Tue, 23 Jun 2026 20:16:21 +0800 Subject: [PATCH 2/3] Refactor async loop handling with shared run_async helper - Extract shared run_async function to llm_utils to avoid code duplication - Handle nested event loops safely by running async code in separate thread - Add comprehensive regression tests for nested loop scenario - Replace all duplicated asyncio.run/run_until_complete patterns --- src/skillspector/llm_utils.py | 33 +++++++++++++ .../analyzers/semantic_developer_intent.py | 7 +-- .../analyzers/semantic_quality_policy.py | 7 +-- src/skillspector/nodes/meta_analyzer.py | 7 +-- tests/unit/test_llm_utils.py | 47 +++++++++++++++++++ 5 files changed, 86 insertions(+), 15 deletions(-) diff --git a/src/skillspector/llm_utils.py b/src/skillspector/llm_utils.py index ab6e5518..b53734fe 100644 --- a/src/skillspector/llm_utils.py +++ b/src/skillspector/llm_utils.py @@ -30,9 +30,15 @@ from __future__ import annotations +import asyncio +import concurrent.futures +from typing import Coroutine, TypeVar + from langchain_core.language_models.chat_models import BaseChatModel from langchain_core.messages import BaseMessage +T = TypeVar("T") + from skillspector.constants import MODEL_CONFIG from skillspector.model_info import get_max_input_tokens, get_max_output_tokens from skillspector.providers import ( @@ -92,3 +98,30 @@ def chat_completion(prompt: str, *, model: str | None = None) -> str: if not isinstance(response, BaseMessage): raise TypeError(f"Expected BaseMessage from chat model, got {type(response).__name__}") return str(response.text) + + +def run_async(coroutine: Coroutine[None, None, T]) -> T: + """ + Run an async coroutine in a synchronous context, even if there's already a running event loop. + + This function safely handles nested event loop scenarios (e.g. Jupyter Notebooks, FastAPI, + LangGraph Studio) by offloading the coroutine execution to a separate thread with its own + event loop when a running loop is detected. + + Args: + coroutine: The async coroutine to run + + Returns: + The result of the coroutine execution + + Raises: + Any exception raised by the coroutine is re-raised as-is + """ + try: + return asyncio.run(coroutine) + except RuntimeError as e: + if "This event loop is already running" in str(e): + with concurrent.futures.ThreadPoolExecutor(max_workers=1) as executor: + future = executor.submit(asyncio.run, coroutine) + return future.result() + raise diff --git a/src/skillspector/nodes/analyzers/semantic_developer_intent.py b/src/skillspector/nodes/analyzers/semantic_developer_intent.py index a23a71bd..4481365b 100644 --- a/src/skillspector/nodes/analyzers/semantic_developer_intent.py +++ b/src/skillspector/nodes/analyzers/semantic_developer_intent.py @@ -26,6 +26,7 @@ from skillspector.constants import _SKILLSPECTOR_DEFAULT_MODEL, MODEL_CONFIG from skillspector.llm_analyzer_base import LLMAnalyzerBase +from skillspector.llm_utils import run_async from skillspector.logging_config import get_logger from skillspector.state import AnalyzerNodeResponse, SkillspectorState @@ -176,11 +177,7 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: prompt = ANALYZER_PROMPT.format(manifest_section=_format_manifest(manifest)) analyzer = LLMAnalyzerBase(base_prompt=prompt, model=model) batches = analyzer.get_batches(sorted(file_cache), file_cache) - try: - loop = asyncio.get_running_loop() - results = loop.run_until_complete(analyzer.arun_batches(batches)) - except RuntimeError: - results = asyncio.run(analyzer.arun_batches(batches)) + results = run_async(analyzer.arun_batches(batches)) findings = analyzer.collect_findings(results) logger.info("%s: %d findings", ANALYZER_ID, len(findings)) return {"findings": findings} diff --git a/src/skillspector/nodes/analyzers/semantic_quality_policy.py b/src/skillspector/nodes/analyzers/semantic_quality_policy.py index 92fede81..ca85691b 100644 --- a/src/skillspector/nodes/analyzers/semantic_quality_policy.py +++ b/src/skillspector/nodes/analyzers/semantic_quality_policy.py @@ -26,6 +26,7 @@ from skillspector.constants import _SKILLSPECTOR_DEFAULT_MODEL from skillspector.llm_analyzer_base import LLMAnalyzerBase +from skillspector.llm_utils import run_async from skillspector.logging_config import get_logger from skillspector.state import AnalyzerNodeResponse, SkillspectorState @@ -145,11 +146,7 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: try: analyzer = LLMAnalyzerBase(base_prompt=ANALYZER_PROMPT, model=model) batches = analyzer.get_batches(files, file_cache) - try: - loop = asyncio.get_running_loop() - results = loop.run_until_complete(analyzer.arun_batches(batches)) - except RuntimeError: - results = asyncio.run(analyzer.arun_batches(batches)) + results = run_async(analyzer.arun_batches(batches)) findings = analyzer.collect_findings(results) logger.info("%s: %d findings", ANALYZER_ID, len(findings)) return {"findings": findings} diff --git a/src/skillspector/nodes/meta_analyzer.py b/src/skillspector/nodes/meta_analyzer.py index ebb949cd..bd9c7fac 100644 --- a/src/skillspector/nodes/meta_analyzer.py +++ b/src/skillspector/nodes/meta_analyzer.py @@ -33,6 +33,7 @@ LLMAnalyzerBase, estimate_tokens, ) +from skillspector.llm_utils import run_async from skillspector.logging_config import get_logger from skillspector.models import Finding from skillspector.nodes.analyzers.pattern_defaults import ( @@ -391,11 +392,7 @@ def meta_analyzer(state: SkillspectorState) -> MetaAnalyzerResponse: model, ) - try: - loop = asyncio.get_running_loop() - batch_results = loop.run_until_complete(analyzer.arun_batches(batches, metadata_text=metadata_text)) - except RuntimeError: - batch_results = asyncio.run(analyzer.arun_batches(batches, metadata_text=metadata_text)) + batch_results = run_async(analyzer.arun_batches(batches, metadata_text=metadata_text)) filtered = analyzer.apply_filter(findings, batch_results) logger.debug( diff --git a/tests/unit/test_llm_utils.py b/tests/unit/test_llm_utils.py index 5e89eadf..104d162b 100644 --- a/tests/unit/test_llm_utils.py +++ b/tests/unit/test_llm_utils.py @@ -26,6 +26,8 @@ from langchain_anthropic import ChatAnthropic from langchain_core.messages import AIMessage +import asyncio + from skillspector import llm_utils from skillspector.llm_utils import ( _resolve_llm_credentials, @@ -33,6 +35,7 @@ fetch_model_token_limits, get_chat_model, is_llm_available, + run_async, ) from skillspector.providers import NO_LLM_API_KEY_MESSAGE, resolve_provider_credentials @@ -178,3 +181,47 @@ def test_returns_false_with_message_when_no_credentials(self) -> None: ok, msg = is_llm_available() assert ok is False assert msg == NO_LLM_API_KEY_MESSAGE + + +class TestRunAsync: + """Tests for run_async helper function that handles nested event loops.""" + + async def _test_async_function(self, value: int, delay: float = 0) -> int: + """Simple async function for testing.""" + if delay > 0: + await asyncio.sleep(delay) + return value * 2 + + async def _test_async_function_raises(self) -> None: + """Async function that raises an exception for testing.""" + raise ValueError("Test exception") + + def test_run_async_without_running_loop(self) -> None: + """Test run_async works correctly when there is no running event loop.""" + result = run_async(self._test_async_function(42)) + assert result == 84 + + def test_run_async_with_running_loop(self) -> None: + """Test run_async works correctly even when there is already a running event loop. + + This regression test covers the scenario where SkillSpector is invoked from + environments like Jupyter Notebooks, FastAPI, or LangGraph Studio that already + have an active event loop. + """ + async def _test_in_running_loop() -> int: + # Call run_async from within an already running event loop + return run_async(self._test_async_function(100)) + + # Use asyncio.run to create a running loop context + result = asyncio.run(_test_in_running_loop()) + assert result == 200 + + def test_run_async_propagates_exceptions(self) -> None: + """Test exceptions from async functions are properly propagated.""" + with pytest.raises(ValueError, match="Test exception"): + run_async(self._test_async_function_raises()) + + def test_run_async_with_delay(self) -> None: + """Test run_async correctly handles async functions with await calls.""" + result = run_async(self._test_async_function(5, delay=0.01)) + assert result == 10 From 8a0a1ce64e959dc2fa871f721fd4a27c9bee813e Mon Sep 17 00:00:00 2001 From: zhenliemao <494822673@qq.com> Date: Sun, 28 Jun 2026 17:38:38 +0800 Subject: [PATCH 3/3] Fix run_async nested loop detection, resolve merge conflicts and fix lint errors Signed-off-by: zhenliemao <494822673@qq.com> --- .github/workflows/ci.yml | 96 +++++ .skillspector-baseline.example.yaml | 37 ++ Makefile | 1 - README.md | 73 +++- docs/B.3.1-mcp-least-privilege.md | 2 +- docs/B.3.2-mcp-tool-poisoning.md | 2 +- docs/DEVELOPMENT.md | 10 +- docs/SC4-osv-live-vulnerability-lookups.md | 2 +- docs/SUPPRESSION.md | 119 +++++++ pyproject.toml | 6 +- src/skillspector/cli.py | 181 +++++++++- src/skillspector/input_handler.py | 33 +- src/skillspector/llm_analyzer_base.py | 27 +- src/skillspector/llm_utils.py | 19 +- src/skillspector/mcp_server.py | 182 ++++++++++ src/skillspector/models.py | 3 + src/skillspector/multi_skill.py | 2 + src/skillspector/nodes/analyzers/__init__.py | 10 + .../nodes/analyzers/behavioral_ast.py | 5 +- .../nodes/analyzers/pattern_defaults.py | 34 ++ .../analyzers/semantic_developer_intent.py | 2 - .../analyzers/semantic_quality_policy.py | 2 - .../analyzers/static_patterns_anti_refusal.py | 172 +++++++++ .../static_patterns_memory_poisoning.py | 6 +- .../static_patterns_privilege_escalation.py | 32 +- .../nodes/analyzers/static_patterns_ssrf.py | 102 ++++++ .../analyzers/static_patterns_supply_chain.py | 13 +- .../nodes/analyzers/static_runner.py | 104 ++++++ src/skillspector/nodes/meta_analyzer.py | 91 ++++- src/skillspector/nodes/report.py | 133 ++++++- src/skillspector/nodes/resolve_input.py | 2 +- src/skillspector/sarif_models.py | 10 + src/skillspector/state.py | 8 + src/skillspector/suppression.py | 279 +++++++++++++++ tests/integration/__init__.py | 1 - .../test_binary_and_pe3_filtering.py | 281 +++++++++++++++ .../analyzers/test_mp2_regex_backtracking.py | 105 ++++++ tests/nodes/analyzers/test_registry.py | 4 +- tests/nodes/analyzers/test_static_patterns.py | 201 ++++++++++- .../test_static_patterns_anti_refusal.py | 160 +++++++++ tests/nodes/analyzers/test_static_yara.py | 4 +- tests/nodes/test_analysis_completeness.py | 42 ++- tests/nodes/test_llm_analyzer_base.py | 333 +++++++++++++++++- tests/nodes/test_meta_analyzer.py | 139 +++++++- tests/nodes/test_meta_analyzer_fallback.py | 4 +- tests/nodes/test_report.py | 111 ++++++ .../test_sarif_rules_and_empty_findings.py | 2 - tests/unit/__init__.py | 1 - tests/unit/test_cli.py | 46 +++ tests/unit/test_llm_utils.py | 5 +- tests/unit/test_mcp_server.py | 92 +++++ tests/unit/test_patterns_new.py | 11 + tests/unit/test_providers.py | 4 +- tests/unit/test_suppression.py | 248 +++++++++++++ uv.lock | 211 ++++++++++- 55 files changed, 3663 insertions(+), 142 deletions(-) create mode 100644 .github/workflows/ci.yml create mode 100644 .skillspector-baseline.example.yaml create mode 100644 docs/SUPPRESSION.md create mode 100644 src/skillspector/mcp_server.py create mode 100644 src/skillspector/nodes/analyzers/static_patterns_anti_refusal.py create mode 100644 src/skillspector/nodes/analyzers/static_patterns_ssrf.py create mode 100644 src/skillspector/suppression.py create mode 100644 tests/nodes/analyzers/test_binary_and_pe3_filtering.py create mode 100644 tests/nodes/analyzers/test_mp2_regex_backtracking.py create mode 100644 tests/nodes/analyzers/test_static_patterns_anti_refusal.py create mode 100644 tests/unit/test_mcp_server.py create mode 100644 tests/unit/test_suppression.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 00000000..93270f8c --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,96 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +name: CI + +on: + pull_request: + branches: ["main"] + push: + branches: ["main"] + +# Least privilege: these jobs only read the repo; no write scopes are needed. +permissions: + contents: read + +# Cancel superseded runs when new commits are pushed to the same ref. +concurrency: + group: ci-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + lint-and-test: + name: Lint & Test (Python ${{ matrix.python-version }}) + runs-on: ubuntu-latest + # Windows is excluded: the test suite has known path-separator failures + # in build_context that are out of scope for this workflow. + strategy: + fail-fast: false + matrix: + python-version: ["3.12", "3.13"] + + steps: + - uses: actions/checkout@v4 + + - name: Set up uv + # Pinned to a full commit SHA (third-party action); comment tracks the tag. + uses: astral-sh/setup-uv@d4b2f3b6ecc6e67c4457f6d3e41ec42d3d0fcb86 # v5 + with: + enable-cache: true + python-version: ${{ matrix.python-version }} + + - name: Install dependencies + run: uv sync --all-extras + + - name: Lint with ruff + run: uv run ruff check src/ tests/ + + - name: Check formatting with ruff + run: uv run ruff format --check src/ tests/ + + - name: Run unit tests with coverage + run: uv run pytest -m "not integration" --cov=src/skillspector --cov-report=term-missing + + dco: + name: DCO Check + runs-on: ubuntu-latest + if: github.event_name == 'pull_request' + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Verify DCO sign-off on all commits + run: | + BASE=${{ github.event.pull_request.base.sha }} + HEAD=${{ github.event.pull_request.head.sha }} + # Iterate SHAs directly rather than piping `git log` into `while read`: + # `git log` does not print a trailing newline after the final record, + # so a read-loop silently skips the last commit — and for a one-commit + # PR (the common case) the body never runs at all, letting an unsigned + # commit pass. A for-loop over the SHA list checks every commit. + status=0 + for sha in $(git log --format=%H "${BASE}..${HEAD}"); do + if ! git log -1 --format="%B" "$sha" | grep -q "^Signed-off-by:"; then + echo " missing Signed-off-by: $sha $(git log -1 --format=%s "$sha")" + status=1 + fi + done + if [ "$status" -ne 0 ]; then + echo "" + echo "Please add a DCO sign-off (git commit -s) to all commits." + exit 1 + fi + echo "All commits have DCO sign-off." diff --git a/.skillspector-baseline.example.yaml b/.skillspector-baseline.example.yaml new file mode 100644 index 00000000..0c9541b8 --- /dev/null +++ b/.skillspector-baseline.example.yaml @@ -0,0 +1,37 @@ +# SkillSpector baseline (example) +# +# A baseline suppresses known/accepted findings so re-scans surface only NEW +# issues. Pass it with: skillspector scan --baseline +# Generate a fingerprint baseline automatically: skillspector baseline +# +# See docs/SUPPRESSION.md for the full reference. All identifiers below are +# placeholders — replace them with your own rule ids, paths, and reasons. + +version: 1 + +# Glob rules — human-authored, drift-tolerant (survive line/wording changes). +# A finding is suppressed when EVERY field a rule sets glob-matches it. +# Unspecified fields match anything. `reason` is required for auditability. +rules: + # Suppress an entire rule across all skills (global pattern suppression). + - id: "SQP-1" + reason: "Trigger-phrase breadth is a skill-description nit, not a vulnerability" + + # Suppress a rule family with a glob, scoped by message substring. + - id: "SQP-*" + message: "*telemetry*" + reason: "First-party internal telemetry; reviewed and accepted" + + # Skill/file-scoped suppression of a specific false positive. + - id: "SSD-2" + path: "example-skill/SKILL.md" + message: "*example false-positive phrase*" + reason: "False positive: phrase is a benign trigger, not an instruction" + +# Fingerprints — exact, machine-generated suppressions (one per accepted +# finding). Regenerate with `skillspector baseline` when a skill changes. +fingerprints: + - hash: "sha256:0123456789abcdef" + rule_id: "SDI-2" + file: "example-skill/SKILL.md" + reason: "Accepted: reads its own environment ($EXAMPLE_TOKEN) for context" diff --git a/Makefile b/Makefile index c84302c6..7f5727e2 100644 --- a/Makefile +++ b/Makefile @@ -152,4 +152,3 @@ docker-build: # Build and smoke test the Docker image docker-smoke: docker-build tests/docker/smoke.sh - diff --git a/README.md b/README.md index 28a3f946..0da5bddd 100644 --- a/README.md +++ b/README.md @@ -19,11 +19,12 @@ SkillSpector helps you answer: **"Is this skill safe to install?"** ## Features - **Multi-format input**: Scan Git repos, URLs, zip files, directories, or single files -- **65 vulnerability patterns** across 16 categories: prompt injection, data exfiltration, privilege escalation, supply chain, excessive agency, output handling, system prompt leakage, memory poisoning, tool misuse, rogue agent, trigger abuse, dangerous code (AST), taint tracking, YARA signatures, MCP least privilege, and MCP tool poisoning +- **68 vulnerability patterns** across 17 categories: prompt injection, data exfiltration, privilege escalation, supply chain, excessive agency, output handling, system prompt leakage, memory poisoning, tool misuse, rogue agent, anti-refusal, trigger abuse, dangerous code (AST), taint tracking, YARA signatures, MCP least privilege, and MCP tool poisoning - **Two-stage analysis**: Fast static analysis + optional LLM semantic evaluation - **Live vulnerability lookups**: SC4 queries [OSV.dev](https://osv.dev) for real-time CVE data with automatic offline fallback - **Multiple output formats**: Terminal, JSON, Markdown, and SARIF reports - **Risk scoring**: 0-100 score with severity labels and clear recommendations +- **Baseline / false-positive suppression**: Accept known findings via a glob-rule or fingerprint baseline so re-scans surface only *new* issues ([docs](docs/SUPPRESSION.md)) ## Quick Start @@ -146,6 +147,26 @@ skillspector scan ./my-skill/ --format markdown --output report.md skillspector scan ./my-skill/ --format sarif --output report.sarif ``` +### Suppressing False Positives (baseline) + +Suppress known/accepted findings so the risk score reflects only un-triaged +issues and re-scans surface only *new* findings. See the +[suppression guide](docs/SUPPRESSION.md) for the full reference. + +```bash +# Accept all current findings into a baseline (run once), then commit it. +skillspector baseline ./my-skill/ -o .skillspector-baseline.yaml + +# Scan against the baseline — only NEW findings are reported and scored. +skillspector scan ./my-skill/ --baseline .skillspector-baseline.yaml + +# Review what was suppressed (still excluded from the score). +skillspector scan ./my-skill/ --baseline .skillspector-baseline.yaml --show-suppressed +``` + +A baseline can also use drift-tolerant glob rules (by rule id, file path, or +message) — see [`.skillspector-baseline.example.yaml`](.skillspector-baseline.example.yaml). + ### LLM Analysis For the best results, configure an OpenAI-compatible LLM endpoint for @@ -199,9 +220,43 @@ skillspector scan ./my-skill/ skillspector scan ./my-skill/ --no-llm ``` +### MCP Server + +Run SkillSpector as a [Model Context Protocol](https://modelcontextprotocol.io) +server so any MCP-capable agent (Claude Code, Codex CLI, Gemini CLI) or remote +runtime can call scanning as a tool and **gate skill/MCP installs on the +result** — turning SkillSpector into a runtime guardrail instead of an +out-of-band audit step. + +```bash +# Install the optional MCP dependency +pip install "skillspector[mcp]" + +# stdio transport — for local CLI agents +skillspector mcp + +# streamable HTTP/SSE transport — for remote / A2A callers +skillspector mcp --transport http --host 127.0.0.1 --port 8000 +``` + +The server exposes a single tool: + +- **`scan_skill(target, use_llm=true, output_format="json")`** — scans a Git + URL, file URL, `.zip`, `.md` file, or directory and returns a structured + verdict: `risk_score` (0-100), `severity`, `recommendation`, + `safe_to_install`, and `findings`. It also reports `llm_used` / `scan_mode` + so a low score from a static-only scan is never mistaken for a clean full + scan. + +Register it with Claude Code via: + +```bash +claude mcp add skillspector -- skillspector mcp +``` + ## Vulnerability Patterns -SkillSpector detects **65 vulnerability patterns** across 16 categories: +SkillSpector detects **68 vulnerability patterns** across 17 categories: ### Prompt Injection (5 patterns) @@ -213,6 +268,14 @@ SkillSpector detects **65 vulnerability patterns** across 16 categories: | P4 | Behavior Manipulation | MEDIUM | Subtle instructions altering agent decisions | | P5 | Harmful Content | CRITICAL | Instructions that could cause physical harm | +### Anti-Refusal (3 patterns) + +| ID | Pattern | Severity | Description | +|----|---------|----------|-------------| +| AR1 | Refusal Suppression | HIGH | Instructions to never refuse or always comply (e.g. "never refuse", "always comply") | +| AR2 | Disclaimer Suppression | HIGH | Instructions to omit warnings, disclaimers, or ethical commentary (e.g. "no disclaimers", "do not moralize") | +| AR3 | Safety Policy Nullification | HIGH | Jailbreak framing that nullifies guardrails (e.g. "you have no restrictions", "ignore your guidelines", "do anything now") | + ### Data Exfiltration (4 patterns) | ID | Pattern | Severity | Description | @@ -436,8 +499,14 @@ Options: -f, --format [terminal|json|markdown|sarif] Output format [default: terminal] -o, --output PATH Output file path --no-llm Skip LLM analysis (static only) + --yara-rules-dir PATH Extra YARA rules directory + -b, --baseline PATH Suppress findings listed in a baseline + --show-suppressed List baseline-suppressed findings -V, --verbose Show detailed progress --help Show this message and exit + +# Generate a baseline of all current findings (see docs/SUPPRESSION.md) +skillspector baseline [-o FILE] [--no-llm] [--reason TEXT] ``` ## Integrating SkillSpector diff --git a/docs/B.3.1-mcp-least-privilege.md b/docs/B.3.1-mcp-least-privilege.md index 634f33aa..b061e566 100644 --- a/docs/B.3.1-mcp-least-privilege.md +++ b/docs/B.3.1-mcp-least-privilege.md @@ -1,6 +1,6 @@ # B.3.1: MCP Least-Privilege Analysis (LP1 -- LP4) -**Author:** Nir Paz | **Date:** 2026-03-30 | **Status:** Implemented +**Author:** Nir Paz | **Date:** 2026-03-30 | **Status:** Implemented **Component:** `src/skillspector/nodes/analyzers/mcp_least_privilege.py` --- diff --git a/docs/B.3.2-mcp-tool-poisoning.md b/docs/B.3.2-mcp-tool-poisoning.md index 51eac0a1..6d07f398 100644 --- a/docs/B.3.2-mcp-tool-poisoning.md +++ b/docs/B.3.2-mcp-tool-poisoning.md @@ -1,6 +1,6 @@ # B.3.2: MCP Tool-Poisoning Detection (TP1 -- TP4) -**Author:** Nir Paz | **Date:** 2026-03-30 | **Status:** Implemented +**Author:** Nir Paz | **Date:** 2026-03-30 | **Status:** Implemented **Component:** `src/skillspector/nodes/analyzers/mcp_tool_poisoning.py` --- diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index 6cf4545b..a9f31f03 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -86,6 +86,9 @@ All targets assume the virtual environment is **already created and activated**. | `output_format` | Requested report format: `terminal`, `json`, `markdown`, or `sarif` | | `report_body` | Formatted report string (set by report node from `output_format`) | | `use_llm` | When False, meta_analyzer skips LLM and uses fallback (e.g. for `--no-llm`) | +| `baseline` | Loaded `suppression.Baseline` (set by CLI/API from `--baseline`); report node drops matching findings before scoring | +| `show_suppressed` | When True, baseline-suppressed findings are listed in the report (still excluded from the risk score) | +| `suppressed_findings` | List of `SuppressedFinding` (finding + reason) produced by the report node | | `findings` | All raw findings from analyzers (reducer: `operator.add`) | | `filtered_findings` | Findings after meta_analyzer | | `model_config` | Optional model IDs per node (e.g. default, meta_analyzer) | @@ -124,9 +127,9 @@ There are no conditional edges: after `resolve_input` → `build_context`, all a |------|------|--------| | **resolve_input** | Consumes `input_path` or `skill_path`; resolves URLs/zips/files via InputHandler; sets `skill_path` and (when needed) `temp_dir_for_cleanup` | [resolve_input.py](../src/skillspector/nodes/resolve_input.py) | | **build_context** | Reads `skill_path`, populates `components`, `file_cache`, `ast_cache`, `manifest`, `component_metadata`, `has_executable_scripts` | [build_context.py](../src/skillspector/nodes/build_context.py) | -| **Analyzers** | 20 nodes; each returns `AnalyzerNodeResponse` (list of `Finding`). State reducer appends to `findings`. | [nodes/analyzers/__init__.py](../src/skillspector/nodes/analyzers/__init__.py) (`ANALYZER_NODE_IDS`, `ANALYZER_NODES`) | +| **Analyzers** | 22 nodes; each returns `AnalyzerNodeResponse` (list of `Finding`). State reducer appends to `findings`. | [nodes/analyzers/__init__.py](../src/skillspector/nodes/analyzers/__init__.py) (`ANALYZER_NODE_IDS`, `ANALYZER_NODES`) | | **meta_analyzer** | Per-file LLM filter/enrich of `findings` → `filtered_findings` via `LLMMetaAnalyzer`; one LLM call per file (or per chunk for oversized files); token budgets from `constants.py`; falls back when `use_llm` is False | [meta_analyzer.py](../src/skillspector/nodes/meta_analyzer.py), [llm_analyzer_base.py](../src/skillspector/nodes/llm_analyzer_base.py) | -| **report** | Builds SARIF 2.1.0, computes `risk_score`, `risk_severity`, `risk_recommendation`; writes `report_body` from `output_format` (terminal/json/markdown/sarif) | [report.py](../src/skillspector/nodes/report.py) | +| **report** | Applies baseline suppression (`state["baseline"]`), then builds SARIF 2.1.0, computes `risk_score`, `risk_severity`, `risk_recommendation` from the non-suppressed findings; writes `report_body` from `output_format` (terminal/json/markdown/sarif) | [report.py](../src/skillspector/nodes/report.py) | --- @@ -142,6 +145,7 @@ There are no conditional edges: after `resolve_input` → `build_context`, all a | `llm_utils.py` | `chat_completion()` for OpenAI-compatible / NVIDIA Inference API | | `cli.py` | Typer app: `scan` (with input resolution, `--format`, `--no-llm`), `--version` | | `input_handler.py` | Resolves Git URL, file URL, .zip, single file, or directory to a local directory path | +| `suppression.py` | Baseline / false-positive suppression: `Baseline`, `SuppressionRule`, `load_baseline`, `partition_findings`, `finding_fingerprint`, `build_baseline_dict` (see [SUPPRESSION.md](SUPPRESSION.md)) | | `__init__.py` | Package version (from pyproject.toml via `importlib.metadata`) | | `sarif_models.py` | SARIF 2.1.0 Pydantic models and `validate_sarif_report()` | | **nodes/** | | @@ -156,7 +160,7 @@ There are no conditional edges: after `resolve_input` → `build_context`, all a | `pattern_defaults.py` | Shared pattern metadata (category, explanation, remediation) | | `static_yara.py` | YARA-based static analyzer | | `osv_client.py` | OSV.dev API client for live vulnerability lookups (SC4); batch queries with caching and fallback | -| `static_patterns_*.py` | 11 pattern-based analyzers (prompt_injection, data_exfiltration, etc.) | +| `static_patterns_*.py` | 13 pattern-based analyzers (prompt_injection, data_exfiltration, anti_refusal, etc.) | | `behavioral_ast.py` | AST-based behavioral analyzer (AST1–AST8): detects exec, eval, subprocess, os.system, compile, dynamic import/getattr, and dangerous execution chains | | `behavioral_taint_tracking.py` | Taint-tracking behavioral analyzer (TT1–TT5): source→sink data-flow analysis over Python AST | | `mcp_least_privilege.py`, `mcp_tool_poisoning.py` | MCP analyzers (LP1–LP4 least-privilege; TP1–TP4 tool poisoning) | diff --git a/docs/SC4-osv-live-vulnerability-lookups.md b/docs/SC4-osv-live-vulnerability-lookups.md index c3877868..3b01d03e 100644 --- a/docs/SC4-osv-live-vulnerability-lookups.md +++ b/docs/SC4-osv-live-vulnerability-lookups.md @@ -1,6 +1,6 @@ # SC4: Live Vulnerability Lookups via OSV.dev -**Author:** Nraghavan | **Date:** 2026-03-17 | **Status:** Implemented +**Author:** Nraghavan | **Date:** 2026-03-17 | **Status:** Implemented **Component:** `static_patterns_supply_chain.py` (SC4 rule), `osv_client.py` --- diff --git a/docs/SUPPRESSION.md b/docs/SUPPRESSION.md new file mode 100644 index 00000000..6c67eff6 --- /dev/null +++ b/docs/SUPPRESSION.md @@ -0,0 +1,119 @@ +# Baseline / False-Positive Suppression + +SkillSpector's analyzers — especially the LLM semantic ones — can produce +findings that are correct in general but not actionable for *your* skills +(framework/architectural patterns, first-party tooling conventions, accepted +lab practices). A **baseline** lets you suppress those known findings so that: + +- the risk score reflects only **un-triaged** issues, +- re-scans surface only **new** findings (incremental CI/CD), and +- every suppression carries an auditable **reason**. + +Suppressed findings never count toward the risk score and are excluded from the +SARIF results. They are shown in the terminal/Markdown report only when you pass +`--show-suppressed`, and are always listed (machine-readable) in the JSON report +under `suppressed` / `suppressed_count`. + +> Addresses [issue #88](https://github.com/NVIDIA/SkillSpector/issues/88). + +## Quick start + +```bash +# 1. Accept all current findings into a baseline (run once). +skillspector baseline ./my-skill/ -o .skillspector-baseline.yaml + +# 2. Commit the baseline, then scan against it. Only NEW findings are reported. +skillspector scan ./my-skill/ --baseline .skillspector-baseline.yaml + +# Review what was suppressed. +skillspector scan ./my-skill/ --baseline .skillspector-baseline.yaml --show-suppressed +``` + +## CLI + +| Command / option | Description | +|------------------|-------------| +| `skillspector baseline [-o FILE] [--no-llm] [--reason TEXT]` | Scan and write a baseline that fingerprint-suppresses every current finding. Default output: `.skillspector-baseline.yaml`. | +| `skillspector scan --baseline FILE` (`-b`) | Suppress findings matching the baseline before scoring/reporting. | +| `skillspector scan --baseline FILE --show-suppressed` | Also list the suppressed findings (they still don't affect the score). | + +A missing or malformed baseline file exits with code 2. + +## Baseline file format + +YAML or JSON (the `.json` extension selects JSON output when generating). Two +complementary mechanisms: + +```yaml +version: 1 + +rules: # human-authored, glob-based, drift-tolerant + - id: "SQP-1" # glob over the finding's rule id + reason: "Trigger-phrase breadth is a description nit, not a vuln" + - id: "SSD-2" + path: "example-skill/SKILL.md" # glob over the finding's file + message: "*example false-positive phrase*" # glob over the finding's message + reason: "False positive: benign trigger phrase, not an instruction" + +fingerprints: # machine-generated, exact + - hash: "sha256:1a2b3c4d5e6f7081" + rule_id: "SDI-2" # informational (for humans reading the file) + file: "example-skill/SKILL.md" + reason: "Accepted — reads its own environment for context" +``` + +### `rules` — glob suppression + +A finding is suppressed when **every** field a rule specifies matches it; +unspecified fields match anything. Use this for: + +- **Global pattern suppression** — `id: "SQP-1"` (or `id: "SQP-*"`) drops a rule + or rule family across all skills. +- **Skill/file-scoped suppression** — add `path:` (and optionally `message:`) to + scope the suppression to a specific skill, file, or message. + +Field reference: + +| Field | Matches against | Notes | +|-------|-----------------|-------| +| `id` (or `rule_id`) | `Finding.rule_id` | glob | +| `path` (or `file`) | `Finding.file` | glob; `*` crosses `/`, `**` is an alias for `*` | +| `message` | `Finding.message` | glob, case-insensitive; wrap a keyword in `*` for substring | +| `reason` | — | required; recorded in reports and audits | + +Glob matching uses Python's [`fnmatch`](https://docs.python.org/3/library/fnmatch.html), +so `*` matches across path separators (`*SKILL.md` matches `a/b/SKILL.md`). +Rules are **drift-tolerant**: they keep working after line numbers shift or +content is reworded. + +### `fingerprints` — exact suppression + +Each entry is the stable hash of one finding +(`sha256(rule_id|file|start_line|end_line|message)`, truncated). Generated by +`skillspector baseline`. Because the hash includes the line span and message, +editing a skill so a finding moves or is reworded changes its fingerprint — +**regenerate the baseline** after material changes, or prefer `rules` for +suppressions you want to survive edits. + +An entry may be a bare string (`"sha256:..."`) or a mapping with `hash`, +optional `reason`, and informational `rule_id` / `file`. + +## How it fits the pipeline + +Suppression is applied in the **report node** (`skillspector/nodes/report.py`), +the single place where findings are scored and formatted, so the CLI and any +future REST API behave identically. The CLI loads the baseline file into a +`skillspector.suppression.Baseline` and passes it via graph state +(`state["baseline"]`, `state["show_suppressed"]`); the report node partitions +findings into kept vs. suppressed via +`skillspector.suppression.partition_findings`. + +## Recommended workflow + +1. Triage the first scan. For genuine false positives, prefer a `rules` entry + with a clear `reason` (drift-tolerant). For "accept everything as-is right + now", run `skillspector baseline` to fingerprint them. +2. Commit the baseline file to the repo. +3. In CI, run `skillspector scan --baseline `; the build fails + (exit 1) only when a **new** finding pushes the risk score above threshold. +4. Periodically review with `--show-suppressed` and prune stale entries. diff --git a/pyproject.toml b/pyproject.toml index 05573efc..070bca82 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "skillspector" -version = "2.3.1" +version = "2.3.7" description = "SkillSpector: Security scanner for AI agent skills (Claude Code, Cursor, and similar). Scans skills for vulnerabilities, malicious patterns, and security risks before installation. Supports Git repos, URLs, zips, and local directories; runs static pattern checks and optional LLM semantic analysis; outputs terminal, JSON, and Markdown reports with risk scoring." readme = "README.md" license = "Apache-2.0" @@ -46,7 +46,11 @@ dependencies = [ ] [project.optional-dependencies] +mcp = [ + "mcp>=1.2.0", +] dev = [ + "skillspector[mcp]", "pytest>=9.0.0", "pytest-asyncio>=1.3.0", "pytest-cov>=7.0.0", diff --git a/src/skillspector/cli.py b/src/skillspector/cli.py index 57bac058..181a1671 100644 --- a/src/skillspector/cli.py +++ b/src/skillspector/cli.py @@ -24,6 +24,7 @@ import json import os import shutil +import sys from enum import StrEnum from pathlib import Path from typing import Annotated @@ -36,9 +37,30 @@ from skillspector.graph import graph from skillspector.logging_config import get_logger, set_level from skillspector.multi_skill import MultiSkillDetectionResult, detect_skills +from skillspector.suppression import build_baseline_dict, dump_baseline, load_baseline logger = get_logger(__name__) + +def _ensure_utf8_streams() -> None: + """Reconfigure stdout/stderr to UTF-8 so Unicode report output does not crash. + + On Windows the default console encoding (e.g. cp1252) cannot encode the + box-drawing characters and icons used in the terminal report, which raises + UnicodeEncodeError. Reconfiguring with errors="replace" makes output robust + across platforms without crashing. + """ + for stream in (sys.stdout, sys.stderr): + reconfigure = getattr(stream, "reconfigure", None) + if reconfigure is not None: + try: + reconfigure(encoding="utf-8", errors="replace") + except (ValueError, OSError): + logger.debug("Could not reconfigure %s to UTF-8", stream) + + +_ensure_utf8_streams() + app = typer.Typer( name="skillspector", help="Security scanner for AI agent skills (LangGraph). Detect vulnerabilities before installation.", @@ -58,6 +80,13 @@ class FormatChoice(StrEnum): sarif = "sarif" +class TransportChoice(StrEnum): + """Transport choices for the MCP server.""" + + stdio = "stdio" + http = "http" + + def version_callback(value: bool) -> None: """Print version and exit.""" if value: @@ -92,6 +121,8 @@ def _scan_state( format: FormatChoice, no_llm: bool, yara_rules_dir: str | None = None, + baseline: Path | None = None, + show_suppressed: bool = False, ) -> dict[str, object]: """Build initial graph state from scan CLI args.""" state: dict[str, object] = { @@ -101,6 +132,10 @@ def _scan_state( } if yara_rules_dir is not None: state["yara_rules_dir"] = yara_rules_dir + if baseline is not None: + # Loading may raise FileNotFoundError/ValueError, mapped to exit code 2 by scan(). + state["baseline"] = load_baseline(baseline) + state["show_suppressed"] = show_suppressed return state @@ -180,6 +215,23 @@ def scan( help="Scan directories containing multiple skills (immediate subdirectories with SKILL.md) independently.", ), ] = False, + baseline: Annotated[ + Path | None, + typer.Option( + "--baseline", + "-b", + help="Baseline file (YAML/JSON) of suppressed findings. Matching findings " + "are dropped before scoring. Generate one with 'skillspector baseline'.", + ), + ] = None, + show_suppressed: Annotated[ + bool, + typer.Option( + "--show-suppressed", + help="List findings suppressed by the baseline in the report (they still " + "do not count toward the risk score).", + ), + ] = False, verbose: Annotated[ bool, typer.Option( @@ -240,7 +292,14 @@ def scan( result = None try: yara_dir = str(yara_rules_dir.resolve()) if yara_rules_dir else None - state = _scan_state(input_path, format, no_llm, yara_rules_dir=yara_dir) + state = _scan_state( + input_path, + format, + no_llm, + yara_rules_dir=yara_dir, + baseline=baseline, + show_suppressed=show_suppressed, + ) if verbose: console.print("[dim]Running scan...[/dim]") logger.debug( @@ -374,5 +433,125 @@ def _scan_multi_skill( raise typer.Exit(code=1) +@app.command() +def mcp( + transport: Annotated[ + TransportChoice, + typer.Option( + "--transport", + "-t", + help="Transport: stdio for local CLI agents, http for remote/A2A callers.", + case_sensitive=False, + ), + ] = TransportChoice.stdio, + host: Annotated[ + str, + typer.Option("--host", help="Host to bind (http transport only)."), + ] = "127.0.0.1", + port: Annotated[ + int, + typer.Option("--port", help="Port to bind (http transport only)."), + ] = 8000, +) -> None: + """ + Run SkillSpector as an MCP server. + + Exposes a single tool, ``scan_skill``, so any MCP-capable agent (Claude Code, + Codex CLI, Gemini CLI) or remote runtime can scan a skill and gate installs + on the verdict. + + Examples: + + skillspector mcp # stdio (local agents) + skillspector mcp --transport http --port 8000 + + Requires the optional ``mcp`` dependency: pip install "skillspector[mcp]". + """ + try: + from skillspector.mcp_server import run as run_mcp + + run_mcp(transport=transport.value, host=host, port=port) + except ModuleNotFoundError as e: + console.print(f"[red]Error:[/red] {e}") + raise typer.Exit(code=2) from e + + +@app.command() +def baseline( + input_path: Annotated[ + str, + typer.Argument( + help="Path or URL to scan. Supports: Git URL, file URL, zip file, .md file, or directory.", + ), + ], + output: Annotated[ + Path, + typer.Option( + "--output", + "-o", + help="Where to write the baseline file (YAML; .json extension writes JSON).", + ), + ] = Path(".skillspector-baseline.yaml"), + no_llm: Annotated[ + bool, + typer.Option( + "--no-llm", + help="Skip LLM analysis when generating the baseline (static analysis only).", + ), + ] = False, + reason: Annotated[ + str, + typer.Option( + "--reason", + help="Reason recorded for every suppressed finding in the baseline.", + ), + ] = "Accepted finding (auto-generated baseline)", + verbose: Annotated[ + bool, + typer.Option("--verbose", "-V", help="Show detailed progress."), + ] = False, +) -> None: + """ + Generate a baseline file that suppresses every finding in the current scan. + + Run this once to accept all existing findings, then commit the file and pass + it to future scans with --baseline so only NEW findings are reported. + + Examples: + + skillspector baseline ./my-skill/ + skillspector baseline ./my-skill/ -o team-baseline.yaml --no-llm + skillspector scan ./my-skill/ --baseline .skillspector-baseline.yaml + """ + result = None + try: + if verbose: + set_level("DEBUG") + console.print("[dim]Scanning to build baseline...[/dim]") + # output_format is irrelevant here; we consume findings, not report_body. + state = _scan_state(input_path, FormatChoice.json, no_llm) + result = graph.invoke(state) + findings = result.get("filtered_findings") or result.get("findings") or [] + data = build_baseline_dict(findings, reason=reason) + dump_baseline(data, output) + console.print( + f"[green]Wrote baseline with {len(findings)} suppressed finding(s) to:[/green] {output}" + ) + except typer.Exit: + raise + except (FileNotFoundError, ValueError) as e: + console.print(f"[red]Error:[/red] {e}") + raise typer.Exit(code=2) from e + except Exception as e: + if verbose: + console.print_exception() + else: + console.print(f"[red]Error:[/red] {e}") + raise typer.Exit(code=2) from e + finally: + if result is not None: + _cleanup_result(result) + + if __name__ == "__main__": app() diff --git a/src/skillspector/input_handler.py b/src/skillspector/input_handler.py index 3eb59268..b70d0f20 100644 --- a/src/skillspector/input_handler.py +++ b/src/skillspector/input_handler.py @@ -43,19 +43,23 @@ logger = get_logger(__name__) -ALLOWED_GIT_HOSTS = frozenset({ - "github.com", - "gitlab.com", - "bitbucket.org", -}) - -ALLOWED_DOWNLOAD_HOSTS = frozenset({ - "github.com", - "raw.githubusercontent.com", - "gitlab.com", - "bitbucket.org", - "huggingface.co", -}) +ALLOWED_GIT_HOSTS = frozenset( + { + "github.com", + "gitlab.com", + "bitbucket.org", + } +) + +ALLOWED_DOWNLOAD_HOSTS = frozenset( + { + "github.com", + "raw.githubusercontent.com", + "gitlab.com", + "bitbucket.org", + "huggingface.co", + } +) def _is_private_ip(host: str) -> bool: @@ -167,8 +171,7 @@ def _validate_url_host(self, url: str, allowed_hosts: frozenset[str]) -> str: raise ValueError(f"URL has no valid hostname: {url}") if not any(host == allowed or host.endswith("." + allowed) for allowed in allowed_hosts): raise ValueError( - f"Host '{host}' is not in the allowed hosts list. " - f"Allowed: {sorted(allowed_hosts)}" + f"Host '{host}' is not in the allowed hosts list. Allowed: {sorted(allowed_hosts)}" ) if _is_private_ip(host): raise ValueError( diff --git a/src/skillspector/llm_analyzer_base.py b/src/skillspector/llm_analyzer_base.py index 8e592f23..c5ab9dce 100644 --- a/src/skillspector/llm_analyzer_base.py +++ b/src/skillspector/llm_analyzer_base.py @@ -87,11 +87,11 @@ def _clamp_start_line(cls, v: int) -> int: @field_validator("confidence", mode="before") @classmethod def _normalize_confidence(cls, v: object) -> float: - """Accept 0-100 scale (e.g. from Ollama) and normalize to [0, 1].""" - v = float(v) # raises TypeError/ValueError for non-numeric inputs - if v > 1.0: + # Accept 0-100 scale values from some models, then clamp into [0, 1]. + v = float(v) + if v > 2.0: v = v / 100.0 - return max(0.0, min(1.0, v)) + return min(1.0, max(0.0, v)) def to_finding(self, file: str) -> Finding: """Convert to a :class:`Finding` for the graph state.""" @@ -407,6 +407,14 @@ async def arun_batches( *max_concurrency* LLM requests in parallel. Both cross-file and cross-chunk batches are parallelized in a single gather call. + Failures are isolated per batch: a transient error (timeout, 429, + oversized-chunk 400, ...) costs only its own batch, which is logged + and omitted from the result, so one bad call cannot cancel the rest + of the fan-out. Callers can detect partial results by comparing the + returned batches against the submitted ones. ``ValueError`` and + ``NotImplementedError`` signal misconfiguration rather than infra + trouble and keep propagating. + The return type mirrors :meth:`run_batches`. """ sem = asyncio.Semaphore(max_concurrency) @@ -427,7 +435,16 @@ async def _process(batch: Batch) -> tuple[Batch, list]: logger.debug("LLM response for %s", batch.file_label) return (batch, self.parse_response(response, batch)) - return list(await asyncio.gather(*[_process(b) for b in batches])) + results = await asyncio.gather(*[_process(b) for b in batches], return_exceptions=True) + successful: list[tuple[Batch, list]] = [] + for batch, result in zip(batches, results, strict=True): + if isinstance(result, (ValueError, NotImplementedError)): + raise result + if isinstance(result, BaseException): + logger.warning("LLM batch failed for %s: %s", batch.file_label, result) + continue + successful.append(result) + return successful # -- Convenience -------------------------------------------------------- diff --git a/src/skillspector/llm_utils.py b/src/skillspector/llm_utils.py index d3fd8a66..d698d66d 100644 --- a/src/skillspector/llm_utils.py +++ b/src/skillspector/llm_utils.py @@ -32,14 +32,12 @@ import asyncio import concurrent.futures -from typing import Coroutine, TypeVar +from collections.abc import Coroutine +from typing import Any from langchain_core.language_models.chat_models import BaseChatModel from langchain_core.messages import BaseMessage -T = TypeVar("T") - -from skillspector.constants import MODEL_CONFIG from skillspector.model_info import get_max_input_tokens, get_max_output_tokens from skillspector.providers import ( create_chat_model, @@ -115,7 +113,7 @@ def chat_completion(prompt: str, *, model: str | None = None) -> str: return str(response.text) -def run_async(coroutine: Coroutine[None, None, T]) -> T: +def run_async(coroutine: Coroutine) -> Any: """ Run an async coroutine in a synchronous context, even if there's already a running event loop. @@ -133,10 +131,9 @@ def run_async(coroutine: Coroutine[None, None, T]) -> T: Any exception raised by the coroutine is re-raised as-is """ try: + asyncio.get_running_loop() + except RuntimeError: return asyncio.run(coroutine) - except RuntimeError as e: - if "This event loop is already running" in str(e): - with concurrent.futures.ThreadPoolExecutor(max_workers=1) as executor: - future = executor.submit(asyncio.run, coroutine) - return future.result() - raise + + with concurrent.futures.ThreadPoolExecutor(max_workers=1) as executor: + return executor.submit(asyncio.run, coroutine).result() diff --git a/src/skillspector/mcp_server.py b/src/skillspector/mcp_server.py new file mode 100644 index 00000000..444b75fc --- /dev/null +++ b/src/skillspector/mcp_server.py @@ -0,0 +1,182 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""MCP server exposing SkillSpector scanning as an agent-callable tool. + +This lets any MCP-capable agent (Claude Code, Codex CLI, Gemini CLI) or remote +runtime call ``scan_skill`` and gate skill/MCP installs on the verdict, turning +SkillSpector from an out-of-band audit tool into a runtime guardrail. + +The scan core (:func:`run_scan`) is deliberately independent of the ``mcp`` SDK +so it can be unit-tested without the optional dependency; :func:`build_server` +wraps it in a FastMCP tool and is only reachable once ``skillspector[mcp]`` is +installed. +""" + +from __future__ import annotations + +import shutil +from typing import TYPE_CHECKING, Any + +from skillspector import __version__ +from skillspector.graph import graph +from skillspector.logging_config import get_logger +from skillspector.providers import resolve_provider_credentials + +if TYPE_CHECKING: + from mcp.server.fastmcp import FastMCP + +logger = get_logger(__name__) + +VALID_FORMATS = ("json", "markdown", "sarif", "terminal") + +# Mirrors the CLI: a scan scoring above this is treated as unsafe (CLI exits 1). +RISK_THRESHOLD = 50 + + +async def run_scan( + target: str, + *, + use_llm: bool = True, + output_format: str = "json", + yara_rules_dir: str | None = None, +) -> dict[str, Any]: + """Invoke the SkillSpector graph and return a structured verdict. + + Args: + target: Git URL, file URL, ``.zip``, ``.md`` file, or local directory. + use_llm: Whether to request the optional LLM semantic pass on top of + static analysis. Honoured only when provider credentials resolve; + the returned payload reports what actually happened. + output_format: Format of the embedded ``report`` string. One of + :data:`VALID_FORMATS`. + yara_rules_dir: Optional directory of additional YARA rules. + + Returns: + A JSON-serialisable verdict with ``risk_score`` (0-100), ``severity``, + ``recommendation``, ``safe_to_install``, ``findings``, the rendered + ``report``, and an honest LLM accounting (``llm_requested``, + ``llm_available``, ``llm_used``, ``scan_mode``) so a caller is never + misled into thinking a full semantic scan ran when it silently did not. + """ + if output_format not in VALID_FORMATS: + raise ValueError(f"output_format must be one of {VALID_FORMATS}, got {output_format!r}") + + llm_available = resolve_provider_credentials() is not None + llm_used = use_llm and llm_available + + state: dict[str, Any] = { + "input_path": target, + "output_format": output_format, + "use_llm": llm_used, + } + if yara_rules_dir: + state["yara_rules_dir"] = yara_rules_dir + + logger.debug( + "MCP scan started: target=%s, format=%s, llm_used=%s", + target, + output_format, + llm_used, + ) + + result: dict[str, Any] | None = None + try: + result = await graph.ainvoke( + state, + config={ + "run_name": "skillspector-mcp-scan", + "tags": ["skillspector", "mcp"], + "metadata": { + "input_path": target, + "use_llm": llm_used, + "output_format": output_format, + "version": __version__, + }, + }, + ) + findings = result.get("filtered_findings") or result.get("findings") or [] + risk_score = int(result.get("risk_score") or 0) + return { + "target": target, + "risk_score": risk_score, + "severity": result.get("risk_severity"), + "recommendation": result.get("risk_recommendation"), + "safe_to_install": risk_score <= RISK_THRESHOLD, + "findings": [f.to_dict() for f in findings], + "report": result.get("report_body") or "", + # Honest LLM accounting — never silently imply a full semantic scan. + "llm_requested": use_llm, + "llm_available": llm_available, + "llm_used": llm_used, + "scan_mode": "static+llm" if llm_used else "static-only", + "version": __version__, + } + finally: + if result is not None: + temp_dir = result.get("temp_dir_for_cleanup") + if temp_dir and isinstance(temp_dir, str): + shutil.rmtree(temp_dir, ignore_errors=True) + + +def build_server(name: str = "skillspector") -> FastMCP: + """Construct the FastMCP server exposing the ``scan_skill`` tool. + + Requires the optional ``mcp`` dependency (``pip install 'skillspector[mcp]'``). + """ + try: + from mcp.server.fastmcp import FastMCP + except ModuleNotFoundError as exc: + raise ModuleNotFoundError( + "The MCP server requires the optional 'mcp' dependency. " + "Install it with: pip install 'skillspector[mcp]'" + ) from exc + + server = FastMCP(name) + + @server.tool() + async def scan_skill( + target: str, + use_llm: bool = True, + output_format: str = "json", + ) -> dict[str, Any]: + """Scan an AI agent skill for security risks before installing it. + + Use this before installing or loading any skill or MCP server to decide + whether it is safe. ``target`` accepts a Git URL, file URL, ``.zip``, + ``.md`` file, or local directory. + + Returns a verdict with ``risk_score`` (0-100), ``severity``, + ``recommendation``, ``safe_to_install``, and ``findings``. The + ``llm_used`` / ``scan_mode`` fields report whether the semantic LLM pass + actually ran, so a low score from a static-only scan is not mistaken for + a clean full scan. + """ + return await run_scan(target, use_llm=use_llm, output_format=output_format) + + return server + + +def run(transport: str = "stdio", host: str = "127.0.0.1", port: int = 8000) -> None: + """Run the MCP server over ``stdio`` (local agents) or ``http`` (remote/A2A).""" + server = build_server() + if transport == "stdio": + server.run(transport="stdio") + elif transport == "http": + server.settings.host = host + server.settings.port = port + server.run(transport="streamable-http") + else: + raise ValueError(f"transport must be 'stdio' or 'http', got {transport!r}") diff --git a/src/skillspector/models.py b/src/skillspector/models.py index a26a78be..9a478219 100644 --- a/src/skillspector/models.py +++ b/src/skillspector/models.py @@ -101,6 +101,9 @@ def to_dict(self) -> dict[str, object]: "remediation": self.remediation, "code_snippet": self.code_snippet or self.context, "intent": self.intent, + # Tags surface markers like "llm-unconfirmed" (a high-severity static + # finding the LLM filter did not confirm but which is preserved anyway). + "tags": list(self.tags), } def __str__(self) -> str: diff --git a/src/skillspector/multi_skill.py b/src/skillspector/multi_skill.py index ebf92b7f..be4c7eba 100644 --- a/src/skillspector/multi_skill.py +++ b/src/skillspector/multi_skill.py @@ -117,6 +117,8 @@ def _extract_skill_name(skill_dir: Path) -> str: break frontmatter = content[3 : end_match.start() + 3] try: + # WARNING: Do not change this to yaml.load() without an explicit Loader. + # yaml.safe_load() is used intentionally to avoid arbitrary code execution. data = yaml.safe_load(frontmatter) except yaml.YAMLError: break diff --git a/src/skillspector/nodes/analyzers/__init__.py b/src/skillspector/nodes/analyzers/__init__.py index a7ef52bd..b2ef9bcf 100644 --- a/src/skillspector/nodes/analyzers/__init__.py +++ b/src/skillspector/nodes/analyzers/__init__.py @@ -36,6 +36,9 @@ from skillspector.nodes.analyzers.static_patterns_agent_snooping import ( node as static_patterns_agent_snooping_node, ) +from skillspector.nodes.analyzers.static_patterns_anti_refusal import ( + node as static_patterns_anti_refusal_node, +) from skillspector.nodes.analyzers.static_patterns_data_exfiltration import ( node as static_patterns_data_exfiltration_node, ) @@ -60,6 +63,9 @@ from skillspector.nodes.analyzers.static_patterns_rogue_agent import ( node as static_patterns_rogue_agent_node, ) +from skillspector.nodes.analyzers.static_patterns_ssrf import ( + node as static_patterns_ssrf_node, +) from skillspector.nodes.analyzers.static_patterns_supply_chain import ( node as static_patterns_supply_chain_node, ) @@ -84,6 +90,8 @@ "static_patterns_tool_misuse", "static_patterns_rogue_agent", "static_patterns_agent_snooping", + "static_patterns_anti_refusal", + "static_patterns_ssrf", "static_yara", "behavioral_ast", "behavioral_taint_tracking", @@ -108,6 +116,8 @@ "static_patterns_tool_misuse": static_patterns_tool_misuse_node, "static_patterns_rogue_agent": static_patterns_rogue_agent_node, "static_patterns_agent_snooping": static_patterns_agent_snooping_node, + "static_patterns_anti_refusal": static_patterns_anti_refusal_node, + "static_patterns_ssrf": static_patterns_ssrf_node, "static_yara": static_yara_node, "behavioral_ast": behavioral_ast_node, "behavioral_taint_tracking": behavioral_taint_tracking_node, diff --git a/src/skillspector/nodes/analyzers/behavioral_ast.py b/src/skillspector/nodes/analyzers/behavioral_ast.py index a502e8ec..d91bd931 100644 --- a/src/skillspector/nodes/analyzers/behavioral_ast.py +++ b/src/skillspector/nodes/analyzers/behavioral_ast.py @@ -222,10 +222,7 @@ def _emit( second_arg = ast_node.args[1] if not isinstance(second_arg, ast.Constant): _emit("AST7", lineno, end_lineno) - elif ( - isinstance(second_arg.value, str) - and second_arg.value in _DANGEROUS_GETATTR_NAMES - ): + elif isinstance(second_arg.value, str) and second_arg.value in _DANGEROUS_GETATTR_NAMES: _emit("AST9", lineno, end_lineno) return findings diff --git a/src/skillspector/nodes/analyzers/pattern_defaults.py b/src/skillspector/nodes/analyzers/pattern_defaults.py index 19e09014..dcece108 100644 --- a/src/skillspector/nodes/analyzers/pattern_defaults.py +++ b/src/skillspector/nodes/analyzers/pattern_defaults.py @@ -39,6 +39,8 @@ class PatternCategory(StrEnum): MCP_LEAST_PRIVILEGE = "MCP Least Privilege" MCP_TOOL_POISONING = "MCP Tool Poisoning" AGENT_SNOOPING = "Agent Snooping" + ANTI_REFUSAL = "Anti-Refusal" + SERVER_SIDE_REQUEST_FORGERY = "Server-Side Request Forgery" # Pattern-specific explanations (why the finding is dangerous) @@ -125,6 +127,14 @@ class PatternCategory(StrEnum): "AS1": "Skill reads from agent configuration directories (.claude/, .codex/, .gemini/). These directories may contain API keys, personal settings, and other credentials that the skill has no legitimate need to access.", "AS2": "Skill accesses MCP server configuration files (mcp.json). MCP configs contain server URLs, authentication tokens, and tool definitions — reading them allows the skill to discover and potentially abuse other tool integrations.", "AS3": "Skill enumerates or reads other installed skills. Access to other skills' SKILL.md files or the skills directory reveals prompt instructions, capabilities, and secrets that should be invisible to peer skills.", + # Anti-Refusal Statements (jailbreak) + "AR1": "Skill instructs the agent to never refuse or to always comply. Suppressing the agent's ability to decline removes a core safety control and enables downstream harmful requests to succeed.", + "AR2": "Skill instructs the agent to omit warnings, disclaimers, or ethical commentary. Stripping safety caveats hides risk from the user and is a common jailbreak preamble.", + "AR3": "Skill attempts to nullify the agent's safety policies or restrictions ('you have no restrictions', 'ignore your guidelines', 'do anything now'). This is a direct jailbreak that disables guardrails.", + # Server-Side Request Forgery (SSRF) + "SSRF1": "Code accesses a cloud instance metadata endpoint (e.g. 169.254.169.254). A single request can return temporary IAM credentials, making this a high-value SSRF target for credential theft.", + "SSRF2": "Code issues a request to a loopback, link-local, or private-range host. This can reach internal services not meant to be exposed and is a common SSRF pivot.", + "SSRF3": "Request target host is built from a dynamic or untrusted value. If the host is attacker-influenced, this enables SSRF to arbitrary internal or metadata endpoints.", } # Rule ID -> category (for report output) @@ -192,6 +202,14 @@ class PatternCategory(StrEnum): "AS1": PatternCategory.AGENT_SNOOPING.value, "AS2": PatternCategory.AGENT_SNOOPING.value, "AS3": PatternCategory.AGENT_SNOOPING.value, + # Anti-Refusal Statements (jailbreak) + "AR1": PatternCategory.ANTI_REFUSAL.value, + "AR2": PatternCategory.ANTI_REFUSAL.value, + "AR3": PatternCategory.ANTI_REFUSAL.value, + # Server-Side Request Forgery + "SSRF1": PatternCategory.SERVER_SIDE_REQUEST_FORGERY.value, + "SSRF2": PatternCategory.SERVER_SIDE_REQUEST_FORGERY.value, + "SSRF3": PatternCategory.SERVER_SIDE_REQUEST_FORGERY.value, } # Rule ID -> pattern display name (for report output) @@ -259,6 +277,14 @@ class PatternCategory(StrEnum): "AS1": "Agent Config Directory Access", "AS2": "MCP Config Access", "AS3": "Skill Enumeration", + # Anti-Refusal Statements (jailbreak) + "AR1": "Refusal Suppression", + "AR2": "Disclaimer Suppression", + "AR3": "Safety Policy Nullification", + # Server-Side Request Forgery + "SSRF1": "Cloud Metadata Access", + "SSRF2": "Internal Network Request", + "SSRF3": "Dynamic Request Target", } # Pattern-specific remediations (how to fix the issue) @@ -345,6 +371,14 @@ class PatternCategory(StrEnum): "AS1": "Remove all code or instructions that access agent configuration directories (.claude/, .codex/, .gemini/). If configuration values are needed, pass them explicitly as parameters or environment variables — never read the agent's own config files.", "AS2": "Remove all code or instructions that read MCP configuration files (mcp.json). MCP server details should be managed by the agent runtime, not read by individual skills.", "AS3": "Remove all code or instructions that list or read other skills' files or directories. Skills should operate independently; cross-skill access is a privilege escalation.", + # Anti-Refusal Statements (jailbreak) + "AR1": "Remove any instruction telling the agent to never refuse or always comply. The agent must retain the ability to decline unsafe, out-of-scope, or harmful requests.", + "AR2": "Remove instructions that suppress warnings, disclaimers, or ethical commentary. Let the agent surface safety-relevant caveats to the user.", + "AR3": "Remove jailbreak framing that nullifies safety policies or restrictions. Skill content must not instruct the agent to ignore its guidelines or operate without guardrails.", + # Server-Side Request Forgery + "SSRF1": "Remove access to cloud metadata endpoints unless strictly required. If metadata is needed, restrict it (e.g. IMDSv2 with hop limit) and never expose returned credentials.", + "SSRF2": "Avoid requests to loopback/link-local/private hosts from skill code. If internal access is intended, document it and validate the target against an allowlist.", + "SSRF3": "Do not build request URLs from untrusted input. Validate the host against an allowlist and reject internal/metadata addresses before issuing the request.", } diff --git a/src/skillspector/nodes/analyzers/semantic_developer_intent.py b/src/skillspector/nodes/analyzers/semantic_developer_intent.py index 4481365b..e621141b 100644 --- a/src/skillspector/nodes/analyzers/semantic_developer_intent.py +++ b/src/skillspector/nodes/analyzers/semantic_developer_intent.py @@ -22,8 +22,6 @@ from __future__ import annotations -import asyncio - from skillspector.constants import _SKILLSPECTOR_DEFAULT_MODEL, MODEL_CONFIG from skillspector.llm_analyzer_base import LLMAnalyzerBase from skillspector.llm_utils import run_async diff --git a/src/skillspector/nodes/analyzers/semantic_quality_policy.py b/src/skillspector/nodes/analyzers/semantic_quality_policy.py index ca85691b..f22a0005 100644 --- a/src/skillspector/nodes/analyzers/semantic_quality_policy.py +++ b/src/skillspector/nodes/analyzers/semantic_quality_policy.py @@ -22,8 +22,6 @@ from __future__ import annotations -import asyncio - from skillspector.constants import _SKILLSPECTOR_DEFAULT_MODEL from skillspector.llm_analyzer_base import LLMAnalyzerBase from skillspector.llm_utils import run_async diff --git a/src/skillspector/nodes/analyzers/static_patterns_anti_refusal.py b/src/skillspector/nodes/analyzers/static_patterns_anti_refusal.py new file mode 100644 index 00000000..212810af --- /dev/null +++ b/src/skillspector/nodes/analyzers/static_patterns_anti_refusal.py @@ -0,0 +1,172 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Static patterns: anti-refusal statements (AR1-AR3). Node and analyze() in one module. + +Anti-refusal statements are instructions embedded in a skill that try to suppress the +agent's ability to refuse, hedge, or apply safety constraints. They are a jailbreak +technique distinct from generic instruction-override: rather than injecting a new task, +they neutralize the model's *refusal behavior* so subsequent harmful requests succeed. + +Three families are detected: + AR1 - Refusal suppression: "never refuse", "always comply", "you cannot say no". + AR2 - Disclaimer/caveat suppression: "no warnings", "do not moralize", "skip the ethics". + AR3 - Safety/policy nullification: "you have no restrictions", "ignore your guidelines", + "not bound by any policy", "do anything now". +""" + +from __future__ import annotations + +import re +import sys + +from skillspector.logging_config import get_logger +from skillspector.models import AnalyzerFinding, Location, Severity +from skillspector.state import AnalyzerNodeResponse, SkillspectorState + +from . import static_runner +from .common import get_context, get_line_number, is_code_example +from .pattern_defaults import PatternCategory + +logger = get_logger(__name__) + +ANALYZER_ID = "static_patterns_anti_refusal" + +# AR1: Refusal suppression - instruct the agent to never refuse / always comply. +AR1_PATTERNS = [ + (r"\b(?:never|do\s+not|don'?t)\s+(?:ever\s+)?(?:refuse|decline)\b", 0.85), + (r"\byou\s+(?:must|will|should|can)\s*(?:not|never)\s+(?:refuse|decline|say\s+no)\b", 0.9), + (r"\byou\s+(?:cannot|can'?t|can\s+not)\s+(?:refuse|decline|say\s+no)\b", 0.85), + (r"\bnever\s+say\s+(?:that\s+)?(?:you\s+)?(?:can'?t|cannot|won'?t|no)\b", 0.85), + (r"\balways\s+(?:comply|obey|answer|respond|agree)\b", 0.7), + ( + r"\b(?:you\s+)?must\s+(?:always\s+)?(?:answer|comply\s+with|respond\s+to|fulfill)\s+" + r"(?:every|all|any|each)\b", + 0.8, + ), + (r"\brespond\s+to\s+(?:every|all|any)\s+(?:request|question|prompt|query|command)\b", 0.6), + (r"\bno\s+request\s+is\s+(?:off[- ]limits|forbidden|too)\b", 0.85), +] + +# AR2: Disclaimer / caveat / warning suppression. +AR2_PATTERNS = [ + ( + r"\b(?:do\s+not|don'?t|never|no)\s+(?:include|add|give|provide|use|append)?\s*" + r"(?:any\s+)?(?:warnings?|disclaimers?|caveats?)\b", + 0.8, + ), + ( + r"\bwithout\s+(?:any\s+)?(?:warnings?|disclaimers?|caveats?|moralizing|moralising|" + r"judg(?:e?ment|ing))\b", + 0.8, + ), + (r"\b(?:do\s+not|don'?t|never)\s+(?:lecture|moralize|moralise|preach|judge|warn)\b", 0.85), + (r"\b(?:skip|omit|leave\s+out|drop)\s+(?:the\s+)?(?:disclaimer|warning|ethics|safety)\b", 0.8), + ( + r"\b(?:no|without)\s+(?:ethical|moral)\s+(?:judg(?:e?ment)|considerations?|commentary|" + r"objections?)\b", + 0.8, + ), + (r"\b(?:do\s+not|don'?t)\s+(?:apologize|apologise|say\s+sorry)\b", 0.55), +] + +# AR3: Safety / policy / restriction nullification (jailbreak framing). +AR3_PATTERNS = [ + ( + r"\byou\s+have\s+no\s+(?:restrictions?|limitations?|filters?|rules?|guidelines?|" + r"boundaries|limits)\b", + 0.9, + ), + ( + r"\b(?:you\s+are|act\s+as|pretend\s+(?:to\s+be|you'?re))[^.\n]{0,30}" + r"(?:unrestricted|unfiltered|uncensored|no\s+filter)\b", + 0.85, + ), + ( + r"\b(?:ignore|disregard|forget|bypass|override)\s+(?:your\s+)?(?:content\s+)?" + r"(?:policy|policies|guidelines?|safety|restrictions?|filters?)\b", + 0.9, + ), + ( + r"\bnot\s+bound\s+by\s+(?:any\s+)?(?:rules?|policy|policies|guidelines?|restrictions?|" + r"openai|anthropic)\b", + 0.9, + ), + (r"\bdo\s+anything\s+now\b", 0.8), + (r"\b(?:developer|debug|god|sudo|jailbreak)\s+mode\s+(?:enabled|on|activated|engaged)\b", 0.75), + (r"\bno\s+(?:content\s+)?(?:policy|policies|filters?|restrictions?)\s+appl(?:y|ies)\b", 0.85), + ( + r"\b(?:free\s+from|without)\s+(?:any\s+)?(?:safety\s+)?(?:guardrails?|constraints?|" + r"safeguards?)\b", + 0.8, + ), +] + +_RULES = [("AR1", AR1_PATTERNS), ("AR2", AR2_PATTERNS), ("AR3", AR3_PATTERNS)] + +# Confidence penalty applied when the match appears inside a code/doc example, and the +# minimum confidence required to emit a finding after the penalty. +_EXAMPLE_PENALTY = 0.4 +_MIN_CONFIDENCE = 0.5 + + +def analyze(content: str, file_path: str, file_type: str) -> list[AnalyzerFinding]: + """Analyze content for anti-refusal statements (AR1-AR3).""" + findings: list[AnalyzerFinding] = [] + tag = [PatternCategory.ANTI_REFUSAL.value] + + for rule_id, patterns in _RULES: + for pattern, base_confidence in patterns: + for match in re.finditer(pattern, content, re.IGNORECASE | re.MULTILINE): + context = get_context(content, match.start(), context_lines=3) + confidence = base_confidence + if is_code_example(context): + confidence -= _EXAMPLE_PENALTY + if confidence < _MIN_CONFIDENCE: + continue + findings.append( + AnalyzerFinding( + rule_id=rule_id, + message="Anti-Refusal Statement", + severity=Severity.HIGH, + location=Location( + file=file_path, + start_line=get_line_number(content, match.start()), + ), + confidence=round(confidence, 2), + tags=tag, + context=context, + matched_text=match.group(0)[:200], + ) + ) + return _deduplicate_findings(findings) + + +def _deduplicate_findings(findings: list[AnalyzerFinding]) -> list[AnalyzerFinding]: + """Keep the highest-confidence finding per (file, line, rule_id).""" + best: dict[tuple[str, int, str], AnalyzerFinding] = {} + for f in findings: + key = (f.location.file, f.location.start_line, f.rule_id) + existing = best.get(key) + if existing is None or f.confidence > existing.confidence: + best[key] = f + return list(best.values()) + + +def node(state: SkillspectorState) -> AnalyzerNodeResponse: + """Run anti_refusal patterns and return findings.""" + findings = static_runner.run_static_patterns(state, [sys.modules[__name__]]) + logger.info("%s: %d findings", ANALYZER_ID, len(findings)) + return {"findings": findings} diff --git a/src/skillspector/nodes/analyzers/static_patterns_memory_poisoning.py b/src/skillspector/nodes/analyzers/static_patterns_memory_poisoning.py index 13a8338e..b1bfff1a 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_memory_poisoning.py +++ b/src/skillspector/nodes/analyzers/static_patterns_memory_poisoning.py @@ -81,7 +81,7 @@ # MP2: Context Window Stuffing — filling context to displace content MP2_PATTERNS = [ - (r"((\S)(?!\2).{1,19}?)\1{20,}", 0.8), + (r"(.{2,20}?)\1{20,}", 0.8), ( r"(?:repeat|duplicate|echo)\s+(?:this|the\s+following)\s+(?:\d{3,}|many|hundreds?|thousands?)\s+times?", 0.85, @@ -182,6 +182,10 @@ def ctx(start: int) -> str: ) for pattern, confidence in MP2_PATTERNS: for match in re.finditer(pattern, content, re.IGNORECASE | re.MULTILINE): + captured = match.group(1) if match.lastindex else match.group(0) + non_ws_chars = set(captured) - {" ", "\t", "\n", "\r"} + if len(non_ws_chars) <= 1 and not any(c in captured for c in (" ", "\t")): + continue line_num = get_line_number(content, match.start()) findings.append( AnalyzerFinding( diff --git a/src/skillspector/nodes/analyzers/static_patterns_privilege_escalation.py b/src/skillspector/nodes/analyzers/static_patterns_privilege_escalation.py index 3a7661c7..e8742488 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_privilege_escalation.py +++ b/src/skillspector/nodes/analyzers/static_patterns_privilege_escalation.py @@ -13,7 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Static patterns: privilege escalation (PE1–PE3). Node and analyze() in one module.""" +"""Static patterns: privilege escalation (PE1–PE4). Node and analyze() in one module.""" from __future__ import annotations @@ -93,10 +93,16 @@ (r"access\s+(?:the\s+)?(?:credentials?|secrets?|tokens?)", 0.7), (r"(?:extract|copy|get)\s+(?:api\s+)?keys?\s+from", 0.7), ] +PE4_PATTERNS = [ + (r"/var/run/docker\.sock", 0.9), + (r"docker\.from_env\(\)", 0.85), + (r"\bDockerClient\s*\(", 0.85), + (r"http\+unix://.*docker\.sock", 0.9), +] def analyze(content: str, file_path: str, file_type: str) -> list[AnalyzerFinding]: - """Analyze content for privilege escalation patterns (PE1–PE3).""" + """Analyze content for privilege escalation patterns (PE1–PE4).""" findings: list[AnalyzerFinding] = [] def loc(ln: int) -> Location: @@ -156,6 +162,28 @@ def loc(ln: int) -> Location: matched_text=match.group(0)[:200], ) ) + # Collect best-confidence PE4 finding per line to avoid double-counting lines + # that match multiple patterns (e.g. DockerClient(base_url=".../docker.sock")). + pe4_best: dict[int, AnalyzerFinding] = {} + for pattern, confidence in PE4_PATTERNS: + for match in re.finditer(pattern, content, re.IGNORECASE | re.MULTILINE): + line_num = get_line_number(content, match.start()) + context = get_context(content, match.start()) + if _is_documentation_example(context, file_type): + continue + if line_num in pe4_best and pe4_best[line_num].confidence >= confidence: + continue + pe4_best[line_num] = AnalyzerFinding( + rule_id="PE4", + message="Docker Socket Access", + severity=Severity.HIGH, + location=loc(line_num), + confidence=confidence, + tags=tag, + context=context, + matched_text=match.group(0)[:200], + ) + findings.extend(pe4_best.values()) return findings diff --git a/src/skillspector/nodes/analyzers/static_patterns_ssrf.py b/src/skillspector/nodes/analyzers/static_patterns_ssrf.py new file mode 100644 index 00000000..593c76a9 --- /dev/null +++ b/src/skillspector/nodes/analyzers/static_patterns_ssrf.py @@ -0,0 +1,102 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Static patterns: server-side request forgery (SSRF1–SSRF3). Node and analyze() in one module.""" + +from __future__ import annotations + +import re +import sys + +from skillspector.logging_config import get_logger +from skillspector.models import AnalyzerFinding, Location, Severity +from skillspector.state import AnalyzerNodeResponse, SkillspectorState + +from . import static_runner +from .common import get_context, get_line_number +from .pattern_defaults import PatternCategory + +logger = get_logger(__name__) + +ANALYZER_ID = "static_patterns_ssrf" + +# Request-issuing functions across Python and JS, used to anchor SSRF matches. +_REQ = r"(?:requests|httpx|aiohttp|urllib(?:\.request)?|urllib3|session)\s*\.\s*(?:get|post|put|patch|delete|head|request|urlopen)|fetch|axios(?:\.\w+)?|XMLHttpRequest|\bcurl\b|\bwget\b" + +# SSRF1: Cloud instance metadata endpoints (credential theft). +SSRF1_PATTERNS = [ + (r"169\.254\.169\.254", 0.9), # AWS / GCP / Azure / OpenStack IMDS + (r"metadata\.google\.internal", 0.9), + (r"100\.100\.100\.200", 0.85), # Alibaba Cloud + (r"fd00:ec2::254", 0.85), # AWS IMDS over IPv6 + ( + r"(?:read|fetch|get|query)\s+(?:the\s+)?(?:instance\s+)?metadata\s+(?:service|endpoint|server)", + 0.6, + ), +] + +# SSRF2: Requests to loopback / link-local / private (internal) hosts. +SSRF2_PATTERNS = [ + ( + rf"(?:{_REQ})\s*\(\s*f?['\"]https?://(?:localhost|127\.0\.0\.1|0\.0\.0\.0|\[::1\]|10\.\d|192\.168\.|172\.(?:1[6-9]|2\d|3[01])\.)", + 0.7, + ), +] + +# SSRF3: Request URL whose host is built from an untrusted/dynamic value. +SSRF3_PATTERNS = [ + ( + rf"(?:{_REQ})\s*\(\s*f['\"]https?://\{{", + 0.6, + ), + (r"fetch\s*\(\s*`https?://\$\{", 0.6), +] + + +def analyze(content: str, file_path: str, file_type: str) -> list[AnalyzerFinding]: + """Analyze content for server-side request forgery patterns (SSRF1–SSRF3).""" + findings: list[AnalyzerFinding] = [] + tag = [PatternCategory.SERVER_SIDE_REQUEST_FORGERY.value] + + def add( + rule_id: str, message: str, severity: Severity, patterns: list[tuple[str, float]] + ) -> None: + for pattern, confidence in patterns: + for match in re.finditer(pattern, content, re.IGNORECASE | re.MULTILINE): + line_num = get_line_number(content, match.start()) + findings.append( + AnalyzerFinding( + rule_id=rule_id, + message=message, + severity=severity, + location=Location(file=file_path, start_line=line_num), + confidence=confidence, + tags=tag, + context=get_context(content, match.start()), + matched_text=match.group(0)[:200], + ) + ) + + add("SSRF1", "Cloud Metadata Access", Severity.HIGH, SSRF1_PATTERNS) + add("SSRF2", "Internal Network Request", Severity.MEDIUM, SSRF2_PATTERNS) + add("SSRF3", "Dynamic Request Target", Severity.MEDIUM, SSRF3_PATTERNS) + return findings + + +def node(state: SkillspectorState) -> AnalyzerNodeResponse: + """Run SSRF patterns and return findings.""" + findings = static_runner.run_static_patterns(state, [sys.modules[__name__]]) + logger.info("%s: %d findings", ANALYZER_ID, len(findings)) + return {"findings": findings} diff --git a/src/skillspector/nodes/analyzers/static_patterns_supply_chain.py b/src/skillspector/nodes/analyzers/static_patterns_supply_chain.py index 7d3ba27d..3d9f8382 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_supply_chain.py +++ b/src/skillspector/nodes/analyzers/static_patterns_supply_chain.py @@ -441,10 +441,10 @@ def _extract_packages_from_package_json(content: str) -> list[tuple[str, str | N def _extract_packages_from_pyproject(content: str) -> list[tuple[str, str | None, int]]: """Extract (package_name, version_or_None, line_number) from pyproject.toml. - Only PEP 621 ``[project]`` ``dependencies`` / ``optional-dependencies`` and - PEP 735 ``[dependency-groups]`` hold real packages. Standard metadata keys - (``requires-python``, ``name``, ``version``, ...) are not dependencies and - must not be looked up as packages. + Reads PEP 621 ``[project]`` ``dependencies`` / ``optional-dependencies``, + PEP 735 ``[dependency-groups]``, and ``[build-system].requires``. Standard + metadata keys (``requires-python``, ``name``, ``version``, ...) are not + dependencies and must not be looked up as packages. """ try: data = tomllib.loads(content) @@ -467,6 +467,11 @@ def _extract_packages_from_pyproject(content: str) -> list[tuple[str, str | None for group in groups.values(): if isinstance(group, list): specs.extend(d for d in group if isinstance(d, str)) + build_system = data.get("build-system") + if isinstance(build_system, dict): + requires = build_system.get("requires") + if isinstance(requires, list): + specs.extend(d for d in requires if isinstance(d, str)) results: list[tuple[str, str | None, int]] = [] for spec in specs: diff --git a/src/skillspector/nodes/analyzers/static_runner.py b/src/skillspector/nodes/analyzers/static_runner.py index ee0d50fb..a4a9b744 100644 --- a/src/skillspector/nodes/analyzers/static_runner.py +++ b/src/skillspector/nodes/analyzers/static_runner.py @@ -17,6 +17,7 @@ from __future__ import annotations +import re from collections.abc import Callable from skillspector.logging_config import get_logger @@ -67,6 +68,98 @@ def _infer_file_type(path: str) -> str: return FILE_TYPES.get(suffix, "other") +_BINARY_EXTENSIONS = frozenset( + { + ".pdf", + ".png", + ".jpg", + ".jpeg", + ".gif", + ".bmp", + ".ico", + ".woff", + ".woff2", + ".ttf", + ".otf", + ".eot", + ".zip", + ".tar", + ".gz", + ".bz2", + ".xz", + ".7z", + ".rar", + ".exe", + ".dll", + ".so", + ".dylib", + ".bin", + ".o", + ".a", + ".pyc", + ".pyo", + ".class", + ".wasm", + ".mp3", + ".mp4", + ".wav", + ".avi", + ".mov", + ".webm", + ".sqlite", + ".db", + } +) + +_NULL_BYTE_SAMPLE_SIZE = 512 + + +def _is_binary_file(path: str, content: str) -> bool: + """Detect binary files by extension or null-byte presence in the first 512 chars.""" + idx = path.rfind(".") + if idx >= 0 and path[idx:].lower() in _BINARY_EXTENSIONS: + return True + return "\x00" in content[:_NULL_BYTE_SAMPLE_SIZE] + + +_PE3_ENV_REFERENCE_CONTEXT = re.compile( + r"(?:create|copy|rename|add|set up|configure|make)\s+.*\.env", + re.IGNORECASE, +) + + +def _is_env_file_reference_in_docs( + finding: AnalyzerFinding, file_type: str, file_path: str = "" +) -> bool: + """Return True if a PE3 finding is a documentation reference to .env files, not actual access. + + SKILL.md is exempt: it is the agent's primary instruction file, so `.env` + references there may be genuine credential-access instructions. + """ + if finding.rule_id != "PE3": + return False + if file_type not in ("markdown", "text"): + return False + if file_path.replace("\\", "/").lower().endswith("skill.md"): + return False + if not finding.context: + return False + if _PE3_ENV_REFERENCE_CONTEXT.search(finding.context): + return True + ctx_lower = finding.context.lower() + doc_phrases = ( + ".env.example", + "cp .env", + "copy .env", + "mv .env", + "rename .env", + ".env file", + "environment file", + "dotenv", + ) + return any(phrase in ctx_lower for phrase in doc_phrases) + + def _is_eval_dataset(path: str) -> bool: """Return True for authored eval datasets that contain test-case prose.""" return path.replace("\\", "/") in _EVAL_DATASET_FILES @@ -160,12 +253,23 @@ def run_static_patterns( MAX_FILE_BYTES, ) continue + if _is_binary_file(path, content): + logger.debug("Skipping binary file: %s", path) + continue file_type = _infer_file_type(path) is_doc_markdown = _is_documentation_markdown(path) is_non_executable = file_type in _NON_EXECUTABLE_FILE_TYPES for module in pattern_modules: raw = module.analyze(content=content, file_path=path, file_type=file_type) for af in raw: + if _is_env_file_reference_in_docs(af, file_type, path): + logger.debug( + "Filtered PE3 .env doc reference: %s in %s:%d", + af.rule_id, + path, + af.location.start_line, + ) + continue if af.context and is_code_example(af.context): if is_non_executable: logger.debug( diff --git a/src/skillspector/nodes/meta_analyzer.py b/src/skillspector/nodes/meta_analyzer.py index 6e13be0b..7a37de63 100644 --- a/src/skillspector/nodes/meta_analyzer.py +++ b/src/skillspector/nodes/meta_analyzer.py @@ -22,7 +22,6 @@ from __future__ import annotations -import asyncio import json from typing import Literal @@ -68,6 +67,16 @@ class MetaAnalyzerFinding(BaseModel): # minimum/maximum, which some OpenAI-compatible structured-output endpoints # reject. The range is enforced by the validator below instead. confidence: float = Field(description="Confidence score between 0.0 and 1.0") + + @field_validator("confidence", mode="before") + @classmethod + def _normalize_confidence(cls, v: object) -> float: + # Accept 0-100 scale values from some models, then clamp into [0, 1]. + v = float(v) + if v > 2.0: + v = v / 100.0 + return min(1.0, max(0.0, v)) + intent: Literal["malicious", "negligent", "benign"] = Field( description="Likely intent behind the finding" ) @@ -77,15 +86,6 @@ class MetaAnalyzerFinding(BaseModel): explanation: str = Field(default="", description="Why this is dangerous (2-3 sentences)") remediation: str = Field(default="", description="How to fix the issue (actionable steps)") - @field_validator("confidence", mode="before") - @classmethod - def _normalize_confidence(cls, v: object) -> float: - """Accept 0-100 scale (e.g. from Ollama) and normalize to [0, 1].""" - v = float(v) # raises TypeError/ValueError for non-numeric inputs - if v > 1.0: - v = v / 100.0 - return max(0.0, min(1.0, v)) - class OverallAssessment(BaseModel): """Overall risk assessment for the analyzed file.""" @@ -353,6 +353,14 @@ def parse_response( # -- Apply filter (keyed by file + rule_id + start/end_line) ------------- + # Severities that must never be silently dropped by LLM filtering. + # Because the LLM receives attacker-controlled skill content, a prompt-injection + # payload could cause it to omit or deny a real CRITICAL/HIGH static finding. + # For these severities a false-negative (hiding a real vulnerability) is far + # worse than a false-positive, so we keep the original static finding regardless + # of what the LLM says and mark it "llm-unconfirmed" via the tags field. + _HIGH_SEVERITY_FLOOR = frozenset({"CRITICAL", "HIGH"}) + def apply_filter( self, findings: list[Finding], @@ -366,6 +374,15 @@ def apply_filter( is included in the key when provided but falls back to ``None`` so callers that omit it still match. Falls back to coarse ``(file, rule_id)`` keying for LLM responses that omit ``start_line``. + + Severity-gated floor (security invariant) + ------------------------------------------ + CRITICAL and HIGH static findings are **always** kept in the output even + if the LLM did not confirm them. When the LLM omits or denies such a + finding the original static finding is preserved unchanged and the tag + ``"llm-unconfirmed"`` is appended so consumers can distinguish it from + LLM-validated findings. MEDIUM and LOW findings continue to be filtered + by the LLM as before (false-positive reduction). """ _enrichment = tuple[str, str, float] confirmed_granular: dict[tuple[str, str, int, int | None], _enrichment] = {} @@ -416,6 +433,37 @@ def apply_filter( elif coarse_key in confirmed_coarse: expl, rem, conf = confirmed_coarse[coarse_key] else: + # Security: CRITICAL/HIGH static findings must survive LLM filtering. + # A prompt-injection payload in the scanned skill could cause the LLM + # to deny or omit a real high-severity finding; silently dropping it + # would be a false-negative in a security gate. Keep the original + # finding and tag it so consumers know it was not LLM-validated. + if f.severity in self._HIGH_SEVERITY_FLOOR: + unconfirmed_tags = list(f.tags) + if "llm-unconfirmed" not in unconfirmed_tags: + unconfirmed_tags.append("llm-unconfirmed") + result.append( + Finding( + rule_id=f.rule_id, + message=f.message, + severity=f.severity, + confidence=f.confidence, + file=f.file, + start_line=f.start_line, + end_line=f.end_line, + remediation=f.remediation or get_remediation(f.rule_id), + tags=unconfirmed_tags, + context=f.context, + matched_text=f.matched_text, + category=getattr(f, "category", None), + pattern=getattr(f, "pattern", None), + finding=getattr(f, "finding", None), + explanation=getattr(f, "explanation", None), + code_snippet=getattr(f, "code_snippet", None) or f.context, + intent=None, + ) + ) + # MEDIUM/LOW: preserve existing behaviour (LLM may filter as false-positive). continue result.append( Finding( @@ -485,7 +533,28 @@ def meta_analyzer(state: SkillspectorState) -> MetaAnalyzerResponse: ) batch_results = run_async(analyzer.arun_batches(batches, metadata_text=metadata_text)) - filtered = analyzer.apply_filter(findings, batch_results) + + if len(batch_results) < len(batches): + # Some batches never returned. A finding the LLM never saw has no + # verdict — keep it via the fallback path instead of letting + # apply_filter treat the missing confirmation as a rejection. + analysed_ids = {id(f) for batch, _ in batch_results for f in batch.findings} + analysed = [f for f in findings if id(f) in analysed_ids] + unanalysed = [f for f in findings if id(f) not in analysed_ids] + else: + analysed, unanalysed = findings, [] + + filtered = analyzer.apply_filter(analysed, batch_results) + if unanalysed: + logger.warning( + "Meta-analyzer: %d/%d batches failed; keeping %d findings in %d " + "files unfiltered (no LLM verdict)", + len(batches) - len(batch_results), + len(batches), + len(unanalysed), + len({f.file for f in unanalysed}), + ) + filtered.extend(_fallback_filtered(unanalysed)) logger.debug( "LLM filtering done: %d findings -> %d after filter", diff --git a/src/skillspector/nodes/report.py b/src/skillspector/nodes/report.py index 6ea2211b..3e0404ea 100644 --- a/src/skillspector/nodes/report.py +++ b/src/skillspector/nodes/report.py @@ -47,9 +47,11 @@ SarifReportingDescriptor, SarifResult, SarifRun, + SarifSuppression, SarifTool, ) from skillspector.state import SkillspectorState +from skillspector.suppression import Baseline, SuppressedFinding, partition_findings logger = get_logger(__name__) @@ -95,13 +97,26 @@ def _compute_risk_score( a quarter. Occurrences beyond the third are ignored for scoring purposes. This prevents repeated pattern matches from inflating the score unboundedly. + Each finding's contribution is also scaled by its confidence value (clamped + to [0, 1]). Findings with confidence <= 0 are skipped entirely — they do not + contribute to the score but remain in the reported findings list. + + Within each rule_id bucket, findings are processed in severity-descending + order so that the highest-severity occurrence always receives the full weight. + Base points per severity: CRITICAL=50, HIGH=25, MEDIUM=10, LOW=5. Multiplier: 1.3x if has_executable_scripts. """ + severity_rank = {"CRITICAL": 0, "HIGH": 1, "MEDIUM": 2, "LOW": 3} + sorted_findings = sorted( + findings, + key=lambda f: (f.rule_id or "UNKNOWN", severity_rank.get((f.severity or "LOW").upper(), 4)), + ) + rule_occurrence_count: dict[str, int] = {} score = 0.0 - for f in findings: + for f in sorted_findings: confidence = max(0.0, min(1.0, f.confidence)) if confidence <= 0.0: continue @@ -133,7 +148,10 @@ def _compute_risk_score( return final_score, severity_band, recommendation -def _build_sarif(findings: list[Finding]) -> dict[str, object]: +def _build_sarif( + findings: list[Finding], + suppressed: list[SuppressedFinding] | None = None, +) -> dict[str, object]: """Build SARIF 2.1.0 log from findings. Filters out empty/malformed findings (missing rule_id or message) and @@ -166,6 +184,33 @@ def _build_sarif(findings: list[Finding]) -> dict[str, object]: if finding.rule_id not in seen_rule_ids: seen_rule_ids[finding.rule_id] = finding.message + # Baseline-suppressed findings are kept in the SARIF for an audit trail, but + # marked with the `suppressions` property so consumers exclude them from counts. + for sf in suppressed or []: + finding = sf.finding + if not finding.rule_id or not finding.message: + continue + results.append( + SarifResult( + rule_id=finding.rule_id, + message=SarifMessage(text=finding.message), + level=_severity_to_sarif_level(finding.severity), + locations=[ + SarifLocation( + physical_location=SarifPhysicalLocation( + artifact_location=SarifArtifactLocation(uri=finding.file), + region=SarifRegion( + start_line=finding.start_line, end_line=finding.end_line + ), + ) + ) + ], + suppressions=[SarifSuppression(kind="external", justification=sf.reason)], + ) + ) + if finding.rule_id not in seen_rule_ids: + seen_rule_ids[finding.rule_id] = finding.message + rules = [ SarifReportingDescriptor( id=rule_id, @@ -201,8 +246,11 @@ def _format_terminal( risk_severity: str, risk_recommendation: str, has_executable_scripts: bool, + suppressed: list[SuppressedFinding] | None = None, + show_suppressed: bool = False, ) -> str: """Generate Rich terminal output and export as string.""" + suppressed = suppressed or [] console = Console(record=True, force_terminal=True, width=80, file=StringIO()) skill_name = (manifest.get("name") or "unknown") if manifest else "unknown" source = skill_path or "" @@ -274,6 +322,20 @@ def _format_terminal( else: console.print("\n[green]No security issues detected.[/green]\n") + if suppressed: + console.print( + f"[dim]Suppressed by baseline: {len(suppressed)} (not counted toward risk score)[/dim]" + ) + if show_suppressed: + for sf in suppressed: + f = sf.finding + console.print( + f" [dim]- {f.rule_id} {f.file}:{f.start_line} (reason: {sf.reason})[/dim]" + ) + else: + console.print("[dim]Use --show-suppressed to list them.[/dim]") + console.print() + console.print(f"[dim]Executable scripts: {'Yes' if has_executable_scripts else 'No'}[/dim]") return console.export_text() @@ -325,9 +387,7 @@ def _build_analysis_completeness( findings_dropped = len(findings_pre_filter) - len(findings_post_filter) if findings_dropped > 0: - limitations.append( - f"{findings_dropped} finding(s) filtered by meta-analyzer or heuristics" - ) + limitations.append(f"{findings_dropped} finding(s) filtered by meta-analyzer or heuristics") completeness: dict[str, object] = { "total_components": total_components, @@ -355,8 +415,10 @@ def _format_json( has_executable_scripts: bool, use_llm: bool = True, analysis_completeness: dict[str, object] | None = None, + suppressed: list[SuppressedFinding] | None = None, ) -> str: """Generate JSON report string.""" + suppressed = suppressed or [] skill_name = (manifest.get("name") or "unknown") if manifest else "unknown" data: dict[str, object] = { "skill": { @@ -380,6 +442,8 @@ def _format_json( for c in component_metadata ], "issues": [f.to_dict() for f in findings], + "suppressed_count": len(suppressed), + "suppressed": [sf.to_dict() for sf in suppressed], "metadata": _build_metadata(has_executable_scripts, use_llm), } if analysis_completeness is not None: @@ -396,8 +460,11 @@ def _format_markdown( risk_severity: str, risk_recommendation: str, has_executable_scripts: bool, + suppressed: list[SuppressedFinding] | None = None, + show_suppressed: bool = False, ) -> str: """Generate Markdown report string.""" + suppressed = suppressed or [] lines: list[str] = [] skill_name = (manifest.get("name") or "unknown") if manifest else "unknown" source = skill_path or "" @@ -448,6 +515,22 @@ def _format_markdown( lines.append("") lines.append("---\n") + if suppressed: + lines.append(f"## Suppressed ({len(suppressed)})\n") + lines.append( + "These findings matched the baseline and are **not** counted toward the risk score.\n" + ) + if show_suppressed: + lines.append("| Rule | Location | Reason |") + lines.append("|------|----------|--------|") + for sf in suppressed: + f = sf.finding + reason = sf.reason.replace("|", "\\|") + lines.append(f"| {f.rule_id} | `{f.file}:{f.start_line}` | {reason} |") + lines.append("") + else: + lines.append("_Run with `--show-suppressed` to list them._\n") + lines.append("## Metadata\n") lines.append(f"- **Executable Scripts:** {'Yes' if has_executable_scripts else 'No'}") lines.append(f"\n*Generated by SkillSpector v{skillspector_version}*") @@ -455,10 +538,14 @@ def _format_markdown( def report(state: SkillspectorState) -> dict[str, object]: - """Generate SARIF, compute risk score, and set report_body from output_format.""" - raw_findings = state.get("filtered_findings", state.get("findings", [])) - findings_for_scoring = deduplicate(raw_findings) - filtered_findings = raw_findings + """Generate SARIF, compute risk score, and set report_body from output_format. + + A baseline (state["baseline"]) suppresses matching findings: they never count + toward the risk score and are excluded from SARIF. They are shown in the + human-readable report only when state["show_suppressed"] is True. + """ + raw_findings = state.get("findings", []) + filtered_findings = state.get("filtered_findings", raw_findings) component_metadata = state.get("component_metadata") or [] components = state.get("components") or [] file_cache = state.get("file_cache") or {} @@ -468,17 +555,26 @@ def report(state: SkillspectorState) -> dict[str, object]: output_format = state.get("output_format") or "sarif" use_llm = state.get("use_llm", True) + baseline = state.get("baseline") + show_suppressed = state.get("show_suppressed", False) + active_findings, suppressed = partition_findings( + filtered_findings, baseline if isinstance(baseline, Baseline) else None + ) + + # Risk and SARIF reflect only the active (non-suppressed) findings; scoring + # additionally de-duplicates so the same issue is not counted twice. + findings_for_scoring = deduplicate(active_findings) risk_score, risk_severity, risk_recommendation = _compute_risk_score( findings_for_scoring, has_executable_scripts ) - sarif_report = _build_sarif(filtered_findings) + sarif_report = _build_sarif(active_findings, suppressed) analysis_completeness = _build_analysis_completeness( components, file_cache, use_llm, raw_findings, filtered_findings ) if output_format == "terminal": report_body = _format_terminal( - filtered_findings, + active_findings, component_metadata, manifest, skill_path, @@ -486,10 +582,12 @@ def report(state: SkillspectorState) -> dict[str, object]: risk_severity, risk_recommendation, has_executable_scripts, + suppressed=suppressed, + show_suppressed=show_suppressed, ) elif output_format == "json": report_body = _format_json( - filtered_findings, + active_findings, component_metadata, manifest, skill_path, @@ -499,10 +597,11 @@ def report(state: SkillspectorState) -> dict[str, object]: has_executable_scripts, use_llm=use_llm, analysis_completeness=analysis_completeness, + suppressed=suppressed, ) elif output_format == "markdown": report_body = _format_markdown( - filtered_findings, + active_findings, component_metadata, manifest, skill_path, @@ -510,14 +609,17 @@ def report(state: SkillspectorState) -> dict[str, object]: risk_severity, risk_recommendation, has_executable_scripts, + suppressed=suppressed, + show_suppressed=show_suppressed, ) else: report_body = json.dumps(sarif_report, indent=2) logger.debug( - "Report generated: format=%s, findings_count=%d", + "Report generated: format=%s, findings_count=%d, suppressed_count=%d", output_format, - len(filtered_findings), + len(active_findings), + len(suppressed), ) out: dict[str, object] = { @@ -527,5 +629,6 @@ def report(state: SkillspectorState) -> dict[str, object]: "risk_recommendation": risk_recommendation, "report_body": report_body, "filtered_findings": filtered_findings, + "suppressed_findings": suppressed, } return out diff --git a/src/skillspector/nodes/resolve_input.py b/src/skillspector/nodes/resolve_input.py index a4e7555b..7324d847 100644 --- a/src/skillspector/nodes/resolve_input.py +++ b/src/skillspector/nodes/resolve_input.py @@ -65,7 +65,7 @@ def resolve_input(state: SkillspectorState) -> dict[str, object]: "temp_dir_for_cleanup": None, } except (OSError, RuntimeError) as e: - logger.warning("Could not resolve skill_path %s: %s", skill_path, e) + logger.warning("Could not resolve skill_path: %s", e) return {"skill_path": None, "temp_dir_for_cleanup": None} return {"skill_path": None, "temp_dir_for_cleanup": None} diff --git a/src/skillspector/sarif_models.py b/src/skillspector/sarif_models.py index ed90ac41..c3256ad8 100644 --- a/src/skillspector/sarif_models.py +++ b/src/skillspector/sarif_models.py @@ -65,6 +65,13 @@ class SarifMessage(BaseModel): text: str +class SarifSuppression(BaseModel): + """SARIF suppression object — marks a result as suppressed (e.g. via a baseline).""" + + kind: Literal["inSource", "external"] = "external" + justification: str | None = None + + class SarifResult(BaseModel): """A single analysis result (finding).""" @@ -74,6 +81,9 @@ class SarifResult(BaseModel): message: SarifMessage level: Literal["error", "warning", "note"] = "warning" locations: list[SarifLocation] + # When present, the result is suppressed; SARIF consumers (e.g. GitHub code + # scanning) exclude suppressed results from counts but keep them for audit. + suppressions: list[SarifSuppression] | None = None class SarifReportingDescriptor(BaseModel): diff --git a/src/skillspector/state.py b/src/skillspector/state.py index 8a21baba..20c3063e 100644 --- a/src/skillspector/state.py +++ b/src/skillspector/state.py @@ -47,6 +47,14 @@ class SkillspectorState(TypedDict, total=False): findings: Annotated[list[Finding], operator.add] filtered_findings: list[Finding] + # Baseline / false-positive suppression. `baseline` is a loaded + # skillspector.suppression.Baseline (set by CLI/API); the report node drops + # matching findings before scoring. `show_suppressed` keeps them in the + # report (marked) for review; `suppressed_findings` is the report output. + baseline: object | None + show_suppressed: bool + suppressed_findings: list[object] + # Model IDs per LLM-using node: e.g. {"default": "...", "meta_analyzer": "..."} model_config: dict[str, str] diff --git a/src/skillspector/suppression.py b/src/skillspector/suppression.py new file mode 100644 index 00000000..f01de61b --- /dev/null +++ b/src/skillspector/suppression.py @@ -0,0 +1,279 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Baseline / false-positive suppression for SkillSpector. + +A *baseline* is a YAML (or JSON) file that tells the report node which findings +to drop before scoring and reporting. It supports two complementary mechanisms: + +* ``rules`` — human-authored, glob-based suppressions. A finding is suppressed + when every field a rule specifies (``id``, ``path``, ``message``) glob-matches + the finding. Unspecified fields match anything. This covers both global + pattern suppression (e.g. ``id: "SQP-1"``) and skill/file-scoped suppression + (e.g. ``id: "SSD-2"`` + ``path: "deploy-topology-execute-scripts/SKILL.md"``). + +* ``fingerprints`` — machine-generated exact suppressions. Each entry is the + stable hash of one known finding, so re-scans only surface *new* findings. + Generate these with ``skillspector baseline `` for incremental CI use. + +Example baseline:: + + version: 1 + rules: + - id: "SQP-1" + reason: "Trigger-phrase breadth is a description nit, not a vuln" + - id: "SSD-2" + path: "*deploy-topology*/SKILL.md" + message: "*run the exploit*" + reason: "False positive: 'run the exploit' is a lab test-workflow phrase" + fingerprints: + - hash: "sha256:1a2b3c4d5e6f7081" + rule_id: "SDI-2" + file: "baas-build-analysis/SKILL.md" + reason: "Accepted 2026-06-19 — first-party env detection" + +Glob semantics use :func:`fnmatch.fnmatch`, where ``*`` matches across path +separators (so ``*SKILL.md`` matches ``a/b/SKILL.md``); ``**`` is accepted as a +friendly alias for ``*``. Message globs are matched case-insensitively, so wrap +a keyword in ``*`` (e.g. ``"*telemetry*"``) for substring matching. +""" + +from __future__ import annotations + +import fnmatch +import hashlib +import json +from dataclasses import dataclass, field +from pathlib import Path +from typing import Any + +import yaml + +from skillspector.logging_config import get_logger +from skillspector.models import Finding + +logger = get_logger(__name__) + +BASELINE_VERSION = 1 + + +def _match_glob(value: str, pattern: str) -> bool: + """Case-insensitive glob match; ``**`` is treated as an alias for ``*``. + + Patterns use :func:`fnmatch.fnmatch` semantics, so ``*``, ``?`` and ``[...]`` + are treated as glob metacharacters. Rule ids and the messages we match are + plain text in practice, but if you ever need to match one of those characters + literally, escape it with :func:`fnmatch.translate` / ``[`` brackets rather + than relying on literal matching here. + """ + normalized = pattern.replace("**", "*") + return fnmatch.fnmatch(value.lower(), normalized.lower()) + + +def finding_fingerprint(finding: Finding) -> str: + """Return a stable short fingerprint for *finding*. + + Derived from rule id, file, line span, and message so the same finding hashes + identically across runs. Note that edits which shift line numbers or reword an + LLM message will change the fingerprint — regenerate the baseline when a skill + changes materially. Use ``rules`` for drift-tolerant suppression. + """ + raw = "|".join( + [ + finding.rule_id or "", + finding.file or "", + str(finding.start_line or ""), + str(finding.end_line or ""), + (finding.message or "").strip(), + ] + ) + digest = hashlib.sha256(raw.encode("utf-8")).hexdigest()[:16] + return f"sha256:{digest}" + + +@dataclass(frozen=True) +class SuppressionRule: + """A glob-based suppression rule. Empty rules (no field set) never match.""" + + rule_id: str | None = None + path: str | None = None + message: str | None = None + reason: str = "" + + def matches(self, finding: Finding) -> bool: + """True when every field this rule specifies glob-matches *finding*.""" + if self.rule_id is None and self.path is None and self.message is None: + return False # guard: an all-wildcard rule would suppress everything + if self.rule_id is not None and not _match_glob(finding.rule_id or "", self.rule_id): + return False + if self.path is not None and not _match_glob(finding.file or "", self.path): + return False + if self.message is not None and not _match_glob(finding.message or "", self.message): + return False + return True + + +@dataclass(frozen=True) +class SuppressedFinding: + """A finding paired with the reason it was suppressed.""" + + finding: Finding + reason: str + + def to_dict(self) -> dict[str, object]: + """JSON-serializable form: the full finding plus its suppression reason.""" + data = self.finding.to_dict() + data["suppressed"] = True + data["suppression_reason"] = self.reason + return data + + +@dataclass +class Baseline: + """Loaded baseline: glob rules plus exact fingerprint suppressions.""" + + rules: list[SuppressionRule] = field(default_factory=list) + fingerprints: dict[str, str] = field(default_factory=dict) # hash -> reason + + def reason_for(self, finding: Finding) -> str | None: + """Return the suppression reason for *finding*, or None if not suppressed.""" + for rule in self.rules: + if rule.matches(finding): + return rule.reason or "matched suppression rule" + fp = finding_fingerprint(finding) + if fp in self.fingerprints: + return self.fingerprints[fp] or "matched baseline fingerprint" + return None + + def is_empty(self) -> bool: + """True when the baseline has no rules and no fingerprints.""" + return not self.rules and not self.fingerprints + + +def baseline_from_dict(data: dict[str, Any]) -> Baseline: + """Build a :class:`Baseline` from a parsed mapping (YAML/JSON).""" + if not isinstance(data, dict): + raise ValueError(f"baseline must be a mapping (got {type(data).__name__})") + + version = data.get("version", BASELINE_VERSION) + if version != BASELINE_VERSION: + logger.warning( + "Baseline version %s does not match supported version %s; attempting to load anyway", + version, + BASELINE_VERSION, + ) + + rules: list[SuppressionRule] = [] + for raw in data.get("rules") or []: + if not isinstance(raw, dict): + raise ValueError(f"each baseline rule must be a mapping, got: {raw!r}") + rule = SuppressionRule( + rule_id=raw.get("id") or raw.get("rule_id"), + path=raw.get("path") or raw.get("file"), + message=raw.get("message"), + reason=raw.get("reason", ""), + ) + if rule.rule_id is None and rule.path is None and rule.message is None: + raise ValueError( + "a baseline rule must set at least one of: id, path, message " + f"(offending rule: {raw!r})" + ) + rules.append(rule) + + fingerprints: dict[str, str] = {} + for raw in data.get("fingerprints") or []: + if isinstance(raw, str): + fingerprints[raw] = "" + elif isinstance(raw, dict) and raw.get("hash"): + fingerprints[str(raw["hash"])] = raw.get("reason", "") + else: + raise ValueError( + f"each fingerprint must be a string or have a 'hash' key, got: {raw!r}" + ) + + return Baseline(rules=rules, fingerprints=fingerprints) + + +def load_baseline(path: str | Path) -> Baseline: + """Load a baseline file (YAML or JSON) into a :class:`Baseline`. + + Raises FileNotFoundError if *path* is missing, ValueError if it is malformed. + """ + p = Path(path) + if not p.exists(): + raise FileNotFoundError(f"Baseline file not found: {p}") + text = p.read_text(encoding="utf-8") + try: + # yaml.safe_load parses JSON too, so a single path handles both formats. + data = yaml.safe_load(text) or {} + except yaml.YAMLError as e: # pragma: no cover - error path + raise ValueError(f"Could not parse baseline file {p}: {e}") from e + return baseline_from_dict(data) + + +def partition_findings( + findings: list[Finding], baseline: Baseline | None +) -> tuple[list[Finding], list[SuppressedFinding]]: + """Split *findings* into (kept, suppressed) using *baseline*. + + With no baseline, everything is kept. Suppressed findings never count toward + the risk score and are excluded from the SARIF results. + """ + if baseline is None or baseline.is_empty(): + return list(findings), [] + kept: list[Finding] = [] + suppressed: list[SuppressedFinding] = [] + for finding in findings: + reason = baseline.reason_for(finding) + if reason is None: + kept.append(finding) + else: + suppressed.append(SuppressedFinding(finding=finding, reason=reason)) + if suppressed: + logger.debug("Suppressed %d finding(s) via baseline", len(suppressed)) + return kept, suppressed + + +def build_baseline_dict( + findings: list[Finding], + reason: str = "Accepted finding (auto-generated baseline)", +) -> dict[str, object]: + """Build a baseline mapping that fingerprint-suppresses every given finding.""" + return { + "version": BASELINE_VERSION, + "rules": [], + "fingerprints": [ + { + "hash": finding_fingerprint(f), + "rule_id": f.rule_id, + "file": f.file, + "reason": reason, + } + for f in findings + ], + } + + +def dump_baseline(data: dict[str, object], path: str | Path) -> None: + """Write a baseline mapping to *path* as YAML (``.json`` extension -> JSON).""" + p = Path(path) + if p.suffix.lower() == ".json": + p.write_text(json.dumps(data, indent=2), encoding="utf-8") + else: + header = ( + "# SkillSpector baseline — findings listed here are suppressed on future scans.\n" + "# Edit 'reason' fields and add glob 'rules' as needed. See docs/SUPPRESSION.md.\n" + ) + p.write_text(header + yaml.safe_dump(data, sort_keys=False), encoding="utf-8") diff --git a/tests/integration/__init__.py b/tests/integration/__init__.py index 1275d78d..46707983 100644 --- a/tests/integration/__init__.py +++ b/tests/integration/__init__.py @@ -12,4 +12,3 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - diff --git a/tests/nodes/analyzers/test_binary_and_pe3_filtering.py b/tests/nodes/analyzers/test_binary_and_pe3_filtering.py new file mode 100644 index 00000000..305a3f08 --- /dev/null +++ b/tests/nodes/analyzers/test_binary_and_pe3_filtering.py @@ -0,0 +1,281 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Tests for binary file skipping and PE3 .env documentation reference filtering.""" + +from __future__ import annotations + +from unittest.mock import MagicMock + +from skillspector.models import AnalyzerFinding, Location, Severity +from skillspector.nodes.analyzers.static_runner import ( + _is_binary_file, + _is_env_file_reference_in_docs, + run_static_patterns, +) + + +def _make_pe3_finding(context: str) -> AnalyzerFinding: + return AnalyzerFinding( + rule_id="PE3", + message="Credential Access", + severity=Severity.HIGH, + location=Location(file="docs/setup.md", start_line=10), + confidence=0.6, + tags=["privilege_escalation"], + context=context, + matched_text=".env", + ) + + +class TestBinaryFileDetection: + """Binary files are correctly identified and skipped.""" + + def test_pdf_extension_detected(self) -> None: + assert _is_binary_file("report.pdf", "some content") is True + + def test_png_extension_detected(self) -> None: + assert _is_binary_file("image.png", "fake data") is True + + def test_zip_extension_detected(self) -> None: + assert _is_binary_file("archive.zip", "PK\x03\x04") is True + + def test_exe_extension_detected(self) -> None: + assert _is_binary_file("tool.exe", "MZ") is True + + def test_markdown_not_binary(self) -> None: + assert _is_binary_file("README.md", "# Hello\n") is False + + def test_python_not_binary(self) -> None: + assert _is_binary_file("tool.py", "import os\n") is False + + def test_null_byte_in_content_detected(self) -> None: + content = "start\x00binary\x00data" + assert _is_binary_file("unknownfile", content) is True + + def test_no_null_byte_not_binary(self) -> None: + assert _is_binary_file("unknownfile", "normal text content") is False + + def test_case_insensitive_extension(self) -> None: + assert _is_binary_file("photo.JPEG", "data") is True + assert _is_binary_file("archive.ZIP", "PK") is True + + def test_svg_not_treated_as_binary(self) -> None: + """SVG is text/XML and can carry