search

jenkins-infra / repository-permissions-updater

Artifactory permissions synchronization tool and data set
78 stars 1.04k forks source link

Hosting Request for MergeBase SCA plugin #2465

Closed delanAtMergebase closed 2 years ago

delanAtMergebase commented 2 years ago

Repository URL

https://github.com/mergebase/mergebase-sca

New Repository Name

mergebase-sca-plugin

Description

The MergeBase SCA plugin allows MergeBase users who are also Jenkins users to run the MergeBase tool with an automated build step.

The plugin runs the MergeBase tool and prints a report, as well as sends the report back to the MergeBase dashboard.

GitHub users to have commit permission

@delanAtMergebase @juliusmusseau

Jenkins project users to have release permission

delanAtMergebase juliusmusseau

Issue tracker

GitHub issues

github-actions[bot] commented 2 years ago

Hello from your friendly Jenkins Hosting Checker

It appears you have some issues with your hosting request. Please see the list below and correct all issues marked Required. Your hosting request will not be approved until these issues are corrected. Issues marked with Warning or Info are just recommendations and will not stall the hosting process.

You can re-trigger a check by editing your hosting request or by commenting /hosting re-check

delanAtMergebase commented 2 years ago

/hosting re-check

github-actions[bot] commented 2 years ago

Hello from your friendly Jenkins Hosting Checker

It appears you have some issues with your hosting request. Please see the list below and correct all issues marked Required. Your hosting request will not be approved until these issues are corrected. Issues marked with Warning or Info are just recommendations and will not stall the hosting process.

You can re-trigger a check by editing your hosting request or by commenting /hosting re-check

delanAtMergebase commented 2 years ago

/hosting re-check

github-actions[bot] commented 2 years ago

Hello from your friendly Jenkins Hosting Checker

It appears you have some issues with your hosting request. Please see the list below and correct all issues marked Required. Your hosting request will not be approved until these issues are corrected. Issues marked with Warning or Info are just recommendations and will not stall the hosting process.

You can re-trigger a check by editing your hosting request or by commenting /hosting re-check

github-actions[bot] commented 2 years ago

Hello from your friendly Jenkins Hosting Checker

It appears you have some issues with your hosting request. Please see the list below and correct all issues marked Required. Your hosting request will not be approved until these issues are corrected. Issues marked with Warning or Info are just recommendations and will not stall the hosting process.

You can re-trigger a check by editing your hosting request or by commenting /hosting re-check

delanAtMergebase commented 2 years ago

/hosting re-check

github-actions[bot] commented 2 years ago

Hello from your friendly Jenkins Hosting Checker

It looks like you have everything in order for your hosting request. A human volunteer will check over things that I am not able to check for (code review, README content, etc) and process the request as quickly as possible. Thank you for your patience.

Hosting team members can host this request with /hosting host

timja commented 2 years ago

cc @Wadeck

From a quick look over likely customToken should be of type Secret

and using f:password for UI: https://github.com/mergebase/mergebase-sca/blob/master/src/main/resources/com/mergebase/jenkins/MergebaseStepBuilder/config.jelly#L13

Wadeck commented 2 years ago

Yes your finding is good.

Also, I have some doubts about https://github.com/mergebase/mergebase-sca/blob/7a044e3708991feb294199a399ba2d0634c4d2fa/src/main/java/com/mergebase/jenkins/MergeBaseRun.java#L34 as the path seems to be configured at the step level, it allow attackers with Job/Configure to download tooling to the controller. Gut feeling: this should be a global config, not a per-job config.

I will file an internal ticket to cover it completely => JENSEC-1800. ℹ️ ETA perhaps a bit longer than usual as we have several other tasks in parallel.

delanAtMergebase commented 2 years ago

I will address these issues before you do your full review. Thanks for the initial findings!

Kevin-CB commented 2 years ago

Hi @delanAtMergebase.

We're able to perform a more complete audit on your plugin here are the feedback:

Moreover in jenkins/MergebaseConfig.java#L7 you're implementing a Serializable interface while it's not needed, this should be removed. (If you want to keep it you should transform customerToken into a Secret type)

Recommendation: https://www.jenkins.io/doc/developer/security/secrets/

Recommendation: You should move this configuration to a global level (by admin only) or restrict it to workspace (forbidding non-relative paths).

We will let you correct the finding and ping us when you are done or if you have questions.

delanAtMergebase commented 2 years ago

Hi @Kevin-CB

I have addressed your feedback:

Let me know if there's anything else. Thank you!

Kevin-CB commented 2 years ago

Hi @delanAtMergebase,

I was able to review and test your latest version and everything seems ok on security point of view!

We can continue the hosting process.

timja commented 2 years ago

I would suggest updating this file: https://github.com/mergebase/mergebase-sca/blob/master/src/main/resources/index.jelly#L3

It will be displayed in the Plugin Manager in the Jenkins UI

timja commented 2 years ago

/hosting re-check

github-actions[bot] commented 2 years ago

Hello from your friendly Jenkins Hosting Checker

It looks like you have everything in order for your hosting request. A human volunteer will check over things that I am not able to check for (code review, README content, etc) and process the request as quickly as possible. Thank you for your patience.

Hosting team members can host this request with /hosting host

timja commented 2 years ago

/hosting host

jenkins-infra-bot commented 2 years ago

Hosting request complete, the code has been forked into the jenkinsci project on GitHub as https://github.com/jenkinsci/mergebase-sca-plugin

GitHub issues has been selected for issue tracking and was enabled for the forked repo.

A pull request has been created against the repository permissions updater to setup release permissions. Additional users can be added by modifying the created file.

Please remove your original repository (if there are no other forks) so that the jenkinsci organization repository is the definitive source for the code. If there are other forks, please contact GitHub support to make the jenkinsci repo the root of the fork network (mention that Jenkins approval was given in support request 569994). Also, please make sure you properly follow the documentation on documenting your plugin so that your plugin is correctly documented.

You will also need to do the following in order to push changes and release your plugin:

In order for your plugin to be built by the Jenkins CI Infrastructure and check pull requests, please add a Jenkinsfile to the root of your repository with the following content: buildPlugin()

Welcome aboard!