(PUP-6494) Support sensitive commands in Exec resource type.

This commit adds support for sensitive commands in the `Exec` resource type:
`command` parameters that are specified as `Sensitive.new(...)` are now
properly redacted when the command fails.  This supports using data from lookup
and hiera.

Example:
```puppet
exec { 'redacted':
  command => Sensitive.new('/usr/bin/false SECRET_TEXT')
}
```

Output:
```
Error: [command redacted] returned 1 instead of one of [0]
Error: /Stage[main]/Main/Exec[redacted]/returns: change from notrun to 0 failed:
[command redacted] returned 1 instead of one of [0]
```

lib/puppet/provider/exec.rb

@@ -8,6 +8,7 @@ def run(command, check = false)
     output = nil
     status = nil
     dir = nil
+    sensitive = resource.parameters[:command].sensitive
 
     checkexe(command)
 
@@ -23,7 +24,7 @@ def run(command, check = false)
 
     dir ||= Dir.pwd
 
-    debug "Executing#{check ? " check": ""} '#{command}'"
+    debug "Executing#{check ? " check": ""} '#{sensitive ? '[redacted]' : command}'"
     begin
       # Do our chdir
       Dir.chdir(dir) do
@@ -59,7 +60,8 @@ def run(command, check = false)
           output = Puppet::Util::Execution.execute(command, :failonfail => false, :combine => true,
                                   :uid => resource[:user], :gid => resource[:group],
                                   :override_locale => false,
-                                  :custom_environment => environment)
+                                  :custom_environment => environment,
+                                  :sensitive => sensitive)
         end
         # The shell returns 127 if the command is missing.
         if output.exitstatus == 127

lib/puppet/type.rb

@@ -1218,6 +1218,10 @@ def self.hash2resource(hash)
     resource = Puppet::Resource.new(self, title)
     resource.catalog = hash.delete(:catalog)
 
+    if sensitive = hash.delete(:sensitive_parameters)
+      resource.sensitive_parameters = sensitive
+    end
+
     hash.each do |param, value|
       resource[param] = value
     end

lib/puppet/type/exec.rb

@@ -157,7 +157,12 @@ def sync
         end
 
         unless self.should.include?(@status.exitstatus.to_s)
-          self.fail("#{self.resource[:command]} returned #{@status.exitstatus} instead of one of [#{self.should.join(",")}]")
+          if @resource.parameter(:command).sensitive
+            # Don't print sensitive commands in the clear
+            self.fail("[command redacted] returned #{@status.exitstatus} instead of one of [#{self.should.join(",")}]")
+          else
+            self.fail("'#{self.resource[:command]}' returned #{@status.exitstatus} instead of one of [#{self.should.join(",")}]")
+          end
         end
 
         event
@@ -597,5 +602,15 @@ def refresh
     def current_username
       Etc.getpwuid(Process.uid).name
     end
+
+    private
+    def set_sensitive_parameters(sensitive_parameters)
+      # Respect sensitive commands
+      if sensitive_parameters.include?(:command)
+        sensitive_parameters.delete(:command)
+        parameter(:command).sensitive = true
+      end
+      super(sensitive_parameters)
+    end
   end
 end

lib/puppet/util/execution.rb

@@ -153,15 +153,18 @@ def self.execute(command, options = NoOptionsSpecified)
         :squelch => false,
         :override_locale => true,
         :custom_environment => {},
+        :sensitive => false,
     }
 
     options = default_options.merge(options)
 
-    if command.is_a?(Array)
+    if options[:sensitive]
+      command_str = '[redacted]'
+    elsif command.is_a?(Array)
       command = command.flatten.map(&:to_s)
-      str = command.join(" ")
+      command_str = command.join(" ")
     elsif command.is_a?(String)
-      str = command
+      command_str = command
     end
 
     user_log_s = ''
@@ -176,9 +179,9 @@ def self.execute(command, options = NoOptionsSpecified)
     end
 
     if respond_to? :debug
-      debug "Executing#{user_log_s}: '#{str}'"
+      debug "Executing#{user_log_s}: '#{command_str}'"
     else
-      Puppet.debug "Executing#{user_log_s}: '#{str}'"
+      Puppet.debug "Executing#{user_log_s}: '#{command_str}'"
     end
 
     null_file = Puppet.features.microsoft_windows? ? 'NUL' : '/dev/null'
@@ -229,7 +232,7 @@ def self.execute(command, options = NoOptionsSpecified)
       end
 
       if options[:failonfail] and exit_status != 0
-        raise Puppet::ExecutionFailure, "Execution of '#{str}' returned #{exit_status}: #{output.strip}"
+        raise Puppet::ExecutionFailure, "Execution of '#{command_str}' returned #{exit_status}: #{output.strip}"
       end
     ensure
       if !options[:squelch] && stdout

spec/unit/type/exec_spec.rb

@@ -67,7 +67,12 @@ def exec_tester(command, exitstatus = 0, rest = {})
 
       it "should report a failure" do
         expect { exec_tester('false', 1).refresh }.
-          to raise_error(Puppet::Error, /^false returned 1 instead of/)
+          to raise_error(Puppet::Error, /^'false' returned 1 instead of/)
+      end
+
+      it "should redact sensitive commands on failure" do
+        expect { exec_tester('false', 1, :sensitive_parameters => [:command]).refresh }.
+            to raise_error(Puppet::Error, /^\[command redacted\] returned 1 instead of/)
       end
 
       it "should not report a failure if the exit status is specified in a returns array" do
@@ -76,7 +81,12 @@ def exec_tester(command, exitstatus = 0, rest = {})
 
       it "should report a failure if the exit status is not specified in a returns array" do
         expect { exec_tester('false', 1, :returns => [0, 100]).refresh }.
-          to raise_error(Puppet::Error, /^false returned 1 instead of/)
+          to raise_error(Puppet::Error, /^'false' returned 1 instead of/)
+      end
+
+      it "should report redact sensitive commands if the exit status is not specified in a returns array" do
+        expect { exec_tester('false', 1, :returns => [0, 100], :sensitive_parameters => [:command]).refresh }.
+            to raise_error(Puppet::Error, /^\[command redacted\] returned 1 instead of/)
       end
 
       it "should log the output on success" do
@@ -106,7 +116,19 @@ def exec_tester(command, exitstatus = 0, rest = {})
       it "should log the output on failure" do
         output = "output1\noutput2\n"
         expect { exec_tester('false', 1, :output => output, :logoutput => :on_failure).refresh }.
-          to raise_error(Puppet::Error, /^false returned 1 instead of/)
+          to raise_error(Puppet::Error, /^'false' returned 1 instead of/)
+
+        output.split("\n").each do |line|
+          log = @logs.shift
+          expect(log.level).to eq(:err)
+          expect(log.message).to eq(line)
+        end
+      end
+
+      it "should redact the command on failure" do
+        output = "output1\noutput2\n"
+        expect { exec_tester('false', 1, :output => output, :logoutput => :on_failure, :sensitive_parameters => [:command]).refresh }.
+            to raise_error(Puppet::Error, /^\[command redacted\] returned 1 instead of/)
 
         output.split("\n").each do |line|
           log = @logs.shift
@@ -121,7 +143,22 @@ def exec_tester(command, exitstatus = 0, rest = {})
         expect {
           exec_tester('false', 1, :output => output, :returns => [0, 100],
                :logoutput => :on_failure).refresh
-        }.to raise_error(Puppet::Error, /^false returned 1 instead of/)
+        }.to raise_error(Puppet::Error, /^'false' returned 1 instead of/)
+
+        output.split("\n").each do |line|
+          log = @logs.shift
+          expect(log.level).to eq(:err)
+          expect(log.message).to eq(line)
+        end
+      end
+
+      it "should redact the command on failure when returns is specified as an array" do
+        output = "output1\noutput2\n"
+
+        expect {
+          exec_tester('false', 1, :output => output, :returns => [0, 100],
+                      :logoutput => :on_failure, :sensitive_parameters => [:command]).refresh
+        }.to raise_error(Puppet::Error, /^\[command redacted\] returned 1 instead of/)
 
         output.split("\n").each do |line|
           log = @logs.shift

spec/unit/util/execution_spec.rb

@@ -512,6 +512,10 @@ def stub_process_wait(exitstatus)
         Puppet::Util::Execution.expects(:debug).with("Executing with uid=100 gid=mygroup: 'echo hello'")
         Puppet::Util::Execution.execute('echo hello', {:uid => 100, :gid => 'mygroup'})
       end
+      it 'should redact commands in debug output when passed sensitive option' do
+        Puppet::Util::Execution.expects(:debug).with("Executing: '[redacted]'")
+        Puppet::Util::Execution.execute('echo hello', {:sensitive => true})
+      end
     end
 
     describe "after execution" do
@@ -587,6 +591,14 @@ def stub_process_wait(exitstatus)
         }.to raise_error(Puppet::ExecutionFailure, /Execution of 'fail command' returned 1/)
       end
 
+      it "should raise an error with redacted sensitive command if failonfail is true and the child failed" do
+        stub_process_wait(1)
+
+        expect {
+          subject.execute('fail command', :failonfail => true, :sensitive => true)
+        }.to raise_error(Puppet::ExecutionFailure, /Execution of '\[redacted]' returned 1/)
+      end
+
       it "should not raise an error if failonfail is false and the child failed" do
         stub_process_wait(1)