Skip to content

fix(license): persist server-rejected keys and honour the revocation list in entitlement selection (D-1110, D-1111) - #943

Merged
acamarata merged 7 commits into
mainfrom
dw1-cli-7
Oct 26, 2026
Merged

acamarata merged 7 commits into
mainfrom
dw1-cli-7

Conversation

@acamarata

Copy link
Copy Markdown
Collaborator

Rows

D-1110 (fixed) A server 401/403 rejection lived only in the per-run map, so a later offline refresh republished the cached JWT. Rejections are now persisted (key hashes, entitlement-rejected.json beside the licence cache, 0600) by revalidateKeys, the scheduled revalidateClassified and validateNewKey, and selectToken honours them. A fresh online acceptance of the same key clears the entry. Test: TestRefreshServerRejectionSurvivesOfflineRefresh (fails without the fix), plus TestRefreshOnlineAcceptanceClearsPersistedRejection.

D-1111 (fixed) selectToken now skips any candidate whose subject (sub = key hash) is on the persistent revocation list (license.IsRecordRevoked, same fail-open staleness rule as the rest of the CLI). All token sources (cachedToken, retained file, renewed) go through selectToken, so one check covers them. Test: TestRefreshSkipsSubjectOnRevocationList (fails without the fix).

D-0870 (no change, already fixed) internal/mcpgen/tools.go skips a plugin command whose tool name is the reserved shortcut (nself_capabilities) with a warning instead of erroring (row.Shortcut branch + skipPlugin), landed in f16cb42 (#779); covered by testPluginShortcutName. A plugin help capabilities cannot reach mcpgen: the registry build rejects it (canon check), verified with a throwaway test (removed).

Commands run

  • go build ./... ok; go vet ./internal/license/... ./internal/mcpgen/ ok; gofmt -l internal/license clean
  • go test ./internal/license/... -count=1 ok
  • go test ./internal/mcpgen/ -count=1 ok
  • the two new rejection/revocation tests confirmed failing with the production changes reverted
  • go test ./cmd/... -run 'License|Entitlement' ok on two reruns (one earlier run of the same command reported a FAIL in cmd/commands that did not reproduce)

🤖 Generated with Claude Code

@acamarata

Copy link
Copy Markdown
Collaborator Author

Review fixes pushed (b65ea6b).

  1. Lost rejections: the rejection record's whole read-modify-write now runs under a process mutex plus an flock on entitlement-rejected.lock (10 s bounded wait; Windows keeps the mutex only, like the audit lock). New test runs 8 processes x 8 hashes: base keeps 50 of 64, fixed keeps 64.
  2. Unreadable state and write failures: loadRejected now returns an error for anything but an absent file, and selectToken fails closed on it. recordVerdict returns its error: a failed rejection write fails Refresh, the scheduled refresh (write-failed class) and license set (E125) instead of vanishing. A fresh verdict repairs a corrupt record; a failed clear of a rejection is ignored because a stale rejection only fails closed.
    Tests: two end-to-end tests (corrupt record, unwritable record then offline refresh) plus unit tests. Run against the pre-fix code, the concurrency and both end-to-end tests fail; with the fix all pass. Race run, windows vet, build, license/mcp tests pass. cmd/commands failed once under load (not reproduced; passes on rerun).

Known limit: if the rejection cannot be written at all, this run still fails closed and the failure is reported, but a later offline run reads an absent record. An unreadable record (corrupt, or a directory in its place) is covered.

@acamarata

Copy link
Copy Markdown
Collaborator Author

Review fixes pushed. (1) Corrupt-record recovery no longer forgives other keys: the rebuilt record marks every configured key rejected, so only a fresh online acceptance of a key lifts it (multi-key regression test added). (2) A rejection that cannot be written to the shared record (lock or record unwritable) now falls back to a per-key marker file that loadRejected unions, so the rejected JWT stays unusable offline; an error is returned only if nothing remembers the key. Fresh acceptance clears the marker. New tests (lock-failure Refresh e2e, lock-failure marker, corrupt-record multi-key) fail on the previous rejected.go and pass now; license package tests, vet, build and gofmt pass.

@acamarata

Copy link
Copy Markdown
Collaborator Author

Review fix: loadRejected no longer uses filepath.Glob. It lists the cache directory with os.ReadDir, matches marker names literally, and returns an error on any listing failure other than a missing directory (selection already fails closed on that error). Added two tests that fail on the previous code: a cache dir named with '[' and a cache dir with listing denied (skipped as root). Package tests pass.

@acamarata

Copy link
Copy Markdown
Collaborator Author

Rebased onto current origin/main (14 commits behind, no conflicts) and force-pushed as 782601d. The logged gate failure was a gate-host glob error (zsh: no matches found on the log path), not a Go test failure. On the rebased head, go build ./... is clean and go test ./... passes for every package, including the nginx call-site, docker funnel, 300-line and panic-baseline ratchets, with none loosened. The one exception is internal/doctor/report TestDoctorReportTimeout/slowgrand, which timed out once under full-suite load and passes when run alone (166 passed). It is outside this PR's files. internal/license/... passes 3 of 3 re-runs.

@acamarata

Copy link
Copy Markdown
Collaborator Author

Fixed the nself-ci failure in TestLicenseRefreshRewritesFile/interactive. Cause: a scheduled refresh the server rejects persists the rejection, and the interactive path (license.RefreshCache) never lifted it after a fresh server acceptance. The entitlement publish then skipped the key and the file was not rewritten. The test only failed when its map iteration ran the timer subtest first, which is why it looked flaky. Fix: interactive refresh now clears the key's persisted rejection on a live acceptance (entitlement.RecordAccepted). Added TestLicenseRefreshInteractiveLiftsPersistedRejection (deterministic order; fails without the fix, passes with it). No ratchet loosened. Rebased onto origin/main; cmd/commands, internal/license and entitlement tests pass.

@acamarata

Copy link
Copy Markdown
Collaborator Author

Fixed in e86ab0d. Interactive license refresh now records a signed valid:false reply as a persisted rejection (new entitlement.RecordRejected), so entitlement.Refresh no longer republishes the still-cached JWT. Added TestLicenseRefreshInteractiveRevocationBlocksOfflineRepublish: fails on the previous head (offline refresh succeeds for a revoked key), passes now. ./internal/license/... and the license tests in cmd/commands pass; the rest of cmd/commands has an unrelated migration-lint failure that does not involve this change.

@acamarata

Copy link
Copy Markdown
Collaborator Author

Correction to the comment above: the full ./cmd/commands package also passes on e86ab0d (no unrelated failure; the migration-lint line I saw was log output from a passing test).

@acamarata

Copy link
Copy Markdown
Collaborator Author

Rebased onto current origin/main (clean, no conflicts; new head 965c0a2). On the rebased head go build ./... and the full go test ./... pass with no failures, including the ratchet tests (nginx call sites, docker funnel allowlist, 300-line budget, panic baseline); no ratchet was changed. The previous gate failure was zsh: no matches found on a log glob on the gate host, not a test failure. Tests needing Docker were not distinguishable as failing here. Please re-run the gate on the new head.

@acamarata

Copy link
Copy Markdown
Collaborator Author

Rebased onto current origin/main (8923b22). Reran go build, go vet, gofmt check and a full uncached go test ./... (11084 passed, 153 packages): all green, including the nginx call-site, docker funnel, 300-line and panic ratchets, with none loosened. The previously reported go:test failure was a gate-host log-glob error (zsh no matches found), not a test failure at this head; no code change was needed beyond the rebase.

admin and others added 7 commits October 26, 2026 09:09
…list when selecting the entitlement token (D-1110, D-1111)
… fail closed on unreadable or unwritable state (D-1110)
…and survive rejection-lock failures (D-1110)
…hen the cache directory cannot be listed (D-1110)
…resh so offline refresh cannot republish the cached token (D-1110)
…eport git's stderr when origin is unreachable
@acamarata

Copy link
Copy Markdown
Collaborator Author

Rebased onto origin/main (clean, no conflicts); local go test ./... is green on the rebased head, including the nginx, docker-funnel, file-budget and panic ratchets (none loosened). The gate log for d676953 was never written (the log glob failed), so I read the other gate logs from today: every one fails go:test only at tools/manifestv2migrate TestReaderScan ('tag v1.4.12 is not available and fetching it failed', exit 128), on unrelated commits too. The gate checkout has no tags and its SSH fetch from origin fails. Fix: the test now retries the single-tag fetch over the public HTTPS URL and prints git's stderr. Verified in a tagless shallow clone with an unreachable origin. If the next gate run still fails there, the printed stderr will say why.

@acamarata
acamarata merged commit 309e2ed into main Oct 26, 2026
33 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant