From db616a2ede14670423b7b2b28aa36c33cabb030e Mon Sep 17 00:00:00 2001 From: Stefan Boberg Date: Thu, 16 Apr 2026 14:22:09 +0200 Subject: Add reduce-allocs skill and string builder infrastructure (#937) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds infrastructure for reducing short-lived heap allocations, to be applied across the codebase in follow-up PRs. - **`reduce-allocs` Claude Code skill** — reviews code for unnecessary heap allocations and suggests fixes using stack-friendly patterns (`ExtendableStringBuilder`, `eastl::fixed_vector`, `TRefCounted`, etc.) - **`TransparentStringHash`** (`zencore/hashutils.h`) — enables `std::string_view` lookups on `std::string`-keyed `unordered_map` without allocating a temporary string (C++20 heterogeneous lookup via `is_transparent`) - **`AppendPaddedInt()`** and **`AppendFill()`** on `StringBuilderBase` (`zencore/string.h`) — zero-padded integer formatting and repeated-character fills without going through `fmt::format` - **`StringBuilderAppender`** output iterator adapter — allows `fmt::format_to` to write directly into a `StringBuilderBase` --- src/zen/cmds/wipe_cmd.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'src/zen/cmds/wipe_cmd.cpp') diff --git a/src/zen/cmds/wipe_cmd.cpp b/src/zen/cmds/wipe_cmd.cpp index 10f5ad8e1..fe4e12906 100644 --- a/src/zen/cmds/wipe_cmd.cpp +++ b/src/zen/cmds/wipe_cmd.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include #include -- 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/wipe_cmd.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/zen/cmds/wipe_cmd.cpp') diff --git a/src/zen/cmds/wipe_cmd.cpp b/src/zen/cmds/wipe_cmd.cpp index fe4e12906..c027f0d67 100644 --- a/src/zen/cmds/wipe_cmd.cpp +++ b/src/zen/cmds/wipe_cmd.cpp @@ -549,7 +549,7 @@ WipeCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv) Quiet = m_Quiet; IsVerbose = m_Verbose; - ProgressMode = (IsVerbose || m_PlainProgress) ? ProgressBar::Mode::Plain : ProgressBar::Mode::Pretty; + ProgressMode = m_PlainProgress ? ProgressBar::Mode::Plain : ProgressBar::Mode::Pretty; BoostWorkerThreads = m_BoostWorkerThreads; MakeSafeAbsolutePathInPlace(m_Directory); -- 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/wipe_cmd.cpp | 49 ++++++++++++++++++++++++----------------------- 1 file changed, 25 insertions(+), 24 deletions(-) (limited to 'src/zen/cmds/wipe_cmd.cpp') diff --git a/src/zen/cmds/wipe_cmd.cpp b/src/zen/cmds/wipe_cmd.cpp index c027f0d67..713c5e386 100644 --- a/src/zen/cmds/wipe_cmd.cpp +++ b/src/zen/cmds/wipe_cmd.cpp @@ -12,7 +12,7 @@ #include #include -#include "../progressbar.h" +#include "consoleprogress.h" #include @@ -35,13 +35,13 @@ ZEN_THIRD_PARTY_INCLUDES_END namespace zen { namespace wipe_impl { - static std::atomic AbortFlag = false; - static std::atomic PauseFlag = false; - static bool IsVerbose = false; - static bool Quiet = false; - static ProgressBar::Mode ProgressMode = ProgressBar::Mode::Pretty; - const bool SingleThreaded = false; - bool BoostWorkerThreads = true; + static std::atomic AbortFlag = false; + static std::atomic PauseFlag = false; + static bool IsVerbose = false; + static bool Quiet = false; + static ConsoleProgressMode ProgressMode = ConsoleProgressMode::Pretty; + const bool SingleThreaded = false; + bool BoostWorkerThreads = true; WorkerThreadPool& GetIOWorkerPool() { @@ -168,7 +168,8 @@ namespace wipe_impl { ZEN_TRACE_CPU("CleanDirectory"); Stopwatch Timer; - ProgressBar Progress(ProgressMode, "Clean Folder"); + std::unique_ptr ProgressOwner(CreateConsoleProgress(ProgressMode)); + std::unique_ptr Progress = ProgressOwner->CreateProgressBar("Clean Folder"); std::atomic CleanWipe = true; std::atomic DiscoveredItemCount = 0; @@ -414,7 +415,7 @@ namespace wipe_impl { GetIOWorkerPool(), Work.PendingWork()); - Work.Wait(ProgressMode == ProgressBar::Mode::Pretty ? 200 : 5000, [&](bool IsAborted, bool IsPaused, ptrdiff_t PendingWork) { + Work.Wait(ProgressOwner->GetProgressUpdateDelayMS(), [&](bool IsAborted, bool IsPaused, ptrdiff_t PendingWork) { ZEN_UNUSED(PendingWork); if (Quiet) { @@ -425,12 +426,12 @@ namespace wipe_impl { uint64_t Deleted = DeletedItemCount.load(); uint64_t DeletedBytes = DeletedByteCount.load(); uint64_t Discovered = DiscoveredItemCount.load(); - Progress.UpdateState({.Task = "Removing files ", - .Details = fmt::format("Found {}, Deleted {} ({})", Discovered, Deleted, NiceBytes(DeletedBytes)), - .TotalCount = Discovered, - .RemainingCount = Discovered - Deleted, - .Status = ProgressBar::State::CalculateStatus(IsAborted, IsPaused)}, - false); + Progress->UpdateState({.Task = "Removing files ", + .Details = fmt::format("Found {}, Deleted {} ({})", Discovered, Deleted, NiceBytes(DeletedBytes)), + .TotalCount = Discovered, + .RemainingCount = Discovered - Deleted, + .Status = ProgressBase::ProgressBar::State::CalculateStatus(IsAborted, IsPaused)}, + false); }); std::vector DirectoriesToDelete; @@ -474,22 +475,22 @@ namespace wipe_impl { } uint64_t NowMs = Timer.GetElapsedTimeMs(); - if ((NowMs - LastUpdateTimeMs) >= GetUpdateDelayMS(ProgressMode)) + if ((NowMs - LastUpdateTimeMs) >= ProgressOwner->GetProgressUpdateDelayMS()) { LastUpdateTimeMs = NowMs; uint64_t Deleted = DeletedItemCount.load(); uint64_t DeletedBytes = DeletedByteCount.load(); uint64_t Discovered = DiscoveredItemCount.load(); - Progress.UpdateState({.Task = "Removing folders", - .Details = fmt::format("Found {}, Deleted {} ({})", Discovered, Deleted, NiceBytes(DeletedBytes)), - .TotalCount = DirectoriesToDelete.size(), - .RemainingCount = DirectoriesToDelete.size() - SubDirectoryIndex}, - false); + Progress->UpdateState({.Task = "Removing folders", + .Details = fmt::format("Found {}, Deleted {} ({})", Discovered, Deleted, NiceBytes(DeletedBytes)), + .TotalCount = DirectoriesToDelete.size(), + .RemainingCount = DirectoriesToDelete.size() - SubDirectoryIndex}, + false); } } - Progress.Finish(); + Progress->Finish(); uint64_t ElapsedTimeMs = Timer.GetElapsedTimeMs(); if (!Quiet) @@ -549,7 +550,7 @@ WipeCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv) Quiet = m_Quiet; IsVerbose = m_Verbose; - ProgressMode = m_PlainProgress ? ProgressBar::Mode::Plain : ProgressBar::Mode::Pretty; + ProgressMode = m_PlainProgress ? ConsoleProgressMode::Plain : ConsoleProgressMode::Pretty; BoostWorkerThreads = m_BoostWorkerThreads; MakeSafeAbsolutePathInPlace(m_Directory); -- 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/wipe_cmd.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/zen/cmds/wipe_cmd.cpp') diff --git a/src/zen/cmds/wipe_cmd.cpp b/src/zen/cmds/wipe_cmd.cpp index 713c5e386..d5344fb01 100644 --- a/src/zen/cmds/wipe_cmd.cpp +++ b/src/zen/cmds/wipe_cmd.cpp @@ -538,10 +538,10 @@ WipeCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv) using namespace wipe_impl; ZEN_UNUSED(GlobalOptions); - signal(SIGINT, SignalCallbackHandler); + ScopedSignalHandler SigIntGuard(SIGINT, SignalCallbackHandler); #if ZEN_PLATFORM_WINDOWS - signal(SIGBREAK, SignalCallbackHandler); -#endif // ZEN_PLATFORM_WINDOWS + ScopedSignalHandler SigBreakGuard(SIGBREAK, SignalCallbackHandler); +#endif if (!ParseOptions(argc, argv)) { -- cgit v1.2.3