-
Bug
-
Resolution: Fixed
-
Minor
-
CentOS 6.8
Jenkins 2.38
durable-task-plugin 1.12
workflow-aggregator (pipeline) 2.4 and all its dependencies:
workflow-cps (version:2.17)
workflow-support (version:2.5)
workflow-basic-steps (version:2.1)
pipeline-input-step (version:2.1)
pipeline-milestone-step (version:1.0)
pipeline-build-step (version:2.2)
pipeline-stage-view (version:2.0)
workflow-multibranch (version:2.8)
workflow-durable-task-step (version:2.4)
workflow-api (version:2.3)
pipeline-stage-step (version:2.2)
workflow-scm-step (version:2.2)
workflow-cps-global-lib (version:2.3)
workflow-step-api (version:2.3)
workflow-job (version:2.6)CentOS 6.8 Jenkins 2.38 durable-task-plugin 1.12 workflow-aggregator (pipeline) 2.4 and all its dependencies: workflow-cps (version:2.17) workflow-support (version:2.5) workflow-basic-steps (version:2.1) pipeline-input-step (version:2.1) pipeline-milestone-step (version:1.0) pipeline-build-step (version:2.2) pipeline-stage-view (version:2.0) workflow-multibranch (version:2.8) workflow-durable-task-step (version:2.4) workflow-api (version:2.3) pipeline-stage-step (version:2.2) workflow-scm-step (version:2.2) workflow-cps-global-lib (version:2.3) workflow-step-api (version:2.3) workflow-job (version:2.6)
When I run a Jenkinsfile that has a sh step that uses an environment variable (such as a password) that has two $$ in a row, they get replaced with one $.
Here's the steps to reproduce:
1. Make a global credential id "foo", username "foo", password "bar$$baz"
2. Use this Jenkinsfile:
node('linux') { withCredentials([[$class: 'UsernamePasswordMultiBinding', credentialsId: 'foo', passwordVariable: 'PASSWORD', usernameVariable: 'USERNAME']]) { echo "Username: ${env.USERNAME}" echo "Password: ${env.PASSWORD}" sh 'echo Username: $USERNAME, Password: $PASSWORD' } }
When the build runs, the echo steps properly echo the user/pass which are then masked. But the shell step doesn't mask the password, which is incorrect. It has lost a $
[Pipeline] echo Username: **** [Pipeline] echo Password: **** [Pipeline] sh [s_example] Running shell script + echo Username: ****, Password: bar$baz Username: ****, Password: bar$baz
- depends on
-
-
- In Review
-
- is blocked by
-
-
- Reopened
-
- relates to
-
-
- Resolved
-
-
-
- Resolved
-
- links to
[JENKINS-40734] Shell step cannot use environment variables that contain $$
Oh, I thought you said that
sh ''' some-cli do stuff "$USERNAME" "$PASSWORD" '''
did work.
I can indeed see how that wouldn't work. However, you should be able to programmatically escape the definition of those environment variables---as opposed to forcing Jenkins to escape such definitions in all cases as the PR does. If you could show/summarize the code that sets those environment variables I'd be happy to try to figure out how.
John Ericson
added a comment - - edited b_dean
Oh, I thought you said that
sh '''
some-cli do stuff "$USERNAME" "$PASSWORD"
'''
did work.
I can indeed see how that wouldn't work. However, you should be able to programmatically escape the definition of those environment variables---as opposed to forcing Jenkins to escape such definitions in all cases as the PR does. If you could show/summarize the code that sets those environment variables I'd be happy to try to figure out how. The environment variables are being set by withCredentials. If you look at the original description on this issue, you'll find complete reproduction steps. The scenario where I was running into this bug was with credentials, but jglick said that it would be a problem with any environment variables. A Jenkinsfile like this would also not have the correct values for the environment variables:
node{
withEnv(['USERNAME=foo', 'PASSWORD=bar$$baz']) {
echo "Username: ${env.USERNAME}"
echo "Password: ${env.PASSWORD}"
sh 'echo Username: "$USERNAME", Password: "$PASSWORD"'
}
}
which outputs:
Started by user admin
[Pipeline] node
Running on master in /var/jenkins_home/workspace/test
[Pipeline] {
[Pipeline] withEnv
[Pipeline] {
[Pipeline] echo
Username: foo
[Pipeline] echo
Password: bar$$baz
[Pipeline] sh
[test] Running shell script
+ echo Username: foo, Password: bar$baz
Username: foo, Password: bar$baz
[Pipeline] }
[Pipeline] // withEnv
[Pipeline] }
[Pipeline] // node
[Pipeline] End of Pipeline
Finished: SUCCESS
note that the environment variables are correct on the env global variable, but they are wrong in the shell step. That output is using durable-task v1.12
John Ericson
added a comment - b_dean Ah, my apologies with regards to the OP---I did not realize withCredentials was itself defining the environment variables.
What I and others do is like
node{
withEnv(['PATH=$PATH:/extra/directory/bin']) {
sh 'some-cmd' // which happens to be in `/extra/directory/bin`
}
}
which works today and becomes impossible if sh escapes environment variables.
Both in the interest of not making breaking changes, and in the interest of maximum expressiveness, I say the solution here is to change withCredentials so that it escapes the username and password. Adding manual escaping to functions like withCredentials may feel like whack-a-mole, but so is adding escaping to sh, and it is not possible for me and others to "un0escape" for our use-case.
John Ericson
added a comment - b_dean Ah, my apologies with regards to the OP---I did not realize withCredentials was itself defining the environment variables.
What I and others do is like
node{
withEnv([ 'PATH=$PATH:/extra/directory/bin' ]) {
sh 'some-cmd' // which happens to be in `/extra/directory/bin`
}
}
which works today and becomes impossible if sh escapes environment variables.
Both in the interest of not making breaking changes, and in the interest of maximum expressiveness, I say the solution here is to change withCredentials so that it escapes the username and password. Adding manual escaping to functions like withCredentials may feel like whack-a-mole, but so is adding escaping to sh , and it is not possible for me and others to "un0escape" for our use-case. I just tested how this would work with the envinject plugin in a freestyle job and it also replaces the $$ with a single $ in a password. Here's the config.xml
<project> <actions/> <description/> <keepDependencies>false</keepDependencies> <properties/> <scm class="hudson.scm.NullSCM"/> <canRoam>true</canRoam> <disabled>false</disabled> <blockBuildWhenDownstreamBuilding>false</blockBuildWhenDownstreamBuilding> <blockBuildWhenUpstreamBuilding>false</blockBuildWhenUpstreamBuilding> <triggers/> <concurrentBuild>false</concurrentBuild> <builders> <EnvInjectBuilder plugin="envinject@1.93.1"> <info> <propertiesContent>DEF=$ABC:456</propertiesContent> </info> </EnvInjectBuilder> <hudson.tasks.Shell> <command> echo Password: "$PASSWORD" echo ABC: $ABC echo DEF: $DEF </command> </hudson.tasks.Shell> </builders> <publishers/> <buildWrappers> <EnvInjectBuildWrapper plugin="envinject@1.93.1"> <info> <propertiesContent>ABC=123</propertiesContent> <loadFilesFromMaster>false</loadFilesFromMaster> </info> </EnvInjectBuildWrapper> <EnvInjectPasswordWrapper plugin="envinject@1.93.1"> <injectGlobalPasswords>false</injectGlobalPasswords> <maskPasswordParameters>true</maskPasswordParameters> <passwordEntries> <EnvInjectPasswordEntry> <name>PASSWORD</name> <value>kGFMn/q9EkRqYVDkmuj/tQMlyX/YuesVmq7bZqTGuBs=</value> </EnvInjectPasswordEntry> </passwordEntries> </EnvInjectPasswordWrapper> </buildWrappers> </project>
that outputs:
Started by user admin [EnvInject] - Loading node environment variables. Building in workspace /var/jenkins_home/workspace/test [EnvInject] - Executing scripts and injecting environment variables after the SCM step. [EnvInject] - Injecting as environment variables the properties content ABC=123 [EnvInject] - Variables injected successfully. [EnvInject] - Mask passwords that will be passed as build parameters. [EnvInject] - Injecting environment variables from a build step. [EnvInject] - Injecting as environment variables the properties content DEF=123:456 [EnvInject] - Variables injected successfully. [test] $ /bin/sh -xe /tmp/hudson7658063203774926176.sh + echo Password: bar$baz Password: bar$baz + echo ABC: 123 ABC: 123 + echo DEF: 123:456 DEF: 123:456 Finished: SUCCESS
I also tried this using the plain-credentials and credentials-binding plugins in a freestyle job. Same thing, $$ in the password gets replaced with $. Here's that config.xml
<project> <actions/> <description/> <keepDependencies>false</keepDependencies> <properties/> <scm class="hudson.scm.NullSCM"/> <canRoam>true</canRoam> <disabled>false</disabled> <blockBuildWhenDownstreamBuilding>false</blockBuildWhenDownstreamBuilding> <blockBuildWhenUpstreamBuilding>false</blockBuildWhenUpstreamBuilding> <triggers/> <concurrentBuild>false</concurrentBuild> <builders> <hudson.tasks.Shell> <command>echo Username: "$USERNAME" Password: "$PASSWORD"</command> </hudson.tasks.Shell> </builders> <publishers/> <buildWrappers> <org.jenkinsci.plugins.credentialsbinding.impl.SecretBuildWrapper plugin="credentials-binding@1.10"> <bindings> <org.jenkinsci.plugins.credentialsbinding.impl.UsernamePasswordMultiBinding> <credentialsId>foo</credentialsId> <usernameVariable>USERNAME</usernameVariable> <passwordVariable>PASSWORD</passwordVariable> </org.jenkinsci.plugins.credentialsbinding.impl.UsernamePasswordMultiBinding> </bindings> </org.jenkinsci.plugins.credentialsbinding.impl.SecretBuildWrapper> </buildWrappers> </project>
which outputs:
Started by user admin [EnvInject] - Loading node environment variables. Building in workspace /var/jenkins_home/workspace/test2 [test2] $ /bin/sh -xe /tmp/hudson6763543920526348076.sh + echo Username: **** Password: bar$baz Username: **** Password: bar$baz Finished: SUCCESS
So I guess durable-task was at least consistent with how things work in the older plugins and freestyle jobs. I'm not really sure what the answer is for dealing with passwords that look like they have bash variable expansion in them.
I'm not really sure what the answer is for dealing with passwords that look like they have bash variable expansion in them.
Having withCredentials escape the username and password before sticking them in the environment variables should do the trick, right?
John Ericson
added a comment - - edited I'm not really sure what the answer is for dealing with passwords that look like they have bash variable expansion in them.
Having withCredentials escape the username and password before sticking them in the environment variables should do the trick, right? Yes, that would be answer for withCredentials. But it would make the pipelines behave a bit differently than the envinject and credentials-binding plugins which do not escape $ in passwords. Maybe they should be fixed too.
Freestyle projects may always have been broken in this respect, but not my problem. This was clearly a bug, and 1.13 fixes it. I am tracking other use cases in JENKINS-41339.
withEnv(['PATH=$PATH:/extra/directory/bin'])
is incorrect. You may use either
withEnv(["PATH=$PATH:/extra/directory/bin"])
(interpolating the current value in Groovy) or
withEnv(['PATH+EXTRA=/extra/directory/bin'])
as documented in the withEnv step.
John Ericson
added a comment - jglick I'm no fan of barely-planned string evaluation either, but I don't think it's not fair to simply say "freestyle jobs are broken": values in env may come from either the Jenkins GUI or groovy (e.g. withEnv) and there is no way of knowing. Long predating any pipeline work, the effective behavior of GUI-defined environment variables has been established that they may contain variable references. Freestyle jobs implement this, not define it.
You can make that the semantics of withEnv if you want, but then to handle the GUI case correctly I see the only option being escaping the withEnv arguments before they are put in env so the GUI ones still get expanded correctly.
John Ericson
added a comment - jglick I'm no fan of barely-planned string evaluation either, but I don't think it's not fair to simply say "freestyle jobs are broken": values in env may come from either the Jenkins GUI or groovy (e.g. withEnv ) and there is no way of knowing. Long predating any pipeline work, the effective behavior of GUI-defined environment variables has been established that they may contain variable references. Freestyle jobs implement this, not define it.
You can make that the semantics of withEnv if you want, but then to handle the GUI case correctly I see the only option being escaping the withEnv arguments before they are put in env so the GUI ones still get expanded correctly.
ericson2314, what aforementioned double-quote solution? None of the things jglick mentioned as corrections to my Jenkinsfile actually worked. The problem is that if the password or environment variable has a double $ (such as "bar$$baz"), the durable task plugin is changing that to a single $ in the actual environment variable set for the sh step. I don't know how any change in quoting will fix that.