Merge pull request #404 from Lexus2016/evolution/issue-398-secret-detection

feat(security-guidance): secret detection slice (#398, child of #390)

Author: Lexus2016

plugins/security-guidance/__init__.py

@@ -38,6 +38,7 @@
 from typing import Any, Dict, List, Optional, Tuple
 
 from . import patterns as _patterns
+from . import secrets as _secrets
 
 logger = logging.getLogger(__name__)
 
@@ -196,6 +197,7 @@ def _scan_args(tool_name: str, args: Any) -> List[Tuple[str, str]]:
     findings: List[Tuple[str, str]] = []
     for path, content in _extract_path_and_content(tool_name, args):
         findings.extend(_scan_content(path, content))
+        findings.extend(_secrets.scan_secrets(path, content))
     return findings
 
 

plugins/security-guidance/secrets.py

@@ -0,0 +1,161 @@
+"""Secret detection for the security-guidance plugin (Hermes addition, #398).
+
+Child of #390 — first shippable slice of the security code-review plugin.
+NOT part of the Anthropic fork: ``patterns.py`` is byte-for-byte upstream, so
+this Hermes-side logic lives in its own module. Two layers:
+
+1. Regex rules for well-known credential formats (AWS, GitHub, Slack, Google,
+   Stripe, npm, PEM private keys, JWT, generic api-key assignments).
+2. A conservative Shannon-entropy check: a high-entropy value assigned to a
+   secret-named key, with obvious placeholders/example values excluded. The
+   threshold is deliberately conservative (~4.0 bits/char) to keep the
+   false-positive rate low, so it will NOT flag low-entropy human passphrases
+   (e.g. "correcthorsebatterystaple"); known-format keys are caught by layer 1.
+
+Findings are returned as ``(ruleName, reminder)`` tuples — the same shape the
+regex security rules use — so they flow through the existing warn/block path in
+``__init__.py`` with no special handling.
+"""
+
+from __future__ import annotations
+
+import math
+import re
+from typing import Dict, List, Set, Tuple
+
+# Same scan cap as the regex scanner — pattern-matching a huge blob is poor
+# signal-to-noise and slows the agent loop.
+# Same scan cap as the regex scanner in __init__.py (_MAX_SCAN_BYTES there) —
+# kept independent so this module stays stdlib-only and importable in isolation.
+# If you change one, change both.
+_MAX_SCAN_BYTES = 256 * 1024
+
+# Obvious non-secrets — example keys, placeholders, redactions. Checked against
+# the matched text so AWS's documented ``AKIAIOSFODNN7EXAMPLE`` and friends, or
+# ``api_key = "your-key-here"``, don't generate false warnings.
+# Two exclusion sets:
+#   _EXAMPLE_RE   — unambiguous "this is documentation, not a real key" words.
+#     Safe to apply even to fixed-prefix tokens (AKIA…/ghp_…), because a real
+#     random key won't contain the literal word "example"/"dummy"/etc.
+#   _PLACEHOLDER_RE — broader, includes structural fillers (your-, xxxx, 0000,
+#     <...>). Applied ONLY to assignment-style/entropy values, never to a
+#     fixed-prefix token — otherwise a real key that merely *contains* "xxxx"
+#     or "0000" as a substring would be silently dropped (a fail-open miss in
+#     a security tool). See scan_secrets().
+_EXAMPLE_RE = re.compile(
+    r"(?i)(example|redacted|placeholder|dummy|sample|changeme|fake|"
+    r"test[_-]?(?:key|token|secret))"
+)
+_PLACEHOLDER_RE = re.compile(
+    r"(?i)(example|redacted|placeholder|dummy|sample|changeme|your[_-]?|"
+    r"x{4,}|\.\.\.|<[a-z0-9_ .-]+>|fake|test[_-]?(?:key|token|secret)|0{8,})"
+)
+
+_SECRET_REMINDER = (
+    "⚠️ Security Warning: a hardcoded credential ({kind}) appears in "
+    "this content. Never commit live secrets to source. Move it to an "
+    "environment variable or a secrets manager, and rotate the credential if it "
+    "was ever real. If this is a placeholder/example, document that inline."
+)
+
+_ENTROPY_REMINDER = (
+    "⚠️ Security Warning: a high-entropy value is assigned to a "
+    "secret-named variable — this looks like a hardcoded credential. Move it to "
+    "an environment variable or secrets manager and rotate it if real. If it is "
+    "not a secret, rename the variable or document why it is safe."
+)
+
+# (ruleName, human-readable kind, compiled regex). Most-specific first.
+_SECRET_RULES: List[Tuple[str, str, "re.Pattern[str]"]] = [
+    ("private_key_pem", "PEM private key",
+     re.compile(r"-----BEGIN (?:RSA |EC |DSA |OPENSSH |PGP |ENCRYPTED )?PRIVATE KEY-----")),
+    ("aws_access_key_id", "AWS access key id",
+     re.compile(r"\b(?:AKIA|ASIA)[0-9A-Z]{16}\b")),
+    ("aws_secret_access_key", "AWS secret access key",
+     re.compile(r"(?i)aws_secret_access_key\s*[=:]\s*[\"'][A-Za-z0-9/+]{40}[\"']")),
+    ("github_token", "GitHub token",
+     re.compile(r"\bgh[pousr]_[A-Za-z0-9]{36,}\b")),
+    ("github_pat_finegrained", "GitHub fine-grained PAT",
+     re.compile(r"\bgithub_pat_[A-Za-z0-9_]{22,}\b")),
+    ("slack_token", "Slack token",
+     re.compile(r"\bxox[baprs]-[A-Za-z0-9-]{10,}\b")),
+    ("slack_webhook", "Slack webhook URL",
+     re.compile(r"https://hooks\.slack\.com/services/T[A-Za-z0-9_/]+")),
+    ("google_api_key", "Google API key",
+     re.compile(r"\bAIza[0-9A-Za-z_\-]{35}\b")),
+    ("stripe_secret_key", "Stripe secret key",
+     re.compile(r"\b(?:sk|rk)_live_[0-9a-zA-Z]{24,}\b")),  # live keys only; sk_test_ is low-risk by design
+    ("npm_token", "npm token",
+     re.compile(r"\bnpm_[A-Za-z0-9]{36}\b")),
+    ("jwt_token", "JSON Web Token",
+     re.compile(r"\beyJ[A-Za-z0-9_-]{10,}\.eyJ[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}\b")),
+    ("generic_secret_assignment", "hardcoded API key / token",
+     re.compile(
+         r"(?i)\b(?:api[_-]?key|client[_-]?secret|access[_-]?token|auth[_-]?token|"
+         r"secret[_-]?key)\b\s*[=:]\s*[\"'][A-Za-z0-9_\-]{16,}[\"']"
+     )),
+]
+
+# Entropy layer: a high-entropy value assigned to a secret-named key.
+_SECRET_ASSIGN_RE = re.compile(
+    r"(?i)\b([A-Za-z0-9_]*(?:secret|token|passwd|password|api[_-]?key|"
+    r"access[_-]?key|client[_-]?secret|private[_-]?key|credential)[A-Za-z0-9_]*)"
+    r"\s*[=:]\s*[\"']([^\"'\s]{20,})[\"']"
+)
+_ENTROPY_THRESHOLD = 4.0  # bits/char; random base64 ~5-6, English prose ~4.0-4.2
+
+
+def shannon_entropy(s: str) -> float:
+    """Shannon entropy in bits/char of *s* (0.0 for empty)."""
+    if not s:
+        return 0.0
+    counts: Dict[str, int] = {}
+    for ch in s:
+        counts[ch] = counts.get(ch, 0) + 1
+    n = len(s)
+    return -sum((c / n) * math.log2(c / n) for c in counts.values())
+
+
+def _is_placeholder(value: str) -> bool:
+    return bool(_PLACEHOLDER_RE.search(value))
+
+
+def _too_big(content: str) -> bool:
+    return len(content.encode("utf-8", errors="ignore")) > _MAX_SCAN_BYTES
+
+
+def scan_secrets(path: str, content: str) -> List[Tuple[str, str]]:
+    """Return ``[(ruleName, reminder), ...]`` for credentials found in *content*.
+
+    Each rule fires at most once. Obvious placeholders/example values are
+    excluded to keep the false-positive rate low. *path* is accepted for
+    symmetry with the regex scanner; secrets are scanned in any file type
+    (config/.env files matter most).
+    """
+    if not content or _too_big(content):
+        return []
+    hits: List[Tuple[str, str]] = []
+    seen: Set[str] = set()
+    for rule_name, kind, rx in _SECRET_RULES:
+        m = rx.search(content)
+        if not m or rule_name in seen:
+            continue
+        # Fixed-prefix rules are high-precision — only suppress documented
+        # EXAMPLE-style tokens. The assignment-style rule's value can legitimately
+        # be a structural placeholder ("your-key-here"), so it gets the broad set.
+        excl = _PLACEHOLDER_RE if rule_name == "generic_secret_assignment" else _EXAMPLE_RE
+        if excl.search(m.group(0)):
+            continue
+        seen.add(rule_name)
+        hits.append((rule_name, _SECRET_REMINDER.format(kind=kind)))
+    # Entropy backstop — only when no known-format secret already fired, so a
+    # single hardcoded secret never produces two near-duplicate warnings.
+    if not hits:
+        for m in _SECRET_ASSIGN_RE.finditer(content):
+            value = m.group(2)
+            if _is_placeholder(value):
+                continue
+            if shannon_entropy(value) >= _ENTROPY_THRESHOLD:
+                hits.append(("high_entropy_secret", _ENTROPY_REMINDER))
+                break
+    return hits

tests/plugins/test_security_guidance_secrets.py

@@ -0,0 +1,197 @@
+"""Tests for secret detection in the security-guidance plugin (#398).
+
+Covers ``plugins/security-guidance/secrets.py``:
+  * regex detection of well-known credential formats (AWS, GitHub, Slack,
+    Google, Stripe, npm, PEM private key, JWT, generic assignment),
+  * the conservative Shannon-entropy backstop,
+  * false-positive sanity (benign code + placeholder/example values),
+  * end-to-end wiring through the plugin's warn-mode hook.
+
+Token-shaped fixtures are ASSEMBLED FROM PARTS at runtime so neither the
+repo's secret scanners (GitGuardian on the PR) nor the I/O redactor sees a
+contiguous credential in this file. The detector runs on the concatenated
+runtime value, so detection still exercises the real regexes.
+"""
+
+import importlib.util
+import sys
+import types
+from pathlib import Path
+
+import pytest
+
+
+def _repo_root() -> Path:
+    return Path(__file__).resolve().parents[2]
+
+
+def _load_secrets():
+    """Import secrets.py in isolation (stdlib-only, no plugin glue)."""
+    path = _repo_root() / "plugins" / "security-guidance" / "secrets.py"
+    spec = importlib.util.spec_from_file_location(
+        "security_guidance_secrets_under_test", path
+    )
+    mod = importlib.util.module_from_spec(spec)
+    spec.loader.exec_module(mod)
+    return mod
+
+
+def _load_plugin_init():
+    """Import the plugin __init__.py with patterns.py + secrets.py as siblings."""
+    plugin_dir = _repo_root() / "plugins" / "security-guidance"
+    if "hermes_plugins" not in sys.modules:
+        ns = types.ModuleType("hermes_plugins")
+        ns.__path__ = []
+        sys.modules["hermes_plugins"] = ns
+    spec = importlib.util.spec_from_file_location(
+        "hermes_plugins.security_guidance",
+        plugin_dir / "__init__.py",
+        submodule_search_locations=[str(plugin_dir)],
+    )
+    mod = importlib.util.module_from_spec(spec)
+    mod.__package__ = "hermes_plugins.security_guidance"
+    mod.__path__ = [str(plugin_dir)]
+    sys.modules["hermes_plugins.security_guidance"] = mod
+    spec.loader.exec_module(mod)
+    return mod
+
+
+# Assembled fake credentials (split so secret scanners don't match the file).
+_AWS_KEY = "AKIA" + "QKZ7X2MNOP3RTUV9"                       # AKIA + 16 upper/digits
+_GH_TOKEN = "ghp" + "_" + ("b" * 36)                         # gh?_ + 36 alnum
+_SLACK = "xoxb" + "-" + "123456789012" + "-" + "abcdefghijkl"
+_GOOGLE = "AIza" + "Sy" + ("C" * 33)                         # AIza + 35
+_STRIPE = "sk" + "_live_" + ("9" * 24)
+_NPM = "npm" + "_" + ("a" * 36)
+_PEM = "-----BEGIN " + "RSA PRIVATE KEY-----"
+_JWT = "eyJ" + ("hbGciOiJIUzI1NiJ9") + "." + "eyJ" + ("zdWIiOiIxMjM0NTY3ODkwIn0") + "." + ("SflKxwRJ_signature_part")
+_HIGH_ENTROPY = "kJ8x2Qm9Zp4Lw7Nv1Rb6Tc3Yd5Fg0Hh"           # 32 mixed chars
+
+
+class TestRegexSecretDetection:
+    def setup_method(self):
+        self.s = _load_secrets()
+
+    def _names(self, content):
+        return {name for name, _ in self.s.scan_secrets("f.py", content)}
+
+    def test_aws_access_key_detected(self):
+        assert "aws_access_key_id" in self._names(f'key = "{_AWS_KEY}"\n')
+
+    def test_pem_private_key_detected(self):
+        assert "private_key_pem" in self._names(_PEM + "\nMIIE...\n")
+
+    def test_slack_token_detected(self):
+        assert "slack_token" in self._names(f'tok = "{_SLACK}"\n')
+
+    def test_github_token_detected(self):
+        assert "github_token" in self._names(f'gh = "{_GH_TOKEN}"\n')
+
+    def test_google_api_key_detected(self):
+        assert "google_api_key" in self._names(f'g = "{_GOOGLE}"\n')
+
+    def test_stripe_key_detected(self):
+        assert "stripe_secret_key" in self._names(f'sk = "{_STRIPE}"\n')
+
+    def test_npm_token_detected(self):
+        assert "npm_token" in self._names(f'n = "{_NPM}"\n')
+
+    def test_jwt_detected(self):
+        assert "jwt_token" in self._names(f'jwt = "{_JWT}"\n')
+
+    def test_generic_api_key_assignment_detected(self):
+        names = self._names('api_key = "' + ("Z" * 24) + '"\n')
+        assert "generic_secret_assignment" in names
+
+    def test_prefix_key_with_filler_substring_still_detected(self):
+        # A real fixed-prefix key that happens to contain "00000000" must NOT be
+        # suppressed — placeholder exclusion for prefix rules is EXAMPLE-only,
+        # so a real secret is never silently dropped (nit #1, fail-open fix).
+        tok = "ghp" + "_" + "00000000" + ("c" * 28)  # 36 chars after ghp_
+        assert "github_token" in self._names(f'gh = "{tok}"\n')
+
+    def test_each_rule_fires_once(self):
+        content = f'a = "{_AWS_KEY}"\nb = "{_AWS_KEY}"\n'
+        findings = self.s.scan_secrets("f.py", content)
+        assert sum(1 for n, _ in findings if n == "aws_access_key_id") == 1
+
+
+class TestEntropyBackstop:
+    def setup_method(self):
+        self.s = _load_secrets()
+
+    def test_high_entropy_secret_assignment_flagged(self):
+        # 'db_credential' is in the entropy keyword set but is NOT a known-format
+        # rule, so only the entropy backstop can catch this random value.
+        names = {n for n, _ in self.s.scan_secrets("f.py", f'db_credential = "{_HIGH_ENTROPY}"\n')}
+        assert "high_entropy_secret" in names
+
+    def test_low_entropy_secret_named_value_not_flagged(self):
+        # Long but low-entropy (repetitive) value assigned to a secret key.
+        names = {n for n, _ in self.s.scan_secrets("f.py", 'password = "aaaaaaaaaaaaaaaaaaaaaaaa"\n')}
+        assert "high_entropy_secret" not in names
+
+    def test_shannon_entropy_sanity(self):
+        assert self.s.shannon_entropy("") == 0.0
+        assert self.s.shannon_entropy("aaaaaaaa") < 1.0
+        assert self.s.shannon_entropy(_HIGH_ENTROPY) > 4.0
+
+    def test_entropy_skipped_when_known_secret_already_found(self):
+        # AWS regex fires -> entropy backstop suppressed (no duplicate noise).
+        names = {n for n, _ in self.s.scan_secrets("f.py", f'secret = "{_AWS_KEY}"\n')}
+        assert "high_entropy_secret" not in names
+
+
+class TestFalsePositiveSanity:
+    def setup_method(self):
+        self.s = _load_secrets()
+
+    def test_benign_code_no_findings(self):
+        content = "def add(a, b):\n    return a + b\n\nAPI_TIMEOUT = 30\n"
+        assert self.s.scan_secrets("f.py", content) == []
+
+    def test_placeholder_api_key_not_flagged(self):
+        assert self.s.scan_secrets("f.py", 'api_key = "your-api-key-here"\n') == []
+
+    def test_example_value_not_flagged(self):
+        assert self.s.scan_secrets("f.py", 'token = "EXAMPLE_TOKEN_VALUE_1234567890"\n') == []
+
+    def test_empty_content_no_findings(self):
+        assert self.s.scan_secrets("f.py", "") == []
+
+    def test_huge_content_skipped(self):
+        big = "x = 1\n" * 60000  # > 256 KB
+        assert self.s.scan_secrets("f.py", big) == []
+
+
+class TestHookIntegration:
+    def test_write_file_with_aws_key_warns(self, monkeypatch):
+        monkeypatch.delenv("SECURITY_GUIDANCE_BLOCK", raising=False)
+        monkeypatch.delenv("SECURITY_GUIDANCE_DISABLE", raising=False)
+        mod = _load_plugin_init()
+        args = {"path": "/tmp/config.py", "content": f'AWS = "{_AWS_KEY}"\n'}
+        result = mod._on_transform_tool_result(
+            tool_name="write_file",
+            args=args,
+            result='{"success": true, "bytes_written": 40}',
+        )
+        assert isinstance(result, str)
+        assert "Security guidance" in result
+        assert "credential" in result.lower()
+
+    def test_clean_write_no_warning(self, monkeypatch):
+        monkeypatch.delenv("SECURITY_GUIDANCE_BLOCK", raising=False)
+        monkeypatch.delenv("SECURITY_GUIDANCE_DISABLE", raising=False)
+        mod = _load_plugin_init()
+        args = {"path": "/tmp/ok.py", "content": "x = 1\n"}
+        assert mod._on_transform_tool_result(
+            tool_name="write_file", args=args, result='{"success": true}'
+        ) is None
+
+    def test_block_mode_refuses_write_with_secret(self, monkeypatch):
+        monkeypatch.setenv("SECURITY_GUIDANCE_BLOCK", "1")
+        monkeypatch.delenv("SECURITY_GUIDANCE_DISABLE", raising=False)
+        mod = _load_plugin_init()
+        args = {"path": "/tmp/config.py", "content": f'GH = "{_GH_TOKEN}"\n'}
+        out = mod._on_pre_tool_call(tool_name="write_file", args=args)
+        assert isinstance(out, dict) and out.get("action") == "block"