TDEAL-16: ZAP improvements

Lombiq.Tests.UI.Samples/Tests/SecurityScanningTests.cs

@@ -65,7 +65,7 @@ public Task SecurityScanWithCustomConfigurationShouldPass() =>
                 configuration => configuration
                     ////.UseAjaxSpider() // This is quite slow so just showing you here but not running it.
                     .ExcludeUrlWithRegex(".*blog.*")
-                    .DisablePassiveScanRule(10037, "Server Leaks Information via \"X-Powered-By\" HTTP Response Header Field(s)")
+                    .DisablePassiveScanRule(10020, "The response does not include either Content-Security-Policy with 'frame-ancestors' directive.")
                     .DisableScanRuleForUrlWithRegex(".*/about", 10038, "Content Security Policy (CSP) Header Not Set")
                     .SignIn(),
                 sarifLog => sarifLog.Runs[0].Results.Count.ShouldBeLessThan(34)));

Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/Baseline.yml

@@ -42,8 +42,14 @@ jobs:
     enableTags: false
     disableAllRules: false
   rules:
-  - id: 10035
-    name: "Strict-Transport-Security Header"
+  # Some stock Orchard Core shapes and pages (including the setup page) contain inline script blocks, so this directive
+  # must be permitted for OC to work at all.
+  - id: 10055
+    name: "script-src includes unsafe-inline"
+    threshold: "off"
+  # This is a low-risk alert, enforcing it is undesirable for branding.
+  - id: 10037
+    name: "Server Leaks Information via 'X-Powered-By' HTTP Response Header Field(s)"
     threshold: "off"
   name: "passiveScan-config"
   type: "passiveScan-config"

Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml

@@ -42,8 +42,14 @@ jobs:
     enableTags: false
     disableAllRules: false
   rules:
-  - id: 10035
-    name: "Strict-Transport-Security Header"
+  # Some stock Orchard Core shapes and pages (including the setup page) contain inline script blocks, so this directive
+  # must be permitted for OC to work at all.
+  - id: 10055
+    name: "script-src includes unsafe-inline"
+    threshold: "off"
+  # This is a low-risk alert, enforcing it is undesirable for branding.
+  - id: 10037
+    name: "Server Leaks Information via 'X-Powered-By' HTTP Response Header Field(s)"
     threshold: "off"
   name: "passiveScan-config"
   type: "passiveScan-config"

Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/GraphQL.yml

@@ -42,8 +42,14 @@ jobs:
     enableTags: false
     disableAllRules: false
   rules:
-  - id: 10035
-    name: "Strict-Transport-Security Header"
+  # Some stock Orchard Core shapes and pages (including the setup page) contain inline script blocks, so this directive
+  # must be permitted for OC to work at all.
+  - id: 10055
+    name: "script-src includes unsafe-inline"
+    threshold: "off"
+  # This is a low-risk alert, enforcing it is undesirable for branding.
+  - id: 10037
+    name: "Server Leaks Information via 'X-Powered-By' HTTP Response Header Field(s)"
     threshold: "off"
   name: "passiveScan-config"
   type: "passiveScan-config"

Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml

@@ -42,8 +42,14 @@ jobs:
     enableTags: false
     disableAllRules: false
   rules:
-  - id: 10035
-    name: "Strict-Transport-Security Header"
+  # Some stock Orchard Core shapes and pages (including the setup page) contain inline script blocks, so this directive
+  # must be permitted for OC to work at all.
+  - id: 10055
+    name: "script-src includes unsafe-inline"
+    threshold: "off"
+  # This is a low-risk alert, enforcing it is undesirable for branding.
+  - id: 10037
+    name: "Server Leaks Information via 'X-Powered-By' HTTP Response Header Field(s)"
     threshold: "off"
   name: "passiveScan-config"
   type: "passiveScan-config"

Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs

@@ -5,7 +5,6 @@
 using Lombiq.Tests.UI.Shortcuts.Controllers;
 using System;
 using System.Collections.Generic;
-using System.Linq;
 using System.Threading.Tasks;
 using YamlDotNet.RepresentationModel;
 
