From ed87f3d0f9432bb68cae305a108c46328c3beec9 Mon Sep 17 00:00:00 2001
From: clavedeluna <danalitovsky+git@gmail.com>
Date: Tue, 2 Jan 2024 16:47:20 -0300
Subject: [PATCH] add review updates

---
 src/core_codemods/subprocess_shell_false.py   | 27 ++++++++++--
 tests/codemods/test_subprocess_shell_false.py | 44 ++++++++++---------
 2 files changed, 48 insertions(+), 23 deletions(-)

diff --git a/src/core_codemods/subprocess_shell_false.py b/src/core_codemods/subprocess_shell_false.py
index 21ecc5cfa..635a663c4 100644
--- a/src/core_codemods/subprocess_shell_false.py
+++ b/src/core_codemods/subprocess_shell_false.py
@@ -10,18 +10,39 @@ class SubprocessShellFalse(BaseCodemod, NameResolutionMixin):
     SUMMARY = "Use `shell=False` in `subprocess` Function Calls"
     REVIEW_GUIDANCE = ReviewGuidance.MERGE_AFTER_CURSORY_REVIEW
     DESCRIPTION = "Set `shell` keyword argument to `False`"
-    REFERENCES: list = []
+    REFERENCES = [
+        {
+            "url": "https://docs.python.org/3/library/subprocess.html#security-considerations",
+            "description": "",
+        },
+        {
+            "url": "https://en.wikipedia.org/wiki/Code_injection#Shell_injection",
+            "description": "",
+        },
+        {
+            "url": "https://stackoverflow.com/a/3172488",
+            "description": "",
+        },
+    ]
     SUBPROCESS_FUNCS = [
         f"subprocess.{func}"
         for func in {"run", "call", "check_output", "check_call", "Popen"}
     ]
 
     def leave_Call(self, original_node: cst.Call, updated_node: cst.Call):
+        if not self.filter_by_path_includes_or_excludes(
+            self.node_position(original_node)
+        ):
+            return
+
         if self.find_base_name(original_node.func) in self.SUBPROCESS_FUNCS:
             for arg in original_node.args:
                 if matchers.matches(
-                    arg.keyword, matchers.Name("shell")
-                ) and matchers.matches(arg.value, matchers.Name("True")):
+                    arg,
+                    matchers.Arg(
+                        keyword=matchers.Name("shell"), value=matchers.Name("True")
+                    ),
+                ):
                     self.report_change(original_node)
                     new_args = self.replace_args(
                         original_node,
diff --git a/tests/codemods/test_subprocess_shell_false.py b/tests/codemods/test_subprocess_shell_false.py
index 782169008..b19c95134 100644
--- a/tests/codemods/test_subprocess_shell_false.py
+++ b/tests/codemods/test_subprocess_shell_false.py
@@ -15,40 +15,44 @@ def test_name(self):
 
     @each_func
     def test_import(self, tmpdir, func):
-        input_code = f"""import subprocess
-subprocess.{func}(args, shell=True)
-"""
-        expexted_output = f"""import subprocess
-subprocess.{func}(args, shell=False)
-"""
-
+        input_code = f"""
+        import subprocess
+        subprocess.{func}(args, shell=True)
+        """
+        expexted_output = f"""
+        import subprocess
+        subprocess.{func}(args, shell=False)
+        """
         self.run_and_assert(tmpdir, input_code, expexted_output)
         assert len(self.file_context.codemod_changes) == 1
 
     @each_func
     def test_from_import(self, tmpdir, func):
-        input_code = f"""from subprocess import {func}
-{func}(args, shell=True)
-"""
-        expexted_output = f"""from subprocess import {func}
-{func}(args, shell=False)
-"""
-
+        input_code = f"""
+        from subprocess import {func}
+        {func}(args, shell=True)
+        """
+        expexted_output = f"""
+        from subprocess import {func}
+        {func}(args, shell=False)
+        """
         self.run_and_assert(tmpdir, input_code, expexted_output)
         assert len(self.file_context.codemod_changes) == 1
 
     @each_func
     def test_no_shell(self, tmpdir, func):
-        input_code = f"""import subprocess
-subprocess.{func}(args, timeout=1)
-"""
+        input_code = f"""
+        import subprocess
+        subprocess.{func}(args, timeout=1)
+        """
         self.run_and_assert(tmpdir, input_code, input_code)
         assert len(self.file_context.codemod_changes) == 0
 
     @each_func
     def test_shell_False(self, tmpdir, func):
-        input_code = f"""import subprocess
-subprocess.{func}(args, shell=False)
-"""
+        input_code = f"""
+        import subprocess
+        subprocess.{func}(args, shell=False)
+        """
         self.run_and_assert(tmpdir, input_code, input_code)
         assert len(self.file_context.codemod_changes) == 0