From 35cd3c7a3625afcfca253b15a81373a2686483df Mon Sep 17 00:00:00 2001
From: Ondrej Nosek <onosek@redhat.com>
Date: Jul 27 2022 02:30:03 +0000
Subject: rpmdefines changes depending on rpkg


'rpmdefines' structure needs a different format after it was changed
in rpkg (RHELCMP-9241).
Also fixes some bandit security findings.

Relates: https://pagure.io/rpkg/pull-request/620
JIRA: RHELCMP-9646

Signed-off-by: Ondrej Nosek <onosek@redhat.com>

---

diff --git a/fedpkg/__init__.py b/fedpkg/__init__.py
index f041b97..067867d 100644
--- a/fedpkg/__init__.py
+++ b/fedpkg/__init__.py
@@ -125,8 +125,8 @@ class Commands(pyrpkg.Commands):
             self.override = 'eln-override'
             self._distunset = 'fedora'
             extra_rpmdefines = [
-                "--define 'el%s 1'" % macros['rhel'],
-                "--define 'rhel %s'" % macros['rhel'],
+                "--define", "el%s 1" % macros['rhel'],
+                "--define", "rhel %s" % macros['rhel'],
             ]
         # rawhide (previously master branch)
         elif re.match(r'(rawhide|main)$', self.branch_merge):
@@ -141,25 +141,24 @@ class Commands(pyrpkg.Commands):
             raise pyrpkg.rpkgError('Could not find the release/dist from branch name '
                                    '%s\nPlease specify with --release' %
                                    self.branch_merge)
-        self._rpmdefines = ["--define '_sourcedir %s'" % self.layout.sourcedir,
-                            "--define '_specdir %s'" % self.layout.specdir,
-                            "--define '_builddir %s'" % self.layout.builddir,
-                            "--define '_srcrpmdir %s'" % self.layout.srcrpmdir,
-                            "--define '_rpmdir %s'" % self.layout.rpmdir,
-                            "--define '_rpmfilename %s'" % self.layout.rpmfilename,
-                            "--define 'dist %%{?distprefix}.%s'" % self._disttag,
-                            "--define '%s %s'" % (self._distvar,
-                                                  self._distval),
-                            "--eval '%%undefine %s'" % self._distunset,
-                            "--define '%s 1'" % self._disttag.replace(".", "_")]
+        self._rpmdefines = ["--define", "_sourcedir %s" % self.layout.sourcedir,
+                            "--define", "_specdir %s" % self.layout.specdir,
+                            "--define", "_builddir %s" % self.layout.builddir,
+                            "--define", "_srcrpmdir %s" % self.layout.srcrpmdir,
+                            "--define", "_rpmdir %s" % self.layout.rpmdir,
+                            "--define", "_rpmfilename %s" % self.layout.rpmfilename,
+                            "--define", "dist %%{?distprefix}.%s" % self._disttag,
+                            "--define", "%s %s" % (self._distvar, self._distval),
+                            "--eval", "%%undefine %s" % self._distunset,
+                            "--define", "%s 1" % self._disttag.replace(".", "_")]
         # TODO: consider removing macro "%s 1; it has unknown/dubious functionality"
 
         if self._runtime_disttag:
             if self._disttag != self._runtime_disttag:
                 # This means that the runtime is known, and is different from
                 # the target, so we need to unset the _runtime_disttag
-                self._rpmdefines.append("--eval '%%undefine %s'" %
-                                        self._runtime_disttag)
+                self._rpmdefines.extend(["--eval", "%%undefine %s" %
+                                        self._runtime_disttag])
 
         if extra_rpmdefines:
             self._rpmdefines.extend(extra_rpmdefines)
diff --git a/fedpkg/cli.py b/fedpkg/cli.py
index d24e83c..638e557 100644
--- a/fedpkg/cli.py
+++ b/fedpkg/cli.py
@@ -873,7 +873,7 @@ class fedpkgClient(cliClient):
         if not self.args.update_type or not self.args.notes:
             # Open the template in a text editor
             editor = os.getenv('EDITOR', 'vi')
-            self.cmd._run_command([editor, template_file], shell=True)
+            self.cmd._run_command([editor, template_file], shell=False)
 
             # Check to see if we got a template written out.  Bail otherwise
             if not os.path.isfile(template_file):
diff --git a/test/test_cli.py b/test/test_cli.py
index f956dcc..43bd20a 100644
--- a/test/test_cli.py
+++ b/test/test_cli.py
@@ -278,7 +278,7 @@ class TestUpdate(CliTestCase):
             self.assert_bodhi_update, cli)
 
         self.mock_run_command.assert_called_once_with(
-            ['vi', 'bodhi.template'], shell=True)
+            ['vi', 'bodhi.template'], shell=False)
 
     def test_request_update(self):
         cli_cmd = ['fedpkg', '--path', self.cloned_repo_path, 'update']
