Skip to content

[JENKINS-46795] Abort builds with untrusted Jenkinsfile, but only given passive cause #474

Description

@jenkins-infra-bot

Currently ForkPullRequestDiscoveryTrait.TrustPermission returns false for PRs from authors without write permission. But this is annoying as it means that there is no good way to, say, contribute a Jenkinsfile to someone else's repository in a pull request—the builds will not run your code, which is safe, but then the maintainer cannot tell whether the script is good without either merging the PR (and potentially causing build breakages on other unrelated PRs), or filing their own PR which simply wraps yours.

It should override checkTrusted(GitHubSCMSourceRequest, PullRequestSCMRevision) to check the GitHub API to see if the current revision has been approved by a maintainer. If so, we can presume it is safe to run.

Revised proposal: ignore the trust level of the PR if the build is being triggered explicitly, for example Build Now inside Jenkins or via Re-run on a check. For builds triggered passively by the SCM commit, continue to pay attention to the trust level; but halt the build for transparency, rather than the current behavior of proceeding but with the trunk Jenkinsfile.


Originally reported by jglick, imported from: Abort builds with untrusted Jenkinsfile, but only given passive cause
  • assignee: jglick
  • status: In Review
  • priority: Major
  • component(s): workflow-multibranch-plugin
  • resolution: Unresolved
  • votes: 0
  • watchers: 5
  • imported: 20251212-090250
Raw content of original issue

Currently ForkPullRequestDiscoveryTrait.TrustPermission returns false for PRs from authors without write permission. But this is annoying as it means that there is no good way to, say, contribute a Jenkinsfile to someone else's repository in a pull request—the builds will not run your code, which is safe, but then the maintainer cannot tell whether the script is good without either merging the PR (and potentially causing build breakages on other unrelated PRs), or filing their own PR which simply wraps yours.

It should override checkTrusted(GitHubSCMSourceRequest, PullRequestSCMRevision) to check the GitHub API to see if the current revision has been approved by a maintainer. If so, we can presume it is safe to run.

Revised proposal: ignore the trust level of the PR if the build is being triggered explicitly, for example Build Now inside Jenkins or via Re-run on a check. For builds triggered passively by the SCM commit, continue to pay attention to the trust level; but halt the build for transparency, rather than the current behavior of proceeding but with the trunk Jenkinsfile.

Activity

  1. jenkins-infra-bot commented on Sep 11, 2017

    @jenkins-infra-bot
    Author
  2. jenkins-infra-bot commented on Sep 25, 2017

    @jenkins-infra-bot
    Author
    stephenconnolly:
    • Original comment link
    • Raw content of original comment:

      NOTE: This is left as an exercise for an extension plugin, not to be implemented in the core github branch source plugin.

    NOTE: This is left as an exercise for an extension plugin, not to be implemented in the core github branch source plugin.

  3. jenkins-infra-bot commented on Nov 28, 2022

    @jenkins-infra-bot
    Author
    jglick:
    • Original comment link
    • Raw content of original comment:

      As per Make SCMBinder try to abort builds in which Jenkinsfile has been modified by an untrusted contributor #69 (comment) currently we proceed with the build even though the proposed Jenkinsfile is not actually being tested, which is not good. I think it would better to fail the build (with NOT_BUILT status) but then make it easy for someone with either build permission in Jenkins, or write permission to the repository, to proceed. The former could be handled by suppressing the trust check when built via UserIdCause; the latter when via CheckRunGHEventSubscriber.GitHubChecksRerunActionCause. Perhaps we could just amend the trust behavior to apply only when the build was triggered by a branch indexing or SCM event cause?

    As per #69 (comment) currently we proceed with the build even though the proposed Jenkinsfile is not actually being tested, which is not good. I think it would better to fail the build (with NOT_BUILT status) but then make it easy for someone with either build permission in Jenkins, or write permission to the repository, to proceed. The former could be handled by suppressing the trust check when built via UserIdCause; the latter when via CheckRunGHEventSubscriber.GitHubChecksRerunActionCause. Perhaps we could just amend the trust behavior to apply only when the build was triggered by a branch indexing or SCM event cause?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions