diff --git a/.github/workflows/CI.yaml b/.github/workflows/CI.yaml index d4d20d8..22568a3 100644 --- a/.github/workflows/CI.yaml +++ b/.github/workflows/CI.yaml @@ -3,6 +3,8 @@ on: pull_request: workflow_dispatch: permissions: + contents: read + issues: write checks: write pull-requests: write jobs: diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bca480..40dc24a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,31 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +## [0.3.0] 2025-12-13 + +### Added + +- `Measure-BasicWebRequestProperty`: Detects when `Invoke-WebRequest` uses + `UseBasicParsing` with incompatible properties like `Forms`, `ParsedHtml`, + `Scripts`, or `AllElements`. Works with both direct property access and + variable assignments. +- `Measure-InvokeWebRequestWithoutBasic`: Flags `Invoke-WebRequest` (and its + aliases `iwr`, `curl`) when used without the `UseBasicParsing` parameter. +- `Get-CommandParameter`: New private helper function to parse command + parameters from AST, including support for positional parameters. +- Documentation for new rules in `docs/en-US/` directory. +- Comprehensive test coverage for new rules. + +### Changed + +- Updated `about_GoodEnoughRules.help.md` with complete module documentation + including examples, rule descriptions, and troubleshooting guidance. +- `Measure-SecureStringWithKey`: Standardized parameter block formatting and + updated to use `Get-CommandParameter` helper function. +- Test files: Added BeforeAll checks to ensure module builds before testing. +- Improved code consistency across all rule files (param block formatting, + using consistent helper function names). + ## [0.2.0] Measure-SecureStringWithKey ### Added diff --git a/GoodEnoughRules/GoodEnoughRules.psd1 b/GoodEnoughRules/GoodEnoughRules.psd1 index 53ff666..5a9c7b2 100644 --- a/GoodEnoughRules/GoodEnoughRules.psd1 +++ b/GoodEnoughRules/GoodEnoughRules.psd1 @@ -12,7 +12,7 @@ RootModule = 'GoodEnoughRules.psm1' # Version number of this module. - ModuleVersion = '0.2.0' + ModuleVersion = '0.3.0' # Supported PSEditions # CompatiblePSEditions = @() @@ -52,10 +52,7 @@ # Modules that must be imported into the global environment prior to importing this module RequiredModules = @( - @{ - ModuleName = 'PSScriptAnalyzer' - ModuleVersion = '1.23' - } + # WARNING: Do not require PSScriptAnalyzer here to avoid circular dependency ) # Assemblies that must be loaded prior to importing this module diff --git a/GoodEnoughRules/Private/Get-CommandParameters.ps1 b/GoodEnoughRules/Private/Get-CommandParameters.ps1 index badfb79..33084d1 100644 --- a/GoodEnoughRules/Private/Get-CommandParameters.ps1 +++ b/GoodEnoughRules/Private/Get-CommandParameters.ps1 @@ -1,20 +1,49 @@ -function Get-CommandParameters { +function Get-CommandParameter { + [CmdletBinding()] + [OutputType([hashtable])] param ( [System.Management.Automation.Language.CommandAst] $Command ) - $commandElements = $CommandAst.CommandElements + $commandElements = $Command.CommandElements + Write-Verbose "Processing command: $($Command.GetCommandName())" + Write-Verbose "Total command elements: $($commandElements.Count - 1)" #region Gather Parameters # Create a hash to hold the parameter name as the key, and the value $parameterHash = @{} - for($i=1; $i -lt $commandElements.Count; $i++){ + # Track positional parameter index + $positionalIndex = 0 + # Start at index 1 to skip the command name + for ($i = 1; $i -lt $commandElements.Count; $i++) { + Write-Debug $commandElements[$i] # Switch on type - switch ($commandElements[$i].GetType().Name){ + switch ($commandElements[$i].GetType().Name) { + 'ParameterAst' { + $parameterName = $commandElements[$i].ParameterName + # Next element is the value + continue + } 'CommandParameterAst' { $parameterName = $commandElements[$i].ParameterName + # Initialize to $true for switch parameters + $parameterHash[$parameterName] = $true + continue + } + 'StringConstantExpressionAst' { + $value = $commandElements[$i].Value + # Check if a parameter name was set + if (-not $parameterName) { + Write-Verbose "Positional parameter or argument detected: $value" + $parameterHash["PositionalParameter$positionalIndex"] = $value + $positionalIndex++ + continue + } + $parameterHash[$parameterName] = $value + continue } default { + Write-Verbose "Unhandled command element type: $($commandElements[$i].GetType().Name)" # Grab the string from the extent $value = $commandElements[$i].SafeGetValue() $parameterHash[$parameterName] = $value diff --git a/GoodEnoughRules/Public/Measure-BasicWebRequestProperty.ps1 b/GoodEnoughRules/Public/Measure-BasicWebRequestProperty.ps1 new file mode 100644 index 0000000..920aa21 --- /dev/null +++ b/GoodEnoughRules/Public/Measure-BasicWebRequestProperty.ps1 @@ -0,0 +1,123 @@ +function Measure-BasicWebRequestProperty { + <# + .SYNOPSIS + Rule to detect if Invoke-WebRequest is used with UseBasicParsing and + incompatible properties. + + .DESCRIPTION + This rule detects if Invoke-WebRequest (or its aliases) is used with the + UseBasicParsing parameter and then attempts to access properties that are + incompatible with UseBasicParsing. This includes properties like 'Forms', + 'ParsedHtml', 'Scripts', and 'AllElements'. This checks for both direct + member access after the command as well as variable assignments. + + .PARAMETER ScriptBlockAst + The scriptblock AST to check. + + .INPUTS + [System.Management.Automation.Language.ScriptBlockAst] + + .OUTPUTS + [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord[]] + + .EXAMPLE + Measure-BasicWebRequestProperty -ScriptBlockAst $ScriptBlockAst + + This will check if the given ScriptBlockAst contains any Invoke-WebRequest + commands with UseBasicParsing that access incompatible properties. + #> + [CmdletBinding()] + [OutputType([Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord])] + param + ( + [Parameter(Mandatory = $true)] + [ValidateNotNullOrEmpty()] + [System.Management.Automation.Language.ScriptBlockAst] + $ScriptBlockAst + ) + begin { + # We need to find any assignments or uses of Invoke-WebRequest (or its aliases) + # to check if they attempt to use incompatible properties with UseBasicParsing. + # Examples to find: + # $bar = (iwr -Uri 'https://example.com' -UseBasicParsing).Forms + $iwrMemberRead = { + param($Ast) + $Ast -is [System.Management.Automation.Language.CommandAst] -and + # IWR Command + $Ast.GetCommandName() -match '(Invoke-WebRequest|iwr|curl)' -and + # With UseBasicParsing + $Ast.CommandElements.ParameterName -contains 'UseBasicParsing' -and + # That is being read as a member expression + $Ast.Parent.Parent -is [System.Management.Automation.Language.ParenExpressionAst] -and + $Ast.Parent.Parent.Parent -is [System.Management.Automation.Language.MemberExpressionAst] -and + # The member being accessed is a string constant + $Ast.Parent.Parent.Parent.Member -is [System.Management.Automation.Language.StringConstantExpressionAst] -and + # The member is one of the incompatible properties + $incompatibleProperties -contains $Ast.Parent.Parent.Parent.Member + } + # Predicate to find assignments involving Invoke-WebRequest with UseBasicParsing + # $foo = Invoke-WebRequest -Uri 'https://example.com' -UseBasicParsing + $varAssignPredicate = { + param($Ast) + $Ast -is [System.Management.Automation.Language.AssignmentStatementAst] -and + $Ast.Right -is [System.Management.Automation.Language.PipelineAst] -and + $Ast.Right.PipelineElements.Count -eq 1 -and + $Ast.Right.PipelineElements[0] -is [System.Management.Automation.Language.CommandAst] -and + $Ast.Right.PipelineElements[0].GetCommandName() -match '(Invoke-WebRequest|iwr|curl)' -and + $Ast.Right.PipelineElements[0].CommandElements.ParameterName -contains 'UseBasicParsing' + } + $incompatibleProperties = @( + 'AllElements', + 'Forms', + 'ParsedHtml', + 'Scripts' + ) + + } + + process { + # Find all member expression ASTs that match our criteria + [System.Management.Automation.Language.Ast[]]$memberReads = $ScriptBlockAst.FindAll($iwrMemberRead, $true) + foreach ($memberRead in $memberReads) { + # ParenExpression would be the whole line: (iwr -Uri ... -UseBasicParsing).Foo + $parenExpression = $memberRead.Parent.Parent + $propertyName = $memberRead.Parent.Parent.Parent.Member.Value + [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord]@{ + Message = "Invoke-WebRequest cannot use the '$propertyName' parameter when 'UseBasicParsing' is specified." + Extent = $parenExpression.Extent + Severity = 'Error' + } + } + # Find all assignment ASTs that match our criteria + [System.Management.Automation.Language.Ast[]]$assignments = $ScriptBlockAst.FindAll($varAssignPredicate, $true) + # Now use that to search for var reads of the assigned variable + foreach ($assignment in $assignments) { + $variableName = $assignment.Left.VariablePath.UserPath + $lineAfter = $assignment.Extent.EndLineNumber + Write-Verbose "Checking variable '$variableName' for incompatible property usage after line $lineAfter" + # Find all reads of that variable + #region Dynamically Build the AST Search Predicate + $varReadPredicateScript = @() + $varReadPredicateScript += 'param($Ast)' + $varReadPredicateScript += '$Ast -is [System.Management.Automation.Language.VariableExpressionAst] -and' + $varReadPredicateScript += '$Ast.VariablePath.UserPath -eq "' + $variableName + '" -and' + $varReadPredicateScript += '$Ast.Extent.StartLineNumber -gt ' + $lineAfter + $varReadPredicate = [scriptblock]::Create($($varReadPredicateScript -join "`n")) + [System.Management.Automation.Language.Ast[]]$varReads = $ScriptBlockAst.FindAll($varReadPredicate, $true) + foreach ($varRead in $varReads) { + Write-Verbose "Found read of variable '$variableName' at line $($varRead.Extent.StartLineNumber)" + if ($varRead.Parent -is [System.Management.Automation.Language.MemberExpressionAst]) { + $propertyName = $varRead.Parent.Member.Value + if ($incompatibleProperties -contains $propertyName) { + [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord]@{ + Message = "Invoke-WebRequest cannot use the '$propertyName' parameter when 'UseBasicParsing' is specified." + RuleName = $PSCmdlet.MyInvocation.InvocationName + Extent = $varRead.Parent.Extent + Severity = 'Error' + } + } + } + } + } + } +} diff --git a/GoodEnoughRules/Public/Measure-InvokeWebRequestWithoutBasic.ps1 b/GoodEnoughRules/Public/Measure-InvokeWebRequestWithoutBasic.ps1 new file mode 100644 index 0000000..412ed75 --- /dev/null +++ b/GoodEnoughRules/Public/Measure-InvokeWebRequestWithoutBasic.ps1 @@ -0,0 +1,58 @@ +function Measure-InvokeWebRequestWithoutBasic { + <# + .SYNOPSIS + Rule to detect if Invoke-WebRequest is used without UseBasicParsing. + + .DESCRIPTION + This rule detects if Invoke-WebRequest (or its aliases) is used without the + UseBasicParsing parameter. + + .PARAMETER ScriptBlockAst + The scriptblock AST to check. + + .INPUTS + [System.Management.Automation.Language.ScriptBlockAst] + + .OUTPUTS + [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord[]] + + .EXAMPLE + Measure-InvokeWebRequestWithoutBasic -ScriptBlockAst $ScriptBlockAst + + This will check if the given ScriptBlockAst contains any Invoke-WebRequest + commands without the UseBasicParsing parameter. + #> + [CmdletBinding()] + [OutputType([Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord])] + param + ( + [Parameter(Mandatory = $true)] + [ValidateNotNullOrEmpty()] + [System.Management.Automation.Language.ScriptBlockAst] + $ScriptBlockAst + ) + begin { + $predicate = { + param($Ast) + $Ast -is [System.Management.Automation.Language.CommandAst] -and + $Ast.GetCommandName() -imatch '(Invoke-WebRequest|iwr|curl)$' + } + } + + process { + [System.Management.Automation.Language.Ast[]]$commands = $ScriptBlockAst.FindAll($predicate, $true) + $commands | ForEach-Object { + Write-Verbose "Analyzing command: $($_.GetCommandName())" + $command = $_ + $parameterHash = Get-CommandParameter -Command $command + if (-not $parameterHash.ContainsKey('UseBasicParsing')) { + [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord]@{ + Message = 'Invoke-WebRequest should be used with the UseBasicParsing parameter.' + Extent = $command.Extent + RuleName = $PSCmdlet.MyInvocation.InvocationName + Severity = 'Error' + } + } + } + } +} diff --git a/GoodEnoughRules/Public/Measure-SecureStringWithKey.ps1 b/GoodEnoughRules/Public/Measure-SecureStringWithKey.ps1 index 3a6bf8e..f405681 100644 --- a/GoodEnoughRules/Public/Measure-SecureStringWithKey.ps1 +++ b/GoodEnoughRules/Public/Measure-SecureStringWithKey.ps1 @@ -21,7 +21,7 @@ function Measure-SecureStringWithKey { #> [CmdletBinding()] [OutputType([Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord])] - Param + param ( [Parameter(Mandatory = $true)] [ValidateNotNullOrEmpty()] @@ -29,7 +29,7 @@ function Measure-SecureStringWithKey { $ScriptBlockAst ) - Begin { + begin { $predicate = { param($Ast) $Ast -is [System.Management.Automation.Language.CommandAst] -and @@ -37,16 +37,17 @@ function Measure-SecureStringWithKey { } } - Process { + process { [System.Management.Automation.Language.Ast[]]$commands = $ScriptBlockAst.FindAll($predicate, $true) $commands | ForEach-Object { $command = $_ - $parameterHash = Get-CommandParameters -Command $command + $parameterHash = Get-CommandParameter -Command $command if (-not $parameterHash.ContainsKey('Key')) { [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord]@{ Message = 'ConvertFrom-SecureString should be used with a Key.' Extent = $command.Extent - Severity = [Microsoft.Windows.PowerShell.ScriptAnalyzer.Severity]::Error + RuleName = $PSCmdlet.MyInvocation.InvocationName + Severity = 'Error' } } } diff --git a/README.md b/README.md index f61c441..d347784 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,20 @@ Install-PSResource GoodEnoughRules The docs are automatically generated from the rule comment based help. You can see the docs at [HeyItsGilbert.GitHub.io/GoodEnoughRules](https://heyitsgilbert.github.io/GoodEnoughRules) +## Examples + +### Running a Single Rule + +```powershell +# Install and import +Install-PSResource GoodEnoughRules +$module = Import-Module GoodEnoughRules -PassThru +# Get the path the psm1 +$modulePath = Join-Path $module.ModuleBase $module.RootModule +# Run against a folder +Invoke-ScriptAnalyzer -CustomRulePath $modulePath -IncludeRule 'Measure-InvokeWebRequestWithoutBasic' -Path '.\scripts\' +``` + ## Walk Through > [!TIP] diff --git a/docs/en-US/Measure-BasicWebRequestProperty.md b/docs/en-US/Measure-BasicWebRequestProperty.md new file mode 100644 index 0000000..de6ca67 --- /dev/null +++ b/docs/en-US/Measure-BasicWebRequestProperty.md @@ -0,0 +1,83 @@ +--- +external help file: GoodEnoughRules-help.xml +Module Name: GoodEnoughRules +online version: +schema: 2.0.0 +--- + +# Measure-BasicWebRequestProperty + +## SYNOPSIS +Rule to detect if Invoke-WebRequest is used with UseBasicParsing and +incompatible properties. + +## SYNTAX + +``` +Measure-BasicWebRequestProperty [-ScriptBlockAst] [-ProgressAction ] + [] +``` + +## DESCRIPTION +This rule detects if Invoke-WebRequest (or its aliases) is used with the +UseBasicParsing parameter and then attempts to access properties that are +incompatible with UseBasicParsing. +This includes properties like 'Forms', +'ParsedHtml', 'Scripts', and 'AllElements'. +This checks for both direct +member access after the command as well as variable assignments. + +## EXAMPLES + +### EXAMPLE 1 +``` +Measure-BasicWebRequestProperty -ScriptBlockAst $ScriptBlockAst +``` + +This will check if the given ScriptBlockAst contains any Invoke-WebRequest +commands with UseBasicParsing that access incompatible properties. + +## PARAMETERS + +### -ScriptBlockAst +The scriptblock AST to check. + +```yaml +Type: ScriptBlockAst +Parameter Sets: (All) +Aliases: + +Required: True +Position: 1 +Default value: None +Accept pipeline input: False +Accept wildcard characters: False +``` + +### -ProgressAction +{{ Fill ProgressAction Description }} + +```yaml +Type: ActionPreference +Parameter Sets: (All) +Aliases: proga + +Required: False +Position: Named +Default value: None +Accept pipeline input: False +Accept wildcard characters: False +``` + +### CommonParameters +This cmdlet supports the common parameters: -Debug, -ErrorAction, -ErrorVariable, -InformationAction, -InformationVariable, -OutVariable, -OutBuffer, -PipelineVariable, -Verbose, -WarningAction, and -WarningVariable. For more information, see [about_CommonParameters](http://go.microsoft.com/fwlink/?LinkID=113216). + +## INPUTS + +### [System.Management.Automation.Language.ScriptBlockAst] +## OUTPUTS + +### [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord[]] +## NOTES + +## RELATED LINKS diff --git a/docs/en-US/Measure-InvokeWebRequestWithoutBasic.md b/docs/en-US/Measure-InvokeWebRequestWithoutBasic.md new file mode 100644 index 0000000..e0e7c87 --- /dev/null +++ b/docs/en-US/Measure-InvokeWebRequestWithoutBasic.md @@ -0,0 +1,77 @@ +--- +external help file: GoodEnoughRules-help.xml +Module Name: GoodEnoughRules +online version: +schema: 2.0.0 +--- + +# Measure-InvokeWebRequestWithoutBasic + +## SYNOPSIS +Rule to detect if Invoke-WebRequest is used without UseBasicParsing. + +## SYNTAX + +``` +Measure-InvokeWebRequestWithoutBasic [-ScriptBlockAst] [-ProgressAction ] + [] +``` + +## DESCRIPTION +This rule detects if Invoke-WebRequest (or its aliases) is used without the +UseBasicParsing parameter. + +## EXAMPLES + +### EXAMPLE 1 +``` +Measure-InvokeWebRequestWithoutBasic -ScriptBlockAst $ScriptBlockAst +``` + +This will check if the given ScriptBlockAst contains any Invoke-WebRequest +commands without the UseBasicParsing parameter. + +## PARAMETERS + +### -ScriptBlockAst +The scriptblock AST to check. + +```yaml +Type: ScriptBlockAst +Parameter Sets: (All) +Aliases: + +Required: True +Position: 1 +Default value: None +Accept pipeline input: False +Accept wildcard characters: False +``` + +### -ProgressAction +{{ Fill ProgressAction Description }} + +```yaml +Type: ActionPreference +Parameter Sets: (All) +Aliases: proga + +Required: False +Position: Named +Default value: None +Accept pipeline input: False +Accept wildcard characters: False +``` + +### CommonParameters +This cmdlet supports the common parameters: -Debug, -ErrorAction, -ErrorVariable, -InformationAction, -InformationVariable, -OutVariable, -OutBuffer, -PipelineVariable, -Verbose, -WarningAction, and -WarningVariable. For more information, see [about_CommonParameters](http://go.microsoft.com/fwlink/?LinkID=113216). + +## INPUTS + +### [System.Management.Automation.Language.ScriptBlockAst] +## OUTPUTS + +### [Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord[]] +## NOTES + +## RELATED LINKS diff --git a/docs/en-US/Measure-SecureStringWithKey.md b/docs/en-US/Measure-SecureStringWithKey.md index 8489f2b..e739b57 100644 --- a/docs/en-US/Measure-SecureStringWithKey.md +++ b/docs/en-US/Measure-SecureStringWithKey.md @@ -18,7 +18,8 @@ Measure-SecureStringWithKey [-ScriptBlockAst] [-ProgressAction ``` ## DESCRIPTION -This rule detects if ConvertFrom-SecureString is used without a Key. +This rule detects if ConvertFrom-SecureString is used without a Key which +means the secret is user and machine bound. ## EXAMPLES diff --git a/docs/en-US/about_GoodEnoughRules.help.md b/docs/en-US/about_GoodEnoughRules.help.md index b74a038..b8f262b 100644 --- a/docs/en-US/about_GoodEnoughRules.help.md +++ b/docs/en-US/about_GoodEnoughRules.help.md @@ -2,58 +2,142 @@ ## about_GoodEnoughRules +# SHORT DESCRIPTION +A set of PSScriptAnalyzer custom rules that help make your PowerShell code +"Good Enough" by detecting common issues and anti-patterns. + +# LONG DESCRIPTION +GoodEnoughRules is a PowerShell module that provides custom rules for +PSScriptAnalyzer. These rules help identify code quality issues, security +concerns, and best practice violations in your PowerShell scripts. + +The module includes rules for: + +- Detecting insecure web request practices (Basic authentication) +- Identifying TODO comments in code +- Finding insecure SecureString usage with hardcoded keys +- Validating web request properties and parameters + +These rules are designed to catch issues that may not be covered by the +standard PSScriptAnalyzer rules, helping you maintain better code quality +and security in your PowerShell projects. + +## Available Rules + +### Measure-BasicWebRequestProperty +Detects the use of Basic authentication in web request cmdlets, which can +expose credentials in plain text. + +### Measure-InvokeWebRequestWithoutBasic +Identifies Invoke-WebRequest and Invoke-RestMethod calls that may be missing +proper authentication methods. + +### Measure-SecureStringWithKey +Finds SecureString operations using hardcoded keys, which undermines the +security of SecureString. + +### Measure-TODOComment +Locates TODO comments in your code to help track pending work items. + +# EXAMPLES + +## Example 1: Run a Single Rule + +```powershell +# Install and import the module +Install-PSResource GoodEnoughRules +$module = Import-Module GoodEnoughRules -PassThru + +# Get the path to the module +$modulePath = Join-Path $module.ModuleBase $module.RootModule + +# Run a specific rule against a folder +Invoke-ScriptAnalyzer -CustomRulePath $modulePath ` + -IncludeRule 'Measure-InvokeWebRequestWithoutBasic' ` + -Path '.\scripts\' ``` -ABOUT TOPIC NOTE: -The first header of the about topic should be the topic name. -The second header contains the lookup name used by the help system. - -IE: -# Some Help Topic Name -## SomeHelpTopicFileName - -This will be transformed into the text file -as `about_SomeHelpTopicFileName`. -Do not include file extensions. -The second header should have no spaces. + +## Example 2: Run All Rules + +```powershell +# Import the module +Import-Module GoodEnoughRules +$module = Get-Module GoodEnoughRules +$modulePath = Join-Path $module.ModuleBase $module.RootModule + +# Run all custom rules +Invoke-ScriptAnalyzer -CustomRulePath $modulePath -Path '.\MyScript.ps1' ``` -# SHORT DESCRIPTION -{{ Short Description Placeholder }} +## Example 3: Include in Settings File +Create a PSScriptAnalyzerSettings.psd1 file: + +```powershell +@{ + CustomRulePath = @( + 'C:\Path\To\GoodEnoughRules\GoodEnoughRules.psm1' + ) + IncludeRules = @( + 'Measure-BasicWebRequestProperty', + 'Measure-TODOComment' + ) +} ``` -ABOUT TOPIC NOTE: -About topics can be no longer than 80 characters wide when rendered to text. -Any topics greater than 80 characters will be automatically wrapped. -The generated about topic will be encoded UTF-8. + +Then run: + +```powershell +Invoke-ScriptAnalyzer -Path .\MyScript.ps1 -Settings .\PSScriptAnalyzerSettings.psd1 ``` -# LONG DESCRIPTION -{{ Long Description Placeholder }} +# NOTE -## Optional Subtopics -{{ Optional Subtopic Placeholder }} +These rules are designed to catch specific patterns that may not be appropriate +for all projects. You should enable rules individually based on your project's +requirements rather than enabling all rules at once. -# EXAMPLES -{{ Code or descriptive examples of how to leverage the functions described. }} +The rules in this module represent "Good Enough" practices - they help catch +common issues but may not be exhaustive or suitable for all scenarios. -# NOTE -{{ Note Placeholder - Additional information that a user needs to know.}} +For best results, use these rules in conjunction with the standard +PSScriptAnalyzer rules. # TROUBLESHOOTING NOTE -{{ Troubleshooting Placeholder - Warns users of bugs}} -{{ Explains behavior that is likely to change with fixes }} +## CustomRulePath Not Found + +If you receive errors about the CustomRulePath not being found, ensure you're +using the full path to the .psm1 file: + +```powershell +$module = Get-Module GoodEnoughRules +$modulePath = Join-Path $module.ModuleBase $module.RootModule +``` + +## Rules Not Running + +Make sure to use the -IncludeRule parameter to explicitly enable the custom +rules you want to run, as they may not be enabled by default. # SEE ALSO -{{ See also placeholder }} -{{ You can also list related articles, blogs, and video URLs. }} +- [Invoke-ScriptAnalyzer](https://docs.microsoft.com/en-us/powershell/module/psscriptanalyzer/invoke-scriptanalyzer) +- [PSScriptAnalyzer](https://github.com/PowerShell/PSScriptAnalyzer) +- [GoodEnoughRules Documentation](https://heyitsgilbert.github.io/GoodEnoughRules) +- [GoodEnoughRules GitHub Repository](https://github.com/HeyItsGilbert/GoodEnoughRules) +- Measure-BasicWebRequestProperty +- Measure-InvokeWebRequestWithoutBasic +- Measure-SecureStringWithKey +- Measure-TODOComment # KEYWORDS -{{List alternate names or titles for this topic that readers might use.}} -- {{ Keyword Placeholder }} -- {{ Keyword Placeholder }} -- {{ Keyword Placeholder }} -- {{ Keyword Placeholder }} +- PSScriptAnalyzer +- CustomRules +- CodeQuality +- StaticAnalysis +- ScriptAnalyzer +- SecurityRules +- BestPractices diff --git a/requirements.psd1 b/requirements.psd1 index 2aedf93..b4058b3 100644 --- a/requirements.psd1 +++ b/requirements.psd1 @@ -7,7 +7,7 @@ Target = 'CurrentUser' } 'Pester' = @{ - Version = '5.4.0' + Version = '5.7.1' Parameters = @{ SkipPublisherCheck = $true } @@ -22,7 +22,7 @@ Version = '0.6.1' } 'PSScriptAnalyzer' = @{ - Version = '1.23.0' + Version = '1.24.0' } 'cspell' = @{ DependencyType = 'Npm' diff --git a/tests/Help.tests.ps1 b/tests/Help.tests.ps1 index 5b20ee6..3783f83 100644 --- a/tests/Help.tests.ps1 +++ b/tests/Help.tests.ps1 @@ -10,10 +10,10 @@ BeforeDiscovery { $params | Where-Object { $_.Name -notin $commonParams } | Sort-Object -Property Name -Unique } - $manifest = Import-PowerShellDataFile -Path $env:BHPSModuleManifest - $outputDir = Join-Path -Path $env:BHProjectPath -ChildPath 'Output' - $outputModDir = Join-Path -Path $outputDir -ChildPath $env:BHProjectName - $outputModVerDir = Join-Path -Path $outputModDir -ChildPath $manifest.ModuleVersion + $manifest = Import-PowerShellDataFile -Path $env:BHPSModuleManifest + $outputDir = Join-Path -Path $env:BHProjectPath -ChildPath 'Output' + $outputModDir = Join-Path -Path $outputDir -ChildPath $env:BHProjectName + $outputModVerDir = Join-Path -Path $outputModDir -ChildPath $manifest.ModuleVersion $outputModVerManifest = Join-Path -Path $outputModVerDir -ChildPath "$($env:BHProjectName).psd1" # Get module commands @@ -21,7 +21,7 @@ BeforeDiscovery { Get-Module $env:BHProjectName | Remove-Module -Force -ErrorAction Ignore Import-Module -Name $outputModVerManifest -Verbose:$false -ErrorAction Stop $params = @{ - Module = (Get-Module $env:BHProjectName) + Module = (Get-Module $env:BHProjectName) CommandType = [System.Management.Automation.CommandTypes[]]'Cmdlet, Function' # Not alias } if ($PSVersionTable.PSVersion.Major -lt 6) { @@ -37,22 +37,22 @@ Describe "Test help for <_.Name>" -ForEach $commands { BeforeDiscovery { # Get command help, parameters, and links - $command = $_ - $commandHelp = Get-Help $command.Name -ErrorAction SilentlyContinue - $commandParameters = global:FilterOutCommonParams -Params $command.ParameterSets.Parameters + $command = $_ + $commandHelp = Get-Help $command.Name -ErrorAction SilentlyContinue + $commandParameters = global:FilterOutCommonParams -Params $command.ParameterSets.Parameters $commandParameterNames = $commandParameters.Name - $helpLinks = $commandHelp.relatedLinks.navigationLink.uri + $helpLinks = $commandHelp.relatedLinks.navigationLink.uri } BeforeAll { # These vars are needed in both discovery and test phases so we need to duplicate them here - $command = $_ - $commandName = $_.Name - $commandHelp = Get-Help $command.Name -ErrorAction SilentlyContinue - $commandParameters = global:FilterOutCommonParams -Params $command.ParameterSets.Parameters - $commandParameterNames = $commandParameters.Name - $helpParameters = global:FilterOutCommonParams -Params $commandHelp.Parameters.Parameter - $helpParameterNames = $helpParameters.Name + $command = $_ + $commandName = $_.Name + $commandHelp = Get-Help $command.Name -ErrorAction SilentlyContinue + $commandParameters = global:FilterOutCommonParams -Params $command.ParameterSets.Parameters + $commandParameterNames = $commandParameters.Name + $helpParameters = global:FilterOutCommonParams -Params $commandHelp.Parameters.Parameter + $helpParameterNames = $helpParameters.Name } # If help is not found, synopsis in auto-generated help is the syntax diagram @@ -79,14 +79,14 @@ Describe "Test help for <_.Name>" -ForEach $commands { (Invoke-WebRequest -Uri $_ -UseBasicParsing).StatusCode | Should -Be '200' } - Context "Parameter <_.Name>" -Foreach $commandParameters { + Context "Parameter <_.Name>" -ForEach $commandParameters { BeforeAll { - $parameter = $_ - $parameterName = $parameter.Name - $parameterHelp = $commandHelp.parameters.parameter | Where-Object Name -eq $parameterName + $parameter = $_ + $parameterName = $parameter.Name + $parameterHelp = $commandHelp.parameters.parameter | Where-Object Name -EQ $parameterName $parameterHelpType = if ($parameterHelp.ParameterValue) { - $parameterHelp.ParameterValue.Trim() + $parameterHelp.ParameterValue.Trim() } } @@ -107,7 +107,7 @@ Describe "Test help for <_.Name>" -ForEach $commands { } } - Context "Test <_> help parameter help for " -Foreach $helpParameterNames { + Context "Test <_> help parameter help for " -ForEach $helpParameterNames { # Shouldn't find extra parameters in help. It "finds help parameter in code: <_>" { diff --git a/tests/Measure-BasicWebRequestProperty.tests.ps1 b/tests/Measure-BasicWebRequestProperty.tests.ps1 new file mode 100644 index 0000000..3cb6efc --- /dev/null +++ b/tests/Measure-BasicWebRequestProperty.tests.ps1 @@ -0,0 +1,67 @@ +Describe 'Measure-TODOComment' { + BeforeAll { + if ( -not $env:BHPSModuleManifest ) { + .\build.ps1 -Task Build -Verbose + } + $manifest = Import-PowerShellDataFile -Path $env:BHPSModuleManifest + $outputDir = Join-Path -Path $env:BHProjectPath -ChildPath 'Output' + $outputModDir = Join-Path -Path $outputDir -ChildPath $env:BHProjectName + $outputModVerDir = Join-Path -Path $outputModDir -ChildPath $manifest.ModuleVersion + $outputModVerManifest = Join-Path -Path $outputModVerDir -ChildPath "$($env:BHProjectName).psd1" + + # Get module commands + # Remove all versions of the module from the session. Pester can't handle multiple versions. + Get-Module $env:BHProjectName | Remove-Module -Force -ErrorAction Ignore + Import-Module -Name $outputModVerManifest -Verbose:$false -ErrorAction Stop + Import-Module -Name 'PSScriptAnalyzer' -Verbose:$false -ErrorAction Inquire + } + Context 'Method usage with Invoke-WebRequest and UseBasicParsing' { + It 'Detects bad method usage' { + $fakeScript = '$bar = (iwr -Uri https://example.com -UseBasicParsing).Forms' + $ast = [System.Management.Automation.Language.Parser]::ParseInput($fakeScript, [ref]$null, [ref]$null) + $result = Measure-BasicWebRequestProperty -ScriptBlockAst $ast + $result.Count | Should -BeExactly 1 + $result[0].Message | Should -Be "Invoke-WebRequest cannot use the 'Forms' parameter when 'UseBasicParsing' is specified." + } + + It 'does not detect correct usage of Images property' { + $fakeScript = '$bar = (iwr -Uri https://example.com -UseBasicParsing).Images' + $ast = [System.Management.Automation.Language.Parser]::ParseInput($fakeScript, [ref]$null, [ref]$null) + $result = Measure-BasicWebRequestProperty -ScriptBlockAst $ast + $result | Should -BeNullOrEmpty + } + } + Context 'Assignment usage with Invoke-WebRequest and UseBasicParsing' { + It 'Does detect usage of incompatible properties after assignment' { + $fakeScript = @' + $foo = Invoke-WebRequest -Uri https://example.com -UseBasicParsing + $foo.Forms # Not valid with UseBasicParsing +'@ + $ast = [System.Management.Automation.Language.Parser]::ParseInput($fakeScript, [ref]$null, [ref]$null) + $result = Measure-BasicWebRequestProperty -ScriptBlockAst $ast + $result | Should -Not -BeNullOrEmpty + $result.Count | Should -BeExactly 1 + $result[0].Message | Should -Be "Invoke-WebRequest cannot use the 'Forms' parameter when 'UseBasicParsing' is specified." + } + It 'Does detect usage of incompatible properties after assignment' { + $fakeScript = @' + $foo = Invoke-WebRequest -Uri https://example.com -UseBasicParsing + $bar = $foo.Forms # Not valid with UseBasicParsing +'@ + $ast = [System.Management.Automation.Language.Parser]::ParseInput($fakeScript, [ref]$null, [ref]$null) + $result = Measure-BasicWebRequestProperty -ScriptBlockAst $ast + $result | Should -Not -BeNullOrEmpty + $result.Count | Should -BeExactly 1 + $result[0].Message | Should -Be "Invoke-WebRequest cannot use the 'Forms' parameter when 'UseBasicParsing' is specified." + } + It 'Does not detect assignments of Invoke-WebRequest with UseBasicParsing' { + $fakeScript = @' + $foo = Invoke-WebRequest -Uri https://example.com -UseBasicParsing + $foo.Images # This is allowed +'@ + $ast = [System.Management.Automation.Language.Parser]::ParseInput($fakeScript, [ref]$null, [ref]$null) + $result = Measure-BasicWebRequestProperty -ScriptBlockAst $ast + $result | Should -BeNullOrEmpty + } + } +} diff --git a/tests/Measure-InvokeWebRequestWithoutBasic.tests.ps1 b/tests/Measure-InvokeWebRequestWithoutBasic.tests.ps1 new file mode 100644 index 0000000..fe94a98 --- /dev/null +++ b/tests/Measure-InvokeWebRequestWithoutBasic.tests.ps1 @@ -0,0 +1,103 @@ +Describe 'Measure-InvokeWebRequestWithoutBasic' { + BeforeAll { + if ( -not $env:BHPSModuleManifest ) { + .\build.ps1 -Task Build -Verbose + } + $manifest = Import-PowerShellDataFile -Path $env:BHPSModuleManifest + $outputDir = Join-Path -Path $env:BHProjectPath -ChildPath 'Output' + $outputModDir = Join-Path -Path $outputDir -ChildPath $env:BHProjectName + $outputModVerDir = Join-Path -Path $outputModDir -ChildPath $manifest.ModuleVersion + $outputModVerManifest = Join-Path -Path $outputModVerDir -ChildPath "$($env:BHProjectName).psd1" + + # Get module commands + # Remove all versions of the module from the session. Pester can't handle multiple versions. + Get-Module $env:BHProjectName | Remove-Module -Force -ErrorAction Ignore + Import-Module -Name $outputModVerManifest -Verbose:$false -ErrorAction Stop + } + + Context 'When Invoke-WebRequest is used without UseBasicParsing' { + It 'Detects Invoke-WebRequest without UseBasicParsing' { + $fakeScript = @" +Invoke-WebRequest -Uri 'https://example.com' +"@ + $ast = [System.Management.Automation.Language.Parser]::ParseInput($fakeScript, [ref]$null, [ref]$null) + $result = Measure-InvokeWebRequestWithoutBasic -ScriptBlockAst $ast + $result.Count | Should -BeExactly 1 + $result[0].Message | Should -Be 'Invoke-WebRequest should be used with the UseBasicParsing parameter.' + $result[0].Severity | Should -Be 'Error' + } + + It 'Detects iwr alias without UseBasicParsing' { + $fakeScript = @" +iwr -Uri 'https://example.com' +"@ + $ast = [System.Management.Automation.Language.Parser]::ParseInput($fakeScript, [ref]$null, [ref]$null) + $result = Measure-InvokeWebRequestWithoutBasic -ScriptBlockAst $ast + $result.Count | Should -BeExactly 1 + $result[0].Message | Should -Be 'Invoke-WebRequest should be used with the UseBasicParsing parameter.' + } + + It 'Detects curl alias without UseBasicParsing' { + $fakeScript = @" +curl 'https://example.com' +"@ + $ast = [System.Management.Automation.Language.Parser]::ParseInput($fakeScript, [ref]$null, [ref]$null) + $result = Measure-InvokeWebRequestWithoutBasic -ScriptBlockAst $ast + $result.Count | Should -BeExactly 1 + $result[0].Message | Should -Be 'Invoke-WebRequest should be used with the UseBasicParsing parameter.' + } + + It 'Does not detect curl.exe usage' { + $fakeScript = @" +curl.exe 'https://example.com' +"@ + $ast = [System.Management.Automation.Language.Parser]::ParseInput($fakeScript, [ref]$null, [ref]$null) + $result = Measure-InvokeWebRequestWithoutBasic -ScriptBlockAst $ast + $result.Count | Should -Be 0 + } + } + + Context 'When Invoke-WebRequest is used with UseBasicParsing' { + It 'Does not flag Invoke-WebRequest with UseBasicParsing' { + $fakeScript = @" +Invoke-WebRequest -Uri 'https://example.com' -UseBasicParsing +"@ + $ast = [System.Management.Automation.Language.Parser]::ParseInput($fakeScript, [ref]$null, [ref]$null) + $result = Measure-InvokeWebRequestWithoutBasic -ScriptBlockAst $ast + $result.Count | Should -Be 0 + } + + It 'Does not flag iwr alias with UseBasicParsing' { + $fakeScript = @" +iwr -Uri 'https://example.com' -UseBasicParsing +"@ + $ast = [System.Management.Automation.Language.Parser]::ParseInput($fakeScript, [ref]$null, [ref]$null) + $result = Measure-InvokeWebRequestWithoutBasic -ScriptBlockAst $ast + $result.Count | Should -Be 0 + } + } + + Context 'Multiple commands in script' { + It 'Detects multiple violations in same script' { + $fakeScript = @" +Invoke-WebRequest -Uri 'https://example.com' +iwr 'https://test.com' +curl 'https://another.com' +"@ + $ast = [System.Management.Automation.Language.Parser]::ParseInput($fakeScript, [ref]$null, [ref]$null) + $result = Measure-InvokeWebRequestWithoutBasic -ScriptBlockAst $ast + $result.Count | Should -BeExactly 3 + } + + It 'Does not flag non-Invoke-WebRequest commands' { + $fakeScript = @" +Write-Host 'Hello, World!' +Get-Process +Invoke-RestMethod -Uri 'https://example.com' +"@ + $ast = [System.Management.Automation.Language.Parser]::ParseInput($fakeScript, [ref]$null, [ref]$null) + $result = Measure-InvokeWebRequestWithoutBasic -ScriptBlockAst $ast + $result.Count | Should -Be 0 + } + } +} diff --git a/tests/Measure-SecureStringWithKey.ps1 b/tests/Measure-SecureStringWithKey.ps1 index 6236b79..4d01da2 100644 --- a/tests/Measure-SecureStringWithKey.ps1 +++ b/tests/Measure-SecureStringWithKey.ps1 @@ -1,5 +1,8 @@ Describe 'Measure-TODOComment' { BeforeAll { + if ( -not $env:BHPSModuleManifest ) { + .\build.ps1 -Task Build -Verbose + } $manifest = Import-PowerShellDataFile -Path $env:BHPSModuleManifest $outputDir = Join-Path -Path $env:BHProjectPath -ChildPath 'Output' $outputModDir = Join-Path -Path $outputDir -ChildPath $env:BHProjectName diff --git a/tests/Measure-TODOComment.tests.ps1 b/tests/Measure-TODOComment.tests.ps1 index 4e8ec66..65d1207 100644 --- a/tests/Measure-TODOComment.tests.ps1 +++ b/tests/Measure-TODOComment.tests.ps1 @@ -1,5 +1,8 @@ Describe 'Measure-TODOComment' { BeforeAll { + if ( -not $env:BHPSModuleManifest ) { + .\build.ps1 -Task Build -Verbose + } $manifest = Import-PowerShellDataFile -Path $env:BHPSModuleManifest $outputDir = Join-Path -Path $env:BHProjectPath -ChildPath 'Output' $outputModDir = Join-Path -Path $outputDir -ChildPath $env:BHProjectName