[release/9.1] Hide secrets in source tooltip, correct ExpressionResol…

…ver logic for non-containers (#7708) * Replace tuple with record type in source view model, hide secrets in tooltip * Remove sourceIsContainer checks in ResolveInternalAsync * Fix test * EndpointReference/Expression need to only be container * Apply suggestion * special case ResourceWithConnectionStringSurrogate * Update comment * Fix discrepant behavior between ReferenceExpression.GetValueAsync and ExpressionResolver.EvalExpressionAsync * Invoke GetValueAsync on ConnectionStringReference before resolving it, to prevent non-optional but missing value from resolving * Update src/Aspire.Hosting/ApplicationModel/ExpressionResolver.cs Co-authored-by: James Newton-King <[email protected]> * Improve comment in ResolveConnectionStringReferenceAsync * Change DashboardUIHelpers.GetMaskingText to return a record * clean up ResourceSourceViewModel * add additional secrets in test case * Add additional ExpressionResolver tests --------- Co-authored-by: Adam Ratzman <[email protected]> Co-authored-by: Dan Moseley <[email protected]> Co-authored-by: James Newton-King <[email protected]>
dotnet · Feb 20, 2025 · a7453c1 · a7453c1
1 parent 87cb282
commit a7453c1
Show file tree

Hide file tree

Showing 8 changed files with 162 additions and 77 deletions.
diff --git a/src/Aspire.Dashboard/Components/Controls/GridValue.razor b/src/Aspire.Dashboard/Components/Controls/GridValue.razor
@@ -10,7 +10,7 @@
     @if (EnableMasking && IsMasked)
     {
         <span class="grid-value masked" id="@_cellTextId">
-            @DashboardUIHelpers.GetMaskingText(length: 8)
+            @DashboardUIHelpers.GetMaskingText(length: 8).MarkupString
         </span>
     }
     else
@@ -32,7 +32,7 @@
                     @((MarkupString)_formattedValue)
                 }
                 @ContentAfterValue
-            }            
+            }
         </span>
     }
 

diff --git a/src/Aspire.Dashboard/Components/ResourcesGridColumns/SourceColumnDisplay.razor b/src/Aspire.Dashboard/Components/ResourcesGridColumns/SourceColumnDisplay.razor
@@ -24,7 +24,7 @@
                 }
                 else
                 {
-                    <span class="subtext">&nbsp;@DashboardUIHelpers.GetMaskingText(length: 6)</span>
+                    <span class="subtext">&nbsp;@DashboardUIHelpers.GetMaskingText(length: 6).MarkupString</span>
                 }
             }
         }

diff --git a/src/Aspire.Dashboard/Model/ResourceSourceViewModel.cs b/src/Aspire.Dashboard/Model/ResourceSourceViewModel.cs
@@ -1,6 +1,8 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using Aspire.Dashboard.Utils;
+
 namespace Aspire.Dashboard.Model;
 
 public class ResourceSourceViewModel(string value, List<LaunchArgument>? contentAfterValue, string valueToVisualize, string tooltip)
@@ -12,65 +14,22 @@ public class ResourceSourceViewModel(string value, List<LaunchArgument>? content
 
     internal static ResourceSourceViewModel? GetSourceViewModel(ResourceViewModel resource)
     {
-        (List<LaunchArgument>? Arguments, string? ArgumentsString) commandLineInfo;
-
-        // If the resource contains launch arguments, these project arguments should be shown in place of all executable arguments,
-        // which include args added by the app host
-        if (resource.TryGetAppArgs(out var launchArguments))
-        {
-            if (launchArguments.IsDefaultOrEmpty)
-            {
-                commandLineInfo = (null, null);
-            }
-            else
-            {
-                var argumentsString = string.Join(" ", launchArguments);
-                if (resource.TryGetAppArgsSensitivity(out var areArgumentsSensitive))
-                {
-                    var arguments = launchArguments
-                        .Select((arg, i) => new LaunchArgument(arg, IsShown: !areArgumentsSensitive[i]))
-                        .ToList();
-
-                    commandLineInfo = (Arguments: arguments, argumentsString);
-                }
-                else
-                {
-                    commandLineInfo = (Arguments: launchArguments.Select(arg => new LaunchArgument(arg, true)).ToList(), argumentsString);
-                }
-            }
-        }
-        else if (resource.TryGetExecutableArguments(out var executableArguments) && !resource.IsProject())
-        {
-            var arguments = executableArguments.IsDefaultOrEmpty ? null : executableArguments.Select(arg => new LaunchArgument(arg, true)).ToList();
-            commandLineInfo = (Arguments: arguments, string.Join(' ', executableArguments));
-        }
-        else
-        {
-            commandLineInfo = (Arguments: null, null);
-        }
+        var commandLineInfo = GetCommandLineInfo(resource);
 
         // NOTE projects are also executables, so we have to check for projects first
         if (resource.IsProject() && resource.TryGetProjectPath(out var projectPath))
         {
-            if (commandLineInfo is { Arguments: { } arguments, ArgumentsString: { } fullCommandLine })
-            {
-                return new ResourceSourceViewModel(value: Path.GetFileName(projectPath), contentAfterValue: arguments, valueToVisualize: $"{projectPath} {fullCommandLine}", tooltip: $"{projectPath} {fullCommandLine}");
-            }
-
-            // default to project path if there is no executable path or executable arguments
-            return new ResourceSourceViewModel(value: Path.GetFileName(projectPath), contentAfterValue: commandLineInfo.Arguments, valueToVisualize: projectPath, tooltip: projectPath);
+            return CreateResourceSourceViewModel(Path.GetFileName(projectPath), projectPath, commandLineInfo);
         }
 
         if (resource.TryGetExecutablePath(out var executablePath))
         {
-            var fullSource = commandLineInfo.ArgumentsString is not null ? $"{executablePath} {commandLineInfo.ArgumentsString}" : executablePath;
-            return new ResourceSourceViewModel(value: Path.GetFileName(executablePath), contentAfterValue: commandLineInfo.Arguments, valueToVisualize: fullSource, tooltip: fullSource);
+            return CreateResourceSourceViewModel(Path.GetFileName(executablePath), executablePath, commandLineInfo);
         }
 
         if (resource.TryGetContainerImage(out var containerImage))
         {
-            var fullSource = commandLineInfo.ArgumentsString is null ? containerImage : $"{containerImage} {commandLineInfo.ArgumentsString}";
-            return new ResourceSourceViewModel(value: containerImage, contentAfterValue: commandLineInfo.Arguments, valueToVisualize: fullSource, tooltip: fullSource);
+            return CreateResourceSourceViewModel(containerImage, containerImage, commandLineInfo);
         }
 
         if (resource.Properties.TryGetValue(KnownProperties.Resource.Source, out var property) && property.Value is { HasStringValue: true, StringValue: var value })
@@ -79,7 +38,56 @@ public class ResourceSourceViewModel(string value, List<LaunchArgument>? content
         }
 
         return null;
+
+        static CommandLineInfo? GetCommandLineInfo(ResourceViewModel resourceViewModel)
+        {
+            // If the resource contains launch arguments, these project arguments should be shown in place of all executable arguments,
+            // which include args added by the app host
+            if (resourceViewModel.TryGetAppArgs(out var launchArguments))
+            {
+                if (launchArguments.IsDefaultOrEmpty)
+                {
+                    return null;
+                }
+
+                var argumentsString = string.Join(" ", launchArguments);
+                if (resourceViewModel.TryGetAppArgsSensitivity(out var areArgumentsSensitive))
+                {
+                    var arguments = launchArguments
+                        .Select((arg, i) => new LaunchArgument(arg, IsShown: !areArgumentsSensitive[i]))
+                        .ToList();
+
+                    return new CommandLineInfo(
+                        Arguments: arguments,
+                        ArgumentsString: argumentsString,
+                        TooltipString: string.Join(" ", arguments.Select(arg => arg.IsShown
+                            ? arg.Value
+                            : DashboardUIHelpers.GetMaskingText(6).Text)));
+                }
+
+                return new CommandLineInfo(Arguments: launchArguments.Select(arg => new LaunchArgument(arg, true)).ToList(), ArgumentsString: argumentsString, TooltipString: argumentsString);
+            }
+
+            if (resourceViewModel.TryGetExecutableArguments(out var executableArguments) && !resourceViewModel.IsProject())
+            {
+                var arguments = executableArguments.IsDefaultOrEmpty ? [] : executableArguments.Select(arg => new LaunchArgument(arg, true)).ToList();
+                var argumentsString = string.Join(" ", executableArguments);
+
+                return new CommandLineInfo(Arguments: arguments, ArgumentsString: argumentsString, TooltipString: argumentsString);
+            }
+
+            return null;
+        }
+
+        static ResourceSourceViewModel CreateResourceSourceViewModel(string value, string path, CommandLineInfo? commandLineInfo)
+        {
+            return commandLineInfo is not null
+                ? new ResourceSourceViewModel(value: value, contentAfterValue: commandLineInfo.Arguments, valueToVisualize: $"{path} {commandLineInfo.ArgumentsString}", tooltip: $"{path} {commandLineInfo.TooltipString}")
+                : new ResourceSourceViewModel(value: value, contentAfterValue: null, valueToVisualize: path, tooltip: path);
+        }
     }
+
+    private record CommandLineInfo(List<LaunchArgument> Arguments, string ArgumentsString, string TooltipString);
 }
 
 public record LaunchArgument(string Value, bool IsShown);
