From b2015c98e24989ec6dffcc5a119f0925c5b3220c Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Thu, 11 Mar 2021 18:57:11 +0100 Subject: [PATCH] validate-modules: make sure that options that potentially contain secret data have no_log set (#73508) * Catch more potential errors (and increase false-positive rate). * Flag some false-positives in lib/ansible/modules/ with no_log=False. Co-authored-by: John Barker --- .../73508-validate-modules-no_log.yml | 2 + .../dev_guide/testing_validate-modules.rst | 1 + lib/ansible/modules/apt_key.py | 2 +- lib/ansible/modules/getent.py | 2 +- lib/ansible/modules/known_hosts.py | 2 +- lib/ansible/modules/rpm_key.py | 2 +- .../validate-modules/validate_modules/main.py | 38 +++++++++++++++++++ 7 files changed, 45 insertions(+), 4 deletions(-) create mode 100644 changelogs/fragments/73508-validate-modules-no_log.yml diff --git a/changelogs/fragments/73508-validate-modules-no_log.yml b/changelogs/fragments/73508-validate-modules-no_log.yml new file mode 100644 index 00000000000..84920b8d5cd --- /dev/null +++ b/changelogs/fragments/73508-validate-modules-no_log.yml @@ -0,0 +1,2 @@ +minor_changes: +- "ansible-test validate-modules - option names that seem to indicate they contain secret information that should be marked ``no_log=True`` are now flagged in the validate-modules sanity test. False positives can be marked by explicitly setting ``no_log=False`` for these options in the argument spec. Please note that many false positives are expected; the assumption is that it is by far better to have false positives than false negatives (https://github.com/ansible/ansible/pull/73508)." diff --git a/docs/docsite/rst/dev_guide/testing_validate-modules.rst b/docs/docsite/rst/dev_guide/testing_validate-modules.rst index 99a4eaebf7e..958c349c2e0 100644 --- a/docs/docsite/rst/dev_guide/testing_validate-modules.rst +++ b/docs/docsite/rst/dev_guide/testing_validate-modules.rst @@ -116,6 +116,7 @@ Codes multiple-utils-per-requires Imports Error ``Ansible.ModuleUtils`` requirements do not support multiple modules per statement multiple-csharp-utils-per-requires Imports Error Ansible C# util requirements do not support multiple utils per statement no-default-for-required-parameter Documentation Error Option is marked as required but specifies a default. Arguments with a default should not be marked as required + no-log-needed Parameters Error Option name suggests that the option contains a secret value, while ``no_log`` is not specified for this option in the argument spec. If this is a false positive, explicitly set ``no_log=False`` nonexistent-parameter-documented Documentation Error Argument is listed in DOCUMENTATION.options, but not accepted by the module option-incorrect-version-added Documentation Error ``version_added`` for new option is incorrect option-invalid-version-added Documentation Error ``version_added`` for option is not a valid version number diff --git a/lib/ansible/modules/apt_key.py b/lib/ansible/modules/apt_key.py index 10dfd359edc..87b6410a630 100644 --- a/lib/ansible/modules/apt_key.py +++ b/lib/ansible/modules/apt_key.py @@ -363,7 +363,7 @@ def main(): url=dict(type='str'), data=dict(type='str'), file=dict(type='path'), - key=dict(type='str', removed_in_version='2.14', removed_from_collection='ansible.builtin'), + key=dict(type='str', removed_in_version='2.14', removed_from_collection='ansible.builtin', no_log=False), keyring=dict(type='path'), validate_certs=dict(type='bool', default=True), keyserver=dict(type='str'), diff --git a/lib/ansible/modules/getent.py b/lib/ansible/modules/getent.py index 4eb471a2634..e5c7aaf2ad1 100644 --- a/lib/ansible/modules/getent.py +++ b/lib/ansible/modules/getent.py @@ -99,7 +99,7 @@ def main(): module = AnsibleModule( argument_spec=dict( database=dict(type='str', required=True), - key=dict(type='str'), + key=dict(type='str', no_log=False), service=dict(type='str'), split=dict(type='str'), fail_key=dict(type='bool', default=True), diff --git a/lib/ansible/modules/known_hosts.py b/lib/ansible/modules/known_hosts.py index aac1e0036da..8d4b226f8ee 100644 --- a/lib/ansible/modules/known_hosts.py +++ b/lib/ansible/modules/known_hosts.py @@ -335,7 +335,7 @@ def main(): module = AnsibleModule( argument_spec=dict( name=dict(required=True, type='str', aliases=['host']), - key=dict(required=False, type='str'), + key=dict(required=False, type='str', no_log=False), path=dict(default="~/.ssh/known_hosts", type='path'), hash_host=dict(required=False, type='bool', default=False), state=dict(default='present', choices=['absent', 'present']), diff --git a/lib/ansible/modules/rpm_key.py b/lib/ansible/modules/rpm_key.py index 46ef34c2466..0ab186dc467 100644 --- a/lib/ansible/modules/rpm_key.py +++ b/lib/ansible/modules/rpm_key.py @@ -233,7 +233,7 @@ def main(): module = AnsibleModule( argument_spec=dict( state=dict(type='str', default='present', choices=['absent', 'present']), - key=dict(type='str', required=True), + key=dict(type='str', required=True, no_log=False), fingerprint=dict(type='str'), validate_certs=dict(type='bool', default=True), ), diff --git a/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/main.py b/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/main.py index 1ffc57ad04c..a5e0806ba9a 100644 --- a/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/main.py +++ b/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/main.py @@ -69,6 +69,9 @@ REJECTLIST_DIRS = frozenset(('.git', 'test', '.github', '.idea')) INDENT_REGEX = re.compile(r'([\t]*)') TYPE_REGEX = re.compile(r'.*(if|or)(\s+[^"\']*|\s+)(? ".join(context) + self.reporter.error( + path=self.object_path, + code='no-log-needed', + msg=msg, + ) + if not isinstance(data, dict): msg = "Argument '%s' in argument_spec" % arg if context: