Merge branch 'main' into users/shaopeng-gh/assemblyattributesobj

microsoft · Mar 29, 2024 · ec52e3e · ec52e3e
2 parents eb8d753 + 528fcce
commit ec52e3e
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 7 deletions.
diff --git a/ReleaseHistory.md b/ReleaseHistory.md
@@ -16,9 +16,10 @@
 - NEW => new feature 
 ## UNRELEASED
 * DEP: Update `Sarif.Sdk` submodule from [bc8cb57 to fd6e615](https://github.com/microsoft/sarif-sdk/compare/bc8cb57...fd6e615). Reference [SARIF SDK Release History](https://github.com/microsoft/sarif-sdk/blob/fd6e615/ReleaseHistory.md).
-* NEW: Add `--disable-telemetry` argument to disable telemetry collection.
-* BUG: Fix `ERR998.ExceptionInAnalyze`: `InvalidOperationException: Unrecognized crypto HRESULT: 0x80096011` for check `BA2022.SignSecurely` when the signature is malformed, by adding missing error code to error description mappings. [969](https://github.com/microsoft/binskim/pull/969).
 * BUG: Exclude system-generated files `AssemblyAttributes.obj`, `AssemblyInfo.obj`, `stdafx.obj` from `BA2004.EnableSecureSourceCodeHashing`. [989](https://github.com/microsoft/binskim/pull/989).
+* BUG: Fix `ERR998.ExceptionInAnalyze`: `InvalidOperationException: Unrecognized crypto HRESULT: 0x80096011` for check `BA2022.SignSecurely` when the signature is malformed, by adding missing error code to error description mappings. [969](https://github.com/microsoft/binskim/pull/969)
+* NEW: `BA4002.ReportElfOrMachoCompilerData`, which collects telemetry data for Elf and Macho files, is now enabled by default.
+* NEW: Add `--disable-telemetry` argument to disable telemetry collection.
 
 ## **v4.2.1**
 * FPS: `BA2004.EnableSecureSourceCodeHashing` now will no longer generate false positives on precompiled headers, they are always without hash. [#965](https://github.com/microsoft/binskim/pull/965)

diff --git a/src/BinSkim.Rules/DwarfRules/BA4002.ReportElfOrMachoCompilerData.cs b/src/BinSkim.Rules/DwarfRules/BA4002.ReportElfOrMachoCompilerData.cs
@@ -30,8 +30,6 @@ public class ReportElfOrMachoCompilerData : DwarfSkimmerBase, IOptionsProvider
         /// </summary>
         public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.BA4002_ReportELFCompilerData_Description };
 
-        public override bool EnabledByDefault => false;
-
         protected override IEnumerable<string> MessageResourceNames => Array.Empty<string>();
 
         public override AnalysisApplicability CanAnalyzeDwarf(IDwarfBinary target, Sarif.PropertiesDictionary policy, out string reasonForNotAnalyzing)

diff --git a/src/BinSkim.Sdk/CompilerDataLogger.cs b/src/BinSkim.Sdk/CompilerDataLogger.cs
@@ -109,9 +109,12 @@ public CompilerDataLogger(string sarifOutputFilePath, Sarif.SarifVersion sarifVe
             this.symbolPath = context.SymbolPath;
             this.telemetryClient = telemetry?.TelemetryClient;
 
-            bool forceOverwrite = context.ForceOverwrite;
-            string csvFilePath = context.Policy.GetProperty(CsvOutputPath);
-            CreateCsvOutputFile(csvFilePath, forceOverwrite);
+            if (!context.DisableTelemetry)
+            {
+                bool forceOverwrite = context.ForceOverwrite;
+                string csvFilePath = context.Policy.GetProperty(CsvOutputPath);
+                CreateCsvOutputFile(csvFilePath, forceOverwrite);
+            }
 
             // If the user has configured compiler telemetry collection, then we require analysis results
             // are persisted to a disk-based log file (to produce the telemetry 'summary' data persisted

diff --git a/src/Test.UnitTests.BinSkim.Driver/TelemetryTests.cs b/src/Test.UnitTests.BinSkim.Driver/TelemetryTests.cs
@@ -119,5 +119,54 @@ public void Telemetry_ShouldHaveConsistentResultsEnabledOrDisabled()
                     "Whether DisableTelemetry is explicitly set to false or not, the results should be the same.");
             }
         }
+
+        [Fact]
+        public void Telemetry_ReportElfOrMachoCompilerData_EnabledAndWorkingByDefault()
+        {
+            if (!PlatformSpecificHelpers.RunningOnWindows()) { return; }
+
+            using var assertionScope = new AssertionScope();
+
+            string sarifFile = Path.Combine(Path.GetTempPath(), "AnalyzeCommand_Telemetry_ReportElfOrMachoCompilerData_EnabledByDefault.sarif");
+            string csvFile = Path.Combine(Path.GetTempPath(), "AnalyzeCommand_Telemetry_ReportElfOrMachoCompilerData_EnabledByDefault.csv");
+            string configFile = Path.Combine(Path.GetTempPath(), "AnalyzeCommand_Telemetry_ReportElfOrMachoCompilerData_EnabledByDefault.xml");
+
+            File.Delete(sarifFile);
+            File.Delete(csvFile);
+            File.Delete(configFile);
+
+            string configFileContent = $"<?xml version=\"1.0\" encoding=\"utf-8\"?><Properties><Properties Key=\"CompilerTelemetry.Options\"><Property Key=\"CsvOutputPath\" Value=\"{csvFile}\"/></Properties></Properties>";
+            File.WriteAllText(configFile, configFileContent);
+
+            string pathToTestFile = Path.Combine(PEBinaryTests.TestData, "Dwarf", "hello-dwarf5-o2");
+            var options = new AnalyzeOptions
+            {
+                TargetFileSpecifiers = new string[] {
+                    pathToTestFile
+                },
+                OutputFilePath = sarifFile,
+                OutputFileOptions = new[] { FilePersistenceOptions.ForceOverwrite },
+                DataToInsert = new[] { OptionallyEmittedData.Hashes },
+                ConfigurationFilePath = configFile,
+            };
+
+            var command = new MultithreadedAnalyzeCommand();
+            command.Run(options);
+
+            bool fileExists = File.Exists(csvFile);
+            fileExists.Should().BeTrue("because the telemetry is enabled");
+
+            if (fileExists)
+            {
+                string fileContent = File.ReadAllText(csvFile);
+                fileContent.Should().Contain("-gdwarf-5", "because the rule ReportElfOrMachoCompilerData is enabled by default");
+            }
+
+            File.Delete(csvFile);
+            options.DisableTelemetry = true;
+            command.Run(options);
+            fileExists = File.Exists(csvFile);
+            fileExists.Should().BeFalse("because the telemetry is disabled");
+        }
     }
 }