Skip to content

Commit 6f1ed89

Browse files
author
Kapil Borle
authored
Merge pull request #683 from PowerShell/kapilmb/FixPSCredentialTypeRule
Fix UsePSCredentialType rule
2 parents 048a017 + 788a79b commit 6f1ed89

File tree

9 files changed

+197
-60
lines changed

9 files changed

+197
-60
lines changed

‎Engine/Commands/InvokeScriptAnalyzerCommand.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
using System.Collections.Concurrent;
2727
using System.Threading;
2828
using System.Management.Automation.Runspaces;
29+
using System.Collections;
2930

3031
namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.Commands
3132
{
@@ -220,6 +221,12 @@ protected override void BeginProcessing()
220221
this);
221222
Helper.Instance.Initialize();
222223

224+
var psVersionTable = this.SessionState.PSVariable.GetValue("PSVersionTable") as Hashtable;
225+
if (psVersionTable != null)
226+
{
227+
Helper.Instance.SetPSVersionTable(psVersionTable);
228+
}
229+
223230
string[] rulePaths = Helper.ProcessCustomRulePaths(customRulePath,
224231
this.SessionState, recurseCustomRulePath);
225232
if (IsFileParameterSet())

‎Engine/Helper.cs‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
using System.Globalization;
2121
using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
2222
using System.Management.Automation.Runspaces;
23+
using System.Collections;
2324

2425
namespace Microsoft.Windows.PowerShell.ScriptAnalyzer
2526
{
@@ -36,6 +37,7 @@ public class Helper
3637
private Object getCommandLock = new object();
3738
private readonly static Version minSupportedPSVersion = new Version(3, 0);
3839
private Dictionary<string, Dictionary<string, object>> ruleArguments;
40+
private PSVersionTable psVersionTable;
3941

4042
#endregion
4143

@@ -1860,6 +1862,26 @@ public static bool IsModuleManifest(string filepath, Version powershellVersion =
18601862
// check if the keys given in module manifest are a proper subset of Keys
18611863
return map.Keys.All(x => allKeys.Concat(deprecatedKeys).Contains(x, StringComparer.OrdinalIgnoreCase));
18621864
}
1865+
1866+
public void SetPSVersionTable(Hashtable psVersionTable)
1867+
{
1868+
if (psVersionTable == null)
1869+
{
1870+
throw new ArgumentNullException("psVersionTable");
1871+
}
1872+
1873+
this.psVersionTable = new PSVersionTable(psVersionTable);
1874+
}
1875+
1876+
#if CORECLR
1877+
public SemanticVersion GetPSVersion()
1878+
#else
1879+
public Version GetPSVersion()
1880+
#endif
1881+
{
1882+
return psVersionTable == null ? null : psVersionTable.PSVersion;
1883+
}
1884+
18631885
#endregion Methods
18641886
}
18651887

@@ -3802,6 +3824,49 @@ private int GetIndex(T vertex)
38023824
int idx;
38033825
return vertexIndexMap.TryGetValue(vertex, out idx) ? idx : -1;
38043826
}
3827+
}
3828+
3829+
internal class PSVersionTable
3830+
{
3831+
private readonly string psVersionKey = "PSVersion";
3832+
private readonly string psEditionKey = "PSEdition";
3833+
#if CORECLR
3834+
public SemanticVersion PSVersion { get; private set; }
3835+
#else
3836+
public Version PSVersion { get; private set; }
3837+
#endif
3838+
public string PSEdition { get; private set; }
38053839

3840+
public PSVersionTable(Hashtable psVersionTable)
3841+
{
3842+
if (psVersionTable == null)
3843+
{
3844+
throw new ArgumentNullException("psVersionTable");
3845+
}
3846+
3847+
if (!psVersionTable.ContainsKey(psVersionKey))
3848+
{
3849+
throw new ArgumentException("Input PSVersionTable does not contain PSVersion key"); // TODO localize
3850+
}
3851+
3852+
#if CORECLR
3853+
PSVersion = psVersionTable[psVersionKey] as SemanticVersion;
3854+
#else
3855+
PSVersion = psVersionTable[psVersionKey] as Version;
3856+
#endif
3857+
if (PSVersion == null)
3858+
{
3859+
throw new ArgumentException("Input PSVersionTable has invalid PSVersion value type"); // TODO localize
3860+
}
3861+
3862+
if (psVersionTable.ContainsKey(psEditionKey))
3863+
{
3864+
PSEdition = psVersionTable[psEditionKey] as string;
3865+
if (PSEdition == null)
3866+
{
3867+
throw new ArgumentException("Input PSVersionTable has invalid PSEdition value type"); // TODO localize
3868+
}
3869+
}
3870+
}
38063871
}
38073872
}

‎Rules/UsePSCredentialType.cs‎

Lines changed: 79 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -44,43 +44,51 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
4444
if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage);
4545

4646
var sbAst = ast as ScriptBlockAst;
47+
48+
var requiresTransformationAttribute = false;
49+
var psVersion = Helper.Instance.GetPSVersion();
50+
if (psVersion != null && psVersion.Major < 5)
51+
{
52+
requiresTransformationAttribute = true;
53+
}
54+
55+
// do not run the rule if the script requires PS version 5
56+
// but PSSA in invoked through PS version < 5
4757
if (sbAst != null
48-
&& sbAst.ScriptRequirements != null
49-
&& sbAst.ScriptRequirements.RequiredPSVersion != null
50-
&& sbAst.ScriptRequirements.RequiredPSVersion.Major == 5)
58+
&& sbAst.ScriptRequirements != null
59+
&& sbAst.ScriptRequirements.RequiredPSVersion != null
60+
&& sbAst.ScriptRequirements.RequiredPSVersion.Major == 5
61+
&& requiresTransformationAttribute)
5162
{
52-
yield break;
63+
yield break;
5364
}
5465

5566
IEnumerable<Ast> funcDefAsts = ast.FindAll(testAst => testAst is FunctionDefinitionAst, true);
5667
IEnumerable<Ast> scriptBlockAsts = ast.FindAll(testAst => testAst is ScriptBlockAst, true);
5768

58-
string funcName;
69+
List<DiagnosticRecord> diagnosticRecords = new List<DiagnosticRecord>();
5970

6071
foreach (FunctionDefinitionAst funcDefAst in funcDefAsts)
6172
{
62-
funcName = funcDefAst.Name;
63-
73+
IEnumerable<ParameterAst> parameterAsts = null;
6474
if (funcDefAst.Parameters != null)
6575
{
66-
foreach (ParameterAst parameter in funcDefAst.Parameters)
67-
{
68-
if (WrongCredentialUsage(parameter))
69-
{
70-
yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.UsePSCredentialTypeError, funcName), funcDefAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName);
71-
}
72-
}
76+
parameterAsts = funcDefAst.Parameters;
7377
}
7478

75-
if (funcDefAst.Body.ParamBlock != null)
79+
if (funcDefAst.Body.ParamBlock != null
80+
&& funcDefAst.Body.ParamBlock.Parameters != null)
7681
{
77-
foreach (ParameterAst parameter in funcDefAst.Body.ParamBlock.Parameters)
78-
{
79-
if (WrongCredentialUsage(parameter))
80-
{
81-
yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.UsePSCredentialTypeError, funcName), funcDefAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName);
82-
}
83-
}
82+
parameterAsts = funcDefAst.Body.ParamBlock.Parameters;
83+
}
84+
85+
if (parameterAsts != null)
86+
{
87+
diagnosticRecords.AddRange(GetViolations(
88+
parameterAsts,
89+
string.Format(CultureInfo.CurrentCulture, Strings.UsePSCredentialTypeError, funcDefAst.Name),
90+
fileName,
91+
requiresTransformationAttribute));
8492
}
8593
}
8694

@@ -94,28 +102,68 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
94102

95103
if (scriptBlockAst.ParamBlock != null && scriptBlockAst.ParamBlock.Parameters != null)
96104
{
97-
foreach (ParameterAst parameter in scriptBlockAst.ParamBlock.Parameters)
105+
diagnosticRecords.AddRange(GetViolations(
106+
scriptBlockAst.ParamBlock.Parameters,
107+
string.Format(CultureInfo.CurrentCulture, Strings.UsePSCredentialTypeErrorSB),
108+
fileName,
109+
requiresTransformationAttribute));
110+
}
111+
}
112+
113+
foreach (var dr in diagnosticRecords)
114+
{
115+
yield return dr;
116+
}
117+
}
118+
119+
private IEnumerable<DiagnosticRecord> GetViolations(
120+
IEnumerable<ParameterAst> parameterAsts,
121+
string errorMessage,
122+
string fileName,
123+
bool requiresTransformationAttribute)
124+
{
125+
foreach (ParameterAst parameter in parameterAsts)
126+
{
127+
if (WrongCredentialUsage(parameter, requiresTransformationAttribute))
98128
{
99-
if (WrongCredentialUsage(parameter))
100-
{
101-
yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.UsePSCredentialTypeErrorSB), scriptBlockAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName);
102-
}
129+
yield return new DiagnosticRecord(
130+
errorMessage,
131+
parameter.Extent,
132+
GetName(),
133+
DiagnosticSeverity.Warning,
134+
fileName);
103135
}
104136
}
105-
}
106137
}
107138

108-
private bool WrongCredentialUsage(ParameterAst parameter)
139+
private bool WrongCredentialUsage(ParameterAst parameter, bool requiresTransformationAttribute)
109140
{
110141
if (parameter.Name.VariablePath.UserPath.Equals("Credential", StringComparison.OrdinalIgnoreCase))
111142
{
112143
var psCredentialType = parameter.Attributes.FirstOrDefault(paramAttribute => (paramAttribute.TypeName.IsArray && (paramAttribute.TypeName as ArrayTypeName).ElementType.GetReflectionType() == typeof(PSCredential))
113144
|| paramAttribute.TypeName.GetReflectionType() == typeof(PSCredential));
114145

115-
var credentialAttribute = parameter.Attributes.FirstOrDefault(paramAttribute => paramAttribute.TypeName.GetReflectionType() == typeof(CredentialAttribute));
146+
if (psCredentialType == null)
147+
{
148+
return true;
149+
}
150+
151+
if (!requiresTransformationAttribute && psCredentialType != null)
152+
{
153+
return false;
154+
}
155+
156+
var credentialAttribute = parameter.Attributes.FirstOrDefault(
157+
paramAttribute =>
158+
paramAttribute.TypeName.GetReflectionType() == typeof(CredentialAttribute)
159+
|| paramAttribute.TypeName.FullName.Equals(
160+
"System.Management.Automation.Credential",
161+
StringComparison.OrdinalIgnoreCase));
116162

117163
// check that both exists and pscredentialtype comes before credential attribute
118-
if (psCredentialType != null && credentialAttribute != null && psCredentialType.Extent.EndOffset <= credentialAttribute.Extent.StartOffset)
164+
if (psCredentialType != null
165+
&& credentialAttribute != null
166+
&& psCredentialType.Extent.EndOffset <= credentialAttribute.Extent.StartOffset)
119167
{
120168
return false;
121169
}

‎Tests/Engine/GetScriptAnalyzerRule.tests.ps1‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ Describe "Test Name parameters" {
6262
It "get Rules with no parameters supplied" {
6363
$defaultRules = Get-ScriptAnalyzerRule
6464
$expectedNumRules = 45
65-
if ((Test-PSEditionCoreClr) -or (Test-PSVersionV3))
65+
if ((Test-PSEditionCoreClr) -or (Test-PSVersionV3) -or (Test-PSVersionV4))
6666
{
6767
# for PSv3 PSAvoidGlobalAliases is not shipped because
6868
# it uses StaticParameterBinder.BindCommand which is

‎Tests/PSScriptAnalyzerTestHelper.psm1‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,11 @@ Function Test-PSVersionV3
4343
$PSVersionTable.PSVersion.Major -eq 3
4444
}
4545

46+
Function Test-PSVersionV4
47+
{
48+
$PSVersionTable.PSVersion.Major -eq 4
49+
}
50+
4651
Function Get-Count
4752
{
4853
Begin {$count = 0}
@@ -55,4 +60,5 @@ Export-ModuleMember -Function Test-CorrectionExtent
5560
Export-ModuleMember -Function Test-PSEditionCoreCLR
5661
Export-ModuleMember -Function Test-PSEditionCoreCLRLinux
5762
Export-ModuleMember -Function Test-PSVersionV3
63+
Export-ModuleMember -Function Test-PSVersionV4
5864
Export-ModuleMember -Function Get-Count

‎Tests/Rules/AvoidGlobalAliases.tests.ps1‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,27 @@
11
$directory = Split-Path -Parent $MyInvocation.MyCommand.Path
22
$testRootDirectory = Split-Path -Parent $directory
33
Import-Module (Join-Path $testRootDirectory 'PSScriptAnalyzerTestHelper.psm1')
4-
if ((Test-PSVersionV3))
5-
{
6-
return
7-
}
8-
94
Import-Module PSScriptAnalyzer
105

116
$AvoidGlobalAliasesError = "Avoid creating aliases with a Global scope."
127
$violationName = "PSAvoidGlobalAliases"
138
$violations = Invoke-ScriptAnalyzer $directory\AvoidGlobalAliases.psm1 | Where-Object {$_.RuleName -eq $violationName}
149
$noViolations = Invoke-ScriptAnalyzer $directory\AvoidGlobalAliasesNoViolations.ps1 | Where-Object {$_.RuleName -eq $violationName}
15-
10+
$IsV3OrV4 = (Test-PSVersionV3) -or (Test-PSVersionV4)
1611

1712
Describe "$violationName " {
1813
Context "When there are violations" {
19-
It "Has 4 avoid global alias violations" {
14+
It "Has 4 avoid global alias violations" -Skip:$IsV3OrV4 {
2015
$violations.Count | Should Be 4
2116
}
2217

23-
It "Has the correct description message" {
18+
It "Has the correct description message" -Skip:$IsV3OrV4 {
2419
$violations[0].Message | Should Match $AvoidGlobalAliasesError
2520
}
2621
}
2722

2823
Context "When there are no violations" {
29-
It "Returns no violations" {
24+
It "Returns no violations" -Skip:$IsV3OrV4 {
3025
$noViolations.Count | Should Be 0
3126
}
3227
}

‎Tests/Rules/PSCredentialType.tests.ps1‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,21 @@
1-
Import-Module PSScriptAnalyzer
1+
$directory = Split-Path -Parent $MyInvocation.MyCommand.Path
2+
$testRootDirectory = Split-Path -Parent $directory
3+
Import-Module (Join-Path $testRootDirectory 'PSScriptAnalyzerTestHelper.psm1')
4+
Import-Module PSScriptAnalyzer
5+
26
$violationMessage = "The Credential parameter in 'Credential' must be of type PSCredential. For PowerShell 4.0 and earlier, please define a credential transformation attribute, e.g. [System.Management.Automation.Credential()], after the PSCredential type attribute."
37
$violationName = "PSUsePSCredentialType"
4-
$directory = Split-Path -Parent $MyInvocation.MyCommand.Path
58
$violations = Invoke-ScriptAnalyzer $directory\PSCredentialType.ps1 | Where-Object {$_.RuleName -eq $violationName}
69
$noViolations = Invoke-ScriptAnalyzer $directory\PSCredentialTypeNoViolations.ps1 | Where-Object {$_.RuleName -eq $violationName}
710

811
Describe "PSCredentialType" {
912
Context "When there are violations" {
10-
It "has 2 PSCredential type violation" {
11-
$violations.Count | Should Be 2
13+
$expectedViolations = 1
14+
if ((Test-PSVersionV3) -or (Test-PSVersionV4)) {
15+
$expectedViolations = 2
16+
}
17+
It ("has {0} PSCredential type violation" -f $expectedViolations) {
18+
$violations.Count | Should Be $expectedViolations
1219
}
1320

1421
It "has the correct description message" {
@@ -31,14 +38,9 @@ function Get-Credential
3138

3239
}
3340

34-
$expectedViolationCount = 0
35-
if ($PSVersionTable.PSVersion -lt [Version]'5.0.0')
36-
{
37-
$expectedViolationCount = 1
38-
}
39-
Context ("When there are {0} violations" -f $expectedViolationCount) {
40-
It ("returns {0} violations" -f $expectedViolationCount) {
41-
$noViolations.Count | Should Be $expectedViolationCount
41+
Context ("When there are no violations") {
42+
It "returns no violations" {
43+
$noViolations.Count | Should Be 0
4244
}
4345
}
4446
}

0 commit comments

Comments
 (0)