@@ -287,7 +287,7 @@ class TestUpdate(CliTestCase):
         self.assert_bodhi_update(cli, update_type='enhancement')
 
         self.mock_run_command.assert_called_once_with(
-            ['vi', 'bodhi.template'], shell=True)
+            ['vi', 'bodhi.template'], shell=False)
 
     def test_request_update_with_karma(self):
         cli_cmd = [
@@ -316,7 +316,7 @@ class TestUpdate(CliTestCase):
         self.assert_bodhi_update(cli, update_type='enhancement')
 
         self.mock_run_command.assert_called_once_with(
-            ['vi', 'bodhi.template'], shell=True)
+            ['vi', 'bodhi.template'], shell=False)
 
     def test_missing_update_type_in_template(self):
         cli_cmd = ['fedpkg-stage', '--path', self.cloned_repo_path, 'update']
@@ -369,7 +369,7 @@ class TestUpdate(CliTestCase):
         self.assert_bodhi_update(
             cli, update_type='enhancement', notes='Mass rebuild')
         self.mock_run_command.assert_called_once_with(
-            ['vi', 'bodhi.template'], shell=True)
+            ['vi', 'bodhi.template'], shell=False)
 
     def test_show_editor_if_notes_is_omitted(self):
         cli_cmd = [
@@ -381,7 +381,7 @@ class TestUpdate(CliTestCase):
 
         self.assert_bodhi_update(cli, update_type='bugfix')
         self.mock_run_command.assert_called_once_with(
-            ['vi', 'bodhi.template'], shell=True)
+            ['vi', 'bodhi.template'], shell=False)
 
     def test_create_with_notes_in_multiple_lines(self):
         cli_cmd = [
diff --git a/test/test_commands.py b/test/test_commands.py
index c7467ae..1ea3e2d 100644
--- a/test/test_commands.py
+++ b/test/test_commands.py
@@ -185,17 +185,17 @@ class TestLoadRpmDefines(CommandTestCase):
     def assert_rpmdefines(self, extra_rpmdefines=[]):
         """Assert Commands._rpmdefines after calling load_rpmdefines"""
         expected_rpmdefines = [
-            "--define '_sourcedir %s'" % self.cmd.layout.sourcedir,
-            "--define '_specdir %s'" % self.cmd.layout.specdir,
-            "--define '_builddir %s'" % self.cmd.layout.builddir,
-            "--define '_srcrpmdir %s'" % self.cmd.layout.srcrpmdir,
-            "--define '_rpmdir %s'" % self.cmd.layout.rpmdir,
-            "--define '_rpmfilename %s'" % self.cmd.layout.rpmfilename,
-            "--define 'dist %%{?distprefix}.%s'" % self.cmd._disttag,
-            "--define '%s %s'" % (self.cmd._distvar, self.cmd._distval),
-            "--eval '%%undefine %s'" % self.cmd._distunset,
-            "--define '%s 1'" % self.cmd._disttag.replace(".", "_"),
-            "--eval '%%undefine %s'" % self.cmd._runtime_disttag
+            "--define", "_sourcedir %s" % self.cmd.layout.sourcedir,
+            "--define", "_specdir %s" % self.cmd.layout.specdir,
+            "--define", "_builddir %s" % self.cmd.layout.builddir,
+            "--define", "_srcrpmdir %s" % self.cmd.layout.srcrpmdir,
+            "--define", "_rpmdir %s" % self.cmd.layout.rpmdir,
+            "--define", "_rpmfilename %s" % self.cmd.layout.rpmfilename,
+            "--define", "dist %%{?distprefix}.%s" % self.cmd._disttag,
+            "--define", "%s %s" % (self.cmd._distvar, self.cmd._distval),
+            "--eval", "%%undefine %s" % self.cmd._distunset,
+            "--define", "%s 1" % self.cmd._disttag.replace(".", "_"),
+            "--eval", "%%undefine %s" % self.cmd._runtime_disttag
         ]
         expected_rpmdefines.extend(extra_rpmdefines)
         self.assertEqual(expected_rpmdefines, self.cmd._rpmdefines)
@@ -338,8 +338,8 @@ class TestLoadRpmDefines(CommandTestCase):
         self.assertEqual('fedora', self.cmd._distunset)
 
         extra_rpmdefines = [
-            "--define 'el9 1'",
-            "--define 'rhel 9'",
+            "--define", "el9 1",
+            "--define", "rhel 9",
         ]
         self.assert_rpmdefines(extra_rpmdefines)
 

