Skip to content

Commit

Permalink
Fix #480 (#481)
Browse files Browse the repository at this point in the history 
* Fix #480

Fixes case sensitivity of Enum based arguments
Fixes default Confidence argument not being respected
Fixes severity not being respected
Fixes custom rules not being validated if provided as a directory
Adds new option to skip validation of custom rules
Adds a TextContainer based API for analyze.

* Simplify post validation action
  • Loading branch information
@gfs
gfs authored Jul 28, 2022
1 parent 6c40a50 commit eb47b55
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 25 deletions.
11 changes: 8 additions & 3 deletions AppInspector.CLI/CLICmdOptions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ public record CLICustomRulesCommandOptions: CLICommandOptions

public record CLIAnalysisSharedCommandOptions : CLICustomRulesCommandOptions
{
[Option("disable-custom-rule-validation", Required = false, HelpText = "By default when providing custom rules they are validated. When set, validation will be skipped.")]
public bool DisableCustomRuleValidation { get; set; } = false;

[Option('i', "ignore-default-rules", Required = false, HelpText = "Exclude default rules bundled with application", Default = false)]
public bool IgnoreDefaultRules { get; set; }

Expand All @@ -62,11 +65,13 @@ public record CLIAnalysisSharedCommandOptions : CLICustomRulesCommandOptions
[Option('u', "scan-unknown-filetypes", Required = false, HelpText = "Scan files of unknown types.")]
public bool ScanUnknownTypes { get; set; }

[Option('c', "confidence-filters", Required = false, Separator = ',', HelpText = "Output only matches with specified confidence <value>,<value>. Default: Medium,High. [High|Medium|Low]")]
[Option('c', "confidence-filters", Required = false, Separator = ',', HelpText = "Output only matches with specified confidence <value>,<value>. Default: Medium,High. [High|Medium|Low]", Default = new Confidence[]{Confidence.High, Confidence.Medium})]
public IEnumerable<Confidence> ConfidenceFilters { get; set; } = new Confidence[] { Confidence.High, Confidence.Medium };

[Option("severity-filters", Required = false, Separator = ',', HelpText = "Output only matches with specified severity <value>,<value>. Default: All are enabled. [Critical|Important|Moderate|BestPractice|ManualReview]")]
public IEnumerable<Severity> SeverityFilters { get; set; } = (IEnumerable<Severity>)Enum.GetValues(typeof(Severity));
[Option("severity-filters", Required = false, Separator = ',',
HelpText =
"Output only matches with specified severity <value>,<value>. Default: All are enabled. [Critical|Important|Moderate|BestPractice|ManualReview]", Default = new Severity[] { Severity.Critical, Severity.Important, Severity.Moderate, Severity.BestPractice, Severity.ManualReview })]
public IEnumerable<Severity> SeverityFilters { get; set; } = new Severity[] { Severity.Critical, Severity.Important, Severity.Moderate, Severity.BestPractice, Severity.ManualReview };
}

/// <summary>
Expand Down
11 changes: 7 additions & 4 deletions AppInspector.CLI/Program.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ namespace Microsoft.ApplicationInspector.CLI
{
using CommandLine;
using Commands;
using Microsoft.ApplicationInspector.Common;
using Microsoft.Extensions.Logging;
using ShellProgressBar;
using System;
using System.IO;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.ApplicationInspector.Common;
using Microsoft.Extensions.Logging;
using ILogger = Extensions.Logging.ILogger;

public static class Program
Expand All @@ -33,7 +33,7 @@ public static int Main(string[] args)
Exception? exception = null;
try
{
var argsResult = Parser.Default.ParseArguments<CLIAnalyzeCmdOptions,
var argsResult = new Parser(settings => settings.CaseInsensitiveEnumValues = true).ParseArguments<CLIAnalyzeCmdOptions,
CLITagDiffCmdOptions,
CLIExportTagsCmdOptions,
CLIVerifyRulesCmdOptions,
Expand Down Expand Up @@ -277,6 +277,7 @@ private static int RunAnalyzeCommand(CLIAnalyzeCmdOptions cliOptions)
CustomLanguagesPath = cliOptions.CustomLanguagesPath,
IgnoreDefaultRules = cliOptions.IgnoreDefaultRules,
ConfidenceFilters = cliOptions.ConfidenceFilters,
SeverityFilters = cliOptions.SeverityFilters,
FilePathExclusions = cliOptions.FilePathExclusions,
SingleThread = cliOptions.SingleThread,
NoShowProgress = cliOptions.NoShowProgressBar,
Expand All @@ -289,7 +290,8 @@ private static int RunAnalyzeCommand(CLIAnalyzeCmdOptions cliOptions)
AllowAllTagsInBuildFiles = cliOptions.AllowAllTagsInBuildFiles,
MaxNumMatchesPerTag = cliOptions.MaxNumMatchesPerTag,
DisableCrawlArchives = cliOptions.DisableArchiveCrawling,
EnumeratingTimeout = cliOptions.EnumeratingTimeout
EnumeratingTimeout = cliOptions.EnumeratingTimeout,
DisableCustomRuleVerification = cliOptions.DisableCustomRuleValidation,
}, adjustedFactory);

