Prevent runspace races during analyzer command metadata access

Engine/CommandInfoCache.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
+using System.Collections.ObjectModel;
 using System.Management.Automation;
 using System.Linq;
 using System.Management.Automation.Runspaces;
@@ -209,6 +210,35 @@ private static bool IsGetCommandResolutionError(ErrorRecord errorRecord)
             return IsGetCommandResolutionException(errorRecord?.Exception);
         }
 
+        /// <summary>
+        /// Retrieves parameter metadata without allowing other threads to drive the command's runspace.
+        /// </summary>
+        public Dictionary<string, ParameterMetadata> GetCommandParameters(
+            string commandName, CommandTypes? commandTypes = null, bool bypassCache = false)
+        {
+            // Resolve the Lazy value before taking the lock: its factory may already be
+            // running on another thread that needs the same lock to finish the lookup.
+            var commandInfo = GetCommandInfo(commandName, commandTypes, bypassCache);
+            lock (_runspaceLock)
+            {
+                // Dynamic parameter getters execute PowerShell code and mutate runspace state,
+                // even though they look like ordinary property reads.
+                return disposed ? null : commandInfo?.Parameters;
+            }
+        }
+
+        /// <summary>
+        /// Retrieves parameter sets under the same lock as command lookups and dynamic parameter queries.
+        /// </summary>
+        public ReadOnlyCollection<CommandParameterSetInfo> GetCommandParameterSets(string commandName)
+        {
+            var commandInfo = GetCommandInfo(commandName);
+            lock (_runspaceLock)
+            {
+                return disposed ? null : commandInfo?.ParameterSets;
+            }
+        }
+
         private static bool IsGetCommandResolutionException(Exception exception)
         {
             if (exception is CommandNotFoundException)

Engine/Helper.cs

@@ -402,15 +402,15 @@ public HashSet<string> GetExportedFunction(Ast ast)
             IEnumerable<Ast> cmdAsts = ast.FindAll(item => item is CommandAst
                 && exportFunctionsCmdlet.Contains((item as CommandAst).GetCommandName(), StringComparer.OrdinalIgnoreCase), true);
 
-            CommandInfo exportMM = Helper.Instance.GetCommandInfo("export-modulemember", CommandTypes.Cmdlet);
-
-            // switch parameters
-            IEnumerable<ParameterMetadata> switchParams = (exportMM != null) ? exportMM.Parameters.Values.Where<ParameterMetadata>(pm => pm.SwitchParameter) : Enumerable.Empty<ParameterMetadata>();
-
-            if (exportMM == null)
+            // Export-ModuleMember has no dynamic parameters. Resolve names from its static
+            // metadata instead of ResolveParameter(), which re-enters the cached command's
+            // runspace and races with command lookups and metadata queries on other rule threads.
+            var parameters = GetCommandParameters("export-modulemember", CommandTypes.Cmdlet);
+            if (parameters == null)
             {
                 return exportedFunctions;
             }
+            IEnumerable<ParameterMetadata> switchParams = parameters.Values.Where(pm => pm.SwitchParameter);
 
             foreach (CommandAst cmdAst in cmdAsts)
             {
@@ -429,7 +429,20 @@ public HashSet<string> GetExportedFunction(Ast ast)
                     if (ceAst is CommandParameterAst)
                     {
                         var paramAst = ceAst as CommandParameterAst;
-                        var param = exportMM.ResolveParameter(paramAst.ParameterName);
+                        ParameterMetadata param;
+                        if (!parameters.TryGetValue(paramAst.ParameterName, out param))
+                        {
+                            param = parameters.Values.FirstOrDefault(pm =>
+                                pm.Aliases.Contains(paramAst.ParameterName, StringComparer.OrdinalIgnoreCase));
+                            if (param == null)
+                            {
+                                var matches = parameters.Values.Where(pm =>
+                                    pm.Name.StartsWith(paramAst.ParameterName, StringComparison.OrdinalIgnoreCase)
+                                    || pm.Aliases.Any(alias => alias.StartsWith(paramAst.ParameterName, StringComparison.OrdinalIgnoreCase)))
+                                    .Take(2).ToArray();
+                                param = matches.Length == 1 ? matches[0] : null;
+                            }
+                        }
 
                         if (param == null)
                         {
@@ -671,6 +684,23 @@ public CommandInfo GetCommandInfo(string name, CommandTypes? commandType = null,
             return CommandInfoCache.GetCommandInfo(name, commandTypes: commandType, bypassCache: bypassCache);
         }
 
+        /// <summary>
+        /// Retrieves command parameters while serializing access to the cached command's runspace.
+        /// </summary>
+        public Dictionary<string, ParameterMetadata> GetCommandParameters(
+            string name, CommandTypes? commandType = null, bool bypassCache = false)
+        {
+            return CommandInfoCache.GetCommandParameters(name, commandType, bypassCache);
+        }
+
+        /// <summary>
+        /// Retrieves command parameter sets while serializing access to the cached command's runspace.
+        /// </summary>
+        public ReadOnlyCollection<CommandParameterSetInfo> GetCommandParameterSets(string name)
+        {
+            return CommandInfoCache.GetCommandParameterSets(name);
+        }
+
         /// <summary>
         /// Returns the get, set and test targetresource dsc function
         /// </summary>

Rules/UseCmdletCorrectly.cs

@@ -148,8 +148,8 @@ private bool MandatoryParameterExists(CommandAst cmdAst)
             var mandatoryParameters = new List<ParameterMetadata>();
             try
             {
-                int noOfParamSets = cmdInfo.ParameterSets.Count;
-                foreach (ParameterMetadata pm in cmdInfo.Parameters.Values)
+                int noOfParamSets = Helper.Instance.GetCommandParameterSets(cmdAst.GetCommandName()).Count;
+                foreach (ParameterMetadata pm in Helper.Instance.GetCommandParameters(cmdAst.GetCommandName()).Values)
                 {
                     int count = 0;
 
@@ -256,4 +256,3 @@ public string GetSourceName()
 
 
 
-

Rules/UseCorrectCasing.cs

@@ -123,7 +123,7 @@ public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string file
                     Dictionary<string, ParameterMetadata> availableParameters;
                     try
                     {
-                        availableParameters = commandInfo.Parameters;
+                        availableParameters = Helper.Instance.GetCommandParameters(commandName);
                     }
                     // It's a known issue that objects from PowerShell can have a runspace affinity,
                     // therefore if that happens, we query a fresh object instead of using the cache.
@@ -176,7 +176,7 @@ private Dictionary<string, ParameterMetadata> GetParametersFromFreshCommandInfo(
         {
             try
             {
-                return Helper.Instance.GetCommandInfo(commandName, bypassCache: true)?.Parameters;
+                return Helper.Instance.GetCommandParameters(commandName, bypassCache: true);
             }
             catch (Exception exception) when (exception is InvalidOperationException || exception is NullReferenceException)
             {

Tests/Engine/CommandInfoCacheConcurrency.tests.ps1

@@ -12,12 +12,48 @@ Describe "Concurrent command lookups" {
         # threads. Invoking a PowerShell script block on a thread pool thread would introduce
         # runspace affinity problems of its own and would not test the command info cache.
         $analyzerAssembly = [Microsoft.Windows.PowerShell.ScriptAnalyzer.Helper].Assembly.Location
-        Add-Type -IgnoreWarnings -WarningAction SilentlyContinue -ReferencedAssemblies $analyzerAssembly, ([System.Management.Automation.PSObject].Assembly.Location) -TypeDefinition @'
+        $references = @($analyzerAssembly, ([System.Management.Automation.PSObject].Assembly.Location))
+        if ($PSVersionTable.PSEdition -eq 'Core') {
+            $references += Join-Path $PSHOME 'ref/System.Collections.dll'
+        }
+        Add-Type -IgnoreWarnings -WarningAction SilentlyContinue -ReferencedAssemblies $references -TypeDefinition @'
 using System.Threading.Tasks;
+using System.Management.Automation.Language;
 using Microsoft.Windows.PowerShell.ScriptAnalyzer;
 
 public static class ConcurrentCommandLookup
 {
+    public static void ResolveExports()
+    {
+        Token[] tokens;
+        ParseError[] errors;
+        var ast = Parser.ParseInput("Export-ModuleMember -Function Test-Example", out tokens, out errors);
+        var helper = Helper.Instance;
+        var tasks = new Task[8];
+        for (int i = 0; i < tasks.Length; i++)
+        {
+            tasks[i] = Task.Run(() =>
+            {
+                for (int j = 0; j < 100; j++)
+                {
+                    helper.GetCommandInfo("Get-Command", bypassCache: true);
+                    var parameters = helper.GetCommandParameters("Get-Item");
+                    if (!parameters.ContainsKey("Path") || helper.GetCommandParameterSets("Get-Item").Count == 0)
+                    {
+                        throw new System.InvalidOperationException("Command parameter metadata was not resolved.");
+                    }
+                    var exports = helper.GetExportedFunction(ast);
+                    if (!exports.SetEquals(new[] { "Test-Example" }))
+                    {
+                        throw new System.InvalidOperationException("Exported function was not resolved.");
+                    }
+                }
+            });
+        }
+
+        Task.WaitAll(tasks);
+    }
+
     public static string[] Lookup(string[] commandNames)
     {
         var helper = Helper.Instance;
@@ -60,5 +96,10 @@ public static class ConcurrentCommandLookup
         for ($i = 0; $i -lt $commandNames.Count; $i++) {
             $results[$i] | Should -BeExactly $commandNames[$i]
         }
+
+    }
+
+    It "resolves exported functions while command lookups run concurrently" {
+        [ConcurrentCommandLookup]::ResolveExports()
     }
 }

Tests/Engine/Helper.tests.ps1

@@ -39,3 +39,28 @@ Describe "Test Directed Graph" {
         }
     }
 }
+
+Describe "Exported function parameter resolution" {
+    BeforeAll {
+        $null = Invoke-ScriptAnalyzer -ScriptDefinition 'Get-Item -Path .'
+    }
+
+    It "resolves exports from <Script>" -TestCases @(
+        @{ Script = 'Export-ModuleMember -Function Test-Example' }
+        @{ Script = 'Export-ModuleMember -fUnCtIoN Test-Example' }
+        @{ Script = 'Export-ModuleMember -Fun Test-Example' }
+        @{ Script = 'Export-ModuleMember -Function:Test-Example' }
+        @{ Script = 'Export-ModuleMember Test-Example' }
+        @{ Script = 'Export-ModuleMember -Verbose Test-Example' }
+        @{ Script = 'Export-ModuleMember -vb Test-Example' }
+        @{ Script = 'Export-ModuleMember -Alias example -Function Test-Example' }
+        @{ Script = 'Export-ModuleMember -ea Stop -Fun Test-Example' }
+    ) {
+        param($Script)
+
+        $ast = [System.Management.Automation.Language.Parser]::ParseInput($Script, [ref]$null, [ref]$null)
+        $exports = [Microsoft.Windows.PowerShell.ScriptAnalyzer.Helper]::Instance.GetExportedFunction($ast)
+        $exports.Count | Should -Be 1
+        $exports | Should -Contain 'Test-Example'
+    }
+}