diff --git a/.travis.yml b/.travis.yml index 978c4106..7b214550 100644 --- a/.travis.yml +++ b/.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 diff --git a/lib/puppet/resource_api/transport.rb b/lib/puppet/resource_api/transport.rb index df6177ee..438333f1 100644 --- a/lib/puppet/resource_api/transport.rb +++ b/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 diff --git a/lib/puppet/resource_api/type_definition.rb b/lib/puppet/resource_api/type_definition.rb index be8ad135..40bd3713 100644 --- a/lib/puppet/resource_api/type_definition.rb +++ b/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 diff --git a/spec/acceptance/device_spec.rb b/spec/acceptance/device_spec.rb index 909313c9..788244dd 100644 --- a/spec/acceptance/device_spec.rb +++ b/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 + <= Gem::Version.new('5.3.6') && Gem::Version.new(Puppet::PUPPETVERSION) != Gem::Version.new('5.4.0') diff --git a/spec/acceptance/sensitive_spec.rb b/spec/acceptance/sensitive_spec.rb index f48341fb..e23c4088 100644 --- a/spec/acceptance/sensitive_spec.rb +++ b/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 diff --git a/spec/acceptance/transport/transport_spec.rb b/spec/acceptance/transport/transport_spec.rb new file mode 100644 index 00000000..3fe7c0b5 --- /dev/null +++ b/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 + <= 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 + <"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 + <> } + 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 + <>} + 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 + <>} + 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 + <>} + end + end + end + end +end diff --git a/spec/fixtures/test_module/lib/puppet/transport/schema/test_device.rb b/spec/fixtures/test_module/lib/puppet/transport/schema/test_device.rb index 5bd6ee45..df2f7a9b 100644 --- a/spec/fixtures/test_module/lib/puppet/transport/schema/test_device.rb +++ b/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, + }, }, ) diff --git a/spec/fixtures/test_module/lib/puppet/transport/schema/test_device_sensitive.rb b/spec/fixtures/test_module/lib/puppet/transport/schema/test_device_sensitive.rb new file mode 100644 index 00000000..a22c43aa --- /dev/null +++ b/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, + }, + }, +) diff --git a/spec/fixtures/test_module/lib/puppet/transport/test_device_sensitive.rb b/spec/fixtures/test_module/lib/puppet/transport/test_device_sensitive.rb new file mode 100644 index 00000000..c2a898a2 --- /dev/null +++ b/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 diff --git a/spec/fixtures/test_module/lib/puppet/type/test_sensitive.rb b/spec/fixtures/test_module/lib/puppet/type/test_sensitive.rb index ce74363d..f42f89c9 100644 --- a/spec/fixtures/test_module/lib/puppet/type/test_sensitive.rb +++ b/spec/fixtures/test_module/lib/puppet/type/test_sensitive.rb @@ -26,7 +26,11 @@ desc: 'An optional secret to protect.', }, array_secret: { - type: 'Array[Sensitive[String]]', + type: 'Optional[Array[Sensitive[String]]]', + desc: 'An array secret to protect.', + }, + variant_secret: { + type: 'Optional[Variant[Array[Sensitive[String]], Integer]]', desc: 'An array secret to protect.', }, }, diff --git a/spec/fixtures/test_module/lib/puppet/util/network_device/test_device_sensitive/device.rb b/spec/fixtures/test_module/lib/puppet/util/network_device/test_device_sensitive/device.rb new file mode 100644 index 00000000..4c04b7cf --- /dev/null +++ b/spec/fixtures/test_module/lib/puppet/util/network_device/test_device_sensitive/device.rb @@ -0,0 +1,12 @@ +require 'puppet/resource_api/transport/wrapper' + +class Puppet::Util::NetworkDevice; end + +module Puppet::Util::NetworkDevice::Test_device_sensitive # rubocop:disable Style/ClassAndModuleCamelCase + # The main class for handling the connection and command parsing to the IOS Catalyst device + class Device < Puppet::ResourceApi::Transport::Wrapper + def initialize(url_or_config, _options = {}) + super('test_device_sensitive', url_or_config) + end + end +end \ No newline at end of file diff --git a/spec/puppet/resource_api/base_type_definition_spec.rb b/spec/puppet/resource_api/base_type_definition_spec.rb index 64ab1cc5..62a918fd 100644 --- a/spec/puppet/resource_api/base_type_definition_spec.rb +++ b/spec/puppet/resource_api/base_type_definition_spec.rb @@ -52,19 +52,58 @@ end describe '#check_schema_values' do - context 'when resource contains only valid values' do - let(:resource) { { name: 'some_resource', prop: 1, ensure: 'present' } } + context 'when the definition is a type' do + context 'when resource contains only valid values' do + let(:resource) { { name: 'some_resource', prop: 1, ensure: 'present' } } - it 'returns an empty array' do - expect(type.check_schema_values(resource)).to eq({}) + it 'returns an empty array' do + expect(type.check_schema_values(resource)).to eq({}) + end + end + + context 'when resource contains invalid values' do + let(:resource) { { name: 'test_string', prop: 'foo', ensure: 1 } } + + it 'returns a hash of the keys that have invalid values' do + expect(type.check_schema_values(resource)).to eq(prop: 'foo', ensure: 1) + end end end - context 'when resource contains invalid values' do - let(:resource) { { name: 'test_string', prop: 'foo', ensure: 1 } } + context 'when the definition is a transport' do + subject(:type) { described_class.new(definition, :connection_info) } + + let(:definition) do + { + name: 'some_transport', + connection_info: { + username: { + type: 'String', + desc: 'The username to connect with', + }, + secret: { + type: 'String', + desc: 'A sensitive value', + sensitive: true, + }, + }, + } + end + + context 'when resource contains only valid values' do + let(:resource) { { username: 'wibble', secret: 'foo' } } - it 'returns a hash of the keys that have invalid values' do - expect(type.check_schema_values(resource)).to eq(prop: 'foo', ensure: 1) + it 'returns an empty array' do + expect(type.check_schema_values(resource)).to eq({}) + end + end + + context 'when resource contains invalid values' do + let(:resource) { { username: 'wibble', secret: 12_345 } } + + it 'returns a hash of the keys that have invalid values' do + expect(type.check_schema_values(resource)).to match(secret: %r{<< redacted value >> expect(s|ed) a String value, got Integer}) + end end end end diff --git a/spec/puppet/resource_api/transport_spec.rb b/spec/puppet/resource_api/transport_spec.rb index 31529c2d..ad59697d 100644 --- a/spec/puppet/resource_api/transport_spec.rb +++ b/spec/puppet/resource_api/transport_spec.rb @@ -302,7 +302,7 @@ class Wibble; end described_class.register(schema) expect(described_class).not_to receive(:require).with('puppet/transport/schema/validate') - expect(schema_def).to receive(:check_schema).with('connection_info').and_return(nil) + expect(schema_def).to receive(:check_schema).with('connection_info', kind_of(String)).and_return(nil) expect(schema_def).to receive(:validate).with('connection_info').and_return(nil) described_class.send :validate, 'validate', 'connection_info' @@ -331,4 +331,41 @@ class Wibble; end end end end + + describe '#wrap_sensitive(name, connection_info)' do + context 'when the connection info contains a `Sensitive` type' do + let(:schema) do + { + name: 'sensitive_transport', + desc: 'a secret', + connection_info: { + secret: { + type: 'String', + desc: 'A secret to protect.', + sensitive: true, + }, + }, + } + end + let(:schema_def) { instance_double('Puppet::ResourceApi::TransportSchemaDef', 'schema_def') } + let(:connection_info) do + { + secret: 'sup3r_secret_str1ng', + } + end + + before(:each) do + allow(Puppet::ResourceApi::TransportSchemaDef).to receive(:new).with(schema).and_return(schema_def) + described_class.register(schema) + end + + it 'wraps the value in a PSensitiveType' do + allow(schema_def).to receive(:definition).and_return(schema) + + conn_info = described_class.send :wrap_sensitive, 'sensitive_transport', connection_info + expect(conn_info[:secret]).to be_a(Puppet::Pops::Types::PSensitiveType::Sensitive) + expect(conn_info[:secret].unwrap).to eq('sup3r_secret_str1ng') + end + end + end end