Hide sensitive parameters #33
Conversation
| if resource.key? 'sensitive_parameters' | ||
| resource['sensitive_parameters'].each do |p| | ||
| hash = Digest::SHA256.hexdigest Marshal.dump(resource['parameters'][p]) | ||
| resource['parameters'][p] = 'Sensitive [hash ' + hash + ']' |
There was a problem hiding this comment.
Maybe use a more Ruby-esque syntax:
"Sensitive [hash #{hash}]"
| hash = Digest::SHA256.hexdigest Marshal.dump(resource['parameters'][p]) | ||
| resource['parameters'][p] = 'Sensitive [hash ' + hash + ']' | ||
| end | ||
| resource.tap { |hs| hs.delete('sensitive_parameters') } |
There was a problem hiding this comment.
I'll own up to taking this from stackoverflow! https://stackoverflow.com/a/9027559
There was a problem hiding this comment.
But why not modify the original hash?
There was a problem hiding this comment.
ah, ok. Doh! I should have looked more carefully. The original poster was after a one-liner. I had just forgotten whether you set a key to nil to delete it, or if there was a method to use.
There was a problem hiding this comment.
Actually, do we really need to remove the sensitive_parameters field, if it's only the list of keys?
There was a problem hiding this comment.
yes, I think so. Otherwise our useful hashes get replaced by standard puppet redacted stuff sometime later. IIRC I think during https://github.com/camptocamp/puppet-catalog-diff/blob/b5d0f72d37a58948f45c920caaa39a4a9c90cf44/lib/puppet/catalog-diff/differ.rb#L28? (But would have to double check to be sure if it was after this line or somewhere else)
| catalog | ||
| end | ||
|
|
||
| def redact_sensitive(data) |
There was a problem hiding this comment.
Could this be used in clean_sensitive_parameters to reduce code duplication?
alexjfisher
force-pushed
the
sensitive
branch
2 times, most recently
from
fb3b7d2
to
fa57725
Compare
|
|
||
| def redact_sensitive(data) | ||
| if data.is_a?(Hash) && data.key?('__ptype') | ||
| data[:catalog_diff_hash] = Digest::SHA256.hexdigest Marshal.dump(data['__pvalue']) |
There was a problem hiding this comment.
Ideally, I think I would have liked to replace this hash with a "Sensitive [hash #{hash}]" style string (instead of replacing the __ptype and __pvalue keys. I couldn't figure out how to do this in place though.
|
Sorry, I didn't see your changes from 2 days ago. I'll review again tomorrow. |
This solves an issue for me where even though both 'to' and 'from' catalogs were being compiled, (not coming from puppetdb), resources with sensitive data were still being marked as 'changed'. With this commit, sensitive data won't be leaked, but replaced by a simple hash so that diffs should still show.
|
Rubocop issue should be sorted now. |
This solves an issue for me where even though both 'to' and 'from'
catalogs were being compiled, (not coming from puppetdb), resources with
sensitive data were still being marked as 'changed'.
With this commit, sensitive data won't be leaked, but replaced by a
simple hash so that diffs should still show.