Skip to content

Commit

Permalink
GHAS validation rule - Message must be flattened (#2580)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
  • Loading branch information
yongyan-gh and michaelcfanning authored Dec 28, 2022
1 parent ae13732 commit 0e842c2
Show file tree
Hide file tree
Showing 13 changed files with 264 additions and 2 deletions.
7 changes: 7 additions & 0 deletions policies/github.config.sarif-external-properties
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,13 @@
"defaultConfiguration": {
"enabled": true
}
},
{
"id": "GH1007",
"name": "ProvideFullyFormattedMessageStrings",
"defaultConfiguration": {
"enabled": true
}
}
]
}
Expand Down
3 changes: 3 additions & 0 deletions policies/github.config.xml
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,7 @@
<Properties Key='GH1006.ProvideCheckoutPath.Options'>
<Property Key='RuleEnabled' Value='Error' />
</Properties>
<Properties Key='GH1007.ProvideFullyFormattedMessageStrings.Options'>
<Property Key='RuleEnabled' Value='Error' />
</Properties>
</Properties>
3 changes: 2 additions & 1 deletion src/ReleaseHistory.md
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion src/Sarif.Driver/Sdk/Skimmer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ public Skimmer()

protected virtual IEnumerable<string> MessageResourceNames => throw new NotImplementedException();

virtual public bool EnabledByDefault => true;
protected virtual ISet<string> ConflictSkimmerSet => null;

public virtual bool EnabledByDefault => true;

public override IDictionary<string, MultiformatMessageString> MessageStrings
{
Expand Down
Original file line number Diff line number Diff line change
@@ -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
};

/// <summary>
/// GH1007
/// </summary>
public override string Id => RuleId.ProvideFullyFormattedMessageStrings;

public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.GH1007_ProvideFullyFormattedMessageStrings_FullDescription_Text };

protected override IEnumerable<string> 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));
}
}
}
}
1 change: 1 addition & 0 deletions src/Sarif.Multitool.Library/Rules/RuleId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
18 changes: 18 additions & 0 deletions src/Sarif.Multitool.Library/Rules/RuleResources.Designer.cs

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

6 changes: 6 additions & 0 deletions src/Sarif.Multitool.Library/Rules/RuleResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -450,4 +450,10 @@ Semantics: Assuming the reader of the log file (an end user or another tool) has
<data name="SARIF2016_FileUrisShouldBeRelative_Note_ShouldNotContainBackSlash_Text" xml:space="preserve">
<value>{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.</value>
</data>
<data name="GH1007_ProvideFullyFormattedMessageStrings_Error_Default_Text" xml:space="preserve">
<value>{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.</value>
</data>
<data name="GH1007_ProvideFullyFormattedMessageStrings_FullDescription_Text" xml:space="preserve">
<value>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.</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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"
}
]
}
Original file line number Diff line number Diff line change
@@ -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"
}
]
}
Original file line number Diff line number Diff line change
@@ -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"
}
]
}
Original file line number Diff line number Diff line change
@@ -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"
}
]
}

0 comments on commit 0e842c2

Please sign in to comment.