PowerShell / PSScriptAnalyzer

Download ScriptAnalyzer from PowerShellGallery
https://www.powershellgallery.com/packages/PSScriptAnalyzer/
MIT License
1.8k stars 366 forks source link

Always return a `RuleSuppressionID` #1958

Closed iRon7 closed 5 months ago

iRon7 commented 5 months ago

Rules that return a long extent property (as e.g. a [ScriptBlock] expression) often have no RuleSuppressionID Which means that one isn't able to suppress that specific warning (by RuleSuppressionID) and still validate similar occurences with a differed RuleSuppressionID.

Purposed solution

When a (custom) rule doesn't return a RuleSuppressionID the engine could technically always add a unique RuleSuppressionID by fabricating (and validating) one using the hashcode of the returned Extent.Text. Basically:

(Invoke-ScriptAnalyzer .\MyScript.ps1).foreach{
  if (-not $_.RuleSuppressionID) { $_.RuleSuppressionID = $_.Extent.Text.GetHashCode() }; $_
} | Select-Object *

Line                 : 64
Column               : 71
Message              : Possible script injection risk via the a dangerous method. Untrusted input can cause arbitrary PowerShell expressions to be run. The PowerShell.AddCommand().AddParameter() APIs should be used instead.
Extent               : [ScriptBlock]::Create($Expression)
RuleName             : InjectionRisk.Create
Severity             : Warning
ScriptName           : Update-Accessor.ps1
ScriptPath           : C:\Users\Gebruiker\My Drive\Scripts\PowerShell\Update-Accessor\Update-Accessor.ps1
RuleSuppressionID    : 227051976
SuggestedCorrections :
IsSuppressed         : False
bergmeister commented 5 months ago

I disagree, the custom rule needs to return the id. PSSA should not auto-generate them because if that ever has to change, one cannot change it because people already rely on that.

iRon7 commented 5 months ago

Good point. Aside from this, I don't thick that using the GetHashCode() method for this is a good idea as it might return different values for the same string content #46406. For something like this it is probably better to create a MD5 hash and also consider the case (.toUpper()) and diacritics. I have closed this issue and create a new request for a rule to check for a dynamic RuleSuppressionID (#1964)