Skip to content

Reject numeric persistence mode values in dashboard configuration - #19966

Open
Muhammad Bilal (MuhammadBilal64) wants to merge 5 commits into
microsoft:mainfrom
MuhammadBilal64:fix-persistence-numeric-validation
Open

Muhammad Bilal (MuhammadBilal64) wants to merge 5 commits into
microsoft:mainfrom
MuhammadBilal64:fix-persistence-numeric-validation

Conversation

@MuhammadBilal64

Copy link
Copy Markdown

Description

PostConfigureDashboardOptions uses Enum.TryParse followed by Enum.IsDefined to validate the --persistence value. Enum.TryParse accepts numeric strings that map to defined enum values, so --persistence 1 silently succeeded and initialized the Dashboard in Run mode, even though only the named values None, Run, and Resume are documented as valid.

This adds a check to reject numeric input before attempting the enum parse, so numeric values are now correctly rejected with the existing parse-error message. Added a regression test covering this case.

Fixes #19781

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No

  Fixes microsoft#19781

`Enum.TryParse` accepts numeric strings that map to defined enum
  values, so `--persistence 1` silently succeeded as `Run` even
  though only named values (None, Run, Resume) are documented.

  Added a check to reject numeric input before parsing, plus a
  regression test.
  Fixes microsoft#19781

`Enum.TryParse` accepts numeric strings that map to defined enum
  values, so `--persistence 1` silently succeeded as `Run` even
  though only named values (None, Run, Resume) are documented.

  Added a check to reject numeric input before parsing, plus a
  regression test.
Copilot AI balanced review requested due to automatic review settings September 5, 2026 18:40
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19966

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19966"

@MuhammadBilal64

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused validation change correctly addresses the reported issue and includes appropriate regression coverage.

Pull request overview

Rejects numeric dashboard persistence modes while preserving named values.

Changes:

  • Rejects numeric values before enum parsing.
  • Adds regression coverage for --persistence 1.
File summaries
File Description
src/Aspire.Dashboard/Configuration/PostConfigureDashboardOptions.cs Rejects numeric persistence modes.
tests/Aspire.Dashboard.Tests/DashboardOptionsTests.cs Verifies numeric input produces a validation error.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

@MuhammadBilal64

Copy link
Copy Markdown
Author

Hi James Newton-King (@JamesNK) Adam Ratzman (@adamint), gentle reminder on this PR when you get a chance. Would appreciate a review. Thanks!

@MuhammadBilal64

Copy link
Copy Markdown
Author

Hi James Newton-King (@JamesNK) Adam Ratzman (@adamint), following up on this PR since it’s still awaiting a maintainer review. Copilot’s review found no issues and recommended approval. When you get a chance, could you please take a look? Thanks!

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Dashboard] Reject numeric persistence mode values

2 participants