path: root/0007-More-robust-spec-file-presence-checking.patch

From 1108810bdefd0d880517b274acd6a3bd0d4156e0 Mon Sep 17 00:00:00 2001
From: Ondrej Nosek <onosek@redhat.com>
Date: Tue, 21 Mar 2023 02:44:04 +0100
Subject: [PATCH 07/12] More robust spec file presence checking

Some commands (verrel, sources, prep, import, ...) need to check
whether the dist-git repository is in the correct state. It means
at least the presence of the specfile.
In the beginning, rpkg detects layouts. Layouts determine the file
structure of the repository. For example, most commands can't
be executed for the RetiredLayout (there is no specfile).
When the repository directory exists, some layout can be always
detected. Therefore '--path' argument is now checked for
a valid directory.
The timeout change in the request fixes the new bandit's finding.

Fixes: #663
JIRA: RHELCMP-11387

Signed-off-by: Ondrej Nosek <onosek@redhat.com>
---
 pyrpkg/__init__.py          |  9 ++++---
 pyrpkg/cli.py               |  8 +++---
 pyrpkg/layout/__init__.py   |  4 +--
 pyrpkg/utils.py             | 14 ++++++++++
 tests/commands/test_push.py | 54 +++++++++++++++++++------------------
 tests/test_cli.py           | 12 ++++++---
 6 files changed, 63 insertions(+), 38 deletions(-)

diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py
index 776cb21..028d195 100644
--- a/pyrpkg/__init__.py
+++ b/pyrpkg/__init__.py
@@ -923,9 +923,8 @@ class Commands(object):
     def load_spec(self):
         """This sets the spec attribute"""
 
-        if self.layout is None:
+        if self.layout is None or isinstance(self.layout, layout.IncompleteLayout):
             raise rpkgError('Spec file is not available')
-
         if self.is_retired():
             raise rpkgError('This package or module is retired. The action has stopped.')
 
@@ -1166,8 +1165,10 @@ class Commands(object):
 
     @property
     def sources_filename(self):
-        if self.layout is None:
-            return os.path.join(self.path, 'sources')
+        if self.layout is None or isinstance(self.layout, layout.IncompleteLayout):
+            raise rpkgError('Spec file is not available')
+        if self.is_retired():
+            raise rpkgError('This package or module is retired. The action has stopped.')
         return os.path.join(
             self.path, self.layout.sources_file_template.replace("{0.repo_name}", self.repo_name))
 
diff --git a/pyrpkg/cli.py b/pyrpkg/cli.py
index c3672b3..1bcf6e4 100644
--- a/pyrpkg/cli.py
+++ b/pyrpkg/cli.py
@@ -386,7 +386,7 @@ class cliClient(object):
                                  help='Run Koji commands as a different user')
         # Let the user define a path to work in rather than cwd
         self.parser.add_argument('--path', default=None,
-                                 type=utils.u,
+                                 type=utils.validate_path,
                                  help='Define the directory to work in '
                                  '(defaults to cwd)')
         # Verbosity
@@ -911,8 +911,9 @@ class cliClient(object):
         if 'path' in args:
             # Without "path", we can't really test...
             url = '%(protocol)s://%(host)s/%(path)s/info/refs?service=git-receive-pack' % args
-            resp = requests.head(url, auth=HTTPBasicAuth(args['username'],
-                                                         args['password']))
+            resp = requests.head(url,
+                                 auth=HTTPBasicAuth(args['username'], args['password']),
+                                 timeout=15)
             if resp.status_code == 401:
                 return self.oidc_client.report_token_issue()
 
@@ -2363,6 +2364,7 @@ class cliClient(object):
 
     def import_srpm(self):
         uploadfiles = self.cmd.import_srpm(self.args.srpm)
+        self.load_cmd()  # to reload layouts - because a specfile could appear during import
         if uploadfiles:
             try:
                 self.cmd.upload(uploadfiles, replace=True, offline=self.args.offline)
diff --git a/pyrpkg/layout/__init__.py b/pyrpkg/layout/__init__.py
index 762af0d..850ddc2 100644
--- a/pyrpkg/layout/__init__.py
+++ b/pyrpkg/layout/__init__.py
@@ -12,8 +12,8 @@
 from pyrpkg.errors import LayoutError
 
 from .base import MetaLayout
