Skip to content

Commit 7d8ccd1

Browse files
committed
(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.
1 parent 85d97d4 commit 7d8ccd1

File tree

3 files changed

+105
-6
lines changed

3 files changed

+105
-6
lines changed

lib/puppet/pops/evaluator/deferred_resolver.rb

+41-6
Original file line numberDiff line numberDiff line change
@@ -89,17 +89,25 @@ def resolve_futures(catalog)
8989
overrides = {}
9090
r.parameters.each_pair do |k, v|
9191
resolved = resolve(v)
92-
# If the value is instance of Sensitive - assign the unwrapped value
93-
# and mark it as sensitive if not already marked
94-
#
9592
case resolved
9693
when Puppet::Pops::Types::PSensitiveType::Sensitive
94+
# If the resolved value is instance of Sensitive - assign the unwrapped value
95+
# and mark it as sensitive if not already marked
96+
#
9797
resolved = resolved.unwrap
9898
mark_sensitive_parameters(r, k)
99-
# If the value is a DeferredValue and it has an argument of type PSensitiveType, mark it as sensitive
100-
# The DeferredValue.resolve method will unwrap it during catalog application
99+
101100
when Puppet::Pops::Evaluator::DeferredValue
102-
if v.arguments.any? { |arg| arg.is_a?(Puppet::Pops::Types::PSensitiveType) }
101+
# If the resolved value is a DeferredValue and it has an argument of type
102+
# PSensitiveType, mark it as sensitive. Since DeferredValues can nest,
103+
# we must walk all arguments, e.g. the DeferredValue may call the `epp`
104+
# function, where one of its arguments is a DeferredValue to call the
105+
# `vault:lookup` function.
106+
#
107+
# The DeferredValue.resolve method will unwrap the sensitive during
108+
# catalog application
109+
#
110+
if contains_sensitive_args?(v)
103111
mark_sensitive_parameters(r, k)
104112
end
105113
end
@@ -109,6 +117,33 @@ def resolve_futures(catalog)
109117
end
110118
end
111119

120+
# Return true if x contains an argument that is an instance of PSensitiveType:
121+
#
122+
# Deferred('new', [Sensitive, 'password'])
123+
#
124+
# Or an instance of PSensitiveType::Sensitive:
125+
#
126+
# Deferred('join', [['a', Sensitive('b')], ':'])
127+
#
128+
# Since deferred values can nest, descend into Arrays and Hash keys and values,
129+
# short-circuiting when the first occurrence is found.
130+
#
131+
def contains_sensitive_args?(x)
132+
case x
133+
when @deferred_class
134+
contains_sensitive_args?(x.arguments)
135+
when Array
136+
x.any? { |v| contains_sensitive_args?(v) }
137+
when Hash
138+
x.any? { |k, v| contains_sensitive_args?(k) || contains_sensitive_args?(v) }
139+
when Puppet::Pops::Types::PSensitiveType, Puppet::Pops::Types::PSensitiveType::Sensitive
140+
true
141+
else
142+
false
143+
end
144+
end
145+
private :contains_sensitive_args?
146+
112147
def mark_sensitive_parameters(r, k)
113148
unless r.sensitive_parameters.include?(k.to_sym)
114149
r.sensitive_parameters = (r.sensitive_parameters + [k.to_sym]).freeze

spec/integration/application/apply_spec.rb

+15
Original file line numberDiff line numberDiff line change
@@ -777,5 +777,20 @@ def bogus()
777777
}.to exit_with(0)
778778
.and output(/ensure: changed \[redacted\] to \[redacted\]/).to_stdout
779779
end
780+
781+
it "applies nested deferred sensitive file content" do
782+
manifest = <<~END
783+
$vars = {'token' => Deferred('new', [Sensitive, "hello"])}
784+
file { '#{deferred_file}':
785+
ensure => file,
786+
content => Deferred('inline_epp', ['<%= $token %>', $vars])
787+
}
788+
END
789+
apply.command_line.args = ['-e', manifest]
790+
expect {
791+
apply.run
792+
}.to exit_with(0)
793+
.and output(/ensure: changed \[redacted\] to \[redacted\]/).to_stdout
794+
end
780795
end
781796
end

spec/unit/pops/evaluator/deferred_resolver_spec.rb

+49
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
require 'spec_helper'
22
require 'puppet_spec/compiler'
33

4+
Puppet::Type.newtype(:test_deferred) do
5+
newparam(:name)
6+
newproperty(:value)
7+
end
8+
49
describe Puppet::Pops::Evaluator::DeferredResolver do
510
include PuppetSpec::Compiler
611

@@ -46,4 +51,48 @@ def compile_and_resolve_catalog(code, preprocess = false)
4651
expect(deferred.resolve).to eq(["a", "b", "c"])
4752
end
4853

54+
it 'marks the parameter as sensitive when passed an array containing a Sensitive instance' do
55+
catalog = compile_and_resolve_catalog(<<~END)
56+
test_deferred { "deferred":
57+
value => Deferred('join', [['a', Sensitive('b')], ':'])
58+
}
59+
END
60+
61+
resource = catalog.resource(:test_deferred, 'deferred')
62+
expect(resource.sensitive_parameters).to eq([:value])
63+
end
64+
65+
it 'marks the parameter as sensitive when passed a hash containing a Sensitive key' do
66+
catalog = compile_and_resolve_catalog(<<~END)
67+
test_deferred { "deferred":
68+
value => Deferred('keys', [{Sensitive('key') => 'value'}])
69+
}
70+
END
71+
72+
resource = catalog.resource(:test_deferred, 'deferred')
73+
expect(resource.sensitive_parameters).to eq([:value])
74+
end
75+
76+
it 'marks the parameter as sensitive when passed a hash containing a Sensitive value' do
77+
catalog = compile_and_resolve_catalog(<<~END)
78+
test_deferred { "deferred":
79+
value => Deferred('values', [{key => Sensitive('value')}])
80+
}
81+
END
82+
83+
resource = catalog.resource(:test_deferred, 'deferred')
84+
expect(resource.sensitive_parameters).to eq([:value])
85+
end
86+
87+
it 'marks the parameter as sensitive when passed a nested Deferred containing a Sensitive type' do
88+
catalog = compile_and_resolve_catalog(<<~END)
89+
$vars = {'token' => Deferred('new', [Sensitive, "hello"])}
90+
test_deferred { "deferred":
91+
value => Deferred('inline_epp', ['<%= $token %>', $vars])
92+
}
93+
END
94+
95+
resource = catalog.resource(:test_deferred, 'deferred')
96+
expect(resource.sensitive_parameters).to eq([:value])
97+
end
4998
end

0 commit comments

Comments
 (0)