Compute git-history secret fingerprints from historical blob content - #943
Compute git-history secret fingerprints from historical blob content#943MikaYuoadas wants to merge 1 commit into
Conversation
|
🎯 Code Coverage (details) 🔗 Commit SHA: 809f90b | Docs | Datadog PR Page | Give us feedback! |
ad15135 to
4393fa6
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the SARIF emission path for git-history secrets so DATADOG_FINGERPRINT is computed from the historical blob’s content (when the file/line may not exist at HEAD), ensuring stable deduplication for history-only findings.
Changes:
- Add
blob_contenttoHistoricalSecretResultand thread it through git-history secret scanning. - Factor fingerprint computation into a new
get_fingerprint_from_contentshelper and use it for historical secrets. - Add/extend regression tests to verify fingerprints are non-empty and match the HEAD algorithm for identical inputs, and that blob content aligns with match positions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/cli/src/sarif/sarif_utils.rs | Store historical blob content on results and compute SARIF fingerprints from it for historical secrets; add regression test. |
| crates/cli/src/file_utils.rs | Introduce get_fingerprint_from_contents and reuse it from the on-disk fingerprint path. |
| crates/bins/src/git_history.rs | Preserve decoded blob content for secret-bearing blobs and attach it to each HistoricalSecretResult; extend end-to-end test to validate blob/match wiring. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
… blob content History-only secrets were emitted with an empty DATADOG_FINGERPRINT because it was computed from the HEAD working tree, where the file or line no longer exists. Compute it from the historical blob content instead, reusing the same hashing algorithm, so these findings deduplicate correctly.
4393fa6 to
809f90b
Compare
|
|
||
| /// A secret-bearing blob: its decoded content (shared, since one blob fans out | ||
| /// into many (path, match) results) alongside every secret matched in it. | ||
| type SecretBlob = (Arc<str>, Vec<SecretResult>); |
There was a problem hiding this comment.
This is keeping the entire blob in memory for the entirety of the scanning process. I would imagine if a rule is prone to overmatching, this could bloat process memory very quickly.
Have you done any capacity estimation to estimate the expected worst case memory bloat this PR would introduce?
What
Compute the SARIF
DATADOG_FINGERPRINTfor history-only secrets (present in git history but removed at HEAD) from the historical blob content, instead of re-reading the HEAD working tree.Why
For a removed secret, the file or line no longer exists at HEAD, so the fingerprint came out empty and downstream deduplication dropped the finding. Sourcing it from the historical blob with the same hashing algorithm yields a stable, non-empty fingerprint so these findings open correctly.
Testing