When whitelisting methods, sometimes you want to whitelist specific implementation (say groovy.json.JsonBuilder.toString()) of a generic signature defined in the base type (java.lang.Object.toString() in this case.)

      In other times, you want to whitelist all the method definitions that override an interface/class method. For example, another person might want to allow all java.lang.Object.toString() invocation regardless of the receiver type.

      script-security plugin currently doesn't have means to do this in Whitelist. GroovyCallSiteSelector.method picks one Method instance and all the decision making happens on this single method call.

      Preferably, the call site selection should find the actual method definition getting invoked, as well as methods in the super types that it overrides, so that Whitelist can make decisions by using them all.

          [JENKINS-24982] Bottom-up white/blacklisting vs top-down

          Jesse Glick added a comment -

          More precisely, StaticWhitelist cannot make this distinction. A custom Whitelist can allow just JsonBuilder.toString (say) by just checking that receiver instanceof JsonBuilder. But this is awkward and cannot be done from the approval UI, only a plugin.

          Jesse Glick added a comment - More precisely, StaticWhitelist cannot make this distinction. A custom Whitelist can allow just JsonBuilder.toString (say) by just checking that receiver instanceof JsonBuilder . But this is awkward and cannot be done from the approval UI, only a plugin.

          Whitelist.permitsMethod should be exposing richer parameters that allow whitelist impls to introspect overriding relationships of methods.

          For example, a whitelist impl should be able to test if the method definition about to be dispatched overrides another method definition from a base class/interface, etc. Method is too dumb for this, and in the presence of bridge method / generics / etc, doing this correctly is quite non-trivial.

          StaticWhitelist syntax should be evolved to support both kinds of whitelisting ("allow all methods that override Object.toString()" as well "only allow JsonBuilder.toString() but not its override methods")

          Kohsuke Kawaguchi added a comment - Whitelist.permitsMethod should be exposing richer parameters that allow whitelist impls to introspect overriding relationships of methods. For example, a whitelist impl should be able to test if the method definition about to be dispatched overrides another method definition from a base class/interface, etc. Method is too dumb for this, and in the presence of bridge method / generics / etc, doing this correctly is quite non-trivial. StaticWhitelist syntax should be evolved to support both kinds of whitelisting ("allow all methods that override Object.toString() " as well "only allow JsonBuilder.toString() but not its override methods")

          Cyrille Le Clerc added a comment - - edited

          As discussed with jglick, we have a similar use case with XML processing and the MarkupBuilder http://www.groovy-lang.org/processing-xml.html

          Problematic whitelistings:

          • Read xml with XmlSlurper: "method groovy.lang.GroovyObject getProperty java.lang.String"
          • Write XML with Markupbuilder: "method groovy.lang.GroovyObject invokeMethod java.lang.String java.lang.Object" but then we have the issue JENKINS-32766

          Cyrille Le Clerc added a comment - - edited As discussed with jglick , we have a similar use case with XML processing and the MarkupBuilder http://www.groovy-lang.org/processing-xml.html Problematic whitelistings: Read xml with XmlSlurper: "method groovy.lang.GroovyObject getProperty java.lang.String" Write XML with Markupbuilder: "method groovy.lang.GroovyObject invokeMethod java.lang.String java.lang.Object" but then we have the issue JENKINS-32766

            jglick Jesse Glick
            kohsuke Kohsuke Kawaguchi
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated: