From 0e842c2f36cafa4872359a8ef133bd83a2ef7d68 Mon Sep 17 00:00:00 2001 From: Yong Yan Date: Tue, 27 Dec 2022 16:01:14 -0800 Subject: [PATCH] GHAS validation rule - Message must be flattened (#2580) * Add a GitHub validation rule. * Fix tests * update rule name to FlatternResultMessage * Fix CodeQL warning * Update release history * Rename to ProvideFullyFormattedMessageStrings * rename rule to ProvideFullyFormattedMessageStrings Co-authored-by: Michael C. Fanning --- .../github.config.sarif-external-properties | 7 ++ policies/github.config.xml | 3 + src/ReleaseHistory.md | 3 +- src/Sarif.Driver/Sdk/Skimmer.cs | 4 +- ...007.ProvideFullyFormattedMessageStrings.cs | 44 ++++++++++++ src/Sarif.Multitool.Library/Rules/RuleId.cs | 1 + .../Rules/RuleResources.Designer.cs | 18 +++++ .../Rules/RuleResources.resx | 6 ++ .../Multitool/ValidateCommandTests.cs | 8 +++ ...FullyFormattedMessageStrings_Invalid.sarif | 72 +++++++++++++++++++ ...deFullyFormattedMessageStrings_Valid.sarif | 28 ++++++++ ...FullyFormattedMessageStrings_Invalid.sarif | 41 +++++++++++ ...deFullyFormattedMessageStrings_Valid.sarif | 31 ++++++++ 13 files changed, 264 insertions(+), 2 deletions(-) create mode 100644 src/Sarif.Multitool.Library/Rules/GH1007.ProvideFullyFormattedMessageStrings.cs create mode 100644 src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/ExpectedOutputs/GH1007.ProvideFullyFormattedMessageStrings_Invalid.sarif create mode 100644 src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/ExpectedOutputs/GH1007.ProvideFullyFormattedMessageStrings_Valid.sarif create mode 100644 src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/GH1007.ProvideFullyFormattedMessageStrings_Invalid.sarif create mode 100644 src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/GH1007.ProvideFullyFormattedMessageStrings_Valid.sarif diff --git a/policies/github.config.sarif-external-properties b/policies/github.config.sarif-external-properties index 0bff8f15f..2ae6fbf8d 100644 --- a/policies/github.config.sarif-external-properties +++ b/policies/github.config.sarif-external-properties @@ -53,6 +53,13 @@ "defaultConfiguration": { "enabled": true } + }, + { + "id": "GH1007", + "name": "ProvideFullyFormattedMessageStrings", + "defaultConfiguration": { + "enabled": true + } } ] } diff --git a/policies/github.config.xml b/policies/github.config.xml index 9c38704f5..f29db5a9b 100644 --- a/policies/github.config.xml +++ b/policies/github.config.xml @@ -30,4 +30,7 @@ + + + \ No newline at end of file diff --git a/src/ReleaseHistory.md b/src/ReleaseHistory.md index 52ee99dd0..38fd2b818 100644 --- a/src/ReleaseHistory.md +++ b/src/ReleaseHistory.md @@ -1,6 +1,6 @@ # SARIF Package Release History (SDK, Driver, Converters, and Multitool) -## **v3.1.0** (UNRELEASED) +## **v3.2.0** (UNRELEASED) * BREAKING: For `Guid` properties defined in SARIF spec, updated Json schema to use `uuid`, and updated C# object model to use `Guid?` instead of `string`. [#2555](https://github.com/microsoft/sarif-sdk/pull/2555) * FEATURE: Provide mechanism to populate `SarifLogger` with a `FileRegionsCache` instance. @@ -12,6 +12,7 @@ * BUGFIX: Eliminate `IOException` and `DirectoryNotFoundException` exceptions thrown by `merge` command when splitting by rule (due to invalid file characters in rule ids). [#2513](https://github.com/microsoft/sarif-sdk/pull/2513) * BUGFIX: Fix classes inside NotYetAutoGenerated folder missing `virtual` keyword for public methods and properties, by regenerate and manually sync the changes. [#2537](https://github.com/microsoft/sarif-sdk/pull/2537) * FEATURE: Allow external set of `MaxFileSizeInKilobytes`, which will allow SDK users to change the value. (Default value is 1024) [#2578](https://github.com/microsoft/sarif-sdk/pull/2578) +* FEATURE: Add a Github validation rule `GH1007`, which requires flattened result message so GHAS code scanning can ingest the log. [#2580](https://github.com/microsoft/sarif-sdk/issues/2580) * BUGFIX: MSBuild Converter now accepts case insensitive keywords and supports PackageValidator msbuild log output. [#2579](https://github.com/microsoft/sarif-sdk/pull/2579) ## **v3.1.0** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/3.1.0) | [Driver](https://www.nuget.org/packages/Sarif.Driver/3.1.0) | [Converters](https://www.nuget.org/packages/Sarif.Converters/3.1.0) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/3.1.0) | [Multitool Library](https://www.nuget.org/packages/Sarif.Multitool.Library/3.1.0) diff --git a/src/Sarif.Driver/Sdk/Skimmer.cs b/src/Sarif.Driver/Sdk/Skimmer.cs index 2d8709842..471f19261 100644 --- a/src/Sarif.Driver/Sdk/Skimmer.cs +++ b/src/Sarif.Driver/Sdk/Skimmer.cs @@ -25,7 +25,9 @@ public Skimmer() protected virtual IEnumerable MessageResourceNames => throw new NotImplementedException(); - virtual public bool EnabledByDefault => true; + protected virtual ISet ConflictSkimmerSet => null; + + public virtual bool EnabledByDefault => true; public override IDictionary MessageStrings { diff --git a/src/Sarif.Multitool.Library/Rules/GH1007.ProvideFullyFormattedMessageStrings.cs b/src/Sarif.Multitool.Library/Rules/GH1007.ProvideFullyFormattedMessageStrings.cs new file mode 100644 index 000000000..cec628cf0 --- /dev/null +++ b/src/Sarif.Multitool.Library/Rules/GH1007.ProvideFullyFormattedMessageStrings.cs @@ -0,0 +1,44 @@ +// 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 Microsoft.Json.Pointer; + +namespace Microsoft.CodeAnalysis.Sarif.Multitool.Rules +{ + public class ProvideFullyFormattedMessageStrings : SarifValidationSkimmerBase + { + public override ReportingConfiguration DefaultConfiguration => new ReportingConfiguration + { + Level = FailureLevel.Error, + Enabled = this.EnabledByDefault + }; + + /// + /// GH1007 + /// + public override string Id => RuleId.ProvideFullyFormattedMessageStrings; + + public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.GH1007_ProvideFullyFormattedMessageStrings_FullDescription_Text }; + + protected override IEnumerable MessageResourceNames => new string[] { + nameof(RuleResources.GH1007_ProvideFullyFormattedMessageStrings_Error_Default_Text) + }; + + public override bool EnabledByDefault => false; + + protected override void Analyze(Result result, string resultPointer) + { + if (string.IsNullOrEmpty(result.Message.Text)) + { + // {0}: The 'text' property of this result message is absent. GitHub Advanced Security code + // scanning will reject this file because it does not support the argumented message now. + // Try to populate the flattened message text in 'message.text' property. + LogResult( + resultPointer.AtProperty(SarifPropertyName.Message), + nameof(RuleResources.GH1007_ProvideFullyFormattedMessageStrings_Error_Default_Text)); + } + } + } +} diff --git a/src/Sarif.Multitool.Library/Rules/RuleId.cs b/src/Sarif.Multitool.Library/Rules/RuleId.cs index 3ce3efc7a..208ae56ac 100644 --- a/src/Sarif.Multitool.Library/Rules/RuleId.cs +++ b/src/Sarif.Multitool.Library/Rules/RuleId.cs @@ -47,6 +47,7 @@ public static class RuleId public const string ReviewArraysThatExceedConfigurableDefaults = "GH1004"; public const string LocationsMustBeRelativeUrisOrFilePaths = "GH1005"; public const string ProvideCheckoutPath = "GH1006"; + public const string ProvideFullyFormattedMessageStrings = "GH1007"; // TEMPLATE: // public const string RuleFriendlyName = "SARIFnnnn"; diff --git a/src/Sarif.Multitool.Library/Rules/RuleResources.Designer.cs b/src/Sarif.Multitool.Library/Rules/RuleResources.Designer.cs index fe5f7759f..4d4abff4f 100644 --- a/src/Sarif.Multitool.Library/Rules/RuleResources.Designer.cs +++ b/src/Sarif.Multitool.Library/Rules/RuleResources.Designer.cs @@ -199,6 +199,24 @@ internal static string GH1006_ProvideCheckoutPath_FullDescription_Text { } } + /// + /// Looks up a localized string similar to {0}: The 'text' property of this result message is absent. GitHub Advanced Security code scanning will reject this file because it does not support the argumented message now. Try to provide fully formatted text in 'message.text' property.. + /// + internal static string GH1007_ProvideFullyFormattedMessageStrings_Error_Default_Text { + get { + return ResourceManager.GetString("GH1007_ProvideFullyFormattedMessageStrings_Error_Default_Text", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to GitHub Advanced Security code scanning will reject a SARIF file that express result messages with 'message.id' and 'message.arguments' but without the 'message.text' property since the arugmented message format is not supported yet. Please provide fully formatted text in 'message.text' property.. + /// + internal static string GH1007_ProvideFullyFormattedMessageStrings_FullDescription_Text { + get { + return ResourceManager.GetString("GH1007_ProvideFullyFormattedMessageStrings_FullDescription_Text", resourceCulture); + } + } + /// /// Looks up a localized string similar to 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 identifier that is understandable to an end user. Therefore if both 'id' and 'name' are pre [rest of string was truncated]";. /// diff --git a/src/Sarif.Multitool.Library/Rules/RuleResources.resx b/src/Sarif.Multitool.Library/Rules/RuleResources.resx index efffa3c69..05573f763 100644 --- a/src/Sarif.Multitool.Library/Rules/RuleResources.resx +++ b/src/Sarif.Multitool.Library/Rules/RuleResources.resx @@ -450,4 +450,10 @@ Semantics: Assuming the reader of the log file (an end user or another tool) has {0}: The relative file URL '{1}' contains one or more backslashes, which will be preserved when concatenating to an absolute URL. This can result in inconsistent representations, compared to URLs created from an absolute file path, which may be regarded as not equivalent. Replace all backslashes with forward slashes. + + {0}: The 'text' property of this result message is absent. GitHub Advanced Security code scanning will reject this file because it does not support the argumented message now. Try to provide fully formatted text in 'message.text' property. + + + GitHub Advanced Security code scanning will reject a SARIF file that express result messages with 'message.id' and 'message.arguments' but without the 'message.text' property since the arugmented message format is not supported yet. Please provide fully formatted text in 'message.text' property. + \ No newline at end of file diff --git a/src/Test.FunctionalTests.Sarif/Multitool/ValidateCommandTests.cs b/src/Test.FunctionalTests.Sarif/Multitool/ValidateCommandTests.cs index 00d3fcbd2..96f581266 100644 --- a/src/Test.FunctionalTests.Sarif/Multitool/ValidateCommandTests.cs +++ b/src/Test.FunctionalTests.Sarif/Multitool/ValidateCommandTests.cs @@ -411,6 +411,14 @@ public void GH1006_ProvideCheckoutPath_Valid() public void GH1006_ProvideCheckoutPath_Invalid() => RunInvalidTestForRule(RuleId.ProvideCheckoutPath); + [Fact] + public void GH1007_ProvideFullyFormattedMessageStrings_Valid() + => RunValidTestForRule(RuleId.ProvideFullyFormattedMessageStrings); + + [Fact] + public void GH1007_ProvideFullyFormattedMessageStrings_Invalid() + => RunInvalidTestForRule(RuleId.ProvideFullyFormattedMessageStrings); + private void RunArrayLimitTest(string testFileNameSuffix) { // Some of the actual limits are impractically large for testing purposes, diff --git a/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/ExpectedOutputs/GH1007.ProvideFullyFormattedMessageStrings_Invalid.sarif b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/ExpectedOutputs/GH1007.ProvideFullyFormattedMessageStrings_Invalid.sarif new file mode 100644 index 000000000..4b7f5c794 --- /dev/null +++ b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/ExpectedOutputs/GH1007.ProvideFullyFormattedMessageStrings_Invalid.sarif @@ -0,0 +1,72 @@ +{ + "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.6.json", + "version": "2.1.0", + "runs": [ + { + "tool": { + "driver": { + "name": "SARIF Functional Testing", + "rules": [ + { + "id": "GH1007", + "name": "ProvideFullyFormattedMessageStrings", + "fullDescription": { + "text": "GitHub Advanced Security code scanning will reject a SARIF file that express result messages with 'message.id' and 'message.arguments' but without the 'message.text' property since the arugmented message format is not supported yet. Please provide fully formatted text in 'message.text' property." + }, + "messageStrings": { + "Error_Default": { + "text": "{0}: The 'text' property of this result message is absent. GitHub Advanced Security code scanning will reject this file because it does not support the argumented message now. Try to provide fully formatted text in 'message.text' property." + } + }, + "defaultConfiguration": { + "enabled": false, + "level": "error" + }, + "helpUri": "http://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html" + } + ] + } + }, + "invocations": [ + { + "executionSuccessful": true + } + ], + "artifacts": [ + { + "location": { + "uri": "FunctionalTestOutput.ValidateCommand/GH1007.ProvideFullyFormattedMessageStrings_Invalid.sarif", + "uriBaseId": "TEST_DIR" + } + } + ], + "results": [ + { + "ruleId": "GH1007", + "ruleIndex": 0, + "level": "error", + "message": { + "id": "Error_Default", + "arguments": [ + "runs[0].results[0].message" + ] + }, + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "index": 0 + }, + "region": { + "startLine": 29, + "startColumn": 22 + } + } + } + ] + } + ], + "columnKind": "utf16CodeUnits" + } + ] +} \ No newline at end of file diff --git a/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/ExpectedOutputs/GH1007.ProvideFullyFormattedMessageStrings_Valid.sarif b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/ExpectedOutputs/GH1007.ProvideFullyFormattedMessageStrings_Valid.sarif new file mode 100644 index 000000000..5d9e571f8 --- /dev/null +++ b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/ExpectedOutputs/GH1007.ProvideFullyFormattedMessageStrings_Valid.sarif @@ -0,0 +1,28 @@ +{ + "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.6.json", + "version": "2.1.0", + "runs": [ + { + "tool": { + "driver": { + "name": "SARIF Functional Testing" + } + }, + "invocations": [ + { + "executionSuccessful": true + } + ], + "artifacts": [ + { + "location": { + "uri": "FunctionalTestOutput.ValidateCommand/GH1007.ProvideFullyFormattedMessageStrings_Valid.sarif", + "uriBaseId": "TEST_DIR" + } + } + ], + "results": [], + "columnKind": "utf16CodeUnits" + } + ] +} \ No newline at end of file diff --git a/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/GH1007.ProvideFullyFormattedMessageStrings_Invalid.sarif b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/GH1007.ProvideFullyFormattedMessageStrings_Invalid.sarif new file mode 100644 index 000000000..fce727ca7 --- /dev/null +++ b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/GH1007.ProvideFullyFormattedMessageStrings_Invalid.sarif @@ -0,0 +1,41 @@ +{ + "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.6.json", + "version": "2.1.0", + "runs": [ + { + "tool": { + "driver": { + "name": "SARIF Functional Testing", + "version": "1.2.3", + "rules": [ + { + "id": "TEST1001", + "fullDescription": { + "text": "Argumented message." + }, + "messageStrings": { + "DoesExist": { + "text": "'{0}' is an apparent access token of '{1}'." + } + } + } + ] + } + }, + "results": [ + { + "ruleId": "TEST1001", + "ruleIndex": 0, + "message": { + "id": "DoesExist", + "arguments": [ + "123456789", + "Alibaba Cloud service" + ] + } + } + ], + "columnKind": "utf16CodeUnits" + } + ] +} \ No newline at end of file diff --git a/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/GH1007.ProvideFullyFormattedMessageStrings_Valid.sarif b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/GH1007.ProvideFullyFormattedMessageStrings_Valid.sarif new file mode 100644 index 000000000..a8ee22733 --- /dev/null +++ b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/GH1007.ProvideFullyFormattedMessageStrings_Valid.sarif @@ -0,0 +1,31 @@ +{ + "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.6.json", + "version": "2.1.0", + "runs": [ + { + "tool": { + "driver": { + "name": "SARIF Functional Testing", + "version": "1.2.3", + "rules": [ + { + "id": "TEST1001", + "fullDescription": { + "text": "Argumented message." + } + } + ] + } + }, + "results": [ + { + "ruleId": "TEST1001", + "message": { + "text": "'123456789' is an apparent access token of 'Alibaba Cloud service'." + } + } + ], + "columnKind": "utf16CodeUnits" + } + ] +} \ No newline at end of file