@@ -23,17 +22,18 @@ namespace Lombiq.Tests.UI.SecurityScanning;
 /// </remarks>
 public class SecurityScanConfiguration
 {
+    private readonly List<Uri> _additionalUris = new();
+    private readonly List<string> _excludedUrlRegexPatterns = new();
+    private readonly List<ScanRule> _disabledActiveScanRules = new();
+    private readonly Dictionary<ScanRule, (ScanRuleThreshold Threshold, ScanRuleStrength Strength)> _configuredActiveScanRules = new();
+    private readonly List<ScanRule> _disabledPassiveScanRules = new();
+    private readonly List<(string Url, int Id, string RuleName)> _disabledRulesForUrls = new();
+    private readonly List<(string Url, int Id, string Justification)> _falsePositives = new();
+    private readonly List<Func<YamlDocument, Task>> _zapPlanModifiers = new();
+
     public Uri StartUri { get; private set; }
-    public IList<Uri> AdditionalUris { get; } = new List<Uri>();
     public bool AjaxSpiderIsUsed { get; private set; }
     public string SignInUserName { get; private set; }
-    public IList<string> ExcludedUrlRegexPatterns { get; } = new List<string>();
-    public IList<ScanRule> DisabledActiveScanRules { get; } = new List<ScanRule>();
-    public IDictionary<ScanRule, (ScanRuleThreshold Threshold, ScanRuleStrength Strength)> ConfiguredActiveScanRules { get; } =
-        new Dictionary<ScanRule, (ScanRuleThreshold, ScanRuleStrength)>();
-    public IList<ScanRule> DisabledPassiveScanRules { get; } = new List<ScanRule>();
-    public IDictionary<string, ScanRule> DisabledRulesForUrls { get; } = new Dictionary<string, ScanRule>();
-    public IList<Func<YamlDocument, Task>> ZapPlanModifiers { get; } = new List<Func<YamlDocument, Task>>();
 
     internal SecurityScanConfiguration()
     {
@@ -56,7 +56,7 @@ public SecurityScanConfiguration StartAtUri(Uri startUri)
     /// <param name="additionalUri">The <see cref="Uri"/> under the app to also cover during the scan.</param>
     public SecurityScanConfiguration AddAdditionalUri(Uri additionalUri)
     {
-        AdditionalUris.Add(additionalUri);
+        _additionalUris.Add(additionalUri);
         return this;
     }
 
@@ -90,7 +90,7 @@ public SecurityScanConfiguration SignIn(string userName = DefaultUser.UserName)
     /// </param>
     public SecurityScanConfiguration ExcludeUrlWithRegex(string excludedUrlRegex)
     {
-        ExcludedUrlRegexPatterns.Add(excludedUrlRegex);
+        _excludedUrlRegexPatterns.Add(excludedUrlRegex);
         return this;
     }
 
@@ -105,7 +105,7 @@ public SecurityScanConfiguration ExcludeUrlWithRegex(string excludedUrlRegex)
     /// </param>
     public SecurityScanConfiguration DisableActiveScanRule(int id, string name = "")
     {
-        DisabledActiveScanRules.Add(new ScanRule(id, name));
+        _disabledActiveScanRules.Add(new ScanRule(id, name));
         return this;
     }
 
@@ -127,7 +127,7 @@ public SecurityScanConfiguration DisableActiveScanRule(int id, string name = "")
     /// </param>
     public SecurityScanConfiguration ConfigureActiveScanRule(int id, ScanRuleThreshold threshold, ScanRuleStrength strength, string name = "")
     {
-        ConfiguredActiveScanRules.Add(new ScanRule(id, name), (threshold, strength));
+        _configuredActiveScanRules.Add(new ScanRule(id, name), (threshold, strength));
         return this;
     }
 
@@ -142,7 +142,7 @@ public SecurityScanConfiguration ConfigureActiveScanRule(int id, ScanRuleThresho
     /// </param>
     public SecurityScanConfiguration DisablePassiveScanRule(int id, string name = "")
     {
-        DisabledPassiveScanRules.Add(new ScanRule(id, name));
+        _disabledPassiveScanRules.Add(new ScanRule(id, name));
         return this;
     }
 
@@ -161,7 +161,30 @@ public SecurityScanConfiguration DisablePassiveScanRule(int id, string name = ""
     /// </param>
     public SecurityScanConfiguration DisableScanRuleForUrlWithRegex(string urlRegex, int ruleId, string ruleName = "")
     {
-        DisabledRulesForUrls[urlRegex] = new ScanRule(ruleId, ruleName);
+        _disabledRulesForUrls.Add((urlRegex, ruleId, ruleName));
+        return this;
+    }
+
+    /// <summary>
+    /// Marks a rule (can be any rule, including e.g. both active or passive scan rules) for just URLs matching the
+    /// given regular expression pattern.
+    /// </summary>
+    /// <param name="urlRegex">
+    /// The regex pattern to match URLs against. It will be matched against the whole absolute URL, e.g., ".*blog.*"
+    /// will match https://example.com/blog, https://example.com/blog/my-post, etc.
+    /// </param>
+    /// <param name="ruleId">The ID of the rule. In the scan report, this is usually displayed as "Plugin Id".</param>
+    /// <param name="justification">
+    /// A human-readable explanation of why the alert is false positive.
+    /// </param>
+    public SecurityScanConfiguration MarkScanRuleAsFalsePositiveForUrlWithRegex(string urlRegex, int ruleId, string justification)
+    {
+        if (string.IsNullOrWhiteSpace(justification))
+        {
+            throw new InvalidOperationException("Please provide a justification for disabling this alert!");
+        }
+
+        _falsePositives.Add((urlRegex, ruleId, justification));
         return this;
     }
 
@@ -176,7 +199,7 @@ public SecurityScanConfiguration DisableScanRuleForUrlWithRegex(string urlRegex,
     /// </param>
     public SecurityScanConfiguration ModifyZapPlan(Func<YamlDocument, Task> modifyPlan)
     {
-        ZapPlanModifiers.Add(modifyPlan);
+        _zapPlanModifiers.Add(modifyPlan);
         return this;
     }
 
@@ -191,7 +214,7 @@ public SecurityScanConfiguration ModifyZapPlan(Func<YamlDocument, Task> modifyPl
     /// </param>
     public SecurityScanConfiguration ModifyZapPlan(Action<YamlDocument> modifyPlan)
     {
-        ZapPlanModifiers.Add(yamlDocument =>
+        _zapPlanModifiers.Add(yamlDocument =>
         {
             modifyPlan(yamlDocument);
             return Task.CompletedTask;
@@ -204,7 +227,7 @@ internal async Task ApplyToPlanAsync(YamlDocument yamlDocument, UITestContext co
     {
         yamlDocument.SetStartUrl(StartUri);
 
-        foreach (var uri in AdditionalUris) yamlDocument.AddUrl(uri);
+        foreach (var uri in _additionalUris) yamlDocument.AddUrl(uri);
 
         if (AjaxSpiderIsUsed) yamlDocument.AddSpiderAjaxAfterSpider();
 
@@ -234,10 +257,10 @@ internal async Task ApplyToPlanAsync(YamlDocument yamlDocument, UITestContext co
             //   pollPostData: ""
         }
 
-        yamlDocument.AddExcludePathsRegex(ExcludedUrlRegexPatterns.ToArray());
-        foreach (var rule in DisabledActiveScanRules) yamlDocument.DisableActiveScanRule(rule.Id, rule.Name);
+        yamlDocument.AddExcludePathsRegex(_excludedUrlRegexPatterns.ToArray());
+        foreach (var rule in _disabledActiveScanRules) yamlDocument.DisableActiveScanRule(rule.Id, rule.Name);
 
-        foreach (var ruleConfiguration in ConfiguredActiveScanRules)
+        foreach (var ruleConfiguration in _configuredActiveScanRules)
         {
             yamlDocument.ConfigureActiveScanRule(
                 ruleConfiguration.Key.Id,
@@ -246,9 +269,10 @@ internal async Task ApplyToPlanAsync(YamlDocument yamlDocument, UITestContext co
                 ruleConfiguration.Key.Name);
         }
 
-        foreach (var rule in DisabledPassiveScanRules) yamlDocument.DisablePassiveScanRule(rule.Id, rule.Name);
-        foreach (var urlToRule in DisabledRulesForUrls) yamlDocument.AddAlertFilter(urlToRule.Key, urlToRule.Value.Id, urlToRule.Value.Name);
-        foreach (var modifier in ZapPlanModifiers) await modifier(yamlDocument);
+        foreach (var rule in _disabledPassiveScanRules) yamlDocument.DisablePassiveScanRule(rule.Id, rule.Name);
+        foreach (var (url, id, name) in _disabledRulesForUrls) yamlDocument.AddAlertFilter(url, id, name);
+        foreach (var (url, id, justification) in _falsePositives) yamlDocument.AddAlertFilter(url, id, justification, isFalsePositive: true);
+        foreach (var modifier in _zapPlanModifiers) await modifier(yamlDocument);
     }
 
     public class ScanRule

Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs

@@ -1,6 +1,8 @@
+using Lombiq.HelpfulLibraries.OrchardCore.Mvc;
 using Lombiq.Tests.UI.Exceptions;
 using Lombiq.Tests.UI.Extensions;
 using Lombiq.Tests.UI.Services;
+using Lombiq.Tests.UI.Shortcuts.Controllers;
 using Microsoft.CodeAnalysis.Sarif;
 using System;
 using System.Threading.Tasks;
@@ -25,7 +27,14 @@ public static Task RunAndAssertBaselineSecurityScanAsync(
         Action<SarifLog> assertSecurityScanResult = null) =>
         context.RunAndAssertSecurityScanAsync(
             AutomationFrameworkPlanPaths.BaselinePlanPath,
-            configure,
+            configuration =>
+            {
+                // Make sure to visit at least one page that throws an exception.
+                var errorPageRelativeUrl = context.GetRelativeUrlOfAction<ErrorController>(error => error.Index());
+                configuration.ModifyZapPlan(plan => plan.AddUrl(context.GetAbsoluteUri(errorPageRelativeUrl)));
+
+                configure?.Invoke(configuration);
+            },
             assertSecurityScanResult);
 
     /// <summary>
@@ -107,6 +116,11 @@ public static async Task RunAndAssertSecurityScanAsync(
         Action<SecurityScanConfiguration> configure = null,
         Action<SarifLog> assertSecurityScanResult = null)
     {
+        // Verify that the app logs are fine right now, then disable app log assertion for the duration of this scan.
+        await context.Configuration.AssertAppLogsAsync(context.Application);
+        var assertAppLogsAsync = context.Configuration.AssertAppLogsAsync;
+        context.Configuration.AssertAppLogsAsync = _ => Task.CompletedTask;
+
         var configuration = context.Configuration.SecurityScanningConfiguration;
 
         SecurityScanResult result = null;
@@ -124,6 +138,10 @@ public static async Task RunAndAssertSecurityScanAsync(
             if (result != null) context.AppendDirectoryToFailureDump(result.ReportsDirectoryPath);
             throw new SecurityScanningAssertionException(ex);
         }
+
+        // Clear app logs before app log assertion is restored.
+        context.ClearLogs();
+        context.Configuration.AssertAppLogsAsync = assertAppLogsAsync;
     }
 
     /// <summary>
@@ -147,6 +165,10 @@ public static Task<SecurityScanResult> RunSecurityScanAsync(
 
         configuration.StartAtUri(context.GetCurrentUri());
 
+        // By default ignore /vendor/ or /vendors/ URLs. This is case-insensitive. We have no control over them, and
+        // they may contain several false positives (e.g. in font-awesome)..
+        configuration.ExcludeUrlWithRegex(@".*/vendors?/.*");
+
         if (context.Configuration.SecurityScanningConfiguration.ZapAutomationFrameworkPlanModifier != null)
         {
             configuration.ModifyZapPlan(async plan =>