Skip to content

Commit

Permalink
Fortify FPR converter fixes (#2014)
Browse files Browse the repository at this point in the history
* FortifyFpr converter improvements.

* Fix typo in file.

* Code changes to convrter

* Add DSP ingestion visitor.

* Files to demonstrate Fortify DSP progress.

* Update script to auto-gen drive letter based on script path.

* Update replacement logic.

* Update tests based on foritfy fopr converter and page command improvements.

* Remove test files for now.

* Delete test files.:
  • Loading branch information
michaelcfanning authored Jul 29, 2020
1 parent 87a9746 commit 839b189
Show file tree
Hide file tree
Showing 15 changed files with 361 additions and 36 deletions.
165 changes: 140 additions & 25 deletions src/Sarif.Converters/FortifyFprConverter.cs

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions src/Sarif.Converters/FortifyFprStrings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,12 @@ internal class FortifyFprStrings
/// <summary>The string constant "Platform".</summary>
public readonly string Platform;

/// <summary>The string constant "EngineVersion".</summary>
public readonly string EngineVersion;

/// <summary>The string constant "InstanceID".</summary>
public readonly string InstanceID;

/// <summary>
/// Initializes a new instance of the <see cref="FortifyFprStrings"/> class.
/// </summary>
Expand Down Expand Up @@ -297,6 +303,8 @@ public FortifyFprStrings(XmlNameTable nameTable)
Hostname = nameTable.Add("Hostname");
Username = nameTable.Add("Username");
Platform = nameTable.Add("Platform");
EngineVersion = nameTable.Add("EngineVersion");
InstanceID = nameTable.Add("InstanceID");
}
}
}
16 changes: 13 additions & 3 deletions src/Sarif.Converters/FortifyUtilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace Microsoft.CodeAnalysis.Sarif.Converters
{
internal static class FortifyUtilities
{
private static readonly Dictionary<string, string> FormattedTextReplacementss = new Dictionary<string, string>
private static readonly Dictionary<string, string> FormattedTextReplacements = new Dictionary<string, string>
{
// XML <Content> tag
{ "<[/]?Content>", string.Empty },
Expand All @@ -36,11 +36,21 @@ internal static class FortifyUtilities
{ "<[/]?pre>", "`" }
};


private static readonly Regex s_replaceKeyRegex = new Regex("<Replace key=\\\"([^\\\"]+)\\\"/>", RegexOptions.Compiled | RegexOptions.CultureInvariant);
private const string ReplacementTokenFormat = "<Replace key=\"{0}\"/>";

internal static string ParseFormattedContentText(string content)
{
foreach (string pattern in FormattedTextReplacementss.Keys)
foreach (string pattern in FormattedTextReplacements.Keys)
{
content = Regex.Replace(content, pattern, FormattedTextReplacements[pattern], RegexOptions.Compiled);
}

foreach(Match match in s_replaceKeyRegex.Matches(content))
{
content = Regex.Replace(content, pattern, FormattedTextReplacementss[pattern], RegexOptions.Compiled);
string key = match.Groups[1].Value;
content = content.Replace(string.Format(ReplacementTokenFormat, key), "{"+ key + "}");
}

return content.Trim(new[] { '\r', '\n', ' ' });
Expand Down
2 changes: 1 addition & 1 deletion src/Sarif.Multitool/CommandBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public abstract class CommandBase
internal const int SUCCESS = 0;
internal const int FAILURE = 1;

protected static bool ValidateNonNegativeCommandLineOption<T>(int optionValue, string optionName)
protected static bool ValidateNonNegativeCommandLineOption<T>(long optionValue, string optionName)
{
bool valid = true;

Expand Down
30 changes: 30 additions & 0 deletions src/Sarif.Multitool/ConvertCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,15 @@
using System;
using System.Diagnostics;
using System.Globalization;
using System.IO;

using Microsoft.CodeAnalysis.Sarif.Converters;
using Microsoft.CodeAnalysis.Sarif.Driver;
using Microsoft.CodeAnalysis.Sarif.Visitors;
using Microsoft.CodeAnalysis.Sarif.Writers;

using Newtonsoft.Json;

namespace Microsoft.CodeAnalysis.Sarif.Multitool
{
public class ConvertCommand : CommandBase
Expand Down Expand Up @@ -56,6 +61,31 @@ public int Run(ConvertOptions convertOptions, IFileSystem fileSystem = null)
loggingOptions,
dataToInsert,
convertOptions.PluginAssemblyPath);

if (convertOptions.NormalizeForGitHubDsp)
{
SarifLog sarifLog;

JsonSerializer serializer = new JsonSerializer()
{
Formatting = convertOptions.PrettyPrint ? Formatting.Indented : 0,
};

using (JsonTextReader reader = new JsonTextReader(new StreamReader(convertOptions.OutputFilePath)))
{
sarifLog = serializer.Deserialize<SarifLog>(reader);
}

var visitor = new GitHubDspIngestionVisitor();
visitor.VisitSarifLog(sarifLog);

using (FileStream stream = File.Create(convertOptions.OutputFilePath))
using (StreamWriter streamWriter = new StreamWriter(stream))
using (JsonTextWriter writer = new JsonTextWriter(streamWriter))
{
serializer.Serialize(writer, sarifLog);
}
}
}
catch (Exception ex) when (!Debugger.IsAttached)
{
Expand Down
6 changes: 6 additions & 0 deletions src/Sarif.Multitool/ConvertOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,11 @@ public class ConvertOptions : SingleFileOptionsBase
"plugin-assembly-path",
HelpText = "Path to plugin assembly containing converter types.")]
public string PluginAssemblyPath { get; internal set; }

[Option(
"normalize-for-github-dsp",
HelpText = "Normalize converted output to conform to GitHub DSP ingestion requirements.")]
public bool NormalizeForGitHubDsp { get; internal set; }

}
}
6 changes: 5 additions & 1 deletion src/Sarif.Multitool/FileWorkItemsCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,11 @@ public int Run(FileWorkItemsOptions options, IFileSystem fileSystem)
if (s_validateOptionsOnly) { return SUCCESS; }

