GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Add new AvoidSecretDisclosure rule by iRon7 · Pull Request #2183 · PowerShell/PSScriptAnalyzer

Conversation

iRon7 commented May 6, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

PR Summary

Closes: #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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Add new AvoidSecretDisclosure rule

586302f

Copilot AI review requested due to automatic review settings

May 6, 2026 09:05

iRon7 requested review from a team, Christoph Bergmeister (bergmeister), Mikey Lombardi (He/Him) (michaeltlombardi) and Sean Wheeler (sdwheeler) as code owners

May 6, 2026 09:05

Copilot started reviewing on behalf of iRon7

May 6, 2026 09:06

View session


          Merge branch 'main' into AvoidSecureStringDisclosure

9b67e56

Copilot AI reviewed

May 6, 2026

View reviewed changes

Copilot AI left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Rules/AvoidSecretDisclosure.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Rules/AvoidSecretDisclosure.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

docs/Rules/AvoidSecretDisclosure.md Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Tests/Rules/AvoidSecretDisclosure.tests.ps1 Resolved

Uh oh!

There was an error while loading. Please reload this page.


          Implemented GitHub Copilot suggestions

014cfba

iRon7 requested a review from Copilot

May 6, 2026 10:48

Copilot started reviewing on behalf of iRon7

May 6, 2026 10:49

View session

Copilot AI reviewed

May 6, 2026

Copilot AI left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

iRon7 mentioned this pull request

May 6, 2026

Add new AvoidDynamicallyCreatingVariableNames rule #2178

Merged

6 tasks done

Sean Wheeler (sdwheeler) suggested changes

May 6, 2026

View reviewed changes

Sean Wheeler (sdwheeler) left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added suggestions for style and one question about the parameter.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

docs/Rules/AvoidSecretDisclosure.md Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

docs/Rules/AvoidSecretDisclosure.md Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

docs/Rules/AvoidSecretDisclosure.md Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

docs/Rules/AvoidSecretDisclosure.md Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

docs/Rules/AvoidSecretDisclosure.md Resolved

Uh oh!

There was an error while loading. Please reload this page.

iRon7 and others added 6 commits

May 7, 2026 15:51


          Update docs/Rules/AvoidSecretDisclosure.md

b2ed757

Co-authored-by: Sean Wheeler <[email protected]>


          Update docs/Rules/AvoidSecretDisclosure.md

83b4194

Co-authored-by: Sean Wheeler <[email protected]>


          Update docs/Rules/AvoidSecretDisclosure.md

45813ad

Co-authored-by: Sean Wheeler <[email protected]>


          Update docs/Rules/AvoidSecretDisclosure.md

Co-authored-by: Sean Wheeler <[email protected]>


          Update docs/Rules/AvoidSecretDisclosure.md

bc94e77

Co-authored-by: Sean Wheeler <[email protected]>


          Disabled new rule by convention, as it is a breaking change. Will ena…

d62673c

…ble in a future release after giving users time to adjust.

Christoph Bergmeister (bergmeister) reviewed

May 7, 2026

View reviewed changes

Christoph Bergmeister (bergmeister) left a comment

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

iRon7 commented May 8, 2026

Copy link

Copy Markdown

Contributor Author

