Commit 265a26a
* feat(rotation): Phase 1 multi-key keyring plumbing (#72)
Foundation for issue #72's rotation work. Adds the data structures and
storage required for N-entry keyrings on both the agent and dashboard
sides, with no operator-visible behaviour change yet — rotation
endpoints and UI follow in Phase 2.
Agent side
----------
- `internal/framework/crypto/keyring.go` — `KeyRing` type with up to
`MaxKeyRingEntries = 4` accepted keys, atomic tmpfile+rename on disk,
AES-256-GCM (GBE1) encryption when `GEARBOX_AGENT_ENCRYPTION_KEY` is
set. Wire token format: `gbx_<6-hex-kid>_<base64url(32 random bytes)>`,
with legacy 64-hex tokens still accepted for one release cycle.
- `LoadOrCreateKeyRing(keyringPath, legacyAPIKeyPath)` migrates an
existing `/var/lib/gearbox-agent/api-key` file into a single keyring
entry tagged `kid="legacy"`, role=primary. Legacy file stays on disk
as a read-only fallback.
- `KeyRingPointer` wraps `atomic.Pointer[KeyRing]` so Phase 2's
install/use/remove endpoints can swap the live keyring without
middleware restart. Verified by the new auth-middleware test
`TestAPIKeyAuth_HotSwapVisibleImmediately`.
- `internal/framework/middleware/auth.go` rewritten to take a keyring
pointer instead of a static key. Accepts both prefixed and legacy
token formats; matched `kid` echoed back as `X-Gearbox-Kid:` header
on every authenticated response so the dashboard can detect drift
(consumed in Phase 5). Auth with a secondary key logs at INFO so
the audit log can later flag "old key still in use after rotation".
- New endpoint `GET /api/v1/system/keyring` (authenticated) returns
metadata only — kids, roles, created_at, sha256-prefix fingerprint
for diagnostic equality checks — never the secret bytes themselves.
- `--show-api-key` and `--rotate-api-key` CLI flags work against the
keyring; the printed key uses the new `gbx_<kid>_<b64>` wire format
the dashboard can paste verbatim.
- `GEARBOX_AGENT_KEYRING_PATH` env var (default
`<DataDir>/keyring.json`) is now a config field alongside the legacy
`HAPROXY_AGENT_API_KEY_PATH`.
Dashboard side
--------------
- Migration `000002_add_box_agent_keys` adds the
`(box_id, kid)`-keyed `box_agent_keys` table and idempotently
backfills one `kid='legacy'` row per existing box from
`boxes.api_key_encrypted`. The legacy column stays for one release.
- `database/box_agent_keys.go` exposes Get/Insert/SetPrimary/Delete/
TouchLastUsed — the storage primitives Phase 2's rotator service
composes into the install→use→remove dance.
Tests
-----
- 19 keyring unit tests covering token parsing (prefixed + legacy +
malformed), keyring mutation, file round-trip with and without
encryption, legacy api-key migration, and pointer hot-swap.
- 8 auth-middleware integration tests covering bearer parsing, kid
header echo, secondary-key acceptance, and the live hot-swap path
Phase 2 depends on.
- 5 storage tests covering primary-key lookup, atomic role flip,
delete-refuses-last guard, and last_used_at touch.
Carry-overs to Phase 2 (intentional gaps surfaced from this PR)
---------------------------------------------------------------
- `DeleteBox` does not yet cascade to `box_agent_keys` (SQLite
`PRAGMA foreign_keys` is off in this codebase; enabling it is a
broader change). Phase 2's box-delete path will clean dependent
rows explicitly. Documented in box_agent_keys_test.go.
- The dashboard's `agent.Client` does not yet send `X-Gearbox-Kid`
on outbound requests — there's no kid to send while every box's
keyring contains only the legacy entry. Phase 2 wires this when
the rotator starts mutating keyrings.
Refs: research summary and implementation plan posted to #72.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(rotation): address Copilot review on PR #128
Nine findings from the Copilot review on PR #128, all valid or
worth addressing. Fixed in this commit; replies + thread-resolves
go with the push.
1. LoadOrCreateKeyRing fall-through (keyring.go:131-167)
Was: any error reading the legacy api-key file (incl. ErrKeyRequired
from a missing encryption-key env, or a permission error) silently
fell through to generating a fresh keyring — would rotate every
dashboard out for a transient operator mistake.
Now: distinguish "file doesn't exist" (proceed to fresh-gen) from
"file exists but errored / malformed" (return the error to the
caller). os.Stat + os.IsNotExist gates the choice explicitly.
2. MatchToken constant-time guarantee (keyring.go ~195)
Was: the prefixed-token path returned early on the first kid match,
making total runtime depend on which kid the request claimed — kid
enumeration via timing. The doc said "All comparisons are
constant-time" but the prefixed branch broke that promise.
Now: walk every entry, compare both kid and secret with
subtle.ConstantTimeCompare, AND the two results. Match is recorded
without short-circuit; runtime is uniform regardless of which kid
(if any) matches. Doc updated to reflect the actual guarantee.
3. writeKeyRingFile mutates input (keyring.go ~415)
Was: the function populated SecretHex on each entry of the passed-
in keyring before marshaling. KeyRing values are shared via
atomic.Pointer and treated as immutable; mutating in-place risks
races with concurrent middleware readers.
Now: marshal off a local snapshot whose entries have SecretHex
backfilled from Secret where needed. Input is never written to.
4. --rotate-api-key zero CreatedAt (main.go ~155)
Was: the fresh KeyRingEntry built for the CLI rotate command
omitted CreatedAt, so the keyring file got 0001-01-01T00:00:00Z
and the /api/v1/system/keyring metadata exposed the same.
Now: CreatedAt: time.Now().UTC().
5. handleGet nil-guard (api/keyring.go ~50)
Was: h.keyring.Load() was dereferenced unconditionally; a future
wiring bug that left the pointer nil would panic the agent on
every keyring request.
Now: nil check + 500 + log line. Fails loud rather than crashing.
6. At-most-one-primary-per-box constraint (migration 000002)
Was: nothing in the schema stopped two rows with role='primary'
for the same box. SetBoxPrimaryKey's transactional flip is
correct, but a buggy code path or a manual DB edit could produce
the invalid state and GetBoxPrimaryKey would return an arbitrary
row.
Now: partial unique index on box_agent_keys(box_id) WHERE
role='primary'. SQLite supports this directly; index is dropped
in the down migration too.
7. Test naming clarity (box_agent_keys_test.go)
Was: TestBoxAgentKeys_MigrationBackfillsLegacyEntry was named as
if it validated migration behaviour but actually only exercised
InsertBoxAgentKey + GetBoxPrimaryKey roundtrip; the comment also
misled.
Now: split into two clearly-named tests —
InsertAndLookup covers the roundtrip, and a new
MigrationBackfillStatementWorks test wipes the migrated rows for
a single box, re-executes the migration's INSERT-FROM-boxes
statement, and asserts the row appears + reruns are idempotent.
8. DeleteBox cascade gap (servers.go DeleteBox)
Was: the schema declared ON DELETE CASCADE but PRAGMA
foreign_keys is off in this codebase, so deleting a box left
orphaned box_agent_keys rows holding encrypted secrets. Phase 1
docs flagged this as a deferred gap; Copilot pushed back, and
fairly — it's a small, contained fix.
Now: DeleteBox runs inside a transaction that wipes
box_agent_keys WHERE box_id = ? before deleting from boxes. Both
succeed or neither does.
Test re-added: TestBoxAgentKeys_DeleteBoxClearsDependentKeys.
9. APIKeyAuth nil-guard (middleware/auth.go)
Was: keyring.Load() was called without first checking the
pointer itself for nil. A miswired ServerConfig would panic on
every authenticated request.
Now: fail-closed nil check at the top of the request handler —
returns 401 + logs at error level. Same defensive treatment as
fix #5.
Tests
-----
All 3 dashboard-side suites pass (`database` package, 7 new tests
including the new DeleteBox cascade test). All 3 agent-side suites
pass (`crypto`, `middleware`, `api`).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(rotation): clarify constant-time precondition on MatchToken
Add a note that subtle.ConstantTimeCompare's length-dependent
fast-fail is fine here because every kid in the system is exactly 6
chars long (kidLength = 6 hex chars; the legacy entry uses 'legacy'
which is also 6 chars by deliberate convention). Custom kids of a
different length would naturally hash-mismatch — which is the
intended failure mode.
Also serves to force a synchronize event so PR #128's CI re-runs
on the fix commit; the prior synchronize from fe3c762 didn't
trigger workflows (still unclear why; not blocking the work).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(rotation): adapt phase-1 to main after rebase
Two changes required by main moving forward (PRs #127, #134, #137):
1. internal/api/server_test.go was added in PR #127 (remote console)
after Phase 1 branched. It uses the old ServerConfig.APIKey field
that Phase 1 replaced with KeyRing. Updated the test to construct
a one-entry KeyRing and send the legacy 64-hex bearer token.
2. PR #127 also added migration 000002_add_box_console_enabled,
colliding with Phase 1's 000002_add_box_agent_keys. Renumbered
Phase 1's migration to 000003. Migrations are content-addressed
by the embedded iofs, so the rename is mechanical — no schema
change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent e09943a commit 265a26a
14 files changed
Lines changed: 1893 additions & 47 deletions
File tree
- gearbox-agent
- cmd/gearbox-agent
- internal
- api
- framework
- config
- crypto
- middleware
- gearbox/internal/framework/database
- migrations/files
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
113 | 113 | | |
114 | 114 | | |
115 | 115 | | |
116 | | - | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
117 | 122 | | |
118 | | - | |
| 123 | + | |
119 | 124 | | |
120 | | - | |
| 125 | + | |
121 | 126 | | |
122 | 127 | | |
123 | | - | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
124 | 134 | | |
125 | 135 | | |
126 | 136 | | |
127 | 137 | | |
128 | | - | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
129 | 143 | | |
130 | | - | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
131 | 159 | | |
132 | 160 | | |
133 | | - | |
134 | | - | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
135 | 170 | | |
136 | 171 | | |
137 | 172 | | |
138 | | - | |
| 173 | + | |
139 | 174 | | |
140 | 175 | | |
141 | 176 | | |
| |||
244 | 279 | | |
245 | 280 | | |
246 | 281 | | |
247 | | - | |
248 | | - | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
249 | 289 | | |
250 | | - | |
| 290 | + | |
251 | 291 | | |
252 | 292 | | |
| 293 | + | |
253 | 294 | | |
254 | | - | |
255 | | - | |
256 | | - | |
257 | | - | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
258 | 302 | | |
259 | | - | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
260 | 309 | | |
261 | 310 | | |
262 | 311 | | |
| |||
398 | 447 | | |
399 | 448 | | |
400 | 449 | | |
401 | | - | |
| 450 | + | |
402 | 451 | | |
403 | 452 | | |
404 | 453 | | |
| |||
493 | 542 | | |
494 | 543 | | |
495 | 544 | | |
| 545 | + | |
496 | 546 | | |
497 | 547 | | |
498 | | - | |
| 548 | + | |
499 | 549 | | |
500 | 550 | | |
501 | 551 | | |
| 552 | + | |
502 | 553 | | |
503 | 554 | | |
504 | 555 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
| 20 | + | |
20 | 21 | | |
21 | 22 | | |
22 | 23 | | |
| |||
26 | 27 | | |
27 | 28 | | |
28 | 29 | | |
29 | | - | |
| 30 | + | |
30 | 31 | | |
31 | 32 | | |
32 | 33 | | |
33 | 34 | | |
34 | 35 | | |
35 | 36 | | |
36 | 37 | | |
37 | | - | |
| 38 | + | |
38 | 39 | | |
39 | 40 | | |
40 | 41 | | |
| |||
147 | 148 | | |
148 | 149 | | |
149 | 150 | | |
150 | | - | |
| 151 | + | |
151 | 152 | | |
152 | 153 | | |
153 | 154 | | |
| |||
193 | 194 | | |
194 | 195 | | |
195 | 196 | | |
196 | | - | |
| 197 | + | |
197 | 198 | | |
198 | 199 | | |
199 | 200 | | |
| |||
204 | 205 | | |
205 | 206 | | |
206 | 207 | | |
207 | | - | |
208 | | - | |
209 | | - | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
210 | 212 | | |
211 | 213 | | |
212 | 214 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | 3 | | |
| 4 | + | |
4 | 5 | | |
5 | 6 | | |
6 | 7 | | |
7 | 8 | | |
| 9 | + | |
8 | 10 | | |
9 | 11 | | |
| 12 | + | |
10 | 13 | | |
11 | 14 | | |
12 | 15 | | |
| |||
29 | 32 | | |
30 | 33 | | |
31 | 34 | | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
32 | 47 | | |
33 | 48 | | |
34 | | - | |
| 49 | + | |
35 | 50 | | |
36 | 51 | | |
37 | 52 | | |
| |||
42 | 57 | | |
43 | 58 | | |
44 | 59 | | |
45 | | - | |
| 60 | + | |
46 | 61 | | |
47 | 62 | | |
48 | 63 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
35 | 35 | | |
36 | 36 | | |
37 | 37 | | |
38 | | - | |
39 | | - | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
40 | 50 | | |
41 | 51 | | |
42 | 52 | | |
| |||
174 | 184 | | |
175 | 185 | | |
176 | 186 | | |
| 187 | + | |
177 | 188 | | |
178 | 189 | | |
179 | 190 | | |
| |||
0 commit comments