[K9VULN-16354] Differentiate local module findings per caller to prevent fingerprint collisions#197
Conversation
…y so identical inputs from different callers do not share one cached subtree.
…ws for instantiated local module resources.
…by FileMetadata clones that carry the call chain and keep line info for detection.
cf0b527 to
1ad0cdd
Compare
|
🎯 Code Coverage (details) 🔗 Commit SHA: ec726e1 | Docs | Datadog PR Page | Give us feedback! |
… platform-specific segment assembly.
1ad0cdd to
ec726e1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec726e1ff2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ResourceSource: linesVulne.ResourceSource, | ||
| FileSource: linesVulne.FileSource, | ||
| BlockLocation: linesVulne.BlockLocation, | ||
| ModuleCallChain: file.ModuleCallChain, |
There was a problem hiding this comment.
Include module chain before storage de-duplication
Fresh evidence for the similarity-ID collision is that the normal scan path persists these results and then calls MemoryStorage.GetVulnerabilities before CreateSummary/SARIF; that de-dup key includes SimilarityID but not ModuleCallChain. Since the chain is only copied here after generateSimilaritiesID ran above, two root callers with identical resolved values still have the same query/file/line/search/keyActualValue and one is dropped before the SARIF fingerprint can use the chain, so the full CLI scan still collapses the exact scenario this change is meant to preserve.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I am dropping similarityId in Remove SimilarityID and legacy exclude-results suppression
Motivation
When the same local Terraform module is instantiated from more than one caller, the resources it produces are attributed to the module body file and share the same finding fingerprint (repository, file path, resource type/name, rule id). Distinct module calls therefore collide and collapse into a single finding, hiding per-caller context and the per-call resolved values.
This stacks on top of #193 and folds the module call chain into the fingerprint so each caller yields its own finding.
JIRA Ticket: K9VULN-16354
Changes
VulnerableFile, and folded into the Datadog fingerprint viaGetDatadogFingerprintHash. When the chain is empty (every non-module finding) the hash is byte-identical to before, so existing fingerprints are unchanged.Author Checklist
QA Instruction
Scan a repo where two root stacks call the same local module (even with identical inputs). Confirm two findings are produced, each attributed to the module body file with a distinct fingerprint, instead of a single collapsed finding. Confirm non-module findings keep their previous fingerprints. See
TestInspect_TwoCallersGetDistinctCallChainsandTestGetDatadogFingerprintHash_ModuleCallChain.Blast Radius
This only affects findings for resources instantiated from local Terraform modules: their document identity and fingerprint now include the call chain. Root resources and all non-module findings are unchanged (empty chain reproduces the previous hash).
Additional Notes
Builds on #193; the call chain is used only for fingerprinting for now and is also carried on the finding so it can be surfaced to users in a follow-up.
I submit this contribution under the Apache-2.0 license.