Skip to content

Hide secrets in source tooltip, correct ExpressionResolver logic for non-containers #7662

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
adamint merged 15 commits into from
Feb 20, 2025
Merged

Hide secrets in source tooltip, correct ExpressionResolver logic for non-containers #7662

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
751ed4a
Replace tuple with record type in source view model, hide secrets in …
Feb 18, 2025
c31bd4e
Remove sourceIsContainer checks in ResolveInternalAsync
Feb 18, 2025
1fcc27a
Fix test
Feb 18, 2025
bc3fe8f
EndpointReference/Expression need to only be container
Feb 18, 2025
247cea7
Apply suggestion
Feb 18, 2025
5f1db82
special case ResourceWithConnectionStringSurrogate
Feb 18, 2025
4357580
Update comment
Feb 18, 2025
844a056
Fix discrepant behavior between ReferenceExpression.GetValueAsync and…
Feb 18, 2025
e102dfe
Invoke GetValueAsync on ConnectionStringReference before resolving it…
Feb 18, 2025
d5505ef
Update src/Aspire.Hosting/ApplicationModel/ExpressionResolver.cs
danmoseley Feb 19, 2025
859060d
Improve comment in ResolveConnectionStringReferenceAsync
Feb 20, 2025
72ceae0
Change DashboardUIHelpers.GetMaskingText to return a record
Feb 20, 2025
07a4b13
clean up ResourceSourceViewModel
Feb 20, 2025
4c27dcc
add additional secrets in test case
Feb 20, 2025
f36a730
Add additional ExpressionResolver tests
Feb 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 29 additions & 18 deletions src/Aspire.Dashboard/Model/ResourceSourceViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Net;
using Aspire.Dashboard.Utils;

namespace Aspire.Dashboard.Model;

public class ResourceSourceViewModel(string value, List<LaunchArgument>? contentAfterValue, string valueToVisualize, string tooltip)
Expand All @@ -12,15 +15,15 @@ public class ResourceSourceViewModel(string value, List<LaunchArgument>? content

internal static ResourceSourceViewModel? GetSourceViewModel(ResourceViewModel resource)
{
(List<LaunchArgument>? Arguments, string? ArgumentsString) commandLineInfo;
CommandLineInfo? 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);
commandLineInfo = null;
}
else
{
Expand All @@ -31,46 +34,52 @@ public class ResourceSourceViewModel(string value, List<LaunchArgument>? content
.Select((arg, i) => new LaunchArgument(arg, IsShown: !areArgumentsSensitive[i]))
.ToList();

commandLineInfo = (Arguments: arguments, argumentsString);
commandLineInfo = new CommandLineInfo(
Arguments: arguments,
ArgumentsString: argumentsString,
TooltipString: string.Join(" ", arguments.Select(arg => arg.IsShown
? arg.Value
: WebUtility.HtmlDecode(DashboardUIHelpers.GetMaskingText(6).Value))));
}
else
{
commandLineInfo = (Arguments: launchArguments.Select(arg => new LaunchArgument(arg, true)).ToList(), argumentsString);
commandLineInfo = new CommandLineInfo(Arguments: launchArguments.Select(arg => new LaunchArgument(arg, true)).ToList(), ArgumentsString: argumentsString, TooltipString: 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));
var arguments = executableArguments.IsDefaultOrEmpty ? [] : executableArguments.Select(arg => new LaunchArgument(arg, true)).ToList();
var argumentsString = string.Join(" ", executableArguments);

commandLineInfo = new CommandLineInfo(Arguments: arguments, ArgumentsString: argumentsString, TooltipString: argumentsString);
}
else
{
commandLineInfo = (Arguments: null, null);
commandLineInfo = null;
}

