feat(storage): share temporary uploads and WebDAV temporary files through S3

[swicken]

S3 asset storage, part 6 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

• Temporary uploads (TempFileAPI, image editor output) are stored in the temporary-assets group with an immutable receipt recording who may use the upload and when it expires. Access and expiry are checked before any bytes download, so another node can serve the upload. A payload is published only after its stream closes. Custom metadata on a temporary upload, such as a focal point, is stored with it in S3. Focal points the image filter writes for a content image (a temp_ id with no receipt) stay on the local path that BinaryCleanupJob already ages out.
• WebDAV temporary files publish payloads and path records in the webdav-temporary group with conditional writes, so a stale writer cannot publish after a deletion; cleanup tombstones expired records. A folder listing reads only the records of its direct children and builds each resource from the record it read, so a file deleted during the listing is skipped, and an S3 failure leaves out only the temporary children instead of failing the CMS listing.
• WebDAV COPY creates an unpublished working version, needing only edit permission, with either flag setting.
• BinaryCleanupJob also expires S3 temporary uploads and WebDAV files. At expiry it removes the S3 objects and leaves the local directory to the existing CLEANUP_TMP_FILES_OLDER_THAN_HOURS rule, and it handles each upload separately so one bad receipt cannot stop the pass.
• Test fixes. FocalPointAPITest now completes its temporary uploads the way the upload API and image editor do; with the flag on, an unfinished upload is intentionally not a usable temp resource. DotWebdavHelperTest retries its eviction asserts briefly, because eviction deliberately declines while a background reindex holds a cache lease.
• DotWebdavHelperTest is now registered in MainSuite2a. It was never in a suite, so CI never ran it. MainSuite2a was the fastest suite in six recent PR runs (about 22 to 35 minutes against 37 to 47 for the others), following the registration guide in docs/testing/INTEGRATION_TESTS.md. Its header comment says to avoid adding tests to it; that comment looks out of date against the timings, so please say if you would rather it went elsewhere.

Behavior with the flag off

Unchanged from main; WebDAV and temporary uploads use the local filesystem as before.

Review fixes

The last commit on this branch (fix(storage): keep WebDAV copies unpublished and ...) addresses a full review of this PR. All of it is flag-on only:

• WebDAV COPY on the live and autopub mounts used to publish the copy and require publish permission with the flag on.
• Image-filter focal points for content images used to go to S3 and never expire.
• One temporary entry that was deleted mid-listing, or an S3 error, used to fail the whole folder listing.
• Listings used to read every record under the folder's subtree. The remaining cost, one read per direct child including deleted ones until tombstones can be reclaimed, is documented next to the tombstone note.
• Cleanup used to delete a temporary upload's local bytes as soon as it expired, and stopped at the first unreadable receipt.
• New WebdavTemporaryStorageTest covers the listing changes against an in-memory remote that models ETags, so they run in CI.

Checklist

• Tests: the 197 unit tests in the doc's run command pass after the review fixes (1 skip, STS), including the new WebdavTemporaryStorageTest. Integration on this branch before the review fixes, flag off: 124 run, 0 failures, including the existing TempFileAPITest, TempFileResourceTest, BinaryCleanupJobTest, BinaryExporterServletTest and DotWebdavHelperTest; flag on: 85 run, 0 failures. After the review fixes, DotWebdavHelperTest, TempFileAPITest, BinaryCleanupJobTest, BinaryExporterServletTest and FocalPointAPITest 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: a managed upload without a receipt cannot fall back to local access, access is checked before any download, and upload paths are validated against the temporary root and symlinks.

[claude]

Claude finished @jcastro-dotcms's task in 3m 16s —— View job

---

Code Review

I reviewed the diff against origin/s3-stack/4-publishing, focusing on the new storage classes and the flag-on WebDAV/temp paths, and rechecked the four open review-thread findings.

The PR is careful and consistent: everything is gated behind AssetStorageFeature.isEnabled(), paths are validated against the temp root and symlinks, S3 writes use conditional (If-Match) semantics with reservation/rollback, and cleanup is per-item and reference-aware. I could not prove any new P0/P1 bug from the diff. The findings below are non-blocking resilience/behavior points, mostly tracking the existing review threads.

Existing

