Do not write stack trace to system log for unauthorized API access#340
Open
MarkEWaite wants to merge 2 commits into
Open
Do not write stack trace to system log for unauthorized API access#340MarkEWaite wants to merge 2 commits into
MarkEWaite wants to merge 2 commits into
Conversation
Fixes jenkinsci#339 Advisory https://www.jenkins.io/security/advisory/2026-06-24/#SECURITY-3745 with the change in jenkinsci@496a59f adds a permission check that requires Item.BUILD permission in order to read the parameter values. It relies on the checkPermission() method to throw an exception. The exception is written to the Jenkins log. The unguarded call to checkPermission() is a relatively common technique, but a user reported that it dramatically increases the size of their system logs. Refer to issue: * jenkinsci#339 Return an empty list instead of throwing an exception. Write a message and the exception to the logger of the class when the logging level is FINE. Changes the behavior of the API when the request does not have permission. The security fix caused the API to return: > "parameterDefinitions": [] The code in this commit returns: > "parameterDefinitions": [ > { > "_class": "net.uaznia.lukanus.hudson.plugins.gitparameter.GitParameterDefinition", > "defaultParameterValue": { > "_class": "net.uaznia.lukanus.hudson.plugins.gitparameter.GitParameterValue", > "value": "jdk-17.0.19" > }, > "description": "Choose tag to checkout", > "name": "PRIVATE_TAG", > "type": "GitParameterDefinition", > "allValueItems": { > "errors": [], > "values": [] > } > } > ] That means the default value for the parameter is now available to the read only user, when previously the default value was not available to the read only user. I'd like an opinion from the Jenkins security team before I merge this change, since the default value might be considered sensitive information. Testing done: * Confirmed that the stack trace is written to the Jenkins log by the latest release * Confirmed that the stack trace is not written to the Jenkins log by this change * Confirmed that the list of values is correctly not returned by this change * Noted that the default value is visible with this change
There was a problem hiding this comment.
Pull request overview
Adjusts how GitParameterDefinition handles unauthorized access to its exported allValueItems data so that Jenkins API calls from users without Job.BUILD do not generate controller log stack traces.
Changes:
- Catch
AccessDeniedExceptionfromjob.checkPermission(Job.BUILD)insidedoFillValueItems(...)and returnItemsErrorModel.EMPTY. - Log the denied access at
Level.FINEinstead of allowing the exception to propagate into Stapler export logging.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+740
to
+742
| LOGGER.log(Level.FINE, job.getDisplayName() + " " + e.getMessage(), e); | ||
| return ItemsErrorModel.EMPTY; | ||
| } |
Contributor
Author
|
@PetersNils have you installed the incremental build on your Jenkins controller to confirm that it behaves the way you expect? |
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
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.
Do not write stack trace to system log for unauthorized API access
Fixes #339
Return an empty list instead of throwing an exception. Write a message and the exception to the logger of the class when the logging level is FINE.
SECURITY-3745 with the change in 496a59 adds a permission check that requires Item.BUILD permission in order to read the parameter values. It relies on the checkPermission() method to throw an exception. The exception is written to the Jenkins log.
The unguarded call to checkPermission() is a relatively common technique, but a user reported that it dramatically increases the size of their system logs. Refer to issue:
This pull request changes the behavior of the API when the request does not have permission. The security fix caused the API to return:
The code in this commit returns:
That means the default value for the parameter is now available to the read only user, when previously the default value was not available to the read only user.
I'd like an opinion from the @jenkinsci/jenkins-security-team before I merge this change, since the default value might be considered sensitive information.
Testing done
I'd also like a report from the issue submitter @PetersNils that the incremental build has been installed and confirmed to work in the environment that had the issue.
Submitter checklist