Issue 35310 block folder delete when not empty

[dsilvam]

Summary

Cherry-pick of PR #35398 — blocks folder deletion when the folder contains live content.

• Prevents deleting folders that contain live (published) content items
• Adds a configurable flag (BLOCK_FOLDER_DELETE_WITH_LIVE_CONTENT) to enable/disable the guard
• Uses lazy initialization for the config flag

Cherry-picked commits

• 839ed42 block-folder-delete-with-live-content
• 32e3936 Issue 35310: block folder delete when not empty
• 1a12d71 Use Lazy for folder delete config flag

Original PR: #35398

Test plan

• Attempt to delete a folder containing live content — should be blocked
• Attempt to delete a folder with only unpublished content — should succeed
• Verify config flag BLOCK_FOLDER_DELETE_WITH_LIVE_CONTENT toggles the behavior

🤖 Generated with Claude Code

This PR fixes: #35310

This PR fixes: #35310

[claude]

Claude finished @dsilvam's task in 13m 22s —— View job

---

dotCMS Backend Review — PR #35500

• Gather context (CLAUDE.md, docs, diff)
• Run security review
• Run database review
• Run Java standards review
• Run REST API / config review
• Consolidate findings and post review

---

Review Findings

[High] EditFolderAction.java:450 — Permission check executes after content probe (information disclosure)

The emptiness guard runs before the user is resolved and before any permission enforcement. A low-privilege caller can distinguish "folder has hidden content" from "folder is empty" by observing which session message is returned — a classic information-disclosure via response differentiation.

if (isDeleteNotEmptyFolderProtectionEnabled() && isNotEmpty(f)) { // uses systemUser BEFORE auth
    SessionMessages.add(req, "message", DELETE_NOT_EMPTY_FOLDER_MESSAGE_KEY);
    return;
}
User user = _getUser(req);          // permission context resolved too late
folderAPI.delete(f, user, false);   // actual permission enforcement

Fix: Move User user = _getUser(req) and a doesUserHavePermission check above isNotEmpty, then pass user (not systemUser) into isNotEmpty.

Fix this →

---

[High] EditFolderAction.java:73 — Lazy<Boolean> permanently caches config; runtime changes silently ignored

Config.getBooleanProperty() is the dotCMS standard for runtime-reconfigurable flags (System Properties UI). Lazy.of() freezes the value at first access for the JVM lifetime — an admin toggling this property sees no effect until server restart.

private static final Lazy<Boolean> DELETE_NOT_EMPTY_FOLDER_PROTECTION =
        Lazy.of(() -> Config.getBooleanProperty(DELETE_NOT_EMPTY_FOLDER_PROPERTY, false));

Fix: Drop Lazy entirely. Call Config.getBooleanProperty(DELETE_NOT_EMPTY_FOLDER_PROPERTY, false) directly inside isDeleteNotEmptyFolderProtectionEnabled().

Fix this →

---

[High] EditFolderAction.java:539 — Blocks on any content, but PR description says live content only

folderAPI.getContent() returns all working contentlets including unpublished drafts. getHTMLPages(folder, false, ...) fetches working (non-live) pages. A folder with only drafts is blocked, contradicting the test plan: "Attempt to delete a folder with only unpublished content — should succeed".

return !folderAPI.getContent(folder, user, false).isEmpty()               // all working, incl. drafts
    || !htmlPageAssetAPI.getHTMLPages(folder, false, false, user, false).isEmpty() // working pages
    || !htmlPageAssetAPI.getHTMLPages(folder, true, false, user, false).isEmpty(); // live pages

Fix: Either (a) update description/test plan to say "non-empty folder (any content)", or (b) restrict to live-only — keep only the live=true call and replace getContent with a live-filtered query.

---

[Medium] EditFolderAction.java:487 — isNotEmpty(Folder) always uses system user, expanding information-disclosure surface

private boolean isNotEmpty(final Folder folder)
        throws DotDataException, DotStateException, DotSecurityException {
    return isNotEmpty(folder, APILocator.getUserAPI().getSystemUser()); // bypasses caller ACL
}

Fix: Remove the single-argument overload; pass the authenticated user from _deleteFolder through the call chain.

---

[Medium] EditFolderAction.java:541 — Redundant getHTMLPages call; working pages cover live pages for emptiness detection

|| !htmlPageAssetAPI.getHTMLPages(folder, false, false, user, false).isEmpty()
|| !htmlPageAssetAPI.getHTMLPages(folder, true, false, user, false).isEmpty();

For non-emptiness, live=false (working) is sufficient — any live page also has a working version. The live=true call is redundant and fires an extra DB query per folder.

Fix: Remove the live=true call.

---

[Medium] EditFolderAction.java:539 — folderAPI.getContent() is @Deprecated

!folderAPI.getContent(folder, user, false).isEmpty()

Fix: Replace with APILocator.getContentletAPI().findContentletsByFolder(folder, user, false).isEmpty().

---

[Medium] EditFolderAction.java:71 / dotmarketing-config.properties:35 — Double-negative property name violates dotCMS naming conventions

no.delete.notempty.folder is a compound negation — no + notempty — making intent ambiguous at a glance. The PR description itself uses the clearer BLOCK_FOLDER_DELETE_WITH_LIVE_CONTENT. Most boolean feature flags use SCREAMING_SNAKE_CASE.

private static final String DELETE_NOT_EMPTY_FOLDER_PROPERTY = "no.delete.notempty.folder";

Fix: Rename to BLOCK_FOLDER_DELETE_WHEN_NOT_EMPTY and update the constant and properties file.

---

[Medium] dotmarketing-config.properties:33 — Property inserted inside a language-fallback block

The new property sits between DEFAULT_PERSONA_TO_DEFAULT_LANGUAGE and PER_PAGE, unrelated to the surrounding language settings.

Fix: Move to a ## FOLDER SETTINGS section or near other content/asset management properties.

---

Next steps

• High severity: Fix locally and push — these need your judgment
• Medium severity: You can ask me to handle mechanical fixes inline: @claude fix <issue> in EditFolderAction.java
• Every new push triggers a fresh review automatically

[claude]

Full review posted in Claude comment 4345258647 on this PR.