• 🟡 Medium: dotCMS/src/main/java/com/dotcms/storage/WebdavTemporaryStorage.java:146 — stat() wraps IOException/DotDataException in an unchecked DotRuntimeException, and TempFolderResourceImpl.getModifiedDate() (TempFolderResourceImpl.java:172) calls it with no guard. A transient S3 read error during a read-only PROPFIND now fails hard, where the local path returned a date and never threw. This is consistent with the "fail loud on S3 error" choice used across the flag-on temp surface (getChildren, getContentLength, getFile all throw), so it's a deliberate trade-off — but getModifiedDate is the one spot that was previously exception-free in a listing path. Consider swallowing transient errors to a folder-time/null fallback here. Fix this →

• 🟡 Medium: dotCMS/src/main/java/com/dotcms/rest/api/v1/temp/TempFileAPI.java:138 — the .s3-upload marker is written at create time, before any receipt exists. If the upload never completes (client abort, or store() throws on an S3 outage), getTempFile/isTempResource see isManagedLocally(id) == true with no receipt and return empty/false with no legacy fallback, so an otherwise-servable local copy is fenced until BinaryCleanupJob ages it out. This is partly intended (the PR states an unfinished upload is deliberately not a usable temp resource with the flag on), but the client-abort case discards a valid local copy that the flag-off path would serve. Writing the marker only after store() verifies the S3 copy (where it is already written) would avoid the fenced window.

• 🟡 Medium: dotCMS/src/main/java/com/dotmarketing/webdav/ResourceFactoryImpl.java:59 — Assumption: WebdavTemporaryStorage.key() records a temp entry's CMS-prefix components with the case they were created with, while reads of the same path are lowercased up to the first temp component by temporaryPath(), and BasicFolderResourceImpl.createNew builds the write path via stripMapping(originalPath) without lowercasing. What to verify: that a temp file created under a mixed-case CMS prefix (e.g. /HOST/… vs /host/…) round-trips — write key and read key must match, or stat() returns null and the resource looks missing. If write and read normalize the prefix differently this is a real lookup miss; if both paths already resolve the host/folder to a canonical case before building the key, it's a non-issue.

Resolved

• ✅ dotCMS/src/main/java/com/dotmarketing/webdav/DotWebdavHelper.java:656 — createTempFile returning early (no parent-dir creation) with the flag on is safe: every flag-on caller (BasicFolderResourceImpl.createNew, TempFolderResourceImpl.createNew) writes through DotWebdavHelper.writeCompletedTempFile, which does Files.createDirectories(target.getParent()) before the move. No NoSuchFileException path.

---

Checks performed: flag-off behavior is preserved (every change is behind an isEnabled() branch); S3 writes are idempotent/fenced via writeObjectIfMatch + reservation/rollback in store() and tombstones in cleanupExpired(); cleanupExpired() isolates per-upload failures and rethrows a combined error so the job still reports; path validation rejects symlinks, .., absolute keys and reserved names. No SQL/secret/logging issues in the diff.

· branch s3-stack/5-webdav-temp

[claude]

Pull Request Unsafe to Rollback!!!

• Category: H-5 — Binary Storage Provider Change
• Risk Level: 🟠 HIGH
• Why it's unsafe: This PR extends dotCMS's existing FEATURE_FLAG_S3_ASSET_STORAGE provider abstraction (previously scoped to committed binary assets, renditions, and publishing bundles) to also cover temporary uploads and WebDAV staging files. When the flag is on, TempFileAPI.createTempFile/completeTempFile now store the actual upload bytes in S3 (group temporary-assets) behind an immutable JSON "receipt", leaving only a local .s3-upload marker file — no real content on local disk. DotWebdavHelper does the same for WebDAV scratch files via the new webdav-temporary S3 group. N-1 has none of TemporaryAssetStorage/WebdavTemporaryStorage and unconditionally assumes temp files live at the plain local filesystem path built from tempFileId. After a rollback to N-1, while the flag remains on in configuration, any temp upload or WebDAV staging file created/completed by N (S3-only, local disk is just a marker) is unreadable by N-1: TempFileAPI.getTempFile, isTempResource, and DotWebdavHelper.loadTempFile/getResourceContent will find no real bytes locally. In-flight uploads, image-editor edits, and WebDAV clients mid-upload during the rollback window break (missing files, failed check-ins, broken image previews) until those temp resources age out (TEMP_RESOURCE_MAX_AGE_SECONDS, default 1800s).
• Code that makes it unsafe:
  • dotCMS/src/main/java/com/dotcms/storage/TemporaryAssetStorage.java (new) — file(), store(), receipt(), retrieve(): routes completed temp-upload bytes to S3 with only a local .s3-upload marker (MANAGED_MARKER) left on disk.
  • dotCMS/src/main/java/com/dotcms/storage/WebdavTemporaryStorage.java (new) — store(), materialize(), state(): WebDAV scratch files are published as S3 objects referenced by a dataKey, with local files treated as disposable cache copies.
  • dotCMS/src/main/java/com/dotcms/rest/api/v1/temp/TempFileAPI.java lines ~114-230, ~392-410, ~450-465: createTempFile, completeTempFile, getTempFile(List, String), isTempResource branch on AssetStorageFeature.isEnabled() to read/write via TemporaryAssetStorage instead of the plain local File.
  • dotCMS/src/main/java/com/dotmarketing/webdav/DotWebdavHelper.java — temporaryPath(), loadTempFile(), createTempFolder(), writeCompletedTempFile(), copyTempFileToStorage(): reroute WebDAV temp file I/O through WebdavTemporaryStorage when the flag is enabled.
