Fix npm registry hard-coded to internal dnceng feed (#19370) - #19388
Conversation
PR #18858 set the internal dnceng Azure Artifacts feed as the customer default for npm operations. This breaks `aspire agent init`: npm resolves a tarball's transitive dependencies through --registry, and the dnceng pull-through feed returns 401 anonymously for packages it hasn't cached (e.g. playwright / playwright-core), so `npm install -g <.tgz>` fails with E401. It also shipped the internal feed as the default in generated Dockerfiles and `aspire new` templates. Restore the public npm registry (https://registry.npmjs.org/) as the customer-facing default across: - Aspire.Cli NpmRunner (npm view/pack/install pass --registry) - Generated pnpm Dockerfile NPM_REGISTRY default (still overridable at build time via --build-arg NPM_REGISTRY=...) - Shipped ts-starter / py-starter / aspire-ts-cs-starter template lockfiles (resolved URL host only; versions and integrity hashes are unchanged) Security hardening is retained: --ignore-scripts on the global install and Sigstore provenance + SRI integrity verification. Repo-internal lockfiles and CI images intentionally remain on the internal feed. Fixes #19370 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19388Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19388" |
github-actions
Bot
added
the
needs-area-label
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Restores the public npm registry for customer-facing CLI, Dockerfile, and template workflows while retaining package-integrity protections.
Changes:
- Uses
registry.npmjs.orgfor CLI package operations and generated Dockerfiles. - Updates shipped template lockfiles without changing versions or integrity hashes.
- Adds regression tests and refreshes Dockerfile snapshots.
Reviewed changes
Copilot reviewed 11 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/Aspire.Cli/Npm/NpmRunner.cs |
Restores the public registry for npm operations. |
src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs |
Changes generated Dockerfiles’ default registry. |
src/Aspire.Cli/Templating/Templates/ts-starter/package-lock.json |
Repoints TypeScript starter dependencies. |
src/Aspire.Cli/Templating/Templates/py-starter/package-lock.json |
Repoints Python starter dependencies. |
src/Aspire.ProjectTemplates/templates/aspire-ts-cs-starter/frontend/package-lock.json |
Repoints frontend dependencies. |
tests/Aspire.Cli.Tests/Npm/NpmRunnerTests.cs |
Verifies public-registry CLI arguments. |
tests/Aspire.Cli.Tests/Templating/TemplatePackageLockTests.cs |
Guards shipped lockfile registry hosts. |
tests/Aspire.Hosting.JavaScript.Tests/AddJavaScriptAppTests.cs |
Updates Dockerfile registry assertions. |
tests/Aspire.Hosting.JavaScript.Tests/Snapshots/AddViteAppTests.VerifyDockerfileWhenPackageScriptUsesPnpm.verified.txt |
Updates pnpm Dockerfile snapshot. |
tests/Aspire.Hosting.JavaScript.Tests/Snapshots/AddViteAppTests.VerifyDockerfileWhenNextJsAppUsesPnpm.verified.txt |
Updates Next.js snapshot. |
tests/Aspire.Hosting.JavaScript.Tests/Snapshots/AddJavaScriptAppTests.VerifyPnpmDockerfileWhenPublishedAsPackageScript_hasLockFile=True.verified.txt |
Updates package-script snapshot. |
tests/Aspire.Hosting.JavaScript.Tests/Snapshots/AddJavaScriptAppTests.VerifyPnpmDockerfileWhenPublishedAsPackageScript_hasLockFile=False.verified.txt |
Updates no-lockfile package-script snapshot. |
tests/Aspire.Hosting.JavaScript.Tests/Snapshots/AddJavaScriptAppTests.VerifyPnpmDockerfile_hasLockFile=True.verified.txt |
Updates lockfile snapshot. |
tests/Aspire.Hosting.JavaScript.Tests/Snapshots/AddJavaScriptAppTests.VerifyPnpmDockerfile_hasLockFile=False.verified.txt |
Updates no-lockfile snapshot. |
Files not reviewed (3)
- src/Aspire.Cli/Templating/Templates/py-starter/package-lock.json: Generated file
- src/Aspire.Cli/Templating/Templates/ts-starter/package-lock.json: Generated file
- src/Aspire.ProjectTemplates/templates/aspire-ts-cs-starter/frontend/package-lock.json: Generated file
Adam Ratzman (adamint)
left a comment
Adam Ratzman (adamint)
left a comment
There was a problem hiding this comment.
Reviewed cf938341.
I left two fixable findings inline: the security spec/diagnostics still describe the internal feed, and the new ProjectTemplates lockfile guard is not selected for a ProjectTemplates-only change.
Dogfood proof passed: CLI 13.6.0-pr.19388.gcf938341, aspire agent init bypassed an unreachable .npmrc, all three changed starters completed npm ci, generated Dockerfiles built with both the public default and an override, and targeted CLI/JavaScript tests passed.
…blic registry and route ProjectTemplates lockfile guard The code fix for #19370 already restores the public npm registry as the customer-facing default. This follow-up resolves reviewer feedback about stale docs/comments and CI test routing that lagged behind that change: - docs/specs/safe-npm-tool-install.md: the security spec still described the internal dotnet-public-npm Azure Artifacts feed as the resolution source (trust-anchor table, Step 1/3 actions, trust basis, limitations, the verification-chain diagram, and the implementation constant). Updated all of these to the public npm registry (registry.npmjs.org) so the spec matches shipped behavior. - src/Aspire.Cli/Npm/INpmRunner.cs and src/Aspire.Cli/Agents/Playwright/PlaywrightCliInstaller.cs: the ResolvePackageAsync doc comment, a Step 1 comment, and a LogDebug message still said "internal npm registry". Corrected to "public npm registry". - tests/Aspire.Cli.Tests/Templating/TemplatePackageLockTests.cs: rewrote the StarterAppHostPackageLock_UsesPublicNpmRegistry comment to state the current invariant (shipped lockfiles pin the resolved registry for deps restored in a generated starter, so they must resolve from the public registry) instead of narrating prior-PR history. - eng/github-ci/test-trigger-map.yml: added an additive path rule routing src/Aspire.ProjectTemplates/templates/aspire-ts-cs-starter/frontend/package-lock.json to test:Aspire.Cli.Tests, so a lockfile-only edit runs the ProjectTemplateFrontendPackageLock_UsesPublicNpmRegistry guard (the existing src/Aspire.ProjectTemplates/** rule only builds Aspire.Templates.Tests). No behavior or registry-value changes; docs/comments/CI-routing only. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fa48dbbe-0b88-44d0-8402-3699a48cd862
Tests selector (audit mode)The full test matrix and all jobs still run in audit mode. The tests and jobs below are what selective CI would run under enforcement. 13 / 100 test projects · 6 jobs, from 17 changed files. Selected test projects (13 / 100)
Selected jobs (6)
How these were chosen — grouped by what changed
🔧 🔧 📦 affected project 📄 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 Job reasons
Selection computed for commit |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 18 changed files in this pull request and generated no new comments.
Files not reviewed (3)
- src/Aspire.Cli/Templating/Templates/py-starter/package-lock.json: Generated file
- src/Aspire.Cli/Templating/Templates/ts-starter/package-lock.json: Generated file
- src/Aspire.ProjectTemplates/templates/aspire-ts-cs-starter/frontend/package-lock.json: Generated file
Adam Ratzman (adamint)
left a comment
Adam Ratzman (adamint)
left a comment
There was a problem hiding this comment.
Re-reviewed 5adae2ada5 against cf9383416.
Both findings are addressed: the npm trust documentation/diagnostics now match the public registry, and the exact ProjectTemplates lockfile path now selects Aspire.Cli.Tests in addition to the existing template coverage. The three threads are resolved.
TestTriggerMapTests passed 15/15, and exact selector validation includes both Aspire.Cli.Tests and Aspire.Templates.Tests. Prior dogfood proof remains valid because no user-facing npm or generated-Dockerfile behavior changed. The current failed template check is an unrelated Azure Artifacts 503 during restore, not a test or PR failure.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
/backport release/13.5 |
|
/backport to release/13.5 |
|
Started backporting to |
|
Jose Perez Rodriguez (@joperezr) backporting to git am output$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch
Applying: Fix npm registry hard-coded to internal dnceng feed (#19370)
Using index info to reconstruct a base tree...
M src/Aspire.Cli/Npm/NpmRunner.cs
M src/Aspire.ProjectTemplates/templates/aspire-ts-cs-starter/frontend/package-lock.json
M tests/Aspire.Cli.Tests/Npm/NpmRunnerTests.cs
M tests/Aspire.Cli.Tests/Templating/TemplatePackageLockTests.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Aspire.Cli/Npm/NpmRunner.cs
Auto-merging src/Aspire.ProjectTemplates/templates/aspire-ts-cs-starter/frontend/package-lock.json
Auto-merging tests/Aspire.Cli.Tests/Npm/NpmRunnerTests.cs
Auto-merging tests/Aspire.Cli.Tests/Templating/TemplatePackageLockTests.cs
CONFLICT (content): Merge conflict in tests/Aspire.Cli.Tests/Templating/TemplatePackageLockTests.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 Fix npm registry hard-coded to internal dnceng feed (#19370)
Error: The process '/usr/bin/git' failed with exit code 128 |
…19370) (#19417) * Fix npm registry hard-coded to internal dnceng feed (#19370) (#19388) PR #18858 set the internal dnceng Azure Artifacts feed as the customer default for npm operations. This breaks `aspire agent init`: npm resolves a tarball's transitive dependencies through --registry, and the dnceng pull-through feed returns 401 anonymously for packages it hasn't cached (e.g. playwright / playwright-core), so `npm install -g <.tgz>` fails with E401. It also shipped the internal feed as the default in generated Dockerfiles and `aspire new` templates. Restore the public npm registry (https://registry.npmjs.org/) as the customer-facing default across: - Aspire.Cli NpmRunner (npm view/pack/install pass --registry) - Generated pnpm Dockerfile NPM_REGISTRY default (still overridable at build time via --build-arg NPM_REGISTRY=...) - Shipped ts-starter / py-starter / aspire-ts-cs-starter template lockfiles (resolved URL host only; versions and integrity hashes are unchanged) Security hardening is retained: --ignore-scripts on the global install and Sigstore provenance + SRI integrity verification. Repo-internal lockfiles and CI images intentionally remain on the internal feed. Fixes #19370 Co-authored-by: David Pine <t@t.test> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3b7c7a65-f065-49bc-8122-83e9f9429b5f * Use public registry for release template lockfile The release/13.5 lockfile still contained entries from an older internal npm mirror that was no longer present on main when #19388 was merged. Convert those resolved URL hosts as well while preserving the release branch package versions, dependency graph, and integrity values. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3b7c7a65-f065-49bc-8122-83e9f9429b5f --------- Co-authored-by: David Pine <david.pine@microsoft.com> Co-authored-by: David Pine <t@t.test> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3b7c7a65-f065-49bc-8122-83e9f9429b5f
Description
PR #18858 hardened npm handling but also changed the customer-facing default registry to the internal dnceng Azure Artifacts feed (
https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-public-npm/npm/registry/). That feed is a private pull-through mirror that returns 401 anonymously for tarballs it hasn't already cached.This breaks
aspire agent init:npm install -g <local .tgz>resolves the tarball's transitive dependencies through--registry, and packages the feed hasn't mirrored (e.g.playwright/playwright-core) fail the final install step withE401. The internal feed was also shipped as the default in generated Dockerfiles andaspire newtemplates.Root cause
--registryis applied to the entire resolution graph, not just the top-level package. Pointing it at an anonymous-401 private mirror means any un-mirrored transitive dependency aborts the install.Fix
Restore the public npm registry (
https://registry.npmjs.org/) as the customer-facing default, while keeping the genuine hardening introduced in #18858.Surfaces changed:
src/Aspire.Cli/Npm/NpmRunner.cs: the--registryvalue passed tonpm view/pack/install -gnow points at public npm. Commands still pass it explicitly so resolution can't inherit a project-level.npmrcthat redirects to a private feed.--ignore-scriptsis retained on the global install.src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs: theNPM_REGISTRYdefault is public npm, and stays overridable at build time via--build-arg NPM_REGISTRY=...for operators who prefer an internal mirror.aspire newtemplate lockfiles —ts-starter,py-starter, andaspire-ts-cs-starter/frontendpackage-lock.json: only theresolvedURL host is swapped to public npm. Versions andintegrity(SRI) hashes are unchanged — the dnceng feed is a pull-through mirror, so the path after the host is identical and the integrity hashes remain valid. This mirrors the precedent already used in [auto-sec] Consolidate Aspire JavaScript template dependency security remediations #18858 for the frontend lockfiles.NpmRunnerTests,AddJavaScriptAppTests, and the six JS Dockerfile snapshots; added regression guards inTemplatePackageLockTestsasserting shipped lockfiles resolve only toregistry.npmjs.org.Intentionally left on the internal feed (repo-internal / CI only, not customer-facing):
playground/**,tests/PolyglotAppHosts/**,tests/Aspire.Cli.EndToEnd.Tests/Fixtures/**,extension/yarn.lock, and.github/workflows/polyglot-validation/Dockerfile.typescript. Reverting these would balloon the diff and undo intended repo-internal supply-chain routing.Security posture unchanged:
--ignore-scripts, Sigstore provenance verification, and SRI integrity checks are all retained. Package authenticity is still cryptographically verified regardless of registry host, so swapping theresolvedhost does not weaken the supply-chain guarantees added in #18858.Fixes #19370
Checklist
<remarks />and<code />elements on your triple slash comments?--ignore-scripts, Sigstore provenance, SRI integrity). No new trust boundary is introduced; integrity verification still enforces package authenticity independent of the registry host.