From 6352ac33648d9b7daeee5a063a782163e17e7463 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20L=C3=B8nner=C3=B8d=20Madsen?= Date: Thu, 28 May 2026 20:05:02 +0200 Subject: [PATCH 1/2] test: cover array-valued custom properties Pin down that JSON arrays inside the product, user, license, and trial property bags survive validate_token and the memory/file store round-trip. The API in moonbase.js now allows array variants in NestedPropertyValue; these tests guard against a future stricter check silently breaking the contract here. --- tests/store_tests.cpp | 19 ++++++++-- tests/validator_tests.cpp | 74 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 3 deletions(-) diff --git a/tests/store_tests.cpp b/tests/store_tests.cpp index cf05220..9e90ea0 100644 --- a/tests/store_tests.cpp +++ b/tests/store_tests.cpp @@ -19,14 +19,18 @@ license sample_license() value.activation_id = "activation-123"; value.trial = false; value.method = activation_method::online; - value.licensed_product = product{"demo-app", "Demo Product", "1.2.3", {{"tier", "pro"}}}; - value.issued_to = user{"user-123", "Jane Developer", "jane@example.com", {{"company", "Acme"}}}; + value.licensed_product = product{ + "demo-app", "Demo Product", "1.2.3", + {{"tier", "pro"}, {"regions", {"us", "eu"}}}}; + value.issued_to = user{ + "user-123", "Jane Developer", "jane@example.com", + {{"company", "Acme"}, {"roles", {"admin", "ops"}}}}; value.issued_at = detail::parse_iso8601_utc("2026-05-08T12:00:00Z"); value.expires_at = detail::parse_iso8601_utc("2026-06-08T12:00:00Z"); value.validated_at = detail::parse_iso8601_utc("2026-05-08T12:30:00Z"); value.owned_sub_product_ids = {"demo-app-pro"}; value.subscription_id = "subscription-123"; - value.properties = {{"seats", 3}}; + value.properties = {{"seats", 3}, {"features", {"export", "sso"}}}; value.token = "jwt"; return value; } @@ -43,6 +47,11 @@ TEST_CASE("memory_license_store round-trips and deletes") REQUIRE(loaded.has_value()); CHECK(loaded->id == "license-123"); CHECK(loaded->licensed_product.properties.at("tier") == "pro"); + REQUIRE(loaded->issued_to.properties.at("roles").is_array()); + CHECK(loaded->issued_to.properties.at("roles").size() == 2); + CHECK(loaded->issued_to.properties.at("roles").at(0) == "admin"); + REQUIRE(loaded->properties.at("features").is_array()); + CHECK(loaded->properties.at("features").at(1) == "sso"); store.delete_local_license(); CHECK_FALSE(store.load_local_license().has_value()); @@ -62,6 +71,10 @@ TEST_CASE("file_license_store round-trips and deletes") CHECK(loaded->id == "license-123"); REQUIRE(loaded->expires_at.has_value()); CHECK(detail::format_iso8601_utc(*loaded->expires_at) == "2026-06-08T12:00:00Z"); + REQUIRE(loaded->licensed_product.properties.at("regions").is_array()); + CHECK(loaded->licensed_product.properties.at("regions").size() == 2); + REQUIRE(loaded->properties.at("features").is_array()); + CHECK(loaded->properties.at("features").at(0) == "export"); store.delete_local_license(); CHECK_FALSE(std::filesystem::exists(path)); diff --git a/tests/validator_tests.cpp b/tests/validator_tests.cpp index de51cf1..bee2209 100644 --- a/tests/validator_tests.cpp +++ b/tests/validator_tests.cpp @@ -124,6 +124,80 @@ TEST_CASE("trial tokens use trial properties") CHECK(result.properties.at("days") == 14); } +TEST_CASE("custom properties carry array values") +{ + // The API surface now allows array-valued custom properties (see the + // `array` variant added to NestedPropertyValue in moonbase.js). Those + // arrive in the JWT as plain JSON arrays inside each properties claim. + // Lock in that arrays — including arrays of objects — round-trip through + // validate_token for product, user, license, and trial property bags. + auto key = moonbase::tests::generate_key(); + + SUBCASE("non-trial license carries arrays in p/u/l properties") + { + auto claims = moonbase::tests::default_claims(); + claims["p:properties"] = { + {"tier", "pro"}, + {"regions", {"us", "eu", "ap"}}, + }; + claims["u:properties"] = { + {"company", "Acme"}, + {"roles", {"admin", "ops"}}, + }; + claims["l:properties"] = { + {"seats", 3}, + {"features", {"export", "sso"}}, + {"contacts", nlohmann::json::array({ + {{"name", "Jane"}, {"email", "jane@example.com"}}, + {{"name", "John"}, {"email", "john@example.com"}}, + })}, + }; + + const auto result = make_validator(key.public_pem).validate_token( + moonbase::tests::make_token(key.key.get(), claims)); + + const auto& regions = result.licensed_product.properties.at("regions"); + REQUIRE(regions.is_array()); + REQUIRE(regions.size() == 3); + CHECK(regions.at(0) == "us"); + CHECK(regions.at(2) == "ap"); + + const auto& roles = result.issued_to.properties.at("roles"); + REQUIRE(roles.is_array()); + REQUIRE(roles.size() == 2); + CHECK(roles.at(0) == "admin"); + + const auto& features = result.properties.at("features"); + REQUIRE(features.is_array()); + CHECK(features.size() == 2); + + const auto& contacts = result.properties.at("contacts"); + REQUIRE(contacts.is_array()); + REQUIRE(contacts.size() == 2); + CHECK(contacts.at(1).at("email") == "john@example.com"); + } + + SUBCASE("trial license carries arrays in t:properties") + { + auto claims = moonbase::tests::default_claims(); + claims["trial"] = "true"; + claims.erase("l:properties"); + claims["t:properties"] = { + {"days", 14}, + {"allowed_hosts", {"localhost", "demo.example.com"}}, + }; + + const auto result = make_validator(key.public_pem).validate_token( + moonbase::tests::make_token(key.key.get(), claims)); + + CHECK(result.trial); + const auto& hosts = result.properties.at("allowed_hosts"); + REQUIRE(hosts.is_array()); + REQUIRE(hosts.size() == 2); + CHECK(hosts.at(1) == "demo.example.com"); + } +} + TEST_CASE("invalid JWTs are rejected") { auto key = moonbase::tests::generate_key(); From 314f7b8f937d64d3110bb15cc05134af6c459881 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20L=C3=B8nner=C3=B8d=20Madsen?= Date: Thu, 28 May 2026 20:21:20 +0200 Subject: [PATCH 2/2] fix: stop racing _chsize_s against the Windows byte lock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Windows, file_lock's constructor called _chsize_s before _locking. With multiple sibling acquirers — exactly the scenario the file_license_store lock_for_update tests exercise — a later thread's SetEndOfFile would race against the byte-range lock held by the first acquirer and fail with EACCES, throwing storage_error out of the worker lambda and aborting the test process with SIGABRT. _locking already accepts ranges past EOF per MSDN, so drop the _chsize_s/_lseek prelude entirely; the open + lock are sufficient. Also wrap the file-lock test workers in a top-level try/catch that bumps an atomic counter, so any future lock-acquisition failure surfaces as a CHECK on the main thread instead of a process abort. --- include/moonbase/detail/file_lock.hpp | 25 ++----- tests/store_tests.cpp | 100 +++++++++++++++----------- 2 files changed, 62 insertions(+), 63 deletions(-) diff --git a/include/moonbase/detail/file_lock.hpp b/include/moonbase/detail/file_lock.hpp index fc99e62..44761b6 100644 --- a/include/moonbase/detail/file_lock.hpp +++ b/include/moonbase/detail/file_lock.hpp @@ -37,8 +37,11 @@ namespace moonbase::detail { // observe each other. // Windows: CRT _locking(_LK_LOCK) on a single-byte range. The range is // required by the API but otherwise arbitrary; all callers using this class -// lock the same range. This intentionally avoids in public SDK -// headers because store.hpp includes this detail header. +// lock the same range. _locking accepts ranges past EOF, so we deliberately +// do not _chsize_s the file — SetEndOfFile on a sibling handle would race +// against the byte lock held by an earlier acquirer and surface as EACCES. +// This intentionally avoids in public SDK headers because +// store.hpp includes this detail header. // // Throws moonbase::storage_error on unrecoverable open/lock failures. class file_lock { @@ -69,24 +72,6 @@ class file_lock { } fd_ = fd; - const auto size_error = ::_chsize_s(fd_, lock_range_size_); - if (size_error != 0) { - ::_close(fd_); - fd_ = -1; - throw storage_error( - "Could not prepare license lock file: " - + std::string(std::strerror(size_error))); - } - - if (::_lseek(fd_, 0, SEEK_SET) < 0) { - const auto err = errno; - ::_close(fd_); - fd_ = -1; - throw storage_error( - "Could not seek license lock file: " - + std::string(std::strerror(err))); - } - if (::_locking(fd_, _LK_LOCK, lock_range_size_) != 0) { const auto err = errno; ::_close(fd_); diff --git a/tests/store_tests.cpp b/tests/store_tests.cpp index 9e90ea0..6f713ca 100644 --- a/tests/store_tests.cpp +++ b/tests/store_tests.cpp @@ -95,25 +95,31 @@ TEST_CASE("file_license_store::lock_for_update serializes concurrent acquirers") std::atomic max_inside{0}; std::atomic ready{0}; std::atomic go{false}; + std::atomic worker_exceptions{0}; auto runner = [&] { - file_license_store store(path); - ready.fetch_add(1); - while (!go.load(std::memory_order_acquire)) { - std::this_thread::yield(); - } - auto guard = store.lock_for_update(); - REQUIRE(guard != nullptr); - - const int now_inside = inside.fetch_add(1, std::memory_order_acq_rel) + 1; - int prev = max_inside.load(std::memory_order_relaxed); - while (now_inside > prev - && !max_inside.compare_exchange_weak(prev, now_inside, - std::memory_order_acq_rel)) { + try { + file_license_store store(path); + ready.fetch_add(1); + while (!go.load(std::memory_order_acquire)) { + std::this_thread::yield(); + } + auto guard = store.lock_for_update(); + REQUIRE(guard != nullptr); + + const int now_inside = inside.fetch_add(1, std::memory_order_acq_rel) + 1; + int prev = max_inside.load(std::memory_order_relaxed); + while (now_inside > prev + && !max_inside.compare_exchange_weak(prev, now_inside, + std::memory_order_acq_rel)) { + } + + std::this_thread::sleep_for(std::chrono::milliseconds(40)); + inside.fetch_sub(1, std::memory_order_acq_rel); + } catch (...) { + worker_exceptions.fetch_add(1, std::memory_order_acq_rel); + ready.fetch_add(1); } - - std::this_thread::sleep_for(std::chrono::milliseconds(40)); - inside.fetch_sub(1, std::memory_order_acq_rel); }; std::vector threads; @@ -127,6 +133,7 @@ TEST_CASE("file_license_store::lock_for_update serializes concurrent acquirers") go.store(true, std::memory_order_release); for (auto& t : threads) t.join(); + CHECK(worker_exceptions.load() == 0); CHECK(max_inside.load() == 1); std::error_code ec; @@ -160,37 +167,43 @@ TEST_CASE("file_license_store::lock_for_update survives delete_local_license") std::atomic max_inside{0}; std::atomic ready{0}; std::atomic go{false}; + std::atomic worker_exceptions{0}; auto runner = [&] { - file_license_store store(path); - ready.fetch_add(1); - while (!go.load(std::memory_order_acquire)) { - std::this_thread::yield(); - } - auto guard = store.lock_for_update(); - REQUIRE(guard != nullptr); - - const int now_inside = inside.fetch_add(1, std::memory_order_acq_rel) + 1; - int prev = max_inside.load(std::memory_order_relaxed); - while (now_inside > prev - && !max_inside.compare_exchange_weak(prev, now_inside, - std::memory_order_acq_rel)) { - } - - // Mutate the license file while holding the lock — alternating - // delete and rewrite hammers exactly the scenario where the old - // (license-on-itself) lock would have failed. - try { - store.delete_local_license(); - } catch (const storage_error&) { - } - std::this_thread::sleep_for(std::chrono::milliseconds(20)); try { - store.store_local_license(sample_license()); - } catch (const storage_error&) { + file_license_store store(path); + ready.fetch_add(1); + while (!go.load(std::memory_order_acquire)) { + std::this_thread::yield(); + } + auto guard = store.lock_for_update(); + REQUIRE(guard != nullptr); + + const int now_inside = inside.fetch_add(1, std::memory_order_acq_rel) + 1; + int prev = max_inside.load(std::memory_order_relaxed); + while (now_inside > prev + && !max_inside.compare_exchange_weak(prev, now_inside, + std::memory_order_acq_rel)) { + } + + // Mutate the license file while holding the lock — alternating + // delete and rewrite hammers exactly the scenario where the old + // (license-on-itself) lock would have failed. + try { + store.delete_local_license(); + } catch (const storage_error&) { + } + std::this_thread::sleep_for(std::chrono::milliseconds(20)); + try { + store.store_local_license(sample_license()); + } catch (const storage_error&) { + } + + inside.fetch_sub(1, std::memory_order_acq_rel); + } catch (...) { + worker_exceptions.fetch_add(1, std::memory_order_acq_rel); + ready.fetch_add(1); } - - inside.fetch_sub(1, std::memory_order_acq_rel); }; std::vector threads; @@ -204,6 +217,7 @@ TEST_CASE("file_license_store::lock_for_update survives delete_local_license") go.store(true, std::memory_order_release); for (auto& t : threads) t.join(); + CHECK(worker_exceptions.load() == 0); CHECK(max_inside.load() == 1); std::error_code ec;