Making copilot suggested edits

joshcorr · joshcorr · commit 17f1a241fd73 · 2026-03-17T10:35:06.000-07:00
diff --git a/Rules/Strings.resx b/Rules/Strings.resx
@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8"?>
+﻿<?xml version="1.0" encoding="utf-8"?>
 <root>
   <!-- 
     Microsoft ResX Schema 
@@ -1282,6 +1282,6 @@
     <value>Module manifest field '{0}' uses wildcard ('*') which is not recommended for Constrained Language Mode. Explicitly list exported items instead.</value>
   </data>
   <data name="UseConstrainedLanguageModeScriptModuleError" xml:space="preserve">
-    <value>Module manifest field '{0}' contains script file '{1}' (.ps1). Use binary modules (.psm1 or .dll) instead for Constrained Language Mode compatibility.</value>
+    <value>Module manifest field '{0}' contains script file '{1}' (.ps1). Use a module file (.psm1) or a binary module (.dll) instead for Constrained Language Mode compatibility.</value>
   </data>
 </root>
diff --git a/Rules/UseConstrainedLanguageMode.cs b/Rules/UseConstrainedLanguageMode.cs
@@ -63,10 +63,20 @@ public class UseConstrainedLanguageMode : ConfigurableRule
         };
 
         /// <summary>
-        /// When true, ignores script signatures and runs all CLM checks regardless of signature status.
-        /// When false (default), scripts with valid signatures are treated as having elevated permissions
-        /// and only critical checks (dot-sourcing, parameter types, manifests) are performed.
-        /// Set to true to enforce full CLM compliance even for signed scripts.
+        /// Cache for typed variable assignments per scope to avoid O(N*M) performance issues.
+        /// Key: Scope AST (FunctionDefinitionAst or ScriptBlockAst)
+        /// Value: Dictionary mapping variable names to their type names
+        /// </summary>
+        private Dictionary<Ast, Dictionary<string, string>> _typedVariableCache;
+
+        /// <summary>
+        /// When True, ignores the presence of script signature blocks and runs all CLM checks
+        /// regardless of whether a script appears to be signed.
+        /// When False (default), scripts that contain a PowerShell signature block (for example,
+        /// one starting with '# SIG # Begin signature block') are treated as having elevated
+        /// permissions for this rule and only critical checks (dot-sourcing, parameter types,
+        /// manifests) are performed. No cryptographic validation or trust evaluation of the
+        /// signature is performed.
         /// </summary>
         [ConfigurableRuleProperty(defaultValue: false)]
         public bool IgnoreSignatures { get; set; }
@@ -93,18 +103,15 @@ private bool IsTypeAllowed(string typeName)
             // Handle array types (e.g., string[], System.String[], int[][])
             // Strip array brackets and check the base type
             string baseTypeName = typeName;
-            if (typeName.EndsWith("[]"))
-            {
-                // Remove all trailing [] pairs
-                baseTypeName = typeName.TrimEnd('[', ']');
+           
                 
-                // Handle multi-dimensional or jagged arrays by removing all brackets
-                while (baseTypeName.EndsWith("[]"))
-                {
-                    baseTypeName = baseTypeName.Substring(0, baseTypeName.Length - 2);
-                }
+            // Handle multi-dimensional or jagged arrays by removing all brackets
+            while (baseTypeName.EndsWith("[]", StringComparison.Ordinal))
+            {
+                baseTypeName = baseTypeName.Substring(0, baseTypeName.Length - 2);
             }
 
+
             // Check exact match first
             if (AllowedTypes.Contains(baseTypeName))
             {
@@ -134,6 +141,9 @@ public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string file
                 throw new ArgumentNullException(nameof(ast));
             }
 
+            // Initialize cache for this analysis to avoid O(N*M) performance issues
+            _typedVariableCache = new Dictionary<Ast, Dictionary<string, string>>();
+
             var diagnosticRecords = new List<DiagnosticRecord>();
 
             // Check if the file is signed (via signature block detection)
@@ -675,7 +685,52 @@ private ParameterAst FindParameterForVariable(VariableExpressionAst varExpr)
         }
 
         /// <summary>
-        /// Looks for a typed assignment to a variable (e.g., [Type]$var = ...)
+        /// Builds and caches typed variable assignments for a given scope.
+        /// This is called once per scope to avoid O(N*M) performance issues.
+        /// </summary>
+        private Dictionary<string, string> GetOrBuildTypedVariableCache(Ast scope)
+        {
+            if (scope == null)
+            {
+                return new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
+            }
+
+            // Check if we already have cached results for this scope
+            if (_typedVariableCache.TryGetValue(scope, out var cachedResults))
+            {
+                return cachedResults;
+            }
+
+            // Build the cache for this scope
+            var typedVariables = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
+
+            // Find all assignment statements in this scope
+            var assignments = scope.FindAll(testAst => testAst is AssignmentStatementAst, true);
+
+            foreach (AssignmentStatementAst assignment in assignments)
+            {
+                // Check if the left side is a convert expression with a variable
+                if (assignment.Left is ConvertExpressionAst convertExpr &&
+                    convertExpr.Child is VariableExpressionAst assignedVar)
+                {
+                    var varName = assignedVar.VariablePath.UserPath;
+                    var typeName = convertExpr.Type.TypeName.FullName;
+                    
+                    // Store in cache (first assignment wins)
+                    if (!typedVariables.ContainsKey(varName))
+                    {
+                        typedVariables[varName] = typeName;
+                    }
+                }
+            }
+
+            // Cache the results
+            _typedVariableCache[scope] = typedVariables;
+            return typedVariables;
+        }
+
+        /// <summary>
+        /// Looks for a typed assignment to a variable using cached results.
         /// </summary>
         private string FindTypedAssignment(VariableExpressionAst varExpr)
         {
@@ -700,19 +755,12 @@ private string FindTypedAssignment(VariableExpressionAst varExpr)
                 return null;
             }
 
-            // Find all assignment statements in this scope
-            var assignments = searchScope.FindAll(testAst => 
-                testAst is AssignmentStatementAst, true);
+            // Use cached results instead of re-scanning the entire scope
+            var typedVariables = GetOrBuildTypedVariableCache(searchScope);
             
-            foreach (AssignmentStatementAst assignment in assignments)
+            if (typedVariables.TryGetValue(varName, out string typeName))
             {
-                // Check if the left side is a convert expression with our variable
-                if (assignment.Left is ConvertExpressionAst convertExpr &&
-                    convertExpr.Child is VariableExpressionAst assignedVar &&
-                    string.Equals(assignedVar.VariablePath.UserPath, varName, StringComparison.OrdinalIgnoreCase))
-                {
-                    return convertExpr.Type.TypeName.FullName;
-                }
+                return typeName;
             }
             
             return null;
diff --git a/Tests/Rules/UseConstrainedLanguageMode.tests.ps1 b/Tests/Rules/UseConstrainedLanguageMode.tests.ps1
@@ -339,8 +339,8 @@ enum MyEnum {
         }
     }
 