Christoph Bergmeister (@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...

andyleejordan reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          Windows PowerShell doesn't have a `ConvertFrom-SecureString -AsPlainT…

635c041

…ext`

Sean Wheeler (sdwheeler) approved these changes

May 15, 2026

View reviewed changes

Sean Wheeler (sdwheeler) left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs look good.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Merge branch 'main' into AvoidSecureStringDisclosure

fc60573

iRon7 commented Jun 10, 2026

Copy link

Copy Markdown

Contributor Author

Since about 3 weeks ago, I have created 6 PR for new rules (the last one was from yesterday: #2186).
From the first 5, 4 PRs have been merged in the main branch by Andy Jordan (@andyleejordan) except for this one.
I am not sure what the reason is that this one is stalled, but I guess that we are waiting for Christoph Christoph Bergmeister (@bergmeister) to react and taking my arguments on his earlier comments in consideration.
Anyways, let me know if there is anything expected from my side to further process this one.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          Merge branch 'main' into AvoidSecureStringDisclosure

31f0bd3

Christoph Bergmeister (bergmeister) requested a balanced review from Copilot

September 14, 2026 19:58

Copilot started reviewing on behalf of Christoph Bergmeister (bergmeister)

September 14, 2026 19:59

View session

Copilot AI reviewed

Sep 14, 2026

View reviewed changes

Copilot AI left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The rule currently misses valid disclosure syntax, reports a secure PSCredential.Password access, and provides misleading remediation guidance.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

Rules/AvoidSecretDisclosure.cs:76

A quoted member name such as [Marshal]::'SecureStringToBSTR'(...) invokes the same conversion, but matching the member's source text can retain the quotes and fail this prefix check. Match StringConstantExpressionAst.Value instead and use that normalized value for the suppression ID.

                testAst is InvokeMemberExpressionAst invokeAst &&
                invokeAst.Member != null &&
                invokeAst.Member.ToString().StartsWith("SecureStringTo", StringComparison.OrdinalIgnoreCase),

Rules/AvoidSecretDisclosure.cs:94

This name-only check also warns on [PSCredential].Password, whose value is already a SecureString; reading it does not disclose plaintext and is a valid input to APIs that accept SecureString. Exclude properties that can be resolved to SecureString (at least statically typed PSCredential instances), while retaining the heuristic only where the property type is unknown or plaintext.

            IEnumerable<MemberExpressionAst> passwordAsts = ast.FindAll(testAst =>
                testAst is MemberExpressionAst memberAst &&
                memberAst.Member != null &&
                string.Equals(memberAst.Member.ToString(), "Password", StringComparison.OrdinalIgnoreCase),

Files reviewed: 5/5 changed files
Comments generated: 2
Review effort level: Balanced

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Rules/AvoidSecretDisclosure.cs

Comment on lines +48 to +49

		cmdAst.GetCommandName() != null &&
		cmdAst.GetCommandName().Equals("ConvertFrom-SecureString", StringComparison.OrdinalIgnoreCase),

Christoph Bergmeister (bergmeister) Sep 14, 2026

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whilst it would be good to also cater for module qualified, current approach is good enough IMHO

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

iRon7 reacted with thumbs up emoji

All reactions

👍 1 reaction

Comment thread

docs/Rules/AvoidSecretDisclosure.md

		In general, avoid any code pattern that involves converting secrets to plaintext or accessing
		plaintext secrets.

		- For `ConvertFrom-SecureString -AsPlainText`: Use `-Credential` parameter instead

Christoph Bergmeister (bergmeister) Sep 14, 2026

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iRon7 maybe I am reading this with not enough context but what do you mean by -Credential parameter?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Merge branch 'main' into AvoidSecureStringDisclosure

5d8b9e0

Christoph Bergmeister (bergmeister) approved these changes

Sep 14, 2026

View reviewed changes

Christoph Bergmeister (bergmeister) left a comment

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

happy with it, first copilot suggestion I see as optional improvement. Just a reply please on last one to clarify what you mean with -Credential param

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

iRon7 reacted with thumbs up emoji

All reactions

👍 1 reaction

iRon7 commented Sep 15, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor Author

Christoph Bergmeister (@bergmeister),

Thanks you for your approvals

Just a reply please on last one to clarify what you mean with -Credential param

I agree, this is indeed a big jump into a "solution", what I meant is exactly how copilot expresses it:

For ConvertFrom-SecureString -AsPlainText: Keep the secret in a credential object and pass that object to the target command's -Credential parameter when supported

(See also the example)

Which, I am aware, isn't often available, leaving just the general approach of dealing with credentials (avoid them). Also knowing that this is in most an issue due to the capabilities or configuration of a third party application and not something that can be securely fixed in the concerned PowerShell script.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Reviewers

Copilot code review Copilot

Copilot review effort

Applies to this pull request for everyone.Learn more about Copilot code review.

Copilot left review comments

Christoph Bergmeister (bergmeister)

bergmeister approved these changes

Mikey Lombardi (He/Him) (michaeltlombardi)

Awaiting requested review from michaeltlombardi

michaeltlombardi is a code owner

+1 more reviewer

Sean Wheeler (sdwheeler)

sdwheeler approved these changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Rule request: AvoidSecureStringDisclosure

Uh oh!

There was an error while loading. Please reload this page.

4 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

iRon7 commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Description

How to Fix

PR Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Sean Wheeler (sdwheeler) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Christoph Bergmeister (bergmeister) left a comment

Choose a reason for hiding this comment

Uh oh!

iRon7 commented May 8, 2026

Uh oh!

Sean Wheeler (sdwheeler) left a comment

Choose a reason for hiding this comment

Uh oh!

iRon7 commented Jun 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (2)

Uh oh!

Christoph Bergmeister (bergmeister) Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

Christoph Bergmeister (bergmeister) Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

Christoph Bergmeister (bergmeister) left a comment

Choose a reason for hiding this comment

Uh oh!

iRon7 commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

iRon7 commented May 6, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

iRon7 commented Sep 15, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.