string logFileContents = fileSystem.ReadAllText(options.InputFilePath);
EnsureValidSarifLogFile(logFileContents, options.InputFilePath);

if (!options.DoNotValidate)
{
EnsureValidSarifLogFile(logFileContents, options.InputFilePath);
}

if (options.SplittingStrategy != SplittingStrategy.None)
{
Expand Down
5 changes: 5 additions & 0 deletions src/Sarif.Multitool/FileWorkItemsOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,10 @@ public class FileWorkItemsOptions : SingleFileOptionsBase
"configuration",
HelpText = "A path to an XML configuration file that will be used to drive work item creation.")]
public string ConfigurationFilePath { get; internal set; }

[Option(
"no-validate",
HelpText = "Do not validate the SARIF log file before filing.")]
public bool DoNotValidate { get; internal set; }
}
}
9 changes: 8 additions & 1 deletion src/Sarif.Multitool/PageCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -169,11 +169,18 @@ private void ExtractPage(PageOptions options, JsonMapNode root)
return;
}

if (options.Index >= results.Count)
{
throw new ArgumentOutOfRangeException($"Index requested was {options.Index} but Run has only {results.Count} results.");
}

if (options.Index + options.Count > results.Count)
{
throw new ArgumentOutOfRangeException($"Page requested from Result {options.Index} to {options.Index + options.Count}, but Run has only {results.Count} results.");
Console.WriteLine($"Page requested from Result {options.Index} to {options.Index + options.Count} but Run has only {results.Count} results.");
options.Count = results.Count - options.Index;
}


Console.WriteLine($"Run {options.RunIndex} in \"{options.InputFilePath}\" has {results.Count:n0} results.");

Func<Stream> inputStreamProvider = () => _fileSystem.OpenRead(options.InputFilePath);
Expand Down
16 changes: 16 additions & 0 deletions src/Sarif/Core/CodeFlow.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright (c) Microsoft. All Rights Reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

namespace Microsoft.CodeAnalysis.Sarif
{
/// <summary>
/// A thread flow location of a SARIF thread flow.
/// </summary>
public partial class CodeFlow
{
public bool ShouldSerializeThreadFlows()
{
return this.ThreadFlows.HasAtLeastOneNonDefaultValue(ThreadFlow.ValueComparer);
}
}
}
13 changes: 13 additions & 0 deletions src/Sarif/Core/ThreadFlow.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright (c) Microsoft. All Rights Reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

namespace Microsoft.CodeAnalysis.Sarif
{
/// <summary>
/// A thread flow location of a SARIF thread flow.
/// </summary>
public partial class ThreadFlow
{
public bool ShouldSerializeLocations() { return this.Locations.HasAtLeastOneNonDefaultValue(ThreadFlowLocation.ValueComparer); }
}
}
2 changes: 1 addition & 1 deletion src/Sarif/Map/JsonMapNode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public class JsonMapNode
/// Count is the number of array elements (for arrays) or properties
/// (for objects) in the mapped object.
/// </summary>
public long Count { get; set; }
public int Count { get; set; }

/// <summary>
/// Return the byte length of this node.
Expand Down
114 changes: 114 additions & 0 deletions src/Sarif/Visitors/GitHubDspIngestionVisitor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
// 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;

