diff --git a/.kiro/specs/ha-term-safety/tasks.md b/.kiro/specs/ha-term-safety/tasks.md index e991735..b44c7b4 100644 --- a/.kiro/specs/ha-term-safety/tasks.md +++ b/.kiro/specs/ha-term-safety/tasks.md @@ -86,7 +86,7 @@ ## 4. Configuration ordering as a unified `(term,lsn)` marker (R7) -- [ ] 4.1 Add the `WAL_CONFIG_MARKER` entry type and reference payload +- [x] 4.1 Add the `WAL_CONFIG_MARKER` entry type and reference payload - Add the new WAL entry type and its marker payload carrying only `{version_tag, sha256}` (no config bytes); no on-disk header layout change - Observable completion: a config marker can be encoded/decoded with its version tag and 32-byte hash and carries no configuration bytes - _Requirements: 7.2, 7.4_ @@ -182,3 +182,6 @@ - **bug-960 (CRITICAL, caught by review — logged in buglog.json):** `ha_raft_on_repl_ack` first applied the votes-index→absolute remap UNCONDITIONALLY, but the walAck call site fires for BOTH promoted-vote eps (`vctx->ctx != NULL`, relative peer_idx) AND heartbeat standby eps (`vctx->ctx == NULL`, ALREADY-absolute peer_idx — see authoritative ha.c ~6172-6189 / ~5808). Over-shift → OOB `match_index[]` write (raft.c:802 has NO bounds check) in a 3-node cluster / wrong-peer FALSE-QUORUM data-loss in larger. FIX: `ha_repl_ack_abs_peer_idx(ha, ep_is_vote, peer_idx)` resolver (vote→remap, heartbeat→passthrough) resolved at the call site via `vctx->ctx != NULL`; `ha_raft_on_repl_ack` now takes ABSOLUTE idx + fail-loud `[0,n_peers)` self-excluding bounds check (logs+returns, never crashes/self-terminates). Unit tests had MASKED it (only exercised votes_idx under my_priority==0). LESSON: any peer-index passed to a library `match_index[]`/`next_index[]` write MUST be resolved per endpoint-type (vote-relative vs heartbeat-absolute) — never blind-remap; test BOTH endpoint types at my_priority 0 AND non-0. - **DO-NOT-REPEAT (cerebrum):** an implementer ran `clang-format -i` (whole-file reformat) then `git checkout HEAD -- ` to undo it, which WIPED the uncommitted task diff (HEAD was at the prior task's commit). It reconstructed the diff from context and re-review confirmed fidelity, but: never `git checkout HEAD --` to undo a reformat on a file holding uncommitted work; reformat only the changed hunks. - **STILL DEFERRED to 5.2/5.4 (carried 3.2→3.3):** live steady-state send is still TERM-0 `repl_broadcast`; the library send loop (`on_send_append_entries`) is not yet the live sender, so the ACK→commit path is proven end-to-end in UNIT TESTS but the first LIVE `on_commit_advanced(>=1)` requires retiring term-0 `repl_broadcast` + driving the library send. Live chaotic convergence (and this retirement) is validated/owned by 5.2 (clean failover) + 5.4 (chaotic regression). Phase 3 (drive library entry points) is structurally complete; phase 4 (config marker) + phase 5 (verification) remain. + +- **4.1 done.** `WAL_CONFIG_MARKER = 50` (wal_types.h, between WAL_NOOP=40/WAL_CHECKPOINT=100) + `wal_config_marker_t {uint64_t version_tag; uint8_t sha256[32];}` + `WAL_CONFIG_MARKER_PAYLOAD_LEN (8u+32u)`. Reference-only codec `config_marker_encode/decode` (config_repl.c/h): explicit BIG-ENDIAN version_tag + raw 32-byte sha256 memcpy; NO config-bytes parameter on any path (R7.4); `-EINVAL`/`-ENOSPC` guards, no OOB on truncated decode/short encode. 4 tests (test_config_repl.c). 40-byte hdr static_assert intact; src/lib/raft untouched. Reviewer APPROVED. + - **For 4.2 (use, don't rediscover):** (1) the marker version_tag is consistent with the EXISTING config tag: `config_version_tag = be64toh(first 8 bytes of sha256)` (config_repl.c ~171-174/382-386), so a marker's BE version_tag bytes == sha256[0..7]. (2) `wal_classify` (persist.c ~2188) has `default: assert(0)` under XCE_TESTING and `replay_wal` (persist.c ~1499) has `default: break;` — 4.2 MUST add WAL_CONFIG_MARKER classification (and update the hand-maintained `test_classify_is_total_over_wal_entry_type` array in test_commit_barrier_rollout.c ~263-281) BEFORE any live path appends a type-50 entry, or the assert fires. The marker rides the `raft_entry_blob_t {type=WAL_CONFIG_MARKER, payload_len=40}` envelope through `ha_primary_log_append`→raft_log_append→on_persist_entry. diff --git a/src/controller/config_repl.c b/src/controller/config_repl.c index bfd833c..d2f7098 100644 --- a/src/controller/config_repl.c +++ b/src/controller/config_repl.c @@ -37,6 +37,57 @@ #include "xce/cluster_def.h" #include "wal_types.h" /* wal_entry_hdr_t, WAL_* constants */ +/* ── WAL_CONFIG_MARKER codec (ha-term-safety task 4.1) ─────────────── + * Fixed, padding-independent serialization of a config-change marker: + * 8-byte big-endian version_tag followed by the raw 32-byte SHA-256. NO + * configuration bytes are ever encoded (R7.2/R7.4) — the bytes travel + * out-of-band on the config-push transport. The explicit byte layout (rather + * than a struct memcpy) keeps the form stable across compiler padding and host + * endianness, since this payload replicates to standbys via AppendEntries. */ + +int config_marker_encode(uint64_t version_tag, const uint8_t sha256[32], + uint8_t *buf, size_t bufsz, size_t *out_len) +{ + if (!sha256 || !buf || !out_len) + return -EINVAL; + if (bufsz < WAL_CONFIG_MARKER_PAYLOAD_LEN) + return -ENOSPC; + + /* version_tag: 8 bytes, big-endian (matches config_repl wire convention). */ + buf[0] = (uint8_t)(version_tag >> 56); + buf[1] = (uint8_t)(version_tag >> 48); + buf[2] = (uint8_t)(version_tag >> 40); + buf[3] = (uint8_t)(version_tag >> 32); + buf[4] = (uint8_t)(version_tag >> 24); + buf[5] = (uint8_t)(version_tag >> 16); + buf[6] = (uint8_t)(version_tag >> 8); + buf[7] = (uint8_t)(version_tag); + + /* sha256: raw 32 bytes — the configuration *reference*, never the bytes. */ + memcpy(buf + 8, sha256, 32); + + *out_len = WAL_CONFIG_MARKER_PAYLOAD_LEN; + return 0; +} + +int config_marker_decode(const uint8_t *buf, size_t len, + uint64_t *version_tag, uint8_t sha256_out[32]) +{ + if (!buf || len < WAL_CONFIG_MARKER_PAYLOAD_LEN) + return -EINVAL; + + if (version_tag) { + *version_tag = ((uint64_t)buf[0] << 56) | ((uint64_t)buf[1] << 48) | + ((uint64_t)buf[2] << 40) | ((uint64_t)buf[3] << 32) | + ((uint64_t)buf[4] << 24) | ((uint64_t)buf[5] << 16) | + ((uint64_t)buf[6] << 8) | ((uint64_t)buf[7]); + } + if (sha256_out) + memcpy(sha256_out, buf + 8, 32); + + return 0; +} + /* SSL context for xce_probe_config_tag. * Stored as void* to avoid SSL_CTX type at this point in the translation unit. */ static void *s_probe_ssl_ctx = NULL; diff --git a/src/controller/config_repl.h b/src/controller/config_repl.h index 42342e8..38548a7 100644 --- a/src/controller/config_repl.h +++ b/src/controller/config_repl.h @@ -9,6 +9,7 @@ #include "clusterconfig.h" #include "netio.h" #include "request.h" +#include "wal_types.h" /* WAL_CONFIG_MARKER, WAL_CONFIG_MARKER_PAYLOAD_LEN */ /* Max config blob size — reject before allocating */ #define CONF_MAX_SIZE (1 * 1024 * 1024) /* 1 MiB */ @@ -29,6 +30,49 @@ /* Certificate expiry warning threshold (days) */ #define CERT_WARN_DAYS 30 +/* ── WAL_CONFIG_MARKER codec (ha-term-safety task 4.1) ─────────────── + * + * A config-change marker payload is a *reference* to a configuration — its + * version tag and full SHA-256 content hash — and carries NO configuration + * bytes (R7.2/R7.4). The bytes always travel out-of-band on the config-push + * transport; they are never placed in a WAL payload. This codec produces the + * fixed, padding-independent on-wire form (big-endian version_tag + raw 32-byte + * sha256) that rides inside the raft_entry_blob_t envelope as the payload of a + * WAL_CONFIG_MARKER entry. + */ + +/* + * config_marker_encode: serialize a {version_tag, sha256} config-change marker + * into a fixed WAL_CONFIG_MARKER_PAYLOAD_LEN-byte buffer. NO config bytes are + * ever written. version_tag is emitted big-endian; sha256 is the raw 32 bytes. + * + * buf — output buffer (must be >= WAL_CONFIG_MARKER_PAYLOAD_LEN) + * bufsz — capacity of buf + * out_len — receives the number of bytes written (always + * WAL_CONFIG_MARKER_PAYLOAD_LEN on success); untouched on failure + * + * Returns 0 on success; -EINVAL if sha256/buf/out_len is NULL; + * -ENOSPC if bufsz < WAL_CONFIG_MARKER_PAYLOAD_LEN. + */ +int config_marker_encode(uint64_t version_tag, const uint8_t sha256[32], + uint8_t *buf, size_t bufsz, size_t *out_len); + +/* + * config_marker_decode: deserialize a WAL_CONFIG_MARKER payload produced by + * config_marker_encode. Reads exactly WAL_CONFIG_MARKER_PAYLOAD_LEN bytes; a + * shorter buffer is rejected without any out-of-bounds read. + * + * buf — input buffer + * len — bytes available in buf + * version_tag — receives the host-order version tag (may be NULL to skip) + * sha256_out — receives the raw 32-byte content hash (may be NULL to skip) + * + * Returns 0 on success; -EINVAL if buf is NULL or len < + * WAL_CONFIG_MARKER_PAYLOAD_LEN. + */ +int config_marker_decode(const uint8_t *buf, size_t len, + uint64_t *version_tag, uint8_t sha256_out[32]); + /* ── Phase 1: Storage ────────────────────────────────────────── */ /* diff --git a/src/controller/testsuite/test_config_repl.c b/src/controller/testsuite/test_config_repl.c index 673f597..b8a3cb1 100644 --- a/src/controller/testsuite/test_config_repl.c +++ b/src/controller/testsuite/test_config_repl.c @@ -504,6 +504,92 @@ static void test_conf_save_config_path_consistency(void) printf("test_conf_save_config_path_consistency: PASS\n"); } +/* ── WAL_CONFIG_MARKER codec (ha-term-safety task 4.1) ─────────────── + * The config-change marker payload carries ONLY {version_tag, sha256} — + * a reference, never the configuration bytes (R7.2/R7.4). These tests + * exercise the fixed-size encode/decode codec that rides inside the + * raft_entry_blob_t envelope with type == WAL_CONFIG_MARKER. */ + +/* Round-trip: encode {version_tag, sha256} then decode → identical, and the + * encoded form is exactly the fixed marker size (tag + 32-byte hash) with NO + * configuration bytes appended. */ +static void test_config_marker_encode_decode_roundtrip(void) +{ + const uint64_t version_tag = 0x0123456789abcdefULL; + uint8_t sha256[32]; + for (int i = 0; i < 32; i++) + sha256[i] = (uint8_t)(0xA0 + i); + + uint8_t buf[64]; + size_t enc_len = 0; + int rc = config_marker_encode(version_tag, sha256, buf, sizeof(buf), &enc_len); + TEST_ASSERT_EQUAL_INT(0, rc); + + /* Encoded form carries NO config bytes: exactly 8 (tag) + 32 (sha256). */ + TEST_ASSERT_EQUAL_UINT(WAL_CONFIG_MARKER_PAYLOAD_LEN, enc_len); + TEST_ASSERT_EQUAL_UINT(8u + 32u, enc_len); + + uint64_t got_tag = 0; + uint8_t got_sha[32]; + memset(got_sha, 0, sizeof(got_sha)); + rc = config_marker_decode(buf, enc_len, &got_tag, got_sha); + TEST_ASSERT_EQUAL_INT(0, rc); + + TEST_ASSERT_EQUAL_HEX64(version_tag, got_tag); + TEST_ASSERT_EQUAL_HEX8_ARRAY(sha256, got_sha, 32); + + printf("test_config_marker_encode_decode_roundtrip: PASS\n"); +} + +/* The WAL entry type value must be exactly 50 and must not collide with the + * neighbouring WAL_NOOP (40) or WAL_CHECKPOINT (100) types (design.md). */ +static void test_config_marker_enum_value_is_50(void) +{ + TEST_ASSERT_EQUAL_INT(50, WAL_CONFIG_MARKER); + TEST_ASSERT_NOT_EQUAL(WAL_NOOP, WAL_CONFIG_MARKER); + TEST_ASSERT_NOT_EQUAL(WAL_CHECKPOINT, WAL_CONFIG_MARKER); + /* Strictly between WAL_NOOP and WAL_CHECKPOINT. */ + TEST_ASSERT_TRUE(WAL_CONFIG_MARKER > WAL_NOOP); + TEST_ASSERT_TRUE(WAL_CONFIG_MARKER < WAL_CHECKPOINT); + printf("test_config_marker_enum_value_is_50: PASS\n"); +} + +/* A truncated (short) buffer must fail cleanly with -EINVAL, never read past + * the end of the supplied buffer. */ +static void test_config_marker_decode_truncated_fails(void) +{ + uint8_t sha256[32]; + memset(sha256, 0x5c, sizeof(sha256)); + uint8_t buf[WAL_CONFIG_MARKER_PAYLOAD_LEN]; + size_t enc_len = 0; + TEST_ASSERT_EQUAL_INT(0, config_marker_encode(0xdeadbeefcafef00dULL, sha256, + buf, sizeof(buf), &enc_len)); + + uint64_t tag = 0; + uint8_t sha_out[32]; + /* One byte short of a full marker payload → reject, no OOB read. */ + int rc = config_marker_decode(buf, enc_len - 1, &tag, sha_out); + TEST_ASSERT_EQUAL_INT(-EINVAL, rc); + + /* Zero-length buffer → reject. */ + rc = config_marker_decode(buf, 0, &tag, sha_out); + TEST_ASSERT_EQUAL_INT(-EINVAL, rc); + + printf("test_config_marker_decode_truncated_fails: PASS\n"); +} + +/* A too-small output buffer must fail with -ENOSPC and not overflow. */ +static void test_config_marker_encode_short_buffer_fails(void) +{ + uint8_t sha256[32]; + memset(sha256, 0x11, sizeof(sha256)); + uint8_t small[WAL_CONFIG_MARKER_PAYLOAD_LEN - 1]; + size_t enc_len = 123; /* sentinel — must be left untouched on failure */ + int rc = config_marker_encode(7, sha256, small, sizeof(small), &enc_len); + TEST_ASSERT_EQUAL_INT(-ENOSPC, rc); + printf("test_config_marker_encode_short_buffer_fails: PASS\n"); +} + /* ── wait_all_with_timeout (XCE_TESTING shim) ───── */ #ifdef XCE_TESTING @@ -2260,6 +2346,11 @@ void test_config_repl(void) RUN_TEST(test_sanitise_cn_control_chars); RUN_TEST(test_sanitise_cn_truncated); RUN_TEST(test_conf_save_config_path_consistency); + /* Task 4.1: WAL_CONFIG_MARKER type + reference-only codec */ + RUN_TEST(test_config_marker_enum_value_is_50); + RUN_TEST(test_config_marker_encode_decode_roundtrip); + RUN_TEST(test_config_marker_decode_truncated_fails); + RUN_TEST(test_config_marker_encode_short_buffer_fails); #ifdef XCE_TESTING RUN_TEST(test_load_primary_config_missing_returns_error); RUN_TEST(test_wait_all_with_timeout_slow_threads); diff --git a/src/include/wal_types.h b/src/include/wal_types.h index 1e18948..2e10d29 100644 --- a/src/include/wal_types.h +++ b/src/include/wal_types.h @@ -76,5 +76,30 @@ typedef enum { WAL_QUEUE_CTRL = 20, WAL_WORKER_CTRL = 30, WAL_NOOP = 40, /* Internal: empty payload, replay no-op; appended only at promotion */ + WAL_CONFIG_MARKER = 50, /* Config-change marker: {version_tag, sha256} reference only — + * NEVER the config bytes (R7.2/R7.4). Ordered + committed through + * the same (term,lsn) log as job state (ha-term-safety). */ WAL_CHECKPOINT = 100, /* Local WAL marker only — never replicated */ } wal_entry_type; + +/* ── WAL_CONFIG_MARKER payload (ha-term-safety task 4.1) ──────────────────── + * The marker payload is a *reference* to a configuration — its version tag and + * full SHA-256 content hash — and carries NO configuration bytes (R7.2/R7.4). + * The configuration bytes always travel out-of-band on the existing config-push + * transport (OPCode_replConfigPush), keyed to {version_tag, sha256}; they are + * NEVER placed in a WAL payload. + * + * This struct rides INSIDE the raft_entry_blob_t envelope with + * type == WAL_CONFIG_MARKER; on persist it lands as the WAL entry payload. The + * 40-byte wal_entry_hdr_t is unchanged (R1.7) — this is a payload, not a header. + * Use config_marker_encode / config_marker_decode (config_repl.c) for a fixed, + * padding-independent on-wire form. */ +typedef struct { + uint64_t version_tag; /* first 8 bytes of the config SHA-256, host order */ + uint8_t sha256[32]; /* full SHA-256 of the config bytes (the reference) */ +} wal_config_marker_t; + +/* Fixed serialized size of a WAL_CONFIG_MARKER payload: 8-byte version_tag + + * 32-byte sha256, with NO config bytes ever appended. This is the exact byte + * count config_marker_encode emits and config_marker_decode requires. */ +#define WAL_CONFIG_MARKER_PAYLOAD_LEN (8u + 32u)