Description

Alek Davis (alekdavis)

opened on Aug 6, 2020

Issue body actions

I have a script parameter that holds a path to the encrypted credential file named $CredentialFile for which I get the warning:

Parameter '$CredentialFile' should use SecureString, otherwise this will expose sensitive information. See ConvertTo-SecureString for more information.

There is no reason for me use SecureString here and I cannot think of a better alternative to the parameter name, so it would be nice to suppress this message for this particular instance, but not to the whole file or project. Besides, the documentation for PSAvoidUsingPlainTextForPassword needs to include Credential and whatever other strings it gets triggered of, since it only mentions one word: Password.

Activity

1. ghost added

   Needs: Triage 🔍

   on Aug 6, 2020
2. rjmholt commented on Aug 6, 2020

   

   Rob Holt (rjmholt)

   on Aug 6, 2020

   Contributor

   More actions

   Addressing your title, see https://github.com/PowerShell/PSScriptAnalyzer#suppressing-rules.

   Besides, the documentation for PSAvoidUsingPlainTextForPassword needs to include Credential and whatever other strings it gets triggered of, since it only mentions one word: Password.

   Agreed.

   It seems the list of terms is:

   PSScriptAnalyzer/Rules/AvoidUsingPlainTextForPassword.cs

   Line 34 in c78f5a6

   List<String> passwords = new List<String>() {"Password", "Passphrase", "Cred", "Credential"};

   And those terms only need to appear within the parameter/variable name to trigger the rule:

   PSScriptAnalyzer/Rules/AvoidUsingPlainTextForPassword.cs

   Line 45 in c78f5a6

   if (paramName.IndexOf(password, StringComparison.OrdinalIgnoreCase) != -1)

   We would welcome your PR to update the docs here

   Reacted by Alek Davis
3. alekdavis commented on Aug 6, 2020

   

   Alek Davis (alekdavis)

   on Aug 6, 2020

   Author

   More actions

   Oh, sweet. I did not realize that SuppressMessageAttribute must be applied to params() (originally tried it next to the parameter itself and it did not help). It's perfect when applied correctly. Thanks for your help.
4. Alek Davis (alekdavis)

   closed this as completedon Aug 6, 2020
5. Rob Holt (rjmholt)

   changed the title [-]No way to turn AvoidUsingPlainTextForPassword off[/-] [+]AvoidUsingPlainTextForPassword docs do not cover all checked parameter names[/+] on Aug 6, 2020
6. rjmholt commented on Aug 6, 2020

   

   Rob Holt (rjmholt)

   on Aug 6, 2020

   Contributor

   More actions

   Reopening to track doc issue
7. Rob Holt (rjmholt)

   reopened this on Aug 6, 2020
8. Sydney Smith (SydneyhSmith)

   added

   Area - Documentation

   Area - Rules

   Up-for-Grabs

   and removed

   Needs: Triage 🔍

   on Aug 11, 2020
9. CptMikhailov commented on Aug 25, 2021

   

   David Host (CptMikhailov)

   on Aug 25, 2021

   More actions

   Is there any movement on this? I'm encountering an issue where a parameter called $CreditUnion is being incorrectly flagged. I think that the way this is implemented should be less broad, possibly. I can look at submitting a pull request after I've had a look.

   Does anyone have any suggestions for how this should be reimplemented? I'm thinking maybe a more robust set of regular expression rules? Or just having a more robust list of terms, and testing for equality instead of pattern matching.
10. rjmholt commented on Aug 25, 2021

    

    Rob Holt (rjmholt)

    on Aug 25, 2021

    Contributor

    More actions

    Is there any movement on this?

    Not yet; we would welcome your contribution.

    Does anyone have any suggestions for how this should be reimplemented?

    Ultimately this is a very heuristic rule and the idea is we can infer what should be a secure string based on the name of parameters. But most users seem to appreciate that. I think a regex is probably the way to go. Something like:

    ([Cc]redential|[Pp]ass(word|phrase)|[Tt]oken)$

Issue actions