The plugin currently has no way to block untrusted users from making a PR from a fork and having this PR built by Jenkins. The GitHub Pull Request Builder does have this feature which is very useful for open source projects to protect the build system from malicious changes. The documentation on the GitHub Pull Request Builder wiki page says to move from the GHPRB plugin to the GitHub Branch source plugin which causes the user to lose this extremely useful functionality.

          [JENKINS-53752] Block PRs from forks from untrusted users

          Jesse Glick added a comment -

          I do not see any evidence that this is fixed.

          github-branch-source will automatically ignore Jenkinsfile modifications from an untrusted forked PR, for security reasons, but it will let the build proceed. I am not aware of any way to disable that other than suppressing discovery of all forked PRs, including those filed by authors who would have had write permission to the origin repository anyway.

          Jesse Glick added a comment - I do not see any evidence that this is fixed. github-branch-source will automatically ignore Jenkinsfile modifications from an untrusted forked PR, for security reasons, but it will let the build proceed. I am not aware of any way to disable that other than suppressing discovery of all forked PRs, including those filed by authors who would have had write permission to the origin repository anyway.

          Some of our repos were attacked yesterday by malicous PRs from Forks that modified our POM files (not the Jenkinsfile) to download and run a rogue shell script.  We used the default settings yet the PR build ran anyway.  

          The recommended setting should be "nobody" until this is fixed...

          Robert Patrick added a comment - Some of our repos were attacked yesterday by malicous PRs from Forks that modified our POM files (not the Jenkinsfile) to download and run a rogue shell script.  We used the default settings yet the PR build ran anyway.   The recommended setting should be "nobody" until this is fixed...

          Jesse Glick added a comment -

          Ouch. Out of curiosity, if you are comfortable sharing—was the damage limited to stealing compute resources (e.g., for coin mining) or did the scripts attempt to use or export something sensitive from the environment?

          This seems too important to leave to an extension plugin like basic-branch-build-strategies, especially if JENKINS-58618 & JENKINS-58683 are accurate. Coming up with a good default policy which balances security of builds running on public repos with usability is not trivial, though. (Compare JENKINS-46795.) GitHub Actions has gone through a few revisions here. Ideally a repository maintainer could quickly approve a new forked PR for CI without needing to navigate to the Jenkins GUI, but TBD whether all subsequent commits to the same PR should also be built automatically, or all subsequent PRs from the same author, etc.

          Jesse Glick added a comment - Ouch. Out of curiosity, if you are comfortable sharing—was the damage limited to stealing compute resources (e.g., for coin mining) or did the scripts attempt to use or export something sensitive from the environment? This seems too important to leave to an extension plugin like basic-branch-build-strategies , especially if JENKINS-58618 & JENKINS-58683 are accurate. Coming up with a good default policy which balances security of builds running on public repos with usability is not trivial, though. (Compare JENKINS-46795 .) GitHub Actions has gone through a few revisions here. Ideally a repository maintainer could quickly approve a new forked PR for CI without needing to navigate to the Jenkins GUI, but TBD whether all subsequent commits to the same PR should also be built automatically, or all subsequent PRs from the same author, etc.

          Unfortunately, I am not allowed to share the details.  

          I agree this is critical functionality for Jenkins.  It seems that other related open issues are only focused on the Jenkinsfile.  I think that is short-sighted.  Build files like POMs, Makefiles, and others are just as vulnerable as Jenkinsfiles.

          Robert Patrick added a comment - Unfortunately, I am not allowed to share the details.   I agree this is critical functionality for Jenkins.  It seems that other related open issues are only focused on the Jenkinsfile.  I think that is short-sighted.  Build files like POMs, Makefiles, and others are just as vulnerable as Jenkinsfiles.

          Jesse Glick added a comment -

          Potentially; it depends on how your system is structured.

          The reasoning behind restricting modifications only to Jenkinsfile is to prevent an unauthorized user from running, say, withCredentials to retrieve secrets in scope. The assumption is that the Jenkinsfile is written so as to assume malicious intent from a PR on the part of any external processes it forks, which would be run on an agent computer that is itself not trusted (container or VM), so a rogue process could waste time but not do much else. (You can choose to readTrusted other configuration files which would structurally affect how a build uses Jenkins resources, such as choice of agent labels or credentials or notification targets.)

          A particular project may not have been set up with this threat model in mind, though, or PR builds might require certain credentials (such as for a database) just to run tests, and in some environments just running unexpected build processes even on an isolated computer could be a threat, so we ought to have something that is easier to secure. See also JENKINS-45970.

          Jesse Glick added a comment - Potentially; it depends on how your system is structured. The reasoning behind restricting modifications only to Jenkinsfile is to prevent an unauthorized user from running, say, withCredentials to retrieve secrets in scope. The assumption is that the Jenkinsfile is written so as to assume malicious intent from a PR on the part of any external processes it forks, which would be run on an agent computer that is itself not trusted (container or VM), so a rogue process could waste time but not do much else. (You can choose to readTrusted other configuration files which would structurally affect how a build uses Jenkins resources, such as choice of agent labels or credentials or notification targets.) A particular project may not have been set up with this threat model in mind, though, or PR builds might require certain credentials (such as for a database) just to run tests, and in some environments just running unexpected build processes even on an isolated computer could be a threat, so we ought to have something that is easier to secure. See also JENKINS-45970 .

          You aren’t thinking this through.  Suppose, for example, you are running a Maven command within a withCredentials block…same for running Make, ant, etc.  change the build file being run and now you can grab the environment variables in any rogue script…

          Robert Patrick added a comment - You aren’t thinking this through.  Suppose, for example, you are running a Maven command within a withCredentials block…same for running Make, ant, etc.  change the build file being run and now you can grab the environment variables in any rogue script…

          Jesse Glick added a comment -

          Right, which is why you must not run a Maven command inside a withCredentials block unless you are on a known (origin) branch.

          Jesse Glick added a comment - Right, which is why you must not run a Maven command inside a withCredentials block unless you are on a known (origin) branch.

          Robert Patrick added a comment - - edited

          jglick We understood the risk of auto-building a fork PR but trusted the Jenkins GitHub Branch Source Plugin to do what it said when we set Trust to "From users with Admin or Write permission"...that was our mistake.  I am appalled by the fact that this issue is 4+ years old and no one seems to care. 

          Robert Patrick added a comment - - edited jglick We understood the risk of auto-building a fork PR but trusted the Jenkins GitHub Branch Source Plugin to do what it said when we set Trust to "From users with Admin or Write permission"...that was our mistake.  I am appalled by the fact that this issue is 4+ years old and no one seems to care. 

          To be frank, we were pretty shocked also to find that PRs were not being blocked from untrusted sources amongst all of this, particularly given the screenshot in https://issues.jenkins.io/browse/JENKINS-53752?focusedCommentId=371866&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-371866.

          We just ended up not allowing anything from forks.  Our users are unhappy about that.  Or repo has nearly a thousand branches now because we can only run PRs opened in our organization.  The whole thing is a mess.

          Our path forward is probably going to be to trigger Jenkins jobs from GitHub Actions since GitHub Actions has some at least half-way sane security settings for when PRs can run actions and when they cannot.  No offence to anyone, but we simply cannot trust Jenkins in this regard any more.

          Brian J Murrell added a comment - To be frank, we were pretty shocked also to find that PRs were not being blocked from untrusted sources amongst all of this, particularly given the screenshot in https://issues.jenkins.io/browse/JENKINS-53752?focusedCommentId=371866&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-371866 . We just ended up not allowing anything from forks.  Our users are unhappy about that.  Or repo has nearly a thousand branches now because we can only run PRs opened in our organization.  The whole thing is a mess. Our path forward is probably going to be to trigger Jenkins jobs from GitHub Actions since GitHub Actions has some at least half-way sane security settings for when PRs can run actions and when they cannot.  No offence to anyone, but we simply cannot trust Jenkins in this regard any more.

          I think Jenkins is starting to show its age.  When CloudBees first got a involved, I was hopeful that the gaps and quality issues would improve and, for awhile, I think that they did.  However, right now, it feels as bad as it did pre-CloudBees.  I would love to help but it feels like the entire project is suffering from a lack of leadership.  😞

          Robert Patrick added a comment - I think Jenkins is starting to show its age.  When CloudBees first got a involved, I was hopeful that the gaps and quality issues would improve and, for awhile, I think that they did.  However, right now, it feels as bad as it did pre-CloudBees.  I would love to help but it feels like the entire project is suffering from a lack of leadership.  😞

            Unassigned Unassigned
            roguishmountain Sam Schwarz
            Votes:
            6 Vote for this issue
            Watchers:
            12 Start watching this issue

              Created:
              Updated: