Rewrite logging, tests, add user customizable languages. (#441)

* Fix most of logging in Analyze Command

* ConvertTagDiff command

* Finish conversion of Analyze

* Rewrite logging

* Bump version. These changes change API surfaces and CLI arguments.

* More fixes.

* More test improvements

* More test fixes.

* Fix analyze tests

* Incrementally adding tests for untested parameters.

* Fix tests for everything except timeouts in analyze command.

* Add logger factory to the tests. Start on verify rules cmd.

* Refactor VerifyRules + Fix Tests to cover all cases checked for AI rules.

* Remove RulePacker, just embed all rules directly

* Existing nuget tests fixed.

* More fixes. start implementing custom languages and comments.

* Fix #418

* Remove unused examples

Remove old CLI tests

* Generate and publish code coverage in pipeline

* Clean up unused resources.

* Update dotnet-test-job.yml

* Update dotnet-test-job.yml

* Add SeverityLevel option.

* Add applies to filename test

* Add override test

* Update TestAnalyzeCmd.cs

* Tests and refactoring/renaming.

* More tests for malformed within clauses.

* Rename Test folder

Fix pipeline
More withinclause tests

* Improvements

* Fix option parsing

* More test coverage for Languages.cs.

* Refactor. Removing Newtonsoft dependency.

* Re-add support for netcore3.1 to lib

* Support netstandard2.1 instead.

* Refactor and Cleanup

Allows tests to run on netstandard by moving the logoptions to a separate project.

* Fixes for #442 and #443 (#444)

* Update bug report

Co-authored-by: Michael Scovetta <michael.scovetta@microsoft.com>

.github/ISSUE_TEMPLATE/bug_report.md

@@ -12,27 +12,23 @@ A clear and concise description of what the bug is.
 
 **To Reproduce**
 Steps to reproduce the behavior:
-1. Go to '...'
-2. Click on '....'
-3. Scroll down to '....'
-4. See error
+1. Run ApplicationInspector with "appinspector -args -you -used"
+2. Go to '...'
+3. Click on '....'
+4. Scroll down to '....'
+5. See error
 
 **Expected behavior**
 A clear and concise description of what you expected to happen.
 
 **Screenshots**
 If applicable, add screenshots to help explain your problem.
 
-**Desktop (please complete the following information):**
- - OS: [e.g. iOS]
- - Browser [e.g. chrome, safari]
- - Version [e.g. 22]
-
-**Smartphone (please complete the following information):**
- - Device: [e.g. iPhone6]
- - OS: [e.g. iOS8.1]
- - Browser [e.g. stock browser, safari]
- - Version [e.g. 22]
+**Operating Environment (please complete the following information):**
+ - Application Inspector Version: [from `appinspector -v`]
+ - OS: [e.g. Windows 11 21H2 22000.613, MacOS 13.0.1, Linux Ubuntu 20.04]
+ - Browser: (if an issue with HTML report) [e.g. Edge, Chrome, Safari]
+ - Browser Version: (if an issue with HTML report) [e.g. 100]
 
 **Additional context**
 Add any other context about the problem here.

.github/workflows/codeql-analysis.yml

@@ -54,8 +54,8 @@ jobs:
     - name: Build Tests
       run: |
          dotnet restore
-         dotnet build UnitTest.Commands.csproj
-      working-directory: ./UnitTest.Commands
+         dotnet build AppInspector.Tests.csproj
+      working-directory: ./AppInspector.Tests
 
     - name: Perform CodeQL Analysis
       uses: github/codeql-action/analyze@v1

.gitignore

@@ -262,10 +262,11 @@ __pycache__/
 
 # App Inspector artifacts
 /AppInspector/Resources/defaultRulesPkd.json
-/UnitTest.Commands/output
-/RulesPacker/log.txt
+/AppInspector.Tests/output
 /AppInspector/ApplicationInspector.Commands.xml
 AppInspector/Resources/defaultRulesPkd.json
 /AppInspector.CLI/Properties/launchSettings.json
+output.html
+result.json
+AppInspector.Tests/logs/
 RulesPacker/appinspector.log.txt
-AppInspector/appinspector.log.txt

AppInspector.CLI/AppInspector.CLI.csproj

@@ -40,6 +40,7 @@
   </ItemGroup>
 
   <ItemGroup>
+    <ProjectReference Include="..\AppInspector.Logging\AppInspector.Logging.csproj" />
     <ProjectReference Include="..\AppInspector\AppInspector.Commands.csproj" />
     <ProjectReference Include="..\RulesEngine\AppInspector.RulesEngine.csproj" />
   </ItemGroup>
@@ -66,8 +67,11 @@
 
   <ItemGroup>
     <PackageReference Include="DotLiquid" Version="2.2.595" />
-    <PackageReference Include="NLog" Version="4.7.13" />
-    <PackageReference Include="Sarif.Sdk" Version="2.4.12" />
+    <PackageReference Include="Sarif.Sdk" Version="2.4.13" />
+    <PackageReference Include="Serilog" Version="2.10.0" />
+    <PackageReference Include="Serilog.Extensions.Logging" Version="3.1.0" />
+    <PackageReference Include="Serilog.Sinks.Console" Version="4.0.1" />
+    <PackageReference Include="Serilog.Sinks.File" Version="5.0.0" />
     <PackageReference Include="ShellProgressBar" Version="5.1.0" />
   </ItemGroup>  
 </Project>

AppInspector.CLI/CLICmdOptions.cs

@@ -1,10 +1,14 @@
 // Copyright (C) Microsoft. All rights reserved.
 // Licensed under the MIT License. See LICENSE.txt in the project root for license information.
 
+using Microsoft.ApplicationInspector.Logging;
+
 namespace Microsoft.ApplicationInspector.CLI
 {
     using CommandLine;
-    using Microsoft.ApplicationInspector.Common;
+    using Microsoft.ApplicationInspector.Commands;
+    using Microsoft.ApplicationInspector.RulesEngine;
+    using System;
     using System.Collections.Generic;
 
     /// <summary>
@@ -20,52 +24,66 @@ public class CLICommandOptions : LogOptions
         public string OutputFileFormat { get; set; } = "text";
     }
 
-    /// <summary>
-    /// CLI command distinct arguments
-    /// </summary>
-    [Verb("analyze", HelpText = "Inspect source directory/file/compressed file (.tgz|zip) against defined characteristics")]
-    public class CLIAnalyzeCmdOptions : CLICommandOptions
+    public class CLICustomRulesCommandOptions: CLICommandOptions
     {
-        [Option('s', "source-path", Required = true, HelpText = "Source file or directory to inspect, comma separated", Separator = ',')]
-        public IEnumerable<string> SourcePath { get; set; } = System.Array.Empty<string>();
-
         [Option('r', "custom-rules-path", Required = false, HelpText = "Custom rules file or directory path")]
         public string? CustomRulesPath { get; set; }
 
+        [Option("custom-languages-path", Required = false, HelpText = "Replace the default languages set with a custom languages.json.")]
+        public string? CustomLanguagesPath { get; set; }
+
+        [Option("custom-comments-path", Required = false, HelpText = "Replace the default comment specification set with a custom comments.json.")]
+        public string? CustomCommentsPath { get; set; }
+    }
+
+    public class CLIAnalysisSharedCommandOptions : CLICustomRulesCommandOptions
+    {
         [Option('i', "ignore-default-rules", Required = false, HelpText = "Exclude default rules bundled with application", Default = false)]
         public bool IgnoreDefaultRules { get; set; }
 
-        [Option('c', "confidence-filters", Required = false, HelpText = "Output only matches with specified confidence <value>,<value> [high|medium|low]", Default = "high,medium")]
-        public string ConfidenceFilters { get; set; } = "high,medium";
+        [Option('F', "file-timeout", Required = false, HelpText = "Maximum amount of time in milliseconds to allow for processing each file. 0 is infinity. Default: 60000.", Default = 60000)]
+        public int FileTimeOut { get; set; } = 60000;
 
-        [Option('g', "exclusion-globs", Required = false, HelpText = "Exclude source files that match glob patterns. Example: \"**/.git/**,*Tests*\".  Use \"none\" to disable.", Default = new string[] { "**/bin/**", "**/obj/**", "**/.vs/**", "**/.git/**" }, Separator = ',')]
+        [Option('p', "processing-timeout", Required = false, HelpText = "Maximum amount of time in milliseconds to allow for processing overall. 0 is infinity. Default: 0.", Default = 0)]
+        public int ProcessingTimeOut { get; set; }
+
+        [Option('S', "single-threaded", Required = false, HelpText = "Disables parallel processing.")]
+        public bool SingleThread { get; set; }
 
+        [Option('g', "exclusion-globs", Required = false, HelpText = "Exclude source files that match glob patterns. Example: \"**/.git/**,*Tests*\".  Use \"none\" to disable.", Default = new string[] { "**/bin/**", "**/obj/**", "**/.vs/**", "**/.git/**" }, Separator = ',')]
         public IEnumerable<string> FilePathExclusions { get; set; } = System.Array.Empty<string>();
 
+        [Option('u', "scan-unknown-filetypes", Required = false, HelpText = "Scan files of unknown types.")]
+        public bool ScanUnknownTypes { get; set; }
+
+        [Option('c', "confidence-filters", Required = false, Separator = ',',  HelpText = "Output only matches with specified confidence <value>,<value>. Default: Medium,High. [High|Medium|Low]")]
+        public IEnumerable<Confidence> ConfidenceFilters { get; set; } = new Confidence[] { Confidence.High, Confidence.Medium };
+
+        [Option("severity-filters", Required = false, Separator = ',', HelpText = "Output only matches with specified severity <value>,<value>. Default: All are enabled. [Critical|Important|Moderate|BestPractice|ManualReview]")]
+        public IEnumerable<Severity> SeverityFilters { get; set; } = (IEnumerable<Severity>)Enum.GetValues(typeof(Severity));
+    }
+
+    /// <summary>
+    /// CLI command distinct arguments
+    /// </summary>
+    [Verb("analyze", HelpText = "Inspect source directory/file/compressed file (.tgz|zip) against defined characteristics")]
+    public class CLIAnalyzeCmdOptions : CLIAnalysisSharedCommandOptions
+    {
+        [Option('s', "source-path", Required = true, HelpText = "Source file or directory to inspect, comma separated", Separator = ',')]
+        public IEnumerable<string> SourcePath { get; set; } = System.Array.Empty<string>();
+
         [Option('f', "output-file-format", Required = false, HelpText = "Output format [html|json|text]", Default = "html")]
         public new string OutputFileFormat { get; set; } = "html";
 
         [Option('e', "text-format", Required = false, HelpText = "Match text format specifiers", Default = "Tag:%T,Rule:%N,Ruleid:%R,Confidence:%X,File:%F,Sourcetype:%t,Line:%L,Sample:%m")]
         public string TextOutputFormat { get; set; } = "Tag:%T,Rule:%N,Ruleid:%R,Confidence:%X,File:%F,Sourcetype:%t,Line:%L,Sample:%m";
 
-        [Option('F',"file-timeout", Required = false, HelpText = "Maximum amount of time in milliseconds to allow for processing each file. 0 is infinity. Default: 60000.", Default = 60000)]
-        public int FileTimeOut { get; set; } = 60000;
-
-        [Option('p',"processing-timeout", Required = false, HelpText = "Maximum amount of time in milliseconds to allow for processing overall. 0 is infinity. Default: 0.", Default = 0)]
-        public int ProcessingTimeOut { get; set; }
-
-        [Option('S',"single-threaded", Required = false, HelpText = "Disables parallel processing.")]
-        public bool SingleThread { get; set; }
-
         [Option('N',"no-show-progress", Required = false, HelpText = "Disable progress information.")]
         public bool NoShowProgressBar { get; set; }
 
         [Option('C',"context-lines", Required = false, HelpText = "Number of lines of context on each side to include in excerpt (up to a maximum of 100 * NumLines characters on each side). 0 to skip exerpt. -1 to not extract samples or excerpts (implied by -t). When outputting sarif use -1 for no snippets, all other values ignored.")]
         public int ContextLines { get; set; } = 3;
 
-        [Option('u',"scan-unknown-filetypes", Required = false, HelpText = "Scan files of unknown types.")]
-        public bool ScanUnknownTypes { get; set; }
-
         [Option('t',"tags-only", Required = false, HelpText = "Only get tags (no detailed match data). Ignored if output format is sarif.")]
         public bool TagsOnly { get; set; }
 
@@ -89,43 +107,19 @@ public class CLIAnalyzeCmdOptions : CLICommandOptions
     }
 
     [Verb("tagdiff", HelpText = "Compares unique tag values between two source paths")]
-    public class CLITagDiffCmdOptions : CLICommandOptions
+    public class CLITagDiffCmdOptions : CLIAnalysisSharedCommandOptions
     {
         [Option("src1", Required = true, HelpText = "Source 1 to compare (commaa separated)")]
         public IEnumerable<string> SourcePath1 { get; set; } = System.Array.Empty<string>();
 
         [Option("src2", Required = true, HelpText = "Source 2 to compare (commaa separated)")]
         public IEnumerable<string> SourcePath2 { get; set; } = System.Array.Empty<string>();
 
-        [Option('t', "test-type", Required = false, HelpText = "Type of test to run [equality|inequality]", Default = "equality")]
-        public string TestType { get; set; } = "equality";
-
-        [Option('g', "exclusion-globs", Required = false, HelpText = "Exclude source files that match glob patterns. Example: \"**/.git/**,*Tests*\".  Use \"none\" to disable.", Default = new string[] { "**/bin/**", "**/obj/**", "**/.vs/**", "**/.git/**" }, Separator = ',')]
-        public IEnumerable<string> FilePathExclusions { get; set; } = System.Array.Empty<string>();
-
-        [Option('r', "custom-rules-path", Required = false, HelpText = "Custom rules file or directory path")]
-        public string? CustomRulesPath { get; set; }
-
-        [Option('i', "ignore-default-rules", Required = false, HelpText = "Exclude default rules bundled with application", Default = false)]
-        public bool IgnoreDefaultRules { get; set; }
-
-        [Option('F', "file-timeout", Required = false, HelpText = "Maximum amount of time in milliseconds to allow for processing each file. 0 is infinity.  Default: 60000.", Default = 60000)]
-        public int FileTimeOut { get; set; } = 60000;
-
-        [Option('p',"processing-timeout", Required = false, HelpText = "Maximum amount of time in milliseconds to allow for processing each source. 0 is infinity. Default: 0.", Default = 0)]
-        public int ProcessingTimeOut { get; set; }
-
-        [Option('u', "scan-unknown-filetypes", Required = false, HelpText = "Scan files of unknown types.")]
-        public bool ScanUnknownTypes { get; set; }
-
-        [Option('S', "single-threaded", Required = false, HelpText = "Disables parallel processing.")]
-        public bool SingleThread { get; set; }
-
-        [Option('c', "confidence-filters", Required = false, HelpText = "Output only matches with specified confidence <value>,<value> [high|medium|low]", Default = "high,medium")]
-        public string ConfidenceFilters { get; set; } = "high,medium";
+        [Option('t', "test-type", Required = false, HelpText = "Type of test to run [Equality|Inequality]", Default = TagTestType.Equality)]
+        public TagTestType TestType { get; set; } = TagTestType.Equality;
     }
 
-    [Verb("exporttags", HelpText = "Export unique rule tags to view what code features may be detected")]
+    [Verb("exporttags", HelpText = "Export the list of tags associated with the specified rules. Does not scan source code.")]
     public class CLIExportTagsCmdOptions : CLICommandOptions
     {
         [Option('r', "custom-rules-path", Required = false, HelpText = "Custom rules file or directory path")]
@@ -136,34 +130,19 @@ public class CLIExportTagsCmdOptions : CLICommandOptions
     }
 
     [Verb("verifyrules", HelpText = "Verify custom rules syntax is valid")]
-    public class CLIVerifyRulesCmdOptions : CLICommandOptions
+    public class CLIVerifyRulesCmdOptions : CLICustomRulesCommandOptions
     {
-        [Option('d', "verify-default-rules", Required = false, Default = false, HelpText = "Verify default rules")]
+        [Option('d', "verify-default-rules", Required = false, Default = false, HelpText = "Verify the rules embedded in the binary.")]
         public bool VerifyDefaultRules { get; set; }
 
-        [Option('r', "custom-rules-path", Required = false, HelpText = "Custom rules file or directory path")]
-        public string? CustomRulesPath { get; set; }
-
         [Option('a', "fail-fast", Required = false, HelpText = "Fail fast", Default = false)]
         public bool Failfast { get; set; }
     }
 
     [Verb("packrules", HelpText = "Combine multiple rule files into one file for ease in distribution")]
-    public class CLIPackRulesCmdOptions : CLICommandOptions
+    public class CLIPackRulesCmdOptions : CLICustomRulesCommandOptions
     {
-        [Option('d', "pack-default-rules", Required = false, HelpText = "Repack rules from default rules path. Deprecated and will be removed in a future update.")]
-        public bool RepackDefaultRules { get; set; }
-
-        [Option('e', "pack-embedded-rules", Required = false, HelpText = "Pack the rules that are embedded in the DevSkim binary.")]
+        [Option('e', "pack-embedded-rules", Required = false, HelpText = "Pack the rules that are embedded in the application inspector binary.")]
         public bool PackEmbeddedRules { get; set; }
-
-        [Option('r', "custom-rules-path", Required = false, HelpText = "Custom rules file or directory path")]
-        public string? CustomRulesPath { get; set; }
-
-        [Option('f', "output-file-format", Required = false, HelpText = "Output format [json]", Default = "json")]
-        public new string OutputFileFormat { get; set; } = "json";
-
-        [Option('i', "not-indented", Required = false, HelpText = "Remove indentation from json output", Default = false)]
-        public bool NotIndented { get; set; }
     }
 }