Update Microsoft.Security.Utilities.Core from v1.17.0 to v1.18.0

src/Agent.Sdk/Agent.Sdk.csproj

@@ -7,7 +7,7 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="Microsoft.Security.Utilities.Core" Version="1.17.0" />
+    <PackageReference Include="Microsoft.Security.Utilities.Core" Version="1.18.0" />
     <PackageReference Include="Microsoft.Win32.Registry" Version="5.0.0" />
     <PackageReference Include="System.IO.FileSystem.AccessControl" Version="6.0.0-preview.5.21301.5" />
     <PackageReference Include="System.Management" Version="4.7.0" />

src/Agent.Sdk/SecretMasking/ILoggedSecretMasker.cs

@@ -3,21 +3,21 @@
 
 using System;
 
-using Microsoft.TeamFoundation.DistributedTask.Logging;
-
 namespace Agent.Sdk.SecretMasking
 {
     /// <summary>
     /// Extended ISecretMasker interface that adds support for logging the origin of
     /// regexes, encoders and literal secret values.
     /// </summary>
-    public interface ILoggedSecretMasker : ISecretMasker, IDisposable
+    public interface ILoggedSecretMasker : IDisposable
     {
-        static int MinSecretLengthLimit { get; }
+        int MinSecretLength { get; set; }
 
-        void AddRegex(String pattern, string origin);
-        void AddValue(String value, string origin);
-        void AddValueEncoder(ValueEncoder encoder, string origin);
+        void AddRegex(string pattern, string origin);
+        void AddValue(string value, string origin);
+        void AddValueEncoder(Func<string, string> encoder, string origin);
+        string MaskSecrets(string input);
+        void RemoveShortSecretsFromDictionary();
         void SetTrace(ITraceWriter trace);
     }
 }

src/Agent.Sdk/SecretMasking/IRawSecretMasker.cs

@@ -0,0 +1,21 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+using System;
+
+namespace Agent.Sdk.SecretMasking
+{
+    /// <summary>
+    /// Rerpresents a raw secret masker without the features that <see
+    /// cref="ILoggedSecretMasker"/> adds.
+    /// </summary>
+    public interface IRawSecretMasker : IDisposable
+    {
+        int MinSecretLength { get; set; }
+
+        void AddRegex(string pattern);
+        void AddValue(string value);
+        void AddValueEncoder(Func<string, string> encoder);
+        string MaskSecrets(string input);
+        void RemoveShortSecretsFromDictionary();
+    }
+}

src/Agent.Sdk/SecretMasking/LegacySecretMasker.cs

@@ -0,0 +1,68 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+using System;
+using Microsoft.TeamFoundation.DistributedTask.Logging;
+
+namespace Agent.Sdk.SecretMasking
+{
+    /// <summary>
+    /// Legacy secret masker that dispatches to <see cref="SecretMasker"/> from
+    /// 'Microsoft.TeamFoundation.DistributedTask.Logging'.
+    /// </summary>
+    public sealed class LegacySecretMasker : IRawSecretMasker
+    {
+        private ISecretMasker _secretMasker;
+
+        public LegacySecretMasker()
+        {
+            _secretMasker = new SecretMasker();
+        }
+
+        private LegacySecretMasker(ISecretMasker secretMasker)
+        {
+            _secretMasker = secretMasker;
+        }
+
+        public int MinSecretLength
+        {
+            get => _secretMasker.MinSecretLength;
+            set => _secretMasker.MinSecretLength = value;
+        }
+
+        public void AddRegex(string pattern)
+        {
+            _secretMasker.AddRegex(pattern);
+        }
+
+        public void AddValue(string value)
+        {
+            _secretMasker.AddValue(value);
+        }
+
+        public void AddValueEncoder(Func<string, string> encoder)
+        {
+            _secretMasker.AddValueEncoder(x => encoder(x));
+        }
+
+        public void Dispose()
+        {
+            (_secretMasker as IDisposable)?.Dispose();
+            _secretMasker = null;
+        }
+
+        public string MaskSecrets(string input)
+        {
+            return _secretMasker.MaskSecrets(input);
+        }
+
+        public void RemoveShortSecretsFromDictionary()
+        {
+            _secretMasker.RemoveShortSecretsFromDictionary();
+        }
+
+        public LegacySecretMasker Clone()
+        {
+            return new LegacySecretMasker(_secretMasker.Clone());
+        }
+    }
+}

src/Agent.Sdk/SecretMasking/LoggedSecretMasker.cs

@@ -1,49 +1,52 @@
 
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
-using System;
 
