From dc4b0ef1707abc81bb96426032031a7c093414a7 Mon Sep 17 00:00:00 2001 From: Giulia Stocco <98900+gfs@users.noreply.github.com> Date: Sat, 1 Aug 2026 11:47:06 -0700 Subject: [PATCH] Fix build-file rule tag filtering Allow explicitly targeted rules to report build-file findings while preserving suppression for universal rules. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f0f9dbe0-e8ba-472b-b645-eda10038e683 --- AppInspector.CLI/CLICmdOptions.cs | 2 +- AppInspector.RulesEngine/AbstractRuleSet.cs | 4 +- AppInspector.RulesEngine/Rule.cs | 8 ++ AppInspector.RulesEngine/RuleProcessor.cs | 9 +- .../RuleProcessor/BuildFileRuleTests.cs | 90 +++++++++++++++++++ .../RuleProcessor/XmlAndJsonTests.cs | 6 +- AppInspector/Commands/AnalyzeCommand.cs | 4 +- 7 files changed, 111 insertions(+), 12 deletions(-) create mode 100644 AppInspector.Tests/RuleProcessor/BuildFileRuleTests.cs diff --git a/AppInspector.CLI/CLICmdOptions.cs b/AppInspector.CLI/CLICmdOptions.cs index 6d493d3e..15deb227 100644 --- a/AppInspector.CLI/CLICmdOptions.cs +++ b/AppInspector.CLI/CLICmdOptions.cs @@ -154,7 +154,7 @@ public record CLIAnalyzeCmdOptions : CLIAnalysisSharedCommandOptions public bool NoFileMetadata { get; set; } [Option('A', "allow-all-tags-in-build-files", Required = false, - HelpText = "Allow all tags (not just Metadata tags) in files of type Build.")] + HelpText = "Allow non-Metadata tags from universal rules in Build files. Rules declaring applies_to or applies_to_file_regex are always eligible.")] public bool AllowAllTagsInBuildFiles { get; set; } [Option('M', "max-num-matches-per-tag", Required = false, diff --git a/AppInspector.RulesEngine/AbstractRuleSet.cs b/AppInspector.RulesEngine/AbstractRuleSet.cs index 970aeeec..f6858153 100644 --- a/AppInspector.RulesEngine/AbstractRuleSet.cs +++ b/AppInspector.RulesEngine/AbstractRuleSet.cs @@ -63,9 +63,7 @@ public IEnumerable ByFilename(string input) /// public IEnumerable GetUniversalRules() { - return _oatRules.Where(x => - (x.AppInspectorRule.FileRegexes is null || x.AppInspectorRule.FileRegexes.Count == 0) && - (x.AppInspectorRule.AppliesTo is null || x.AppInspectorRule.AppliesTo.Count == 0)); + return _oatRules.Where(x => x.AppInspectorRule.IsUniversal); } /// diff --git a/AppInspector.RulesEngine/Rule.cs b/AppInspector.RulesEngine/Rule.cs index 1c45d940..23725e67 100644 --- a/AppInspector.RulesEngine/Rule.cs +++ b/AppInspector.RulesEngine/Rule.cs @@ -96,6 +96,14 @@ public IList? FileRegexes _updateCompiledFileRegex = true; } } + + /// + /// Gets whether the rule applies universally instead of declaring a target language or file name. + /// + [JsonIgnore] + public bool IsUniversal => + (FileRegexes is null || FileRegexes.Count == 0) && + (AppliesTo is null || AppliesTo.Count == 0); /// /// Internal API to cache construction of diff --git a/AppInspector.RulesEngine/RuleProcessor.cs b/AppInspector.RulesEngine/RuleProcessor.cs index f3ec8201..fbdf81fd 100644 --- a/AppInspector.RulesEngine/RuleProcessor.cs +++ b/AppInspector.RulesEngine/RuleProcessor.cs @@ -157,8 +157,10 @@ public List AnalyzeFile(TextContainer textContainer, FileEntry file { var patternIndex = match.Item1; var boundary = match.Item2; - //restrict adds from build files to tags with "metadata" only to avoid false feature positives that are not part of executable code - if (!_opts.AllowAllTagsInBuildFiles && languageInfo.Type == LanguageInfo.LangFileType.Build && + // Universal rules can reach build files incidentally, so suppress their non-Metadata tags by default. + if (!_opts.AllowAllTagsInBuildFiles && + languageInfo.Type == LanguageInfo.LangFileType.Build && + oatRule.AppInspectorRule.IsUniversal && (oatRule.AppInspectorRule.Tags?.Any(v => !v.Contains("Metadata")) ?? false)) { continue; @@ -366,9 +368,10 @@ List ProcessBoundary(ClauseCapture cap) var patternIndex = match.Item1; var boundary = match.Item2; - //restrict adds from build files to tags with "metadata" only to avoid false feature positives that are not part of executable code + // Universal rules can reach build files incidentally, so suppress their non-Metadata tags by default. if (!_opts.AllowAllTagsInBuildFiles && languageInfo.Type == LanguageInfo.LangFileType.Build && + oatRule.AppInspectorRule.IsUniversal && (oatRule.AppInspectorRule.Tags?.Any(v => !v.Contains("Metadata")) ?? false)) { continue; diff --git a/AppInspector.Tests/RuleProcessor/BuildFileRuleTests.cs b/AppInspector.Tests/RuleProcessor/BuildFileRuleTests.cs new file mode 100644 index 00000000..d284ba16 --- /dev/null +++ b/AppInspector.Tests/RuleProcessor/BuildFileRuleTests.cs @@ -0,0 +1,90 @@ +using System.Collections.Generic; +using System.IO; +using System.Text; +using System.Threading.Tasks; +using Microsoft.ApplicationInspector.RulesEngine; +using Microsoft.CST.RecursiveExtractor; +using Xunit; + +namespace AppInspector.Tests.RuleProcessor; + +public class BuildFileRuleTests +{ + private const string BuildFileContents = "{\"value\":\"build-marker\"}"; + private const string BuildFileName = "test.json"; + private const string FeatureTag = "Testing.Build.Feature"; + private const string Marker = "build-marker"; + private readonly Microsoft.ApplicationInspector.RulesEngine.Languages _languages = new(); + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ExplicitBuildLanguageRuleEmitsFeatureTagByDefault(bool analyzeAsync) + { + var languageInfo = GetBuildLanguage(); + var rule = CreateRule("BUILD000001", new[] { languageInfo.Name }); + + var matches = await AnalyzeAsync(rule, languageInfo, false, analyzeAsync); + + var match = Assert.Single(matches); + Assert.Equal(FeatureTag, Assert.Single(match.Rule!.Tags!)); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task UniversalBuildRuleEmitsFeatureTagOnlyWhenAllowed(bool analyzeAsync) + { + var languageInfo = GetBuildLanguage(); + var rule = CreateRule("BUILD000002"); + + var defaultMatches = await AnalyzeAsync(rule, languageInfo, false, analyzeAsync); + var allowedMatches = await AnalyzeAsync(rule, languageInfo, true, analyzeAsync); + + Assert.Empty(defaultMatches); + var match = Assert.Single(allowedMatches); + Assert.Equal(FeatureTag, Assert.Single(match.Rule!.Tags!)); + } + + private static async Task> AnalyzeAsync(Rule rule, LanguageInfo languageInfo, + bool allowAllTagsInBuildFiles, bool analyzeAsync) + { + RuleSet rules = new(); + rules.AddRule(rule); + Microsoft.ApplicationInspector.RulesEngine.RuleProcessor processor = new(rules, + new RuleProcessorOptions { AllowAllTagsInBuildFiles = allowAllTagsInBuildFiles }); + using MemoryStream stream = new(Encoding.UTF8.GetBytes(BuildFileContents)); + FileEntry fileEntry = new(BuildFileName, stream); + + return analyzeAsync + ? await processor.AnalyzeFileAsync(fileEntry, languageInfo) + : processor.AnalyzeFile(BuildFileContents, fileEntry, languageInfo); + } + + private static Rule CreateRule(string id, string[]? appliesTo = null) + { + return new Rule + { + Id = id, + Name = "Build file filtering test", + AppliesTo = appliesTo, + Tags = new[] { FeatureTag }, + Patterns = new[] + { + new SearchPattern + { + Pattern = Marker, + PatternType = PatternType.Substring, + Confidence = Confidence.High + } + } + }; + } + + private LanguageInfo GetBuildLanguage() + { + Assert.True(_languages.FromFileNameOut(BuildFileName, out var languageInfo)); + Assert.Equal(LanguageInfo.LangFileType.Build, languageInfo.Type); + return languageInfo; + } +} diff --git a/AppInspector.Tests/RuleProcessor/XmlAndJsonTests.cs b/AppInspector.Tests/RuleProcessor/XmlAndJsonTests.cs index 15e3a566..268d00dd 100644 --- a/AppInspector.Tests/RuleProcessor/XmlAndJsonTests.cs +++ b/AppInspector.Tests/RuleProcessor/XmlAndJsonTests.cs @@ -62,7 +62,7 @@ public void XPathVersionElementSampleBoundary() RuleSet rules = new(); rules.AddString(rule, "TestRules"); Microsoft.ApplicationInspector.RulesEngine.RuleProcessor processor = new(rules, - new RuleProcessorOptions { AllowAllTagsInBuildFiles = true }); + new RuleProcessorOptions()); if (_languages.FromFileNameOut("pom.xml", out var info)) { @@ -347,7 +347,7 @@ public void XmlWithNamespaces() //var verification= verifier.Verify(rules); //Assert.Equal(true,verification.Verified); Microsoft.ApplicationInspector.RulesEngine.RuleProcessor processor = new(rules, - new RuleProcessorOptions { AllowAllTagsInBuildFiles = true }); + new RuleProcessorOptions()); if (_languages.FromFileNameOut("AndroidManifest.xml", out var info)) { var matches = processor.AnalyzeFile(@"", new FileEntry("AndroidManifest.xml", new MemoryStream()), info); @@ -390,7 +390,7 @@ public void XmlAttributeTest() RuleSet rules = new(); rules.AddString(attributeRule, "JsonTestRules"); Microsoft.ApplicationInspector.RulesEngine.RuleProcessor processor = new(rules, - new RuleProcessorOptions { AllowAllTagsInBuildFiles = true }); + new RuleProcessorOptions()); if (_languages.FromFileNameOut("test.config", out var info)) { var matches = processor.AnalyzeFile(attributeContent, new FileEntry("test.config", new MemoryStream()), info); diff --git a/AppInspector/Commands/AnalyzeCommand.cs b/AppInspector/Commands/AnalyzeCommand.cs index 095e4dbc..8c58217d 100644 --- a/AppInspector/Commands/AnalyzeCommand.cs +++ b/AppInspector/Commands/AnalyzeCommand.cs @@ -61,8 +61,8 @@ public class AnalyzeOptions public bool SingleThread { get; set; } /// - /// Treat files as if they were - /// when determining if tags should apply. + /// Allow universal rules to emit non-Metadata tags in files. + /// Rules declaring applies_to or applies_to_file_regex are always eligible. /// public bool AllowAllTagsInBuildFiles { get; set; }