Merge feature branch with new SARIF validation rules (#1984)

* Users/hakohli/validation rules defaultmsgs (#1917) * remove stale rule references - 1006 and 1009 * house keeping changes for 1001 * more cleanup- remove fulldecsription private field * updates after decisions on resx naming * sarif file rule name should be shorter Co-authored-by: Harleen Kaur Kohli <erferferfg> * sarif validation rules 1002 1006 2001 (#1918) * changing rule ids only * updating rule names and message ids Co-authored-by: Harleen Kaur Kohli <erferferfg> * Validation rules: 1005 1008 1009 1010 (#1920) * rules 1011 2008 (split from original) (#1921) * rule id changed and tested * changing rule name and tested * description resx id updated * resx updated and test cases regened * final changes after splitting the rule in two. * reviews++ * fix one thing Co-authored-by: Harleen Kaur Kohli <erferferfg> * rule 1007 (combine) (#1922) Co-authored-by: Harleen Kaur Kohli <erferferfg> * validation rule 1004 (#1923) * renaming ruleid and tested * rulename changed and tested * description resx changed * merged test cases into one rule * cleanup and reordering Co-authored-by: Harleen Kaur Kohli <erferferfg> * Adding rule SARIF2005.ProvideHelpfulToolInformation (#1926) * Adding ContextRegionMustBeProperSupersetOfRegion to SARIF1008. (#1925) * Fix test break to due failure to pre-merge. (#1928) * validation rule sarif1004 (#1930) * changing file contents to follow conventions * validation rule 1004 * reviews++ * tiny thing ;) * another tiny thing! Co-authored-by: Harleen Kaur Kohli <erferferfg> * Add rules spreadsheet and document. (#1931) * Provide messages for SARIF1005. (#1934) * validation rule sarif1002 (part 1) (#1933) * formatting changes only * sub-rule: FileUrisMustNotIncludeDotDotSegments * another test case output * removing brnach comments from newly wrtiten rules. Co-authored-by: Harleen Kaur Kohli <erferferfg> * Adding rule SARIF2001.AuthorHighQualityMessages (#1929) * sarif1007 subrule: RegionStartPropertyMustBePresent (#1935) * changing file formatting per convention * adding sub-rule: RegionStartPropertyMustBePresent Co-authored-by: Harleen Kaur Kohli <erferferfg> * Update rules factoring spreadsheet. (#1936) * Update version-related comment in rules spreadsheet. (#1937) * Update coding status on spreadsheet. * Adding Rule SARIF2009 Adding tests * Update rule status spreadsheet. * code review - 1 * code review - 2 * code review - 3 * code review - 4 * code review - 5 * code review - 6 * Standardize and add messages to SARIF1001. * Provide messages for SARIF1002 (except for the RFC 8089 message). * Provide messages for SARIF1007. * Rename SARIF2005 to ProvideToolProperties. * Adding rule SARIF2004.OptimizeFileSize: EliminateLocationOnlyArtifacts (#1939) * Provide messages for SARIF2001; update code to populate arguments. * Provide message strings for SARIF1006. * Move a rule description message to the right place. * Standardize and provide messages for SARIF1009. * Add description for SARIF1009. * Reformat SARIF1005, update spreadsheet. (#1940) * Author "Principles" section. (#1941) * More about tool information. * Copy edits to "Principles" section. * More spreadsheet updates. * More spreadsheet updates. * More spreadsheet updates. * More spreadsheet updates. * More spreadsheet updates. * adding placeholders for all resource strings and rule ids. (#1943) * adding placeholders for all resource strings and rule ids. * remove unneeded using refernece Co-authored-by: Harleen Kaur Kohli <erferferfg> * Adding rule SARIF1012 (#1944) * More spreadsheet updates. * Adding Rule SARIF2006 (#1942) * Adding rule SARIF2002 (#1946) * More spreadsheet updates, a little document work. * Adding Rule SARIF2003 (#1947) * More spreadsheet updates and document work. * Adding Rule SARIF2011 (#1948) * split rule sarif2001 into multiple (#1945) * rename original rule - tested * copies of the same rule created * added test cases * cleaned up resx strings * pushing changes so far - 2 test cases fail * expected outputs * fixes for test cases Co-authored-by: Harleen Kaur Kohli <erferferfg> * More spreadsheet updates and document work. * Adding Rule SARIF2012 (#1949) * More spreadsheet updates and document work. * Add rule SARIF2004.OptimizeFileSize.EliminateIdOnlyRules (#1950) * sub-rule added * reviews answered and merge from latest faetures branch Co-authored-by: Harleen Kaur Kohli <erferferfg> * More spreadsheet updates and document work. * More spreadsheet updates and document work. * More spreadsheet updates and document work. * Adding rule SARIF2013 (#1951) * More spreadsheet updates and document work. * Updating Rule SARIF2009 and SARIF2014 (#1954) * Update spreadsheet. * sarif validation rule 2010 - provide code snippets (#1953) * rule + test cases * reviews++ * remove blank line Co-authored-by: Harleen Kaur Kohli <erferferfg> * Updating rules based on the guidelines (#1955) * More spreadsheet updates and document work. * Update spreadsheet. * Document: "high quality" => "effective" everywhere. * Document: Split up "enriched SARIF" rule. * Author messages for SARIF2008.ProvideSchema. * Remove obsolete "uriBaseId conventions" text. * Rule description for SARIF2007.ExpressPathsRelativeToRepoRoot * Fix ExpressUriBaseIdsCorrectly messages. * Fix doc errors; update spreadsheet. * user msgs verified for 1006 to 1010 (#1957) * user messages updated for 1006 to 1010 * adding period back for 1008 description Co-authored-by: Harleen Kaur Kohli <erferferfg> * Adding Rule SARIF2007 (#1958) * Bugfix null reference Rule SARIF2007 (#1959) * Update spreadsheet: last rule written! * Introduce SARIF1003 in spreadsheet. * user msgs verified for 1001 to 1005 (#1956) * usewr msgs verified for 1001 to 1005 * changing implementation for one sub-rule Co-authored-by: Harleen Kaur Kohli <erferferfg> * Fix missing cross-ref in doc. * Remove backticks from plain text message. * user messages for rules 1011, 1012, 2001, 2002. (#1960) * user messages for rules 1011, 1012, 2001, 2002. * fixing wrong message * fixed updated string and merge from features Co-authored-by: Harleen Kaur Kohli <erferferfg> * Fix 2005 messages. * user msgs verified for 2005 2008 2009 (#1961) * user msgs verified for 2005, 2008, 2009 * 2005 msgs updated * proof read 2008 & 2009 Co-authored-by: Harleen Kaur Kohli <erferferfg> * Doc update for 2005/8/9. * user msgs verified for 2014 & 2015 (#1965) * user msgs verified for 2014 & 2015 * proof read 2014 and 2015 Co-authored-by: Harleen Kaur Kohli <erferferfg> * Doc update for 2014/15. * Restoring original functionality for sub rule: UriBaseIdRequiresRelativeUri (#1967) Authored-by: Harleen Kaur Kohli * Spreadsheet update for 1004. * Update messages and code for SARIF1004. (#1968) * Fix bug in 1012. (#1969) * Provide messages for SARIF2003.ProvideVersionControlProvenance. (#1970) * Provide messages for SARIF2004.OptimizeFileSize. (#1973) * Provide messages for SARIF2006.MessagesShouldBeReachable. (#1974) * Provide messages for SARIF2007.ExpressLocationsRelativeToRepoRoot. (#1975) * Provide messages for SARIF1012.ProvideHelpUris. (#1976) * Provide messages for SARIF2013.ProvideEmbeddedFileContent. (#1977) * Fix broken functional test due to typo in message. (#1978) * Provide messages for SARIF2010.ProvideCodeSnippets. (#1979) * Provide messages for SARIF2011.ProvideContextRegion. (#1980) * Remove overactive assertion. (#1981) * Fix empty 2005 message (wrong argument order to LogResult). (#1982) * Update release history and bump minor version number. (#1983) * Update version Co-authored-by: Harleen Kaur Kohli <hakohli@microsoft.com> Co-authored-by: Eddy Nakamura <eddynaka@gmail.com> Co-authored-by: Larry Golding (Myriad Consulting Inc) <v-lgold@microsoft.com> Co-authored-by: Larry Golding <lgolding@comcast.net> Co-authored-by: Michael C. Fanning <michael.fanning@microsoft.com> Co-authored-by: Michael Fanning <mikefan@microsoft.com>
microsoft · Jul 10, 2020 · 2e39882 · 2e39882
1 parent d8d4cc7
commit 2e39882
Show file tree

Hide file tree

Showing 194 changed files with 11,177 additions and 3,795 deletions.
diff --git a/docs/Producing effective SARIF.md b/docs/Producing effective SARIF.md
diff --git a/docs/Rule factoring.xlsx b/docs/Rule factoring.xlsx
diff --git a/src/ReleaseHistory.md b/src/ReleaseHistory.md
@@ -1,6 +1,7 @@
 # SARIF Package Release History (SDK, Driver, Converters, and Multitool)
 
 ## **v2.3.1** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/2.3.1) | [Driver](https://www.nuget.org/packages/Sarif.Driver/2.3.1) | [Converters](https://www.nuget.org/packages/Sarif.Converters/2.3.1) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/2.3.1)
+* FEATURE: Revised and improved validation rules in `Sarif.Multitool`.
 * FEATURE: Properties serialization performance improved (~20% faster load when Results use Properties).
 * FEATURE: Allow result messages to be truncated for display. [#1915](https://github.com/microsoft/sarif-sdk/issues/1915)
 * BUGFIX: Rebase URI command now honors `--insert` and `--remove` arguments for injecting or eliding optional data (such as region snippets).

diff --git a/src/Sarif.Multitool/Rules/RuleId.cs b/src/Sarif.Multitool/Rules/RuleId.cs
@@ -5,23 +5,39 @@ namespace Microsoft.CodeAnalysis.Sarif.Multitool.Rules
 {
     public static class RuleId
     {
-        public const string DoNotUseFriendlyNameAsRuleId = "SARIF1001";
-        public const string UrisMustBeValid = "SARIF1003";
-        public const string HashAlgorithmsMustBeUnique = "SARIF1006";
-        public const string EndTimeMustNotBeBeforeStartTime = "SARIF1007";
-        public const string MessagesShouldEndWithPeriod = "SARIF1008";
-        public const string StepValuesMustFormOneBasedSequence = "SARIF1009";
-        public const string EndLineMustNotBeLessThanStartLine = "SARIF1012";
-        public const string EndColumnMustNotBeLessThanStartColumn = "SARIF1013";
-        public const string UriBaseIdRequiresRelativeUri = "SARIF1014";
-        public const string UriMustBeAbsolute = "SARIF1015";
-        public const string ContextRegionRequiresRegion = "SARIF1016";
-        public const string InvalidIndex = "SARIF1017";
-        public const string InvalidUriInOriginalUriBaseIds = "SARIF1018";
-        public const string RuleIdMustBePresentAndConsistent = "SARIF1019";
-        public const string ReferToFinalSchema = "SARIF1020";
+        public const string RuleIdentifiersMustBeValid = "SARIF1001";
+        public const string UrisMustBeValid = "SARIF1002";
+        public const string ExpressUriBaseIdsCorrectly = "SARIF1004";
+        public const string UriMustBeAbsolute = "SARIF1005";
+
+        public const string InvocationPropertiesMustBeConsistent = "SARIF1006";
+        public const string RegionPropertiesMustBeConsistent = "SARIF1007";
+        public const string PhysicalLocationPropertiesMustBeConsistent = "SARIF1008";
+        public const string IndexPropertiesMustBeConsistentWithArrays = "SARIF1009";
+        public const string RuleIdMustBeConsistent = "SARIF1010";
+
+        public const string ReferenceFinalSchema = "SARIF1011";
+        public const string MessageArgumentsMustBeConsistentWithRule = "SARIF1012";
+
+        public const string TerminateMessagesWithPeriod = "SARIF2001";
+        public const string ProvideMessageArguments = "SARIF2002";
+        public const string ProvideVersionControlProvenance = "SARIF2003";
+        public const string OptimizeFileSize = "SARIF2004";
+        public const string ProvideToolProperties = "SARIF2005";
+
+        public const string UrisShouldBeReachable = "SARIF2006"; 
+        public const string ExpressPathsRelativeToRepoRoot = "SARIF2007";
+        public const string ProvideSchema = "SARIF2008";
+        public const string ConsiderConventionalIdentifierValues = "SARIF2009";
+        public const string ProvideCodeSnippets = "SARIF2010";
+
+        public const string ProvideContextRegion = "SARIF2011";
+        public const string ProvideHelpUris = "SARIF2012";
+        public const string ProvideEmbeddedFileContent = "SARIF2013";
+        public const string ProvideDynamicMessageContent = "SARIF2014";
+        public const string EnquoteDynamicMessageContent = "SARIF2015";
 
         // TEMPLATE:
-        // public const string RULEFRIENDLYNAME = "RULEID";
+        // public const string RuleFriendlyName = "SARIFnnnn";
     }
 }
diff --git a/src/Sarif.Multitool/Rules/RuleResources.Designer.cs b/src/Sarif.Multitool/Rules/RuleResources.Designer.cs
diff --git a/src/Sarif.Multitool/Rules/RuleResources.resx b/src/Sarif.Multitool/Rules/RuleResources.resx
diff --git a/src/Sarif.Multitool/Rules/SARIF1001.DoNotUseFriendlyNameAsRuleId.cs b/src/Sarif.Multitool/Rules/SARIF1001.DoNotUseFriendlyNameAsRuleId.cs
diff --git a/src/Sarif.Multitool/Rules/SARIF1001.RuleIdentifiersMustBeValid.cs b/src/Sarif.Multitool/Rules/SARIF1001.RuleIdentifiersMustBeValid.cs
@@ -0,0 +1,57 @@
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System;
+using System.Collections.Generic;
+
+namespace Microsoft.CodeAnalysis.Sarif.Multitool.Rules
+{
+    public class RuleIdentifiersMustBeValid : SarifValidationSkimmerBase
+    {
+        /// <summary>
+        /// SARIF2001
+        /// </summary>
+        public override string Id => RuleId.RuleIdentifiersMustBeValid;
+
+        /// <summary>
+        /// The two identity-related properties of a SARIF rule must be consistent. The required 'id'
+        /// property must be a "stable, opaque identifier" (the SARIF specification
+        /// ([3.49.3](https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317839))
+        /// explains the reasons for this). The optional 'name' property
+        /// ([3.49.7](https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317843))
+        /// is an identifer that is understandable to an end user. Therefore if both 'id' and 'name'
+        /// are present, they must be different. If both 'name' and 'id' are opaque identifiers,
+        /// omit the 'name' property. If both 'name' and 'id' are human-readable identifiers, then
+        /// consider assigning an opaque identifier to each rule, but in the meantime, omit the 'name'
+        /// property.
+        /// </summary>
+        public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.SARIF1001_RuleIdentifiersMustBeValid_FullDescription_Text };
+
+        protected override IEnumerable<string> MessageResourceNames => new string[] {
+            nameof(RuleResources.SARIF1001_RuleIdentifiersMustBeValid_Error_Default_Text)
+        };
+
+        public override FailureLevel DefaultLevel => FailureLevel.Warning;
+
+        protected override void Analyze(ReportingDescriptor reportingDescriptor, string reportingDescriptorPointer)
+        {
+            if (reportingDescriptor.Id != null &&
+                reportingDescriptor.Name != null &&
+                reportingDescriptor.Id.Equals(reportingDescriptor.Name, StringComparison.OrdinalIgnoreCase))
+            {
+                // {0}: The rule '{1}' has a 'name' property that is identical to its 'id' property.
+                // The required 'id' property must be a "stable, opaque identifier" (the SARIF specification
+                // ([3.49.3](https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317839))
+                // explains the reasons for this). The optional 'name' property
+                // ([3.49.7](https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317843))
+                // is an identifer that is understandable to an end user. Therefore if both 'id' and
+                // 'name' are present, they must be different. If they are identical, the tool must
+                // omit the 'name' property.
+                LogResult(
+                    reportingDescriptorPointer,
+                    nameof(RuleResources.SARIF1001_RuleIdentifiersMustBeValid_Error_Default_Text),
+                    reportingDescriptor.Id);
+            }
+        }
+    }
+}
diff --git a/...titool/Rules/SARIF1003.UrisMustBeValid.cs → ...titool/Rules/SARIF1002.UrisMustBeValid.cs b/...titool/Rules/SARIF1003.UrisMustBeValid.cs → ...titool/Rules/SARIF1002.UrisMustBeValid.cs
@@ -4,31 +4,35 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+
 using Microsoft.Json.Pointer;
 
 namespace Microsoft.CodeAnalysis.Sarif.Multitool.Rules
 {
     public class UrisMustBeValid : SarifValidationSkimmerBase
     {
-        private readonly MultiformatMessageString _fullDescription = new MultiformatMessageString
-        {
-            Text = RuleResources.SARIF1003_UrisMustBeValid
-        };
-
-        public override MultiformatMessageString FullDescription => _fullDescription;
-
-        public override FailureLevel DefaultLevel => FailureLevel.Error;
-
         /// <summary>
-        /// SARIF1003
+        /// SARIF1002
         /// </summary>
         public override string Id => RuleId.UrisMustBeValid;
 
-        protected override IEnumerable<string> MessageResourceNames => new string[]
-        {
-            nameof(RuleResources.SARIF1003_Default)
+        /// <summary>
+        /// Specify a valid URI reference for every URI-valued property.
+        ///
+        /// URIs must conform to [RFC 3986](https://tools.ietf.org/html/rfc3986). In addition,
+        /// 'file' URIs must not include '..' segments. If symbolic links are present, '..'
+        /// might have different meanings on the machine that produced the log file and the
+        /// machine where an end user or a tool consumes it.
+        /// </summary>
+        public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.SARIF1002_UrisMustBeValid_FullDescription_Text };
+
+        protected override IEnumerable<string> MessageResourceNames => new string[] {
+            nameof(RuleResources.SARIF1002_UrisMustBeValid_Error_UrisMustConformToRfc3986_Text),
+            nameof(RuleResources.SARIF1002_UrisMustBeValid_Error_FileUrisMustNotIncludeDotDotSegments_Text)
         };
 
+        public override FailureLevel DefaultLevel => FailureLevel.Error;
+
         protected override void Analyze(SarifLog log, string logPointer)
         {
             AnalyzeUri(log.SchemaUri, logPointer.AtProperty(SarifPropertyName.Schema));
@@ -58,19 +62,6 @@ protected override void Analyze(ReportingDescriptor reportingDescriptor, string
             AnalyzeUri(reportingDescriptor.HelpUri, messageDescriptorPointer.AtProperty(SarifPropertyName.HelpUri));
         }
 
-        protected override void Analyze(Run run, string runPointer)
-        {
-            if (run.OriginalUriBaseIds != null)
-            {
-                string originalUriBaseIdsPointer = runPointer.AtProperty(SarifPropertyName.OriginalUriBaseIds);
-
-                foreach (string key in run.OriginalUriBaseIds.Keys)
-                {
-                    AnalyzeUri(run.OriginalUriBaseIds[key].Uri, originalUriBaseIdsPointer.AtProperty(key).AtProperty(SarifPropertyName.Uri));
-                }
-            }
-        }
-
         protected override void Analyze(ToolComponent toolComponent, string toolComponentPointer)
         {
             AnalyzeUri(toolComponent.DownloadUri, toolComponentPointer.AtProperty(SarifPropertyName.DownloadUri));
@@ -83,16 +74,32 @@ protected override void Analyze(VersionControlDetails versionControlDetails, str
 
         private void AnalyzeUri(Uri uri, string pointer)
         {
-            AnalyzeUri(uri?.OriginalString, pointer);
-        }
-
-        private void AnalyzeUri(string uri, string pointer)
-        {
-            if (uri != null)
+            string uriString = uri?.OriginalString;
+            if (uriString != null)
             {
-                if (!Uri.IsWellFormedUriString(uri, UriKind.RelativeOrAbsolute))
+                if (!Uri.IsWellFormedUriString(uriString, UriKind.RelativeOrAbsolute))
+                {
+                    // {0}: The string '{1}' is not a valid URI reference. URIs must conform to
+                    // [RFC 3986](https://tools.ietf.org/html/rfc3986).
+                    LogResult(
+                        pointer,
+                        nameof(RuleResources.SARIF1002_UrisMustBeValid_Error_UrisMustConformToRfc3986_Text),
+                        uriString);
+                }
+
+                if (uri.IsAbsoluteUri && uri.IsFile)
                 {
-                    LogResult(pointer, nameof(RuleResources.SARIF1003_Default), uri);
+                    if (uriString.Split('/').Any(x => x.Equals("..")))
+                    {
+                        // {0}: The 'file' URI '{1}' contains a '..' segment. This is dangerous because
+                        // if symbolic links are present, '..' might have different meanings on the
+                        // machine that produced the log file and the machine where an end user or
+                        // a tool consumes it.
+                        LogResult(
+                            pointer,
+                            nameof(RuleResources.SARIF1002_UrisMustBeValid_Error_FileUrisMustNotIncludeDotDotSegments_Text),
+                            uriString);
+                    }
                 }
             }
         }