-    Context "Informational severity" {
-        It "Should have Information severity" {
+    Context "Rule severity" {
+        It "Should have Warning severity" {
             $def = 'Add-Type -AssemblyName System.Windows.Forms'
             $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
             $matchingViolations = $violations | Where-Object { $_.RuleName -eq $violationName }
@@ -350,27 +350,27 @@ enum MyEnum {
 
     Context "When type constraints are used" {
         It "Should flag disallowed type constraint on parameter" {
-            $def = 'function Test { param([System.Net.WebClient]$Client) }'
+            $def = 'function Test { param([System.IO.File]$FileHelper) }'
             $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
             $matchingViolations = $violations | Where-Object { $_.RuleName -eq $violationName }
             $matchingViolations.Count | Should -BeGreaterThan 0
-            $matchingViolations[0].Message | Should -BeLike "*System.Net.WebClient*not permitted*"
+            $matchingViolations[0].Message | Should -BeLike "*System.IO.File*not permitted*"
         }
 
         It "Should flag disallowed type constraint on variable declaration" {
-            $def = '[System.Net.WebClient]$client = $null'
+            $def = '[System.IO.File]$fileHelper = $null'
             $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
             $matchingViolations = $violations | Where-Object { $_.RuleName -eq $violationName }
             $matchingViolations.Count | Should -BeGreaterThan 0
-            $matchingViolations[0].Message | Should -BeLike "*System.Net.WebClient*not permitted*"
+            $matchingViolations[0].Message | Should -BeLike "*System.IO.File*not permitted*"
         }
 
         It "Should flag disallowed type cast on variable assignment" {
-            $def = '$client = [System.Net.WebClient]$value'
+            $def = '$fileHelper = [System.IO.File]$value'
             $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
             $matchingViolations = $violations | Where-Object { $_.RuleName -eq $violationName }
             $matchingViolations.Count | Should -BeGreaterThan 0
-            $matchingViolations[0].Message | Should -BeLike "*System.Net.WebClient*not permitted*"
+            $matchingViolations[0].Message | Should -BeLike "*System.IO.File*not permitted*"
         }
 
         It "Should NOT flag allowed type constraint" {
@@ -441,8 +441,8 @@ $result = $client.DownloadString("http://example.com")
             $matchingViolations = $violations | Where-Object { $_.RuleName -eq $violationName }
             # Should flag both the type constraint AND the member access
             $matchingViolations.Count | Should -BeGreaterThan 1
-            # At least one violation should mention DownloadString
-            ($matchingViolations.Message | Where-Object { $_ -like "*DownloadString*" }).Count | Should -BeGreaterThan 0
+            # At least one violation should mention ReadAllText
+            ($matchingViolations.Message | Where-Object { $_ -like "*ReadAllText*" }).Count | Should -BeGreaterThan 0
         }
 
         It "Should NOT flag method invocation on allowed types" {
@@ -458,19 +458,19 @@ function Test {
         }
 
         It "Should NOT flag static method calls on disallowed types (already caught by type expression check)" {
-            $def = '[System.Net.WebClient]::New()'
+            $def = '[System.IO.File]::Exists("test.txt")'
             $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
             $matchingViolations = $violations | Where-Object { $_.RuleName -eq $violationName }
             # Should only flag once for the type expression, not for member access
             $matchingViolations.Count | Should -Be 1
-            $matchingViolations[0].Message | Should -BeLike "*System.Net.WebClient*"
+            $matchingViolations[0].Message | Should -BeLike "*System.IO.File*"
         }
 
         It "Should flag chained method calls on disallowed types" {
             $def = @'
 function Test {
-    param([System.Net.WebClient]$Client)
-    $result = $Client.DownloadString("http://example.com").ToUpper()
+    param([System.IO.FileInfo]$FileHelper)
+    $result = $FileHelper.OpenText().ReadToEnd()
 }
 '@
             $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
@@ -483,13 +483,13 @@ function Test {
             $def = @'
 function Complex-Test {
     param(
-        [System.Net.WebClient]$WebClient,
-        [System.Net.Sockets.TcpClient]$TcpClient,
+        [System.IO.File]$FileHelper,
+        [System.IO.Directory]$DirHelper,
         [string]$SafeString
     )
     
-    $data = $WebClient.DownloadData("http://test.com")
-    $TcpClient.Connect("localhost", 8080)
+    $data = $FileHelper.ReadAllBytes("C:\test.bin")
+    $DirHelper.GetFiles("C:\temp")
     $upper = $SafeString.ToUpper()
 }
 '@
@@ -526,8 +526,8 @@ Add-Type -TypeDefinition "public class Test { }"
         It "Should NOT flag disallowed types in signed scripts" {
             $scriptPath = Join-Path $tempPath "SignedWithDisallowedType.ps1"
             $scriptContent = @'
-$client = New-Object System.Net.WebClient
-$data = $client.DownloadString("http://example.com")
+$fileHelper = New-Object System.IO.FileInfo("C:\test.txt")
+$data = $fileHelper.OpenText()
 
 # SIG # Begin signature block
 # MIIFFAYJKoZIhvcNAQcCoIIFBTCCBQECAQExCzAJ...
@@ -536,7 +536,7 @@ $data = $client.DownloadString("http://example.com")
             Set-Content -Path $scriptPath -Value $scriptContent
             $violations = Invoke-ScriptAnalyzer -Path $scriptPath -Settings $settings
             $typeViolations = $violations | Where-Object { 
-                $_.RuleName -eq $violationName -and $_.Message -like "*WebClient*type*" 
+                $_.RuleName -eq $violationName -and $_.Message -like "*FileInfo*type*" 
             }
             $typeViolations | Should -BeNullOrEmpty
         }
@@ -582,7 +582,7 @@ class MyClass {
             $scriptPath = Join-Path $tempPath "SignedWithBadParam.ps1"
             $scriptContent = @'
 function Test {
-    param([System.Net.WebClient]$Client)
+    param([System.IO.File]$FileHelper)
     Write-Output "Test"
 }
 
@@ -593,7 +593,7 @@ function Test {
             Set-Content -Path $scriptPath -Value $scriptContent
             $violations = Invoke-ScriptAnalyzer -Path $scriptPath -Settings $settings
             $paramViolations = $violations | Where-Object { 
-                $_.RuleName -eq $violationName -and $_.Message -like "*WebClient*"
+                $_.RuleName -eq $violationName -and $_.Message -like "*File*"
             }
             # Parameter type constraints should still be flagged
             $paramViolations.Count | Should -BeGreaterThan 0
@@ -643,5 +643,71 @@ function Test {
             $scriptModuleViolations.Count | Should -BeGreaterThan 0
         }
     }
+
+    Context "Performance with large scripts" {
+        It "Should handle scripts with many typed variables and member invocations efficiently" {
+            # This test verifies the O(N+M) cache optimization
+            # Without caching, this would be O(N*M) and very slow
+            
+            # Build a script with many typed variables and member invocations
+            $scriptBuilder = [System.Text.StringBuilder]::new()
+            [void]$scriptBuilder.AppendLine('function Test-Performance {')
+            [void]$scriptBuilder.AppendLine('    param([string]$Path)')
+            
+            # Add 30 typed variable assignments
+            for ($i = 1; $i -le 30; $i++) {
+                [void]$scriptBuilder.AppendLine("    [System.IO.File]`$file$i = `$null")
+            }
+            
+            # Add 50 member invocations (testing cache reuse)
+            for ($i = 1; $i -le 50; $i++) {
+                $varNum = ($i % 30) + 1
+                [void]$scriptBuilder.AppendLine("    `$result$i = `$file$varNum.ReadAllText(`$Path)")
+            }
+            
+            [void]$scriptBuilder.AppendLine('}')
+            $def = $scriptBuilder.ToString()
+            
+            # Measure performance
+            $stopwatch = [System.Diagnostics.Stopwatch]::StartNew()
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
+            $stopwatch.Stop()
+            
+            $matchingViolations = $violations | Where-Object { $_.RuleName -eq $violationName }
+            
+            # Should detect violations (30 type constraints + 50 member accesses = 80+)
+            $matchingViolations.Count | Should -BeGreaterThan 50
+            
+            # Performance check: Should complete quickly (under 500ms)
+            # With cache: O(N+M) = 30+50 = 80 operations
+            # Without cache: O(N*M) = 50*30 = 1,500 operations (much slower)
+            $stopwatch.ElapsedMilliseconds | Should -BeLessThan 500
+        }
+
+        It "Should cache results per scope correctly" {
+            # Test that cache is scoped properly and doesn't leak between functions
+            $def = @'
+function Function1 {
+    [System.IO.File]$file1 = $null
+    $result1 = $file1.ReadAllText("C:\test1.txt")
 }
 
+function Function2 {
+    [System.IO.Directory]$file1 = $null
+    $result2 = $file1.GetFiles("C:\temp")
+}
+'@
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
+            $matchingViolations = $violations | Where-Object { $_.RuleName -eq $violationName }
+            
+            # Should detect violations in both functions
+            # Each function has: 1 type constraint + 1 member access = 2 violations each
+            $matchingViolations.Count | Should -BeGreaterOrEqual 4
+            
+            # Verify both File and Directory are mentioned
+            $messages = $matchingViolations.Message -join ' '
+            $messages | Should -BeLike "*File*"
+            $messages | Should -BeLike "*Directory*"
+        }
+    }
+}
