Skip to content

Constrain aspire-starter port replacers to localhost: URLs - #20031

Open
Bart Koelman (bart-vmware) wants to merge 1 commit into
microsoft:mainfrom
SteeltoeOSS:fix-dynamic-port-replacements
Open

Constrain aspire-starter port replacers to localhost: URLs#20031
Bart Koelman (bart-vmware) wants to merge 1 commit into
microsoft:mainfrom
SteeltoeOSS:fix-dynamic-port-replacements

Conversation

@bart-vmware

Copy link
Copy Markdown
Contributor

Description

The dynamic port symbols (webHttpPortReplacer, apiServiceHttpPortReplacer, appHostHttpPortReplacer, etc.) used a bare numeric "replaces" value, so the template engine substituted that number everywhere in generated content, not just in launchSettings.json. Since the default ports (5000, 7000, 5301, 7301, 15000, 17000, 19000, 20000, 21000, 22000) also occur as plain numeric literals in vendored files such as wwwroot/lib/bootstrap/dist/js/bootstrap.bundle.js (e.g. carousel interval: 5000), those files ended up with churned values that differ between generated projects, making diffs across template runs noisy.

Add "onlyIf": [{"after": "localhost:"}] to each port replacer so substitution only fires in "localhost:" contexts, matching the fix already applied upstream for the same class of bug (dotnet/aspnetcore#65165, dotnet/sdk#48811).

Fixes #20030

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, tested manually (see below)
  • 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

Manual testing outcome

I tested it by installing both the pre-fix and post-fix versions of the aspire-starter template locally (via dotnet new install, using the global .NET 10 SDK on the machine to avoid needing this repo's pinned .NET 11 RC1 SDK) and generating two projects with deliberately different ports each time.

Before the fix (template.json at HEAD~1), generating with --webHttpPort 5010 vs --webHttpPort 5222 produced two bootstrap.bundle.js files that differed:

1131c1131
<     interval: 5010,
---
>     interval: 5222,
6140c6140
<     delay: 5010
---
>     delay: 5222

This reproduces the exact bug from #20030 — the port number leaking into an unrelated numeric literal in the vendored JS file.

After the fix (current working tree), the same two-port test produced launchSettings.json files with the expected different ports (5010 vs 5222), but bootstrap.bundle.js was byte-for-byte identical between the two generated projects.

That confirms the onlyIf: [{"after": "localhost:"}] constraint does what it's supposed to: the port substitution now only fires in localhost:<port> contexts and no longer corrupts unrelated numeric literals elsewhere in the template output.

Note

This PR was produced using Sonnet 5 (effort High) in Claude Code.

The dynamic port symbols (webHttpPortReplacer, apiServiceHttpPortReplacer,
appHostHttpPortReplacer, etc.) used a bare numeric "replaces" value, so the
template engine substituted that number everywhere in generated content, not
just in launchSettings.json. Since the default ports (5000, 7000, 5301, 7301,
15000, 17000, 19000, 20000, 21000, 22000) also occur as plain numeric literals
in vendored files such as wwwroot/lib/bootstrap/dist/js/bootstrap.bundle.js
(e.g. carousel interval: 5000), those files ended up with churned values that
differ between generated projects, making diffs across template runs noisy.

Add "onlyIf": [{"after": "localhost:"}] to each port replacer so substitution
only fires in "localhost:<port>" contexts, matching the fix already applied
upstream for the same class of bug (dotnet/aspnetcore#65165, dotnet/sdk#48811).

Fixes microsoft#20030
@github-actions

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 -- 20031

Or

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

@github-actions
github-actions Bot requested a balanced review from Copilot September 10, 2026 10:18
@bart-vmware

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree company="Broadcom"

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.

🟡 Changes recommended

The behavioral bug fix lacks automated regression coverage in the existing template test suite.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Constrains Aspire Starter port replacement to localhost URLs, preventing unintended modifications to vendored assets.

Changes:

  • Adds onlyIf constraints to all ten port replacers.
  • Preserves unrelated numeric literals such as Bootstrap timing values.
File summaries
File Description
src/Aspire.ProjectTemplates/templates/aspire-starter/.template.config/template.json Restricts port substitutions to localhost: contexts.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment on lines +338 to +340
"onlyIf": [{
"after": "localhost:"
}]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I decided to test only manually because the upstream fix didn't add tests either. Please let me know when adding an automated test is a prerequisite to get this merged.

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.

Randomized contents of bootstrap.bundle.js in project template

2 participants