Skip to content

Commit

Permalink
Add Support for Some DevSkim rule parameters (#471)
Browse files Browse the repository at this point in the history 
* Adds the "does_not_apply_to" parameter for languages
* Adds other types of conditions supported by devskim

OnlyBefore - Condition matches if it occurs anywhere in the same file before the initial match
OnlyAfter - Condition matches if it occurs anywhere in the same file after the initial match
SameFile - Condition matches if it occurs anywhere in the same file, including the original match
  • Loading branch information
@gfs
gfs authored Jul 13, 2022
1 parent b6084dd commit 8da88b2
Show file tree
Hide file tree
Showing 7 changed files with 269 additions and 23 deletions.
90 changes: 90 additions & 0 deletions AppInspector.Tests/Commands/TestAnalyzeCmd.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ public class TestAnalyzeCmd
{
private static string testFilePath = string.Empty;
private static string testRulesPath = string.Empty;
private static string appliesToTestRulePath = string.Empty;
private static string doesNotApplyToTestRulePath = string.Empty;

// Test files for timeout tests
private static List<string> enumeratingTimeOutTestsFiles = new();
Expand All @@ -38,9 +40,15 @@ public static void ClassInit(TestContext context)
testFilePath = Path.Combine(TestHelpers.GetPath(TestHelpers.AppPath.testOutput),"TestFile.js");
testRulesPath = Path.Combine(TestHelpers.GetPath(TestHelpers.AppPath.testOutput), "TestRules.json");
heavyRulePath = Path.Combine(TestHelpers.GetPath(TestHelpers.AppPath.testOutput), "HeavyRule.json");
appliesToTestRulePath = Path.Combine(TestHelpers.GetPath(TestHelpers.AppPath.testOutput), "AppliesToTestRules.json");
doesNotApplyToTestRulePath = Path.Combine(TestHelpers.GetPath(TestHelpers.AppPath.testOutput), "DoesNotApplyToTestRules.json");

File.WriteAllText(heavyRulePath, heavyRule);
File.WriteAllText(testFilePath, fourWindowsOneLinux);
File.WriteAllText(testRulesPath, findWindows);
File.WriteAllText(appliesToTestRulePath, findWindowsWithAppliesTo);
File.WriteAllText(doesNotApplyToTestRulePath, findWindowsWithDoesNotApplyTo);

for (int i = 0; i < numTimeOutFiles; i++)
{
string newPath = Path.Combine(TestHelpers.GetPath(TestHelpers.AppPath.testOutput),$"TestFile-{i}.js");
Expand Down Expand Up @@ -90,6 +98,51 @@ public static void CleanUp()
}
]
}]";

private const string findWindowsWithAppliesTo = @"[
{
""name"": ""Platform: Microsoft Windows"",
""id"": ""AI_TEST_WINDOWS"",
""description"": ""This rule checks for the string 'windows'"",
""tags"": [
""Test.Tags.Windows""
],
""applies_to"": [ ""javascript"" ],
""severity"": ""Important"",
""patterns"": [
{
""confidence"": ""Medium"",
""modifiers"": [
""i""
],
""pattern"": ""windows"",
""type"": ""String"",
}
]
}]";

private const string findWindowsWithDoesNotApplyTo = @"[
{
""name"": ""Platform: Microsoft Windows"",
""id"": ""AI_TEST_WINDOWS"",
""description"": ""This rule checks for the string 'windows'"",
""tags"": [
""Test.Tags.Windows""
],
""does_not_apply_to"": [ ""javascript"" ],
""severity"": ""Important"",
""patterns"": [
{
""confidence"": ""Medium"",
""modifiers"": [
""i""
],
""pattern"": ""windows"",
""type"": ""String"",
}
]
}]";


// These simple test rules rules look for the string "windows" and "linux"
const string findWindows = @"[
Expand Down Expand Up @@ -895,5 +948,42 @@ public void FileMetadata()
Assert.AreEqual(0, resultWithoutMetadata.Metadata.TotalFiles);
}

