Skip to content
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.

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

sops_encrypt: mark parameters containing secrets with no_log=True #54

Merged
merged 2 commits into from
Feb 6, 2021

Conversation

felixfontein
Copy link
Collaborator

More fallout from ansible/ansible#73508.

We should release this as 1.0.4 ASAP.

@codecov
Copy link

codecov bot commented Feb 6, 2021

Codecov Report

Merging #54 (5b11a9a) into main (4fc784c) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #54   +/-   ##
=======================================
  Coverage   62.44%   62.44%           
=======================================
  Files           8        8           
  Lines         900      900           
  Branches      201      201           
=======================================
  Hits          562      562           
  Misses        260      260           
  Partials       78       78           
Impacted Files Coverage Δ
plugins/module_utils/sops.py 86.53% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4fc784c...fddba4f. Read the comment docs.

Copy link

@tadeboro tadeboro left a comment

Choose a reason for hiding this comment

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

Looks good. Not 100% sure why we should hide the threshold value, but that is probably because I do not know enough about crypto stuff.

@felixfontein
Copy link
Collaborator Author

@tadeboro the Shamir threshold value says how many keys are needed to decrypt a secret. That's part of the algorithm configuration, and definitely does not need to be secret. It's also stored directly in the metadata of an encrypted file (https://github.com/mozilla/sops/blob/develop/sops.go#L485).

@felixfontein
Copy link
Collaborator Author

Ah, sorry, I misread you. We don't hide it, I set no_log=False to explicitly mark it as "this is not secret", as ansible/ansible#73508 will report it because of "secret" in the name :)

@felixfontein felixfontein merged commit 8d6a84d into ansible-collections:main Feb 6, 2021
@felixfontein felixfontein deleted the no_log-fixes branch February 6, 2021 12:47
@felixfontein
Copy link
Collaborator Author

@tadeboro thanks for reviewing this!
@endorama I'll create a new release for this later today

@tadeboro
Copy link

tadeboro commented Feb 6, 2021

Ugh, that was my bad. I did not notice that you are silencing the false positive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants