E208: Added MissingFilePermissionsRule #943
Merged
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
ssbarnea
force-pushed
the
feature/MissingFilePermissions
branch
2 times, most recently
from
25b987d
to
cb3a07d
Compare
yoctozepto
reviewed
Aug 11, 2020
ssbarnea
force-pushed
the
feature/MissingFilePermissions
branch
from
cb3a07d
to
2d51ec6
Compare
|
recheck |
webknjaz
reviewed
Aug 11, 2020
webknjaz
suggested changes
Aug 11, 2020
ssbarnea
force-pushed
the
feature/MissingFilePermissions
branch
from
40c8da8
to
4e3552e
Compare
ssbarnea
force-pushed
the
feature/MissingFilePermissions
branch
from
4e3552e
to
941ac95
Compare
webknjaz
reviewed
Aug 12, 2020
test/conftest.py
Outdated
| def rule_runner(request): | ||
| """Return runner for a specific rule class.""" | ||
| rule_name = request.param | ||
| module = import_module(f"ansiblelint.rules.{rule_name}") |
There was a problem hiding this comment.
I don't think that imports should be delegated to the fixture.
There was a problem hiding this comment.
That is only for internal use and not doing that would make its use much more complex, especially if we start to migrate tests to reuse the runner. The hole point of this fixture was to avoid the old repeating setUp code.
There was a problem hiding this comment.
Import errors should be happening in the test modules, not in the middle of the machinery.
webknjaz
reviewed
Aug 12, 2020
Makes missing file permissions a linting error in order to proactively detect security issues that were fixed via ansible/ansible#67794 at the expense of breaking usage that relied on default settings.
ssbarnea
force-pushed
the
feature/MissingFilePermissions
branch
from
941ac95
to
8a580e1
Compare
webknjaz
approved these changes
Aug 12, 2020
ssbarnea
changed the title
ssato
added a commit
to ssato/ansible-role-simple-httpd-example
that referenced
this pull request
Aug 26, 2020
… by ansible Unfortunatelly, catastrophic and (IMHO) completely wrong changes which normal users don't expect, without enough discussions and notice to users using security as an excuse, looks merged into ansible upstream. To avoid accidents, we need to instruct ansible not change the file permission mode *explicitly* as of now. See also the followings: - ansible/ansible#71200 - ansible/ansible-lint#943
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.
Makes missing file permissions a linting error in order to proactively detect security issues that were fixed via ansible/ansible#67794
While that was fixed in last Ansible hotfix, it changed default value and thus it has a high risk
of breaking any usage without
mode.Related: ansible/ansible#71200