Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding rule SARIF2016 #1996

Merged
merged 3 commits into from
Jul 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion docs/Producing effective SARIF.md
Original file line number Diff line number Diff line change
Expand Up @@ -586,4 +586,27 @@ This is part of a set of authoring practices that make your rule messages more r

##### `Default`: note

{0}: In rule '{1}', the message with id '{2}' includes dynamic content that is not enclosed in single quotes. Enquoting dynamic content makes it easier to spot, and single quotes give a less cluttered appearance.
{0}: In rule '{1}', the message with id '{2}' includes dynamic content that is not enclosed in single quotes. Enquoting dynamic content makes it easier to spot, and single quotes give a less cluttered appearance.

---
### Rule `SARIF2016.FileUrisShouldBeRelative`

#### Description

When an artifact location refers to a file on the local file system, specify a relative reference for the uri property and provide a uriBaseId property, rather than specifying an absolute URI.

There are several advantages to this approach:

Portability: A log file that contains relative references together with uriBaseI properties can be interpreted on a machine where the files are located at a different absolute location.

Determinism: A log file that uses uriBaseId properties has a better chance of being “deterministic”; that is, of being identical from run to run if none of its inputs have changed, even if those runs occur on machines where the files are located at different absolute locations.

Security: The use of uriBaseId properties avoids the persistence of absolute path names in the log file. Absolute path names can reveal information that might be sensitive.

Semantics: Assuming the reader of the log file (an end user or another tool) has the necessary context, they can understand the meaning of the location specified by the uri property, for example, “this is a source file”.

#### Messages

##### `Default`: note

{0}: The file location '{1}' is specified with absolute URI. Prefer a relative reference together with a uriBaseId property.
1 change: 1 addition & 0 deletions src/Sarif.Multitool/Rules/RuleId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ public static class RuleId
public const string ProvideEmbeddedFileContent = "SARIF2013";
public const string ProvideDynamicMessageContent = "SARIF2014";
public const string EnquoteDynamicMessageContent = "SARIF2015";
public const string FileUrisShouldBeRelative = "SARIF2016";

// TEMPLATE:
// public const string RuleFriendlyName = "SARIFnnnn";
Expand Down
24 changes: 24 additions & 0 deletions src/Sarif.Multitool/Rules/RuleResources.Designer.cs

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

16 changes: 16 additions & 0 deletions src/Sarif.Multitool/Rules/RuleResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -375,4 +375,20 @@ This is part of a set of authoring practices that make your rule messages more r
<data name="SARIF2004_OptimizeFileSize_Warning_PreferRuleId_Text" xml:space="preserve">
<value>The result at '{0}' uses the 'rule' property to specify the violated rule, but this is not necessary because the rule is defined by 'tool.driver'. Use the 'ruleId' and 'ruleIndex' instead, because they are shorter and just as clear.</value>
</data>
<data name="SARIF2016_FileUrisShouldBeRelative_FullDescription_Text" xml:space="preserve">
<value>When an artifact location refers to a file on the local file system, specify a relative reference for the uri property and provide a uriBaseId property, rather than specifying an absolute URI.

There are several advantages to this approach:

Portability: A log file that contains relative references together with uriBaseI properties can be interpreted on a machine where the files are located at a different absolute location.

Determinism: A log file that uses uriBaseId properties has a better chance of being 'deterministic'; that is, of being identical from run to run if none of its inputs have changed, even if those runs occur on machines where the files are located at different absolute locations.

Security: The use of uriBaseId properties avoids the persistence of absolute path names in the log file. Absolute path names can reveal information that might be sensitive.

Semantics: Assuming the reader of the log file (an end user or another tool) has the necessary context, they can understand the meaning of the location specified by the uri property, for example, 'this is a source file'.</value>
</data>
<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>
</root>
88 changes: 88 additions & 0 deletions src/Sarif.Multitool/Rules/SARIF2016.FileUrisShouldBeRelative.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// 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 FileUrisShouldBeRelative : SarifValidationSkimmerBase
{
/// <summary>
/// SARIF2016
/// </summary>
public override string Id => RuleId.FileUrisShouldBeRelative;

/// <summary>
/// When an artifact location refers to a file on the local file system, specify a relative reference
/// for the uri property and provide a uriBaseId property, rather than specifying an absolute URI.
///
/// There are several advantages to this approach:
/// Portability: A log file that contains relative references together with uriBaseI properties can
/// be interpreted on a machine where the files are located at a different absolute location.
///
/// Determinism: A log file that uses uriBaseId properties has a better chance of being “deterministic”;
/// that is, of being identical from run to run if none of its inputs have changed, even if those runs
/// occur on machines where the files are located at different absolute locations.
///
/// Security: The use of uriBaseId properties avoids the persistence of absolute path names in the
/// log file.Absolute path names can reveal information that might be sensitive.
///
/// Semantics: Assuming the reader of the log file (an end user or another tool) has the necessary
/// context, they can understand the meaning of the location specified by the uri property, for
/// example, “this is a source file”.
/// </summary>
public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.SARIF2016_FileUrisShouldBeRelative_FullDescription_Text };

