From 586302f685a09b8fae4d5d38881c15e167c257ad Mon Sep 17 00:00:00 2001 From: iRon7 Date: Wed, 6 May 2026 11:03:02 +0200 Subject: [PATCH 1/8] Add new AvoidSecretDisclosure rule --- Rules/AvoidSecretDisclosure.cs | 179 +++++++++++++++++ Rules/Strings.resx | 12 ++ Tests/Rules/AvoidSecretDisclosure.tests.ps1 | 201 ++++++++++++++++++++ docs/Rules/AvoidSecretDisclosure.md | 67 +++++++ docs/Rules/README.md | 1 + 5 files changed, 460 insertions(+) create mode 100644 Rules/AvoidSecretDisclosure.cs create mode 100644 Tests/Rules/AvoidSecretDisclosure.tests.ps1 create mode 100644 docs/Rules/AvoidSecretDisclosure.md diff --git a/Rules/AvoidSecretDisclosure.cs b/Rules/AvoidSecretDisclosure.cs new file mode 100644 index 000000000..2e64f2288 --- /dev/null +++ b/Rules/AvoidSecretDisclosure.cs @@ -0,0 +1,179 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +#if !CORECLR +using System.ComponentModel.Composition; +#endif +using System.Globalization; +using System.Management.Automation.Language; +using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; + +namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules +{ + /// + /// AvoidSecretDisclosure: Checks whether a plaintext secret is being retrieved which can lead to + /// security vulnerabilities such as memory trails or logging trails. + /// The general approach of dealing with credentials is to avoid them and instead rely on other means + /// to authenticate, such as certificates or Windows authentication. + /// +#if !CORECLR + [Export(typeof(IScriptRule))] +#endif + public class AvoidSecretDisclosure : ConfigurableRule + { + + /// + /// Construct an object of AvoidSecretDisclosure type. + /// + public AvoidSecretDisclosure() { + Enable = true; + } + + /// + /// Analyzes the given ast to find the [violation] + /// + /// AST to be analyzed. This should be non-null + /// Name of file that corresponds to the input AST. + /// A an enumerable type containing the violations + public override IEnumerable AnalyzeScript(Ast ast, string fileName) + { + if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); + + // Check for ConvertFrom-SecureString with -AsPlainText parameter + IEnumerable convertFromSecureStringAsts = ast.FindAll(testAst => + testAst is CommandAst cmdAst && + cmdAst.GetCommandName() != null && + cmdAst.GetCommandName().Equals("ConvertFrom-SecureString", StringComparison.OrdinalIgnoreCase), + true + ); + + foreach (CommandAst cmdAst in convertFromSecureStringAsts) + { + // Use StaticParameterBinder to reliably get parameter values + var bindingResult = StaticParameterBinder.BindCommand(cmdAst, true); + + // Check for -AsPlainText parameter + if ( + bindingResult.BoundParameters.ContainsKey("AsPlainText") && + (bool)bindingResult.BoundParameters["AsPlainText"].ConstantValue == true + ) { + yield return GetDiagnosticRecord(cmdAst.Extent, fileName, "AsPlainText"); + } + } + + // Check for any invocation of a method that starts with "SecureStringTo" + // (e.g. SecureStringToBSTR, SecureStringToCoTaskMemAnsi, etc.) + IEnumerable secureStringToAsts = ast.FindAll(testAst => + testAst is InvokeMemberExpressionAst invokeAst && + invokeAst.Member != null && + invokeAst.Member.ToString().StartsWith("SecureStringTo", StringComparison.OrdinalIgnoreCase), + true + ); + + foreach (InvokeMemberExpressionAst secureStringToAst in secureStringToAsts) { + yield return GetDiagnosticRecord(secureStringToAst.Extent, fileName, secureStringToAst.Member.ToString()); + } + + // Check for any member access of a property named "Password". + // Note that this is a heuristic and may lead to false positives, + // as not all properties named "Password" necessarily contain secrets, + // and there may be secrets stored in properties with other names. + // However, it is too complex to reliably determine whether a Password + // property is a result of e.g. a PSCredential.GetNetworkCredential() call. + // Anyways, this is still a useful common check to have. + IEnumerable passwordAsts = ast.FindAll(testAst => + testAst is MemberExpressionAst memberAst && + memberAst.Member != null && + string.Equals(memberAst.Member.ToString(), "Password", StringComparison.OrdinalIgnoreCase), + true + ); + + foreach (MemberExpressionAst passwordAst in passwordAsts) { + yield return GetDiagnosticRecord(passwordAst.Extent, fileName, passwordAst.Member.ToString()); + } + + } + + /// + /// Helper function to create a DiagnosticRecord for a given violation + /// + private DiagnosticRecord GetDiagnosticRecord(IScriptExtent Extent, string fileName, string suppressionId) + { + return new DiagnosticRecord( + string.Format( + CultureInfo.CurrentCulture, + Strings.AvoidSecretDisclosureError, + Extent.Text), + Extent, + GetName(), + DiagnosticSeverity.Warning, + fileName, + suppressionId + ); + } + + /// + /// Retrieves the common name of this rule. + /// + public override string GetCommonName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidSecretDisclosureCommonName); + } + + /// + /// Retrieves the description of this rule. + /// + public override string GetDescription() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidSecretDisclosureDescription); + } + + /// + /// Retrieves the name of this rule. + /// + public override string GetName() + { + return string.Format( + CultureInfo.CurrentCulture, + Strings.NameSpaceFormat, + GetSourceName(), + Strings.AvoidSecretDisclosureName); + } + + /// + /// Retrieves the severity of the rule: error, warning or information. + /// + public override RuleSeverity GetSeverity() + { + return RuleSeverity.Warning; + } + + /// + /// Gets the severity of the returned diagnostic record: error, warning, or information. + /// + /// + public DiagnosticSeverity GetDiagnosticSeverity() + { + return DiagnosticSeverity.Warning; + } + + /// + /// Retrieves the name of the module/assembly the rule is from. + /// + public override string GetSourceName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); + } + + /// + /// Retrieves the type of the rule, Builtin, Managed or Module. + /// + public override SourceType GetSourceType() + { + return SourceType.Builtin; + } + } +} + diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 2a04fd759..61c4f2b08 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -921,6 +921,18 @@ Line exceeds the configured maximum length of {0} characters + + AvoidSecretDisclosure + + + Avoid secret disclosure + + + Disclosing a secret might result in security vulnerabilities such as memory trails or logging trails + + + Avoid disclosing a secret + AvoidSemicolonsAsLineTerminators diff --git a/Tests/Rules/AvoidSecretDisclosure.tests.ps1 b/Tests/Rules/AvoidSecretDisclosure.tests.ps1 new file mode 100644 index 000000000..8be4621b9 --- /dev/null +++ b/Tests/Rules/AvoidSecretDisclosure.tests.ps1 @@ -0,0 +1,201 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +[Diagnostics.CodeAnalysis.SuppressMessage('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'False positive')] +param() + +BeforeAll { + $ruleName = "PSAvoidSecretDisclosure" + $ruleMessage = "Avoid disclosing a secret" +} + +Describe "AvoidSecretDisclosure" { + + # Secret disclosure examples we like to discourage: + # https://stackoverflow.com/questions/28352141/convert-a-secure-string-to-plain-text + # https://stackoverflow.com/questions/7468389/powershell-decode-system-security-securestring-to-readable-password + + Context "Violates" { + It "ConvertFrom-SecureString -AsPlainText" { + $scriptDefinition = { + $SecureString = ConvertTo-SecureString 'P@ssW0rd' -AsPlainText + $Null = $SecureString | ConvertFrom-SecureString -AsPlainText + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Warning + $violations.Extent.Text | Should -Be {ConvertFrom-SecureString -AsPlainText}.ToString() + $violations.Message | Should -Be $ruleMessage + $violations.RuleSuppressionID | Should -Be 'AsPlainText' + } + + It "SecureStringToBSTR()" { + $scriptDefinition = { + $SecureString = ConvertTo-SecureString 'P@ssW0rd' -AsPlainText + $BSTR = [System.Runtime.InteropServices.Marshal]::SecureStringToBSTR($SecureString) + $Password = [System.Runtime.InteropServices.Marshal]::PtrToStringAuto($BSTR) + [Runtime.InteropServices.Marshal]::ZeroFreeBSTR($BSTR) + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Warning + $violations.Extent.Text | Should -Be {[System.Runtime.InteropServices.Marshal]::SecureStringToBSTR($SecureString)}.ToString() + $violations.Message | Should -Be $ruleMessage + $violations.RuleSuppressionID | Should -Be 'SecureStringToBSTR' + } + + It "SecureStringToCoTaskMemUnicode()" { + $scriptDefinition = { + $password = ConvertTo-SecureString 'P@ssw0rd' -AsPlainText -Force + $Ptr = [System.Runtime.InteropServices.Marshal]::SecureStringToCoTaskMemUnicode($password) + $result = [System.Runtime.InteropServices.Marshal]::PtrToStringUni($Ptr) + [System.Runtime.InteropServices.Marshal]::ZeroFreeCoTaskMemUnicode($Ptr) + $result + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Warning + $violations.Extent.Text | Should -Be {[System.Runtime.InteropServices.Marshal]::SecureStringToCoTaskMemUnicode($password)}.ToString() + $violations.Message | Should -Be $ruleMessage + $violations.RuleSuppressionID | Should -Be 'SecureStringToCoTaskMemUnicode' + } + + It "GetNetworkCredential().Password" { + $scriptDefinition = { + $SecureString = ConvertTo-SecureString 'P@ssW0rd' -AsPlainText + $PSCredential = [PSCredential]::new(0, $SecureString) + $Credential = $PSCredential.GetNetworkCredential() + $Password = $Credential.Password + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Warning + $violations.Extent.Text | Should -Be {$Credential.Password}.ToString() + $violations.Message | Should -Be $ruleMessage + $violations.RuleSuppressionID | Should -Be 'Password' + } + + It "Custom password property" { + $scriptDefinition = { + $Cred = ConvertFrom-Json ' + { + "Account": { + "password": "Welcome123!", + "username": "JohnDoe" + } + }' + schtasks /change /s $env:COMPUTERNAME /tn $myTask /ru $Cred.username /rp $Cred.password + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Warning + $violations.Extent.Text | Should -Be {$Cred.password}.ToString() + $violations.Message | Should -Be $ruleMessage + $violations.RuleSuppressionID | Should -Be 'Password' + } + } + + Context "Compliant" { + It "Correct" { + $scriptDefinition = { + $credential = Get-Credential + $url = "https://server.contoso.com:8089/services/search/jobs/export" + $body = @{ + search = "search index=_internal | reverse | table index,host,source,sourceType,_raw" + output_mode = "csv" + earliest_time = "-2d@d" + latest_time = "-1d@d" + } + Invoke-RestMethod -Method 'Post' -Uri $url -Credential $credential -Body $body -OutFile output.csv + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + + It "Write-Host" { + $scriptDefinition = { + Write-Host AsPlainText SecureStringToBSTR Password + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + } + + Context "Suppressed" { + It "AsPlainText" { + $scriptDefinition = { + [Diagnostics.CodeAnalysis.SuppressMessage('PSAvoidSecretDisclosure', 'AsPlainText', Justification = 'Test')] + Param() + $SecureString = ConvertTo-SecureString 'P@ssW0rd' -AsPlainText + $Null = $SecureString | ConvertFrom-SecureString -AsPlainText + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + + It "SecureStringToBSTR" { + $scriptDefinition = { + [Diagnostics.CodeAnalysis.SuppressMessage('PSAvoidSecretDisclosure', 'SecureStringToBSTR', Justification = 'Test')] + Param() + $SecureString = ConvertTo-SecureString 'P@ssW0rd' -AsPlainText + $BSTR = [System.Runtime.InteropServices.Marshal]::SecureStringToBSTR($SecureString) + $Password = [System.Runtime.InteropServices.Marshal]::PtrToStringAuto($BSTR) + [Runtime.InteropServices.Marshal]::ZeroFreeBSTR($BSTR) + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + + It "SecureStringToCoTaskMemUnicode" { + $scriptDefinition = { + [Diagnostics.CodeAnalysis.SuppressMessage('PSAvoidSecretDisclosure', 'SecureStringToCoTaskMemUnicode', Justification = 'Test')] + Param() + $password = ConvertTo-SecureString 'P@ssw0rd' -AsPlainText -Force + $Ptr = [System.Runtime.InteropServices.Marshal]::SecureStringToCoTaskMemUnicode($password) + $result = [System.Runtime.InteropServices.Marshal]::PtrToStringUni($Ptr) + [System.Runtime.InteropServices.Marshal]::ZeroFreeCoTaskMemUnicode($Ptr) + $result + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + + It "Password" { + $scriptDefinition = { + [Diagnostics.CodeAnalysis.SuppressMessage('PSAvoidSecretDisclosure', 'Password', Justification = 'Test')] + Param() + $SecureString = ConvertTo-SecureString 'P@ssW0rd' -AsPlainText + $PSCredential = [PSCredential]::new(0, $SecureString) + $Password = $PSCredential.GetNetworkCredential().Password + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + + It "All" { + $scriptDefinition = { + [Diagnostics.CodeAnalysis.SuppressMessage('PSAvoidSecretDisclosure', '', Justification = 'Test')] + Param() + $SecureString = ConvertTo-SecureString 'P@ssW0rd' -AsPlainText + $Null = $SecureString | ConvertFrom-SecureString -AsPlainText + + $SecureString = ConvertTo-SecureString 'P@ssW0rd' -AsPlainText + $BSTR = [System.Runtime.InteropServices.Marshal]::SecureStringToBSTR($SecureString) + $Password = [System.Runtime.InteropServices.Marshal]::PtrToStringAuto($BSTR) + [Runtime.InteropServices.Marshal]::ZeroFreeBSTR($BSTR) + + $password = ConvertTo-SecureString 'P@ssw0rd' -AsPlainText -Force + $Ptr = [System.Runtime.InteropServices.Marshal]::SecureStringToCoTaskMemUnicode($password) + $result = [System.Runtime.InteropServices.Marshal]::PtrToStringUni($Ptr) + [System.Runtime.InteropServices.Marshal]::ZeroFreeCoTaskMemUnicode($Ptr) + $result + + $SecureString = ConvertTo-SecureString 'P@ssW0rd' -AsPlainText + $PSCredential = [PSCredential]::new(0, $SecureString) + $Password = $PSCredential.GetNetworkCredential().Password + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + } +} \ No newline at end of file diff --git a/docs/Rules/AvoidSecretDisclosure.md b/docs/Rules/AvoidSecretDisclosure.md new file mode 100644 index 000000000..d72400b58 --- /dev/null +++ b/docs/Rules/AvoidSecretDisclosure.md @@ -0,0 +1,67 @@ +--- +description: Avoid secret disclosure +ms.date: 05/03/2026 +ms.topic: reference +title: AvoidSecretDisclosure +--- +# AvoidSecretDisclosure + +**Severity Level: Warning** + +## Description + +Disclosing a secret might result in security vulnerabilities such as memory trails or logging trails that could +be exploited by attackers. This rule identifies instances where a secret is being converted to plain text, +which can lead to unintended exposure of sensitive information. + +> [!IMPORTANT] +> +> The general approach of dealing with credentials is to avoid them and instead rely on other means +> to authenticate, such as certificates or Windows authentication. + +## How to Fix + +In general, avoid any code pattern that involves converting secrets to plaintext or accessing plaintext secrets. + +* For `ConvertFrom-SecureString -AsPlainText`: Use `-Credential` parameter instead +* For `SecureStringTo*` methods: Avoid converting to plaintext +* For `Password` properties: Use secure credential objects directly or the SecureString equivalent + `SecurePassword` instead of accessing plaintext passwords. + +> [!NOTE] +> +> For custom properties named "Password", it is recommended to rename them to something that does not imply they +> contain secrets, or to ensure that they do not actually contain secrets. If renaming is not possible, consider +> suppressing the warning for those specific cases. + +## Example + +### Wrong + +```powershell +$credential = Get-Credential +$url = "https://server.contoso.com:8089/services/search/jobs/export" +$body = @{ + search = "search index=_internal | reverse | table index,host,source,sourceType,_raw" + output_mode = "csv" + earliest_time = "-2d@d" + latest_time = "-1d@d" + username = $credential.UserName + password = $credential.GetNetworkCredential().Password +} +Invoke-RestMethod -Method 'Post' -Uri $url -Body $body -OutFile output.csv +``` + +### Correct + +```powershell +$credential = Get-Credential +$url = "https://server.contoso.com:8089/services/search/jobs/export" +$body = @{ + search = "search index=_internal | reverse | table index,host,source,sourceType,_raw" + output_mode = "csv" + earliest_time = "-2d@d" + latest_time = "-1d@d" +} +Invoke-RestMethod -Method 'Post' -Uri $url -Credential $credential -Body $body -OutFile output.csv +``` diff --git a/docs/Rules/README.md b/docs/Rules/README.md index fca031e33..400ff1ed3 100644 --- a/docs/Rules/README.md +++ b/docs/Rules/README.md @@ -24,6 +24,7 @@ The PSScriptAnalyzer contains the following rule definitions. | [AvoidNullOrEmptyHelpMessageAttribute](./AvoidNullOrEmptyHelpMessageAttribute.md) | Warning | Yes | | | [AvoidOverwritingBuiltInCmdlets](./AvoidOverwritingBuiltInCmdlets.md) | Warning | Yes | Yes | | [AvoidReservedWordsAsFunctionNames](./AvoidReservedWordsAsFunctionNames.md) | Warning | Yes | | +| [AvoidSecretDisclosure](./AvoidSecretDisclosure.md) | Warning | Yes | Yes | | [AvoidSemicolonsAsLineTerminators](./AvoidSemicolonsAsLineTerminators.md) | Warning | No | | | [AvoidShouldContinueWithoutForce](./AvoidShouldContinueWithoutForce.md) | Warning | Yes | | | [AvoidTrailingWhitespace](./AvoidTrailingWhitespace.md) | Warning | Yes | | From 014cfbad92a300bedb5be4b6e2f7ee937d19e090 Mon Sep 17 00:00:00 2001 From: iRon7 Date: Wed, 6 May 2026 11:56:14 +0200 Subject: [PATCH 2/8] Implemented GitHub Copilot suggestions --- Rules/AvoidSecretDisclosure.cs | 11 +++--- Tests/Rules/AvoidSecretDisclosure.tests.ps1 | 39 +++++++++++++++++++++ docs/Rules/AvoidSecretDisclosure.md | 6 ++++ 3 files changed, 51 insertions(+), 5 deletions(-) diff --git a/Rules/AvoidSecretDisclosure.cs b/Rules/AvoidSecretDisclosure.cs index 2e64f2288..fb7eccf1c 100644 --- a/Rules/AvoidSecretDisclosure.cs +++ b/Rules/AvoidSecretDisclosure.cs @@ -55,9 +55,13 @@ testAst is CommandAst cmdAst && var bindingResult = StaticParameterBinder.BindCommand(cmdAst, true); // Check for -AsPlainText parameter + // The constantValue appears true even the value is a variable + // This is ok because the rule should still trigger in that case since the value of the + // variable could be true at runtime, and we want to catch all potential violations if ( bindingResult.BoundParameters.ContainsKey("AsPlainText") && - (bool)bindingResult.BoundParameters["AsPlainText"].ConstantValue == true + bindingResult.BoundParameters["AsPlainText"].ConstantValue is bool constantValue && + constantValue == true ) { yield return GetDiagnosticRecord(cmdAst.Extent, fileName, "AsPlainText"); } @@ -102,10 +106,7 @@ testAst is MemberExpressionAst memberAst && private DiagnosticRecord GetDiagnosticRecord(IScriptExtent Extent, string fileName, string suppressionId) { return new DiagnosticRecord( - string.Format( - CultureInfo.CurrentCulture, - Strings.AvoidSecretDisclosureError, - Extent.Text), + Strings.AvoidSecretDisclosureError, Extent, GetName(), DiagnosticSeverity.Warning, diff --git a/Tests/Rules/AvoidSecretDisclosure.tests.ps1 b/Tests/Rules/AvoidSecretDisclosure.tests.ps1 index 8be4621b9..3cb95bb2e 100644 --- a/Tests/Rules/AvoidSecretDisclosure.tests.ps1 +++ b/Tests/Rules/AvoidSecretDisclosure.tests.ps1 @@ -29,6 +29,19 @@ Describe "AvoidSecretDisclosure" { $violations.RuleSuppressionID | Should -Be 'AsPlainText' } + It "ConvertFrom-SecureString -AsPlainText:$true" { + $scriptDefinition = { + $SecureString = ConvertTo-SecureString 'P@ssW0rd' -AsPlainText + $Null = $SecureString | ConvertFrom-SecureString -AsPlainText:$true + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Warning + $violations.Extent.Text | Should -Be {ConvertFrom-SecureString -AsPlainText:$true}.ToString() + $violations.Message | Should -Be $ruleMessage + $violations.RuleSuppressionID | Should -Be 'AsPlainText' + } + It "SecureStringToBSTR()" { $scriptDefinition = { $SecureString = ConvertTo-SecureString 'P@ssW0rd' -AsPlainText @@ -198,4 +211,30 @@ Describe "AvoidSecretDisclosure" { $violations | Should -BeNullOrEmpty } } + + Context "should not crash" { + + It "-AsPlainText:$false" { + $scriptDefinition = { + $SecureString = ConvertTo-SecureString 'P@ssW0rd' -AsPlainText + $Null = $SecureString | ConvertFrom-SecureString -AsPlainText:$false + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + + It "-AsPlainText:$someVar" { + $scriptDefinition = { + param ([bool] $someVar) + $SecureString = ConvertTo-SecureString 'P@ssW0rd' -AsPlainText + $Null = $SecureString | ConvertFrom-SecureString -AsPlainText:$someVar + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Warning + $violations.Extent.Text | Should -Be {ConvertFrom-SecureString -AsPlainText:$someVar}.ToString() + $violations.Message | Should -Be $ruleMessage + $violations.RuleSuppressionID | Should -Be 'AsPlainText' + } + } } \ No newline at end of file diff --git a/docs/Rules/AvoidSecretDisclosure.md b/docs/Rules/AvoidSecretDisclosure.md index d72400b58..107fa9613 100644 --- a/docs/Rules/AvoidSecretDisclosure.md +++ b/docs/Rules/AvoidSecretDisclosure.md @@ -34,6 +34,12 @@ In general, avoid any code pattern that involves converting secrets to plaintext > contain secrets, or to ensure that they do not actually contain secrets. If renaming is not possible, consider > suppressing the warning for those specific cases. + ## Configuration + + ### Parameters + + * `Enable` - Enables or disables the rule. Default value is `True`. + ## Example ### Wrong From b2ed7579bb40649937f3eb11ea4c63b6cdb56944 Mon Sep 17 00:00:00 2001 From: iRon7 Date: Thu, 7 May 2026 15:51:19 +0200 Subject: [PATCH 3/8] Update docs/Rules/AvoidSecretDisclosure.md Co-authored-by: Sean Wheeler --- docs/Rules/AvoidSecretDisclosure.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/Rules/AvoidSecretDisclosure.md b/docs/Rules/AvoidSecretDisclosure.md index 107fa9613..71a38ea6f 100644 --- a/docs/Rules/AvoidSecretDisclosure.md +++ b/docs/Rules/AvoidSecretDisclosure.md @@ -10,9 +10,9 @@ title: AvoidSecretDisclosure ## Description -Disclosing a secret might result in security vulnerabilities such as memory trails or logging trails that could -be exploited by attackers. This rule identifies instances where a secret is being converted to plain text, -which can lead to unintended exposure of sensitive information. +Disclosing a secret might result in security vulnerabilities such as memory trails or logging trails +that could be exploited by attackers. This rule identifies instances where a secret is being +converted to plain text, which can lead to unintended exposure of sensitive information. > [!IMPORTANT] > From 83b4194cd1de1926bd09dc010555f2dfae89b944 Mon Sep 17 00:00:00 2001 From: iRon7 Date: Thu, 7 May 2026 15:51:41 +0200 Subject: [PATCH 4/8] Update docs/Rules/AvoidSecretDisclosure.md Co-authored-by: Sean Wheeler --- docs/Rules/AvoidSecretDisclosure.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/Rules/AvoidSecretDisclosure.md b/docs/Rules/AvoidSecretDisclosure.md index 71a38ea6f..dc5dddffb 100644 --- a/docs/Rules/AvoidSecretDisclosure.md +++ b/docs/Rules/AvoidSecretDisclosure.md @@ -15,7 +15,6 @@ that could be exploited by attackers. This rule identifies instances where a sec converted to plain text, which can lead to unintended exposure of sensitive information. > [!IMPORTANT] -> > The general approach of dealing with credentials is to avoid them and instead rely on other means > to authenticate, such as certificates or Windows authentication. From 45813adc3f4618ddc49d32f6c2855194594e8c22 Mon Sep 17 00:00:00 2001 From: iRon7 Date: Thu, 7 May 2026 15:51:57 +0200 Subject: [PATCH 5/8] Update docs/Rules/AvoidSecretDisclosure.md Co-authored-by: Sean Wheeler --- docs/Rules/AvoidSecretDisclosure.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/Rules/AvoidSecretDisclosure.md b/docs/Rules/AvoidSecretDisclosure.md index dc5dddffb..87ff1eba7 100644 --- a/docs/Rules/AvoidSecretDisclosure.md +++ b/docs/Rules/AvoidSecretDisclosure.md @@ -20,11 +20,12 @@ converted to plain text, which can lead to unintended exposure of sensitive info ## How to Fix -In general, avoid any code pattern that involves converting secrets to plaintext or accessing plaintext secrets. +In general, avoid any code pattern that involves converting secrets to plaintext or accessing +plaintext secrets. -* For `ConvertFrom-SecureString -AsPlainText`: Use `-Credential` parameter instead -* For `SecureStringTo*` methods: Avoid converting to plaintext -* For `Password` properties: Use secure credential objects directly or the SecureString equivalent +- For `ConvertFrom-SecureString -AsPlainText`: Use `-Credential` parameter instead +- For `SecureStringTo*` methods: Avoid converting to plaintext +- For `Password` properties: Use secure credential objects directly or the SecureString equivalent `SecurePassword` instead of accessing plaintext passwords. > [!NOTE] From 3473048854c26dd05450dde6d40839d697e28bfc Mon Sep 17 00:00:00 2001 From: iRon7 Date: Thu, 7 May 2026 15:52:21 +0200 Subject: [PATCH 6/8] Update docs/Rules/AvoidSecretDisclosure.md Co-authored-by: Sean Wheeler --- docs/Rules/AvoidSecretDisclosure.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/Rules/AvoidSecretDisclosure.md b/docs/Rules/AvoidSecretDisclosure.md index 87ff1eba7..159edc08b 100644 --- a/docs/Rules/AvoidSecretDisclosure.md +++ b/docs/Rules/AvoidSecretDisclosure.md @@ -29,10 +29,9 @@ plaintext secrets. `SecurePassword` instead of accessing plaintext passwords. > [!NOTE] -> -> For custom properties named "Password", it is recommended to rename them to something that does not imply they -> contain secrets, or to ensure that they do not actually contain secrets. If renaming is not possible, consider -> suppressing the warning for those specific cases. +> For custom properties named "Password", it is recommended to rename them to something that does +> not imply they contain secrets, or to ensure that they do not actually contain secrets. If +> renaming is not possible, consider suppressing the warning for those specific cases. ## Configuration From bc94e77ce5b58a54d0f118fc7fc26ae3127a0b34 Mon Sep 17 00:00:00 2001 From: iRon7 Date: Thu, 7 May 2026 15:53:00 +0200 Subject: [PATCH 7/8] Update docs/Rules/AvoidSecretDisclosure.md Co-authored-by: Sean Wheeler --- docs/Rules/AvoidSecretDisclosure.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/Rules/AvoidSecretDisclosure.md b/docs/Rules/AvoidSecretDisclosure.md index 159edc08b..652f717da 100644 --- a/docs/Rules/AvoidSecretDisclosure.md +++ b/docs/Rules/AvoidSecretDisclosure.md @@ -33,11 +33,11 @@ plaintext secrets. > not imply they contain secrets, or to ensure that they do not actually contain secrets. If > renaming is not possible, consider suppressing the warning for those specific cases. - ## Configuration +## Configuration - ### Parameters +### Parameters - * `Enable` - Enables or disables the rule. Default value is `True`. +- `Enable` - Enables or disables the rule. Default value is `$true`. ## Example From d62673cb9eaad6ee6ab7af9f8dbffa8cf09d0827 Mon Sep 17 00:00:00 2001 From: iRon7 Date: Thu, 7 May 2026 16:15:46 +0200 Subject: [PATCH 8/8] Disabled new rule by convention, as it is a breaking change. Will enable in a future release after giving users time to adjust. --- Rules/AvoidSecretDisclosure.cs | 2 +- Tests/Rules/AvoidSecretDisclosure.tests.ps1 | 56 +++++++++++++++------ docs/Rules/AvoidSecretDisclosure.md | 2 +- docs/Rules/README.md | 2 +- 4 files changed, 44 insertions(+), 18 deletions(-) diff --git a/Rules/AvoidSecretDisclosure.cs b/Rules/AvoidSecretDisclosure.cs index fb7eccf1c..6ac67b247 100644 --- a/Rules/AvoidSecretDisclosure.cs +++ b/Rules/AvoidSecretDisclosure.cs @@ -28,7 +28,7 @@ public class AvoidSecretDisclosure : ConfigurableRule /// Construct an object of AvoidSecretDisclosure type. /// public AvoidSecretDisclosure() { - Enable = true; + Enable = false; } /// diff --git a/Tests/Rules/AvoidSecretDisclosure.tests.ps1 b/Tests/Rules/AvoidSecretDisclosure.tests.ps1 index 3cb95bb2e..95d59c704 100644 --- a/Tests/Rules/AvoidSecretDisclosure.tests.ps1 +++ b/Tests/Rules/AvoidSecretDisclosure.tests.ps1 @@ -15,13 +15,20 @@ Describe "AvoidSecretDisclosure" { # https://stackoverflow.com/questions/28352141/convert-a-secure-string-to-plain-text # https://stackoverflow.com/questions/7468389/powershell-decode-system-security-securestring-to-readable-password + BeforeAll { + $Settings = @{ + IncludeRules = @($ruleName) + Rules = @{ $ruleName = @{ Enable = $true } } + } + } + Context "Violates" { It "ConvertFrom-SecureString -AsPlainText" { $scriptDefinition = { $SecureString = ConvertTo-SecureString 'P@ssW0rd' -AsPlainText $Null = $SecureString | ConvertFrom-SecureString -AsPlainText }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations.Count | Should -Be 1 $violations.Severity | Should -Be Warning $violations.Extent.Text | Should -Be {ConvertFrom-SecureString -AsPlainText}.ToString() @@ -34,7 +41,7 @@ Describe "AvoidSecretDisclosure" { $SecureString = ConvertTo-SecureString 'P@ssW0rd' -AsPlainText $Null = $SecureString | ConvertFrom-SecureString -AsPlainText:$true }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations.Count | Should -Be 1 $violations.Severity | Should -Be Warning $violations.Extent.Text | Should -Be {ConvertFrom-SecureString -AsPlainText:$true}.ToString() @@ -49,7 +56,7 @@ Describe "AvoidSecretDisclosure" { $Password = [System.Runtime.InteropServices.Marshal]::PtrToStringAuto($BSTR) [Runtime.InteropServices.Marshal]::ZeroFreeBSTR($BSTR) }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations.Count | Should -Be 1 $violations.Severity | Should -Be Warning $violations.Extent.Text | Should -Be {[System.Runtime.InteropServices.Marshal]::SecureStringToBSTR($SecureString)}.ToString() @@ -65,7 +72,7 @@ Describe "AvoidSecretDisclosure" { [System.Runtime.InteropServices.Marshal]::ZeroFreeCoTaskMemUnicode($Ptr) $result }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations.Count | Should -Be 1 $violations.Severity | Should -Be Warning $violations.Extent.Text | Should -Be {[System.Runtime.InteropServices.Marshal]::SecureStringToCoTaskMemUnicode($password)}.ToString() @@ -80,7 +87,7 @@ Describe "AvoidSecretDisclosure" { $Credential = $PSCredential.GetNetworkCredential() $Password = $Credential.Password }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations.Count | Should -Be 1 $violations.Severity | Should -Be Warning $violations.Extent.Text | Should -Be {$Credential.Password}.ToString() @@ -99,7 +106,7 @@ Describe "AvoidSecretDisclosure" { }' schtasks /change /s $env:COMPUTERNAME /tn $myTask /ru $Cred.username /rp $Cred.password }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations.Count | Should -Be 1 $violations.Severity | Should -Be Warning $violations.Extent.Text | Should -Be {$Cred.password}.ToString() @@ -121,7 +128,7 @@ Describe "AvoidSecretDisclosure" { } Invoke-RestMethod -Method 'Post' -Uri $url -Credential $credential -Body $body -OutFile output.csv }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations | Should -BeNullOrEmpty } @@ -129,7 +136,7 @@ Describe "AvoidSecretDisclosure" { $scriptDefinition = { Write-Host AsPlainText SecureStringToBSTR Password }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations | Should -BeNullOrEmpty } } @@ -142,7 +149,7 @@ Describe "AvoidSecretDisclosure" { $SecureString = ConvertTo-SecureString 'P@ssW0rd' -AsPlainText $Null = $SecureString | ConvertFrom-SecureString -AsPlainText }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations | Should -BeNullOrEmpty } @@ -155,7 +162,7 @@ Describe "AvoidSecretDisclosure" { $Password = [System.Runtime.InteropServices.Marshal]::PtrToStringAuto($BSTR) [Runtime.InteropServices.Marshal]::ZeroFreeBSTR($BSTR) }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations | Should -BeNullOrEmpty } @@ -169,7 +176,7 @@ Describe "AvoidSecretDisclosure" { [System.Runtime.InteropServices.Marshal]::ZeroFreeCoTaskMemUnicode($Ptr) $result }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations | Should -BeNullOrEmpty } @@ -181,7 +188,7 @@ Describe "AvoidSecretDisclosure" { $PSCredential = [PSCredential]::new(0, $SecureString) $Password = $PSCredential.GetNetworkCredential().Password }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations | Should -BeNullOrEmpty } @@ -207,7 +214,26 @@ Describe "AvoidSecretDisclosure" { $PSCredential = [PSCredential]::new(0, $SecureString) $Password = $PSCredential.GetNetworkCredential().Password }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations | Should -BeNullOrEmpty + } + } + + Context "Disabled" { + + BeforeAll { + $Settings = @{ + IncludeRules = @($ruleName) + Rules = @{ $ruleName = @{ Enable = $false } } + } + } + + It "ConvertFrom-SecureString -AsPlainText" { + $scriptDefinition = { + $SecureString = ConvertTo-SecureString 'P@ssW0rd' -AsPlainText + $Null = $SecureString | ConvertFrom-SecureString -AsPlainText + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations | Should -BeNullOrEmpty } } @@ -219,7 +245,7 @@ Describe "AvoidSecretDisclosure" { $SecureString = ConvertTo-SecureString 'P@ssW0rd' -AsPlainText $Null = $SecureString | ConvertFrom-SecureString -AsPlainText:$false }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations | Should -BeNullOrEmpty } @@ -229,7 +255,7 @@ Describe "AvoidSecretDisclosure" { $SecureString = ConvertTo-SecureString 'P@ssW0rd' -AsPlainText $Null = $SecureString | ConvertFrom-SecureString -AsPlainText:$someVar }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations.Count | Should -Be 1 $violations.Severity | Should -Be Warning $violations.Extent.Text | Should -Be {ConvertFrom-SecureString -AsPlainText:$someVar}.ToString() diff --git a/docs/Rules/AvoidSecretDisclosure.md b/docs/Rules/AvoidSecretDisclosure.md index 652f717da..21a967d4e 100644 --- a/docs/Rules/AvoidSecretDisclosure.md +++ b/docs/Rules/AvoidSecretDisclosure.md @@ -37,7 +37,7 @@ plaintext secrets. ### Parameters -- `Enable` - Enables or disables the rule. Default value is `$true`. +- `Enable` - Enables or disables the rule. Default value is `$false`. ## Example diff --git a/docs/Rules/README.md b/docs/Rules/README.md index 400ff1ed3..8f8a17c06 100644 --- a/docs/Rules/README.md +++ b/docs/Rules/README.md @@ -24,7 +24,7 @@ The PSScriptAnalyzer contains the following rule definitions. | [AvoidNullOrEmptyHelpMessageAttribute](./AvoidNullOrEmptyHelpMessageAttribute.md) | Warning | Yes | | | [AvoidOverwritingBuiltInCmdlets](./AvoidOverwritingBuiltInCmdlets.md) | Warning | Yes | Yes | | [AvoidReservedWordsAsFunctionNames](./AvoidReservedWordsAsFunctionNames.md) | Warning | Yes | | -| [AvoidSecretDisclosure](./AvoidSecretDisclosure.md) | Warning | Yes | Yes | +| [AvoidSecretDisclosure](./AvoidSecretDisclosure.md) | Warning | No | Yes | | [AvoidSemicolonsAsLineTerminators](./AvoidSemicolonsAsLineTerminators.md) | Warning | No | | | [AvoidShouldContinueWithoutForce](./AvoidShouldContinueWithoutForce.md) | Warning | Yes | | | [AvoidTrailingWhitespace](./AvoidTrailingWhitespace.md) | Warning | Yes | |