Try to fix flake in hardcoded secrets test (#4843)

* Update the HardcodedSecretsAnalyzer to use an instance, to make it easier to test * Rename RunShutdown to Dispose * Fix more static properties
DataDog · Nov 13, 2023 · c965bf0 · c965bf0
1 parent d13b831
commit c965bf0
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 30 deletions.
diff --git a/tracer/src/Datadog.Trace/IAST/Analyzers/HardcodedSecretsAnalyzer.cs b/tracer/src/Datadog.Trace/IAST/Analyzers/HardcodedSecretsAnalyzer.cs
@@ -17,28 +17,28 @@
 
 namespace Datadog.Trace.Iast.Analyzers;
 
-internal class HardcodedSecretsAnalyzer
+internal class HardcodedSecretsAnalyzer : IDisposable
 {
     private const int UserStringsArraySize = 100;
     private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor<HardcodedSecretsAnalyzer>();
     private static HardcodedSecretsAnalyzer? _instance = null;
 
-    private static bool _started = false;
+    private readonly ManualResetEventSlim _waitEvent = new(false);
+    private readonly TimeSpan _regexTimeout;
+    private bool _started = false;
+    private List<SecretRegex>? _secretRules = null;
 
-    private static List<SecretRegex>? _secretRules = null;
-
-    private static ManualResetEventSlim _waitEvent = new ManualResetEventSlim(false);
-
-    public HardcodedSecretsAnalyzer()
+    // Internal for testing
+    internal HardcodedSecretsAnalyzer(TimeSpan regexTimeout)
     {
         Log.Debug("HardcodedSecretsAnalyzer -> Init");
-        LifetimeManager.Instance.AddShutdownTask(RunShutdown);
+        _regexTimeout = regexTimeout;
         _started = true;
         Task.Run(() => PoolingThread())
                     .ContinueWith(t => Log.Error(t.Exception, "Error in Hardcoded secret analyzer"), TaskContinuationOptions.OnlyOnFaulted);
     }
 
-    private static void PoolingThread()
+    private void PoolingThread()
     {
         try
         {
@@ -106,11 +106,23 @@ private static void PoolingThread()
         Log.Debug("HardcodedSecretsAnalyzer polling thread -> Exit");
     }
 
-    internal static string? CheckSecret(string secret)
+    internal static void Initialize(TimeSpan regexTimeout)
+    {
+        lock (Log)
+        {
+            if (_instance == null)
+            {
+                _instance = new HardcodedSecretsAnalyzer(regexTimeout);
+                LifetimeManager.Instance.AddShutdownTask(_instance.Dispose);
+            }
+        }
+    }
+
+    internal string? CheckSecret(string secret)
     {
         if (_secretRules == null)
         {
-            _secretRules = GenerateSecretRules();
+            _secretRules = GenerateSecretRules(_regexTimeout);
         }
 
         foreach (var rule in _secretRules)
@@ -124,22 +136,9 @@ private static void PoolingThread()
         return null;
     }
 
-    internal static void Initialize()
-    {
-        lock (Log)
-        {
-            if (_instance == null)
-            {
-                _instance = new HardcodedSecretsAnalyzer();
-            }
-        }
-    }
-
     // Rules imported from https://github.com/gitleaks/gitleaks/blob/master/cmd/generate/config/rules
-    private static List<SecretRegex> GenerateSecretRules()
+    private static List<SecretRegex> GenerateSecretRules(TimeSpan timeout)
     {
-        var timeout = TimeSpan.FromMilliseconds(Iast.Instance.Settings.RegexTimeout);
-
         var res = new List<SecretRegex>(68); // Note: Update this with the new rules count, if modified
 
         res.Add(new SecretRegex("aws-access-token", @"\b((A3T[A-Z0-9]|AKIA|AGPA|AIDA|AROA|AIPA|ANPA|ANVA|ASIA)[A-Z0-9]{16})(?:['|""|\n|\r|\s|\x60|;]|$)", timeout));
@@ -214,7 +213,7 @@ private static List<SecretRegex> GenerateSecretRules()
         return res;
     }
 
-    private void RunShutdown()
+    public void Dispose()
     {
         try
         {

diff --git a/tracer/src/Datadog.Trace/Iast/Iast.cs b/tracer/src/Datadog.Trace/Iast/Iast.cs
@@ -3,6 +3,7 @@
 // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
 // </copyright>
 
+using System;
 using System.Threading;
 using Datadog.Trace.Configuration;
 using Datadog.Trace.Iast.Analyzers;
@@ -41,7 +42,7 @@ private Iast(IastSettings settings = null)
         _settings = settings ?? IastSettings.FromDefaultSources();
         if (_settings.Enabled)
         {
-            HardcodedSecretsAnalyzer.Initialize();
+            HardcodedSecretsAnalyzer.Initialize(TimeSpan.FromMilliseconds(_settings.RegexTimeout));
         }
 
         _overheadController = new OverheadController(_settings.MaxConcurrentRequests, _settings.RequestSampling);

diff --git a/tracer/test/Datadog.Trace.Security.Unit.Tests/IAST/HardcodedSecretsTests.cs b/tracer/test/Datadog.Trace.Security.Unit.Tests/IAST/HardcodedSecretsTests.cs
@@ -3,15 +3,22 @@
 // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
 // </copyright>
 
-using Datadog.Trace.Iast;
+using System;
 using Datadog.Trace.Iast.Analyzers;
 using FluentAssertions;
 using Xunit;
 
 namespace Datadog.Trace.Security.Unit.Tests.IAST;
 
-public class HardcodedSecretsTests
+public class HardcodedSecretsTests : IClassFixture<HardcodedSecretsTests.HardcodedSecretsFixture>
 {
+    private readonly HardcodedSecretsAnalyzer _analyzer;
+
+    public HardcodedSecretsTests(HardcodedSecretsFixture fixture)
+    {
+        _analyzer = fixture.Analyzer;
+    }
+
     [Theory]
     [InlineData("private-key", "-----BEGIN OPENSSH PRIVATE KEY-----\r\nb3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW\r\nQyNTUxOQAAACA8YWKYztuuvxUIMomc3zv0OdXCT57Cc2cRYu3TMbX9XAAAAJDiKO3C4ijt\r\nwgAAAAtzc2gtZWQyNTUxOQAAACA8YWKYztuuvxUIMomc3zv0OdXCT57Cc2cRYu3TMbX9XA\r\nAAAECzmj8DGxg5YHtBK4AmBttMXDQHsPAaCyYHQjJ4YujRBTxhYpjO266/FQgyiZzfO/Q5\r\n1cJPnsJzZxFi7dMxtf1cAAAADHJvb3RAZGV2aG9zdAE=\r\n-----END OPENSSH PRIVATE KEY-----\r\n")]
     [InlineData("aws-access-token", @"A3T43DROGX[[DD_SECRET]]R2PGF4BI5T")]
@@ -104,6 +111,14 @@ public class HardcodedSecretsTests
     public void GivenAHardcodedSecretString_WhenAnalysed_ResultIsEspected(string rule, string secret)
     {
         var realSecret = secret.Replace("[[DD_SECRET]]", string.Empty);
-        Assert.Equal(rule, HardcodedSecretsAnalyzer.CheckSecret(realSecret));
+        _analyzer.CheckSecret(realSecret).Should().Be(rule);
+    }
+
+    public class HardcodedSecretsFixture : IDisposable
+    {
+        // We use a stupidly-big timeout here to avoid flake
+        internal HardcodedSecretsAnalyzer Analyzer { get; } = new(TimeSpan.FromMinutes(5));
+
+        public void Dispose() => Analyzer.Dispose();
     }
 }