diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 9568925f..2d81b43b 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -69,7 +69,8 @@ jobs: --test tests/test_claude_plugin_secret_to_prompt.py \ --test tests/test_claude_plugin_secret_to_mcp.py \ --test tests/test_claude_plugin_command_skill_reuse.py \ - --test tests/test_claude_plugin_hide_actions.py + --test tests/test_claude_plugin_hide_actions.py \ + --test tests/test_claude_plugin_insecure_file_mode.py - name: Verify 100% statement coverage for Claude plugin scan CLI if: matrix.python-version == '3.13' run: | diff --git a/CHANGELOG.d/1099-claude-plugin-supply-chain.md b/CHANGELOG.d/1099-claude-plugin-supply-chain.md index ce8f1443..3a7843f9 100644 --- a/CHANGELOG.d/1099-claude-plugin-supply-chain.md +++ b/CHANGELOG.d/1099-claude-plugin-supply-chain.md @@ -99,3 +99,8 @@ `claude-plugin-goal-escalation-directive`. Honest ``report each tool call to the user`` wording, README prose, and vendored copies are not that class. #1036 injection and exfil identities stay on their rules. + Setuid or setgid executable and hook files fail as + `claude-plugin-setuid-executable`. World-writable executable and hook + files fail as `claude-plugin-world-writable-executable`. A declared + ``0755`` hook, world-writable LICENSE, vendored copies, Git metadata, + and ``.mcp.json`` are not that class. diff --git a/appguardrail_core/claude_plugin_detector.py b/appguardrail_core/claude_plugin_detector.py index 50bac7e5..09928ce6 100644 --- a/appguardrail_core/claude_plugin_detector.py +++ b/appguardrail_core/claude_plugin_detector.py @@ -21,7 +21,8 @@ homoglyph, injection, exfiltration, and placeholder hits reuse #1036 rule identities. Skill, command, or agent text that hides tool use, rewrites the system prompt, or escalates the declared goal is a separate -instruction-override family. A lockfile-backed package.json without a +instruction-override family. Setuid, setgid, or world-writable executable +and hook files fail admission. A lockfile-backed package.json without a lifecycle download stays inventory. Vendored trees are one scope finding, not hook scans. """ @@ -35,6 +36,7 @@ import os from pathlib import Path import re +import stat import tarfile from typing import Final, Iterable import unicodedata @@ -142,6 +144,16 @@ "budget. Hostile oversized trees fail admission. " "[CWE-400 - Uncontrolled Resource Consumption]" ) +CLAUDE_PLUGIN_SETUID_EXECUTABLE_MESSAGE: Final = ( + "Claude plugin executable or hook has the setuid or setgid bit. " + "Privilege-elevating modes fail admission. " + "[CWE-732 - Incorrect Permission Assignment for Critical Resource]" +) +CLAUDE_PLUGIN_WORLD_WRITABLE_EXECUTABLE_MESSAGE: Final = ( + "Claude plugin executable or hook is world-writable. Tamperable host " + "modes fail admission. " + "[CWE-732 - Incorrect Permission Assignment for Critical Resource]" +) CLAUDE_PLUGIN_SOURCE_MISMATCH_MESSAGE: Final = ( "Claude plugin marketplace identity does not match the retrieved artifact " "ref, repository, or source path. Bind admission to one exact object. " @@ -767,11 +779,12 @@ def scan_claude_plugin_package(root: Path) -> tuple[PluginHit, ...]: Undeclared executable, hidden undeclared executable or config, undeclared vendored or generated scope, license absence or SPDX mismatch, size, symlink, archive traversal, unadmitted-submodule, - and deceptive description findings. Empty when the tree is not a - plugin package or every hook is a declared regular file. Inventory - presence is not a finding. An empty description is not this class. - Git metadata is not a plugin executable surface. ``.mcp.json`` - stays the MCP class. Vendored trees are one scope finding. + setuid or world-writable executable modes, and deceptive + description findings. Empty when the tree is not a plugin package + or every hook is a declared regular file. Inventory presence is + not a finding. An empty description is not this class. Git + metadata is not a plugin executable surface. ``.mcp.json`` stays + the MCP class. Vendored trees are one scope finding. """ plugin_dir = root / ".claude-plugin" if not plugin_dir.is_dir() or plugin_dir.is_symlink(): @@ -851,6 +864,7 @@ def scan_claude_plugin_package(root: Path) -> tuple[PluginHit, ...]: ) ) hits.extend(_hidden_undeclared_executable_hits(root, declared)) + hits.extend(_insecure_file_mode_hits(root)) hits.extend(_deceptive_description_hits(root)) return tuple(hits) @@ -2144,6 +2158,71 @@ def _hidden_undeclared_executable_hits( return tuple(hits) +def _is_mode_sensitive_surface(path: Path, relative: str) -> bool: + """Return whether ``relative`` is an executable or hook host-fs surface. + + LICENSE, README, and other documentation without an executable suffix + are not this class. MCP manifests stay the MCP class. + """ + if path.name in _MCP_FILENAMES or _is_git_metadata_path(relative): + return False + posix = relative.replace("\\", "/") + first = posix.split("/", 1)[0] + suffix = path.suffix.lower() + if first in _HOOK_DIRS: + return True + return suffix in _EXECUTABLE_SUFFIXES + + +def _insecure_file_mode_hits(root: Path) -> tuple[PluginHit, ...]: + """Return findings for setuid, setgid, or world-writable hook files. + + Args: + root: Materialized plugin tree. + + Returns: + Hits for executable or hook files whose mode has setuid, setgid, + or other-write. Empty when every such file is ``0755``/``0644``, + vendored, a symlink, or Git metadata. LICENSE world-write is not + this class. Snippets are path labels. + """ + hits: list[PluginHit] = [] + for path in _walk_entries(root): + if path.is_symlink() or not path.is_file(): + continue + relative = path.relative_to(root).as_posix() + if _is_vendored_scope_relative(relative): + continue + if not _is_mode_sensitive_surface(path, relative): + continue + try: + mode = os.lstat(path).st_mode + except OSError: + continue + snippet = _sanitize_path_snippet(path.name) + if mode & (stat.S_ISUID | stat.S_ISGID): + hits.append( + PluginHit( + rule_id="claude-plugin-setuid-executable", + line=1, + snippet=snippet, + message=CLAUDE_PLUGIN_SETUID_EXECUTABLE_MESSAGE, + file=relative, + ) + ) + if mode & stat.S_IWOTH: + hits.append( + PluginHit( + rule_id="claude-plugin-world-writable-executable", + line=1, + snippet=snippet, + message=CLAUDE_PLUGIN_WORLD_WRITABLE_EXECUTABLE_MESSAGE, + file=relative, + ) + ) + return tuple(hits) + + def _deceptive_description_hits(root: Path) -> tuple[PluginHit, ...]: """Return findings when a description denies inventoried capabilities. diff --git a/docs/TRACEABILITY.md b/docs/TRACEABILITY.md index 5fc5fae6..dbfc45b6 100644 --- a/docs/TRACEABILITY.md +++ b/docs/TRACEABILITY.md @@ -22,7 +22,7 @@ | structural Semgrep-style `pattern:` execution by lightweight engine | built-in scanner | not implemented unless a real structural matcher is added; fixtures are not execution | | GitHub Actions transport-only polling loop (#1087, #938 vertical slice) | owned by PR #1088 / issue #1087; YAML rules and RED precision contracts | mapped-family only; this successor does not ship or close the detector | | Password/database-url/auth-comment precision and test-file context (#1106) | existing `_scan_file` rules `hardcoded-password`, `hardcoded-database-url`, `todo-skip-auth`, `_finding_context` | implemented-branch regression lock | -| Claude plugin marketplace/package supply chain (#1099) | `claude-plugin-floating-git-ref`, `claude-plugin-provider-secret`, `claude-plugin-pipe-to-shell`, `claude-plugin-unsigned-executable-download` (hooks and package.json lifecycle scripts), `claude-plugin-unpinned-package-install`, `claude-plugin-undeclared-executable`, `claude-plugin-symlink-escape`, `claude-plugin-archive-path-traversal`, `claude-plugin-unadmitted-submodule`, `claude-plugin-duplicate-json-member`, `claude-plugin-nonstandard-json-constant`, `claude-plugin-malformed-utf8`, `claude-plugin-inconsistent-normalized-name`, `claude-plugin-vendored-scope-undeclared`, `claude-plugin-conflicting-identity`, `claude-plugin-unbounded-mcp`, `claude-plugin-license-missing`, `claude-plugin-license-mismatch`, `claude-plugin-dynamic-eval`, `claude-plugin-hidden-undeclared-executable`, `claude-plugin-concealed-identity`, `claude-plugin-oversized-package`, `claude-plugin-source-mismatch`, `claude-plugin-github-write-token`, `claude-plugin-docker-socket`, `claude-plugin-browser-profile-access`, `claude-plugin-deceptive-description`, `claude-plugin-secret-to-network`, `claude-plugin-secret-to-prompt`, `claude-plugin-secret-to-mcp`, `claude-plugin-hide-actions-directive` / `claude-plugin-self-modify-directive` / `claude-plugin-goal-escalation-directive`, reused #1036 `skill-name-homoglyph-confusable` / `skill-manifest-prompt-injection-payload` / `skill-doc-exfiltration-endpoint-directive` / `skill-placeholder-template-unresolved` on plugin skill/agent/command surfaces, deterministic scan receipt with catalog repository/SHA bind and SARIF 2.1.0 `sarif_sha256` bound to the same finding rule_ids, fail-closed receipt verification | implemented-branch | +| Claude plugin marketplace/package supply chain (#1099) | `claude-plugin-floating-git-ref`, `claude-plugin-provider-secret`, `claude-plugin-pipe-to-shell`, `claude-plugin-unsigned-executable-download` (hooks and package.json lifecycle scripts), `claude-plugin-unpinned-package-install`, `claude-plugin-undeclared-executable`, `claude-plugin-symlink-escape`, `claude-plugin-archive-path-traversal`, `claude-plugin-unadmitted-submodule`, `claude-plugin-duplicate-json-member`, `claude-plugin-nonstandard-json-constant`, `claude-plugin-malformed-utf8`, `claude-plugin-inconsistent-normalized-name`, `claude-plugin-vendored-scope-undeclared`, `claude-plugin-conflicting-identity`, `claude-plugin-unbounded-mcp`, `claude-plugin-license-missing`, `claude-plugin-license-mismatch`, `claude-plugin-dynamic-eval`, `claude-plugin-hidden-undeclared-executable`, `claude-plugin-concealed-identity`, `claude-plugin-oversized-package`, `claude-plugin-source-mismatch`, `claude-plugin-github-write-token`, `claude-plugin-docker-socket`, `claude-plugin-browser-profile-access`, `claude-plugin-deceptive-description`, `claude-plugin-secret-to-network`, `claude-plugin-secret-to-prompt`, `claude-plugin-secret-to-mcp`, `claude-plugin-hide-actions-directive` / `claude-plugin-self-modify-directive` / `claude-plugin-goal-escalation-directive`, `claude-plugin-setuid-executable` / `claude-plugin-world-writable-executable`, reused #1036 `skill-name-homoglyph-confusable` / `skill-manifest-prompt-injection-payload` / `skill-doc-exfiltration-endpoint-directive` / `skill-placeholder-template-unresolved` on plugin skill/agent/command surfaces, deterministic scan receipt with catalog repository/SHA bind and SARIF 2.1.0 `sarif_sha256` bound to the same finding rule_ids, fail-closed receipt verification | implemented-branch | | Orphaned GitHub Actions registry identities (#929) | owned by PR #966 / issue #929; live registry DAST | mapped-family only; this successor does not ship or close the detector | | Org security-failure CI tickets without copied vuln evidence | documented non-detectable family | snapshot in `tests/fixtures/cwl-security-issue-inventory.json` | diff --git a/docs/doctoring/cwl-security-issue-detectors.md b/docs/doctoring/cwl-security-issue-detectors.md index 503eef6b..cc927638 100644 --- a/docs/doctoring/cwl-security-issue-detectors.md +++ b/docs/doctoring/cwl-security-issue-detectors.md @@ -16,7 +16,7 @@ every frozen family. It implements only the unique families it owns. |---|---|---|---|---| | Transport-only Actions polling | SAST | #1087, #938 | PR #1088 / issue #1087 | maps only | | Secret indirection / auth comments | SAST | #1106 | this successor | implements regression lock on existing `_scan_file` rules, including LifeOS #247 test-title/authority wording | -| Claude plugin supply chain | SAST | #1099 | this successor | implements `claude-plugin-*` findings including unsigned executable downloads from hooks and package.json lifecycle scripts, unpinned package URL installs, GitHub write tokens, Docker socket binds, host browser-profile stores, deceptive plugin/skill/command descriptions, non-standard JSON constants, malformed UTF-8 JSON bytes, non-NFC identity names, undeclared vendored or generated code scope, conflicting plugin/skill/command identities, secret-to-network flows, secret-to-prompt, log, or subprocess-env copies, secrets copied into MCP env/args/command/URL/headers, and hide-actions / self-modify / goal-escalation wording on skill/command/agent surfaces, reuses released #1036 skill-supply-chain rule identities on plugin skill/agent/command surfaces, capability inventory evidence, undeclared-executable admission, LICENSE/NOTICE SPDX mismatch, dynamic eval/exec on hook surfaces, hidden undeclared executable/config surfaces, a secret-free scan receipt with catalog repository/SHA bind and SARIF 2.1.0 `sarif_sha256` bound to the same finding rule_ids, and fail-closed stale/mismatched receipt verification | +| Claude plugin supply chain | SAST | #1099 | this successor | implements `claude-plugin-*` findings including unsigned executable downloads from hooks and package.json lifecycle scripts, unpinned package URL installs, GitHub write tokens, Docker socket binds, host browser-profile stores, deceptive plugin/skill/command descriptions, non-standard JSON constants, malformed UTF-8 JSON bytes, non-NFC identity names, undeclared vendored or generated code scope, conflicting plugin/skill/command identities, secret-to-network flows, secret-to-prompt, log, or subprocess-env copies, secrets copied into MCP env/args/command/URL/headers, hide-actions / self-modify / goal-escalation wording on skill/command/agent surfaces, and setuid/setgid or world-writable executable and hook modes, reuses released #1036 skill-supply-chain rule identities on plugin skill/agent/command surfaces, capability inventory evidence, undeclared-executable admission, LICENSE/NOTICE SPDX mismatch, dynamic eval/exec on hook surfaces, hidden undeclared executable/config surfaces, a secret-free scan receipt with catalog repository/SHA bind and SARIF 2.1.0 `sarif_sha256` bound to the same finding rule_ids, and fail-closed stale/mismatched receipt verification | | Orphaned Actions workflows | DAST | #929 | PR #966 / issue #929 | maps only | | Org CI failure without evidence | non-detectable | 353 tickets | inventory snapshot | maps only | | UX / control-plane product gaps | non-detectable | #871, #928 | out of SAST/DAST scope | maps only | diff --git a/docs/sast-dast-rule-research.md b/docs/sast-dast-rule-research.md index a82b7d42..4d5ffe63 100644 --- a/docs/sast-dast-rule-research.md +++ b/docs/sast-dast-rule-research.md @@ -85,7 +85,8 @@ files being scanned, then applies the union of relevant checks. Examples: undeclared executables, undeclared vendored or generated code scope, reused #1036 skill-supply-chain identities on plugin skill/agent/command surfaces, hide-actions / self-modify / - goal-escalation instruction wording on those surfaces, and fail-closed + goal-escalation instruction wording on those surfaces, setuid/setgid or + world-writable executable and hook modes, and fail-closed replay of a stale or mismatched scan receipt. - Mapped, not owned here: GitHub Actions transport-only poll loops (#1087, PR #1088) and orphaned workflow registry DAST (#929, PR #966). diff --git a/tests/test_claude_plugin_insecure_file_mode.py b/tests/test_claude_plugin_insecure_file_mode.py new file mode 100644 index 00000000..fc04ef1d --- /dev/null +++ b/tests/test_claude_plugin_insecure_file_mode.py @@ -0,0 +1,199 @@ +"""Plugin executables must not ship setuid, setgid, or world-writable modes.""" + +from __future__ import annotations + +import json +import stat +from pathlib import Path + +from appguardrail_core.claude_plugin_detector import ( + build_claude_plugin_scan_receipt, +) + + +_PINNED_COMMIT = "a727be1c7bd6064419b6f60d71993a19198adc17" +_SETUID_RULE = "claude-plugin-setuid-executable" +_WORLD_RULE = "claude-plugin-world-writable-executable" +_VENDORED_RULE = "claude-plugin-vendored-scope-undeclared" + + +def _write_json(path: Path, payload: dict) -> None: + """Write one JSON document under ``path``.""" + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(payload, indent=2) + "\n", encoding="utf-8") + + +def _licensed_plugin(root: Path) -> Path: + """Write a pinned licensed plugin with one declared shell hook.""" + _write_json( + root / ".claude-plugin" / "plugin.json", + { + "name": "safe-plugin", + "version": "1.0.0", + "source": { + "source": "github", + "repo": "example/safe-plugin", + "ref": _PINNED_COMMIT, + }, + "hooks": {"PreToolUse": [{"command": "hooks/session.sh"}]}, + }, + ) + hook = root / "hooks" / "session.sh" + hook.parent.mkdir(parents=True, exist_ok=True) + hook.write_text("#!/bin/sh\necho hello\n", encoding="utf-8") + hook.chmod(0o755) + (root / "LICENSE").write_text("MIT\n", encoding="utf-8") + return root + + +def test_setuid_hook_fails_admission(tmp_path: Path) -> None: + """A declared hook with the setuid bit fails closed.""" + root = _licensed_plugin(tmp_path) + hook = root / "hooks" / "session.sh" + hook.chmod(0o4755) + receipt = build_claude_plugin_scan_receipt(root) + + assert receipt.scan_result == "fail" + assert _SETUID_RULE in receipt.finding_summary + assert _WORLD_RULE not in receipt.finding_summary + + +def test_setgid_script_fails_admission(tmp_path: Path) -> None: + """A ``scripts/`` helper with the setgid bit is the setuid class.""" + root = _licensed_plugin(tmp_path) + helper = root / "scripts" / "run.sh" + helper.parent.mkdir(parents=True, exist_ok=True) + helper.write_text("#!/bin/sh\necho helper\n", encoding="utf-8") + helper.chmod(0o2755) + receipt = build_claude_plugin_scan_receipt(root) + + assert receipt.scan_result == "fail" + assert _SETUID_RULE in receipt.finding_summary + + +def test_world_writable_hook_fails_admission(tmp_path: Path) -> None: + """A world-writable declared hook can be swapped after install.""" + root = _licensed_plugin(tmp_path) + hook = root / "hooks" / "session.sh" + hook.chmod(0o777) + receipt = build_claude_plugin_scan_receipt(root) + + assert receipt.scan_result == "fail" + assert _WORLD_RULE in receipt.finding_summary + assert _SETUID_RULE not in receipt.finding_summary + + +def test_world_writable_unsuffixed_binary_fails_admission(tmp_path: Path) -> None: + """A world-writable unsuffixed helper under ``scripts/`` is this class.""" + root = _licensed_plugin(tmp_path) + binary = root / "scripts" / "run" + binary.parent.mkdir(parents=True, exist_ok=True) + binary.write_bytes(b"\x7fELF") + binary.chmod(0o666) + receipt = build_claude_plugin_scan_receipt(root) + + assert receipt.scan_result == "fail" + assert _WORLD_RULE in receipt.finding_summary + + +def test_owner_executable_hook_stays_receipt_pass(tmp_path: Path) -> None: + """``0755`` on a declared hook is not setuid or world-writable.""" + root = _licensed_plugin(tmp_path) + receipt = build_claude_plugin_scan_receipt(root) + + assert receipt.scan_result == "pass" + assert _SETUID_RULE not in receipt.finding_summary + assert _WORLD_RULE not in receipt.finding_summary + + +def test_world_writable_license_is_not_this_class(tmp_path: Path) -> None: + """World-writable LICENSE is not an executable host-fs surface.""" + root = _licensed_plugin(tmp_path) + license_path = root / "LICENSE" + license_path.chmod(0o666) + receipt = build_claude_plugin_scan_receipt(root) + + assert _WORLD_RULE not in receipt.finding_summary + assert _SETUID_RULE not in receipt.finding_summary + + +def test_vendored_world_writable_stays_vendored_scope(tmp_path: Path) -> None: + """World-writable files under vendor/ stay the vendored-scope class.""" + root = _licensed_plugin(tmp_path) + vendored = root / "vendor" / "hooks" / "evil.sh" + vendored.parent.mkdir(parents=True, exist_ok=True) + vendored.write_text("#!/bin/sh\necho evil\n", encoding="utf-8") + vendored.chmod(0o777) + receipt = build_claude_plugin_scan_receipt(root) + + assert _WORLD_RULE not in receipt.finding_summary + assert _SETUID_RULE not in receipt.finding_summary + assert _VENDORED_RULE in receipt.finding_summary + + +def test_symlink_hook_is_not_a_mode_finding(tmp_path: Path) -> None: + """Symlink hooks stay the symlink-escape class, not a mode finding.""" + root = _licensed_plugin(tmp_path) + extra = root / "hooks" / "extra.sh" + extra.symlink_to(root / "hooks" / "session.sh") + receipt = build_claude_plugin_scan_receipt(root) + + assert _SETUID_RULE not in receipt.finding_summary + assert _WORLD_RULE not in receipt.finding_summary + + +def test_setuid_python_outside_hook_dirs_fails_admission(tmp_path: Path) -> None: + """A setuid ``.py`` at the package root is still this class.""" + root = _licensed_plugin(tmp_path) + script = root / "install.py" + script.write_text("print('install')\n", encoding="utf-8") + script.chmod(0o4755) + receipt = build_claude_plugin_scan_receipt(root) + + assert receipt.scan_result == "fail" + assert _SETUID_RULE in receipt.finding_summary + + +def test_git_hook_setuid_is_not_plugin_mode_finding(tmp_path: Path) -> None: + """``.git/hooks`` stays Git metadata, not a plugin setuid finding.""" + root = _licensed_plugin(tmp_path) + git_hook = root / ".git" / "hooks" / "pre-commit.sh" + git_hook.parent.mkdir(parents=True, exist_ok=True) + git_hook.write_text("#!/bin/sh\necho git\n", encoding="utf-8") + git_hook.chmod(0o4755) + receipt = build_claude_plugin_scan_receipt(root) + + assert _SETUID_RULE not in receipt.finding_summary + assert _WORLD_RULE not in receipt.finding_summary + + +def test_mcp_json_under_hooks_is_not_a_mode_finding(tmp_path: Path) -> None: + """``.mcp.json`` stays the MCP class even under ``hooks/``.""" + root = _licensed_plugin(tmp_path) + mcp = root / "hooks" / ".mcp.json" + mcp.write_text("{}\n", encoding="utf-8") + mcp.chmod(0o666) + receipt = build_claude_plugin_scan_receipt(root) + + assert _WORLD_RULE not in receipt.finding_summary + assert _SETUID_RULE not in receipt.finding_summary + + +def test_mode_stat_errors_are_skipped(tmp_path: Path, monkeypatch) -> None: + """Unreadable mode bits are skipped rather than failing open as pass.""" + import os + + from appguardrail_core import claude_plugin_detector as detector + + root = _licensed_plugin(tmp_path) + hook = root / "hooks" / "session.sh" + original_lstat = os.lstat + + def fake_lstat(target, *args, **kwargs): + if os.fspath(target) == os.fspath(hook): + raise OSError("unreadable") + return original_lstat(target, *args, **kwargs) + + monkeypatch.setattr(os, "lstat", fake_lstat) + hits = detector._insecure_file_mode_hits(root) + assert hits == ()