diff --git a/.kiro/specs/raft-adapter-conformance/tasks.md b/.kiro/specs/raft-adapter-conformance/tasks.md index 27d89d2..a488245 100644 --- a/.kiro/specs/raft-adapter-conformance/tasks.md +++ b/.kiro/specs/raft-adapter-conformance/tasks.md @@ -23,7 +23,7 @@ - _Requirements: 7.4, 7.5_ - _Boundary: cluster.c reply handling_ -- [ ] 2.3 Test configuration-push reply integrity end-to-end +- [x] 2.3 Test configuration-push reply integrity end-to-end - Unit test: each of the four reject conditions in `handle_repl_config_push` produces `accepted=0` with the current term, via the new send-hook - Unit test: the `-EAGAIN` activation-defer path still produces `accepted=1` - Unit test: an inbound config-push reply carrying a higher term triggers adoption via the `cluster.c` wiring from 2.2 diff --git a/src/controller/testsuite/test_config_repl.c b/src/controller/testsuite/test_config_repl.c index f189cce..5c40758 100644 --- a/src/controller/testsuite/test_config_repl.c +++ b/src/controller/testsuite/test_config_repl.c @@ -3728,6 +3728,325 @@ static void test_config_push_reply_higher_term_adopts_and_steps_down(void) printf("test_config_push_reply_higher_term_adopts_and_steps_down: PASS\n"); } +/* raft-adapter-conformance task 2.3 (Req 2.2, 2.3, 2.5, 7.4): task 2.1's own + * test (above) proved only ONE of the four handle_repl_config_push reject + * early-returns actually invokes the send-hook (SHA-256 mismatch). This + * closes the gap for the FIRST reject condition: a malformed request + * (body_which mismatch or a NULL replconfigpushrequestmsg payload). */ +static void test_config_push_malformed_request_sends_explicit_reject_reply(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_cr23_malformed_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + cluster_t *c = t42_make_cluster(tmp); + TEST_ASSERT_NOT_NULL(c); + t43_attach_myconf(c, tmp); + + atomic_store(&c->ha.role, CTRL_STANDBY); + atomic_store(&c->ha.term, 11); + + /* Malformed: body_which claims a config-push request, but the payload + * pointer itself is NULL -- the adapter's own dereference guard. */ + xce_request_t req; + memset(&req, 0, sizeof(req)); + req.body_which = Request_body_replconfigpushrequestmsg; + req.body.replconfigpushrequestmsg = NULL; + + g_cpr21_hook_called = 0; + config_push_reply_send_hook = cpr21_reply_capture; + + int rc = handle_repl_config_push(c, /*ep*/NULL, &req); + + config_push_reply_send_hook = NULL; /* uninstall before any assertion can fail */ + + TEST_ASSERT_EQUAL_INT(-EINVAL, rc); + TEST_ASSERT_TRUE_MESSAGE(g_cpr21_hook_called, + "raft-adapter-conformance 2.3: a malformed config-push request must " + "send an explicit ReplConfigPushReply, not silence it entirely"); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, g_cpr21_hook_accepted, + "the reply for a malformed config-push request must carry accepted=0"); + TEST_ASSERT_EQUAL_UINT64(11, g_cpr21_hook_term); + + t43_free_myconf(c); + t42_teardown(c); + char cmd[512]; + snprintf(cmd, sizeof(cmd), "rm -rf %s", tmp); + system(cmd); + printf("test_config_push_malformed_request_sends_explicit_reject_reply: PASS\n"); +} + +/* raft-adapter-conformance task 2.3 (Req 2.2, 2.3, 2.5, 7.4): the SECOND + * reject condition -- a well-formed envelope whose payload is missing bytes + * and/or a sha256 -- must also send an explicit reject reply. */ +static void test_config_push_missing_bytes_sends_explicit_reject_reply(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_cr23_missing_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + cluster_t *c = t42_make_cluster(tmp); + TEST_ASSERT_NOT_NULL(c); + t43_attach_myconf(c, tmp); + + atomic_store(&c->ha.role, CTRL_STANDBY); + atomic_store(&c->ha.term, 12); + + /* Well-formed envelope, but bytes/len/sha256 are entirely absent. */ + repl_config_push_req_t rcpr; + memset(&rcpr, 0, sizeof(rcpr)); + rcpr.version_tag = 0x4242; + rcpr.raw_bytes = NULL; + rcpr.raw_bytes_len = 0; + rcpr.raw_sha256 = NULL; + + xce_request_t req; + memset(&req, 0, sizeof(req)); + req.body_which = Request_body_replconfigpushrequestmsg; + req.body.replconfigpushrequestmsg = &rcpr; + + g_cpr21_hook_called = 0; + config_push_reply_send_hook = cpr21_reply_capture; + + int rc = handle_repl_config_push(c, /*ep*/NULL, &req); + + config_push_reply_send_hook = NULL; /* uninstall before any assertion can fail */ + + TEST_ASSERT_EQUAL_INT(-EINVAL, rc); + TEST_ASSERT_TRUE_MESSAGE(g_cpr21_hook_called, + "raft-adapter-conformance 2.3: missing bytes/sha256 must send an " + "explicit ReplConfigPushReply, not silence it entirely"); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, g_cpr21_hook_accepted, + "the reply for missing bytes/sha256 must carry accepted=0"); + TEST_ASSERT_EQUAL_UINT64(12, g_cpr21_hook_term); + + t43_free_myconf(c); + t42_teardown(c); + char cmd[512]; + snprintf(cmd, sizeof(cmd), "rm -rf %s", tmp); + system(cmd); + printf("test_config_push_missing_bytes_sends_explicit_reject_reply: PASS\n"); +} + +/* raft-adapter-conformance task 2.3 (Req 2.2, 2.3, 2.5, 7.4): the FOURTH + * reject condition -- a local conf_save_config() failure -- must also send + * an explicit reject reply. conf_save_config is forced to fail + * deterministically by planting its target "conf" directory as a plain FILE + * instead: mkdir(conf_dir, 0700) then fails with EEXIST (tolerated -- + * conf_save_config only bails on a DIFFERENT errno), but the very next step + * -- open()ing a path INSIDE conf_dir -- hits ENOTDIR because conf_dir is + * not actually a directory. This gives a real, deterministic failure without + * relying on filesystem permission semantics (chmod-based EACCES tricks are + * unreliable when the test runs as a privileged/sandboxed uid). */ +static void test_config_push_save_failure_sends_explicit_reject_reply(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_cr23_savefail_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + cluster_t *c = t42_make_cluster(tmp); + TEST_ASSERT_NOT_NULL(c); + t43_attach_myconf(c, tmp); + + atomic_store(&c->ha.role, CTRL_STANDBY); + atomic_store(&c->ha.term, 13); + + /* persist_init (t42_make_cluster) already creates tmp/ctrl-test/data/... + * which brings tmp/ctrl-test itself into existence -- tolerate EEXIST. */ + char instdir[512]; + snprintf(instdir, sizeof(instdir), "%s/ctrl-test", tmp); + TEST_ASSERT_TRUE_MESSAGE(mkdir(instdir, 0700) == 0 || errno == EEXIST, + "must be able to ensure the myconf effective_dir exists for the fixture"); + char conf_as_file[560]; + snprintf(conf_as_file, sizeof(conf_as_file), "%s/conf", instdir); + int trap_fd = open(conf_as_file, O_WRONLY | O_CREAT | O_TRUNC, 0600); + TEST_ASSERT_TRUE_MESSAGE(trap_fd >= 0, "must be able to plant the conf-as-file trap"); + close(trap_fd); + + uint8_t payload[16]; + memset(payload, 0xCD, sizeof(payload)); + uint8_t sha[32]; + SHA256(payload, sizeof(payload), sha); + + repl_config_push_req_t rcpr; + memset(&rcpr, 0, sizeof(rcpr)); + rcpr.version_tag = 0x5353; + rcpr.raw_bytes = payload; + rcpr.raw_bytes_len = sizeof(payload); + rcpr.raw_sha256 = sha; + + xce_request_t req; + memset(&req, 0, sizeof(req)); + req.body_which = Request_body_replconfigpushrequestmsg; + req.body.replconfigpushrequestmsg = &rcpr; + + g_cpr21_hook_called = 0; + config_push_reply_send_hook = cpr21_reply_capture; + + int rc = handle_repl_config_push(c, /*ep*/NULL, &req); + + config_push_reply_send_hook = NULL; /* uninstall before any assertion can fail */ + + TEST_ASSERT_TRUE_MESSAGE(rc != 0, + "the conf-as-file trap must make conf_save_config fail (rc != 0)"); + TEST_ASSERT_TRUE_MESSAGE(g_cpr21_hook_called, + "raft-adapter-conformance 2.3: a conf_save_config failure must send " + "an explicit ReplConfigPushReply, not silence it entirely"); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, g_cpr21_hook_accepted, + "the reply for a conf_save_config failure must carry accepted=0"); + TEST_ASSERT_EQUAL_UINT64(13, g_cpr21_hook_term); + + t43_free_myconf(c); + t42_teardown(c); + char cmd[512]; + snprintf(cmd, sizeof(cmd), "rm -rf %s", tmp); + system(cmd); + printf("test_config_push_save_failure_sends_explicit_reject_reply: PASS\n"); +} + +/* raft-adapter-conformance task 2.3 (Req 2.2, 2.3, 2.5, 7.5): the + * `-EAGAIN`-from-config_activate_committed activation-defer branch must + * still send accepted=1 -- activation-deferred is NOT a rejection (the + * bytes WERE saved out-of-band successfully; only the separate, + * committed-marker-gated activation step is deferred). Fixture mirrors + * test_push_path_mismatch_does_not_preactivate (config-upgrade-snapshot + * task 4.3 remediation r1) but drives the REAL handle_repl_config_push + * entry point end-to-end instead of calling config_activate_committed + * directly, and observes the reply via the send-hook installed by task + * 2.1. */ +static void test_config_push_eagain_activation_deferred_still_sends_accepted(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_cr23_eagain_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + cluster_t *c = t42_make_cluster(tmp); + TEST_ASSERT_NOT_NULL(c); + t42_make_leader(c, /*term*/14, /*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); + 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 config than the committed + * marker references. conf_save_config succeeds (the transit sha256 + * verification is self-consistent), but config_activate_committed() + * then defers with -EAGAIN because blob A doesn't hash-verify against + * the committed marker. */ + 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); + + repl_config_push_req_t rcpr; + memset(&rcpr, 0, sizeof(rcpr)); + { uint64_t t; memcpy(&t, shaA, 8); rcpr.version_tag = be64toh(t); } + rcpr.raw_bytes = bytesA; + rcpr.raw_bytes_len = blenA; + rcpr.raw_sha256 = shaA; + + xce_request_t req; + memset(&req, 0, sizeof(req)); + req.body_which = Request_body_replconfigpushrequestmsg; + req.body.replconfigpushrequestmsg = &rcpr; + + g_cpr21_hook_called = 0; + config_push_reply_send_hook = cpr21_reply_capture; + + int rc = handle_repl_config_push(c, /*ep*/NULL, &req); + + config_push_reply_send_hook = NULL; /* uninstall before any assertion can fail */ + free(bytesA); + + TEST_ASSERT_EQUAL_INT(0, rc); + /* Still fenced -- proves the -EAGAIN branch was genuinely taken, not + * the plain-success branch (which would clear the fence). */ + TEST_ASSERT_EQUAL_INT(1, config_serve_fenced(c)); + TEST_ASSERT_TRUE_MESSAGE(g_cpr21_hook_called, + "raft-adapter-conformance 2.3: the -EAGAIN activation-defer path " + "must still send an explicit ReplConfigPushReply"); + TEST_ASSERT_EQUAL_INT_MESSAGE(1, g_cpr21_hook_accepted, + "raft-adapter-conformance 2.5/7.5: activation-deferred (-EAGAIN) is " + "NOT a rejection -- the reply must carry accepted=1"); + TEST_ASSERT_EQUAL_UINT64(14, g_cpr21_hook_term); + + t43_free_myconf(c); + commit_engine_fini(&c->commit); + t42_teardown(c); + char cmd[512]; + snprintf(cmd, sizeof(cmd), "rm -rf %s", tmp); + system(cmd); + printf("test_config_push_eagain_activation_deferred_still_sends_accepted: PASS\n"); +} + +/* raft-adapter-conformance task 2.3 (Req 7.4, 7.5): task 2.2's own test + * (test_config_push_reply_higher_term_adopts_and_steps_down, above) only + * exercised the term-adoption check on a REJECTED (accepted=0) reply. + * Req 7.4/7.5 require the term check to fire "regardless of accept/reject" + * -- this closes that gap by driving the identical real dispatch path + * (myreq_handler_for_test) with accepted=1 instead. */ +static void test_config_push_reply_higher_term_adopts_on_accept(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_cr23_termacc_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + cluster_t *c = t42_make_cluster(tmp); + TEST_ASSERT_NOT_NULL(c); + + atomic_store(&c->ha.role, CTRL_STANDBY); + atomic_store(&c->ha.term, 5); + ha_init_raft(&c->ha, c, /*n_peers*/3); + c->ha.raft.term = 5; + c->ha.raft.role = RAFT_LEADER; + + endpoint_t ep; + memset(&ep, 0, sizeof(ep)); + ep.owner = c; + + /* Accept (accepted=1) carrying a higher term (9 > 5): the term-adoption + * check must fire on the ACCEPT branch exactly as it does on reject. */ + io_packet_t *pkt = + cpr22_build_config_push_reply_pkt(/*accepted*/1, + /*version_tag*/0xBEEF, + /*term*/9); + TEST_ASSERT_NOT_NULL(pkt); + + int rc = myreq_handler_for_test(&ep, pkt); + TEST_ASSERT_EQUAL_INT(0, rc); + + TEST_ASSERT_EQUAL_UINT64_MESSAGE(9, c->ha.raft.term, + "raft-adapter-conformance 7.4: a config-push ACCEPT reply's higher " + "term must also be adopted via raft_on_higher_term_observed"); + TEST_ASSERT_NOT_EQUAL_INT_MESSAGE(RAFT_LEADER, c->ha.raft.role, + "raft-adapter-conformance 7.5: leadership held at the lower term " + "must be stepped down once a higher term is observed, even on the " + "accept branch"); + + /* pkt is already fully freed at this point (myreq_handler_for_test -> + * xce_decode() unconditionally frees it) -- see the ownership note on + * test_config_push_reply_higher_term_adopts_and_steps_down above + * (bug-1482). Do NOT free pkt here. */ + + t42_teardown(c); + char cmd[512]; + snprintf(cmd, sizeof(cmd), "rm -rf %s", tmp); + system(cmd); + printf("test_config_push_reply_higher_term_adopts_on_accept: PASS\n"); +} + #endif /* XCE_TESTING */ /* ── raw capnp shims (Task A) ───────────────────── */ @@ -7097,6 +7416,14 @@ void test_config_repl(void) RUN_TEST(test_config_push_sha256_mismatch_sends_explicit_reject_reply); /* raft-adapter-conformance task 2.2: config-push reply term adoption */ RUN_TEST(test_config_push_reply_higher_term_adopts_and_steps_down); + /* raft-adapter-conformance task 2.3: full config-push reply-integrity + * coverage -- remaining reject paths, the EAGAIN accept path, and the + * term-adoption check on the accept branch. */ + RUN_TEST(test_config_push_malformed_request_sends_explicit_reject_reply); + RUN_TEST(test_config_push_missing_bytes_sends_explicit_reject_reply); + RUN_TEST(test_config_push_save_failure_sends_explicit_reject_reply); + RUN_TEST(test_config_push_eagain_activation_deferred_still_sends_accepted); + RUN_TEST(test_config_push_reply_higher_term_adopts_on_accept); /* Task 2.1: OPCode_walCatchupEntry handler */ RUN_TEST(test_wal_catchup_entry_advances_applied_lsn); RUN_TEST(test_wal_catchup_entry_error_skipped_no_crash);