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

RejectedAccessException thrown but no pending script approval added

    • workflow-cps 2.67

      When using

      new GetMethod(url)

      from

      import org.apache.commons.httpclient.HttpClient
      import org.apache.commons.httpclient.methods.GetMethod

      directly in a Workflow script pasted into the UI, everything works as expected.

      When the script is loaded with the file loader plugin during the Workflow script, the following error occurs:

      org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException: Scripts not permitted to use new org.apache.commons.httpclient.methods.GetMethod java.lang.String

      No pending script approval is generated.

          [JENKINS-34973] RejectedAccessException thrown but no pending script approval added

          Jesse Glick added a comment -

          Adjusted summary to reflect the actual bug.

          Anyway you should avoid using libraries like this from Pipeline script. See this plugin, or simply call tools like curl from a sh or bat step.

          Jesse Glick added a comment - Adjusted summary to reflect the actual bug. Anyway you should avoid using libraries like this from Pipeline script. See this plugin , or simply call tools like curl from a sh or bat step.

          Marcus Philip added a comment -

          I have same problem using groovy.json.JsonSlurper().parse(url).
          How am I supposed to do that?

          Marcus Philip added a comment - I have same problem using groovy.json.JsonSlurper().parse(url). How am I supposed to do that?

          Marcus Philip added a comment -

          It seems the workaround is to do groovy.json.JsonSlurper().parseText(url.getText()).

          This might be due to the groovy version used under the hood in pipeline plugin, but I fail to find out what this is.

          Marcus Philip added a comment - It seems the workaround is to do groovy.json.JsonSlurper().parseText(url.getText()). This might be due to the groovy version used under the hood in pipeline plugin, but I fail to find out what this is.

          Sebastian Pieck added a comment - - edited

          Hi,

          I'm having the same problem opening a URL-connection. This code:

          def soapUrl = new URL("http://testURL")
          def connection = soapUrl.openConnection()
          

          triggers this message: Scripts not permitted to use method java.net.URL openConnection
          So far so good, but I can't approve it, I have no option to do so.
          Using the http-request-plugin instead of native calls is a good alternative for me.
          Thanks for the suggestion

          Sebastian Pieck added a comment - - edited Hi, I'm having the same problem opening a URL-connection. This code: def soapUrl = new URL( "http: //testURL" ) def connection = soapUrl.openConnection() triggers this message: Scripts not permitted to use method java.net.URL openConnection So far so good, but I can't approve it, I have no option to do so. Using the http-request-plugin instead of native calls is a good alternative for me. Thanks for the suggestion

          Tom FENNELLY added a comment -

          I have seen this too and in my case it was not added for script approval (to /scriptApproval) because the RejectedAccessException was getting swallowed in a try catch.

          So the following would result in URLEncoder.encode getting added for script approval in /scriptApproval

          node {
              def encodedMessage = URLEncoder.encode('Hello World', 'UTF-8');
          }
          

          But the following would not ....

          node {
              try {
                  def encodedMessage = URLEncoder.encode('Hello World', 'UTF-8');
              } catch(err) {
                  echo 'There was an error';
              }
          }
          

          Tom FENNELLY added a comment - I have seen this too and in my case it was not added for script approval (to /scriptApproval) because the RejectedAccessException was getting swallowed in a try catch. So the following would result in URLEncoder.encode getting added for script approval in /scriptApproval node { def encodedMessage = URLEncoder.encode( 'Hello World' , 'UTF-8' ); } But the following would not .... node { try { def encodedMessage = URLEncoder.encode( 'Hello World' , 'UTF-8' ); } catch (err) { echo 'There was an error' ; } }

          Tom FENNELLY added a comment -

          So basically, it appears as though the RejectedAccessException must be thrown all the way out in order for the approval request to get registered.

          Tom FENNELLY added a comment - So basically, it appears as though the RejectedAccessException must be thrown all the way out in order for the approval request to get registered.

          Code changed in jenkins
          User: tfennelly
          Path:
          Jenkinsfile
          http://jenkins-ci.org/commit/blueocean-acceptance-test/540dc68522d2ceac177eae5141b8ad4fe564117f
          Log:
          Fix URLEncoder issue related to JENKINS-34973

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: tfennelly Path: Jenkinsfile http://jenkins-ci.org/commit/blueocean-acceptance-test/540dc68522d2ceac177eae5141b8ad4fe564117f Log: Fix URLEncoder issue related to JENKINS-34973

          Jesse Glick added a comment -

          So either SandboxContinuable.findRejectedAccessException is not working, or SandboxContinuable.run0 is not getting called as expected.

          Possibly I could just make ScriptApproval.accessRejected be automatic at the throw site rather than requiring integrators to catch and rethrow.

          Jesse Glick added a comment - So either SandboxContinuable.findRejectedAccessException is not working, or SandboxContinuable.run0 is not getting called as expected. Possibly I could just make ScriptApproval.accessRejected be automatic at the throw site rather than requiring integrators to catch and rethrow.

          Tom FENNELLY added a comment -

          Yeah, seems like it should get registered immediately at the point of detection Vs throwing something that then needs to be caught by whatever does the registering.

          Tom FENNELLY added a comment - Yeah, seems like it should get registered immediately at the point of detection Vs throwing something that then needs to be caught by whatever does the registering.

          Reinhold Füreder added a comment - - edited

          Since the "that then needs to be caught by whatever does the registering" is already happening, maybe the currently working approach should just be officially documented:

          try {
          	... // Something that could also throw a RejectedAccessException
          } catch (e) {
          	... // Do something due to the failure or based on the failure like sending a notification email
          
          	// While there would be no need to re-throw the exception to propagate the error (because the build result must be set
          	// to failure for email-ext anyhow before), re-throw it for e.g. script approval requests:
          	throw e
          }
          

          Reinhold Füreder added a comment - - edited Since the " that then needs to be caught by whatever does the registering " is already happening, maybe the currently working approach should just be officially documented: try { ... // Something that could also throw a RejectedAccessException } catch (e) { ... // Do something due to the failure or based on the failure like sending a notification email // While there would be no need to re- throw the exception to propagate the error (because the build result must be set // to failure for email-ext anyhow before), re- throw it for e.g. script approval requests: throw e }

          Tom FENNELLY added a comment -

          should just be officially documented

          Who reads documentation?

          (Tongue in cheek)

          Tom FENNELLY added a comment - should just be officially documented Who reads documentation? (Tongue in cheek)

          Jesse Glick added a comment -

          Possibly I could just make ScriptApproval.accessRejected be automatic at the throw site

          Actually I cannot—there is no ApprovalContext. Therefore I consider this purely a bug in workflow-cps. Unclear if there is only one mechanism by which this problem might occur, or several.

          Jesse Glick added a comment - Possibly I could just make ScriptApproval.accessRejected be automatic at the throw site Actually I cannot—there is no ApprovalContext . Therefore I consider this purely a bug in workflow-cps . Unclear if there is only one mechanism by which this problem might occur, or several.

          Jesse Glick added a comment -

          …though if GroovySandbox had an API to set a thread-local ApprovalContext, to be used from StaticWhitelist.blacklist, then SandboxContinuable could pass this rather than inspecting the Outcome, probably increasing reliability.

          Jesse Glick added a comment - …though if GroovySandbox had an API to set a thread-local ApprovalContext , to be used from StaticWhitelist.blacklist , then SandboxContinuable could pass this rather than inspecting the Outcome , probably increasing reliability.

          Sam Van Oort added a comment -

          abayer Could you please TAL?

          Sam Van Oort added a comment - abayer Could you please TAL?

          Andrew Bayer added a comment -

          Only thing that comes to mind at first glance is maybe having SandboxContinuable#findRejectedAccessException actually traverse the flow graph looking for a RejectedAccessException anywhere? Though I guess that wouldn't actually show up there if it's caught, which is the whole problem here.

          Andrew Bayer added a comment - Only thing that comes to mind at first glance is maybe having SandboxContinuable#findRejectedAccessException actually traverse the flow graph looking for a RejectedAccessException anywhere? Though I guess that wouldn't actually show up there if it's caught, which is the whole problem here.

          Jesse Glick added a comment -

          The minimal test case (thanks to abayer in JENKINS-40333) is

          catchError {Jenkins.instance}
          

          From inspecting the issue in a debugger, it is clear that Continuable.run0 is simply too coarse-grained for this purpose: it is designed to keep stepping through the program until it needs to yield, whereas a RejectedAccessException could be thrown and caught inside a single CPS VM chunk.

          So I am again looking into deprecating ScriptApproval.accessRejected and just making the approval entry addition be automatic at the call site. This will require a new API which callers like workflow-cps would need to opt in to. At the same time I think I could move ScriptApprovalNote into script-security so it would be available for all plugins, not just Pipeline. The net effect would be an API that it is both easier to use and more reliable.

          Jesse Glick added a comment - The minimal test case (thanks to abayer in JENKINS-40333 ) is catchError {Jenkins.instance} From inspecting the issue in a debugger, it is clear that Continuable.run0 is simply too coarse-grained for this purpose: it is designed to keep stepping through the program until it needs to yield, whereas a RejectedAccessException could be thrown and caught inside a single CPS VM chunk. So I am again looking into deprecating ScriptApproval.accessRejected and just making the approval entry addition be automatic at the call site. This will require a new API which callers like workflow-cps would need to opt in to. At the same time I think I could move ScriptApprovalNote into script-security so it would be available for all plugins, not just Pipeline. The net effect would be an API that it is both easier to use and more reliable.

          William Will added a comment -

          I will note that I was using a Jenkins pipeline and was having this issue when putting scripts in the Jenkins "failure" block, though not within a catch. It would seem that the "failure" block is implicitly a catch block. The workaround for me was running the script arbitrarily in one of the steps of the pipeline, so that it would come up in script approval, then moving it back to the failure block.

          William Will added a comment - I will note that I was using a Jenkins pipeline and was having this issue when putting scripts in the Jenkins "failure" block, though not within a catch. It would seem that the "failure" block is implicitly a catch block. The workaround for me was running the script arbitrarily in one of the steps of the pipeline, so that it would come up in script approval, then moving it back to the failure block.

          leemeador added a comment -

          Same thing happens when the rejected code is in the initialization code to give a @Field its value.  I suspect it becomes a static variable and the exception gets swallowed because its in the initialization for the class or called from inside, so to speak, the constructor.

          I used a line like this at the top of the script

          @Field List<String> list = [''] * 21

          leemeador added a comment - Same thing happens when the rejected code is in the initialization code to give a @Field its value.  I suspect it becomes a static variable and the exception gets swallowed because its in the initialization for the class or called from inside, so to speak, the constructor. I used a line like this at the top of the script @Field List<String> list = [''] * 21

            jglick Jesse Glick
            tobilarscheid Tobias Larscheid
            Votes:
            10 Vote for this issue
            Watchers:
            28 Start watching this issue

              Created:
              Updated:
              Resolved: