feat(rotation): Phase 5 — drift detection in agent.Client (#72) - #132
Merged
sarg3nt merged 1 commit intoMay 17, 2026
Conversation
When the dashboard signs an outbound request with kid X but the agent matches kid Y instead (because rotation propagated on one side but not the other), drift detection logs the disagreement so an operator can resync. Closes the observability loop on the install→use→remove cycle: Phase 2 added the request header, the agent's auth middleware already echoes the matched kid, and this commit wires the dashboard-side comparison. What's here ----------- `agent.HeaderKID = "X-Gearbox-Kid"` is the shared constant the dashboard sends and the agent echoes back. The agent's middleware sets it on every authenticated response (see Phase 1). `agent.Client` - `SetDriftHandler(DriftHandler)` installs an optional callback invoked when `resp.Header.Get(HeaderKID)` differs from the kid the client was built with. Reads c.onDrift at RoundTrip time, so the handler can be installed AFTER construction (typical for long- lived per-box clients held by the dashboard). - Transport wrap: `kidObservingTransport` sits between the http client and the underlying TLS transport, calling `c.checkDrift` on every successful response. One central point of inspection — no invasive edits to every `doRequest*` method. - `LogDriftHandler(logger, boxID)` builds a ready-to-use DriftHandler that emits a structured warn-level log. Lowest-friction wiring for the long-lived clients held in the WebSocketManager. Test fix -------- `TestClientTimeout` was a flaky pre-existing test whose substring check was case-sensitive — Go's net/http error message capitalises "Timeout" sometimes and emits "context deadline exceeded" other times. Fixed by lower-casing the error message before substring matching. Verified stable across 5 runs. Tests ----- `client_drift_test.go` exercises the four corners of the matrix: - Drift handler fires when kid mismatches. - Doesn't fire when kid matches. - Doesn't fire when the agent omits the header (older agents). - Doesn't fire when the dashboard client has no kid. Not yet wired into production code ---------------------------------- Adding `agent.LogDriftHandler` is the small API surface; deciding *where* to call SetDriftHandler is a separate design choice (WebSocketManager? capability poller? every short-lived handler.agentClient()?). Deferring that integration so this PR stays focused on the observability primitive itself. A follow-up can install LogDriftHandler at every site that constructs a kid- bearing client. Refs: Phase 5 of the implementation plan posted to #72. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sarg3nt
force-pushed
the
feature/issue-72-phase-4-cleanup-scheduler
branch
from
May 17, 2026 19:03
71f92b0 to
7645c9b
Compare
sarg3nt
force-pushed
the
feature/issue-72-phase-5-drift-detection
branch
from
May 17, 2026 19:03
2f91727 to
7a07574
Compare
sarg3nt
merged commit May 17, 2026
e9cbf6d
into
feature/issue-72-phase-4-cleanup-scheduler
3 checks passed
4 tasks
sarg3nt
added a commit
that referenced
this pull request
May 17, 2026
…139) * feat(rotation): Phase 2 install/use/remove endpoints + rotator service (#72) Builds on Phase 1's keyring plumbing with the active machinery needed to actually rotate keys: three new agent endpoints implementing the controller-orchestrated three-phase dance (install → use → remove), the dashboard-side rotator service that drives it, and the `X-Gearbox-Kid` request/response header that Phase 5 will use for drift detection. Agent endpoints --------------- `POST /api/v1/system/keyring/install` - Body `{kid, secret_b64, role?}`. New entries default to `secondary`; the controller flips primary in a separate `/use` call. - Idempotent on `(kid, same_secret)` — re-installing returns 200 with current state. Same kid with a different secret returns 409 so a divergence between dashboard and agent surfaces loudly. - Returns 507 when the keyring is at `MaxKeyRingEntries` (4) — guards against runaway growth from a buggy rotator. `POST /api/v1/system/keyring/use` - Body `{kid}`. Flips named entry to primary, demotes the prior primary to secondary. Both stay accepted; the agent doesn't distinguish primary vs secondary for inbound auth. `DELETE /api/v1/system/keyring/{kid}` - Refuses 409 on the only remaining entry — agent never bricks itself. All three serialize through a handler-level mutex so concurrent controller calls can't race their read-modify-write of the on-disk keyring. The atomic-tmpfile+rename + `atomic.Pointer` swap from Phase 1 means the auth middleware sees the new keyring on the very next request after the swap, no restart. Dashboard side -------------- `agent.Client` - New `NewClientWithKID(url, key, kid)` constructor + `WithKID(kid)` setter. Existing `NewClient` callers untouched. - All outbound requests now go through a `setAuthHeaders(req)` helper that sets `Authorization: Bearer …` and, when the client was built with a kid, the `X-Gearbox-Kid` request header. Agent middleware echoes the matched kid in the response header of the same name; Phase 5 compares the two to detect drift. - New `KeyRingGet`, `KeyRingInstall(kid, secret, role)`, `KeyRingUse(kid)`, `KeyRingDelete(kid)` methods. `services/agent_keyring` - New package containing the `Rotator`, which composes `RotateBox(boxID, overlap)` from the install/use/remove primitives: decrypt current primary → build authenticated client → install new key on agent as secondary → persist new key to box_agent_keys → flip primary on agent → flip primary in DB (which also stamps `retired_at` on the old entry). - `CleanupRetiredKeys(boxID, overlap)` removes any entries whose `retired_at` is older than the overlap window. Uses `time.Since` for the cutoff comparison so SQLite-driver timezone behaviour doesn't bite (modernc.org/sqlite scans bare DATETIMEs in local time; comparing against `time.Now()` in UTC would otherwise mis- fire). Removes from agent first, then DB; tolerates the agent having already lost the entry (404) or refusing to remove the last key (409) since neither leaves us in a bad state. - `DefaultOverlapWindow = 24h`. Tunable per-call so Phase 4's scheduler can pick the operator's configured value. `SetBoxPrimaryKey` now stamps `retired_at` with `time.Now().UTC()` explicitly rather than letting SQLite emit `CURRENT_TIMESTAMP`, so the value round-trips correctly through the driver's date parser. Tests ----- - 11 keyring-endpoint integration tests (agent side): install adds secondary, install is idempotent, KID collision with different secret returns 409, malformed secret returns 400, use flips primary, use of unknown kid returns 404, delete works, delete of only-remaining-entry returns 409, mutations persist across in-process reload + on-disk reload, keyring file mode is 0600, install over MaxKeyRingEntries returns 507. - 4 rotator integration tests against a `httptest` mock of the agent's keyring API (the dashboard module can't import the agent module): happy path covers full install → use → DB-flip, cleanup removes retired keys past the overlap window, cleanup leaves keys within the overlap window alone, missing-box returns error. - All existing tests in both modules still pass. Refs: Phase 2 of the implementation plan posted to #72. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(rotation): Phase 3 manual rotate UI + handlers (#72) Surfaces the Phase 2 rotator behind two operator-visible buttons — the bit that makes the keyring work actually usable. No new back-end abstractions; just two thin handlers that compose the existing rotator with the dashboard's box management. Backend ------- `POST /settings/boxes/{id}/rotate-key` - Rotates one box. Returns 200 with `{success, new_kid, old_kid, retire_after}` or 4xx/5xx with `{success: false, message}`. - Constructs a rotator per request from the handler's existing DB + encryptor — no new singletons. `POST /settings/boxes/rotate-key-all` - Iterates every enabled box and rotates each through the same rotator. Reports per-box success/failure so the operator sees exactly which boxes need follow-up. Failures on one box don't halt the run; this matches the homelab use case better than a strict circuit-breaker — operator decides whether to investigate one bad box or move on. Both routes are wired into the admin-only `/settings` group in cmd/server/main.go alongside the existing box CRUD routes; same permission gate as `HAProxyBoxUpdatePost` etc. UI -- Box edit form (`HAProxyBoxEditPage`) - New "Rotate API key" section under the API-Key field with a Rotate Key button. Visible only on edit (server != nil). - Click → `showConfirmDialog` (warning style) explaining the 24h overlap → POST → toast on success or alert dialog on failure. - Reuses the in-page rotate-spinner SVG to surface in-flight state. Boxes list (`HAProxyBoxesPageContent`) - New "Rotate All Keys" button next to the existing "Add Box" button. Visible only when at least one box exists. - Click → confirm → POST → success toast or alert dialog with a per-box failure list when partial. JS uses the established `showConfirmDialog` / `showAlertDialog` / `showToast` APIs from `layouts.Base`, per the CLAUDE.md "never use native confirm/alert/prompt" rule. Tests ----- No new tests — the rotator's behaviour is already covered by `services/agent_keyring/rotator_test.go` (Phase 2). The handlers are thin enough that adding HTTP-level tests would duplicate the rotator-side coverage. Browser-level testing of the new UI was not performed in this commit; operator should smoke-test by hitting both buttons end-to-end before merging. Refs: Phase 3 of the implementation plan posted to #72. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(rotation): Phase 4 retired-key cleaner (#72) Adds a background sweeper that walks every enabled box on a tick and removes any keys whose retired_at + overlap window has passed — completing the install→use→remove three-phase rotation cycle without operator intervention. Originally Phase 4 in the issue plan also covered auto-rotation on a schedule (off by default, configurable cadence). That needs a new global-settings surface in the dashboard (no app-level config table exists today; only user_preferences), which is its own design pass. Deferring the auto-rotate scheduler to a follow-up so this PR stays focused on the piece that's actually needed regardless of whether auto-rotation is enabled. What's here ----------- `services/agent_keyring/cleaner.go` - `RetiredKeyCleaner` runs as a goroutine off the dashboard's process-lifetime context. - Hourly tick (`CleanerInterval`) — short enough that a 24h rotation cleans up within a few hours of its target, long enough that the sweep is cheap. - Immediate sweep on start so a freshly-deployed dashboard catches up on any retired keys left from manual rotations done while the prior instance was down. - Per-box failures are logged but don't halt the sweep; one unreachable agent shouldn't block cleanup on the others. `cmd/server/main.go` - Wires the cleaner into startup alongside the existing alert evaluator. Cancelled when main returns. Tests ----- - `TestCleaner_RemovesRetiredKeyOnTick` — rotates a box, then runs the cleaner with a 1ms overlap and 20ms interval; verifies the retired key is removed from both the mock agent and the DB. - `TestCleaner_NoopWhenNothingRetired` — no rotation happens, so the cleaner finds nothing to do; verifies the seeded entry survives a sweep. Refs: Phase 4 (lite) of the implementation plan posted to #72. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(rotation): Phase 5 drift detection in agent.Client (#72) (#132) When the dashboard signs an outbound request with kid X but the agent matches kid Y instead (because rotation propagated on one side but not the other), drift detection logs the disagreement so an operator can resync. Closes the observability loop on the install→use→remove cycle: Phase 2 added the request header, the agent's auth middleware already echoes the matched kid, and this commit wires the dashboard-side comparison. What's here ----------- `agent.HeaderKID = "X-Gearbox-Kid"` is the shared constant the dashboard sends and the agent echoes back. The agent's middleware sets it on every authenticated response (see Phase 1). `agent.Client` - `SetDriftHandler(DriftHandler)` installs an optional callback invoked when `resp.Header.Get(HeaderKID)` differs from the kid the client was built with. Reads c.onDrift at RoundTrip time, so the handler can be installed AFTER construction (typical for long- lived per-box clients held by the dashboard). - Transport wrap: `kidObservingTransport` sits between the http client and the underlying TLS transport, calling `c.checkDrift` on every successful response. One central point of inspection — no invasive edits to every `doRequest*` method. - `LogDriftHandler(logger, boxID)` builds a ready-to-use DriftHandler that emits a structured warn-level log. Lowest-friction wiring for the long-lived clients held in the WebSocketManager. Test fix -------- `TestClientTimeout` was a flaky pre-existing test whose substring check was case-sensitive — Go's net/http error message capitalises "Timeout" sometimes and emits "context deadline exceeded" other times. Fixed by lower-casing the error message before substring matching. Verified stable across 5 runs. Tests ----- `client_drift_test.go` exercises the four corners of the matrix: - Drift handler fires when kid mismatches. - Doesn't fire when kid matches. - Doesn't fire when the agent omits the header (older agents). - Doesn't fire when the dashboard client has no kid. Not yet wired into production code ---------------------------------- Adding `agent.LogDriftHandler` is the small API surface; deciding *where* to call SetDriftHandler is a separate design choice (WebSocketManager? capability poller? every short-lived handler.agentClient()?). Deferring that integration so this PR stays focused on the observability primitive itself. A follow-up can install LogDriftHandler at every site that constructs a kid- bearing client. Refs: Phase 5 of the implementation plan posted to #72. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
Stacked on #131 (Phase 4). When the dashboard signs an outbound request with kid X but the agent matches kid Y instead (because rotation propagated on one side but not the other), drift detection logs the disagreement so an operator can resync. Closes the observability loop on the install→use→remove cycle: Phase 2 added the request header, the agent's auth middleware already echoes the matched kid, and this commit wires the dashboard-side comparison.
What's here
agent.Client.SetDriftHandler(DriftHandler)installs an optional callback invoked whenresp.Header.Get(\"X-Gearbox-Kid\")differs from the kid the client was built with. Handler can be installed after construction (typical for long-lived per-box clients)kidObservingTransportwraps the http client's transport so every successful response is inspected at one central point — no invasive edits to everydoRequest*methodagent.LogDriftHandler(logger, boxID)builds a ready-to-use DriftHandler that emits a structured warn-level logBonus: fix flaky
TestClientTimeoutThe pre-existing test had a case-sensitive substring check that occasionally missed Go's
\"Client.Timeout exceeded\"formatting (capital T). Lower-cased the match. Verified stable across 5 runs.Not yet wired into production code
Adding
agent.LogDriftHandleris the small API surface; deciding where to callSetDriftHandleris a separate design choice (WebSocketManager? capability poller? every short-lived handler.agentClient()?). Deferring that integration so this PR stays focused on the observability primitive itself. A follow-up can installLogDriftHandlerat every site that constructs a kid-bearing client.Test plan
TestClientTimeoutis stable across multiple runsLogDriftHandlerinto agent.Client construction sites in a follow-up PRStack
Phase 5 of 5 (final). Base:
feature/issue-72-phase-4-cleanup-scheduler(#131).Closes part of #72.
🤖 Generated with Claude Code