Introduce GitHub DSP analysis rules (#2021)

* Bump version; update release history.

* Add GitHub DSP policy file.

* Fix broken functional test.

* Add user-facing strings for SARIF2017.

* Define rule id for SARIF2017.

* Introduce SARIF2017.LocationsMustHaveRequiredProperties.

* Add "valid" functional test for SARIF2017.LocationsMustHaveRequiredProperties.

* Add "invalid" functional test for SARIF2017.LocationsMustHaveRequiredProperties.

* Cover case where result.locations is empty.

* Move location property bags up to expose JPointer bug.

* Introduce Skimmer.EnabledByDefault

* Skimmer: Populate DefaultConfiguration so it appears in rule metadata.

* Don't execute default-disabled rules unless the configuration enables them.

* Add first rule to policy file; add file to solution.

* Add DSP XML config file.

* Remove associated tool GUID from SARIF config file.

* Update solution file for renamed policy file.

* Adjust line numbers to fix test broken by JPointer bug.

* Update a comment.

* Rename misnamed resource strings.

* Implement SARIF2018.InlineThreadFlowLocations.

* Implement SARIF2019.RegionsMustProvideRequiredProperties.

* Update policy files for SARIF2019.

* Implement SARIF2020.ReviewArraysThatExceedConfigurableDefaults.

* Fix broken formatted messages; improve messages.

* Fix naming errors in policy files and a bug in SARIF2019.

* Implement SARIF2021.LocationsMustBeRelativeUrisOrFilePaths.

* Implement SARIF2022.ProvideCheckoutPath.

* SARIF2017 now covers related locations.

* SARIF2017: Add tests for relatedLocations.

* SARIF2017: Really add relatedLocations logic this time.

* Rename "policy" files to "config".

* Protect SARIF1004 against a null ref.

* Correct user-facing strings for SARIF2019 to match DSP behavior.

* Improve user-facing strings for SARIF2021.

* Avoid null ref in SARIF2022.

* Implement SARIF2023.RelatedLocationsMustProvideRequiredProperties.

* Update test for changed message.

* Fix typo in summary comment.

* Refactor SARIF2021 to prepare for related locations.

* Apply SARIF2021 to related locations.

Co-authored-by: Larry Golding <lgolding@comcast.net>

policies/github-dsp.config.sarif-external-properties

@@ -0,0 +1,67 @@
+{
+ "$schema": "https://json.schemastore.org/sarif-external-property-file-2.1.0-rtm.5",
+ "version": "2.1.0",
+ "guid": "89072b7a-3d16-43f2-ac0b-3d9cffc814be",
+ "policies": [
+ {
+ "name": "GitHub Developer Security Portal policy",
+ "version": "0.0.1",
+ "organization": "Microsoft",
+ "product": "Microsoft SARIF SDK",
+ "associatedComponent": {
+ "name": "Sarif.Multitool"
+ },
+ "rules": [
+ {
+ "id": "SARIF2017",
+ "name": "LocationsMustProvideRequiredProperties",
+ "defaultConfiguration": {
+ "enabled": true
+ }
+ },
+ {
+ "id": "SARIF2018",
+ "name": "InlineThreadFlowLocations",
+ "defaultConfiguration": {
+ "enabled": true
+ }
+ },
+ {
+ "id": "SARIF2019",
+ "name": "RegionsMustProvideRequiredProperties",
+ "defaultConfiguration": {
+ "enabled": true
+ }
+ },
+ {
+ "id": "SARIF2020",
+ "name": "ReviewArraysThatExceedConfigurableDefaults",
+ "defaultConfiguration": {
+ "enabled": true
+ }
+ },
+ {
+ "id": "SARIF2021",
+ "name": "LocationsMustBeRelativeUrisOrFilePaths",
+ "defaultConfiguration": {
+ "enabled": true
+ }
+ },
+ {
+ "id": "SARIF2022",
+ "name": "ProvideCheckoutPath",
+ "defaultConfiguration": {
+ "enabled": true
+ }
+ },
+ {
+ "id": "SARIF2023",
+ "name": "RelatedLocationMustProvideRequiredProperties",
+ "defaultConfiguration": {
+ "enabled": true
+ }
+ }
+ ]
+ }
+ ]
+}

policies/github-dsp.config.xml

@@ -0,0 +1,24 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Properties>
+ <Properties Key='SARIF2017.LocationsMustProvideRequiredProperties.Options'>
+ <Property Key='RuleEnabled' Value='Error' />
+ </Properties>
+ <Properties Key='SARIF2018.InlineThreadFlowLocations.Options'>
+ <Property Key='RuleEnabled' Value='Error' />
+ </Properties>
+ <Properties Key='SARIF2019.RegionsMustProvideRequiredProperties.Options'>
+ <Property Key='RuleEnabled' Value='Error' />
+ </Properties>
+ <Properties Key='SARIF2020.ReviewArraysThatExceedConfigurableDefaults.Options'>
+ <Property Key='RuleEnabled' Value='Error' />
+ </Properties>
+ <Properties Key='SARIF2021.LocationsMustBeRelativeUrisOrFilePaths.Options'>
+ <Property Key='RuleEnabled' Value='Error' />
+ </Properties>
+ <Properties Key='SARIF2022.ProvideCheckoutPath.Options'>
+ <Property Key='RuleEnabled' Value='Error' />
+ </Properties>
+ <Properties Key='SARIF2023.RelatedLocationsMustProvideRequiredProperties.Options'>
+ <Property Key='RuleEnabled' Value='Error' />
+ </Properties>
+</Properties>

src/ReleaseHistory.md

@@ -3,6 +3,10 @@
 * BUGFIX: Various Fortify FPR converter improvements (such as improve variable expansion in result messages).
 * COMMAND-LINE BREAKING: Change `merge` command output directory argument name to `output-directory`.
 
+## **v2.3.4** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/2.3.4) | [Driver](https://www.nuget.org/packages/Sarif.Driver/2.3.4) | [Converters](https://www.nuget.org/packages/Sarif.Converters/2.3.4) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/2.3.4)
+* FEATURE: Add analysis rules appropriate for SARIF files that are to be uploaded to the GitHub Developer Security Portal.
+* BUGFIX: The validator no longer reports `SARIF2010.ProvideCodeSnippets` if embedded file content for the specified artifact is present. [#2003](https://github.com/microsoft/sarif-sdk/issues/2003)
+
 ## **v2.3.3** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/2.3.3) | [Driver](https://www.nuget.org/packages/Sarif.Driver/2.3.3) | [Converters](https://www.nuget.org/packages/Sarif.Converters/2.3.3) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/2.3.3)
 * FEATURE: Improve `SarifSdkSample` application: use `uriBaseIds`.
 * FEATURE: Add additional checks to SARIF analysis rule `SARIF2004.OptimizeFileSize`.

src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs

@@ -546,12 +546,12 @@ protected virtual void AnalyzeTargets(
  TContext rootContext,
  IEnumerable<string> targets)
  {
- SortedSet<string> disabledSkimmers = new SortedSet<string>();
+ var disabledSkimmers = new SortedSet<string>();
 
  foreach (Skimmer<TContext> skimmer in skimmers)
  {
- PerLanguageOption<RuleEnabledState> ruleEnabledProperty;
- ruleEnabledProperty = DefaultDriverOptions.CreateRuleSpecificOption(skimmer, DefaultDriverOptions.RuleEnabled);
+ PerLanguageOption<RuleEnabledState> ruleEnabledProperty =
+   DefaultDriverOptions.CreateRuleSpecificOption(skimmer, DefaultDriverOptions.RuleEnabled);
 
  RuleEnabledState ruleEnabled = rootContext.Policy.GetProperty(ruleEnabledProperty);
 
@@ -561,6 +561,12 @@ protected virtual void AnalyzeTargets(
  Warnings.LogRuleExplicitlyDisabled(rootContext, skimmer.Id);
  RuntimeErrors |= RuntimeConditions.RuleWasExplicitlyDisabled;
  }
+ else if (!skimmer.DefaultConfiguration.Enabled && ruleEnabled == RuleEnabledState.Default)
+ {
+ // This skimmer is disabled by default, and the configuration file didn't mention it.
+ // So disable it, but don't complain that the rule was explicitly disabled.
+ disabledSkimmers.Add(skimmer.Id);
+ }
  }
 
  if (disabledSkimmers.Count == skimmers.Count())

src/Sarif.Driver/Sdk/RuleEnabledState.cs

@@ -22,7 +22,7 @@ public enum RuleEnabledState
  Disabled = 0x1,
 
  /// <summary>
- /// User has reduced all signal from a rule to warning. Warnings
+ /// User has reduced all signal from a rule to warnings. Warnings
  /// should always be persisted to reports but may not block
  /// engineering processes.
  /// </summary>
@@ -31,6 +31,11 @@ public enum RuleEnabledState
  /// <summary>
  /// User has elevated all failures from a check to errors.
  /// </summary>
- Error = 0x4
+ Error = 0x4,
+
+ /// <summary>
+ /// User has reduced all signal from a rule to notes.
+ /// </summary>
+ Note = 0x8
  }
 }

src/Sarif.Driver/Sdk/Skimmer.cs

@@ -12,6 +12,11 @@ public abstract class Skimmer<TContext> : ReportingDescriptor
  public Skimmer()
  {
  this.Options = new Dictionary<string, string>();
+ this.DefaultConfiguration = new ReportingConfiguration
+ {
+ Level = this.DefaultLevel,
+ Enabled = this.EnabledByDefault
+ };
  }
 
  private IDictionary<string, MultiformatMessageString> multiformatMessageStrings;
@@ -20,7 +25,9 @@ public Skimmer()
 
  protected virtual IEnumerable<string> MessageResourceNames => throw new NotImplementedException();
 
- virtual public FailureLevel DefaultLevel { get { return FailureLevel.Warning; } }
+ virtual public FailureLevel DefaultLevel => FailureLevel.Warning;
+
+ virtual public bool EnabledByDefault => true;
 
  public override IDictionary<string, MultiformatMessageString> MessageStrings
  {

src/Sarif.Multitool/Rules/RuleId.cs

@@ -38,6 +38,17 @@ public static class RuleId
  public const string EnquoteDynamicMessageContent = "SARIF2015";
  public const string FileUrisShouldBeRelative = "SARIF2016";
 
+ // Rules required to upload SARIF files to the GitHub Developer Security Portal.
+ // These rules are disabled by default. The can be enabled by running the MultiTool
+ // with the configuration file policies/github-dsp.sarif-external-properties.
+ public const string LocationsMustProvideRequiredProperties = "SARIF2017";
+ public const string InlineThreadFlowLocations = "SARIF2018";
+ public const string RegionsMustProvideRequiredProperties = "SARIF2019";
+ public const string ReviewArraysThatExceedConfigurableDefaults = "SARIF2020";
+ public const string LocationsMustBeRelativeUrisOrFilePaths = "SARIF2021";
+ public const string ProvideCheckoutPath = "SARIF2022";
+ public const string RelatedLocationsMustProvideRequiredProperties = "SARIF2023";
+
  // TEMPLATE:
  // public const string RuleFriendlyName = "SARIFnnnn";
  }