Skip to content

Always return a RuleSuppressionID #1958

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
iRon7 opened this issue Jan 16, 2024 · 2 comments
Closed

Always return a RuleSuppressionID #1958

iRon7 opened this issue Jan 16, 2024 · 2 comments

Comments

@iRon7
Copy link

iRon7 commented Jan 16, 2024

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
Copy link
Collaborator

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
Copy link
Author

iRon7 commented Feb 4, 2024

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)

@iRon7 iRon7 closed this as completed Feb 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants