feat: keep an accepted agent's public key after enrollment - #516
Merged
Merged
Conversation
The controller had nowhere to keep an agent's public key: it lived only on the pending-enrollment record, which acceptance deletes. Every controller-side verification path therefore found no key and skipped, which is why the agent's response signing is currently decorative. Store the key in the enrollment bucket under an accepted.<machine-id> prefix, written only by acceptance and removed on rejection, with a per-machine-ID cache invalidated by both. A missing record, an unreadable store and a signature mismatch are three distinct sentinels, so an operator can tell "not enrolled yet" from "something is forging messages". Both scans of the bucket unmarshalled whatever keys they found as pending agents, which worked only because every key shared one prefix. An accepted record's JSON overlaps PendingAgent exactly, so it would have surfaced as a phantom pending agent; both scans now filter on the pending prefix. Verification itself lands next, on top of this store. Foundational phase of specs 002-agent-key-store. No behaviour changes while PKI is disabled. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FuKUsHFG1EqZXamffh9M2c
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #516 +/- ##
=======================================
Coverage 99.95% 99.95%
=======================================
Files 489 490 +1
Lines 23180 23249 +69
=======================================
+ Hits 23170 23239 +69
Misses 10 10
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
The canceled-context case closed the stop channel immediately after starting the keepalive goroutine, so the goroutine often reached its select with both stop and ctx.Done() already ready. Select picks uniformly at random among ready cases, so the ctx.Done() return ran only about half the time. That made coverage of that line flap between runs: it shows 0, 1 or 2 misses across recent commits on main with the file unchanged. Measured here at 4 of 10 runs missing it before this change, 0 of 10 after. The other three cases already slept before stopping; this drops the condition so every case does, which lets the goroutine reach its select while a canceled context is the only ready case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FuKUsHFG1EqZXamffh9M2c
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.
Foundational phase (T001-T013) of
components/osapi/specs/002-agent-key-store.US1 and US2 build on this and close GHSA-3jh4's deferred half and GHSA-j73r
respectively.
What this adds
AcceptedAgentrecord and anaccepted.<machine-id>prefix in the existingenrollment bucket — no new bucket, config field, or provisioning step.
Record/Lookup/Removeininternal/controller/enrollment/keystore.go,with a per-machine-ID cache invalidated on write and removal rather than by
time, since a TTL would leave a removed agent verifying until it expired.
ErrAgentKeyNotFound,ErrAgentKeyStoreUnavailable,ErrAgentKeySignatureMismatch— mirroring theagent-side taxonomy added for GHSA-3jh4. A missing record and an unreadable
store are never the same answer.
AgentKeyStore, the narrow lookup the job client and target resolution willdepend on, so neither imports this package wholesale.
A bug found while implementing
ListPendingandfindPendingByiterated every key in the bucket andunmarshalled each as a
PendingAgent. That worked only because every keyhappened to share the
enrollment.prefix. Anaccepted.*record unmarshalsinto
PendingAgentcleanly —machine_id,hostname,public_keyandfingerprintall overlap — so it would have appeared as a pending agent thatcould then be "accepted" again. Both scans now filter on the pending prefix,
and a test in each proves the skip by expecting no
Getfor anaccepted.*key.
Two deviations from the task list
types.go; the existingkvPrefixis actuallyin
watcher.go, so both now sit together there, which is what the taskintended.
cmd/controller_setup.go) is deferred toUS1. The
Watcheris the store, and no consumer exists yet — wiring it nowwould add an unused parameter.
pki.FingerprintOfwas extracted fromManager.Fingerprintso the fingerprintstored at acceptance is recomputed from the key rather than copied from the
self-reported request, with one digest format in the codebase.
Gate
just react-build && just ready && just testgreen: coverage 100.0%against the 99.9% target.
The new suite is deliberately not
t.Parallel(). This package swapspackage-level marshal/now seams, and Go runs parallel tests only alongside other
parallel tests — so the rotation suite was safe as the only one. Adding a second
produced a real data race, caught by
-raceand confirmed with-count=5.🤖 Generated with Claude Code
https://claude.ai/code/session_01FuKUsHFG1EqZXamffh9M2c