• Alternative (if possible): Same as the reference's H-5 alternative — treat this scope expansion of the storage provider as infrastructure configuration, not a regular-release change: either (a) don't flip FEATURE_FLAG_S3_ASSET_STORAGE to true in any environment until N-1 is fully retired from the rollback window, or (b) keep a fallback read path so TempFileAPI/DotWebdavHelper on N-1 can still resolve temp resources that N wrote to S3 (e.g., write a compatibility local copy alongside the S3 receipt during the transition release), removing the local fallback only in a later release.

[claude]

Pull Request Unsafe to Rollback!!!

• Category: H-5 — Binary Storage Provider Change
• Risk Level: 🟠 HIGH (conditional — only manifests where the feature flag has been turned on; the change ships disabled by default)
• Why it's unsafe: The whole PR is gated by AssetStorageFeature.isEnabled(), backed by FEATURE_FLAG_S3_ASSET_STORAGE which defaults to false (AssetStorageFeature.java:20,42), so for the vast majority of deployments N-1 behaves identically after rollback — this PR is safe by default. But for any environment that turns the flag on, this introduces a brand-new S3-backed storage model for temp uploads (TemporaryAssetStorage) and WebDAV staging (WebdavTemporaryStorage) that N-1's binary has no code path for at all (these are new classes, not present pre-PR). Concretely:
  • TemporaryAssetStorage.store() (TemporaryAssetStorage.java:411-439) uploads completed temp-file bytes to S3 and stamps a local .s3-upload marker (MANAGED_MARKER, line 293). TempFileAPI.getTempFile/isTempResource (TempFileAPI.java:392-408, 450-464) check store.isManagedLocally(id) and, if true, refuse to fall back to the legacy local-file lookup. After rollback, N-1's (pre-PR) TempFileAPI doesn't know about any of this — it just reads the plain local temp path — so any temp upload that was routed through S3 while the flag was on is unreadable/inconsistent once N-1 is running again.
  • WebdavTemporaryStorage is worse: when the flag is enabled, the entire WebDAV staging directory tree (files and folders) is represented only as S3 objects under entries//data/ prefixes (see mkdir, store, children in WebdavTemporaryStorage.java:705-752, 685-703) — there is no local filesystem mirror of the hierarchy at all. DotWebdavHelper.createTempFile/createTempFolder (DotWebdavHelper.java:945-1007, 1048-1057) skip creating the local directory structure entirely when the flag is on. N-1's WebDAV code (getTempDir().getPath() + url, unchanged pre-PR logic) has zero way to see those S3-only entries — any in-progress WebDAV upload/folder created while the flag was on becomes invisible after rollback.
  • The data is transient (temp uploads expire in TEMP_RESOURCE_MAX_AGE_SECONDS — default 1800s; WebDAV staging expires in CLEANUP_TMP_FILES_OLDER_THAN_HOURS — default 3h) and self-heals once expired, and no persisted/published content or core entity storage is touched — hence HIGH rather than CRITICAL, and conditional on flag adoption rather than universal.
• Code that makes it unsafe:
  • dotCMS/src/main/java/com/dotcms/storage/AssetStorageFeature.java:20,42 (the flag and its default)
  • dotCMS/src/main/java/com/dotcms/storage/TemporaryAssetStorage.java (new file — S3-backed temp file storage, no N-1 equivalent)
  • dotCMS/src/main/java/com/dotcms/storage/WebdavTemporaryStorage.java (new file — S3-only WebDAV staging tree, no N-1 equivalent)
  • dotCMS/src/main/java/com/dotcms/rest/api/v1/temp/TempFileAPI.java:120-126, 392-408, 450-464
  • dotCMS/src/main/java/com/dotmarketing/webdav/DotWebdavHelper.java:945-1007, 1048-1057, 1058-1090
• Alternative (if possible): This already follows the safer pattern for the default case (never flips the default provider — H-5's first recommendation). For the opt-in path itself, apply H-5's second recommendation more completely: keep writing a local filesystem mirror of the WebDAV staging tree (not just temp files) while the flag is on, so a rollback finds the same files/folders N-1 expects. Where that isn't feasible, call this out explicitly in release notes as "do not roll back while FEATURE_FLAG_S3_ASSET_STORAGE is/was enabled without first draining in-flight temp/WebDAV uploads (wait out the TTL or disable the flag and let cleanup run first)."

[claude]

test

[claude]

(Disregard the previous comment — it was posted in error while verifying tooling access.)

[claude]

Pull Request Unsafe to Rollback!!!

• Category: H-5 — Binary Storage Provider Configuration Change
• Risk Level: 🟡 MEDIUM
• Why it's unsafe: This PR extends the existing (default-off) FEATURE_FLAG_S3_ASSET_STORAGE flag (AssetStorageFeature.isEnabled(), default false in dotCMS/src/main/java/com/dotcms/storage/AssetStorageFeature.java:42) to temporary uploads and WebDAV scratch files. For actual file bytes, the implementation is careful to dual-write to both the local filesystem and S3 (e.g. WebdavTemporaryStorage.store() and DotWebdavHelper.writeCompletedTempFile() both push to S3 and atomically move the completed file to its local path), so N-1 can generally still find the bytes locally after a rollback. However, FileMetadataAPIImpl.putCustomMetadataAttributes() does not follow that pattern: when the flag is enabled, custom metadata for a temp resource (e.g. a focal point set via the image editor before check-in) is written only to remote storage via fileStorageAPI.setMetadata(...storageKey(temporaryMetadataKey(tempResourceId, false))...), with no local mirror write. N-1 has no code path aware of this S3 metadata key — it only knows the legacy local tempResourcePath(id) — so after a binary-only rollback (the config flag itself doesn't revert with the binary), any custom temp-resource metadata written during N's tenure is silently invisible to N-1.
• Code that makes it unsafe: dotCMS/src/main/java/com/dotcms/storage/FileMetadataAPIImpl.java, new putCustomMetadataAttributes branch (~lines 1033-1052): the if (AssetStorageFeature.isEnabled()) block builds updated and calls fileStorageAPI.setMetadata(..., temporaryMetadataKey(tempResourceId, false), updated) then returns — no corresponding local-path write exists in that branch (contrast with temporaryMetadata(id) at lines ~1018-1023, which does read-fallback to the legacy local path, but only for reads, not writes).
• Alternative (if possible): Apply the same dual-write pattern already used for the binary content paths — write custom metadata to both the local legacy path and the S3 key while the flag is enabled, or apply the two-phase pattern from the reference doc (write-to-both in this release, cut over to S3-only in a later release once N-1 is fully retired).

Note: this is narrow in practice — FEATURE_FLAG_S3_ASSET_STORAGE defaults to false, so it only matters for environments that have explicitly opted in, and the affected data is ephemeral pre-check-in temp-resource metadata (max lifetime governed by TEMP_RESOURCE_MAX_AGE_SECONDS, default 1800s), not persisted content. No backup restore or reindex is required to recover; affected users would just need to re-apply the metadata (e.g. re-set the focal point) after a rollback.

[dotCMS-Machine-User]

✅ dotbot review: all reviewer models (meta/muse-spark-1.3, ~z-ai/glm-latest) agree — patch is correct.

_{approved automatically by dotbot}

[github-actions]

Reject new WebDAV temp uploads whose name lacks a temp-resource component in path validation

Current code:

public File createTempFile(String path) throws IOException{
	File file = new File(getTempDir().getPath() + temporaryPath(path));
	if (com.dotcms.storage.AssetStorageFeature.isEnabled()) return file;

Problem: With the flag on, createTempFile no longer creates parent directories, and callers that pass plain file paths rely on writeCompletedTempFile (which does Files.createDirectories). Verify all callers pass paths whose parents get created, or store() fails with NoSuchFileException. See FileMetadataAPIImpl and TempFileAPI for the analogous directory-creation ordering concerns.

[github-actions]

WebdavTemporaryStorage.stat(folder) throws DotRuntimeException on a transient S3 read error

Current code:

} catch (IOException | DotDataException failure) {
    throw new DotRuntimeException("Unable to inspect WebDAV staging file", failure);
}

