Fix LanguageRuleIgnoreMap and Add Positive Globs (#665)

gfs · web-flow · commit bb81662003f5 · 2024-12-06T14:04:40.000-08:00
* Fix incorrect inversion in LanguageRuleMap codepath

* Improve option operation tests

* Add Include Globs option. When specified, only files that match one of the include globs and that don't match one of the exclude globs are scanned.

* New test cases for glob options.

* Update changelog.
diff --git a/Changelog.md b/Changelog.md
@@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [1.0.50] - 2024-12-05
+## Fix
+Fixes #664 handling of options from IgnoreRuleMap when using OptionsJson
+
+## New Functionality
+Adds `include-globs` argument to require all scanned files match a specific glob pattern #663.
+
 ## [1.0.49] - 2024-12-03
 ## Rules
 Fixed false positives and false negatives in outdated/banned SSL/TLS protocols. #649
diff --git a/DevSkim-DotNet/Microsoft.DevSkim.CLI/Commands/AnalyzeCommand.cs b/DevSkim-DotNet/Microsoft.DevSkim.CLI/Commands/AnalyzeCommand.cs
@@ -65,7 +65,7 @@ public int Run()
 
             IEnumerable<FileEntry> fileListing;
             Extractor extractor = new Extractor();
-            ExtractorOptions extractorOpts = new ExtractorOptions() { ExtractSelfOnFail = false, DenyFilters = _opts.Globs };
+            ExtractorOptions extractorOpts = new ExtractorOptions() { ExtractSelfOnFail = false, AllowFilters = _opts.AllowGlobs, DenyFilters = _opts.Globs };
             // Analysing a single file
             if (!Directory.Exists(fullPath))
             {
@@ -424,7 +424,7 @@ void parseFileEntry(FileEntry fileEntry)
                         if (serializedAnalyzeCommandOptions.LanguageRuleIgnoreMap.TryGetValue(languageInfo.Name,
                                 out List<string>? maybeRulesToIgnore) && maybeRulesToIgnore is { } rulesToIgnore)
                         {
-                            var numRemoved = issues.RemoveAll(x => !rulesToIgnore.Contains(x.Rule.Id));
+                            var numRemoved = issues.RemoveAll(x => rulesToIgnore.Contains(x.Rule.Id));
                             _logger.LogDebug($"Removed {numRemoved} results because of language rule filters.");
                         }
                     }
diff --git a/DevSkim-DotNet/Microsoft.DevSkim.CLI/Options/BaseAnalyzeCommandOptions.cs b/DevSkim-DotNet/Microsoft.DevSkim.CLI/Options/BaseAnalyzeCommandOptions.cs
@@ -45,6 +45,9 @@ public record BaseAnalyzeCommandOptions : LogOptions
     [Option('g', "ignore-globs", HelpText = "Comma-separated Globs for files to skip analyzing", Separator = ',', Default = new[] { "**/.git/**", "**/bin/**" })]
     public IEnumerable<string> Globs { get; set; } = new[] { "**/.git/**", "**/bin/**" };
 
+    [Option("include-globs", HelpText = "If set, files must match one of these globs to be analyzed", Separator = ',', Default = new string[]{})]
+    public IEnumerable<string> AllowGlobs { get; set; } = new string[]{};
+    
     [Option('d', "disable-supression", HelpText = "Disable comment suppressions", Default = false)]
     public bool DisableSuppression { get; set; }
 
diff --git a/DevSkim-DotNet/Microsoft.DevSkim.Tests/OptionsTests.cs b/DevSkim-DotNet/Microsoft.DevSkim.Tests/OptionsTests.cs
@@ -8,6 +8,214 @@ namespace Microsoft.DevSkim.Tests;
 [TestClass]
 public class OptionsTests
 {
+    [TestMethod]
+    public void TestExcludeGlobs()
+    {
+        var serializedOptsExcludeGlobs = new SerializedAnalyzeCommandOptions()
+        {
+            Severities = new[] { Severity.Critical | Severity.Important },
+            ExitCodeIsNumIssues = true,
+            Globs = new List<string>() {"*.js"}
+        };
+        var testContent = "Hello World";
+        var testRule =
+@"[
+{
+    ""name"": ""Weak/Broken Hash Algorithm"",
+    ""id"": ""JsonOptionParseTest"",
+    ""description"": ""A test that finds hello"",
+    ""tags"": [
+        ""Tests.JsonOptionsTest""
+    ],
+    ""severity"": ""critical"",
+    ""patterns"": [
+        {
+            ""pattern"": ""Hello"",
+            ""type"": ""regex"",
+            ""scopes"": [
+                ""code""
+            ]
+        }
+    ]
+}]";
+        var rulesPath = PathHelper.GetRandomTempFile("json");
+        var serializedJsonPath = PathHelper.GetRandomTempFile("json");
+        var csharpTestPath = PathHelper.GetRandomTempFile("cs");
+        var jsTestPath = PathHelper.GetRandomTempFile("js");
+        {
+            using var serializedJsonStream = File.Create(serializedJsonPath);
+            JsonSerializer.Serialize(serializedJsonStream, serializedOptsExcludeGlobs, new JsonSerializerOptions() { });
+            using var csharpStream = File.Create(csharpTestPath);
+            JsonSerializer.Serialize(csharpStream, testContent);
+            using var jsStream = File.Create(jsTestPath);
+            JsonSerializer.Serialize(jsStream, testContent);
+            File.WriteAllText(rulesPath, testRule);
+        }
+
+        // Create an AnalyzeCommandOptions object referencing our serialized options
+        var analyzeOpts = new AnalyzeCommandOptions()
+        {
+            Path = csharpTestPath,
+            Rules = new[] { rulesPath },
+            PathToOptionsJson = serializedJsonPath
+        };
+
+        var analyzerWithSerialized = new AnalyzeCommand(analyzeOpts);
+        // We set exit code is num issues so this should be 1, as csharp files aren't ignored
+        Assert.AreEqual(1, analyzerWithSerialized.Run());
+        
+        // Create an AnalyzeCommandOptions object referencing our serialized options
+        analyzeOpts = new AnalyzeCommandOptions()
+        {
+            Path = jsTestPath,
+            Rules = new[] { rulesPath },
+            PathToOptionsJson = serializedJsonPath
+        };
+
+        analyzerWithSerialized = new AnalyzeCommand(analyzeOpts);
+        // We set exit code is num issues so this should be 0, as js files are ignored
+        Assert.AreEqual(0, analyzerWithSerialized.Run());
+    }
+    
+    [TestMethod]
+    public void TestIncludeGlobs()
+    {
+        var serializedOptsExcludeGlobs = new SerializedAnalyzeCommandOptions()
+        {
+            Severities = new[] { Severity.Critical | Severity.Important },
+            ExitCodeIsNumIssues = true,
+            AllowGlobs = new List<string>() {"*.js"}
+        };
+        var testContent = "Hello World";
+        var testRule =
+@"[
+{
+    ""name"": ""Weak/Broken Hash Algorithm"",
+    ""id"": ""JsonOptionParseTest"",
+    ""description"": ""A test that finds hello"",
+    ""tags"": [
+        ""Tests.JsonOptionsTest""
+    ],
+    ""severity"": ""critical"",
+    ""patterns"": [
+        {
+            ""pattern"": ""Hello"",
+            ""type"": ""regex"",
+            ""scopes"": [
+                ""code""
+            ]
+        }
+    ]
+}]";
+        var rulesPath = PathHelper.GetRandomTempFile("json");
+        var serializedJsonPath = PathHelper.GetRandomTempFile("json");
+        var csharpTestPath = PathHelper.GetRandomTempFile("cs");
+        var jsTestPath = PathHelper.GetRandomTempFile("js");
+        {
+            using var serializedJsonStream = File.Create(serializedJsonPath);
+            JsonSerializer.Serialize(serializedJsonStream, serializedOptsExcludeGlobs, new JsonSerializerOptions() { });
+            using var csharpStream = File.Create(csharpTestPath);
+            JsonSerializer.Serialize(csharpStream, testContent);
+            using var jsStream = File.Create(jsTestPath);
+            JsonSerializer.Serialize(jsStream, testContent);
+            File.WriteAllText(rulesPath, testRule);
+        }
+
+        // Create an AnalyzeCommandOptions object referencing our serialized options
+        var analyzeOpts = new AnalyzeCommandOptions()
+        {
+            Path = csharpTestPath,
+            Rules = new[] { rulesPath },
+            PathToOptionsJson = serializedJsonPath
+        };
+
+        var analyzerWithSerialized = new AnalyzeCommand(analyzeOpts);
+        // We set exit code is num issues so this should be 0, as csharp are implicitly ignored
+        Assert.AreEqual(0, analyzerWithSerialized.Run());
+        
+        // Create an AnalyzeCommandOptions object referencing our serialized options
+        analyzeOpts = new AnalyzeCommandOptions()
+        {
+            Path = jsTestPath,
+            Rules = new[] { rulesPath },
+            PathToOptionsJson = serializedJsonPath
+        };
+
+        analyzerWithSerialized = new AnalyzeCommand(analyzeOpts);
+        // We set exit code is num issues so this should be 1, as js files are included
+        Assert.AreEqual(1, analyzerWithSerialized.Run());
+    }
+    
+    [TestMethod]
+    public void TestIncludeAndExcludeGlobs()
+    {
+        var serializedOptsExcludeGlobs = new SerializedAnalyzeCommandOptions()
+        {
+            Severities = new[] { Severity.Critical | Severity.Important },
+            ExitCodeIsNumIssues = true,
+            AllowGlobs = new List<string>() {"*.js"},
+            Globs = new List<string>() {"*hello.js"}
+        };
+        var testContent = "Hello World";
+        var testRule =
+@"[
+{
+    ""name"": ""Weak/Broken Hash Algorithm"",
+    ""id"": ""JsonOptionParseTest"",
+    ""description"": ""A test that finds hello"",
+    ""tags"": [
+        ""Tests.JsonOptionsTest""
+    ],
+    ""severity"": ""critical"",
+    ""patterns"": [
+        {
+            ""pattern"": ""Hello"",
+            ""type"": ""regex"",
+            ""scopes"": [
+                ""code""
+            ]
+        }
+    ]
+}]";
+        var rulesPath = PathHelper.GetRandomTempFile("json");
+        var serializedJsonPath = PathHelper.GetRandomTempFile("json");
+        var helloJsTestPath = PathHelper.GetRandomTempFile("hello.js");
+        var jsTestPath = PathHelper.GetRandomTempFile("js");
+        {
+            using var serializedJsonStream = File.Create(serializedJsonPath);
+            JsonSerializer.Serialize(serializedJsonStream, serializedOptsExcludeGlobs, new JsonSerializerOptions() { });
+            using var helloJsStream = File.Create(helloJsTestPath);
+            JsonSerializer.Serialize(helloJsStream, testContent);
+            using var jsStream = File.Create(jsTestPath);
+            JsonSerializer.Serialize(jsStream, testContent);
+            File.WriteAllText(rulesPath, testRule);
+        }
+
+        // Create an AnalyzeCommandOptions object referencing our serialized options
+        var analyzeOpts = new AnalyzeCommandOptions()
+        {
+            Path = helloJsTestPath,
+            Rules = new[] { rulesPath },
+            PathToOptionsJson = serializedJsonPath
+        };
+
+        var analyzerWithSerialized = new AnalyzeCommand(analyzeOpts);
+        // We set exit code is num issues so this should be 0, as hello.js files are ignored
+        Assert.AreEqual(0, analyzerWithSerialized.Run());
+        
+        // Create an AnalyzeCommandOptions object referencing our serialized options
+        analyzeOpts = new AnalyzeCommandOptions()
+        {
+            Path = jsTestPath,
+            Rules = new[] { rulesPath },
+            PathToOptionsJson = serializedJsonPath
+        };
+
+        analyzerWithSerialized = new AnalyzeCommand(analyzeOpts);
+        // We set exit code is num issues so this should be 1, as regular js files are included
+        Assert.AreEqual(1, analyzerWithSerialized.Run());
+    }
+    
     [TestMethod]
     public void TestParsingJsonOptions()
     {
@@ -29,7 +237,8 @@ public void TestParsingJsonOptions()
             Globs = new List<string>() {"*.js"}
         };
         // Serialize it to a file