protected override IEnumerable<string> MessageResourceNames => new string[] {
nameof(RuleResources.SARIF2016_FileUrisShouldBeRelative_Note_Default_Text)
};

public override FailureLevel DefaultLevel => FailureLevel.Note;

protected override void Analyze(Run run, string runPointer)
{
if (run.Results != null)
{
AnalyzeResults(run.Results, runPointer.AtProperty(SarifPropertyName.Results));
}
}

private void AnalyzeResults(IList<Result> results, string resultsPointer)
{
for (int i = 0; i < results.Count; i++)
{
string resultPointer = resultsPointer.AtIndex(i);
Result result = results[i];

if (result.Locations != null)
{
AnalyzeResultLocations(result.Locations, resultPointer.AtProperty(SarifPropertyName.Locations));
}
}
}

private void AnalyzeResultLocations(IList<Location> locations, string locationsPointer)
{
for (int i = 0; i < locations.Count; i++)
{
string locationPointer = locationsPointer.AtIndex(i);
Location location = locations[i];

if (location.PhysicalLocation?.ArtifactLocation.Uri != null)
{
if (location.PhysicalLocation.ArtifactLocation.Uri.IsAbsoluteUri && location.PhysicalLocation.ArtifactLocation.Uri.IsFile)
{
// {0}: The file location '{1}' is specified with absolute URI. Prefer a relative
// reference together with a uriBaseId property.
LogResult(
locationPointer.AtProperty(SarifPropertyName.PhysicalLocation).AtProperty(SarifPropertyName.ArtifactLocation).AtProperty(SarifPropertyName.Uri),
nameof(RuleResources.SARIF2016_FileUrisShouldBeRelative_Note_Default_Text),
location.PhysicalLocation.ArtifactLocation.Uri.OriginalString);
}
}
}
}
}
}
11 changes: 11 additions & 0 deletions src/Test.FunctionalTests.Sarif/Multitool/ValidateCommandTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ public void SARIF2006_UrisShouldBeReachable_Invalid()
public void SARIF2007_ExpressPathsRelativeToRepoRoot_Valid()
=> RunTest(MakeValidTestFileName(RuleId.ExpressPathsRelativeToRepoRoot, nameof(RuleId.ExpressPathsRelativeToRepoRoot)),
parameter: new TestParameters(configFileName: "enable2007.configuration.xml"));

