WIP: Add E8011 discouraging shlex.quote() usages on Windows (#7)

* WIP: Add E8010 discouraging shlex.quote() usages on Windows

TODO : write Pylint tests

* Add unit tests and minor improvements

* Update CHANGELOG

* Fix typo during last merge commit

Co-authored-by: Nguyen Damien <[email protected]>

Author: HorlogeSkynet

CHANGELOG.md

@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 
 -   Added E8010 to avoid using `os.popen()` as it internally uses `subprocess.Popen` with `shell=True`
+-   Added E8011 to avoid using `shlex.quote()` on non-POSIX platforms.
 
 ## [1.1.0] - 2021-07-02
 

README.md

@@ -24,6 +24,7 @@ pylint plugin that enforces some secure coding standards.
 | C8008 | Avoid `assert` statements in production code                                                                 |
 | R8009 | Use of builtin `open` for writing is discouraged in favor of `os.open` to allow for setting file permissions |
 | E8010 | Avoid using `os.popen()` as it internally uses `subprocess.Popen` with `shell=True`                          |
+| E8011 | Use of `shlex.quote()` should be avoided on non-POSIX platforms                                              |
 
 
 ## Pre-commit hook

pylint_secure_coding_standard.py

@@ -15,12 +15,25 @@
 
 """Main file for the pylint_secure_coding_standard plugin"""
 
+import platform
+
 import astroid
 
 from pylint.checkers import BaseChecker
 from pylint.interfaces import IAstroidChecker
 
 
+# ==============================================================================
+# Helper functions
+
+
+def _is_posix():
+    """Return True if the current system is POSIX-compatible."""
+    # NB: we could simply use `os.name` instead of `platform.system()`. However, that solution would be difficult to
+    #     test using `mock` as a few modules (like `pytest`) actually use it internally...
+    return platform.system() in ('Linux', 'Darwin')
+
+
 # ==============================================================================
 
 
@@ -171,6 +184,15 @@ def _is_jsonpickle_encode_call(node):
     return False
 
 
+def _is_shlex_quote_call(node):
+    return not _is_posix() and (
+        isinstance(node.func, astroid.Attribute)
+        and isinstance(node.func.expr, astroid.Name)
+        and node.func.expr.name == 'shlex'
+        and node.func.attrname == 'quote'
+    )
+
+
 # ==============================================================================
 
 
@@ -235,6 +257,11 @@ class SecureCodingStandardChecker(BaseChecker):
             'avoid-os-popen',
             'Use of `os.popen()` should be avoided, as it internally uses `subprocess.Popen` with `shell=True`',
         ),
+        'E8011': (
+            'Avoid using `shlex.quote()` on non-POSIX platforms',
+            'avoid-shlex-quote-on-non-posix',
+            'Use of `shlex.quote()` should be avoided on non-POSIX platforms (such as Windows)',
+        ),
     }
 
     options = {}
@@ -263,6 +290,8 @@ def visit_call(self, node):
             self.add_message('replace-builtin-open', node=node)
         elif isinstance(node.func, astroid.Name) and (node.func.name in ('eval', 'exec')):
             self.add_message('avoid-eval-exec', node=node)
+        elif _is_shlex_quote_call(node):
+            self.add_message('avoid-shlex-quote-on-non-posix', node=node)
 
     def visit_import(self, node):
         """
@@ -288,6 +317,8 @@ def visit_importfrom(self, node):
             self.add_message('avoid-os-system', node=node)
         elif node.modname == 'os' and [name for (name, _) in node.names if name == 'popen']:
             self.add_message('avoid-os-popen', node=node)
+        elif not _is_posix() and node.modname == 'shlex' and [name for (name, _) in node.names if name == 'quote']:
+            self.add_message('avoid-shlex-quote-on-non-posix', node=node)
 
     def visit_with(self, node):
         """

pyproject.toml

@@ -66,6 +66,7 @@ build-backend = "setuptools.build_meta"
 minversion = '6.0'
 addopts = '-pno:warnings'
 testpaths = ['tests']
+mock_traceback_monkeypatch = false
 
 [tool.setuptools_scm]
 write_to = 'VERSION.txt'

setup.cfg

@@ -31,8 +31,10 @@ install_requires =
 
 [options.extras_require]
 test =
+    mock
     pytest
     pytest-cov
+    pytest-mock
 
 
 [bdist_wheel]

tests/shlex_quote_test.py

@@ -0,0 +1,70 @@
+# -*- coding: utf-8 -*-
+# Copyright 2021 Damien Nguyen
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import astroid
+import pytest
+
+import pylint_secure_coding_standard as pylint_scs
+import pylint.testutils
+
+
+class TestSecureCodingStandardChecker(pylint.testutils.CheckerTestCase):
+    CHECKER_CLASS = pylint_scs.SecureCodingStandardChecker
+
+    @pytest.mark.parametrize(
+        'platform, expected_success',
+        (
+            ('Linux', True),
+            ('Darwin', True),
+            ('Java', False),
+            ('Windows', False),
+        ),
+    )
+    @pytest.mark.parametrize('s', ('from shlex import quote',))
+    def test_shlex_quote_importfrom(self, mocker, platform, expected_success, s):
+        mocker.patch('platform.system', lambda: platform)
+
+        node = astroid.extract_node(s + ' #@')
+        if expected_success:
+            self.checker.visit_importfrom(node)
+        else:
+            with self.assertAddsMessages(pylint.testutils.Message(msg_id='avoid-shlex-quote-on-non-posix', node=node)):
+                self.checker.visit_importfrom(node)
+
+    @pytest.mark.parametrize(
+        'platform, expected_success',
+        (
+            ('Linux', True),
+            ('Darwin', True),
+            ('Java', False),
+            ('Windows', False),
+        ),
+    )
+    @pytest.mark.parametrize(
+        's',
+        (
+            'shlex.quote("ls -l")',
+            'shlex.quote(command_str)',
+        ),
+    )
+    def test_shlex_call_quote(self, mocker, platform, expected_success, s):
+        mocker.patch('platform.system', lambda: platform)
+
+        node = astroid.extract_node(s + ' #@')
+        if expected_success:
+            self.checker.visit_call(node)
+        else:
+            with self.assertAddsMessages(pylint.testutils.Message(msg_id='avoid-shlex-quote-on-non-posix', node=node)):
+                self.checker.visit_call(node)