Skip to content

Commit 014cfba

Browse files
committed
Implemented GitHub Copilot suggestions
1 parent 9b67e56 commit 014cfba

3 files changed

Lines changed: 51 additions & 5 deletions

File tree

Rules/AvoidSecretDisclosure.cs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,13 @@ testAst is CommandAst cmdAst &&
5555
var bindingResult = StaticParameterBinder.BindCommand(cmdAst, true);
5656

5757
// Check for -AsPlainText parameter
58+
// The constantValue appears true even the value is a variable
59+
// This is ok because the rule should still trigger in that case since the value of the
60+
// variable could be true at runtime, and we want to catch all potential violations
5861
if (
5962
bindingResult.BoundParameters.ContainsKey("AsPlainText") &&
60-
(bool)bindingResult.BoundParameters["AsPlainText"].ConstantValue == true
63+
bindingResult.BoundParameters["AsPlainText"].ConstantValue is bool constantValue &&
64+
constantValue == true
6165
) {
6266
yield return GetDiagnosticRecord(cmdAst.Extent, fileName, "AsPlainText");
6367
}
@@ -102,10 +106,7 @@ testAst is MemberExpressionAst memberAst &&
102106
private DiagnosticRecord GetDiagnosticRecord(IScriptExtent Extent, string fileName, string suppressionId)
103107
{
104108
return new DiagnosticRecord(
105-
string.Format(
106-
CultureInfo.CurrentCulture,
107-
Strings.AvoidSecretDisclosureError,
108-
Extent.Text),
109+
Strings.AvoidSecretDisclosureError,
109110
Extent,
110111
GetName(),
111112
DiagnosticSeverity.Warning,

Tests/Rules/AvoidSecretDisclosure.tests.ps1

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,19 @@ Describe "AvoidSecretDisclosure" {
2929
$violations.RuleSuppressionID | Should -Be 'AsPlainText'
3030
}
3131

32+
It "ConvertFrom-SecureString -AsPlainText:$true" {
33+
$scriptDefinition = {
34+
$SecureString = ConvertTo-SecureString 'P@ssW0rd' -AsPlainText
35+
$Null = $SecureString | ConvertFrom-SecureString -AsPlainText:$true
36+
}.ToString()
37+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
38+
$violations.Count | Should -Be 1
39+
$violations.Severity | Should -Be Warning
40+
$violations.Extent.Text | Should -Be {ConvertFrom-SecureString -AsPlainText:$true}.ToString()
41+
$violations.Message | Should -Be $ruleMessage
42+
$violations.RuleSuppressionID | Should -Be 'AsPlainText'
43+
}
44+
3245
It "SecureStringToBSTR()" {
3346
$scriptDefinition = {
3447
$SecureString = ConvertTo-SecureString 'P@ssW0rd' -AsPlainText
@@ -198,4 +211,30 @@ Describe "AvoidSecretDisclosure" {
198211
$violations | Should -BeNullOrEmpty
199212
}
200213
}
214+
215+
Context "should not crash" {
216+
217+
It "-AsPlainText:$false" {
218+
$scriptDefinition = {
219+
$SecureString = ConvertTo-SecureString 'P@ssW0rd' -AsPlainText
220+
$Null = $SecureString | ConvertFrom-SecureString -AsPlainText:$false
221+
}.ToString()
222+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
223+
$violations | Should -BeNullOrEmpty
224+
}
225+
226+
It "-AsPlainText:$someVar" {
227+
$scriptDefinition = {
228+
param ([bool] $someVar)
229+
$SecureString = ConvertTo-SecureString 'P@ssW0rd' -AsPlainText
230+
$Null = $SecureString | ConvertFrom-SecureString -AsPlainText:$someVar
231+
}.ToString()
232+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
233+
$violations.Count | Should -Be 1
234+
$violations.Severity | Should -Be Warning
235+
$violations.Extent.Text | Should -Be {ConvertFrom-SecureString -AsPlainText:$someVar}.ToString()
236+
$violations.Message | Should -Be $ruleMessage
237+
$violations.RuleSuppressionID | Should -Be 'AsPlainText'
238+
}
239+
}
201240
}

docs/Rules/AvoidSecretDisclosure.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@ In general, avoid any code pattern that involves converting secrets to plaintext
3434
> contain secrets, or to ensure that they do not actually contain secrets. If renaming is not possible, consider
3535
> suppressing the warning for those specific cases.
3636
37+
## Configuration
38+
39+
### Parameters
40+
41+
* `Enable` - Enables or disables the rule. Default value is `True`.
42+
3743
## Example
3844

3945
### Wrong

0 commit comments

Comments
 (0)