Added MissingFilePermissionsRule

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.

lib/ansiblelint/rules/MissingFilePermissionsRule.py

@@ -0,0 +1,52 @@
+# Copyright (c) 2020 Sorin Sbarnea <sorin.sbarnea@gmail.com>
+#
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the "Software"), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice shall be included in
+# all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+# THE SOFTWARE.
+from ansiblelint.rules import AnsibleLintRule
+
+
+class MissingFilePermissionsRule(AnsibleLintRule):
+    id = "208"
+    shortdesc = 'File permissions not mentioned'
+    description = (
+        "Missing mode parameter can cause unexpected file permissions based "
+        "on version of Ansible being used. Be explicit, or if you still "
+        "want the default behavior you can use ``mode: preserve`` to avoid "
+        "hitting this rule. See "
+        "https://github.com/ansible/ansible/issues/71200"
+    )
+    severity = 'VERY_HIGH'
+    tags = ['unpredictability']
+    version_added = 'v4.3.0'
+
+    _modules = (
+        'copy',
+        'file',
+        'ini_file',
+        'lineinfile',
+        'replace',
+        'template',
+        'unarchive',
+    )
+
+    def matchtask(self, file, task):
+        if task["action"]["__ansible_module__"] not in self._modules:
+            return False
+
+        mode = task['action'].get('mode', None)
+        return not isinstance(mode, str)

test/TestMissingFilePermissionsRule.py

@@ -0,0 +1,54 @@
+# Copyright (c) 2020 Sorin Sbarnea <sorin.sbarnea@gmail.com>
+#
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the "Software"), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice shall be included in
+# all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+# THE SOFTWARE.
+"""MissingFilePermissionsRule tests."""
+import pytest
+
+SUCCESS_TASKS = '''
+---
+- hosts: hosts
+  tasks:
+    - name: permissions not missing
+      file:
+        path: foo
+        mode: preserve
+'''
+
+FAIL_TASKS = '''
+---
+- hosts: hosts
+  tasks:
+    - name: permissions missing
+      file:
+        path: foo
+'''
+
+
+@pytest.mark.parametrize('rule_runner', ("MissingFilePermissionsRule", ), indirect=['rule_runner'])
+def test_success(rule_runner):
+    """Validate that mode presence avoids hitting the rule."""
+    results = rule_runner.run_playbook(SUCCESS_TASKS)
+    assert len(results) == 0
+
+
+@pytest.mark.parametrize('rule_runner', ("MissingFilePermissionsRule", ), indirect=['rule_runner'])
+def test_fail(rule_runner):
+    """Validate that missing mode triggers the rule."""
+    results = rule_runner.run_playbook(FAIL_TASKS)
+    assert len(results) == 1

test/conftest.py

@@ -1,4 +1,5 @@
 import os
+from importlib import import_module
 
 import pytest
 
@@ -19,4 +20,15 @@ def default_text_runner(default_rules_collection):
     return RunFromText(default_rules_collection)
 
 
+@pytest.fixture
+def rule_runner(request):
+    """Return runner for a specific rule class."""
+    rule_name = request.param
+    module = import_module(f"ansiblelint.rules.{rule_name}")
+    collection = RulesCollection()
+    rule = getattr(module, rule_name)
+    collection.register(rule())
+    return RunFromText(collection)
+
+
 # vim: et:sw=4:syntax=python:ts=4:

test/nomatchestest.yml

@@ -6,4 +6,4 @@
       action: debug msg="Hello!"
 
     - name: this should be fine too
-      action: file state=touch dest=./wherever
+      action: file state=touch dest=./wherever mode=preserve

test/unicode.yml

@@ -6,4 +6,4 @@
 
   tasks:
   - name: bonjour, ça va?
-    file: state=touch dest=/tmp/naïve.yml
+    file: state=touch dest=/tmp/naïve.yml mode=preserve