feat(licensing): add a verify command to prove the key pair before a release (ADR D84) - #284
Merged
Merged
Conversation
…release (ADR D84) D83's guard catches the burned placeholder coming back — a revert. It does not catch the other way of getting the key wrong on a rotation, which is at least as likely: committing a public key that belongs to a DIFFERENT pair than the private key in the vault. A second keygen run, a copy from the wrong scrollback. Nothing detected that. It compiles, packs, passes CI and publishes. The failure surfaces later and all at once, as every license the maintainer issues being rejected by every customer — and since D83 a v* tag publishes with no human step, against immutable NuGet versions. The tool had keygen and sign but no way to exercise both halves together. `verify --license <key>` runs ProLicense.Validate against the key embedded in that build — deliberately the same code path a customer's process takes, so what it proves is what they will experience. The README documents the pre-release ritual: sign a throwaway one-day license with the vaulted key, verify it, expect VALID. It takes the license key, never a .pem. Verification needs only the public half, so the command has no reason to be able to read a private key at all. The test pins that it can FAIL, not that it works. A verify that reports success regardless would be worse than not having one — it would launder the exact mistake it exists to catch into a green tick. A freshly generated pair stands in for a mismatched one, being by construction not the embedded pair. The positive path cannot run in CI because it needs the vaulted key; that asymmetry is the design. Verified by making verify always return 0 — that test, and only that test, fails.
…it away The coverage gate rejected the first cut, correctly. The three uncovered lines were the VALID branch — the one the maintainer depends on before every release — and the PR had rationalized that as inherent: "the positive path needs the vaulted private key, so CI cannot run it." True, and not the same as untestable. As shipped, the branch would have gone out having never executed once, so a null licensee or a bad format string in it would surface on the single occasion it matters, while the operator trusted the answer. The body moves behind an internal seam taking an explicit verifying key — null for every call the CLI makes, meaning the embedded key — so a test drives the same code with a generated pair. The CLI surface is unchanged; the command still means "validated against what we shipped". A --public-key flag would have been simpler and was rejected: it would let the pre-release check run against the key it was just handed, verifying nothing, and that footgun would sit on the one command whose purpose is catching exactly that class of mistake.
|
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Follow-up to #283, from the question "should I test a key before publishing the version?" — yes, and the check that matters was not possible with the tooling as it stood.
The gap D83 does not cover
D83's guard catches the burned placeholder coming back — a revert. It does not catch the other way of getting the key wrong on a rotation, which is at least as likely: committing a public key belonging to a different pair than the private key in the vault (a second
keygenrun, a copy from the wrong terminal scrollback).Nothing detected that. It compiles, packs, passes CI and publishes. Then it surfaces later and all at once, as every license the maintainer issues being rejected by every customer — and since D83 a
v*tag publishes with no human step, against immutable NuGet versions.The tool had
keygenandsignbut no way to exercise both halves together.verify --license <key>Runs
ProLicense.Validateagainst the key embedded in that build — deliberately the same code path a customer's process takes, so what it proves is what they will experience.dotnet run --project tools/NeoReports.LicenseTool -- \ sign --key <vaulted-key.pem> --licensee "Key pair check" --days 1 > check.key dotnet run --project tools/NeoReports.LicenseTool -- verify --license "$(cat check.key)"VALID(exit 0) → the halves match, safe to tag.INVALID (SignatureInvalid)→ the committed public key is from another pair; stop before tagging. Documented in the tool README as a pre-release step.It takes the license key, never a
.pem: verification needs only the public half, so the command has no reason to be able to read a private key at all.The test pins that it can fail
A
verifythat reports success regardless would be worse than not having one — it would launder the exact mistake it exists to catch into a green tick. So the test asserts rejection, not acceptance.A freshly generated pair stands in for a mismatched one (by construction it is not the embedded pair). The positive path cannot run in CI — it needs the vaulted key, which never touches CI. That asymmetry is the design: CI pins that the command discriminates; the maintainer runs the positive half by hand.
Verified failing without the fix: making
verifyalways return 0 fails that test and only that test.Verification
NEOREPORTS_REQUIRE_DOCKER=1); Licensing 50/50.NeoReports.LicenseToolis not packable and ships nowhere.