merge queue: checking main (de0fe19), #36344 and #36593 together - #36596
Closed
mergify[bot] wants to merge 11 commits into
Closed
merge queue: checking main (de0fe19), #36344 and #36593 together#36596mergify[bot] wants to merge 11 commits into
mergify[bot] wants to merge 11 commits into
Conversation
ToolGroupResource._addtouser and ._removefromuser now require the caller to be a CMS Administrator before assigning or removing any layout other than the gettingStarted onboarding layout. Previously any backend user could self-assign the admin Settings layout (which contains the roles portlet), bypassing the intended privilege boundary. RoleAjax.addUserToRole and .removeUsersFromRole now require the caller to be a CMS Administrator in addition to the existing portlet-access check. Previously a user who had gained roles-portlet access could grant themselves (or any user) any role including CMS Administrator via DWR. Together these two gaps formed a privilege-escalation chain that allowed any authenticated backend user to reach CMS Administrator and subsequently execute arbitrary OS commands via OSGi bundle upload. Closes: dotCMS/private-issues#640 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Move the isAdmin() check above the loadUserById() call in both _removefromuser and _addtouser so unauthorized callers are rejected before any DB round-trip for the target user. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JHg1W7beD4Z1yLoTJpyXss
Covers the four new admin checks: RoleAjax.addUserToRole / removeUsersFromRole reject non-admin callers even with roles-portlet permission, ToolGroupResource add/remove reject non-admins, the gettingstarted layout exemption still works for non-admins, and admin callers pass through with the expected API side effects. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JHg1W7beD4Z1yLoTJpyXss
dsilvam found that _addtouser exempts the "gettingstarted" layout from the admin check but _removefromuser did not, so a non-admin could enable the Getting Started page but got a DotSecurityException dismissing it (reachable from My Account and the onboarding portlet toggles). Mirror the exemption on the remove path, and tighten both paths so the gettingstarted exemption is self-service only — targeting another userid still requires admin (also addresses the dotBot userid-validation finding on the add path). Extracted the shared check into isSelfServiceGettingStarted(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JHg1W7beD4Z1yLoTJpyXss
Moves main forward to the current Tomcat 9.0.x release (2026-07-03), picking up the CVE-2024-56337 startup-protection check, the CVE-2025-49125 regression fix, WebDAV serveSubpathOnly hardening, and the permessage-deflate buffer fix. Single property drives the dependency, the assembled distribution, and the Docker base image. Closes #36592 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JHg1W7beD4Z1yLoTJpyXss
2 tasks
Contributor
❌ Issue Linking RequiredThis PR could not be linked to an issue. All PRs must be linked to an issue for tracking purposes. How to fix this:Option 1: Add keyword to PR body (Recommended - auto-removes this comment)
Option 2: Link via GitHub UI (Note: won't clear the failed check)
Why is this required?Issue linking ensures proper tracking, documentation, and helps maintain project history. It connects your code changes to the problem they solve. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
✨ Pull request #36344 ahead in the queue was removed (reason:
pull request dequeued). The pull request #36593 has been requeued. ✨Branch main (de0fe19), #36344 and #36593 are queued together for merge.
This pull request has been created by Mergify to speculatively check the mergeability of #36593.
You don't need to do anything. Mergify will close this pull request automatically when it is complete.
Required conditions of queue rule
defaultfor merge:check-neutral = Finalize / Final Statuscheck-skipped = Finalize / Final Statuscheck-success = Finalize / Final Statuscheck-neutral = Initialize / Initializecheck-skipped = Initialize / Initializecheck-success = Initialize / Initialize#review-threads-unresolved = 0[🛡 GitHub branch protection]github-review-approved[🛡 GitHub branch protection]github-review-decision = APPROVED[🛡 GitHub branch protection]Required conditions to stay in the queue:
#review-threads-unresolved = 0[🛡 GitHub branch protection]github-review-approved[🛡 GitHub branch protection]github-review-decision = APPROVED[🛡 GitHub branch protection]check-success = Finalize / Final Statuscheck-neutral = Finalize / Final Statuscheck-skipped = Finalize / Final Statuscheck-success = Initialize / Initializecheck-neutral = Initialize / Initializecheck-skipped = Initialize / Initialize