+using System;
 using Microsoft.TeamFoundation.DistributedTask.Logging;
 
 namespace Agent.Sdk.SecretMasking
 {
     /// <summary>
     /// Extended secret masker service that allows specifying the origin of any
-    /// masking operation. It works by wrapping an existing ISecretMasker
+    /// masking operation. It works by wrapping an existing IRawSecretMasker
     /// implementation and an optionally settable ITraceWriter instance for
     /// secret origin logging operations. In the agent today, this class can be
-    /// initialized with two distinct ISecretMasker implementations, the one
+    /// initialized with two distinct IRawSecretMasker implementations, the one
     /// that ships in VSO itself, and the official Microsoft open source secret
     /// masker, implemented at https://github/microsoft/security-utilities.
     /// </summary>
     public class LoggedSecretMasker : ILoggedSecretMasker
     {
-        private ISecretMasker _secretMasker;
+        private IRawSecretMasker _secretMasker;
         private ITraceWriter _trace;
 
-
         private void Trace(string msg)
         {
             this._trace?.Info(msg);
         }
 
-        public LoggedSecretMasker(ISecretMasker secretMasker)
+        private LoggedSecretMasker(IRawSecretMasker secretMasker)
         {
-            this._secretMasker = secretMasker;
+            _secretMasker = secretMasker;
         }
 
-        public void SetTrace(ITraceWriter trace)
+        public static LoggedSecretMasker Create(IRawSecretMasker secretMasker)
         {
-            this._trace = trace;
+            return secretMasker switch
+            {
+                LegacySecretMasker lsm => new LegacyLoggedSecretMasker(lsm),
+                _ => new LoggedSecretMasker(secretMasker),
+            };
         }
 
-        public void AddValue(string pattern)
+        public void SetTrace(ITraceWriter trace)
         {
-            this._secretMasker.AddValue(pattern);
+            this._trace = trace;
         }
 
         /// <summary>
-        /// Overloading of AddValue method with additional logic for logging origin of provided secret
+        /// AddValue method with additional logic for logging origin of provided secret
         /// </summary>
         /// <param name="value">Secret to be added</param>
         /// <param name="origin">Origin of the secret</param>
@@ -57,18 +60,14 @@ public void AddValue(string value, string origin)
                 return;
             }
 
-            AddValue(value);
-        }
-        public void AddRegex(string pattern)
-        {
-            this._secretMasker.AddRegex(pattern);
+            _secretMasker.AddValue(value);
         }
 
         /// <summary>
-        /// Overloading of AddRegex method with additional logic for logging origin of provided secret
+        /// AddRegex method with additional logic for logging origin of provided secret
         /// </summary>
-        /// <param name="pattern"></param>
-        /// <param name="origin"></param>
+        /// <param name="pattern">Regex to be added</param>
+        /// <param name="origin">Origin of the regex</param>
         public void AddRegex(string pattern, string origin)
         {
             // WARNING: Do not log the pattern here, it could be very specifc and contain a secret!
@@ -79,7 +78,7 @@ public void AddRegex(string pattern, string origin)
                 return;
             }
 
-            AddRegex(pattern);
+            _secretMasker.AddRegex(pattern);
         }
 
         // We don't allow to skip secrets longer than 5 characters.
@@ -111,18 +110,17 @@ public void RemoveShortSecretsFromDictionary()
             _secretMasker.RemoveShortSecretsFromDictionary();
         }
 
-        public void AddValueEncoder(ValueEncoder encoder)
+        public void AddValueEncoder(Func<string, string> encoder)
         {
             this._secretMasker.AddValueEncoder(encoder);
         }
 
-
         /// <summary>
         /// Overloading of AddValueEncoder method with additional logic for logging origin of provided secret
         /// </summary>
         /// <param name="encoder"></param>
         /// <param name="origin"></param>
-        public void AddValueEncoder(ValueEncoder encoder, string origin)
+        public void AddValueEncoder(Func<string, string> encoder, string origin)
         {
             this.Trace($"Setting up value for origin: {origin}");
             if (encoder == null)
@@ -134,18 +132,11 @@ public void AddValueEncoder(ValueEncoder encoder, string origin)
             AddValueEncoder(encoder);
         }
 
-        public LoggedSecretMasker Clone()
-        {
-            return new LoggedSecretMasker(this._secretMasker.Clone());
-        }
-
         public string MaskSecrets(string input)
         {
             return this._secretMasker.MaskSecrets(input);
         }
 
