Count runes in CEL entropy() to match reported entropy - #184
Conversation
The CEL entropy() helper used by rule filters (e.g. entropy(secret) <= 3.0) counted bytes, while the entropy reported on findings (detect.shannonEntropy) counts runes. For non-ASCII secrets the two disagreed, so a rule could filter on a different value than the one shown in the report. Compute over runes with the same formula; ASCII results are unchanged.
|
| Filename | Overview |
|---|---|
| internal/celenv/bindings_filter.go | Switches entropy counting from bytes to runes; denominator intentionally kept as byte-length to match detect.shannonEntropy — formula is consistent with the reference implementation. |
| internal/celenv/entropy_test.go | New test pins rune-based behavior but uses a want function that is a copy of the implementation, making it tautological for future divergence detection. |
Reviews (1): Last reviewed commit: "count runes in celShannonEntropy to matc..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| want := func(s string) float64 { | ||
| if len(s) == 0 { | ||
| return 0 | ||
| } | ||
| counts := map[rune]int{} | ||
| for _, r := range s { | ||
| counts[r]++ | ||
| } | ||
| inv := 1.0 / float64(len(s)) | ||
| var h float64 | ||
| for _, c := range counts { | ||
| p := float64(c) * inv | ||
| h -= p * math.Log2(p) | ||
| } | ||
| return h | ||
| } | ||
|
|
||
| for _, s := range []string{"", "a", "aabb", "abcd1234", "日本語テスト", "🔑🔑🔒"} { | ||
| got := celShannonEntropy(s) | ||
| if exp := want(s); math.Abs(got-exp) > 1e-9 { | ||
| t.Errorf("celShannonEntropy(%q) = %v, want %v (rune-based)", s, got, exp) |
There was a problem hiding this comment.
The want lambda is a verbatim copy of celShannonEntropy, so the test only proves the function equals itself. If a future refactor accidentally re-diverges from detect.shannonEntropy (e.g. someone fixes the denominator to use rune count), the test will still pass even though the two would disagree again. Calling detect.shannonEntropy directly (or at least hard-coding known expected values for the non-ASCII cases) would catch that drift.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
The CEL
entropy()helper used by rule filters (e.g.entropy(secret) <= 3.0) counted bytes, while the entropy reported on findings (detect.shannonEntropy) counts runes. For non-ASCII secrets the two disagreed, so a rule could filter on a different value than the one shown in the report.Compute entropy over runes with the same formula; ASCII results are unchanged. Added a test pinning the rune-based behavior.