fix(containers): use POST for duplicate + sanitize id in log (#34900)

[dsolistorres]

Summary

• Align the Containers portlet Duplicate action on POST (non-idempotent create semantics): flip DotContainersService.copy() from http.put to http.post so it matches the backend's existing @POST /_copy handler. This resolves the HTTP 405 that broke duplication.
• Harden ContainerResource against log injection: every Logger.* call that concatenated a caller-controlled string (path/query containerId, body containerForm.getIdentifier(), body hostId, and bulk id lists) now runs through SecurityUtils.sanitizeForLogging(...), which strips CR/LF and control characters. Trusted values (objects returned from successful API lookups, Throwable second-args) are left alone.
• Update the matching frontend unit test (dot-containers.service.spec.ts) to assert POST.

Fixes

• Closes [DEFECT] Cannot duplicate containers #34900

Files changed

• core-web/apps/dotcms-ui/src/app/api/services/dot-containers/dot-containers.service.ts — http.put → http.post
• core-web/apps/dotcms-ui/src/app/api/services/dot-containers/dot-containers.service.spec.ts — expect POST
• dotCMS/src/main/java/com/dotcms/rest/api/v1/container/ContainerResource.java — wrap every untrusted-input Logger.* call with SecurityUtils.sanitizeForLogging(...):
  • resolveHost (warn): hostId
  • update (error): containerForm.getIdentifier()
  • getLiveById / getWorkingById (debug + error): containerId
  • publish / unpublish / archive / unarchive / delete (debug + error): containerId
  • copy (error): id
  • bulkDelete / bulkPublish / bulkUnpublish / bulkArchive / bulkUnarchive (debug on joined payload + error on loop containerId)
• dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml — regenerated
• dotcms-postman/src/main/resources/postman/Containers.postman_collection.json — no net change vs. main (verb stays POST)

Test plan

• yarn nx test dotcms-ui --testPathPattern=dot-containers.service.spec
• ./mvnw verify -pl :dotcms-postman -Dpostman.test.skip=false -Dpostman.collections=Containers
• In the UI, open the Containers portlet, pick a container, choose Duplicate from the action menu, and confirm the copy is created with no 405 in the browser network tab or server logs.
• Smoke-check the log-sanitization path by requesting, e.g., /api/v1/containers?containerId=bogus%0AINJECTED against publish/unpublish/archive/delete and confirming the CR/LF in the id is replaced with _ in the server log.

Notes / out of scope

• ActivityLogger.logInfo(...) audit-log calls and JsonUtil.getJsonStringFromObject(containerForm) dumps were intentionally left alone; JSON serialization already escapes CR/LF, and audit-log hardening can be addressed in a follow-up if desired.
• Pre-existing Sonar/IDE warnings in unrelated methods (raw generics on ResponseEntityView, etc.) were not touched to keep the diff focused.

🤖 Generated with Claude Code

[claude]

Claude finished @dsolistorres's task in 3m 16s —— View job

---

PR Review

• Gather context
• Load dotCMS conventions
• Get Java diff
• Run specialized sub-agent reviews (security, database, Java standards, REST API)
• Consolidate findings and post review

All four reviewers completed. The backend review comment has been updated — all previous findings are resolved in this push. No new issues found.

[claude]

Claude finished @dsolistorres's task in 1m 15s —— View job

---

PR Review

• Gather context
• Read changed files
• Review diff and provide feedback

---

Fix is correct and complete.

The root cause was a clear mismatch: DotContainersService.copy() has always issued http.put (service.ts:213), while the JAX-RS handler was still annotated @POST. This PR closes that gap and the three changed files are all that needed updating.

Sibling consistency: TemplateResource (line 836) and SiteResource (line 1477) both already use @PUT on their _copy paths — this change makes ContainerResource consistent.

Test coverage: No Java integration tests cover the container /_copy REST endpoint. All three Postman copy-tests are updated. The frontend unit test (dot-containers.service.spec.ts:217) already asserted PUT and requires no change.

One note worth acknowledging: PUT /{id}/_copy is not idempotent in the RFC sense — repeated calls create multiple copies — so POST would be the more semantically correct HTTP verb. That said, the codebase has made this pragmatic choice consistently across all _copy endpoints, so this is the right call for consistency here.

No issues to block merge.

[claude]

🔍 dotCMS Backend Review

[🟠 High] dotCMS/src/main/java/com/dotcms/rest/api/v1/container/ContainerResource.java:1545,1623,1700,1778,1856 — raw containerId in FailedResultView response body ("does not exist" branch)

In all five bulk operations (bulkDelete, bulkPublish, bulkUnpublish, bulkArchive, bulkUnarchive), the "does not exist" branch sanitizes the Logger call but passes the raw, unsanitized containerId directly into FailedResultView, which is serialized verbatim into the JSON response body. An attacker can inject CR/LF or arbitrary content into the element field of the HTTP 200 response payload.

Logger.error(this, MessageConstants.CONTAINER_ID_WITH + SecurityUtils.sanitizeForLogging(containerId) + ...);
failedToDelete.add(new FailedResultView(containerId, "Container does not exist")); // ← raw id in response body

💡 Apply SecurityUtils.sanitizeForLogging(containerId) to all five FailedResultView(containerId, "Container does not exist") calls (same pattern in bulkPublish line 1623, bulkUnpublish line 1700, bulkArchive line 1778, bulkUnarchive line 1856).

---

[🟠 High] ContainerResource.java:~1549,1628,1704,1782,1860 — raw containerId in FailedResultView response body (catch(Exception) branch)

The same issue exists in the catch(Exception) branches of all five bulk operations. Neither the Logger call nor the FailedResultView construction was touched by this diff. The raw containerId is passed into the response body.

} catch(Exception e){
    Logger.debug(this, e.getMessage(), e);
    failedToDelete.add(new FailedResultView(containerId, e.getMessage())); // raw id in response
}

