The Puppet Labs Issue Tracker has Moved: https://tickets.puppetlabs.com
An exec provider that executes unfiltered bash code.
|Affected Puppet version:||Branch:||https://github.com/daniel-pittman/puppet/tree/feature/2.6.next/4884-an-exec-provider-that-executes-unfiltered-bash-code|
Ticket tracking is now hosted in JIRA: https://tickets.puppetlabs.com
This issue is currently not available for export. If you are experiencing the issue described below, please file a new ticket in JIRA. Once a new ticket has been created, please add a link to it that points back to this Redmine ticket.
Some users have been using a workaround to allow Exec resources to run inline bash scripts (rather than calling an executable), as seen in the comments on #4288
We should consider adding this as a feature; I am opening this ticket for discussion.
#1 Updated by Alan Barrett over 3 years ago
I have some very complex exec commands, extending over several lines, with lots of embedded backslashes and quotation marks. Having to change from command => “complex command” to command => “/bin/sh -c ‘complex command with even more backslashes than before’” will be both error prone (it’s easy to get the wrong number of backslashes) and annoying (it seems as though puppet is deliberately removing useful behaviour for no obvious benefit).
If we get the ability to pass arrays to exec, as in command => [“/bin/sh”, “-c”, “complex command”], then it solves my problem.
#3 Updated by Alan Barrett over 3 years ago
Alan Harder wrote:
Perhaps command => ‘…’ keeps current behavior […]
Current behaviour is that the command gets passed to the shell. This happens after some sanity checks which sometimes give the wrong answers (see issues #4859, #4288, #4299), but it does happen. Or did puppet-2.6 change this to not use the shell at all?
#7 Updated by Ian Ward Comfort over 3 years ago
Supplying an array would work for me. It’s probably the way execs always should have worked; we’ve gotten used to treating commands like system(3) invocations and now have a lot of manifests to update before moving to 2.6.x. (And having Puppet parse and split my string is not a whole lot better than the shell doing it — at least I know shell parsing rules.)
FWIW (maybe not much), this feature is probably a 2.6.x blocker for us, since double- and triple-quoting things for the shell is likely to get painful.
#12 Updated by Stefan Schulte over 3 years ago
Maybe we can simulate the behaviour of perl’s system(). If you pass a single value that has metacharacters in it (space, star, simicolon etc) use the shell. Otherwise don’t use it. So we would have the following behaviour:
1) exec => 'uname -a' # use shell (cause the space) 2) exec => 'uname' # don't use shell 3) exec => 'start_$HOSTNAME' # use shell (cause the dollar sign) 4) exec => [ 'uname -a' ] # use shell (cause the space) 5) exec => [ 'uname', '-a' ] # don't use shell
Note: There is no way for the type (at least not that I am aware of) to differ between 1) and 4). Propertyvalues are always arrays.
The described behaviour may break in a few situations (programname with whitespaces in it) and maybe we can spent a parameter (surpress_shell or use_shell or anything similar) for this situations
#14 Updated by konrad rzentarzewski about 3 years ago
wouldn’t it be reasonable to revert to previous behaviour for backwards compatibility sake? at least unless it’s fixed. for now it only leads to changing all inline execs from “if true ; then script ; fi” to “/bin/true ; if true ; then script ; fi” which provides no additional security whatsoever and thus defeats a reason for this change.
#17 Updated by Daniel Pittman about 3 years ago
After investigation the sanest long term approach seems to be that we do two things:
First, we define that the behaviour of
command as an array is that it will directly execute the arguments as passed. This extends the current
exec parameter to add new semantics, and allows people to pass things to alternate interpreters reasonably easily.
Second, we add a “shell” provider to the exec type (not default), which passes the string unmodified to
/bin/sh to execute; this does not provide any additional configuration to allow other shells or anything like that. If those are desired features then the array form with the default provider should be used.
Finally, this interacts with the
path parameter as follows:
Default provider, string: as 2.6 behaves now, which treats the first whitespace-separated word as a command, and qualifies that with path if it is not already absolute.
Default provider, array: treats the first array entry as the command, qualifies it across path if not absolute.
Shell provider: sets PATH in the environment to the supplied value before executing the subshell, does nothing to try and make this live through the shell initialization files.
#19 Updated by Daniel Pittman about 3 years ago
- Branch set to https://github.com/daniel-pittman/puppet/tree/feature/2.6.next/4884-an-exec-provider-that-executes-unfiltered-bash-code
Nigel Kersten wrote:
Are you working on this issue actively Daniel?
Yes, and I failed to communicate that to the community at large. I might just fix that failure of my process.
I have a branch in progress at https://github.com/daniel-pittman/puppet/tree/feature/2.6.next/4884-an-exec-provider-that-executes-unfiltered-bash-code that is working toward an implementation.
Be warned, if you plan to depend on this code, that I am force-pushing that branch occasionally if I rewrite history, and I will eventually squash the long series of work-in-progress commits into a coherent patch series for public consumption.
The code is not currently usable, but instead is focused on adding tests to the
exec type, and eventually providers, to ensure they don’t regress during this set of code changes.
#24 Updated by Matt Robinson about 3 years ago
Moving forward the shell provider should use the array form of commands to Util.execute to prepend the “sh -c” instead of doing it in string interpolation. Also, to test this, rather than doing expects on execute we should actually let a few simple shell commands run (true, false, echo) and then assert what the output is on the type returns property.
#30 Updated by eric sorenson almost 3 years ago
- Status changed from Closed to Re-opened
- Target version deleted (
Sorry to update on a closed ticket but this is a pretty big behavioural change that isn’t documented in the release notes, except the following under 2.6.8:
Notable Features and Bug Fixes Bug #4884: Shell exec provider that executes code as a raw shell script
Nor under the current type reference except to note that ‘provider =>’ shell is available.
Could this be punched up a bit? Maybe to include Markus' discussion of the security issues at http://projects.puppetlabs.com/issues/4288#note-16
Minimally, some kind of caveat in the 2.6.0 “New features” section of release notes, ideally there plus a description on the exec type about how the command attribute’s parameters get passed to the shell.