-        var testContent = "Hello World";
+        // Include world twice so we can disinguish between the two rules
+        var testContent = "Hello World World";
         var testRule =
 @"[
     {
@@ -95,8 +304,8 @@ public void TestParsingJsonOptions()
         };
 
         var analyzerWithSerialized = new AnalyzeCommand(analyzeOpts);
-        // We set exit code is num issues so this should be 1, from the 1 rule that isn't ignored
-        Assert.AreEqual(1, analyzerWithSerialized.Run());
+        // We set exit code is num issues so this should be 2, from the two matchs for the rule that isn't ignored
+        Assert.AreEqual(2, analyzerWithSerialized.Run());
         // Create an AnalyzeCommandOptions object that references the path to the file which ignores a specific rule
         analyzeOpts = new AnalyzeCommandOptions()
         {
@@ -117,8 +326,8 @@ public void TestParsingJsonOptions()
             PathToOptionsJson = serializedJsonPath
         };
         analyzerWithSerialized = new AnalyzeCommand(analyzeOpts);
-        // This should be 2, because 2 rules aren't ignored
-        Assert.AreEqual(2, analyzerWithSerialized.Run());
+        // This should be 3, because no rules are ignored
+        Assert.AreEqual(3, analyzerWithSerialized.Run());
         // Try the js which it should find both
         analyzeOpts = new AnalyzeCommandOptions()
         {
@@ -140,8 +349,8 @@ public void TestParsingJsonOptions()
             PathToOptionsJson = serializedJsonPath2
         };
         analyzerWithSerialized = new AnalyzeCommand(analyzeOpts);
