(PDK-1091) Implement handling Sensitive values

lib/puppet/resource_api.rb

@@ -79,10 +79,21 @@ def type_definition
         if attributes.is_a? Puppet::Resource
           @title = attributes.title
           @catalog = attributes.catalog
+          sensitives = attributes.sensitive_parameters
           attributes = attributes.to_hash
         else
           @ral_find_absent = true
+          sensitives = []
         end
+
+        # undo puppet's unwrapping of Sensitive values to provide a uniform experience for providers
+        # See https://tickets.puppetlabs.com/browse/PDK-1091 for investigation and background
+        sensitives.each do |name|
+          if attributes.key?(name) && !attributes[name].is_a?(Puppet::Pops::Types::PSensitiveType::Sensitive)
+            attributes[name] = Puppet::Pops::Types::PSensitiveType::Sensitive.new(attributes[name])
+          end
+        end
+
         # $stderr.puts "B: #{attributes.inspect}"
         if type_definition.feature?('canonicalize')
           attributes = my_provider.canonicalize(context, [attributes])[0]

spec/acceptance/device_spec.rb

@@ -11,6 +11,10 @@
     'ensure_param=present variant_pattern_param=0xAE321EEF url_param="https://www.google.com"'
   end
 
+  before(:all) do
+    FileUtils.mkdir_p(File.expand_path('~/.puppetlabs/opt/puppet/cache/devices/the_node/state'))
+  end
+
   describe 'using `puppet resource`' do
     it 'manages resources on the target system' do
       stdout_str, status = Open3.capture2e("puppet resource #{common_args} device_provider foo ensure=present #{default_type_values}")

spec/acceptance/sensitive_spec.rb

@@ -0,0 +1,34 @@
+require 'spec_helper'
+require 'tempfile'
+require 'open3'
+
+RSpec.describe 'sensitive data' do
+  # these common_args *have* to use debug to check *all* log messages for the sensitive value
+  let(:common_args) { '--verbose --trace --strict=error --modulepath spec/fixtures  --debug' }
+
+  describe 'using `puppet apply`' do
+    it 'is not exposed by notify' do
+      stdout_str, _status = Open3.capture2e("puppet apply #{common_args} -e \"notice(Sensitive('foo'))\"")
+      expect(stdout_str).to match %r{redacted}
+      expect(stdout_str).not_to match %r{foo}
+      expect(stdout_str).not_to match %r{warn|error}i
+    end
+
+    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'), array_secret => [Sensitive('array foo')] }\"")
+      expect(stdout_str).to match %r{redacted}
+      expect(stdout_str).not_to match %r{foo}
+      expect(stdout_str).not_to match %r{warn|error}i
+    end
+  end
+
+  describe 'using `puppet resource`' do
+    it 'is not exposed in the output' do
+      stdout_str, _status = Open3.capture2e("puppet resource #{common_args} test_sensitive")
+      expect(stdout_str).to match %r{redacted}
+      expect(stdout_str).not_to match %r{(foo|bar)secret}
+      expect(stdout_str).not_to match %r{warn|error}i
+    end
+  end
+end

spec/fixtures/test_module/lib/puppet/provider/test_sensitive/test_sensitive.rb

@@ -0,0 +1,31 @@
+require 'puppet/resource_api/simple_provider'
+
+# Implementation for the test_sensitive type using the Resource API.
+class Puppet::Provider::TestSensitive::TestSensitive < Puppet::ResourceApi::SimpleProvider
+  def get(_context)
+    [
+      {
+        name: 'foo',
+        ensure: 'present',
+        secret: Puppet::Pops::Types::PSensitiveType::Sensitive.new('foosecret')
+      },
+      {
+        name: 'bar',
+        ensure: 'present',
+        secret: Puppet::Pops::Types::PSensitiveType::Sensitive.new('barsecret')
+      },
+    ]
+  end
+
+  def create(context, name, should)
+    context.notice("Creating '#{name}' with #{should.inspect}")
+  end
+
+  def update(context, name, should)
+    context.notice("Updating '#{name}' with #{should.inspect}")
+  end
+
+  def delete(context, name)
+    context.notice("Deleting '#{name}'")
+  end
+end