// 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 commandLineInfo is not null
? new ResourceSourceViewModel(value: Path.GetFileName(projectPath), contentAfterValue: commandLineInfo.Arguments, valueToVisualize: $"{projectPath} {commandLineInfo.ArgumentsString}", tooltip: $"{projectPath} {commandLineInfo.TooltipString}")
// default to project path if there is no executable path or executable arguments
: new ResourceSourceViewModel(value: Path.GetFileName(projectPath), contentAfterValue: null, valueToVisualize: projectPath, tooltip: projectPath);
}

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 commandLineInfo is not null
? new ResourceSourceViewModel(value: Path.GetFileName(executablePath), contentAfterValue: commandLineInfo.Arguments, valueToVisualize: $"{executablePath} {commandLineInfo.ArgumentsString}", tooltip: $"{executablePath} {commandLineInfo.TooltipString}")
: new ResourceSourceViewModel(value: Path.GetFileName(executablePath), contentAfterValue: null, valueToVisualize: executablePath, tooltip: executablePath);
}

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 commandLineInfo is not null
? new ResourceSourceViewModel(value: containerImage, contentAfterValue: commandLineInfo.Arguments, valueToVisualize: $"{containerImage} {commandLineInfo.ArgumentsString}", tooltip: $"{containerImage} {commandLineInfo.TooltipString}")
: new ResourceSourceViewModel(value: containerImage, contentAfterValue: null, valueToVisualize: containerImage, tooltip: containerImage);
}

if (resource.Properties.TryGetValue(KnownProperties.Resource.Source, out var property) && property.Value is { HasStringValue: true, StringValue: var value })
Expand All @@ -80,6 +89,8 @@ public class ResourceSourceViewModel(string value, List<LaunchArgument>? content

return null;
}

private record CommandLineInfo(List<LaunchArgument> Arguments, string ArgumentsString, string TooltipString);
}

public record LaunchArgument(string Value, bool IsShown);
16 changes: 7 additions & 9 deletions src/Aspire.Hosting/ApplicationModel/ExpressionResolver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,16 +154,14 @@ async Task<ResolvedValue> EvalValueProvider(IValueProvider vp)
/// </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 ResolveInternalAsync(cs.Resource.ConnectionStringExpression).ConfigureAwait(false),
IResourceWithConnectionString cs => await ResolveInternalAsync(cs.ConnectionStringExpression).ConfigureAwait(false),
Copy link
Member

@eerhardt eerhardt Feb 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davidfowl @mitchdenny @davidebbo - do we even need these 2 cases? Since ConnectionStringReference and IResourceWithConnectionString implement IValueProvider, shouldn't this just fall to that case?

(Note we don't necessarily need to change this in this PR, but I'm just trying to understand if I have the logic correct. If we are going to backport this to 9.1, we may not want to risk a change here.)

Copy link
Member

@eerhardt eerhardt Feb 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, I think I understand why this is necessary - because we want to intercept any "host" references inside the ConnectionString and redirect them to the containerHostName.

Would it ever work if someone implemented an IValueProvider that contained a "host" reference, but wasn't one of these?

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()
};
}
Expand Down
4 changes: 3 additions & 1 deletion tests/Aspire.Dashboard.Tests/Model/ResourceSourceViewModelTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Net;
using Aspire.Dashboard.Model;
using Aspire.Dashboard.Utils;
using Aspire.Tests.Shared.DashboardModel;
using Google.Protobuf.WellKnownTypes;
using Microsoft.Extensions.Logging.Abstractions;
Expand Down Expand Up @@ -94,7 +96,7 @@ void AddStringProperty(string propertyName, string? propertyValue)
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"));
tooltip: $"path/to/project arg2 --key {WebUtility.HtmlDecode(DashboardUIHelpers.GetMaskingText(6).Value)}"));
Copy link
Member

@eerhardt eerhardt Feb 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there other tests that should be added for this change? Specifically for the case(s) where we are changing ExpressionResolver - to catch secrets inside connection strings.

Copy link
Member Author

@adamint adamint Feb 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added those tests in ExpressionResolverTests


// Project without executable arguments
data.Add(new TestData(
Expand Down
Loading