fix: populate fileHash in CompilerDataLogger telemetry#1199
Closed
martin-reznik wants to merge 7 commits into
Closed
fix: populate fileHash in CompilerDataLogger telemetry#1199martin-reznik wants to merge 7 commits into
martin-reznik wants to merge 7 commits into
Conversation
The hash field in CompilerDataLogger telemetry (AppInsights + CSV) was always empty because context.Hashes is not populated by the SARIF framework during rule execution. Fix: compute the hash in BA4001/BA4002 and pass it via CompilerData.FileHash. - PE: uses PE.SHA256Hash (zero I/O, already loaded in memory) - ELF/Mach-O: reads file and computes SHA-256 CompilerDataLogger.Write() now prefers compilerData.FileHash, falling back to context.Hashes?.Sha256 for backward compatibility. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Merged
Add cached SHA256Hash property on BinaryBase so ELF, Mach-O, and PE all share the same pattern. BA4002 now uses BinaryBase.SHA256Hash instead of a local helper. For PE, BA4001 still uses target.PE.SHA256Hash which benefits from the already-loaded in-memory image. For Mach-O fat binaries (multiple arch slices), the hash is of the whole file - all slices share one file and one hash. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Make BinaryBase.SHA256Hash virtual - Override in PEBinary to delegate to PE.SHA256Hash (uses in-memory image) - Add SHA256Hash to IDwarfBinary interface so BA4002 can use it without casting to BinaryBase - Update StubDwarfBinary in tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ssException Addresses CodeQL 'Generic catch clause' finding. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nal build trigger Replace System.AccessToken with AzureCLI@2 using the binskim-internal-pipeline-runner service connection to get a token for the ADO API. Fixes permission error when queuing the internal pipeline. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tryFileHash Add IncludeTelemetryFileHash policy option (default: true). When set to false, BA4001/BA4002 skip SHA-256 computation. Configurable via policy XML. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
|
Closing - hash fix will be handled in Guardian instead. Pipeline fixes moved to #1201. |
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
2 participants
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.
Summary
The hash field in CompilerDataLogger telemetry (AppInsights CompilerInformation events + CSV) has always been empty because context.Hashes is not populated by the SARIF framework during rule execution.
Root Cause
CompilerDataLogger.Write() reads context.Hashes?.Sha256, but the SARIF SDK only computes hashes for SARIF artifact entries after analysis - not before dispatching rules.
Fix
Compute the hash directly in the rules via new CompilerData.FileHash property:
Tests
Two tests verify hash is populated in CSV output with and without DataToInsert=Hashes.