diff --git a/src/Aspire.Dashboard/Utils/DashboardUIHelpers.cs b/src/Aspire.Dashboard/Utils/DashboardUIHelpers.cs
@@ -43,16 +43,25 @@ public static (ColumnResizeLabels resizeLabels, ColumnSortLabels sortLabels) Cre
         return (resizeLabels, sortLabels);
     }
 
-    private static readonly ConcurrentDictionary<int, string> s_cachedMasking = new();
+    private static readonly ConcurrentDictionary<int, TextMask> s_cachedMasking = new();
 
-    public static MarkupString GetMaskingText(int length)
+    public static TextMask GetMaskingText(int length)
     {
-        return new MarkupString(s_cachedMasking.GetOrAdd(length, static i =>
+        return s_cachedMasking.GetOrAdd(length, static i =>
         {
-            const string maskingChar = "&#x25cf;";
-            return new StringBuilder(maskingChar.Length * i)
-              .Insert(0, maskingChar, i)
-              .ToString();
-        }));
+            const string markupMaskingChar = "&#x25cf;";
+            const string textMaskingChar = "●";
+
+            return new TextMask(
+                new MarkupString(Repeat(markupMaskingChar, i)),
+                Repeat(textMaskingChar, i)
+            );
+
+            static string Repeat(string s, int n) => new StringBuilder(s.Length * n)
+                .Insert(0, s, n)
+                .ToString();
+        });
     }
 }
+
+internal record TextMask(MarkupString MarkupString, string Text);
diff --git a/src/Aspire.Hosting/ApplicationModel/ExpressionResolver.cs b/src/Aspire.Hosting/ApplicationModel/ExpressionResolver.cs
@@ -94,7 +94,10 @@ async Task<ResolvedValue> EvalExpressionAsync(ReferenceExpression expr)
             }
         }
 
-        return new(string.Format(CultureInfo.InvariantCulture, expr.Format, args), isSensitive);
+        // Identically to ReferenceExpression.GetValueAsync, we return null if the format is empty
+        var value = expr.Format.Length == 0 ? null : string.Format(CultureInfo.InvariantCulture, expr.Format, args);
+
+        return new ResolvedValue(value, isSensitive);
     }
 
     async Task<ResolvedValue> EvalValueProvider(IValueProvider vp)
@@ -148,22 +151,29 @@ async Task<ResolvedValue> EvalValueProvider(IValueProvider vp)
         return new ResolvedValue(value, false);
     }
 
+    async Task<ResolvedValue> ResolveConnectionStringReferenceAsync(ConnectionStringReference cs)
+    {
+        // We are substituting our own logic for ConnectionStringReference's GetValueAsync.
+        // However, ConnectionStringReference#GetValueAsync will throw if the connection string is not optional but is not present.
+        // To avoid duplicating that logic, we can defer to ConnectionStringReference#GetValueAsync, which will throw if needed.
+        await ((IValueProvider)cs).GetValueAsync(cancellationToken).ConfigureAwait(false);
+
+        return await ResolveInternalAsync(cs.Resource.ConnectionStringExpression).ConfigureAwait(false);
+    }
+
     /// <summary>
-    /// Resolve an expression when it is being used from inside a container.
-    /// So it's either a container-to-container or container-to-exe communication.
+    /// Resolve an expression. When it is being used from inside a container, endpoints may be evaluated (either in a container-to-container or container-to-exe communication).
     /// </summary>
     async ValueTask<ResolvedValue> ResolveInternalAsync(object? value)
     {
-        return (value, sourceIsContainer) switch
+        return value switch
         {
-            (ConnectionStringReference cs, true) => await ResolveInternalAsync(cs.Resource.ConnectionStringExpression).ConfigureAwait(false),
-            (IResourceWithConnectionString cs, true) => await ResolveInternalAsync(cs.ConnectionStringExpression).ConfigureAwait(false),
-            (ReferenceExpression ex, false) => await EvalExpressionAsync(ex).ConfigureAwait(false),
-            (ReferenceExpression ex, true) => await EvalExpressionAsync(ex).ConfigureAwait(false),
-            (EndpointReference endpointReference, true) => new(await EvalEndpointAsync(endpointReference, EndpointProperty.Url).ConfigureAwait(false), false),
-            (EndpointReferenceExpression ep, true) => new(await EvalEndpointAsync(ep.Endpoint, ep.Property).ConfigureAwait(false), false),
-            (IValueProvider vp, false) => await EvalValueProvider(vp).ConfigureAwait(false),
-            (IValueProvider vp, true) => await EvalValueProvider(vp).ConfigureAwait(false),
+            ConnectionStringReference cs => await ResolveConnectionStringReferenceAsync(cs).ConfigureAwait(false),
+            IResourceWithConnectionString cs and not ResourceWithConnectionStringSurrogate => await ResolveInternalAsync(cs.ConnectionStringExpression).ConfigureAwait(false),
+            ReferenceExpression ex => await EvalExpressionAsync(ex).ConfigureAwait(false),
+            EndpointReference endpointReference when sourceIsContainer => new ResolvedValue(await EvalEndpointAsync(endpointReference, EndpointProperty.Url).ConfigureAwait(false), false),
+            EndpointReferenceExpression ep when sourceIsContainer => new ResolvedValue(await EvalEndpointAsync(ep.Endpoint, ep.Property).ConfigureAwait(false), false),
+            IValueProvider vp => await EvalValueProvider(vp).ConfigureAwait(false),
             _ => throw new NotImplementedException()
         };
     }

diff --git a/src/Aspire.Hosting/ApplicationModel/ReferenceExpression.cs b/src/Aspire.Hosting/ApplicationModel/ReferenceExpression.cs
@@ -55,6 +55,7 @@ private ReferenceExpression(string format, IValueProvider[] valueProviders, stri
     /// <returns></returns>
     public async ValueTask<string?> GetValueAsync(CancellationToken cancellationToken)
     {
+        // NOTE: any logical changes to this method should also be made to ExpressionResolver.EvalExpressionAsync
         if (Format.Length == 0)
         {
             return null;

diff --git a/tests/Aspire.Dashboard.Tests/Model/ResourceSourceViewModelTests.cs b/tests/Aspire.Dashboard.Tests/Model/ResourceSourceViewModelTests.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using Aspire.Dashboard.Model;
+using Aspire.Dashboard.Utils;
 using Aspire.Tests.Shared.DashboardModel;
 using Google.Protobuf.WellKnownTypes;
 using Microsoft.Extensions.Logging.Abstractions;
@@ -80,21 +81,22 @@ void AddStringProperty(string propertyName, string? propertyValue)
                 valueToVisualize: "path/to/project arg2",
                 tooltip: "path/to/project arg2"));
 
+        var maskingText = DashboardUIHelpers.GetMaskingText(6).Text;
         // Project with app arguments, as well as a secret (format argument)
         data.Add(new TestData(
                 ResourceType: "Project",
                 ExecutablePath: "path/to/executable",
                 ExecutableArguments: ["arg1", "arg2"],
-                AppArgs: ["arg2", "--key", "secret"],
-                AppArgsSensitivity: [false, false, true],
+                AppArgs: ["arg2", "--key", "secret", "secret2", "notsecret"],
+                AppArgsSensitivity: [false, false, true, true, false],
                 ProjectPath: "path/to/project",
                 ContainerImage: null,
                 SourceProperty: null),
             new ResourceSourceViewModel(
                 value: "project",
-                contentAfterValue: [new LaunchArgument("arg2", true), new LaunchArgument("--key", true), new LaunchArgument("secret", false)],
-                valueToVisualize: "path/to/project arg2 --key secret",
-                tooltip: "path/to/project arg2 --key secret"));
+                contentAfterValue: [new LaunchArgument("arg2", true), new LaunchArgument("--key", true), new LaunchArgument("secret", false), new LaunchArgument("secret2", false), new LaunchArgument("notsecret", true)],
+                valueToVisualize: "path/to/project arg2 --key secret secret2 notsecret",
+                tooltip: $"path/to/project arg2 --key {maskingText} {maskingText} notsecret"));
 
         // Project without executable arguments
         data.Add(new TestData(
-Original file line number
+Diff line change
@@ Expand Up / @@ -24,7 +24,7 @@ @@
                     }
                     else
                     {
-                        <span class="subtext">&nbsp;@DashboardUIHelpers.GetMaskingText(length: 6)</span>
+                        <span class="subtext">&nbsp;@DashboardUIHelpers.GetMaskingText(length: 6).MarkupString</span>
                     }
                 }
             }
@@ Expand Down @@