Skip to content
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

AvoidUsingPositionalParameter: Check if command has parameters to avoid having az in default CommandAllowList #1850

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions Engine/Helper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -609,14 +609,15 @@ public bool HasSplattedVariable(CommandAst cmdAst)
/// </summary>
/// <param name="cmdAst"></param>
/// <returns></returns>
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;
Expand Down
7 changes: 5 additions & 2 deletions Rules/AvoidPositionalParameters.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()
Expand Down Expand Up @@ -61,9 +62,11 @@ public override IEnumerable<DiagnosticRecord> 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?.CommandType == CommandTypes.Application) continue;

PipelineAst parent = cmdAst.Parent as PipelineAst;

string commandName = cmdAst.GetCommandName();
Expand Down
2 changes: 1 addition & 1 deletion Rules/UseCmdletCorrectly.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
6 changes: 3 additions & 3 deletions docs/Rules/AvoidUsingPositionalParameters.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,17 @@ supplied. A simple example where the risk of using positional parameters is negl
```powershell
Rules = @{
PSAvoidUsingPositionalParameters = @{
CommandAllowList = 'az', 'Join-Path'
CommandAllowList = 'Join-Path', 'MyCmdletOrScript'
Enable = $true
}
}
```

### Parameters

#### CommandAllowList: string[] (Default value is 'az')
#### CommandAllowList: 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`)

Expand Down