Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SARIF2005.ProvideToolProperties: Allow dottedQuadFileVersion; require informationUri. #2044

Merged
14 commits merged into from
Aug 31, 2020
Merged
2 changes: 2 additions & 0 deletions src/Sarif.Driver/Sdk/Skimmer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,5 +68,7 @@ public virtual AnalysisApplicability CanAnalyze(TContext context, out string rea
}

public abstract void Analyze(TContext context);

protected static string MakeAnalyzerMoniker(string id, string name) => $"{id}.{name}";
}
}
11 changes: 10 additions & 1 deletion src/Sarif.Multitool/Rules/RuleResources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion src/Sarif.Multitool/Rules/RuleResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ If 'version' is used, facilitate comparison between versions by specifying a ver
<value>{0}: The tool name '{1}' contains {2} words, which is more than the recommended maximum of {3} words. A short tool name is easy to remember and fits into a narrow column when displaying a list of results. If you need to provide more information about your tool, use the 'fullName' property.</value>
</data>
<data name="SARIF2005_ProvideToolProperties_Warning_ProvideToolVersion_Text" xml:space="preserve">
<value>{0}: The tool '{1}' provides neither a 'version' property nor a 'semanticVersion' property. Providing a version enables the log file consumer to determine whether the file was produced by an up to date version, and to avoid accidentally comparing log files produced by different tool versions.</value>
<value>{0}: The tool '{1}' does not provide any of the version-related properties {2}. Providing version information enables the log file consumer to determine whether the file was produced by an up to date version, and to avoid accidentally comparing log files produced by different tool versions.</value>
Copy link
Author

@ghost ghost Aug 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{2} [](start = 86, length = 3)

The quoting is done in the code. #ByDesign

</data>
<data name="SARIF2005_ProvideToolProperties_Warning_UseNumericToolVersions_Text" xml:space="preserve">
<value>{0}: The tool '{1}' contains the 'version' property '{2}', which is not numeric. To facilitate comparison between versions, specify a 'version' that starts with an integer, optionally followed by any desired characters.</value>
Expand Down Expand Up @@ -448,4 +448,7 @@ Semantics: Assuming the reader of the log file (an end user or another tool) has
<data name="SARIF2023_RelatedLocationsMustProvideRequiredProperties_FullDescription_Text" xml:space="preserve">
<value>The GitHub Developer Security Portal (DSP) will reject a SARIF file that includes a "related location" with no 'message' property. This is a bug in the DSP. You can set 'message' to an empty string if you don't have anything else to say about the location.</value>
</data>
<data name="SARIF2005_ProvideToolProperties_Warning_ProvideToolnformationUri_Text" xml:space="preserve">
<value>{0}: The tool '{1}' does not provide 'informationUri'. This property helps the developer responsible for addessing a result by providing a way to learn more about the tool.</value>
</data>
</root>
72 changes: 65 additions & 7 deletions src/Sarif.Multitool/Rules/SARIF2005.ProvideToolProperties.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text.RegularExpressions;

using Microsoft.Json.Pointer;
Expand Down Expand Up @@ -36,11 +37,42 @@ public class ProvideToolProperties : SarifValidationSkimmerBase
protected override IEnumerable<string> MessageResourceNames => new string[] {
nameof(RuleResources.SARIF2005_ProvideToolProperties_Warning_ProvideToolVersion_Text),
nameof(RuleResources.SARIF2005_ProvideToolProperties_Warning_ProvideConciseToolName_Text),
nameof(RuleResources.SARIF2005_ProvideToolProperties_Warning_UseNumericToolVersions_Text)
nameof(RuleResources.SARIF2005_ProvideToolProperties_Warning_UseNumericToolVersions_Text),
nameof(RuleResources.SARIF2005_ProvideToolProperties_Warning_ProvideToolnformationUri_Text)
};

public override FailureLevel DefaultLevel => FailureLevel.Warning;

// This rule configuration parameter specifies which of the three version-related properties
// satisfy the requirement that the tool provide version information. By default, any of
// them is acceptable.
public static PerLanguageOption<StringSet> AcceptableVersionProperties =>
new PerLanguageOption<StringSet>(
AnalyzerMoniker, nameof(AcceptableVersionProperties), defaultValue: () => DefaultAcceptableVersionProperties);