-        // This should be 2, because the globs dont exclude cs files
-        Assert.AreEqual(2, analyzerWithSerialized.Run());
+        // This should be 3, because the globs dont exclude cs files
+        Assert.AreEqual(3, analyzerWithSerialized.Run());
         // set of options to test enumerable parsing
         analyzeOpts = new AnalyzeCommandOptions()
         {

Original file line number	Diff line number	Diff line change
`@@ -65,7 +65,7 @@ public int Run()`
`65`	`65`
`66`	`66`	`IEnumerable<FileEntry> fileListing;`
`67`	`67`	`Extractor extractor = new Extractor();`
`68`		`- ExtractorOptions extractorOpts = new ExtractorOptions() { ExtractSelfOnFail = false, DenyFilters = _opts.Globs };`
	`68`	`+ ExtractorOptions extractorOpts = new ExtractorOptions() { ExtractSelfOnFail = false, AllowFilters = _opts.AllowGlobs, DenyFilters = _opts.Globs };`
`69`	`69`	`// Analysing a single file`
`70`	`70`	`if (!Directory.Exists(fullPath))`
`71`	`71`	`{`
`@@ -424,7 +424,7 @@ void parseFileEntry(FileEntry fileEntry)`
`424`	`424`	`if (serializedAnalyzeCommandOptions.LanguageRuleIgnoreMap.TryGetValue(languageInfo.Name,`
`425`	`425`	`out List<string>? maybeRulesToIgnore) && maybeRulesToIgnore is { } rulesToIgnore)`
`426`	`426`	`{`
`427`		`- var numRemoved = issues.RemoveAll(x => !rulesToIgnore.Contains(x.Rule.Id));`
	`427`	`+ var numRemoved = issues.RemoveAll(x => rulesToIgnore.Contains(x.Rule.Id));`
`428`	`428`	`_logger.LogDebug($"Removed {numRemoved} results because of language rule filters.");`
`429`	`429`	`}`
`430`	`430`	`}`