From 8750ec743da8979aca9473bf5bed457867280b19 Mon Sep 17 00:00:00 2001 From: Dan Engelbrecht Date: Fri, 17 Apr 2026 10:16:22 +0200 Subject: operationlogoutput refactor (#967) - Improvement: Replaced `OperationLogOutput` with `ProgressBase` in `zenutil`; logging and progress reporting are now separate concerns. Operation classes receive a `LoggerRef` for logging and a `ProgressBase&` for progress bars --- src/zen/cmds/projectstore_cmd.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'src/zen/cmds/projectstore_cmd.cpp') diff --git a/src/zen/cmds/projectstore_cmd.cpp b/src/zen/cmds/projectstore_cmd.cpp index d31c34fd0..76e081b1a 100644 --- a/src/zen/cmds/projectstore_cmd.cpp +++ b/src/zen/cmds/projectstore_cmd.cpp @@ -24,10 +24,10 @@ #include #include #include -#include #include #include #include +#include #include #include "../progressbar.h" @@ -2565,7 +2565,7 @@ OplogDownloadCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** a m_BoostWorkerMemory = true; } - std::unique_ptr OperationLogOutput(CreateConsoleLogOutput(ProgressMode)); + std::unique_ptr Progress(CreateConsoleProgress(ProgressMode)); TransferThreadWorkers Workers(m_BoostWorkerCount, /*SingleThreaded*/ false); if (!m_Quiet) @@ -2594,7 +2594,7 @@ OplogDownloadCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** a /*Hidden*/ false, m_Verbose); - BuildStorageResolveResult ResolveRes = ResolveBuildStorage(*OperationLogOutput, + BuildStorageResolveResult ResolveRes = ResolveBuildStorage(ConsoleLog(), ClientSettings, m_Host, m_OverrideHost, @@ -2677,7 +2677,7 @@ OplogDownloadCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** a } ProjectStoreOperationOplogState State( - *OperationLogOutput, + ConsoleLog(), Storage, BuildId, {.IsQuiet = m_Quiet, .IsVerbose = m_Verbose, .ForceDownload = m_ForceDownload, .TempFolderPath = StorageTempPath}); @@ -2704,7 +2704,8 @@ OplogDownloadCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** a } std::atomic PauseFlag; - ProjectStoreOperationDownloadAttachments Op(*OperationLogOutput, + ProjectStoreOperationDownloadAttachments Op(ConsoleLog(), + *Progress, Storage, AbortFlag, PauseFlag, -- cgit v1.2.3 From f07e04aa501b26b96e345f2e8ac42d231a015e55 Mon Sep 17 00:00:00 2001 From: Dan Engelbrecht Date: Fri, 17 Apr 2026 15:00:01 +0200 Subject: replace pretty progress with prettyscroll implementation (#970) --- src/zen/cmds/projectstore_cmd.cpp | 4 ---- 1 file changed, 4 deletions(-) (limited to 'src/zen/cmds/projectstore_cmd.cpp') diff --git a/src/zen/cmds/projectstore_cmd.cpp b/src/zen/cmds/projectstore_cmd.cpp index 76e081b1a..7f94bf2df 100644 --- a/src/zen/cmds/projectstore_cmd.cpp +++ b/src/zen/cmds/projectstore_cmd.cpp @@ -2500,10 +2500,6 @@ OplogDownloadCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** a { ProgressMode = ProgressBar::Mode::Plain; } - else if (m_Verbose) - { - ProgressMode = ProgressBar::Mode::Plain; - } else if (m_Quiet) { ProgressMode = ProgressBar::Mode::Quiet; -- cgit v1.2.3 From c7c59cdc5a70bfd6e5f66f3b032ea3f8f6b4d12a Mon Sep 17 00:00:00 2001 From: Dan Engelbrecht Date: Mon, 20 Apr 2026 07:27:35 +0200 Subject: builds cmd refactor (#975) - Bugfix: `builds download` partial-block fetch decisions now account for build storage host latency - Bugfix: Transfer rate displays in `builds` commands now smooth correctly - Split `buildstorageoperations.cpp` (8.5k lines) into per-operation TUs: buildinspect, buildprimecache, buildstorageresolve, buildupdatefolder, builduploadfolder, buildvalidatebuildpart; stats moved to buildstoragestats.h. - FilteredRate extracted to zenutil. - BuildsCommand shared state consolidated into a BuildsConfiguration struct; subcommands inherit from BuildsSubCmdBase holding a `const BuildsConfiguration&` instead of a `BuildsCommand&`. - `ProgressBar` renamed to `ConsoleProgressBar`; mode enum (`ConsoleProgressMode`) lifted to namespace scope; `PushLogOperation`/`PopLogOperation`/`ForceLinebreak` promoted to virtuals on `ProgressBase`. - Free-function wrappers (`UploadFolder`, `DownloadFolder`, `ValidateBuildPart`) added around the existing operation classes so callers stop reimplementing setup + stats logging. --- src/zen/cmds/projectstore_cmd.cpp | 69 ++++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 30 deletions(-) (limited to 'src/zen/cmds/projectstore_cmd.cpp') diff --git a/src/zen/cmds/projectstore_cmd.cpp b/src/zen/cmds/projectstore_cmd.cpp index 7f94bf2df..bad2728ef 100644 --- a/src/zen/cmds/projectstore_cmd.cpp +++ b/src/zen/cmds/projectstore_cmd.cpp @@ -21,16 +21,18 @@ #include #include #include +#include #include #include #include #include #include #include +#include #include #include -#include "../progressbar.h" +#include "consoleprogress.h" ZEN_THIRD_PARTY_INCLUDES_START #include @@ -131,13 +133,16 @@ namespace projectstore_impl { throw std::runtime_error(fmt::format("invalid job id returned, received '{}'", JobIdText)); } - ProgressBar ProgressBar(PlainProgress ? ProgressBar::Mode::Plain : ProgressBar::Mode::Pretty, ""sv); + std::unique_ptr ProgressOwner( + CreateConsoleProgress(PlainProgress ? ConsoleProgressMode::Plain : ConsoleProgressMode::Pretty)); + std::unique_ptr Bar = ProgressOwner->CreateProgressBar(""sv); + std::string ActiveTask; - auto OuputMessages = [&](CbObjectView StatusObject) { + auto OutputMessages = [&](CbObjectView StatusObject) { CbArrayView Messages = StatusObject["Messages"sv].AsArrayView(); if (Messages.Num() > 0) { - ProgressBar.ForceLinebreak(); + Bar->ForceLinebreak(); for (auto M : Messages) { std::string_view Message = M.AsString(); @@ -169,33 +174,36 @@ namespace projectstore_impl { uint64_t RemainingCount = StatusObject["RemainingCount"sv].AsUInt64(); uint64_t ProgressElapsedTimeMs = StatusObject["ProgressElapsedTimeMs"sv].AsUInt64((uint64_t)-1); - if (!ProgressBar.IsSameTask(CurrentOp)) + if (ActiveTask != CurrentOp) { - ProgressBar.Finish(); + Bar->Finish(); + ActiveTask = ""; } - if (!ProgressBar.HasActiveTask()) + if (ActiveTask.empty()) { - OuputMessages(StatusObject); + OutputMessages(StatusObject); MessagesDone = true; + ActiveTask = std::string(CurrentOp); } - ProgressBar.UpdateState({.Task = std::string(CurrentOp), - .Details = std::string(CurrentOpDetails), - .TotalCount = TotalCount, - .RemainingCount = RemainingCount, - .OptionalElapsedTime = ProgressElapsedTimeMs}, - false); + Bar->UpdateState({.Task = std::string(CurrentOp), + .Details = std::string(CurrentOpDetails), + .TotalCount = TotalCount, + .RemainingCount = RemainingCount, + .OptionalElapsedTime = ProgressElapsedTimeMs}, + false); } if ((Status == "Complete") || (Status == "Aborted")) { - ProgressBar.Finish(); + Bar->Finish(); + ActiveTask = ""; } if (!MessagesDone) { - OuputMessages(StatusObject); + OutputMessages(StatusObject); } if (Status == "Complete") @@ -246,13 +254,13 @@ namespace projectstore_impl { #endif // ZEN_PLATFORM_WINDOWS if (HttpClient::Response DeleteResult = Http.Delete(fmt::format("/admin/jobs/{}", JobId))) { - ProgressBar.ForceLinebreak(); + Bar->ForceLinebreak(); ZEN_CONSOLE("Requested cancel..."); Cancelled = true; } else { - ProgressBar.ForceLinebreak(); + Bar->ForceLinebreak(); ZEN_CONSOLE("Failed cancelling job {}", DeleteResult); } continue; @@ -2196,17 +2204,18 @@ OplogMirrorCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg ZEN_CONSOLE("Fetched oplog in {}", NiceTimeSpanMs(uint64_t(Response.ElapsedSeconds * 1000.0))); if (CbObject ResponseObject = Response.AsObject()) { - std::unique_ptr EmitProgressBar; + std::unique_ptr ProgressOwner2(CreateConsoleProgress(ConsoleProgressMode::Pretty)); + std::unique_ptr EmitProgressBar; { - ProgressBar ParseProgressBar(ProgressBar::Mode::Pretty, ""); - CbArrayView Entries = ResponseObject["entries"sv].AsArrayView(); - uint64_t Remaining = Entries.Num(); + std::unique_ptr ParseProgressBar = ProgressOwner2->CreateProgressBar(""); + CbArrayView Entries = ResponseObject["entries"sv].AsArrayView(); + uint64_t Remaining = Entries.Num(); for (auto EntryIter : Entries) { if (!AbortFlag) { CbObjectView Entry = EntryIter.AsObjectView(); - ParseProgressBar.UpdateState( + ParseProgressBar->UpdateState( {.Task = "Parsing oplog", .Details = "", .TotalCount = Entries.Num(), .RemainingCount = Remaining}, false); Remaining--; @@ -2219,7 +2228,7 @@ OplogMirrorCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg } if (!EmitProgressBar) { - EmitProgressBar = std::make_unique(ProgressBar::Mode::Pretty, ""sv); + EmitProgressBar = ProgressOwner2->CreateProgressBar(""sv); WriteStopWatch.Reset(); } @@ -2229,7 +2238,7 @@ OplogMirrorCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg ++OplogEntryCount; } } - ParseProgressBar.Finish(); + ParseProgressBar->Finish(); } WorkRemaining.CountDown(); @@ -2472,7 +2481,7 @@ OplogDownloadCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** a }; ParseSystemOptions(); - ProgressBar::Mode ProgressMode = ProgressBar::Mode::Pretty; + ConsoleProgressMode ProgressMode = ConsoleProgressMode::Pretty; auto ParseOutputOptions = [&]() { if (m_Verbose && m_Quiet) @@ -2494,19 +2503,19 @@ OplogDownloadCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** a if (m_LogProgress) { - ProgressMode = ProgressBar::Mode::Log; + ProgressMode = ConsoleProgressMode::Log; } else if (m_PlainProgress) { - ProgressMode = ProgressBar::Mode::Plain; + ProgressMode = ConsoleProgressMode::Plain; } else if (m_Quiet) { - ProgressMode = ProgressBar::Mode::Quiet; + ProgressMode = ConsoleProgressMode::Quiet; } else { - ProgressMode = ProgressBar::Mode::Pretty; + ProgressMode = ConsoleProgressMode::Pretty; } }; ParseOutputOptions(); -- 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/projectstore_cmd.cpp | 124 ++++++++++---------------------------- 1 file changed, 33 insertions(+), 91 deletions(-) (limited to 'src/zen/cmds/projectstore_cmd.cpp') diff --git a/src/zen/cmds/projectstore_cmd.cpp b/src/zen/cmds/projectstore_cmd.cpp index bad2728ef..2bd4803f6 100644 --- a/src/zen/cmds/projectstore_cmd.cpp +++ b/src/zen/cmds/projectstore_cmd.cpp @@ -2,6 +2,8 @@ #include "projectstore_cmd.h" +#include "zenserviceclient.h" + #include #include #include @@ -9,9 +11,11 @@ #include #include #include +#include #include #include #include +#include #include #include #include @@ -29,7 +33,9 @@ #include #include #include +#include #include +#include #include #include "consoleprogress.h" @@ -538,14 +544,8 @@ DropProjectCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg return; } - m_HostName = ResolveTargetHostSpec(m_HostName); - - if (m_HostName.empty()) - { - throw OptionParseException("Unable to resolve server specification", m_Options.help()); - } - - HttpClient Http = CreateHttpClient(m_HostName); + ZenServiceClient Service({.HostSpec = m_HostName, .CommandName = Name}); + HttpClient& Http = Service.Http(); m_ProjectName = ResolveProject(Http, m_ProjectName); if (m_ProjectName.empty()) @@ -628,19 +628,13 @@ ProjectInfoCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg return; } - m_HostName = ResolveTargetHostSpec(m_HostName); - - if (m_HostName.empty()) - { - throw OptionParseException("Unable to resolve server specification", m_Options.help()); - } - if (!m_OplogName.empty() && m_ProjectName.empty()) { throw OptionParseException("'--project' is required", m_Options.help()); } - HttpClient Http = CreateHttpClient(m_HostName); + ZenServiceClient Service({.HostSpec = m_HostName, .CommandName = Name}); + HttpClient& Http = Service.Http(); std::string Url; if (m_ProjectName.empty()) @@ -717,19 +711,13 @@ CreateProjectCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** a return; } - m_HostName = ResolveTargetHostSpec(m_HostName); - - if (m_HostName.empty()) - { - throw OptionParseException("Unable to resolve server specification", m_Options.help()); - } - if (m_ProjectId.empty()) { throw OptionParseException("'--project' is required", m_Options.help()); } - HttpClient Http = CreateHttpClient(m_HostName); + ZenServiceClient Service({.HostSpec = m_HostName, .CommandName = Name}); + HttpClient& Http = Service.Http(); std::string Url = fmt::format("/prj/{}", m_ProjectId); @@ -787,20 +775,14 @@ CreateOplogCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg return; } - m_HostName = ResolveTargetHostSpec(m_HostName); - - if (m_HostName.empty()) - { - throw OptionParseException("Unable to resolve server specification", m_Options.help()); - } - if (m_ProjectId.empty()) { throw OptionParseException("'--project' is required", m_Options.help()); } - HttpClient Http = CreateHttpClient(m_HostName); - m_ProjectId = ResolveProject(Http, m_ProjectId); + ZenServiceClient Service({.HostSpec = m_HostName, .CommandName = Name}); + HttpClient& Http = Service.Http(); + m_ProjectId = ResolveProject(Http, m_ProjectId); if (m_ProjectId.empty()) { throw std::runtime_error("Project can not be found"); @@ -1018,20 +1000,14 @@ ExportOplogCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg m_BoostWorkerMemory = true; } - m_HostName = ResolveTargetHostSpec(m_HostName); - - if (m_HostName.empty()) - { - throw OptionParseException("Unable to resolve server specification", m_Options.help()); - } - if (m_ProjectName.empty()) { throw OptionParseException("'--project' is required", m_Options.help()); } - HttpClient Http = CreateHttpClient(m_HostName); - m_ProjectName = ResolveProject(Http, m_ProjectName); + ZenServiceClient Service({.HostSpec = m_HostName, .CommandName = Name}); + HttpClient& Http = Service.Http(); + m_ProjectName = ResolveProject(Http, m_ProjectName); if (m_ProjectName.empty()) { throw std::runtime_error("Project can not be found"); @@ -1525,13 +1501,6 @@ ImportOplogCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg m_BoostWorkerMemory = true; } - m_HostName = ResolveTargetHostSpec(m_HostName); - - if (m_HostName.empty()) - { - throw OptionParseException("Unable to resolve server specification", m_Options.help()); - } - if (m_ProjectName.empty()) { throw OptionParseException("'--project' is required", m_Options.help()); @@ -1549,8 +1518,9 @@ ImportOplogCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg m_Options.help()); } - HttpClient Http = CreateHttpClient(m_HostName); - m_ProjectName = ResolveProject(Http, m_ProjectName); + ZenServiceClient Service({.HostSpec = m_HostName, .CommandName = Name}); + HttpClient& Http = Service.Http(); + m_ProjectName = ResolveProject(Http, m_ProjectName); if (m_ProjectName.empty()) { throw std::runtime_error("Project can not be found"); @@ -1814,14 +1784,8 @@ SnapshotOplogCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** a return; } - m_HostName = ResolveTargetHostSpec(m_HostName); - - if (m_HostName.empty()) - { - throw OptionParseException("Unable to resolve server specification", m_Options.help()); - } - - HttpClient Http = CreateHttpClient(m_HostName); + ZenServiceClient Service({.HostSpec = m_HostName, .CommandName = Name}); + HttpClient& Http = Service.Http(); if (m_ProjectName.empty()) { @@ -1877,14 +1841,8 @@ ProjectStatsCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** ar return; } - m_HostName = ResolveTargetHostSpec(m_HostName); - - if (m_HostName.empty()) - { - throw OptionParseException("Unable to resolve server specification", m_Options.help()); - } - - HttpClient Http = CreateHttpClient(m_HostName); + ZenServiceClient Service({.HostSpec = m_HostName, .CommandName = Name}); + HttpClient& Http = Service.Http(); if (HttpClient::Response Result = Http.Get("/stats/prj", HttpClient::Accept(ZenContentType::kJSON))) { ZEN_CONSOLE("{}", Result.ToText()); @@ -1930,14 +1888,8 @@ ProjectOpDetailsCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char* return; } - m_HostName = ResolveTargetHostSpec(m_HostName); - - if (m_HostName.empty()) - { - throw OptionParseException("Unable to resolve server specification", m_Options.help()); - } - - HttpClient Http = CreateHttpClient(m_HostName); + ZenServiceClient Service({.HostSpec = m_HostName, .CommandName = Name}); + HttpClient& Http = Service.Http(); m_ProjectName = ResolveProject(Http, m_ProjectName); if (m_ProjectName.empty()) @@ -2046,14 +1998,8 @@ OplogMirrorCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg return; } - m_HostName = ResolveTargetHostSpec(m_HostName); - - if (m_HostName.empty()) - { - throw OptionParseException("Unable to resolve server specification", m_Options.help()); - } - - HttpClient Http = CreateHttpClient(m_HostName); + ZenServiceClient Service({.HostSpec = m_HostName, .CommandName = Name}); + HttpClient& Http = Service.Http(); m_ProjectName = ResolveProject(Http, m_ProjectName); if (m_ProjectName.empty()) @@ -2315,14 +2261,8 @@ OplogValidateCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** a return; } - m_HostName = ResolveTargetHostSpec(m_HostName); - - if (m_HostName.empty()) - { - throw OptionParseException("Unable to resolve server specification", m_Options.help()); - } - - HttpClient Http = CreateHttpClient(m_HostName); + ZenServiceClient Service({.HostSpec = m_HostName, .CommandName = Name}); + HttpClient& Http = Service.Http(); m_ProjectName = ResolveProject(Http, m_ProjectName); if (m_ProjectName.empty()) @@ -2634,6 +2574,8 @@ OplogDownloadCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** a .MaximumInMemoryDownloadSize = m_BoostWorkerMemory ? RemoteStoreOptions::DefaultMaxBlockSize : 1024u * 1024u}, [&AbortFlag]() { return AbortFlag.load(); }); Storage.CacheHost = ResolveRes.Cache; + + Storage.SetupCacheSession(ResolveRes.Cache.Address, Name, GetSessionId()); } if (!m_Quiet) -- 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/projectstore_cmd.cpp | 91 +++++++++++++++++++++++++++++++++++---- 1 file changed, 83 insertions(+), 8 deletions(-) (limited to 'src/zen/cmds/projectstore_cmd.cpp') diff --git a/src/zen/cmds/projectstore_cmd.cpp b/src/zen/cmds/projectstore_cmd.cpp index 2bd4803f6..d9ce1cfa7 100644 --- a/src/zen/cmds/projectstore_cmd.cpp +++ b/src/zen/cmds/projectstore_cmd.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -120,6 +121,26 @@ namespace projectstore_impl { } } + // `OplogMirrorCommand::Run` uses a latching boolean flag rather than the + // SignalCounter above, because it drives a worker pool that aborts on any + // interrupt. Kept separate from SignalCallbackHandler so neither interferes + // with the other when both are installed in the same process. + static std::atomic MirrorAbortFlag{false}; + + static void MirrorSignalCallbackHandler(int SigNum) + { + if (SigNum == SIGINT) + { + MirrorAbortFlag.store(true); + } +#if ZEN_PLATFORM_WINDOWS + if (SigNum == SIGBREAK) + { + MirrorAbortFlag.store(true); + } +#endif + } + void ExecuteAsyncOperation(HttpClient& Http, std::string_view Url, IoBuffer&& Payload, bool PlainProgress) { signal(SIGINT, SignalCallbackHandler); @@ -577,7 +598,7 @@ DropProjectCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg m_OplogName = ResolveOplog(Http, m_ProjectName, m_OplogName); if (m_OplogName.empty()) { - throw std::runtime_error(fmt::format("Can't find oplog in project '{}'", m_OplogName, m_ProjectName)); + throw zen::runtime_error("Can't find oplog '{}' in project '{}'", m_OplogName, m_ProjectName); } if (m_DryRun) { @@ -1098,7 +1119,7 @@ ExportOplogCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg std::string TargetUrlBase = m_ZenUrl; if (TargetUrlBase.find("://") == std::string::npos) { - // Assume https URL + // Assume http URL TargetUrlBase = fmt::format("http://{}", TargetUrlBase); } @@ -1287,7 +1308,14 @@ ExportOplogCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg std::filesystem::path MetadataPath(m_BuildsMetadataPath); IoBuffer MetaDataJson = ReadFile(MetadataPath).Flatten(); std::string_view Json(reinterpret_cast(MetaDataJson.GetData()), MetaDataJson.GetSize()); - CbFieldIterator MetaData = LoadCompactBinaryFromJson(Json); + std::string JsonError; + CbFieldIterator MetaData = LoadCompactBinaryFromJson(Json, JsonError); + if (!JsonError.empty()) + { + throw zen::runtime_error("builds metadata file '{}' is malformed. Reason: '{}'", + MetadataPath.string(), + JsonError); + } Writer.AddBinary("metadata"sv, MetaData.GetBuffer()); } if (!m_BuildsMetadata.empty()) @@ -1297,7 +1325,7 @@ ExportOplogCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg size_t SplitPos = Pair.find('='); if (SplitPos == std::string::npos || SplitPos == 0) { - throw std::runtime_error(fmt::format("builds metadata key-value pair '{}' is malformed", Pair)); + throw zen::runtime_error("builds metadata key-value pair '{}' is malformed", Pair); } MetaDataWriter.AddString(Pair.substr(0, SplitPos), Pair.substr(SplitPos + 1)); return true; @@ -1307,7 +1335,7 @@ ExportOplogCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg } } Writer.EndObject(); // "builds" - TargetDescription = fmt::format("[builds] {}/{}/{}/{}", m_CloudUrl, m_JupiterNamespace, m_JupiterBucket, m_BuildsId); + TargetDescription = fmt::format("[builds] {}/{}/{}/{}", m_BuildsUrl, m_JupiterNamespace, m_JupiterBucket, m_BuildsId); } if (!m_ZenUrl.empty()) { @@ -1716,7 +1744,11 @@ ImportOplogCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg } } Writer.EndObject(); // "builds" - SourceDescription = fmt::format("[builds] {}/{}/{}/{}", m_CloudUrl, m_JupiterNamespace, m_JupiterBucket, m_BuildsId); + SourceDescription = fmt::format("[builds] {}/{}/{}/{}", + m_BuildsHost.empty() ? m_BuildsOverrideHost : m_BuildsHost, + m_JupiterNamespace, + m_JupiterBucket, + m_BuildsId); } if (!m_ZenUrl.empty()) { @@ -2062,16 +2094,59 @@ OplogMirrorCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg std::unordered_set FileNames; std::atomic WrittenByteCount = 0; - std::atomic AbortFlag(false); + // Install Ctrl-C handler so SIGINT aborts the worker pool rather than killing + // the process. Without this the local AbortFlag would shadow whatever global + // handler is installed elsewhere and interrupts would be dropped. RAII so + // the previous handler is restored when the function returns or throws. + MirrorAbortFlag.store(false); + ScopedSignalHandler SigIntGuard(SIGINT, MirrorSignalCallbackHandler); +#if ZEN_PLATFORM_WINDOWS + ScopedSignalHandler SigBreakGuard(SIGBREAK, MirrorSignalCallbackHandler); +#endif + std::atomic& AbortFlag = MirrorAbortFlag; Stopwatch WriteStopWatch; + // Filenames come from the remote oplog, which may be compromised or untrusted. + // Reject anything that could escape the mirror root via an absolute path, drive + // letter / UNC / device path prefix, or '..' component before it is joined to + // RootPath. Returns nullptr when the filename is safe. + auto UnsafeFileNameReason = [](const std::filesystem::path& FileName) -> const char* { + if (FileName.empty()) + { + return "filename is empty"; + } + if (FileName.has_root_name()) + { + return "filename has a root name (drive letter, UNC share, or device path)"; + } + if (FileName.has_root_directory()) + { + return "filename is absolute"; + } + for (const std::filesystem::path& Component : FileName) + { + const std::u8string C = Component.u8string(); + if (C.empty() || C == u8"..") + { + return "filename contains a '..' or empty component"; + } + } + return nullptr; + }; + auto EmitFilesForDataArray = [&](CbArrayView DataArray) { for (auto DataIter : DataArray) { if (CbObjectView Data = DataIter.AsObjectView()) { std::filesystem::path FileName(Data["filename"sv].AsU8String()); + if (const char* Reason = UnsafeFileNameReason(FileName)) + { + ZEN_CONSOLE_ERROR("Rejecting unsafe filename '{}' from remote oplog: {}", FileName.string(), Reason); + AbortFlag.store(true); + break; + } if (!m_FilenameFilter.empty()) { std::string FileNameLowerCase = ToLower(FileName.string()); @@ -2217,7 +2292,7 @@ OplogMirrorCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg if (AbortFlag) { - throw std::runtime_error("Failed top mirror oplog"); + throw std::runtime_error("Failed to mirror oplog"); } } else -- 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/projectstore_cmd.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/zen/cmds/projectstore_cmd.cpp') diff --git a/src/zen/cmds/projectstore_cmd.cpp b/src/zen/cmds/projectstore_cmd.cpp index d9ce1cfa7..c6a3434f8 100644 --- a/src/zen/cmds/projectstore_cmd.cpp +++ b/src/zen/cmds/projectstore_cmd.cpp @@ -2191,7 +2191,7 @@ OplogMirrorCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg IoBuffer ChunkData = m_Decompress ? TryDecompress(ChunkResponse.ResponsePayload) : ChunkResponse.ResponsePayload; - if (!MoveToFile(TargetPath, ChunkData)) + if (std::error_code MoveEc = MoveToFile(TargetPath, ChunkData); MoveEc) { WriteFile(TargetPath, ChunkData); } -- cgit v1.2.3