[Fact]
public void SARIF2007_ExpressPathsRelativeToRepoRoot_WithoutVersionControlProvenance_Valid()
=> RunTest("SARIF2007.ExpressPathsRelativeToRepoRoot_WithoutVersionControlProvenance_Valid.sarif",
Expand Down Expand Up @@ -306,6 +307,16 @@ public void SARIF2015_EnquoteDynamicMessageContent_Invalid()
=> RunTest(MakeInvalidTestFileName(RuleId.EnquoteDynamicMessageContent, nameof(RuleId.EnquoteDynamicMessageContent)),
parameter: new TestParameters(verbose: true));

[Fact]
public void SARIF2016_FileUrisShouldBeRelative_Valid()
=> RunTest(MakeValidTestFileName(RuleId.FileUrisShouldBeRelative, nameof(RuleId.FileUrisShouldBeRelative)),
parameter: new TestParameters(verbose: true, configFileName: "enable2016.configuration.xml"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enable2016 [](start = 78, length = 10)

If the point is simply to enable a specific rule, we should have a helper that constructs the appropriate config using a rule id. And dispense with maintaining a checked in file.


[Fact]
public void SARIF2016_FileUrisShouldBeRelative_Invalid()
=> RunTest(MakeInvalidTestFileName(RuleId.FileUrisShouldBeRelative, nameof(RuleId.FileUrisShouldBeRelative)),
parameter: new TestParameters(verbose: true, configFileName: "enable2016.configuration.xml"));

private const string ValidTestFileNameSuffix = "_Valid.sarif";
private const string InvalidTestFileNameSuffix = "_Invalid.sarif";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,9 @@
<None Update="enable2002.configuration.xml">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Update="enable2016.configuration.xml">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Update="enable2007.configuration.xml">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,14 @@
"descriptor": {
"id": "WRN999.RuleExplicitlyDisabled"
}
},
{
"message": {
"text": "Rule 'SARIF2016' was explicitly disabled by the user. As result, this tool run cannot be used for compliance or other auditing processes that require a comprehensive analysis."
},
"descriptor": {
"id": "WRN999.RuleExplicitlyDisabled"
}
}
],
"executionSuccessful": true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,14 @@
"descriptor": {
"id": "WRN999.RuleExplicitlyDisabled"
}
},
{
"message": {
"text": "Rule 'SARIF2016' was explicitly disabled by the user. As result, this tool run cannot be used for compliance or other auditing processes that require a comprehensive analysis."
},
"descriptor": {
"id": "WRN999.RuleExplicitlyDisabled"
}
}
],
"executionSuccessful": true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@
"descriptor": {
"id": "WRN999.RuleExplicitlyDisabled"
}
},
{
"message": {
"text": "Rule 'SARIF2016' was explicitly disabled by the user. As result, this tool run cannot be used for compliance or other auditing processes that require a comprehensive analysis."
},
"descriptor": {
"id": "WRN999.RuleExplicitlyDisabled"
}
}
],
"executionSuccessful": true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@
"descriptor": {
"id": "WRN999.RuleExplicitlyDisabled"
}
},
{
"message": {
"text": "Rule 'SARIF2016' was explicitly disabled by the user. As result, this tool run cannot be used for compliance or other auditing processes that require a comprehensive analysis."
},
"descriptor": {
"id": "WRN999.RuleExplicitlyDisabled"
}
}
],
"executionSuccessful": true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,14 @@
"descriptor": {
"id": "WRN999.RuleExplicitlyDisabled"
}
},
{
"message": {
"text": "Rule 'SARIF2016' was explicitly disabled by the user. As result, this tool run cannot be used for compliance or other auditing processes that require a comprehensive analysis."
},
"descriptor": {
"id": "WRN999.RuleExplicitlyDisabled"
}
}
],
"executionSuccessful": true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@
"descriptor": {
"id": "WRN999.RuleExplicitlyDisabled"
}
},
{
"message": {
"text": "Rule 'SARIF2016' was explicitly disabled by the user. As result, this tool run cannot be used for compliance or other auditing processes that require a comprehensive analysis."
},
"descriptor": {
"id": "WRN999.RuleExplicitlyDisabled"
}
}
],
"executionSuccessful": true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,14 @@
"descriptor": {
"id": "WRN999.RuleExplicitlyDisabled"
}
},
{
"message": {
"text": "Rule 'SARIF2016' was explicitly disabled by the user. As result, this tool run cannot be used for compliance or other auditing processes that require a comprehensive analysis."
},
"descriptor": {
"id": "WRN999.RuleExplicitlyDisabled"
}
}
],
"executionSuccessful": true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@
"descriptor": {
"id": "WRN999.RuleExplicitlyDisabled"
}
},
{
"message": {
"text": "Rule 'SARIF2016' was explicitly disabled by the user. As result, this tool run cannot be used for compliance or other auditing processes that require a comprehensive analysis."
},
"descriptor": {
"id": "WRN999.RuleExplicitlyDisabled"
}
}
],
"executionSuccessful": true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@
"descriptor": {
"id": "WRN999.RuleExplicitlyDisabled"
}
},
{
"message": {
"text": "Rule 'SARIF2016' was explicitly disabled by the user. As result, this tool run cannot be used for compliance or other auditing processes that require a comprehensive analysis."
},
"descriptor": {
"id": "WRN999.RuleExplicitlyDisabled"
}
}
],
"executionSuccessful": true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@
"descriptor": {
"id": "WRN999.RuleExplicitlyDisabled"
}
},
{
"message": {
"text": "Rule 'SARIF2016' was explicitly disabled by the user. As result, this tool run cannot be used for compliance or other auditing processes that require a comprehensive analysis."
},
"descriptor": {
"id": "WRN999.RuleExplicitlyDisabled"
}
}
],
"executionSuccessful": true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@
"descriptor": {
"id": "WRN999.RuleExplicitlyDisabled"
}
},
{
"message": {
"text": "Rule 'SARIF2016' was explicitly disabled by the user. As result, this tool run cannot be used for compliance or other auditing processes that require a comprehensive analysis."
},
"descriptor": {
"id": "WRN999.RuleExplicitlyDisabled"
}
}
],
"executionSuccessful": true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@
"descriptor": {
"id": "WRN999.RuleExplicitlyDisabled"
}
},
{
"message": {
"text": "Rule 'SARIF2016' was explicitly disabled by the user. As result, this tool run cannot be used for compliance or other auditing processes that require a comprehensive analysis."
},
"descriptor": {
"id": "WRN999.RuleExplicitlyDisabled"
}
}
],
"executionSuccessful": true
Expand Down
Loading