-from .layouts import (DistGitLayout, IncompleteLayout,  # noqa: F401
-                      RetiredLayout, SRPMLayout)
+from .layouts import (DistGitLayout, DistGitResultsDirLayout,  # noqa: F401
+                      IncompleteLayout, RetiredLayout, SRPMLayout)
 
 
 def build(path, hint=None):
diff --git a/pyrpkg/utils.py b/pyrpkg/utils.py
index ceb4906..3337bdb 100644
--- a/pyrpkg/utils.py
+++ b/pyrpkg/utils.py
@@ -26,11 +26,25 @@ if six.PY3:
     def u(s):
         return s
 
+    def validate_path(s):
+        abspath = os.path.abspath(s)
+        if os.path.exists(abspath):
+            return s
+        else:
+            raise argparse.ArgumentTypeError('given path \'{0}\' doesn\'t exist'.format(abspath))
+
     getcwd = os.getcwd
 else:
     def u(s):
         return s.decode('utf-8')
 
+    def validate_path(s):
+        abspath = os.path.abspath(s.decode('utf-8'))
+        if os.path.exists(abspath):
+            return s.decode('utf-8')
+        else:
+            raise argparse.ArgumentTypeError('given path \'{0}\' doesn\'t exist'.format(abspath))
+
     getcwd = os.getcwdu
 
 
diff --git a/tests/commands/test_push.py b/tests/commands/test_push.py
index ef8057a..79c3a8b 100644
--- a/tests/commands/test_push.py
+++ b/tests/commands/test_push.py
@@ -1,9 +1,13 @@
 # -*- coding: utf-8 -*-
 
 import os
+import subprocess
 
 import git
 
+import pyrpkg
+from pyrpkg.sources import SourcesFile
+
 from . import CommandTestCase
 
 SPECFILE_TEMPLATE = """Name:           test
@@ -22,11 +26,6 @@ Test
 %%install
 rm -f $RPM_BUILD_ROOT%%{_sysconfdir}/"""
 
-CLONE_CONFIG = '''
-    bz.default-component %(module)s
-    sendemail.to %(module)s-owner@fedoraproject.org
-'''
-
 
 class CommandPushTestCase(CommandTestCase):
 
@@ -45,28 +44,30 @@ class CommandPushTestCase(CommandTestCase):
 
         self.make_new_git(self.module)
 
-        import pyrpkg
-        cmd = pyrpkg.Commands(self.path, self.lookaside,
-                              self.lookasidehash,
-                              self.lookaside_cgi, self.gitbaseurl,
-                              self.anongiturl, self.branchre, self.kojiprofile,
-                              self.build_client, self.user, self.dist,
-                              self.target, self.quiet)
-        cmd.clone_config_rpms = CLONE_CONFIG
-        cmd.clone(self.module, anon=True)
-        cmd.path = os.path.join(self.path, self.module)
-        os.chdir(os.path.join(self.path, self.module))
+        moduledir = os.path.join(self.gitroot, self.module)
+        subprocess.check_call(['git', 'clone', 'file://%s' % moduledir],
+                              cwd=self.path, stdout=subprocess.PIPE,
+                              stderr=subprocess.PIPE)
+
+        self.cloned_dir = os.path.join(self.path, self.module)
+        self.cmd = pyrpkg.Commands(self.cloned_dir, self.lookaside,
+                                   self.lookasidehash,
+                                   self.lookaside_cgi, self.gitbaseurl,
+                                   self.anongiturl, self.branchre, self.kojiprofile,
+                                   self.build_client, self.user, self.dist,
+                                   self.target, self.quiet)
+        os.chdir(self.cloned_dir)
 
         spec_file = 'module.spec'
         with open(spec_file, 'w') as f:
             f.write(SPECFILE_TEMPLATE % '')
 
-        cmd.repo.index.add([spec_file])
-        cmd.repo.index.commit("add SPEC")
+        self.cmd.repo.index.add([spec_file])
+        self.cmd.repo.index.commit("add SPEC")
 
         # Now, change directory to parent and test the push
         os.chdir(self.path)
