Don't log the agent configuration changes

[martinisoft]

This will work on any Chef Client 11.14+ by suppressing the diff output in the logs to prevent API credentials from leaking into logs. This could go even farther if needed by using the new ability in Chef to node.rm('datadog', 'api_key') the key from the node data before it is saved back to the Chef server.

[martinisoft]

As an additional note, this code will also work on Chef Clients earlier than 11.14 with the if guard.

[miketheman]

@martinisoft Thanks for the patch - I am curious why you would evaluate the ChefGem resource for its inclusion of sensitive, when this is a template resource - can you elaborate?

[martinisoft]

@miketheman sensitive is a common resource option so we have to sniff out the option from the Chef Gem itself to see if it includes that as an instance method. You'll see it logged as "diff output suppressed" when a given resource is marked as sensitive. So this includes file and template resources.

[miketheman]

@martinisoft Thanks for explaining.
I think I'm not getting it - since when attempting to use chef-shell to suss out behavior here, I can't seem to find the method used to determine behavior.

chef (12.7.2)> Chef::Resource::ChefGem.instance_methods(false).include?(:sensitive)
 => false
chef (12.7.2)> Chef::Resource::ChefGem.instance_methods(false)
 => [:gem_binary, :gem_binary=, :compile_time, :compile_time=, :after_created]

I think the module under inspection is not the Chef gem, rather the chef_gem resource - which doesn't have a notion of sensitive, as it inherits from class ChefGem < Chef::Resource::Package::GemPackage.

Pursuing the ancestors path, we can see that none of the declared resources have this instance_method, other than the main Resource - where it is defined as a common

chef (12.7.2)> Chef::Resource::Template.instance_methods(false).include?(:sensitive)
 => false
chef (12.7.2)> Chef::Resource::File.instance_methods(false).include?(:sensitive)
 => false
chef (12.7.2)> Chef::Resource::Execute.instance_methods(false).include?(:sensitive)
 => false
chef (12.7.2)> Chef::Resource.instance_methods(false).include?(:sensitive)
 => true

Turning it slightly around, we can see:

chef (12.7.2)> Chef::Resource::Template.instance_methods(true).include?(:sensitive)
=> true

So it may appear that this would be the best approach, as we interrogate the Template module for all of the instance_methods it has available to it to find :sensitive - which it gets from inheriting from Chef::Resource.

Does that make sense?

[martinisoft]

@miketheman Makes a lot of sense actually. Just did a little testing with Chef 11.10 and 11.14 and I see your point. I'll update my PR shortly. 👍

[martinisoft]

@miketheman Have updated the change. I did some testing locally and it looks like the Template instance methods will return a false positive there so I had to use the Resource class to detect the sensitive attribute.

On Chef 11.10.4

chef > Chef::Resource::Template.instance_methods(true).include?(:sensitive)
 => true
chef > Chef::Resource.instance_methods(false).include?(:sensitive)
 => false

[miketheman]

👍 Thanks! We'll add this to the merge range for the next minor release cycle.

[martinisoft]

@miketheman Sounds good. Did you want me to also add in the node.rm stuff as well to remove the api key from the node data?

[miketheman]

@martinisoft I think that's a great optimization, I think that's best placed in its own PR - it might be best to wait for the next Major revision for that, since it removes something from the node's saved attributes (hopefully nobody else uses that, but you never know...).

[martinisoft]

@miketheman Fair enough. 👍

[martinisoft]

@miketheman So I completely forgot the LWRP and just included that as well on the file and template resources called as well because secrets can get exposed through there as well.

[iiro]

This is superb stuff! Glad I found this as credentials are all over logs now...