From 9b983225b929e535f098bc145c3234b308f28997 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Mon, 24 Oct 2022 12:06:54 +0100 Subject: [PATCH 1/4] AvoidUsingPositionalParameter : Check if command has parameters --- Engine/Helper.cs | 5 +++-- Rules/AvoidPositionalParameters.cs | 6 ++++-- Rules/UseCmdletCorrectly.cs | 2 +- Tests/Rules/AvoidPositionalParameters.tests.ps1 | 5 +++++ docs/Rules/AvoidUsingPositionalParameters.md | 6 +++--- 5 files changed, 16 insertions(+), 8 deletions(-) diff --git a/Engine/Helper.cs b/Engine/Helper.cs index feea8a5ec..3f69c6b6c 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -615,14 +615,15 @@ public bool HasSplattedVariable(CommandAst cmdAst) /// /// /// - public bool IsKnownCmdletFunctionOrExternalScript(CommandAst cmdAst) + public bool IsKnownCmdletFunctionOrExternalScript(CommandAst cmdAst, out CommandInfo commandInfo) { + commandInfo = null; if (cmdAst == null) { return false; } - var commandInfo = GetCommandInfo(cmdAst.GetCommandName()); + commandInfo = GetCommandInfo(cmdAst.GetCommandName()); if (commandInfo == null) { return false; diff --git a/Rules/AvoidPositionalParameters.cs b/Rules/AvoidPositionalParameters.cs index 3c6ec9626..91293b17a 100644 --- a/Rules/AvoidPositionalParameters.cs +++ b/Rules/AvoidPositionalParameters.cs @@ -6,6 +6,7 @@ using System.Management.Automation.Language; using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; using System.Linq; +using System.Management.Automation; #if !CORECLR using System.ComponentModel.Composition; #endif @@ -21,7 +22,7 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules #endif public class AvoidPositionalParameters : ConfigurableRule { - [ConfigurableRuleProperty(defaultValue: new string[] { "az" })] + [ConfigurableRuleProperty(defaultValue: new string[] { })] public string[] CommandAllowList { get; set; } public AvoidPositionalParameters() @@ -61,9 +62,10 @@ public override IEnumerable AnalyzeScript(Ast ast, string file // MSDN: CommandAst.GetCommandName Method if (cmdAst.GetCommandName() == null) continue; - if ((Helper.Instance.IsKnownCmdletFunctionOrExternalScript(cmdAst) || declaredFunctionNames.Contains(cmdAst.GetCommandName())) && + if ((Helper.Instance.IsKnownCmdletFunctionOrExternalScript(cmdAst, out CommandInfo commandInfo) || declaredFunctionNames.Contains(cmdAst.GetCommandName())) && (Helper.Instance.PositionalParameterUsed(cmdAst, true))) { + if (commandInfo?.Parameters.Count == 0) continue; PipelineAst parent = cmdAst.Parent as PipelineAst; string commandName = cmdAst.GetCommandName(); diff --git a/Rules/UseCmdletCorrectly.cs b/Rules/UseCmdletCorrectly.cs index 0239d95fc..ccec27e0b 100644 --- a/Rules/UseCmdletCorrectly.cs +++ b/Rules/UseCmdletCorrectly.cs @@ -100,7 +100,7 @@ private bool MandatoryParameterExists(CommandAst cmdAst) } // Positional parameters could be mandatory, so we assume all is well - if (Helper.Instance.PositionalParameterUsed(cmdAst) && Helper.Instance.IsKnownCmdletFunctionOrExternalScript(cmdAst)) + if (Helper.Instance.PositionalParameterUsed(cmdAst) && Helper.Instance.IsKnownCmdletFunctionOrExternalScript(cmdAst, out _)) { return true; } diff --git a/Tests/Rules/AvoidPositionalParameters.tests.ps1 b/Tests/Rules/AvoidPositionalParameters.tests.ps1 index ec5a91e24..56c65fcd7 100644 --- a/Tests/Rules/AvoidPositionalParameters.tests.ps1 +++ b/Tests/Rules/AvoidPositionalParameters.tests.ps1 @@ -55,6 +55,11 @@ Describe "AvoidPositionalParameters" { Rules = @{ PSAvoidUsingPositionalParameters = @{ CommandAllowList = 'az', 'Join-Path' } } } | Should -BeNullOrEmpty } + + It "returns no violations for script with no defined parameters" { + Invoke-ScriptAnalyzer -ScriptDefinition 'join-patH a b c' + } | Should -BeNullOrEmpty + } } Context "Function defined and called in script, which has 3 or more positional parameters triggers rule." { diff --git a/docs/Rules/AvoidUsingPositionalParameters.md b/docs/Rules/AvoidUsingPositionalParameters.md index 4cecb2965..34d0ad4ac 100644 --- a/docs/Rules/AvoidUsingPositionalParameters.md +++ b/docs/Rules/AvoidUsingPositionalParameters.md @@ -25,7 +25,7 @@ supplied. A simple example where the risk of using positional parameters is negl ```powershell Rules = @{ AvoidUsingPositionalParameters = @{ - CommandAllowList = 'az', 'Join-Path' + CommandAllowList = 'Join-Path', 'MyCmdletOrScript' Enable = $true } } @@ -33,9 +33,9 @@ Rules = @{ ### Parameters -#### AvoidUsingPositionalParameters: string[] (Default value is 'az') +#### AvoidUsingPositionalParameters: string[] (Default value is @()') -Commands to be excluded from this rule. `az` is excluded by default because starting with version 2.40.0 the entrypoint of the AZ CLI became an `az.ps1` script but this script does not have any named parameters and just passes them on using `$args` as is to the Python process that it starts, therefore it is still a CLI and not a PowerShell command. +Commands or scripts to be excluded from this rule. #### Enable: bool (Default value is `$true`) From 5e9cbedf76ddbee5c126af733267e7b9fc20e29a Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Mon, 24 Oct 2022 12:24:48 +0100 Subject: [PATCH 2/4] fix syntax --- Tests/Rules/AvoidPositionalParameters.tests.ps1 | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Tests/Rules/AvoidPositionalParameters.tests.ps1 b/Tests/Rules/AvoidPositionalParameters.tests.ps1 index 56c65fcd7..9f77f457b 100644 --- a/Tests/Rules/AvoidPositionalParameters.tests.ps1 +++ b/Tests/Rules/AvoidPositionalParameters.tests.ps1 @@ -57,8 +57,7 @@ Describe "AvoidPositionalParameters" { } It "returns no violations for script with no defined parameters" { - Invoke-ScriptAnalyzer -ScriptDefinition 'join-patH a b c' - } | Should -BeNullOrEmpty + Invoke-ScriptAnalyzer -ScriptDefinition 'join-patH a b c' | Should -BeNullOrEmpty } } From 8127a1649d2af31a2af9b878693cc160c32df670 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Tue, 25 Oct 2022 17:27:07 +0100 Subject: [PATCH 3/4] remove unneeded test --- Tests/Rules/AvoidPositionalParameters.tests.ps1 | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Tests/Rules/AvoidPositionalParameters.tests.ps1 b/Tests/Rules/AvoidPositionalParameters.tests.ps1 index 9f77f457b..ec5a91e24 100644 --- a/Tests/Rules/AvoidPositionalParameters.tests.ps1 +++ b/Tests/Rules/AvoidPositionalParameters.tests.ps1 @@ -55,10 +55,6 @@ Describe "AvoidPositionalParameters" { Rules = @{ PSAvoidUsingPositionalParameters = @{ CommandAllowList = 'az', 'Join-Path' } } } | Should -BeNullOrEmpty } - - It "returns no violations for script with no defined parameters" { - Invoke-ScriptAnalyzer -ScriptDefinition 'join-patH a b c' | Should -BeNullOrEmpty - } } Context "Function defined and called in script, which has 3 or more positional parameters triggers rule." { From f7e55eede0c7ecc58eba2b16d18b35ffdb0f8c65 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Mon, 12 Feb 2024 16:53:47 +0000 Subject: [PATCH 4/4] Update Rules/AvoidPositionalParameters.cs --- Rules/AvoidPositionalParameters.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Rules/AvoidPositionalParameters.cs b/Rules/AvoidPositionalParameters.cs index 91293b17a..2071baebd 100644 --- a/Rules/AvoidPositionalParameters.cs +++ b/Rules/AvoidPositionalParameters.cs @@ -65,7 +65,8 @@ public override IEnumerable AnalyzeScript(Ast ast, string file if ((Helper.Instance.IsKnownCmdletFunctionOrExternalScript(cmdAst, out CommandInfo commandInfo) || declaredFunctionNames.Contains(cmdAst.GetCommandName())) && (Helper.Instance.PositionalParameterUsed(cmdAst, true))) { - if (commandInfo?.Parameters.Count == 0) continue; + if (commandInfo?.CommandType == CommandTypes.Application) continue; + PipelineAst parent = cmdAst.Parent as PipelineAst; string commandName = cmdAst.GetCommandName();