Merge pull request #9410 from joshcooper/deferred_sensitive_12050

(PUP-12050) Check for nested Sensitive arguments

Author: cthorn42

lib/puppet/pops/evaluator/deferred_resolver.rb

@@ -89,17 +89,25 @@ def resolve_futures(catalog)
       overrides = {}
       r.parameters.each_pair do |k, v|
         resolved = resolve(v)
-        # If the value is instance of Sensitive - assign the unwrapped value
-        # and mark it as sensitive if not already marked
-        #
         case resolved
         when Puppet::Pops::Types::PSensitiveType::Sensitive
+          # If the resolved value is instance of Sensitive - assign the unwrapped value
+          # and mark it as sensitive if not already marked
+          #
           resolved = resolved.unwrap
           mark_sensitive_parameters(r, k)
-        # If the value is a DeferredValue and it has an argument of type PSensitiveType, mark it as sensitive
-        # The DeferredValue.resolve method will unwrap it during catalog application
+
         when Puppet::Pops::Evaluator::DeferredValue
-          if v.arguments.any? { |arg| arg.is_a?(Puppet::Pops::Types::PSensitiveType) }
+          # If the resolved value is a DeferredValue and it has an argument of type
+          # PSensitiveType, mark it as sensitive. Since DeferredValues can nest,
+          # we must walk all arguments, e.g. the DeferredValue may call the `epp`
+          # function, where one of its arguments is a DeferredValue to call the
+          # `vault:lookup` function.
+          #
+          # The DeferredValue.resolve method will unwrap the sensitive during
+          # catalog application
+          #
+          if contains_sensitive_args?(v)
             mark_sensitive_parameters(r, k)
           end
         end
@@ -109,6 +117,33 @@ def resolve_futures(catalog)
     end
   end
 
+  # Return true if x contains an argument that is an instance of PSensitiveType:
+  #
+  #   Deferred('new', [Sensitive, 'password'])
+  #
+  # Or an instance of PSensitiveType::Sensitive:
+  #
+  #   Deferred('join', [['a', Sensitive('b')], ':'])
+  #
+  # Since deferred values can nest, descend into Arrays and Hash keys and values,
+  # short-circuiting when the first occurrence is found.
+  #
+  def contains_sensitive_args?(x)
+    case x
+    when @deferred_class
+      contains_sensitive_args?(x.arguments)
+    when Array
+      x.any? { |v| contains_sensitive_args?(v) }
+    when Hash
+      x.any? { |k, v| contains_sensitive_args?(k) || contains_sensitive_args?(v) }
+    when Puppet::Pops::Types::PSensitiveType, Puppet::Pops::Types::PSensitiveType::Sensitive
+      true
+    else
+      false
+    end
+  end
+  private :contains_sensitive_args?
+
   def mark_sensitive_parameters(r, k)
     unless r.sensitive_parameters.include?(k.to_sym)
       r.sensitive_parameters = (r.sensitive_parameters + [k.to_sym]).freeze

spec/integration/application/apply_spec.rb

@@ -777,5 +777,20 @@ def bogus()
       }.to exit_with(0)
         .and output(/ensure: changed \[redacted\] to \[redacted\]/).to_stdout
     end
+
+    it "applies nested deferred sensitive file content" do
+      manifest = <<~END
+      $vars = {'token' => Deferred('new', [Sensitive, "hello"])}
+      file { '#{deferred_file}':
+        ensure => file,
+        content => Deferred('inline_epp', ['<%= $token %>', $vars])
+      }
+      END
+      apply.command_line.args = ['-e', manifest]
+      expect {
+        apply.run
+      }.to exit_with(0)
+        .and output(/ensure: changed \[redacted\] to \[redacted\]/).to_stdout
+    end
   end
 end

spec/unit/pops/evaluator/deferred_resolver_spec.rb

@@ -1,46 +1,98 @@
 require 'spec_helper'
 require 'puppet_spec/compiler'
 
