Add PackageValidator tool for NuGet package inspection and validation#4407
Add PackageValidator tool for NuGet package inspection and validation#4407paulmedynski wants to merge 10 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new standalone maintainer CLI (tools/PackageValidator) to inspect .nupkg/.snupkg artifacts (versions, signing, dependencies, and symbols) using metadata-only inspection, plus a dedicated xUnit test project for the tool.
Changes:
- Added a
net10.0PackageValidator console app with JSON/human output and--fail-ongating. - Implemented inspection/validation components: nuspec/dependencies parsing, managed/native binary inspection, symbol-package resolution, and a rules engine emitting categorized findings.
- Added xUnit tests covering version range evaluation, expectation parsing, symbol/signing/version validations, and helper utilities.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/PackageValidator/PackageValidator.csproj | Defines the standalone CLI tool project and its package dependencies. |
| tools/PackageValidator/Directory.Build.props | Isolates build settings for the tool directory (central package management). |
| tools/PackageValidator/Directory.Packages.props | Centralizes tool package versions (System.CommandLine). |
| tools/PackageValidator/Program.cs | CLI surface: args/options, help augmentation, run orchestration, gating/exit codes. |
| tools/PackageValidator/PackageInspector.cs | Reads .nuspec, signature entry, and enumerates DLLs in a .nupkg. |
| tools/PackageValidator/AssemblyInspector.cs | Metadata-only inspection of managed assemblies + native fallback handling. |
| tools/PackageValidator/BinaryClassifier.cs | Categorizes binaries (implementation/reference/satellite/native/other) from paths. |
| tools/PackageValidator/Models.Common.cs | Shared model types (enums/findings/dependency models/native info). |
| tools/PackageValidator/Models.Report.cs | Report models for run/package/binary/symbol-package output. |
| tools/PackageValidator/Validator.cs | Intrinsic rule engine + cross-package dependency consistency checks. |
| tools/PackageValidator/VersionExpectations.cs | Parses/holds --expect-* inputs and resolves wildcard/id overrides. |
| tools/PackageValidator/VersionRange.cs | Minimal NuGet version-range evaluator used for dependency checks. |
| tools/PackageValidator/SymbolResolver.cs | Matches .snupkg PDBs to assemblies by GUID and verifies checksums. |
| tools/PackageValidator/PortablePdb.cs | Portable PDB GUID extraction + checksum verification support. |
| tools/PackageValidator/NativeVersionReader.cs | Reads native PE architecture and Win32 version-resource info (temp-file based). |
| tools/PackageValidator/HumanReporter.cs | Human-readable console renderer for reports/findings/summary. |
| tools/PackageValidator/Json.cs | Source-generated JSON serialization context/options for machine output. |
| tools/PackageValidator/tests/PackageValidator.Tests.csproj | Test project wiring (net10.0) and tool project reference. |
| tools/PackageValidator/tests/Directory.Build.props | Isolates build settings for the test directory. |
| tools/PackageValidator/tests/Directory.Packages.props | Centralizes test package versions (xUnit, test SDK). |
| tools/PackageValidator/tests/VersionRangeTests.cs | Unit tests for version-range evaluation behavior. |
| tools/PackageValidator/tests/VersionExpectationsTests.cs | Unit tests for expectation parsing and validation integration. |
| tools/PackageValidator/tests/ValidatorTests.cs | Unit tests for rules engine findings and batch dependency validation. |
| tools/PackageValidator/tests/BinaryClassifierTests.cs | Unit tests for binary path classification. |
| tools/PackageValidator/tests/AssemblyInspectorTests.cs | Unit tests for PKT computation and portable PDB helpers. |
A standalone .NET 10 CLI under tools/PackageValidator that inspects one or more .nupkg files and validates their versions, signing, and symbols using metadata-only reading (no assembly loading), so it works cross-platform and on assemblies built for TFMs the host does not run. Capabilities: - Package id/version, dependency groups, and NuGet signature from the .nuspec. - Per-assembly AssemblyVersion/FileVersion/InformationalVersion, culture, public key token, strong-name signing status, and target framework. - Native binary Win32 version info and architecture. - Binary classification (implementation/reference/satellite/native) so symbol coverage is judged only over implementation assemblies. - Sibling .snupkg symbol matching by debug GUID with portable-PDB checksum verification, embedded-symbol detection, and orphan/mismatch reporting. - Intrinsic validation rules emitting severity-tagged findings, plus optional --expect-package/file/assembly-version assertions for inter-package version-match validation. - Batch/directory input, JSON or human output, --fail-on exit-code gating, and a free-form Notes help section. - xUnit test project (41 tests). Exploratory maintainer tooling; isolated from the main build (its own Directory.Build.props / Directory.Packages.props).
Layout: - Move sources to src/ and tests to test/ to match the PackageCompatibility tool; share one Directory.*.props; add global.json (xUnit v3 / MTP). - Convert the test project to xUnit v3 (Exe + IsTestProject, xunit.v3). - Wire BuildPackageValidator(Test)/TestPackageValidator (and the PackageCompatibility equivalents) into build.proj; BuildTools builds tests. Review feedback (PR #4407): - NativeVersionReader: handle UnauthorizedAccessException/SecurityException on temp-file staging and cleanup instead of letting them escape. - Validator: flag a missing AssemblyFileVersion against --expect-file-version (and assembly version) instead of silently passing. - VersionRange: follow SemVer 2.0 precedence (prerelease ordering, ignore build metadata) instead of dropping the prerelease/metadata suffix. - PortablePdb: dispose the BinaryReader/MemoryStream in FindPdbIdOffset. Adds regression tests (48 total, all passing).
7a8dc98 to
eebc229
Compare
- Add XML docs, intent comments, and arrange/act/assert structure to the PackageValidator test classes, methods, and helpers. - Add a /tools/ solution folder to Microsoft.Data.SqlClient.slnx containing the PackageCompatibility and PackageValidator src and test projects.
… DLLs in multiple places in the nupkg.
…fier lib/ matching - VersionRange.Satisfies: validate the closing bracket delimiter before slicing so malformed ranges (e.g. "[") return null instead of throwing. - VersionRange.Parse: reject empty release components (e.g. "1..0") instead of collapsing them via RemoveEmptyEntries. - BinaryClassifier: only treat nested /lib/ paths under runtimes/ as Implementation so build/lib/... is not misclassified. - Add regression tests for all three cases.
…te loading - PortablePdb.FindPdbIdOffset: defensively align the stream position to a 4-byte boundary after the version string. This is a no-op for ECMA-335-compliant PDBs but prevents misaligned flags/stream-header reads (and a missed #Pdb stream) if a producer records an unrounded version length. - SymbolResolver.MatchSymbolPackage: stop retaining the bytes of every .pdb in the symbol package. PDBs are now read only transiently to compute their GUID, and the matched entry is re-opened on demand for checksum verification, keeping peak memory to a single PDB for large symbol packages.
Document what the tool does, how to build/run it, the full CLI surface (--fail-on categories, --expect-* semantics, exit codes), the source-file layout and per-file responsibilities, related public tools, and tests.
- VersionRange.Parse: a trailing hyphen (e.g. "1.0.0-") is now rejected as an empty prerelease rather than treated as "no prerelease", and empty prerelease identifiers (e.g. "1.0.0-alpha..1") are rejected, so both return null per the method contract instead of evaluating as valid versions. - Add regression rows covering both malformed prerelease forms.
- VersionRange.Satisfies: a comma-less bracketed range is only a valid exact pin
when both delimiters are inclusive ("[1.0.0]"). Other combinations such as
"(1.0.0)" or "(1.0.0]" are now rejected as unparseable and return null per the
method contract instead of evaluating as an exact match.
- Add regression rows for the invalid exact-pin delimiter combinations.
- PortablePdb.FindPdbIdOffset: validate the metadata version length before seeking. A negative or oversized length now returns -1 (unparseable) instead of letting the seek throw and fail the whole validation run. Broaden the catch to also swallow IOException/ArgumentOutOfRangeException from hostile layouts. - Add a regression test asserting TryVerifyChecksum returns null (no throw) for a header encoding a negative version length.
mdaigle
left a comment
There was a problem hiding this comment.
My questions on the tests are the only things blocking my review. Would like to understand why more things aren't marked as error.
| @@ -0,0 +1,305 @@ | |||
| using System.IO.Compression; | |||
| $"{name}, Version={assemblyVersion}, Culture={culture}, " + | ||
| $"PublicKeyToken={publicKeyToken ?? "null"}"; | ||
|
|
||
| return new BinaryReport |
There was a problem hiding this comment.
it's possible to render markdown in a pipeline summary using a snippet like this. @David-Engel showed an example where he was publishing performance test results. Just a fun fact for when we integrate this in the pipeline.
steps:
- script: |
echo "# Deployment Summary" > summary.md
echo "### Status: Success :rocket:" >> summary.md
echo "- **Environment:** Production" >> summary.md
echo "- **Artifact Version:** $(Build.BuildNumber)" >> summary.md
# This command uploads the markdown file to the pipeline UI
echo "##vso[task.uploadsummary]$(System.DefaultWorkingDirectory)/summary.md"
displayName: 'Generate and Display Markdown Summary'|
|
||
| // Assert: the mismatch surfaces as a Warning rather than an Error. | ||
| Assert.Contains(report.Findings!, f => | ||
| f.Category == Categories.VersionInconsistency && f.Severity == Severity.Warning); |
There was a problem hiding this comment.
Why not mark everything as error? Shouldn't we be able to enforce the outcomes we want?
Is there some case where we want to allow these to be different?
| /// flag unset) raises a Warning-severity delay-signed finding. | ||
| /// </summary> | ||
| [Fact] | ||
| public void Flags_delay_signed_assembly() |
There was a problem hiding this comment.
when does this case happen? is it just in the nonofficial pipeline?
| /// <see cref="PackageReport"/> so a single rule can be exercised in isolation, then asserts on the | ||
| /// findings the validator attaches. | ||
| /// </summary> | ||
| public class ValidatorTests |
There was a problem hiding this comment.
These tests could be strengthened by asserting the exact findings content.
Summary
Adds a standalone .NET 10 CLI under
tools/PackageValidatorthat inspects one or more.nupkgfiles and validates their versions, signing, and symbols using metadata-only reading (no assembly loading). This keeps it cross-platform and able to read assemblies built for TFMs the host does not run (e.g.net462from a Linux host).This is a first-class maintainer tool that we will use going forward — not an exploratory prototype. It mirrors the
PackageCompatibilitytool layout (src/+test/, its ownDirectory.*.props/global.json) and is isolated from the main driver build. A follow-up CI pipeline will run this tool to validate the SqlClient family of packages as part of our release/publish flow.Documentation
Full usage — capabilities, the complete CLI reference (
--fail-oncategories,--expect-*semantics, and exit codes), the source-file layout with per-file responsibilities, and comparisons to related public tools — lives in the tool's README. Reviewers should refer to the README for those details; this description keeps only a high-level summary and sample output.At a glance, PackageValidator reports and validates package identity/signature, per-assembly versions and strong-name state, native version info, binary classification, and
.snupkgsymbol matching (debug GUID + portable-PDB checksum), plus optional--expect-*inter-package version-match assertions and--fail-onCI gating.Example output
Inspect a single package (human-readable)
PackageValidator Microsoft.Data.SqlClient.7.1.0-preview1.nupkgMachine-readable output (
--json)Per-binary object (symbols matched and checksum-verified against the sibling
.snupkg):{ "path": "lib/net8.0/Microsoft.Data.SqlClient.dll", "kind": "Implementation", "isManagedAssembly": true, "assemblyName": "Microsoft.Data.SqlClient", "assemblyVersion": "7.0.0.0", "fileVersion": "7.1.0.26176", "informationalVersion": "7.1.0-preview1+9c72dc0b4e8eed7fe6e83684e86800f9b4ac0b57", "targetFramework": ".NETCoreApp,Version=v8.0", "culture": "neutral", "publicKeyToken": "23ec7fc2d6eaa4a5", "signingStatus": "Signed", "debugId": "546ac289-357a-4bf7-8c42-dabbcb8963ec", "pdbChecksums": [ "SHA256:89c26a547a35f7fbcc42dabbcb8963ec78d7cd167992f9de97b134e4db63bc73" ], "hasEmbeddedSymbols": false, "hasSymbols": true, "hasSymbolPackageSymbols": true, "symbolPackageSymbolsMatch": true, "symbolPackageVerifiedByChecksum": true, "symbolPackageFile": "lib/net8.0/Microsoft.Data.SqlClient.pdb" }Gate a directory against expected versions (
--expect-*+--fail-on)Assert the family file version uniformly (with a per-package override for
Microsoft.SqlServer.Server) and fail on any error:On a build that stamps the file-version revision on only some family packages, the gate trips (exit code
2):On a build that stamps the revision uniformly, the same assertion passes (exit code
0).Notes for Reviewers
This app was implemented with heavy use of AI agents. The concern was to create a tool we can rely on, not produce source code that meets the same standards of the driver codebase. The README discusses why this is a from-scratch tool versus using off-the-shelf tooling. Reviewers are free to gloss over some of the specifics (reading assembly info, matching PDBs, etc) and instead focus on the main Program.cs and trying the tool against real NuGets.