feat(storage): store push-publishing bundles durably in S3 asset storage

[swicken]

S3 asset storage, part 5 of 7. Stacked PRs, review bottom up. Each one builds on the one below.
1a storage layer #37770 · 1b binary asset API #37771 · 2 content #37772 · 3 recovery #37773 · 4 publishing #37774 · 5 temporary uploads and WebDAV #37775 · 6 rendering #37776
Everything is behind FEATURE_FLAG_S3_ASSET_STORAGE, off by default. With the flag off, behavior matches main.

Rebased on 2026-10-06 onto a fix in #37772 (3523198625). This PR's own commits are unchanged; the test counts below were taken before that rebase.

Refs #37868

Proposed Changes

• BundleArchiveStorage stores completed push-publishing archives (<bundle-id>.tar.gz, manifest included) in the publishing-bundles group, with the bundle directory as a cache. Bundle ids keep their case and cannot resolve outside the bundle directory. Stored and received archives use the same id rule the receivers use (the name up to the first .tar.gz).
• Generated and received bundles (BundlePublisherResource, BundleResource, RemotePublishAjaxAction, TarGzipBundleOutput) are staged privately and published to S3 before they replace the last complete archive, so a partial upload never becomes visible. Manifest and payload reads restore the archive only when needed.
• Retention. BinaryCleanupJob expires the S3 copies after the same CLEANUP_BUNDLES_OLDER_THAN_DAYS (default 4) as the local bundle directory.
• Committed bundle deletion records a bundleArchiveCleanup job, which keeps the local bytes if remote deletion fails. If the bundle's row exists again when the job runs, because the bundle was received again, the job ends as a no-op.
• The bundle list and audit pages use a display-only existence check that logs a storage error and shows the bundle as not generated, so the pages still render when S3 is down. Publishing and retry decisions keep the strict check.

Behavior with the flag off

Unchanged from main. The bundle path is built the same way (only the File construction differs), and the new path checks apply only with the flag on. The bundle pages now evaluate the existence check once per bundle instead of up to three times, with the same result. No REST annotations change, so openapi.yaml is unchanged.

Review fixes

The last commit on this branch (fix(storage): bound durable bundle retention and harden flag-on bundle paths) addresses a full review of this PR. All of it is flag-on only:

• S3 bundle archives used to be kept forever; they now expire with the local ones.
• The bundle pages no longer make an S3 request per row on every check, and no longer fail to render when S3 is unavailable.
• Static publishing fails with a retryable error when a file asset's binary is missing, as it does with the flag off, instead of silently publishing without the file. The cache lease is held across the lookup and the copy.
• Store and read derive the bundle id the same way.
• The cleanup job's check-then-delete is closed against a re-received bundle on the same node, by a per-bundle lock held by the store and by the job. Two narrow windows remain and are documented: another cluster node, and a receive whose row has not committed yet. The worst case is a failed receive that the sender retries, with no data loss.
• An S3 error during download cleanup is caught and logged instead of escaping the handler.

Checklist

• Tests: the 174 unit tests in the doc's run command pass after the review fixes (1 skip, STS), including the new BundleArchiveStorageTest and FileAssetBundlerTest. Integration on this branch before the review fixes, flag off: 75 run, 0 failures, across the new PublishingArchiveStorageTest and eight existing publishing tests (PublisherAPIImplTest x2, BundlePublisherTest, BundlerUtilTest, BundlerUtilIntegrationTest, ManifestReaderFactoryTest, ManifestUtilTest, BundleResourceTest, PushPublishBundleGeneratorTest); flag on: 21 run, 0 failures. After the review fixes, PublishingArchiveStorageTest and BundleResourceTest passed flag off and on in the run on the top of the stack (feat(storage): serve renditions, compiled CSS and templates through S3 asset storage #37776).
• Translations: not applicable.
• Security Implications Contemplated: with the flag on, bundle ids are validated and must resolve inside the bundle directory before any path is used, and received uploads are written to a separate staging file.

[claude]

Claude finished @jcastro-dotcms's task in 4m 1s —— View job

