From 56b35f9f9526a023bc1e797dd706f6fc8668aae9 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Wed, 3 Jul 2024 17:13:15 -0700 Subject: [PATCH] (PUP-12050) Check for nested Sensitive arguments Previously, a manifest containing nested Deferred values did not mark the corresponding parameter as sensitive, resulting in the following: $ cat manifest.pp $vars = {'token' => Deferred('new', [Sensitive, "password"])} file { '/tmp/a.sh': ensure => file, content => Deferred('inline_epp', ['<%= $token %>', $vars]) } $ truncate --size 0 /tmp/a.sh $ puppet apply --show_diff manifest.pp Notice: Compiled catalog for localhost in environment production in 0.01 seconds Notice: /Stage[main]/Main/File[/tmp/a.sh]/content: --- /tmp/a.sh 2024-07-03 17:30:37.024543314 -0700 +++ /tmp/puppet-file20240703-1784698-2cu5s9 2024-07-03 17:30:41.880572413 -0700 @@ -0,0 +1 @@ +password \ No newline at end of file The issue occurred because we were only checking if the outermost DeferredValue contained any Sensitive arguments, in this case the arguments passed to `inline_epp` function, but not the `password` passed to the `new` function. This is not an issue when deferred values are preprocessed, because Deferred values are completely resolved and we can check if resolved value is Sensitive. --- .../pops/evaluator/deferred_resolver.rb | 36 +++++++++++++++---- 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/lib/puppet/pops/evaluator/deferred_resolver.rb b/lib/puppet/pops/evaluator/deferred_resolver.rb index 6c4bf989693..7aa9699fe93 100644 --- a/lib/puppet/pops/evaluator/deferred_resolver.rb +++ b/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,22 @@ def resolve_futures(catalog) end end + # Return true if x contains an argument of type PSensitiveType. It + # short-circuits as soon as a value is found. + # + def contains_sensitive_args(x) + if x.instance_of?(@deferred_class) + contains_sensitive_args(x.arguments) + elsif x.is_a?(Array) + x.any? { |v| contains_sensitive_args(v) } + elsif x.is_a?(Hash) + x.any? { |k, v| contains_sensitive_args(k) || contains_sensitive_args(v) } + elsif x.is_a?(Puppet::Pops::Types::PSensitiveType) + true + 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