Merge pull request #442 from PowerShell/FixAvoidUserNameAndPasswordParam

Fix a bug where avoidusernameandpasswordparam is too noisy

Author: Quoc Truong

Rules/AvoidUserNameAndPasswordParams.cs

@@ -53,24 +53,31 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
                 // Iterates all ParamAsts and check if their names are on the list.
                 foreach (ParameterAst paramAst in paramAsts)
                 {
+                    // this will be null if there is no [pscredential] attached to the parameter
                     var psCredentialType = paramAst.Attributes.FirstOrDefault(paramAttribute =>
                         (paramAttribute.TypeName.IsArray && (paramAttribute.TypeName as ArrayTypeName).ElementType.GetReflectionType() == typeof(PSCredential))
                         || paramAttribute.TypeName.GetReflectionType() == typeof(PSCredential));
 
+                    // this will be null if there are no [credential()] attribute attached
                     var credentialAttribute = paramAst.Attributes.FirstOrDefault(paramAttribute => paramAttribute.TypeName.GetReflectionType() == typeof(CredentialAttribute));
 
-                    String paramName = paramAst.Name.VariablePath.ToString();
+                    // this will be null if there are no [securestring] attached to the parameter
+                    var secureStringType = paramAst.Attributes.FirstOrDefault(paramAttribute =>
+                        (paramAttribute.TypeName.IsArray && (paramAttribute.TypeName as ArrayTypeName).ElementType.GetReflectionType() == typeof (System.Security.SecureString))
+                        || paramAttribute.TypeName.GetReflectionType() == typeof(System.Security.SecureString));
 
-                    // if this is pscredential type with credential attribute where pscredential type comes first
-                    if (psCredentialType != null && credentialAttribute != null && psCredentialType.Extent.EndOffset < credentialAttribute.Extent.StartOffset)
-                    {
-                        continue;
-                    }
+                    String paramName = paramAst.Name.VariablePath.ToString();                    
 
                     foreach (String password in passwords)
                     {
                         if (paramName.IndexOf(password, StringComparison.OrdinalIgnoreCase) != -1)
                         {
+                            // if this is a secure string, pscredential or credential attribute, don't count
+                            if (secureStringType != null || credentialAttribute != null || psCredentialType != null)
+                            {
+                                continue;
+                            }
+
                             hasPwd = true;
                             break;
                         }

Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1

@@ -1,15 +1,15 @@
 Import-Module PSScriptAnalyzer
 
-$violationMessage = "Function 'Verb-Noun' has both username and password parameters. A credential parameter of type PSCredential with a CredentialAttribute where PSCredential comes before CredentialAttribute should be used."
+$violationMessage = "Function 'TestFunction' has both username and password parameters. A credential parameter of type PSCredential with a CredentialAttribute where PSCredential comes before CredentialAttribute should be used."
 $violationName = "PSAvoidUsingUserNameAndPasswordParams"
 $directory = Split-Path -Parent $MyInvocation.MyCommand.Path
 $violations = Invoke-ScriptAnalyzer $directory\AvoidUserNameAndPasswordParams.ps1 | Where-Object {$_.RuleName -eq $violationName}
 $noViolations = Invoke-ScriptAnalyzer $directory\AvoidUserNameAndPasswordParamsNoViolations.ps1 | Where-Object {$_.RuleName -eq $violationName}
 
 Describe "AvoidUserNameAndPasswordParams" {
     Context "When there are violations" {
-        It "has 3 avoid username and password parameter violations" {
-            $violations.Count | Should Be 3
+        It "has 1 avoid username and password parameter violations" {
+            $violations.Count | Should Be 1
         }
 
         It "has the correct violation message" {