Apache Airflow Pinot provider allowed Command Injection

@@ -24,6 +24,28 @@
 Changelog
 ---------
 
+4.0.0
+.....
+
+This release of provider is only available for Airflow 2.3+ as explained in the Apache Airflow
+providers support policy https://github.com/apache/airflow/blob/main/README.md#support-for-providers
+
+Breaking changes
+~~~~~~~~~~~~~~~~
+
+The admin command is now hard-coded to ``pinot-admin.sh``. The ``pinot-admin.sh`` command must be available
+on the path in order to use PinotAdminHook.
+
+Misc
+~~~~
+
+* ``Move min airflow version to 2.3.0 for all providers (#27196)``
+* ``Bump pinotdb version (#27201)``
+
+.. Below changes are excluded from the changelog. Move them to
+   appropriate section above if needed. Do not delete the lines(!):
+   * ``Enable string normalization in python formatting - providers (#27205)``
+
 3.2.1
 .....
 

@@ -46,7 +46,10 @@ class PinotAdminHook(BaseHook):
     following PR: https://github.com/apache/incubator-pinot/pull/4110
 
     :param conn_id: The name of the connection to use.
-    :param cmd_path: The filepath to the pinot-admin.sh executable
+    :param cmd_path: Do not modify the parameter. It used to be the filepath to the pinot-admin.sh
+           executable but in version 4.0.0 of apache-pinot provider, value of this parameter must
+           remain the default value: `pinot-admin.sh`. It is left here to not accidentally override
+           the `pinot_admin_system_exit` in case positional parameters were used to initialize the hook.
     :param pinot_admin_system_exit: If true, the result is evaluated based on the status code.
                                     Otherwise, the result is evaluated as a failure if "Error" or
                                     "Exception" is in the output message.
@@ -62,7 +65,15 @@ def __init__(
         conn = self.get_connection(conn_id)
         self.host = conn.host
         self.port = str(conn.port)
-        self.cmd_path = conn.extra_dejson.get("cmd_path", cmd_path)
+        if cmd_path != "pinot-admin.sh":
+            raise RuntimeError(
+                "In version 4.0.0 of the PinotAdminHook the cmd_path has been hard-coded to"
+                " pinot-admin.sh. In order to avoid accidental using of this parameter as"
+                " positional `pinot_admin_system_exit` the `cmd_parameter`"
+                " parameter is left here but you should not modify it. Make sure that "
+                " `pinot-admin.sh` is on your PATH and do not change cmd_path value."
+            )
+        self.cmd_path = "pinot-admin.sh"
         self.pinot_admin_system_exit = conn.extra_dejson.get(
             "pinot_admin_system_exit", pinot_admin_system_exit
         )
@@ -213,7 +224,6 @@ def run_cli(self, cmd: list[str], verbose: bool = True) -> str:
 
         if verbose:
             self.log.info(" ".join(command))
-
         with subprocess.Popen(
             command, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, close_fds=True, env=env
         ) as sub_process:

@@ -22,6 +22,7 @@ description: |
     `Apache Pinot <https://pinot.apache.org/>`__
 
 versions:
+  - 4.0.0
   - 3.2.1
   - 3.2.0
   - 3.1.0

@@ -35,7 +35,7 @@ def setUp(self):
         self.conn = conn = mock.MagicMock()
         self.conn.host = "host"
         self.conn.port = "1000"
-        self.conn.extra_dejson = {"cmd_path": "./pinot-admin.sh"}
+        self.conn.extra_dejson = {}
 
         class PinotAdminHookTest(PinotAdminHook):
             def get_connection(self, conn_id):
@@ -165,7 +165,7 @@ def test_run_cli_success(self, mock_popen):
 
         params = ["foo", "bar", "baz"]
         self.db_hook.run_cli(params)
-        params.insert(0, self.conn.extra_dejson.get("cmd_path"))
+        params.insert(0, "pinot-admin.sh")
         mock_popen.assert_called_once_with(
             params, stderr=subprocess.STDOUT, stdout=subprocess.PIPE, close_fds=True, env=None
         )
@@ -180,7 +180,7 @@ def test_run_cli_failure_error_message(self, mock_popen):
         params = ["foo", "bar", "baz"]
         with pytest.raises(AirflowException):
             self.db_hook.run_cli(params)
-        params.insert(0, self.conn.extra_dejson.get("cmd_path"))
+        params.insert(0, "pinot-admin.sh")
         mock_popen.assert_called_once_with(
             params, stderr=subprocess.STDOUT, stdout=subprocess.PIPE, close_fds=True, env=None
         )
@@ -196,14 +196,23 @@ def test_run_cli_failure_status_code(self, mock_popen):
         params = ["foo", "bar", "baz"]
         with pytest.raises(AirflowException):
             self.db_hook.run_cli(params)
-        params.insert(0, self.conn.extra_dejson.get("cmd_path"))
+        params.insert(0, "pinot-admin.sh")
         env = os.environ.copy()
         env.update({"JAVA_OPTS": "-Dpinot.admin.system.exit=true "})
         mock_popen.assert_called_once_with(
             params, stderr=subprocess.STDOUT, stdout=subprocess.PIPE, close_fds=True, env=env
         )
 
 
+class TestPinotAdminHookCreation:
+    def test_exception_when_overriding_cmd_path(self):
+        with pytest.raises(RuntimeError):
+            PinotAdminHook(cmd_path="some_path.sh")
+
+    def test_exception_when_keeping_cmd_path(self):
+        PinotAdminHook(cmd_path="pinot-admin.sh")
+
+
 class TestPinotDbApiHook(unittest.TestCase):
     def setUp(self):
         super().setUp()