Add new AvoidSecretDisclosure rule

[iRon7]

PR Summary

Rule request: #1997

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.

PR Checklist

• PR has a meaningful title
  • Use the present tense and imperative mood when describing your changes
• Summarized changes
• Change is not breaking
• Make sure all .cs, .ps1 and .psm1 files have the correct copyright header
• Make sure you've added a new test if existing tests do not effectively test the code changed and/or updated documentation
• This PR is ready to merge and is not Work in Progress.
  • If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready.

[Copilot]

Pull request overview

Adds a new built-in PSScriptAnalyzer rule (PSAvoidSecretDisclosure) to flag common patterns that convert secrets (e.g., SecureString) into plaintext, with accompanying tests, localized strings, and documentation.

Changes:

• Introduces AvoidSecretDisclosure rule implementation to detect ConvertFrom-SecureString -AsPlainText, SecureStringTo* calls, and .Password member access.
• Adds Pester tests covering violations, compliant examples, and suppression scenarios.
• Updates rule documentation and the rules index, and adds localized strings for name/common name/description/error.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
Tests/Rules/AvoidSecretDisclosure.tests.ps1	Adds Pester coverage for the new rule (violations/compliance/suppression).
Rules/Strings.resx	Adds localized strings for the new rule’s name/common name/description/message.
Rules/AvoidSecretDisclosure.cs	Implements the new analyzer rule logic and diagnostic creation.
docs/Rules/README.md	Registers the rule in the published rules list/table.
docs/Rules/AvoidSecretDisclosure.md	Adds the rule’s public documentation page and examples.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[sdwheeler]

Added suggestions for style and one question about the parameter.

[bergmeister]

There is already a AvoidUsingConvertToSecureStringWithPlainText therefore if we were ti have a general rule to alert on things like SecureString methods, then the old rule logic should be merged into this one and there should be configuration on what to alert for. For example on Linux, Credential does not apply. And on Linux SecureString is not implemented and since .NET 5, Securestring was marked as obsolete and not recommended any more as it is understood that practically it is and was never more secure in first place.
Since it's an optional rule, we can be opinionated but in practice many warnings will not be fixable and we found that this can be a source of frustration for community.

[iRon7]

@bergmeister,

The difference with the existing AvoidUsingConvertToSecureStringWithPlainText rule is that this rule warns for a convert from plain text in contrast to this new AvoidSecretDisclosure rule which warns for converting a secret to plain text.
Imho, for the first rule there are a few exceptions to put something in a SecureString (e.g. Read-Host -AsSecureString), for the new AvoidSecretDisclosure rule, any action that requires disclosing a secret (SecureString) in a script is questionable from a security standpoint.

Since it's an optional rule, we can be opinionated but in practice many warnings will not be fixable and we found that this can be a source of frustration for community.

Agree, this rule should be disabled by default for continuous integration usage and enabled manually as it will certainly reveal a lot of security violations in the currently installed bases...