amadan
edge+host: mesh key review fixes — standalone mint loop, canonical host, tolerant .next, honest rotation comment, quiet mint
FINDING 1 (Critical): mintMeshKey rode retentionLoop's tick, which returns early — never arming at all — on a box missing CARLOS_RELEASE_PUBKEY; such a box still wins the housekeeper election, so the mesh key would silently never mint fleet-wide. Fixed with mintMeshKeyLoop, its own loop in housekeeper.go, gated on nothing but deployment-store presence (mirrors edgesFileLoop's shape exactly), armed beside edgesFileLoop in RunAgent. retentionLoop is back to prune alone. Plan's Task 2 wiring sentence and Step 3 sample corrected in-place (dated amendments) to name the standalone loop, matching the File Structure section's original intent. FINDING 2 (Important): hopMAC now canonicalizes host (lowercase, strip trailing dot, strip :port via net.SplitHostPort) before it enters the MAC, so a signer's and a verifier's differing-but-equivalent host forms can never diverge into spurious refusals. Doc comments state the MAC is over the concrete request host, never a wildcard map key. FINDING 3 (Important): loadMeshKeys now tolerates a malformed control/edge-mesh-key.next per-key — an unparseable .next logs loudly and is treated as "no rotation in flight" rather than failing the whole call and taking the live current key down with it. FINDING 4 (Important): the file header's rotation-safety claim rewritten to state the truth: dual-accept removes the PROMOTION window, not the PROPAGATION window (a signer prefers .next instantly; a remote verifier learns via its own ~30s poll), and names the real cost of that window (served as an ordinary client, stripped XFF — not an outage). FINDING 5 (Important): a lost CAS race (key already minted) is now completely silent, matching housekeeper.go's own silence on election contention — no log level existed to hedge with, so the honest fix is no line at all, not "already minted" every tick forever. Covering tests: TestMintMeshKeyLoopArmsWithoutReleasePubkey (the Finding 1 regression guard), TestMintMeshKeyLoopIsAPlatformBoxJob, TestHopMACCanonicalizesHost, TestLoadMeshKeysToleratesAnUnreadableNext, TestMintMeshKeySecondMintIsSilent. Full gate green: build/vet/gofmt/darwin, all 21 packages, -race on internal/edge and internal/host.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
6 files changed,
+350
−46
docs/superpowers/plans/2026-08-22-multi-region-backhaul.md+24 −7internal/edge/mesh.go+102 −29internal/edge/mesh_test.go+119 −0internal/host/agent.go+8 −10internal/host/housekeeper.go+48 −0internal/host/housekeeper_test.go+49 −0
diff --git a/docs/superpowers/plans/2026-08-22-multi-region-backhaul.md b/docs/superpowers/plans/2026-08-22-multi-region-backhaul.md| index f99fb432d..34741bc8a 100644 |
| --- a/docs/superpowers/plans/2026-08-22-multi-region-backhaul.md |
| +++ b/docs/superpowers/plans/2026-08-22-multi-region-backhaul.md |
| @@ -177,8 +177,18 @@ git commit -m "host: per-box edges file — what this box homes, for the fleet's |
| **Files:** |
| - Create: `internal/edge/mesh.go` |
| - Test: `internal/edge/mesh_test.go` |
| -- Modify: `internal/host/agent.go` (the `runHousekeeperDuties` call at |
| - ~:1632 gains the mint duty beside `prune`) |
| +- Modify: `internal/host/housekeeper.go` (a standalone `mintMeshKeyLoop`, |
| + armed beside `edgesFileLoop` in `RunAgent` — **Amendment (2026-08-22, |
| + review):** the line above originally read "the `runHousekeeperDuties` |
| + call at ~:1632 gains the mint duty beside `prune`", i.e. folded into |
| + `retentionLoop`'s tick; that is wrong and this File Structure section's |
| + own `internal/host/housekeeper.go` entry ("mesh-key mint duty, T2, |
| + additive") was the plan's real intent all along. `retentionLoop` returns |
| + early — never arming at all — on a box without `CARLOS_RELEASE_PUBKEY`, |
| + and such a box still wins the housekeeper election, so a mint duty living |
| + only inside it would silently never run fleet-wide. The mint needs its |
| + own loop, gated on nothing but deployment-store presence, mirroring |
| + `edgesFileLoop`'s shape.) |
| - Modify: `internal/host/housekeeper_test.go` (duty-count expectations, |
| if any assert exact duty sets) |
| @@ -241,15 +251,22 @@ func TestHopSignatureRoundTripsAndBoundsSkew(t *testing.T) { |
| ``` |
| - [ ] **Step 2: verify failure. Step 3: implement** per the interfaces |
| - above; wire the host-side duty: |
| + above; wire the host-side duty as its own loop (**Amendment |
| + (2026-08-22, review):** NOT folded into `retentionLoop`'s |
| + `runHousekeeperDuties(housekeeperGate, prune, mintMesh)` call as this |
| + step originally showed — see the Files section above for why): |
| ```go |
| -runHousekeeperDuties(housekeeperGate, prune, mintMesh) |
| +runHousekeeperDuties(housekeeperGate, mint) |
| ``` |
| - where `mintMesh` closes over the deployment store and calls |
| - `edge.MintMeshKey`; a mint error logs and does not fail the tick |
| - (idempotent next tick). Single-box inertness: key mints once, nothing |
| + called from `mintMeshKeyLoop`'s own `for { sleep; ... }`, armed |
| + unconditionally on store presence (mirroring `edgesFileLoop`) rather |
| + than riding `retentionLoop`'s `CARLOS_RELEASE_PUBKEY` gate. |
| + |
| + `mint` closes over the deployment store and calls `edge.MintMeshKey`; |
| + a mint error logs and does not fail the tick (idempotent next tick). |
| + Single-box inertness: key mints once, nothing |
| reads it until Tasks 3-4. |
| - [ ] **Step 4:** package gates both packages. **Step 5: Commit** |
| `edge+host: mesh key — housekeeper mints create-only, HMAC hop vocabulary`. |
diff --git a/internal/edge/mesh.go b/internal/edge/mesh.go| index 9f056ea5d..25d14fe24 100644 |
| --- a/internal/edge/mesh.go |
| +++ b/internal/edge/mesh.go |
| @@ -11,34 +11,59 @@ package edge |
| // housekeeper's other duties already use: exactly ONE box mints a shared |
| // secret, every box mirrors it, and no second election is invented. |
| // |
| -// - mintMeshKey is the housekeeper duty (wired beside the releases |
| -// retention prune, internal/host/agent.go): 32 random bytes, |
| -// hex-encoded, written to control/edge-mesh-key with an empty ifMatch — |
| -// release.Store's create-only form. That IS the compare-and-swap: two |
| -// housekeepers racing the same tick (the transient double-holder window |
| -// housekeeper.go's poll cadence allows) both attempt the Put, exactly |
| -// one wins, and the loser's release.ErrCASConflict is not a failure — |
| -// it is proof a key already exists, which is the only thing this func |
| -// promises. Every later tick calls it again and finds the same |
| -// conflict, forever, for the deployment's life: minting is a one-time |
| -// event, not a duty that ever "finishes". |
| +// - mintMeshKey is the housekeeper duty (wired as its own loop, |
| +// internal/host/housekeeper.go's mintMeshKeyLoop — see that func's |
| +// comment for why it is NOT folded into retentionLoop's tick): 32 |
| +// random bytes, hex-encoded, written to control/edge-mesh-key with an |
| +// empty ifMatch — release.Store's create-only form. That IS the |
| +// compare-and-swap: two housekeepers racing the same tick (the |
| +// transient double-holder window housekeeper.go's poll cadence allows) |
| +// both attempt the Put, exactly one wins, and the loser's |
| +// release.ErrCASConflict is not a failure — it is proof a key already |
| +// exists, which is the only thing this func promises. Every later tick |
| +// calls it again and finds the same conflict, forever, for the |
| +// deployment's life: minting is a one-time event, not a duty that ever |
| +// "finishes" — and it stays SILENT after the first tick (see |
| +// mintMeshKey's own comment): the steady state of every tick but one, |
| +// forever, is not something worth a log line, exactly like the |
| +// housekeeper election's own contention. |
| // |
| // - signHop / verifyHop are the hop vocabulary Task 3/4 wire onto the |
| // request path under the headers X-Carlos-Hop: 1 and X-Carlos-Hop-Auth: |
| // <unix>:<hex>. The MAC binds the target host into the signature so one |
| // hop's header cannot be replayed against another, and the timestamp |
| -// bounds replay in time (±hopSkew) independent of the host check. |
| +// bounds replay in time (±hopSkew) independent of the host check. The |
| +// host is canonicalized (canonicalizeHopHost: lowercased, trailing dot |
| +// stripped, :port stripped) before it ever reaches the MAC, and the MAC |
| +// is always over the CONCRETE request host a hop is bound for — never a |
| +// wildcard map key (e.g. "*.example.com") — because Task 3 verifies |
| +// against the literal net/http request host and Task 4 signs the same; |
| +// canonicalizing in one place (hopMAC) is what keeps a signer's and a |
| +// verifier's idea of "host" from ever diverging on case, a trailing |
| +// root-label dot, or a stray port. |
| // |
| // - Rotation's READ side ships in this task, ahead of any code that |
| // writes .next: control/edge-mesh-key.next mirrors a rotation |
| // successor while one is in flight, loadMeshKeys reads both, |
| // verifyHopEitherKey accepts a signature under either, and signingKey |
| // is what a signer consults to prefer .next the instant it exists — |
| -// never waiting for a runbook step. That is what makes the promotion |
| -// itself (copy .next over control/edge-mesh-key, delete .next) safe as |
| -// a plain runbook action: every verifier already accepts both keys |
| -// before promotion happens, so there is no window where a box that |
| -// picked up .next early gets refused by a box that hasn't promoted yet. |
| +// never waiting for a runbook step. |
| +// |
| +// What this removes is the PROMOTION window, not the PROPAGATION one. |
| +// A signer starts using .next as soon as its OWN box has picked it up; |
| +// a remote verifier accepts a .next-signed hop only once ITS OWN |
| +// mirror has too (Task 3/4's poll of the control/ prefix, up to ~30s |
| +// behind a write). Between those two moments a hop signed under .next |
| +// CAN be refused by a verifier that hasn't caught up — that is a real, |
| +// if short, window, and per Task 3's rules the cost of that refusal is |
| +// the request being served as an ordinary client (its X-Forwarded-For |
| +// stripped), not dropped: a degraded hop, not an outage. What |
| +// dual-accept actually buys is that the runbook's promotion step |
| +// itself (copy .next over control/edge-mesh-key, delete .next) needs |
| +// no synchronized cutover: because every verifier already accepts |
| +// EITHER key well before promotion happens, that write can land as an |
| +// ordinary, un-sequenced action with no box needing to be told "stop |
| +// signing with the old key" first. |
| // |
| // Single-box inertness: on a deployment with one platform box, this box is |
| // always the housekeeper (housekeeper.go's housekeeperSole), so it mints its |
| @@ -54,6 +79,7 @@ import ( |
| "errors" |
| "fmt" |
| "log" |
| + "net" |
| "os" |
| "strconv" |
| "strings" |
| @@ -88,6 +114,15 @@ const hopSkew = 30 * time.Second |
| // minted one yet. crypto/rand is required — this key authenticates hops |
| // between platform boxes, so it gets the same random source every other |
| // credential in this codebase does, never math/rand. |
| +// |
| +// A lost race (release.ErrCASConflict — a key already exists, from this |
| +// box's own earlier tick or another housekeeper that won the same race) is |
| +// deliberately SILENT, not logged even at "already minted": that outcome is |
| +// the steady state of every tick but the very first, forever, for the |
| +// deployment's whole life — one line every tick would be ~264 lines/day of |
| +// nothing happening, exactly the noise housekeeper.go's own election |
| +// contention refuses to log for the same reason. Only an actual, novel mint |
| +// gets a line. |
| func mintMeshKey(ctx context.Context, s release.Store) error { |
| buf := make([]byte, meshKeyBytes) |
| if _, err := rand.Read(buf); err != nil { |
| @@ -96,11 +131,6 @@ func mintMeshKey(ctx context.Context, s release.Store) error { |
| body := []byte(hex.EncodeToString(buf)) |
| if _, err := s.Put(ctx, meshKeyKey, body, ""); err != nil { |
| if errors.Is(err, release.ErrCASConflict) { |
| - // Success-by-other: a key already exists (this box's earlier |
| - // tick, or another housekeeper that won the same race). Debug, |
| - // not error — this is the steady state for every tick after the |
| - // first, forever. |
| - log.Printf("edge: mesh key: already minted") |
| return nil |
| } |
| return fmt.Errorf("mesh key: put: %w", err) |
| @@ -110,8 +140,8 @@ func mintMeshKey(ctx context.Context, s release.Store) error { |
| } |
| // MintMeshKey is mintMeshKey's exported wrapper — the seam |
| -// internal/host's housekeeper duty calls through, so the duty lives beside |
| -// the releases-retention prune in internal/host/agent.go while the key's own |
| +// internal/host's housekeeper duty (mintMeshKeyLoop, housekeeper.go) calls |
| +// through, so the duty lives beside the housekeeper while the key's own |
| // shape (its object name, its random-byte discipline) stays edge's to own. |
| func MintMeshKey(ctx context.Context, s release.Store) error { |
| return mintMeshKey(ctx, s) |
| @@ -141,14 +171,25 @@ func getMeshKeyHex(ctx context.Context, s release.Store, key string) ([]byte, er |
| // only before the housekeeper has minted anything; next is nil the whole |
| // rest of the time — nearly always, since a rotation is a deliberate, |
| // short-lived operator action. |
| +// |
| +// A decode failure on meshKeyNextKey does NOT propagate: .next is written |
| +// by hand during a rotation (there is no code in this task that writes it), |
| +// so one bad upload is a real possibility, and the safe response to "I |
| +// can't read the rotation successor" is to act as though no rotation is in |
| +// flight — not to take every hop fleet-wide down by refusing the perfectly |
| +// good current key alongside it. The failure is loud (an operator mistake |
| +// worth paging on), just not fatal to this call. A decode failure on |
| +// meshKeyKey itself DOES propagate: that is the live, day-to-day signing |
| +// and verifying key, and there is no safe fallback for it being unreadable. |
| func loadMeshKeys(ctx context.Context, s release.Store) (current, next []byte, err error) { |
| current, err = getMeshKeyHex(ctx, s, meshKeyKey) |
| if err != nil { |
| return nil, nil, err |
| } |
| - next, err = getMeshKeyHex(ctx, s, meshKeyNextKey) |
| - if err != nil { |
| - return nil, nil, err |
| + next, nextErr := getMeshKeyHex(ctx, s, meshKeyNextKey) |
| + if nextErr != nil { |
| + log.Printf("edge: mesh key: %s is unreadable, treating as no rotation in flight until fixed: %v", meshKeyNextKey, nextErr) |
| + next = nil |
| } |
| return current, next, nil |
| } |
| @@ -169,6 +210,11 @@ func signingKey(current, next []byte) []byte { |
| // HMAC-SHA256 over "host\nunix-timestamp", hex-encoded and prefixed with the |
| // timestamp so a verifier does not have to guess it. host is bound into the |
| // MAC so a signature minted for one hop cannot be replayed against another. |
| +// host must be the CONCRETE host a hop is bound for (a net/http request |
| +// host, e.g. from Task 4's forwarding decision) — never a wildcard home-map |
| +// key like "*.example.com" — because hopMAC canonicalizes it the same way a |
| +// verifier's literal request host is canonicalized, and a wildcard pattern |
| +// is not a request host under any canonicalization. |
| func signHop(key []byte, host string, now time.Time) string { |
| ts := strconv.FormatInt(now.Unix(), 10) |
| return ts + ":" + hex.EncodeToString(hopMAC(key, host, ts)) |
| @@ -220,11 +266,38 @@ func verifyHopEitherKey(current, next []byte, host, header string, now time.Time |
| } |
| // hopMAC is the MAC signHop and verifyHop both compute — one place so the |
| -// two can never drift out of sync on what bytes are actually signed. |
| +// two can never drift out of sync on what bytes are actually signed, and |
| +// the one choke point that canonicalizes host before it enters the MAC. |
| func hopMAC(key []byte, host, ts string) []byte { |
| mac := hmac.New(sha256.New, key) |
| - mac.Write([]byte(host)) |
| + mac.Write([]byte(canonicalizeHopHost(host))) |
| mac.Write([]byte("\n")) |
| mac.Write([]byte(ts)) |
| return mac.Sum(nil) |
| } |
| + |
| +// canonicalizeHopHost normalizes a request host into the exact form the MAC |
| +// is computed over, so a signer and a verifier can never disagree on what |
| +// "host" means for the same request: |
| +// |
| +// - a :port is stripped via net.SplitHostPort when present — Task 3 |
| +// verifies against a literal net/http request Host, which carries a |
| +// port when the client set one (or the request came in over a |
| +// non-default port), while a signer's own idea of the host (e.g. a |
| +// home-map lookup key) never does; |
| +// - lowercased — HTTP host headers are case-insensitive, but bytes |
| +// entering an HMAC are not, so a client or intermediary that |
| +// capitalizes a label must not be able to produce a request this MAC |
| +// treats as a different host; |
| +// - a single trailing root-label dot is stripped — "example.com." and |
| +// "example.com" name the same host, and a client is free to send |
| +// either. |
| +// |
| +// host is returned unchanged (net.SplitHostPort's error path) when it |
| +// carries no port — the common case, and not a failure. |
| +func canonicalizeHopHost(host string) string { |
| + if h, _, err := net.SplitHostPort(host); err == nil { |
| + host = h |
| + } |
| + return strings.TrimSuffix(strings.ToLower(host), ".") |
| +} |
diff --git a/internal/edge/mesh_test.go b/internal/edge/mesh_test.go| index ebadd551d..59be378da 100644 |
| --- a/internal/edge/mesh_test.go |
| +++ b/internal/edge/mesh_test.go |
| @@ -4,12 +4,26 @@ import ( |
| "bytes" |
| "context" |
| "encoding/hex" |
| + "log" |
| + "strings" |
| "testing" |
| "time" |
| "github.com/carlosframework/platform/internal/release" |
| ) |
| +// captureLog redirects the standard logger for the duration of a test — |
| +// synchronous callers only (mintMeshKey, loadMeshKeys direct calls here are |
| +// never run concurrently with the read), same shape certsync_test.go uses. |
| +func captureLog(t *testing.T) *bytes.Buffer { |
| + t.Helper() |
| + var buf bytes.Buffer |
| + prev := log.Writer() |
| + log.SetOutput(&buf) |
| + t.Cleanup(func() { log.SetOutput(prev) }) |
| + return &buf |
| +} |
| + |
| // meshTestStore is a fresh directory-backed store, the same DirStore every |
| // other edge test in this package uses in place of live S3. |
| func meshTestStore(t *testing.T) release.Store { |
| @@ -48,6 +62,28 @@ func TestMintMeshKeyExportedWrapperMintsTheSameKey(t *testing.T) { |
| } |
| } |
| +// A lost race is the steady state of every tick but the first, forever — it |
| +// must not log anything, matching housekeeper.go's own silence on election |
| +// contention. Only the first, real mint gets a line. |
| +func TestMintMeshKeySecondMintIsSilent(t *testing.T) { |
| + buf := captureLog(t) |
| + s := meshTestStore(t) |
| + if err := mintMeshKey(context.Background(), s); err != nil { |
| + t.Fatal(err) |
| + } |
| + first := buf.String() |
| + if !strings.Contains(first, "minted") { |
| + t.Fatalf("first mint log = %q, want it to mention minting", first) |
| + } |
| + buf.Reset() |
| + if err := mintMeshKey(context.Background(), s); err != nil { |
| + t.Fatalf("second mint must be success-by-other: %v", err) |
| + } |
| + if got := buf.String(); got != "" { |
| + t.Fatalf("second mint (a lost CAS race) logged %q, want silence", got) |
| + } |
| +} |
| + |
| func TestHopSignatureRoundTripsAndBoundsSkew(t *testing.T) { |
| key := []byte("0123456789abcdef0123456789abcdef") |
| now := time.Unix(1_766_000_000, 0) |
| @@ -99,6 +135,50 @@ func TestVerifyHopRefusesMalformedHeaders(t *testing.T) { |
| } |
| } |
| +// The MAC is over the CANONICAL host, so a signer's and a verifier's |
| +// differing-but-equivalent forms of the same host must both verify against |
| +// the same signature: Task 4 signs one form (e.g. a home-map lookup key, |
| +// unlikely to carry a port), Task 3 verifies a literal net/http request Host |
| +// (may carry a port, whatever case and trailing dot the client sent) — |
| +// divergence here would be spurious refusals presenting as regional |
| +// flakiness, not a security property either side intends. |
| +func TestHopMACCanonicalizesHost(t *testing.T) { |
| + key := []byte("0123456789abcdef0123456789abcdef") |
| + now := time.Unix(1_766_000_000, 0) |
| + canonical := signHop(key, "app.example.com", now) |
| + |
| + // A signature made for the canonical host verifies against every |
| + // equivalent form a verifier might see the request host in. |
| + for _, host := range []string{ |
| + "app.example.com", |
| + "APP.EXAMPLE.COM", |
| + "App.Example.Com", |
| + "app.example.com.", |
| + "app.example.com:443", |
| + "app.example.com.:8443", |
| + "APP.EXAMPLE.COM.:443", |
| + } { |
| + if !verifyHop(key, host, canonical, now) { |
| + t.Fatalf("verifyHop(%q) against a signature made for the canonical host = false, want true", host) |
| + } |
| + } |
| + |
| + // And signing under any of those same equivalent forms produces a |
| + // signature that verifies against the canonical form too — the |
| + // canonicalization is symmetric, not a one-sided allowance. |
| + for _, host := range []string{"APP.EXAMPLE.COM", "app.example.com.", "app.example.com:9443"} { |
| + h := signHop(key, host, now) |
| + if !verifyHop(key, "app.example.com", h, now) { |
| + t.Fatalf("a signature made for host %q must verify against the canonical form", host) |
| + } |
| + } |
| + |
| + // A genuinely different host must still refuse. |
| + if verifyHop(key, "other.example.com", canonical, now) { |
| + t.Fatal("a different host must still refuse — canonicalization must not collapse distinct hosts") |
| + } |
| +} |
| + |
| // ---- rotation dual-accept (plan's global constraint: the READ side ships |
| // now — verifyHopEitherKey and signingKey are what Task 3/4's request path |
| // calls once it has loaded both mirrored keys) -------------------------------- |
| @@ -189,3 +269,42 @@ func TestLoadMeshKeysReadsBothMirroredObjects(t *testing.T) { |
| t.Fatalf("key lengths = (%d, %d), want both %d", len(current), len(next), meshKeyBytes) |
| } |
| } |
| + |
| +// A malformed .next — hand-written during a rotation, so a bad upload is a |
| +// real possibility — must not take the live current key down with it: the |
| +// safe response is to treat it as no rotation in flight (loudly logged), |
| +// never to refuse every hop fleet-wide over one bad object. |
| +func TestLoadMeshKeysToleratesAnUnreadableNext(t *testing.T) { |
| + ctx := context.Background() |
| + s := meshTestStore(t) |
| + if err := mintMeshKey(ctx, s); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := s.Put(ctx, meshKeyNextKey, []byte("not-valid-hex-zzzz"), ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + buf := captureLog(t) |
| + current, next, err := loadMeshKeys(ctx, s) |
| + if err != nil { |
| + t.Fatalf("loadMeshKeys with a malformed .next must not fail: %v", err) |
| + } |
| + if len(current) != meshKeyBytes { |
| + t.Fatalf("current key length = %d, want %d — a malformed .next took the live key down with it", len(current), meshKeyBytes) |
| + } |
| + if next != nil { |
| + t.Fatal("a malformed .next must be treated as absent (no rotation in flight), not surfaced") |
| + } |
| + if got := buf.String(); !strings.Contains(got, meshKeyNextKey) { |
| + t.Fatalf("log = %q, want a loud mention of %s — this is an operator mistake worth paging on", got, meshKeyNextKey) |
| + } |
| + |
| + // And a signature made under the still-good current key verifies fine — |
| + // the tolerance is not merely "does not error", it's "hop auth keeps |
| + // working". |
| + now := time.Unix(1_766_000_000, 0) |
| + h := signHop(current, "app.example.com", now) |
| + if !verifyHopEitherKey(current, next, "app.example.com", h, now) { |
| + t.Fatal("hop auth must keep working off the current key despite the malformed .next") |
| + } |
| +} |
diff --git a/internal/host/agent.go b/internal/host/agent.go| index 34d64e6a0..4438131c2 100644 |
| --- a/internal/host/agent.go |
| +++ b/internal/host/agent.go |
| @@ -322,6 +322,13 @@ func RunAgent(args []string) { |
| go retentionLoop(reg) |
| go registrySnapshotLoop(reg) |
| go edgesFileLoop(reg) |
| + // The mesh key's own loop (multi-region backhaul spec, Part 3 Task 2) — |
| + // deliberately NOT a duty riding retentionLoop's tick: that loop returns |
| + // early, never arming at all, on a box missing CARLOS_RELEASE_PUBKEY, |
| + // and such a box still WINS the housekeeper election. A mint duty living |
| + // only inside retentionLoop would then silently never run fleet-wide. |
| + // See mintMeshKeyLoop's own comment (housekeeper.go). |
| + go mintMeshKeyLoop() |
| // The sweep reads the registry only through this closure — the stamped |
| // bespoke units to sample beside the carlos- ones, re-read each tick. |
| go metricsLoop(cgm.Root(), func() []string { return stampedUnitNames(reg) }) |
| @@ -1626,20 +1633,11 @@ func retentionLoop(reg *registry.Registry) { |
| prune := housekeeperDuty{"retention pass", func(ctx context.Context) error { |
| return retentionPass(ctx, store, pub, reg) |
| }} |
| - // The mesh key mint (multi-region backhaul spec, Part 3 Task 2) rides |
| - // the same tick and the same store: it is a second deployment-global |
| - // duty, so it belongs behind the same one election this loop already |
| - // gates prune on, not a second loop contending for a second lock. |
| - // mintMeshKey is create-only and idempotent — every tick after the |
| - // first finds the key already minted and does nothing, quietly. |
| - mintMesh := housekeeperDuty{"mesh key mint", func(ctx context.Context) error { |
| - return edge.MintMeshKey(ctx, store) |
| - }} |
| delay := time.Minute |
| for { |
| time.Sleep(delay) |
| delay = 5*time.Minute + rand.N(time.Minute) |
| - runHousekeeperDuties(housekeeperGate, prune, mintMesh) |
| + runHousekeeperDuties(housekeeperGate, prune) |
| } |
| } |
diff --git a/internal/host/housekeeper.go b/internal/host/housekeeper.go| index ab7386fa0..baf4442c4 100644 |
| --- a/internal/host/housekeeper.go |
| +++ b/internal/host/housekeeper.go |
| @@ -58,10 +58,12 @@ import ( |
| "context" |
| "errors" |
| "log" |
| + "math/rand/v2" |
| "os" |
| "sync/atomic" |
| "time" |
| + "github.com/carlosframework/platform/internal/edge" |
| "github.com/carlosframework/platform/internal/lease" |
| ) |
| @@ -422,3 +424,49 @@ func runHousekeeperDuties(gate func() (context.Context, bool), duties ...houseke |
| } |
| } |
| } |
| + |
| +// mintMeshKeyLoop mints the deployment's mesh key (multi-region backhaul |
| +// spec, Part 3 Task 2) — a housekeeper duty, but deliberately its OWN loop |
| +// rather than a duty folded into retentionLoop's runHousekeeperDuties call |
| +// (agent.go's retentionLoop, gated on prune's cadence). |
| +// |
| +// The plan first drafted the mint as riding retentionLoop's tick; review |
| +// caught the trap: retentionLoop returns early — never arming its loop at |
| +// all, so runHousekeeperDuties is never even called — on a box missing |
| +// CARLOS_RELEASE_PUBKEY (release-signature verification, something wholly |
| +// unrelated to the mesh key). Critically, a box in that state still WINS |
| +// the housekeeper election (newHousekeeper/hk.run don't consult |
| +// CARLOS_RELEASE_PUBKEY at all), so the one box holding the lease would |
| +// simply never mint, and no other box would ever try either — the mesh key |
| +// would silently never exist, and Tasks 3/4's hop auth would never arm, |
| +// fleet-wide, with no error anywhere. This loop is gated on nothing but |
| +// deployment-store presence (shouldSkipPublisherElection, openDeployStore), |
| +// mirroring edgesfile.go's edgesFileLoop shape exactly, so minting is |
| +// available on every housekeeper independent of every other loop's |
| +// preconditions. |
| +// |
| +// mintMeshKey itself is create-only and idempotent (mesh.go's own comment): |
| +// every tick after the deployment's first successful mint finds the key |
| +// already there and does nothing, silently. |
| +func mintMeshKeyLoop() { |
| + ctx := context.Background() |
| + if shouldSkipPublisherElection(os.Getenv) { |
| + log.Printf("agent: mesh key mint loop is a platform-box job — skipped on a fleet box") |
| + return |
| + } |
| + store, err := openDeployStore(ctx) |
| + if err != nil { |
| + log.Printf("agent: mesh key mint loop disabled: %v", err) |
| + return |
| + } |
| + log.Printf("agent: mesh key mint loop armed (5min + jitter) — mints only while this box holds the housekeeper lease") |
| + mint := housekeeperDuty{"mesh key mint", func(ctx context.Context) error { |
| + return edge.MintMeshKey(ctx, store) |
| + }} |
| + delay := time.Minute |
| + for { |
| + time.Sleep(delay) |
| + delay = 5*time.Minute + rand.N(time.Minute) |
| + runHousekeeperDuties(housekeeperGate, mint) |
| + } |
| +} |
diff --git a/internal/host/housekeeper_test.go b/internal/host/housekeeper_test.go| index 13ec86c3e..31d19e64f 100644 |
| --- a/internal/host/housekeeper_test.go |
| +++ b/internal/host/housekeeper_test.go |
| @@ -511,3 +511,52 @@ func TestHousekeeperHeldIsFalseBeforeAnyElectionIsArmed(t *testing.T) { |
| t.Fatal("housekeeperGate() reports held with no housekeeper armed") |
| } |
| } |
| + |
| +// ---- mintMeshKeyLoop (multi-region backhaul spec, Part 3 Task 2's review |
| +// fix): the mesh key's own arming loop, independent of retentionLoop ------- |
| + |
| +// Mirrors TestRetentionLoopIsAPlatformBoxJob's shape (agent_test.go): a |
| +// fleet box must skip before ever opening a store. |
| +func TestMintMeshKeyLoopIsAPlatformBoxJob(t *testing.T) { |
| + const skipLine = "agent: mesh key mint loop is a platform-box job — skipped on a fleet box" |
| + |
| + logs := captureLog(t) |
| + door := newDoorFake(t) |
| + for k, v := range fleetDoorEnv(door.url) { |
| + t.Setenv(k, v) |
| + } |
| + t.Setenv("CARLOS_DEPLOYMENT_BUCKET", "") |
| + t.Setenv("CARLOS_DEPLOYMENT_DIR", "") |
| + |
| + mintMeshKeyLoop() // must return; a fleet box never reaches the loop |
| + |
| + if got := logs(); !strings.Contains(got, skipLine) { |
| + t.Fatalf("log = %q, want the skip line verbatim", got) |
| + } |
| + if hits := door.seen(); len(hits) != 0 { |
| + t.Fatalf("door saw %+v, want zero — the skip must land BEFORE the store opens", hits) |
| + } |
| +} |
| + |
| +// The regression guard for the Critical review finding: the mint must NOT be |
| +// gated on CARLOS_RELEASE_PUBKEY (retentionLoop's own gate, unrelated to the |
| +// mesh key) — a box missing that var still wins the housekeeper election, so |
| +// a mint duty tied to retentionLoop's arming would silently never run |
| +// fleet-wide. Proven by explicitly leaving CARLOS_RELEASE_PUBKEY unset and |
| +// confirming the loop still arms. |
| +func TestMintMeshKeyLoopArmsWithoutReleasePubkey(t *testing.T) { |
| + t.Setenv("CARLOS_FLEET", "") |
| + t.Setenv("CARLOS_DEPLOYMENT_BUCKET", "") |
| + t.Setenv("CARLOS_DEPLOYMENT_DIR", t.TempDir()) |
| + t.Setenv("CARLOS_RELEASE_PUBKEY", "") // deliberately unset — the whole point |
| + |
| + buf := captureRacyLog(t) |
| + // mintMeshKeyLoop never returns once armed: run it in its own goroutine |
| + // (it parks in a time.Sleep for a minute, long past this test's end) and |
| + // wait for the arming log line against a synchronized sink. |
| + go mintMeshKeyLoop() |
| + |
| + waitFor(t, "the mint loop to arm despite no CARLOS_RELEASE_PUBKEY", func() bool { |
| + return strings.Contains(buf.String(), "agent: mesh key mint loop armed") |
| + }) |
| +} |