From dde485f0b777a62d65d817906a8e05caf2d18bc3 Mon Sep 17 00:00:00 2001 From: Stefan Boberg Date: Mon, 20 Apr 2026 10:00:10 +0200 Subject: consolidate cache commands into `cache` subcommand (#978) Consolidate the scattered cache-related top-level commands into a single `zen cache ` command tree, keeping the old names as hidden deprecated aliases so any existing scripts keep working. ## Motivation `zen` has accumulated a flat list of cache-adjacent commands (`cache-info`, `cache-stats`, `cache-details`, `cache-gen`, `cache-get`, `drop`, `rpc-record-start/stop`, `rpc-record-replay`). Each one re-declares `--hosturl` parsing and host resolution, and there is no natural home for new cache tooling. Grouping them under `cache` gives a consistent UX and a shared base class to hang common options off of. ## Changes ### Subcommand consolidation - Moved into `cache ` form: - `cache info`, `cache stats`, `cache details`, `cache gen`, `cache get`, `cache drop` - `cache record ` / `cache record stop` (formerly `rpc-record-start` / `rpc-record-stop`) - `cache replay` (formerly `rpc-record-replay`) - All old top-level names remain as deprecated aliases and forward through a shared legacy-shim dispatcher that rewrites `argv` and re-enters the new dispatcher, so behavior is byte-identical for existing callers. - Deprecated aliases are now hidden from the top-level `zen --help` listing (new `ZenCmdBase::IsHidden()` + `DeprecatedCacheStoreCommand` base). They still dispatch normally; `zen cache --help` is the canonical discovery surface. ### Shared base class - New `CacheSubCmdBase` owns the `--hosturl` option and `ResolveHost()` logic, eliminating the copy/pasted block at the top of every `Run()`. ### Output format - Added `--yaml` to `cache info`, `cache stats`, and `cache details` (negotiated server-side via `Accept: text/yaml`). `cache details` now rejects `--csv --yaml` combined. ### Hardening - `cache gen`: bounds-check requested sizes before allocating. - `cache replay`: validate `--stride` / `--offset` and fix progress-math overflow edge cases. --- src/zen/cmds/cache_cmd.cpp | 840 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 651 insertions(+), 189 deletions(-) (limited to 'src/zen/cmds/cache_cmd.cpp') diff --git a/src/zen/cmds/cache_cmd.cpp b/src/zen/cmds/cache_cmd.cpp index a8c15f119..f024ea57e 100644 --- a/src/zen/cmds/cache_cmd.cpp +++ b/src/zen/cmds/cache_cmd.cpp @@ -8,19 +8,32 @@ #include #include #include +#include #include +#include +#include #include +#include #include +#include #include #include #include #include +#include + +ZEN_THIRD_PARTY_INCLUDES_START +#include +#include +ZEN_THIRD_PARTY_INCLUDES_END #include #include namespace zen { +using namespace std::literals; + namespace { IoBuffer CreateRandomBlob(uint64_t Size) { @@ -56,37 +69,110 @@ namespace { } } // namespace -DropCommand::DropCommand() +//////////////////////////////////////////////////////////////////////////////// +// CacheCommand + +CacheCommand::CacheCommand() { m_Options.add_options()("h,help", "Print help"); - m_Options.add_option("", "u", "hosturl", kHostUrlHelp, cxxopts::value(m_HostName)->default_value(""), ""); - m_Options.add_option("", "n", "namespace", "Namespace name", cxxopts::value(m_NamespaceName), ""); - m_Options.add_option("", "b", "bucket", "Bucket name", cxxopts::value(m_BucketName), ""); - m_Options.parse_positional({"namespace", "bucket"}); + + AddSubCommand(m_DetailsSubCmd); + AddSubCommand(m_DropSubCmd); + AddSubCommand(m_GenSubCmd); + AddSubCommand(m_GetSubCmd); + AddSubCommand(m_InfoSubCmd); + AddSubCommand(m_RecordSubCmd); + AddSubCommand(m_ReplaySubCmd); + AddSubCommand(m_StatsSubCmd); } -DropCommand::~DropCommand() = default; +CacheCommand::~CacheCommand() = default; -void -DropCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv) +//////////////////////////////////////////////////////////////////////////////// +// CacheSubCmdBase + +CacheSubCmdBase::CacheSubCmdBase(std::string_view Name, std::string_view Description) : ZenSubCmdBase(Name, Description) { - ZEN_UNUSED(GlobalOptions); + m_SubOptions.add_option("", "u", "hosturl", ZenCmdBase::kHostUrlHelp, cxxopts::value(m_HostName)->default_value(""), ""); +} - if (!ParseOptions(argc, argv)) +void +CacheSubCmdBase::ResolveHost() +{ + m_HostName = ZenCmdBase::ResolveTargetHostSpec(m_HostName); + if (m_HostName.empty()) { - return; + throw OptionParseException("Unable to resolve server specification", m_SubOptions.help()); } +} - m_HostName = ResolveTargetHostSpec(m_HostName); +//////////////////////////////////////////////////////////////////////////////// +// Legacy shim dispatcher - if (m_HostName.empty()) +namespace cache_legacy_shim { + static void Dispatch(std::span Injected, const ZenCliOptions& GlobalOptions, int argc, char** argv) + { + // cxxopts treats argv as writable char** in the style of C main(argv). + // Stage the injected tokens in writable std::string storage so we never + // hand out pointers to string literals. + std::vector Storage; + Storage.reserve(Injected.size()); + for (std::string_view Token : Injected) + { + Storage.emplace_back(Token); + } + + std::vector NewArgv; + NewArgv.reserve(static_cast(argc) + Storage.size()); + NewArgv.push_back(argv[0]); + for (std::string& Token : Storage) + { + NewArgv.push_back(Token.data()); + } + for (int i = 1; i < argc; ++i) + { + NewArgv.push_back(argv[i]); + } + + CacheCommand Impl; + Impl.Run(GlobalOptions, static_cast(NewArgv.size()), NewArgv.data()); + } + + void RunAs(const char* SubCommandName, const ZenCliOptions& GlobalOptions, int argc, char** argv) { - throw OptionParseException("Unable to resolve server specification", m_Options.help()); + const std::string_view Tokens[] = {std::string_view(SubCommandName)}; + Dispatch(Tokens, GlobalOptions, argc, argv); } +} // namespace cache_legacy_shim + +// RpcStopRecordingCommand is unique among legacy shims in that it needs to +// inject two tokens ("record" and "stop") rather than a single subcommand name. +void +RpcStopRecordingCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv) +{ + using namespace std::literals; + const std::string_view Tokens[] = {"record"sv, "stop"sv}; + cache_legacy_shim::Dispatch(Tokens, GlobalOptions, argc, argv); +} + +//////////////////////////////////////////////////////////////////////////////// +// CacheDropSubCmd + +CacheDropSubCmd::CacheDropSubCmd() : CacheSubCmdBase("drop", "Drop cache namespace or bucket") +{ + m_SubOptions.add_option("", "n", "namespace", "Namespace name", cxxopts::value(m_NamespaceName), ""); + m_SubOptions.add_option("", "b", "bucket", "Bucket name", cxxopts::value(m_BucketName), ""); + m_SubOptions.parse_positional({"namespace", "bucket"}); +} + +void +CacheDropSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) +{ + ResolveHost(); if (m_NamespaceName.empty()) { - throw OptionParseException("'--namespace' is required", m_Options.help()); + throw OptionParseException("'--namespace' is required", m_SubOptions.help()); } std::string Url; @@ -105,7 +191,7 @@ DropCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv) ZEN_CONSOLE("Dropping {}", DropDescription); - HttpClient Http = CreateHttpClient(m_HostName); + HttpClient Http = CacheCommand::CreateHttpClient(m_HostName); if (HttpClient::Response Response = Http.Delete(Url)) { ZEN_CONSOLE("{}", Response.ToText()); @@ -116,52 +202,39 @@ DropCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv) } } -CacheInfoCommand::CacheInfoCommand() +//////////////////////////////////////////////////////////////////////////////// +// CacheInfoSubCmd + +CacheInfoSubCmd::CacheInfoSubCmd() : CacheSubCmdBase("info", "Info on cache, namespace or bucket") { - m_Options.add_options()("h,help", "Print help"); - m_Options.add_option("", "u", "hosturl", kHostUrlHelp, cxxopts::value(m_HostName)->default_value(""), ""); - m_Options.add_option("", "n", "namespace", "Namespace name", cxxopts::value(m_NamespaceName), ""); - m_Options.add_option("", - "", - "bucketsizes", - "Comma delimited list of bucket names to get size info from, * to get info on all buckets", - cxxopts::value(m_SizeInfoBucketNames), - ""); - m_Options.add_option("", "b", "bucket", "Bucket name", cxxopts::value(m_BucketName), ""); - m_Options.add_option("", "", "bucketsize", "Show detailed bucket size info", cxxopts::value(m_BucketSizeInfo), ""); - - m_Options.parse_positional({"namespace", "bucket"}); + m_SubOptions.add_option("", "n", "namespace", "Namespace name", cxxopts::value(m_NamespaceName), ""); + m_SubOptions.add_option("", + "", + "bucketsizes", + "Comma delimited list of bucket names to get size info from, * to get info on all buckets", + cxxopts::value(m_SizeInfoBucketNames), + ""); + m_SubOptions.add_option("", "b", "bucket", "Bucket name", cxxopts::value(m_BucketName), ""); + m_SubOptions.add_option("", "", "bucketsize", "Show detailed bucket size info", cxxopts::value(m_BucketSizeInfo), ""); + m_SubOptions.add_option("", "y", "yaml", "Output as YAML instead of JSON", cxxopts::value(m_YAML), ""); + m_SubOptions.parse_positional({"namespace", "bucket"}); } -CacheInfoCommand::~CacheInfoCommand() = default; - void -CacheInfoCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv) +CacheInfoSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) { - ZEN_UNUSED(GlobalOptions); - - if (!ParseOptions(argc, argv)) - { - return; - } - - m_HostName = ResolveTargetHostSpec(m_HostName); - - if (m_HostName.empty()) - { - throw OptionParseException("Unable to resolve server specification", m_Options.help()); - } + ResolveHost(); std::string Url; - if (m_HostName.empty()) + if (m_NamespaceName.empty()) { if (!m_SizeInfoBucketNames.empty()) { - throw OptionParseException("'--bucketsizes' requires '--namespace'", m_Options.help()); + throw OptionParseException("'--bucketsizes' requires '--namespace'", m_SubOptions.help()); } if (m_BucketSizeInfo) { - throw OptionParseException("'--bucketsize' requires '--namespace' and '--bucket'", m_Options.help()); + throw OptionParseException("'--bucketsize' requires '--namespace' and '--bucket'", m_SubOptions.help()); } ZEN_CONSOLE("Info on cache from '{}'", m_HostName); Url = "/z$"; @@ -171,7 +244,7 @@ CacheInfoCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv) if (m_BucketSizeInfo) { throw OptionParseException(fmt::format("'--bucketsize' requires '--namespace' and '--bucket' ('{}')", m_BucketName), - m_Options.help()); + m_SubOptions.help()); } ZEN_CONSOLE("Info on cache namespace '{}' from '{}'", m_NamespaceName, m_HostName); Url = fmt::format("/z$/{}", m_NamespaceName); @@ -180,7 +253,7 @@ CacheInfoCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv) { if (!m_SizeInfoBucketNames.empty()) { - throw OptionParseException("'--bucketsizes' conflicts with '--bucket'", m_Options.help()); + throw OptionParseException("'--bucketsizes' conflicts with '--bucket'", m_SubOptions.help()); } ZEN_CONSOLE("Info on cache bucket '{}/{}' from '{}'", m_NamespaceName, m_BucketName, m_HostName); Url = fmt::format("/z$/{}/{}", m_NamespaceName, m_BucketName); @@ -196,8 +269,10 @@ CacheInfoCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv) Parameters.Entries.insert({"bucketsize", "true"}); } - HttpClient Http = CreateHttpClient(m_HostName); - if (HttpClient::Response Response = Http.Get(Url, HttpClient::Accept(ZenContentType::kJSON), Parameters)) + const ZenContentType AcceptType = m_YAML ? ZenContentType::kYAML : ZenContentType::kJSON; + + HttpClient Http = CacheCommand::CreateHttpClient(m_HostName); + if (HttpClient::Response Response = Http.Get(Url, HttpClient::Accept(AcceptType), Parameters)) { ZEN_CONSOLE("{}", Response.ToText()); } @@ -207,76 +282,59 @@ CacheInfoCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv) } } -CacheStatsCommand::CacheStatsCommand() +//////////////////////////////////////////////////////////////////////////////// +// CacheStatsSubCmd + +CacheStatsSubCmd::CacheStatsSubCmd() : CacheSubCmdBase("stats", "Stats on cache") { - m_Options.add_options()("h,help", "Print help"); - m_Options.add_option("", "u", "hosturl", kHostUrlHelp, cxxopts::value(m_HostName)->default_value(""), ""); + m_SubOptions.add_option("", "y", "yaml", "Output as YAML instead of JSON", cxxopts::value(m_YAML), ""); } -CacheStatsCommand::~CacheStatsCommand() = default; - void -CacheStatsCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv) +CacheStatsSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) { - ZEN_UNUSED(GlobalOptions); - - if (!ParseOptions(argc, argv)) - { - return; - } + ResolveHost(); - m_HostName = ResolveTargetHostSpec(m_HostName); + const ZenContentType AcceptType = m_YAML ? ZenContentType::kYAML : ZenContentType::kJSON; - if (m_HostName.empty()) - { - throw OptionParseException("Unable to resolve server specification", m_Options.help()); - } - - HttpClient Http = CreateHttpClient(m_HostName); - if (HttpClient::Response Response = Http.Get("/stats/z$", HttpClient::Accept(ZenContentType::kJSON))) + HttpClient Http = CacheCommand::CreateHttpClient(m_HostName); + if (HttpClient::Response Response = Http.Get("/stats/z$", HttpClient::Accept(AcceptType))) { ZEN_CONSOLE("{}", Response.ToText()); } else { - Response.ThrowError("Info failed"); + Response.ThrowError("Stats failed"); } } -CacheDetailsCommand::CacheDetailsCommand() +//////////////////////////////////////////////////////////////////////////////// +// CacheDetailsSubCmd + +CacheDetailsSubCmd::CacheDetailsSubCmd() : CacheSubCmdBase("details", "Details on cache") { - m_Options.add_options()("h,help", "Print help"); - m_Options.add_option("", "u", "hosturl", kHostUrlHelp, cxxopts::value(m_HostName)->default_value(""), ""); - m_Options.add_option("", "c", "csv", "Info on csv format", cxxopts::value(m_CSV), ""); - m_Options.add_option("", "d", "details", "Get detailed information about records", cxxopts::value(m_Details), "
"); - m_Options.add_option("", - "a", - "attachmentdetails", - "Get detailed information about attachments", - cxxopts::value(m_AttachmentDetails), - ""); - m_Options.add_option("", "n", "namespace", "Namespace name to get info for", cxxopts::value(m_Namespace), ""); - m_Options.add_option("", "b", "bucket", "Filter on bucket name", cxxopts::value(m_Bucket), ""); - m_Options.add_option("", "v", "valuekey", "Filter on value key hash string", cxxopts::value(m_ValueKey), ""); + m_SubOptions.add_option("", "c", "csv", "Output as CSV instead of JSON", cxxopts::value(m_CSV), ""); + m_SubOptions.add_option("", "y", "yaml", "Output as YAML instead of JSON", cxxopts::value(m_YAML), ""); + m_SubOptions.add_option("", "d", "details", "Get detailed information about records", cxxopts::value(m_Details), "
"); + m_SubOptions.add_option("", + "a", + "attachmentdetails", + "Get detailed information about attachments", + cxxopts::value(m_AttachmentDetails), + ""); + m_SubOptions.add_option("", "n", "namespace", "Namespace name to get info for", cxxopts::value(m_Namespace), ""); + m_SubOptions.add_option("", "b", "bucket", "Filter on bucket name", cxxopts::value(m_Bucket), ""); + m_SubOptions.add_option("", "v", "valuekey", "Filter on value key hash string", cxxopts::value(m_ValueKey), ""); } -CacheDetailsCommand::~CacheDetailsCommand() = default; - void -CacheDetailsCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv) +CacheDetailsSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) { - ZEN_UNUSED(GlobalOptions); - - if (!ParseOptions(argc, argv)) - { - return; - } + ResolveHost(); - m_HostName = ResolveTargetHostSpec(m_HostName); - - if (m_HostName.empty()) + if (m_CSV && m_YAML) { - throw OptionParseException("Unable to resolve server specification", m_Options.help()); + throw OptionParseException("'--csv' conflicts with '--yaml'", m_SubOptions.help()); } HttpClient::KeyValueMap Parameters; @@ -296,7 +354,7 @@ CacheDetailsCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** ar } else { - Headers = HttpClient::Accept(ZenContentType::kJSON); + Headers = HttpClient::Accept(m_YAML ? ZenContentType::kYAML : ZenContentType::kJSON); } std::string Url; @@ -304,11 +362,11 @@ CacheDetailsCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** ar { if (m_Namespace.empty()) { - throw OptionParseException("'--namespace' is required", m_Options.help()); + throw OptionParseException("'--namespace' is required", m_SubOptions.help()); } if (m_Bucket.empty()) { - throw OptionParseException("'--bucket' is required", m_Options.help()); + throw OptionParseException("'--bucket' is required", m_SubOptions.help()); } Url = fmt::format("/z$/details$/{}/{}/{}", m_Namespace, m_Bucket, m_ValueKey); } @@ -316,7 +374,7 @@ CacheDetailsCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** ar { if (m_Namespace.empty()) { - throw OptionParseException("'--namespace' is required", m_Options.help()); + throw OptionParseException("'--namespace' is required", m_SubOptions.help()); } Url = fmt::format("/z$/details$/{}/{}", m_Namespace, m_Bucket); } @@ -329,61 +387,48 @@ CacheDetailsCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** ar Url = "/z$/details$"; } - HttpClient Http = CreateHttpClient(m_HostName); + HttpClient Http = CacheCommand::CreateHttpClient(m_HostName); if (HttpClient::Response Response = Http.Get(Url, Headers, Parameters)) { ZEN_CONSOLE("{}", Response.ToText()); } else { - Response.ThrowError("Info failed"); + Response.ThrowError("Details failed"); } } -CacheGenerateCommand::CacheGenerateCommand() +//////////////////////////////////////////////////////////////////////////////// +// CacheGenSubCmd + +CacheGenSubCmd::CacheGenSubCmd() : CacheSubCmdBase("gen", "Generates cache values into a bucket") { - m_Options.add_options()("h,help", "Print help"); - m_Options.add_option("", "u", "hosturl", kHostUrlHelp, cxxopts::value(m_HostName)->default_value(""), ""); - m_Options + m_SubOptions .add_option("", "n", "namespace", "Namespace to generate cache values/records for", cxxopts::value(m_Namespace), ""); - m_Options.add_option("", "b", "bucket", "Bucket name to generate cache values/records for", cxxopts::value(m_Bucket), ""); - m_Options.add_option("", "", "count", "Number of cache values/records to generate", cxxopts::value(m_Count), ""); - m_Options.add_option("", "", "min-size", "Minimum size of cache value/attachments", cxxopts::value(m_MinSize), ""); - m_Options.add_option("", "", "max-size", "Maximum size of cache value/attachments", cxxopts::value(m_MaxSize), ""); - m_Options.add_option("", - "", - "min-attachments", - "Minimum number of attachments when creating record based values", - cxxopts::value(m_MinAttachmentCount), - ""); - m_Options.add_option("", - "", - "max-attachments", - "Minimum number of attachments when creating record based values, 0 to only create cache values", - cxxopts::value(m_MaxAttachmentCount), - ""); - m_Options.parse_positional({"namespace", "bucket", "count"}); - m_Options.positional_help("namespace bucket count"); + m_SubOptions.add_option("", "b", "bucket", "Bucket name to generate cache values/records for", cxxopts::value(m_Bucket), ""); + m_SubOptions.add_option("", "", "count", "Number of cache values/records to generate", cxxopts::value(m_Count), ""); + m_SubOptions.add_option("", "", "min-size", "Minimum size of cache value/attachments", cxxopts::value(m_MinSize), ""); + m_SubOptions.add_option("", "", "max-size", "Maximum size of cache value/attachments", cxxopts::value(m_MaxSize), ""); + m_SubOptions.add_option("", + "", + "min-attachments", + "Minimum number of attachments when creating record based values", + cxxopts::value(m_MinAttachmentCount), + ""); + m_SubOptions.add_option("", + "", + "max-attachments", + "Minimum number of attachments when creating record based values, 0 to only create cache values", + cxxopts::value(m_MaxAttachmentCount), + ""); + m_SubOptions.parse_positional({"namespace", "bucket", "count"}); + m_SubOptions.positional_help("namespace bucket count"); } -CacheGenerateCommand::~CacheGenerateCommand() = default; - void -CacheGenerateCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv) +CacheGenSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) { - ZEN_UNUSED(GlobalOptions); - - if (!ParseOptions(argc, argv)) - { - return; - } - - m_HostName = ResolveTargetHostSpec(m_HostName); - - if (m_HostName.empty()) - { - throw OptionParseException("Unable to resolve server specification", m_Options.help()); - } + ResolveHost(); if (m_MaxSize == 0 && m_MinSize == 0) { @@ -400,6 +445,16 @@ CacheGenerateCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** a } } + // The size-range expansion below requires MinSize >= 1 (it uses + // `MinSize - 1` as a uniform distribution upper bound, which would + // underflow on an unsigned zero) and MaxSize >= MinSize. + if (m_MinSize == 0 || m_MaxSize < m_MinSize) + { + throw OptionParseException( + fmt::format("'--min-size' ({}) must be >= 1 and '--max-size' ({}) must be >= '--min-size'", m_MinSize, m_MaxSize), + m_SubOptions.help()); + } + std::vector> Variations; std::vector SizeRanges; SizeRanges.push_back(m_MinSize); @@ -431,7 +486,7 @@ CacheGenerateCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** a std::uniform_int_distribution KeyDistribution; - HttpClient Http = CreateHttpClient(m_HostName); + HttpClient Http = CacheCommand::CreateHttpClient(m_HostName); auto GeneratePutCacheValueRequest( [this, &KeyDistribution, &Generator](std::span BatchSizes, uint64_t RequestIndex) -> CbPackage { @@ -583,68 +638,52 @@ CacheGenerateCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** a } } -CacheGetCommand::CacheGetCommand() +//////////////////////////////////////////////////////////////////////////////// +// CacheGetSubCmd + +CacheGetSubCmd::CacheGetSubCmd() : CacheSubCmdBase("get", "Get cache values/records or attachments from a bucket") { - m_Options.add_options()("h,help", "Print help"); - m_Options.add_option("", "u", "hosturl", kHostUrlHelp, cxxopts::value(m_HostName)->default_value(""), ""); - m_Options - .add_option("", "n", "namespace", "Namespace to generate cache values/records for", cxxopts::value(m_Namespace), ""); - m_Options.add_option("", "b", "bucket", "Bucket name to generate cache values/records for", cxxopts::value(m_Bucket), ""); - m_Options.add_option("", "v", "valuekey", "Cache entry iohash id", cxxopts::value(m_ValueKey), ""); - m_Options.add_option("", - "a", - "attachmenthash", - "For a cache entry record, get a particular attachment based on the 'RawHash'", - cxxopts::value(m_AttachmentHash), - ""); - m_Options.add_option("", "o", "output-path", "File path for output data", cxxopts::value(m_OutputPath), ""); - m_Options.add_option("", "t", "text", "Ouput content of cache entry record as text", cxxopts::value(m_AsText), ""); - m_Options + m_SubOptions.add_option("", "n", "namespace", "Namespace of the cache entry", cxxopts::value(m_Namespace), ""); + m_SubOptions.add_option("", "b", "bucket", "Bucket of the cache entry", cxxopts::value(m_Bucket), ""); + m_SubOptions.add_option("", "v", "valuekey", "Cache entry iohash id", cxxopts::value(m_ValueKey), ""); + m_SubOptions.add_option("", + "a", + "attachmenthash", + "For a cache entry record, get a particular attachment based on the 'RawHash'", + cxxopts::value(m_AttachmentHash), + ""); + m_SubOptions.add_option("", "o", "output-path", "File path for output data", cxxopts::value(m_OutputPath), ""); + m_SubOptions.add_option("", "t", "text", "Output content of cache entry record as text", cxxopts::value(m_AsText), ""); + m_SubOptions .add_option("", "d", "decompress", "Decompress data when applicable. Default = true", cxxopts::value(m_Decompress), ""); - m_Options.parse_positional({"namespace", "bucket", "valuekey", "attachmenthash"}); - m_Options.positional_help("namespace bucket valuekey attachmenthash"); + m_SubOptions.parse_positional({"namespace", "bucket", "valuekey", "attachmenthash"}); + m_SubOptions.positional_help("namespace bucket valuekey attachmenthash"); } -CacheGetCommand::~CacheGetCommand() = default; - void -CacheGetCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv) +CacheGetSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) { - ZEN_UNUSED(GlobalOptions); - - using namespace std::literals; - - if (!ParseOptions(argc, argv)) - { - return; - } - - m_HostName = ResolveTargetHostSpec(m_HostName); - - if (m_HostName.empty()) - { - throw OptionParseException("Unable to resolve server specification", m_Options.help()); - } + ResolveHost(); if (m_Namespace.empty()) { - throw OptionParseException("'--namespace' is required", m_Options.help()); + throw OptionParseException("'--namespace' is required", m_SubOptions.help()); } if (m_Bucket.empty()) { - throw OptionParseException("'--bucket' is required", m_Options.help()); + throw OptionParseException("'--bucket' is required", m_SubOptions.help()); } if (m_ValueKey.empty()) { - throw OptionParseException("'--valuekey' is required", m_Options.help()); + throw OptionParseException("'--valuekey' is required", m_SubOptions.help()); } IoHash ValueId; if (!IoHash::TryParse(m_ValueKey, ValueId)) { - throw OptionParseException(fmt::format("'--value-key' ('{}') is malformed", m_ValueKey), m_Options.help()); + throw OptionParseException(fmt::format("'--valuekey' ('{}') is malformed", m_ValueKey), m_SubOptions.help()); } IoHash AttachmentHash; @@ -652,11 +691,11 @@ CacheGetCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv) { if (!IoHash::TryParse(m_AttachmentHash, AttachmentHash)) { - throw OptionParseException(fmt::format("'--attachmenthash' ('{}') is malformed", m_AttachmentHash), m_Options.help()); + throw OptionParseException(fmt::format("'--attachmenthash' ('{}') is malformed", m_AttachmentHash), m_SubOptions.help()); } } - HttpClient Http = CreateHttpClient(m_HostName); + HttpClient Http = CacheCommand::CreateHttpClient(m_HostName); if (!m_OutputPath.empty()) { @@ -674,7 +713,7 @@ CacheGetCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv) m_OutputPath = (m_AttachmentHash.empty() ? m_ValueKey : m_AttachmentHash); } - std::string Url = fmt::format("/z$/{}/{}/{}", m_Namespace, m_Bucket, m_ValueKey); + std::string Url = fmt::format("/z$/{}/{}/{}", m_Namespace, m_Bucket, ValueId); if (AttachmentHash != IoHash::Zero) { Url = fmt::format("{}/{}", Url, AttachmentHash); @@ -720,4 +759,427 @@ CacheGetCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv) } } +//////////////////////////////////////////////////////////////////////////////// +// CacheRecordSubCmd + +CacheRecordSubCmd::CacheRecordSubCmd() +: CacheSubCmdBase("record", "Start recording cache rpc requests ('cache record '), or stop ('cache record stop')") +{ + m_SubOptions.add_option("", "p", "path", "Recording file path, or 'stop' to stop recording", cxxopts::value(m_Path), ""); + m_SubOptions.parse_positional("path"); + m_SubOptions.positional_help("|stop"); +} + +void +CacheRecordSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) +{ + ResolveHost(); + + HttpClient Http = CacheCommand::CreateHttpClient(m_HostName); + + if (m_Path == "stop") + { + if (HttpClient::Response Response = Http.Post("/z$/exec$/stop-recording"sv)) + { + ZEN_CONSOLE("{}", Response.ToText()); + } + else + { + Response.ThrowError("Failed to stop recording"); + } + return; + } + + if (m_Path.empty()) + { + throw OptionParseException("recording path is required (use '' to start, 'stop' to stop)", m_SubOptions.help()); + } + + if (HttpClient::Response Response = + Http.Post("/z$/exec$/start-recording"sv, HttpClient::KeyValueMap{}, HttpClient::KeyValueMap({{"path", m_Path}}))) + { + ZEN_CONSOLE("{}", Response.ToText()); + } + else + { + Response.ThrowError("Failed to start recording"); + } +} + +//////////////////////////////////////////////////////////////////////////////// +// CacheReplaySubCmd + +CacheReplaySubCmd::CacheReplaySubCmd() : CacheSubCmdBase("replay", "Replays a previously recorded session of rpc requests") +{ + m_SubOptions.add_option("", "p", "path", "Recording file path", cxxopts::value(m_RecordingPath), ""); + m_SubOptions.add_option("", "", "dry", "Do a dry run", cxxopts::value(m_DryRun), ""); + m_SubOptions.add_option("", + "w", + "numthreads", + "Number of worker threads per process", + cxxopts::value(m_ThreadCount)->default_value(fmt::format("{}", GetHardwareConcurrency())), + ""); + m_SubOptions.add_option("", "", "onhost", "Replay on host, bypassing http/network layer", cxxopts::value(m_OnHost), ""); + m_SubOptions.add_option("", + "", + "showmethodstats", + "Show statistics of which RPC methods are used", + cxxopts::value(m_ShowMethodStats), + ""); + m_SubOptions.add_option("", + "", + "offset", + "Offset into request recording to start replay", + cxxopts::value(m_Offset)->default_value("0"), + ""); + m_SubOptions.add_option("", + "", + "stride", + "Stride for request recording when replaying requests", + cxxopts::value(m_Stride)->default_value("1"), + ""); + m_SubOptions.add_option("", "", "numproc", "Number of worker processes", cxxopts::value(m_ProcessCount)->default_value("1"), ""); + m_SubOptions.add_option("", + "", + "forceallowlocalrefs", + "Force enable local refs in requests", + cxxopts::value(m_ForceAllowLocalRefs), + ""); + m_SubOptions + .add_option("", "", "disablelocalrefs", "Force disable local refs in requests", cxxopts::value(m_DisableLocalRefs), ""); + m_SubOptions.add_option("", + "", + "forceallowlocalhandlerefs", + "Force enable local refs as handles in requests", + cxxopts::value(m_ForceAllowLocalHandleRef), + ""); + m_SubOptions.add_option("", + "", + "disablelocalhandlerefs", + "Force disable local refs as handles in requests", + cxxopts::value(m_DisableLocalHandleRefs), + ""); + m_SubOptions.add_option("", + "", + "forceallowpartiallocalrefs", + "Force enable local refs for all sizes", + cxxopts::value(m_ForceAllowPartialLocalRefs), + ""); + m_SubOptions.add_option("", + "", + "disablepartiallocalrefs", + "Force disable local refs for all sizes", + cxxopts::value(m_DisablePartialLocalRefs), + ""); + m_SubOptions.parse_positional("path"); +} + +void +CacheReplaySubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) +{ + ResolveHost(); + + if (m_RecordingPath.empty()) + { + throw OptionParseException("'--path' is required", m_SubOptions.help()); + } + + if (!IsDir(m_RecordingPath)) + { + throw std::runtime_error(fmt::format("could not find recording at '{}'", m_RecordingPath)); + } + + if (m_Stride == 0) + { + throw OptionParseException("'--stride' must be >= 1", m_SubOptions.help()); + } + + m_ThreadCount = Max(m_ThreadCount, 1); + + ZEN_CONSOLE("Replay '{}' (start offset {}, stride {}) to '{}', {} threads", + m_RecordingPath, + m_Offset, + m_Stride, + m_HostName, + m_ThreadCount); + + Stopwatch TotalTimer; + + if (m_OnHost) + { + HttpClient Http = CacheCommand::CreateHttpClient(m_HostName); + if (HttpClient::Response Response = + Http.Post("/z$/exec$/replay-recording"sv, + HttpClient::KeyValueMap{}, + HttpClient::KeyValueMap({{"path", m_RecordingPath}, {"thread-count", fmt::format("{}", m_ThreadCount)}}))) + { + ZEN_CONSOLE("{}", Response.ToText()); + + return; + } + else + { + Response.ThrowError("Failed to start replay"); + } + } + + std::unique_ptr Replayer = cache::MakeDiskRequestReplayer(m_RecordingPath, true); + uint64_t EntryCount = Replayer->GetRequestCount(); + + if (m_Offset >= EntryCount) + { + ZEN_CONSOLE("Offset {} is at or past the end of the recording ({} entries); nothing to replay", m_Offset, EntryCount); + return; + } + + std::atomic_uint64_t EntryOffset = m_Offset; + std::atomic_uint64_t BytesSent = 0; + std::atomic_uint64_t BytesReceived = 0; + + Stopwatch Timer; + + // The subcommand API does not receive argv, so look the zen executable path + // up from the current process to spawn child workers. + const std::filesystem::path SelfExePath = GetRunningExecutablePath(); + + if (m_ProcessCount > 1) + { + std::vector> WorkerProcesses; + WorkerProcesses.resize(m_ProcessCount); + + ProcessMonitor Monitor; + for (int ProcessIndex = 0; ProcessIndex < m_ProcessCount; ++ProcessIndex) + { + std::string CommandLine = + fmt::format("{} cache replay --hosturl {} --path \"{}\" --offset {} --stride {} --numthreads {} --numproc {}"sv, + SelfExePath.string(), + m_HostName, + m_RecordingPath, + m_Stride == 1 ? 0 : m_Offset + ProcessIndex, + m_Stride, + m_ThreadCount, + 1); + CreateProcResult Result(CreateProc(SelfExePath, CommandLine)); + WorkerProcesses[ProcessIndex] = std::make_unique(); + WorkerProcesses[ProcessIndex]->Initialize(Result); + Monitor.AddPid(WorkerProcesses[ProcessIndex]->Pid()); + } + while (Monitor.IsRunning()) + { + ZEN_CONSOLE("Waiting for worker processes..."); + Sleep(1000); + } + return; + } + else + { + std::map MethodTypes; + RwLock MethodTypesLock; + + WorkerThreadPool WorkerPool(m_ThreadCount); + + Latch WorkLatch(m_ThreadCount); + for (int WorkerIndex = 0; WorkerIndex < m_ThreadCount; ++WorkerIndex) + { + WorkerPool.ScheduleWork( + [this, &WorkLatch, EntryCount, &EntryOffset, &Replayer, &BytesSent, &BytesReceived, &MethodTypes, &MethodTypesLock]() { + auto _ = MakeGuard([&WorkLatch]() { WorkLatch.CountDown(); }); + + std::map LocalMethodTypes; + + auto ReduceTypes = MakeGuard([&] { + RwLock::ExclusiveLockScope __(MethodTypesLock); + + for (auto& Entry : LocalMethodTypes) + { + MethodTypes[Entry.first] += Entry.second; + } + }); + + HttpClient Http = CacheCommand::CreateHttpClient(m_HostName); + + uint64_t EntryIndex = EntryOffset.fetch_add(m_Stride); + while (EntryIndex < EntryCount) + { + IoBuffer Payload; + const zen::cache::RecordedRequestInfo RequestInfo = Replayer->GetRequest(EntryIndex, /* out */ Payload); + + if (RequestInfo != zen::cache::RecordedRequestInfo::NullRequest) + { + CbPackage RequestPackage; + CbObject Request; + + switch (RequestInfo.ContentType) + { + case ZenContentType::kCbPackage: + { + if (ParsePackageMessageWithLegacyFallback(Payload, RequestPackage)) + { + Request = RequestPackage.GetObject(); + } + } + break; + case ZenContentType::kCbObject: + { + Request = LoadCompactBinaryObject(Payload); + } + break; + } + + RpcAcceptOptions OriginalAcceptOptions = static_cast(Request["AcceptFlags"sv].AsUInt16(0u)); + int OriginalProcessPid = Request["Pid"sv].AsInt32(0); + + int AdjustedPid = 0; + RpcAcceptOptions AdjustedAcceptOptions = RpcAcceptOptions::kNone; + + if (!m_DisableLocalRefs) + { + if (EnumHasAnyFlags(OriginalAcceptOptions, RpcAcceptOptions::kAllowLocalReferences) || + m_ForceAllowLocalRefs) + { + AdjustedAcceptOptions |= RpcAcceptOptions::kAllowLocalReferences; + if (!m_DisablePartialLocalRefs) + { + if (EnumHasAnyFlags(OriginalAcceptOptions, RpcAcceptOptions::kAllowPartialLocalReferences) || + m_ForceAllowPartialLocalRefs) + { + AdjustedAcceptOptions |= RpcAcceptOptions::kAllowPartialLocalReferences; + } + } + if (!m_DisableLocalHandleRefs) + { + if (OriginalProcessPid != 0 || m_ForceAllowLocalHandleRef) + { + AdjustedPid = GetCurrentProcessId(); + } + } + } + } + + if (m_ShowMethodStats) + { + std::string MethodName = std::string(Request["Method"sv].AsString()); + if (auto It = LocalMethodTypes.find(MethodName); It != LocalMethodTypes.end()) + { + It->second++; + } + else + { + LocalMethodTypes[MethodName] = 1; + } + } + + if (OriginalAcceptOptions != AdjustedAcceptOptions || OriginalProcessPid != AdjustedPid) + { + CbObjectWriter RequestCopyWriter; + for (const CbFieldView& Field : Request) + { + if (!Field.HasName()) + { + RequestCopyWriter.AddField(Field); + continue; + } + std::string_view FieldName = Field.GetName(); + if (FieldName == "Pid"sv) + { + continue; + } + if (FieldName == "AcceptFlags"sv) + { + continue; + } + RequestCopyWriter.AddField(FieldName, Field); + } + if (AdjustedPid != 0) + { + RequestCopyWriter.AddInteger("Pid"sv, AdjustedPid); + } + if (AdjustedAcceptOptions != RpcAcceptOptions::kNone) + { + RequestCopyWriter.AddInteger("AcceptFlags"sv, static_cast(AdjustedAcceptOptions)); + } + + if (RequestInfo.ContentType == ZenContentType::kCbPackage) + { + RequestPackage.SetObject(RequestCopyWriter.Save()); + std::vector Buffers = FormatPackageMessage(RequestPackage); + std::vector SharedBuffers(Buffers.begin(), Buffers.end()); + Payload = CompositeBuffer(std::move(SharedBuffers)).Flatten().AsIoBuffer(); + } + else + { + RequestCopyWriter.Finalize(); + Payload = IoBuffer(RequestCopyWriter.GetSaveSize()); + RequestCopyWriter.Save(Payload.GetMutableView()); + } + } + + if (!m_DryRun) + { + Http.SetSessionId(RequestInfo.SessionId); + Payload.SetContentType(RequestInfo.ContentType); + + HttpClient::Response Response = + Http.Post("/z$/$rpc", Payload, {HttpClient::Accept(RequestInfo.AcceptType)}); + + BytesSent.fetch_add(Payload.GetSize()); + if (!Response) + { + ZEN_CONSOLE_ERROR("{}", Response); + break; + } + BytesReceived.fetch_add(Response.DownloadedBytes); + } + } + + EntryIndex = EntryOffset.fetch_add(m_Stride); + } + }, + WorkerThreadPool::EMode::EnableBacklog); + } + + while (!WorkLatch.Wait(1000)) + { + // EntryCount > m_Offset is guaranteed by the early-return above. + // EntryOffset atomically overshoots EntryCount (fetch_add past the + // end) when the workload finishes, so clamp before subtracting. + const uint64_t RequestsTotal = (EntryCount - m_Offset) / m_Stride; + const uint64_t CurrentOffset = EntryOffset.load(); + const uint64_t RequestsRemaining = CurrentOffset < EntryCount ? (EntryCount - CurrentOffset) / m_Stride : 0; + const uint64_t PercentDone = RequestsTotal > 0 ? (RequestsTotal - RequestsRemaining) * 100 / RequestsTotal : 100; + + ZEN_CONSOLE("[{:3}%] [{}] {} requests, {} remaining (sent {}, received {})", + PercentDone, + NiceTimeSpanMs(Timer.GetElapsedTimeMs()), + RequestsTotal, + RequestsRemaining, + NiceBytes(BytesSent.load()), + NiceBytes(BytesReceived.load())); + } + + if (m_ShowMethodStats) + { + for (const auto& It : MethodTypes) + { + ZEN_CONSOLE("{:18}: {:10}", It.first, It.second); + } + } + } + + const uint64_t RequestsSent = (EntryOffset.load() - m_Offset) / m_Stride; + const uint64_t ElapsedMS = Timer.GetElapsedTimeMs(); + const uint64_t Sent = BytesSent.load(); + const uint64_t Received = BytesReceived.load(); + + ZEN_CONSOLE("Processed requests: {} ({}), payloads sent {} ({}), payloads received {} ({}) in {}.\nTotal runtime: {}", + RequestsSent, + NiceRate(RequestsSent, ElapsedMS, "req"), + NiceBytes(Sent), + NiceByteRate(Sent, ElapsedMS), + NiceBytes(Received), + NiceByteRate(Received, ElapsedMS), + NiceTimeSpanMs(ElapsedMS), + NiceTimeSpanMs(TotalTimer.GetElapsedTimeMs())); +} + } // namespace zen -- cgit v1.2.3 From 27d72af24a8de9a81500e68a0874f1430297b3bc Mon Sep 17 00:00:00 2001 From: Stefan Boberg Date: Mon, 20 Apr 2026 23:52:38 +0200 Subject: Zen CLI common server interface (#920) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces a common `ZenServiceClient` RAII wrapper for zen CLI commands that interact with a zenserver instance. CLI operations (admin, builds, cache, exec, hub, info, projectstore, trace, ui, version, vfs, workspaces) automatically register sessions so they become visible in the server's session list, and forward log output to the server's session log endpoint. All session HTTP I/O (announce, remove, log batches) runs on a single background worker thread, so CLI startup and shutdown never block on server availability. ### Key changes - **`ZenServiceClient`** — new RAII class that wraps host resolution, HTTP client creation, and session lifecycle (register on connect, remove on exit). Replaces ad-hoc boilerplate across all command files that talk to a server, including the new `trace` subcommands (`start`, `stop`, `status`). - **Async session I/O** — `SessionsServiceClient` now owns a single worker thread and command queue. `Announce()`, `Remove()`, and `UpdateMetadata()` enqueue commands and return immediately. The worker creates one `HttpClient` with a 5-second total timeout, bounding any individual request. Eliminates main-thread stalls when the server is unreachable. - **Session log forwarding** — `SessionLogSink` is a thin enqueuer that posts log messages to the same worker queue (no separate thread or HTTP client). Log levels are serialized as integers; the server-side ingest handles both string and integer formats for backwards compatibility, with bounds checking on integer values. - **Build & projectstore session registration** — Long-running `builds` and projectstore cache (oplog-download) connections register sessions too, making them visible alongside regular CLI command sessions. ### Cleanup - Extract `SetupCacheSession` helper on `StorageInstance` to reduce duplication. - Remove unused `HttpClient` reference in ui command. --- src/zen/cmds/cache_cmd.cpp | 46 +++++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 19 deletions(-) (limited to 'src/zen/cmds/cache_cmd.cpp') diff --git a/src/zen/cmds/cache_cmd.cpp b/src/zen/cmds/cache_cmd.cpp index f024ea57e..b6f272ee7 100644 --- a/src/zen/cmds/cache_cmd.cpp +++ b/src/zen/cmds/cache_cmd.cpp @@ -2,6 +2,8 @@ #include "cache_cmd.h" +#include "zenserviceclient.h" + #include #include #include @@ -169,6 +171,8 @@ void CacheDropSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) { ResolveHost(); + ZenServiceClient Service({.HostSpec = m_HostName, .CommandName = "drop"}); + HttpClient& Http = Service.Http(); if (m_NamespaceName.empty()) { @@ -180,18 +184,16 @@ CacheDropSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) if (m_BucketName.empty()) { - DropDescription = fmt::format("cache namespace '{}' from '{}'", m_NamespaceName, m_HostName); + DropDescription = fmt::format("cache namespace '{}' from '{}'", m_NamespaceName, Service.HostSpec()); Url = fmt::format("/z$/{}", m_NamespaceName); } else { - DropDescription = fmt::format("cache bucket '{}/{}' from '{}'", m_NamespaceName, m_BucketName, m_HostName); + DropDescription = fmt::format("cache bucket '{}/{}' from '{}'", m_NamespaceName, m_BucketName, Service.HostSpec()); Url = fmt::format("/z$/{}/{}", m_NamespaceName, m_BucketName); } ZEN_CONSOLE("Dropping {}", DropDescription); - - HttpClient Http = CacheCommand::CreateHttpClient(m_HostName); if (HttpClient::Response Response = Http.Delete(Url)) { ZEN_CONSOLE("{}", Response.ToText()); @@ -224,6 +226,8 @@ void CacheInfoSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) { ResolveHost(); + ZenServiceClient Service({.HostSpec = m_HostName, .CommandName = "info"}); + HttpClient& Http = Service.Http(); std::string Url; if (m_NamespaceName.empty()) @@ -236,7 +240,7 @@ CacheInfoSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) { throw OptionParseException("'--bucketsize' requires '--namespace' and '--bucket'", m_SubOptions.help()); } - ZEN_CONSOLE("Info on cache from '{}'", m_HostName); + ZEN_CONSOLE("Info on cache from '{}'", Service.HostSpec()); Url = "/z$"; } else if (m_BucketName.empty()) @@ -246,7 +250,7 @@ CacheInfoSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) throw OptionParseException(fmt::format("'--bucketsize' requires '--namespace' and '--bucket' ('{}')", m_BucketName), m_SubOptions.help()); } - ZEN_CONSOLE("Info on cache namespace '{}' from '{}'", m_NamespaceName, m_HostName); + ZEN_CONSOLE("Info on cache namespace '{}' from '{}'", m_NamespaceName, Service.HostSpec()); Url = fmt::format("/z$/{}", m_NamespaceName); } else @@ -255,7 +259,7 @@ CacheInfoSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) { throw OptionParseException("'--bucketsizes' conflicts with '--bucket'", m_SubOptions.help()); } - ZEN_CONSOLE("Info on cache bucket '{}/{}' from '{}'", m_NamespaceName, m_BucketName, m_HostName); + ZEN_CONSOLE("Info on cache bucket '{}/{}' from '{}'", m_NamespaceName, m_BucketName, Service.HostSpec()); Url = fmt::format("/z$/{}/{}", m_NamespaceName, m_BucketName); } @@ -271,7 +275,6 @@ CacheInfoSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) const ZenContentType AcceptType = m_YAML ? ZenContentType::kYAML : ZenContentType::kJSON; - HttpClient Http = CacheCommand::CreateHttpClient(m_HostName); if (HttpClient::Response Response = Http.Get(Url, HttpClient::Accept(AcceptType), Parameters)) { ZEN_CONSOLE("{}", Response.ToText()); @@ -294,10 +297,11 @@ void CacheStatsSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) { ResolveHost(); + ZenServiceClient Service({.HostSpec = m_HostName, .CommandName = "stats"}); + HttpClient& Http = Service.Http(); const ZenContentType AcceptType = m_YAML ? ZenContentType::kYAML : ZenContentType::kJSON; - HttpClient Http = CacheCommand::CreateHttpClient(m_HostName); if (HttpClient::Response Response = Http.Get("/stats/z$", HttpClient::Accept(AcceptType))) { ZEN_CONSOLE("{}", Response.ToText()); @@ -331,6 +335,8 @@ void CacheDetailsSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) { ResolveHost(); + ZenServiceClient Service({.HostSpec = m_HostName, .CommandName = "details"}); + HttpClient& Http = Service.Http(); if (m_CSV && m_YAML) { @@ -387,7 +393,6 @@ CacheDetailsSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) Url = "/z$/details$"; } - HttpClient Http = CacheCommand::CreateHttpClient(m_HostName); if (HttpClient::Response Response = Http.Get(Url, Headers, Parameters)) { ZEN_CONSOLE("{}", Response.ToText()); @@ -429,6 +434,8 @@ void CacheGenSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) { ResolveHost(); + ZenServiceClient Service({.HostSpec = m_HostName, .CommandName = "gen"}); + HttpClient& Http = Service.Http(); if (m_MaxSize == 0 && m_MinSize == 0) { @@ -486,8 +493,6 @@ CacheGenSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) std::uniform_int_distribution KeyDistribution; - HttpClient Http = CacheCommand::CreateHttpClient(m_HostName); - auto GeneratePutCacheValueRequest( [this, &KeyDistribution, &Generator](std::span BatchSizes, uint64_t RequestIndex) -> CbPackage { CbPackage Package; @@ -663,7 +668,11 @@ CacheGetSubCmd::CacheGetSubCmd() : CacheSubCmdBase("get", "Get cache values/reco void CacheGetSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) { + using namespace std::literals; + ResolveHost(); + ZenServiceClient Service({.HostSpec = m_HostName, .CommandName = "get"}); + HttpClient& Http = Service.Http(); if (m_Namespace.empty()) { @@ -695,8 +704,6 @@ CacheGetSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) } } - HttpClient Http = CacheCommand::CreateHttpClient(m_HostName); - if (!m_OutputPath.empty()) { if (IsDir(m_OutputPath)) @@ -774,8 +781,8 @@ void CacheRecordSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) { ResolveHost(); - - HttpClient Http = CacheCommand::CreateHttpClient(m_HostName); + ZenServiceClient Service({.HostSpec = m_HostName, .CommandName = "record"}); + HttpClient& Http = Service.Http(); if (m_Path == "stop") { @@ -877,8 +884,6 @@ CacheReplaySubCmd::CacheReplaySubCmd() : CacheSubCmdBase("replay", "Replays a pr void CacheReplaySubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) { - ResolveHost(); - if (m_RecordingPath.empty()) { throw OptionParseException("'--path' is required", m_SubOptions.help()); @@ -896,6 +901,9 @@ CacheReplaySubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) m_ThreadCount = Max(m_ThreadCount, 1); + ZenServiceClient Service({.HostSpec = m_HostName, .CommandName = "replay"}); + m_HostName = Service.HostSpec(); + ZEN_CONSOLE("Replay '{}' (start offset {}, stride {}) to '{}', {} threads", m_RecordingPath, m_Offset, @@ -907,7 +915,7 @@ CacheReplaySubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) if (m_OnHost) { - HttpClient Http = CacheCommand::CreateHttpClient(m_HostName); + HttpClient& Http = Service.Http(); if (HttpClient::Response Response = Http.Post("/z$/exec$/replay-recording"sv, HttpClient::KeyValueMap{}, -- cgit v1.2.3 From 245d2e562165d048e5ee2ab97f1260975a8142d3 Mon Sep 17 00:00:00 2001 From: Stefan Boberg Date: Tue, 21 Apr 2026 13:48:41 +0200 Subject: zen CLI security review fixes (#974) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security review follow-ups to the `zen` CLI. Each fix stands on its own commit. Grouped by category below. ## Credentials and secrets - **Per-install random auth encryption key instead of a hardcoded literal.** The default AES key and IV used to encrypt persisted OIDC refresh tokens / OAuth client secrets were ASCII literals compiled into the public source. Replaced with 32+16 random bytes persisted to `/auth/machinekey.dat`. `SecureRandomBytes` added in zencore/crypto wrapping BCryptGenRandom / OpenSSL / mbedTLS CTR_DRBG. Partial override (only one of `--encryption-aes-key`/`--encryption-aes-iv`) is now rejected instead of silently using the hardcoded half. - **Wrap the machine key with OS-protected storage.** `machinekey.dat` is now a tagged format (4-byte magic + flags + wrapped-or-raw payload). Windows wraps via DPAPI (`CryptProtectData` at per-user scope) so a stolen disk copy cannot decrypt without the OS master key. macOS uses Keychain Services (GenericPassword under `org.unrealengine.zen.auth`, `kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly`). Linux uses libsecret (opt-in via `--zenlibsecret=yes`, off by default because headless servers typically have no Secret Service daemon). All platforms fall back to raw persistence with `0600` perms on POSIX when wrapping is unavailable. Legacy files from the prior commit are detected by size and still read. > Note: argv-redaction before Sentry on crash was previously part of this PR but was superseded by `ScrubSensitiveValues()` from #989; this PR now just calls that helper instead of walking argv itself. ## Path traversal - **Reject unsafe filenames from the remote oplog in `oplog-mirror`.** The filename from each oplog entry was joined to the mirror root without normalisation; a compromised remote could use drive letters, UNC shares, device path prefixes, absolute paths, or `..` components to write anywhere the zen user could write. An `UnsafeFileNameReason` check runs immediately after extraction, logs the offending filename, and aborts the mirror. - **Use the resolved absolute download-spec path in `builds download`.** `--download-spec-path` was computed into a sanitised absolute path, then the original unresolved path was passed to `ParseBuildManifest`, bypassing the `MakeSafeAbsolutePath` mitigations and reading from the process cwd rather than `--local-path`. ## Input validation - **Stop asserting on malformed `--build-id` / `--build-part-id`.** `Oid::FromHexString` asserts on bad input and `ZEN_ASSERT` is active in release, so a too-short or non-hex user value aborted the process instead of surfacing an `OptionParseException`. Routed all callers through `TryFromHexString`. Also fixes `ParseBuildPartId` reporting errors under the wrong option name. - **Check the JSON parse error in `oplog-export --builds-metadata-path`.** The single-arg `LoadCompactBinaryFromJson` overload discarded the parser error; malformed JSON shipped a truncated compact-binary `metadata` field to the server with no indication. Switched to the two-arg overload and throws a descriptive error naming the file and reason. - **Format the actual value in the malformed `--url` error.** The message was constructed with a literal `{}` placeholder and no `fmt::format` call, so users saw the placeholder instead of the offending URL. - **Require `--output-path` in `cache get` unless `--as-text` is set.** Previously an empty path auto-filled from the value key / attachment hash and wrote into the process cwd; the `--as-text && empty path` stdout branch was unreachable because the auto-fill ran first. - **Clear the cxxopts `allow_unrecognised_options` flag after permissive parse.** `ParseOptionsPermissive` set the flag on the Options it received and never cleared it, priming that Options for silent typo acceptance on any later reuse. Added `disallow_unrecognised_options()` to the vendored cxxopts (local patch — flagged at the declaration) and wrapped the toggle in RAII. ## Resource lifecycle - **Restore signal handlers via RAII.** `wipe`, `builds`, and `oplog-mirror` installed SIGINT/SIGBREAK handlers with raw `signal()` and never restored them; an option-parse throw left the handler targeting an abort flag nothing reads. Added `zen::ScopedSignalHandler` in zen.h and applied at all three sites (builds uses `std::optional` members so the guards survive past `OnParentOptionsParsed` into the subcommand's Run). - **Route SIGINT in `oplog-mirror` to the worker-pool abort flag.** The command declared a local `std::atomic AbortFlag` but no handler targeted it — Ctrl-C killed the process instead of cleanly aborting. Added a `MirrorAbortFlag` / `MirrorSignalCallbackHandler` pair in projectstore_impl and bound the local as a reference; existing `.store`/`.load`/capture sites unchanged. - **Clean up the `cache get` temp download on every exit path.** `Http.Download` parks the payload in the system temp dir; a failed `MoveToFile` (cross-volume, denied target) or an exception could leave the temp file behind. The downloaded buffer is already flagged delete-on-close by `HttpClient`, so the fix is just to clear that flag after a successful `MoveToFile` so the renamed-out file isn't reaped. ## Other - **Fix wrong URL fields in `oplog-export` / `oplog-import` builds-branch descriptions.** Two operator-facing "[builds] URL/namespace/bucket/buildsid" messages formatted `m_CloudUrl` instead of `m_BuildsUrl` / `m_BuildsHost` (copy-paste from neighbouring `[cloud]` branches), shown as empty or stale at the start of an export/import. - **Fix "Can't find oplog in project '{}'" formatting and a "Failed top mirror" typo in projectstore_cmd.** - **Fix a misleading `oplog-export` comment on the `--zen` scheme default** ("Assume https" vs. the `http://` the code writes). - **Fail `ScrambleDir` when `RemoveFile` doesn't delete.** The `zen builds test` scramble phase used `(void)RemoveFile(FilePath)`, discarding both the bool return and the error. A quiet delete failure let verification run against stale state; switched to the two-arg overload and throw on false return or non-empty `error_code`. --- src/zen/cmds/cache_cmd.cpp | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) (limited to 'src/zen/cmds/cache_cmd.cpp') diff --git a/src/zen/cmds/cache_cmd.cpp b/src/zen/cmds/cache_cmd.cpp index b6f272ee7..c03284462 100644 --- a/src/zen/cmds/cache_cmd.cpp +++ b/src/zen/cmds/cache_cmd.cpp @@ -704,6 +704,11 @@ CacheGetSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) } } + if (m_OutputPath.empty() && !m_AsText) + { + throw OptionParseException("'--output-path' is required (or pass '--as-text' to print to stdout)", m_SubOptions.help()); + } + if (!m_OutputPath.empty()) { if (IsDir(m_OutputPath)) @@ -715,10 +720,6 @@ CacheGetSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) CreateDirectories(m_OutputPath.parent_path()); } } - if (m_OutputPath.empty()) - { - m_OutputPath = (m_AttachmentHash.empty() ? m_ValueKey : m_AttachmentHash); - } std::string Url = fmt::format("/z$/{}/{}/{}", m_Namespace, m_Bucket, ValueId); if (AttachmentHash != IoHash::Zero) @@ -727,6 +728,11 @@ CacheGetSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) } if (HttpClient::Response Result = Http.Download(Url, std::filesystem::temp_directory_path()); Result) { + // `Http.Download` parks the payload in the system temp dir and returns + // a buffer that already has delete-on-close set, so every exit path + // (exception, fallback WriteFile, `--as-text` console print) reaps it. + // A successful MoveToFile below clears the flag so the payload's + // handle-close doesn't delete the caller's output afterwards. auto TryDecompress = [](const IoBuffer& Buffer) -> IoBuffer { IoHash RawHash; uint64_t RawSize; @@ -753,7 +759,17 @@ CacheGetSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) } else { - if (!MoveToFile(m_OutputPath, ChunkData)) + if (MoveToFile(m_OutputPath, ChunkData)) + { + // The file was renamed into place; clearing DeleteOnClose prevents + // the move'd-out file at m_OutputPath from being deleted when the + // payload's handle closes. When m_Decompress is false ChunkData + // shares a core with ResponsePayload so either clear suffices; + // when decompressed ChunkData is in-memory and MoveToFile would + // have failed, so we don't reach this branch. + Result.ResponsePayload.SetDeleteOnClose(false); + } + else { WriteFile(m_OutputPath, ChunkData); } -- cgit v1.2.3 From 82e222bf23dee04e6fb825037fbb4d86a9571ce0 Mon Sep 17 00:00:00 2001 From: Dan Engelbrecht Date: Tue, 21 Apr 2026 17:22:18 +0200 Subject: filesystem.h surface error codes (#998) - Improvement: File copy, scan, clone, and move operations now report the underlying OS error in failure messages --- src/zen/cmds/cache_cmd.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/zen/cmds/cache_cmd.cpp') diff --git a/src/zen/cmds/cache_cmd.cpp b/src/zen/cmds/cache_cmd.cpp index c03284462..f93a5318c 100644 --- a/src/zen/cmds/cache_cmd.cpp +++ b/src/zen/cmds/cache_cmd.cpp @@ -759,7 +759,7 @@ CacheGetSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) } else { - if (MoveToFile(m_OutputPath, ChunkData)) + if (std::error_code MoveEc = MoveToFile(m_OutputPath, ChunkData); MoveEc) { // The file was renamed into place; clearing DeleteOnClose prevents // the move'd-out file at m_OutputPath from being deleted when the -- cgit v1.2.3