Problem: TempFolderResourceImpl.getModifiedDate() calls stat(folder), so a transient S3 outage now causes an unchecked exception instead of the previous graceful null/folder-time fallback. A read-only PROPFIND on a WebDAV client fails hard. Consider catching DotRuntimeException in getModifiedDate() or swallowing transient errors in stat.

[github-actions]

case-insensitive temp-resource lookup may miss entries created with case-mapped path

Current code:

if (!com.dotcms.storage.AssetStorageFeature.isEnabled() || !dotDavHelper.isTempResource(url)) url = url.toLowerCase();

Problem: The temporaryPath helper in DotWebdavHelper lowercases only components before the first temp-resource component (e.g. (.DS_Store) or .AppleDouble style entries). But WebdavTemporaryStorage.key() records the temp component with original case, while loadTempFile builds the key with temporaryPath which stops lowercasing at the temp boundary — good. Verify recordPath round-trip matches for entries whose CMS prefix components differ in case between write and read (e.g. .../HOST/ vs .../host/); the CMS prefix is lowercased on read but recorded as-created on write from a different code path (BasicFolderResourceImpl.createNew uses stripMapping(originalPath) without lowercasing). If cases diverge, stat(tempFile) returns null and the resource appears missing.

[github-actions]

mark temp file managed only after completeTempFile succeeds, not during create

Current code:

if (AssetStorageFeature.isEnabled()) {
  try {
    Files.writeString(java.nio.file.Path.of(com.dotmarketing.util.ConfigUtils.getAssetTempPath(),
            tempFileId, TemporaryAssetStorage.MANAGED_MARKER), "");

Problem: The .s3-upload marker is written at creation, before any receipt exists. If the upload never completes (client abort, or store() fails on an S3 outage), the local bytes are fenced off: getTempFile returns empty because isManagedLocally is true but receipt() is absent, and there is no legacy fallback. Until BinaryCleanupJob ages the file out, a valid local copy is unreadable even though the flag-off path would serve it. Consider writing the marker only after store() verifies the S3 copy (store() already writes it), or removing the marker in store()'s failure path.

[claude]

...

[claude]

Pull Request Unsafe to Rollback!!!

• Category: H-5 — Binary Storage Provider Change
• Risk Level: 🟡 MEDIUM
• Why it's unsafe: This PR extends the existing opt-in FEATURE_FLAG_S3_ASSET_STORAGE flag (default off, unchanged by this PR - see AssetStorageFeature.java) to two new groups: temporary uploads (TempFileAPI/TemporaryAssetStorage) and WebDAV scratch files (WebdavTemporaryStorage). For a site that already runs with this flag enabled (the flag predates this PR and already covers binary/generated assets and publishing bundles), upgrading to this version means in-flight temp uploads and WebDAV staging files start living in S3 with an immutable receipt instead of the local filesystem. If that site then rolls back to N-1, the N-1 binary has no knowledge of TemporaryAssetStorage/WebdavTemporaryStorage and only looks at the local temp directory - any upload/staging file that was written to S3 under N is invisible to N-1. Per FileMetadataAPIImpl.temporaryMetadata() and TemporaryAssetStorage's managed-marker check, a managed upload with an S3 receipt has no legacy local fallback.
  Impact is intentionally narrow: this only affects ephemeral, in-flight data (an upload mid-session or a WebDAV scratch file), not committed/published content - the user simply has to retry the upload or WebDAV operation. No permanent data loss, no backup/reindex needed, which is why this is MEDIUM rather than HIGH/CRITICAL despite matching the H-5 signal.
• Code that makes it unsafe:
  • dotCMS/src/main/java/com/dotcms/rest/api/v1/temp/TempFileAPI.java - createTempFile/completeTempFile/getTempFile/isTempResource branch on AssetStorageFeature.isEnabled() to read/write via TemporaryAssetStorage instead of the local filesystem.
  • dotCMS/src/main/java/com/dotcms/storage/TemporaryAssetStorage.java (new file) - S3-backed temp upload storage with receipt/expiry semantics; MANAGED_MARKER / isManagedLocally() show there is no local fallback once an upload is S3-managed.
  • dotCMS/src/main/java/com/dotcms/storage/WebdavTemporaryStorage.java (new file) and dotCMS/src/main/java/com/dotmarketing/webdav/DotWebdavHelper.java (temporaryPath, temporaryChildren, createTempFolder, copyTempDirToStorage) - same pattern for WebDAV scratch files.
  • dotCMS/src/main/java/com/dotmarketing/quartz/job/BinaryCleanupJob.java - now also cleans up S3-side temp/WebDAV uploads when the flag is on.
• Alternative (if possible): Already substantially followed - the flag defaults off (AssetStorageFeature.FLAG, default false) and the change is purely additive/opt-in, matching the reference's "never change the default storage provider" guidance. The remaining gap for sites that opt in is the missing read-fallback to local storage for S3-managed temp/WebDAV entries on the N-1 binary; since N-1 can't be patched retroactively, document in release notes that rollback while FEATURE_FLAG_S3_ASSET_STORAGE=true may drop in-flight uploads/WebDAV scratch sessions (users must retry) - the same caveat already accepted for the rest of the S3 asset storage feature.

[github-actions]

dotbot code review:

• Reviewer: meta/muse-spark-1.3 (medium)
• Overall: patch is correct
• New findings this run: 0
• Prior unresolved dotbot findings still relevant: 0
• Active findings total: 0

Prior case-sensitivity note is speculative verification without a proven divergent write/read path in the current patch, so it does not meet flag-worthy evidence standards. No new P0/P1 defects proven in this review run.

Tip: comment with "/dotbot address comments" to attempt automated fixes for unresolved review threads.

_{reviewed by dotbot · meta/muse-spark-1.3 · medium}

[github-actions]

🟡 [P2] TempFileAPI.java:139 managed marker written before upload completes, fencing servable local bytes

Current code:

if (AssetStorageFeature.isEnabled()) {
  try {
    Files.writeString(java.nio.file.Path.of(com.dotmarketing.util.ConfigUtils.getAssetTempPath(),
            tempFileId, TemporaryAssetStorage.MANAGED_MARKER), "");

Problem: Marker is written at create time, before any receipt exists. If the upload aborts or store() fails on an S3 outage, getTempFile sees isManagedLocally true with no receipt and returns empty with no legacy fallback, fencing a valid local copy until cleanup ages it out.

Fix:

if (AssetStorageFeature.isEnabled()) {
  // marker is written by TemporaryAssetStorage.store() once the S3 copy is verified
}

(store() already writes owner.resolve(MANAGED_MARKER); writing it only there, or deleting it in store()'s failure path, closes the window.)

[github-actions]

🟡 [P2] WebdavTemporaryStorage.java:150 stat() throws unchecked on transient S3 error in PROPFIND path

Current code:

} catch (IOException | DotDataException failure) {
    throw new DotRuntimeException("Unable to inspect WebDAV staging file", failure);
}

Problem: TempFolderResourceImpl.getModifiedDate() (TempFolderResourceImpl.java:173) calls stat(folder) unguarded, so a transient S3 read error now fails a read-only PROPFIND that previously never threw.

Fix:

} catch (IOException | DotDataException failure) {
    throw new DotRuntimeException("Unable to inspect WebDAV staging file", failure);
}

(Either catch DotRuntimeException in getModifiedDate() and return null, or degrade stat() errors to a logged null for this listing-only caller. This is consistent with the deliberate "fail loud on S3 error" choice elsewhere, so confirm intent before changing.)

[github-actions]

dotbot code review:

• Reviewer: ~z-ai/glm-latest (medium)
• Overall: patch is incorrect
• New findings this run: 2
• Prior unresolved dotbot findings still relevant: 0
• Active findings total: 2

The change is consistently gated behind the default-off FEATURE_FLAG_S3_ASSET_STORAGE, preserving legacy behavior when the flag is off. S3 paths are validated against traversal/symlinks, writes use conditional reservations with rollback, and cleanup is per-item with retry semantics. The prior case-sensitivity concern is resolved: both the write path (BasicFolderResourceImpl.createNew -> createTempFile -> temporaryPath) and the read path (loadTempFile -> temporaryPath) normalize the CMS prefix identically, so write and read keys match. The remaining points are opt-in-path resilience trade-offs, not blocking correctness bugs.

Tip: comment with "/dotbot address comments" to attempt automated fixes for unresolved review threads.

_{reviewed by dotbot · ~z-ai/glm-latest · medium}