Skip to content

Commit

Permalink
Fail NoLogPassword only when loops are used
Browse files Browse the repository at this point in the history
Fair note has been made in #1589 regarding modules already marking
password fields with `no_log`.
However, when loop is used, passwords are still being exposed in case
of task failure or running in debug.

So we change logic of the check and fail it only when task has some
iteration in it.
  • Loading branch information
Dmitriy Rabotyagov committed May 27, 2021
1 parent 7169480 commit 431e083
Showing 1 changed file with 36 additions and 11 deletions.
47 changes: 36 additions & 11 deletions src/ansiblelint/rules/NoLogPasswordsRule.py
Expand Up @@ -59,10 +59,15 @@ def matchtask(
else:
has_password = False

has_loop = [key for key in task if key.startswith("with_") or key == 'loop']
# No no_log and no_log: False behave the same way
# and should return a failure (return True), so we
# need to invert the boolean
return bool(has_password and not convert_to_boolean(task.get('no_log', False)))
return bool(
has_password
and not convert_to_boolean(task.get('no_log', False))
and len(has_loop) > 0
)


if "pytest" in sys.modules:
Expand All @@ -71,7 +76,7 @@ def matchtask(
NO_LOG_UNUSED = '''
- hosts: all
tasks:
- name: Fail when no_log is not used
- name: Succeed when no_log is not used but no loop present
user:
name: bidule
password: "wow"
Expand All @@ -84,8 +89,11 @@ def matchtask(
- name: Fail when no_log is set to False
user:
name: bidule
user_password: "wow"
user_password: "{{ item }}"
state: absent
with_items:
- wow
- now
no_log: False
'''

Expand All @@ -95,28 +103,39 @@ def matchtask(
- name: Fail when no_log is set to no
user:
name: bidule
password: "wow"
password: "{{ item }}"
state: absent
no_log: no
loop:
- wow
- now
'''

PASSWORD_WITH_LOCK = '''
- hosts: all
tasks:
- name: Fail when password is set and password_lock is true
user:
name: lint
name: "{{ item }}"
password: "wow"
password_lock: true
with_random_choice:
- ansible
- lint
'''

NO_LOG_YES = '''
- hosts: all
tasks:
- name: Succeed when no_log is set to yes
with_list:
- name: user
password: wow
- password: now
name: ansible
user:
name: bidule
password: "wow"
name: "{{ item.name }}"
password: "{{ item.password }}"
state: absent
no_log: yes
'''
Expand All @@ -127,19 +146,25 @@ def matchtask(
- name: Succeed when no_log is set to True
user:
name: bidule
user_password: "wow"
user_password: "{{ item }}"
state: absent
no_log: True
loop:
- wow
- now
'''

PASSWORD_LOCK_YES = '''
- hosts: all
tasks:
- name: Succeed when only password locking account
user:
name: lint
name: "{{ item }}"
password_lock: yes
# user_password: "this is a comment, not a password"
with_list:
- ansible
- lint
'''

PASSWORD_LOCK_FALSE = '''
Expand All @@ -155,9 +180,9 @@ def matchtask(
'rule_runner', (NoLogPasswordsRule,), indirect=['rule_runner']
)
def test_no_log_unused(rule_runner: RunFromText) -> None:
"""The task does not use no_log."""
"""The task does not use no_log but also no loop."""
results = rule_runner.run_playbook(NO_LOG_UNUSED)
assert len(results) == 1
assert len(results) == 0

@pytest.mark.parametrize(
'rule_runner', (NoLogPasswordsRule,), indirect=['rule_runner']
Expand Down

0 comments on commit 431e083

Please sign in to comment.