Skip to content

Commit 0e2fce7

Browse files
authored
Fix Spectre markup escaping bugs across CLI commands (#14422)
* Add failing tests for unescaped Spectre markup in CLI output * Fix Spectre markup escaping bugs across CLI commands - ConsoleInteractionService: Escape appHostHostingVersion, RequiredCapability in DisplayIncompatibleVersionError, and newerVersion/updateCommand in DisplayVersionUpdateNotification - RunCommand: Remove double-escaping of ex.Message before DisplayError (lines 355,362,371), escape resource/endpoint names in Markup (line 313), escape log file paths in DisplayMessage (lines 366,375,850) - LogsCommand: Escape logLine.ResourceName in MarkupLine (line 337) - TelemetryLogsCommand/TelemetrySpansCommand: Escape resourceName in MarkupLine (lines 261,267) - Add tests: DisplayError double-escape verification, DisplayMessage with escaped/unescaped paths, DisplaySubtleMessage default escaping, choice prompt with bracket-containing Azure subscription names * Fix Spectre markup escaping: escape link targets and CLI version string - RunCommand: escape endpoint URL in link= attribute (IPv6 [::1] URLs crash Spectre) - ConsoleInteractionService: escape cliInformationalVersion (brackets in build metadata crash Spectre) - Verified: LogsCommand double-escaping is NOT a bug (both approaches produce identical markup) * Remove tests that don't exercise actual Spectre escaping * Fix DisplayIncompatibleVersionError showing capability name instead of hosting version
1 parent ea1aa0d commit 0e2fce7

File tree

9 files changed

+215
-62
lines changed

9 files changed

+215
-62
lines changed

src/Aspire.Cli/Backchannel/AppHostIncompatibleException.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33

44
namespace Aspire.Cli.Backchannel;
55

6-
internal class AppHostIncompatibleException(string message, string requiredCapability) : Exception(message)
6+
internal class AppHostIncompatibleException(string message, string requiredCapability, string? aspireHostingVersion = null) : Exception(message)
77
{
88
public string RequiredCapability { get; } = requiredCapability;
9+
public string? AspireHostingVersion { get; } = aspireHostingVersion;
910
}

src/Aspire.Cli/Commands/LogsCommand.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ private void OutputLogLine(ResourceLogLine logLine, OutputFormat format)
334334
// Colorized output: assign a consistent color to each resource
335335
var color = GetResourceColor(logLine.ResourceName);
336336
var escapedContent = logLine.Content.EscapeMarkup();
337-
AnsiConsole.MarkupLine($"[{color}][[{logLine.ResourceName}]][/] {escapedContent}");
337+
AnsiConsole.MarkupLine($"[{color}][[{logLine.ResourceName.EscapeMarkup()}]][/] {escapedContent}");
338338
}
339339
else
340340
{

src/Aspire.Cli/Commands/RunCommand.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ protected override async Task<int> ExecuteAsync(ParseResult parseResult, Cancell
310310

311311
endpointsGrid.AddRow(
312312
firstEndpoint ? new Align(new Markup($"[bold green]{endpointsLocalizedString}[/]:"), HorizontalAlignment.Right) : Text.Empty,
313-
new Markup($"[bold]{resource}[/] [grey]has endpoint[/] [link={endpoint}]{endpoint}[/]")
313+
new Markup($"[bold]{resource.EscapeMarkup()}[/] [grey]has endpoint[/] [link={endpoint.EscapeMarkup()}]{endpoint.EscapeMarkup()}[/]")
314314
);
315315

316316
var endpointsPadder = new Padder(endpointsGrid, new Padding(3, 0));
@@ -348,31 +348,31 @@ protected override async Task<int> ExecuteAsync(ParseResult parseResult, Cancell
348348
catch (AppHostIncompatibleException ex)
349349
{
350350
Telemetry.RecordError(ex.Message, ex);
351-
return InteractionService.DisplayIncompatibleVersionError(ex, ex.RequiredCapability);
351+
return InteractionService.DisplayIncompatibleVersionError(ex, ex.AspireHostingVersion ?? ex.RequiredCapability);
352352
}
353353
catch (CertificateServiceException ex)
354354
{
355-
var errorMessage = string.Format(CultureInfo.CurrentCulture, TemplatingStrings.CertificateTrustError, ex.Message.EscapeMarkup());
355+
var errorMessage = string.Format(CultureInfo.CurrentCulture, TemplatingStrings.CertificateTrustError, ex.Message);
356356
Telemetry.RecordError(errorMessage, ex);
357357
InteractionService.DisplayError(errorMessage);
358358
return ExitCodeConstants.FailedToTrustCertificates;
359359
}
360360
catch (FailedToConnectBackchannelConnection ex)
361361
{
362-
var errorMessage = string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.ErrorConnectingToAppHost, ex.Message.EscapeMarkup());
362+
var errorMessage = string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.ErrorConnectingToAppHost, ex.Message);
363363
Telemetry.RecordError(errorMessage, ex);
364364
InteractionService.DisplayError(errorMessage);
365365
// Don't display raw output - it's already in the log file
366-
InteractionService.DisplayMessage("page_facing_up", string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.SeeLogsAt, ExecutionContext.LogFilePath));
366+
InteractionService.DisplayMessage("page_facing_up", string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.SeeLogsAt, ExecutionContext.LogFilePath.EscapeMarkup()));
367367
return ExitCodeConstants.FailedToDotnetRunAppHost;
368368
}
369369
catch (Exception ex)
370370
{
371-
var errorMessage = string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.UnexpectedErrorOccurred, ex.Message.EscapeMarkup());
371+
var errorMessage = string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.UnexpectedErrorOccurred, ex.Message);
372372
Telemetry.RecordError(errorMessage, ex);
373373
InteractionService.DisplayError(errorMessage);
374374
// Don't display raw output - it's already in the log file
375-
InteractionService.DisplayMessage("page_facing_up", string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.SeeLogsAt, ExecutionContext.LogFilePath));
375+
InteractionService.DisplayMessage("page_facing_up", string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.SeeLogsAt, ExecutionContext.LogFilePath.EscapeMarkup()));
376376
return ExitCodeConstants.FailedToDotnetRunAppHost;
377377
}
378378
}
@@ -850,7 +850,7 @@ private async Task<int> ExecuteDetachedAsync(ParseResult parseResult, FileInfo?
850850
_interactionService.DisplayMessage("magnifying_glass_tilted_right", string.Format(
851851
CultureInfo.CurrentCulture,
852852
RunCommandStrings.CheckLogsForDetails,
853-
_fileLoggerProvider.LogFilePath));
853+
_fileLoggerProvider.LogFilePath.EscapeMarkup()));
854854

855855
return ExitCodeConstants.FailedToDotnetRunAppHost;
856856
}

src/Aspire.Cli/Commands/TelemetryLogsCommand.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,6 @@ private static void DisplayLogEntry(string resourceName, OtlpLogRecordJson log)
258258
var severityColor = TelemetryCommandHelpers.GetSeverityColor(log.SeverityNumber);
259259

260260
var escapedBody = body.EscapeMarkup();
261-
AnsiConsole.MarkupLine($"[grey]{timestamp}[/] [{severityColor}]{severity,-5}[/] [cyan]{resourceName}[/] {escapedBody}");
261+
AnsiConsole.MarkupLine($"[grey]{timestamp}[/] [{severityColor}]{severity,-5}[/] [cyan]{resourceName.EscapeMarkup()}[/] {escapedBody}");
262262
}
263263
}

src/Aspire.Cli/Commands/TelemetrySpansCommand.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,6 @@ private static void DisplaySpanEntry(string resourceName, OtlpSpanJson span)
264264
var durationStr = TelemetryCommandHelpers.FormatDuration(duration);
265265

266266
var escapedName = name.EscapeMarkup();
267-
AnsiConsole.MarkupLine($"[grey]{shortSpanId}[/] [cyan]{resourceName,-15}[/] [{statusColor}]{statusText}[/] [white]{durationStr,8}[/] {escapedName}");
267+
AnsiConsole.MarkupLine($"[grey]{shortSpanId}[/] [cyan]{resourceName.EscapeMarkup(),-15}[/] [{statusColor}]{statusText}[/] [white]{durationStr,8}[/] {escapedName}");
268268
}
269269
}

src/Aspire.Cli/Interaction/ConsoleInteractionService.cs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ public async Task<T> PromptForSelectionAsync<T>(string promptText, IEnumerable<T
145145

146146
var prompt = new SelectionPrompt<T>()
147147
.Title(promptText)
148-
.UseConverter(choiceFormatter)
148+
.UseConverter(item => choiceFormatter(item).EscapeMarkup())
149149
.AddChoices(choices)
150150
.PageSize(10)
151151
.EnableSearch();
@@ -174,7 +174,7 @@ public async Task<IReadOnlyList<T>> PromptForSelectionsAsync<T>(string promptTex
174174

175175
var prompt = new MultiSelectionPrompt<T>()
176176
.Title(promptText)
177-
.UseConverter(choiceFormatter)
177+
.UseConverter(item => choiceFormatter(item).EscapeMarkup())
178178
.AddChoices(choices)
179179
.PageSize(10);
180180

@@ -189,9 +189,9 @@ public int DisplayIncompatibleVersionError(AppHostIncompatibleException ex, stri
189189
DisplayError(InteractionServiceStrings.AppHostNotCompatibleConsiderUpgrading);
190190
Console.WriteLine();
191191
_outConsole.MarkupLine(
192-
$"\t[bold]{InteractionServiceStrings.AspireHostingSDKVersion}[/]: {appHostHostingVersion}");
193-
_outConsole.MarkupLine($"\t[bold]{InteractionServiceStrings.AspireCLIVersion}[/]: {cliInformationalVersion}");
194-
_outConsole.MarkupLine($"\t[bold]{InteractionServiceStrings.RequiredCapability}[/]: {ex.RequiredCapability}");
192+
$"\t[bold]{InteractionServiceStrings.AspireHostingSDKVersion}[/]: {appHostHostingVersion.EscapeMarkup()}");
193+
_outConsole.MarkupLine($"\t[bold]{InteractionServiceStrings.AspireCLIVersion}[/]: {cliInformationalVersion.EscapeMarkup()}");
194+
_outConsole.MarkupLine($"\t[bold]{InteractionServiceStrings.RequiredCapability}[/]: {ex.RequiredCapability.EscapeMarkup()}");
195195
Console.WriteLine();
196196
return ExitCodeConstants.AppHostIncompatible;
197197
}
@@ -303,11 +303,11 @@ public void DisplayVersionUpdateNotification(string newerVersion, string? update
303303
{
304304
// Write to stderr to avoid corrupting stdout when JSON output is used
305305
_errorConsole.WriteLine();
306-
_errorConsole.MarkupLine(string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.NewCliVersionAvailable, newerVersion));
306+
_errorConsole.MarkupLine(string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.NewCliVersionAvailable, newerVersion.EscapeMarkup()));
307307

308308
if (!string.IsNullOrEmpty(updateCommand))
309309
{
310-
_errorConsole.MarkupLine(string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.ToUpdateRunCommand, updateCommand));
310+
_errorConsole.MarkupLine(string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.ToUpdateRunCommand, updateCommand.EscapeMarkup()));
311311
}
312312

313313
_errorConsole.MarkupLine(string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.MoreInfoNewCliVersion, UpdateUrl));

src/Aspire.Cli/Projects/DotNetAppHostProject.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,8 @@ public async Task<int> PublishAsync(PublishContext context, CancellationToken ca
379379
{
380380
var exception = new AppHostIncompatibleException(
381381
$"The app host is not compatible. Aspire.Hosting version: {compatibilityCheck.AspireHostingVersion}",
382-
"Aspire.Hosting");
382+
"Aspire.Hosting",
383+
compatibilityCheck.AspireHostingVersion);
383384
// Signal the backchannel completion source so the caller doesn't wait forever
384385
context.BackchannelCompletionSource?.TrySetException(exception);
385386
throw exception;

tests/Aspire.Cli.Tests/Commands/PublishCommandPromptingIntegrationTests.cs

Lines changed: 0 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -720,48 +720,6 @@ public async Task PublishCommand_SingleInputPrompt_WhenStatusTextEqualsLabel_Sho
720720
// Should show: [bold]Environment Name[/]
721721
Assert.Equal("[bold]Environment Name[/]", promptCall.PromptText);
722722
}
723-
724-
[Fact]
725-
public async Task PublishCommand_SingleInputPrompt_EscapesSpectreMarkupInLabels()
726-
{
727-
// Arrange
728-
using var workspace = TemporaryWorkspace.Create(outputHelper);
729-
var promptBackchannel = new TestPromptBackchannel();
730-
var consoleService = new TestConsoleInteractionServiceWithPromptTracking();
731-
732-
// Set up a single-input prompt with Spectre markup characters in both StatusText and Label
733-
promptBackchannel.AddPrompt("markup-prompt", "Value [required]", InputTypes.Text, "Enter value [1-10]", isRequired: true);
734-
735-
// Set up the expected user response
736-
consoleService.SetupStringPromptResponse("5");
737-
738-
var services = CliTestHelper.CreateServiceCollection(workspace, outputHelper, options =>
739-
{
740-
options.ProjectLocatorFactory = (sp) => new TestProjectLocator();
741-
options.DotNetCliRunnerFactory = (sp) => CreateTestRunnerWithPromptBackchannel(promptBackchannel);
742-
});
743-
744-
services.AddSingleton<IInteractionService>(consoleService);
745-
746-
var serviceProvider = services.BuildServiceProvider();
747-
var command = serviceProvider.GetRequiredService<RootCommand>();
748-
749-
// Act
750-
var result = command.Parse("publish");
751-
var exitCode = await result.InvokeAsync().DefaultTimeout();
752-
753-
// Assert
754-
Assert.Equal(0, exitCode);
755-
756-
// Verify that square brackets are properly escaped
757-
var promptCalls = consoleService.StringPromptCalls;
758-
Assert.Single(promptCalls);
759-
var promptCall = promptCalls[0];
760-
761-
// Square brackets should be escaped to [[bracket]]
762-
Assert.Contains("[[1-10]]", promptCall.PromptText);
763-
Assert.Contains("[[required]]", promptCall.PromptText);
764-
}
765723
}
766724

767725
// Test implementation of IAppHostCliBackchannel that simulates prompt interactions

0 commit comments

Comments
 (0)