Guard against infinite recursion (StackOverflowError) in BranchJobProperty.decorateACL#574
Open
y-c-s wants to merge 1 commit into
Open
Guard against infinite recursion (StackOverflowError) in BranchJobProperty.decorateACL#574y-c-s wants to merge 1 commit into
y-c-s wants to merge 1 commit into
Conversation
…perty.decorateACL When the delegate ACL re-enters the same decorator (a cyclic base ACL produced by the cloudbees-folder child-ACL composition under many branches, the JENKINS-44809 class of bug), hasPermission2 recursed forever and threw StackOverflowError. This surfaced as HTTP 500 on /queue/api/json and the full job REST APIs, which broke build triggering. This adds a thread-local re-entrancy guard. On re-entry the decorator answers from the instance-wide terminal root ACL (Jenkins.get().getAuthorizationStrategy().getRootACL()), which yields the same permission decision the strategy intends without looping. Related: JENKINS-44809 Signed-off-by: Yann Sommer <ysommer@gmail.com>
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.
Problem
BranchJobProperty.decorateACLwraps the delegate ACL and, in its finalelsebranch, callsacl.hasPermission2(a, permission). Under certain configurations the delegate ACL is cyclic: the cloudbees-folder child-ACL composition can produce a base ACL that re-enters this very same decorator. When that happenshasPermission2recurses into itself indefinitely and throwsStackOverflowError.This is the JENKINS-44809 class of bug. It is sensitive to scale — it shows up on controllers with many branches/children where the folder ACL composition becomes cyclic.
Symptom
The
StackOverflowErrorpropagates out of permission checks performed while rendering API responses, so the affected endpoints return HTTP 500:/queue/api/json.../api/jsonfor multibranch jobs)Because
/queue/api/jsonis used to drive build triggering, the practical effect is that build triggering breaks entirely for the instance.Fix
Add a thread-local re-entrancy guard around the delegate call. On first entry the guard is set and the delegate ACL is consulted as before. If the delegate re-enters the same decorator on the same thread, the guard short-circuits and answers from the instance-wide terminal root ACL:
The root ACL is the non-decorating terminal of the strategy, so it yields the same permission decision the strategy intends for that authentication/permission — without looping. The guard is reset in a
finallyblock so it never leaks across requests on a pooled thread.The change is scoped to the single
elsebranch; the existingSYSTEM2,CONFIGURE, andDELETEshort-circuits are untouched.Verification
We ran the patched plugin on a production controller that was reliably reproducing the failure. After deploying it:
/queue/api/jsonand the previously-failing job REST endpoints returned 200 instead of 500.StackOverflowError/ recursion appeared in the controller logs.Notes