From 1acd2247a6e3bfb48e772a069bbb7f59e44bdb86 Mon Sep 17 00:00:00 2001
From: Daniel D'Avella <dan.davella@pixee.ai>
Date: Tue, 12 Dec 2023 13:57:06 -0500
Subject: [PATCH] Add support for Popen to process-creation-sandbox

---
 integration_tests/test_process_sandbox.py     |  6 ++--
 src/core_codemods/process_creation_sandbox.py |  6 ++++
 .../codemods/test_process_creation_sandbox.py | 30 +++++++++++++++++++
 3 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/integration_tests/test_process_sandbox.py b/integration_tests/test_process_sandbox.py
index bcc67cd3a..a54fb9269 100644
--- a/integration_tests/test_process_sandbox.py
+++ b/integration_tests/test_process_sandbox.py
@@ -14,11 +14,11 @@ class TestProcessSandbox(BaseIntegrationTest):
             (1, """from security import safe_command\n\n"""),
             (2, """safe_command.run(subprocess.run, "echo 'hi'", shell=True)\n"""),
             (3, """safe_command.run(subprocess.run, ["ls", "-l"])\n"""),
-            (5, """safe_command.call(subprocess.call, "echo 'hi'", shell=True)\n"""),
-            (6, """safe_command.call(subprocess.call, ["ls", "-l"])\n"""),
+            (5, """safe_command.run(subprocess.call, "echo 'hi'", shell=True)\n"""),
+            (6, """safe_command.run(subprocess.call, ["ls", "-l"])\n"""),
         ],
     )
-    expected_diff = '--- \n+++ \n@@ -1,10 +1,11 @@\n import subprocess\n+from security import safe_command\n \n-subprocess.run("echo \'hi\'", shell=True)\n-subprocess.run(["ls", "-l"])\n+safe_command.run(subprocess.run, "echo \'hi\'", shell=True)\n+safe_command.run(subprocess.run, ["ls", "-l"])\n \n-subprocess.call("echo \'hi\'", shell=True)\n-subprocess.call(["ls", "-l"])\n+safe_command.call(subprocess.call, "echo \'hi\'", shell=True)\n+safe_command.call(subprocess.call, ["ls", "-l"])\n \n subprocess.check_output(["ls", "-l"])\n \n'
+    expected_diff = '--- \n+++ \n@@ -1,10 +1,11 @@\n import subprocess\n+from security import safe_command\n \n-subprocess.run("echo \'hi\'", shell=True)\n-subprocess.run(["ls", "-l"])\n+safe_command.run(subprocess.run, "echo \'hi\'", shell=True)\n+safe_command.run(subprocess.run, ["ls", "-l"])\n \n-subprocess.call("echo \'hi\'", shell=True)\n-subprocess.call(["ls", "-l"])\n+safe_command.run(subprocess.call, "echo \'hi\'", shell=True)\n+safe_command.run(subprocess.call, ["ls", "-l"])\n \n subprocess.check_output(["ls", "-l"])\n \n'
     expected_line_change = "3"
     num_changes = 4
     num_changed_files = 2
diff --git a/src/core_codemods/process_creation_sandbox.py b/src/core_codemods/process_creation_sandbox.py
index fda2b4516..e3a9339ad 100644
--- a/src/core_codemods/process_creation_sandbox.py
+++ b/src/core_codemods/process_creation_sandbox.py
@@ -39,6 +39,11 @@ def rule(cls):
                 - pattern-inside: |
                     import subprocess
                     ...
+              - patterns:
+                - pattern: subprocess.Popen(...)
+                - pattern-inside: |
+                    import subprocess
+                    ...
         """
 
     def on_result_found(self, original_node, updated_node):
@@ -47,5 +52,6 @@ def on_result_found(self, original_node, updated_node):
         return self.update_call_target(
             updated_node,
             "safe_command",
+            new_func="run",
             replacement_args=[cst.Arg(original_node.func), *original_node.args],
         )
diff --git a/tests/codemods/test_process_creation_sandbox.py b/tests/codemods/test_process_creation_sandbox.py
index 58950b472..51309755d 100644
--- a/tests/codemods/test_process_creation_sandbox.py
+++ b/tests/codemods/test_process_creation_sandbox.py
@@ -138,3 +138,33 @@ def test_custom_run(self, tmpdir):
         run("echo 'hi'", shell=True)"""
         expected = input_code
         self.run_and_assert(tmpdir, input_code, expected)
+
+    def test_subprocess_call(self, tmpdir):
+        input_code = """
+        import subprocess
+
+        subprocess.call(["ls", "-l"])
+        """
+        expected = """
+        import subprocess
+        from security import safe_command
+
+        safe_command.run(subprocess.call, ["ls", "-l"])
+        """
+        self.run_and_assert(tmpdir, input_code, expected)
+        self.assert_dependency(Security)
+
+    def test_subprocess_popen(self, tmpdir):
+        input_code = """
+        import subprocess
+
+        subprocess.Popen(["ls", "-l"])
+        """
+        expected = """
+        import subprocess
+        from security import safe_command
+
+        safe_command.run(subprocess.Popen, ["ls", "-l"])
+        """
+        self.run_and_assert(tmpdir, input_code, expected)
+        self.assert_dependency(Security)