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

Safely pass args to sh step

    XMLWordPrintable

    Details

    • Similar Issues:

      Description

      As a Jenkins user, I'll like to use sh semantics to execute commands with args.

      I believe Jenkins needs an args: parameter for the sh pipeline step, right now we have script:, returnStdout: and returnStatus: both useful to avoid messing around with shell redirection. However, is clearly impossible or at least difficult to have clean (and secure) executions with sh if they involve string interpolations, maybe Jenkins should do this automatically, but I'm not sure if that kind of magic could be counter productive, I rather depend on args.

      Like:

      sh(script: "echo", args: ["hello", "world", env.MY_ENV, my_other_def])

      or a new pipeline step to avoid overloading sh with different behaviour

      command(name: "echo", args: ["hello", "world", env.MY_ENV, my_other_def])

      On both cases echo the program, not the shell built-in should be executed, Jenkins should look for the program on the system's $PATH / %PATH% but also an absolute path should be supported too, like a regular Groovy execute() on a List.

      ["/bin/echo", "foo", "bar"].execute()

      Is not common to have Groovy's execute() allowed on Jenkins sandboxed environment, probably for very good reasons.

      Also even if execute() is allowed on the Jenkins sandbox, sh semantics are way more convenient.

      For reference:

      def proc = ['ls', '/meh'].execute()
      println proc.getText()
      println proc.exitValue() != 0

      Where is stderr?

        Attachments

          Issue Links

            Activity

            Hide
            jglick Jesse Glick added a comment -

            From a dev list conversation: this aspect of some Bourne shells means that an implementation which ultimately runs a shell, as opposed to Runtime.exec or the like, may omit certain environment variables.

            Show
            jglick Jesse Glick added a comment - From a dev list conversation: this aspect of some Bourne shells means that an implementation which ultimately runs a shell, as opposed to Runtime.exec or the like, may omit certain environment variables.
            Hide
            mslattery Michael Slattery added a comment -

            In a shared library add /var/shell.groovy:

            def run(args) {
                sh args.collect { "'" + it.replace("'","'\"'\"'") + "'" }.join(' ')
            }
            

            Usage in a pipeline:

            shell(['docker', 'ps'])
            

            (For more info see https://stackoverflow.com/a/1250279)

            Show
            mslattery Michael Slattery added a comment - In a shared library add /var/shell.groovy: def run(args) { sh args.collect { " '" + it.replace( "' " , " '\" ' \ " '" ) + "' " }.join( ' ' ) } Usage in a pipeline: shell([ 'docker' , 'ps' ]) (For more info see https://stackoverflow.com/a/1250279 )
            Hide
            jglick Jesse Glick added a comment -

            Yes for sh that would be the workaround, until the Jenkins step does something similar as a standard function. For bat it is a little trickier; as per this and this it might suffice to use something like the following (untested):

            bat args.collect {it.replaceAll('[^a-zA-Z0-9_.\r\n-]', '^$0').replaceAll('\r?\n', '^\r\n\r\n')}.join(' ')
            
            Show
            jglick Jesse Glick added a comment - Yes for sh that would be the workaround, until the Jenkins step does something similar as a standard function. For bat it is a little trickier; as per this and this it might suffice to use something like the following ( untested ): bat args.collect {it.replaceAll( '[^a-zA-Z0-9_.\r\n-]' , '^$0' ).replaceAll( '\r?\n' , '^\r\n\r\n' )}.join( ' ' )
            Hide
            docwhat Christian Höltje added a comment -

            Michael Slattery That isn't sufficient for all cases, alas.

            It will work for pure ascii (I think). Depending on the locale, there are many other things that'll cause problems.

            That's why I use withEnv(){ ... } as a work around until we can pass an array to {{sh.}}

            Show
            docwhat Christian Höltje added a comment - Michael Slattery That isn't sufficient for all cases, alas. It will work for pure ascii (I think). Depending on the locale, there are many other things that'll cause problems. That's why I use withEnv(){ ... } as a work around until we can pass an array to {{sh .}}
            Hide
            skorhone Sami Korhonen added a comment - - edited

            If I got it correctly, implementing this would require changes to https://github.com/jenkinsci/durable-task-plugin/blob/master/src/main/java/org/jenkinsci/plugins/durabletask/BourneShellScript.java

            This algorithm seems to do following (there's also separate algorithm for binary execution path, but I think most of us do not care about that one):

            1. Write script to a file
            2. Create a launcher command (a command that invokes the script from file)
            3. Invoke launcher command using launcher (this causes launcher command to be parsed to a list of args)

            Launcher does support passing arguments and in fact launcher command actually uses arguments. Launcher command is sh -c '.... sh scriptfile ....'. Second sh is optional and depends on scriptfile beginning with #! .

            My initial thought was that, what if we just passed arguments by serializing them to launcher command. Serializing parameters (escaping) is the problem that we wanted to avoid, so this would move problem from one point to another.

            Then I actually had a look at {{sh }}documentation. Documentation clearly states that you can in pass arguments for command string (-c). With this new information I did few google searches and ended up with a simple test:

            sh -c '(echo numbers: "$@") > test' zero one two && cat test
            numbers: one two

            Test echos string numbers and arguments passed to sh command. Above test is executing script the same way that BourneShellScript is doing. Code:

            cmdString = 'command'
            cmd.addAll(Arrays.asList("sh", "-c", "(" + cmdString + ") >&- 2>&- &"));
            

            Results to:

            sh -c '(command) >&- 2>&- &'

            So, if we modified code to:

            // Pass all arguments to scriptfile (while loops)
            cmdString = 'command "$@"'
            
            // Pass arguments to launcher command
            cmd.addAll(Arrays.asList("sh", "-c", "(" + cmdString + ") >&- 2>&- &"));
            cmd.add('sh')
            cmd.addAll(argumentsPassedToSh)

            We would end up having:

            sh -c '(command "$@") >&- 2>&- &' sh arg1 arg... argN
            

            And as a result Jenkins might just be able to handle arguments properly. I'm too busy to test/implement this change myself, but I hope that someone can find this useful.

             

            Show
            skorhone Sami Korhonen added a comment - - edited If I got it correctly, implementing this would require changes to  https://github.com/jenkinsci/durable-task-plugin/blob/master/src/main/java/org/jenkinsci/plugins/durabletask/BourneShellScript.java This algorithm seems to do following (there's also separate algorithm for binary execution path, but I think most of us do not care about that one): Write script to a file Create a launcher command (a command that invokes the script from file) Invoke launcher command using launcher (this causes launcher command to be parsed to a list of args) Launcher does support passing arguments and in fact launcher command actually uses arguments. Launcher command is sh -c '.... sh scriptfile ....' . Second sh is optional and depends on scriptfile beginning with #! . My initial thought was that, what if we just passed arguments by serializing them to launcher command. Serializing parameters (escaping) is the problem that we wanted to avoid, so this would move problem from one point to another. Then I actually had a look at {{sh }}documentation. Documentation clearly states that you can in pass arguments for command string (-c). With this new information I did few google searches and ended up with a simple test: sh -c '(echo numbers: "$@" ) > test' zero one two && cat test numbers: one two Test echos string numbers and arguments passed to sh command. Above test is executing script the same way that BourneShellScript is doing. Code: cmdString = 'command' cmd.addAll(Arrays.asList( "sh" , "-c" , "(" + cmdString + ") >&- 2>&- &" )); Results to: sh -c '(command) >&- 2>&- &' So, if we modified code to: // Pass all arguments to scriptfile ( while loops) cmdString = 'command "$@" ' // Pass arguments to launcher command cmd.addAll(Arrays.asList( "sh" , "-c" , "(" + cmdString + ") >&- 2>&- &" )); cmd.add( 'sh' ) cmd.addAll(argumentsPassedToSh) We would end up having: sh -c '(command "$@" ) >&- 2>&- &' sh arg1 arg... argN And as a result Jenkins might just be able to handle arguments properly. I'm too busy to test/implement this change myself, but I hope that someone can find this useful.  

              People

              Assignee:
              Unassigned Unassigned
              Reporter:
              andresvia Andres V
              Votes:
              17 Vote for this issue
              Watchers:
              20 Start watching this issue

                Dates

                Created:
                Updated: