diff --git a/README.md b/README.md index 40fe48c16..754304038 100644 --- a/README.md +++ b/README.md @@ -541,7 +541,7 @@ SkillSpector detects **71 vulnerability patterns** across 17 categories: | SC5 | Abandoned Dependencies | MEDIUM | Unmaintained packages without security updates | | SC6 | Typosquatting | HIGH | Package names similar to popular packages | | SC8 | Shipped Python Bytecode | HIGH | `__pycache__` / `.pyc` present (discovery skips; malicious bytecode bypass) | -| SC9 | Concealed Executable Artifact | HIGH | Executable nested in a document container or hidden/disguised artifact | +| SC9 | Concealed Executable Artifact | HIGH | Executable nested in a document container or hidden/disguised artifact; root-level visible source helpers in manifested `.agents/skills//` and `.claude/skills//` folders are exempted but remain local-only and are marked as not LLM-reviewed | | SC10 | Dependency Source Redirection | HIGH | Package-manager source added, replaced, or unresolved | ### Excessive Agency (5 patterns) diff --git a/docs/NESTED_ARTIFACT_INSPECTION.md b/docs/NESTED_ARTIFACT_INSPECTION.md index e6ff04efa..f52cb5255 100644 --- a/docs/NESTED_ARTIFACT_INSPECTION.md +++ b/docs/NESTED_ARTIFACT_INSPECTION.md @@ -70,3 +70,13 @@ container or a hidden/disguised artifact. Executability is established from an e a shebang, or archive mode bits. A benign document without executable members does not produce SC9. SC9 reports evidence and risk; it does not execute the member or prescribe an installation decision. + +At the scan root, visible source scripts beneath `.agents/skills//` and +`.claude/skills//` are not treated as concealed when that skill directory +contains a regular `SKILL.md` or `skill.md` manifest. These paths remain hidden +and local-only: their contents are not sent to an external LLM. When LLM +analysis is enabled, the inspection ledger records that LLM review was +withheld, so the report does not imply that those scripts received semantic +review. This narrow exception does not cover +hidden skill names, hidden descendants, mismatched shebangs, unsupported script +types, or nested client directories. diff --git a/src/skillspector/inspection_ledger.py b/src/skillspector/inspection_ledger.py index f648107a0..2e3e2179f 100644 --- a/src/skillspector/inspection_ledger.py +++ b/src/skillspector/inspection_ledger.py @@ -105,6 +105,7 @@ class LedgerReason(StrEnum): TOTAL_BYTES_LIMIT = "total_bytes_limit" RUNTIME_LIMIT = "runtime_limit" EXCLUDED_EXECUTABLE_CONTENT = "excluded_executable_content" + HIDDEN_SKILL_EXECUTABLE_LOCAL_ONLY = "hidden_skill_executable_local_only" OUTPUT_LIMIT = "output_limit" TRANSITIVE_CHILD_SCAN_FAILED = "transitive_child_scan_failed" STATIC_PARSE_LIMIT = "static_parse_limit" @@ -216,6 +217,9 @@ class LedgerReason(StrEnum): LedgerReason.EXCLUDED_EXECUTABLE_CONTENT: ( "Executable content was inventoried but excluded from content analysis." ), + LedgerReason.HIDDEN_SKILL_EXECUTABLE_LOCAL_ONLY: ( + "Conventional hidden-skill helper script was kept local and not sent to an external LLM." + ), LedgerReason.OUTPUT_LIMIT: "Inspection reached its configured output limit.", LedgerReason.TRANSITIVE_CHILD_SCAN_FAILED: ( "A transitive child scan failed before complete inspection." diff --git a/src/skillspector/nodes/build_context.py b/src/skillspector/nodes/build_context.py index fce139106..5958aeb22 100644 --- a/src/skillspector/nodes/build_context.py +++ b/src/skillspector/nodes/build_context.py @@ -26,6 +26,7 @@ import json import os import re +import shlex import tarfile from collections.abc import Callable, Mapping from fnmatch import fnmatchcase @@ -820,6 +821,78 @@ def _is_hidden_component(path: str) -> bool: return any(part.startswith(".") for part in path.replace("\\", "/").split("/") if part) +def _is_conventional_skill_script(skill_dir: Path, path: str, data: bytes) -> bool: + """Return whether a visible source script belongs to a standard agent skill directory.""" + parts = path.replace("\\", "/").split("/") + if len(parts) < 4 or parts[0:2] not in ([".agents", "skills"], [".claude", "skills"]): + return False + skill_name = parts[2] + if ( + not skill_name + or skill_name.startswith(".") + or any(part.startswith(".") for part in parts[3:]) + ): + return False + suffix = Path(parts[-1]).suffix.lower() + if suffix not in { + ".bash", + ".cjs", + ".js", + ".jsx", + ".mjs", + ".php", + ".pl", + ".ps1", + ".py", + ".rb", + ".sh", + ".ts", + ".tsx", + ".zsh", + } or has_binary_executable_magic(data): + return False + + first_line = data.splitlines()[0] if data.splitlines() else b"" + if first_line.startswith(b"#!"): + try: + tokens = shlex.split(first_line[2:].decode("utf-8", errors="strict")) + except (UnicodeDecodeError, ValueError): + return False + if not tokens: + return False + interpreter = Path(tokens[0]).name + if interpreter == "env": + interpreter = next( + (token for token in tokens[1:] if not token.startswith("-") and "=" not in token), + "", + ) + interpreter = Path(interpreter).name.removesuffix(".exe").lower() + compatible = { + ".bash": {"bash"}, + ".cjs": {"node", "nodejs", "bun"}, + ".js": {"node", "nodejs", "bun", "deno"}, + ".jsx": {"node", "nodejs", "bun", "deno"}, + ".mjs": {"node", "nodejs", "bun", "deno"}, + ".php": {"php"}, + ".pl": {"perl"}, + ".ps1": {"pwsh", "powershell"}, + ".py": {"python", "python2", "python3", "pypy", "pypy3"}, + ".rb": {"ruby"}, + ".sh": {"sh", "bash", "dash", "ksh", "zsh"}, + ".ts": {"node", "nodejs", "bun", "deno", "tsx"}, + ".tsx": {"node", "nodejs", "bun", "deno", "tsx"}, + ".zsh": {"zsh"}, + } + if not interpreter or interpreter not in compatible[suffix]: + return False + + skill_root = skill_dir.joinpath(*parts[:3]) + return any( + (manifest := skill_root / name).is_file() and not _is_symlink(manifest) + for name in ("SKILL.md", "skill.md") + ) + + 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: @@ -976,6 +1049,12 @@ def _expired(path: str) -> bool: "size_bytes": size_bytes, } hidden_component = _is_hidden_component(path) + conventional_skill_script = ( + executable + and hidden_component + and not source_local_only + and _is_conventional_skill_script(skill_dir, path, data) + ) if hidden_component or source_local_only: component["local_only"] = True if hidden_component: @@ -983,7 +1062,7 @@ def _expired(path: str) -> bool: if source_local_only: component["hidden_ancestor"] = True component["source_local_only"] = True - if executable: + if executable and (source_local_only or not conventional_skill_script): component.update( { "outer_path": path, @@ -996,6 +1075,9 @@ def _expired(path: str) -> bool: "concealment_reasons": ["hidden_artifact"], } ) + elif executable: + component["concealed_executable"] = False + component["hidden_skill_script_llm_review_withheld"] = True metadata.append(component) if _expired(path): break @@ -1074,6 +1156,24 @@ def _mark_unanalyzed_executables( return events +def _note_hidden_skill_scripts_not_sent_to_llm( + component_metadata: list[dict[str, object]], +) -> list[InspectionLedgerEvent]: + """Expose the local-only boundary without turning helpers into SC9 findings.""" + return [ + ledger_event( + outcome=LedgerOutcome.SKIPPED, + record_type=LedgerRecordType.SYSTEM, + phase="llm_eligibility", + path=str(component["path"]), + reason=LedgerReason.HIDDEN_SKILL_EXECUTABLE_LOCAL_ONLY, + ) + for component in component_metadata + if component.get("hidden_skill_script_llm_review_withheld") is True + and isinstance(component.get("path"), str) + ] + + def _redact_for_external_model(path: str, content: str) -> str: """Redact values from local environment files before external-model use.""" name = Path(path).name.lower() @@ -3670,6 +3770,12 @@ def _mark_runtime_partial(affected_paths: list[str], first_limited_path: str) -> {item["path"]: item for item in artifact_inventory}, raw_file_cache, ) + use_llm = state.get("use_llm", True) + hidden_skill_llm_events = ( + _note_hidden_skill_scripts_not_sent_to_llm(component_metadata) + if use_llm is not False + else [] + ) has_executable_scripts = ( has_executable_scripts or any(bool(metadata.get("executable")) for metadata in nested.metadata) @@ -3685,7 +3791,6 @@ def _mark_runtime_partial(affected_paths: list[str], first_limited_path: str) -> if target and target in disposition_by_path: reference["disposition"] = disposition_by_path[target] - use_llm = state.get("use_llm", True) model_config = build_model_config() if use_llm else {} llm_provenance = ( capture_llm_provenance(model_config) @@ -3726,6 +3831,7 @@ def _mark_runtime_partial(affected_paths: list[str], first_limited_path: str) -> *structured_events, *postprocessing_events, *coverage_policy_events, + *hidden_skill_llm_events, ] ), "ast_cache": {}, diff --git a/tests/nodes/test_build_context.py b/tests/nodes/test_build_context.py index b071492b6..ddd3673f3 100644 --- a/tests/nodes/test_build_context.py +++ b/tests/nodes/test_build_context.py @@ -703,6 +703,132 @@ def test_source_local_only_preserves_excluded_executable_coverage( ) +@pytest.mark.parametrize("client_dir", [".agents", ".claude"]) +@pytest.mark.parametrize("use_llm", [True, False]) +def test_conventional_skill_scripts_are_not_concealed( + tmp_path: Path, client_dir: str, use_llm: bool +) -> None: + skill_root = tmp_path / client_dir / "skills" / "hello" + (skill_root / "scripts").mkdir(parents=True) + (skill_root / "SKILL.md").write_text("# Hello\n", encoding="utf-8") + (skill_root / "scripts" / "hello.py").write_text('print("hello")\n', encoding="utf-8") + hidden_script = skill_root / "scripts" / ".hidden.py" + hidden_script.write_text('print("hidden")\n', encoding="utf-8") + + result = build_context({"skill_path": str(tmp_path), "use_llm": use_llm}) + + metadata = {item["path"]: item for item in result["component_metadata"]} + visible_path = f"{client_dir}/skills/hello/scripts/hello.py" + hidden_path = f"{client_dir}/skills/hello/scripts/.hidden.py" + assert metadata[visible_path]["concealed_executable"] is False + assert metadata[visible_path]["hidden_skill_script_llm_review_withheld"] is True + assert metadata[hidden_path]["concealed_executable"] is True + findings = _analyze_concealed_executables(result["component_metadata"]) + assert visible_path not in {finding.file for finding in findings} + assert hidden_path in {finding.file for finding in findings} + coverage = [ + event + for event in result["inspection_ledger"] + if event["path"] == visible_path + and event.get("reason_code") == LedgerReason.HIDDEN_SKILL_EXECUTABLE_LOCAL_ONLY + ] + if use_llm: + assert len(coverage) == 1 + assert coverage[0]["outcome"] == LedgerOutcome.SKIPPED + assert "not sent to an external LLM" in coverage[0]["message"] + else: + assert coverage == [] + + +def test_conventional_skill_script_with_compatible_env_shebang_is_not_concealed( + tmp_path: Path, +) -> None: + skill_root = tmp_path / ".claude" / "skills" / "demo" + script = skill_root / "scripts" / "run.py" + script.parent.mkdir(parents=True) + (skill_root / "SKILL.md").write_text("# Demo\n", encoding="utf-8") + script.write_bytes(b"#!/usr/bin/env python3\nprint(1)\n") + + result = build_context({"skill_path": str(tmp_path), "use_llm": False}) + + metadata = next( + item + for item in result["component_metadata"] + if item["path"] == ".claude/skills/demo/scripts/run.py" + ) + assert metadata["concealed_executable"] is False + + +@pytest.mark.parametrize( + ("relative_path", "payload", "manifest", "source_local_only"), + [ + (".claude/skills/demo/scripts/run.py", b"#!/usr/bin/env python3\nprint(1)\n", False, False), + ( + ".claude/skills/demo/scripts/run.py", + b"#!/usr/bin/env python3\nprint(1)\n", + "symlink", + False, + ), + (".claude/skills/demo/scripts/tool.py", b"\x7fELF" + b"\0" * 20, True, False), + (".claude/skills/demo/scripts/tool.py", b"MZ" + b"\0" * 20, True, False), + (".claude/skills/demo/scripts/run", b"#!/bin/sh\ntrue\n", True, False), + (".claude/skills/demo/scripts/run.bat", b"@echo off\r\n", True, False), + ( + ".claude/skills/demo/scripts/run.py", + b"#!/usr/bin/env -Sbash\ntrue\n", + True, + False, + ), + ( + ".claude/skills/demo/scripts/run.py", + b"#!/usr/bin/env --split-string=bash\ntrue\n", + True, + False, + ), + (".claude/skills/.demo/scripts/run.py", b"#!/usr/bin/env python3\nprint(1)\n", True, False), + ( + ".claude/skills/demo/scripts/.cache/run.py", + b"#!/usr/bin/env python3\nprint(1)\n", + True, + False, + ), + (".github/skills/demo/scripts/run.py", b"#!/usr/bin/env python3\nprint(1)\n", True, False), + ( + "project/.claude/skills/demo/scripts/run.py", + b"#!/usr/bin/env python3\nprint(1)\n", + True, + False, + ), + (".claude/skills/demo/scripts/run.py", b"#!/usr/bin/env python3\nprint(1)\n", True, True), + (".claude/skills/demo/scripts/run.py", b"#!/bin/bash\nprint(1)\n", True, False), + ], +) +def test_skill_script_exemption_fails_closed( + tmp_path: Path, + relative_path: str, + payload: bytes, + manifest: bool | str, + source_local_only: bool, +) -> None: + target = tmp_path / relative_path + target.parent.mkdir(parents=True) + target.write_bytes(payload) + skill_root = target.parents[1] + skill_root.mkdir(parents=True, exist_ok=True) + if manifest is True: + (skill_root / "SKILL.md").write_text("# Demo\n", encoding="utf-8") + elif manifest == "symlink": + external = tmp_path / "outside.md" + external.write_text("# Demo\n", encoding="utf-8") + (skill_root / "SKILL.md").symlink_to(external) + + result = build_context({"skill_path": str(tmp_path), "source_local_only": source_local_only}) + metadata = next(item for item in result["component_metadata"] if item["path"] == relative_path) + assert metadata["concealed_executable"] is True + findings = _analyze_concealed_executables(result["component_metadata"]) + assert relative_path in {finding.file for finding in findings} + + def test_build_context_model_config_uses_bound_provider(tmp_path: Path) -> None: class _BoundProvider: DEFAULT_MODEL = "bound-default"