namespace Microsoft.CodeAnalysis.Sarif.Visitors
{
public class GitHubDspIngestionVisitor : SarifRewritingVisitor
{
private IList<Artifact> artifacts;

public override Run VisitRun(Run node)
{
this.artifacts = node.Artifacts;

// DSP 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
// DSP users in context of the security tab.
node.Invocations = null;

if (node.Results != null)
{
int errorsCount = 0;
foreach (Result result in node.Results)
{
if (result.Level == FailureLevel.Error)
{
errorsCount++;
}
}

if (errorsCount != node.Results.Count)
{
var errors = new List<Result>();

foreach (Result result in node.Results)
{
if (result.Level == FailureLevel.Error)
{
errors.Add(result);
}

// GitHub DSP reportedly has an ingestion limit of 500 issues
if (errors.Count > 500) { break; }
}
node.Results = errors;
}
}

node = base.VisitRun(node);

// DSP prefers a relative path local to the result. We clear
// the artifacts table, as all artifact information is now
// inlined with each result.
node.Artifacts = null;

return node;
}


public override ArtifactLocation VisitArtifactLocation(ArtifactLocation node)
{
if (this.artifacts != null && node.Index > -1)
{
node.Uri = this.artifacts[node.Index].Location.Uri;
node.UriBaseId = this.artifacts[node.Index].Location.UriBaseId;
node.Index = -1;
}
return base.VisitArtifactLocation(node);
}

public override Result VisitResult(Result node)
{
if (node.RelatedLocations != null)
{
foreach (Location relatedLocation in node.RelatedLocations)
{
// DSP 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).
if (string.IsNullOrEmpty(relatedLocation.Message?.Text))
{
relatedLocation.Message = new Message
{
Text = "[No message provided.]"
};
}
}
}

if (node.Fingerprints != null)
{
// DSP appears to require that fingerprints be emitted to the
// partial fingerprints property in order to prefer these
// values for matching (over DSP's built-in SARIF-driven
// results matching heuristics).
foreach(string fingerprintKey in node.Fingerprints.Keys)
{
node.PartialFingerprints ??= new Dictionary<string, string>();
node.PartialFingerprints[fingerprintKey] = node.Fingerprints[fingerprintKey];
}
node.Fingerprints = null;
}

node.CodeFlows = null;

return base.VisitResult(node);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public void FortifyUtilities_ParseFormattedContentText_Correct()
string content = @"<Content><Paragraph><b>General relativity</b> is the <pre>geometric theory of gravitation</pre> published by <i>Albert Einstein</i> in 1915 and the current description of <IfDef var=""ConditionalDescriptions""><ConditionalText attr=""value"">gravitation in modern physics</ConditionalText></IfDef>.<AltParagraph>General relativity generalizes special relativity and <Replace key=""Scientist.lastName""/>'s law of universal gravitation, providing a unified description of gravity as a geometric property of space and time, or spacetime. In particular, the curvature of spacetime is directly related to the energy and momentum of whatever matter and radiation are present.</AltParagraph>The relation is specified by the <code>Einstein field equations</code>, a system of <code>partial differential equations</code>.";
content += "\r\nAnd here's a garbage\n\r\n \n \n\ndouble line break!</Paragraph></Content>";
string expected = @"**General relativity** is the `geometric theory of gravitation` published by _Albert Einstein_ in 1915 and the current description of gravitation in modern physics.
General relativity generalizes special relativity and <Replace key=""Scientist.lastName""/>'s law of universal gravitation, providing a unified description of gravity as a geometric property of space and time, or spacetime. In particular, the curvature of spacetime is directly related to the energy and momentum of whatever matter and radiation are present.
General relativity generalizes special relativity and {Scientist.lastName}'s law of universal gravitation, providing a unified description of gravity as a geometric property of space and time, or spacetime. In particular, the curvature of spacetime is directly related to the energy and momentum of whatever matter and radiation are present.
The relation is specified by the `Einstein field equations`, a system of `partial differential equations`.
And here's a garbage
Expand Down
3 changes: 0 additions & 3 deletions src/Test.UnitTests.Sarif.Multitool/PageCommandTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,6 @@ public void PageCommand_DynamicOptionValidation()
// Index >= Count
Assert.Throws<ArgumentOutOfRangeException>(() => RunAndCompare(new PageOptions() { Index = 5, Count = 1, InputFilePath = sampleFilePath, OutputFilePath = pagedSamplePath }));

// Index + Count > Count
Assert.Throws<ArgumentOutOfRangeException>(() => RunAndCompare(new PageOptions() { Index = 0, Count = 6, InputFilePath = sampleFilePath, OutputFilePath = pagedSamplePath }));

// RunIndex >= RunCount
Assert.Throws<ArgumentOutOfRangeException>(() => RunAndCompare(new PageOptions() { RunIndex = 1, Index = 1, Count = 1, InputFilePath = sampleFilePath, OutputFilePath = pagedSamplePath }));
}
Expand Down

0 comments on commit 839b189

Please sign in to comment.