From c4a8b868b33acf82b693b4eeabc6ce185c5a74d8 Mon Sep 17 00:00:00 2001 From: marmegh <30842915+marmegh@users.noreply.github.com> Date: Wed, 15 Jun 2022 15:13:27 -0700 Subject: [PATCH] Fix missing command line id from compiler events (#652) * Adjusted event order and updated test to prevent regression. * Updated release history. * Updated comment. * Adding string constants for testing. * Addressed additional PR feedback. * Removed extra whitespace, added suggested validations, and cleaned up variable usage. * Fix whitespace. --- src/BinSkim.Sdk/CompilerDataLogger.cs | 20 ++- src/ReleaseHistory.md | 1 + .../CompilerDataLoggerTests.cs | 170 +++++++++++++++++- 3 files changed, 181 insertions(+), 10 deletions(-) diff --git a/src/BinSkim.Sdk/CompilerDataLogger.cs b/src/BinSkim.Sdk/CompilerDataLogger.cs index 0b61f00a6..cbe31f1c3 100644 --- a/src/BinSkim.Sdk/CompilerDataLogger.cs +++ b/src/BinSkim.Sdk/CompilerDataLogger.cs @@ -48,10 +48,12 @@ public class CompilerDataLogger : IDisposable internal const string ProjectId = "projectId"; internal const string SymbolPath = "symbolPath"; internal const string BuildRunId = "buildRunId"; + internal const string CommandLine = "commandLine"; internal const string ProjectName = "projectName"; internal const string ToolVersion = "toolVersion"; internal const string TimeConsumed = "timeConsumed"; internal const string RepositoryId = "repositoryId"; + internal const string CommandLineId = "commandLineId"; internal const string RepositoryName = "repositoryName"; internal const string OrganizationId = "organizationId"; internal const string NormalizedPath = "normalizedPath"; @@ -59,8 +61,12 @@ public class CompilerDataLogger : IDisposable internal const string OrganizationName = "organizationName"; internal const string AnalysisStartTime = "analysisStartTime"; internal const string BuildDefinitionId = "buildDefinitionId"; + internal const string AssemblyReferences = "assemblyReferences"; + internal const string ChunkedCommandLine = "chunkedcommandLine"; internal const string BuildDefinitionName = "buildDefinitionName"; + internal const string AssemblyReferencesId = "assemblyReferencesId"; internal const string NumberOfBinaryAnalyzed = "numberOfBinaryAnalyzed"; + internal const string ChunkedAssemblyReferences = "chunkedassemblyReferences"; // This object is required to synchronize multi-threaded writes // to the CSV writer only. The AppInsights client is already @@ -260,29 +266,31 @@ public void Write(BinaryAnalyzerContext context, CompilerData compilerData) { "error", string.Empty } }; - this.telemetryClient.TrackEvent(CompilerEventName, properties: properties); - if (!string.IsNullOrWhiteSpace(compilerData.CommandLine)) { string commandLineId = Guid.NewGuid().ToString(); - properties.Add("commandLineId", commandLineId); + properties.Add(CommandLineId, commandLineId); SendChunkedContent(CommandLineEventName, commandLineId, - "commandLine", + CommandLine, compilerData.CommandLine); } if (!string.IsNullOrWhiteSpace(compilerData.AssemblyReferences)) { string assemblyReferencesId = Guid.NewGuid().ToString(); - properties.Add("assemblyReferencesId", assemblyReferencesId); + properties.Add(AssemblyReferencesId, assemblyReferencesId); SendChunkedContent(AssemblyReferencesEventName, assemblyReferencesId, - "assemblyReferences", + AssemblyReferences, compilerData.AssemblyReferences); } + + // To prevent loss of relationship between compiler and assembly/commandline data + // this event cannot be sent before AssemblyReferences or CommandLine. + this.telemetryClient.TrackEvent(CompilerEventName, properties: properties); } public void WriteException(BinaryAnalyzerContext context, string errorMessage) diff --git a/src/ReleaseHistory.md b/src/ReleaseHistory.md index f082b3e60..a32420006 100644 --- a/src/ReleaseHistory.md +++ b/src/ReleaseHistory.md @@ -8,6 +8,7 @@ * FEATURE: Enable BinSkim for MacOS. [#576](https://github.com/microsoft/binskim/pull/576) * Bump Sarif.Sdk by updating submodule from [4e9f606 to fc9a9df](https://github.com/microsoft/sarif-sdk/compare/4e9f606bb0e88428866e253352cdc70dc68f98cb...fc9a9dfb865096b5aaa9fa3651854670940f7459). [#638](https://github.com/microsoft/binskim/pull/638) * FALSE POSITIVE FIX: Skip `BA2025.EnableShadowStack` rule for ARM Binaries which cannot use `/CETCOMPAT`. [#650](https://github.com/microsoft/binskim/pull/650) +* BUGFIX: Fix missing `commandLineId` from `CommandLineInformation` event. [#652](https://github.com/microsoft/binskim/pull/652) ## **v1.9.4** [NuGet Package](https://www.nuget.org/packages/Microsoft.CodeAnalysis.BinSkim/1.9.4) diff --git a/src/Test.UnitTests.BinSkim.Driver/CompilerDataLoggerTests.cs b/src/Test.UnitTests.BinSkim.Driver/CompilerDataLoggerTests.cs index c67272811..ed55b81b1 100644 --- a/src/Test.UnitTests.BinSkim.Driver/CompilerDataLoggerTests.cs +++ b/src/Test.UnitTests.BinSkim.Driver/CompilerDataLoggerTests.cs @@ -43,9 +43,16 @@ public void CompilerDataLogger_Write_ShouldSendAssemblyReferencesInChunks_WhenTe string assemblies = "Microsoft.DiaSymReader (1.3.0);Newtonsoft.Json (13.0.1)"; CompilerDataLogger.s_chunkSize = SmallChunkSize; - int chunkNumber = logger.CalculateChunkedContentSize(assemblies.Length); + int expectedChunkCount = logger.CalculateChunkedContentSize(assemblies.Length); logger.Write(context, new CompilerData { CompilerName = ".NET Compiler", AssemblyReferences = assemblies }); - telemetryEventOutput.Count.Should().Be(chunkNumber + 1); + + StringBuilder sb = ValidateChunkedEvents( + expectedEventName: CompilerDataLogger.AssemblyReferencesEventName, + telemetryGeneratedEvents: telemetryEventOutput, + expectedChunkedContent: assemblies, + chunkCount: expectedChunkCount); + + Assert.Equal(string.Empty, sb.ToString()); } [Fact] @@ -58,11 +65,13 @@ public void CompilerDataLogger_Write_ShouldSendCommandLineDataInChunks_WhenTelem string commandLine = "TestCommandLine"; CompilerDataLogger.s_chunkSize = SmallChunkSize; - int chunksize = CompilerDataLogger.s_chunkSize; int chunkNumber = logger.CalculateChunkedContentSize(commandLine.Length); CompilerData compilerData = new CompilerData { CompilerName = ".NET Compiler", CommandLine = commandLine }; logger.Write(context, compilerData); - telemetryEventOutput.Count.Should().Be(chunkNumber + 1); + + StringBuilder sb = ValidateChunkedEvents(CompilerDataLogger.CommandLineEventName, telemetryEventOutput, commandLine, chunkNumber); + + Assert.Equal(string.Empty, sb.ToString()); } [Fact] @@ -622,6 +631,154 @@ private StringBuilder ValidateExceptionEvents(string csvOutputPath, return sb; } + private StringBuilder ValidateChunkedEvents(string expectedEventName, + List telemetryGeneratedEvents, + string expectedChunkedContent, + int chunkCount) + { + var sb = new StringBuilder(); + string assemblyEventsId = string.Empty; + string commandLineEventsId = string.Empty; + + var compilerEvents = new List(); + var assemblyReferencesEvents = new List(); + var commandLineEvents = new List(); + var summaryEvents = new List(); + + foreach (EventTelemetry telemetryEvent in telemetryGeneratedEvents) + { + IDictionary properties = telemetryEvent.Properties; + + if (string.IsNullOrWhiteSpace(commandLineEventsId) + && properties.TryGetValue(CompilerDataLogger.CommandLineId, out string commandLineEventId)) + { + commandLineEventsId = commandLineEventId; + } + + if (string.IsNullOrWhiteSpace(assemblyEventsId) + && properties.TryGetValue(CompilerDataLogger.AssemblyReferencesId, out string assemblyEventId)) + { + assemblyEventsId = assemblyEventId; + } + + switch (telemetryEvent.Name) + { + case CompilerDataLogger.CompilerEventName: + { + compilerEvents.Add(telemetryEvent); + + if (expectedEventName == CompilerDataLogger.AssemblyReferencesEventName) + { + if (!properties.TryGetValue(CompilerDataLogger.AssemblyReferencesId, out string compilerAssemblyReferencesId)) + { + sb.AppendLine("Compiler Event is missing the `assemblyReferencesId`"); + } + else if (assemblyEventsId != compilerAssemblyReferencesId) + { + sb.AppendLine( + $"`{CompilerDataLogger.CompilerEventName}` event detected with unexpected Id. " + + $"Expected `{assemblyEventsId}` but found `{compilerAssemblyReferencesId}`."); + } + } + else if (expectedEventName == CompilerDataLogger.CommandLineEventName) + { + if (!properties.TryGetValue(CompilerDataLogger.CommandLineId, out string compilerCommandLineEventId)) + { + sb.AppendLine("Compiler Event is missing the `commandLineId`"); + } + else if (commandLineEventsId != compilerCommandLineEventId) + { + sb.AppendLine( + $"`{CompilerDataLogger.CommandLineEventName}` event detected with unexpected Id. " + + $"Expected `{commandLineEventsId}` but found `{compilerCommandLineEventId}`."); + } + } + break; + } + case CompilerDataLogger.AssemblyReferencesEventName: + { + assemblyReferencesEvents.Add(telemetryEvent); + + if (expectedEventName == CompilerDataLogger.AssemblyReferencesEventName + && !expectedChunkedContent.Contains(properties[CompilerDataLogger.ChunkedAssemblyReferences])) + { + sb.AppendLine( + $"Unexpected `{CompilerDataLogger.AssemblyReferencesEventName}` chunked content: " + + $"`{properties[CompilerDataLogger.ChunkedAssemblyReferences]}` " + + $"expected: `{expectedChunkedContent}`"); + } + + string currentAssemblyEventId = properties[CompilerDataLogger.AssemblyReferencesId]; + + if (assemblyEventsId != currentAssemblyEventId) + { + sb.AppendLine( + $"`{CompilerDataLogger.AssemblyReferencesEventName}` event detected with unexpected Id. " + + $"Expected `{assemblyEventsId}` but found `{currentAssemblyEventId}`."); + } + + break; + } + case CompilerDataLogger.CommandLineEventName: + { + commandLineEvents.Add(telemetryEvent); + + if (expectedEventName == CompilerDataLogger.CommandLineEventName + && !expectedChunkedContent.Contains(properties["chunkedcommandLine"])) + { + sb.AppendLine( + $"Unexpected `{CompilerDataLogger.CommandLineEventName}` chunked content: " + + $"`{properties[CompilerDataLogger.ChunkedCommandLine]}` " + + $"expected: `{expectedChunkedContent}`"); + } + + string currentCommandLineEventId = properties["commandLineId"]; + + if (commandLineEventsId != currentCommandLineEventId) + { + sb.AppendLine( + $"`{CompilerDataLogger.CommandLineEventName}` event detected with unexpected Id. " + + $"Expected `{commandLineEventsId}` but found `{currentCommandLineEventId}`."); + } + break; + } + case CompilerDataLogger.SummaryEventName: + { + summaryEvents.Add(telemetryEvent); + break; + } + } + } + + if ((expectedEventName == CompilerDataLogger.AssemblyReferencesEventName + && assemblyReferencesEvents.Count != chunkCount) || + (expectedEventName == CompilerDataLogger.CommandLineEventName + && commandLineEvents.Count != chunkCount)) + { + sb.AppendLine( + $"Expected `{chunkCount}` `{expectedEventName}` events, " + + $"but `{assemblyReferencesEvents.Count}` were found"); + } + + return sb; + } + + private StringBuilder ValidateChunkedContent(StringBuilder sb, + int expectedChunkSize, + List chunkedEvents) + { + if (chunkedEvents.Count != expectedChunkSize) + { + sb.AppendLine( + string.Format( + "Incorrect number of chunkedEvents de tected. Expected {0}, but found {1}", + expectedChunkSize, + chunkedEvents.Count)); + } + + return sb; + } + public static string GetExampleSarifPath(Sarif.SarifVersion sarifVersion) { return sarifVersion == Sarif.SarifVersion.Current @@ -638,5 +795,10 @@ internal static string GetTestDirectory(string relativeDirectory) dirPath = Path.GetFullPath(dirPath); return Path.Combine(dirPath, relativeDirectory); } + + internal static int CalculateChunkedContentSize(int contentLength) + { + return (int)Math.Ceiling(1.0 * contentLength / CompilerDataLogger.s_chunkSize); + } } }