-        cmd.push(no_verify=True)
+        self.cmd.push(no_verify=True)
 
 
 class TestPushWithPatches(CommandTestCase):
@@ -76,18 +77,20 @@ class TestPushWithPatches(CommandTestCase):
 
         self.make_new_git(self.module)
 
-        import pyrpkg
-        self.cmd = pyrpkg.Commands(self.path, self.lookaside,
+        moduledir = os.path.join(self.gitroot, self.module)
+        subprocess.check_call(['git', 'clone', 'file://%s' % moduledir],
+                              cwd=self.path, stdout=subprocess.PIPE,
+                              stderr=subprocess.PIPE)
+
+        self.cloned_dir = os.path.join(self.path, self.module)
+        self.cmd = pyrpkg.Commands(self.cloned_dir, self.lookaside,
                                    self.lookasidehash,
                                    self.lookaside_cgi, self.gitbaseurl,
                                    self.anongiturl, self.branchre,
                                    self.kojiprofile,
                                    self.build_client, self.user, self.dist,
                                    self.target, self.quiet)
-        self.cmd.clone_config_rpms = CLONE_CONFIG
-        self.cmd.clone(self.module, anon=True)
-        self.cmd.path = os.path.join(self.path, self.module)
-        os.chdir(os.path.join(self.path, self.module))
+        os.chdir(self.cloned_dir)
 
         # Track SPEC and a.patch in git
         spec_file = 'module.spec'
@@ -103,7 +106,6 @@ Patch3: d.path
                 f.write(patch_file)
 
         # Track c.patch in sources
-        from pyrpkg.sources import SourcesFile
         sources_file = SourcesFile(self.cmd.sources_filename,
                                    self.cmd.source_entry_type)
         file_hash = self.cmd.lookasidecache.hash_file('c.patch')
diff --git a/tests/test_cli.py b/tests/test_cli.py
index df053aa..868ad1f 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -1841,9 +1841,11 @@ class TestMockbuild(CliTestCase):
     @patch('pyrpkg.Commands._config_dir_basic')
     @patch('pyrpkg.Commands._config_dir_other')
     @patch('os.path.exists', return_value=False)
+    @patch('pyrpkg.utils.validate_path')
     def test_use_mock_config_got_from_koji(
-            self, exists, config_dir_other, config_dir_basic):
+            self, validate_path, exists, config_dir_other, config_dir_basic):
         mock_layout = layout.DistGitLayout(root_dir=self.cloned_repo_path)
+        validate_path.return_value = self.cloned_repo_path
         with patch('pyrpkg.layout.build', return_value=mock_layout):
             config_dir_basic.return_value = '/path/to/config-dir'
 
@@ -1859,9 +1861,11 @@ class TestMockbuild(CliTestCase):
 
     @patch('pyrpkg.Commands._config_dir_basic')
     @patch('os.path.exists', return_value=False)
+    @patch('pyrpkg.utils.validate_path')
     def test_fail_to_store_mock_config_in_created_config_dir(
-            self, exists, config_dir_basic):
+            self, validate_path, exists, config_dir_basic):
         config_dir_basic.side_effect = rpkgError
+        validate_path.return_value = self.cloned_repo_path
 
         cli_cmd = ['rpkg', '--path', self.cloned_repo_path,
                    '--release', 'rhel-7', 'mockbuild']
@@ -1870,10 +1874,12 @@ class TestMockbuild(CliTestCase):
     @patch('pyrpkg.Commands._config_dir_basic')
     @patch('pyrpkg.Commands._config_dir_other')
     @patch('os.path.exists', return_value=False)
+    @patch('pyrpkg.utils.validate_path')
     def test_fail_to_populate_mock_config(
-            self, exists, config_dir_other, config_dir_basic):
+            self, validate_path, exists, config_dir_other, config_dir_basic):
         config_dir_basic.return_value = '/path/to/config-dir'
         config_dir_other.side_effect = rpkgError
+        validate_path.return_value = self.cloned_repo_path
 
         cli_cmd = ['rpkg', '--path', self.cloned_repo_path,
                    '--release', 'rhel-7', 'mockbuild']
-- 
2.39.2