Fix Spectre markup escaping bugs across CLI commands #14422
Merged
+215
−62
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 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
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 14422Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 14422" |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes multiple Spectre.Console markup-escaping issues in Aspire CLI output/prompting paths to prevent crashes or corrupted rendering when external data contains markup characters (notably [ / ]), aligning with the root cause described in #13955.
Changes:
- Escapes external data used in Spectre markup output across
run,logs, and telemetry commands. - Escapes choice display text in
ConsoleInteractionServiceselection prompts and escapes version/capability strings in version-related messages. - Adds tests covering markup escaping behavior for several interaction-service output paths and a publish prompting scenario.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Cli.Tests/Interaction/ConsoleInteractionServiceTests.cs | Adds tests validating escaping behavior for version/error/message output paths. |
| tests/Aspire.Cli.Tests/Commands/PublishCommandPromptingIntegrationTests.cs | Adds an integration test scenario involving bracketed choice labels (Azure subscription-like strings). |
| src/Aspire.Cli/Interaction/ConsoleInteractionService.cs | Escapes selection prompt converter output; escapes some version/capability values in markup output. |
| src/Aspire.Cli/Commands/TelemetrySpansCommand.cs | Escapes resource name in span output markup. |
| src/Aspire.Cli/Commands/TelemetryLogsCommand.cs | Escapes resource name in log output markup. |
| src/Aspire.Cli/Commands/RunCommand.cs | Escapes resource/endpoint display in endpoint grid; adjusts escaping responsibilities between DisplayError and DisplayMessage call sites. |
| src/Aspire.Cli/Commands/LogsCommand.cs | Escapes resource name in colorized log prefix. |
tests/Aspire.Cli.Tests/Commands/PublishCommandPromptingIntegrationTests.cs
Outdated
Show resolved
Hide resolved
mitchdenny
approved these changes
Feb 10, 2026
Member
mitchdenny
left a comment
mitchdenny
left a comment
There was a problem hiding this comment.
Looks good! The fix correctly moves escaping to the right boundary — caller-side for DisplayMessage (which passes through to MarkupLine), removed from DisplayError callers (which escapes internally). Tests are thorough and document the contract well.
One minor nit: cliInformationalVersion on line 193 of ConsoleInteractionService.cs is not escaped while the adjacent appHostHostingVersion and ex.RequiredCapability are — worth a quick fix for consistency.
Contributor
🎬 CLI E2E Test RecordingsThe following terminal recordings are available for commit
📹 Recordings uploaded automatically from CI run #21855293083 |
- 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)
davidfowl
force-pushed
the
davidfowl/fix-cli-output
branch
from
054ba91 to
a3130b9
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fixes multiple Spectre Console markup escaping bugs across CLI commands that could cause crashes or garbled output when resource names, file paths, version strings, or other external data contain characters that Spectre interprets as markup (e.g., square brackets
[], angle brackets<>).Root cause: Several places in the CLI pass user/external data directly into
AnsiConsole.MarkupLine()orSelectionPrompt.UseConverter()without calling.EscapeMarkup()first. When that data contains[brackets], Spectre treats them as formatting directives, causingInvalidOperationExceptionor corrupted output.Changes:
.EscapeMarkup()from exception messages (which feed intoDisplayError, which already escapes) to log file paths (which feed intoDisplayMessage, which does not).PromptForSelectionAsyncandPromptForSelectionsAsyncconverters. Escape version strings and capability names inDisplayIncompatibleVersionErrorandDisplayVersionUpdateNotification.Fixes #13955
Checklist