-        ISecretMasker ISecretMasker.Clone() => this.Clone();
-
         public void Dispose()
         {
             Dispose(true);
@@ -156,12 +147,43 @@ protected virtual void Dispose(bool disposing)
         {
             if (disposing)
             {
-                if (_secretMasker is IDisposable disposable)
-                {
-                    disposable.Dispose();
-                }
+                _secretMasker?.Dispose();
                 _secretMasker = null;
             }
         }
+
+        // When backed by legacy secret masker, we can still implement Clone and
+        // the server ISecretMasker interface. This is done to minimize churn
+        // when running without the feature flag that enables the new secret
+        // masker.
+        private sealed class LegacyLoggedSecretMasker : LoggedSecretMasker, ISecretMasker
+        {
+            public LegacyLoggedSecretMasker(LegacySecretMasker secretMasker) : base(secretMasker) { }
+
+            void ISecretMasker.AddRegex(string pattern)
+            {
+                _secretMasker.AddRegex(pattern);
+            }
+
+            void ISecretMasker.AddValue(string value)
+            {
+                _secretMasker.AddValue(value);
+            }
+
+            void ISecretMasker.AddValueEncoder(ValueEncoder encoder)
+            {
+                _secretMasker.AddValueEncoder(x => encoder(x));
+            }
+
+            ISecretMasker ISecretMasker.Clone()
+            {
+                // NOTE: It has always been the case that trace does not flow to
+                // clones and this code path exists to preserve legacy behavior
+                // in the absence of a feature flag, so that behavior is
+                // retained here.
+                var lsm = (LegacySecretMasker)_secretMasker;
+                return new LegacyLoggedSecretMasker(lsm.Clone());
+            }
+        }
     }
 }

src/Agent.Sdk/SecretMasking/OssSecretMasker.cs

@@ -5,12 +5,9 @@
 using System.Text.RegularExpressions;
 using Microsoft.Security.Utilities;
 
-using ISecretMasker = Microsoft.TeamFoundation.DistributedTask.Logging.ISecretMasker;
-using ValueEncoder = Microsoft.TeamFoundation.DistributedTask.Logging.ValueEncoder;
-
 namespace Agent.Sdk.SecretMasking;
 
-public sealed class OssSecretMasker : ISecretMasker, IDisposable
+public sealed class OssSecretMasker : IRawSecretMasker
 {
     private SecretMasker _secretMasker;
 
@@ -24,10 +21,6 @@ public OssSecretMasker(IEnumerable<RegexPattern> patterns)
         _secretMasker.DefaultRegexRedactionToken = "***";
     }
 
-    private OssSecretMasker(OssSecretMasker copy)
-    {
-        _secretMasker = copy._secretMasker.Clone();
-    }
 
     /// <summary>
     /// This property allows to set the minimum length of a secret for masking
@@ -70,13 +63,11 @@ public void AddValue(string test)
     /// <summary>
     /// This implementation assumes no more than one thread is adding regexes, values, or encoders at any given time.
     /// </summary>
-    public void AddValueEncoder(ValueEncoder encoder)
+    public void AddValueEncoder(Func<string, string> encoder)
     {
        _secretMasker.AddLiteralEncoder(x => encoder(x));
     }
 
-    public OssSecretMasker Clone() => new OssSecretMasker(this);
-
     public void Dispose()
     {
         _secretMasker?.Dispose();
@@ -152,6 +143,4 @@ public void RemoveShortSecretsFromDictionary()
             }
         }
     }
-
-    ISecretMasker ISecretMasker.Clone() => this.Clone();
 }

src/Agent.Worker/ContainerOperationProvider.cs

@@ -298,7 +298,7 @@ private async Task<string> GetAcrPasswordFromAADToken(IExecutionContext executio
             }
 
             // Mark retrieved password as secret
-            HostContext.SecretMasker.AddValue(AcrPassword);
+            HostContext.SecretMasker.AddValue(AcrPassword, origin: "AcrPassword");
 
             return AcrPassword;
         }

src/Agent.Worker/ExpressionManager.cs

@@ -7,6 +7,7 @@
 using Microsoft.TeamFoundation.DistributedTask.WebApi;
 using Microsoft.VisualStudio.Services.Agent.Util;
 using Microsoft.TeamFoundation.DistributedTask.Expressions;
+using Microsoft.TeamFoundation.DistributedTask.Logging;
 using System.Text;
 
 namespace Microsoft.VisualStudio.Services.Agent.Worker
@@ -52,7 +53,13 @@ public ConditionResult Evaluate(IExecutionContext executionContext, IExpressionN
             ConditionResult result = new ConditionResult();
             var expressionTrace = new TraceWriter(Trace, hostTracingOnly ? null : executionContext);
 
-            result.Value = tree.Evaluate<bool>(trace: expressionTrace, secretMasker: HostContext.SecretMasker, state: executionContext);
+            // NOTE: When the non-legacy secret masker is enabled via feature
+            // flag, this conversion will fail and we will pass null here. This
+            // is deliberate and OK because the trace that we pass will handle
+            // secret masking as will upstream exception handlers.
+            var secretMasker = HostContext.SecretMasker as ISecretMasker;
+
+            result.Value = tree.Evaluate<bool>(trace: expressionTrace, secretMasker, state: executionContext);
             result.Trace = expressionTrace.Trace;
 
             return result;