(FM-7698) implement sensitive:true handling

.travis.yml

@@ -25,7 +25,7 @@ matrix:
     - env: RVM="jruby-9.1.9.0" PUPPET_GEM_VERSION='~> 5' JRUBY_OPTS="--debug"
       before_cache: pushd ~/.rvm && rm -rf archives rubies/ruby-2.2.7 rubies/ruby-2.3.4 && popd
       cache:
-        bundler: true
+        bundler: false
         directories: ~/.rvm
       before_install: rvm use jruby-9.1.9.0 --install --binary --fuzzy
     - rvm: 2.4.3

lib/puppet/resource_api/transport.rb

@@ -31,8 +31,7 @@ def connect(name, connection_info)
     validate(name, connection_info)
     require "puppet/transport/#{name}"
     class_name = name.split('_').map { |e| e.capitalize }.join
-    # passing the copy as it may have been stripped on invalid key/values by validate
-    Puppet::Transport.const_get(class_name).new(get_context(name), connection_info)
+    Puppet::Transport.const_get(class_name).new(get_context(name), wrap_sensitive(name, connection_info))
   end
   module_function :connect # rubocop:disable Style/AccessModifierDeclarations
 
@@ -57,8 +56,8 @@ def self.validate(name, connection_info)
         environment: @environment,
       }
     end
-
-    transport_schema.check_schema(connection_info)
+    message_prefix = 'The connection info provided does not match the Transport Schema'
+    transport_schema.check_schema(connection_info, message_prefix)
     transport_schema.validate(connection_info)
   end
   private_class_method :validate
@@ -80,4 +79,17 @@ def self.init_transports
     @transports[@environment] ||= {}
   end
   private_class_method :init_transports
+
+  def self.wrap_sensitive(name, connection_info)
+    transport_schema = @transports[@environment][name]
+    if transport_schema
+      transport_schema.definition[:connection_info].each do |attr_name, options|
+        if options.key?(:sensitive) && (options[:sensitive] == true)
+          connection_info[attr_name] = Puppet::Pops::Types::PSensitiveType::Sensitive.new(connection_info[attr_name])
+        end
+      end
+    end
+    connection_info
+  end
+  private_class_method :wrap_sensitive
 end

lib/puppet/resource_api/type_definition.rb

@@ -120,16 +120,17 @@ def validate_schema(definition, attr_key)
     end
 
     # validates a resource hash against its type schema
-    def check_schema(resource)
+    def check_schema(resource, message_prefix = nil)
       namevars.each do |namevar|
         if resource[namevar].nil?
           raise Puppet::ResourceError, "`#{name}.get` did not return a value for the `#{namevar}` namevar attribute"
         end
       end
 
-      message = "Provider returned data that does not match the Type Schema for `#{name}[#{resource[namevars.first]}]`"
+      message_prefix = 'Provider returned data that does not match the Type Schema' if message_prefix.nil?
+      message = "#{message_prefix} for `#{name}[#{resource[namevars.first]}]`"
 
-      rejected_keys = check_schema_keys(resource) # removes bad keys
+      rejected_keys = check_schema_keys(resource)
       bad_values = check_schema_values(resource)
 
       unless rejected_keys.empty?
@@ -168,12 +169,17 @@ def check_schema_values(resource)
       resource.each do |key, value|
         next unless attributes[key]
         type = @data_type_cache[attributes[key][:type]]
+        is_sensitive = (attributes[key].key?(:sensitive) && (attributes[key][:sensitive] == true))
         error_message = Puppet::ResourceApi::DataTypeHandling.try_validate(
           type,
           value,
           '',
         )
-        bad_vals[key] = value unless error_message.nil?
+        if is_sensitive
+          bad_vals[key] = '<< redacted value >> ' + error_message unless error_message.nil?
+        else
+          bad_vals[key] = value unless error_message.nil?
+        end
       end
       bad_vals
     end

spec/acceptance/device_spec.rb

@@ -78,7 +78,14 @@
 DEVICE_CONF
     end
     let(:device_credentials) { Tempfile.new('credentials.txt') }
-    let(:device_credentials_content) { {} }
+    let(:device_credentials_content) do
+      <<DEVICE_CREDS
+{
+  username: foo
+  secret: wibble
+}
+DEVICE_CREDS
+    end
 
     def is_device_apply_supported?
       Gem::Version.new(Puppet::PUPPETVERSION) >= Gem::Version.new('5.3.6') && Gem::Version.new(Puppet::PUPPETVERSION) != Gem::Version.new('5.4.0')

spec/acceptance/sensitive_spec.rb

@@ -21,6 +21,22 @@
       expect(stdout_str).not_to match %r{foo}
       expect(stdout_str).not_to match %r{warn|error}i
     end
