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

Add rule for checking no_log is set when passwords are used #1558

Merged
merged 5 commits into from May 19, 2021

Conversation

noonedeadpunk
Copy link
Contributor

It's pretty important to ensure, that password won't be logged
and thus exposed in the output. So we add linter rule that will
check that all tasks, that have "password" in argument are not logged.

Signed-Off-By: Dmitriy Rabotyagov noonedeadpunk@ya.ru

@noonedeadpunk noonedeadpunk force-pushed the rule/no_log branch 2 times, most recently from 71cbb5b to 110aa83 Compare May 19, 2021 11:16
It's pretty important to ensure, that password won't be logged
and thus exposed in the output. So we add linter rule that will
check that all tasks, that have "password" in argument are not logged.

Signed-Off-By: Dmitriy Rabotyagov <noonedeadpunk@ya.ru>
@ssbarnea ssbarnea changed the title Add rule for checking no_log is set Add rule for checking no_log is set when passwords are used May 19, 2021
@ssbarnea
Copy link
Member

You still need to: increase number of rules from 39 to 40 (specific test that fails). For lint I am sure you know how to test locally (tox -e lint).

examples/playbooks/no-log-passwords-failure.yml Outdated Show resolved Hide resolved
examples/playbooks/no-log-passwords-success.yml Outdated Show resolved Hide resolved
@noonedeadpunk
Copy link
Contributor Author

Not sure how to satisfy Codacy Static Code Analysis though. As it seems other rules are set the same way with same issues, but it's not complaining about them

@ssbarnea
Copy link
Member

Not sure how to satisfy Codacy Static Code Analysis though. As it seems other rules are set the same way with same issues, but it's not complaining about them

You can fully ignore Codacy is there only as an experiment, but the others must pass, especially the unittesting. Run locally with tox -e py39-core or similar and if it passed you should be fine.

@noonedeadpunk
Copy link
Contributor Author

Yep, also saw things failing, but barely understand why tbh. As how good_runner.run() could result in [no-log-password] (password should not be[270 chars]True

@ssbarnea
Copy link
Member

Yep, also saw things failing, but barely understand why tbh. As how good_runner.run() could result in [no-log-password] (password should not be[270 chars]True

I fixed them.

Since we already have old-style unittest, leaving it as is till mass
migration to pytest. And ingoring raised by this choice warnings.
@ssbarnea ssbarnea merged commit 8bef056 into ansible:master May 19, 2021
@noonedeadpunk noonedeadpunk deleted the rule/no_log branch May 20, 2021 18:29
hswong3i added a commit to alvistack/docker-gitlab-runner that referenced this pull request May 27, 2021
hswong3i added a commit to alvistack/vagrant-centos that referenced this pull request May 27, 2021
hswong3i added a commit to alvistack/vagrant-fedora that referenced this pull request May 27, 2021
hswong3i added a commit to alvistack/vagrant-debian that referenced this pull request May 27, 2021
hswong3i added a commit to alvistack/vagrant-rhel that referenced this pull request May 27, 2021
hswong3i added a commit to alvistack/vagrant-gitlab-runner that referenced this pull request May 27, 2021
hswong3i added a commit to alvistack/vagrant-opensuse that referenced this pull request May 27, 2021
hswong3i added a commit to alvistack/vagrant-ubuntu that referenced this pull request May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants