Skip to content

Commit 87199e5

Browse files
bergmeisterbergmeister
authored
Also return suggestion to use PSCredential for AvoidUsingPlainTextForPassword rule (#1782)
* Also return suggestion to use PSCredential for AvoidUsingPlainTextForPassword rule * fix tests
1 parent 5cd65bd commit 87199e5

File tree

4 files changed

+43
-37
lines changed

4 files changed

+43
-37
lines changed

‎Rules/AvoidUsingPlainTextForPassword.cs

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -64,37 +64,43 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
6464
}
6565
}
6666

67-
private List<CorrectionExtent> GetCorrectionExtent(ParameterAst paramAst)
67+
private IEnumerable<CorrectionExtent> GetCorrectionExtent(ParameterAst paramAst)
6868
{
6969
//Find the parameter type extent and replace that with secure string
7070
IScriptExtent extent;
7171
var typeAttributeAst = GetTypeAttributeAst(paramAst);
7272
var corrections = new List<CorrectionExtent>();
7373
string correctionText;
74-
if (typeAttributeAst == null)
75-
{
76-
// cannot find any type attribute
77-
extent = paramAst.Name.Extent;
78-
correctionText = string.Format("[SecureString] {0}", paramAst.Name.Extent.Text);
79-
}
80-
else
74+
75+
foreach (string correctionType in new[] { "SecureString", "PSCredential" })
8176
{
82-
// replace only the existing type with [SecureString]
83-
extent = typeAttributeAst.Extent;
84-
correctionText = typeAttributeAst.TypeName.IsArray ? "[SecureString[]]" : "[SecureString]";
77+
if (typeAttributeAst == null)
78+
{
79+
// cannot find any type attribute
80+
extent = paramAst.Name.Extent;
81+
correctionText = $"[{correctionType}] {paramAst.Name.Extent.Text}";
82+
}
83+
else
84+
{
85+
// replace only the existing type with [SecureString]
86+
extent = typeAttributeAst.Extent;
87+
correctionText = typeAttributeAst.TypeName.IsArray ? $"[{correctionType}[]]" : $"[{correctionType}]";
88+
}
89+
string description = string.Format(
90+
CultureInfo.CurrentCulture,
91+
Strings.AvoidUsingPlainTextForPasswordCorrectionDescription,
92+
paramAst.Name.Extent.Text,
93+
correctionType);
94+
corrections.Add(new CorrectionExtent(
95+
extent.StartLineNumber,
96+
extent.EndLineNumber,
97+
extent.StartColumnNumber,
98+
extent.EndColumnNumber,
99+
correctionText.ToString(),
100+
paramAst.Extent.File,
101+
description));
85102
}
86-
string description = string.Format(
87-
CultureInfo.CurrentCulture,
88-
Strings.AvoidUsingPlainTextForPasswordCorrectionDescription,
89-
paramAst.Name.Extent.Text);
90-
corrections.Add(new CorrectionExtent(
91-
extent.StartLineNumber,
92-
extent.EndLineNumber,
93-
extent.StartColumnNumber,
94-
extent.EndColumnNumber,
95-
correctionText.ToString(),
96-
paramAst.Extent.File,
97-
description));
103+
98104
return corrections;
99105
}
100106

‎Rules/Strings.Designer.cs

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Rules/Strings.resx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,7 @@
292292
<value>Password parameters that take in plaintext will expose passwords and compromise the security of your system.</value>
293293
</data>
294294
<data name="AvoidUsingPlainTextForPasswordError" xml:space="preserve">
295-
<value>Parameter '{0}' should use SecureString, otherwise this will expose sensitive information. See ConvertTo-SecureString for more information.</value>
295+
<value>Parameter '{0}' should not use String type but either SecureString or PSCredential, otherwise it increases the chance to to expose this sensitive information.</value>
296296
</data>
297297
<data name="AvoidUsingPlainTextForPasswordCommonName" xml:space="preserve">
298298
<value>Avoid Using Plain Text For Password Parameter</value>
@@ -778,7 +778,7 @@
778778
<value>Replace {0} with {1}</value>
779779
</data>
780780
<data name="AvoidUsingPlainTextForPasswordCorrectionDescription" xml:space="preserve">
781-
<value>Set {0} type to SecureString</value>
781+
<value>Set {0} type to {1}</value>
782782
</data>
783783
<data name="MissingModuleManifestFieldCorrectionDescription" xml:space="preserve">
784784
<value>Add {0} = {1} to the module manifest</value>
@@ -1164,4 +1164,4 @@
11641164
<data name="AvoidMultipleTypeAttributesName" xml:space="preserve">
11651165
<value>AvoidMultipleTypeAttributes</value>
11661166
</data>
1167-
</root>
1167+
</root>

‎Tests/Rules/AvoidUsingPlainTextForPassword.tests.ps1

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
# Licensed under the MIT License.
33

44
BeforeAll {
5-
$violationMessage = [regex]::Escape("Parameter '`$password' should use SecureString, otherwise this will expose sensitive information. See ConvertTo-SecureString for more information.")
5+
$violationMessage = [regex]::Escape("Parameter '`$password' should not use String type but either ")
66
$violationName = "PSAvoidUsingPlainTextForPassword"
77
$violationFilepath = Join-Path $PSScriptRoot 'AvoidUsingPlainTextForPassword.ps1'
88
$violations = Invoke-ScriptAnalyzer $violationFilepath | Where-Object { $_.RuleName -eq $violationName }
@@ -17,15 +17,15 @@ Describe "AvoidUsingPlainTextForPassword" {
1717
}
1818

1919
It "suggests corrections" {
20-
Test-CorrectionExtent $violationFilepath $violations[0] 1 '$passphrases' '[SecureString] $passphrases'
20+
Test-CorrectionExtent $violationFilepath $violations[0] 2 '$passphrases' '[SecureString] $passphrases'
2121
$violations[0].SuggestedCorrections[0].Description | Should -Be 'Set $passphrases type to SecureString'
2222

23-
Test-CorrectionExtent $violationFilepath $violations[1] 1 '$passwordparam' '[SecureString] $passwordparam'
24-
Test-CorrectionExtent $violationFilepath $violations[2] 1 '$credential' '[SecureString] $credential'
25-
Test-CorrectionExtent $violationFilepath $violations[3] 1 '$password' '[SecureString] $password'
26-
Test-CorrectionExtent $violationFilepath $violations[4] 1 '[string]' '[SecureString]'
27-
Test-CorrectionExtent $violationFilepath $violations[5] 1 '[string[]]' '[SecureString[]]'
28-
Test-CorrectionExtent $violationFilepath $violations[6] 1 '[string]' '[SecureString]'
23+
Test-CorrectionExtent $violationFilepath $violations[1] 2 '$passwordparam' '[SecureString] $passwordparam'
24+
Test-CorrectionExtent $violationFilepath $violations[2] 2 '$credential' '[SecureString] $credential'
25+
Test-CorrectionExtent $violationFilepath $violations[3] 2 '$password' '[SecureString] $password'
26+
Test-CorrectionExtent $violationFilepath $violations[4] 2 '[string]' '[SecureString]'
27+
Test-CorrectionExtent $violationFilepath $violations[5] 2 '[string[]]' '[SecureString[]]'
28+
Test-CorrectionExtent $violationFilepath $violations[6] 2 '[string]' '[SecureString]'
2929
}
3030

3131
It "has the correct violation message" {

0 commit comments

Comments
 (0)