Drop misleading "partialFingerprints required" check from ADO/GH rules#2893
Open
michaelcfanning wants to merge 1 commit into
Open
Drop misleading "partialFingerprints required" check from ADO/GH rules#2893michaelcfanning wants to merge 1 commit into
michaelcfanning wants to merge 1 commit into
Conversation
The shared base class for the SARIF/ADO/GH "ProvideRequiredResultProperties"
validators (Base1015) flagged any result that omitted partialFingerprints as
an error against the ADO Advanced Security and GitHub Advanced Security
service rule kinds. The message text said the property "is required by the
{service} service." That is wrong for both consumers:
* Advanced Security for Azure DevOps computes partialFingerprints
automatically when the AdvancedSecurity-Publish task ingests a SARIF log
that omits them (and Sprint 255 added an explicit opt-in,
advancedsecurity.publish.allowmissingpartialfingerprints, for findings
that aren't located on source).
* GitHub code scanning back-fills partialFingerprints from source when a
SARIF log uploaded via the upload-sarif action omits them. They are only
recommended for direct REST uploads via /code-scanning/sarifs to avoid
duplicate alerts.
Producers — particularly AI producers covered by AI2011 — have legitimate
reasons to omit partialFingerprints rather than fabricate identity values.
This change removes the partialFingerprints check from Base1015 entirely
so ADO1015/ADO1017 and GH1015/GH1017 no longer fire on omission. The
genuinely-required checks in the same base (message, message.text,
locations array, non-empty locations) are preserved.
Scope:
* Base1015.ProvideRequiredResultProperties.cs — remove the
result.PartialFingerprints == null check and its message-resource list
entry; add an explanatory comment with the GHAZDO and GH docs citations.
* RuleResources.resx / .Designer.cs — remove the
Base1015_..._MissingPartialFingerprints_Text resource; remove the
"Provide the 'partialFingerprints' dictionary..." sentence from the
ADO1015 and GH1015 FullDescription resource strings.
* BaselineOption functional-test expected outputs (TEST1001-TEST1008) —
regenerated via the test framework's standard rebaseline flow. The diff
reflects only the disappearance of Error_MissingPartialFingerprints
firings and the corresponding messageStrings / fullDescription text;
all other rule firings, baselining behavior, and result counts are
preserved.
* ReleaseHistory.md — UNRELEASED entry.
Verified: full Sarif.Multitool.Library unit suite (90/90) and full
Sarif functional suite (118/118) green after the change.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
michaelcfanning
commented
May 10, 2026
| # SARIF Package Release History (SDK, Driver, Converters, and Multitool) | ||
|
|
||
| ## **UNRELEASED** | ||
| * BUGFIX: Drop the missing-`partialFingerprints` check from `BaseProvideRequiredResultProperties` (Base1015), which removes the firing for ADO1015/ADO1017 and GH1015/GH1017. Both Advanced Security for Azure DevOps and GitHub code scanning compute `partialFingerprints` automatically when omitted, so the `error`-level "this property is required by the {service} service" message was misleading. See [GHAZDO third-party SARIF docs](https://learn.microsoft.com/azure/devops/repos/security/github-advanced-security-code-scanning-third-party) (Sprint 245 `ruleId` inclusion, Sprint 255 `advancedsecurity.publish.allowmissingpartialfingerprints`) and [GitHub code scanning SARIF support — Fingerprint generation](https://docs.github.com/code-security/code-scanning/integrating-with-code-scanning/sarif-support-for-code-scanning#fingerprint-generation). AI producers are already advised against persisting fingerprints by AI2011. |
Member
Author
There was a problem hiding this comment.
this should be BUG and also check ordering, BUG doesn't usually come before BRK
cfaucon
approved these changes
May 11, 2026
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.
Drop misleading "partialFingerprints required" check from ADO/GH rules
What
BaseProvideRequiredResultProperties(Base1015), the shared base for the fourProvideRequiredResultPropertiesvalidators (ADO1015,ADO1017,GH1015,GH1017), flagged any result missingpartialFingerprintsas an error against theAdoandGhasrule kinds, with the message:That
is requiredis wrong for both consumers.Why
Advanced Security for Azure DevOps
AdvancedSecurity-Publishtask computespartialFingerprintsautomatically when a SARIF log omits them. (Integrate non-Microsoft scanning tools — Result fingerprint generation)ruleIdto the computed fingerprint inputs.advancedsecurity.publish.allowmissingpartialfingerprints, for URI-located findings (containers, DAST) where source-derived fingerprints don't apply.GitHub code scanning
upload-sarifaction back-fillspartialFingerprintsfrom source when a SARIF log uploaded via that path omits them. (SARIF support for code scanning — Fingerprint generation)/code-scanning/sarifsREST uploads, where back-filling does not occur, to avoid duplicate alerts.AI producers
AI2011already advises AI producers not to persist fingerprints, which is the opposite of whatBase1015was demanding. Producers should not be in the position of choosing which Multitool rule to violate.The change
Base1015.ProvideRequiredResultProperties.cs— drop theresult.PartialFingerprints == nullcheck and its message-resource-list entry. Add a comment with the GHAZDO and GitHub doc citations.RuleResources.resx/.Designer.cs— drop theBase1015_..._MissingPartialFingerprints_Textresource. Strip the "Provide the 'partialFingerprints' dictionary..." sentence from theADO1015andGH1015FullDescriptionresource strings.Test.FunctionalTests.Sarif/.../BaselineOption/ExpectedOutputs/TEST1001..TEST1008.*.sarif— regenerated via the framework's standardRebaseline<suite>.cmdflow. The diff is exactly the disappearance ofError_MissingPartialFingerprintsfirings and the matchingmessageStrings/fullDescriptiontext. All other rule firings, baselining behavior, and result counts are preserved.ReleaseHistory.md— UNRELEASED entry.The genuinely-required checks in
Base1015(message,message.text,locationsarray, non-emptylocations) are kept. Only the stale partial-fingerprints check is removed.Diff scope
Verification
Sarif.Multitool.Libraryunit suite: 90/90 pass (1 pre-existing skip)Sariffunctional suite: 118/118 pass