fix: suppress S4790 on the MD5 and SHA-1 hash providers - #16
Merged
Conversation
SonarCloud rates new code on main D for security, on the strength of eight S4790 "use a stronger hashing algorithm" findings across these two providers. They are false positives by construction: a package named ktsu.Essentials.HashProviders.MD5 has to call MD5, and the algorithm is fixed by the type's contract rather than chosen per call. Suppress at class level, alongside the CA5351 and CA5350 suppressions already there for the same reason, and with justifications in the same shape. Class level is the smallest scope that covers it, since all four sites per file are the algorithm the type exists to expose. These findings predate the async stream work; they were raised 2026-08-13 and only started failing the gate once main's new code window moved.
|
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
1 participant
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.
Takes main's SonarCloud security rating on new code from D back to A. That rating is currently the only thing failing the quality gate on
mainother than duplication.Why the gate is red
Eight
S4790findings, "use a stronger hashing algorithm", four in each of the MD5 and SHA-1 providers:They're false positives by construction. A package named
ktsu.Essentials.HashProviders.MD5has to call MD5, and the algorithm is fixed by the type's contract rather than picked per call. There is no fix that keeps the package doing what it says on the tin.Worth noting they have nothing to do with the recent async stream work: SonarCloud dates all eight to 2026-08-13, and they only began failing the gate once main's new code window moved past the 2.2.0 release.
The change
One class-level suppression per file, sitting directly beneath the
CA5351andCA5350suppressions that are already there for exactly this reason, and written in the same shape:Class level is the smallest scope that actually covers this, because all four sites per file are the algorithm the type exists to expose, split across two overloads and their
#if NET6_0_OR_GREATERbranches. Suppressing per line would mean eight attributes saying the same sentence.Preferred over triaging the findings in the SonarCloud UI because the reasoning stays in version control next to the code it excuses, and it survives any future project re-import.
Verification
Both projects build clean, 0 warnings and 0 errors under warnings-as-errors.
Local verification stops there, and I want to be straight about it: SonarAnalyzer only runs through the CI scanner, so a plain
dotnet buildnever exercises S4790 and can't confirm the suppression binds. Roslyn matchesSuppressMessageAttributeon the check ID, so this should hold, but the real proof is this PR's own SonarCloud run. If the rating doesn't move, the suppression is wrong and I'll adjust it rather than leave it sitting there looking like it worked.