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

Script Security translates some groovy functions incorrectly/not as expected

    XMLWordPrintable

    Details

    • Sprint:
      Pipeline - July/August
    • Similar Issues:

      Description

      I have a custom class which I am using inside of some groovy functions with a list of custom methods. When I use that custom class inside of a script needed to be approved by script security it is being transformed differently than expected.

       

      When I try to do a for loop through an array and do `i++` it is being translated as i.next() instead of adding 1 and then when I am calling the creator method `def $ITEM` it is being created as a hashmap. Neither of those methods exists inside of my custom class so it is throwing errors like `No signature of method X.next()` and `unclassified new X java.util.LinkedHashMap`

       

      I would either like to get more visibility about hot the translation is happening or potentially have other default method calls.

       

      Let me know if I can provide more context here.

        Attachments

          Issue Links

            Activity

            Hide
            abayer Andrew Bayer added a comment - - edited

            I've got a minimal reproduction with script-security 1.29:

            def l = ['a/b/c', 'd/e', 'f']
            
            def str = ''
            l.each { s ->
                def tokens = s.split('/')
                for (int i = 0; i < tokens.size(); i++) {
                    str += tokens[i]
                }
            }
            
            return str
            

            Passed through the sandbox, you get org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException: Scripts not permitted to use method groovy.lang.GroovyObject invokeMethod java.lang.String java.lang.Object (Script1 next). This only happens when you've got the for (...) loop inside the .each loop, so there's something going wrong there specifically.

            Show
            abayer Andrew Bayer added a comment - - edited I've got a minimal reproduction with script-security 1.29: def l = [ 'a/b/c' , 'd/e' , 'f' ] def str = '' l.each { s -> def tokens = s.split( '/' ) for ( int i = 0; i < tokens.size(); i++) { str += tokens[i] } } return str Passed through the sandbox, you get org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException: Scripts not permitted to use method groovy.lang.GroovyObject invokeMethod java.lang.String java.lang.Object (Script1 next) . This only happens when you've got the for (...) loop inside the .each loop, so there's something going wrong there specifically.
            Hide
            abayer Andrew Bayer added a comment -

            Even more minimal!

            def cnt = 0
            [0, 1, 2, 3, 4].each {
              cnt++
            }
            return cnt
            

            That gets org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException: Scripts not permitted to use method groovy.lang.GroovyObject invokeMethod java.lang.String java.lang.Object (Script1 next) as well. So the problem seems to be the ++ inside the each.

            Show
            abayer Andrew Bayer added a comment - Even more minimal! def cnt = 0 [0, 1, 2, 3, 4].each { cnt++ } return cnt That gets org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException: Scripts not permitted to use method groovy.lang.GroovyObject invokeMethod java.lang.String java.lang.Object (Script1 next) as well. So the problem seems to be the ++ inside the each .
            Hide
            jglick Jesse Glick added a comment -

            Somehow the next method is being applied to the Script rather than the Number as it should. No idea why. Some sort of confusion about Closure handling. Is this reproducible in general script-security (not Pipeline)?

            Show
            jglick Jesse Glick added a comment - Somehow the next method is being applied to the Script rather than the Number as it should. No idea why. Some sort of confusion about Closure handling. Is this reproducible in general script-security (not Pipeline)?
            Hide
            abayer Andrew Bayer added a comment -

            Jesse Glick Yup, this is script-security, not Pipeline - I haven't verified but am pretty sure it can't be reproduced in Pipeline at all.

            I thought for a bit that it was a local vs not local variable issue, but it's not that. Still digging.

            Show
            abayer Andrew Bayer added a comment - Jesse Glick Yup, this is script-security , not Pipeline - I haven't verified but am pretty sure it can't be reproduced in Pipeline at all. I thought for a bit that it was a local vs not local variable issue, but it's not that. Still digging.
            Hide
            abayer Andrew Bayer added a comment -

            The difference between calling cnt++ outside a each loop and inside a each loop is that the first argument to Checker.checkedCall changes from this for outside:

            org.codehaus.groovy.ast.expr.VariableExpression@5c30a9b0[variable: cnt]
            

            to this for inside:

            - method 'ConstantExpression[getOwner]':
              - method 'ConstantExpression[asWritable]':
                - org.codehaus.groovy.ast.expr.VariableExpression@15c43bd9[variable: this]
                  - args:
                - args:
            

            (excuse the formatting - I've got some tools I'm polishing for debugging AST transformations that I'm using here)
            Now I'm figuring out why. =)

            Show
            abayer Andrew Bayer added a comment - The difference between calling cnt++ outside a each loop and inside a each loop is that the first argument to Checker.checkedCall changes from this for outside: org.codehaus.groovy.ast.expr.VariableExpression@5c30a9b0[variable: cnt] to this for inside: - method 'ConstantExpression[getOwner]' : - method 'ConstantExpression[asWritable]' : - org.codehaus.groovy.ast.expr.VariableExpression@15c43bd9[variable: this ] - args: - args: (excuse the formatting - I've got some tools I'm polishing for debugging AST transformations that I'm using here) Now I'm figuring out why. =)
            Hide
            abayer Andrew Bayer added a comment -

            Ok, looks like the problem is that our checks to determine whether to use the getOwner etc closure call are based on whether we're in a closure and the expression in question returns true for isImplicitThis() - but that almost always returns true, and isn't fine-grained enough a heuristic. I'm working on a better solution.

            Show
            abayer Andrew Bayer added a comment - Ok, looks like the problem is that our checks to determine whether to use the getOwner etc closure call are based on whether we're in a closure and the expression in question returns true for isImplicitThis() - but that almost always returns true, and isn't fine-grained enough a heuristic. I'm working on a better solution.
            Hide
            abayer Andrew Bayer added a comment -
            Show
            abayer Andrew Bayer added a comment - Preliminary PR up at https://github.com/jenkinsci/groovy-sandbox/pull/36

              People

              • Assignee:
                abayer Andrew Bayer
                Reporter:
                ataylor Alex Taylor
              • Votes:
                0 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: