Merge pull request #277 from Azure/verabe/securityRulesByDefault

Run security rules from PSRule by default, and exclude repeated rules
Azure · Aug 23, 2022 · 9bb8736 · 9bb8736
2 parents 0fba685 + 7668330
commit 9bb8736
Show file tree

Hide file tree

Showing 17 changed files with 447 additions and 106 deletions.
diff --git a/README.md b/README.md
@@ -35,7 +35,7 @@ Argument | Description
 **(Optional)** `--report-format` | Valid formats:<br/>*Console*: output results to the console in plain text. **(default)**<br/>*Sarif*: output results to a file in [SARIF](https://sarifweb.azurewebsites.net) format.
 `-o` or `--output-file-path` | **(Required if `--report-format` is *Sarif*)**  File path to output SARIF results to.
 **(Optional)** `-v` or `--verbose` | Shows details about the analysis
-**(Optional)** `--run-powershell` | Also runs the PowerShell-based rules
+**(Optional)** `--include-non-security-rules` | Run all the rules against the templates, including non-security rules
 
  The Template BPA runs the [configured rules](#understanding-and-customizing-rules) against the provided template and its corresponding [template parameters](https://docs.microsoft.com/azure/azure-resource-manager/templates/parameter-files), if specified. If no template parameters are specified, then the Template BPA generates the minimum number of placeholder parameters to properly evaluate [template functions](https://docs.microsoft.com/azure/azure-resource-manager/templates/template-functions) in the template.
 

diff --git a/docs/built-in-bpa-rules.md b/docs/built-in-bpa-rules.md
@@ -2,7 +2,7 @@
 
 ## PowerShell-Based Rules:
 
-Information about the PowerShell-based rules included by our integration with [PSRule](https://microsoft.github.io/PSRule/) can be found [here](https://azure.github.io/PSRule.Rules.Azure/en/rules/module/). The tool will only evaluate these rules if the option `-run-powershell` is used.
+Information about the PowerShell-based rules included by our integration with [PSRule for Azure](https://aka.ms/ps-rule-azure/rules). The tool will only evaluate the rules under the [Security pillar](https://azure.github.io/PSRule.Rules.Azure/en/rules/module/#security) unless the option `--include-non-security-rules` is used.
 
 ## JSON-Based Rules:
 

diff --git a/docs/customizing-evaluation-outputs.md b/docs/customizing-evaluation-outputs.md
@@ -1,7 +1,7 @@
 # Customizing Evaluation Outputs
 
 ## Overview
-Template BPA customization is authored in JSON.  Custom evaluation settings are written in a separate configuration and used on tool execution. See the main README for details on [how to run Template BPA with a configuration file](https://github.com/Azure/template-analyzer#using-the-template-bpa). 
+Using an additional JSON configuration file, the Template BPA can be customized in how it runs **JSON-based rules**. For example, specific rules can be included in/excluded from execution, or the severity of a rule can be changed. See the main README for details on [how to run Template BPA with a configuration file](https://github.com/Azure/template-analyzer#using-the-template-bpa).
 
 ## Template BPA Rule Object
 Here are the fields that make up a custom configuration file:

diff --git a/src/Analyzer.Cli.FunctionalTests/CommandLineParserTests.cs b/src/Analyzer.Cli.FunctionalTests/CommandLineParserTests.cs
@@ -83,6 +83,22 @@ public void AnalyzeTemplate_ReportFormatAsSarif_ReturnExpectedExitCodeUsingOptio
             File.Delete(outputFilePath);
         }
 
+        [TestMethod]
+        public void AnalyzeTemplate_IncludesOrNotNonSecurityRules_ReturnsExpectedExitCode()
+        {
+            var templatePath = GetFilePath("TriggersOnlyNonSecurityRules.json");
+
+            var args = new string[] { "analyze-template", templatePath };
+            var result = _commandLineParser.InvokeCommandLineAPIAsync(args);
+
+            Assert.AreEqual((int)ExitCode.Success, result.Result);
+
+            args = new string[] { "analyze-template", templatePath, "--include-non-security-rules" };
+            result = _commandLineParser.InvokeCommandLineAPIAsync(args);
+
+            Assert.AreEqual((int)ExitCode.Violation, result.Result);
+        }
+
         [TestMethod]
         public void AnalyzeDirectory_ValidInputValues_AnalyzesExpectedNumberOfFiles()
         {
@@ -94,7 +110,7 @@ public void AnalyzeDirectory_ValidInputValues_AnalyzesExpectedNumberOfFiles()
             var result = _commandLineParser.InvokeCommandLineAPIAsync(args);
 
             Assert.AreEqual((int)ExitCode.ErrorAndViolation, result.Result);
-            StringAssert.Contains(outputWriter.ToString(), "Analyzed 8 files");
+            StringAssert.Contains(outputWriter.ToString(), "Analyzed 9 files");
         }
 
         [DataTestMethod]
@@ -243,10 +259,20 @@ public void FilterRules_ValidConfig_RulesFiltered(bool isBicep, string configNam
                     .ToString());
 
                 if (specifyInCommand)
+                {
                     args = args.Concat(new[] { "--config-file-path", configName }).ToArray();
+                }
+
+                using StringWriter outputWriter = new();
+                Console.SetOut(outputWriter);
 
                 result = _commandLineParser.InvokeCommandLineAPIAsync(args);
-                Assert.AreEqual((int)ExitCode.Success, result.Result);
+
+                var cliConsoleOutput = outputWriter.ToString();
+
+                // All JSON rules are filtered out; PSRule rules are currently not filtered by the config file and should appear in the output
+                Assert.IsTrue(!cliConsoleOutput.Contains("TA-"));
+                Assert.AreEqual((int)ExitCode.Violation, result.Result);
             }
             finally
             {

diff --git a/src/Analyzer.Cli.FunctionalTests/Tests/TriggersOnlyNonSecurityRules.json b/src/Analyzer.Cli.FunctionalTests/Tests/TriggersOnlyNonSecurityRules.json
@@ -0,0 +1,17 @@
+{
+  "$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
+  "contentVersion": "1.0.0.0",
+  "parameters": {},
+  "functions": [],
+  "variables": {},
+  "resources": [
+    {
+      "type": "Microsoft.MachineLearningServices/workspaces",
+      "name": "aWorkspace",
+      "apiVersion": "2016-12-01",
+      "location": "",
+      "properties": {}
+    }
+  ],
+  "outputs": {}
+}
diff --git a/src/Analyzer.Cli/CommandLineParser.cs b/src/Analyzer.Cli/CommandLineParser.cs
@@ -160,8 +160,8 @@ private void SetupCommonOptionsForCommands(List<Command> commands)
                     "Shows details about the analysis"),
 
                 new Option(
-                    "--run-powershell",
-                    "Run PowerShell based checks against templates")
+                    "--include-non-security-rules",
+                    "Run all the rules against the templates, including non-security rules")
             };
 
             commands.ForEach(c => options.ForEach(c.AddOption));
@@ -174,7 +174,7 @@ private int AnalyzeTemplateCommandHandler(
             FileInfo configFilePath,
             ReportFormat reportFormat,
             FileInfo outputFilePath,
-            bool runPowerShell,
+            bool includeNonSecurityRules,
             bool verbose)
         {
             // Check that template file paths exist
@@ -184,7 +184,7 @@ private int AnalyzeTemplateCommandHandler(
                 return (int)ExitCode.ErrorInvalidPath;
             }
 
-            var setupResult = SetupAnalysis(configFilePath, directoryToAnalyze: null, reportFormat, outputFilePath, runPowerShell, verbose);
+            var setupResult = SetupAnalysis(configFilePath, directoryToAnalyze: null, reportFormat, outputFilePath, includeNonSecurityRules, verbose);
             if (setupResult != ExitCode.Success)
             {
                 return (int)setupResult;
@@ -210,7 +210,7 @@ private int AnalyzeDirectoryCommandHandler(
             FileInfo configFilePath,
             ReportFormat reportFormat,
             FileInfo outputFilePath,
-            bool runPowerShell,
+            bool includeNonSecurityRules,
             bool verbose)
         {
             if (!directoryPath.Exists)
@@ -219,7 +219,7 @@ private int AnalyzeDirectoryCommandHandler(
                 return (int)ExitCode.ErrorInvalidPath;
             }
 
-            var setupResult = SetupAnalysis(configFilePath, directoryPath, reportFormat, outputFilePath, runPowerShell, verbose);
+            var setupResult = SetupAnalysis(configFilePath, directoryPath, reportFormat, outputFilePath, includeNonSecurityRules, verbose);
             if (setupResult != ExitCode.Success)
             {
                 return (int)setupResult;
@@ -274,7 +274,7 @@ private ExitCode AnalyzeTemplate(FileInfo templateFilePath, FileInfo parametersF
                 string templateFileContents = File.ReadAllText(templateFilePath.FullName);
                 string parameterFileContents = parametersFilePath == null ? null : File.ReadAllText(parametersFilePath.FullName);
 
-                IEnumerable<IEvaluation> evaluations = this.templateAnalyzer.AnalyzeTemplate(templateFileContents, parameterFileContents, templateFilePath.FullName);
+                IEnumerable<IEvaluation> evaluations = this.templateAnalyzer.AnalyzeTemplate(templateFileContents, templateFilePath.FullName, parameterFileContents);
 
                 this.reportWriter.WriteResults(evaluations, (FileInfoBase)templateFilePath, (FileInfoBase)parametersFilePath);
 
@@ -295,7 +295,7 @@ private ExitCode SetupAnalysis(
             DirectoryInfo directoryToAnalyze,
             ReportFormat reportFormat,
             FileInfo outputFilePath,
-            bool runPowerShell,
+            bool includeNonSecurityRules,
             bool verbose)
         {
             // Output file path must be specified if SARIF was chosen as the report format
@@ -308,7 +308,7 @@ private ExitCode SetupAnalysis(
             this.reportWriter = GetReportWriter(reportFormat, outputFilePath, directoryToAnalyze?.FullName);
             CreateLoggers(verbose);
 
-            this.templateAnalyzer = TemplateAnalyzer.Create(runPowerShell, this.logger);
+            this.templateAnalyzer = TemplateAnalyzer.Create(includeNonSecurityRules, this.logger);
 
             if (!TryReadConfigurationFile(configurationFile, out var config))
             {

diff --git a/src/Analyzer.Core.NuGet/Analyzer.Core.nuspec b/src/Analyzer.Core.NuGet/Analyzer.Core.nuspec
@@ -12,12 +12,15 @@
 
     <dependencies>
       <group targetFramework="net6.0">
-        <dependency id="Azure.Deployments.Core" version="1.0.568" />
-        <dependency id="Azure.Deployments.Expression" version="1.0.568" />
-        <dependency id="Azure.Deployments.Templates" version="1.0.568" />
+        <dependency id="Azure.Bicep.Core" version="0.8.9" />
+        <dependency id="Azure.Deployments.Core" version="1.0.635" />
+        <dependency id="Azure.Deployments.Expression" version="1.0.635" />
+        <dependency id="Azure.Deployments.Templates" version="1.0.635" />
         <dependency id="Newtonsoft.Json" version="13.0.1" />
         <dependency id="Microsoft.Extensions.Logging.Abstractions" version="6.0.1" />
         <dependency id="Microsoft.PowerShell.SDK" version="7.2.4" />
+        <dependency id="Microsoft.PSRule.Rules.Azure" version="1.19.0-B0010" />
+        <dependency id="Microsoft.PSRule.SDK" version="2.4.0-B0039" />
         <dependency id="Sarif.Sdk" version="2.4.12" />
         <dependency id="System.IO.Abstractions" version="13.2.47" />
       </group>

diff --git a/src/Analyzer.Core.UnitTests/TemplateAnalyzerTests.cs b/src/Analyzer.Core.UnitTests/TemplateAnalyzerTests.cs
@@ -5,7 +5,6 @@
 using System.IO;
 using System.Linq;
 using System.Reflection;
-using Microsoft.Azure.Templates.Analyzer.RuleEngines.PowerShellEngine;
 using Microsoft.Azure.Templates.Analyzer.Types;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 
@@ -14,57 +13,47 @@ namespace Microsoft.Azure.Templates.Analyzer.Core.UnitTests
     [TestClass]
     public class TemplateAnalyzerTests
     {
-        private static TemplateAnalyzer templateAnalyzerWithPowerShell;
-        private static TemplateAnalyzer templateAnalyzerWithoutPowerShell;
+        private static TemplateAnalyzer templateAnalyzerAllRules;
+        private static TemplateAnalyzer templateAnalyzerSecurityRules;
 
         [AssemblyInitialize]
         public static void AssemblyInitialize(TestContext context)
         {
-            templateAnalyzerWithPowerShell = TemplateAnalyzer.Create(usePowerShell: true);
-            templateAnalyzerWithoutPowerShell = TemplateAnalyzer.Create(usePowerShell: false);
+            templateAnalyzerAllRules = TemplateAnalyzer.Create(true);
+            templateAnalyzerSecurityRules = TemplateAnalyzer.Create(false);
         }
 
         [DataTestMethod]
-        [DataRow(@"{ ""azureActiveDirectory"": { ""tenantId"": ""tenantId"" } }", "Microsoft.ServiceFabric/clusters", 1, 1, DisplayName = "1 matching Resource with 1 passing evaluation")]
-        [DataRow(@"{ ""azureActiveDirectory"": { ""someProperty"": ""propertyValue"" } }", "Microsoft.ServiceFabric/clusters", 1, 0, DisplayName = "1 matching Resource with 1 failing evaluation")]
-        [DataRow(@"{ ""property1"": { ""someProperty"": ""propertyValue"" } }", "Microsoft.Storage/storageAccounts", 0, 0, DisplayName = "0 matching Resources with no results")]
-        [DataRow(@"{ ""azureActiveDirectory"": { ""tenantId"": ""tenantId"" } }", "Microsoft.ServiceFabric/clusters", 2, 1, @"{ ""azureActiveDirectory"": { ""someProperty"": ""propertyValue"" } }", DisplayName = "2 matching Resources with 1 passing evaluation")]
-        [DataRow(@"{ ""azureActiveDirectory"": { ""tenantId"": ""tenantId"" } }", "Microsoft.ServiceFabric/clusters", 1, 1, null, @"aFilePath", DisplayName = "1 matching Resource with 1 passing evaluation, specifying a template file path")]
-        [DataRow(@"{ ""azureActiveDirectory"": { ""someProperty"": ""propertyValue"" } }", "Microsoft.ServiceFabric/clusters", 1, 0, null, @"anotherFilePath", DisplayName = "1 matching Resource with 1 failing evaluation, specifying a template file path")]
-        public void AnalyzeTemplate_ValidInputValues_ReturnCorrectEvaluations(string resource1Properties, string resourceType, int expectedEvaluationCount, int expectedEvaluationPassCount, string resource2Properties = null, string templateFilePath = null)
+        [DataRow(@"{ ""azureActiveDirectory"": { ""tenantId"": ""tenantId"" } }", "Microsoft.ServiceFabric/clusters", 1, 1, DisplayName = "1 matching resource with 1 passing evaluation")]
+        [DataRow(@"{ ""azureActiveDirectory"": { ""someProperty"": ""propertyValue"" } }", "Microsoft.ServiceFabric/clusters", 1, 0, DisplayName = "1 matching resource with 1 failing evaluation")]
+        [DataRow(@"{ ""property1"": { ""someProperty"": ""propertyValue"" } }", "Microsoft.MachineLearningServices/workspaces", 0, 0, DisplayName = "0 matching resources with no results")]
+        [DataRow(@"{ ""azureActiveDirectory"": { ""tenantId"": ""tenantId"" } }", "Microsoft.ServiceFabric/clusters", 2, 1, @"{ ""azureActiveDirectory"": { ""someProperty"": ""propertyValue"" } }", DisplayName = "2 matching resources with 1 passing evaluation")]
+        public void AnalyzeTemplate_ValidInputValues_ReturnCorrectEvaluations(string resource1Properties, string resourceType, int expectedEvaluationCount, int expectedEvaluationPassCount, string resource2Properties = null)
         {
             string[] resourceProperties = { GenerateResource(resource1Properties, resourceType, "resource1"), GenerateResource(resource2Properties, resourceType, "resource2") };
             string template = GenerateTemplate(resourceProperties);
 
-            var evaluations = templateAnalyzerWithoutPowerShell.AnalyzeTemplate(template, templateFilePath: templateFilePath); // A template file path is not required if PowerShell is not run
+            var evaluations = templateAnalyzerSecurityRules.AnalyzeTemplate(template, "aFilePath");
             var evaluationsWithResults = evaluations.ToList().FindAll(evaluation => evaluation.HasResults); // EvaluateRulesAgainstTemplate will always return at least an evaluation for each built-in rule
 
             Assert.AreEqual(expectedEvaluationCount, evaluationsWithResults.Count);
             Assert.AreEqual(expectedEvaluationPassCount, evaluationsWithResults.Count(e => e.Passed));
         }
 
         [TestMethod]
-        public void AnalyzeTemplate_NotUsingPowerShell_NoPowerShellViolations()
+        public void AnalyzeTemplate_RunningAllRules_ReturnsMoreEvaluations()
         {
-            // Arrange
             string[] resourceProperties = {
                 GenerateResource(
                     @"{ ""azureActiveDirectory"": { ""tenantId"": ""tenantIdValue"" } }",
                     "Microsoft.ServiceFabric/clusters", "resource1")
             };
             string template = GenerateTemplate(resourceProperties);
 
-            // Analyze with PowerShell disabled
-            var evaluations = templateAnalyzerWithoutPowerShell.AnalyzeTemplate(template);
+            var securityEvaluations = templateAnalyzerSecurityRules.AnalyzeTemplate(template, "aFilePath");
+            var allEvaluations = templateAnalyzerAllRules.AnalyzeTemplate(template, "aFilePath");
 
-            // There should be no PowerShell rule evaluations because the PowerShell engine should not have run
-            Assert.IsFalse(evaluations.Any(e => e is PowerShellRuleEvaluation));
-
-            // Analyze with PowerShell enabled
-            evaluations = templateAnalyzerWithPowerShell.AnalyzeTemplate(template, templateFilePath: "aTemplateFilePath");
-
-            // There should be at least one PowerShell rule evaluation because the PowerShell engine should have run
-            Assert.IsTrue(evaluations.Any(e => e is PowerShellRuleEvaluation));
+            Assert.IsTrue(securityEvaluations.Count() < allEvaluations.Count());
         }
 
         private string GenerateTemplate(string[] resourceProperties)
@@ -95,14 +84,14 @@ private string GenerateResource(string resourceProperties, string resourceType,
         [ExpectedException(typeof(ArgumentNullException))]
         public void AnalyzeTemplate_TemplateIsNull_ThrowArgumentNullException()
         {
-            templateAnalyzerWithPowerShell.AnalyzeTemplate(null);
+            templateAnalyzerAllRules.AnalyzeTemplate(null, "aFilePath");
         }
 
         [TestMethod]
         [ExpectedException(typeof(TemplateAnalyzerException))]
         public void AnalyzeTemplate_JsonTemplateIsInvalid_ThrowTemplateAnalyzerException()
         {
-            templateAnalyzerWithPowerShell.AnalyzeTemplate("{}");
+            templateAnalyzerAllRules.AnalyzeTemplate("{}", "aFilePath");
         }
 
         [TestMethod]
@@ -115,7 +104,7 @@ public void AnalyzeTemplate_BicepTemplateIsInvalid_ThrowTemplateAnalyzerExceptio
             try
             {
                 File.WriteAllText(templateFilePath, invalidBicep);
-                templateAnalyzerWithPowerShell.AnalyzeTemplate(invalidBicep, templateFilePath: templateFilePath);
+                templateAnalyzerAllRules.AnalyzeTemplate(invalidBicep, templateFilePath: templateFilePath);
             }
             finally
             {
@@ -124,15 +113,15 @@ public void AnalyzeTemplate_BicepTemplateIsInvalid_ThrowTemplateAnalyzerExceptio
         }
 
         [TestMethod]
-        [ExpectedException(typeof(TemplateAnalyzerException))]
-        public void AnalyzeTemplate_MissingFilePathWithPowerShellOn_ThrowTemplateAnalyzerException()
+        [ExpectedException(typeof(ArgumentNullException))]
+        public void AnalyzeTemplate_MissingFilePath_ThrowTemplateAnalyzerException()
         {
-            templateAnalyzerWithPowerShell.AnalyzeTemplate(@"
+            templateAnalyzerAllRules.AnalyzeTemplate(@"
                 {
                   ""$schema"": ""https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#"",
                   ""contentVersion"": ""1.0.0.0"",
                   ""resources"": []
-                }");
+                }", null);
         }
 
         [TestMethod]
@@ -169,7 +158,7 @@ public void FilterRules_ValidConfiguration_NoExceptionThrown()
         [ExpectedException(typeof(ArgumentNullException))]
         public void FilterRules_ConfigurationNull_ExceptionThrown()
         {
-            templateAnalyzerWithPowerShell.FilterRules(null);
+            templateAnalyzerAllRules.FilterRules(null);
         }
     }
 }