diff --git a/docs/usage.md b/docs/usage.md index b6f01a5..c1b43a5 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -130,7 +130,7 @@ sources and in configuration files. |---|---|---| | `hardcoded-secret` | high | A provider-specific format: AWS, GitHub, Slack, Stripe, Google, private keys, JWTs, SendGrid, Twilio. | | `hardcoded-credential` | medium | A credential-like name (`password`, `token`, `api_key`, `app.config['SECRET_KEY']`, …) bound to a real value. Placeholders are excluded. | -| `high-entropy-string` | low | An opaque high-entropy token. | +| `high-entropy-string` | low | An opaque high-entropy token. Hex digests (MD5, SHA-1, SHA-256, SHA-512 lengths, or a name such as `hash`, `checksum`, `commit`), subresource-integrity hashes (`sha512-…`) and character-set constants are not secrets unless a credential name says so. | ### Dependencies diff --git a/src/coretrace_python/plugins/secrets.py b/src/coretrace_python/plugins/secrets.py index 31fe8b4..3cad3e1 100644 --- a/src/coretrace_python/plugins/secrets.py +++ b/src/coretrace_python/plugins/secrets.py @@ -37,6 +37,35 @@ {"", "changeme", "change_me", "password", "passwd", "secret", "token", "example", "xxx", "todo", "none", "null"} ) _NOT_CREDENTIAL_SUFFIXES = ("_name", "_field", "_file", "_path", "_url", "_id", "_env", "_var", "_header", "_param") +# MD5, SHA-1, SHA-256 and SHA-512 digests, as hex. +_DIGEST_LENGTHS = frozenset({32, 40, 64, 128}) +_DIGEST_WORDS = ("hash", "digest", "sha", "md5", "checksum", "commit", "etag", "fingerprint", "revision", "version") +# Subresource integrity, as in lock files: ``sha512-``. +_INTEGRITY = re.compile(r"^(md5|sha1|sha256|sha384|sha512)-[A-Za-z0-9+/=]+$", re.IGNORECASE) +_ALPHABET_WORDS = ("alphabet", "charset", "characters", "letters", "digits") + + +def looks_like_digest(value: str, name: str | None) -> bool: + """A hex value of digest length, a hex value whose name names a hash, or a + subresource-integrity hash: a commit id, a checksum, a content hash, never a secret + unless a credential name says so.""" + + if _INTEGRITY.match(value) is not None or (name is not None and name.lower() == "integrity"): + return True + if _HEX.match(value) is None: + return False + if len(value) in _DIGEST_LENGTHS: + return True + return name is not None and any(word in name.lower() for word in _DIGEST_WORDS) + + +def is_alphabet(value: str, name: str | None) -> bool: + """A constant that spells out a character set is high-entropy by construction: its + name says so, or every character in it is distinct, which no random token is.""" + + if name is not None and any(word in name.lower() for word in _ALPHABET_WORDS): + return True + return len(value) >= 16 and len(set(value)) == len(value) def shannon_entropy(text: str) -> float: @@ -302,7 +331,7 @@ def judge(self, value: str, name: str | None, span: SourceSpan, function: str | {"name": name, "length": str(len(value))}, ) entropy = self.entropy_of(value) - if entropy is not None: + if entropy is not None and not looks_like_digest(value, name) and not is_alphabet(value, name): return Finding( "high-entropy-string", f"High-entropy string {where}: {redacted(value)}", diff --git a/tests/regression/expected/MiniBookApiServer.json b/tests/regression/expected/MiniBookApiServer.json index 28b8033..dbbe5a2 100644 --- a/tests/regression/expected/MiniBookApiServer.json +++ b/tests/regression/expected/MiniBookApiServer.json @@ -30,12 +30,6 @@ "rule": "hardcoded-credential", "function": null }, - { - "file": "docs/insomnia/insomnia_config_file.yaml", - "line": 71, - "rule": "high-entropy-string", - "function": null - }, { "file": "docs/insomnia/insomnia_config_file.yaml", "line": 231, diff --git a/tests/regression/expected/healthchecks.json b/tests/regression/expected/healthchecks.json index 4f9f090..5e94173 100644 --- a/tests/regression/expected/healthchecks.json +++ b/tests/regression/expected/healthchecks.json @@ -174,12 +174,6 @@ "rule": "hardcoded-credential", "function": "test_it_handles_bad_signature" }, - { - "file": "hc/api/tests/test_tokenbucket.py", - "line": 12, - "rule": "high-entropy-string", - "function": null - }, { "file": "hc/api/tests/test_tokenbucket.py", "line": 15, diff --git a/tests/test_hex_digests.py b/tests/test_hex_digests.py new file mode 100644 index 0000000..fcb281f --- /dev/null +++ b/tests/test_hex_digests.py @@ -0,0 +1,89 @@ +"""Acceptance tests for hex digests in the secret scanners (issue #71). + +On real projects, pure-hex strings of digest length (MD5, SHA-1, SHA-256, SHA-512) under +names such as ``version``, ``sha`` or ``checksum`` flooded ``high-entropy-string``: 5 500 +findings in one library's benchmark results, all commit ids. A hex digest is not a secret +unless its name says so: a digest-shaped value, or a hex value whose name names a hash, +is skipped by the entropy rule, while a credential-like name still reports it. + +Expected to remain red until ``coretrace_python.plugins.secrets.looks_like_digest`` exists. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from coretrace_python import engine +from coretrace_python.findings import Finding +from coretrace_python.plugins import secrets +from coretrace_python.source import SourceManager + +MISSING = None if hasattr(secrets, "looks_like_digest") else "looks_like_digest is missing" + + +@pytest.fixture(autouse=True) +def require_digests() -> None: + if MISSING is not None: + pytest.fail(f"hex digest handling is not implemented yet: {MISSING}") + + +REPO = Path(__file__).resolve().parent.parent +PLUGINS = REPO / "src" / "coretrace_python" / "bundled" + +SHA256 = "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08" +SHA1 = "0a3fcb9c2b4d6e8f1a3c5e7b9d1f3a5c7e9b1d3f" +MD5 = "d41d8cd98f00b204e9800998ecf8427e" +HEX48 = "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822c" + + +def check(text: str) -> tuple[Finding, ...]: + return engine.check(SourceManager().add_source("m.py", text), [PLUGINS]) + + +def rules(findings: tuple[Finding, ...]) -> list[str]: + return sorted(f.rule_id for f in findings) + + +def test_digest_shaped_hex_values_are_not_secrets() -> None: + assert looks(SHA256, None) and looks(SHA1, "commit") and looks(MD5, "checksum_md5") + assert not looks(HEX48, None) + assert not looks("Qm9vdHN0cmFwcGluZyBhIHJhbmRvbSBrZXkgMjA0OA==", "digest") + assert check(f"digest = '{SHA256}'\nCOMMIT = '{SHA1}'\nchecksum = '{MD5}'\nblob = '{SHA256}'\n") == () + + +def looks(value: str, name: str | None) -> bool: + return secrets.looks_like_digest(value, name) + + +def test_hex_values_named_after_a_hash_are_not_secrets_whatever_their_length() -> None: + assert looks(HEX48, "file_sha") and looks(HEX48, "etag") and looks(HEX48, "fingerprint") + assert check(f"file_sha = '{HEX48}'\n") == () + + +def test_other_hex_values_and_credential_names_are_still_reported() -> None: + assert rules(check(f"blob = '{HEX48}'\n")) == ["high-entropy-string"] + assert rules(check(f"api_key = '{SHA256}'\n")) == ["hardcoded-credential"] + assert rules(check(f"password_hash = '{SHA256}'\n")) == ["hardcoded-credential"] + + +SRI = "sha512-Qm9vdHN0cmFwcGluZyBhIHJhbmRvbSBrZXkgMjA0OA/Qm9vdHN0cmFwcGluZyBhIHJhbmRvbSBrZXkgMjA0OA==" + + +def test_subresource_integrity_hashes_and_alphabets_are_not_secrets(tmp_path: Path) -> None: + assert looks(SRI, None) and looks(SRI, "integrity") + assert check("BASE62 = '0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz'\n") == () + (tmp_path / "app.py").write_text("", encoding="utf-8") + (tmp_path / "package-lock.json").write_text(f'{{"integrity": "{SRI}"}}\n', encoding="utf-8") + + assert engine.analyze_project(tmp_path, [PLUGINS]).findings == () + + +def test_configuration_files_follow_the_same_rule(tmp_path: Path) -> None: + (tmp_path / "app.py").write_text("", encoding="utf-8") + (tmp_path / "results.json").write_text(f'{{"version": "{SHA256}", "token": "{SHA256}"}}\n', encoding="utf-8") + + findings = engine.analyze_project(tmp_path, [PLUGINS]).findings + + assert [(f.rule_id, f.metadata["name"]) for f in findings] == [("hardcoded-credential", "token")] diff --git a/tests/test_secrets_sbom.py b/tests/test_secrets_sbom.py index 2c447ba..ad48b85 100644 --- a/tests/test_secrets_sbom.py +++ b/tests/test_secrets_sbom.py @@ -208,9 +208,10 @@ def test_high_entropy_tokens_without_context_are_low_confidence() -> None: assert RANDOM_BASE64 not in finding.message -def test_hex_digests_count_as_high_entropy() -> None: - (finding,) = check(f"digest = '{RANDOM_HEX}'\n") - assert finding.rule_id == "high-entropy-string" +def test_hex_digests_are_secrets_only_under_a_credential_name() -> None: + assert check(f"digest = '{RANDOM_HEX}'\n") == () + (finding,) = check(f"api_token = '{RANDOM_HEX}'\n") + assert finding.rule_id == "hardcoded-credential" def test_prose_urls_and_short_strings_are_not_secrets() -> None: