Introduce GitHub DSP analysis rules

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

@@ -0,0 +1,25 @@
+{
+  "$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": "LocationsMustHaveRequiredProperties",
+          "defaultConfiguration": {
+            "enabled": true
+          }
+        }
+      ]
+    }
+  ]
+}

policies/github-dsp-policy.config.xml

@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Properties>
+  <Properties Key='SARIF2017.LocationsMustHaveRequiredProperties.Options'>
+    <Property Key='RuleEnabled' Value='Error' />
+  </Properties>
+</Properties>

src/ReleaseHistory.md

@@ -1,5 +1,9 @@
 # SARIF Package Release History (SDK, Driver, Converters, and Multitool)
 
+## **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,11 @@ 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 LocationsMustHaveRequiredProperties = "SARIF2017";
+
         // TEMPLATE:
         // public const string RuleFriendlyName = "SARIFnnnn";
     }

src/Sarif.Multitool/Rules/RuleResources.resx

@@ -391,4 +391,16 @@ Semantics: Assuming the reader of the log file (an end user or another tool) has
   <data name="SARIF2016_FileUrisShouldBeRelative_Note_Default_Text" xml:space="preserve">
     <value>{0}: The file location '{1}' is specified with absolute URI. Prefer a relative reference together with a uriBaseId property.</value>
   </data>
+  <data name="SARIF2017_LocationsMustHaveRequiredProperties_FullDescription_Text" xml:space="preserve">
+    <value>Each result location must provide the property 'physicalLocation.artifactLocation.uri'. The GitHub Developer Security Portal will not display a result whose location does not provide the URI of the artifact that contains the result.</value>
+  </data>
+  <data name="SARIF2017_LocationsMustHaveRequired_Properties_Error_Default_Text" xml:space="preserve">
+    <value>{0}: '{1}' is absent. The GitHub Developer Security Portal will not display a result location that does not provide the URI of the artifact that contains the result.</value>
+  </data>
+  <data name="SARIF2017_LocationsMustHaveRequired_Properties_Error_EmptyLocationsArray_Text" xml:space="preserve">
+    <value>{0}: The 'locations' array is empty. The GitHub Developer Security Portal will not display a result unless it provides a location that specifies the URI of the artifact that contains the result.</value>
+  </data>
+  <data name="SARIF2017_LocationsMustHaveRequired_Properties_Error_NoLocationsArray_Text" xml:space="preserve">
+    <value>{0}: The 'locations' property is absent. The GitHub Developer Security Portal will not display a result unless it provides a location that specifies the URI of the artifact that contains the result.</value>
+  </data>
 </root>

src/Sarif.Multitool/Rules/SARIF2016.FileUrisShouldBeRelative.cs

@@ -70,7 +70,7 @@ private void AnalyzeResultLocations(IList<Location> locations, string locationsP
                 string locationPointer = locationsPointer.AtIndex(i);
                 Location location = locations[i];
 
-                if (location.PhysicalLocation?.ArtifactLocation.Uri != null)
+                if (location?.PhysicalLocation?.ArtifactLocation?.Uri != null)
                 {
                     if (location.PhysicalLocation.ArtifactLocation.Uri.IsAbsoluteUri && location.PhysicalLocation.ArtifactLocation.Uri.IsFile)
                     {

src/Sarif.Multitool/Rules/SARIF2017.LocationsMustHaveRequiredProperties.cs

@@ -0,0 +1,99 @@
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Linq;
+
+using Microsoft.Json.Pointer;
+
+namespace Microsoft.CodeAnalysis.Sarif.Multitool.Rules
+{
+    public class LocationsMustHaveRequiredProperties : SarifValidationSkimmerBase
+    {
+        /// <summary>
+        /// SARIF2017
+        /// </summary>
+        public override string Id => RuleId.LocationsMustHaveRequiredProperties;
+
+        /// <summary>
+        /// Each result location must provide the property 'physicalLocation.artifactLocation.uri'.
+        /// The GitHub Developer Security Portal will not display a result whose location does not
+        /// provide the URI of the artifact that was analyzed.
+        /// </summary>
+        public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.SARIF2017_LocationsMustHaveRequiredProperties_FullDescription_Text };
+
+        protected override IEnumerable<string> MessageResourceNames => new string[] {
+            nameof(RuleResources.SARIF2017_LocationsMustHaveRequired_Properties_Error_NoLocationsArray_Text),
+            nameof(RuleResources.SARIF2017_LocationsMustHaveRequired_Properties_Error_EmptyLocationsArray_Text),
+            nameof(RuleResources.SARIF2017_LocationsMustHaveRequired_Properties_Error_Default_Text)
+        };
+
+        public override FailureLevel DefaultLevel => FailureLevel.Error;
+
+        public override bool EnabledByDefault => false;
+
+        protected override void Analyze(Result result, string resultPointer)
+        {
+            if (result.Locations == null)
+            {
+                // {0}: The 'locations' property is absent. The GitHub Developer Security Portal
+                // will not display a result unless it provides a location that specifies the URI
+                // of the artifact that contains the result.
+                LogResult(
+                    resultPointer,
+                    nameof(RuleResources.SARIF2017_LocationsMustHaveRequired_Properties_Error_NoLocationsArray_Text),
+                    SarifPropertyName.Locations);
+                return;
+            }
+
+            string locationsPointer = resultPointer.AtProperty(SarifPropertyName.Locations);
+            if (!result.Locations.Any())
+            {
+                // {0}: The 'locations' array is empty. The GitHub Developer Security Portal will
+                // not display a result unless it provides a location that specifies the URI of the
+                // artifact that contains the result.
+                LogResult(
+                    locationsPointer,
+                    nameof(RuleResources.SARIF2017_LocationsMustHaveRequired_Properties_Error_EmptyLocationsArray_Text));
+                return;
+            }
+
+            for (int i = 0; i < result.Locations.Count; i++)
+            {
+                Location location = result.Locations[i];
+                string missingProperty = null;
+                string jsonPointer = null;
+
+                if (location.PhysicalLocation == null)
+                {
+                    missingProperty = SarifPropertyName.PhysicalLocation;
+                    jsonPointer = locationsPointer.AtIndex(i);
+                }
+                else if (location.PhysicalLocation.ArtifactLocation == null)
+                {
+                    missingProperty = SarifPropertyName.ArtifactLocation;
+                    jsonPointer = locationsPointer.AtIndex(i).AtProperty(SarifPropertyName.PhysicalLocation);
+                }
+                else if (location.PhysicalLocation.ArtifactLocation.Uri == null)
+                {
+                    missingProperty = SarifPropertyName.Uri;
+                    jsonPointer = locationsPointer.AtIndex(i).AtProperty(SarifPropertyName.PhysicalLocation).AtProperty(SarifPropertyName.ArtifactLocation);
+                }
+
+                if (missingProperty != null)
+                {
+                    Debug.Assert(jsonPointer != null);
+
+                    // {0}: The '{1}' property is absent. The GitHub Developer Security Portal will
+                    // not display a result whose location does not provide the URI of the artifact
+                    // that contains the result.
+                    LogResult(
+                        jsonPointer,
+                        nameof(RuleResources.SARIF2017_LocationsMustHaveRequired_Properties_Error_Default_Text),
+                        missingProperty);
+                }
+            }
+        }
+    }
+}

src/Sarif.Sdk.sln

@@ -110,6 +110,12 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "ValidationRules", "Validati
 		..\docs\ValidationRules\RULEID.RULEFRIENDLYNAME.cs = ..\docs\ValidationRules\RULEID.RULEFRIENDLYNAME.cs
 	EndProjectSection
 EndProject
+Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "policies", "policies", "{C2397737-B10C-42BF-B2AB-11A86817ACCD}"
+	ProjectSection(SolutionItems) = preProject
+		..\policies\github-dsp-policy.config.xml = ..\policies\github-dsp-policy.config.xml
+		..\policies\github-dsp-policy.sarif-external-properties = ..\policies\github-dsp-policy.sarif-external-properties
+	EndProjectSection
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -322,6 +328,7 @@ Global
 		{A2D43A0B-F2ED-42A5-A980-D6F4476FE455} = {7D46D6BE-C3CD-424C-9A20-245DE63A9C10}
 		{6EEAC61E-A362-4C46-A9AD-00EDB74CFCF2} = {BBDE0751-B8A3-4EA6-A3DF-3CE7BA41EA0F}
 		{81910EAA-C010-4940-90E5-BF64DE64381C} = {6EEAC61E-A362-4C46-A9AD-00EDB74CFCF2}
+		{C2397737-B10C-42BF-B2AB-11A86817ACCD} = {BBDE0751-B8A3-4EA6-A3DF-3CE7BA41EA0F}
 	EndGlobalSection
 	GlobalSection(ExtensibilityGlobals) = postSolution
 		SolutionGuid = {9EFA21E3-70A0-467C-9D1F-D5AD0DC1C1E6}

src/Sarif/Core/ReportingDescriptor.cs

@@ -9,6 +9,20 @@ namespace Microsoft.CodeAnalysis.Sarif
 {
     public partial class ReportingDescriptor
     {
+        public string Moniker
+        {
+            get
+            {
+                string moniker = this.Id;
+                if (!string.IsNullOrWhiteSpace(this.Name))
+                {
+                    moniker += $".{this.Name}";
+                }
+
+                return moniker;
+            }
+        }
+
         public string Format(string messageId, IEnumerable<string> arguments)
         {
             return string.Format(CultureInfo.CurrentCulture, this.MessageStrings[messageId].Text, arguments.ToArray());

src/Sarif/PerLanguageOption.cs

@@ -13,7 +13,7 @@ namespace Microsoft.CodeAnalysis.Sarif
     public class PerLanguageOption<T> : IOption
     {
         /// <summary>
-        /// A description of this specificoption.
+        /// A description of this specific option.
         /// </summary>
         public string Description { get; }