+
+    context 'when a sensitive value is not the top level type' do
+      it 'is not exposed by a provider' do
+        stdout_str, _status = Open3.capture2e("puppet apply #{common_args} -e \"test_sensitive { bar: secret => Sensitive('foo'), "\
+          "optional_secret => Sensitive('optional foo'), variant_secret => [Sensitive('variant foo')] }\"")
+        expect(stdout_str).to match %r{redacted}
+        expect(stdout_str).not_to match %r{variant foo}
+        expect(stdout_str).not_to match %r{warn|error}i
+      end
+      it 'properly validates the sensitive type value' do
+        stdout_str, _status = Open3.capture2e("puppet apply #{common_args} -e \"test_sensitive { bar: secret => Sensitive('foo'), "\
+          "optional_secret => Sensitive('optional foo'), variant_secret => [Sensitive(134679)] }\"")
+        expect(stdout_str).to match %r{Sensitive\[String\]( value)?, got Sensitive\[Integer\]}
+        expect(stdout_str).not_to match %r{134679}
+      end
+    end
   end
 
   describe 'using `puppet resource`' do

spec/acceptance/transport/transport_spec.rb

@@ -0,0 +1,185 @@
+require 'spec_helper'
+require 'tempfile'
+require 'open3'
+
+RSpec.describe 'a transport' do
+  let(:common_args) { '--verbose --trace --debug --strict=error --modulepath spec/fixtures' }
+
+  before(:all) do
+    FileUtils.mkdir_p(File.expand_path('~/.puppetlabs/opt/puppet/cache/devices/the_node/state'))
+  end
+
+  describe 'using `puppet device`' do
+    let(:common_args) { super() + ' --target the_node' }
+    let(:device_conf) { Tempfile.new('device.conf') }
+    let(:device_conf_content) do
+      <<DEVICE_CONF
+[the_node]
+type test_device_sensitive
+url  file://#{device_credentials.path}
+DEVICE_CONF
+    end
+    let(:device_credentials) { Tempfile.new('credentials.txt') }
+
+    def is_device_apply_supported?
+      Gem::Version.new(Puppet::PUPPETVERSION) >= Gem::Version.new('5.3.6') && Gem::Version.new(Puppet::PUPPETVERSION) != Gem::Version.new('5.4.0')
+    end
+
+    before(:each) do
+      skip "No device --apply in puppet before v5.3.6 nor in v5.4.0 (v#{Puppet::PUPPETVERSION} is installed)" unless is_device_apply_supported?
+      device_conf.write(device_conf_content)
+      device_conf.close
+
+      device_credentials.write(device_credentials_content)
+      device_credentials.close
+    end
+
+    after(:each) do
+      device_conf.unlink
+      device_credentials.unlink
+    end
+
+    context 'when all sensitive values are valid' do
+      let(:device_credentials_content) do
+        <<DEVICE_CREDS
+{
+  username: foo
+  secret_string: wibble
+  optional_secret: bar
+  array_secret: [meep]
+  variant_secret: 1234567890
+}
+DEVICE_CREDS
+      end
+
+      it 'does not throw' do
+        Tempfile.create('apply_success') do |f|
+          f.write 'notify { "foo": }'
+          f.close
+
+          stdout_str, _status = Open3.capture2e("puppet device #{common_args} --deviceconfig #{device_conf.path}  --apply #{f.path}")
+          expect(stdout_str).not_to match %r{Value type mismatch}
+
+          expect(stdout_str).to match %r{transport connection_info:}
+          expect(stdout_str).to match %r{:username=>"foo"}
+          expect(stdout_str).not_to match %r{wibble}
+          expect(stdout_str).not_to match %r{bar}
+          expect(stdout_str).not_to match %r{meep}
+          expect(stdout_str).not_to match %r{1234567890}
+        end
+      end
+    end
+
+    context 'with a sensitive string value that is invalid' do
+      let(:device_credentials_content) do
+        <<DEVICE_CREDS
+{
+  username: foo
+  secret_string: 12345
+  optional_secret: wibble
+  array_secret: [meep]
+  variant_secret: 21
+}
+DEVICE_CREDS
+      end
+
+      it 'Value type mismatch' do
+        Tempfile.create('apply_success') do |f|
+          f.write 'notify { "foo": }'
+          f.close
+
+          stdout_str, _status = Open3.capture2e("puppet device #{common_args} --deviceconfig #{device_conf.path}  --apply #{f.path}")
+          expect(stdout_str).to match %r{Value type mismatch}
+          expect(stdout_str).to match %r{secret_string: << redacted value >> }
+          expect(stdout_str).not_to match %r{optional_secret}
+          expect(stdout_str).not_to match %r{array_secret}
+          expect(stdout_str).not_to match %r{variant_secret}
+        end
+      end
+    end
+
+    context 'with an optional sensitive string value that is invalid' do
+      let(:device_credentials_content) do
+        <<DEVICE_CREDS
+{
+  username: foo
+  secret_string: wibble
+  optional_secret: 12345
+  array_secret: [meep]
+  variant_secret: 21
+}
+DEVICE_CREDS
+      end
+
+      it 'Value type mismatch' do
+        Tempfile.create('apply_success') do |f|
+          f.write 'notify { "foo": }'
+          f.close
+
+          stdout_str, _status = Open3.capture2e("puppet device #{common_args} --deviceconfig #{device_conf.path}  --apply #{f.path}")
+          expect(stdout_str).to match %r{Value type mismatch}
+          expect(stdout_str).not_to match %r{secret_string }
+          expect(stdout_str).to match %r{optional_secret: << redacted value >>}
+          expect(stdout_str).not_to match %r{array_secret}
+          expect(stdout_str).not_to match %r{variant_secret}
+        end
+      end
+    end
+
+    context 'with an array of sensitive strings that is invalid' do
+      let(:device_credentials_content) do
+        <<DEVICE_CREDS
+{
+  username: foo
+  secret_string: wibble
+  optional_secret: bar
+  array_secret: [17]
+  variant_secret: 21
+}
+DEVICE_CREDS
+      end
+
+      it 'Value type mismatch' do
+        Tempfile.create('apply_success') do |f|
+          f.write 'notify { "foo": }'
+          f.close
+
+          stdout_str, _status = Open3.capture2e("puppet device #{common_args} --deviceconfig #{device_conf.path}  --apply #{f.path}")
+          expect(stdout_str).to match %r{Value type mismatch}
+          expect(stdout_str).not_to match %r{secret_string }
+          expect(stdout_str).not_to match %r{optional_secret}
+          expect(stdout_str).to match %r{array_secret: << redacted value >>}
+          expect(stdout_str).not_to match %r{variant_secret}
+        end
+      end
+    end
+
+    context 'with an variant containing a sensitive value that is invalid' do
+      let(:device_credentials_content) do
+        <<DEVICE_CREDS
+{
+  username: foo
+  secret_string: wibble
+  optional_secret: bar
+  array_secret: [meep]
+  variant_secret: wobble
+}
+DEVICE_CREDS
+      end
+
+      it 'Value type mismatch' do
+        Tempfile.create('apply_success') do |f|
+          f.write 'notify { "foo": }'
+          f.close
+
+          stdout_str, _status = Open3.capture2e("puppet device #{common_args} --deviceconfig #{device_conf.path}  --apply #{f.path}")
+          expect(stdout_str).to match %r{Value type mismatch}
+          expect(stdout_str).not_to match %r{secret_string }
+          expect(stdout_str).not_to match %r{optional_secret}
+          expect(stdout_str).not_to match %r{array_secret}
+          expect(stdout_str).to match %r{variant_secret: << redacted value >>}
+        end
+      end
+    end
+  end
+end