AnalyzeResult analyzeResult = command.GetResult();
Expand Down Expand Up @@ -355,6 +357,7 @@ private static int RunTagDiffCommand(CLITagDiffCmdOptions cliOptions)
ProcessingTimeOut = cliOptions.ProcessingTimeOut,
ScanUnknownTypes = cliOptions.ScanUnknownTypes,
SingleThread = cliOptions.SingleThread,
DisableCustomRuleValidation = cliOptions.DisableCustomRuleValidation,
}, loggerFactory);

TagDiffResult tagDiffResult = command.GetResult();
Expand Down
43 changes: 41 additions & 2 deletions AppInspector.RulesEngine/RuleProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,21 @@ private static string ExtractDependency(TextContainer? text, int startIndex, str
return rawResult;
}

public List<MatchRecord> AnalyzeFile(string contents, FileEntry fileEntry, LanguageInfo languageInfo, IEnumerable<string>? tagsToIgnore = null, int numLinesContext = 3)
/// <summary>
/// Analyzes a file and returns a list of <see cref="MatchRecord"/>
/// </summary>
/// <param name="textContainer">TextContainer which holds the text to analyze</param>
/// <param name="fileEntry">FileEntry which has the name of the file being analyzed.</param>
/// <param name="languageInfo">The LanguageInfo for the file</param>
/// <param name="tagsToIgnore">Ignore rules that match tags that are only in the tags to ignore list</param>
/// <param name="numLinesContext">Number of lines of text to extract for the sample. Set to 0 to disable context gathering. Set to -1 to also disable sampling the match.</param>
/// <returns>A List of the matches against the Rules the processor is configured with.</returns>
public List<MatchRecord> AnalyzeFile(TextContainer textContainer, FileEntry fileEntry,
LanguageInfo languageInfo, IEnumerable<string>? tagsToIgnore = null, int numLinesContext = 3)
{
var rules = GetRulesForFile(languageInfo, fileEntry, tagsToIgnore);
List<MatchRecord> resultsList = new();

TextContainer textContainer = new(contents, languageInfo.Name, _languages);
var caps = analyzer.GetCaptures(rules, textContainer);
foreach (var ruleCapture in caps)
{
Expand Down Expand Up @@ -220,6 +229,28 @@ List<MatchRecord> ProcessBoundary(ClauseCapture cap)
return resultsList;
}

/// <summary>
/// Analyzes a file and returns a list of <see cref="MatchRecord"/>
/// </summary>
/// <param name="contents">A string containing the text to analyze</param>
/// <param name="fileEntry">FileEntry which has the name of the file being analyzed</param>
/// <param name="languageInfo">The LanguageInfo for the file</param>
/// <param name="tagsToIgnore">Ignore rules that match tags that are only in the tags to ignore list</param>
/// <param name="numLinesContext">Number of lines of text to extract for the sample. Set to 0 to disable context gathering. Set to -1 to also disable sampling the match.</param>
/// <returns>A List of the matches against the Rules the processor is configured with.</returns>
public List<MatchRecord> AnalyzeFile(string contents, FileEntry fileEntry, LanguageInfo languageInfo, IEnumerable<string>? tagsToIgnore = null, int numLinesContext = 3)
{
TextContainer textContainer = new(contents, languageInfo.Name, _languages);
return AnalyzeFile(textContainer, fileEntry, languageInfo, tagsToIgnore, numLinesContext);
}

/// <summary>
/// Get the Rules which apply to the FileName of the FileEntry provided.
/// </summary>
/// <param name="languageInfo"></param>
/// <param name="fileEntry"></param>
/// <param name="tagsToIgnore"></param>
/// <returns></returns>
public IEnumerable<ConvertedOatRule> GetRulesForFile(LanguageInfo languageInfo, FileEntry fileEntry, IEnumerable<string>? tagsToIgnore)
{
return GetRulesByLanguage(languageInfo.Name)
Expand All @@ -229,6 +260,14 @@ public IEnumerable<ConvertedOatRule> GetRulesForFile(LanguageInfo languageInfo,
.Where(x => !x.AppInspectorRule.Disabled && SeverityLevel.HasFlag(x.AppInspectorRule.Severity));
}

/// <summary>
/// Analyzes a file and returns a list of <see cref="MatchRecord"/>
/// </summary>
/// <param name="fileEntry">FileEntry which holds the name of the file being analyzed as well as a Stream containing the contents to analyze</param>
/// <param name="languageInfo">The LanguageInfo for the file</param>
/// <param name="tagsToIgnore">Ignore rules that match tags that are only in the tags to ignore list</param>
/// <param name="numLinesContext">Number of lines of text to extract for the sample. Set to 0 to disable context gathering. Set to -1 to also disable sampling the match.</param>
/// <returns>A List of the matches against the Rules the processor is configured with.</returns>
public List<MatchRecord> AnalyzeFile(FileEntry fileEntry, LanguageInfo languageInfo, IEnumerable<string>? tagsToIgnore = null, int numLinesContext = 3)
{
using var sr = new StreamReader(fileEntry.Content);
Expand Down
57 changes: 41 additions & 16 deletions AppInspector/Commands/AnalyzeCommand.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ public class AnalyzeOptions
/// If <see cref="DisableCrawlArchives"/> is not set, will restrict the amount of time allowed to extract each archive. Not supported in async operations.
/// </summary>
public int EnumeratingTimeout { get; set; }

public bool DisableCustomRuleVerification { get; set; }
}

/// <summary>
Expand Down Expand Up @@ -205,32 +207,55 @@ private void ConfigRules()
if (!string.IsNullOrEmpty(_options.CustomRulesPath))
{
rulesSet ??= new RuleSet(_loggerFactory);

RulesVerifierOptions rulesVerifierOptions = new()
{
FailFast = false,
LanguageSpecs = _languages,
LoggerFactory = _loggerFactory,
};
RulesVerifier verifier = new(rulesVerifierOptions);
bool anyFails = false;
if (Directory.Exists(_options.CustomRulesPath))
{
rulesSet.AddDirectory(_options.CustomRulesPath);
foreach (string filename in Directory.EnumerateFileSystemEntries(_options.CustomRulesPath, "*.json",
SearchOption.AllDirectories))
{
VerifyFile(filename);
}
}
else if (File.Exists(_options.CustomRulesPath)) //verify custom rules before use
{
RulesVerifierOptions rulesVerifierOptions = new()
{
FailFast = false,
LanguageSpecs = _languages,
LoggerFactory = _loggerFactory,
};
RulesVerifier verifier = new(rulesVerifierOptions);
RulesVerifierResult verification = verifier.Verify(_options.CustomRulesPath);
if (!verification.Verified)
{
throw new OpException(MsgHelp.FormatString(MsgHelp.ID.VERIFY_RULE_LOADFILE_FAILED, _options.CustomRulesPath));
}

rulesSet.AddRange(verification.CompiledRuleSet.GetAppInspectorRules());
VerifyFile(_options.CustomRulesPath);
}
else
{
throw new OpException(MsgHelp.FormatString(MsgHelp.ID.CMD_INVALID_RULE_PATH, _options.CustomRulesPath));
}

if (anyFails)
{
throw new OpException(MsgHelp.FormatString(MsgHelp.ID.VERIFY_RULE_LOADFILE_FAILED, _options.CustomRulesPath));
}

void VerifyFile(string filename)
{
if (!_options.DisableCustomRuleVerification)
{
RulesVerifierResult verification = verifier.Verify(_options.CustomRulesPath);
if (!verification.Verified)
{
anyFails = true;
}
else
{
rulesSet.AddFile(filename);
}
}
else
{
rulesSet.AddFile(filename);
}
}
}

//error check based on ruleset not path enumeration
Expand Down
2 changes: 2 additions & 0 deletions AppInspector/Commands/TagDiffCommand.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public class TagDiffOptions
public IEnumerable<Severity> SeverityFilters { get; set; } = new Severity[] { Severity.Critical | Severity.Important | Severity.Moderate | Severity.BestPractice | Severity.ManualReview };
public string? CustomCommentsPath { get; set; }
public string? CustomLanguagesPath { get; set; }
public bool DisableCustomRuleValidation { get; set; }
}

/// <summary>
Expand Down Expand Up @@ -151,6 +152,7 @@ public TagDiffResult GetResult()
NoShowProgress = true,
ScanUnknownTypes = _options.ScanUnknownTypes,
SingleThread = _options.SingleThread,
DisableCustomRuleVerification = _options.DisableCustomRuleValidation
}, _factory);
AnalyzeCommand cmd2 = new(new AnalyzeOptions()
{
Expand Down

0 comments on commit eb47b55

Please sign in to comment.