---

Review: durable S3 push-publishing bundles (#37774)

• Read core new/changed files
• Analyze bugs, security, error paths, replay safety
• Recheck prior rollback-safety findings
• Post review

This is a careful, well-staged change. The private-staging-then-publish pattern (TarGzipBundleOutput.create()→complete(), BundleArchiveStorage.receive()) correctly prevents a partial archive from ever becoming visible; complete() closes and nulls the stream before close() runs, so there's no double-close and the temp is always cleaned up. Path handling in localArchive() and the delete()/ManifestUtil canonical-path checks block traversal, bundle ids keep their case via normalizePath, and the SQL in the cleanup job uses addParam(). The strict (exists) vs display (existsForDisplay) split is applied consistently — publish/retry use the throwing check, the JSPs and audit page use the logged-and-false check — and the JSPs now evaluate it once per row. Flag-off paths are untouched.

New Issues

• 🟡 Medium: dotCMS/src/main/java/com/dotcms/publishing/output/BundleArchiveCleanupProcessor.java:49-51 — If FEATURE_FLAG_S3_ASSET_STORAGE is toggled off after a bundle delete has already enqueued a bundleArchiveCleanup job (the enqueue happened while it was on), process() throws JobProcessingException on every attempt and can never succeed while the flag is off. The job exhausts its retries and the S3 archive is orphaned — BinaryCleanupJob.expireDurableBundles also no-ops when the flag is off (BundleArchiveStorage.expireOlderThan returns 0), so nothing reclaims it until the flag is turned back on. No data loss, but worth considering an idempotent no-op/skip (as the "bundle exists again" branch already does) rather than a hard failure for the flag-off case. Assumption: operators may disable the flag with in-flight cleanup jobs queued. What to verify: retry-policy terminal behavior and whether an operator ever expects to disable the flag after enabling it.

Prior findings (rollback-safety bot comments)

• The two 🟠 H-5 "unsafe to rollback" notices are accurate and are not code bugs — they're the inherent forward-only consequence of treating the local bundle directory as a cache in front of S3. This PR documents exactly that in docs/testing/BINARY_S3_STORAGE.md ("## Publishing bundles" + the pre-existing "Enabling the flag is not rollback-safe" warning), consistent with how binary assets are already handled. No change required beyond the operational awareness the doc already provides.

Notes (non-blocking)

• getBundleTarGzipFile() now resolves via BundleArchiveStorage.get(), which does a full pullFile (S3 download) on every call. In the existence-check callers (PublishingRetryHelper.verifyBundle:286, RemotePublishAjaxAction:413) this downloads the whole archive just to test .exists(). It's not wasted — those paths go on to read the bundle, so the download primes the cache — but if any future caller only needs presence, prefer BundleArchiveStorage.exists().

Nothing here is blocking. The one Medium is a narrow flag-toggle edge with no data-loss impact.

· s3-stack/4-publishing

[claude]

Pull Request Unsafe to Rollback!!!

• Category: H-5 — Binary/Storage Provider Configuration Change
• Risk Level: 🟠 HIGH
• Why it's unsafe: This PR wires push-publishing bundle archives (<bundle-id>.tar.gz + manifest) into the existing FEATURE_FLAG_S3_ASSET_STORAGE opt-in storage chain, which docs/testing/BINARY_S3_STORAGE.md already documents (pre-existing text, unchanged by this PR) as "Enabling the flag is not rollback-safe." BundleArchiveStorage (new class) treats the local bundle directory as merely a cache in front of S3 — the doc's new "## Publishing bundles" section states archives are "stored durably in the publishing-bundles group, with the local bundle directory as a cache." BundleArchiveStorage.store()/.get() (BundleArchiveStorage.java:104-137) dual-write to local FS + S3 at creation/receive time, but that local copy is not durable across node/pod lifecycle: in a container-orchestrated rollback (redeploy to the N-1 image), the new N-1 pod starts with an empty/ephemeral local bundle directory. N-1 has no knowledge of BundleArchiveStorage/S3 and only ever does plain File I/O against the conventional path (e.g. PublishingRetryHelper.java, BundlePublisher.java, RemotePublishAjaxAction.java before this diff). Any bundle whose bytes only live in S3 relative to that fresh pod (i.e., not re-pulled onto that specific node) becomes unreadable after rollback — bundleFile.exists() returns false, and push-publish retry/receive/download flows fail with "No Bundle Found" / "not found" errors. The integration test added in this PR (PublishingArchiveStorageTest.java:74-78, assertRemoteCopyAndClearLocal + assertFalse(archive.exists(), "Listing a bundle must not download its archive")) itself demonstrates that the local file is expected to go missing while only the S3 copy remains authoritative — exactly the condition N-1 cannot handle.
• Code that makes it unsafe: dotCMS/src/main/java/com/dotcms/publishing/output/BundleArchiveStorage.java (new file, the whole class), dotCMS/src/main/java/com/dotcms/publishing/output/TarGzipBundleOutput.java (getBundleTarGzipFile now delegates to BundleArchiveStorage.getInstance().get(bundleId)), dotCMS/src/main/java/com/dotcms/storage/AssetStorageFeature.java (adds BundleArchiveCleanupProcessor to the shared FEATURE_FLAG_S3_ASSET_STORAGE lifecycle), docs/testing/BINARY_S3_STORAGE.md ("## Publishing bundles" section + the pre-existing, still-applicable "Enabling the flag is not rollback-safe" section).
• Alternative (if possible): Per H-5's safer alternative, guarantee the legacy local path is always populated and never treated as merely a cache for this data type — i.e., never evict/rely-on-S3-only for bundle archives, or explicitly extend the existing "not rollback-safe" doc warning to call out push-publishing bundles by name so operators know that enabling the flag also makes bundle rollback forward-only, consistent with how binary assets are already documented.

[claude]

Pull Request Unsafe to Rollback!!!

• Category: H-5 — Binary / Storage Provider Change (adapted to push-publishing bundle archives)
• Risk Level: 🟠 HIGH
• Why it's unsafe: When FEATURE_FLAG_S3_ASSET_STORAGE is on, completed push-publishing archives are now stored durably in S3's publishing-bundles group, with the local bundle directory acting only as a cache (new BundleArchiveStorage). N-1 (the pre-PR binary) has no concept of BundleArchiveStorage or the publishing-bundles S3 group — it only knows how to read/write <bundle-id>.tar.gz directly under ConfigUtils.getBundlePath(). After a rollback, any archive whose bytes live primarily in S3 (generated/received on a different cluster node, or evicted/never cached locally on this node) becomes invisible to N-1: downloads, retries, and the sender/receiver code paths (RemotePublishAjaxAction, BundleResource, PublishingRetryHelper, BundlerUtil.tarGzipExists) will report the bundle as missing even though it still exists in S3. This is the same forward-only storage model that docs/testing/BINARY_S3_STORAGE.md already documents as 'not rollback-safe' for binary assets — this PR extends that identical risk to push-publishing bundles (see the PR's own new '## Publishing bundles' section in that same doc).
• Code that makes it unsafe: dotCMS/src/main/java/com/dotcms/publishing/output/BundleArchiveStorage.java (new — store(), receive(), get(), delete(), expireOlderThan() route archives through the S3-backed chain with local disk as cache only); dotCMS/src/main/java/com/dotcms/publishing/output/TarGzipBundleOutput.java (getBundleTarGzipFile() now resolves via BundleArchiveStorage.getInstance().get(bundleId)); dotCMS/src/main/java/com/dotcms/publishing/BundlerUtil.java (tarGzipExists()/bundleExists() now check BundleArchiveStorage when the flag is on).
• Alternative (if possible): The flag is opt-in and off by default, so most deployments are unaffected on rollback. For deployments that enable FEATURE_FLAG_S3_ASSET_STORAGE, treat it as the PR's own doc already recommends for binaries — a forward-only operational decision — and call out in release notes that rolling a node back after this flag has been enabled is unsafe for any push-publishing bundle generated or received while it was on.