Convert File.sensitive_data to a tri-state variable to allow overriding sensitivity auto-detection #447
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ctheune
requested changes
Mar 28, 2024
zagy
requested changes
Apr 19, 2024
From the initial pull request description:
I haven't added a changelog entry yet because I'm not sure in which section of the changelog file it should be added :) |
Allow both positive and negative overrides of the logic which automatically detects whether a diff contains sensitive data which should not be printed to the terminal.
elikoga
force-pushed
the
sysvinit/sensitive-data-tristate
branch
from
46110b4
to
7c732dd
Compare
zagy
approved these changes
Sep 10, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
Filecomponent has asensitive_dataflag, which is a boolean value which can be set by the operator to indicate that the content of a file is sensitive and so batou should not print a diff to the terminal. In addition, theFilecomponent will automatically check if file content contains words which appear in the encrypted secrets, and if so will not print a diff to avoid secrets being leaked (see #198). However, it's not possible to turn the automatic sensitivity detection off, and there are some cases where this logic is undesirable.A concrete example: I have a deployment where an SSH public key is provided in the environment's encrypted secrets. This means that batou will include the key type string (e.g.
ssh-ed25519) in the list of sensitive words. As a consequence, if I later add a known_hosts file to the deployment, batou will consider this file sensitive (because it contains other key type strings) and will not show a diff -- however a known_hosts file is one instance where I really want to see a diff, as changes in the contents of the known_hosts file might be security-relevant.This change converts
sensitive_datato a tri-state value, which defaults to None. This means by default the automatic detection logic determines if a diff is sensitive, which can be overridden with True so that a diff is never printed (as with the previous behaviour), or with False so that a diff is always printed. There is additionally a new end-to-end test to check all three possible cases of auto-detection, positive override, and negative override.I intentionally have not added a changelog entry just yet, as I'm not sure if this is suitable for the 2.4.2 point release, or whether this change is better introduced in 2.5.