+Puppet::Type.newtype(:test_deferred) do
+  newparam(:name)
+  newproperty(:value)
+end
+
 describe Puppet::Pops::Evaluator::DeferredResolver do
   include PuppetSpec::Compiler
 
   let(:environment) { Puppet::Node::Environment.create(:testing, []) }
   let(:facts) { Puppet::Node::Facts.new('node.example.com') }
 
+  def compile_and_resolve_catalog(code, preprocess = false)
+    catalog = compile_to_catalog(code)
+    described_class.resolve_and_replace(facts, catalog, environment, preprocess)
+    catalog
+  end
+
   it 'resolves deferred values in a catalog' do
-    catalog = compile_to_catalog(<<~END)
+    catalog = compile_and_resolve_catalog(<<~END, true)
       notify { "deferred":
         message => Deferred("join", [[1,2,3], ":"])
       }
     END
-    described_class.resolve_and_replace(facts, catalog)
 
     expect(catalog.resource(:notify, 'deferred')[:message]).to eq('1:2:3')
   end
 
   it 'lazily resolves deferred values in a catalog' do
-    catalog = compile_to_catalog(<<~END)
+    catalog = compile_and_resolve_catalog(<<~END)
       notify { "deferred":
         message => Deferred("join", [[1,2,3], ":"])
       }
     END
-    described_class.resolve_and_replace(facts, catalog, environment, false)
 
     deferred = catalog.resource(:notify, 'deferred')[:message]
     expect(deferred.resolve).to eq('1:2:3')
   end
 
   it 'lazily resolves nested deferred values in a catalog' do
-    catalog = compile_to_catalog(<<~END)
+    catalog = compile_and_resolve_catalog(<<~END)
       $args = Deferred("inline_epp", ["<%= 'a,b,c' %>"])
       notify { "deferred":
         message => Deferred("split", [$args, ","])
       }
     END
-    described_class.resolve_and_replace(facts, catalog, environment, false)
 
     deferred = catalog.resource(:notify, 'deferred')[:message]
     expect(deferred.resolve).to eq(["a", "b", "c"])
   end
 
+  it 'marks the parameter as sensitive when passed an array containing a Sensitive instance' do
+    catalog = compile_and_resolve_catalog(<<~END)
+      test_deferred { "deferred":
+        value => Deferred('join', [['a', Sensitive('b')], ':'])
+      }
+    END
+
+    resource = catalog.resource(:test_deferred, 'deferred')
+    expect(resource.sensitive_parameters).to eq([:value])
+  end
+
+  it 'marks the parameter as sensitive when passed a hash containing a Sensitive key' do
+    catalog = compile_and_resolve_catalog(<<~END)
+      test_deferred { "deferred":
+        value => Deferred('keys', [{Sensitive('key') => 'value'}])
+      }
+    END
+
+    resource = catalog.resource(:test_deferred, 'deferred')
+    expect(resource.sensitive_parameters).to eq([:value])
+  end
+
+  it 'marks the parameter as sensitive when passed a hash containing a Sensitive value' do
+    catalog = compile_and_resolve_catalog(<<~END)
+      test_deferred { "deferred":
+        value => Deferred('values', [{key => Sensitive('value')}])
+      }
+    END
+
+    resource = catalog.resource(:test_deferred, 'deferred')
+    expect(resource.sensitive_parameters).to eq([:value])
+  end
+
+  it 'marks the parameter as sensitive when passed a nested Deferred containing a Sensitive type' do
+    catalog = compile_and_resolve_catalog(<<~END)
+      $vars = {'token' => Deferred('new', [Sensitive, "hello"])}
+      test_deferred { "deferred":
+        value => Deferred('inline_epp', ['<%= $token %>', $vars])
+      }
+    END
+
+    resource = catalog.resource(:test_deferred, 'deferred')
+    expect(resource.sensitive_parameters).to eq([:value])
+  end
 end