/// <summary>
/// Test that the applies_to parameter allows the specified types
/// </summary>
[TestMethod]
public void TestAppliesTo()
{
AnalyzeOptions options = new()
{
SourcePath = new string[1] { testFilePath },
CustomRulesPath = appliesToTestRulePath,
IgnoreDefaultRules = true
};

AnalyzeCommand command = new(options, factory);
AnalyzeResult result = command.GetResult();
Assert.AreEqual(AnalyzeResult.ExitCode.Success, result.ResultCode);
Assert.AreEqual(4, result.Metadata.TotalMatchesCount);
}

/// <summary>
/// Test that the does_not_apply_to parameter excludes the specified types
/// </summary>
[TestMethod]
public void TestDoesNotApplyTo()
{
AnalyzeOptions options = new()
{
SourcePath = new string[1] { testFilePath },
CustomRulesPath = doesNotApplyToTestRulePath,
IgnoreDefaultRules = true
};

AnalyzeCommand command = new(options, factory);
AnalyzeResult result = command.GetResult();
Assert.AreEqual(AnalyzeResult.ExitCode.NoMatches, result.ResultCode);
Assert.AreEqual(0, result.Metadata.TotalMatchesCount);
}
}
}
61 changes: 61 additions & 0 deletions AppInspector.Tests/RuleProcessor/WithinClauseTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ public class WithinClauseTests
[DataRow("WithinClauseWithoutInvertWithFindingRange")]
[DataRow("WithinClauseWithInvertWithSameLine")]
[DataRow("WithinClauseWithoutInvertWithSameLine")]
[DataRow("WithinClauseWithInvertWithBeforeOnly")]
[DataRow("WithinClauseWithoutInvertWithBeforeOnly")]
[DataRow("WithinClauseWithInvertWithAfterOnly")]
[DataRow("WithinClauseWithoutInvertWithAfterOnly")]
[DataRow("WithinClauseWithInvertWithSameFile")]
[DataRow("WithinClauseWithoutInvertWithSameFile")]

[DataTestMethod]
public void WithinClauseInvertTest(string testDataKey)
{
Expand Down Expand Up @@ -219,6 +226,30 @@ public void MultiLineRegexCondition()
{
"WithinClauseWithoutInvertWithSameLine",
(sameLineData, "same-line", false, 1, new[] { 14 })
},
{
"WithinClauseWithInvertWithBeforeOnly",
(beforeOnlyData, "only-before", true, 0, new int[] { })
},
{
"WithinClauseWithoutInvertWithBeforeOnly",
(beforeOnlyData, "only-before", false, 1, new[] { 3 })
},
{
"WithinClauseWithInvertWithAfterOnly",
(afterOnlyData, "only-after", true, 0, new int[] { })
},
{
"WithinClauseWithoutInvertWithAfterOnly",
(afterOnlyData, "only-after", false, 1, new[] { 2 })
},
{
"WithinClauseWithInvertWithSameFile",
(sameFileData, "same-file", true, 0, new int[] { })
},
{
"WithinClauseWithoutInvertWithSameFile",
(sameFileData, "same-file", false, 1, new[] { 2 })
}
};

Expand Down Expand Up @@ -377,6 +408,36 @@ int main(int argc, char **argv)
#define BUFSIZER1 512
#define BUFSIZER2 ((BUFSIZER1 / 2) - 8)
int main(int argc, char **argv)
{
char *buf1R1;
char *buf2R1;
buf1R1 = (char *)malloc(BUFSIZER1);
buf2R1 = (char *)malloc(BUFSIZER1);free(buf2R1);
strncpy(buf2R1, argv[1], BUFSIZER1 - 1);
free(buf1R1);
}";
const string beforeOnlyData = @"#include <stdio.h>
free(buf2R1);
buf2R1 = (char *)malloc(BUFSIZER1);
";
const string afterOnlyData = @"#include <stdio.h>
buf2R1 = (char *)malloc(BUFSIZER1);
free(buf2R1);";

const string sameFileData = @"#include <stdio.h>
buf1R1 = (char *)malloc(BUFSIZER1);
free(buf1R1);";

