PowerShell / PowerShell/PSScriptAnalyzer
PSShouldProcess behavior allows for unsafe code to pass where it should fail
Nobody has claimed this yet.
- Dominant language
- C#
- Stars
- 2.2k
- Forks
- 414
- Avg merge
- 13h 1m
- Merged PRs (30d)
- 2
Description
Steps to reproduce
The following function will pass PSShouldProcess
function Test-Fail {
[CmdletBinding(SupportsShouldProcess)]
param ()
process {
Remove-Item -Path C:\temp\a_testy.txt -Force -Confirm:$false -ErrorAction SilentlyContinue
}
}
@seeminglyscience pointesd me to this
https://github.com/PowerShell/PSScriptAnalyzer/blob/a7d23f7f4dbf4ee92077f5471e28ca334caef429/Rules/UseShouldProcessCorrectly.cs#L245-L267
Where it is checking upstream functions for supports should process. While on the surface it may seem like a good idea to allow the upstream functions to handle the confirmation, this allows for some unsafe code to pass where it should fail.
IMO, the best course would be to always warn when $PSCmdlet.ShouldProcess() is not present in the function itself and require suppression when allowing the upstream functions to handle the confirmation.
Expected behavior
Either a warning or the requirement to suppress PSShouldProcess
Actual behavior
Test passes
Environment data
> $PSVersionTable
Name Value
---- -----
PSVersion 5.1.16299.251
PSEdition Desktop
PSCompatibleVersions {1.0, 2.0, 3.0, 4.0...}
BuildVersion 10.0.16299.251
CLRVersion 4.0.30319.42000
WSManStackVersion 3.0
PSRemotingProtocolVersion 2.3
SerializationVersion 1.1.0.1
> (Get-Module -ListAvailable PSScriptAnalyzer).Version | ForEach-Object { $_.ToString() }
1.16.0
1.15.0
1.13.0
1.12.0
1.8.1
1.16.1
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with Rules/UseShouldProcessCorrectly.cs, especially the upstream-function check around lines 245-267, and reproduce the issue with the PowerShell function shown. Confirm that the analyzer reports the missing $PSCmdlet.ShouldProcess() behavior or requires suppression, then add coverage for the expected warning or suppression requirement.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp, powershell
- Domain
- devtools, security
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100