spec/fixtures/test_module/lib/puppet/transport/schema/test_device.rb

@@ -4,5 +4,29 @@
   name: 'test_device', # points at class Puppet::Transport::TestDevice
   desc: 'Connects to a device',
   connection_info: {
+    username:        {
+      type:      'String',
+      desc:      'The name of the resource you want to manage.',
+    },
+    secret: {
+      type:      'String',
+      desc:      'A secret to protect.',
+      sensitive:  true,
+    },
+    optional_secret: {
+      type:      'Optional[String]',
+      desc:      'An optional secret to protect.',
+      sensitive:  true,
+    },
+    array_secret: {
+      type:      'Optional[Array[String]]',
+      desc:      'An array secret to protect.',
+      sensitive:  true,
+    },
+    variant_secret: {
+      type:      'Optional[Variant[Array[String], Integer]]',
+      desc:      'An array secret to protect.',
+      sensitive:  true,
+    },
   },
 )

spec/fixtures/test_module/lib/puppet/transport/schema/test_device_sensitive.rb

@@ -0,0 +1,32 @@
+require 'puppet/resource_api'
+
+Puppet::ResourceApi.register_transport(
+  name: 'test_device_sensitive', # points at class Puppet::Transport::TestDevice
+  desc: 'Connects to a device',
+  connection_info: {
+    username:        {
+      type:      'String',
+      desc:      'The name of the resource you want to manage.',
+    },
+    secret_string: {
+      type:      'String',
+      desc:      'A secret to protect.',
+      sensitive:  true,
+    },
+    optional_secret: {
+      type:      'Optional[String]',
+      desc:      'An optional secret to protect.',
+      sensitive:  true,
+    },
+    array_secret: {
+      type:      'Optional[Array[String]]',
+      desc:      'An array secret to protect.',
+      sensitive:  true,
+    },
+    variant_secret: {
+      type:      'Optional[Variant[Array[String], Integer]]',
+      desc:      'An array secret to protect.',
+      sensitive:  true,
+    },
+  },
+)

spec/fixtures/test_module/lib/puppet/transport/test_device_sensitive.rb

@@ -0,0 +1,21 @@
+module Puppet::Transport
+# a transport for a test_device
+class TestDeviceSensitive
+  def initialize(_context, connection_info);
+    puts  "transport connection_info: #{connection_info}"
+  end
+
+  def facts(_context)
+    { 'foo' => 'bar' }
+  end
+
+  def verify(_context)
+    return true
+  end
+
+  def close(_context)
+    return
+  end
+end
+
+end