Skip to content

(PUP-8969) Preserve sensitiveness of epp templates #8330

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 22, 2020
Merged

(PUP-8969) Preserve sensitiveness of epp templates #8330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions lib/puppet/face/epp.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,12 @@ def get_values(compiler, options)

def render_inline(epp_source, compiler, options)
template_args = get_values(compiler, options)
Puppet::Pops::Evaluator::EppEvaluator.inline_epp(compiler.topscope, epp_source, template_args)
result = Puppet::Pops::Evaluator::EppEvaluator.inline_epp(compiler.topscope, epp_source, template_args)
if result.instance_of?(Puppet::Pops::Types::PSensitiveType::Sensitive)
result.unwrap
else
result
end
end

def render_file(epp_template_name, compiler, options, show_filename, file_nbr)
Expand All @@ -457,7 +462,12 @@ def render_file(epp_template_name, compiler, options, show_filename, file_nbr)
if template_file.nil? && Puppet::FileSystem.exist?(epp_template_name)
epp_template_name = File.expand_path(epp_template_name)
end
output << Puppet::Pops::Evaluator::EppEvaluator.epp(compiler.topscope, epp_template_name, compiler.environment, template_args)
result = Puppet::Pops::Evaluator::EppEvaluator.epp(compiler.topscope, epp_template_name, compiler.environment, template_args)
if result.instance_of?(Puppet::Pops::Types::PSensitiveType::Sensitive)
output << result.unwrap
else
output << result
end
rescue Puppet::ParseError => detail
Puppet.err("--- #{epp_template_name}") if show_filename
raise detail
Expand Down
1 change: 1 addition & 0 deletions lib/puppet/functions/epp.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
scope_param
param 'String', :path
optional_param 'Hash[Pattern[/^\w+$/], Any]', :parameters
return_type 'Variant[String, Sensitive[String]]'
end

def epp(scope, path, parameters = nil)
Expand Down
1 change: 1 addition & 0 deletions lib/puppet/functions/inline_epp.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
scope_param()
param 'String', :template
optional_param 'Hash[Pattern[/^\w+$/], Any]', :parameters
return_type 'Variant[String, Sensitive[String]]'
end

def inline_epp(scope, template, parameters = nil)
Expand Down
25 changes: 22 additions & 3 deletions lib/puppet/pops/evaluator/evaluator_impl.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,10 +462,24 @@ def calculate(left, right, bin_expr, scope)
end

def eval_EppExpression(o, scope)
contains_sensitive = false

scope["@epp"] = []
evaluate(o.body, scope)
result = scope["@epp"].join
result
result = scope["@epp"].map do |r|
if r.instance_of?(Puppet::Pops::Types::PSensitiveType::Sensitive)
contains_sensitive = true
string(r.unwrap, scope)
else
r
end
end.join

if contains_sensitive
Puppet::Pops::Types::PSensitiveType::Sensitive.new(result)
else
result
end
end

def eval_RenderStringExpression(o, scope)
Expand All @@ -474,7 +488,12 @@ def eval_RenderStringExpression(o, scope)
end

def eval_RenderExpression(o, scope)
scope["@epp"] << string(evaluate(o.expr, scope), scope)
result = evaluate(o.expr, scope)
if result.instance_of?(Puppet::Pops::Types::PSensitiveType::Sensitive)
scope["@epp"] << result
else
scope["@epp"] << string(result, scope)
end
nil
end

Expand Down
27 changes: 26 additions & 1 deletion spec/unit/functions/inline_epp_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@

require 'spec_helper'

require 'puppet_spec/compiler'

describe "the inline_epp function" do
include PuppetSpec::Files
include PuppetSpec::Compiler

let :node do Puppet::Node.new('localhost') end
let :compiler do Puppet::Parser::Compiler.new(node) end
Expand Down Expand Up @@ -73,6 +75,29 @@
expect(eval_template("string was: <%= $string %>")).to eq("string was: the string value")
end

context "when using Sensitive" do
it "returns an unwrapped sensitive value as a String" do
expect(eval_and_collect_notices(<<~END)).to eq(["opensesame"])
notice(inline_epp("<%= Sensitive('opensesame').unwrap %>"))
END
end

it "rewraps a sensitive value" do
# note entire result is redacted, not just sensitive part
expect(eval_and_collect_notices(<<~END)).to eq(["Sensitive [value redacted]"])
notice(inline_epp("This is sensitive <%= Sensitive('opensesame') %>"))
END
end

it "can be double wrapped" do
catalog = compile_to_catalog(<<~END)
notify { 'title':
message => Sensitive(inline_epp("<%= Sensitive('opensesame') %>"))
}
END
expect(catalog.resource(:notify, 'title')['message']).to eq('opensesame')
end
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% about the location of these tests, but the second and third tests fail as expected when run against puppet code that doesn't have this change:

  1) the inline_epp function when using Sensitive rewraps a sensitive value
     Failure/Error:
       expect(eval_and_collect_notices(<<~END)).to eq(["Sensitive [value redacted]"])
         notice(inline_epp("This is <%= Sensitive('opensesame') %>"))
       END
     
       expected: ["Sensitive [value redacted]"]
            got: ["This is sensitive Sensitive [value redacted]"]
     
       (compared using ==)
     # ./spec/unit/functions/inline_epp_spec.rb:87:in `block (3 levels) in <top (required)>'
     # ./spec/spec_helper.rb:180:in `block (2 levels) in <top (required)>'
     # ./.bundle/ruby/2.7.0/gems/webmock-3.9.3/lib/webmock/rspec.rb:37:in `block (2 levels) in <top (required)>'

  2) the inline_epp function when using Sensitive can be double wrapped
     Failure/Error: expect(catalog.resource(:notify, 'title')['message']).to eq('opensesame')
     
       expected: "opensesame"
            got: "Sensitive [value redacted]"
     
       (compared using ==)
     # ./spec/unit/functions/inline_epp_spec.rb:98:in `block (3 levels) in <top (required)>'
     # ./spec/spec_helper.rb:180:in `block (2 levels) in <top (required)>'
     # ./.bundle/ruby/2.7.0/gems/webmock-3.9.3/lib/webmock/rspec.rb:37:in `block (2 levels) in <top (required)>'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the location is weird, there's 4 places that these changes could be (evaluator, face, inline_epp, and epp) but to actually put something in those places is largely copying the same tests around.


def eval_template_with_args(content, args_hash)
epp_function.call(scope, content, args_hash)
Expand Down