Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-40652

origin pr builds not treated as trusted

    XMLWordPrintable

    Details

    • Similar Issues:

      Description

      When building PRs from origin (not forks), their Jenkinsfile is not treated as trusted and loaded from the base branch instead:

      Loading trusted files from base branch develop at 3ad383ee0eeffc92c9712dc8e3022c4b43a75c94 rather than 3e2b6e35cbf0fd2d4c029fcd23560f04b1976618

      Settings:

      [x] Build origin branches
      [x] Build origin PRs (unmerged head)

      IMHO any PR from origin should be treated as trusted. There is no issue with building their branches via "Build origin branches also filed as PRs".

        Attachments

          Issue Links

            Activity

            chrono Martin Ringehahn created issue -
            chrono Martin Ringehahn made changes -
            Field Original Value New Value
            Summary origin pr builds treated as trusted origin pr builds not treated as trusted
            Hide
            jzila John Zila added a comment -

            +1. In my config this makes PR builds useless, because they're always building master.

            Show
            jzila John Zila added a comment - +1. In my config this makes PR builds useless, because they're always building master.
            Hide
            patthiel Patrick Thiel added a comment -

            +1.. This is pretty much a blocker for us. We were right in the middle of migrating to using Jenkinsfiles and multibranch pipelines when we encountered this.

            Basically, if you don't already have a Jenkinsfile merged to the base branch (say your PR contains a WIP Jenkinsfile, like in our case), then Jenkins throws an error stating no Jenkinsfile found. It's only loading trusted files from the base branch which at this point doesn't contain a jenkins file, so nothing runs.
            Here are some steps to reproduce that i posted in another comment:

            Using a multibranch pipeline project with the latest SCM API 2.0 release, we have also noticed PR's from contributors getting flagged as untrusted sources.. Despite the PR author having admin privileges as a contributor and is the member of a Github team that also has Write permissions for the repository.

            To test this..

            Build settings:
            [x] Build origin PRs (unmerged head)

            1. Submit a PR to origin base branch with changes to the Jenkinsfile (add an echo or something)
            2. Open up a PR and scan the repository.
            3. Observe, In the scan log, the source will appear untrusted
            4. Jenkins will checkout the base branch instead
            5. The base branch Jenkinsfile is executed

            The repository scan log looks like this:

                Checking pull request #1817
                (not from a trusted source)
                Job name: PR-1817
                  ‘Jenkinsfile’ found
                Met criteria
            

            Jenkins PR job log:

            Loading trusted files from base branch dev at {commit} rather than {commit}
            
            Show
            patthiel Patrick Thiel added a comment - +1.. This is pretty much a blocker for us. We were right in the middle of migrating to using Jenkinsfiles and multibranch pipelines when we encountered this. Basically, if you don't already have a Jenkinsfile merged to the base branch (say your PR contains a WIP Jenkinsfile, like in our case), then Jenkins throws an error stating no Jenkinsfile found. It's only loading trusted files from the base branch which at this point doesn't contain a jenkins file, so nothing runs. Here are some steps to reproduce that i posted in another comment: Using a multibranch pipeline project with the latest SCM API 2.0 release, we have also noticed PR's from contributors getting flagged as untrusted sources.. Despite the PR author having admin privileges as a contributor and is the member of a Github team that also has Write permissions for the repository. To test this.. Build settings: [x] Build origin PRs (unmerged head) Submit a PR to origin base branch with changes to the Jenkinsfile (add an echo or something) Open up a PR and scan the repository. Observe, In the scan log, the source will appear untrusted Jenkins will checkout the base branch instead The base branch Jenkinsfile is executed The repository scan log looks like this: Checking pull request #1817 (not from a trusted source) Job name: PR-1817 ‘Jenkinsfile’ found Met criteria Jenkins PR job log: Loading trusted files from base branch dev at {commit} rather than {commit}
            patthiel Patrick Thiel made changes -
            Epic Link JENKINS-41234 [ 177999 ]
            Hide
            myoung34 marc young added a comment -
            Show
            myoung34 marc young added a comment - I have the same issue. Linking https://issues.jenkins-ci.org/browse/JENKINS-37931
            myoung34 marc young made changes -
            Link This issue is related to JENKINS-37931 [ JENKINS-37931 ]
            myoung34 marc young made changes -
            Rank Ranked higher
            Hide
            danielbeck Daniel Beck added a comment -

            This issue predated SCM API 2.0.

            Show
            danielbeck Daniel Beck added a comment - This issue predated SCM API 2.0.
            danielbeck Daniel Beck made changes -
            Epic Link JENKINS-41234 [ 177999 ]
            Hide
            myoung34 marc young added a comment - - edited

            FWIW the relevant plugin code is here

            Show
            myoung34 marc young added a comment - - edited FWIW the relevant plugin code is here
            Hide
            myoung34 marc young added a comment -

            It's definitely a bug. I built the latest github-branch-source-plugin (2.0.1-beta-2) and all dependencies.

            I inserted logger statements inside the

            getTrustedRevision

            as:

                        PullRequestSCMHead head = (PullRequestSCMHead) revision.getHead();
                        listener.getLogger().format("head source owner: " + head.getSourceOwner() + "%n");
                        listener.getLogger().format("head target repo owner: " + head.getTargetRepo().getOwner() + "%n");
                        listener.getLogger().format("collaboratorNames: " + collaboratorNames + "%n");
                        listener.getLogger().format("collaboratorNames empty: " + collaboratorNames.isEmpty() + "%n");
                        if (fork && collaboratorNames == null) {
            

            and during a build of a PR the output is:

            Connecting to https://api.github.com using myoung34/******
            head source owner: _redactedowner_
            head target repo owner: GHUser@2e80c396[login=_redactedowner_,location=Nashville, TN,blog=http://www._redactedowner_.com,email=<null>,name=_redactedowner_,company=<null>,followers=0,following=0,url=https://api.github.com/users/_redactedowner_,id=1073954]
            collaboratorNames: [...snip..., myoung34, ...snip...]
            collaboratorNames empty: false
            Loading trusted files from base branch master at 784081c38d601c3602e76ba10394e8839333e22b rather than 90f516ebf3af5d9b2abfc6e8e6b2699a73b2ca3f
            

            Even though I'm a collaborator it's still saying that it's untrusted. The if condition is not being hit during this time, so it's not re-checking. I'm making some bugfixes now and hopefully the upstream owners will accept them when finished.

            Show
            myoung34 marc young added a comment - It's definitely a bug. I built the latest github-branch-source-plugin (2.0.1-beta-2) and all dependencies. I inserted logger statements inside the getTrustedRevision as: PullRequestSCMHead head = (PullRequestSCMHead) revision.getHead(); listener.getLogger().format( "head source owner: " + head.getSourceOwner() + "%n" ); listener.getLogger().format( "head target repo owner: " + head.getTargetRepo().getOwner() + "%n" ); listener.getLogger().format( "collaboratorNames: " + collaboratorNames + "%n" ); listener.getLogger().format( "collaboratorNames empty: " + collaboratorNames.isEmpty() + "%n" ); if (fork && collaboratorNames == null ) { and during a build of a PR the output is: Connecting to https: //api.github.com using myoung34/****** head source owner: _redactedowner_ head target repo owner: GHUser@2e80c396[login=_redactedowner_,location=Nashville, TN,blog=http: //www._redactedowner_.com,email=< null >,name=_redactedowner_,company=< null >,followers=0,following=0,url=https://api.github.com/users/_redactedowner_,id=1073954] collaboratorNames: [...snip..., myoung34, ...snip...] collaboratorNames empty: false Loading trusted files from base branch master at 784081c38d601c3602e76ba10394e8839333e22b rather than 90f516ebf3af5d9b2abfc6e8e6b2699a73b2ca3f Even though I'm a collaborator it's still saying that it's untrusted. The if condition is not being hit during this time, so it's not re-checking. I'm making some bugfixes now and hopefully the upstream owners will accept them when finished.
            Show
            myoung34 marc young added a comment - Fixed code in PR: https://github.com/jenkinsci/github-branch-source-plugin/pull/109
            Hide
            recampbell Ryan Campbell added a comment - - edited

            Was this issue present in github-branch-source-plugin prior to 2.0?

            We are trying to determine if this is a regression introduced by the github-branch-source-plugin 2.0 beta releases.

            Show
            recampbell Ryan Campbell added a comment - - edited Was this issue present in github-branch-source-plugin prior to 2.0? We are trying to determine if this is a regression introduced by the github-branch-source-plugin 2.0 beta releases.
            recampbell Ryan Campbell made changes -
            Epic Link JENKINS-41234 [ 177999 ]
            Hide
            recampbell Ryan Campbell added a comment -

            Provisionally marked as SCM 2.0 Upgrade Fallout until we determine otherwise.

            Show
            recampbell Ryan Campbell added a comment - Provisionally marked as SCM 2.0 Upgrade Fallout until we determine otherwise.
            recampbell Ryan Campbell made changes -
            Assignee Stephen Connolly [ stephenconnolly ]
            Hide
            stephenconnolly Stephen Connolly added a comment -

            https://github.com/jenkinsci/github-branch-source-plugin/pull/114 is a minimal fix of the regression for merging into the 2.0.x upgrade fallout

            Show
            stephenconnolly Stephen Connolly added a comment - https://github.com/jenkinsci/github-branch-source-plugin/pull/114 is a minimal fix of the regression for merging into the 2.0.x upgrade fallout
            stephenconnolly Stephen Connolly made changes -
            Remote Link This issue links to "PR#114 (Web Link)" [ 15350 ]
            Show
            stephenconnolly Stephen Connolly added a comment - marc young I have created https://issues.jenkins-ci.org/browse/JENKINS-41522 to track the other changes in https://github.com/jenkinsci/github-branch-source-plugin/pull/109
            stephenconnolly Stephen Connolly made changes -
            Link This issue is related to JENKINS-41522 [ JENKINS-41522 ]
            Hide
            myoung34 marc young added a comment -

            Stephen Connolly thanks so much!

            Show
            myoung34 marc young added a comment - Stephen Connolly thanks so much!
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Stephen Connolly
            Path:
            src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource.java
            http://jenkins-ci.org/commit/github-branch-source-plugin/93d81cf26e8b46afdedf91c617c342b278257b4b
            Log:
            JENKINS-40652 Minimal fix for trusted revision

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Stephen Connolly Path: src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource.java http://jenkins-ci.org/commit/github-branch-source-plugin/93d81cf26e8b46afdedf91c617c342b278257b4b Log: JENKINS-40652 Minimal fix for trusted revision
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Stephen Connolly
            Path:
            src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource.java
            http://jenkins-ci.org/commit/github-branch-source-plugin/5607e401b9ac4b3f02b1c44e5b967630ad7b90b0
            Log:
            Merge pull request #114 from stephenc/jenkins-40652

            JENKINS-40652 Minimal fix for trusted revision

            Compare: https://github.com/jenkinsci/github-branch-source-plugin/compare/df02d731c5c1...5607e401b9ac

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Stephen Connolly Path: src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource.java http://jenkins-ci.org/commit/github-branch-source-plugin/5607e401b9ac4b3f02b1c44e5b967630ad7b90b0 Log: Merge pull request #114 from stephenc/jenkins-40652 JENKINS-40652 Minimal fix for trusted revision Compare: https://github.com/jenkinsci/github-branch-source-plugin/compare/df02d731c5c1...5607e401b9ac
            Hide
            stephenconnolly Stephen Connolly added a comment -

            Marking this as fixed in github-branch-source-plugin 2.0.1-beta-5 but there are likely follow-up side issues being tracked in JENKINS-41522

            Show
            stephenconnolly Stephen Connolly added a comment - Marking this as fixed in github-branch-source-plugin 2.0.1-beta-5 but there are likely follow-up side issues being tracked in JENKINS-41522
            stephenconnolly Stephen Connolly made changes -
            Resolution Fixed [ 1 ]
            Status Open [ 1 ] Resolved [ 5 ]
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Stephen Connolly
            Path:
            src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource.java
            http://jenkins-ci.org/commit/github-branch-source-plugin/bb18410cff11efd7b21ccf4c55dfb6719bb61c12
            Log:
            JENKINS-40652 Late fix

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Stephen Connolly Path: src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource.java http://jenkins-ci.org/commit/github-branch-source-plugin/bb18410cff11efd7b21ccf4c55dfb6719bb61c12 Log: JENKINS-40652 Late fix
            Hide
            stephenconnolly Stephen Connolly added a comment -

            scm-api-2.0.2
            git-2.6.4 or git-3.0.4
            branch-api-2.0.2
            bitbucket-branch-source-2.0.2
            github-branch-source-2.0.1
            cloudbees-folder-5.17
            workflow-multibranch-2.11

            Show
            stephenconnolly Stephen Connolly added a comment - scm-api-2.0.2 git-2.6.4 or git-3.0.4 branch-api-2.0.2 bitbucket-branch-source-2.0.2 github-branch-source-2.0.1 cloudbees-folder-5.17 workflow-multibranch-2.11
            stephenconnolly Stephen Connolly made changes -
            Status Resolved [ 5 ] Closed [ 6 ]
            cloudbees CloudBees Inc. made changes -
            Remote Link This issue links to "CloudBees Internal OSS-1913 (Web Link)" [ 18513 ]
            szevez John Szivós made changes -
            Link This issue is blocked by JENKINS-50290 [ JENKINS-50290 ]
            szevez John Szivós made changes -
            Link This issue is blocked by JENKINS-50290 [ JENKINS-50290 ]
            szevez John Szivós made changes -
            Link This issue is related to JENKINS-50290 [ JENKINS-50290 ]

              People

              Assignee:
              stephenconnolly Stephen Connolly
              Reporter:
              chrono Martin Ringehahn
              Votes:
              1 Vote for this issue
              Watchers:
              8 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: