-
Notifications
You must be signed in to change notification settings - Fork 272
Warn when creating packages with non-ASCII characters in the package ID #14950
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
nkolev92
wants to merge
5
commits into
dev
Choose a base branch
from
dev-nkolev92-nonAscii
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
bf0cea0
Warn when creating packages with non-ASCII characters in the package ID
nkolev92 66544e5
cleanup future possibility
nkolev92 6d2b843
cleanup
nkolev92 1d39c99
addreess feedback
nkolev92 702b327
Address review feedback: use 'allowed set' phrasing and update warnin…
nkolev92 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| # ***Warn when creating packages with non-ASCII characters in the package ID*** | ||
|
|
||
| - Nikolche Kolev <https://github.com/nkolev92> | ||
| - [Pack Warning when Package ID doesn't meet the standards](https://github.com/NuGet/Home/issues/14949) | ||
|
|
||
| ## Summary | ||
|
|
||
| Raise a NuGet pack warning when the package ID being authored contains characters outside of the ASCII range. | ||
| The warning is opt-out via `NoWarn`, framework agnostic, and applies to both the MSBuild `Pack` target (`dotnet pack` / `msbuild /t:Pack`) and `nuget.exe pack`. | ||
| This is the client-side companion to the nuget.org change that blocks new pushes with non-ASCII package IDs. | ||
| See [Strengthening NuGet Security with Package ID Standards](https://github.com/NuGet/Announcements/issues/75). | ||
|
|
||
| ## Motivation | ||
|
|
||
| As announced in [Strengthening NuGet Security with Package ID Standards](https://github.com/NuGet/Announcements/issues/75), nuget.org now enforces stricter package ID rules. New package IDs must match `^[A-Za-z0-9_](?!.*[.-]{2})[A-Za-z0-9.-]{0,99}$`, and starting July 15th, even existing IDs that don't conform will be blocked from pushing new versions. | ||
|
|
||
| Without a corresponding client-side signal, authors only learn about the restriction at `dotnet nuget push` time - typically late in the release process, after CI has produced a `.nupkg` they cannot ship. | ||
| Warning at pack time gives authors the earliest possible signal, before any artifact is uploaded, and avoids late-stage release surprises. | ||
|
|
||
| The longer-term plan is to promote this warning to an error in a later .NET SDK release (targeted at .NET 12). | ||
| See [Strengthening NuGet Security with Package ID Standards](https://github.com/NuGet/Announcements/issues/75) for the broader rollout plan and timelines. | ||
|
|
||
| ## Explanation | ||
|
|
||
| ### Functional explanation | ||
|
|
||
| When a user runs `dotnet pack` (or any other entry point that invokes the NuGet `Pack` task) on a project whose effective package ID contains characters outside the ASCII range, NuGet emits a warning: | ||
|
|
||
| ```text | ||
| warning NU5052: The package ID 'Contöso.Utilities' contains non-ASCII characters. Non-ASCII characters in package IDs are not allowed and will become an error in a future release. Consider renaming the package to use ASCII characters only. | ||
| ``` | ||
|
|
||
| The warning: | ||
|
|
||
| - Is raised once per pack invocation, against the package being built. | ||
| - Honors `NoWarn`, `TreatWarningsAsErrors`, `WarningsNotAsErrors`, and `WarningsAsErrors` like any other NuGet pack warning. | ||
|
|
||
| The warning is enabled by default gated by `SdkAnalysisLevel`: | ||
|
|
||
| - `SdkAnalysisLevel` >= 11.0.100 (or unset, per the rules in the [NuGet feature guide](https://github.com/NuGet/NuGet.Client/blob/dev/docs/feature-guide.md#warnings-and-defaults)) → warning is on by default. | ||
| - `SdkAnalysisLevel` < 11.0.100 → warning is off. | ||
| - Non-SDK projects (`nuget.exe pack`, `msbuild /t:Pack` without `Microsoft.NET.Sdk`) → warning is on by default. | ||
|
|
||
| #### Examples | ||
|
|
||
| | Package ID | Warns? | Notes | | ||
| |------------|--------|-------| | ||
| | `Contoso.Utilities` | No | All ASCII. | | ||
| | `Contöso.Utilities` | Yes | `ö` is outside ASCII. | | ||
| | `Contoso.Ütil` | Yes | `Ü` is outside ASCII. | | ||
| | `Сontoso.Utilities` (Cyrillic `С`) | Yes | Homoglyph. | | ||
| | `My.Package-1.0` | No | Hyphen and digits are ASCII. | | ||
|
|
||
| The check applies only to the **package ID**. | ||
|
|
||
| ## Drawbacks | ||
|
|
||
| - The nuget.org rule enforcement is not necessarily going to apply to all other feeds. | ||
| - Local feeds do not apply the same rules. | ||
|
|
||
| ## Rationale and alternatives | ||
|
|
||
| ### Why warn in `pack`, rather than rely solely on the nuget.org push block | ||
|
|
||
| - Earliest possible feedback to the author - the issue is caught locally before CI even produces an artifact, instead of failing late at `dotnet nuget push`. | ||
| - Reaches authors who push to feeds other than nuget.org (Azure Artifacts, GitHub Packages, internal feeds), where the nuget.org-side block does not apply. | ||
|
|
||
| The pack warning and the nuget.org push block are complementary, not alternatives. | ||
|
|
||
| ## Prior Art | ||
|
|
||
| - [Package ID validation today](https://github.com/NuGet/NuGet.Client/blob/dev/src/NuGet.Core/NuGet.Packaging/PackageCreation/Utility/PackageIdValidator.cs) - the existing regex-based check that this warning slots in next to. | ||
| - [SdkAnalysisLevel design](https://github.com/dotnet/designs/blob/main/proposed/sdk-analysis-level.md) - the gating mechanism we use here, per the [NuGet feature guide](https://github.com/NuGet/NuGet.Client/blob/dev/docs/feature-guide.md#warnings-and-defaults). | ||
| - npm has restricted new package names to a [strict character set](https://github.com/npm/validate-npm-package-name) (URL-safe ASCII) for years. | ||
| - PyPI [PEP 541](https://peps.python.org/pep-0541/) addresses name squatting administratively rather than via name restrictions, which has been less effective at preventing homoglyph attacks. | ||
| - Crates.io restricts crate names to ASCII alphanumerics, `-`, and `_`. | ||
|
|
||
| ## Unresolved Questions | ||
|
|
||
| - **How does this get rolled out in msbuild.exe /t:pack for non-SDK projects. Visual Studio doesn't have a major version where the switch could be easily introduced.** The need for this is there given that customers won't be able to nuget.org. | ||
| - **Patch back to .NET 10.0.400 / 10.0.100.** Given the nuget.org push block is already in effect, should we patch the warning into the .NET 10.0.400 SDK to reach more customers sooner? Should we also patch .NET 10.0.100 to reach Linux customers earlier in the rollout? | ||
| - **Which .NET SDK band makes the warning an error?** The current plan is .NET 12 (one major after the warning ships in .NET 10/11), but the exact `SdkAnalysisLevel` value and band need to be confirmed with the .NET SDK team. | ||
|
|
||
| ## Future Possibilities | ||
|
|
||
| - **Restore time validation non-ASCII character validation** | ||
| - Is there a real user demand for this type of validation. With nuget.org providing the no collision guarantee, the need isn't there since nuget.org is the only true public feed. The rest of feeds would likely be controlled by the consumers. | ||
| - Should that validation be a warning or an error? A warning is more expensive to technically implement and may add some perf cost. An error is relatively straightforward. | ||
| - **Promote to error in .NET 12.** Once telemetry shows the warning is being seen and acted on, promote `NU5052` from warning to error under a higher `SdkAnalysisLevel`. | ||
| - **Surface in Visual Studio pack UI.** A more prominent in-IDE prompt when authoring a new package project with a non-ASCII ID. | ||
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.
Uh oh!
There was an error while loading. Please reload this page.