Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-68404] Add script listener to track usage #416

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

meiswjn
Copy link

@meiswjn meiswjn commented May 7, 2022

This PR relates to jenkinsci/jenkins#6539 and https://issues.jenkins.io/browse/JENKINS-68404. It serves the purpose to track potentially dangerous usages of groovy scripts.
The script listener is called when a script is considered to be allowed to use. This also means that scripts running in a sandbox are not logged.

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@meiswjn
Copy link
Author

meiswjn commented May 7, 2022

@daniel-beck jenkinsci/jenkins#6539 (comment)

Let me know what you think.

@meiswjn meiswjn force-pushed the feature/add-script-listener branch from 2338f9c to b9eafc1 Compare May 9, 2022 09:59
@meiswjn meiswjn changed the title Add script listener to track usage [JENKINS-68404] Add script listener to track usage May 11, 2022
@meiswjn meiswjn requested a review from daniel-beck May 14, 2022 07:56
@meiswjn
Copy link
Author

meiswjn commented May 17, 2022

Extends listener introduced in jenkinsci/jenkins#6539

pom.xml Outdated Show resolved Hide resolved
Copy link
Member

@daniel-beck daniel-beck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#configuring might be a good alternative to #using because that can determine who the user actually submitting the script is. It also has ApprovalContext which would probably remove the need for the API change.

@daniel-beck
Copy link
Member

A basic pipeline:

Script: 'echo "hello world"' from origin: 'N/A' by user: 'null'

Once we've finalized the API here, looks like this needs a downstream change in workflow-cps or so.

@daniel-beck
Copy link
Member

Looks like this PR needs to be re-filed from an origin branch, or from the fork of a committer (e.g. me), to pick up the changes to the Jenkinsfile, so we have an incremental build of this.

@daniel-beck daniel-beck self-requested a review August 31, 2022 21:10
@lemeurherve
Copy link
Member

@daniel-beck I've replayed it with the updated Jenkinsfile.

@daniel-beck
Copy link
Member

@daniel-beck I've replayed it with the updated Jenkinsfile.

Cheater! (Thanks 😄)

@jglick jglick marked this pull request as draft November 16, 2022 17:19
@meiswjn
Copy link
Author

meiswjn commented Mar 21, 2024

With jenkinsci/jenkins#7056 being merged, I will continue on this PR soon :)

@jglick
Copy link
Member

jglick commented Aug 28, 2024

@meiswjn are you still planning to work on this?

@meiswjn
Copy link
Author

meiswjn commented Sep 9, 2024

@meiswjn are you still planning to work on this?

Since I would love to see this feature, yes. However, there are many other more pressing things right now, but I definitely want to do this. However, if someone stumbles upon this before I find time, feel free!

@daniel-beck daniel-beck force-pushed the feature/add-script-listener branch from 2b65531 to 81eef7b Compare September 9, 2024 17:09
@daniel-beck
Copy link
Member

daniel-beck commented Sep 9, 2024

(Sorry for the noise, I thought this was an easy UI merge, but this was still on top of the old core PR 😬)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants