From 8373d75aece5e9d2b1c86e6582ea9ffeea3879e2 Mon Sep 17 00:00:00 2001 From: Gabe Stocco <98900+gfs@users.noreply.github.com> Date: Wed, 1 Mar 2023 16:30:51 -0800 Subject: [PATCH 01/13] Adds DependsOnTags field to Rule After processing of rules DependsOnTags will be checked and then matches whose matching rule have required tags which are not all present in the UniqueTags will be removed before returning results. Adds tests for new functionality and rule verifier will check to make sure all tags which are depended on are present in the rule set. SemVer changes. Public properties in the Rule object have been changed to IList from a combination of Array and List. --- AppInspector.Benchmarks/DistinctBenchmarks.cs | 8 +- AppInspector.Common/MsgHelp.cs | 3 +- .../Properties/Resources.Designer.cs | 3 +- AppInspector.Common/Properties/Resources.resx | 3 + AppInspector.RulesEngine/AbstractRuleSet.cs | 4 +- AppInspector.RulesEngine/MatchRecord.cs | 3 +- AppInspector.RulesEngine/Rule.cs | 25 +- AppInspector.RulesEngine/RuleProcessor.cs | 8 +- AppInspector.RulesEngine/RulesVerifier.cs | 22 +- AppInspector.Tests/Commands/TestAnalyzeCmd.cs | 219 +++++++++++++++++- .../Commands/TestVerifyRulesCmd.cs | 42 ++++ AppInspector/Commands/AnalyzeCommand.cs | 28 ++- AppInspector/Commands/ExportTagsCommand.cs | 2 +- AppInspector/MetaDataHelper.cs | 89 +++---- version.json | 2 +- 15 files changed, 374 insertions(+), 87 deletions(-) diff --git a/AppInspector.Benchmarks/DistinctBenchmarks.cs b/AppInspector.Benchmarks/DistinctBenchmarks.cs index 2c9c5e62..be8ccd1a 100644 --- a/AppInspector.Benchmarks/DistinctBenchmarks.cs +++ b/AppInspector.Benchmarks/DistinctBenchmarks.cs @@ -19,7 +19,7 @@ public List OldCode() List outList = new(); foreach (var r in ruleSet) //builds a list of unique tags - foreach (var t in r?.Tags ?? Array.Empty()) + foreach (var t in (IList?)r?.Tags ?? Array.Empty()) if (uniqueTags.ContainsKey(t)) { continue; @@ -41,7 +41,7 @@ public List HashSet() HashSet hashSet = new(); foreach (var r in ruleSet) //builds a list of unique tags - foreach (var t in r?.Tags ?? Array.Empty()) + foreach (var t in (IList?)r?.Tags ?? Array.Empty()) hashSet.Add(t); var theList = hashSet.ToList(); @@ -52,14 +52,14 @@ public List HashSet() [Benchmark] public List WithLinq() { - return ruleSet.SelectMany(x => x.Tags ?? Array.Empty()).Distinct().OrderBy(x => x) + return ruleSet.SelectMany(x => (IList?)x.Tags ?? Array.Empty()).Distinct().OrderBy(x => x) .ToList(); } [Benchmark] public List WithLinqAndHashSet() { - var theList = ruleSet.SelectMany(x => x.Tags ?? Array.Empty()).ToHashSet().ToList(); + var theList = ruleSet.SelectMany(x => (IList?)x.Tags ?? Array.Empty()).ToHashSet().ToList(); theList.Sort(); return theList; } diff --git a/AppInspector.Common/MsgHelp.cs b/AppInspector.Common/MsgHelp.cs index 38149137..34e8750d 100644 --- a/AppInspector.Common/MsgHelp.cs +++ b/AppInspector.Common/MsgHelp.cs @@ -77,7 +77,8 @@ public enum ID BROWSER_START_SUCCESS, PACK_MISSING_OUTPUT_ARG, PACK_RULES_NO_CLI_DEFAULT, - PACK_RULES_NO_DEFAULT + PACK_RULES_NO_DEFAULT, + VERIFY_RULES_DEPENDS_ON_TAG_MISSING } public static string GetString(ID id) diff --git a/AppInspector.Common/Properties/Resources.Designer.cs b/AppInspector.Common/Properties/Resources.Designer.cs index b4e7f5ac..431c38e8 100644 --- a/AppInspector.Common/Properties/Resources.Designer.cs +++ b/AppInspector.Common/Properties/Resources.Designer.cs @@ -1,6 +1,7 @@ //------------------------------------------------------------------------------ // // This code was generated by a tool. +// Runtime Version:4.0.30319.42000 // // Changes to this file may cause incorrect behavior and will be lost if // the code is regenerated. @@ -584,7 +585,7 @@ internal static string VERIFY_RULE_LOADFILE_FAILED { } /// - /// Looks up a localized string similar to Rule {0} failed from dupicate rule id specified. + /// Looks up a localized string similar to Rule {0} failed verification. depends_on_tags is set but the specified tags are not set by any rules in the set.. /// internal static string VERIFY_RULES_DUPLICATEID_FAIL { get { diff --git a/AppInspector.Common/Properties/Resources.resx b/AppInspector.Common/Properties/Resources.resx index 80ca370c..6896b83a 100644 --- a/AppInspector.Common/Properties/Resources.resx +++ b/AppInspector.Common/Properties/Resources.resx @@ -301,6 +301,9 @@ Rule {0} failed from dupicate rule id specified + + + Rule {0} failed verification. depends_on_tags is set but the specified tags are not set by any rules in the set. Rule {0} failed from unrecognized language {1} specified diff --git a/AppInspector.RulesEngine/AbstractRuleSet.cs b/AppInspector.RulesEngine/AbstractRuleSet.cs index d1bd665c..cbf4e7d1 100644 --- a/AppInspector.RulesEngine/AbstractRuleSet.cs +++ b/AppInspector.RulesEngine/AbstractRuleSet.cs @@ -63,8 +63,8 @@ public IEnumerable ByFilename(string input) public IEnumerable GetUniversalRules() { return _oatRules.Where(x => - (x.AppInspectorRule.FileRegexes is null || x.AppInspectorRule.FileRegexes.Length == 0) && - (x.AppInspectorRule.AppliesTo is null || x.AppInspectorRule.AppliesTo.Length == 0)); + (x.AppInspectorRule.FileRegexes is null || x.AppInspectorRule.FileRegexes.Count == 0) && + (x.AppInspectorRule.AppliesTo is null || x.AppInspectorRule.AppliesTo.Count == 0)); } /// diff --git a/AppInspector.RulesEngine/MatchRecord.cs b/AppInspector.RulesEngine/MatchRecord.cs index fd69bc88..7718d6c2 100644 --- a/AppInspector.RulesEngine/MatchRecord.cs +++ b/AppInspector.RulesEngine/MatchRecord.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. See LICENSE.txt in the project root for license information. using System.Diagnostics.CodeAnalysis; +using System.Linq; using System.Text.Json.Serialization; namespace Microsoft.ApplicationInspector.RulesEngine; @@ -21,7 +22,7 @@ public MatchRecord(Rule rule) RuleId = rule.Id; RuleName = rule.Name; RuleDescription = rule.Description; - Tags = rule.Tags; + Tags = rule.Tags?.ToArray(); Severity = rule.Severity; } diff --git a/AppInspector.RulesEngine/Rule.cs b/AppInspector.RulesEngine/Rule.cs index 0b95091b..4b7a7f6c 100644 --- a/AppInspector.RulesEngine/Rule.cs +++ b/AppInspector.RulesEngine/Rule.cs @@ -14,9 +14,9 @@ namespace Microsoft.ApplicationInspector.RulesEngine; /// public class Rule { - private IEnumerable _compiled = Array.Empty(); + private IList _compiled = Array.Empty(); - private string[]? _fileRegexes; + private IList? _fileRegexes; private bool _updateCompiledFileRegex; /// @@ -38,6 +38,11 @@ public class Rule [JsonIgnore] public bool Disabled { get; set; } + /// + /// Tags that are required to be present from other rules - regardless of file - for this match to be valid + /// + [JsonPropertyName("depends_on_tags")] public IList? DependsOnTags { get; set; } + [JsonPropertyName("name")] public string Name { get; set; } = ""; [JsonPropertyName("id")] public string Id { get; set; } = ""; @@ -46,13 +51,13 @@ public class Rule public string? Description { get; set; } = ""; [JsonPropertyName("does_not_apply_to")] - public List? DoesNotApplyTo { get; set; } + public IList? DoesNotApplyTo { get; set; } [JsonPropertyName("applies_to")] - public string[]? AppliesTo { get; set; } + public IList? AppliesTo { get; set; } [JsonPropertyName("applies_to_file_regex")] - public string[]? FileRegexes + public IList? FileRegexes { get => _fileRegexes; set @@ -69,7 +74,7 @@ public IEnumerable CompiledFileRegexes { if (_updateCompiledFileRegex) { - _compiled = FileRegexes?.Select(x => new Regex(x, RegexOptions.Compiled)) ?? Array.Empty(); + _compiled = (IList?)FileRegexes?.Select(x => new Regex(x, RegexOptions.Compiled)).ToList() ?? Array.Empty(); _updateCompiledFileRegex = false; } @@ -77,14 +82,14 @@ public IEnumerable CompiledFileRegexes } } - [JsonPropertyName("tags")] public string[]? Tags { get; set; } + [JsonPropertyName("tags")] public IList? Tags { get; set; } [JsonPropertyName("severity")] [JsonConverter(typeof(JsonStringEnumConverter))] public Severity Severity { get; set; } = Severity.Moderate; [JsonPropertyName("overrides")] - public string[]? Overrides { get; set; } + public IList? Overrides { get; set; } [JsonPropertyName("patterns")] public SearchPattern[] Patterns { get; set; } = Array.Empty(); @@ -93,8 +98,8 @@ public IEnumerable CompiledFileRegexes public SearchCondition[]? Conditions { get; set; } [JsonPropertyName("must-match")] - public string[]? MustMatch { get; set; } + public IList? MustMatch { get; set; } [JsonPropertyName("must-not-match")] - public string[]? MustNotMatch { get; set; } + public IList? MustNotMatch { get; set; } } \ No newline at end of file diff --git a/AppInspector.RulesEngine/RuleProcessor.cs b/AppInspector.RulesEngine/RuleProcessor.cs index f3368aa1..4c5513e4 100644 --- a/AppInspector.RulesEngine/RuleProcessor.cs +++ b/AppInspector.RulesEngine/RuleProcessor.cs @@ -229,8 +229,8 @@ public List AnalyzeFile(TextContainer textContainer, FileEntry file List removes = new(); - foreach (var m in resultsList.Where(x => x.Rule?.Overrides?.Length > 0)) - foreach (var idsToOverride in m.Rule?.Overrides ?? Array.Empty()) + foreach (var m in resultsList.Where(x => x.Rule?.Overrides?.Count > 0)) + foreach (var idsToOverride in (IList)m.Rule?.Overrides ?? Array.Empty()) // Find all overriden rules and mark them for removal from issues list foreach (var om in resultsList.FindAll(x => x.Rule?.Id == idsToOverride)) // If the overridden match is a subset of the overriding match @@ -416,14 +416,14 @@ List ProcessBoundary(ClauseCapture cap) List removes = new(); - foreach (var m in resultsList.Where(x => x.Rule?.Overrides?.Length > 0)) + foreach (var m in resultsList.Where(x => x.Rule?.Overrides?.Count > 0)) { if (cancellationToken?.IsCancellationRequested is true) { return resultsList; } - foreach (var ovrd in m.Rule?.Overrides ?? Array.Empty()) + foreach (var ovrd in (IList?)m.Rule?.Overrides ?? Array.Empty()) // Find all overriden rules and mark them for removal from issues list foreach (var om in resultsList.FindAll(x => x.Rule?.Id == ovrd)) if (om.Boundary.Index >= m.Boundary.Index && diff --git a/AppInspector.RulesEngine/RulesVerifier.cs b/AppInspector.RulesEngine/RulesVerifier.cs index 25fab0ee..ba9859af 100644 --- a/AppInspector.RulesEngine/RulesVerifier.cs +++ b/AppInspector.RulesEngine/RulesVerifier.cs @@ -92,6 +92,16 @@ public List CheckIntegrity(AbstractRuleSet ruleSet) } } + var allTags = ruleSet.GetAppInspectorRules().SelectMany(x => x.Tags ?? Array.Empty()).ToList(); + var rulesWithDependsOnWithNoMatchingTags = ruleSet.GetAppInspectorRules().Where(x => !x.DependsOnTags?.All(tag => allTags.Contains(tag)) ?? true); + foreach(var dependslessRule in rulesWithDependsOnWithNoMatchingTags) + { + _logger.LogError(MsgHelp.GetString(MsgHelp.ID.VERIFY_RULES_DEPENDS_ON_TAG_MISSING), dependslessRule.Id); + foreach(var status in ruleStatuses.Where(x => x.RulesId == dependslessRule.Id)) + { + status.Errors = status.Errors.Append(MsgHelp.FormatString(MsgHelp.ID.VERIFY_RULES_DEPENDS_ON_TAG_MISSING, dependslessRule.Id)); + } + } return ruleStatuses; } @@ -124,7 +134,7 @@ public RuleStatus CheckIntegrity(ConvertedOatRule convertedOatRule) } } - foreach (var pattern in rule.FileRegexes ?? Array.Empty()) + foreach (var pattern in (IList?)rule.FileRegexes ?? Array.Empty()) try { _ = new Regex(pattern, RegexOptions.Compiled); @@ -273,7 +283,7 @@ public RuleStatus CheckIntegrity(ConvertedOatRule convertedOatRule) StringComparer.InvariantCultureIgnoreCase) ?? true) ?? "csharp"; // validate all must match samples are matched - foreach (var mustMatchElement in rule.MustMatch ?? Array.Empty()) + foreach (var mustMatchElement in (IList?)rule.MustMatch ?? Array.Empty()) { var tc = new TextContainer(mustMatchElement, language, _options.LanguageSpecs); if (!_analyzer.Analyze(singleList, tc).Any()) @@ -285,7 +295,7 @@ public RuleStatus CheckIntegrity(ConvertedOatRule convertedOatRule) } // validate no must not match conditions are matched - foreach (var mustNotMatchElement in rule.MustNotMatch ?? Array.Empty()) + foreach (var mustNotMatchElement in (IList?)rule.MustNotMatch ?? Array.Empty()) { var tc = new TextContainer(mustNotMatchElement, language, _options.LanguageSpecs); if (_analyzer.Analyze(singleList, tc).Any()) @@ -296,7 +306,7 @@ public RuleStatus CheckIntegrity(ConvertedOatRule convertedOatRule) } } - if (rule.Tags?.Length == 0) + if (rule.Tags?.Count == 0) { _logger?.LogError("Rule must specify tags. {0}", rule.Id); errors.Add($"Rule must specify tags. {rule.Id}"); @@ -326,8 +336,8 @@ public RuleStatus CheckIntegrity(ConvertedOatRule convertedOatRule) RulesName = rule.Name, Errors = errors, OatIssues = _analyzer.EnumerateRuleIssues(convertedOatRule), - HasPositiveSelfTests = rule.MustMatch?.Length > 0, - HasNegativeSelfTests = rule.MustNotMatch?.Length > 0 + HasPositiveSelfTests = rule.MustMatch?.Count > 0, + HasNegativeSelfTests = rule.MustNotMatch?.Count > 0 }; } } \ No newline at end of file diff --git a/AppInspector.Tests/Commands/TestAnalyzeCmd.cs b/AppInspector.Tests/Commands/TestAnalyzeCmd.cs index 8ab33747..3658bad5 100644 --- a/AppInspector.Tests/Commands/TestAnalyzeCmd.cs +++ b/AppInspector.Tests/Commands/TestAnalyzeCmd.cs @@ -25,6 +25,107 @@ public class TestAnalyzeCmd private const int numTimeOutFiles = 25; private const int numTimesContent = 25; + + private const string dependsOnOneWay = @"[ + { + ""id"": ""SA000005"", + ""name"": ""Testing.Rules.DependsOnTags.OneWay"", + ""tags"": [ + ""Dependant"" + ], + ""depends_on_tags"": [""Dependee""], + ""severity"": ""Critical"", + ""description"": ""This rule finds windows 2000"", + ""patterns"": [ + { + ""pattern"": ""windows 2000"", + ""type"": ""regex"", + ""confidence"": ""High"", + ""modifiers"": [ + ""m"" + ], + ""scopes"": [ + ""code"" + ] + } + ], + ""_comment"": """" + }, + { + ""id"": ""SA000006"", + ""name"": ""Testing.Rules.DependsOnTags.OneWay"", + ""tags"": [ + ""Dependee"" + ], + ""severity"": ""Critical"", + ""description"": ""This rule finds linux"", + ""patterns"": [ + { + ""pattern"": ""linux"", + ""type"": ""regex"", + ""confidence"": ""High"", + ""modifiers"": [ + ""m"" + ], + ""scopes"": [ + ""code"" + ] + } + ], + ""_comment"": """" + } +]"; + private const string dependsOnTwoWay = @"[ + { + ""id"": ""SA000005"", + ""name"": ""Testing.Rules.DependsOnTags.TwoWay"", + ""tags"": [ + ""RuleOne"" + ], + ""depends_on_tags"": [""RuleTwo""], + ""severity"": ""Critical"", + ""description"": ""This rule finds windows 2000"", + ""patterns"": [ + { + ""pattern"": ""windows 2000"", + ""type"": ""regex"", + ""confidence"": ""High"", + ""modifiers"": [ + ""m"" + ], + ""scopes"": [ + ""code"" + ] + } + ], + ""_comment"": """" + }, + { + ""id"": ""SA000006"", + ""name"": ""Testing.Rules.DependsOnTags.TwoWay"", + ""tags"": [ + ""RuleTwo"" + ], + ""depends_on_tags"": [""RuleOne""], + ""severity"": ""Critical"", + ""description"": ""This rule finds linux"", + ""patterns"": [ + { + ""pattern"": ""linux"", + ""type"": ""regex"", + ""confidence"": ""High"", + ""modifiers"": [ + ""m"" + ], + ""scopes"": [ + ""code"" + ] + } + ], + ""_comment"": """" + } +]"; + private const string hardToFindContent = @" asefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlakasefljkajsdfklasjdfklasjdfklasdfjklasdjfaklsdfjaklsdjfaklsfaksdjfkasdasdklfalskdfjalskdjfalskdjflaksdjflaskjdflaksjdflaksjdfljaskldfjjdkfaklsdfjlak@company.com1 buy@tacos.com @@ -289,6 +390,8 @@ windows 2000 private static string testRulesPath = string.Empty; private static string appliesToTestRulePath = string.Empty; private static string doesNotApplyToTestRulePath = string.Empty; + private static string dependsOnOneWayRulePath; + private static string dependsOnTwoWayRulePath; // Test files for timeout tests private static readonly List enumeratingTimeOutTestsFiles = new(); @@ -296,6 +399,7 @@ windows 2000 private static string heavyRulePath = string.Empty; private ILoggerFactory factory = new NullLoggerFactory(); + private static string fourWindowsOne2000Path; [ClassInitialize] public static void ClassInit(TestContext context) @@ -308,13 +412,21 @@ public static void ClassInit(TestContext context) Path.Combine(TestHelpers.GetPath(TestHelpers.AppPath.testOutput), "AppliesToTestRules.json"); doesNotApplyToTestRulePath = Path.Combine(TestHelpers.GetPath(TestHelpers.AppPath.testOutput), "DoesNotApplyToTestRules.json"); - + dependsOnOneWayRulePath = Path.Combine(TestHelpers.GetPath(TestHelpers.AppPath.testOutput), + "DependsOnOneWay.json"); + dependsOnTwoWayRulePath = Path.Combine(TestHelpers.GetPath(TestHelpers.AppPath.testOutput), + "DependsOnTwoWay.json"); File.WriteAllText(heavyRulePath, heavyRule); File.WriteAllText(testFilePath, fourWindowsOneLinux); File.WriteAllText(testRulesPath, findWindows); File.WriteAllText(appliesToTestRulePath, findWindowsWithAppliesTo); File.WriteAllText(doesNotApplyToTestRulePath, findWindowsWithDoesNotApplyTo); + File.WriteAllText(dependsOnOneWayRulePath, dependsOnOneWay); + File.WriteAllText(dependsOnTwoWayRulePath, dependsOnTwoWay); + fourWindowsOne2000Path = + Path.Combine(TestHelpers.GetPath(TestHelpers.AppPath.testOutput), "FourWindowsOne2000.cs"); + File.WriteAllText(fourWindowsOne2000Path, threeWindowsOneWindows2000); for (var i = 0; i < numTimeOutFiles; i++) { var newPath = Path.Combine(TestHelpers.GetPath(TestHelpers.AppPath.testOutput), $"TestFile-{i}.js"); @@ -349,10 +461,7 @@ public void Overrides() Path.Combine(TestHelpers.GetPath(TestHelpers.AppPath.testOutput), "OverrideTestRule.json"); var overridesWithoutOverrideTestRulePath = Path.Combine(TestHelpers.GetPath(TestHelpers.AppPath.testOutput), "OverrideTestRuleWithoutOverride.json"); - var fourWindowsOne2000Path = - Path.Combine(TestHelpers.GetPath(TestHelpers.AppPath.testOutput), "FourWindowsOne2000.cs"); - File.WriteAllText(fourWindowsOne2000Path, threeWindowsOneWindows2000); File.WriteAllText(overridesTestRulePath, findWindowsWithOverride); File.WriteAllText(overridesWithoutOverrideTestRulePath, findWindowsWithOverrideRuleWithoutOverride); @@ -1026,6 +1135,108 @@ public void TestAppliesTo() Assert.AreEqual(4, result.Metadata.TotalMatchesCount); } + /// + /// Test that the depends_on rule parameter properly limits matches one way + /// + [TestMethod] + public void TestDependsOnOneWay() + { + AnalyzeOptions options = new() + { + SourcePath = new string[] { testFilePath, fourWindowsOne2000Path }, + CustomRulesPath = dependsOnOneWayRulePath, + IgnoreDefaultRules = true + }; + + AnalyzeCommand command = new(options, factory); + var result = command.GetResult(); + Assert.AreEqual(AnalyzeResult.ExitCode.Success, result.ResultCode); + Assert.AreEqual(2, result.Metadata.TotalMatchesCount); + Assert.IsTrue(result.Metadata.UniqueTags.Contains("Dependee")); + Assert.IsTrue(result.Metadata.UniqueTags.Contains("Dependant")); + } + + /// + /// Test that the depends_on rule parameter properly limits matches one way + /// + [TestMethod] + public void TestDependsOnOneWayWithoutDependee() + { + AnalyzeOptions options = new() + { + SourcePath = new string[] { testFilePath }, + CustomRulesPath = dependsOnOneWayRulePath, + IgnoreDefaultRules = true + }; + + AnalyzeCommand command = new(options, factory); + var result = command.GetResult(); + Assert.AreEqual(AnalyzeResult.ExitCode.Success, result.ResultCode); + Assert.AreEqual(1, result.Metadata.TotalMatchesCount); + Assert.IsTrue(result.Metadata.UniqueTags.Contains("Dependee")); + Assert.IsFalse(result.Metadata.UniqueTags.Contains("Dependant")); + } + + /// + /// Test that the depends_on rule parameter properly limits matches two ways + /// + [TestMethod] + public void TestDependsOnTwoWay() + { + AnalyzeOptions options = new() + { + SourcePath = new string[] { testFilePath, fourWindowsOne2000Path }, + CustomRulesPath = dependsOnTwoWayRulePath, + IgnoreDefaultRules = true + }; + + AnalyzeCommand command = new(options, factory); + var result = command.GetResult(); + Assert.AreEqual(AnalyzeResult.ExitCode.Success, result.ResultCode); + Assert.AreEqual(2, result.Metadata.TotalMatchesCount); + Assert.IsTrue(result.Metadata.UniqueTags.Contains("RuleOne")); + Assert.IsTrue(result.Metadata.UniqueTags.Contains("RuleTwo")); + } + + /// + /// Test that the depends_on rule parameter properly limits matches two ways + /// + [TestMethod] + public void TestDependsOnTwoWayWithoutDependee() + { + AnalyzeOptions options = new() + { + SourcePath = new string[] { testFilePath }, + CustomRulesPath = dependsOnTwoWayRulePath, + IgnoreDefaultRules = true + }; + + AnalyzeCommand command = new(options, factory); + var result = command.GetResult(); + Assert.AreEqual(AnalyzeResult.ExitCode.NoMatches, result.ResultCode); + Assert.AreEqual(0, result.Metadata.TotalMatchesCount); + Assert.IsFalse(result.Metadata.UniqueTags.Contains("RuleOne")); + Assert.IsFalse(result.Metadata.UniqueTags.Contains("RuleTwo")); + } + + [TestMethod] + public void TestDependsOnTwoWayWithoutDependant() + { + AnalyzeOptions options = new() + { + SourcePath = new string[] { fourWindowsOne2000Path }, + CustomRulesPath = dependsOnTwoWayRulePath, + IgnoreDefaultRules = true + }; + + AnalyzeCommand command = new(options, factory); + var result = command.GetResult(); + Assert.AreEqual(AnalyzeResult.ExitCode.NoMatches, result.ResultCode); + Assert.AreEqual(0, result.Metadata.TotalMatchesCount); + Assert.IsFalse(result.Metadata.UniqueTags.Contains("RuleOne")); + Assert.IsFalse(result.Metadata.UniqueTags.Contains("RuleTwo")); + } + /// /// Test that the does_not_apply_to parameter excludes the specified types /// diff --git a/AppInspector.Tests/Commands/TestVerifyRulesCmd.cs b/AppInspector.Tests/Commands/TestVerifyRulesCmd.cs index b3d030c5..a9954b4c 100644 --- a/AppInspector.Tests/Commands/TestVerifyRulesCmd.cs +++ b/AppInspector.Tests/Commands/TestVerifyRulesCmd.cs @@ -136,6 +136,32 @@ public class TestVerifyRulesCmd ], ""must-match"" : [ ""wimdoos""] } +]"; + + // This rule depends on another tag but that tag isn't in the ruleset + private readonly string _dependsOnTagMissingRule = @"[ +{ + ""name"": ""Platform: Microsoft Windows"", + ""id"": ""AI_TEST_WINDOWS_MUST_NOT_MATCH"", + ""description"": ""This rule checks for the string 'windows'"", + ""tags"": [ + ""Test.Tags.Windows"" + ], + ""depends_on_tags"": [""tag_not_present""], + ""applies_to"": [ ""csharp""], + ""severity"": ""Important"", + ""patterns"": [ + { + ""confidence"": ""Medium"", + ""modifiers"": [ + ""i"" + ], + ""pattern"": ""windows"", + ""type"": ""String"", + } + ], + ""must-not-match"" : [ ""linux""] +} ]"; // MustNotMatch if specified must not be matched @@ -385,6 +411,22 @@ public void DuplicateId() File.Delete(path); } + [TestMethod] + public void MissingDependsOnTag() + { + var path = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + File.WriteAllText(path, _dependsOnTagMissingRule); + VerifyRulesOptions options = new() + { + CustomRulesPath = path + }; + + VerifyRulesCommand command = new(options, _factory); + var result = command.GetResult(); + Assert.AreEqual(VerifyRulesResult.ExitCode.NotVerified, result.ResultCode); + File.Delete(path); + } + [TestMethod] public void DuplicateIdCheckDisabled() { diff --git a/AppInspector/Commands/AnalyzeCommand.cs b/AppInspector/Commands/AnalyzeCommand.cs index f4e53dcf..94c074ad 100644 --- a/AppInspector/Commands/AnalyzeCommand.cs +++ b/AppInspector/Commands/AnalyzeCommand.cs @@ -150,7 +150,7 @@ public class AnalyzeCommand private readonly Severity _severity = Severity.Unspecified; private readonly List _srcfileList = new(); private readonly Languages _languages = new(); - private readonly MetaDataHelper _metaDataHelper; //wrapper containing MetaData object to be assigned to result + private MetaDataHelper _metaDataHelper; //wrapper containing MetaData object to be assigned to result private readonly RuleProcessor _rulesProcessor; /// @@ -335,6 +335,17 @@ private AnalyzeResult.ExitCode PopulateRecords(CancellationToken cancellationTok } } + // Remove match records that don't have the matching depends on tags + List filteredMatchRecords = _metaDataHelper.Matches.Where(x => x.Rule?.DependsOnTags?.All(tag => _metaDataHelper.UniqueTags.ContainsKey(tag)) ?? true).ToList(); + if (filteredMatchRecords.Count != _metaDataHelper.Matches.Count) + { + _metaDataHelper = _metaDataHelper.CreateFresh(); + foreach (MatchRecord matchRecord in filteredMatchRecords) + { + _metaDataHelper.AddMatchRecord(matchRecord); + } + } + return AnalyzeResult.ExitCode.Success; void ProcessAndAddToMetadata(FileEntry file) @@ -445,6 +456,7 @@ void ProcessLambda() } foreach (var matchRecord in results) + { if (_options.TagsOnly) { _metaDataHelper.AddTagsFromMatchRecord(matchRecord); @@ -463,6 +475,7 @@ void ProcessLambda() { _metaDataHelper.AddMatchRecord(matchRecord); } + } } } @@ -501,6 +514,17 @@ void ProcessLambda() await ProcessAndAddToMetadata(entry, cancellationToken); } + // Remove match records that don't have the matching depends on tags + List filteredMatchRecords = _metaDataHelper.Matches.Where(x => x.Rule?.DependsOnTags?.All(tag => _metaDataHelper.UniqueTags.ContainsKey(tag)) ?? true).ToList(); + if (filteredMatchRecords.Count != _metaDataHelper.Matches.Count) + { + _metaDataHelper = _metaDataHelper.CreateFresh(); + foreach (MatchRecord matchRecord in filteredMatchRecords) + { + _metaDataHelper.AddMatchRecord(matchRecord); + } + } + return AnalyzeResult.ExitCode.Success; async Task ProcessAndAddToMetadata(FileEntry file, CancellationToken cancellationToken) @@ -565,6 +589,7 @@ async Task ProcessAndAddToMetadata(FileEntry file, CancellationToken cancellatio } foreach (var matchRecord in results) + { if (_options.TagsOnly) { _metaDataHelper.AddTagsFromMatchRecord(matchRecord); @@ -583,6 +608,7 @@ async Task ProcessAndAddToMetadata(FileEntry file, CancellationToken cancellatio { _metaDataHelper.AddMatchRecord(matchRecord); } + } } } } diff --git a/AppInspector/Commands/ExportTagsCommand.cs b/AppInspector/Commands/ExportTagsCommand.cs index 9b03e974..958fe0ae 100644 --- a/AppInspector/Commands/ExportTagsCommand.cs +++ b/AppInspector/Commands/ExportTagsCommand.cs @@ -114,7 +114,7 @@ public ExportTagsResult GetResult() HashSet tags = new(); foreach (var rule in _rules.GetAppInspectorRules()) - foreach (var tag in rule.Tags ?? Array.Empty()) + foreach (var tag in (IList?)rule.Tags ?? Array.Empty()) tags.Add(tag); exportTagsResult.TagsList = tags.ToList(); diff --git a/AppInspector/MetaDataHelper.cs b/AppInspector/MetaDataHelper.cs index a72b3d5f..49239133 100644 --- a/AppInspector/MetaDataHelper.cs +++ b/AppInspector/MetaDataHelper.cs @@ -22,14 +22,17 @@ public class MetaDataHelper { public MetaDataHelper(string sourcePath) { + SourcePath = sourcePath; if (!sourcePath.Contains(',')) { - sourcePath = Path.GetFullPath(sourcePath); //normalize for .\ and similar + SourcePath = Path.GetFullPath(SourcePath); //normalize for .\ and similar } - Metadata = new MetaData(sourcePath, sourcePath); + Metadata = new MetaData(SourcePath, SourcePath); } + internal string SourcePath { get; set; } + //visible to callers i.e. AnalyzeCommand internal ConcurrentDictionary PackageTypes { get; set; } = new(); internal ConcurrentDictionary FileExtensions { get; set; } = new(); @@ -43,7 +46,7 @@ public MetaDataHelper(string sourcePath) private ConcurrentDictionary CloudTargets { get; } = new(); private ConcurrentDictionary OSTargets { get; } = new(); private ConcurrentDictionary TagCounters { get; } = new(); - private ConcurrentDictionary Languages { get; } = new(); + private ConcurrentDictionary Languages { get; set; } = new(); internal ConcurrentBag Matches { get; set; } = new(); internal ConcurrentBag Files { get; set; } = new(); @@ -62,7 +65,7 @@ public MetaDataHelper(string sourcePath) public void AddTagsFromMatchRecord(MatchRecord matchRecord) { //special handling for standard characteristics in report - foreach (var tag in matchRecord.Tags ?? Array.Empty()) + foreach (string tag in matchRecord.Tags ?? Array.Empty()) switch (tag) { case "Metadata.Application.Author": @@ -107,31 +110,33 @@ public void AddTagsFromMatchRecord(MatchRecord matchRecord) } //Special handling; attempt to detect app types...review for multiple pattern rule limitation - var solutionType = DetectSolutionType(matchRecord); + string solutionType = DetectSolutionType(matchRecord); if (!string.IsNullOrEmpty(solutionType)) { AppTypes.TryAdd(solutionType, 0); } - var CounterOnlyTagSet = false; - var selected = matchRecord.Tags is not null + bool CounterOnlyTagSet = false; + IEnumerable> selected = matchRecord.Tags is not null ? TagCounters.Where(x => matchRecord.Tags.Any(y => y.Contains(x.Value.Tag ?? ""))) : new Dictionary(); - foreach (var select in selected) + foreach (KeyValuePair select in selected) { CounterOnlyTagSet = true; select.Value.IncrementCount(); } - //omit adding if ther a counter metric tag + //omit adding if there is a counter metric tag if (!CounterOnlyTagSet) //update list of unique tags as we go { - foreach (var tag in matchRecord.Tags ?? Array.Empty()) + foreach (string tag in matchRecord.Tags ?? Array.Empty()) + { if (!UniqueTags.TryAdd(tag, 1)) { UniqueTags[tag]++; } + } } } @@ -144,7 +149,7 @@ public void AddMatchRecord(MatchRecord matchRecord) { AddTagsFromMatchRecord(matchRecord); - var nonCounters = matchRecord.Tags?.Where(x => !TagCounters.Any(y => y.Key == x)) ?? Array.Empty(); + IEnumerable nonCounters = matchRecord.Tags?.Where(x => !TagCounters.Any(y => y.Key == x)) ?? Array.Empty(); //omit adding if it if all the tags were counters if (nonCounters.Any()) @@ -185,7 +190,7 @@ public void PrepareReport() Metadata.Languages = new SortedDictionary(Languages); - foreach (var metricTagCounter in TagCounters.Values) Metadata.TagCounters?.Add(metricTagCounter); + foreach (MetricTagCounter metricTagCounter in TagCounters.Values) Metadata.TagCounters?.Add(metricTagCounter); } /// @@ -197,38 +202,6 @@ public void AddLanguage(string language) Languages.AddOrUpdate(language, 1, (language, count) => count + 1); } - /// - /// Initial best guess to deduce project name; if scanned metadata from project solution value is replaced later - /// - /// - /// - private string GetDefaultProjectName(string sourcePath) - { - var applicationName = string.Empty; - - if (Directory.Exists(sourcePath)) - { - if (sourcePath != string.Empty) - { - if (sourcePath[^1] == Path.DirectorySeparatorChar) //in case path ends with dir separator; remove - { - applicationName = sourcePath.Trim(Path.DirectorySeparatorChar); - } - - if (applicationName.LastIndexOf(Path.DirectorySeparatorChar) is int idx && idx != -1) - { - applicationName = applicationName[idx..].Trim(); - } - } - } - else - { - applicationName = Path.GetFileNameWithoutExtension(sourcePath); - } - - return applicationName; - } - /// /// Attempt to map application type tags or file type or language to identify /// WebApplications, Windows Services, Client Apps, WebServices, Azure Functions etc. @@ -236,12 +209,12 @@ private string GetDefaultProjectName(string sourcePath) /// public string DetectSolutionType(MatchRecord match) { - var result = ""; + string result = ""; if (match.Tags is not null && match.Tags.Any(s => s.Contains("Application.Type"))) { - foreach (var tag in match.Tags ?? Array.Empty()) + foreach (string tag in match.Tags ?? Array.Empty()) { - var index = tag.IndexOf("Application.Type"); + int index = tag.IndexOf("Application.Type"); if (-1 != index) { result = tag[(index + 17)..]; @@ -323,7 +296,7 @@ private string ExtractValue(string s) private static string ExtractJSONValue(string s) { - var parts = s.Split(':'); + string[] parts = s.Split(':'); if (parts.Length == 2) { return parts[1].Replace("\"", "").Trim(); @@ -334,10 +307,10 @@ private static string ExtractJSONValue(string s) private string ExtractXMLValue(string s) { - var firstTag = s.IndexOf(">"); + int firstTag = s.IndexOf(">"); if (firstTag > -1 && firstTag < s.Length - 1) { - var endTag = s.IndexOf(" -1) { return s[(firstTag + 1)..endTag]; @@ -349,7 +322,7 @@ private string ExtractXMLValue(string s) private string ExtractXMLValueMultiLine(string s) { - var firstTag = s.IndexOf(">"); + int firstTag = s.IndexOf(">"); if (firstTag > -1 && firstTag < s.Length - 1) { return s[(firstTag + 1)..]; @@ -357,4 +330,18 @@ private string ExtractXMLValueMultiLine(string s) return s; } + + /// + /// Returns a new MetaDataHelper with the same SourcePath, Files, Languages and FileExtensions + /// + /// + internal MetaDataHelper CreateFresh() + { + return new MetaDataHelper(SourcePath) + { + Files = Files, + FileExtensions = FileExtensions, + Languages = Languages + }; + } } \ No newline at end of file diff --git a/version.json b/version.json index 5b110db6..9da0829d 100644 --- a/version.json +++ b/version.json @@ -1,6 +1,6 @@ { "$schema": "https://raw.githubusercontent.com/dotnet/Nerdbank.GitVersioning/master/src/NerdBank.GitVersioning/version.schema.json", - "version": "1.7", + "version": "1.8", "publicReleaseRefSpec": [ "^refs/heads/main$", "^refs/heads/v\\d+(?:\\.\\d+)?$" From a73ed505c1d036239f865a0ffdf0071bd6f3d6e3 Mon Sep 17 00:00:00 2001 From: Gabe Stocco <98900+gfs@users.noreply.github.com> Date: Thu, 2 Mar 2023 11:39:58 -0800 Subject: [PATCH 02/13] Fix query for rules with null depends on field --- AppInspector.RulesEngine/RulesVerifier.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AppInspector.RulesEngine/RulesVerifier.cs b/AppInspector.RulesEngine/RulesVerifier.cs index ba9859af..30fe7c32 100644 --- a/AppInspector.RulesEngine/RulesVerifier.cs +++ b/AppInspector.RulesEngine/RulesVerifier.cs @@ -93,7 +93,7 @@ public List CheckIntegrity(AbstractRuleSet ruleSet) } var allTags = ruleSet.GetAppInspectorRules().SelectMany(x => x.Tags ?? Array.Empty()).ToList(); - var rulesWithDependsOnWithNoMatchingTags = ruleSet.GetAppInspectorRules().Where(x => !x.DependsOnTags?.All(tag => allTags.Contains(tag)) ?? true); + var rulesWithDependsOnWithNoMatchingTags = ruleSet.GetAppInspectorRules().Where(x => !x.DependsOnTags?.All(tag => allTags.Contains(tag)) ?? false); foreach(var dependslessRule in rulesWithDependsOnWithNoMatchingTags) { _logger.LogError(MsgHelp.GetString(MsgHelp.ID.VERIFY_RULES_DEPENDS_ON_TAG_MISSING), dependslessRule.Id); From d6dc1fc5a757daf070d7ca74dbc0c32efb349600 Mon Sep 17 00:00:00 2001 From: Gabe Stocco <98900+gfs@users.noreply.github.com> Date: Thu, 2 Mar 2023 11:45:11 -0800 Subject: [PATCH 03/13] Formatting: Add Missing Braces Back --- .editorconfig | 228 ++++++++++++++++++ AppInspector.Benchmarks/DistinctBenchmarks.cs | 21 +- AppInspector.CLI/Program.cs | 5 +- AppInspector.CLI/Writers/AnalyzeHtmlWriter.cs | 94 ++++++-- .../Writers/AnalyzeSarifWriter.cs | 5 +- AppInspector.CLI/Writers/AnalyzeTextWriter.cs | 22 +- .../Writers/ExportTagsTextWriter.cs | 5 +- AppInspector.CLI/Writers/TagDiffTextWriter.cs | 2 + .../Writers/VerifyRulesTextWriter.cs | 2 + AppInspector.RulesEngine/AbstractRuleSet.cs | 4 + AppInspector.RulesEngine/Languages.cs | 6 + .../OatRegexWithIndexOperation.cs | 8 + .../OatExtensions/WithinOperation.cs | 5 +- AppInspector.RulesEngine/RuleProcessor.cs | 34 ++- AppInspector.RulesEngine/RulesVerifier.cs | 28 ++- AppInspector.RulesEngine/TextContainer.cs | 11 +- AppInspector.RulesEngine/TypedRuleSet.cs | 9 +- AppInspector.Tests/Commands/TestAnalyzeCmd.cs | 5 +- .../DefaultRules/TestDefaultRules.cs | 15 +- .../RuleProcessor/WithinClauseTests.cs | 6 +- AppInspector.Tests/TestHelpers.cs | 2 + AppInspector/Commands/AnalyzeCommand.cs | 33 ++- AppInspector/Commands/ExportTagsCommand.cs | 8 +- AppInspector/Commands/TagDiffCommand.cs | 6 +- AppInspector/MetaDataHelper.cs | 7 +- 25 files changed, 503 insertions(+), 68 deletions(-) create mode 100644 .editorconfig diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 00000000..835ef2b8 --- /dev/null +++ b/.editorconfig @@ -0,0 +1,228 @@ +# Remove the line below if you want to inherit .editorconfig settings from higher directories +root = true + +# C# files +[*.cs] + +#### Core EditorConfig Options #### + +# Indentation and spacing +indent_size = 4 +indent_style = space +tab_width = 4 + +# New line preferences +end_of_line = crlf +insert_final_newline = false + +#### .NET Coding Conventions #### + +# Organize usings +dotnet_separate_import_directive_groups = false +dotnet_sort_system_directives_first = false +file_header_template = unset + +# this. and Me. preferences +dotnet_style_qualification_for_event = false:warning +dotnet_style_qualification_for_field = false +dotnet_style_qualification_for_method = false:warning +dotnet_style_qualification_for_property = false:warning + +# Language keywords vs BCL types preferences +dotnet_style_predefined_type_for_locals_parameters_members = true:warning +dotnet_style_predefined_type_for_member_access = true:warning + +# Parentheses preferences +dotnet_style_parentheses_in_arithmetic_binary_operators = always_for_clarity +dotnet_style_parentheses_in_other_binary_operators = always_for_clarity +dotnet_style_parentheses_in_other_operators = never_if_unnecessary +dotnet_style_parentheses_in_relational_binary_operators = always_for_clarity + +# Modifier preferences +dotnet_style_require_accessibility_modifiers = for_non_interface_members + +# Expression-level preferences +dotnet_style_coalesce_expression = true +dotnet_style_collection_initializer = true +dotnet_style_explicit_tuple_names = true +dotnet_style_namespace_match_folder = true +dotnet_style_null_propagation = true +dotnet_style_object_initializer = true +dotnet_style_operator_placement_when_wrapping = beginning_of_line +dotnet_style_prefer_auto_properties = true:warning +dotnet_style_prefer_compound_assignment = true +dotnet_style_prefer_conditional_expression_over_assignment = true +dotnet_style_prefer_conditional_expression_over_return = true +dotnet_style_prefer_foreach_explicit_cast_in_source = when_strongly_typed +dotnet_style_prefer_inferred_anonymous_type_member_names = true +dotnet_style_prefer_inferred_tuple_names = true +dotnet_style_prefer_is_null_check_over_reference_equality_method = true +dotnet_style_prefer_simplified_boolean_expressions = true +dotnet_style_prefer_simplified_interpolation = true + +# Field preferences +dotnet_style_readonly_field = true + +# Parameter preferences +dotnet_code_quality_unused_parameters = all + +# Suppression preferences +dotnet_remove_unnecessary_suppression_exclusions = none + +# New line preferences +dotnet_style_allow_multiple_blank_lines_experimental = false +dotnet_style_allow_statement_immediately_after_block_experimental = false + +#### C# Coding Conventions #### + +# var preferences +csharp_style_var_elsewhere = false:warning +csharp_style_var_for_built_in_types = false:warning +csharp_style_var_when_type_is_apparent = false:warning + +# Expression-bodied members +csharp_style_expression_bodied_accessors = true +csharp_style_expression_bodied_constructors = false +csharp_style_expression_bodied_indexers = true +csharp_style_expression_bodied_lambdas = true +csharp_style_expression_bodied_local_functions = false +csharp_style_expression_bodied_methods = false +csharp_style_expression_bodied_operators = when_on_single_line +csharp_style_expression_bodied_properties = true + +# Pattern matching preferences +csharp_style_pattern_matching_over_as_with_null_check = true +csharp_style_pattern_matching_over_is_with_cast_check = true +csharp_style_prefer_extended_property_pattern = true +csharp_style_prefer_not_pattern = true +csharp_style_prefer_pattern_matching = true:suggestion +csharp_style_prefer_switch_expression = true + +# Null-checking preferences +csharp_style_conditional_delegate_call = true + +# Modifier preferences +csharp_prefer_static_local_function = true +csharp_preferred_modifier_order = public,private,protected,internal,file,static,extern,new,virtual,abstract,sealed,override,readonly,unsafe,required,volatile,async +csharp_style_prefer_readonly_struct = true + +# Code-block preferences +csharp_prefer_braces = true:warning +csharp_prefer_simple_using_statement = true:warning +csharp_style_namespace_declarations = block_scoped:warning +csharp_style_prefer_method_group_conversion = true:warning +csharp_style_prefer_top_level_statements = false:warning + +# Expression-level preferences +csharp_prefer_simple_default_expression = true +csharp_style_deconstructed_variable_declaration = true +csharp_style_implicit_object_creation_when_type_is_apparent = true +csharp_style_inlined_variable_declaration = true +csharp_style_prefer_index_operator = true +csharp_style_prefer_local_over_anonymous_function = true +csharp_style_prefer_null_check_over_type_check = true +csharp_style_prefer_range_operator = true +csharp_style_prefer_tuple_swap = true +csharp_style_prefer_utf8_string_literals = true +csharp_style_throw_expression = true +csharp_style_unused_value_assignment_preference = discard_variable +csharp_style_unused_value_expression_statement_preference = discard_variable + +# 'using' directive preferences +csharp_using_directive_placement = inside_namespace:suggestion + +# New line preferences +csharp_style_allow_blank_line_after_colon_in_constructor_initializer_experimental = true +csharp_style_allow_blank_line_after_token_in_arrow_expression_clause_experimental = true +csharp_style_allow_blank_line_after_token_in_conditional_expression_experimental = true +csharp_style_allow_blank_lines_between_consecutive_braces_experimental = false +csharp_style_allow_embedded_statements_on_same_line_experimental = true + +#### C# Formatting Rules #### + +# New line preferences +csharp_new_line_before_catch = true +csharp_new_line_before_else = true +csharp_new_line_before_finally = true +csharp_new_line_before_members_in_anonymous_types = true +csharp_new_line_before_members_in_object_initializers = true +csharp_new_line_before_open_brace = all +csharp_new_line_between_query_expression_clauses = true + +# Indentation preferences +csharp_indent_block_contents = true +csharp_indent_braces = false +csharp_indent_case_contents = true +csharp_indent_case_contents_when_block = true +csharp_indent_labels = one_less_than_current +csharp_indent_switch_labels = true + +# Space preferences +csharp_space_after_cast = false +csharp_space_after_colon_in_inheritance_clause = true +csharp_space_after_comma = true +csharp_space_after_dot = false +csharp_space_after_keywords_in_control_flow_statements = true +csharp_space_after_semicolon_in_for_statement = true +csharp_space_around_binary_operators = before_and_after +csharp_space_around_declaration_statements = false +csharp_space_before_colon_in_inheritance_clause = true +csharp_space_before_comma = false +csharp_space_before_dot = false +csharp_space_before_open_square_brackets = false +csharp_space_before_semicolon_in_for_statement = false +csharp_space_between_empty_square_brackets = false +csharp_space_between_method_call_empty_parameter_list_parentheses = false +csharp_space_between_method_call_name_and_opening_parenthesis = false +csharp_space_between_method_call_parameter_list_parentheses = false +csharp_space_between_method_declaration_empty_parameter_list_parentheses = false +csharp_space_between_method_declaration_name_and_open_parenthesis = false +csharp_space_between_method_declaration_parameter_list_parentheses = false +csharp_space_between_parentheses = false +csharp_space_between_square_brackets = false + +# Wrapping preferences +csharp_preserve_single_line_blocks = true +csharp_preserve_single_line_statements = true + +#### Naming styles #### + +# Naming rules + +dotnet_naming_rule.interface_should_be_begins_with_i.severity = suggestion +dotnet_naming_rule.interface_should_be_begins_with_i.symbols = interface +dotnet_naming_rule.interface_should_be_begins_with_i.style = begins_with_i + +dotnet_naming_rule.types_should_be_pascal_case.severity = suggestion +dotnet_naming_rule.types_should_be_pascal_case.symbols = types +dotnet_naming_rule.types_should_be_pascal_case.style = pascal_case + +dotnet_naming_rule.non_field_members_should_be_pascal_case.severity = suggestion +dotnet_naming_rule.non_field_members_should_be_pascal_case.symbols = non_field_members +dotnet_naming_rule.non_field_members_should_be_pascal_case.style = pascal_case + +# Symbol specifications + +dotnet_naming_symbols.interface.applicable_kinds = interface +dotnet_naming_symbols.interface.applicable_accessibilities = public, internal, private, protected, protected_internal, private_protected +dotnet_naming_symbols.interface.required_modifiers = + +dotnet_naming_symbols.types.applicable_kinds = class, struct, interface, enum +dotnet_naming_symbols.types.applicable_accessibilities = public, internal, private, protected, protected_internal, private_protected +dotnet_naming_symbols.types.required_modifiers = + +dotnet_naming_symbols.non_field_members.applicable_kinds = property, event, method +dotnet_naming_symbols.non_field_members.applicable_accessibilities = public, internal, private, protected, protected_internal, private_protected +dotnet_naming_symbols.non_field_members.required_modifiers = + +# Naming styles + +dotnet_naming_style.pascal_case.required_prefix = +dotnet_naming_style.pascal_case.required_suffix = +dotnet_naming_style.pascal_case.word_separator = +dotnet_naming_style.pascal_case.capitalization = pascal_case + +dotnet_naming_style.begins_with_i.required_prefix = I +dotnet_naming_style.begins_with_i.required_suffix = +dotnet_naming_style.begins_with_i.word_separator = +dotnet_naming_style.begins_with_i.capitalization = pascal_case diff --git a/AppInspector.Benchmarks/DistinctBenchmarks.cs b/AppInspector.Benchmarks/DistinctBenchmarks.cs index be8ccd1a..a02553c2 100644 --- a/AppInspector.Benchmarks/DistinctBenchmarks.cs +++ b/AppInspector.Benchmarks/DistinctBenchmarks.cs @@ -18,9 +18,11 @@ public List OldCode() SortedDictionary uniqueTags = new(); List outList = new(); foreach (var r in ruleSet) + { //builds a list of unique tags - foreach (var t in (IList?)r?.Tags ?? Array.Empty()) - if (uniqueTags.ContainsKey(t)) + foreach (var t in (IList?)r?.Tags ?? Array.Empty()) + { + if (uniqueTags.ContainsKey(t)) { continue; } @@ -28,9 +30,14 @@ public List OldCode() { uniqueTags.Add(t, t); } + } + } //generate results list - foreach (var s in uniqueTags.Values) outList.Add(s); + foreach (var s in uniqueTags.Values) + { + outList.Add(s); + } return outList; } @@ -40,9 +47,13 @@ public List HashSet() { HashSet hashSet = new(); foreach (var r in ruleSet) + { //builds a list of unique tags - foreach (var t in (IList?)r?.Tags ?? Array.Empty()) - hashSet.Add(t); + foreach (var t in (IList?)r?.Tags ?? Array.Empty()) + { + hashSet.Add(t); + } + } var theList = hashSet.ToList(); theList.Sort(); diff --git a/AppInspector.CLI/Program.cs b/AppInspector.CLI/Program.cs index 067a7a61..d43a11c5 100644 --- a/AppInspector.CLI/Program.cs +++ b/AppInspector.CLI/Program.cs @@ -335,7 +335,10 @@ private static int RunAnalyzeCommand(CLIAnalyzeCmdOptions cliOptions) done = true; }); - while (!done) Thread.Sleep(100); + while (!done) + { + Thread.Sleep(100); + } pbar.ObservedError = !initialSuccess; pbar.Message = !initialSuccess diff --git a/AppInspector.CLI/Writers/AnalyzeHtmlWriter.cs b/AppInspector.CLI/Writers/AnalyzeHtmlWriter.cs index 6944fa0e..4330c867 100644 --- a/AppInspector.CLI/Writers/AnalyzeHtmlWriter.cs +++ b/AppInspector.CLI/Writers/AnalyzeHtmlWriter.cs @@ -111,7 +111,9 @@ private void WriteHtmlResult() //add summary values for sorted tags lists of taginfo foreach (var outerKey in KeyedSortedTagInfoLists.Keys) + { hashData.Add(outerKey, KeyedSortedTagInfoLists[outerKey]); + } hashData["cputargets"] = _appMetaData?.CPUTargets; hashData["apptypes"] = _appMetaData?.AppTypes ?? new List(); @@ -212,7 +214,10 @@ private void RenderResultsSafeforHTML() private void SafeList(List? valuesList) { - for (var i = 0; i < valuesList?.Count; i++) valuesList[i] = SafeString(valuesList[i]); + for (var i = 0; i < valuesList?.Count; i++) + { + valuesList[i] = SafeString(valuesList[i]); + } } private string SafeString(string? value) @@ -260,7 +265,8 @@ public void PopulateTagGroups() //for each preferred group of tag patterns determine if at least one instance was detected foreach (var tagCategory in TagGroupPreferences ?? new List()) - foreach (var tagGroup in tagCategory.Groups ?? new List()) + { + foreach (var tagGroup in tagCategory.Groups ?? new List()) { if (string.IsNullOrEmpty(tagGroup.Title)) { @@ -296,6 +302,7 @@ public void PopulateTagGroups() } } } + } //create simple ranked page lists HTML use KeyedSortedTagInfoLists["tagGrpAllTagsByConfidence"] = GetTagInfoListByConfidence(); @@ -313,14 +320,19 @@ public List GetCategoryTagGroups(string category) List result = new(); //get all tag groups for specified category foreach (var categoryTagGroup in TagGroupPreferences ?? new List()) + { if (categoryTagGroup.Name == category) { result = categoryTagGroup.Groups ?? new List(); break; } + } //now get all matches for that group i.e. Authentication - foreach (var group in result) GetTagInfoListByTagGroup(group); + foreach (var group in result) + { + GetTagInfoListByTagGroup(group); + } return result; } @@ -334,8 +346,12 @@ private HashSet GetUniqueTags() HashSet results = new(); foreach (var match in _appMetaData?.Matches ?? new List()) - foreach (var tag in match.Tags ?? Array.Empty()) - results.Add(tag); + { + foreach (var tag in match.Tags ?? Array.Empty()) + { + results.Add(tag); + } + } return results; } @@ -353,14 +369,17 @@ private List GetTagInfoListByTagGroup(TagGroup tagGroup, bool addNotFou HashSet hashSet = new(); foreach (var pattern in tagGroup.Patterns ?? new List()) + { if (pattern.Detected) //set at program.RollUp already so don't search for again { var tagPatternRegex = pattern.Expression; if (_appMetaData?.TotalMatchesCount > 0) { foreach (var match in _appMetaData?.Matches ?? new List()) - foreach (var tagItem in match.Tags ?? Array.Empty()) - if (tagPatternRegex.IsMatch(tagItem)) + { + foreach (var tagItem in match.Tags ?? Array.Empty()) + { + if (tagPatternRegex.IsMatch(tagItem)) { if (!hashSet.Contains(pattern.SearchPattern)) { @@ -382,7 +401,8 @@ private List GetTagInfoListByTagGroup(TagGroup tagGroup, bool addNotFou { //ensure we get highest confidence, severity as there are likely multiple matches for this tag pattern foreach (var updateItem in result) - if (updateItem.Tag == tagItem) + { + if (updateItem.Tag == tagItem) { Confidence oldConfidence; Enum.TryParse(updateItem.Confidence, out oldConfidence); @@ -402,12 +422,16 @@ private List GetTagInfoListByTagGroup(TagGroup tagGroup, bool addNotFou break; } - } + } + } } + } + } } else { foreach (var tagItem in _appMetaData?.UniqueTags ?? new List()) + { if (tagPatternRegex.IsMatch(tagItem) && !hashSet.Contains(pattern.SearchPattern)) { result.Add(new TagInfo @@ -420,6 +444,7 @@ private List GetTagInfoListByTagGroup(TagGroup tagGroup, bool addNotFou hashSet.Add(tagItem); } + } } } else if (addNotFound) //allow to report on false presense items @@ -438,6 +463,7 @@ private List GetTagInfoListByTagGroup(TagGroup tagGroup, bool addNotFou result.Add(tagInfo); hashSet.Add(tagInfo.Tag); } + } return result; } @@ -454,13 +480,16 @@ private List GetAllMatchingTagInfoList(TagGroup tagGroup, bool addNotFo HashSet hashSet = new(); foreach (var pattern in tagGroup.Patterns ?? new List()) + { if (pattern.Detected) { var tagPatternRegex = pattern.Expression; foreach (var match in _appMetaData?.Matches ?? new List()) - foreach (var tagItem in match.Tags ?? Array.Empty()) - if (tagPatternRegex.IsMatch(tagItem)) + { + foreach (var tagItem in match.Tags ?? Array.Empty()) + { + if (tagPatternRegex.IsMatch(tagItem)) { if (!hashSet.Contains(tagItem)) { @@ -480,7 +509,8 @@ private List GetAllMatchingTagInfoList(TagGroup tagGroup, bool addNotFo { //ensure we have highest confidence, severity as there are likly multiple matches for this tag pattern foreach (var updateItem in result) - if (updateItem.Tag == tagItem) + { + if (updateItem.Tag == tagItem) { Confidence oldConfidence; Enum.TryParse(updateItem.Confidence, out oldConfidence); @@ -500,9 +530,13 @@ private List GetAllMatchingTagInfoList(TagGroup tagGroup, bool addNotFo break; } - } + } + } } + } + } } + } return result; } @@ -517,11 +551,14 @@ private List GetTagInfoListByName() List result = new(); foreach (var tag in _appMetaData?.UniqueTags ?? new List()) + { if (_appMetaData?.TotalMatchesCount > 0) { foreach (var match in _appMetaData?.Matches ?? new List()) - foreach (var testTag in match.Tags ?? Array.Empty()) - if (tag == testTag) + { + foreach (var testTag in match.Tags ?? Array.Empty()) + { + if (tag == testTag) { if (dupCheck.Add(testTag)) { @@ -536,6 +573,8 @@ private List GetTagInfoListByName() break; } } + } + } } else { @@ -545,6 +584,7 @@ private List GetTagInfoListByName() ShortTag = tag[(tag.LastIndexOf('.') + 1)..] }); } + } return result; } @@ -563,9 +603,12 @@ private List GetTagInfoListByConfidence() { var searchPattern = new Regex(tag, RegexOptions.IgnoreCase); foreach (var confidence in confidences) - foreach (var match in _appMetaData?.Matches ?? new List()) - foreach (var testTag in match.Tags ?? Array.Empty()) - if (searchPattern.IsMatch(testTag)) + { + foreach (var match in _appMetaData?.Matches ?? new List()) + { + foreach (var testTag in match.Tags ?? Array.Empty()) + { + if (searchPattern.IsMatch(testTag)) { if (match.Confidence == confidence && dupCheck.Add(tag)) { @@ -578,6 +621,9 @@ private List GetTagInfoListByConfidence() }); } } + } + } + } } return result; @@ -598,9 +644,12 @@ private List GetTagInfoListBySeverity() { var searchPattern = new Regex(tag, RegexOptions.IgnoreCase); foreach (var severity in severities) - foreach (var match in _appMetaData?.Matches ?? new List()) - foreach (var testTag in match.Tags ?? Array.Empty()) - if (searchPattern.IsMatch(testTag)) + { + foreach (var match in _appMetaData?.Matches ?? new List()) + { + foreach (var testTag in match.Tags ?? Array.Empty()) + { + if (searchPattern.IsMatch(testTag)) { if (match.Severity == severity && dupCheck.Add(tag)) { @@ -613,6 +662,9 @@ private List GetTagInfoListBySeverity() }); } } + } + } + } } return result; diff --git a/AppInspector.CLI/Writers/AnalyzeSarifWriter.cs b/AppInspector.CLI/Writers/AnalyzeSarifWriter.cs index 37adfca2..dc763f1a 100644 --- a/AppInspector.CLI/Writers/AnalyzeSarifWriter.cs +++ b/AppInspector.CLI/Writers/AnalyzeSarifWriter.cs @@ -26,7 +26,10 @@ internal static void AddRange(this TagsCollection tc, IEnumerable? tagsT return; } - foreach (var tag in tagsToAdd) tc.Add(tag); + foreach (var tag in tagsToAdd) + { + tc.Add(tag); + } } } diff --git a/AppInspector.CLI/Writers/AnalyzeTextWriter.cs b/AppInspector.CLI/Writers/AnalyzeTextWriter.cs index 826eb6d1..a8001461 100644 --- a/AppInspector.CLI/Writers/AnalyzeTextWriter.cs +++ b/AppInspector.CLI/Writers/AnalyzeTextWriter.cs @@ -48,7 +48,10 @@ public override void WriteResults(Result result, CLICommandOptions commandOption WriteDependencies(analyzeResult.Metadata); TextWriter.WriteLine(MakeHeading("Match Details")); - foreach (var match in analyzeResult.Metadata.Matches ?? new List()) WriteMatch(match); + foreach (var match in analyzeResult.Metadata.Matches ?? new List()) + { + WriteMatch(match); + } if (autoClose) { @@ -89,7 +92,10 @@ private string MakeHeading(string header) { StringBuilder build = new(); build.Append(string.Format("[{0}]", header)); - for (var i = header.Length; i < COLUMN_MAX; i++) build.Append("-"); + for (var i = header.Length; i < COLUMN_MAX; i++) + { + build.Append("-"); + } return build.ToString(); } @@ -130,11 +136,16 @@ public void WriteAppMeta(MetaData metaData) TextWriter.WriteLine($"Unique matches: {metaData.UniqueMatchesCount}"); TextWriter.WriteLine(MakeHeading("UniqueTags")); - foreach (var tag in metaData.UniqueTags) TextWriter.WriteLine(tag); + foreach (var tag in metaData.UniqueTags) + { + TextWriter.WriteLine(tag); + } TextWriter.WriteLine(MakeHeading("Select Counters")); foreach (var tagCounter in metaData.TagCounters ?? new List()) + { TextWriter.WriteLine($"Tagname: {tagCounter.Tag}, Count: {tagCounter.Count}"); + } } public void WriteMatch(MatchRecord match) @@ -171,6 +182,9 @@ private void WriteDependencies(MetaData metaData) TextWriter.WriteLine(MakeHeading("Dependencies")); - foreach (var s in metaData.UniqueDependencies ?? new List()) TextWriter.WriteLine(s); + foreach (var s in metaData.UniqueDependencies ?? new List()) + { + TextWriter.WriteLine(s); + } } } \ No newline at end of file diff --git a/AppInspector.CLI/Writers/ExportTagsTextWriter.cs b/AppInspector.CLI/Writers/ExportTagsTextWriter.cs index fd7d50a3..ef8360bc 100644 --- a/AppInspector.CLI/Writers/ExportTagsTextWriter.cs +++ b/AppInspector.CLI/Writers/ExportTagsTextWriter.cs @@ -31,7 +31,10 @@ public override void WriteResults(Result result, CLICommandOptions commandOption { TextWriter.WriteLine("Results"); - foreach (var tag in exportTagsResult.TagsList) TextWriter.WriteLine(tag); + foreach (var tag in exportTagsResult.TagsList) + { + TextWriter.WriteLine(tag); + } } else { diff --git a/AppInspector.CLI/Writers/TagDiffTextWriter.cs b/AppInspector.CLI/Writers/TagDiffTextWriter.cs index 87e85f67..e100a7c3 100644 --- a/AppInspector.CLI/Writers/TagDiffTextWriter.cs +++ b/AppInspector.CLI/Writers/TagDiffTextWriter.cs @@ -39,7 +39,9 @@ public override void WriteResults(Result result, CLICommandOptions commandOption { TextWriter.WriteLine("Differences"); foreach (var tagDiff in tagDiffResult.TagDiffList) + { TextWriter.WriteLine("Tag: {0}, Only found in file: {1}", tagDiff.Tag, tagDiff.Source); + } } if (autoClose) diff --git a/AppInspector.CLI/Writers/VerifyRulesTextWriter.cs b/AppInspector.CLI/Writers/VerifyRulesTextWriter.cs index 6b819dff..38849fce 100644 --- a/AppInspector.CLI/Writers/VerifyRulesTextWriter.cs +++ b/AppInspector.CLI/Writers/VerifyRulesTextWriter.cs @@ -40,8 +40,10 @@ public override void WriteResults(Result result, CLICommandOptions commandOption { TextWriter.WriteLine("Rule status"); foreach (var ruleStatus in verifyRulesResult.RuleStatusList) + { TextWriter.WriteLine("Ruleid: {0}, Rulename: {1}, Status: {2}", ruleStatus.RulesId, ruleStatus.RulesName, ruleStatus.Verified); + } } if (autoClose) diff --git a/AppInspector.RulesEngine/AbstractRuleSet.cs b/AppInspector.RulesEngine/AbstractRuleSet.cs index cbf4e7d1..c5186ae8 100644 --- a/AppInspector.RulesEngine/AbstractRuleSet.cs +++ b/AppInspector.RulesEngine/AbstractRuleSet.cs @@ -78,6 +78,7 @@ public IEnumerable GetUniversalRules() var clauseNumber = 0; var expression = new StringBuilder("("); foreach (var pattern in rule.Patterns) + { if (GenerateClause(pattern, clauseNumber) is { } clause) { clauses.Add(clause); @@ -93,6 +94,7 @@ public IEnumerable GetUniversalRules() { _logger.LogWarning("Clause could not be generated from pattern {pattern}", pattern.Pattern); } + } if (clauses.Count > 0) { @@ -150,6 +152,7 @@ public IEnumerable GetUniversalRules() if (m.Success) { for (var i = 1; i < m.Groups.Count; i++) + { if (int.TryParse(m.Groups[i].Value, out var value)) { argList.Add(value); @@ -158,6 +161,7 @@ public IEnumerable GetUniversalRules() { break; } + } } if (argList.Count == 2) diff --git a/AppInspector.RulesEngine/Languages.cs b/AppInspector.RulesEngine/Languages.cs index 51f8d9da..f6e492c1 100644 --- a/AppInspector.RulesEngine/Languages.cs +++ b/AppInspector.RulesEngine/Languages.cs @@ -141,11 +141,13 @@ public string GetCommentInline(string language) if (language != null) { foreach (var comment in _comments) + { if (Array.Exists(comment.Languages ?? new[] { "" }, x => x.Equals(language, StringComparison.InvariantCultureIgnoreCase)) && comment.Inline is { }) { return comment.Inline; } + } } return result; @@ -163,11 +165,13 @@ public string GetCommentPrefix(string language) if (language != null) { foreach (var comment in _comments) + { if ((comment.Languages?.Contains(language.ToLower(CultureInfo.InvariantCulture)) ?? false) && comment.Prefix is { }) { return comment.Prefix; } + } } return result; @@ -185,11 +189,13 @@ public string GetCommentSuffix(string language) if (language != null) { foreach (var comment in _comments) + { if (Array.Exists(comment.Languages ?? new[] { "" }, x => x.Equals(language, StringComparison.InvariantCultureIgnoreCase)) && comment.Suffix is { }) { return comment.Suffix; } + } } return result; diff --git a/AppInspector.RulesEngine/OatExtensions/OatRegexWithIndexOperation.cs b/AppInspector.RulesEngine/OatExtensions/OatRegexWithIndexOperation.cs index 0d942d70..645b1183 100644 --- a/AppInspector.RulesEngine/OatExtensions/OatRegexWithIndexOperation.cs +++ b/AppInspector.RulesEngine/OatExtensions/OatRegexWithIndexOperation.cs @@ -49,6 +49,7 @@ private static IEnumerable RegexWithIndexValidationDelegate(CST.OAT.R else if (clause.Data is List regexList) { foreach (var regex in regexList) + { if (!Helpers.IsValidRegex(regex)) { yield return new Violation( @@ -56,6 +57,7 @@ private static IEnumerable RegexWithIndexValidationDelegate(CST.OAT.R clause.Label ?? rule.Clauses.IndexOf(clause).ToString(CultureInfo.InvariantCulture), regex), rule, clause); } + } } if (clause.DictData?.Count > 0) @@ -91,6 +93,7 @@ private OperationResult RegexWithIndexOperationDelegate(Clause clause, object? s if (Analyzer != null) { foreach (var regexString in RegexList) + { if (StringToRegex(regexString, regexOpts) is { } regex) { if (src.XPaths is not null) @@ -112,7 +115,9 @@ private OperationResult RegexWithIndexOperationDelegate(Clause clause, object? s { var targets = tc.GetStringFromJsonPath(jsonPath); foreach (var target in targets) + { outmatches.AddRange(GetMatches(regex, tc, clause, src.Scopes, target.Item2)); + } } } @@ -140,6 +145,7 @@ private OperationResult RegexWithIndexOperationDelegate(Clause clause, object? s } } } + } var result = src.Invert ? outmatches.Count == 0 : outmatches.Count > 0; return new OperationResult(result, @@ -156,6 +162,7 @@ private OperationResult RegexWithIndexOperationDelegate(Clause clause, object? s Boundary? boundary) { foreach (var match in regex.Matches(boundary is null ? tc.FullContent : tc.GetBoundaryText(boundary))) + { if (match is Match m) { Boundary translatedBoundary = new() @@ -173,6 +180,7 @@ private OperationResult RegexWithIndexOperationDelegate(Clause clause, object? s yield return (patternIndex, translatedBoundary); } } + } } /// diff --git a/AppInspector.RulesEngine/OatExtensions/WithinOperation.cs b/AppInspector.RulesEngine/OatExtensions/WithinOperation.cs index af0a2308..de2d17a4 100644 --- a/AppInspector.RulesEngine/OatExtensions/WithinOperation.cs +++ b/AppInspector.RulesEngine/OatExtensions/WithinOperation.cs @@ -192,7 +192,10 @@ public IEnumerable WithinValidationDelegate(CST.OAT.Rule rule, Clause } else { - foreach (var violation in subOp.ValidationDelegate.Invoke(rule, wc.SubClause)) yield return violation; + foreach (var violation in subOp.ValidationDelegate.Invoke(rule, wc.SubClause)) + { + yield return violation; + } if (wc.SubClause is OatRegexWithIndexClause oatRegexWithIndexClause) { diff --git a/AppInspector.RulesEngine/RuleProcessor.cs b/AppInspector.RulesEngine/RuleProcessor.cs index 4c5513e4..8faa95ab 100644 --- a/AppInspector.RulesEngine/RuleProcessor.cs +++ b/AppInspector.RulesEngine/RuleProcessor.cs @@ -211,17 +211,22 @@ public List AnalyzeFile(TextContainer textContainer, FileEntry file // WithinClauses are always ANDed, but each contains all the captures that passed *that* clause. // We need the captures that passed every clause. foreach (var aCapture in allCaptured) + { numberOfInstances.AddOrUpdate(aCapture, 1, (tuple, i) => i + 1); + } + return numberOfInstances.Where(x => x.Value == onlyWithinCaptures.Count).Select(x => x.Key) .ToList(); } var outList = new List<(int, Boundary)>(); foreach (var cap in captures) + { if (cap is TypedClauseCapture> tcc) { outList.AddRange(tcc.Result); } + } return outList; } @@ -230,15 +235,21 @@ public List AnalyzeFile(TextContainer textContainer, FileEntry file List removes = new(); foreach (var m in resultsList.Where(x => x.Rule?.Overrides?.Count > 0)) - foreach (var idsToOverride in (IList)m.Rule?.Overrides ?? Array.Empty()) - // Find all overriden rules and mark them for removal from issues list - foreach (var om in resultsList.FindAll(x => x.Rule?.Id == idsToOverride)) - // If the overridden match is a subset of the overriding match - if (om.Boundary.Index >= m.Boundary.Index && + { + foreach (var idsToOverride in (IList)m.Rule?.Overrides ?? Array.Empty()) + { + // Find all overriden rules and mark them for removal from issues list + foreach (var om in resultsList.FindAll(x => x.Rule?.Id == idsToOverride)) + { + // If the overridden match is a subset of the overriding match + if (om.Boundary.Index >= m.Boundary.Index && om.Boundary.Index <= m.Boundary.Index + m.Boundary.Length) { removes.Add(om); } + } + } + } // Remove overriden rules resultsList.RemoveAll(x => removes.Contains(x)); @@ -338,7 +349,10 @@ public async Task> AnalyzeFileAsync(FileEntry fileEntry, Langu return resultsList; } - foreach (var cap in filteredCaptures) resultsList.AddRange(ProcessBoundary(cap)); + foreach (var cap in filteredCaptures) + { + resultsList.AddRange(ProcessBoundary(cap)); + } List ProcessBoundary(ClauseCapture cap) { @@ -424,13 +438,17 @@ List ProcessBoundary(ClauseCapture cap) } foreach (var ovrd in (IList?)m.Rule?.Overrides ?? Array.Empty()) + { // Find all overriden rules and mark them for removal from issues list - foreach (var om in resultsList.FindAll(x => x.Rule?.Id == ovrd)) - if (om.Boundary.Index >= m.Boundary.Index && + foreach (var om in resultsList.FindAll(x => x.Rule?.Id == ovrd)) + { + if (om.Boundary.Index >= m.Boundary.Index && om.Boundary.Index <= m.Boundary.Index + m.Boundary.Length) { removes.Add(om); } + } + } } // Remove overriden rules diff --git a/AppInspector.RulesEngine/RulesVerifier.cs b/AppInspector.RulesEngine/RulesVerifier.cs index 30fe7c32..523d4035 100644 --- a/AppInspector.RulesEngine/RulesVerifier.cs +++ b/AppInspector.RulesEngine/RulesVerifier.cs @@ -87,17 +87,19 @@ public List CheckIntegrity(AbstractRuleSet ruleSet) _logger.LogError(MsgHelp.GetString(MsgHelp.ID.VERIFY_RULES_DUPLICATEID_FAIL), rule.Key); var relevantStati = ruleStatuses.Where(x => x.RulesId == rule.Key); foreach (var status in relevantStati) + { status.Errors = status.Errors.Append(MsgHelp.FormatString(MsgHelp.ID.VERIFY_RULES_DUPLICATEID_FAIL, rule.Key)); + } } } var allTags = ruleSet.GetAppInspectorRules().SelectMany(x => x.Tags ?? Array.Empty()).ToList(); var rulesWithDependsOnWithNoMatchingTags = ruleSet.GetAppInspectorRules().Where(x => !x.DependsOnTags?.All(tag => allTags.Contains(tag)) ?? false); - foreach(var dependslessRule in rulesWithDependsOnWithNoMatchingTags) + foreach (var dependslessRule in rulesWithDependsOnWithNoMatchingTags) { _logger.LogError(MsgHelp.GetString(MsgHelp.ID.VERIFY_RULES_DEPENDS_ON_TAG_MISSING), dependslessRule.Id); - foreach(var status in ruleStatuses.Where(x => x.RulesId == dependslessRule.Id)) + foreach (var status in ruleStatuses.Where(x => x.RulesId == dependslessRule.Id)) { status.Errors = status.Errors.Append(MsgHelp.FormatString(MsgHelp.ID.VERIFY_RULES_DEPENDS_ON_TAG_MISSING, dependslessRule.Id)); } @@ -122,19 +124,20 @@ public RuleStatus CheckIntegrity(ConvertedOatRule convertedOatRule) if (rule.AppliesTo != null) { var languages = _options.LanguageSpecs.GetNames(); - // Check for unknown language - foreach (var lang in rule.AppliesTo) - if (!string.IsNullOrEmpty(lang)) - { - if (!languages.Any(x => x.Equals(lang, StringComparison.CurrentCultureIgnoreCase))) - { - _logger.LogError(MsgHelp.GetString(MsgHelp.ID.VERIFY_RULES_LANGUAGE_FAIL), rule.Id ?? "", lang); - errors.Add(MsgHelp.FormatString(MsgHelp.ID.VERIFY_RULES_LANGUAGE_FAIL, rule.Id ?? "", lang)); - } - } + foreach (string lang in + // Check for unknown language + from lang in rule.AppliesTo + where !string.IsNullOrEmpty(lang) + where !languages.Any(x => x.Equals(lang, StringComparison.CurrentCultureIgnoreCase)) + select lang) + { + _logger.LogError(MsgHelp.GetString(MsgHelp.ID.VERIFY_RULES_LANGUAGE_FAIL), rule.Id ?? "", lang); + errors.Add(MsgHelp.FormatString(MsgHelp.ID.VERIFY_RULES_LANGUAGE_FAIL, rule.Id ?? "", lang)); + } } foreach (var pattern in (IList?)rule.FileRegexes ?? Array.Empty()) + { try { _ = new Regex(pattern, RegexOptions.Compiled); @@ -146,6 +149,7 @@ public RuleStatus CheckIntegrity(ConvertedOatRule convertedOatRule) errors.Add(MsgHelp.FormatString(MsgHelp.ID.VERIFY_RULES_REGEX_FAIL, rule.Id ?? "", pattern ?? "", e.Message)); } + } //valid search pattern foreach (var searchPattern in rule.Patterns ?? Array.Empty()) diff --git a/AppInspector.RulesEngine/TextContainer.cs b/AppInspector.RulesEngine/TextContainer.cs index f86b2687..a8eb63fc 100644 --- a/AppInspector.RulesEngine/TextContainer.cs +++ b/AppInspector.RulesEngine/TextContainer.cs @@ -146,6 +146,7 @@ public TextContainer(string content, string language, Languages languages, ILogg else { foreach (var ele in values) + { // Private access hack // The idx field is the start of the JSON element, including markup that isn't directly part of the element itself if (field.GetValue(ele) is int idx) @@ -162,6 +163,7 @@ public TextContainer(string content, string language, Languages languages, ILogg yield return (eleString, location); } } + } } } @@ -243,7 +245,10 @@ private void PopulateCommentedStatesInternal(int index, string prefix, string su suffixLoc = FullContent.Length - 1; } - for (var i = prefixLoc; i <= suffixLoc; i++) CommentedStates[i] = true; + for (var i = prefixLoc; i <= suffixLoc; i++) + { + CommentedStates[i] = true; + } } } } @@ -320,12 +325,14 @@ public Boundary GetLineBoundary(int index) Boundary result = new(); for (var i = 0; i < LineEnds.Count; i++) + { if (LineEnds[i] >= index) { result.Index = LineStarts[i]; result.Length = LineEnds[i] - LineStarts[i] + 1; break; } + } return result; } @@ -355,6 +362,7 @@ public string GetLineContent(int line) public Location GetLocation(int index) { for (var i = 1; i < LineEnds.Count; i++) + { if (LineEnds[i] >= index) { return new Location @@ -363,6 +371,7 @@ public Location GetLocation(int index) Line = i }; } + } return new Location(); } diff --git a/AppInspector.RulesEngine/TypedRuleSet.cs b/AppInspector.RulesEngine/TypedRuleSet.cs index a61ba47a..56e9b347 100644 --- a/AppInspector.RulesEngine/TypedRuleSet.cs +++ b/AppInspector.RulesEngine/TypedRuleSet.cs @@ -48,10 +48,12 @@ public IEnumerator GetEnumerator() private IEnumerable AppInspectorRulesAsEnumerableT() { foreach (var rule in _rules) + { if (rule is T ruleAsT) { yield return ruleAsT; } + } } @@ -93,7 +95,9 @@ public void AddDirectory(string path, string? tag = null) } foreach (var filename in Directory.EnumerateFileSystemEntries(path, "*.json", SearchOption.AllDirectories)) + { AddFile(filename, tag); + } } /// @@ -140,7 +144,10 @@ public void AddString(string jsonString, string sourceName, string? tag = null) /// Collection of rules public void AddRange(IEnumerable? collection) { - foreach (var rule in collection ?? Array.Empty()) AddRule(rule); + foreach (var rule in collection ?? Array.Empty()) + { + AddRule(rule); + } } /// diff --git a/AppInspector.Tests/Commands/TestAnalyzeCmd.cs b/AppInspector.Tests/Commands/TestAnalyzeCmd.cs index 3658bad5..eeb82a3a 100644 --- a/AppInspector.Tests/Commands/TestAnalyzeCmd.cs +++ b/AppInspector.Tests/Commands/TestAnalyzeCmd.cs @@ -537,7 +537,10 @@ public async Task OverridesAsync() private void DeleteTestFiles(IEnumerable pathsToDelete) { - foreach (var path in pathsToDelete) File.Delete(path); + foreach (var path in pathsToDelete) + { + File.Delete(path); + } } /// diff --git a/AppInspector.Tests/DefaultRules/TestDefaultRules.cs b/AppInspector.Tests/DefaultRules/TestDefaultRules.cs index c3838c30..14ef6fcb 100644 --- a/AppInspector.Tests/DefaultRules/TestDefaultRules.cs +++ b/AppInspector.Tests/DefaultRules/TestDefaultRules.cs @@ -42,9 +42,15 @@ public void VerifyDefaultRulesAreValid() foreach (var unverified in result.Unverified) { logger.Log(LogLevel.Error, "Failed to validate {0}", unverified.RulesId); - foreach (var error in unverified.Errors) logger.Log(LogLevel.Error, error); + foreach (var error in unverified.Errors) + { + logger.Log(LogLevel.Error, error); + } - foreach (var oatError in unverified.OatIssues) logger.Log(LogLevel.Error, oatError.Description); + foreach (var oatError in unverified.OatIssues) + { + logger.Log(LogLevel.Error, oatError.Description); + } } logger.Log(LogLevel.Information, "{0} of {1} rules have positive self tests.", @@ -53,9 +59,14 @@ public void VerifyDefaultRulesAreValid() result.RuleStatusList.Count(x => x.HasNegativeSelfTests), result.RuleStatusList.Count); foreach (var rule in result.RuleStatusList.Where(x => !x.HasPositiveSelfTests)) + { logger.Log(LogLevel.Warning, "Rule {0} does not have any positive test cases", rule.RulesId); + } + foreach (var rule in result.RuleStatusList.Where(x => !x.HasNegativeSelfTests)) + { logger.Log(LogLevel.Warning, "Rule {0} does not have any negative test cases", rule.RulesId); + } Assert.AreEqual(VerifyRulesResult.ExitCode.Verified, result.ResultCode); Assert.AreNotEqual(0, result.RuleStatusList.Count); diff --git a/AppInspector.Tests/RuleProcessor/WithinClauseTests.cs b/AppInspector.Tests/RuleProcessor/WithinClauseTests.cs index 3b5c8b1c..2201fa3e 100644 --- a/AppInspector.Tests/RuleProcessor/WithinClauseTests.cs +++ b/AppInspector.Tests/RuleProcessor/WithinClauseTests.cs @@ -451,7 +451,11 @@ public void WithinClauseValidationTests(bool findingOnlySetting, bool findingReg RulesVerifier verifier = new(new RulesVerifierOptions { LoggerFactory = _loggerFactory }); var oatIssues = verifier.CheckIntegrity(rules).SelectMany(x => x.OatIssues); - foreach (var violation in oatIssues) _logger.LogDebug(violation.Description); + foreach (var violation in oatIssues) + { + _logger.LogDebug(violation.Description); + } + Assert.AreEqual(expectedNumIssues, verifier.CheckIntegrity(rules).Sum(x => x.OatIssues.Count())); } diff --git a/AppInspector.Tests/TestHelpers.cs b/AppInspector.Tests/TestHelpers.cs index 032e3f03..5205b47a 100644 --- a/AppInspector.Tests/TestHelpers.cs +++ b/AppInspector.Tests/TestHelpers.cs @@ -96,10 +96,12 @@ public static List GetTagsFromFile(string[] contentLines) int i; for (i = 0; i < contentLines.Length; i++) + { if (contentLines[i].Contains("[UniqueTags]")) { break; } + } i++; //get past marker while (!contentLines[i].Contains("Select Counters")) diff --git a/AppInspector/Commands/AnalyzeCommand.cs b/AppInspector/Commands/AnalyzeCommand.cs index 94c074ad..237c92d5 100644 --- a/AppInspector/Commands/AnalyzeCommand.cs +++ b/AppInspector/Commands/AnalyzeCommand.cs @@ -171,8 +171,15 @@ public AnalyzeCommand(AnalyzeOptions opt, ILoggerFactory? loggerFactory = null) //create metadata helper to wrap and help populate metadata from scan _metaDataHelper = new MetaDataHelper(string.Join(',', _options.SourcePath)); DateScanned = DateTime.Now; - foreach (var confidence in _options.ConfidenceFilters) _confidence |= confidence; - foreach (var severity in _options.SeverityFilters) _severity |= severity; + foreach (var confidence in _options.ConfidenceFilters) + { + _confidence |= confidence; + } + + foreach (var severity in _options.SeverityFilters) + { + _severity |= severity; + } _logger.LogTrace("AnalyzeCommand::ConfigSourcetoScan"); @@ -182,6 +189,7 @@ public AnalyzeCommand(AnalyzeOptions opt, ILoggerFactory? loggerFactory = null) } foreach (var entry in _options.SourcePath) + { if (Directory.Exists(entry)) { _srcfileList.AddRange(Directory.EnumerateFiles(entry, "*.*", SearchOption.AllDirectories)); @@ -194,6 +202,7 @@ public AnalyzeCommand(AnalyzeOptions opt, ILoggerFactory? loggerFactory = null) { throw new OpException(MsgHelp.FormatString(MsgHelp.ID.CMD_INVALID_FILE_OR_DIR, entry)); } + } if (_srcfileList.Count == 0) { @@ -233,7 +242,9 @@ public AnalyzeCommand(AnalyzeOptions opt, ILoggerFactory? loggerFactory = null) { foreach (var filename in Directory.EnumerateFileSystemEntries(_options.CustomRulesPath, "*.json", SearchOption.AllDirectories)) + { VerifyFile(filename); + } } else if (File.Exists(_options.CustomRulesPath)) { @@ -632,6 +643,7 @@ private IEnumerable EnumerateFileEntries() Extractor extractor = new(); // For every file, if the file isn't excluded return it, and if it is track the exclusion in the metadata foreach (var srcFile in _srcfileList) + { if (_fileExclusionList.Any(x => x.IsMatch(srcFile))) { _metaDataHelper?.Metadata.Files.Add(new FileRecord { FileName = srcFile, Status = ScanState.Skipped }); @@ -667,13 +679,17 @@ private IEnumerable EnumerateFileEntries() // This works if the contents contain any kind of file. // If the file is an archive this gets all the entries it contains. // If the file is not an archive, the stream is wrapped in a FileEntry container and yielded - foreach (var entry in extractor.Extract(srcFile, contents, opts)) yield return entry; + foreach (var entry in extractor.Extract(srcFile, contents, opts)) + { + yield return entry; + } } } // Be sure to close the stream after we are done processing it. contents?.Dispose(); } + } } /// @@ -686,6 +702,7 @@ private async IAsyncEnumerable GetFileEntriesAsync() Extractor extractor = new(); foreach (var srcFile in _srcfileList) + { if (_fileExclusionList.Any(x => x.IsMatch(srcFile))) { _metaDataHelper?.Metadata.Files.Add(new FileRecord { FileName = srcFile, Status = ScanState.Skipped }); @@ -698,8 +715,11 @@ private async IAsyncEnumerable GetFileEntriesAsync() Parallel = false, DenyFilters = _options.FilePathExclusions, MemoryStreamCutoff = 1 })) + { yield return entry; + } } + } } @@ -862,7 +882,10 @@ public AnalyzeResult GetResult() { try { - foreach (var entry in EnumerateFileEntries()) fileQueue.Add(entry); + foreach (var entry in EnumerateFileEntries()) + { + fileQueue.Add(entry); + } } catch (OverflowException e) { @@ -1020,11 +1043,13 @@ private void DoProcessing(IEnumerable fileEntries, CancellationTokenS cts.Cancel(); // Populate skips for all the entries we didn't process foreach (var entry in fileEntries.Where(x => _metaDataHelper.Files.All(y => x.FullPath != y.FileName))) + { _metaDataHelper.Files.Add(new FileRecord { AccessTime = entry.AccessTime, CreateTime = entry.CreateTime, ModifyTime = entry.ModifyTime, FileName = entry.FullPath, Status = ScanState.TimeOutSkipped }); + } } } else diff --git a/AppInspector/Commands/ExportTagsCommand.cs b/AppInspector/Commands/ExportTagsCommand.cs index 958fe0ae..2ce39b14 100644 --- a/AppInspector/Commands/ExportTagsCommand.cs +++ b/AppInspector/Commands/ExportTagsCommand.cs @@ -114,8 +114,12 @@ public ExportTagsResult GetResult() HashSet tags = new(); foreach (var rule in _rules.GetAppInspectorRules()) - foreach (var tag in (IList?)rule.Tags ?? Array.Empty()) - tags.Add(tag); + { + foreach (var tag in (IList?)rule.Tags ?? Array.Empty()) + { + tags.Add(tag); + } + } exportTagsResult.TagsList = tags.ToList(); exportTagsResult.TagsList.Sort(); diff --git a/AppInspector/Commands/TagDiffCommand.cs b/AppInspector/Commands/TagDiffCommand.cs index 8ac67495..98550b53 100644 --- a/AppInspector/Commands/TagDiffCommand.cs +++ b/AppInspector/Commands/TagDiffCommand.cs @@ -224,18 +224,22 @@ public TagDiffResult GetResult() var added = list2.Except(list1); foreach (var add in added) + { tagDiffResult.TagDiffList.Add(new TagDiff { Source = TagDiff.DiffSource.Source2, Tag = add }); + } + foreach (var remove in removed) + { tagDiffResult.TagDiffList.Add(new TagDiff { Source = TagDiff.DiffSource.Source1, Tag = remove }); - + } if (tagDiffResult.TagDiffList.Count > 0) { diff --git a/AppInspector/MetaDataHelper.cs b/AppInspector/MetaDataHelper.cs index 49239133..c3c0fec0 100644 --- a/AppInspector/MetaDataHelper.cs +++ b/AppInspector/MetaDataHelper.cs @@ -66,6 +66,7 @@ public void AddTagsFromMatchRecord(MatchRecord matchRecord) { //special handling for standard characteristics in report foreach (string tag in matchRecord.Tags ?? Array.Empty()) + { switch (tag) { case "Metadata.Application.Author": @@ -108,6 +109,7 @@ public void AddTagsFromMatchRecord(MatchRecord matchRecord) break; } + } //Special handling; attempt to detect app types...review for multiple pattern rule limitation string solutionType = DetectSolutionType(matchRecord); @@ -190,7 +192,10 @@ public void PrepareReport() Metadata.Languages = new SortedDictionary(Languages); - foreach (MetricTagCounter metricTagCounter in TagCounters.Values) Metadata.TagCounters?.Add(metricTagCounter); + foreach (MetricTagCounter metricTagCounter in TagCounters.Values) + { + Metadata.TagCounters?.Add(metricTagCounter); + } } /// From 997493c87c95f06382eaf5a5c8894c3300b6075a Mon Sep 17 00:00:00 2001 From: Gabe Stocco <98900+gfs@users.noreply.github.com> Date: Thu, 2 Mar 2023 11:45:32 -0800 Subject: [PATCH 04/13] Revert "Formatting: Add Missing Braces Back" This reverts commit d6dc1fc5a757daf070d7ca74dbc0c32efb349600. --- .editorconfig | 228 ------------------ AppInspector.Benchmarks/DistinctBenchmarks.cs | 21 +- AppInspector.CLI/Program.cs | 5 +- AppInspector.CLI/Writers/AnalyzeHtmlWriter.cs | 94 ++------ .../Writers/AnalyzeSarifWriter.cs | 5 +- AppInspector.CLI/Writers/AnalyzeTextWriter.cs | 22 +- .../Writers/ExportTagsTextWriter.cs | 5 +- AppInspector.CLI/Writers/TagDiffTextWriter.cs | 2 - .../Writers/VerifyRulesTextWriter.cs | 2 - AppInspector.RulesEngine/AbstractRuleSet.cs | 4 - AppInspector.RulesEngine/Languages.cs | 6 - .../OatRegexWithIndexOperation.cs | 8 - .../OatExtensions/WithinOperation.cs | 5 +- AppInspector.RulesEngine/RuleProcessor.cs | 34 +-- AppInspector.RulesEngine/RulesVerifier.cs | 28 +-- AppInspector.RulesEngine/TextContainer.cs | 11 +- AppInspector.RulesEngine/TypedRuleSet.cs | 9 +- AppInspector.Tests/Commands/TestAnalyzeCmd.cs | 5 +- .../DefaultRules/TestDefaultRules.cs | 15 +- .../RuleProcessor/WithinClauseTests.cs | 6 +- AppInspector.Tests/TestHelpers.cs | 2 - AppInspector/Commands/AnalyzeCommand.cs | 33 +-- AppInspector/Commands/ExportTagsCommand.cs | 8 +- AppInspector/Commands/TagDiffCommand.cs | 6 +- AppInspector/MetaDataHelper.cs | 7 +- 25 files changed, 68 insertions(+), 503 deletions(-) delete mode 100644 .editorconfig diff --git a/.editorconfig b/.editorconfig deleted file mode 100644 index 835ef2b8..00000000 --- a/.editorconfig +++ /dev/null @@ -1,228 +0,0 @@ -# Remove the line below if you want to inherit .editorconfig settings from higher directories -root = true - -# C# files -[*.cs] - -#### Core EditorConfig Options #### - -# Indentation and spacing -indent_size = 4 -indent_style = space -tab_width = 4 - -# New line preferences -end_of_line = crlf -insert_final_newline = false - -#### .NET Coding Conventions #### - -# Organize usings -dotnet_separate_import_directive_groups = false -dotnet_sort_system_directives_first = false -file_header_template = unset - -# this. and Me. preferences -dotnet_style_qualification_for_event = false:warning -dotnet_style_qualification_for_field = false -dotnet_style_qualification_for_method = false:warning -dotnet_style_qualification_for_property = false:warning - -# Language keywords vs BCL types preferences -dotnet_style_predefined_type_for_locals_parameters_members = true:warning -dotnet_style_predefined_type_for_member_access = true:warning - -# Parentheses preferences -dotnet_style_parentheses_in_arithmetic_binary_operators = always_for_clarity -dotnet_style_parentheses_in_other_binary_operators = always_for_clarity -dotnet_style_parentheses_in_other_operators = never_if_unnecessary -dotnet_style_parentheses_in_relational_binary_operators = always_for_clarity - -# Modifier preferences -dotnet_style_require_accessibility_modifiers = for_non_interface_members - -# Expression-level preferences -dotnet_style_coalesce_expression = true -dotnet_style_collection_initializer = true -dotnet_style_explicit_tuple_names = true -dotnet_style_namespace_match_folder = true -dotnet_style_null_propagation = true -dotnet_style_object_initializer = true -dotnet_style_operator_placement_when_wrapping = beginning_of_line -dotnet_style_prefer_auto_properties = true:warning -dotnet_style_prefer_compound_assignment = true -dotnet_style_prefer_conditional_expression_over_assignment = true -dotnet_style_prefer_conditional_expression_over_return = true -dotnet_style_prefer_foreach_explicit_cast_in_source = when_strongly_typed -dotnet_style_prefer_inferred_anonymous_type_member_names = true -dotnet_style_prefer_inferred_tuple_names = true -dotnet_style_prefer_is_null_check_over_reference_equality_method = true -dotnet_style_prefer_simplified_boolean_expressions = true -dotnet_style_prefer_simplified_interpolation = true - -# Field preferences -dotnet_style_readonly_field = true - -# Parameter preferences -dotnet_code_quality_unused_parameters = all - -# Suppression preferences -dotnet_remove_unnecessary_suppression_exclusions = none - -# New line preferences -dotnet_style_allow_multiple_blank_lines_experimental = false -dotnet_style_allow_statement_immediately_after_block_experimental = false - -#### C# Coding Conventions #### - -# var preferences -csharp_style_var_elsewhere = false:warning -csharp_style_var_for_built_in_types = false:warning -csharp_style_var_when_type_is_apparent = false:warning - -# Expression-bodied members -csharp_style_expression_bodied_accessors = true -csharp_style_expression_bodied_constructors = false -csharp_style_expression_bodied_indexers = true -csharp_style_expression_bodied_lambdas = true -csharp_style_expression_bodied_local_functions = false -csharp_style_expression_bodied_methods = false -csharp_style_expression_bodied_operators = when_on_single_line -csharp_style_expression_bodied_properties = true - -# Pattern matching preferences -csharp_style_pattern_matching_over_as_with_null_check = true -csharp_style_pattern_matching_over_is_with_cast_check = true -csharp_style_prefer_extended_property_pattern = true -csharp_style_prefer_not_pattern = true -csharp_style_prefer_pattern_matching = true:suggestion -csharp_style_prefer_switch_expression = true - -# Null-checking preferences -csharp_style_conditional_delegate_call = true - -# Modifier preferences -csharp_prefer_static_local_function = true -csharp_preferred_modifier_order = public,private,protected,internal,file,static,extern,new,virtual,abstract,sealed,override,readonly,unsafe,required,volatile,async -csharp_style_prefer_readonly_struct = true - -# Code-block preferences -csharp_prefer_braces = true:warning -csharp_prefer_simple_using_statement = true:warning -csharp_style_namespace_declarations = block_scoped:warning -csharp_style_prefer_method_group_conversion = true:warning -csharp_style_prefer_top_level_statements = false:warning - -# Expression-level preferences -csharp_prefer_simple_default_expression = true -csharp_style_deconstructed_variable_declaration = true -csharp_style_implicit_object_creation_when_type_is_apparent = true -csharp_style_inlined_variable_declaration = true -csharp_style_prefer_index_operator = true -csharp_style_prefer_local_over_anonymous_function = true -csharp_style_prefer_null_check_over_type_check = true -csharp_style_prefer_range_operator = true -csharp_style_prefer_tuple_swap = true -csharp_style_prefer_utf8_string_literals = true -csharp_style_throw_expression = true -csharp_style_unused_value_assignment_preference = discard_variable -csharp_style_unused_value_expression_statement_preference = discard_variable - -# 'using' directive preferences -csharp_using_directive_placement = inside_namespace:suggestion - -# New line preferences -csharp_style_allow_blank_line_after_colon_in_constructor_initializer_experimental = true -csharp_style_allow_blank_line_after_token_in_arrow_expression_clause_experimental = true -csharp_style_allow_blank_line_after_token_in_conditional_expression_experimental = true -csharp_style_allow_blank_lines_between_consecutive_braces_experimental = false -csharp_style_allow_embedded_statements_on_same_line_experimental = true - -#### C# Formatting Rules #### - -# New line preferences -csharp_new_line_before_catch = true -csharp_new_line_before_else = true -csharp_new_line_before_finally = true -csharp_new_line_before_members_in_anonymous_types = true -csharp_new_line_before_members_in_object_initializers = true -csharp_new_line_before_open_brace = all -csharp_new_line_between_query_expression_clauses = true - -# Indentation preferences -csharp_indent_block_contents = true -csharp_indent_braces = false -csharp_indent_case_contents = true -csharp_indent_case_contents_when_block = true -csharp_indent_labels = one_less_than_current -csharp_indent_switch_labels = true - -# Space preferences -csharp_space_after_cast = false -csharp_space_after_colon_in_inheritance_clause = true -csharp_space_after_comma = true -csharp_space_after_dot = false -csharp_space_after_keywords_in_control_flow_statements = true -csharp_space_after_semicolon_in_for_statement = true -csharp_space_around_binary_operators = before_and_after -csharp_space_around_declaration_statements = false -csharp_space_before_colon_in_inheritance_clause = true -csharp_space_before_comma = false -csharp_space_before_dot = false -csharp_space_before_open_square_brackets = false -csharp_space_before_semicolon_in_for_statement = false -csharp_space_between_empty_square_brackets = false -csharp_space_between_method_call_empty_parameter_list_parentheses = false -csharp_space_between_method_call_name_and_opening_parenthesis = false -csharp_space_between_method_call_parameter_list_parentheses = false -csharp_space_between_method_declaration_empty_parameter_list_parentheses = false -csharp_space_between_method_declaration_name_and_open_parenthesis = false -csharp_space_between_method_declaration_parameter_list_parentheses = false -csharp_space_between_parentheses = false -csharp_space_between_square_brackets = false - -# Wrapping preferences -csharp_preserve_single_line_blocks = true -csharp_preserve_single_line_statements = true - -#### Naming styles #### - -# Naming rules - -dotnet_naming_rule.interface_should_be_begins_with_i.severity = suggestion -dotnet_naming_rule.interface_should_be_begins_with_i.symbols = interface -dotnet_naming_rule.interface_should_be_begins_with_i.style = begins_with_i - -dotnet_naming_rule.types_should_be_pascal_case.severity = suggestion -dotnet_naming_rule.types_should_be_pascal_case.symbols = types -dotnet_naming_rule.types_should_be_pascal_case.style = pascal_case - -dotnet_naming_rule.non_field_members_should_be_pascal_case.severity = suggestion -dotnet_naming_rule.non_field_members_should_be_pascal_case.symbols = non_field_members -dotnet_naming_rule.non_field_members_should_be_pascal_case.style = pascal_case - -# Symbol specifications - -dotnet_naming_symbols.interface.applicable_kinds = interface -dotnet_naming_symbols.interface.applicable_accessibilities = public, internal, private, protected, protected_internal, private_protected -dotnet_naming_symbols.interface.required_modifiers = - -dotnet_naming_symbols.types.applicable_kinds = class, struct, interface, enum -dotnet_naming_symbols.types.applicable_accessibilities = public, internal, private, protected, protected_internal, private_protected -dotnet_naming_symbols.types.required_modifiers = - -dotnet_naming_symbols.non_field_members.applicable_kinds = property, event, method -dotnet_naming_symbols.non_field_members.applicable_accessibilities = public, internal, private, protected, protected_internal, private_protected -dotnet_naming_symbols.non_field_members.required_modifiers = - -# Naming styles - -dotnet_naming_style.pascal_case.required_prefix = -dotnet_naming_style.pascal_case.required_suffix = -dotnet_naming_style.pascal_case.word_separator = -dotnet_naming_style.pascal_case.capitalization = pascal_case - -dotnet_naming_style.begins_with_i.required_prefix = I -dotnet_naming_style.begins_with_i.required_suffix = -dotnet_naming_style.begins_with_i.word_separator = -dotnet_naming_style.begins_with_i.capitalization = pascal_case diff --git a/AppInspector.Benchmarks/DistinctBenchmarks.cs b/AppInspector.Benchmarks/DistinctBenchmarks.cs index a02553c2..be8ccd1a 100644 --- a/AppInspector.Benchmarks/DistinctBenchmarks.cs +++ b/AppInspector.Benchmarks/DistinctBenchmarks.cs @@ -18,11 +18,9 @@ public List OldCode() SortedDictionary uniqueTags = new(); List outList = new(); foreach (var r in ruleSet) - { //builds a list of unique tags - foreach (var t in (IList?)r?.Tags ?? Array.Empty()) - { - if (uniqueTags.ContainsKey(t)) + foreach (var t in (IList?)r?.Tags ?? Array.Empty()) + if (uniqueTags.ContainsKey(t)) { continue; } @@ -30,14 +28,9 @@ public List OldCode() { uniqueTags.Add(t, t); } - } - } //generate results list - foreach (var s in uniqueTags.Values) - { - outList.Add(s); - } + foreach (var s in uniqueTags.Values) outList.Add(s); return outList; } @@ -47,13 +40,9 @@ public List HashSet() { HashSet hashSet = new(); foreach (var r in ruleSet) - { //builds a list of unique tags - foreach (var t in (IList?)r?.Tags ?? Array.Empty()) - { - hashSet.Add(t); - } - } + foreach (var t in (IList?)r?.Tags ?? Array.Empty()) + hashSet.Add(t); var theList = hashSet.ToList(); theList.Sort(); diff --git a/AppInspector.CLI/Program.cs b/AppInspector.CLI/Program.cs index d43a11c5..067a7a61 100644 --- a/AppInspector.CLI/Program.cs +++ b/AppInspector.CLI/Program.cs @@ -335,10 +335,7 @@ private static int RunAnalyzeCommand(CLIAnalyzeCmdOptions cliOptions) done = true; }); - while (!done) - { - Thread.Sleep(100); - } + while (!done) Thread.Sleep(100); pbar.ObservedError = !initialSuccess; pbar.Message = !initialSuccess diff --git a/AppInspector.CLI/Writers/AnalyzeHtmlWriter.cs b/AppInspector.CLI/Writers/AnalyzeHtmlWriter.cs index 4330c867..6944fa0e 100644 --- a/AppInspector.CLI/Writers/AnalyzeHtmlWriter.cs +++ b/AppInspector.CLI/Writers/AnalyzeHtmlWriter.cs @@ -111,9 +111,7 @@ private void WriteHtmlResult() //add summary values for sorted tags lists of taginfo foreach (var outerKey in KeyedSortedTagInfoLists.Keys) - { hashData.Add(outerKey, KeyedSortedTagInfoLists[outerKey]); - } hashData["cputargets"] = _appMetaData?.CPUTargets; hashData["apptypes"] = _appMetaData?.AppTypes ?? new List(); @@ -214,10 +212,7 @@ private void RenderResultsSafeforHTML() private void SafeList(List? valuesList) { - for (var i = 0; i < valuesList?.Count; i++) - { - valuesList[i] = SafeString(valuesList[i]); - } + for (var i = 0; i < valuesList?.Count; i++) valuesList[i] = SafeString(valuesList[i]); } private string SafeString(string? value) @@ -265,8 +260,7 @@ public void PopulateTagGroups() //for each preferred group of tag patterns determine if at least one instance was detected foreach (var tagCategory in TagGroupPreferences ?? new List()) - { - foreach (var tagGroup in tagCategory.Groups ?? new List()) + foreach (var tagGroup in tagCategory.Groups ?? new List()) { if (string.IsNullOrEmpty(tagGroup.Title)) { @@ -302,7 +296,6 @@ public void PopulateTagGroups() } } } - } //create simple ranked page lists HTML use KeyedSortedTagInfoLists["tagGrpAllTagsByConfidence"] = GetTagInfoListByConfidence(); @@ -320,19 +313,14 @@ public List GetCategoryTagGroups(string category) List result = new(); //get all tag groups for specified category foreach (var categoryTagGroup in TagGroupPreferences ?? new List()) - { if (categoryTagGroup.Name == category) { result = categoryTagGroup.Groups ?? new List(); break; } - } //now get all matches for that group i.e. Authentication - foreach (var group in result) - { - GetTagInfoListByTagGroup(group); - } + foreach (var group in result) GetTagInfoListByTagGroup(group); return result; } @@ -346,12 +334,8 @@ private HashSet GetUniqueTags() HashSet results = new(); foreach (var match in _appMetaData?.Matches ?? new List()) - { - foreach (var tag in match.Tags ?? Array.Empty()) - { - results.Add(tag); - } - } + foreach (var tag in match.Tags ?? Array.Empty()) + results.Add(tag); return results; } @@ -369,17 +353,14 @@ private List GetTagInfoListByTagGroup(TagGroup tagGroup, bool addNotFou HashSet hashSet = new(); foreach (var pattern in tagGroup.Patterns ?? new List()) - { if (pattern.Detected) //set at program.RollUp already so don't search for again { var tagPatternRegex = pattern.Expression; if (_appMetaData?.TotalMatchesCount > 0) { foreach (var match in _appMetaData?.Matches ?? new List()) - { - foreach (var tagItem in match.Tags ?? Array.Empty()) - { - if (tagPatternRegex.IsMatch(tagItem)) + foreach (var tagItem in match.Tags ?? Array.Empty()) + if (tagPatternRegex.IsMatch(tagItem)) { if (!hashSet.Contains(pattern.SearchPattern)) { @@ -401,8 +382,7 @@ private List GetTagInfoListByTagGroup(TagGroup tagGroup, bool addNotFou { //ensure we get highest confidence, severity as there are likely multiple matches for this tag pattern foreach (var updateItem in result) - { - if (updateItem.Tag == tagItem) + if (updateItem.Tag == tagItem) { Confidence oldConfidence; Enum.TryParse(updateItem.Confidence, out oldConfidence); @@ -422,16 +402,12 @@ private List GetTagInfoListByTagGroup(TagGroup tagGroup, bool addNotFou break; } - } - } - } + } } - } } else { foreach (var tagItem in _appMetaData?.UniqueTags ?? new List()) - { if (tagPatternRegex.IsMatch(tagItem) && !hashSet.Contains(pattern.SearchPattern)) { result.Add(new TagInfo @@ -444,7 +420,6 @@ private List GetTagInfoListByTagGroup(TagGroup tagGroup, bool addNotFou hashSet.Add(tagItem); } - } } } else if (addNotFound) //allow to report on false presense items @@ -463,7 +438,6 @@ private List GetTagInfoListByTagGroup(TagGroup tagGroup, bool addNotFou result.Add(tagInfo); hashSet.Add(tagInfo.Tag); } - } return result; } @@ -480,16 +454,13 @@ private List GetAllMatchingTagInfoList(TagGroup tagGroup, bool addNotFo HashSet hashSet = new(); foreach (var pattern in tagGroup.Patterns ?? new List()) - { if (pattern.Detected) { var tagPatternRegex = pattern.Expression; foreach (var match in _appMetaData?.Matches ?? new List()) - { - foreach (var tagItem in match.Tags ?? Array.Empty()) - { - if (tagPatternRegex.IsMatch(tagItem)) + foreach (var tagItem in match.Tags ?? Array.Empty()) + if (tagPatternRegex.IsMatch(tagItem)) { if (!hashSet.Contains(tagItem)) { @@ -509,8 +480,7 @@ private List GetAllMatchingTagInfoList(TagGroup tagGroup, bool addNotFo { //ensure we have highest confidence, severity as there are likly multiple matches for this tag pattern foreach (var updateItem in result) - { - if (updateItem.Tag == tagItem) + if (updateItem.Tag == tagItem) { Confidence oldConfidence; Enum.TryParse(updateItem.Confidence, out oldConfidence); @@ -530,13 +500,9 @@ private List GetAllMatchingTagInfoList(TagGroup tagGroup, bool addNotFo break; } - } - } - } + } } - } } - } return result; } @@ -551,14 +517,11 @@ private List GetTagInfoListByName() List result = new(); foreach (var tag in _appMetaData?.UniqueTags ?? new List()) - { if (_appMetaData?.TotalMatchesCount > 0) { foreach (var match in _appMetaData?.Matches ?? new List()) - { - foreach (var testTag in match.Tags ?? Array.Empty()) - { - if (tag == testTag) + foreach (var testTag in match.Tags ?? Array.Empty()) + if (tag == testTag) { if (dupCheck.Add(testTag)) { @@ -573,8 +536,6 @@ private List GetTagInfoListByName() break; } } - } - } } else { @@ -584,7 +545,6 @@ private List GetTagInfoListByName() ShortTag = tag[(tag.LastIndexOf('.') + 1)..] }); } - } return result; } @@ -603,12 +563,9 @@ private List GetTagInfoListByConfidence() { var searchPattern = new Regex(tag, RegexOptions.IgnoreCase); foreach (var confidence in confidences) - { - foreach (var match in _appMetaData?.Matches ?? new List()) - { - foreach (var testTag in match.Tags ?? Array.Empty()) - { - if (searchPattern.IsMatch(testTag)) + foreach (var match in _appMetaData?.Matches ?? new List()) + foreach (var testTag in match.Tags ?? Array.Empty()) + if (searchPattern.IsMatch(testTag)) { if (match.Confidence == confidence && dupCheck.Add(tag)) { @@ -621,9 +578,6 @@ private List GetTagInfoListByConfidence() }); } } - } - } - } } return result; @@ -644,12 +598,9 @@ private List GetTagInfoListBySeverity() { var searchPattern = new Regex(tag, RegexOptions.IgnoreCase); foreach (var severity in severities) - { - foreach (var match in _appMetaData?.Matches ?? new List()) - { - foreach (var testTag in match.Tags ?? Array.Empty()) - { - if (searchPattern.IsMatch(testTag)) + foreach (var match in _appMetaData?.Matches ?? new List()) + foreach (var testTag in match.Tags ?? Array.Empty()) + if (searchPattern.IsMatch(testTag)) { if (match.Severity == severity && dupCheck.Add(tag)) { @@ -662,9 +613,6 @@ private List GetTagInfoListBySeverity() }); } } - } - } - } } return result; diff --git a/AppInspector.CLI/Writers/AnalyzeSarifWriter.cs b/AppInspector.CLI/Writers/AnalyzeSarifWriter.cs index dc763f1a..37adfca2 100644 --- a/AppInspector.CLI/Writers/AnalyzeSarifWriter.cs +++ b/AppInspector.CLI/Writers/AnalyzeSarifWriter.cs @@ -26,10 +26,7 @@ internal static void AddRange(this TagsCollection tc, IEnumerable? tagsT return; } - foreach (var tag in tagsToAdd) - { - tc.Add(tag); - } + foreach (var tag in tagsToAdd) tc.Add(tag); } } diff --git a/AppInspector.CLI/Writers/AnalyzeTextWriter.cs b/AppInspector.CLI/Writers/AnalyzeTextWriter.cs index a8001461..826eb6d1 100644 --- a/AppInspector.CLI/Writers/AnalyzeTextWriter.cs +++ b/AppInspector.CLI/Writers/AnalyzeTextWriter.cs @@ -48,10 +48,7 @@ public override void WriteResults(Result result, CLICommandOptions commandOption WriteDependencies(analyzeResult.Metadata); TextWriter.WriteLine(MakeHeading("Match Details")); - foreach (var match in analyzeResult.Metadata.Matches ?? new List()) - { - WriteMatch(match); - } + foreach (var match in analyzeResult.Metadata.Matches ?? new List()) WriteMatch(match); if (autoClose) { @@ -92,10 +89,7 @@ private string MakeHeading(string header) { StringBuilder build = new(); build.Append(string.Format("[{0}]", header)); - for (var i = header.Length; i < COLUMN_MAX; i++) - { - build.Append("-"); - } + for (var i = header.Length; i < COLUMN_MAX; i++) build.Append("-"); return build.ToString(); } @@ -136,16 +130,11 @@ public void WriteAppMeta(MetaData metaData) TextWriter.WriteLine($"Unique matches: {metaData.UniqueMatchesCount}"); TextWriter.WriteLine(MakeHeading("UniqueTags")); - foreach (var tag in metaData.UniqueTags) - { - TextWriter.WriteLine(tag); - } + foreach (var tag in metaData.UniqueTags) TextWriter.WriteLine(tag); TextWriter.WriteLine(MakeHeading("Select Counters")); foreach (var tagCounter in metaData.TagCounters ?? new List()) - { TextWriter.WriteLine($"Tagname: {tagCounter.Tag}, Count: {tagCounter.Count}"); - } } public void WriteMatch(MatchRecord match) @@ -182,9 +171,6 @@ private void WriteDependencies(MetaData metaData) TextWriter.WriteLine(MakeHeading("Dependencies")); - foreach (var s in metaData.UniqueDependencies ?? new List()) - { - TextWriter.WriteLine(s); - } + foreach (var s in metaData.UniqueDependencies ?? new List()) TextWriter.WriteLine(s); } } \ No newline at end of file diff --git a/AppInspector.CLI/Writers/ExportTagsTextWriter.cs b/AppInspector.CLI/Writers/ExportTagsTextWriter.cs index ef8360bc..fd7d50a3 100644 --- a/AppInspector.CLI/Writers/ExportTagsTextWriter.cs +++ b/AppInspector.CLI/Writers/ExportTagsTextWriter.cs @@ -31,10 +31,7 @@ public override void WriteResults(Result result, CLICommandOptions commandOption { TextWriter.WriteLine("Results"); - foreach (var tag in exportTagsResult.TagsList) - { - TextWriter.WriteLine(tag); - } + foreach (var tag in exportTagsResult.TagsList) TextWriter.WriteLine(tag); } else { diff --git a/AppInspector.CLI/Writers/TagDiffTextWriter.cs b/AppInspector.CLI/Writers/TagDiffTextWriter.cs index e100a7c3..87e85f67 100644 --- a/AppInspector.CLI/Writers/TagDiffTextWriter.cs +++ b/AppInspector.CLI/Writers/TagDiffTextWriter.cs @@ -39,9 +39,7 @@ public override void WriteResults(Result result, CLICommandOptions commandOption { TextWriter.WriteLine("Differences"); foreach (var tagDiff in tagDiffResult.TagDiffList) - { TextWriter.WriteLine("Tag: {0}, Only found in file: {1}", tagDiff.Tag, tagDiff.Source); - } } if (autoClose) diff --git a/AppInspector.CLI/Writers/VerifyRulesTextWriter.cs b/AppInspector.CLI/Writers/VerifyRulesTextWriter.cs index 38849fce..6b819dff 100644 --- a/AppInspector.CLI/Writers/VerifyRulesTextWriter.cs +++ b/AppInspector.CLI/Writers/VerifyRulesTextWriter.cs @@ -40,10 +40,8 @@ public override void WriteResults(Result result, CLICommandOptions commandOption { TextWriter.WriteLine("Rule status"); foreach (var ruleStatus in verifyRulesResult.RuleStatusList) - { TextWriter.WriteLine("Ruleid: {0}, Rulename: {1}, Status: {2}", ruleStatus.RulesId, ruleStatus.RulesName, ruleStatus.Verified); - } } if (autoClose) diff --git a/AppInspector.RulesEngine/AbstractRuleSet.cs b/AppInspector.RulesEngine/AbstractRuleSet.cs index c5186ae8..cbf4e7d1 100644 --- a/AppInspector.RulesEngine/AbstractRuleSet.cs +++ b/AppInspector.RulesEngine/AbstractRuleSet.cs @@ -78,7 +78,6 @@ public IEnumerable GetUniversalRules() var clauseNumber = 0; var expression = new StringBuilder("("); foreach (var pattern in rule.Patterns) - { if (GenerateClause(pattern, clauseNumber) is { } clause) { clauses.Add(clause); @@ -94,7 +93,6 @@ public IEnumerable GetUniversalRules() { _logger.LogWarning("Clause could not be generated from pattern {pattern}", pattern.Pattern); } - } if (clauses.Count > 0) { @@ -152,7 +150,6 @@ public IEnumerable GetUniversalRules() if (m.Success) { for (var i = 1; i < m.Groups.Count; i++) - { if (int.TryParse(m.Groups[i].Value, out var value)) { argList.Add(value); @@ -161,7 +158,6 @@ public IEnumerable GetUniversalRules() { break; } - } } if (argList.Count == 2) diff --git a/AppInspector.RulesEngine/Languages.cs b/AppInspector.RulesEngine/Languages.cs index f6e492c1..51f8d9da 100644 --- a/AppInspector.RulesEngine/Languages.cs +++ b/AppInspector.RulesEngine/Languages.cs @@ -141,13 +141,11 @@ public string GetCommentInline(string language) if (language != null) { foreach (var comment in _comments) - { if (Array.Exists(comment.Languages ?? new[] { "" }, x => x.Equals(language, StringComparison.InvariantCultureIgnoreCase)) && comment.Inline is { }) { return comment.Inline; } - } } return result; @@ -165,13 +163,11 @@ public string GetCommentPrefix(string language) if (language != null) { foreach (var comment in _comments) - { if ((comment.Languages?.Contains(language.ToLower(CultureInfo.InvariantCulture)) ?? false) && comment.Prefix is { }) { return comment.Prefix; } - } } return result; @@ -189,13 +185,11 @@ public string GetCommentSuffix(string language) if (language != null) { foreach (var comment in _comments) - { if (Array.Exists(comment.Languages ?? new[] { "" }, x => x.Equals(language, StringComparison.InvariantCultureIgnoreCase)) && comment.Suffix is { }) { return comment.Suffix; } - } } return result; diff --git a/AppInspector.RulesEngine/OatExtensions/OatRegexWithIndexOperation.cs b/AppInspector.RulesEngine/OatExtensions/OatRegexWithIndexOperation.cs index 645b1183..0d942d70 100644 --- a/AppInspector.RulesEngine/OatExtensions/OatRegexWithIndexOperation.cs +++ b/AppInspector.RulesEngine/OatExtensions/OatRegexWithIndexOperation.cs @@ -49,7 +49,6 @@ private static IEnumerable RegexWithIndexValidationDelegate(CST.OAT.R else if (clause.Data is List regexList) { foreach (var regex in regexList) - { if (!Helpers.IsValidRegex(regex)) { yield return new Violation( @@ -57,7 +56,6 @@ private static IEnumerable RegexWithIndexValidationDelegate(CST.OAT.R clause.Label ?? rule.Clauses.IndexOf(clause).ToString(CultureInfo.InvariantCulture), regex), rule, clause); } - } } if (clause.DictData?.Count > 0) @@ -93,7 +91,6 @@ private OperationResult RegexWithIndexOperationDelegate(Clause clause, object? s if (Analyzer != null) { foreach (var regexString in RegexList) - { if (StringToRegex(regexString, regexOpts) is { } regex) { if (src.XPaths is not null) @@ -115,9 +112,7 @@ private OperationResult RegexWithIndexOperationDelegate(Clause clause, object? s { var targets = tc.GetStringFromJsonPath(jsonPath); foreach (var target in targets) - { outmatches.AddRange(GetMatches(regex, tc, clause, src.Scopes, target.Item2)); - } } } @@ -145,7 +140,6 @@ private OperationResult RegexWithIndexOperationDelegate(Clause clause, object? s } } } - } var result = src.Invert ? outmatches.Count == 0 : outmatches.Count > 0; return new OperationResult(result, @@ -162,7 +156,6 @@ private OperationResult RegexWithIndexOperationDelegate(Clause clause, object? s Boundary? boundary) { foreach (var match in regex.Matches(boundary is null ? tc.FullContent : tc.GetBoundaryText(boundary))) - { if (match is Match m) { Boundary translatedBoundary = new() @@ -180,7 +173,6 @@ private OperationResult RegexWithIndexOperationDelegate(Clause clause, object? s yield return (patternIndex, translatedBoundary); } } - } } /// diff --git a/AppInspector.RulesEngine/OatExtensions/WithinOperation.cs b/AppInspector.RulesEngine/OatExtensions/WithinOperation.cs index de2d17a4..af0a2308 100644 --- a/AppInspector.RulesEngine/OatExtensions/WithinOperation.cs +++ b/AppInspector.RulesEngine/OatExtensions/WithinOperation.cs @@ -192,10 +192,7 @@ public IEnumerable WithinValidationDelegate(CST.OAT.Rule rule, Clause } else { - foreach (var violation in subOp.ValidationDelegate.Invoke(rule, wc.SubClause)) - { - yield return violation; - } + foreach (var violation in subOp.ValidationDelegate.Invoke(rule, wc.SubClause)) yield return violation; if (wc.SubClause is OatRegexWithIndexClause oatRegexWithIndexClause) { diff --git a/AppInspector.RulesEngine/RuleProcessor.cs b/AppInspector.RulesEngine/RuleProcessor.cs index 8faa95ab..4c5513e4 100644 --- a/AppInspector.RulesEngine/RuleProcessor.cs +++ b/AppInspector.RulesEngine/RuleProcessor.cs @@ -211,22 +211,17 @@ public List AnalyzeFile(TextContainer textContainer, FileEntry file // WithinClauses are always ANDed, but each contains all the captures that passed *that* clause. // We need the captures that passed every clause. foreach (var aCapture in allCaptured) - { numberOfInstances.AddOrUpdate(aCapture, 1, (tuple, i) => i + 1); - } - return numberOfInstances.Where(x => x.Value == onlyWithinCaptures.Count).Select(x => x.Key) .ToList(); } var outList = new List<(int, Boundary)>(); foreach (var cap in captures) - { if (cap is TypedClauseCapture> tcc) { outList.AddRange(tcc.Result); } - } return outList; } @@ -235,21 +230,15 @@ public List AnalyzeFile(TextContainer textContainer, FileEntry file List removes = new(); foreach (var m in resultsList.Where(x => x.Rule?.Overrides?.Count > 0)) - { - foreach (var idsToOverride in (IList)m.Rule?.Overrides ?? Array.Empty()) - { - // Find all overriden rules and mark them for removal from issues list - foreach (var om in resultsList.FindAll(x => x.Rule?.Id == idsToOverride)) - { - // If the overridden match is a subset of the overriding match - if (om.Boundary.Index >= m.Boundary.Index && + foreach (var idsToOverride in (IList)m.Rule?.Overrides ?? Array.Empty()) + // Find all overriden rules and mark them for removal from issues list + foreach (var om in resultsList.FindAll(x => x.Rule?.Id == idsToOverride)) + // If the overridden match is a subset of the overriding match + if (om.Boundary.Index >= m.Boundary.Index && om.Boundary.Index <= m.Boundary.Index + m.Boundary.Length) { removes.Add(om); } - } - } - } // Remove overriden rules resultsList.RemoveAll(x => removes.Contains(x)); @@ -349,10 +338,7 @@ public async Task> AnalyzeFileAsync(FileEntry fileEntry, Langu return resultsList; } - foreach (var cap in filteredCaptures) - { - resultsList.AddRange(ProcessBoundary(cap)); - } + foreach (var cap in filteredCaptures) resultsList.AddRange(ProcessBoundary(cap)); List ProcessBoundary(ClauseCapture cap) { @@ -438,17 +424,13 @@ List ProcessBoundary(ClauseCapture cap) } foreach (var ovrd in (IList?)m.Rule?.Overrides ?? Array.Empty()) - { // Find all overriden rules and mark them for removal from issues list - foreach (var om in resultsList.FindAll(x => x.Rule?.Id == ovrd)) - { - if (om.Boundary.Index >= m.Boundary.Index && + foreach (var om in resultsList.FindAll(x => x.Rule?.Id == ovrd)) + if (om.Boundary.Index >= m.Boundary.Index && om.Boundary.Index <= m.Boundary.Index + m.Boundary.Length) { removes.Add(om); } - } - } } // Remove overriden rules diff --git a/AppInspector.RulesEngine/RulesVerifier.cs b/AppInspector.RulesEngine/RulesVerifier.cs index 523d4035..30fe7c32 100644 --- a/AppInspector.RulesEngine/RulesVerifier.cs +++ b/AppInspector.RulesEngine/RulesVerifier.cs @@ -87,19 +87,17 @@ public List CheckIntegrity(AbstractRuleSet ruleSet) _logger.LogError(MsgHelp.GetString(MsgHelp.ID.VERIFY_RULES_DUPLICATEID_FAIL), rule.Key); var relevantStati = ruleStatuses.Where(x => x.RulesId == rule.Key); foreach (var status in relevantStati) - { status.Errors = status.Errors.Append(MsgHelp.FormatString(MsgHelp.ID.VERIFY_RULES_DUPLICATEID_FAIL, rule.Key)); - } } } var allTags = ruleSet.GetAppInspectorRules().SelectMany(x => x.Tags ?? Array.Empty()).ToList(); var rulesWithDependsOnWithNoMatchingTags = ruleSet.GetAppInspectorRules().Where(x => !x.DependsOnTags?.All(tag => allTags.Contains(tag)) ?? false); - foreach (var dependslessRule in rulesWithDependsOnWithNoMatchingTags) + foreach(var dependslessRule in rulesWithDependsOnWithNoMatchingTags) { _logger.LogError(MsgHelp.GetString(MsgHelp.ID.VERIFY_RULES_DEPENDS_ON_TAG_MISSING), dependslessRule.Id); - foreach (var status in ruleStatuses.Where(x => x.RulesId == dependslessRule.Id)) + foreach(var status in ruleStatuses.Where(x => x.RulesId == dependslessRule.Id)) { status.Errors = status.Errors.Append(MsgHelp.FormatString(MsgHelp.ID.VERIFY_RULES_DEPENDS_ON_TAG_MISSING, dependslessRule.Id)); } @@ -124,20 +122,19 @@ public RuleStatus CheckIntegrity(ConvertedOatRule convertedOatRule) if (rule.AppliesTo != null) { var languages = _options.LanguageSpecs.GetNames(); - foreach (string lang in - // Check for unknown language - from lang in rule.AppliesTo - where !string.IsNullOrEmpty(lang) - where !languages.Any(x => x.Equals(lang, StringComparison.CurrentCultureIgnoreCase)) - select lang) - { - _logger.LogError(MsgHelp.GetString(MsgHelp.ID.VERIFY_RULES_LANGUAGE_FAIL), rule.Id ?? "", lang); - errors.Add(MsgHelp.FormatString(MsgHelp.ID.VERIFY_RULES_LANGUAGE_FAIL, rule.Id ?? "", lang)); - } + // Check for unknown language + foreach (var lang in rule.AppliesTo) + if (!string.IsNullOrEmpty(lang)) + { + if (!languages.Any(x => x.Equals(lang, StringComparison.CurrentCultureIgnoreCase))) + { + _logger.LogError(MsgHelp.GetString(MsgHelp.ID.VERIFY_RULES_LANGUAGE_FAIL), rule.Id ?? "", lang); + errors.Add(MsgHelp.FormatString(MsgHelp.ID.VERIFY_RULES_LANGUAGE_FAIL, rule.Id ?? "", lang)); + } + } } foreach (var pattern in (IList?)rule.FileRegexes ?? Array.Empty()) - { try { _ = new Regex(pattern, RegexOptions.Compiled); @@ -149,7 +146,6 @@ from lang in rule.AppliesTo errors.Add(MsgHelp.FormatString(MsgHelp.ID.VERIFY_RULES_REGEX_FAIL, rule.Id ?? "", pattern ?? "", e.Message)); } - } //valid search pattern foreach (var searchPattern in rule.Patterns ?? Array.Empty()) diff --git a/AppInspector.RulesEngine/TextContainer.cs b/AppInspector.RulesEngine/TextContainer.cs index a8eb63fc..f86b2687 100644 --- a/AppInspector.RulesEngine/TextContainer.cs +++ b/AppInspector.RulesEngine/TextContainer.cs @@ -146,7 +146,6 @@ public TextContainer(string content, string language, Languages languages, ILogg else { foreach (var ele in values) - { // Private access hack // The idx field is the start of the JSON element, including markup that isn't directly part of the element itself if (field.GetValue(ele) is int idx) @@ -163,7 +162,6 @@ public TextContainer(string content, string language, Languages languages, ILogg yield return (eleString, location); } } - } } } @@ -245,10 +243,7 @@ private void PopulateCommentedStatesInternal(int index, string prefix, string su suffixLoc = FullContent.Length - 1; } - for (var i = prefixLoc; i <= suffixLoc; i++) - { - CommentedStates[i] = true; - } + for (var i = prefixLoc; i <= suffixLoc; i++) CommentedStates[i] = true; } } } @@ -325,14 +320,12 @@ public Boundary GetLineBoundary(int index) Boundary result = new(); for (var i = 0; i < LineEnds.Count; i++) - { if (LineEnds[i] >= index) { result.Index = LineStarts[i]; result.Length = LineEnds[i] - LineStarts[i] + 1; break; } - } return result; } @@ -362,7 +355,6 @@ public string GetLineContent(int line) public Location GetLocation(int index) { for (var i = 1; i < LineEnds.Count; i++) - { if (LineEnds[i] >= index) { return new Location @@ -371,7 +363,6 @@ public Location GetLocation(int index) Line = i }; } - } return new Location(); } diff --git a/AppInspector.RulesEngine/TypedRuleSet.cs b/AppInspector.RulesEngine/TypedRuleSet.cs index 56e9b347..a61ba47a 100644 --- a/AppInspector.RulesEngine/TypedRuleSet.cs +++ b/AppInspector.RulesEngine/TypedRuleSet.cs @@ -48,12 +48,10 @@ public IEnumerator GetEnumerator() private IEnumerable AppInspectorRulesAsEnumerableT() { foreach (var rule in _rules) - { if (rule is T ruleAsT) { yield return ruleAsT; } - } } @@ -95,9 +93,7 @@ public void AddDirectory(string path, string? tag = null) } foreach (var filename in Directory.EnumerateFileSystemEntries(path, "*.json", SearchOption.AllDirectories)) - { AddFile(filename, tag); - } } /// @@ -144,10 +140,7 @@ public void AddString(string jsonString, string sourceName, string? tag = null) /// Collection of rules public void AddRange(IEnumerable? collection) { - foreach (var rule in collection ?? Array.Empty()) - { - AddRule(rule); - } + foreach (var rule in collection ?? Array.Empty()) AddRule(rule); } /// diff --git a/AppInspector.Tests/Commands/TestAnalyzeCmd.cs b/AppInspector.Tests/Commands/TestAnalyzeCmd.cs index eeb82a3a..3658bad5 100644 --- a/AppInspector.Tests/Commands/TestAnalyzeCmd.cs +++ b/AppInspector.Tests/Commands/TestAnalyzeCmd.cs @@ -537,10 +537,7 @@ public async Task OverridesAsync() private void DeleteTestFiles(IEnumerable pathsToDelete) { - foreach (var path in pathsToDelete) - { - File.Delete(path); - } + foreach (var path in pathsToDelete) File.Delete(path); } /// diff --git a/AppInspector.Tests/DefaultRules/TestDefaultRules.cs b/AppInspector.Tests/DefaultRules/TestDefaultRules.cs index 14ef6fcb..c3838c30 100644 --- a/AppInspector.Tests/DefaultRules/TestDefaultRules.cs +++ b/AppInspector.Tests/DefaultRules/TestDefaultRules.cs @@ -42,15 +42,9 @@ public void VerifyDefaultRulesAreValid() foreach (var unverified in result.Unverified) { logger.Log(LogLevel.Error, "Failed to validate {0}", unverified.RulesId); - foreach (var error in unverified.Errors) - { - logger.Log(LogLevel.Error, error); - } + foreach (var error in unverified.Errors) logger.Log(LogLevel.Error, error); - foreach (var oatError in unverified.OatIssues) - { - logger.Log(LogLevel.Error, oatError.Description); - } + foreach (var oatError in unverified.OatIssues) logger.Log(LogLevel.Error, oatError.Description); } logger.Log(LogLevel.Information, "{0} of {1} rules have positive self tests.", @@ -59,14 +53,9 @@ public void VerifyDefaultRulesAreValid() result.RuleStatusList.Count(x => x.HasNegativeSelfTests), result.RuleStatusList.Count); foreach (var rule in result.RuleStatusList.Where(x => !x.HasPositiveSelfTests)) - { logger.Log(LogLevel.Warning, "Rule {0} does not have any positive test cases", rule.RulesId); - } - foreach (var rule in result.RuleStatusList.Where(x => !x.HasNegativeSelfTests)) - { logger.Log(LogLevel.Warning, "Rule {0} does not have any negative test cases", rule.RulesId); - } Assert.AreEqual(VerifyRulesResult.ExitCode.Verified, result.ResultCode); Assert.AreNotEqual(0, result.RuleStatusList.Count); diff --git a/AppInspector.Tests/RuleProcessor/WithinClauseTests.cs b/AppInspector.Tests/RuleProcessor/WithinClauseTests.cs index 2201fa3e..3b5c8b1c 100644 --- a/AppInspector.Tests/RuleProcessor/WithinClauseTests.cs +++ b/AppInspector.Tests/RuleProcessor/WithinClauseTests.cs @@ -451,11 +451,7 @@ public void WithinClauseValidationTests(bool findingOnlySetting, bool findingReg RulesVerifier verifier = new(new RulesVerifierOptions { LoggerFactory = _loggerFactory }); var oatIssues = verifier.CheckIntegrity(rules).SelectMany(x => x.OatIssues); - foreach (var violation in oatIssues) - { - _logger.LogDebug(violation.Description); - } - + foreach (var violation in oatIssues) _logger.LogDebug(violation.Description); Assert.AreEqual(expectedNumIssues, verifier.CheckIntegrity(rules).Sum(x => x.OatIssues.Count())); } diff --git a/AppInspector.Tests/TestHelpers.cs b/AppInspector.Tests/TestHelpers.cs index 5205b47a..032e3f03 100644 --- a/AppInspector.Tests/TestHelpers.cs +++ b/AppInspector.Tests/TestHelpers.cs @@ -96,12 +96,10 @@ public static List GetTagsFromFile(string[] contentLines) int i; for (i = 0; i < contentLines.Length; i++) - { if (contentLines[i].Contains("[UniqueTags]")) { break; } - } i++; //get past marker while (!contentLines[i].Contains("Select Counters")) diff --git a/AppInspector/Commands/AnalyzeCommand.cs b/AppInspector/Commands/AnalyzeCommand.cs index 237c92d5..94c074ad 100644 --- a/AppInspector/Commands/AnalyzeCommand.cs +++ b/AppInspector/Commands/AnalyzeCommand.cs @@ -171,15 +171,8 @@ public AnalyzeCommand(AnalyzeOptions opt, ILoggerFactory? loggerFactory = null) //create metadata helper to wrap and help populate metadata from scan _metaDataHelper = new MetaDataHelper(string.Join(',', _options.SourcePath)); DateScanned = DateTime.Now; - foreach (var confidence in _options.ConfidenceFilters) - { - _confidence |= confidence; - } - - foreach (var severity in _options.SeverityFilters) - { - _severity |= severity; - } + foreach (var confidence in _options.ConfidenceFilters) _confidence |= confidence; + foreach (var severity in _options.SeverityFilters) _severity |= severity; _logger.LogTrace("AnalyzeCommand::ConfigSourcetoScan"); @@ -189,7 +182,6 @@ public AnalyzeCommand(AnalyzeOptions opt, ILoggerFactory? loggerFactory = null) } foreach (var entry in _options.SourcePath) - { if (Directory.Exists(entry)) { _srcfileList.AddRange(Directory.EnumerateFiles(entry, "*.*", SearchOption.AllDirectories)); @@ -202,7 +194,6 @@ public AnalyzeCommand(AnalyzeOptions opt, ILoggerFactory? loggerFactory = null) { throw new OpException(MsgHelp.FormatString(MsgHelp.ID.CMD_INVALID_FILE_OR_DIR, entry)); } - } if (_srcfileList.Count == 0) { @@ -242,9 +233,7 @@ public AnalyzeCommand(AnalyzeOptions opt, ILoggerFactory? loggerFactory = null) { foreach (var filename in Directory.EnumerateFileSystemEntries(_options.CustomRulesPath, "*.json", SearchOption.AllDirectories)) - { VerifyFile(filename); - } } else if (File.Exists(_options.CustomRulesPath)) { @@ -643,7 +632,6 @@ private IEnumerable EnumerateFileEntries() Extractor extractor = new(); // For every file, if the file isn't excluded return it, and if it is track the exclusion in the metadata foreach (var srcFile in _srcfileList) - { if (_fileExclusionList.Any(x => x.IsMatch(srcFile))) { _metaDataHelper?.Metadata.Files.Add(new FileRecord { FileName = srcFile, Status = ScanState.Skipped }); @@ -679,17 +667,13 @@ private IEnumerable EnumerateFileEntries() // This works if the contents contain any kind of file. // If the file is an archive this gets all the entries it contains. // If the file is not an archive, the stream is wrapped in a FileEntry container and yielded - foreach (var entry in extractor.Extract(srcFile, contents, opts)) - { - yield return entry; - } + foreach (var entry in extractor.Extract(srcFile, contents, opts)) yield return entry; } } // Be sure to close the stream after we are done processing it. contents?.Dispose(); } - } } /// @@ -702,7 +686,6 @@ private async IAsyncEnumerable GetFileEntriesAsync() Extractor extractor = new(); foreach (var srcFile in _srcfileList) - { if (_fileExclusionList.Any(x => x.IsMatch(srcFile))) { _metaDataHelper?.Metadata.Files.Add(new FileRecord { FileName = srcFile, Status = ScanState.Skipped }); @@ -715,11 +698,8 @@ private async IAsyncEnumerable GetFileEntriesAsync() Parallel = false, DenyFilters = _options.FilePathExclusions, MemoryStreamCutoff = 1 })) - { yield return entry; - } } - } } @@ -882,10 +862,7 @@ public AnalyzeResult GetResult() { try { - foreach (var entry in EnumerateFileEntries()) - { - fileQueue.Add(entry); - } + foreach (var entry in EnumerateFileEntries()) fileQueue.Add(entry); } catch (OverflowException e) { @@ -1043,13 +1020,11 @@ private void DoProcessing(IEnumerable fileEntries, CancellationTokenS cts.Cancel(); // Populate skips for all the entries we didn't process foreach (var entry in fileEntries.Where(x => _metaDataHelper.Files.All(y => x.FullPath != y.FileName))) - { _metaDataHelper.Files.Add(new FileRecord { AccessTime = entry.AccessTime, CreateTime = entry.CreateTime, ModifyTime = entry.ModifyTime, FileName = entry.FullPath, Status = ScanState.TimeOutSkipped }); - } } } else diff --git a/AppInspector/Commands/ExportTagsCommand.cs b/AppInspector/Commands/ExportTagsCommand.cs index 2ce39b14..958fe0ae 100644 --- a/AppInspector/Commands/ExportTagsCommand.cs +++ b/AppInspector/Commands/ExportTagsCommand.cs @@ -114,12 +114,8 @@ public ExportTagsResult GetResult() HashSet tags = new(); foreach (var rule in _rules.GetAppInspectorRules()) - { - foreach (var tag in (IList?)rule.Tags ?? Array.Empty()) - { - tags.Add(tag); - } - } + foreach (var tag in (IList?)rule.Tags ?? Array.Empty()) + tags.Add(tag); exportTagsResult.TagsList = tags.ToList(); exportTagsResult.TagsList.Sort(); diff --git a/AppInspector/Commands/TagDiffCommand.cs b/AppInspector/Commands/TagDiffCommand.cs index 98550b53..8ac67495 100644 --- a/AppInspector/Commands/TagDiffCommand.cs +++ b/AppInspector/Commands/TagDiffCommand.cs @@ -224,22 +224,18 @@ public TagDiffResult GetResult() var added = list2.Except(list1); foreach (var add in added) - { tagDiffResult.TagDiffList.Add(new TagDiff { Source = TagDiff.DiffSource.Source2, Tag = add }); - } - foreach (var remove in removed) - { tagDiffResult.TagDiffList.Add(new TagDiff { Source = TagDiff.DiffSource.Source1, Tag = remove }); - } + if (tagDiffResult.TagDiffList.Count > 0) { diff --git a/AppInspector/MetaDataHelper.cs b/AppInspector/MetaDataHelper.cs index c3c0fec0..49239133 100644 --- a/AppInspector/MetaDataHelper.cs +++ b/AppInspector/MetaDataHelper.cs @@ -66,7 +66,6 @@ public void AddTagsFromMatchRecord(MatchRecord matchRecord) { //special handling for standard characteristics in report foreach (string tag in matchRecord.Tags ?? Array.Empty()) - { switch (tag) { case "Metadata.Application.Author": @@ -109,7 +108,6 @@ public void AddTagsFromMatchRecord(MatchRecord matchRecord) break; } - } //Special handling; attempt to detect app types...review for multiple pattern rule limitation string solutionType = DetectSolutionType(matchRecord); @@ -192,10 +190,7 @@ public void PrepareReport() Metadata.Languages = new SortedDictionary(Languages); - foreach (MetricTagCounter metricTagCounter in TagCounters.Values) - { - Metadata.TagCounters?.Add(metricTagCounter); - } + foreach (MetricTagCounter metricTagCounter in TagCounters.Values) Metadata.TagCounters?.Add(metricTagCounter); } /// From ab3813ab47de92a733fae4f13cb155305f63026d Mon Sep 17 00:00:00 2001 From: Gabe Stocco <98900+gfs@users.noreply.github.com> Date: Fri, 3 Mar 2023 14:27:12 -0800 Subject: [PATCH 05/13] Set beta flag --- version.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/version.json b/version.json index 9da0829d..1d49c9f3 100644 --- a/version.json +++ b/version.json @@ -1,6 +1,6 @@ { "$schema": "https://raw.githubusercontent.com/dotnet/Nerdbank.GitVersioning/master/src/NerdBank.GitVersioning/version.schema.json", - "version": "1.8", + "version": "1.8-beta", "publicReleaseRefSpec": [ "^refs/heads/main$", "^refs/heads/v\\d+(?:\\.\\d+)?$" @@ -10,4 +10,4 @@ "enabled": true } } -} \ No newline at end of file +} From 3af3afc37a6a7eb97c709ea308f35e066bf28a2a Mon Sep 17 00:00:00 2001 From: Gabe Stocco <98900+gfs@users.noreply.github.com> Date: Fri, 3 Mar 2023 14:58:46 -0800 Subject: [PATCH 06/13] Fix Verification message for missing depends_on_tags Change a few more List to IList. Improve edge case in verification where if two rules shared the same id they would both receive the error message about depends_on_tags. --- .../Properties/Resources.Designer.cs | 11 +++++++- AppInspector.Common/Properties/Resources.resx | 4 +-- AppInspector.RulesEngine/RuleStatus.cs | 1 + AppInspector.RulesEngine/RulesVerifier.cs | 26 +++++++++++++++---- .../RulesVerifierResult.cs | 4 +-- AppInspector/Commands/VerifyRulesCommand.cs | 2 +- 6 files changed, 37 insertions(+), 11 deletions(-) diff --git a/AppInspector.Common/Properties/Resources.Designer.cs b/AppInspector.Common/Properties/Resources.Designer.cs index 431c38e8..4a00b4f9 100644 --- a/AppInspector.Common/Properties/Resources.Designer.cs +++ b/AppInspector.Common/Properties/Resources.Designer.cs @@ -585,7 +585,16 @@ internal static string VERIFY_RULE_LOADFILE_FAILED { } /// - /// Looks up a localized string similar to Rule {0} failed verification. depends_on_tags is set but the specified tags are not set by any rules in the set.. + /// Looks up a localized string similar to Rule {0} failed verification. depends_on_tags is set but the the following tags do not exist in the RuleSet: {1}. + /// + internal static string VERIFY_RULES_DEPENDS_ON_TAG_MISSING { + get { + return ResourceManager.GetString("VERIFY_RULES_DEPENDS_ON_TAG_MISSING", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Rule {0} failed from dupicate rule id specified. /// internal static string VERIFY_RULES_DUPLICATEID_FAIL { get { diff --git a/AppInspector.Common/Properties/Resources.resx b/AppInspector.Common/Properties/Resources.resx index 6896b83a..22f38a51 100644 --- a/AppInspector.Common/Properties/Resources.resx +++ b/AppInspector.Common/Properties/Resources.resx @@ -302,8 +302,8 @@ Rule {0} failed from dupicate rule id specified - - Rule {0} failed verification. depends_on_tags is set but the specified tags are not set by any rules in the set. + + Rule {0} failed verification. depends_on_tags is set but the the following tags do not exist in the RuleSet: {1} Rule {0} failed from unrecognized language {1} specified diff --git a/AppInspector.RulesEngine/RuleStatus.cs b/AppInspector.RulesEngine/RuleStatus.cs index 239f2db7..e57762d8 100644 --- a/AppInspector.RulesEngine/RuleStatus.cs +++ b/AppInspector.RulesEngine/RuleStatus.cs @@ -13,4 +13,5 @@ public class RuleStatus public IEnumerable OatIssues { get; set; } = Enumerable.Empty(); public bool HasPositiveSelfTests { get; set; } public bool HasNegativeSelfTests { get; set; } + internal Rule Rule { get; set; } } \ No newline at end of file diff --git a/AppInspector.RulesEngine/RulesVerifier.cs b/AppInspector.RulesEngine/RulesVerifier.cs index 30fe7c32..e7df9d2a 100644 --- a/AppInspector.RulesEngine/RulesVerifier.cs +++ b/AppInspector.RulesEngine/RulesVerifier.cs @@ -69,7 +69,12 @@ public RulesVerifierResult Verify(AbstractRuleSet ruleset) return new RulesVerifierResult(CheckIntegrity(ruleset), ruleset); } - public List CheckIntegrity(AbstractRuleSet ruleSet) + /// + /// Check an for rules errors + /// + /// The rule set to check + /// An with a for each in the + public IList CheckIntegrity(AbstractRuleSet ruleSet) { List ruleStatuses = new(); foreach (var rule in ruleSet.GetOatRules()) @@ -79,6 +84,7 @@ public List CheckIntegrity(AbstractRuleSet ruleSet) ruleStatuses.Add(ruleVerified); } + // By default unique IDs are required for rules if (!_options.DisableRequireUniqueIds) { var duplicatedRules = ruleSet.GetAppInspectorRules().GroupBy(x => x.Id).Where(y => y.Count() > 1); @@ -92,14 +98,15 @@ public List CheckIntegrity(AbstractRuleSet ruleSet) } } + // Check for the presence of the `depends_on` field and ensure that any tags which are depended on exist in the full set of rules var allTags = ruleSet.GetAppInspectorRules().SelectMany(x => x.Tags ?? Array.Empty()).ToList(); var rulesWithDependsOnWithNoMatchingTags = ruleSet.GetAppInspectorRules().Where(x => !x.DependsOnTags?.All(tag => allTags.Contains(tag)) ?? false); foreach(var dependslessRule in rulesWithDependsOnWithNoMatchingTags) { - _logger.LogError(MsgHelp.GetString(MsgHelp.ID.VERIFY_RULES_DEPENDS_ON_TAG_MISSING), dependslessRule.Id); - foreach(var status in ruleStatuses.Where(x => x.RulesId == dependslessRule.Id)) + _logger.LogError(MsgHelp.GetString(MsgHelp.ID.VERIFY_RULES_DEPENDS_ON_TAG_MISSING), dependslessRule.Id, string.Join(',', dependslessRule.Tags?.Where(tag => !allTags.Contains(tag)) ?? Array.Empty())); + foreach(var status in ruleStatuses.Where(x => x.Rule == dependslessRule)) { - status.Errors = status.Errors.Append(MsgHelp.FormatString(MsgHelp.ID.VERIFY_RULES_DEPENDS_ON_TAG_MISSING, dependslessRule.Id)); + status.Errors = status.Errors.Append(MsgHelp.FormatString(MsgHelp.ID.VERIFY_RULES_DEPENDS_ON_TAG_MISSING, dependslessRule.Id, string.Join(',',dependslessRule.DependsOnTags?.Where(tag => !allTags.Contains(tag)) ?? Array.Empty()))); } } return ruleStatuses; @@ -134,6 +141,7 @@ public RuleStatus CheckIntegrity(ConvertedOatRule convertedOatRule) } } + // Check that regexes for filenames are valid foreach (var pattern in (IList?)rule.FileRegexes ?? Array.Empty()) try { @@ -150,6 +158,7 @@ public RuleStatus CheckIntegrity(ConvertedOatRule convertedOatRule) //valid search pattern foreach (var searchPattern in rule.Patterns ?? Array.Empty()) { + // Check that pattern regex arguments are valid if (searchPattern.PatternType == PatternType.RegexWord || searchPattern.PatternType == PatternType.Regex) { try @@ -173,6 +182,7 @@ public RuleStatus CheckIntegrity(ConvertedOatRule convertedOatRule) } } + // Check that JsonPaths are valid if (searchPattern.JsonPaths is not null) { foreach (var jsonPath in searchPattern.JsonPaths) @@ -192,6 +202,7 @@ public RuleStatus CheckIntegrity(ConvertedOatRule convertedOatRule) } } + // Check that XPaths are valid if (searchPattern.XPaths is not null) { foreach (var xpath in searchPattern.XPaths) @@ -209,6 +220,7 @@ public RuleStatus CheckIntegrity(ConvertedOatRule convertedOatRule) } } + // Check that YamlPaths are valid if (searchPattern.YamlPaths is not null) { foreach (var yamlPath in searchPattern.YamlPaths) @@ -306,12 +318,14 @@ public RuleStatus CheckIntegrity(ConvertedOatRule convertedOatRule) } } - if (rule.Tags?.Count == 0) + // Check for at least one tag being populated + if ((rule.Tags?.Count ?? 0) == 0) { _logger?.LogError("Rule must specify tags. {0}", rule.Id); errors.Add($"Rule must specify tags. {rule.Id}"); } + // If RequireMustMatch is set every rule must have a must-match self-test if (_options.RequireMustMatch) { if (rule.MustMatch?.Any() is not true) @@ -321,6 +335,7 @@ public RuleStatus CheckIntegrity(ConvertedOatRule convertedOatRule) } } + // If RequireMustNotMatch is set every rule must have a must-not-match self-test if (_options.RequireMustNotMatch) { if (rule.MustNotMatch?.Any() is not true) @@ -332,6 +347,7 @@ public RuleStatus CheckIntegrity(ConvertedOatRule convertedOatRule) return new RuleStatus { + Rule = rule, RulesId = rule.Id, RulesName = rule.Name, Errors = errors, diff --git a/AppInspector.RulesEngine/RulesVerifierResult.cs b/AppInspector.RulesEngine/RulesVerifierResult.cs index 1541d4dc..ea8afe0d 100644 --- a/AppInspector.RulesEngine/RulesVerifierResult.cs +++ b/AppInspector.RulesEngine/RulesVerifierResult.cs @@ -5,13 +5,13 @@ namespace Microsoft.ApplicationInspector.RulesEngine; public class RulesVerifierResult { - public RulesVerifierResult(List ruleStatuses, AbstractRuleSet compiledRuleSets) + public RulesVerifierResult(IList ruleStatuses, AbstractRuleSet compiledRuleSets) { RuleStatuses = ruleStatuses; CompiledRuleSet = compiledRuleSets; } - public List RuleStatuses { get; } + public IList RuleStatuses { get; } public AbstractRuleSet CompiledRuleSet { get; } public bool Verified => RuleStatuses.All(x => x.Verified); } \ No newline at end of file diff --git a/AppInspector/Commands/VerifyRulesCommand.cs b/AppInspector/Commands/VerifyRulesCommand.cs index f3a0a958..40fe9a7d 100644 --- a/AppInspector/Commands/VerifyRulesCommand.cs +++ b/AppInspector/Commands/VerifyRulesCommand.cs @@ -44,7 +44,7 @@ public VerifyRulesResult() public ExitCode ResultCode { get; set; } [JsonPropertyName("ruleStatusList")] - public List RuleStatusList { get; set; } + public IList RuleStatusList { get; set; } [JsonIgnore] public IEnumerable Unverified => RuleStatusList.Where(x => !x.Verified); } From f6e1aea3c28a81b7d543880bb5ec2e77d897aea0 Mon Sep 17 00:00:00 2001 From: Gabe Stocco <98900+gfs@users.noreply.github.com> Date: Fri, 3 Mar 2023 15:01:46 -0800 Subject: [PATCH 07/13] Improve description for test rules --- AppInspector.Tests/Commands/TestAnalyzeCmd.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/AppInspector.Tests/Commands/TestAnalyzeCmd.cs b/AppInspector.Tests/Commands/TestAnalyzeCmd.cs index 3658bad5..737c50c4 100644 --- a/AppInspector.Tests/Commands/TestAnalyzeCmd.cs +++ b/AppInspector.Tests/Commands/TestAnalyzeCmd.cs @@ -25,7 +25,6 @@ public class TestAnalyzeCmd private const int numTimeOutFiles = 25; private const int numTimesContent = 25; - private const string dependsOnOneWay = @"[ { ""id"": ""SA000005"", @@ -35,7 +34,7 @@ public class TestAnalyzeCmd ], ""depends_on_tags"": [""Dependee""], ""severity"": ""Critical"", - ""description"": ""This rule finds windows 2000"", + ""description"": ""This rule finds windows 2000 and is dependent on the Dependee tag"", ""patterns"": [ { ""pattern"": ""windows 2000"", @@ -58,7 +57,7 @@ public class TestAnalyzeCmd ""Dependee"" ], ""severity"": ""Critical"", - ""description"": ""This rule finds linux"", + ""description"": ""This rule finds linux and is depended on to provide the Dependee tag"", ""patterns"": [ { ""pattern"": ""linux"", @@ -84,7 +83,7 @@ public class TestAnalyzeCmd ], ""depends_on_tags"": [""RuleTwo""], ""severity"": ""Critical"", - ""description"": ""This rule finds windows 2000"", + ""description"": ""This rule finds windows 2000 and is dependent the RuleTwo tag"", ""patterns"": [ { ""pattern"": ""windows 2000"", @@ -108,7 +107,7 @@ public class TestAnalyzeCmd ], ""depends_on_tags"": [""RuleOne""], ""severity"": ""Critical"", - ""description"": ""This rule finds linux"", + ""description"": ""This rule finds linux and is dependent the RuleOne tag"", ""patterns"": [ { ""pattern"": ""linux"", From ca294e7a45672f68c1ebd5479112eb65aa0c1b76 Mon Sep 17 00:00:00 2001 From: Gabe Stocco <98900+gfs@users.noreply.github.com> Date: Fri, 3 Mar 2023 15:24:37 -0800 Subject: [PATCH 08/13] Formatting and additional comments on the Rule and AnalyzeCommand objects --- AppInspector.RulesEngine/Rule.cs | 223 +++++++++++++--------- AppInspector.RulesEngine/RuleProcessor.cs | 22 ++- AppInspector/Commands/AnalyzeCommand.cs | 45 +++++ 3 files changed, 195 insertions(+), 95 deletions(-) diff --git a/AppInspector.RulesEngine/Rule.cs b/AppInspector.RulesEngine/Rule.cs index 4b7a7f6c..59caea0d 100644 --- a/AppInspector.RulesEngine/Rule.cs +++ b/AppInspector.RulesEngine/Rule.cs @@ -1,105 +1,152 @@ // Copyright (C) Microsoft. All rights reserved. // Licensed under the MIT License. See LICENSE.txt in the project root for license information. -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text.Json.Serialization; -using System.Text.RegularExpressions; - -namespace Microsoft.ApplicationInspector.RulesEngine; - -/// -/// Class to hold the Rule -/// -public class Rule +namespace Microsoft.ApplicationInspector.RulesEngine { - private IList _compiled = Array.Empty(); - - private IList? _fileRegexes; - private bool _updateCompiledFileRegex; - - /// - /// Name of the source where the rule definition came from. - /// Typically file, database or other storage. - /// - [JsonIgnore] - public string? Source { get; set; } + using System; + using System.Collections.Generic; + using System.Linq; + using System.Text.Json.Serialization; + using System.Text.RegularExpressions; /// - /// Optional tag assigned to the rule during runtime + /// Class to hold the Rule /// - [JsonIgnore] - public string? RuntimeTag { get; set; } - - /// - /// Runtime flag to disable the rule - /// - [JsonIgnore] - public bool Disabled { get; set; } - - /// - /// Tags that are required to be present from other rules - regardless of file - for this match to be valid - /// - [JsonPropertyName("depends_on_tags")] public IList? DependsOnTags { get; set; } - - [JsonPropertyName("name")] public string Name { get; set; } = ""; - - [JsonPropertyName("id")] public string Id { get; set; } = ""; - - [JsonPropertyName("description")] - public string? Description { get; set; } = ""; - - [JsonPropertyName("does_not_apply_to")] - public IList? DoesNotApplyTo { get; set; } - - [JsonPropertyName("applies_to")] - public IList? AppliesTo { get; set; } - - [JsonPropertyName("applies_to_file_regex")] - public IList? FileRegexes + public class Rule { - get => _fileRegexes; - set + private IList _compiled = Array.Empty(); + + private IList? _fileRegexes; + private bool _updateCompiledFileRegex; + + /// + /// Name of the source where the rule definition came from. + /// Typically file, database or other storage. + /// + [JsonIgnore] + public string? Source { get; set; } + + /// + /// Optional tag assigned to the rule during runtime + /// + [JsonIgnore] + public string? RuntimeTag { get; set; } + + /// + /// Runtime flag to disable the rule + /// + [JsonIgnore] + public bool Disabled { get; set; } + + /// + /// Tags that are required to be present in the total result set - even from other rules matched against other files - for this match to be valid + /// Does not work with `TagsOnly` option. + /// + [JsonPropertyName("depends_on_tags")] public IList? DependsOnTags { get; set; } + + /// + /// Human readable name for the rule + /// + [JsonPropertyName("name")] + public string Name { get; set; } = ""; + + /// + /// Id for the rule, by default IDs must be unique. + /// + [JsonPropertyName("id")] + public string Id { get; set; } = ""; + + /// + /// Human readable description for the rule + /// + [JsonPropertyName("description")] + public string? Description { get; set; } = ""; + + /// + /// Languages that the rule does not apply to + /// + [JsonPropertyName("does_not_apply_to")] + public IList? DoesNotApplyTo { get; set; } + + /// + /// Languages that the rule does apply to, if empty, applies to all languages not in + /// + [JsonPropertyName("applies_to")] + public IList? AppliesTo { get; set; } + + /// + /// Regular expressions for file names that the Rule applies to + /// + [JsonPropertyName("applies_to_file_regex")] + public IList? FileRegexes { - _fileRegexes = value; - _updateCompiledFileRegex = true; + get => _fileRegexes; + set + { + _fileRegexes = value; + _updateCompiledFileRegex = true; + } } - } - [JsonIgnore] - public IEnumerable CompiledFileRegexes - { - get + /// + /// Internal API to cache construction of + /// + [JsonIgnore] + internal IEnumerable CompiledFileRegexes { - if (_updateCompiledFileRegex) + get { - _compiled = (IList?)FileRegexes?.Select(x => new Regex(x, RegexOptions.Compiled)).ToList() ?? Array.Empty(); - _updateCompiledFileRegex = false; - } + if (_updateCompiledFileRegex) + { + _compiled = (IList?)FileRegexes?.Select(x => new Regex(x, RegexOptions.Compiled)).ToList() ?? Array.Empty(); + _updateCompiledFileRegex = false; + } - return _compiled; + return _compiled; + } } - } - [JsonPropertyName("tags")] public IList? Tags { get; set; } - - [JsonPropertyName("severity")] - [JsonConverter(typeof(JsonStringEnumConverter))] - public Severity Severity { get; set; } = Severity.Moderate; - - [JsonPropertyName("overrides")] - public IList? Overrides { get; set; } - - [JsonPropertyName("patterns")] - public SearchPattern[] Patterns { get; set; } = Array.Empty(); - - [JsonPropertyName("conditions")] - public SearchCondition[]? Conditions { get; set; } - - [JsonPropertyName("must-match")] - public IList? MustMatch { get; set; } - - [JsonPropertyName("must-not-match")] - public IList? MustNotMatch { get; set; } + /// + /// The Tags the rule provides + /// + [JsonPropertyName("tags")] + public IList? Tags { get; set; } + + /// + /// Severity for the rule + /// + [JsonPropertyName("severity")] + [JsonConverter(typeof(JsonStringEnumConverter))] + public Severity Severity { get; set; } = Severity.Moderate; + + /// + /// Other rules that this rule overrides + /// + [JsonPropertyName("overrides")] + public IList? Overrides { get; set; } + + /// + /// When any pattern matches, the rule applies + /// + [JsonPropertyName("patterns")] + public SearchPattern[] Patterns { get; set; } = Array.Empty(); + + /// + /// If any patterns match, and any conditions are set, all conditions must also match + /// + [JsonPropertyName("conditions")] + public SearchCondition[]? Conditions { get; set; } + + /// + /// Optional list of self-test sample texts that the rule must match to be considered valid. + /// + [JsonPropertyName("must-match")] + public IList? MustMatch { get; set; } + + /// + /// Optional list of self-test sample texts that the rule must not match to be considered valid. + /// + [JsonPropertyName("must-not-match")] + public IList? MustNotMatch { get; set; } + } } \ No newline at end of file diff --git a/AppInspector.RulesEngine/RuleProcessor.cs b/AppInspector.RulesEngine/RuleProcessor.cs index 4c5513e4..6e30126f 100644 --- a/AppInspector.RulesEngine/RuleProcessor.cs +++ b/AppInspector.RulesEngine/RuleProcessor.cs @@ -211,7 +211,9 @@ public List AnalyzeFile(TextContainer textContainer, FileEntry file // WithinClauses are always ANDed, but each contains all the captures that passed *that* clause. // We need the captures that passed every clause. foreach (var aCapture in allCaptured) + { numberOfInstances.AddOrUpdate(aCapture, 1, (tuple, i) => i + 1); + } return numberOfInstances.Where(x => x.Value == onlyWithinCaptures.Count).Select(x => x.Key) .ToList(); } @@ -230,15 +232,21 @@ public List AnalyzeFile(TextContainer textContainer, FileEntry file List removes = new(); foreach (var m in resultsList.Where(x => x.Rule?.Overrides?.Count > 0)) - foreach (var idsToOverride in (IList)m.Rule?.Overrides ?? Array.Empty()) - // Find all overriden rules and mark them for removal from issues list - foreach (var om in resultsList.FindAll(x => x.Rule?.Id == idsToOverride)) - // If the overridden match is a subset of the overriding match - if (om.Boundary.Index >= m.Boundary.Index && - om.Boundary.Index <= m.Boundary.Index + m.Boundary.Length) + { + foreach (var idsToOverride in m.Rule?.Overrides ?? Array.Empty()) { - removes.Add(om); + // Find all overriden rules and mark them for removal from issues list + foreach (var om in resultsList.FindAll(x => x.Rule?.Id == idsToOverride)) + { + // If the overridden match is a subset of the overriding match + if (om.Boundary.Index >= m.Boundary.Index && + om.Boundary.Index <= m.Boundary.Index + m.Boundary.Length) + { + removes.Add(om); + } + } } + } // Remove overriden rules resultsList.RemoveAll(x => removes.Contains(x)); diff --git a/AppInspector/Commands/AnalyzeCommand.cs b/AppInspector/Commands/AnalyzeCommand.cs index 94c074ad..fd4c06a7 100644 --- a/AppInspector/Commands/AnalyzeCommand.cs +++ b/AppInspector/Commands/AnalyzeCommand.cs @@ -26,15 +26,36 @@ namespace Microsoft.ApplicationInspector.Commands; /// public class AnalyzeOptions { + /// + /// Any number of source paths to scan. + /// public IEnumerable SourcePath { get; set; } = Array.Empty(); + /// + /// A path (file or directory) on disk to your custom rule file(s). + /// public string? CustomRulesPath { get; set; } + /// + /// Disable the Default ApplicationInspector RuleSet. + /// public bool IgnoreDefaultRules { get; set; } + /// + /// Which confidence values to use. + /// public IEnumerable ConfidenceFilters { get; set; } = new[] { Confidence.High, Confidence.Medium }; + /// + /// Which severity values to use. + /// public IEnumerable SeverityFilters { get; set; } = new[] { Severity.Critical | Severity.Important | Severity.Moderate | Severity.BestPractice | Severity.ManualReview }; + /// + /// File paths which should be excluded from scanning. + /// public IEnumerable FilePathExclusions { get; set; } = Array.Empty(); + /// + /// If enabled, processing will be performed on one file at a time. + /// public bool SingleThread { get; set; } /// @@ -48,6 +69,9 @@ public class AnalyzeOptions /// public bool NoShowProgress { get; set; } = true; + /// + /// If enabled, only tags are collected, with no detailed match or file information. + /// public bool TagsOnly { get; set; } /// @@ -60,8 +84,17 @@ public class AnalyzeOptions /// public int ProcessingTimeOut { get; set; } = 0; + /// + /// Number of lines of Context to collect from each file for the Excerpt. Set to -1 to disable gathering context entirely. + /// public int ContextLines { get; set; } = 3; + /// + /// Run rules against files for which the appropriate Language cannot be determined. + /// public bool ScanUnknownTypes { get; set; } + /// + /// Don't gather metadata about the files scanned. + /// public bool NoFileMetadata { get; set; } /// @@ -70,7 +103,13 @@ public class AnalyzeOptions /// public int MaxNumMatchesPerTag { get; set; } = 0; + /// + /// A path to a custom comments.json file to modify the set of comment styles understood by Application Inspector. + /// public string? CustomCommentsPath { get; set; } + /// + /// A path to a custom languages.json file to modify the set of languages understood by Application Inspector. + /// public string? CustomLanguagesPath { get; set; } /// @@ -100,7 +139,13 @@ public class AnalyzeOptions /// public bool SuccessErrorCodeOnNoMatches { get; set; } + /// + /// If set, when validating rules, require that every rule have a must-match self-test with at least one entry + /// public bool RequireMustMatch { get; set; } + /// + /// If set, when validating rules, require that every rule have a must-not-match self-test with at least one entry + /// public bool RequireMustNotMatch { get; set; } } From 12571a3f70f56f44d39a9d806b43448bc2acdf7e Mon Sep 17 00:00:00 2001 From: Gabe Stocco <98900+gfs@users.noreply.github.com> Date: Fri, 3 Mar 2023 15:51:29 -0800 Subject: [PATCH 09/13] Catch edge case in verification where an overridden rule doesn't have the depends on of its overrider This verification step prevents a potential issue if a ruleset contains Rule A, which is Overridden by Rule B which depends on TagX. If RuleA and RuleB match, but TagX is not present, no results would be returned. This is because overrides are performed on a file by file basis, as the last step in processing each file. Depends on tags are checked after all files have been processed, and so the overridden matches are no longer tracked. --- AppInspector.Common/MsgHelp.cs | 3 +- .../Properties/Resources.Designer.cs | 9 ++ AppInspector.Common/Properties/Resources.resx | 5 +- AppInspector.RulesEngine/RulesVerifier.cs | 24 +++++- .../Commands/TestVerifyRulesCmd.cs | 85 +++++++++++++++++++ 5 files changed, 123 insertions(+), 3 deletions(-) diff --git a/AppInspector.Common/MsgHelp.cs b/AppInspector.Common/MsgHelp.cs index 34e8750d..c3de9f17 100644 --- a/AppInspector.Common/MsgHelp.cs +++ b/AppInspector.Common/MsgHelp.cs @@ -78,7 +78,8 @@ public enum ID PACK_MISSING_OUTPUT_ARG, PACK_RULES_NO_CLI_DEFAULT, PACK_RULES_NO_DEFAULT, - VERIFY_RULES_DEPENDS_ON_TAG_MISSING + VERIFY_RULES_DEPENDS_ON_TAG_MISSING, + VERIFY_RULES_OVERRIDDEN_RULE_DEPENDS_ON_TAG_MISSING } public static string GetString(ID id) diff --git a/AppInspector.Common/Properties/Resources.Designer.cs b/AppInspector.Common/Properties/Resources.Designer.cs index 4a00b4f9..d3d89d64 100644 --- a/AppInspector.Common/Properties/Resources.Designer.cs +++ b/AppInspector.Common/Properties/Resources.Designer.cs @@ -629,6 +629,15 @@ internal static string VERIFY_RULES_NULLID_FAIL { } } + /// + /// Looks up a localized string similar to Rule {0} failed verification. When a rule is overridden it must have all the depends_on_tags of the overridder, these tags are missing: {1}. + /// + internal static string VERIFY_RULES_OVERRIDDEN_RULE_DEPENDS_ON_TAG_MISSING { + get { + return ResourceManager.GetString("VERIFY_RULES_OVERRIDDEN_RULE_DEPENDS_ON_TAG_MISSING", resourceCulture); + } + } + /// /// Looks up a localized string similar to Rule {0} failed from invalid regex '{1}' with {2}. /// diff --git a/AppInspector.Common/Properties/Resources.resx b/AppInspector.Common/Properties/Resources.resx index 22f38a51..38d356da 100644 --- a/AppInspector.Common/Properties/Resources.resx +++ b/AppInspector.Common/Properties/Resources.resx @@ -305,7 +305,10 @@ Rule {0} failed verification. depends_on_tags is set but the the following tags do not exist in the RuleSet: {1} - + + Rule {0} failed verification. When a rule is overridden it must have all the depends_on_tags of the overridder, these tags are missing: {1} + + Rule {0} failed from unrecognized language {1} specified diff --git a/AppInspector.RulesEngine/RulesVerifier.cs b/AppInspector.RulesEngine/RulesVerifier.cs index e7df9d2a..622338c5 100644 --- a/AppInspector.RulesEngine/RulesVerifier.cs +++ b/AppInspector.RulesEngine/RulesVerifier.cs @@ -103,12 +103,34 @@ public IList CheckIntegrity(AbstractRuleSet ruleSet) var rulesWithDependsOnWithNoMatchingTags = ruleSet.GetAppInspectorRules().Where(x => !x.DependsOnTags?.All(tag => allTags.Contains(tag)) ?? false); foreach(var dependslessRule in rulesWithDependsOnWithNoMatchingTags) { - _logger.LogError(MsgHelp.GetString(MsgHelp.ID.VERIFY_RULES_DEPENDS_ON_TAG_MISSING), dependslessRule.Id, string.Join(',', dependslessRule.Tags?.Where(tag => !allTags.Contains(tag)) ?? Array.Empty())); + _logger.LogError(MsgHelp.GetString(MsgHelp.ID.VERIFY_RULES_DEPENDS_ON_TAG_MISSING), dependslessRule.Id, string.Join(',', dependslessRule.DependsOnTags?.Where(tag => !allTags.Contains(tag)) ?? Array.Empty())); foreach(var status in ruleStatuses.Where(x => x.Rule == dependslessRule)) { status.Errors = status.Errors.Append(MsgHelp.FormatString(MsgHelp.ID.VERIFY_RULES_DEPENDS_ON_TAG_MISSING, dependslessRule.Id, string.Join(',',dependslessRule.DependsOnTags?.Where(tag => !allTags.Contains(tag)) ?? Array.Empty()))); } } + + // Overrides are removed on a per file basis where depends_on is removed on a cross scan basis. Because of this, if you have RuleA with no DependsOnTags which is overriden with RuleB which does have tags, + // and then those tags are not present, you may expect to get RuleA but will not. + // This checks to ensure if a rule is overridden it has at least all the depends on tags of its overrider + var appInsStyleRules = ruleSet.GetAppInspectorRules(); + foreach (var rule in ruleSet.GetAppInspectorRules()) + { + foreach(var overrde in rule.Overrides ?? Array.Empty()) + { + foreach(var overriddenRule in appInsStyleRules.Where(x => x.Id == overrde)) + { + var missingTags = rule.DependsOnTags?.Where(x => !(overriddenRule.DependsOnTags?.Contains(x) ?? false)); + _logger.LogError(MsgHelp.GetString(MsgHelp.ID.VERIFY_RULES_OVERRIDDEN_RULE_DEPENDS_ON_TAG_MISSING), overriddenRule.Id, string.Join(',', missingTags ?? Array.Empty())); + foreach (var status in ruleStatuses.Where(x => x.Rule == overriddenRule)) + { + status.Errors = status.Errors.Append(MsgHelp.FormatString(MsgHelp.ID.VERIFY_RULES_OVERRIDDEN_RULE_DEPENDS_ON_TAG_MISSING, overriddenRule.Id, string.Join(',', missingTags ?? Array.Empty()))); + } + } + + } + } + return ruleStatuses; } diff --git a/AppInspector.Tests/Commands/TestVerifyRulesCmd.cs b/AppInspector.Tests/Commands/TestVerifyRulesCmd.cs index a9954b4c..78568306 100644 --- a/AppInspector.Tests/Commands/TestVerifyRulesCmd.cs +++ b/AppInspector.Tests/Commands/TestVerifyRulesCmd.cs @@ -164,6 +164,75 @@ public class TestVerifyRulesCmd } ]"; + // One of these rules overrides the other and depends on another tag but that tag isn't set on the rule being overridden + private readonly string _overriddenDependsOnTagMissingRule = @"[ + { + ""name"": ""Platform: Microsoft Windows"", + ""id"": ""_overriddenDependsOnTagMissingRule"", + ""description"": ""This rule checks for the string 'windows'"", + ""tags"": [ + ""Test.Tags.Windows"" + ], + ""applies_to"": [ ""csharp""], + ""severity"": ""Important"", + ""patterns"": [ + { + ""confidence"": ""Medium"", + ""modifiers"": [ + ""i"" + ], + ""pattern"": ""windows"", + ""type"": ""String"", + } + ], + ""must-not-match"" : [ ""linux""] + }, + { + ""name"": ""Platform: Microsoft Windows"", + ""id"": ""_overriddenDependsOnTagMissingRule_2"", + ""description"": ""This rule checks for the string 'windows'"", + ""tags"": [ + ""Test.Tags.Windows"" + ], + ""overrides"": [""_overriddenDependsOnTagMissingRule""], + ""depends_on_tags"": [""a_tag""], + ""applies_to"": [ ""csharp""], + ""severity"": ""Important"", + ""patterns"": [ + { + ""confidence"": ""Medium"", + ""modifiers"": [ + ""i"" + ], + ""pattern"": ""windows"", + ""type"": ""String"", + } + ], + ""must-not-match"" : [ ""linux""] + }, + { + ""name"": ""Platform: Microsoft Windows"", + ""id"": ""_overriddenDependsOnTagMissingRule_3"", + ""description"": ""This rule checks for the string 'windows'"", + ""tags"": [ + ""a_tag"" + ], + ""applies_to"": [ ""csharp""], + ""severity"": ""Important"", + ""patterns"": [ + { + ""confidence"": ""Medium"", + ""modifiers"": [ + ""i"" + ], + ""pattern"": ""windows"", + ""type"": ""String"", + } + ], + ""must-not-match"" : [ ""linux""] + } +]"; + // MustNotMatch if specified must not be matched private readonly string _mustNotMatchRule = @"[ { @@ -411,6 +480,22 @@ public void DuplicateId() File.Delete(path); } + [TestMethod] + public void OverriddenRuleMissingDependsOnTag() + { + var path = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + File.WriteAllText(path, _overriddenDependsOnTagMissingRule); + VerifyRulesOptions options = new() + { + CustomRulesPath = path + }; + + VerifyRulesCommand command = new(options, _factory); + var result = command.GetResult(); + Assert.AreEqual(VerifyRulesResult.ExitCode.NotVerified, result.ResultCode); + File.Delete(path); + } + [TestMethod] public void MissingDependsOnTag() { From 52084fe76b1ebeac5ca55cbc896e76e8266683b1 Mon Sep 17 00:00:00 2001 From: Gabe Stocco <98900+gfs@users.noreply.github.com> Date: Fri, 3 Mar 2023 16:50:59 -0800 Subject: [PATCH 10/13] Adds Support for Chained Dependent Rules And tests for same. --- AppInspector.RulesEngine/RulesVerifier.cs | 9 +- AppInspector.Tests/Commands/TestAnalyzeCmd.cs | 164 +++++++++++++++++- AppInspector/Commands/AnalyzeCommand.cs | 39 +++-- 3 files changed, 191 insertions(+), 21 deletions(-) diff --git a/AppInspector.RulesEngine/RulesVerifier.cs b/AppInspector.RulesEngine/RulesVerifier.cs index 622338c5..9c361644 100644 --- a/AppInspector.RulesEngine/RulesVerifier.cs +++ b/AppInspector.RulesEngine/RulesVerifier.cs @@ -121,10 +121,13 @@ public IList CheckIntegrity(AbstractRuleSet ruleSet) foreach(var overriddenRule in appInsStyleRules.Where(x => x.Id == overrde)) { var missingTags = rule.DependsOnTags?.Where(x => !(overriddenRule.DependsOnTags?.Contains(x) ?? false)); - _logger.LogError(MsgHelp.GetString(MsgHelp.ID.VERIFY_RULES_OVERRIDDEN_RULE_DEPENDS_ON_TAG_MISSING), overriddenRule.Id, string.Join(',', missingTags ?? Array.Empty())); - foreach (var status in ruleStatuses.Where(x => x.Rule == overriddenRule)) + if (missingTags?.Any() ?? false) { - status.Errors = status.Errors.Append(MsgHelp.FormatString(MsgHelp.ID.VERIFY_RULES_OVERRIDDEN_RULE_DEPENDS_ON_TAG_MISSING, overriddenRule.Id, string.Join(',', missingTags ?? Array.Empty()))); + _logger.LogError(MsgHelp.GetString(MsgHelp.ID.VERIFY_RULES_OVERRIDDEN_RULE_DEPENDS_ON_TAG_MISSING), overriddenRule.Id, string.Join(',', missingTags ?? Array.Empty())); + foreach (var status in ruleStatuses.Where(x => x.Rule == overriddenRule)) + { + status.Errors = status.Errors.Append(MsgHelp.FormatString(MsgHelp.ID.VERIFY_RULES_OVERRIDDEN_RULE_DEPENDS_ON_TAG_MISSING, overriddenRule.Id, string.Join(',', missingTags ?? Array.Empty()))); + } } } diff --git a/AppInspector.Tests/Commands/TestAnalyzeCmd.cs b/AppInspector.Tests/Commands/TestAnalyzeCmd.cs index 737c50c4..67edf64d 100644 --- a/AppInspector.Tests/Commands/TestAnalyzeCmd.cs +++ b/AppInspector.Tests/Commands/TestAnalyzeCmd.cs @@ -25,6 +25,79 @@ public class TestAnalyzeCmd private const int numTimeOutFiles = 25; private const int numTimesContent = 25; + private const string dependsOnChain = @"[ + { + ""id"": ""SA000001"", + ""name"": ""Testing.Rules.DependsOnTags.Chain.A"", + ""tags"": [ + ""Category.A"" + ], + ""severity"": ""Critical"", + ""description"": ""This rule finds A"", + ""patterns"": [ + { + ""pattern"": ""A"", + ""type"": ""regex"", + ""confidence"": ""High"", + ""modifiers"": [ + ""m"" + ], + ""scopes"": [ + ""code"" + ] + } + ], + ""_comment"": """" + }, + { + ""id"": ""SA000002"", + ""name"": ""Testing.Rules.DependsOnTags.Chain.B"", + ""tags"": [ + ""Category.B"" + ], + ""depends_on_tags"": [""Category.A""], + ""severity"": ""Critical"", + ""description"": ""This rule finds B"", + ""patterns"": [ + { + ""pattern"": ""B"", + ""type"": ""regex"", + ""confidence"": ""High"", + ""modifiers"": [ + ""m"" + ], + ""scopes"": [ + ""code"" + ] + } + ], + ""_comment"": """" + }, + { + ""id"": ""SA000003"", + ""name"": ""Testing.Rules.DependsOnTags.Chain.C"", + ""tags"": [ + ""Category.C"" + ], + ""depends_on_tags"": [""Category.B""], + ""severity"": ""Critical"", + ""description"": ""This rule finds B"", + ""patterns"": [ + { + ""pattern"": ""C"", + ""type"": ""regex"", + ""confidence"": ""High"", + ""modifiers"": [ + ""m"" + ], + ""scopes"": [ + ""code"" + ] + } + ], + ""_comment"": """" + } +]"; private const string dependsOnOneWay = @"[ { ""id"": ""SA000005"", @@ -384,12 +457,17 @@ public class TestAnalyzeCmd windows 2000 windows "; + /// Used for the depends on chain tests + private const string justA = "A"; + private const string justB = "B"; + private const string justC = "C"; private static string testFilePath = string.Empty; private static string testRulesPath = string.Empty; private static string appliesToTestRulePath = string.Empty; private static string doesNotApplyToTestRulePath = string.Empty; private static string dependsOnOneWayRulePath; + private static string dependsOnChainRulePath; private static string dependsOnTwoWayRulePath; // Test files for timeout tests @@ -399,6 +477,9 @@ windows 2000 private ILoggerFactory factory = new NullLoggerFactory(); private static string fourWindowsOne2000Path; + private static string justAPath; + private static string justBPath; + private static string justCPath; [ClassInitialize] public static void ClassInit(TestContext context) @@ -413,17 +494,29 @@ public static void ClassInit(TestContext context) "DoesNotApplyToTestRules.json"); dependsOnOneWayRulePath = Path.Combine(TestHelpers.GetPath(TestHelpers.AppPath.testOutput), "DependsOnOneWay.json"); + dependsOnChainRulePath = Path.Combine(TestHelpers.GetPath(TestHelpers.AppPath.testOutput), + "DependsOnChain.json"); dependsOnTwoWayRulePath = Path.Combine(TestHelpers.GetPath(TestHelpers.AppPath.testOutput), "DependsOnTwoWay.json"); + fourWindowsOne2000Path = + Path.Combine(TestHelpers.GetPath(TestHelpers.AppPath.testOutput), "FourWindowsOne2000.cs"); + justAPath = + Path.Combine(TestHelpers.GetPath(TestHelpers.AppPath.testOutput), "justA.cs"); + justBPath = + Path.Combine(TestHelpers.GetPath(TestHelpers.AppPath.testOutput), "justB.cs"); + justCPath = + Path.Combine(TestHelpers.GetPath(TestHelpers.AppPath.testOutput), "justC.cs"); File.WriteAllText(heavyRulePath, heavyRule); File.WriteAllText(testFilePath, fourWindowsOneLinux); File.WriteAllText(testRulesPath, findWindows); File.WriteAllText(appliesToTestRulePath, findWindowsWithAppliesTo); File.WriteAllText(doesNotApplyToTestRulePath, findWindowsWithDoesNotApplyTo); File.WriteAllText(dependsOnOneWayRulePath, dependsOnOneWay); + File.WriteAllText(dependsOnChainRulePath, dependsOnChain); File.WriteAllText(dependsOnTwoWayRulePath, dependsOnTwoWay); - fourWindowsOne2000Path = - Path.Combine(TestHelpers.GetPath(TestHelpers.AppPath.testOutput), "FourWindowsOne2000.cs"); + File.WriteAllText(justAPath, justA); + File.WriteAllText(justBPath, justB); + File.WriteAllText(justCPath, justC); File.WriteAllText(fourWindowsOne2000Path, threeWindowsOneWindows2000); for (var i = 0; i < numTimeOutFiles; i++) @@ -1155,6 +1248,73 @@ public void TestDependsOnOneWay() Assert.IsTrue(result.Metadata.UniqueTags.Contains("Dependant")); } + /// + /// Test that the depends_on rule parameter properly limits matches one way + /// + [TestMethod] + public void TestDependsOnChain() + { + AnalyzeOptions options = new() + { + SourcePath = new string[] { justAPath, justBPath, justCPath }, + CustomRulesPath = dependsOnChainRulePath, + IgnoreDefaultRules = true + }; + + AnalyzeCommand command = new(options, factory); + var result = command.GetResult(); + Assert.AreEqual(AnalyzeResult.ExitCode.Success, result.ResultCode); + Assert.AreEqual(3, result.Metadata.TotalMatchesCount); + Assert.IsTrue(result.Metadata.UniqueTags.Contains("Category.A")); + Assert.IsTrue(result.Metadata.UniqueTags.Contains("Category.B")); + Assert.IsTrue(result.Metadata.UniqueTags.Contains("Category.C")); + + options = new() + { + SourcePath = new string[] { justBPath, justCPath }, + CustomRulesPath = dependsOnChainRulePath, + IgnoreDefaultRules = true + }; + + command = new(options, factory); + result = command.GetResult(); + Assert.AreEqual(AnalyzeResult.ExitCode.NoMatches, result.ResultCode); + Assert.AreEqual(0, result.Metadata.TotalMatchesCount); + Assert.IsFalse(result.Metadata.UniqueTags.Contains("Category.A")); + Assert.IsFalse(result.Metadata.UniqueTags.Contains("Category.B")); + Assert.IsFalse(result.Metadata.UniqueTags.Contains("Category.C")); + + options = new() + { + SourcePath = new string[] { justAPath, justCPath }, + CustomRulesPath = dependsOnChainRulePath, + IgnoreDefaultRules = true + }; + + command = new(options, factory); + result = command.GetResult(); + Assert.AreEqual(AnalyzeResult.ExitCode.Success, result.ResultCode); + Assert.AreEqual(1, result.Metadata.TotalMatchesCount); + Assert.IsTrue(result.Metadata.UniqueTags.Contains("Category.A")); + Assert.IsFalse(result.Metadata.UniqueTags.Contains("Category.B")); + Assert.IsFalse(result.Metadata.UniqueTags.Contains("Category.C")); + + options = new() + { + SourcePath = new string[] { justAPath, justBPath }, + CustomRulesPath = dependsOnChainRulePath, + IgnoreDefaultRules = true + }; + + command = new(options, factory); + result = command.GetResult(); + Assert.AreEqual(AnalyzeResult.ExitCode.Success, result.ResultCode); + Assert.AreEqual(2, result.Metadata.TotalMatchesCount); + Assert.IsTrue(result.Metadata.UniqueTags.Contains("Category.A")); + Assert.IsTrue(result.Metadata.UniqueTags.Contains("Category.B")); + Assert.IsFalse(result.Metadata.UniqueTags.Contains("Category.C")); + } + /// /// Test that the depends_on rule parameter properly limits matches one way /// diff --git a/AppInspector/Commands/AnalyzeCommand.cs b/AppInspector/Commands/AnalyzeCommand.cs index fd4c06a7..41d4d8d2 100644 --- a/AppInspector/Commands/AnalyzeCommand.cs +++ b/AppInspector/Commands/AnalyzeCommand.cs @@ -380,15 +380,9 @@ private AnalyzeResult.ExitCode PopulateRecords(CancellationToken cancellationTok } } - // Remove match records that don't have the matching depends on tags - List filteredMatchRecords = _metaDataHelper.Matches.Where(x => x.Rule?.DependsOnTags?.All(tag => _metaDataHelper.UniqueTags.ContainsKey(tag)) ?? true).ToList(); - if (filteredMatchRecords.Count != _metaDataHelper.Matches.Count) + if (!_options.TagsOnly) { - _metaDataHelper = _metaDataHelper.CreateFresh(); - foreach (MatchRecord matchRecord in filteredMatchRecords) - { - _metaDataHelper.AddMatchRecord(matchRecord); - } + RemoveDependsOnNotPresent(); } return AnalyzeResult.ExitCode.Success; @@ -535,6 +529,25 @@ void ProcessLambda() } } + // Remove matches from the metadata when the DependsOnTags are not satisfied. + private void RemoveDependsOnNotPresent() + { + // Remove match records that don't have the matching depends on tags + HashSet tags = _metaDataHelper.UniqueTags.Select(x => x.Key).ToHashSet(); + List originalMatches = _metaDataHelper.Matches.ToList(); + List filteredMatchRecords = originalMatches.Where(x => x.Rule?.DependsOnTags?.All(tag => tags.Contains(tag)) ?? true).ToList(); + while (filteredMatchRecords.Count != originalMatches.Count) + { + tags = filteredMatchRecords.SelectMany(x => x.Tags).Distinct().ToHashSet(); + (filteredMatchRecords, originalMatches) = (filteredMatchRecords.Where(x => x.Rule?.DependsOnTags?.All(tag => tags.Contains(tag)) ?? true).ToList(), filteredMatchRecords); + } + _metaDataHelper = _metaDataHelper.CreateFresh(); + foreach (MatchRecord matchRecord in filteredMatchRecords) + { + _metaDataHelper.AddMatchRecord(matchRecord); + } + } + /// /// Populate the records in the metadata asynchronously. /// @@ -559,15 +572,9 @@ void ProcessLambda() await ProcessAndAddToMetadata(entry, cancellationToken); } - // Remove match records that don't have the matching depends on tags - List filteredMatchRecords = _metaDataHelper.Matches.Where(x => x.Rule?.DependsOnTags?.All(tag => _metaDataHelper.UniqueTags.ContainsKey(tag)) ?? true).ToList(); - if (filteredMatchRecords.Count != _metaDataHelper.Matches.Count) + if (!_options.TagsOnly) { - _metaDataHelper = _metaDataHelper.CreateFresh(); - foreach (MatchRecord matchRecord in filteredMatchRecords) - { - _metaDataHelper.AddMatchRecord(matchRecord); - } + RemoveDependsOnNotPresent(); } return AnalyzeResult.ExitCode.Success; From a22a1dced9383f203e5497323e43acc6128892f1 Mon Sep 17 00:00:00 2001 From: Gabe Stocco <98900+gfs@users.noreply.github.com> Date: Fri, 3 Mar 2023 16:56:06 -0800 Subject: [PATCH 11/13] Typo in test rule description --- AppInspector.Tests/Commands/TestAnalyzeCmd.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AppInspector.Tests/Commands/TestAnalyzeCmd.cs b/AppInspector.Tests/Commands/TestAnalyzeCmd.cs index 67edf64d..0aacd1f6 100644 --- a/AppInspector.Tests/Commands/TestAnalyzeCmd.cs +++ b/AppInspector.Tests/Commands/TestAnalyzeCmd.cs @@ -81,7 +81,7 @@ public class TestAnalyzeCmd ], ""depends_on_tags"": [""Category.B""], ""severity"": ""Critical"", - ""description"": ""This rule finds B"", + ""description"": ""This rule finds C"", ""patterns"": [ { ""pattern"": ""C"", From 2b9301d05c603062c1c891b64d1b8e900032a469 Mon Sep 17 00:00:00 2001 From: Gabe Stocco <98900+gfs@users.noreply.github.com> Date: Fri, 3 Mar 2023 17:00:58 -0800 Subject: [PATCH 12/13] Small refactor to deduplicate logic --- AppInspector/Commands/AnalyzeCommand.cs | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/AppInspector/Commands/AnalyzeCommand.cs b/AppInspector/Commands/AnalyzeCommand.cs index 41d4d8d2..bbabce6f 100644 --- a/AppInspector/Commands/AnalyzeCommand.cs +++ b/AppInspector/Commands/AnalyzeCommand.cs @@ -529,17 +529,17 @@ void ProcessLambda() } } - // Remove matches from the metadata when the DependsOnTags are not satisfied. + /// + /// Remove matches from the metadata when the DependsOnTags are not satisfied. + /// private void RemoveDependsOnNotPresent() { - // Remove match records that don't have the matching depends on tags - HashSet tags = _metaDataHelper.UniqueTags.Select(x => x.Key).ToHashSet(); List originalMatches = _metaDataHelper.Matches.ToList(); - List filteredMatchRecords = originalMatches.Where(x => x.Rule?.DependsOnTags?.All(tag => tags.Contains(tag)) ?? true).ToList(); + List filteredMatchRecords = FilterRecordsByMissingDependsOnTags(originalMatches); + // Continue iterating as long as records were removed in the last iteration, as their tags may have been depended on by another rule while (filteredMatchRecords.Count != originalMatches.Count) { - tags = filteredMatchRecords.SelectMany(x => x.Tags).Distinct().ToHashSet(); - (filteredMatchRecords, originalMatches) = (filteredMatchRecords.Where(x => x.Rule?.DependsOnTags?.All(tag => tags.Contains(tag)) ?? true).ToList(), filteredMatchRecords); + (filteredMatchRecords, originalMatches) = (FilterRecordsByMissingDependsOnTags(filteredMatchRecords), filteredMatchRecords); } _metaDataHelper = _metaDataHelper.CreateFresh(); foreach (MatchRecord matchRecord in filteredMatchRecords) @@ -548,6 +548,18 @@ private void RemoveDependsOnNotPresent() } } + /// + /// Return a new List of MatchRecords with records removed which depend on tags not present in the set of records. + /// + /// + /// + private List FilterRecordsByMissingDependsOnTags(List listToFilter) + { + HashSet tags = listToFilter.SelectMany(x => x.Tags).Distinct().ToHashSet(); + return listToFilter.Where(x => x.Rule?.DependsOnTags?.All(tag => tags.Contains(tag)) ?? true).ToList(); + + } + /// /// Populate the records in the metadata asynchronously. /// From a9d89f074232c17195802cb2150adf3ea0d7e967 Mon Sep 17 00:00:00 2001 From: Gabe Stocco <98900+gfs@users.noreply.github.com> Date: Fri, 3 Mar 2023 17:03:19 -0800 Subject: [PATCH 13/13] Improve variable names --- AppInspector/Commands/AnalyzeCommand.cs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/AppInspector/Commands/AnalyzeCommand.cs b/AppInspector/Commands/AnalyzeCommand.cs index bbabce6f..969631cf 100644 --- a/AppInspector/Commands/AnalyzeCommand.cs +++ b/AppInspector/Commands/AnalyzeCommand.cs @@ -534,15 +534,15 @@ void ProcessLambda() /// private void RemoveDependsOnNotPresent() { - List originalMatches = _metaDataHelper.Matches.ToList(); - List filteredMatchRecords = FilterRecordsByMissingDependsOnTags(originalMatches); + List previousMatches = _metaDataHelper.Matches.ToList(); + List nextMatches = FilterRecordsByMissingDependsOnTags(previousMatches); // Continue iterating as long as records were removed in the last iteration, as their tags may have been depended on by another rule - while (filteredMatchRecords.Count != originalMatches.Count) + while (nextMatches.Count != previousMatches.Count) { - (filteredMatchRecords, originalMatches) = (FilterRecordsByMissingDependsOnTags(filteredMatchRecords), filteredMatchRecords); + (nextMatches, previousMatches) = (FilterRecordsByMissingDependsOnTags(nextMatches), nextMatches); } _metaDataHelper = _metaDataHelper.CreateFresh(); - foreach (MatchRecord matchRecord in filteredMatchRecords) + foreach (MatchRecord matchRecord in nextMatches) { _metaDataHelper.AddMatchRecord(matchRecord); } @@ -550,6 +550,7 @@ private void RemoveDependsOnNotPresent() /// /// Return a new List of MatchRecords with records removed which depend on tags not present in the set of records. + /// Does not modify the original list. /// /// ///