💡 Wrap containerId with SecurityUtils.sanitizeForLogging() at all five catch sites.

---

[🟡 Medium] ContainerResource.java:~995,1054,1129,1199,1266,1334,1408,1477 — sanitizeForLogging() applied inside DoesNotExistException constructor modifies the HTTP error response body

DoesNotExistExceptionMapper.toResponse() extracts exception.getMessage() verbatim and places it in the JSON error response under "message". By applying sanitizeForLogging() inside the DoesNotExistException constructor argument, the client now receives a sanitized string (e.g., foo_INJECTED instead of foo\nINJECTED) rather than either the original submitted value or a fixed message. This can confuse legitimate API consumers who compare submitted IDs to error messages. The sanitization method is named for logging — its use in user-facing messages is a semantic mismatch.

// Sanitized string goes to both log AND HTTP response body:
throw new DoesNotExistException("Live Version of the Container with Id: "
    + SecurityUtils.sanitizeForLogging(containerId) + MessageConstants.DOES_NOT_EXIST);

💡 Sanitize only the Logger.error() call. For the exception, either use a fixed message that does not echo caller input, or keep the raw containerId in the exception message:

Logger.error(this, "Live Version ... Id: " + SecurityUtils.sanitizeForLogging(containerId) + ...);
throw new DoesNotExistException("Live Version ... Id: " + containerId + MessageConstants.DOES_NOT_EXIST);

---

[🟡 Medium] ContainerResource.java:~613-615,689-692 — unsanitized containerId in ResourceNotFoundException messages (private helpers)

Private helpers getContainer() and removeContentletFromContainer() construct ResourceNotFoundException with a raw caller-supplied containerId. If ResourceNotFoundException propagates its message to the HTTP response body, this is the same injection vector the PR aims to close.

throw new ResourceNotFoundException("Can't find Container:" + containerId); // unsanitized

💡 Apply SecurityUtils.sanitizeForLogging() to these throw sites for consistency with the rest of the file.

---

[🟡 Medium] ContainerResource.java:906-907 — containerForm.getIdentifier() called twice through sanitizeForLogging()

In the update method, SecurityUtils.sanitizeForLogging(containerForm.getIdentifier()) is evaluated twice — once for Logger.error() and once for the exception constructor. While getIdentifier() is a simple accessor today, the double call is a maintenance hazard.

Logger.error(this, MessageConstants.CONTAINER + SecurityUtils.sanitizeForLogging(containerForm.getIdentifier()) + ", does not exists");
throw new DoesNotExistException(MessageConstants.CONTAINER + SecurityUtils.sanitizeForLogging(containerForm.getIdentifier()) + " does not exists");

💡 Extract to a local variable: final String sanitizedId = SecurityUtils.sanitizeForLogging(containerForm.getIdentifier());

---

Next steps

• 🟠 Fix locally and push — these need your judgment
• 🟡 You can ask me to handle mechanical fixes inline: @claude fix FailedResultView raw containerId in bulk operations or @claude fix ResourceNotFoundException unsanitized containerId in private helpers
• Every new push triggers a fresh review automatically

[claude]

✅ dotCMS Backend Review: no issues found.

All findings from the previous two review cycles are resolved in this push:

• 🟠 High (x2, bulk ops) — raw containerId in FailedResultView response body resolved: all five bulk operations now wrap containerId with SecurityUtils.sanitizeForLogging() at both FailedResultView call sites.
• 🟡 Medium — ResourceNotFoundException in private helpers getContainer() and removeContentletFromContainer() echoed unsanitized caller input — fixed: all four throw sites now sanitize.
• 🟡 Medium — containerForm.getIdentifier() called twice without a local variable — fixed: final String identifier extracted, used consistently.
• 🟡 Medium — sanitizeForLogging() applied inside DoesNotExistException constructors (semantic mismatch) — resolved: Logger.error calls are sanitized; exception constructors retain the raw caller value, which is JSON-serialized safely.

Database, Java standards, and REST API reviewers found no issues.