From 43b5bd911a569eb8545adc145cbe415ac21261e8 Mon Sep 17 00:00:00 2001 From: Larry Golding Date: Wed, 23 Sep 2020 14:10:54 -0700 Subject: [PATCH 1/5] Remove GH1007.ProvideRequiredRelatedLocationProperties. --- docs/ValidationRules.md | 16 +--- .../github.config.sarif-external-properties | 7 -- policies/github.config.xml | 3 - ...rovideRequiredRelatedLocationProperties.cs | 57 -------------- src/Sarif.Multitool.Library/Rules/RuleId.cs | 1 - .../Rules/RuleResources.Designer.cs | 18 ----- .../Rules/RuleResources.resx | 6 -- .../Multitool/ValidateCommandTests.cs | 8 -- ...redRelatedLocationProperties_Invalid.sarif | 75 ------------------- ...uiredRelatedLocationProperties_Valid.sarif | 28 ------- ...redRelatedLocationProperties_Invalid.sarif | 41 ---------- ...uiredRelatedLocationProperties_Valid.sarif | 44 ----------- 12 files changed, 1 insertion(+), 303 deletions(-) delete mode 100644 src/Sarif.Multitool.Library/Rules/GH1007.ProvideRequiredRelatedLocationProperties.cs delete mode 100644 src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/ExpectedOutputs/GH1007.ProvideRequiredRelatedLocationProperties_Invalid.sarif delete mode 100644 src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/ExpectedOutputs/GH1007.ProvideRequiredRelatedLocationProperties_Valid.sarif delete mode 100644 src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/GH1007.ProvideRequiredRelatedLocationProperties_Invalid.sarif delete mode 100644 src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/GH1007.ProvideRequiredRelatedLocationProperties_Valid.sarif diff --git a/docs/ValidationRules.md b/docs/ValidationRules.md index 2b5bac5f2..a3aa46d9e 100644 --- a/docs/ValidationRules.md +++ b/docs/ValidationRules.md @@ -86,7 +86,7 @@ GitHub Advanced Security code scanning only displays results whose locations are ### Description -GitHub Advanced Security code scanning will reject a SARIF file that expresses result locations as absolute 'file' scheme URIs unless it can determine the URI of the repository root (which GitHub refers to as the "checkout path"). There are three ways to address this issue. +GitHub Advanced Security code scanning will reject a SARIF file that expresses result locations as absolute 'file' scheme URIs unless GitHub can determine the URI of the repository root (which GitHub refers to as the "checkout path"). There are three ways to address this issue. 1. Recommended: Express all result locations as relative URI references with respect to the checkout path. @@ -102,20 +102,6 @@ GitHub Advanced Security code scanning will reject a SARIF file that expresses r --- -## Rule `GH1007.ProvideRequiredRelatedLocationProperties` - -### Description - -GitHub Advanced Security code scanning will reject a SARIF file that includes a "related location" with no 'message' property. This is a bug in code scanning. You can set 'message' to an empty string if you don't have anything else to say about the location. - -### Messages - -#### `Default`: Error - -{0}: This related location does not have a 'message' property, so GitHub Advanced Security code scanning will reject the entire log file. This is a bug in code scanning. You can set 'message' to an empty string if you don't have anything else to say about the location. - ---- - ## Rule `SARIF1001.RuleIdentifiersMustBeValid` ### Description diff --git a/policies/github.config.sarif-external-properties b/policies/github.config.sarif-external-properties index 9ef7e0f3d..298c12b12 100644 --- a/policies/github.config.sarif-external-properties +++ b/policies/github.config.sarif-external-properties @@ -53,13 +53,6 @@ "defaultConfiguration": { "enabled": true } - }, - { - "id": "GH1007", - "name": "ProvideRequiredRelatedLocationProperties", - "defaultConfiguration": { - "enabled": true - } } ] } diff --git a/policies/github.config.xml b/policies/github.config.xml index f61e0a2b9..4c5abb65b 100644 --- a/policies/github.config.xml +++ b/policies/github.config.xml @@ -69,7 +69,4 @@ - - - \ No newline at end of file diff --git a/src/Sarif.Multitool.Library/Rules/GH1007.ProvideRequiredRelatedLocationProperties.cs b/src/Sarif.Multitool.Library/Rules/GH1007.ProvideRequiredRelatedLocationProperties.cs deleted file mode 100644 index f7b2849f1..000000000 --- a/src/Sarif.Multitool.Library/Rules/GH1007.ProvideRequiredRelatedLocationProperties.cs +++ /dev/null @@ -1,57 +0,0 @@ -// 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.Linq; - -using Microsoft.Json.Pointer; - -namespace Microsoft.CodeAnalysis.Sarif.Multitool.Rules -{ - public class ProvideRequiredRelatedLocationProperties : SarifValidationSkimmerBase - { - /// - /// GH1007 - /// - public override string Id => RuleId.ProvideRequiredRelatedLocationProperties; - - // GitHub Advanced Security code scanning will reject a SARIF file that includes a - // "related location" with no 'message' property. This is a bug in GitHub. You can set - // 'message' to an empty string if you don't have anything else to say about the location. - public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.GH1007_ProvideRequiredRelatedLocationProperties_FullDescription_Text }; - - protected override IEnumerable MessageResourceNames => new string[] { - nameof(RuleResources.GH1007_ProvideRequiredRelatedLocationProperties_Error_Default_Text) - }; - - public override FailureLevel DefaultLevel => FailureLevel.Error; - - public override bool EnabledByDefault => false; - - protected override void Analyze(Result result, string resultPointer) - { - if (result.RelatedLocations?.Any() == true) - { - string relatedLocationsPointer = resultPointer.AtProperty(SarifPropertyName.RelatedLocations); - for (int i = 0; i < result.RelatedLocations.Count; i++) - { - ValidateRelatedLocation(result.RelatedLocations[i], relatedLocationsPointer.AtIndex(i)); - } - } - } - - private void ValidateRelatedLocation(Location relatedLocation, string relatedLocationPointer) - { - if (relatedLocation.Message == null) - { - // {0}: This related location does not have a 'message' property, so GitHub - // Advanced Security code scanning will reject the entire log file. This is a bug - // in GitHub. You can set 'message' to an empty string if you don't have anything - // else to say about the location. - LogResult( - relatedLocationPointer, - nameof(RuleResources.GH1007_ProvideRequiredRelatedLocationProperties_Error_Default_Text)); - } - } - } -} diff --git a/src/Sarif.Multitool.Library/Rules/RuleId.cs b/src/Sarif.Multitool.Library/Rules/RuleId.cs index 282d7283b..4a4001ccc 100644 --- a/src/Sarif.Multitool.Library/Rules/RuleId.cs +++ b/src/Sarif.Multitool.Library/Rules/RuleId.cs @@ -47,7 +47,6 @@ public static class RuleId public const string ReviewArraysThatExceedConfigurableDefaults = "GH1004"; public const string LocationsMustBeRelativeUrisOrFilePaths = "GH1005"; public const string ProvideCheckoutPath = "GH1006"; - public const string ProvideRequiredRelatedLocationProperties = "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 58dc91f63..d7482e078 100644 --- a/src/Sarif.Multitool.Library/Rules/RuleResources.Designer.cs +++ b/src/Sarif.Multitool.Library/Rules/RuleResources.Designer.cs @@ -199,24 +199,6 @@ internal static string GH1006_ProvideCheckoutPath_FullDescription_Text { } } - /// - /// Looks up a localized string similar to {0}: This related location does not have a 'message' property, so GitHub Advanced Security code scanning will reject the entire log file. This is a bug in GitHub. You can set 'message' to an empty string if you don't have anything else to say about the location.. - /// - internal static string GH1007_ProvideRequiredRelatedLocationProperties_Error_Default_Text { - get { - return ResourceManager.GetString("GH1007_ProvideRequiredRelatedLocationProperties_Error_Default_Text", resourceCulture); - } - } - - /// - /// Looks up a localized string similar to GitHub Advanced Security code scanning will reject a SARIF file that includes a "related location" with no 'message' property. This is a bug in GitHub. You can set 'message' to an empty string if you don't have anything else to say about the location.. - /// - internal static string GH1007_ProvideRequiredRelatedLocationProperties_FullDescription_Text { - get { - return ResourceManager.GetString("GH1007_ProvideRequiredRelatedLocationProperties_FullDescription_Text", resourceCulture); - } - } - /// /// Looks up a localized string similar to {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 [rest of string was truncated]";. /// diff --git a/src/Sarif.Multitool.Library/Rules/RuleResources.resx b/src/Sarif.Multitool.Library/Rules/RuleResources.resx index 2cac2c25f..5df9e7bcb 100644 --- a/src/Sarif.Multitool.Library/Rules/RuleResources.resx +++ b/src/Sarif.Multitool.Library/Rules/RuleResources.resx @@ -446,12 +446,6 @@ Semantics: Assuming the reader of the log file (an end user or another tool) has {0}: This result location is expressed as an absolute 'file' URI. GitHub Advanced Security code scanning will reject this file because it cannot determine the location of the repository root (which it refers to as the "checkout path"). Either express result locations as relative URI references with respect to the checkout path, place the checkout path in 'invocations[].workingDirectory', or place the checkout path in a configuration file at the root of the repository. - - {0}: This related location does not have a 'message' property, so GitHub Advanced Security code scanning will reject the entire log file. This is a bug in GitHub. You can set 'message' to an empty string if you don't have anything else to say about the location. - - - GitHub Advanced Security code scanning will reject a SARIF file that includes a "related location" with no 'message' property. This is a bug in GitHub. You can set 'message' to an empty string if you don't have anything else to say about the location. - {0}: The tool '{1}' does not provide 'informationUri'. This property helps the developer responsible for addessing a result by providing a way to learn more about the tool. diff --git a/src/Test.FunctionalTests.Sarif/Multitool/ValidateCommandTests.cs b/src/Test.FunctionalTests.Sarif/Multitool/ValidateCommandTests.cs index e4c2f3add..ef4cf34cb 100644 --- a/src/Test.FunctionalTests.Sarif/Multitool/ValidateCommandTests.cs +++ b/src/Test.FunctionalTests.Sarif/Multitool/ValidateCommandTests.cs @@ -379,14 +379,6 @@ public void GH1006_ProvideCheckoutPath_Valid() public void GH1006_ProvideCheckoutPath_Invalid() => RunInvalidTestForRule(RuleId.ProvideCheckoutPath); - [Fact] - public void GH1007_ProvideRequiredRelatedLocationProperties_Valid() - => RunValidTestForRule(RuleId.ProvideRequiredRelatedLocationProperties); - - [Fact] - public void GH1007_ProvideRequiredRelatedLocationProperties_Invalid() - => RunInvalidTestForRule(RuleId.ProvideRequiredRelatedLocationProperties); - 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.ProvideRequiredRelatedLocationProperties_Invalid.sarif b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/ExpectedOutputs/GH1007.ProvideRequiredRelatedLocationProperties_Invalid.sarif deleted file mode 100644 index 992970600..000000000 --- a/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/ExpectedOutputs/GH1007.ProvideRequiredRelatedLocationProperties_Invalid.sarif +++ /dev/null @@ -1,75 +0,0 @@ -{ - "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json", - "version": "2.1.0", - "runs": [ - { - "tool": { - "driver": { - "name": "SARIF Functional Testing", - "rules": [ - { - "id": "GH1007", - "name": "ProvideRequiredRelatedLocationProperties", - "shortDescription": { - "text": "GitHub Advanced Security code scanning will reject a SARIF file that includes a \"related location\" with no 'message' property." - }, - "fullDescription": { - "text": "GitHub Advanced Security code scanning will reject a SARIF file that includes a \"related location\" with no 'message' property. This is a bug in GitHub. You can set 'message' to an empty string if you don't have anything else to say about the location." - }, - "messageStrings": { - "Error_Default": { - "text": "{0}: This related location does not have a 'message' property, so GitHub Advanced Security code scanning will reject the entire log file. This is a bug in GitHub. You can set 'message' to an empty string if you don't have anything else to say about the location." - } - }, - "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/Inputs.GH1007.ProvideRequiredRelatedLocationProperties_Invalid.sarif", - "uriBaseId": "TEST_DIR" - } - } - ], - "results": [ - { - "ruleId": "GH1007", - "ruleIndex": 0, - "level": "error", - "message": { - "id": "Error_Default", - "arguments": [ - "runs[0].results[0].relatedLocations[0]" - ] - }, - "locations": [ - { - "physicalLocation": { - "artifactLocation": { - "index": 0 - }, - "region": { - "startLine": 28, - "startColumn": 13 - } - } - } - ] - } - ], - "columnKind": "utf16CodeUnits" - } - ] -} \ No newline at end of file diff --git a/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/ExpectedOutputs/GH1007.ProvideRequiredRelatedLocationProperties_Valid.sarif b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/ExpectedOutputs/GH1007.ProvideRequiredRelatedLocationProperties_Valid.sarif deleted file mode 100644 index 902281c86..000000000 --- a/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/ExpectedOutputs/GH1007.ProvideRequiredRelatedLocationProperties_Valid.sarif +++ /dev/null @@ -1,28 +0,0 @@ -{ - "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json", - "version": "2.1.0", - "runs": [ - { - "tool": { - "driver": { - "name": "SARIF Functional Testing" - } - }, - "invocations": [ - { - "executionSuccessful": true - } - ], - "artifacts": [ - { - "location": { - "uri": "FunctionalTestOutput.ValidateCommand/Inputs.GH1007.ProvideRequiredRelatedLocationProperties_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.ProvideRequiredRelatedLocationProperties_Invalid.sarif b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/GH1007.ProvideRequiredRelatedLocationProperties_Invalid.sarif deleted file mode 100644 index 32b6622f9..000000000 --- a/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/GH1007.ProvideRequiredRelatedLocationProperties_Invalid.sarif +++ /dev/null @@ -1,41 +0,0 @@ -{ - "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json", - "version": "2.1.0", - "runs": [ - { - "tool": { - "driver": { - "name": "Sarif Functional Testing", - "version": "1.0" - } - }, - "results": [ - { - "ruleId": "TEST1001", - "message": { - "text": "Message." - }, - "locations": [ - { - "physicalLocation": { - "artifactLocation": { - "uri": "file:///C:/code/file.cs" - } - } - } - ], - "relatedLocations": [ - { - "physicalLocation": { - "artifactLocation": { - "uri": "file:///C:/code/file.cs" - } - } - } - ] - } - ], - "columnKind": "utf16CodeUnits" - } - ] -} \ No newline at end of file diff --git a/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/GH1007.ProvideRequiredRelatedLocationProperties_Valid.sarif b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/GH1007.ProvideRequiredRelatedLocationProperties_Valid.sarif deleted file mode 100644 index 36cddab98..000000000 --- a/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/GH1007.ProvideRequiredRelatedLocationProperties_Valid.sarif +++ /dev/null @@ -1,44 +0,0 @@ -{ - "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json", - "version": "2.1.0", - "runs": [ - { - "tool": { - "driver": { - "name": "Sarif Functional Testing", - "version": "1.0" - } - }, - "results": [ - { - "ruleId": "TEST1001", - "message": { - "text": "Message." - }, - "locations": [ - { - "physicalLocation": { - "artifactLocation": { - "uri": "file:///C:/code/file.cs" - } - } - } - ], - "relatedLocations": [ - { - "physicalLocation": { - "artifactLocation": { - "uri": "file:///C:/code/file.cs" - } - }, - "message": { - "text": "" - } - } - ] - } - ], - "columnKind": "utf16CodeUnits" - } - ] -} \ No newline at end of file From 721ec6fdb39ed46945aeb6291fbd98bf72e44590 Mon Sep 17 00:00:00 2001 From: Larry Golding Date: Wed, 23 Sep 2020 14:12:17 -0700 Subject: [PATCH 2/5] Rename file GitHubDspIngestionVisitor.cs to GitHubIngestionVisitor.cs (class already has correct name). --- .../{GitHubDspIngestionVisitor.cs => GitHubIngestionVisitor.cs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/Sarif/Visitors/{GitHubDspIngestionVisitor.cs => GitHubIngestionVisitor.cs} (100%) diff --git a/src/Sarif/Visitors/GitHubDspIngestionVisitor.cs b/src/Sarif/Visitors/GitHubIngestionVisitor.cs similarity index 100% rename from src/Sarif/Visitors/GitHubDspIngestionVisitor.cs rename to src/Sarif/Visitors/GitHubIngestionVisitor.cs From ba6777a765e0ff3612ad8b5f771e77a6be5615f5 Mon Sep 17 00:00:00 2001 From: Larry Golding Date: Wed, 23 Sep 2020 14:17:19 -0700 Subject: [PATCH 3/5] Don't insert placedholder related locations message. --- src/Sarif/Visitors/GitHubIngestionVisitor.cs | 19 ------- .../Test.UnitTests.Sarif.csproj | 56 +++++++++---------- .../ExpectedOutputs/Fingerprints.sarif | 0 .../ExpectedOutputs/NonErrorResults.sarif | 0 .../RelatedLocationMessage.sarif | 0 .../ExpectedOutputs/ThreadFlowLocations.sarif | 0 .../ExpectedOutputs/TooManyResults.sarif | 0 .../ExpectedOutputs/WithArtifacts.sarif | 0 .../ExpectedOutputs/WithInvocation.sarif | 0 .../Inputs/Fingerprints.sarif | 0 .../Inputs/NonErrorResults.sarif | 0 .../Inputs}/RelatedLocationMessage.sarif | 3 - .../Inputs/ThreadFlowLocations.sarif | 0 .../Inputs/TooManyResults.sarif | 0 .../Inputs/WithArtifacts.sarif | 0 .../Inputs/WithInvocation.sarif | 0 ...ests.cs => GitHubIngestionVisitorTests.cs} | 4 +- 17 files changed, 30 insertions(+), 52 deletions(-) rename src/Test.UnitTests.Sarif/TestData/{GitHubDspIngestionVisitor => GitHubIngestionVisitor}/ExpectedOutputs/Fingerprints.sarif (100%) rename src/Test.UnitTests.Sarif/TestData/{GitHubDspIngestionVisitor => GitHubIngestionVisitor}/ExpectedOutputs/NonErrorResults.sarif (100%) rename src/Test.UnitTests.Sarif/TestData/{GitHubDspIngestionVisitor/Inputs => GitHubIngestionVisitor/ExpectedOutputs}/RelatedLocationMessage.sarif (100%) rename src/Test.UnitTests.Sarif/TestData/{GitHubDspIngestionVisitor => GitHubIngestionVisitor}/ExpectedOutputs/ThreadFlowLocations.sarif (100%) rename src/Test.UnitTests.Sarif/TestData/{GitHubDspIngestionVisitor => GitHubIngestionVisitor}/ExpectedOutputs/TooManyResults.sarif (100%) rename src/Test.UnitTests.Sarif/TestData/{GitHubDspIngestionVisitor => GitHubIngestionVisitor}/ExpectedOutputs/WithArtifacts.sarif (100%) rename src/Test.UnitTests.Sarif/TestData/{GitHubDspIngestionVisitor => GitHubIngestionVisitor}/ExpectedOutputs/WithInvocation.sarif (100%) rename src/Test.UnitTests.Sarif/TestData/{GitHubDspIngestionVisitor => GitHubIngestionVisitor}/Inputs/Fingerprints.sarif (100%) rename src/Test.UnitTests.Sarif/TestData/{GitHubDspIngestionVisitor => GitHubIngestionVisitor}/Inputs/NonErrorResults.sarif (100%) rename src/Test.UnitTests.Sarif/TestData/{GitHubDspIngestionVisitor/ExpectedOutputs => GitHubIngestionVisitor/Inputs}/RelatedLocationMessage.sarif (90%) rename src/Test.UnitTests.Sarif/TestData/{GitHubDspIngestionVisitor => GitHubIngestionVisitor}/Inputs/ThreadFlowLocations.sarif (100%) rename src/Test.UnitTests.Sarif/TestData/{GitHubDspIngestionVisitor => GitHubIngestionVisitor}/Inputs/TooManyResults.sarif (100%) rename src/Test.UnitTests.Sarif/TestData/{GitHubDspIngestionVisitor => GitHubIngestionVisitor}/Inputs/WithArtifacts.sarif (100%) rename src/Test.UnitTests.Sarif/TestData/{GitHubDspIngestionVisitor => GitHubIngestionVisitor}/Inputs/WithInvocation.sarif (100%) rename src/Test.UnitTests.Sarif/Visitors/{GitHubDspIngestionVisitorTests.cs => GitHubIngestionVisitorTests.cs} (92%) diff --git a/src/Sarif/Visitors/GitHubIngestionVisitor.cs b/src/Sarif/Visitors/GitHubIngestionVisitor.cs index a877199cc..d5b11ddce 100644 --- a/src/Sarif/Visitors/GitHubIngestionVisitor.cs +++ b/src/Sarif/Visitors/GitHubIngestionVisitor.cs @@ -8,11 +8,6 @@ namespace Microsoft.CodeAnalysis.Sarif.Visitors { public class GitHubIngestionVisitor : SarifRewritingVisitor { - // GitHub requires that every related location have a message. It's not clear why this - // requirement exists, as this data is mostly used to build embedded links from - // results (where the link anchor text actually resides). - private const string PlaceholderRelatedLocationMessage = "[No message provided.]"; - // GitHub reportedly has an ingestion limit of 500 issues. // Internal static rather than private const to allow a unit test with a practical limit. internal static int s_MaxResults = 500; @@ -157,20 +152,6 @@ public override ArtifactLocation VisitArtifactLocation(ArtifactLocation node) public override Result VisitResult(Result node) { - if (node.RelatedLocations != null) - { - foreach (Location relatedLocation in node.RelatedLocations) - { - if (string.IsNullOrEmpty(relatedLocation.Message?.Text)) - { - relatedLocation.Message = new Message - { - Text = PlaceholderRelatedLocationMessage - }; - } - } - } - if (node.Fingerprints != null) { // GitHub appears to require that fingerprints be emitted to the diff --git a/src/Test.UnitTests.Sarif/Test.UnitTests.Sarif.csproj b/src/Test.UnitTests.Sarif/Test.UnitTests.Sarif.csproj index 60698b5c1..27479dc5a 100644 --- a/src/Test.UnitTests.Sarif/Test.UnitTests.Sarif.csproj +++ b/src/Test.UnitTests.Sarif/Test.UnitTests.Sarif.csproj @@ -17,20 +17,20 @@ - - - - - - - - - - - - - - + + + + + + + + + + + + + + @@ -88,20 +88,20 @@ - - - - - - - - - - - - - - + + + + + + + + + + + + + + diff --git a/src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/ExpectedOutputs/Fingerprints.sarif b/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/ExpectedOutputs/Fingerprints.sarif similarity index 100% rename from src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/ExpectedOutputs/Fingerprints.sarif rename to src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/ExpectedOutputs/Fingerprints.sarif diff --git a/src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/ExpectedOutputs/NonErrorResults.sarif b/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/ExpectedOutputs/NonErrorResults.sarif similarity index 100% rename from src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/ExpectedOutputs/NonErrorResults.sarif rename to src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/ExpectedOutputs/NonErrorResults.sarif diff --git a/src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/Inputs/RelatedLocationMessage.sarif b/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/ExpectedOutputs/RelatedLocationMessage.sarif similarity index 100% rename from src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/Inputs/RelatedLocationMessage.sarif rename to src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/ExpectedOutputs/RelatedLocationMessage.sarif diff --git a/src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/ExpectedOutputs/ThreadFlowLocations.sarif b/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/ExpectedOutputs/ThreadFlowLocations.sarif similarity index 100% rename from src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/ExpectedOutputs/ThreadFlowLocations.sarif rename to src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/ExpectedOutputs/ThreadFlowLocations.sarif diff --git a/src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/ExpectedOutputs/TooManyResults.sarif b/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/ExpectedOutputs/TooManyResults.sarif similarity index 100% rename from src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/ExpectedOutputs/TooManyResults.sarif rename to src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/ExpectedOutputs/TooManyResults.sarif diff --git a/src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/ExpectedOutputs/WithArtifacts.sarif b/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/ExpectedOutputs/WithArtifacts.sarif similarity index 100% rename from src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/ExpectedOutputs/WithArtifacts.sarif rename to src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/ExpectedOutputs/WithArtifacts.sarif diff --git a/src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/ExpectedOutputs/WithInvocation.sarif b/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/ExpectedOutputs/WithInvocation.sarif similarity index 100% rename from src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/ExpectedOutputs/WithInvocation.sarif rename to src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/ExpectedOutputs/WithInvocation.sarif diff --git a/src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/Inputs/Fingerprints.sarif b/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/Inputs/Fingerprints.sarif similarity index 100% rename from src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/Inputs/Fingerprints.sarif rename to src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/Inputs/Fingerprints.sarif diff --git a/src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/Inputs/NonErrorResults.sarif b/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/Inputs/NonErrorResults.sarif similarity index 100% rename from src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/Inputs/NonErrorResults.sarif rename to src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/Inputs/NonErrorResults.sarif diff --git a/src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/ExpectedOutputs/RelatedLocationMessage.sarif b/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/Inputs/RelatedLocationMessage.sarif similarity index 90% rename from src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/ExpectedOutputs/RelatedLocationMessage.sarif rename to src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/Inputs/RelatedLocationMessage.sarif index b144a0f02..83da11ca5 100644 --- a/src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/ExpectedOutputs/RelatedLocationMessage.sarif +++ b/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/Inputs/RelatedLocationMessage.sarif @@ -31,9 +31,6 @@ "artifactLocation": { "uri": "file2.c" } - }, - "message": { - "text": "[No message provided.]" } } ] diff --git a/src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/Inputs/ThreadFlowLocations.sarif b/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/Inputs/ThreadFlowLocations.sarif similarity index 100% rename from src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/Inputs/ThreadFlowLocations.sarif rename to src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/Inputs/ThreadFlowLocations.sarif diff --git a/src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/Inputs/TooManyResults.sarif b/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/Inputs/TooManyResults.sarif similarity index 100% rename from src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/Inputs/TooManyResults.sarif rename to src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/Inputs/TooManyResults.sarif diff --git a/src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/Inputs/WithArtifacts.sarif b/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/Inputs/WithArtifacts.sarif similarity index 100% rename from src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/Inputs/WithArtifacts.sarif rename to src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/Inputs/WithArtifacts.sarif diff --git a/src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/Inputs/WithInvocation.sarif b/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/Inputs/WithInvocation.sarif similarity index 100% rename from src/Test.UnitTests.Sarif/TestData/GitHubDspIngestionVisitor/Inputs/WithInvocation.sarif rename to src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/Inputs/WithInvocation.sarif diff --git a/src/Test.UnitTests.Sarif/Visitors/GitHubDspIngestionVisitorTests.cs b/src/Test.UnitTests.Sarif/Visitors/GitHubIngestionVisitorTests.cs similarity index 92% rename from src/Test.UnitTests.Sarif/Visitors/GitHubDspIngestionVisitorTests.cs rename to src/Test.UnitTests.Sarif/Visitors/GitHubIngestionVisitorTests.cs index 357d934cf..ad8db4c9d 100644 --- a/src/Test.UnitTests.Sarif/Visitors/GitHubDspIngestionVisitorTests.cs +++ b/src/Test.UnitTests.Sarif/Visitors/GitHubIngestionVisitorTests.cs @@ -7,9 +7,9 @@ namespace Microsoft.CodeAnalysis.Sarif.Visitors { - public class GitHubDspIngestionVisitorTests : FileDiffingUnitTests + public class GitHubIngestionVisitorTests : FileDiffingUnitTests { - public GitHubDspIngestionVisitorTests(ITestOutputHelper outputHelper) : base(outputHelper) { } + public GitHubIngestionVisitorTests(ITestOutputHelper outputHelper) : base(outputHelper) { } protected override string ConstructTestOutputFromInputResource(string inputResourceName, object parameter) { From cc242f7b8bcf42abeffe845d5b5666ba8659d788 Mon Sep 17 00:00:00 2001 From: Larry Golding Date: Wed, 23 Sep 2020 14:18:25 -0700 Subject: [PATCH 4/5] Remove obsolete test. --- .../Test.UnitTests.Sarif.csproj | 3 -- .../RelatedLocationMessage.sarif | 41 ------------------- .../Inputs/RelatedLocationMessage.sarif | 41 ------------------- .../Visitors/GitHubIngestionVisitorTests.cs | 4 -- 4 files changed, 89 deletions(-) delete mode 100644 src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/ExpectedOutputs/RelatedLocationMessage.sarif delete mode 100644 src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/Inputs/RelatedLocationMessage.sarif diff --git a/src/Test.UnitTests.Sarif/Test.UnitTests.Sarif.csproj b/src/Test.UnitTests.Sarif/Test.UnitTests.Sarif.csproj index 27479dc5a..002bb2f52 100644 --- a/src/Test.UnitTests.Sarif/Test.UnitTests.Sarif.csproj +++ b/src/Test.UnitTests.Sarif/Test.UnitTests.Sarif.csproj @@ -19,7 +19,6 @@ - @@ -92,7 +91,6 @@ - @@ -101,7 +99,6 @@ - diff --git a/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/ExpectedOutputs/RelatedLocationMessage.sarif b/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/ExpectedOutputs/RelatedLocationMessage.sarif deleted file mode 100644 index 83da11ca5..000000000 --- a/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/ExpectedOutputs/RelatedLocationMessage.sarif +++ /dev/null @@ -1,41 +0,0 @@ -{ - "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json", - "version": "2.1.0", - "runs": [ - { - "tool": { - "driver": { - "name": "Sarif.UnitTests" - } - }, - "results": [ - { - "ruleId": "TEST1001", - "level": "error", - "message": { - "text": "The message." - }, - "relatedLocations": [ - { - "physicalLocation": { - "artifactLocation": { - "uri": "file.c" - } - }, - "message": { - "text": "Related location message." - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "file2.c" - } - } - } - ] - } - ] - } - ] -} \ No newline at end of file diff --git a/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/Inputs/RelatedLocationMessage.sarif b/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/Inputs/RelatedLocationMessage.sarif deleted file mode 100644 index 83da11ca5..000000000 --- a/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/Inputs/RelatedLocationMessage.sarif +++ /dev/null @@ -1,41 +0,0 @@ -{ - "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json", - "version": "2.1.0", - "runs": [ - { - "tool": { - "driver": { - "name": "Sarif.UnitTests" - } - }, - "results": [ - { - "ruleId": "TEST1001", - "level": "error", - "message": { - "text": "The message." - }, - "relatedLocations": [ - { - "physicalLocation": { - "artifactLocation": { - "uri": "file.c" - } - }, - "message": { - "text": "Related location message." - } - }, - { - "physicalLocation": { - "artifactLocation": { - "uri": "file2.c" - } - } - } - ] - } - ] - } - ] -} \ No newline at end of file diff --git a/src/Test.UnitTests.Sarif/Visitors/GitHubIngestionVisitorTests.cs b/src/Test.UnitTests.Sarif/Visitors/GitHubIngestionVisitorTests.cs index ad8db4c9d..8db686a15 100644 --- a/src/Test.UnitTests.Sarif/Visitors/GitHubIngestionVisitorTests.cs +++ b/src/Test.UnitTests.Sarif/Visitors/GitHubIngestionVisitorTests.cs @@ -50,10 +50,6 @@ public void GitHubDspIngestionVisitor_RemovesInvocations() public void GitHubDspIngestionVisitor_RemovesArtifactsAndRetainsIndirectArtifactLocations() => RunTest("WithArtifacts.sarif"); - [Fact] - public void GitHubDspIngestionVisitor_ProvidesPlaceholderRelatedLocationMessage() - => RunTest("RelatedLocationMessage.sarif"); - [Fact] public void GitHubDspIngestionVisitor_MovesFingerprintsToPartialFingerprints() => RunTest("Fingerprints.sarif"); From 25140f13e9a9a127bc3a7350c2979c77d1775843 Mon Sep 17 00:00:00 2001 From: Larry Golding Date: Wed, 23 Sep 2020 14:20:42 -0700 Subject: [PATCH 5/5] Don't remove invocations. --- src/Sarif/Visitors/GitHubIngestionVisitor.cs | 8 ------ .../Test.UnitTests.Sarif.csproj | 4 --- .../ExpectedOutputs/WithInvocation.sarif | 22 --------------- .../Inputs/WithInvocation.sarif | 27 ------------------- .../Visitors/GitHubIngestionVisitorTests.cs | 4 --- 5 files changed, 65 deletions(-) delete mode 100644 src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/ExpectedOutputs/WithInvocation.sarif delete mode 100644 src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/Inputs/WithInvocation.sarif diff --git a/src/Sarif/Visitors/GitHubIngestionVisitor.cs b/src/Sarif/Visitors/GitHubIngestionVisitor.cs index d5b11ddce..341385fb4 100644 --- a/src/Sarif/Visitors/GitHubIngestionVisitor.cs +++ b/src/Sarif/Visitors/GitHubIngestionVisitor.cs @@ -20,14 +20,6 @@ public override Run VisitRun(Run node) this.artifacts = node.Artifacts; this.threadFlowLocations = node.ThreadFlowLocations; - // GitHub does not support submitting invocation objects. Invocations - // contains potentially sensitive environment details, such as - // account names embedded in paths. Invocations also store - // notifications of catastrophic tool failures, however, which - // means there is current no mechanism for reporting these to - // GitHub users in context of the security tab. - node.Invocations = null; - if (node.Results != null) { int errorsCount = 0; diff --git a/src/Test.UnitTests.Sarif/Test.UnitTests.Sarif.csproj b/src/Test.UnitTests.Sarif/Test.UnitTests.Sarif.csproj index 002bb2f52..dacdeb925 100644 --- a/src/Test.UnitTests.Sarif/Test.UnitTests.Sarif.csproj +++ b/src/Test.UnitTests.Sarif/Test.UnitTests.Sarif.csproj @@ -22,13 +22,11 @@ - - @@ -91,13 +89,11 @@ - - diff --git a/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/ExpectedOutputs/WithInvocation.sarif b/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/ExpectedOutputs/WithInvocation.sarif deleted file mode 100644 index 0e94c720b..000000000 --- a/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/ExpectedOutputs/WithInvocation.sarif +++ /dev/null @@ -1,22 +0,0 @@ -{ - "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json", - "version": "2.1.0", - "runs": [ - { - "tool": { - "driver": { - "name": "Sarif.UnitTests" - } - }, - "results": [ - { - "ruleId": "TEST1001", - "level": "error", - "message": { - "text": "The message." - } - } - ] - } - ] -} \ No newline at end of file diff --git a/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/Inputs/WithInvocation.sarif b/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/Inputs/WithInvocation.sarif deleted file mode 100644 index 8678051da..000000000 --- a/src/Test.UnitTests.Sarif/TestData/GitHubIngestionVisitor/Inputs/WithInvocation.sarif +++ /dev/null @@ -1,27 +0,0 @@ -{ - "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json", - "version": "2.1.0", - "runs": [ - { - "tool": { - "driver": { - "name": "Sarif.UnitTests" - } - }, - "invocations": [ - { - "executionSuccessful": true - } - ], - "results": [ - { - "ruleId": "TEST1001", - "level": "error", - "message": { - "text": "The message." - } - } - ] - } - ] -} \ No newline at end of file diff --git a/src/Test.UnitTests.Sarif/Visitors/GitHubIngestionVisitorTests.cs b/src/Test.UnitTests.Sarif/Visitors/GitHubIngestionVisitorTests.cs index 8db686a15..73df73664 100644 --- a/src/Test.UnitTests.Sarif/Visitors/GitHubIngestionVisitorTests.cs +++ b/src/Test.UnitTests.Sarif/Visitors/GitHubIngestionVisitorTests.cs @@ -42,10 +42,6 @@ public void GitHubDspIngestionVisitor_LimitsNumberOfResults() } } - [Fact] - public void GitHubDspIngestionVisitor_RemovesInvocations() - => RunTest("WithInvocation.sarif"); - [Fact] public void GitHubDspIngestionVisitor_RemovesArtifactsAndRetainsIndirectArtifactLocations() => RunTest("WithArtifacts.sarif");