Repository navigation
feat: entropy-based secret detection (kind: high_entropy_string + SEC-SECRETS-ENTROPY-001) - #34
Open
ChrisAdkin8 wants to merge 1 commit into
Open
ChrisAdkin8 wants to merge 1 commit into
ChrisAdkin8 wants to merge 1 commit into
Conversation
…high_entropy_string) The audit's #1 capability quick-win — complements the name-based grep secret rules (which only fire on password/secret/api_key-style fields) by catching high-entropy tokens dropped into *any* argument, regardless of its name. New pattern kind `high_entropy_string` (scripts/_handlers_security.py): for each resource body (comments stripped first, length-preserving so lines stay accurate), it scans `name = "literal"` assignments and flags values that are charset-gated (base64/token), length-bounded (20–100), mix ≥2 character classes, and clear a Shannon-entropy threshold (default 4.0 bits/char). Catalogue- configurable: min_length / max_length / min_entropy. Design calibrated empirically against real secrets vs. common Terraform non-secrets: - base64 charset + H≥4.0 separates genuine tokens (AWS secret 4.61, GitHub PAT 4.14) from hex strings / git SHAs / image digests (only 16 symbols → H≈3.7) — so no special hex handling and the git-SHA false-positive class is avoided. - excludes interpolations/refs (`${...}`, `var.*`), cloud resource-ids (ami-/ vpc-/…), and single-character-class kebab/snake names (bucket names) that can clear the entropy bar by length alone. - max_length drops long base64 blobs (certs / user_data) that aren't tokens. Rule SEC-SECRETS-ENTROPY-001 (HIGH, CWE-798/CWE-259, T1552.001) is a heuristic — findings ask the user to verify + rotate. No fix_hcl (can't auto-fix a secret). Tests: tests/test_entropy_secrets.py (23 — entropy math, classifier exclusions, handler) + dirty/clean fixtures via self_test (262/262 clean). Terragoat unchanged (the rule adds zero findings there — conservative by design). Catalogue 353 → 354; rule docs regenerated; catalog/README + docs/custom-rules kinds tables + README counts updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR introduces SEC-SECRETS-ENTROPY-001, a new security detection rule that identifies probable hardcoded secrets in Terraform by analyzing string-literal entropy. The implementation includes a handler function computing Shannon entropy, comprehensive unit and integration tests, a complete rule catalog entry, test fixtures, and documentation. ChangesEntropy-based secret detection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/custom-rules.md`:
- Line 134: Update the documentation line for the `high_entropy_string` rule to
insert the missing word "as" so the phrase reads "marks it as a probable
hardcoded secret"; locate the table row containing `high_entropy_string` and
update the description accordingly.
In `@docs/rules/SEC-SECRETS-ENTROPY-001.md`:
- Line 40: Fix the grammar by hyphenating the compound adjective in the sentence
that reads "**High-entropy string literal — probable hardcoded secret (API token
/ access key).** This rule has `default_urgency: HIGH` and operates on a
infrastructure wide blast radius." — replace "infrastructure wide" with
"infrastructure-wide" so the phrase correctly reads "operates on an
infrastructure-wide blast radius."
In `@scripts/_handlers_security.py`:
- Around line 436-462: The handler currently only iterates c.resources, but the
rule text promises "any argument" — either expand the scan to include other
block kinds (e.g., iterate c.modules, c.data, c.variables, c.outputs,
c.providers or a generic c.blocks collection if available) and apply the same
logic using _ENTROPY_ASSIGN_RE, _is_high_entropy_secret and the out.append
payload, or else tighten the rule docs to say it only checks resource arguments;
update the loop that references c.resources and the comment/contract accordingly
so high-entropy literals in module/data/variable/output/provider blocks are
covered or the contract is explicit.
In `@tests/test_entropy_secrets.py`:
- Around line 23-24: The test uses a provider-shaped AWS secret fixture named
TOKEN (and additional provider-like fixtures in the same file) which can trigger
secret scanners; replace TOKEN and the other similar constants with neutral
high-entropy synthetic strings (random-looking but not matching AWS/GitHub
formats) that preserve the entropy characteristics the tests assert against,
update any assertions or expectations if they reference the literal value, and
ensure all other provider-shaped fixtures in that test file are similarly
replaced.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dcecd10b-6ec7-4b54-b77a-0b37cdb228ea
📒 Files selected for processing (10)
README.mdcatalog/README.mdcatalog/SEC-SECRETS-ENTROPY-001.yamldocs/custom-rules.mddocs/rules/SEC-SECRETS-ENTROPY-001.mddocs/rules/index.mdfixtures/SEC-SECRETS-ENTROPY-001_clean/main.tffixtures/sec_secrets_entropy_001/main.tfscripts/_handlers_security.pytests/test_entropy_secrets.py
| | `hcl_attr` | A resource attribute equals (or doesn't equal) a specific value | | ||
| | `grep` | Plain-text regex search | | ||
| | `iam_policy_analysis` | Walk `data.aws_iam_policy_document` statements (see below) | | ||
| | `high_entropy_string` | A string literal whose Shannon entropy marks it a probable hardcoded secret (any arg name) — complements the name-based secret greps | |
There was a problem hiding this comment.
Grammar: Insert missing word "as".
The phrase "marks it a probable hardcoded secret" should be "marks it as a probable hardcoded secret".
📝 Proposed fix
-| `high_entropy_string` | A string literal whose Shannon entropy marks it a probable hardcoded secret (any arg name) — complements the name-based secret greps |
+| `high_entropy_string` | A string literal whose Shannon entropy marks it as a probable hardcoded secret (any arg name) — complements the name-based secret greps |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| | `high_entropy_string` | A string literal whose Shannon entropy marks it a probable hardcoded secret (any arg name) — complements the name-based secret greps | | |
| | `high_entropy_string` | A string literal whose Shannon entropy marks it as a probable hardcoded secret (any arg name) — complements the name-based secret greps | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/custom-rules.md` at line 134, Update the documentation line for the
`high_entropy_string` rule to insert the missing word "as" so the phrase reads
"marks it as a probable hardcoded secret"; locate the table row containing
`high_entropy_string` and update the description accordingly.
|
|
||
| <p><a href="vscode://tfanalyze.tf-analyze/rule/SEC-SECRETS-ENTROPY-001" style="display:inline-block;padding:6px 12px;background:#157878;color:#fff;text-decoration:none;border-radius:4px;font-weight:600;font-size:14px;margin-top:6px">📂 Open in VS Code</a><a href="vscode://tfanalyze.tf-analyze/suppress?id=SEC-SECRETS-ENTROPY-001" style="display:inline-block;padding:6px 12px;background:#fff;color:#c27a00;text-decoration:none;border:1px solid #c27a00;border-radius:4px;font-weight:600;font-size:14px;margin-top:6px;margin-left:6px" title="Add SEC-SECRETS-ENTROPY-001 to .tf-analyze.yaml's ignore_rules in your workspace">📝 Suppress in workspace</a> <span style="color:#666;font-size:12px;margin-left:4px">(requires the <a href="https://marketplace.visualstudio.com/items?itemName=tfanalyze.tf-analyze" style="color:#157878">tf-analyze extension</a>)</span></p> | ||
|
|
||
| > **High-entropy string literal — probable hardcoded secret (API token / access key).** This rule has `default_urgency: HIGH` and operates on a infrastructure wide blast radius. |
There was a problem hiding this comment.
Grammar: Hyphenate compound adjective.
"infrastructure wide" should be "infrastructure-wide" when used as a compound adjective.
📝 Proposed fix
-> **High-entropy string literal — probable hardcoded secret (API token / access key).** This rule has `default_urgency: HIGH` and operates on a infrastructure wide blast radius.
+> **High-entropy string literal — probable hardcoded secret (API token / access key).** This rule has `default_urgency: HIGH` and operates on an infrastructure-wide blast radius. 📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| > **High-entropy string literal — probable hardcoded secret (API token / access key).** This rule has `default_urgency: HIGH` and operates on a infrastructure wide blast radius. | |
| > **High-entropy string literal — probable hardcoded secret (API token / access key).** This rule has `default_urgency: HIGH` and operates on an infrastructure-wide blast radius. |
🧰 Tools
🪛 LanguageTool
[grammar] ~40-~40: Use a hyphen to join words.
Context: ...: HIGH` and operates on a infrastructure wide blast radius. ## What this checks...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/rules/SEC-SECRETS-ENTROPY-001.md` at line 40, Fix the grammar by
hyphenating the compound adjective in the sentence that reads "**High-entropy
string literal — probable hardcoded secret (API token / access key).** This rule
has `default_urgency: HIGH` and operates on a infrastructure wide blast radius."
— replace "infrastructure wide" with "infrastructure-wide" so the phrase
correctly reads "operates on an infrastructure-wide blast radius."
Comment on lines
+436
to
+462
| for blk in c.resources: | ||
| btype, bname = blk["groups"] | ||
| body = strip_hcl_context(blk["body"]) # blank comments, keep offsets | ||
| seen: set[tuple[str, str]] = set() | ||
| for m in _ENTROPY_ASSIGN_RE.finditer(body): | ||
| arg, value = m.group(1), m.group(2) | ||
| if (arg, value) in seen: | ||
| continue | ||
| if not _is_high_entropy_secret( | ||
| value, min_len=min_len, max_len=max_len, min_entropy=min_entropy | ||
| ): | ||
| continue | ||
| seen.add((arg, value)) | ||
| line = blk["start_line"] + body.count("\n", 0, m.start()) | ||
| ent = _shannon_entropy(value) | ||
| out.append({ | ||
| "id": c.eid, | ||
| "file": str(c.file_path), | ||
| "line": line, | ||
| "resource": f"{btype}.{bname}", | ||
| "context": ( | ||
| f"high-entropy string assigned to `{arg}` " | ||
| f"(entropy {ent:.2f} bits/char, length {len(value)}) " | ||
| f"— probable hardcoded secret" | ||
| ), | ||
| }) | ||
| return out |
There was a problem hiding this comment.
Broaden this beyond resource blocks or narrow the rule contract.
The rule text says this catches high-entropy literals in any argument, but the handler only walks c.resources. That misses the same hardcoded secret in module, data, variable default, output, and provider blocks, so the implementation under-delivers on the advertised coverage.
At minimum, either scan the additional block kinds from c.text as well, or explicitly scope the catalogue/docs to “resource arguments only.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/_handlers_security.py` around lines 436 - 462, The handler currently
only iterates c.resources, but the rule text promises "any argument" — either
expand the scan to include other block kinds (e.g., iterate c.modules, c.data,
c.variables, c.outputs, c.providers or a generic c.blocks collection if
available) and apply the same logic using _ENTROPY_ASSIGN_RE,
_is_high_entropy_secret and the out.append payload, or else tighten the rule
docs to say it only checks resource arguments; update the loop that references
c.resources and the comment/contract accordingly so high-entropy literals in
module/data/variable/output/provider blocks are covered or the contract is
explicit.
Comment on lines
+23
to
+24
| # A well-known *example* AWS secret-key string (not a live credential). | ||
| TOKEN = "wJalrXUtnFEMIK7MDENGbPxRfiCYEXAMPLEKEY" |
There was a problem hiding this comment.
Use neutral high-entropy fixtures instead of provider-shaped credentials.
These literals are realistic enough to trip secret scanners already, which will create noisy CI and false incident churn even though they are test data. Since the behavior under test is entropy-based, you can keep the same coverage with synthetic tokens that do not match AWS/GitHub credential formats.
Suggested test-data swap
-# A well-known *example* AWS secret-key string (not a live credential).
-TOKEN = "wJalrXUtnFEMIK7MDENGbPxRfiCYEXAMPLEKEY"
+# Neutral high-entropy fixture; shaped for entropy tests, not vendor scanners.
+TOKEN = "kT9xQ2mZ7pL4vR8nW1cF3hJ6dS5gB0yA7zXqWeRt"
@@
`@pytest.mark.parametrize`("v", [
TOKEN, # AWS secret-style, H=4.61
- "ghp_16C7e42F292c6912E7710c838347Ae178B4a", # GitHub PAT-ish, H=4.14
- "kT9xQ2mZ7pL4vR8nW1cF3hJ6dS5gB0yA7zXqWeRt", # random base64 token
+ "Q7mR2xLp8Vn4cF1hJ6dS5gB0yA7zXqWeRt3NkTuPw", # neutral token
+ "M4vR8nW1cF3hJ6dS5gB0yA7zXqWeRt9PkLs2DfGh", # neutral token
])Also applies to: 51-55
🧰 Tools
🪛 Ruff (0.15.14)
[error] 24-24: Possible hardcoded password assigned to: "TOKEN"
(S105)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_entropy_secrets.py` around lines 23 - 24, The test uses a
provider-shaped AWS secret fixture named TOKEN (and additional provider-like
fixtures in the same file) which can trigger secret scanners; replace TOKEN and
the other similar constants with neutral high-entropy synthetic strings
(random-looking but not matching AWS/GitHub formats) that preserve the entropy
characteristics the tests assert against, update any assertions or expectations
if they reference the literal value, and ensure all other provider-shaped
fixtures in that test file are similarly replaced.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The audit's #1 capability quick-win: pure-stdlib entropy-based secret
detection that complements the existing name-based grep secret rules.
Those only fire on
password/secret/api_key-style fields; this catches ahigh-entropy token dropped into any argument, regardless of its name.
New pattern kind:
high_entropy_stringscripts/_handlers_security.py. For each resource body (comments stripped first— length-preserving, so line numbers stay accurate) it scans
name = "literal"assignments and flags values that are:
All three thresholds are catalogue-configurable (
min_length/max_length/min_entropy).Why these thresholds (calibrated empirically, not guessed)
Measured real secrets vs. common Terraform non-secrets:
GitHub PAT 4.14, random 4.58) from hex strings / git SHAs / image
digests — which use only 16 symbols so top out at H≈3.7, below the
bar. So v1 needs no separate hex handling and dodges the git-SHA
false-positive class entirely.
${…},var.*), cloud resource-ids(
ami-/vpc-/…), and single-character-class kebab/snake names(
my-app-prod-logs-bucket) that can clear the entropy bar on length alone.max_lengthdrops long base64 blobs (certs /user_data) that aren't tokens.Rule
SEC-SECRETS-ENTROPY-001— HIGH, CWE-798/CWE-259, MITRE T1552.001. Aheuristic, so findings ask the operator to verify + rotate; no
fix_hcl(youcan't auto-fix a leaked secret).
Verification
tests/test_entropy_secrets.py— 23 tests (entropy math, classifierexclusions incl. the kebab-name FP a test caught during dev, handler
location/line/threshold/comment-awareness).
design, so no snapshot/CI-tolerance churn).
sync, both doc-drift gates clean. Catalogue 353 → 354 (rule docs regenerated;
catalog/README + docs/custom-rules kinds tables + README counts updated).
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation