Add license signature trust-store extension seam - #7
Merged
tonythethompson merged 4 commits intoJul 27, 2026
Conversation
Gated (private desktop repo) needs multi-key trust-ring lookup and revocation for license verification, but under the Phase 2 submodule model it consumes Trackdub.Licensing read-only and can no longer fork LicenseTokenValidator/LicenseService in place to get it, per docs/plans/trackdub-gated-split-manifest.md (Trackdub-gated PR #2) §1. Adds ILicenseSignatureTrustStore: resolves a PEM public key per token key id, or null to reject an unknown/revoked key. LicenseService takes it as a new optional constructor parameter; omitting it preserves the existing single embedded-key behavior exactly. LicenseTokenClaims gains an optional KeyId parsed from the token's "kid" claim and threaded into LicenseTokenValidator.VerifySignature, which now resolves the key per-store when one is supplied instead of always using the embedded key. Revocation falls out of the same seam: a trust store returning null for a given key id fails verification closed, so no separate revocation plumbing is needed. Everything added is internal to Trackdub.Licensing except the new public interface and the optional constructor parameter, so the project keeps its zero-ProjectReference, BCL-only-crypto invariants (LicensingIsolationTests) and the dependency graph in AGENTS.md is unchanged. Production-policy concerns from the same manifest section -- rejecting dev-unlimited tokens under a production trust ring, and startup validation of production ring configuration -- are deliberately left out of this seam. Both read from already-public LicenseValidationResult fields (UnlimitedActivations, degradation reason) and are naturally implementable as a decorator over ILicenseInitializer/ILicenseTierProvider in the consuming product, without needing any further core change. The manifest's other blocker, headless export-tier enforcement, needs no core change at all: Trackdub.Sdk's TrackdubBuilder.ConfigureServices (-> TrackdubOptions/HeadlessTrackdubOptions.ServiceConfigurator -> HeadlessCompositionRoot.AddHeadlessTrackdub step 7) already lets a consumer register a custom IExportTierGate for Cli/Sdk hosts. Trackdub.Cli goes through this same builder. That seam already exists; using it is Phase 2.3 work in the desktop repo, not something this PR needs to add. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Yd883rGkFx5xN1TyT1Lzf
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f408b90aa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Found by review on PR #7. All three LicenseSignatureTrustStoreTests constructed LicenseService via its public constructor, which always builds its own LicenseFileStore() (the real platform app-data directory) regardless of the trustStore argument. The tempBase LicenseFileStore each test wrote the token to was never passed in, so in a clean environment the service would read no token, short-circuit to Free with a null degradation reason, and the assertions would fail without ever exercising the trust store. Switched all three to the internal 5-arg constructor already used by every other test in this file's project (GracefulDegradationTests, DevUnlimitedTokenTests, TierResolutionCorrectnessTests), passing the temp-backed store explicitly and wrapping the trust store in LicenseTokenValidator(trustStore) directly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Yd883rGkFx5xN1TyT1Lzf
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (8 files)
Previous Review Summaries (2 snapshots, latest commit 6b054a5)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 6b054a5)Status: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Previous review (commit f0cf730)Status: No Issues Found | Recommendation: Merge Files Reviewed (7 files)
Reviewed by step-3.7-flash · Input: 75.9K · Output: 8.1K · Cached: 237.2K |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review completed against the latest diff
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0cf730114
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Found by review on PR #7. The trust-store parameter was added as an optional third parameter on the existing two-arg constructor. Optional parameters are a compile-time/source feature only -- the CLR method signature still requires all parameters, so a precompiled consumer holding a reference to the original two-arg constructor would hit MissingMethodException if Trackdub.Licensing.dll were upgraded without recompiling that consumer. This project publishes NuGet packages (submit-nuget workflow), so that scenario is real. Splits into two real constructor overloads: the original two-arg signature is now a distinct compiled method that delegates to a non-optional three-arg overload carrying the trust store. Both existing source call sites (implicit two-arg, and any future explicit three-arg) keep compiling; the two-arg CLR signature itself no longer changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Yd883rGkFx5xN1TyT1Lzf
Self-hosted runner availability was already flagged as a Phase 1 publish-readiness blocker and deprioritized rather than fixed; the queued-forever Build & Test / Verify Code Format / Verify Repository Boundary checks on this PR are that blocker showing up directly. Switches ci.yml (format, repository-boundary, build-windows, build-linux, build-macos), model-audit.yml (all three jobs), and code-coverage.yml to ubuntu-latest/windows-latest/macos-latest as appropriate, and dependabot-auto-merge.yml to ubuntu-latest. None of these have a hardware dependency -- standard dotnet build/test/format, Python scripts, or gh CLI. codeql.yml already targeted ubuntu-latest/windows-latest and needed no change. Deliberately left trt-rtx-smoke.yml on self-hosted. It validates the TensorRT-RTX GPU execution provider against real NVIDIA GPU hardware, which standard GitHub-hosted runners don't have; flipping its runs-on would silently turn it into a no-op (it already has continue-on-error: true and is gated behind an opt-in repo variable) rather than fix anything. That one needs an actual GPU runner, not a relabel. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Yd883rGkFx5xN1TyT1Lzf
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
2 participants
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.
Summary
Adds
ILicenseSignatureTrustStoretoTrackdub.Licensing: a small extension seam that lets a consuming product supply its own multi-key trust-ring and revocation policy for license-token signature verification, without forking core's licensing internals.Linked issue
Not a GitHub issue — resolves the Phase 2 blocker recorded in
docs/plans/trackdub-gated-split-manifest.md§1 (Trackdub-gatedPR #2), landed as the recommended option 2 from that manifest.Why
Under the Phase 2 rebuild, the private desktop repo (
Trackdub-gated) consumes this repo as a git submodule — read-only. Its current trust-ring work (TrustRingConfiguration,RevocationConfiguration) isn't additive: it forksLicenseTokenValidator,LicenseService,LicenseTokenClaims, andLicenseTokenParserin place, replacing single-key ES256 verification with multi-key lookup plus revocation. That can't survive the submodule model as-is.What this adds
ILicenseSignatureTrustStore.ResolvePublicKeyPem(string? keyId)— resolves the PEM public key trusted for a key id, ornullto reject (unknown or revoked key). Revocation falls out of this one method; no separate revocation plumbing needed.LicenseServicegains an optional third constructor parameter,ILicenseSignatureTrustStore? trustStore = null. Omitted, behavior is byte-for-byte what it is today (single embedded development key). Supplied,LicenseTokenValidatorresolves the key per-token instead.LicenseTokenClaims(internal) gains an optionalKeyId, parsed byLicenseTokenParserfrom a newkidclaim.Everything except the new interface and the one optional constructor parameter is
internal.Trackdub.Licensingkeeps its zero-ProjectReference, BCL-only-crypto invariants (LicensingIsolationTests), and theAGENTS.mddependency graph is unchanged since no project reference moved.What this deliberately does not add
The manifest's trust-ring section also covers production-only policy: rejecting dev-unlimited tokens under a production trust ring, and validating production ring configuration at startup and throwing if it's invalid. Both of those read from fields already public on
LicenseValidationResult(UnlimitedActivations, the degradation-reason string) and are naturally implementable as a decorator overILicenseInitializer/ILicenseTierProviderin the consuming product — no further core change needed, so none is included here.The manifest's other Phase 2 blocker — headless export-tier enforcement for
Trackdub.Cli/Trackdub.Sdk— turned out to need no core change at all.TrackdubBuilder.ConfigureServices(→TrackdubOptions/HeadlessTrackdubOptions.ServiceConfigurator→HeadlessCompositionRoot.AddHeadlessTrackdubstep 7) already lets a consumer register a customIExportTierGatefor headless hosts, andTrackdub.Cliitself is built on this same path (TrackdubBuilder→TrackdubSessionFactory). That seam already exists; wiring it up is Phase 2.3 work in the desktop repo, not something to add here.Scope
Testing
dotnet build Trackdub.slnx -m:1 -p:Platform=x64dotnet test Trackdub.slnx -m:1 -p:Platform=x64Test notes
No .NET SDK is available in this session's environment, so I could not run a local build or test pass — relying on CI, which I'll monitor and drive to green.
Added
LicenseSignatureTrustStoreTests.cscovering: a token signed with a trust-store-resolved key verifies and resolves the correct tier; an unknown/revoked key id fails closed even though the signature itself would otherwise verify against some key; a key id that resolves to the wrong public key fails closed. Fixed the one existing call site (TokenSignatureRoundTripTests.cs) broken byVerifySignature's newkeyIdparameter; all other existingLicenseTokenValidator/LicenseServiceconstruction call sites use overloads I did not change and are unaffected. Also ranscripts/ci/check-repository-boundary.pylocally — passes.Architecture review
Trackdub.AppLicense/model impact
Risk and rollback
Trackdub-gatedstill needs (a) anILicenseSignatureTrustStoreimplementation carrying the actual trust-ring/revocation data, (b) the production-policy decorator described above, and (c) wiringDesktopExportTierGatethroughConfigureServicesfor its headless hosts — all tracked as Phase 2.3 work, not part of this PR.Milestone notes
Phase 2 of
docs/plans/open-core-split-continuation.md.Agent notes
Generated by Claude Code
Summary by cubic
Adds a trust-store seam for per-token ES256 verification with multi-key rotation and revocation. Keeps binary compatibility via the original two-arg
LicenseServiceconstructor and adds a new three-arg overload; also moves CI to GitHub-hosted runners to unblock checks.New Features
ILicenseSignatureTrustStore.ResolvePublicKeyPem(keyId)resolves a trusted PEM public key; return null to reject (covers revocation).LicenseServiceaccepts an optionalILicenseSignatureTrustStorevia a new three-arg overload; omitting it keeps prior single-key behavior.kidintoLicenseTokenClaims.KeyId;LicenseTokenValidator.VerifySignature(keyId, ...)uses the trust store when provided.Bug Fixes
LicenseServiceconstructor and delegating to the new overload.ubuntu-latest,windows-latest, andmacos-latestto restore reliability.Written for commit 2ff45f7. Summary will update on new commits.