spec/fixtures/test_module/lib/puppet/type/test_sensitive.rb

@@ -0,0 +1,33 @@
+require 'puppet/resource_api'
+
+Puppet::ResourceApi.register_type(
+  name: 'test_sensitive',
+  docs: <<-EOS,
+      This type provides Puppet with the capabilities to manage ...
+    EOS
+  features: [],
+  attributes:   {
+    ensure:      {
+      type:    'Enum[present, absent]',
+      desc:    'Whether this resource should be present or absent on the target system.',
+      default: 'present',
+    },
+    name:        {
+      type:      'String',
+      desc:      'The name of the resource you want to manage.',
+      behaviour: :namevar,
+    },
+    secret: {
+      type:      'Sensitive[String]',
+      desc:      'A secret to protect.',
+    },
+    optional_secret: {
+      type:      'Optional[Sensitive[String]]',
+      desc:      'An optional secret to protect.',
+    },
+    array_secret: {
+      type:      'Array[Sensitive[String]]',
+      desc:      'An array secret to protect.',
+    },
+  },
+)

spec/fixtures/test_module/spec/unit/puppet/provider/test_sensitive/test_sensitive_spec.rb

@@ -0,0 +1,49 @@
+require 'spec_helper'
+
+ensure_module_defined('Puppet::Provider::TestSensitive')
+require 'puppet/provider/test_sensitive/test_sensitive'
+
+RSpec.describe Puppet::Provider::TestSensitive::TestSensitive do
+  subject(:provider) { described_class.new }
+
+  let(:context) { instance_double('Puppet::ResourceApi::BaseContext', 'context') }
+
+  describe '#get' do
+    it 'processes resources' do
+      expect(provider.get(context)).to eq [
+        {
+          name: 'foo',
+          ensure: 'present',
+        },
+        {
+          name: 'bar',
+          ensure: 'present',
+        },
+      ]
+    end
+  end
+
+  describe 'create(context, name, should)' do
+    it 'creates the resource' do
+      expect(context).to receive(:notice).with(%r{\ACreating 'a'})
+
+      provider.create(context, 'a', name: 'a', ensure: 'present')
+    end
+  end
+
+  describe 'update(context, name, should)' do
+    it 'updates the resource' do
+      expect(context).to receive(:notice).with(%r{\AUpdating 'foo'})
+
+      provider.update(context, 'foo', name: 'foo', ensure: 'present')
+    end
+  end
+
+  describe 'delete(context, name, should)' do
+    it 'deletes the resource' do
+      expect(context).to receive(:notice).with(%r{\ADeleting 'foo'})
+
+      provider.delete(context, 'foo')
+    end
+  end
+end

spec/fixtures/test_module/spec/unit/puppet/type/test_sensitive_spec.rb

@@ -0,0 +1,8 @@
+require 'spec_helper'
+require 'puppet/type/test_sensitive'
+
+RSpec.describe 'the test_sensitive type' do
+  it 'loads' do
+    expect(Puppet::Type.type(:test_sensitive)).not_to be_nil
+  end
+end

spec/integration/resource_api_spec.rb

@@ -42,6 +42,10 @@
             type: 'Pattern[/\A((hkp|http|https):\/\/)?([a-z\d])([a-z\d-]{0,61}\.)+[a-z\d]+(:\d{2,5})?$/]',
             desc: 'a hkp or http(s) url attribute',
           },
