fix(license): unify offline grace ladder to one canonical source - #404
Merged
Merged
Conversation
The CLI shipped three descriptions of "license server unreachable, cache present": grace.go's GraceSoftThreshold/GraceHardThreshold (24h/7d, the only one with live callers), validator.go's FailOpenSoftTTL/FailOpenHardTTL (72h/14d, dead code with no non-test callers), and a third ladder published in the web docs. They had drifted apart. grace.go is now the single owner of the offline ladder: soft=72h (silent, covers a full weekend outage on our side without alarming a paying customer), hard=7d (fail closed - checker.go sends only the license key with no machine identifier, so the cache is a copyable bare credential and widening the ceiling multiplies that exposure). - validator.go's FailOpenSoftTTL/FailOpenHardTTL become aliases of grace.go's constants instead of a second declaration. Validate/ ValidateFull are unused in production (no non-test callers in cmd/ or sdk/) but carry ~35 direct unit tests across four files exercising branches (signature verification, revocation-during-fail-open, atomic cache write) not otherwise covered elsewhere; kept and repointed rather than deleted, per the "keep and read the unified constants" option. - internal/plugin/license.go's cacheTTL/offlineGraceTTL (a separate HMAC-signed cache file) now alias the same two constants instead of hand-typing their own 24h/7d. - sdk/go/license/license.go's DefaultGracePeriod keeps its own literal (sdk/go is a separate Go module and cannot import cli/internal/license) but its comment now says so instead of citing a nonexistent F07-PRICING-TIERS.md. - Removed LicenseConfig.GraceDays (internal/config): declared with an env tag but never read anywhere, and grace.go's comment claiming the ladder was "Configurable via LICENSE_GRACE_DAYS env var" was false. Test boundaries updated for the new 72h soft threshold (several tests used 24-48h ages to exercise the old soft-grace window) and added an explicit just-under-72h silent case. No behavior change outside the ladder itself: gofmt clean, vet clean, 836 tests passing, license package coverage 91.1% (unchanged from baseline), plugin package coverage unchanged at 50.9%.
The doc-sync gate flagged this branch for changing env vars without touching F09-ENV-VAR-INVENTORY.md or .github/wiki/Config-Env-Vars.md. The gate is right: this branch removes the GraceDays config field. Config-Env-Vars.md had no licensing section at all, so rather than a token edit this adds one covering the variables the code actually reads, each verified against its env tag or os.Getenv call site. Records explicitly that the offline grace window is NOT settable by env, and that LICENSE_GRACE_DAYS never had any effect despite a code comment advertising it. Someone who set that variable is entitled to know it did nothing. Also documents NSELF_LICENSE_FAIL_OPEN and NSELF_LICENSE_SKIP_VERIFY as escape hatches with their real limits: neither overrides a server that answers, and neither overrides revocation.
…e env vars
The parity gate failed with '.github/surface-parity.{md,json} is stale (run
make parity)'. Two things on this branch move that surface: the GraceDays
config field is removed, and the new Licensing section in
.github/wiki/Config-Env-Vars.md documents NSELF_LICENSE_KEY, which the tool had
been counting as undocumented.
Regenerated with make parity. The only change is nself init flipping from
'undocumented: NSELF_LICENSE_KEY' to 'documented', and the undocumented env var
total dropping from 17 to 16.
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.
Summary
internal/license/grace.go'sGraceSoftThreshold(72h, silent) andGraceHardThreshold(7d, fail closed).validator.go's previously-separateFailOpenSoftTTL/FailOpenHardTTL(72h/14d) become aliases of the grace.go constants instead of a second declaration.internal/plugin/license.go'scacheTTL/offlineGraceTTL(a separate HMAC-signed cache) now alias the same constants instead of hand-typing 24h/7d.sdk/go/license/license.go'sDefaultGracePeriodkeeps its own literal (separate Go module, cannot import cli/internal) but its comment no longer cites the nonexistentF07-PRICING-TIERS.md.LicenseConfig.GraceDays(declared, never read) and the false "Configurable via LICENSE_GRACE_DAYS env var" claim in grace.go.Validate/ValidateFull(no non-test callers, but ~35 direct unit tests covering branches not exercised elsewhere) rather than deleting them, repointing them to the unified constants instead.Companion web docs fix (bundle license §4, licensing.mdx, offline.mdx) lands separately on the existing PR #226 branch in
nself/web.Test plan
gofmt -l .emptymake vetcleango test ./internal/license/... ./internal/plugin/...— 836 passinginternal/licensecoverage 91.1% (baseline 91.2%, no regression)internal/plugincoverage 50.9% (unchanged from baseline)