// This rule configuration parameter specifies whether the informationUri property (which
// helps the responsible developer learn more about the tool that produced the result) is
// required.
public static PerLanguageOption<bool> InformationUriRequired =>
new PerLanguageOption<bool>(
AnalyzerMoniker, nameof(InformationUriRequired), defaultValue: () => true);

private static readonly string AnalyzerMoniker = MakeAnalyzerMoniker(RuleId.ProvideToolProperties, nameof(RuleId.ProvideToolProperties));

// We instantiate this object just so we can access its property names below. It's internal
// rather than private so we can do the same thing in the tests.
internal static readonly ToolComponent s_dummyToolComponent = new ToolComponent();

private static StringSet DefaultAcceptableVersionProperties =>
new StringSet(
new string[]
{
nameof(s_dummyToolComponent.Version),
nameof(s_dummyToolComponent.SemanticVersion),
nameof(s_dummyToolComponent.DottedQuadFileVersion)
}
);

private static readonly Regex s_versionRegex = new Regex(@"^\d+.*", RegexOptions.Compiled | RegexOptions.CultureInvariant);

protected override void Analyze(Tool tool, string toolPointer)
Expand Down Expand Up @@ -74,17 +106,36 @@ private void AnalyzeToolDriver(ToolComponent toolComponent, string toolDriverPoi
}
}

if (string.IsNullOrWhiteSpace(toolComponent.Version) && string.IsNullOrWhiteSpace(toolComponent.SemanticVersion))
bool informationUriRequired = this.Context.Policy.GetProperty(InformationUriRequired);
if (informationUriRequired && toolComponent.InformationUri == null)
{
// {0}: The tool '{1}' provides neither a 'version' property nor a 'semanticVersion'
// property. Providing a version enables the log file consumer to determine whether
// the file was produced by an up to date version, and to avoid accidentally comparing
// log files produced by different tool versions.
// {0}: The tool '{1}' does not provide 'informationUri'. This property helps the
// developer responsible for addessing a result by providing a way to learn more
// about the tool.
LogResult(
toolDriverPointer,
nameof(RuleResources.SARIF2005_ProvideToolProperties_Warning_ProvideToolVersion_Text),
nameof(RuleResources.SARIF2005_ProvideToolProperties_Warning_ProvideToolnformationUri_Text),
toolComponent.Name);
}

StringSet acceptableVersionProperties = this.Context.Policy.GetProperty(AcceptableVersionProperties);
bool toolDriverProvidesVersion = false;
toolDriverProvidesVersion |= acceptableVersionProperties.Contains(nameof(toolComponent.Version)) && !string.IsNullOrWhiteSpace(toolComponent.Version);
toolDriverProvidesVersion |= acceptableVersionProperties.Contains(nameof(toolComponent.SemanticVersion)) && !string.IsNullOrWhiteSpace(toolComponent.SemanticVersion);
toolDriverProvidesVersion |= acceptableVersionProperties.Contains(nameof(toolComponent.DottedQuadFileVersion)) && !string.IsNullOrWhiteSpace(toolComponent.DottedQuadFileVersion);

if (!toolDriverProvidesVersion)
{
// {0}: The tool '{1}' does not provide any of the version-related properties {2}.
// Providing version information enables the log file consumer to determine whether
// the file was produced by an up to date version, and to avoid accidentally
// comparing log files produced by different tool versions.
LogResult(
toolDriverPointer,
nameof(RuleResources.SARIF2005_ProvideToolProperties_Warning_ProvideToolVersion_Text),
toolComponent.Name,
$"'{string.Join("', '", acceptableVersionProperties.Select(ToCamelCase))}'");
}
else
{
if (!string.IsNullOrWhiteSpace(toolComponent.Version))
Expand All @@ -108,5 +159,12 @@ private void AnalyzeVersion(string name, string version, string pointer)
version);
}
}

private static string ToCamelCase(string name)
=> name == null
? throw new ArgumentNullException(nameof(name))
: name.Length == 1
? name.ToLowerInvariant()
: $"{name.Substring(0, 1).ToLowerInvariant()}{name.Substring(1)}";
}
}
83 changes: 71 additions & 12 deletions src/Test.FunctionalTests.Sarif/Multitool/ValidateCommandTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using System.IO;
using System.Linq;
using System.Reflection;
using System.Text;

using FluentAssertions;

Expand Down Expand Up @@ -176,6 +175,66 @@ public void SARIF2005_ProvideToolProperties_Valid()
public void SARIF2005_ProvideToolProperties_Invalid()
=> RunInvalidTestForRule(RuleId.ProvideToolProperties);

[Fact]
public void SARIF2005_ProvideToolProperties_DottedQuadFileVersion_AcceptedByConfiguration()
{
var acceptableVersionProperties = new KeyValuePair<string, object>(
nameof(ProvideToolProperties.AcceptableVersionProperties),
new StringSet
{
nameof(ProvideToolProperties.s_dummyToolComponent.SemanticVersion),
nameof(ProvideToolProperties.s_dummyToolComponent.DottedQuadFileVersion)
});

RunTest(
inputResourceName: "SARIF2005.ProvideToolProperties_DottedQuadFileVersion.sarif",
expectedOutputResourceName: "SARIF2005.ProvideToolProperties_DottedQuadFileVersion_Valid.sarif",
parameter: acceptableVersionProperties);
}

[Fact]
public void SARIF2005_ProvideToolProperties_DottedQuadFileVersion_RejectedByConfiguration()
{
var acceptableVersionProperties = new KeyValuePair<string, object>(
nameof(ProvideToolProperties.AcceptableVersionProperties),
new StringSet
{
nameof(ProvideToolProperties.s_dummyToolComponent.Version),
nameof(ProvideToolProperties.s_dummyToolComponent.SemanticVersion)
});

RunTest(
inputResourceName: "SARIF2005.ProvideToolProperties_DottedQuadFileVersion.sarif",
expectedOutputResourceName: "SARIF2005.ProvideToolProperties_DottedQuadFileVersion_Invalid.sarif",
parameter: acceptableVersionProperties);
}

[Fact]
public void SARIF2005_ProvideToolProperties_MissingInformationUri_AcceptedByConfiguration()
{
var informationUriRequired = new KeyValuePair<string, object>(
nameof(ProvideToolProperties.InformationUriRequired),
false);

RunTest(
inputResourceName: "SARIF2005.ProvideToolProperties_MissingInformationUri.sarif",
expectedOutputResourceName: "SARIF2005.ProvideToolProperties_MissingInformationUri_Valid.sarif",
parameter: informationUriRequired);
}

[Fact]
public void SARIF2005_ProvideToolProperties_MissingInformationUri_RejectedByConfiguration()
{
var informationUriRequired = new KeyValuePair<string, object>(
nameof(ProvideToolProperties.InformationUriRequired),
true);

RunTest(
inputResourceName: "SARIF2005.ProvideToolProperties_MissingInformationUri.sarif",
expectedOutputResourceName: "SARIF2005.ProvideToolProperties_MissingInformationUri_Invalid.sarif",
parameter: informationUriRequired);
}