+          sensitive: {
+            type: 'Sensitive[String]',
+            desc: 'A sensitive string, like a password',
+          },
           string_array: {
             type: 'Array[String]',
             desc: 'An attribute to exercise Array handling.',
@@ -195,7 +199,8 @@ def get(_context)
       let(:catalog) { instance_double('Puppet::Resource::Catalog', 'catalog') }
       let(:instance) do
         type.new(name: 'somename', ensure: 'present', boolean: true, integer: 15, float: 1.23,
-                 variant_pattern: '0x1234ABCD', url: 'http://www.google.com', string_array: %w[a b c],
+                 variant_pattern: '0x1234ABCD', url: 'http://www.google.com', sensitive: Puppet::Pops::Types::PSensitiveType::Sensitive.new('a password'),
+                 string_array: %w[a b c],
                  variant_array: 'not_an_array', array_of_arrays: [%w[a b c], %w[d e f]],
                  array_from_hell: ['a', %w[subb subc], 'd'],
                  boolean_param: false, integer_param: 99, float_param: 3.21, ensure_param: 'present',
@@ -214,6 +219,7 @@ def get(_context)
 
       describe '.to_resource' do
         it { expect(instance.to_resource).to be_a Puppet::ResourceApi::ResourceShim }
+
         describe 'its title' do
           it { expect(instance.to_resource.title).to eq 'somename' }
         end

spec/puppet/resource_api_spec.rb

@@ -352,6 +352,106 @@ def set(_context, _changes); end
     end
   end
 
+  context 'when registering a type with a sensitive attributes' do
+    let(:definition) do
+      {
+        name: type_name,
+        attributes: {
+          name: {
+            type: 'String',
+            behaviour: :namevar,
+            desc: 'the title',
+          },
+          secret: {
+            type: 'Sensitive[String]',
+            desc: 'a password',
+          },
+        },
+      }
+    end
+    let(:type_name) { 'with_sensitive' }
+
+    before(:each) do
+      described_class.register_type(definition)
+    end
+
+    describe 'the registered type' do
+      subject(:type) { Puppet::Type.type(type_name.to_sym) }
+
+      it { is_expected.not_to be_nil }
+    end
+
+    describe 'an instance of this type' do
+      subject(:instance) { Puppet::Type.type(type_name.to_sym).new(params) }
+
+      let(:params) do
+        { title: 'test', secret: Puppet::Pops::Types::PSensitiveType::Sensitive.new('a password value') }
+      end
+
+      it('has the secret value is set correctly') { expect(instance[:secret]).to be_a Puppet::Pops::Types::PSensitiveType::Sensitive }
+
+      context 'with a basic provider', agent_test: true do
+        let(:provider_class) do
+          Class.new do
+            def get(_context)
+              []
+            end
+
+            def set(_context, _changes); end
+          end
+        end
+
+        before(:each) do
+          stub_const('Puppet::Provider::WithSensitive', Module.new)
+          stub_const('Puppet::Provider::WithSensitive::WithSensitive', provider_class)
+        end
+
+        context 'when mandatory attributes are missing' do
+          let(:params) do
+            {
+              title: 'test',
+            }
+          end
+
+          it {
+            expect {
+              instance.validate
+              instance.retrieve
+            }.not_to raise_exception }
+        end
+
+        context 'when loading from a Puppet::Resource' do
+          let(:params) { instance_double('Puppet::Resource', 'resource') }
+          let(:provider_instance) { instance_double(provider_class, 'provider_instance') }
+          let(:catalog) { instance_double('Unknown', 'catalog') }
+
+          before(:each) do
+            allow(provider_class).to receive(:new).with(no_args).and_return(provider_instance)
+            allow(provider_instance).to receive(:get).and_return([])
+            allow(params).to receive(:is_a?).with(Puppet::Resource).and_return(true)
+            allow(params).to receive(:title).with(no_args).and_return('a title')
+            allow(params).to receive(:catalog).with(no_args).and_return(catalog)
+            allow(params).to receive(:sensitive_parameters).with(no_args).and_return([:secret])
+            allow(params).to receive(:to_hash).with(no_args).and_return(title: 'test', secret: 'a password value')
+            allow(catalog).to receive(:host_config?).and_return(true)
+          end
+
+          it 'massages unwrapped sensitive values' do
+            expect(provider_instance).to receive(:set)
+              .with(anything,
+                    'test' => {
+                      is: { title: 'test' },
+                      should: { name: 'test', secret: a_kind_of(Puppet::Pops::Types::PSensitiveType::Sensitive) },
+                    })
+            instance.retrieve
+            instance[:secret] = Puppet::Pops::Types::PSensitiveType::Sensitive.new('a new password')
+            instance.flush
+          end
+        end
+      end
+    end
+  end
+
   context 'when registering a type that is ensurable', agent_test: true do
     context 'when ensurable is correctly declared' do
       let(:definition) do