diff --git a/.kiro/specs/ha-term-safety/tasks.md b/.kiro/specs/ha-term-safety/tasks.md index 6dcf428f..9eb5ea5b 100644 --- a/.kiro/specs/ha-term-safety/tasks.md +++ b/.kiro/specs/ha-term-safety/tasks.md @@ -98,7 +98,7 @@ - _Requirements: 7.1, 7.3, 7.6, 7.7, 7.11_ - _Depends: 3.1_ -- [ ] 4.3 Deliver config bytes out-of-band, hash-verify before activate, fence on mismatch +- [x] 4.3 Deliver config bytes out-of-band, hash-verify before activate, fence on mismatch - Key the existing out-of-band config-push transport to the committed marker's `{version_tag, sha256}`; obtain bytes when not held locally, verify their hash against the committed marker before activating; keep a member non-serving under the new configuration (loud fence, keep retrying) on mismatch; on recovery activate the configuration referenced by the latest committed marker - Observable completion: a committed marker whose bytes match the hash activates; an injected hash mismatch keeps the member fenced and retrying (never activates mismatched config); config bytes never enter a WAL payload - _Requirements: 7.5, 7.8, 7.9, 7.10_ @@ -190,3 +190,9 @@ - **Round-1 REJECT lesson (design settles scope):** the live cutover (do_upgrade_config_with_bytes appends the marker + retire committed_config_tag as authoritative) is **4.2's** job per design §306-320 (Config marker component owns R7.1/7.3/7.6/7.7/7.11); task 4.3's component §336-364 is EXCLUSIVELY byte-delivery R7.5/7.8/7.9/7.10. Providing append/derive PRIMITIVES without wiring the live path = incomplete task (dead code). When scoping "is the live wiring this task or the next", read the design COMPONENT→requirement mapping, not just the task title. - **Compaction hazard FIXED (Finding 2):** `trim_wal_segments` can unlink a committed marker's segment. `persist_checkpoint` now calls `scan_latest_committed_config_ref` (committed-prefix, latest-wins) BEFORE write_snapshot+trim, stamps the 41-byte `{version_tag,sha256}` ref into an OPTIONAL snapshot-sentinel TAIL (backward-compat: load_snapshot needs only n>=16; tail gated on `n>=57 && header[16]!=0`, so OLD 16-byte sentinels still load — no durable-format regression). `config_committed_marker` falls back to the durable `persist.cfgref_*` ONLY on live-scan -ENOENT (newer live marker always wins; durable ref is the last-checkpoint FLOOR, monotonic, never a shadow). This is a 40-byte REFERENCE in the snapshot, NOT the out-of-scope full `config-upgrade-snapshot`. - **DEFERRED to 4.3:** `config_committed_marker` has no production CONSUMER yet (only tests) — activation/serve-fence/hash-verify-before-activate + the status-from-derived-value consumer is 4.3 (R7.5/7.8/7.9/7.10). 4.2 only requires the derive to ADVANCE on commit (proven). cb5 fixture in test_commit_barrier_rollout.c made persist-backed so the live marker append succeeds (assertions unchanged). + +- **4.3 done (REJECTED ×2, APPROVED round 3). PHASE 4 COMPLETE.** Byte delivery + hash-verify-before-activate + serve-fence. NEW `config_activate_committed(c)` = the production consumer: reads `config_committed_marker`, hashes the locally-held out-of-band bytes (`local_config_sha256`), VERIFIES sha256 == committed marker BEFORE `load_primary_config`, else sets `ha.config_serve_fence` (loud, `-EAGAIN`, stay-alive). `config_serve_fenced` accessor. ENFORCED by `ha_dispatch_gate` (CTRL_PRIMARY denies config-dependent state-changing ops e.g. jobsub while fenced; admits replConfigPush/read/status — no un-fence deadlock) + `ha_config_promotion_fenced` (refuse promotion in raft_cb_become_leader, stay follower). Wired at: (A) `handle_repl_config_push` (save-then-activate, SOLE activation point — no pre-activate), (B) `myapp.c` boot step 7 (recovery activates latest committed marker; -ENOENT→load_primary_config fallback, NO false fence), (D) `ha_cb_commit_advanced_stub` schedules an OFF-IOMGR delaymgr retry. bug-981 logged. Reviewer APPROVED round 3. + - **bug-981 (CRITICAL liveness, caught round 2):** push-before-commit on a 2nd+ upgrade — a standby that saves NEW bytes before its committed prefix advances past the NEW marker fences (config_committed_marker still returns the OLDER marker → hash mismatch), and NOTHING re-activated post-commit (on_commit_advanced is async-I/O-bound so can't do disk reads; primary's config-behind re-push defeated because conf_save_config already advanced the advertised tag) → PERMANENT fence until reboot, violating R7.9 "keep retrying". FIX: `ha_cb_commit_advanced_stub` (iomgr ACK thread) does ONLY an atomic `config_serve_fenced` read + non-blocking `config_schedule_activate_retry` (delay_task_ms→ordq_put, NO disk I/O); the disk-reading `config_activate_retry_fn` runs on the delaymgr worker (mirrors ha_convergence.c delay_task pattern); clears the fence once the NEW marker commits + bytes verify; no-op once unfenced (no busy-spin, single worker serializes, atomic fence). + - **bug (verify-bypass, caught round 1 Finding A):** handle_repl_config_push originally called load_primary_config (activating pushed bytes) BEFORE the hash gate → mismatched config went live. Fixed: gate is the SOLE activation point. + - **PATTERN (Do-Not-Repeat):** for a marker-keyed out-of-band-bytes consumer, (1) verify-before-activate must be the SOLE activation path (no earlier load); (2) the consumer must be wired into ALL production reach paths — byte-arrival AND boot-recovery AND commit-advance — not just one (dead-code-in-production = incomplete task, same trap as 4.2); (3) a fence flag must have a real production READER or it's not a fence; (4) marker (WAL) and bytes (out-of-band) commit on INDEPENDENT transports → always handle push-before-commit with a commit-advance retry. async-I/O: schedule disk work off the iomgr ACK thread via delaymgr. + - **CONSOLIDATED 5.5/cleanup list:** (a) `ha_cb_*_stub` rename + stale stub-header comment ha.c ~1326; (b) ha_convergence.c ~292-297 stale cluster_apply_ctrlhello_reply doc-comment; (c) ha.h rejoin_reconciled dead field; (d) committed_config_tag now cache-only (audit remaining readers). All cosmetic. diff --git a/src/controller/config_repl.c b/src/controller/config_repl.c index f6054e61..974d32eb 100644 --- a/src/controller/config_repl.c +++ b/src/controller/config_repl.c @@ -244,6 +244,211 @@ int config_committed_marker(const cluster_t *c, uint64_t *version_tag, return rc; /* genuinely no committed config marker anywhere */ } +/* ── Config byte delivery: hash-verify-before-activate + serve-fence (task 4.3) ─ + * + * config_committed_marker (4.2) is the AUTHORITY for which configuration is in + * effect; the configuration BYTES travel out-of-band on the existing config-push + * transport (OPCode_replConfigPush / handle_repl_config_push), never inside a WAL + * payload (R7.4/R7.5). config_activate_committed keys that byte delivery to the + * committed marker's {version_tag, sha256}: it VERIFIES the locally-held bytes' + * sha256 against the committed marker BEFORE activating (R7.8), and on a mismatch + * (or bytes absent locally) keeps the member from serving under the new config and + * KEEPS RETRYING rather than activating mismatched config (R7.9) — the loud + * non-serving fence that NEVER self-terminates. On recovery it activates the config + * referenced by the LATEST committed marker (R7.10). + */ + +int config_serve_fenced(const cluster_t *c) +{ + if (!c) + return 0; + return atomic_load_explicit(&c->ha.config_serve_fence, + memory_order_acquire) ? 1 : 0; +} + +/* Read the locally-held config bytes (resolved via myconf->conffile or + * conf/current) and compute their sha256 into out_sha. Returns 0 with the hash on + * success; -ENOENT when no local config bytes exist yet (the out-of-band push has + * not delivered them). The bytes are the out-of-band-delivered config, NEVER a WAL + * payload (R7.4/R7.5). */ +static int local_config_sha256(cluster_t *c, uint8_t out_sha[32]) +{ + myconf_t *mc = c->myconf; + if (!mc || !mc->workdir || !mc->name) + return -EINVAL; + + /* Resolve the current locally-held versioned config file. */ + char conf_dir[4096]; + snprintf(conf_dir, sizeof(conf_dir), "%s/%s", mc->workdir, mc->name); + + char fname[256] = {0}; + int rc = conf_read_current_filename(conf_dir, fname, sizeof(fname)); + if (rc != 0) + return -ENOENT; /* no local bytes yet — caller fences + retries */ + + char path[4352]; + snprintf(path, sizeof(path), "%s/conf/%s", conf_dir, fname); + + uint8_t *bytes = NULL; + size_t blen = 0; + rc = conf_read_raw(path, &bytes, &blen); + if (rc != 0) + return -ENOENT; + + SHA256(bytes, blen, out_sha); + free(bytes); + return 0; +} + +int config_activate_committed(cluster_t *c) +{ + if (!c) + return -EINVAL; + + /* 1. The committed marker is the authority for the target config (R7.7/R7.10: + * on recovery this is the LATEST committed marker). */ + uint64_t marker_tag = 0; + uint8_t marker_sha[32]; + int rc = config_committed_marker(c, &marker_tag, marker_sha); + if (rc == -ENOENT) { + /* No committed config marker exists (legacy / first boot, or the marker + * has not committed yet). Absent-marker is NOT a hash MISMATCH, so it must + * NOT fence the node (a benign "awaiting bytes/marker" state, never a hard + * divergence fence — R7.9/R7.10). FALL BACK to the existing + * load_primary_config behavior so any locally-held bytes are activated + * (config_version_tag is set for the freshness gate and the push ACK). + * A missing local config file (-ENOENT from load_primary_config) is also + * benign first-boot — the node fetches config post-election. Clear any + * stale fence either way: an absent marker is never a divergence. */ + atomic_store_explicit(&c->ha.config_serve_fence, 0, memory_order_release); + int lrc = load_primary_config(c); + if (lrc != 0 && lrc != -ENOENT) + WARN("config_activate_committed: no committed marker; " + "load_primary_config fallback rc=%d", lrc); + return 0; + } + if (rc != 0) + return rc; /* -EINVAL / -EIO surfaced to the caller */ + + /* 2. Hash-verify the LOCALLY-HELD (out-of-band-delivered) bytes against the + * committed marker BEFORE activating (R7.8). */ + uint8_t local_sha[32]; + rc = local_config_sha256(c, local_sha); + bool verified = (rc == 0 && memcmp(local_sha, marker_sha, 32) == 0); + + if (!verified) { + /* 3. MISMATCH or bytes not yet present locally: the bytes for this + * committed marker have not arrived/verified out-of-band. Keep the + * member NON-SERVING under the new config and KEEP RETRYING — the loud + * non-self-terminating fence (R7.9). NEVER activate mismatched bytes. + * Stays ALIVE: returns -EAGAIN so the caller retries when the matching + * bytes arrive via the existing config-push transport. */ + int was = atomic_exchange_explicit(&c->ha.config_serve_fence, 1, + memory_order_acq_rel); + if (!was) + ERROR("config serve-fence: committed config marker tag=%016" PRIx64 + " references bytes whose sha256 is not present/matching locally " + "(rc=%d) -- member NON-SERVING under the new config, awaiting " + "out-of-band config-push bytes; staying alive and retrying", + marker_tag, rc); + return -EAGAIN; + } + + /* 4. VERIFIED: the locally-held bytes hash-match the committed marker — + * activate the configuration (load it into myconf), then clear the fence. */ + rc = load_primary_config(c); + if (rc != 0) { + /* The verified bytes failed to load (e.g. decode error). Stay fenced and + * retrying rather than serving an unloadable config — never self-terminate. */ + int was = atomic_exchange_explicit(&c->ha.config_serve_fence, 1, + memory_order_acq_rel); + if (!was) + ERROR("config serve-fence: hash-verified config bytes for committed " + "marker tag=%016" PRIx64 " failed to load (rc=%d) -- member " + "NON-SERVING, staying alive and retrying", marker_tag, rc); + return rc; + } + + atomic_store_explicit(&c->ha.config_serve_fence, 0, memory_order_release); + INFO("config activated from committed marker tag=%016" PRIx64 + " (hash-verified out-of-band bytes)", marker_tag); + return 0; +} + +/* ── Commit-advance retry trigger (task 4.3 remediation r2, Finding D / R7.9) ── + * + * The config MARKER (WAL stream) and the config BYTES (out-of-band config-push) + * arrive on INDEPENDENT transports. On a 2nd-or-later upgrade the bytes can be + * saved locally BEFORE the standby's committed prefix advances past the NEW + * marker: at that moment config_committed_marker still returns the OLDER marker, + * so the freshly-saved NEW bytes hash-MISMATCH it and config_activate_committed + * sets the serve-fence (-EAGAIN). The NEW marker commits later via the WAL path, + * but on_commit_advanced runs on the iomgr ACK thread and MUST NOT do disk I/O, + * so it cannot re-activate inline; and the primary's "config-behind" re-push net + * does NOT fire because conf_save_config already advanced this member's advertised + * config_version_tag. Without a retry the standby stays PERMANENTLY fenced (and + * non-promotable) for that upgrade until reboot — a §7.9 liveness violation. + * + * The fix: when commit advances WHILE this member is serve-fenced, schedule a + * one-shot OFF-IOMGR delaymgr task that re-runs config_activate_committed. The + * delaymgr worker runs on a thread allowed to block on the disk reads + * (scan_latest_committed_marker pread + load_primary_config). Once the NEW marker + * is in the committed prefix AND the locally-held bytes hash-match it, the retry + * activates and CLEARS the fence; a genuine still-MISMATCH (or no-bytes-yet) keeps + * the fence and the next commit-advance reschedules. -ENOENT still falls back + * without fencing (the no-marker path is benign). Idempotent + self-limiting: the + * scheduler is a no-op once unfenced, so there is no busy-spin. */ + +/* config_activate_retry_fn: the delaymgr task body (runs OFF the iomgr ACK + * thread). Re-runs config_activate_committed so a freshly-committed config marker + * is activated — and a stale serve-fence cleared — once its out-of-band bytes + * verify. `arg` is the cluster_t* (passed directly; no heap ctx to free, so the + * delaymgr node-killer that frees only the delay_task_t never leaks). */ +int config_activate_retry_fn(void *arg) +{ + cluster_t *c = (cluster_t *)arg; + if (!c) + return -EINVAL; + if (!config_serve_fenced(c)) + return 0; /* already cleared by another path — nothing to retry */ + + int rc = config_activate_committed(c); + if (rc == 0) + INFO("config commit-advance retry: serve-fence CLEARED — newly committed " + "config marker activated (hash-verified out-of-band bytes)"); + else if (rc == -EAGAIN) + TRACE("config commit-advance retry: still fenced (marker/bytes not yet " + "matching) — will retry on the next commit advance"); + else + WARN("config commit-advance retry: config_activate_committed rc=%d", rc); + return rc; +} + +/* config_schedule_activate_retry: enqueue config_activate_retry_fn on the + * delaymgr (NON-BLOCKING — delay_task_ms only does ordq_put). Called from the + * commit-advance path (iomgr ACK thread) so activation's disk I/O runs OFF that + * thread (ha-async-io invariant). No-op when the member is NOT serve-fenced (so + * the steady state schedules nothing) or when the delaymgr is not running (the + * unit-test fixture path, which never inits delaymgr — avoids leaking a task into + * an un-drained queue). Returns 0 on enqueue/no-op, negative on enqueue failure. */ +int config_schedule_activate_retry(cluster_t *c) +{ + if (!c) + return -EINVAL; + if (!config_serve_fenced(c)) + return 0; /* steady state: nothing to retry, schedule nothing */ + if (!c->delaymgr.running) + return 0; /* delaymgr not running (test fixture) — caller drives directly */ + + /* One-shot, minimal delay: run promptly but off the iomgr ACK thread. */ + int rc = delay_task_ms(&c->delaymgr, 1, 1, + (delay_task_entry_fn_t)config_activate_retry_fn, c); + if (rc < 0) + WARN("config commit-advance retry: delay_task_ms failed: %d — fence will " + "clear on a later commit advance or config-push", rc); + return rc; +} + /* config_commit_change (ha-term-safety task 4.2 — LIVE wiring): order a config * change through the unified (term,lsn) log as the CONSENSUS RECORD and make the * derived committed marker authoritative, retiring committed_config_tag to a @@ -991,7 +1196,14 @@ int handle_repl_config_push(cluster_t *c, endpoint_t *ep, xce_request_t *req) return -EBADMSG; } - /* Save to disk and reload */ + /* ha-term-safety task 4.3 (remediation r1, Finding A): SAVE the pushed bytes + * to disk out-of-band, but do NOT activate them here. config_activate_committed + * is the SOLE activation point — it loads the bytes into the live config ONLY + * after hash-verifying them against the latest committed WAL_CONFIG_MARKER + * (R7.5/R7.8). A pre-emptive load_primary_config here would make the just-pushed + * bytes LIVE before the gate runs, turning verify-before-activate into an + * after-the-fact overlay. The bytes travel out-of-band via this transport and + * NEVER inside a WAL payload (R7.4/R7.5). */ int rc = conf_save_config(c->myconf, rcpr->raw_bytes, rcpr->raw_bytes_len, rcpr->raw_sha256); if (rc != 0) { @@ -999,14 +1211,19 @@ int handle_repl_config_push(cluster_t *c, endpoint_t *ep, xce_request_t *req) return rc; } - rc = load_primary_config(c); - if (rc != 0) { - WARN("handle_repl_config_push: load_primary_config failed: %d", rc); - return rc; - } + INFO("handle_repl_config_push: config bytes saved out-of-band " + "(pushed tag=%016" PRIx64 ") — activation gated on committed marker", + rcpr->version_tag); - INFO("handle_repl_config_push: config saved and loaded (tag=%016" PRIx64 ")", - c->myconf->config_version_tag); + /* Key activation to the committed marker. On mismatch (the marker references + * different/later bytes than the ones just pushed) config_activate_committed + * sets the loud non-serving serve-fence and returns -EAGAIN; the member STAYS + * ALIVE and a later config-push / commit-advance retries (R7.9). -ENOENT + * (no committed marker yet — first boot/bootstrap) falls back to loading the + * locally-held bytes; a successful verify activates. */ + int act_rc = config_activate_committed(c); + if (act_rc != 0 && act_rc != -EAGAIN) + WARN("handle_repl_config_push: config_activate_committed rc=%d", act_rc); /* ACK the primary */ repl_config_push_reply_t reply_body = { diff --git a/src/controller/config_repl.h b/src/controller/config_repl.h index 01e2f4cb..d4c2f8e0 100644 --- a/src/controller/config_repl.h +++ b/src/controller/config_repl.h @@ -119,6 +119,81 @@ int config_append_marker(cluster_t *c, uint64_t version_tag, int config_committed_marker(const cluster_t *c, uint64_t *version_tag, uint8_t sha256_out[32]); +/* ── Config byte delivery: hash-verify-before-activate + serve-fence (task 4.3) + * + * The committed WAL_CONFIG_MARKER (config_committed_marker) is the AUTHORITY for + * which configuration is in effect; the configuration BYTES travel out-of-band on + * the existing config-push transport (OPCode_replConfigPush / handle_repl_config_push), + * never inside a WAL payload (R7.4/R7.5). config_activate_committed is the consumer + * that keys that byte delivery to the marker's {version_tag, sha256}: it obtains the + * bytes (already saved locally by the config-push handler when present), VERIFIES + * their sha256 against the committed marker BEFORE activating (R7.8), and on a + * mismatch (or bytes not yet present locally) keeps the member from serving under + * the new configuration and KEEPS RETRYING rather than activating mismatched config + * (R7.9) — the "controller never self-terminates" loud non-serving fence. On + * recovery it activates the configuration referenced by the LATEST committed marker + * (R7.10). + */ + +/* + * config_activate_committed: ensure the locally-held config bytes match the latest + * committed WAL_CONFIG_MARKER's content hash, then activate (load) that config. + * + * Behaviour: + * - No committed marker (config_committed_marker == -ENOENT): nothing to + * activate; clears any serve-fence and returns 0 (the legacy bootstrap config + * stays in effect). + * - Locally-held bytes' sha256 == marker sha256: HASH-VERIFIED → activate via + * load_primary_config, clear the serve-fence, return 0 (R7.8/R7.10). + * - Locally-held bytes absent or sha256 != marker sha256: do NOT activate; + * set the loud serve-fence (member stays non-serving under the new config, + * stays ALIVE) and return -EAGAIN so the caller retries when the matching + * bytes arrive out-of-band (R7.9). NEVER activates mismatched bytes; NEVER + * self-terminates. + * + * c — cluster (uses c->myconf for the locally-held bytes, c->ha for the fence) + * + * Returns 0 on activate (or no-op when no marker); -EAGAIN while fenced + * (bytes missing/mismatched); -EINVAL on a NULL/misconfigured cluster; the + * load_primary_config rc if the verified bytes fail to load. + */ +int config_activate_committed(cluster_t *c); + +/* + * config_serve_fenced: 1 when the member is serve-fenced under config_activate_- + * committed (locally-held config bytes do not yet hash-match the latest committed + * marker), 0 otherwise. The fence is non-serving + non-self-terminating (R7.9). + */ +int config_serve_fenced(const cluster_t *c); + +/* + * config_schedule_activate_retry: when commit advances WHILE this member is + * config serve-fenced, schedule a one-shot OFF-IOMGR delaymgr task that re-runs + * config_activate_committed (task 4.3 r2, Finding D / R7.9). Closes the + * push-before-commit liveness gap: the config MARKER (WAL) and config BYTES + * (out-of-band push) arrive on independent transports, so a standby that saves + * the NEW bytes before the NEW marker commits fences against the OLDER committed + * marker; nothing else re-runs activation post-commit (on_commit_advanced must + * not do disk I/O on the iomgr ACK thread; the primary's config-behind re-push + * net does not fire once conf_save_config advanced the advertised tag). This + * scheduler is NON-BLOCKING (ordq_put only) — safe to call from the commit-advance + * path; the delaymgr worker does the disk reads. No-op when not serve-fenced or + * when the delaymgr is not running (unit-test fixture). Returns 0 on enqueue/no-op, + * negative on enqueue failure. + */ +int config_schedule_activate_retry(cluster_t *c); + +/* + * config_activate_retry_fn: the delaymgr task body scheduled by + * config_schedule_activate_retry (runs OFF the iomgr ACK thread, may block on + * disk). Re-runs config_activate_committed; clears the serve-fence once the + * newly-committed marker's bytes verify. Exposed so tests can drive the same + * off-thread activation entry the production scheduler enqueues. `arg` is the + * cluster_t*. Returns the config_activate_committed rc (0 on activate/clear, + * -EAGAIN while still fenced). + */ +int config_activate_retry_fn(void *arg); + /* * config_commit_change: order + commit a config change through the unified * (term,lsn) log (ha-term-safety task 4.2 LIVE wiring). Appends a diff --git a/src/controller/ha.c b/src/controller/ha.c index 064ca13b..cd359b33 100644 --- a/src/controller/ha.c +++ b/src/controller/ha.c @@ -87,6 +87,17 @@ extern int push_config_transport_only(cluster_t *c, const uint8_t *bytes, uint64_t new_tag, int *n_out); #endif +/* config_repl.c is linked in BOTH production and XCE_TESTING builds. + * ha-term-safety task 4.3 (remediation r1): the commit-advance hook activates a + * newly-committed config marker (Finding B); ha_dispatch_gate / the pre-promotion + * gate read the serve-fence (Finding C). */ +extern int config_activate_committed(cluster_t *c); +extern int config_serve_fenced(const cluster_t *c); +/* task 4.3 (remediation r2, Finding D): the commit-advance path schedules an + * off-iomgr retry that re-activates a newly-committed config marker and clears a + * stale serve-fence (non-blocking ordq_put only). */ +extern int config_schedule_activate_retry(cluster_t *c); + static void on_vote_connected(endpoint_t *ep); static void on_pre_vote_connected(endpoint_t *ep); static int vote_ep_err_handler(void *ep_raw); @@ -1028,6 +1039,23 @@ static void raft_cb_become_leader(void *ctx, uint64_t term) } } + /* ── ha-term-safety task 4.3 (remediation r1, Finding C / R7.9): config + * serve-fence promotion gate ────────────────────────────────────────────── + * A member whose latest committed config marker references bytes it does not + * hold (config_serve_fenced) MUST NOT become authoritative CTRL_PRIMARY — it + * cannot serve under the committed configuration. Refuse promotion and STAY + * ALIVE (controller-never-self-terminate): the member remains non-promotable + * and keeps retrying; once the correct bytes arrive out-of-band and re-verify + * clears the fence, a later promotion attempt passes this gate. The check is a + * pure atomic read (no I/O) — safe on the election path. */ + if (config_serve_fenced(c)) { + WARN("HA: promotion REFUSED — config serve-FENCED (committed config " + "marker references bytes not present/matching locally); member " + "stays ALIVE, non-promotable, awaiting out-of-band config bytes " + "(ha-term-safety R7.9)"); + return; /* do NOT become CTRL_PRIMARY; never self-exit */ + } + int is_first_boot = !ha->phase2_done; /* First boot: call cluster_init_phase2 */ @@ -1636,6 +1664,37 @@ static void ha_cb_commit_advanced_stub(void *ctx, uint64_t commit_index) if (!ha || !ha->raft_c) return; /* test mode without a cluster: no barrier to trigger */ commit_set_raft_commit_index(&ha->raft_c->commit, commit_index); + + /* ha-term-safety task 4.3 (remediation r1, Finding B): a config marker may + * have just entered the committed prefix. Activation is NOT performed HERE: + * this callback runs on the iomgr ACK path, where the async-I/O invariant + * forbids the disk reads config_activate_committed performs + * (scan_latest_committed_marker pread + load_primary_config). The committed + * marker is instead consumed off the iomgr thread by the call sites that make + * §7.10 true in production: + * - boot/recovery (myapp.c step 7): activates the latest committed marker, + * hash-verified, after persist recovery. + * - handle_repl_config_push: after the out-of-band bytes are saved, gates + * activation on the committed marker (verify-before-activate). + * + * ha-term-safety task 4.3 (remediation r2, Finding D / R7.9): the two sites + * above are NOT sufficient when the BYTES arrive (and are saved) BEFORE the + * NEW marker commits — the config MARKER (WAL) and the config BYTES (out-of- + * band push) travel on INDEPENDENT transports. In that ordering the push path + * hash-checks the freshly-saved NEW bytes against the still-OLDER committed + * marker, MISMATCHES, and sets the serve-fence; when the NEW marker later + * commits HERE, nothing re-runs activation (this callback can't do disk I/O, + * and the primary's config-behind re-push net does not fire because the push + * already advanced this member's advertised config_version_tag) — so the + * standby would stay PERMANENTLY fenced. Close the gap: while serve-fenced, + * schedule an OFF-IOMGR delaymgr task (config_schedule_activate_retry, NON- + * BLOCKING ordq_put only) that re-runs config_activate_committed on a thread + * allowed to block on disk; once the NEW marker is in the committed prefix and + * the saved bytes verify, the retry clears the fence and activates. No-op once + * unfenced (no busy-spin); a still-genuine mismatch reschedules on the next + * commit advance. */ + if (config_serve_fenced(ha->raft_c)) + config_schedule_activate_retry(ha->raft_c); } static const raft_callbacks_t s_real_cbs = { @@ -2630,6 +2689,41 @@ int ha_dispatch_gate(ha_state_t *ha, int opcode) switch (role) { case CTRL_PRIMARY: + /* ha-term-safety task 4.3 (remediation r1, Finding C / R7.9): the config + * serve-fence is the PRODUCTION READER that makes the fence real. When + * set, the latest committed WAL_CONFIG_MARKER references config bytes that + * do not hash-match the locally-held bytes (or the bytes are absent), so + * this member MUST NOT serve under the new configuration. Refuse + * config-dependent state-changing ops (loud non-serving), but keep + * replication / read-only / status ops flowing so the correct bytes can + * arrive out-of-band and re-verify clears the fence. The member STAYS + * ALIVE (returns an error code, never exits — controller-never-self- + * terminate). Checked before the quorum gate so a fenced primary never + * serves divergent config even with quorum. */ + if (config_serve_fenced(ha->raft_c)) { + switch (opcode) { + /* Replication ops — let the correct config bytes arrive + re-verify */ + case OPCode_heartbeat: + case OPCode_replConfig: + case OPCode_replConfigPush: + case OPCode_upgradeConfigAck: + case OPCode_hello: + case OPCode_walAck: + case OPCode_replEntry: + case OPCode_replAck: + case OPCode_replCatchUpDone: + case OPCode_walCatchupEntry: + case OPCode_snapshotTransfer: + /* Read-only queries / status — answerable while fenced */ + case OPCode_get: + case OPCode_probeConfigTag: + case OPCode_ctrlstatus: + break; /* fall through to the quorum check below */ + default: + /* Config-dependent state-changing op while fenced: refuse. */ + return -XCE_ERR_NOT_READY; + } + } /* State-changing operations require majority so committed entries * are durable across failover. Read-only queries and replication * ops are always allowed. */ @@ -2755,6 +2849,17 @@ int ha_dispatch_gate(ha_state_t *ha, int opcode) } } +/* ha_config_promotion_fenced (ha-term-safety task 4.3, R7.9): a member is + * non-promotable while config serve-fenced — its latest committed config marker + * references bytes it does not hold/match, so it must not become authoritative. + * Pure atomic read via config_serve_fenced (no I/O) — safe on the election path. */ +int ha_config_promotion_fenced(const ha_state_t *ha) +{ + if (!ha) + return 0; + return config_serve_fenced(ha->raft_c) ? 1 : 0; +} + /* ── Redirect helpers used by join.c ───────────────────────── */ /* diff --git a/src/controller/ha.h b/src/controller/ha.h index 0d238fac..17531b64 100644 --- a/src/controller/ha.h +++ b/src/controller/ha.h @@ -237,6 +237,23 @@ typedef struct { * caught-up or stopped member never * FATALs. NULL at init / when no fence * timer is pending. */ + /* ── New field added by ha-term-safety spec (Task 4.3) ──────── */ + _Atomic int config_serve_fence; /* Config byte-delivery serve-fence + * (R7.9): set when the latest committed + * WAL_CONFIG_MARKER references config + * bytes whose sha256 does NOT match the + * locally-held bytes (or the bytes are + * absent locally). While set, the member + * MUST NOT serve under the new + * configuration; the bytes travel + * out-of-band via the existing config-push + * transport, so the member STAYS ALIVE and + * config_activate_committed KEEPS RETRYING + * until the matching bytes arrive and + * hash-verify, at which point the fence is + * cleared and the config is activated. + * NEVER self-terminates (loud non-serving + * fence only). 0 at init (ha_init memset). */ } ha_state_t; /* ── Public API ────────────────────────────────────────────── */ @@ -528,6 +545,17 @@ int ha_is_primary(const ha_state_t *ha); */ int ha_dispatch_gate(ha_state_t *ha, int opcode); +/* + * ha_config_promotion_fenced: 1 when this member is config serve-fenced and + * therefore NON-PROMOTABLE — its latest committed WAL_CONFIG_MARKER references + * config bytes it does not hold/match, so it must not become authoritative + * CTRL_PRIMARY (ha-term-safety task 4.3, R7.9). 0 otherwise. The pre-promotion + * gate in raft_cb_become_leader consults this; a fenced member stays ALIVE and + * non-promotable until the correct bytes arrive out-of-band and re-verify clears + * the fence (controller-never-self-terminate). Pure atomic read (no I/O). + */ +int ha_config_promotion_fenced(const ha_state_t *ha); + /* * ha_apply_wal_catchup_entry: apply a single WAL catch-up entry to local * persistent storage and advance ha->applied_lsn on success. diff --git a/src/controller/myapp.c b/src/controller/myapp.c index a62323d3..25001e6a 100644 --- a/src/controller/myapp.c +++ b/src/controller/myapp.c @@ -25,6 +25,9 @@ int validate_startup_config(const char *config_path, /* forward declaration — implemented in config_repl.c */ int load_primary_config(cluster_t *c); +/* ha-term-safety task 4.3: hash-verified recovery-activate from the latest + * committed WAL_CONFIG_MARKER (-ENOENT falls back to load_primary_config). */ +int config_activate_committed(cluster_t *c); time_t last_print_time = 0; unsigned long npkts = 0; @@ -193,12 +196,30 @@ int myapp_run(myconf_t *myconf) * will receive the config via the primary's replication push post-election * (Req 7.3). */ { - int pre_rc = load_primary_config(&c); + /* ha-term-safety task 4.3 (remediation r1, Finding B / R7.10): activate + * the config referenced by the LATEST COMMITTED WAL_CONFIG_MARKER, + * hash-VERIFIED against the locally-held bytes, instead of a blind + * load_primary_config. persist recovery (step 6 above) has already + * re-derived committed_lsn, so config_committed_marker reads the committed + * prefix here. Semantics: + * - committed marker present + bytes hash-match → activate (verified). + * - committed marker present + bytes mismatch/absent → loud serve-fence, + * member STAYS ALIVE and retries when correct bytes arrive + * out-of-band post-election (NEVER self-terminates). + * - NO committed marker (legacy / first boot) → -ENOENT falls back to + * the existing load_primary_config blind-load (no false fence). */ + int pre_rc = config_activate_committed(&c); if (pre_rc == 0) { - INFO("Pre-election config loaded: tag=%016llx", + INFO("Pre-election config activated: tag=%016llx", (unsigned long long)c.myconf->config_version_tag); + } else if (pre_rc == -EAGAIN) { + ERROR("Pre-election config serve-FENCED: committed config marker " + "references bytes not yet present/matching locally — member " + "NON-SERVING under the new config, staying alive and awaiting " + "out-of-band config-push (R7.9)"); } - /* Ignore errors — node may have no config yet (first boot or standby) */ + /* Ignore other errors — node may have no config yet (first boot/standby); + * it fetches config via the primary's push post-election (Req 7.3). */ } /* ── Step 8: ha_start_election_async ─────────────────────────────────── */ diff --git a/src/controller/testsuite/test_config_repl.c b/src/controller/testsuite/test_config_repl.c index f07d1a72..5a69522e 100644 --- a/src/controller/testsuite/test_config_repl.c +++ b/src/controller/testsuite/test_config_repl.c @@ -952,6 +952,603 @@ static void test_committed_marker_survives_checkpoint_trim(void) printf("test_committed_marker_survives_checkpoint_trim: PASS\n"); } +/* ── Config byte delivery: hash-verify-before-activate + serve-fence (task 4.3) ─ + * + * config_activate_committed is the CONSUMER of config_committed_marker (4.2's + * derive). It keys the out-of-band config-push byte delivery to the committed + * marker's {version_tag, sha256}: it VERIFIES the locally-held bytes' sha256 + * against the committed marker BEFORE activating (R7.8), and on a mismatch (or + * bytes absent locally) keeps the member from serving under the new configuration + * and KEEPS RETRYING rather than activating mismatched config (R7.9) — the loud + * non-serving fence that NEVER self-terminates. On recovery it activates the + * configuration referenced by the LATEST committed marker (R7.10). + * + * These tests reuse the 4.2 leader fixture (t42_*) but additionally wire c->myconf + * so the locally-held config bytes path (conf_save_config / conf_read_raw / + * load_primary_config) is exercised. */ + +/* Build a VALID Cap'n Proto ClusterConfig blob (decodable by load_primary_config) + * so the match path can actually ACTIVATE. Mirrors cb5_make_config_blob. */ +static uint8_t *t43_make_config_blob(size_t *len_out) +{ + cluster_config_t cfg; + struct capn cap; + struct capn_segment *cs; + ClusterConfig_ptr p; + uint8_t *buf; + size_t sz; + + memset(&cfg, 0, sizeof(cfg)); + cfg.clustername = (char *)"t43-upgrade"; + cfg.port = 9991; + cfg.config_generation = 1; + + capn_init_malloc(&cap); + cs = capn_root(&cap).seg; + encode_ClusterConfig_ptr(cs, &p, &cfg); + capn_setp(capn_root(&cap), 0, p.p); + sz = (size_t)capn_size(&cap); + buf = (uint8_t *)malloc(sz); + TEST_ASSERT_NOT_NULL(buf); + capn_write_mem(&cap, buf, sz, 0); + capn_free(&cap); + *len_out = sz; + return buf; +} + +/* Wire a myconf_t into the cluster so conf_save_config / load_primary_config + * resolve under //conf (the production layout). */ +static myconf_t *t43_attach_myconf(cluster_t *c, const char *tmpdir) +{ + myconf_t *mc = calloc(1, sizeof(myconf_t)); + TEST_ASSERT_NOT_NULL(mc); + mc->workdir = strdup(tmpdir); + mc->name = strdup("ctrl-test"); + mc->conf_keep_n = 2; + mc->conffile = NULL; + c->myconf = mc; + return mc; +} + +static void t43_free_myconf(cluster_t *c) +{ + myconf_t *mc = c->myconf; + if (!mc) return; + if (mc->cluster_config) { + free_ClusterConfig(mc->cluster_config); + free(mc->cluster_config); + } + free(mc->conffile); + free(mc->workdir); + free(mc->name); + free(mc); + c->myconf = NULL; +} + +/* Commit a WAL_CONFIG_MARKER for `sha`/`tag` through the real commit engine so + * config_committed_marker returns it (the committed-prefix authority 4.3 reads). */ +static void t43_commit_marker(cluster_t *c, uint64_t tag, const uint8_t sha[32]) +{ + uint64_t mlsn = 0; + TEST_ASSERT_EQUAL_INT(0, config_append_marker(c, tag, sha, &mlsn)); + TEST_ASSERT_EQUAL_INT(0, persist_flush(&c->persist)); + TEST_ASSERT_EQUAL_INT(0, + commit_submit(&c->commit, mlsn, COMMIT_MODE_WAIT, 1000)); + TEST_ASSERT_TRUE( + atomic_load_explicit(&c->persist.committed_lsn, memory_order_acquire) >= mlsn); +} + +/* (4.3-a) MATCH activates: a committed marker whose locally-held bytes' sha256 + * MATCHES the marker activates the configuration and leaves the member UN-fenced + * (R7.8). */ +static void test_config_activate_match_activates_and_unfences(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_cr43_match_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + cluster_t *c = t42_make_cluster(tmp); + TEST_ASSERT_NOT_NULL(c); + t42_make_leader(c, /*term*/3, /*n_peers*/1); + TEST_ASSERT_EQUAL_INT(0, + commit_engine_init(&c->commit, &c->persist, 1, 8)); + repl_set_commit_engine(&c->persist.repl, &c->commit); + myconf_t *mc = t43_attach_myconf(c, tmp); + + /* The locally-held bytes (saved exactly as the config-push handler would). */ + size_t blen = 0; + uint8_t *bytes = t43_make_config_blob(&blen); + uint8_t sha[32]; + SHA256(bytes, blen, sha); + uint64_t tag; + { uint64_t t; memcpy(&t, sha, 8); tag = be64toh(t); } + TEST_ASSERT_EQUAL_INT(0, conf_save_config(mc, bytes, blen, sha)); + free(bytes); + + /* Commit a marker that references EXACTLY those bytes. */ + t43_commit_marker(c, tag, sha); + + /* HASH-VERIFIED → activate, fence stays clear. */ + TEST_ASSERT_EQUAL_INT(0, config_activate_committed(c)); + TEST_ASSERT_EQUAL_INT(0, config_serve_fenced(c)); + /* Activation loaded the config into myconf. */ + TEST_ASSERT_NOT_NULL(mc->cluster_config); + TEST_ASSERT_EQUAL_HEX64(tag, mc->config_version_tag); + + t43_free_myconf(c); + commit_engine_fini(&c->commit); + t42_teardown(c); + printf("test_config_activate_match_activates_and_unfences: PASS\n"); +} + +/* (4.3-b) MISMATCH fences + retries, NEVER activates: an injected hash mismatch + * (locally-held bytes whose sha256 != the committed marker) keeps the member + * FENCED (non-serving), ALIVE, and retrying, and does NOT activate the mismatched + * config (R7.9). */ +static void test_config_activate_mismatch_fences_and_does_not_activate(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_cr43_mismatch_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + cluster_t *c = t42_make_cluster(tmp); + TEST_ASSERT_NOT_NULL(c); + t42_make_leader(c, /*term*/4, /*n_peers*/1); + TEST_ASSERT_EQUAL_INT(0, + commit_engine_init(&c->commit, &c->persist, 1, 8)); + repl_set_commit_engine(&c->persist.repl, &c->commit); + myconf_t *mc = t43_attach_myconf(c, tmp); + + /* Locally-held bytes are blob A (sha A). */ + size_t blenA = 0; + uint8_t *bytesA = t43_make_config_blob(&blenA); + uint8_t shaA[32]; + SHA256(bytesA, blenA, shaA); + TEST_ASSERT_EQUAL_INT(0, conf_save_config(mc, bytesA, blenA, shaA)); + free(bytesA); + + /* But the committed marker references a DIFFERENT hash (injected mismatch: + * the bytes for this marker have NOT arrived out-of-band yet). */ + uint8_t shaB[32]; + uint64_t tagB = t42_make_ref(shaB, 0x99); /* deterministic != shaA */ + TEST_ASSERT_FALSE(memcmp(shaA, shaB, 32) == 0); + t43_commit_marker(c, tagB, shaB); + + /* MISMATCH → fenced, NOT activated, member stays alive (returns, no abort). */ + TEST_ASSERT_EQUAL_INT(-EAGAIN, config_activate_committed(c)); + TEST_ASSERT_EQUAL_INT(1, config_serve_fenced(c)); + /* The mismatched marker's config was NOT activated: the active config tag is + * still the locally-held blob A's tag, NEVER the marker's tagB. */ + TEST_ASSERT_NOT_EQUAL(tagB, mc->config_version_tag); + + /* RETRYING: a second call while still mismatched stays fenced (idempotent, + * never activates mismatched) — the member keeps retrying, alive. */ + TEST_ASSERT_EQUAL_INT(-EAGAIN, config_activate_committed(c)); + TEST_ASSERT_EQUAL_INT(1, config_serve_fenced(c)); + TEST_ASSERT_NOT_EQUAL(tagB, mc->config_version_tag); + + t43_free_myconf(c); + commit_engine_fini(&c->commit); + t42_teardown(c); + printf("test_config_activate_mismatch_fences_and_does_not_activate: PASS\n"); +} + +/* (4.3-c) OBTAINED-OUT-OF-BAND bytes are verified before activate: a member is + * fenced while the marker's bytes are absent locally; once the matching bytes + * arrive out-of-band (saved exactly as handle_repl_config_push would), the next + * activate hash-verifies them against the marker and activates, clearing the + * fence (R7.5/R7.8). */ +static void test_config_activate_obtained_bytes_verified_before_activate(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_cr43_obtain_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + cluster_t *c = t42_make_cluster(tmp); + TEST_ASSERT_NOT_NULL(c); + t42_make_leader(c, /*term*/5, /*n_peers*/1); + TEST_ASSERT_EQUAL_INT(0, + commit_engine_init(&c->commit, &c->persist, 1, 8)); + repl_set_commit_engine(&c->persist.repl, &c->commit); + myconf_t *mc = t43_attach_myconf(c, tmp); + + /* The marker's referenced bytes. Compute its hash; commit the marker BEFORE + * the bytes are present locally (no conf_save_config yet). */ + size_t blen = 0; + uint8_t *bytes = t43_make_config_blob(&blen); + uint8_t sha[32]; + SHA256(bytes, blen, sha); + uint64_t tag; + { uint64_t t; memcpy(&t, sha, 8); tag = be64toh(t); } + t43_commit_marker(c, tag, sha); + + /* Bytes NOT present locally yet → fenced, not activated (R7.9). */ + TEST_ASSERT_EQUAL_INT(-EAGAIN, config_activate_committed(c)); + TEST_ASSERT_EQUAL_INT(1, config_serve_fenced(c)); + + /* Bytes arrive out-of-band: the config-push transport saves them locally + * exactly as handle_repl_config_push does (conf_save_config). */ + TEST_ASSERT_EQUAL_INT(0, conf_save_config(mc, bytes, blen, sha)); + free(bytes); + + /* Now the obtained bytes hash-verify against the committed marker → activate, + * fence clears. */ + TEST_ASSERT_EQUAL_INT(0, config_activate_committed(c)); + TEST_ASSERT_EQUAL_INT(0, config_serve_fenced(c)); + TEST_ASSERT_NOT_NULL(mc->cluster_config); + TEST_ASSERT_EQUAL_HEX64(tag, mc->config_version_tag); + + t43_free_myconf(c); + commit_engine_fini(&c->commit); + t42_teardown(c); + printf("test_config_activate_obtained_bytes_verified_before_activate: PASS\n"); +} + +/* (4.3-d) RECOVERY activates the LATEST committed marker: with two committed + * markers, config_activate_committed activates the config referenced by the + * LATEST committed marker (the one config_committed_marker derives), obtaining + * its bytes from local out-of-band storage (R7.10). */ +static void test_config_activate_recovery_uses_latest_committed_marker(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_cr43_recover_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + cluster_t *c = t42_make_cluster(tmp); + TEST_ASSERT_NOT_NULL(c); + t42_make_leader(c, /*term*/6, /*n_peers*/1); + TEST_ASSERT_EQUAL_INT(0, + commit_engine_init(&c->commit, &c->persist, 1, 8)); + repl_set_commit_engine(&c->persist.repl, &c->commit); + myconf_t *mc = t43_attach_myconf(c, tmp); + + /* Two distinct configs, each committed as a marker in turn; the LATEST wins. */ + size_t blen1 = 0, blen2 = 0; + uint8_t *b1 = t43_make_config_blob(&blen1); + /* Make blob 2 differ so its hash differs from blob 1. */ + uint8_t *b2 = t43_make_config_blob(&blen2); + TEST_ASSERT_TRUE(blen2 >= 1); + b2[blen2 - 1] ^= 0xFF; /* perturb → distinct sha */ + uint8_t sha1[32], sha2[32]; + SHA256(b1, blen1, sha1); + SHA256(b2, blen2, sha2); + TEST_ASSERT_FALSE(memcmp(sha1, sha2, 32) == 0); + uint64_t tag2; + { uint64_t t; memcpy(&t, sha2, 8); tag2 = be64toh(t); } + + uint64_t tag1; { uint64_t t; memcpy(&t, sha1, 8); tag1 = be64toh(t); } + t43_commit_marker(c, tag1, sha1); + t43_commit_marker(c, tag2, sha2); + + /* On recovery, the LOCALLY-held bytes are the LATEST config (blob 2 — what the + * out-of-band store holds after the latest push). */ + TEST_ASSERT_EQUAL_INT(0, conf_save_config(mc, b2, blen2, sha2)); + free(b1); + free(b2); + + /* Activation targets the LATEST committed marker (tag2), hash-verifies the + * local bytes, and activates it. */ + TEST_ASSERT_EQUAL_INT(0, config_activate_committed(c)); + TEST_ASSERT_EQUAL_INT(0, config_serve_fenced(c)); + TEST_ASSERT_EQUAL_HEX64(tag2, mc->config_version_tag); + + t43_free_myconf(c); + commit_engine_fini(&c->commit); + t42_teardown(c); + printf("test_config_activate_recovery_uses_latest_committed_marker: PASS\n"); +} + +/* ── task 4.3 REMEDIATION r1: production integration of the consumer ─────────── + * + * Round-1 review REJECTED 4.3 for three integration gaps, fixed below: + * A — verify-before-activate was BYPASSED on the push path (handle_repl_config_ + * push pre-activated via load_primary_config before the gate ran). + * B — recovery-activate had NO production call site (boot loaded blindly). + * C — the serve-fence had NO production reader (ha_dispatch_gate ignored it). + */ + +/* (4.3-A) PUSH-PATH SOLE ACTIVATION: config_activate_committed is the SOLE + * activation point. With a committed marker that references DIFFERENT bytes than + * the ones just delivered out-of-band, the push-path sequence (conf_save_config + * then config_activate_committed — NO pre-emptive load_primary_config) must NOT + * make the pushed bytes live (verify-before-activate is a real GATE, not an + * after-the-fact overlay). Regression for Finding A. */ +static void test_push_path_mismatch_does_not_preactivate(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_cr43_pushgate_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + cluster_t *c = t42_make_cluster(tmp); + TEST_ASSERT_NOT_NULL(c); + t42_make_leader(c, /*term*/7, /*n_peers*/1); + TEST_ASSERT_EQUAL_INT(0, + commit_engine_init(&c->commit, &c->persist, 1, 8)); + repl_set_commit_engine(&c->persist.repl, &c->commit); + myconf_t *mc = t43_attach_myconf(c, tmp); + + /* A committed marker references blob B (sha B) — the config the cluster + * agreed to, whose bytes have NOT yet arrived here. */ + size_t blenB = 0; + uint8_t *bytesB = t43_make_config_blob(&blenB); + bytesB[blenB - 1] ^= 0x5A; /* perturb → distinct sha */ + uint8_t shaB[32]; + SHA256(bytesB, blenB, shaB); + uint64_t tagB; { uint64_t t; memcpy(&t, shaB, 8); tagB = be64toh(t); } + t43_commit_marker(c, tagB, shaB); + free(bytesB); + + /* The push delivers blob A (a DIFFERENT/older config). The push handler now + * SAVES the bytes but does NOT pre-activate; activation is gated. */ + size_t blenA = 0; + uint8_t *bytesA = t43_make_config_blob(&blenA); + uint8_t shaA[32]; + SHA256(bytesA, blenA, shaA); + TEST_ASSERT_FALSE(memcmp(shaA, shaB, 32) == 0); + TEST_ASSERT_EQUAL_INT(0, conf_save_config(mc, bytesA, blenA, shaA)); + free(bytesA); + + /* The push-path SOLE activation: gate fences (marker references blob B, not + * the just-saved blob A) and does NOT load blob A. */ + TEST_ASSERT_EQUAL_INT(-EAGAIN, config_activate_committed(c)); + TEST_ASSERT_EQUAL_INT(1, config_serve_fenced(c)); + /* Blob A was NOT made live: no decoded config, no version tag for A. */ + TEST_ASSERT_NULL(mc->cluster_config); + + t43_free_myconf(c); + commit_engine_fini(&c->commit); + t42_teardown(c); + printf("test_push_path_mismatch_does_not_preactivate: PASS\n"); +} + +/* (4.3-A/B) NO-MARKER FALLBACK: when there is NO committed marker yet (legacy / + * first boot, or the marker has not committed), config_activate_committed FALLS + * BACK to the existing load_primary_config behavior — it activates the + * locally-held bytes (so config_version_tag is set for the freshness gate and the + * push ACK) and does NOT fence. Absent-marker is NOT a mismatch. */ +static void test_config_activate_no_marker_falls_back_and_loads(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_cr43_fallback_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + cluster_t *c = t42_make_cluster(tmp); + TEST_ASSERT_NOT_NULL(c); + t42_make_leader(c, /*term*/8, /*n_peers*/1); + TEST_ASSERT_EQUAL_INT(0, + commit_engine_init(&c->commit, &c->persist, 1, 8)); + repl_set_commit_engine(&c->persist.repl, &c->commit); + myconf_t *mc = t43_attach_myconf(c, tmp); + + /* Local bytes present, but NO committed marker exists. */ + size_t blen = 0; + uint8_t *bytes = t43_make_config_blob(&blen); + uint8_t sha[32]; + SHA256(bytes, blen, sha); + uint64_t tag; { uint64_t t; memcpy(&t, sha, 8); tag = be64toh(t); } + TEST_ASSERT_EQUAL_INT(0, conf_save_config(mc, bytes, blen, sha)); + free(bytes); + + /* No marker → fall back to load_primary_config (blind load), NOT a fence. */ + TEST_ASSERT_EQUAL_INT(0, config_activate_committed(c)); + TEST_ASSERT_EQUAL_INT(0, config_serve_fenced(c)); + TEST_ASSERT_NOT_NULL(mc->cluster_config); + TEST_ASSERT_EQUAL_HEX64(tag, mc->config_version_tag); + + t43_free_myconf(c); + commit_engine_fini(&c->commit); + t42_teardown(c); + printf("test_config_activate_no_marker_falls_back_and_loads: PASS\n"); +} + +/* (4.3-C) FENCE IS ENFORCED by a production reader: while the config serve-fence + * is set, ha_dispatch_gate refuses config-dependent state-changing operations on + * the PRIMARY (loud non-serving, member stays alive) and a fenced member is + * non-promotable (ha_config_promotion_fenced). Read-only/replication ops still + * pass so the bytes can arrive out-of-band. Once correct bytes arrive and + * re-verify clears the fence, the member serves again. Regression for Finding C. */ +static void test_serve_fence_enforced_by_dispatch_gate_and_promotion(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_cr43_fence_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + cluster_t *c = t42_make_cluster(tmp); + TEST_ASSERT_NOT_NULL(c); + t42_make_leader(c, /*term*/9, /*n_peers*/1); + TEST_ASSERT_EQUAL_INT(0, + commit_engine_init(&c->commit, &c->persist, 1, 8)); + repl_set_commit_engine(&c->persist.repl, &c->commit); + myconf_t *mc = t43_attach_myconf(c, tmp); + + /* The committed config (blob TARGET) whose bytes have not yet arrived. */ + size_t tlen = 0; + uint8_t *tbytes = t43_make_config_blob(&tlen); + uint8_t tsha[32]; + SHA256(tbytes, tlen, tsha); + uint64_t ttag; { uint64_t t; memcpy(&t, tsha, 8); ttag = be64toh(t); } + t43_commit_marker(c, ttag, tsha); + + /* Stale local bytes (blob A) that do NOT match the committed marker. */ + size_t alen = 0; + uint8_t *abytes = t43_make_config_blob(&alen); + abytes[alen - 1] ^= 0x33; + uint8_t asha[32]; + SHA256(abytes, alen, asha); + TEST_ASSERT_FALSE(memcmp(asha, tsha, 32) == 0); + TEST_ASSERT_EQUAL_INT(0, conf_save_config(mc, abytes, alen, asha)); + free(abytes); + + /* Mismatch → fenced. */ + TEST_ASSERT_EQUAL_INT(-EAGAIN, config_activate_committed(c)); + TEST_ASSERT_EQUAL_INT(1, config_serve_fenced(c)); + + /* PRODUCTION READER: a fenced PRIMARY refuses config-dependent state-changing + * ops (loud non-serving) but stays alive — the gate returns an error, not a + * crash/exit. */ + TEST_ASSERT_TRUE(ha_dispatch_gate(&c->ha, OPCode_jobsub) < 0); + /* Replication / read-only ops still pass so the correct bytes can arrive. */ + TEST_ASSERT_EQUAL_INT(0, ha_dispatch_gate(&c->ha, OPCode_replConfigPush)); + TEST_ASSERT_EQUAL_INT(0, ha_dispatch_gate(&c->ha, OPCode_get)); + /* Non-promotable while fenced. */ + TEST_ASSERT_EQUAL_INT(1, ha_config_promotion_fenced(&c->ha)); + + /* Correct bytes arrive out-of-band (saved as the push handler would). */ + TEST_ASSERT_EQUAL_INT(0, conf_save_config(mc, tbytes, tlen, tsha)); + free(tbytes); + + /* Re-verify clears the fence → member serves and is promotable again. */ + TEST_ASSERT_EQUAL_INT(0, config_activate_committed(c)); + TEST_ASSERT_EQUAL_INT(0, config_serve_fenced(c)); + TEST_ASSERT_EQUAL_HEX64(ttag, mc->config_version_tag); + TEST_ASSERT_EQUAL_INT(0, ha_dispatch_gate(&c->ha, OPCode_jobsub)); + TEST_ASSERT_EQUAL_INT(0, ha_config_promotion_fenced(&c->ha)); + + t43_free_myconf(c); + commit_engine_fini(&c->commit); + t42_teardown(c); + printf("test_serve_fence_enforced_by_dispatch_gate_and_promotion: PASS\n"); +} + +/* Append a WAL_CONFIG_MARKER for `sha`/`tag` and flush it to the WAL, but do NOT + * advance the committed prefix past it — returns its LSN. Models a STANDBY whose + * local WAL tip runs ahead of its committed_lsn: on a standby committed_lsn is + * driven by the primary's commit-advance (commit_set_raft_commit_index / the + * library), NOT by local durability, so a flushed-but-not-yet-committed marker is + * the real push-before-commit state. The caller pins committed_lsn back to the + * prior value (see the test) so config_committed_marker still derives the OLDER + * marker until the NEW one is explicitly committed. */ +static uint64_t t43_append_marker_uncommitted(cluster_t *c, uint64_t tag, + const uint8_t sha[32]) +{ + uint64_t mlsn = 0; + TEST_ASSERT_EQUAL_INT(0, config_append_marker(c, tag, sha, &mlsn)); + TEST_ASSERT_EQUAL_INT(0, persist_flush(&c->persist)); + return mlsn; +} + +/* (4.3-D / R7.9) COMMIT-ADVANCE RETRY clears a push-before-commit fence: the + * config MARKER (WAL) and config BYTES (out-of-band push) arrive on INDEPENDENT + * transports. On a 2nd upgrade the NEW bytes are saved BEFORE the NEW marker + * enters the committed prefix, so config_committed_marker still returns the OLDER + * marker → the freshly-saved NEW bytes hash-MISMATCH it → the member is serve- + * FENCED. The NEW marker commits later via the WAL path, but on_commit_advanced + * cannot re-activate (no disk I/O on the iomgr ACK thread) and the primary's + * config-behind re-push net does not fire (the push already advanced the advertised + * tag) — WITHOUT the commit-advance retry trigger the standby would stay + * PERMANENTLY fenced. This test reproduces that ordering and asserts the retry + * trigger (config_activate_retry_fn — the SAME off-thread entry the production + * scheduler enqueues) CLEARS the fence and activates the NEW config once the NEW + * marker is committed. Regression for Finding D. + * + * The single-member commit engine advances committed_lsn to the durable WAL tip, + * so to hold the NEW marker out of the committed prefix this test PINS + * committed_lsn at the OLD marker's LSN (modeling a standby whose committed prefix + * lags its local WAL tip), then advances it past the NEW marker to model the NEW + * marker committing via the WAL path. */ +static void test_commit_advance_retry_clears_push_before_commit_fence(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_cr43_retry_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + cluster_t *c = t42_make_cluster(tmp); + TEST_ASSERT_NOT_NULL(c); + t42_make_leader(c, /*term*/10, /*n_peers*/1); + TEST_ASSERT_EQUAL_INT(0, + commit_engine_init(&c->commit, &c->persist, 1, 8)); + repl_set_commit_engine(&c->persist.repl, &c->commit); + myconf_t *mc = t43_attach_myconf(c, tmp); + + /* ── Upgrade 1 (OLD): commit OLD marker + save OLD bytes + activate. The + * member is UN-fenced and serving the OLD config. ───────────────────────── */ + size_t blenO = 0; + uint8_t *bytesO = t43_make_config_blob(&blenO); + uint8_t shaO[32]; + SHA256(bytesO, blenO, shaO); + uint64_t tagO; { uint64_t t; memcpy(&t, shaO, 8); tagO = be64toh(t); } + TEST_ASSERT_EQUAL_INT(0, conf_save_config(mc, bytesO, blenO, shaO)); + free(bytesO); + t43_commit_marker(c, tagO, shaO); + TEST_ASSERT_EQUAL_INT(0, config_activate_committed(c)); + TEST_ASSERT_EQUAL_INT(0, config_serve_fenced(c)); + TEST_ASSERT_EQUAL_HEX64(tagO, mc->config_version_tag); + + /* The OLD marker is the committed prefix boundary the standby currently sees. */ + uint64_t committed_at_old = + atomic_load_explicit(&c->persist.committed_lsn, memory_order_acquire); + + /* ── Upgrade 2 (NEW): the BYTES arrive (and are saved) BEFORE the NEW marker + * enters the committed prefix — the push-before-commit ordering. ────────── */ + size_t blenN = 0; + uint8_t *bytesN = t43_make_config_blob(&blenN); + bytesN[blenN - 1] ^= 0xA5; /* perturb → distinct sha */ + uint8_t shaN[32]; + SHA256(bytesN, blenN, shaN); + uint64_t tagN; { uint64_t t; memcpy(&t, shaN, 8); tagN = be64toh(t); } + TEST_ASSERT_FALSE(memcmp(shaO, shaN, 32) == 0); + + /* The NEW marker is APPENDED + flushed to the WAL (local tip advances). */ + uint64_t new_mlsn = t43_append_marker_uncommitted(c, tagN, shaN); + TEST_ASSERT_TRUE(new_mlsn > committed_at_old); + + /* The OLD config is currently LIVE (activated in upgrade 1). Capture the live + * decoded-config pointer as the activation witness: load_primary_config frees + * and reallocates mc->cluster_config, so a CHANGED pointer proves a (re)load + * happened. conf_save_config (below) only writes bytes + bumps the advertised + * config_version_tag — it does NOT decode/activate — so it must NOT change this + * pointer; only a successful config_activate_committed does. */ + cluster_config_t *live_before = mc->cluster_config; + TEST_ASSERT_NOT_NULL(live_before); + + /* The NEW bytes are saved out-of-band (exactly as handle_repl_config_push + * does). Note conf_save_config in this single-member fixture re-evaluates the + * commit barrier, which would advance committed_lsn to the durable WAL tip, and + * it bumps the advertised config_version_tag to tagN (this is the very reason + * the primary's config-behind re-push net does not re-fire — Finding D); we + * save FIRST, then PIN committed_lsn back below the NEW marker to model a + * standby whose committed prefix (driven by the primary's commit-advance, not + * local durability) has NOT yet advanced past the NEW marker. */ + TEST_ASSERT_EQUAL_INT(0, conf_save_config(mc, bytesN, blenN, shaN)); + atomic_store_explicit(&c->persist.committed_lsn, committed_at_old, + memory_order_release); + + /* Activate now: config_committed_marker still returns the OLDER marker (tagO), + * so the freshly-saved NEW bytes MISMATCH it → serve-FENCED, NEW config NOT + * ACTIVATED. The live decoded config is UNCHANGED (still the OLD one) — the + * mismatch gate never loaded the NEW bytes. This is the permanent-fence state + * the retry trigger must rescue. */ + TEST_ASSERT_EQUAL_INT(-EAGAIN, config_activate_committed(c)); + TEST_ASSERT_EQUAL_INT(1, config_serve_fenced(c)); + TEST_ASSERT_TRUE(mc->cluster_config == live_before); /* NEW not activated */ + + /* ── The NEW marker COMMITS later via the WAL path (the standby's committed + * prefix advances past it, as the primary's commit-advance heartbeat would + * drive). config_committed_marker now derives the NEW marker. ───────────── */ + atomic_store_explicit(&c->persist.committed_lsn, new_mlsn, + memory_order_release); + + /* DRIVE THE RETRY TRIGGER: the SAME off-thread activation entry the production + * commit-advance path schedules (ha_cb_commit_advanced_stub → + * config_schedule_activate_retry → delaymgr → config_activate_retry_fn). In + * production the delaymgr worker runs this OFF the iomgr ACK thread; here we + * invoke the entry directly (the fixture's delaymgr is not started). Now + * config_committed_marker returns the NEW marker and the saved NEW bytes + * hash-verify → the fence CLEARS and the NEW config ACTIVATES (cluster_config + * is reloaded — a fresh pointer). */ + TEST_ASSERT_EQUAL_INT(0, config_activate_retry_fn(c)); + TEST_ASSERT_EQUAL_INT(0, config_serve_fenced(c)); /* fence CLEARED */ + TEST_ASSERT_EQUAL_HEX64(tagN, mc->config_version_tag); /* NEW now live */ + TEST_ASSERT_NOT_NULL(mc->cluster_config); + TEST_ASSERT_TRUE(mc->cluster_config != live_before); /* (re)loaded NEW */ + + free(bytesN); + t43_free_myconf(c); + commit_engine_fini(&c->commit); + t42_teardown(c); + printf("test_commit_advance_retry_clears_push_before_commit_fence: PASS\n"); +} + /* ── wait_all_with_timeout (XCE_TESTING shim) ───── */ #ifdef XCE_TESTING @@ -2721,6 +3318,19 @@ void test_config_repl(void) /* Task 4.2 r1: live wiring + compaction-safe derive */ RUN_TEST(test_config_commit_change_live_path_commits_and_derives); RUN_TEST(test_committed_marker_survives_checkpoint_trim); + + RUN_TEST(test_config_activate_match_activates_and_unfences); + RUN_TEST(test_config_activate_mismatch_fences_and_does_not_activate); + RUN_TEST(test_config_activate_obtained_bytes_verified_before_activate); + RUN_TEST(test_config_activate_recovery_uses_latest_committed_marker); + /* Task 4.3 r1 remediation: production integration (push-path sole activation, + * no-marker fallback, serve-fence enforced by dispatch gate + promotion). */ + RUN_TEST(test_push_path_mismatch_does_not_preactivate); + RUN_TEST(test_config_activate_no_marker_falls_back_and_loads); + RUN_TEST(test_serve_fence_enforced_by_dispatch_gate_and_promotion); + /* Task 4.3 r2 remediation: commit-advance retry clears a push-before-commit + * serve-fence (Finding D / R7.9). */ + RUN_TEST(test_commit_advance_retry_clears_push_before_commit_fence); #ifdef XCE_TESTING RUN_TEST(test_load_primary_config_missing_returns_error); RUN_TEST(test_wait_all_with_timeout_slow_threads);