diff --git a/README.md b/README.md index 4a09b50b..6a2349ce 100644 --- a/README.md +++ b/README.md @@ -651,10 +651,18 @@ SkillSpector uses a two-stage detection pipeline: - Fast regex-based pattern matching across 11 static analyzers - AST-based behavioral analysis detecting dangerous calls (exec, eval, subprocess, etc.) - Live vulnerability lookups via OSV.dev for known CVEs in dependencies -- Scans all files in the skill +- Scans all analyzer-eligible files in the skill - High recall (catches most issues) - Moderate precision (some false positives) +A valid, root-level OpenSSF Model Signing signature (`skill.oms.sig`) is retained in the +component inventory as type `oms_signature`, but excluded from static and LLM content analysis. +OMS bundles necessarily contain long base64-encoded payload, signature, and certificate fields; +generic obfuscated-code checks can otherwise misclassify those fields as hidden executable content. +The recognizer checks the minimal OMS DSSE/in-toto structure; it does not verify the signature, +certificate chain, transparency-log entry, or signer identity. Invalid or unrecognized signature +files are scanned normally. + ### Stage 2: LLM Semantic Analysis (Optional) - Evaluates context and intent - Filters false positives @@ -679,7 +687,7 @@ The tool requires outbound HTTPS access to `api.osv.dev` for live vulnerability SkillSpector is defense-in-depth, not a sandbox. Know what it does and does not do before relying on it: - **It never executes the scanned skill.** All analysis is static (regex, Python AST, YARA) plus optional LLM evaluation of file *contents* — the skill's code is never run. -- **LLM analysis sends file contents to the configured provider.** When LLM analysis is enabled (the default), file contents are sent to the active `SKILLSPECTOR_PROVIDER` endpoint. Use `--no-llm` to keep contents local (static analysis only). +- **LLM analysis sends analyzer-eligible file contents to the configured provider.** When LLM analysis is enabled (the default), file contents are sent to the active `SKILLSPECTOR_PROVIDER` endpoint. Recognized OMS signature files are excluded. Use `--no-llm` to keep contents local (static analysis only). - **SC4 sends dependency names to OSV.dev.** The supply-chain check queries [OSV.dev](https://osv.dev) with the package names and versions the skill declares, to look up known CVEs. This is fundamental to the check and runs even with `--no-llm`. It sends dependency coordinates (not file contents), requires no API key, and falls back to a bundled list when OSV.dev is unreachable. - **It does not sandbox the host.** SkillSpector flags risky patterns *before* you install a skill; it does not contain or isolate a skill you choose to install anyway. diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index 65bdc9a8..a9d391cd 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -79,6 +79,7 @@ All targets assume the virtual environment is **already created and activated**. | `zip_bytes`, `mode` | Optional zip input and scan mode | | `components` | List of relative file paths in the skill | | `file_cache` | Map of path → file contents | +| `analysis_excluded_components` | Inventoried paths intentionally withheld from analyzers (for example, a recognized OMS signature) | | `ast_cache` | Map of path → AST representation (for future use) | | `manifest`, `previous_manifest` | Parsed skill metadata (e.g. from SKILL.md) | | `component_metadata` | List of dicts: path, type, lines, executable, size_bytes (from build_context) | diff --git a/src/skillspector/constants.py b/src/skillspector/constants.py index c7e0d5e6..3981e48c 100644 --- a/src/skillspector/constants.py +++ b/src/skillspector/constants.py @@ -28,6 +28,9 @@ DEFAULT_CONTEXT_LENGTH = 128_000 # Risk score threshold above which a scan is treated as unsafe. RISK_THRESHOLD = 50 +# Maximum text-file size processed by static analyzers and lightweight +# format recognizers. +MAX_FILE_BYTES = 1_000_000 # Default-model selection lives on each provider (see providers//provider.py # for ``DEFAULT_MODEL`` and ``SLOT_DEFAULTS``). The active provider's diff --git a/src/skillspector/nodes/analyzers/semantic_security_discovery.py b/src/skillspector/nodes/analyzers/semantic_security_discovery.py index 72a0dde1..2e130a12 100644 --- a/src/skillspector/nodes/analyzers/semantic_security_discovery.py +++ b/src/skillspector/nodes/analyzers/semantic_security_discovery.py @@ -75,7 +75,10 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: return {"findings": []} file_cache: dict[str, str] = state.get("file_cache") or {} - components: list[str] = state.get("components") or sorted(file_cache.keys()) + requested_components: list[str] = state.get("components") or sorted(file_cache.keys()) + # Components intentionally excluded from file_cache (for example a + # recognized OMS signature) must not become placeholder LLM batches. + components = [path for path in requested_components if path in file_cache] if not components: return {"findings": []} diff --git a/src/skillspector/nodes/analyzers/static_runner.py b/src/skillspector/nodes/analyzers/static_runner.py index ccd10e98..b0c6b79a 100644 --- a/src/skillspector/nodes/analyzers/static_runner.py +++ b/src/skillspector/nodes/analyzers/static_runner.py @@ -20,6 +20,7 @@ import re from collections.abc import Callable +from skillspector.constants import MAX_FILE_BYTES from skillspector.logging_config import get_logger from skillspector.models import AnalyzerFinding, Finding @@ -48,7 +49,6 @@ ".rs": "rust", } -MAX_FILE_BYTES = 1_000_000 _EVAL_DATASET_FILES = { "evals/evals.json", "evals/evals.jsonl", diff --git a/src/skillspector/nodes/build_context.py b/src/skillspector/nodes/build_context.py index a905844a..491706ad 100644 --- a/src/skillspector/nodes/build_context.py +++ b/src/skillspector/nodes/build_context.py @@ -21,12 +21,15 @@ from __future__ import annotations +import base64 +import binascii +import json import re from pathlib import Path import yaml -from skillspector.constants import MODEL_CONFIG +from skillspector.constants import MAX_FILE_BYTES, MODEL_CONFIG from skillspector.logging_config import get_logger from skillspector.state import SkillspectorState @@ -60,6 +63,12 @@ {".py", ".sh", ".bash", ".zsh", ".js", ".ts", ".rb", ".go", ".rs", ".pl"} ) +_OMS_SIGNATURE_PATH = "skill.oms.sig" +_SIGSTORE_BUNDLE_MEDIA_TYPE = "application/vnd.dev.sigstore.bundle.v0.3+json" +_IN_TOTO_PAYLOAD_TYPE = "application/vnd.in-toto+json" +_IN_TOTO_STATEMENT_TYPE = "https://in-toto.io/Statement/v1" +_OMS_PREDICATE_TYPE = "https://model_signing/signature/v1.0" + def _resolve_skill_dir(state: SkillspectorState) -> Path: """Resolve state skill_path to an existing directory Path.""" @@ -108,6 +117,68 @@ def _infer_file_type(path: str) -> str: return _FILE_TYPES.get(suffix, "other") +def _decode_base64_json(value: object) -> dict[str, object] | None: + """Decode a strict base64 JSON object, returning ``None`` on malformed input.""" + if not isinstance(value, str) or not value: + return None + try: + decoded = base64.b64decode(value, validate=True) + parsed = json.loads(decoded.decode("utf-8")) + except (binascii.Error, UnicodeDecodeError, json.JSONDecodeError): + return None + return parsed if isinstance(parsed, dict) else None + + +def _is_valid_oms_signature(file_path: Path) -> bool: + """Recognize the minimal root-level OMS DSSE/in-toto signature structure. + + This intentionally does not parse verification material or verify the + cryptographic signature. Its purpose is to distinguish detached OMS + metadata from agent-facing content before analyzers inspect the skill. + """ + try: + if file_path.stat().st_size > MAX_FILE_BYTES: + return False + content = file_path.read_text(encoding="utf-8") + bundle = json.loads(content) + except (OSError, UnicodeDecodeError, json.JSONDecodeError): + return False + + if not isinstance(bundle, dict): + return False + if bundle.get("mediaType") != _SIGSTORE_BUNDLE_MEDIA_TYPE: + return False + if not isinstance(bundle.get("verificationMaterial"), dict): + return False + + envelope = bundle.get("dsseEnvelope") + if not isinstance(envelope, dict): + return False + if envelope.get("payloadType") != _IN_TOTO_PAYLOAD_TYPE: + return False + + signatures = envelope.get("signatures") + if not isinstance(signatures, list) or len(signatures) != 1: + return False + signature = signatures[0] + if not isinstance(signature, dict): + return False + signature_bytes = signature.get("sig") + if not isinstance(signature_bytes, str) or not signature_bytes: + return False + try: + base64.b64decode(signature_bytes, validate=True) + except (binascii.Error, ValueError): + return False + + statement = _decode_base64_json(envelope.get("payload")) + return bool( + statement + and statement.get("_type") == _IN_TOTO_STATEMENT_TYPE + and statement.get("predicateType") == _OMS_PREDICATE_TYPE + ) + + def _count_lines(file_path: Path) -> int: """Count lines in a file, handling binary and errors gracefully.""" try: @@ -119,7 +190,9 @@ def _count_lines(file_path: Path) -> int: def _build_component_metadata( - skill_dir: Path, components: list[str] + skill_dir: Path, + components: list[str], + recognized_oms_signatures: frozenset[str] = frozenset(), ) -> tuple[list[dict[str, object]], bool]: """Build component_metadata list and has_executable_scripts from paths.""" metadata: list[dict[str, object]] = [] @@ -129,7 +202,7 @@ def _build_component_metadata( if not full.is_file(): continue suffix = full.suffix.lower() - file_type = _infer_file_type(path) + file_type = "oms_signature" if path in recognized_oms_signatures else _infer_file_type(path) lines = _count_lines(full) executable = suffix in _EXECUTABLE_EXTENSIONS if executable: @@ -151,10 +224,16 @@ def _build_component_metadata( return metadata, has_executable -def _read_file_cache(skill_dir: Path, components: list[str]) -> dict[str, str]: +def _read_file_cache( + skill_dir: Path, + components: list[str], + excluded_paths: frozenset[str] = frozenset(), +) -> dict[str, str]: """Build file_cache: relative path -> file contents. Uses utf-8 with replace for errors.""" file_cache: dict[str, str] = {} for path in components: + if path in excluded_paths: + continue full = skill_dir / path if not full.is_file(): continue @@ -236,13 +315,22 @@ def build_context(state: SkillspectorState) -> dict[str, object]: skill_dir = _resolve_skill_dir(state) components = _walk_skill_files(skill_dir) - file_cache = _read_file_cache(skill_dir, components) + recognized_oms_signatures = frozenset( + {_OMS_SIGNATURE_PATH} + if _OMS_SIGNATURE_PATH in components + and _is_valid_oms_signature(skill_dir / _OMS_SIGNATURE_PATH) + else set() + ) + file_cache = _read_file_cache(skill_dir, components, recognized_oms_signatures) manifest = _parse_manifest(skill_dir) - component_metadata, has_executable_scripts = _build_component_metadata(skill_dir, components) + component_metadata, has_executable_scripts = _build_component_metadata( + skill_dir, components, recognized_oms_signatures + ) return { "components": components, "file_cache": file_cache, + "analysis_excluded_components": sorted(recognized_oms_signatures), "ast_cache": {}, "manifest": manifest, "previous_manifest": None, diff --git a/src/skillspector/nodes/report.py b/src/skillspector/nodes/report.py index 95160398..6c6fb497 100644 --- a/src/skillspector/nodes/report.py +++ b/src/skillspector/nodes/report.py @@ -495,21 +495,26 @@ def _build_analysis_completeness( use_llm: bool, findings_pre_filter: list[Finding], findings_post_filter: list[Finding], + excluded_components: list[str] | None = None, ) -> dict[str, object]: """Build analysis_completeness section indicating scan coverage and limitations. Helps consumers understand what was NOT analyzed and whether findings can be trusted as comprehensive. """ + excluded = frozenset(excluded_components or []) total_components = len(components) - scanned_components = sum(1 for c in components if c in file_cache) + missing_components = [c for c in components if c not in file_cache and c not in excluded] + # Intentionally excluded metadata is fully accounted for by the scan even + # though analyzers do not inspect its content. + scanned_components = total_components - len(missing_components) llm_available, llm_error = is_llm_available() llm_used = use_llm and llm_available limitations: list[str] = [] - if scanned_components < total_components: - skipped = total_components - scanned_components + if missing_components: + skipped = len(missing_components) limitations.append(f"{skipped} component(s) had no content in file_cache (skipped)") if use_llm and not llm_available: limitations.append(f"LLM meta-analysis unavailable: {llm_error or 'unknown reason'}") @@ -727,7 +732,12 @@ def report(state: SkillspectorState) -> dict[str, object]: ) sarif_report = _build_sarif(active_findings, suppressed, degraded_notice=degraded_notice) analysis_completeness = _build_analysis_completeness( - components, file_cache, use_llm, raw_findings, filtered_findings + components, + file_cache, + use_llm, + raw_findings, + filtered_findings, + state.get("analysis_excluded_components") or [], ) # Fail closed on a degraded deep scan: when the LLM stage was requested but diff --git a/src/skillspector/state.py b/src/skillspector/state.py index 68d41d91..ddf488d5 100644 --- a/src/skillspector/state.py +++ b/src/skillspector/state.py @@ -39,6 +39,9 @@ class SkillspectorState(TypedDict, total=False): # build_context node populates these components: list[str] file_cache: dict[str, str] + # Components inventoried for reporting but intentionally omitted from all + # analyzer inputs (for example, a recognized root-level OMS signature). + analysis_excluded_components: list[str] ast_cache: dict[str, str] manifest: dict[str, object] previous_manifest: dict[str, object] | None diff --git a/tests/fixtures/oms/mcore-split-pr.skill.oms.sig b/tests/fixtures/oms/mcore-split-pr.skill.oms.sig new file mode 100644 index 00000000..74630cc1 --- /dev/null +++ b/tests/fixtures/oms/mcore-split-pr.skill.oms.sig @@ -0,0 +1 @@ +{"mediaType":"application/vnd.dev.sigstore.bundle.v0.3+json","verificationMaterial":{"x509CertificateChain":{"certificates":[{"rawBytes":"MIICgzCCAgmgAwIBAgIUKIyS7SxNteQIiWzK1dWj85E6520wCgYIKoZIzj0EAwMwVTELMAkGA1UEBhMCVVMxGzAZBgNVBAoMEk5WSURJQSBDb3Jwb3JhdGlvbjEpMCcGA1UEAwwgTlZJRElBIEFnZW50IENhcGFiaWxpdGllcyBJQ0EgMDEwHhcNMjYwNDAxMDAwMDAwWhcNMjgwNDIyMTUzMzA5WjBUMQswCQYDVQQGEwJVUzEbMBkGA1UECgwSTlZJRElBIENvcnBvcmF0aW9uMSgwJgYDVQQDDB9OVklESUEgQWdlbnQgU2tpbGxzIFNpZ25pbmcgMDAxMHYwEAYHKoZIzj0CAQYFK4EEACIDYgAEYoRM9bQl/dGlwSRNi6bTpIJUXH8Nv9GciP6LSflJYYMLCc296kpyuTSsk5ddbAWiDcFX3C/ydX3jwc+qCLYP6uHy9XphyLjOQ27Yb2J6rBLVtRBS1mgGco/Gr7fL6ODco4GaMIGXMB0GA1UdDgQWBBRQ/5ZW3nJ6lmo9SVk7I15o7UGmpTAfBgNVHSMEGDAWgBRPGpILxMBBleJSsBGjrMKsby1CgjAMBgNVHRMBAf8EAjAAMA4GA1UdDwEB/wQEAwIHgDA3BggrBgEFBQcBAQQrMCkwJwYIKwYBBQUHMAGGG2h0dHA6Ly9vY3NwLm5kaXMubnZpZGlhLmNvbTAKBggqhkjOPQQDAwNoADBlAjAUygu/GiOCIXrgGr4SmLgeEVDcEitfFUv7ALbvLVGVyMysB3mxmO/uInZfXzWcJZsCMQDxuoxj4ZmO30jhkPIcCxGFCOvnUsnfU3TfGcouYm4M6iRpbKvtVnHPiy4bi6pcKf0="},{"rawBytes":"MIICiDCCAg6gAwIBAgIUZsIuSv9NkpJCNqtYEfCouVv5BzowCgYIKoZIzj0EAwMwUTELMAkGA1UEBhMCVVMxGzAZBgNVBAoMEk5WSURJQSBDb3Jwb3JhdGlvbjElMCMGA1UEAwwcTlZJRElBIEFnZW50IENhcGFiaWxpdGllcyBDQTAgFw0yNjA0MDEwMDAwMDBaGA85OTk5MTIzMTIzNTk1OVowVTELMAkGA1UEBhMCVVMxGzAZBgNVBAoMEk5WSURJQSBDb3Jwb3JhdGlvbjEpMCcGA1UEAwwgTlZJRElBIEFnZW50IENhcGFiaWxpdGllcyBJQ0EgMDEwdjAQBgcqhkjOPQIBBgUrgQQAIgNiAASI72cR3ctKGg4VWnB3bNja6g1Z2PnOmFEopkPof+QeIcPk9rT+g9MjJnq51EQXL93a7C2GJ9J985G4o2V85VD7wJ1RaXhluHW2rf3y8bQGeAYaKMr5s/hUgn+M3/9WlWejgaAwgZ0wHQYDVR0OBBYEFE8akgvEwEGV4lKwEaOswqxvLUKCMB8GA1UdIwQYMBaAFItnoAjjfuCEUvzyvWyI2vOGvwPjMBIGA1UdEwEB/wQIMAYBAf8CAQAwDgYDVR0PAQH/BAQDAgEGMDcGCCsGAQUFBwEBBCswKTAnBggrBgEFBQcwAYYbaHR0cDovL29jc3AubmRpcy5udmlkaWEuY29tMAoGCCqGSM49BAMDA2gAMGUCMQCeIMMfAbyzPDacw2MxG+Yt1cikrJX/DVxiGfXuHmkkXn6VgSzE79+lkqDErpVO2gYCMCNEColOyvUvkzZGUEI1hQ3PfMgi3FIo9tHoBKMw4/wGBLFpu/0ubtmbBXM6/UMOEw=="},{"rawBytes":"MIICRTCCAcygAwIBAgIUeJdY3rV86EdvFmG7L8LJBsyQFYkwCgYIKoZIzj0EAwMwUTELMAkGA1UEBhMCVVMxGzAZBgNVBAoMEk5WSURJQSBDb3Jwb3JhdGlvbjElMCMGA1UEAwwcTlZJRElBIEFnZW50IENhcGFiaWxpdGllcyBDQTAgFw0yNjA0MDEwMDAwMDBaGA85OTk5MTIzMTIzNTk1OVowUTELMAkGA1UEBhMCVVMxGzAZBgNVBAoMEk5WSURJQSBDb3Jwb3JhdGlvbjElMCMGA1UEAwwcTlZJRElBIEFnZW50IENhcGFiaWxpdGllcyBDQTB2MBAGByqGSM49AgEGBSuBBAAiA2IABAYpiXCDjJ9NT2eSDhyHJVSw1Tbze18cGG2F/578oWvHxg23eQAhNRYdq88i1iOshZSO6C29doKui5Xpmo/7Ctw9Sx4PP2RzOmIuOLCuTdNtKcTRwi4GEsd5BAFvWj42M6NjMGEwHQYDVR0OBBYEFItnoAjjfuCEUvzyvWyI2vOGvwPjMB8GA1UdIwQYMBaAFItnoAjjfuCEUvzyvWyI2vOGvwPjMA8GA1UdEwEB/wQFMAMBAf8wDgYDVR0PAQH/BAQDAgEGMAoGCCqGSM49BAMDA2cAMGQCMCwtAjWLaNwgGWNCgdyNoTyvNhqWRECRJV2r3+7w8g0PL6NHLOsbkgE09BH95h8XlgIwTaQmbbUh2ChAJ5TA1wRiVDnCcvbzHlZl2jM2FcwQQZlk19LOAbyGMRixbu2Ww/rj"}]},"tlogEntries":[]},"dsseEnvelope":{"payload":"ewogICJfdHlwZSI6ICJodHRwczovL2luLXRvdG8uaW8vU3RhdGVtZW50L3YxIiwKICAic3ViamVjdCI6IFsKICAgIHsKICAgICAgIm5hbWUiOiAibWNvcmUtc3BsaXQtcHIiLAogICAgICAiZGlnZXN0IjogewogICAgICAgICJzaGEyNTYiOiAiNWMzNDc0ZGViZWJhOWYxYWNlYTg0Y2ZjNGJhMmY4YzA0MGExMTU2YWFkYjhlMjlmMzhlZGZiMTgxMTE1Y2JkMiIKICAgICAgfQogICAgfQogIF0sCiAgInByZWRpY2F0ZVR5cGUiOiAiaHR0cHM6Ly9tb2RlbF9zaWduaW5nL3NpZ25hdHVyZS92MS4wIiwKICAicHJlZGljYXRlIjogewogICAgInNlcmlhbGl6YXRpb24iOiB7CiAgICAgICJhbGxvd19zeW1saW5rcyI6IGZhbHNlLAogICAgICAiaWdub3JlX3BhdGhzIjogWwogICAgICAgICIuZ2l0YXR0cmlidXRlcyIsCiAgICAgICAgIi5naXRodWIiLAogICAgICAgICIuZ2l0IiwKICAgICAgICAiLmdpdGlnbm9yZSIKICAgICAgXSwKICAgICAgIm1ldGhvZCI6ICJmaWxlcyIsCiAgICAgICJoYXNoX3R5cGUiOiAic2hhMjU2IgogICAgfSwKICAgICJyZXNvdXJjZXMiOiBbCiAgICAgIHsKICAgICAgICAiZGlnZXN0IjogIjUxOWRmZDI1NGQ3NDU4MzJlYWU5MzRhNGNjOThlZGExN2NjYzljNGQ0MjgwM2U2MzJiYmE2OTJkMjE3ZjIwODciLAogICAgICAgICJhbGdvcml0aG0iOiAic2hhMjU2IiwKICAgICAgICAibmFtZSI6ICJCRU5DSE1BUksubWQiCiAgICAgIH0sCiAgICAgIHsKICAgICAgICAiZGlnZXN0IjogIjBmM2Q0MDkyMmQzYzU3ZWI2ZWJhMWVkNzRiOGI2Y2RiM2U4N2E0NjljMWU2YWZkNDE1NDFkMmZlZDcyZTIxYTAiLAogICAgICAgICJhbGdvcml0aG0iOiAic2hhMjU2IiwKICAgICAgICAibmFtZSI6ICJTS0lMTC5tZCIKICAgICAgfSwKICAgICAgewogICAgICAgICJkaWdlc3QiOiAiMzc1MTdlNWYzZGM2NjgxOWY2MWY1YTdiYjhhY2UxOTIxMjgyNDE1ZjEwNTUxZDJkZWZhNWMzZWIwOTg1YjU3MCIsCiAgICAgICAgImFsZ29yaXRobSI6ICJzaGEyNTYiLAogICAgICAgICJuYW1lIjogImV2YWxzL2V2YWxzLmpzb24iCiAgICAgIH0sCiAgICAgIHsKICAgICAgICAiZGlnZXN0IjogImJjY2MxZTk0YWJmZDc0NDcyMjdjZmU5MDg0N2U1MTE5YWJiMTA0Y2UzOGE0NzAxYmIzYjVkM2JlOWRjYjVlZGIiLAogICAgICAgICJhbGdvcml0aG0iOiAic2hhMjU2IiwKICAgICAgICAibmFtZSI6ICJza2lsbC1jYXJkLm1kIgogICAgICB9CiAgICBdCiAgfQp9","payloadType":"application/vnd.in-toto+json","signatures":[{"sig":"MGQCMBexfplum16efCE5Z+c2uymUa1slPFLC+0CJV5i+Gl9DZnzsUStO5pEnlz5N/BlL8wIwS9tDLOLlxp0c0vYTjV2EEnVRv1zjrnHxUbD2jFrZpE7my7zwTcRW28UWqWBS3N6H","keyid":""}]}} diff --git a/tests/integration/test_graph.py b/tests/integration/test_graph.py index ad7db1d8..a7af7b12 100644 --- a/tests/integration/test_graph.py +++ b/tests/integration/test_graph.py @@ -42,6 +42,34 @@ def test_graph_invoke_with_output_format_json(tmp_path: Path) -> None: assert "components" in data +def test_graph_excludes_valid_oms_signature_from_static_findings(tmp_path: Path) -> None: + """A real OMS signature remains inventoried without producing scan findings.""" + fixture = Path(__file__).parents[1] / "fixtures" / "oms" / "mcore-split-pr.skill.oms.sig" + (tmp_path / "SKILL.md").write_text("---\nname: signed\n---\n# Signed\n", encoding="utf-8") + (tmp_path / "skill.oms.sig").write_text(fixture.read_text(encoding="utf-8"), encoding="utf-8") + + result = graph.invoke( + { + "skill_path": str(tmp_path), + "output_format": "json", + "use_llm": False, + } + ) + + report = json.loads(result["report_body"]) + signature_component = next( + component for component in report["components"] if component["path"] == "skill.oms.sig" + ) + assert signature_component["type"] == "oms_signature" + assert report["analysis_completeness"]["coverage_percent"] == 100.0 + assert not any( + "no content in file_cache" in limitation + for limitation in (report["analysis_completeness"]["limitations"] or []) + ) + assert all(finding.file != "skill.oms.sig" for finding in result["findings"]) + assert all(issue["file"] != "skill.oms.sig" for issue in report["issues"]) + + def test_graph_invoke_returns_findings_and_report(tmp_path: Path) -> None: """Graph runs to completion; returns findings, SARIF report, report_body, risk_score.""" result = graph.invoke({"skill_path": str(tmp_path), "use_llm": False}) diff --git a/tests/nodes/analyzers/test_semantic_security_discovery.py b/tests/nodes/analyzers/test_semantic_security_discovery.py index ec77aded..7b907752 100644 --- a/tests/nodes/analyzers/test_semantic_security_discovery.py +++ b/tests/nodes/analyzers/test_semantic_security_discovery.py @@ -168,6 +168,19 @@ def test_single_file_one_batch(self, base_state) -> None: assert len(batches_arg) == 1 assert batches_arg[0].file_path == "SKILL.md" + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) + def test_component_excluded_from_file_cache_is_not_batched(self, base_state) -> None: + """Recognized OMS metadata never becomes an LLM placeholder batch.""" + base_state["components"] = ["SKILL.md", "skill.oms.sig"] + + from skillspector.llm_analyzer_base import LLMAnalyzerBase + + with patch.object(LLMAnalyzerBase, "run_batches", return_value=[]) as mock_run: + node(base_state) + + batches_arg = mock_run.call_args[0][0] + assert [batch.file_path for batch in batches_arg] == ["SKILL.md"] + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) def test_oversized_file_multiple_batches(self, base_state) -> None: # A file large enough to exceed the reduced token budget diff --git a/tests/nodes/test_analysis_completeness.py b/tests/nodes/test_analysis_completeness.py index 4e517eff..e7bc55ba 100644 --- a/tests/nodes/test_analysis_completeness.py +++ b/tests/nodes/test_analysis_completeness.py @@ -89,6 +89,23 @@ def test_partial_coverage_reports_skipped(self) -> None: assert result["is_complete"] is False assert any("2 component(s)" in lim for lim in result["limitations"]) + def test_intentionally_excluded_component_counts_as_covered(self) -> None: + """Recognized metadata exclusions do not make an otherwise complete scan partial.""" + with patch("skillspector.nodes.report.is_llm_available", return_value=(True, None)): + result = _build_analysis_completeness( + ["SKILL.md", "skill.oms.sig"], + {"SKILL.md": "# Skill"}, + use_llm=True, + findings_pre_filter=[], + findings_post_filter=[], + excluded_components=["skill.oms.sig"], + ) + assert result["total_components"] == 2 + assert result["scanned_components"] == 2 + assert result["coverage_percent"] == 100.0 + assert result["limitations"] is None + assert result["is_complete"] is True + def test_llm_unavailable_noted(self) -> None: with patch( "skillspector.nodes.report.is_llm_available", diff --git a/tests/nodes/test_build_context.py b/tests/nodes/test_build_context.py index d9daca67..92f63967 100644 --- a/tests/nodes/test_build_context.py +++ b/tests/nodes/test_build_context.py @@ -20,6 +20,7 @@ from __future__ import annotations +import json from pathlib import Path import pytest @@ -28,6 +29,17 @@ from skillspector.nodes.build_context import build_context from skillspector.state import SkillspectorState +_OMS_FIXTURE = Path(__file__).parents[1] / "fixtures" / "oms" / "mcore-split-pr.skill.oms.sig" +# Pinned from NVIDIA/skills at commit 1f01acfe1aece58ba95d124eafdfb5bb93523db6: +# skills/mcore-split-pr/skill.oms.sig + + +def _write_real_oms_signature(root: Path, relative_path: str = "skill.oms.sig") -> Path: + target = root / relative_path + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(_OMS_FIXTURE.read_text(encoding="utf-8"), encoding="utf-8") + return target + def _make_skill_spec_dir(root: Path, *, skill_md_name: str = "SKILL.md") -> None: """Populate root with skill spec: SKILL.md, references/, scripts/, assets/.""" @@ -131,6 +143,75 @@ def test_build_context_empty_directory_is_valid_empty_scan(tmp_path: Path) -> No assert result["model_config"] == MODEL_CONFIG +def test_build_context_inventories_but_excludes_valid_root_oms_signature( + tmp_path: Path, +) -> None: + """A real OMS signature is reported as metadata but withheld from analyzers.""" + (tmp_path / "SKILL.md").write_text("---\nname: signed\n---\n# Signed\n", encoding="utf-8") + signature_path = _write_real_oms_signature(tmp_path) + + result = build_context({"skill_path": str(tmp_path)}) + + assert "skill.oms.sig" in result["components"] + assert "skill.oms.sig" not in result["file_cache"] + assert result["analysis_excluded_components"] == ["skill.oms.sig"] + signature_meta = next( + item for item in result["component_metadata"] if item["path"] == "skill.oms.sig" + ) + assert signature_meta == { + "path": "skill.oms.sig", + "type": "oms_signature", + "lines": 1, + "executable": False, + "size_bytes": signature_path.stat().st_size, + } + + +@pytest.mark.parametrize( + "invalid_case", ["malformed_json", "wrong_media_type", "message_signature"] +) +def test_build_context_scans_unrecognized_root_oms_signature( + tmp_path: Path, + invalid_case: str, +) -> None: + """Malformed and non-OMS Sigstore files retain normal scanner behavior.""" + content = _OMS_FIXTURE.read_text(encoding="utf-8") + if invalid_case == "malformed_json": + content = "{not-json" + else: + bundle = json.loads(content) + if invalid_case == "wrong_media_type": + bundle["mediaType"] = "application/vnd.dev.sigstore.bundle.v0.2+json" + else: + bundle["messageSignature"] = {"signature": "YWJj"} + del bundle["dsseEnvelope"] + content = json.dumps(bundle) + (tmp_path / "skill.oms.sig").write_text(content, encoding="utf-8") + + result = build_context({"skill_path": str(tmp_path)}) + + assert result["file_cache"]["skill.oms.sig"] == content + assert result["analysis_excluded_components"] == [] + signature_meta = next( + item for item in result["component_metadata"] if item["path"] == "skill.oms.sig" + ) + assert signature_meta["type"] == "other" + + +def test_build_context_scans_nested_oms_signature(tmp_path: Path) -> None: + """Only the signature at the skill root is eligible for recognition.""" + nested = _write_real_oms_signature(tmp_path, "nested/skill.oms.sig") + + result = build_context({"skill_path": str(tmp_path)}) + + assert result["file_cache"]["nested/skill.oms.sig"] == nested.read_text(encoding="utf-8") + assert result["analysis_excluded_components"] == [] + signature_meta = next( + item for item in result["component_metadata"] if item["path"] == "nested/skill.oms.sig" + ) + assert signature_meta["type"] == "other" + + def test_build_context_skips_skip_dirs(tmp_path: Path) -> None: """Skip dirs like __pycache__ and node_modules are not included in components.""" _make_skill_spec_dir(tmp_path)