From 74d69f5c76649cbed45ee4d632f29a241a0c7244 Mon Sep 17 00:00:00 2001 From: VectorPeak <73048950+VectorPeak@users.noreply.github.com> Date: Sat, 18 Jul 2026 21:39:16 +0800 Subject: [PATCH 1/4] [Bugfix][Store] Reject invalid metadata client IDs Reject malformed client_id values while deserializing object metadata so corrupted snapshot entries are not restored with an invalid UUID. Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com> --- mooncake-store/src/master_service.cpp | 7 ++++++- .../snapshot/snapshot_child_process_test.cpp | 13 +++++++++++++ .../tests/ha/snapshot/snapshot_test_utils.h | 19 ++++++++++++++++--- 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/mooncake-store/src/master_service.cpp b/mooncake-store/src/master_service.cpp index fe95b20ea9..0a21710bce 100644 --- a/mooncake-store/src/master_service.cpp +++ b/mooncake-store/src/master_service.cpp @@ -7852,7 +7852,12 @@ MasterService::MetadataSerializer::DeserializeMetadata( // Deserialize client_id string std::string client_id_str = array[index++].as(); UUID client_id; - StringToUuid(client_id_str, client_id); + if (!StringToUuid(client_id_str, client_id)) { + return tl::unexpected(SerializationError( + ErrorCode::DESERIALIZE_FAIL, + fmt::format("deserialize ObjectMetadata invalid client_id UUID: {}", + client_id_str))); + } // Deserialize put_start_time uint64_t put_start_time_timestamp = array[index++].as(); diff --git a/mooncake-store/tests/ha/snapshot/snapshot_child_process_test.cpp b/mooncake-store/tests/ha/snapshot/snapshot_child_process_test.cpp index 45250559ad..4f1a326df1 100644 --- a/mooncake-store/tests/ha/snapshot/snapshot_child_process_test.cpp +++ b/mooncake-store/tests/ha/snapshot/snapshot_child_process_test.cpp @@ -666,6 +666,19 @@ TEST_F(SnapshotChildProcessTest, EXPECT_FALSE(ObjectIsGroupedInMetadata(key, shard_idx)); } +TEST_F(SnapshotChildProcessTest, DeserializeMetadataSkipsInvalidClientId) { + CreateDefaultService(); + const std::string key = "invalid_client_id_snapshot_key"; + + auto deserialize_result = DeserializeMetadataForTest( + BuildMetadataPayloadWithClientIdString( + "not-a-uuid", std::string_view(key.data(), key.size()))); + ASSERT_TRUE(deserialize_result.has_value()) + << deserialize_result.error().message; + + EXPECT_FALSE(KeyExistsInMetadata(service_.get(), key)); +} + TEST_F(SnapshotChildProcessTest, LegacyEtcdConnstringFallbackIsPreserved) { MasterConfig legacy_config; legacy_config.enable_ha = true; diff --git a/mooncake-store/tests/ha/snapshot/snapshot_test_utils.h b/mooncake-store/tests/ha/snapshot/snapshot_test_utils.h index 01262f671c..0d1630f961 100644 --- a/mooncake-store/tests/ha/snapshot/snapshot_test_utils.h +++ b/mooncake-store/tests/ha/snapshot/snapshot_test_utils.h @@ -167,8 +167,9 @@ inline std::vector WrapShardIntoMetadataRoot( return ToByteVector(root_buffer); } -inline std::vector BuildMetadataPayload( - const UUID& client_id, std::string_view object_key = kDefaultTestObjectKey, +inline std::vector BuildMetadataPayloadWithClientIdString( + std::string_view client_id, + std::string_view object_key = kDefaultTestObjectKey, std::string_view disk_file_path = kDefaultTestDiskFilePath, uint64_t object_size = kDefaultTestObjectSize, uint64_t put_start_time_ms = kDefaultTestPutStartTimeMs, @@ -199,7 +200,7 @@ inline std::vector BuildMetadataPayload( shard_packer.pack(std::string(object_key)); shard_packer.pack_array(array_size); - shard_packer.pack(UuidToString(client_id)); + shard_packer.pack(std::string(client_id)); shard_packer.pack(put_start_time_ms); shard_packer.pack(object_size); shard_packer.pack(lease_timeout_ms); @@ -220,6 +221,18 @@ inline std::vector BuildMetadataPayload( return WrapShardIntoMetadataRoot(shard_buffer); } +inline std::vector BuildMetadataPayload( + const UUID& client_id, std::string_view object_key = kDefaultTestObjectKey, + std::string_view disk_file_path = kDefaultTestDiskFilePath, + uint64_t object_size = kDefaultTestObjectSize, + uint64_t put_start_time_ms = kDefaultTestPutStartTimeMs, + uint64_t lease_timeout_ms = kDefaultTestLeaseTimeoutMs, + SnapshotMetadataFormat format = SnapshotMetadataFormat::kLegacy) { + return BuildMetadataPayloadWithClientIdString( + UuidToString(client_id), object_key, disk_file_path, object_size, + put_start_time_ms, lease_timeout_ms, format); +} + // Builds a metadata payload whose declared replica_count field is set to // `declared_replica_count` while no replicas are actually packed (the entry // array stays at the 7 leading fields). Used to verify the deserializer From 356497558063ba2364fb9ef1e446422c91f1e9e2 Mon Sep 17 00:00:00 2001 From: VectorPeak <73048950+VectorPeak@users.noreply.github.com> Date: Sat, 18 Jul 2026 21:42:27 +0800 Subject: [PATCH 2/4] [Bugfix][Store] Format invalid client ID regression test Apply the format expected by the repository clang-format check. Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com> --- .../tests/ha/snapshot/snapshot_child_process_test.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mooncake-store/tests/ha/snapshot/snapshot_child_process_test.cpp b/mooncake-store/tests/ha/snapshot/snapshot_child_process_test.cpp index 4f1a326df1..1562d053b7 100644 --- a/mooncake-store/tests/ha/snapshot/snapshot_child_process_test.cpp +++ b/mooncake-store/tests/ha/snapshot/snapshot_child_process_test.cpp @@ -671,8 +671,9 @@ TEST_F(SnapshotChildProcessTest, DeserializeMetadataSkipsInvalidClientId) { const std::string key = "invalid_client_id_snapshot_key"; auto deserialize_result = DeserializeMetadataForTest( - BuildMetadataPayloadWithClientIdString( - "not-a-uuid", std::string_view(key.data(), key.size()))); + BuildMetadataPayloadWithClientIdString("not-a-uuid", + std::string_view(key.data(), + key.size()))); ASSERT_TRUE(deserialize_result.has_value()) << deserialize_result.error().message; From 78d1ebff005f0d1eca215a6bd03ed7641beec20f Mon Sep 17 00:00:00 2001 From: VectorPeak <73048950+VectorPeak@users.noreply.github.com> Date: Sat, 18 Jul 2026 21:45:27 +0800 Subject: [PATCH 3/4] [Bugfix][Store] Apply clang-format to metadata test Use clang-format 20.1.8 output for the invalid metadata client ID regression test. Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com> --- .../tests/ha/snapshot/snapshot_child_process_test.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/mooncake-store/tests/ha/snapshot/snapshot_child_process_test.cpp b/mooncake-store/tests/ha/snapshot/snapshot_child_process_test.cpp index 1562d053b7..de81e5c033 100644 --- a/mooncake-store/tests/ha/snapshot/snapshot_child_process_test.cpp +++ b/mooncake-store/tests/ha/snapshot/snapshot_child_process_test.cpp @@ -670,10 +670,9 @@ TEST_F(SnapshotChildProcessTest, DeserializeMetadataSkipsInvalidClientId) { CreateDefaultService(); const std::string key = "invalid_client_id_snapshot_key"; - auto deserialize_result = DeserializeMetadataForTest( - BuildMetadataPayloadWithClientIdString("not-a-uuid", - std::string_view(key.data(), - key.size()))); + auto deserialize_result = + DeserializeMetadataForTest(BuildMetadataPayloadWithClientIdString( + "not-a-uuid", std::string_view(key.data(), key.size()))); ASSERT_TRUE(deserialize_result.has_value()) << deserialize_result.error().message; From 5eb6729e90e09a668f36ee90dcfcd956c9ce459f Mon Sep 17 00:00:00 2001 From: VectorPeak <73048950+VectorPeak@users.noreply.github.com> Date: Sun, 19 Jul 2026 00:10:35 +0800 Subject: [PATCH 4/4] [Bugfix][Store] Fix invalid client ID test payload Pack the invalid client_id regression test payload with the numeric shard key shape expected by MetadataSerializer::Deserialize(). Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com> --- .../snapshot/snapshot_child_process_test.cpp | 42 ++++++++++++++++++- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/mooncake-store/tests/ha/snapshot/snapshot_child_process_test.cpp b/mooncake-store/tests/ha/snapshot/snapshot_child_process_test.cpp index de81e5c033..68085d52b1 100644 --- a/mooncake-store/tests/ha/snapshot/snapshot_child_process_test.cpp +++ b/mooncake-store/tests/ha/snapshot/snapshot_child_process_test.cpp @@ -669,10 +669,48 @@ TEST_F(SnapshotChildProcessTest, TEST_F(SnapshotChildProcessTest, DeserializeMetadataSkipsInvalidClientId) { CreateDefaultService(); const std::string key = "invalid_client_id_snapshot_key"; + const uint32_t shard_idx = GetShardIndexForTest(key); + + msgpack::sbuffer shard_buffer; + MsgpackPacker shard_packer(&shard_buffer); + shard_packer.pack_map(1); + shard_packer.pack(std::string("metadata")); + shard_packer.pack_array(1); + shard_packer.pack_array(2); + shard_packer.pack(key); + + shard_packer.pack_array(8); + shard_packer.pack(std::string("not-a-uuid")); + shard_packer.pack(kDefaultTestPutStartTimeMs); + shard_packer.pack(kDefaultTestObjectSize); + shard_packer.pack(kDefaultTestLeaseTimeoutMs); + shard_packer.pack(false); + shard_packer.pack(uint64_t{0}); + shard_packer.pack(uint32_t{1}); + PackDiskReplica(shard_packer, kDefaultTestDiskFilePath, + kDefaultTestObjectSize); + + auto compressed_shard = + zstd_compress(reinterpret_cast(shard_buffer.data()), + shard_buffer.size(), 3); + + msgpack::sbuffer root_buffer; + MsgpackPacker root_packer(&root_buffer); + root_packer.pack_map(3); + root_packer.pack(std::string("shards")); + root_packer.pack_map(1); + root_packer.pack(shard_idx); + root_packer.pack_bin(compressed_shard.size()); + root_packer.pack_bin_body( + reinterpret_cast(compressed_shard.data()), + compressed_shard.size()); + root_packer.pack(std::string("discarded_replicas")); + root_packer.pack_array(0); + root_packer.pack(std::string("replica_next_id")); + root_packer.pack(uint64_t{10}); auto deserialize_result = - DeserializeMetadataForTest(BuildMetadataPayloadWithClientIdString( - "not-a-uuid", std::string_view(key.data(), key.size()))); + DeserializeMetadataForTest(ToByteVector(root_buffer)); ASSERT_TRUE(deserialize_result.has_value()) << deserialize_result.error().message;