const string findingOnlyData = @"#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#define BUFSIZER1 512
#define BUFSIZER2 ((BUFSIZER1 / 2) - 8)
int main(int argc, char **argv)
{
char *buf1R1;
Expand Down
3 changes: 3 additions & 0 deletions RulesEngine/OatExtensions/WithinClause.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ public WithinClause(string? field = null) : base(Operation.Custom, field)

public int After { get; set; }
public int Before { get; set; }
public bool OnlyBefore { get; set; }
public bool OnlyAfter { get; set; }
public bool SameFile { get; set; }
public bool FindingOnly { get; set; }
public bool SameLineOnly { get; set; }
public PatternScope[] Scopes { get; set; } = new PatternScope[1] { PatternScope.All };
Expand Down
54 changes: 52 additions & 2 deletions RulesEngine/OatExtensions/WithinOperation.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,56 @@ public OperationResult WithinOperationDelegate(Clause c, object? state1, object?
toRemove.Add((clauseNum, capture));
}
}
else if (wc.SameFile)
{
var start = tc.LineStarts[0];
var end = tc.LineEnds[^1];
var res = ProcessLambda(tc.FullContent[start..end], capture);
if (res.Result)
{
if (res.Capture is TypedClauseCapture<List<Boundary>> boundaryList)
{
passed.AddRange(boundaryList.Result);
}
}
else
{
toRemove.Add((clauseNum, capture));
}
}
else if (wc.OnlyBefore)
{
var start = tc.LineStarts[0];
var end = capture.Index;
var res = ProcessLambda(tc.FullContent[start..end], capture);
if (res.Result)
{
if (res.Capture is TypedClauseCapture<List<Boundary>> boundaryList)
{
passed.AddRange(boundaryList.Result);
}
}
else
{
toRemove.Add((clauseNum, capture));
} }
else if (wc.OnlyAfter)
{
var start = capture.Index + capture.Length;
var end = tc.LineEnds[^1];
var res = ProcessLambda(tc.FullContent[start..end], capture);
if (res.Result)
{
if (res.Capture is TypedClauseCapture<List<Boundary>> boundaryList)
{
passed.AddRange(boundaryList.Result);
}
}
else
{
toRemove.Add((clauseNum, capture));
}
}
}
tcc.Result.RemoveAll(x => toRemove.Contains(x));
}
Expand Down Expand Up @@ -137,9 +187,9 @@ public IEnumerable<Violation> WithinValidationDelegate(CST.OAT.Rule rule, Clause
{
if (clause is WithinClause wc)
{
if (new bool[] {wc.FindingOnly, wc.SameLineOnly, wc.FindingRegion}.Count(x => x) != 1)
if (new bool[] {wc.FindingOnly, wc.SameLineOnly, wc.FindingRegion, wc.OnlyAfter, wc.OnlyBefore, wc.SameFile}.Count(x => x) != 1)
{
yield return new Violation($"Exactly one of: FindingOnly, SameLineOnly or FindingRegion must be set", rule, clause);
yield return new Violation($"Exactly one of: FindingOnly, SameLineOnly, OnlyAfter, OnlyBefore, SameFile or FindingRegion must be set", rule, clause);
}

if (wc.FindingRegion)
Expand Down
24 changes: 14 additions & 10 deletions RulesEngine/Rule.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,22 @@ public class Rule
[JsonIgnore]
public bool Disabled { get; set; }

[JsonProperty(PropertyName ="name")]
[JsonProperty(PropertyName = "name")]
public string Name { get; set; } = "";

[JsonProperty(PropertyName ="id")]
[JsonProperty(PropertyName = "id")]
public string Id { get; set; } = "";

[JsonProperty(PropertyName ="description")]
[JsonProperty(PropertyName = "description")]
public string? Description { get; set; } = "";

[JsonProperty(PropertyName ="applies_to")]
[JsonProperty(PropertyName = "does_not_apply_to")]
public List<string>? DoesNotApplyTo { get; set; }

[JsonProperty(PropertyName = "applies_to")]
public string[]? AppliesTo { get; set; }

[JsonProperty(PropertyName ="applies_to_file_regex")]
[JsonProperty(PropertyName = "applies_to_file_regex")]
public string[]? FileRegexes
{
get => _fileRegexes;
Expand All @@ -70,27 +73,28 @@ public IEnumerable<Regex> CompiledFileRegexes
_compiled = FileRegexes?.Select(x => new Regex(x, RegexOptions.Compiled)) ?? Array.Empty<Regex>();
_updateCompiledFileRegex = false;
}

return _compiled;
}
}

private IEnumerable<Regex> _compiled = Array.Empty<Regex>();
private bool _updateCompiledFileRegex = false;

[JsonProperty(PropertyName ="tags")]
[JsonProperty(PropertyName = "tags")]
public string[]? Tags { get; set; }

[JsonProperty(PropertyName ="severity")]
[JsonProperty(PropertyName = "severity")]
[JsonConverter(typeof(StringEnumConverter))]
public Severity Severity { get; set; } = Severity.Moderate;

[JsonProperty(PropertyName ="overrides")]
[JsonProperty(PropertyName = "overrides")]
public string[]? Overrides { get; set; }

[JsonProperty(PropertyName ="patterns")]
[JsonProperty(PropertyName = "patterns")]
public SearchPattern[] Patterns { get; set; } = Array.Empty<SearchPattern>();

[JsonProperty(PropertyName ="conditions")]
[JsonProperty(PropertyName = "conditions")]
public SearchCondition[]? Conditions { get; set; }
}
}
18 changes: 7 additions & 11 deletions RulesEngine/RuleProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ public IEnumerable<ConvertedOatRule> GetRulesForFile(LanguageInfo languageInfo,
{
return GetRulesByLanguage(languageInfo.Name)
.Union(GetRulesByFileName(fileEntry.FullPath))
.Union(GetUniversalRules())
.Union(GetUniversalRules().Where(x => !x.AppInspectorRule.DoesNotApplyTo?.Contains(languageInfo.Name) ?? true))
.Where(x => !x.AppInspectorRule.Tags?.Any(y => tagsToIgnore?.Contains(y) ?? false) ?? true)
.Where(x => !x.AppInspectorRule.Disabled && SeverityLevel.HasFlag(x.AppInspectorRule.Severity));
}
Expand All @@ -241,11 +241,7 @@ public List<MatchRecord> AnalyzeFile(FileEntry fileEntry, LanguageInfo languageI
{
_logger.LogDebug("Failed to analyze file {path}. {type}:{message}. ({stackTrace}), fileRecord.FileName", fileEntry.FullPath, e.GetType(), e.Message, e.StackTrace);
}
if (contents is not null)
{
return AnalyzeFile(contents, fileEntry, languageInfo, tagsToIgnore, numLinesContext);
}
return new List<MatchRecord>();
return AnalyzeFile(contents, fileEntry, languageInfo, tagsToIgnore, numLinesContext);
}

public async Task<List<MatchRecord>> AnalyzeFileAsync(FileEntry fileEntry, LanguageInfo languageInfo, CancellationToken? cancellationToken = null, IEnumerable<string>? tagsToIgnore = null, int numLinesContext = 3)
Expand Down Expand Up @@ -363,19 +359,19 @@ List<MatchRecord> ProcessBoundary(ClauseCapture cap)
/// </summary>
/// <param name="languages"> Languages to filter rules for </param>
/// <returns> List of rules </returns>
private IEnumerable<ConvertedOatRule> GetRulesByLanguage(string input)
private IEnumerable<ConvertedOatRule> GetRulesByLanguage(string language)
{
if (EnableCache)
{
if (_languageRulesCache.ContainsKey(input))
return _languageRulesCache[input];
if (_languageRulesCache.ContainsKey(language))
return _languageRulesCache[language];
}

IEnumerable<ConvertedOatRule> filteredRules = _ruleset.ByLanguage(input);
IEnumerable<ConvertedOatRule> filteredRules = _ruleset.ByLanguage(language);

if (EnableCache)
{
_languageRulesCache.TryAdd(input, filteredRules);
_languageRulesCache.TryAdd(language, filteredRules);
}

return filteredRules;
Expand Down
Loading

0 comments on commit 8da88b2

Please sign in to comment.