[Fact]
public void SARIF2006_UrisShouldBeReachable_Valid()
=> RunValidTestForRule(RuleId.UrisShouldBeReachable);
Expand Down Expand Up @@ -426,7 +485,7 @@ protected override string ConstructTestOutputFromInputResource(string inputResou

// Some rules are disabled by default, so create a configuration file that explicitly
// enables the rule under test.
using (TempFile configFile = CreateTempConfigFile(ruleUnderTest))
using (TempFile configFile = CreateTempConfigFile(ruleUnderTest, parameter))
{
validateOptions.ConfigurationFilePath = configFile.Name;
mockFileSystem.Setup(x => x.FileExists(validateOptions.ConfigurationFilePath)).Returns(true);
Expand Down Expand Up @@ -495,26 +554,26 @@ protected override string ConstructTestOutputFromInputResource(string inputResou
private static bool IsSarifRule(string ruleId)
=> ruleId.StartsWith("SARIF");

private TempFile CreateTempConfigFile(string ruleId)
private TempFile CreateTempConfigFile(string ruleId, object parameter)
{
var sb = new StringBuilder();
sb.AppendLine("<?xml version='1.0' encoding='utf-8' ?>");
sb.AppendLine("<Properties>");
var propertiesDictionary = new PropertiesDictionary();

if (IsSarifRule(ruleId))
{
var rulePropertiesDictionary = new PropertiesDictionary();
SarifValidationSkimmerBase rule = GetRuleFromId(ruleId);
RuleEnabledState ruleEnabledState = GetRuleEnabledState(rule);
rulePropertiesDictionary.Add(nameof(DefaultDriverOptions.RuleEnabled), ruleEnabledState);
if (parameter is KeyValuePair<string, object> pair)
{
rulePropertiesDictionary.Add(pair.Key, pair.Value);
}

sb.AppendLine($" <Properties Key='{rule.Moniker}.Options'>");
sb.AppendLine($" <Property Key='RuleEnabled' Value='{ruleEnabledState}' />");
sb.AppendLine(" </Properties>");
propertiesDictionary.Add($"{rule.Moniker}.Options", rulePropertiesDictionary);
}

sb.AppendLine("</Properties>");

var tempFile = new TempFile(".xml");
File.WriteAllText(tempFile.Name, sb.ToString());
propertiesDictionary.SaveToXml(tempFile.Name);
return tempFile;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
{
"$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json",
"version": "2.1.0",
"runs": [
{
"tool": {
"driver": {
"name": "SARIF Functional Testing",
"rules": [
{
"id": "SARIF2005",
"name": "ProvideToolProperties",
"shortDescription": {
"text": "Provide information that makes it easy to identify the name and version of your tool."
},
"fullDescription": {
"text": "Provide information that makes it easy to identify the name and version of your tool.\r\n\r\nThe tool's 'name' property should be no more than three words long. This makes it easy to remember and allows it to fit into a narrow column when displaying a list of results. If you need to provide more information about your tool, use the 'fullName' property.\r\n\r\nThe tool should provide either or both of the 'version' and 'semanticVersion' properties. This enables the log file consumer to determine whether the file was produced by an up to date version, and to avoid accidentally comparing log files produced by different tool versions.\r\n\r\nIf 'version' is used, facilitate comparison between versions by specifying a version number that starts with an integer, optionally followed by any desired characters."
},
"messageStrings": {
"Warning_ProvideToolVersion": {
"text": "{0}: The tool '{1}' does not provide any of the version-related properties {2}. Providing version information enables the log file consumer to determine whether the file was produced by an up to date version, and to avoid accidentally comparing log files produced by different tool versions."
},
"Warning_ProvideConciseToolName": {
"text": "{0}: The tool name '{1}' contains {2} words, which is more than the recommended maximum of {3} words. A short tool name is easy to remember and fits into a narrow column when displaying a list of results. If you need to provide more information about your tool, use the 'fullName' property."
},
"Warning_UseNumericToolVersions": {
"text": "{0}: The tool '{1}' contains the 'version' property '{2}', which is not numeric. To facilitate comparison between versions, specify a 'version' that starts with an integer, optionally followed by any desired characters."
},
"Warning_ProvideToolnformationUri": {
"text": "{0}: The tool '{1}' does not provide 'informationUri'. This property helps the developer responsible for addessing a result by providing a way to learn more about the tool."
}
},
"helpUri": "http://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html"
}
]
}
},
"invocations": [
{
"executionSuccessful": true
}
],
"artifacts": [
{
"location": {
"uri": "FunctionalTestOutput.ValidateCommand/Inputs.SARIF2005.ProvideToolProperties_DottedQuadFileVersion.sarif",
"uriBaseId": "TEST_DIR"
}
}
],
"results": [
{
"ruleId": "SARIF2005",
"ruleIndex": 0,
"message": {
"id": "Warning_ProvideToolVersion",
"arguments": [
"runs[0].tool.driver",
"SARIF Functional Testing",
"'semanticVersion', 'version'"
Copy link
Author

@ghost ghost Aug 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

semanticVersion [](start = 16, length = 15)

With the properties dictionary created by XML serialization rather than hard-coded string, the properties came out in a different order (presumably alphabetical). #WontFix

]
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"index": 0
},
"region": {
"startLine": 7,
"startColumn": 19
}
}
}
]
}
],
"columnKind": "utf16CodeUnits"
}
]
}
Loading