ci(binskim): fix 'expected a result message' SARIF upload errors#175
Merged
Conversation
…ed a result message' errors BinSkim emits results that reference rule messageStrings by id (e.g. message.id=Warning_NativeWithInsecureStaticLibraryCompilands plus arguments) without an inlined message.text. GitHub Code Scanning's SARIF ingester requires message.text directly, so the binskim-rust configuration on main was failing with: 'expected a result message, expected a result message' (2 errors corresponding to the 2 suppressed BA2004/BA2024 results). Patch the SARIF in filter_binskim_sarif.py: walk runs->results, find results missing message.text, look up tool.driver.rules[ruleIndex] .messageStrings[message.id].text, .format(*arguments), and inline. No security findings change: both results are already suppressed with documented upstream-toolchain justifications. This only fixes the SARIF-upload error so the BinSkim configuration page shows clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
prsasattms
pushed a commit
that referenced
this pull request
Jun 3, 2026
The BinSkim go configuration on main was showing the warning 'unsuccessful tool execution, exit code 0' because BinSkim sets invocations[0].executionSuccessful=false when it can't load a PDB for the Go binary (ERR997.ExceptionLoadingPdb). Go's toolchain does not emit MSVC PDBs — this is the documented limitation, not a tool failure or a security finding. Changes: - scripts/security/filter_binskim_sarif.py: new _normalize_invocations step. Walks toolConfigurationNotifications, demotes ones matching binskim_suppressions.json[invocation_notifications] from error to note (with audit-trail suppression entry), and flips executionSuccessful back to True when no error-level notifications remain. - scripts/security/binskim_suppressions.json: add documented invocation_notifications entry for ERR997.ExceptionLoadingPdb on omnivec.exe with a security justification (Go security posture is enforced by Go toolchain + runtime, not by MSVC linker switches). - .github/workflows/binskim.yml: wire binskim-dotnet and binskim-go jobs through filter_binskim_sarif.py before SARIF upload. Rust was already wired (PR #175). After this PR all three configurations (binskim-dotnet, binskim-go, binskim-rust) should show clean on the BinSkim configuration page with no errors and no warnings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
prsasattms
added a commit
that referenced
this pull request
Jun 3, 2026
The BinSkim go configuration on main was showing the warning 'unsuccessful tool execution, exit code 0' because BinSkim sets invocations[0].executionSuccessful=false when it can't load a PDB for the Go binary (ERR997.ExceptionLoadingPdb). Go's toolchain does not emit MSVC PDBs — this is the documented limitation, not a tool failure or a security finding. Changes: - scripts/security/filter_binskim_sarif.py: new _normalize_invocations step. Walks toolConfigurationNotifications, demotes ones matching binskim_suppressions.json[invocation_notifications] from error to note (with audit-trail suppression entry), and flips executionSuccessful back to True when no error-level notifications remain. - scripts/security/binskim_suppressions.json: add documented invocation_notifications entry for ERR997.ExceptionLoadingPdb on omnivec.exe with a security justification (Go security posture is enforced by Go toolchain + runtime, not by MSVC linker switches). - .github/workflows/binskim.yml: wire binskim-dotnet and binskim-go jobs through filter_binskim_sarif.py before SARIF upload. Rust was already wired (PR #175). After this PR all three configurations (binskim-dotnet, binskim-go, binskim-rust) should show clean on the BinSkim configuration page with no errors and no warnings. Co-authored-by: Pradeep Sasatt <prsasatt@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Fixes the BinSkim "Code Scanning failed - 2 errors" banner on the
binskim-rustconfiguration page (https://github.com/AzureCosmosDB/OmniVec/security/code-scanning).Root cause
BinSkim emits SARIF results that reference rule
messageStringsby id (e.g.message.id = "Warning_NativeWithInsecureStaticLibraryCompilands"plusarguments), without an inlinedmessage.textfield.GitHub Code Scanning's SARIF ingester requires
message.textto be present directly on every result, so the upload was failing with:The 2 errors correspond to the 2 suppressed BinSkim results on
docgrok-router.exe(rulesBA2004andBA2024, both with documented upstream-toolchain justifications inscripts/security/binskim_suppressions.json).Fix
scripts/security/filter_binskim_sarif.pyalready walked the SARIF to suppress documented findings. Extended it to also synthesizemessage.textwhen missing:message.text, look uptool.driver.rules[ruleIndex].messageStrings[message.id].text.argumentsinto the{0},{1}, … placeholders.message.text.Falls back to a deterministic placeholder (
<ruleId>: <messageId>) if the template cannot be resolved, so the SARIF always uploads.Verification
Tested locally against the actual SARIF artifact downloaded from the most recent main-branch run:
No security findings change — the 2 results remain suppressed at
level: notewith their existing justifications. This PR only fixes the SARIF format so the upload succeeds and the BinSkim configuration page shows clean instead of "2 errors".