Fix input validation

Fixes: https://pagure.io/koji/issue/3319

hub/kojihub.py

@@ -105,17 +105,19 @@ def convert_value(value, cast=None, message=None,
     """
     if value is None:
         if not none_allowed:
-            raise(exc_type(message or "Invalid type"))
+            raise(exc_type(message or f"Invalid type, expected type {cast}"))
         else:
             return value
     if check_only:
         if not isinstance(value, cast):
-            raise(exc_type(message or f"Invalid type for value '{value}': {type(value)}"))
+            raise(exc_type(message or f"Invalid type for value '{value}': {type(value)}, "
+                                      f"expected type {cast}"))
     else:
         try:
             value = cast(value)
         except (ValueError, TypeError):
-            raise(exc_type(message or f"Invalid type for value '{value}': {type(value)}"))
+            raise(exc_type(message or f"Invalid type for value '{value}': {type(value)}, "
+                                      f"expected type {cast}"))
     return value
 
 
@@ -2507,9 +2509,9 @@ def add_channel(channel_name, description=None):
     :param str description: description of channel
     """
     context.session.assertPerm('admin')
-    convert_value(description, cast=str, none_allowed=True,
-                  message="Channel name must be a string")
     verify_name_internal(channel_name)
+    convert_value(description, cast=str, none_allowed=True,
+                  message="Channel description must be a string")
     dup_check = get_channel(channel_name, strict=False)
     if dup_check:
         raise koji.GenericError("channel %(name)s already exists (id=%(id)i)" % dup_check)

plugins/hub/runroot_hub.py

@@ -31,17 +31,14 @@ def runroot(tagInfo, arch, command, channel=None, **opts):
     """ Create a runroot task """
     context.session.assertPerm('runroot')
     arch = koji.parse_arches(arch, strict=True, allow_none=False)
-    kojihub.convert_value(command, cast=str, check_only=True)
+    if not isinstance(command, (str, list)):
+        raise koji.GenericError(fr"Invalid type for value '{command}': {type(command)}")
     taskopts = {
         'priority': 15,
         'arch': arch,
     }
 
-    if channel is None:
+    taskopts['channel'] = channel or 'runroot'
-        taskopts['channel'] = 'runroot'
-    else:
-        taskopts['channel'] = kojihub.get_channel(channel, strict=True)['name']
-
     tag = kojihub.get_tag(tagInfo, strict=True)
     if arch == 'noarch':
         # not all arches can generate a proper buildroot for all tags

tests/test_hub/test_add_archivetype.py

@@ -60,15 +60,15 @@ class TestAddArchiveType(unittest.TestCase):
         description = ['Debian package']
         with self.assertRaises(koji.ParameterError) as ex:
             kojihub.add_archive_type('deb', description, 'deb')
-        self.assertEqual(f"Invalid type for value '{description}': {type(description)}",
+        self.assertEqual(f"Invalid type for value '{description}': {type(description)}, "
-                         str(ex.exception))
+                         f"expected type <class 'str'>", str(ex.exception))
 
     def test_add_archive_type_invalid_value_extensions(self):
         extensions = ['deb']
         with self.assertRaises(koji.ParameterError) as ex:
             kojihub.add_archive_type('deb', 'Debian package', extensions)
-        self.assertEqual(f"Invalid type for value '{extensions}': {type(extensions)}",
+        self.assertEqual(f"Invalid type for value '{extensions}': {type(extensions)}, "
-                         str(ex.exception))
+                         f"expected type <class 'str'>", str(ex.exception))
 
     def test_add_archive_type_wrong_name_verify(self):
         # name is longer as expected

tests/test_hub/test_add_external_repo_to_tag.py

@@ -28,5 +28,5 @@ class TestAddExternalRepoToTag(unittest.TestCase):
         self.get_external_repo.return_value = {'id': 123}
         with self.assertRaises(koji.GenericError) as cm:
             kojihub.add_external_repo_to_tag(self.tag_name, 'repo', priority, merge_mode=None)
-        self.assertEqual(f"Invalid type for value '{priority}': {type(priority)}",
+        self.assertEqual(f"Invalid type for value '{priority}': {type(priority)}, "
-                         str(cm.exception))
+                         f"expected type <class 'int'>", str(cm.exception))

tests/test_hub/test_build.py

@@ -23,7 +23,8 @@ class TestBuild(unittest.TestCase):
         src = ['test-priority']
         with self.assertRaises(koji.GenericError) as cm:
             self.exports.build(src, self.target)
-        self.assertEqual(f"Invalid type for value '{src}': {type(src)}", str(cm.exception))
+        self.assertEqual(f"Invalid type for value '{src}': {type(src)}, "
+                         f"expected type <class 'str'>", str(cm.exception))
 
     def test_priority_without_admin(self):
         priority = -10

tests/test_hub/test_build_image.py

@@ -35,21 +35,24 @@ class TestBuildImage(unittest.TestCase):
         with self.assertRaises(koji.GenericError) as cm:
             self.exports.buildImage(name, self.version, self.arch, self.target, self.ksfile,
                                     self.image_type)
-        self.assertEqual(f"Invalid type for value '{name}': {type(name)}", str(cm.exception))
+        self.assertEqual(f"Invalid type for value '{name}': {type(name)}, "
+                         f"expected type <class 'str'>", str(cm.exception))
 
     def test_version_wrong_type(self):
         version = ['test-version']
         with self.assertRaises(koji.GenericError) as cm:
             self.exports.buildImage(self.name, version, self.arch, self.target, self.ksfile,
                                     self.image_type)
-        self.assertEqual(f"Invalid type for value '{version}': {type(version)}", str(cm.exception))
+        self.assertEqual(f"Invalid type for value '{version}': {type(version)}, "
+                         f"expected type <class 'str'>", str(cm.exception))
 
     def test_ksfile_wrong_type(self):
         ksfile = ['test-ksfile']
         with self.assertRaises(koji.GenericError) as cm:
             self.exports.buildImage(self.name, self.version, self.arch, self.target, ksfile,
                                     self.image_type)
-        self.assertEqual(f"Invalid type for value '{ksfile}': {type(ksfile)}", str(cm.exception))
+        self.assertEqual(f"Invalid type for value '{ksfile}': {type(ksfile)}, "
+                         f"expected type <class 'str'>", str(cm.exception))
 
     def test_priority_without_admin(self):
         priority = -10

tests/test_hub/test_build_image_oz.py

@@ -25,20 +25,22 @@ class TestBuildImageOz(unittest.TestCase):
         name = ['image-name']
         with self.assertRaises(koji.ParameterError) as cm:
             self.exports.buildImageOz(name, self.version, self.arches, self.target, self.inst_tree)
-        self.assertEqual(f"Invalid type for value '{name}': {type(name)}", str(cm.exception))
+        self.assertEqual(f"Invalid type for value '{name}': {type(name)}, "
+                         f"expected type <class 'str'>", str(cm.exception))
 
     def test_inst_tree_wrong_type(self):
         inst_tree = ['test-tree']
         with self.assertRaises(koji.ParameterError) as cm:
             self.exports.buildImageOz(self.name, self.version, self.arches, self.target, inst_tree)
-        self.assertEqual(f"Invalid type for value '{inst_tree}': {type(inst_tree)}",
+        self.assertEqual(f"Invalid type for value '{inst_tree}': {type(inst_tree)}, "
-                         str(cm.exception))
+                         f"expected type <class 'str'>", str(cm.exception))
 
     def test_version_wrong_type(self):
         version = ['test-version']
         with self.assertRaises(koji.ParameterError) as cm:
             self.exports.buildImageOz(self.name, version, self.arches, self.target, self.inst_tree)
-        self.assertEqual(f"Invalid type for value '{version}': {type(version)}", str(cm.exception))
+        self.assertEqual(f"Invalid type for value '{version}': {type(version)}, "
+                         f"expected type <class 'str'>", str(cm.exception))
 
     def test_priority_without_admin(self):
         priority = -10

tests/test_hub/test_cg_importer.py

@@ -37,8 +37,8 @@ class TestCGImporter(unittest.TestCase):
         metadata = 42
         with self.assertRaises(GenericError) as ex:
             x.get_metadata(metadata, '')
-        self.assertEqual(f"Invalid type for value '{metadata}': {type(metadata)}",
+        self.assertEqual(f"Invalid type for value '{metadata}': {type(metadata)}, "
-                         str(ex.exception))
+                         f"expected type <class 'str'>", str(ex.exception))
 
     def test_get_metadata_is_none(self):
         x = kojihub.CG_Importer()

tests/test_hub/test_chain_build.py

@@ -23,7 +23,8 @@ class TestChainBuild(unittest.TestCase):
         srcs = 'pkg'
         with self.assertRaises(koji.GenericError) as cm:
             self.exports.chainBuild(srcs, self.target)
-        self.assertEqual(f"Invalid type for value '{srcs}': {type(srcs)}", str(cm.exception))
+        self.assertEqual(f"Invalid type for value '{srcs}': {type(srcs)}, "
+                         f"expected type <class 'list'>", str(cm.exception))
 
     def test_priority_without_admin(self):
         priority = -10

tests/test_hub/test_chain_maven.py

@@ -30,7 +30,8 @@ class TestChainMaven(unittest.TestCase):
         self.context.opts.get.return_value = True
         with self.assertRaises(koji.ParameterError) as cm:
             self.exports.chainMaven(builds, self.target)
-        self.assertEqual(f"Invalid type for value '{builds}': {type(builds)}", str(cm.exception))
+        self.assertEqual(f"Invalid type for value '{builds}': {type(builds)}, "
+                         f"expected type <class 'list'>", str(cm.exception))
 
     def test_priority_without_admin(self):
         priority = -10

tests/test_hub/test_disable_channel.py

@@ -34,7 +34,8 @@ class TestDisableChannel(unittest.TestCase):
         comment = ['test-comment']
         with self.assertRaises(koji.GenericError) as cm:
             self.exports.disableChannel(self.channelname, comment=comment)
-        self.assertEqual(f"Invalid type for value '{comment}': {type(comment)}", str(cm.exception))
+        self.assertEqual(f"Invalid type for value '{comment}': {type(comment)}, "
+                         f"expected type <class 'str'>", str(cm.exception))
 
     def test_valid(self):
         self.get_channel.return_value = {'comment': None, 'description': None,

tests/test_hub/test_dist_repo.py

@@ -75,14 +75,16 @@ class TestDistRepoInit(unittest.TestCase):
         keys = 'key1 key2'
         with self.assertRaises(koji.ParameterError) as cm:
             kojihub.dist_repo_init('tag', keys, {'arch': ['x86_64']})
-        self.assertEqual(f"Invalid type for value '{keys}': {type(keys)}", str(cm.exception))
+        self.assertEqual(f"Invalid type for value '{keys}': {type(keys)}, "
+                         f"expected type <class 'list'>", str(cm.exception))
         self.InsertProcessor.assert_not_called()
 
     def test_simple_dist_repo_init_wrong_type_task_opts(self):
         task_opts = 'opts'
         with self.assertRaises(koji.ParameterError) as cm:
             kojihub.dist_repo_init('tag', ['key'], task_opts)
-        self.assertEqual(f"Invalid type for value '{task_opts}': {type(task_opts)}",
+        self.assertEqual(f"Invalid type for value '{task_opts}': {type(task_opts)}, "
+                         f"expected type <class 'dict'>",
                          str(cm.exception))
         self.InsertProcessor.assert_not_called()
 
@@ -90,7 +92,8 @@ class TestDistRepoInit(unittest.TestCase):
         event = 'test-event'
         with self.assertRaises(koji.ParameterError) as cm:
             kojihub.dist_repo_init('tag', ['key'], {'arch': ['x86_64'], 'event': event})
-        self.assertEqual(f"Invalid type for value '{event}': {type(event)}", str(cm.exception))
+        self.assertEqual(f"Invalid type for value '{event}': {type(event)}, "
+                         f"expected type <class 'int'>", str(cm.exception))
         self.InsertProcessor.assert_not_called()
 
     def test_simple_dist_repo_init_wrong_type_volume(self):

tests/test_hub/test_download_task_output.py

@@ -20,7 +20,8 @@ class TestDownloadTaskOutput(unittest.TestCase):
         size = 'test-size'
         with self.assertRaises(koji.ParameterError) as cm:
             self.exports.downloadTaskOutput(self.task_id, self.filename, size=size)
-        self.assertEqual(f"Invalid type for value '{size}': {type(size)}", str(cm.exception))
+        self.assertEqual(f"Invalid type for value '{size}': {type(size)}, "
+                         f"expected type <class 'int'>", str(cm.exception))
 
     def test_volume_non_exist_wrong_type(self):
         self.exports.getVolume.side_effect = koji.GenericError

tests/test_hub/test_edit_channel.py

@@ -131,8 +131,8 @@ class TestEditChannel(unittest.TestCase):
         with self.assertRaises(koji.ParameterError) as ex:
             self.exports.editChannel(self.channel_name, description=description)
         self.assertEqual(self.updates, [])
-        self.assertEqual(f"Invalid type for value '{description}': {type(description)}",
+        self.assertEqual(f"Invalid type for value '{description}': {type(description)}, "
-                         str(ex.exception))
+                         f"expected type <class 'str'>", str(ex.exception))
         self.get_channel.assert_called_once_with(self.channel_name, strict=True)
         self.verify_name_internal.assert_not_called()
 
@@ -142,6 +142,7 @@ class TestEditChannel(unittest.TestCase):
         with self.assertRaises(koji.ParameterError) as ex:
             self.exports.editChannel(self.channel_name, comment=comment)
         self.assertEqual(self.updates, [])
-        self.assertEqual(f"Invalid type for value '{comment}': {type(comment)}", str(ex.exception))
+        self.assertEqual(f"Invalid type for value '{comment}': {type(comment)}, "
+                         f"expected type <class 'str'>", str(ex.exception))
         self.get_channel.assert_called_once_with(self.channel_name, strict=True)
         self.verify_name_internal.assert_not_called()

tests/test_hub/test_edit_permission.py

@@ -47,7 +47,7 @@ class TestEditPermission(unittest.TestCase):
         description = ['test-description']
         with self.assertRaises(koji.GenericError) as ex:
             self.exports.editPermission(self.perm_name, description=description)
-        self.assertEqual(f"Invalid type for value '{description}': {type(description)}",
+        self.assertEqual(f"Invalid type for value '{description}': {type(description)}, "
-                         str(ex.exception))
+                         f"expected type <class 'str'>", str(ex.exception))
         self.update_processor.assert_not_called()
         self.context.session.assertPerm.assert_called_with('admin')

tests/test_hub/test_edit_tag.py

@@ -283,7 +283,8 @@ WHERE id = %(tagID)i""", {'name': 'newtag', 'tagID': 333})
         with self.assertRaises(koji.ParameterError) as ex:
             kojihub._edit_tag('tag', **kwargs)
         self.assertEqual(f"Invalid type for value '{kwargs['remove_extra']}': "
-                         f"{type(kwargs['remove_extra'])}", str(ex.exception))
+                         f"{type(kwargs['remove_extra'])}, expected type <class 'list'>",
+                         str(ex.exception))
 
     def test_edit_tag_block_extra_wrong_format(self):
         kwargs = {
@@ -300,4 +301,5 @@ WHERE id = %(tagID)i""", {'name': 'newtag', 'tagID': 333})
         with self.assertRaises(koji.ParameterError) as ex:
             kojihub._edit_tag('tag', **kwargs)
         self.assertEqual(f"Invalid type for value '{kwargs['block_extra']}': "
-                         f"{type(kwargs['block_extra'])}", str(ex.exception))
+                         f"{type(kwargs['block_extra'])}, expected type <class 'list'>",
+                         str(ex.exception))

tests/test_hub/test_enable_channel.py

@@ -47,4 +47,5 @@ class TestEnableChannel(unittest.TestCase):
         comment = ['test-comment']
         with self.assertRaises(koji.GenericError) as cm:
             self.exports.enableChannel(self.channelname, comment=comment)
-        self.assertEqual(f"Invalid type for value '{comment}': {type(comment)}", str(cm.exception))
+        self.assertEqual(f"Invalid type for value '{comment}': {type(comment)}, "
+                         f"expected type <class 'str'>", str(cm.exception))

tests/test_hub/test_get_archive_type.py

@@ -10,8 +10,8 @@ class TestGetArchiveType(unittest.TestCase):
         filename = ['test-filename']
         with self.assertRaises(koji.ParameterError) as cm:
             kojihub.get_archive_type(filename=filename)
-        self.assertEqual(f"Invalid type for value '{filename}': {type(filename)}",
+        self.assertEqual(f"Invalid type for value '{filename}': {type(filename)}, "
-                         str(cm.exception))
+                         f"expected type <class 'str'>", str(cm.exception))
 
     def test_get_archive_without_opt(self):
         with self.assertRaises(koji.GenericError) as cm:

tests/test_hub/test_grant_permissions.py

@@ -128,7 +128,7 @@ class TestGrantPermission(unittest.TestCase):
         with self.assertRaises(koji.ParameterError) as ex:
             self.exports.grantPermission(self.user_name, self.perms_name,
                                          description=description, create=True)
-        self.assertEqual(f"Invalid type for value '{description}': {type(description)}",
+        self.assertEqual(f"Invalid type for value '{description}': {type(description)}, "
-                         str(ex.exception))
+                         f"expected type <class 'str'>", str(ex.exception))
         self.insert_processor.assert_not_called()
         self.context.session.assertPerm.assert_called_with('admin')

tests/test_hub/test_import_build.py

@@ -136,16 +136,19 @@ class TestImportBuild(unittest.TestCase):
         brmap = 'test-brmap'
         with self.assertRaises(koji.GenericError) as cm:
             kojihub.import_build(self.src_filename, [self.filename], brmap=brmap)
-        self.assertEqual(f"Invalid type for value '{brmap}': {type(brmap)}", str(cm.exception))
+        self.assertEqual(f"Invalid type for value '{brmap}': {type(brmap)}, "
+                         f"expected type <class 'dict'>", str(cm.exception))
 
     def test_import_build_wrong_type_srpm(self):
         srpm = ['test-srpm']
         with self.assertRaises(koji.GenericError) as cm:
             kojihub.import_build(srpm, [self.filename])
-        self.assertEqual(f"Invalid type for value '{srpm}': {type(srpm)}", str(cm.exception))
+        self.assertEqual(f"Invalid type for value '{srpm}': {type(srpm)}, "
+                         f"expected type <class 'str'>", str(cm.exception))
 
     def test_import_build_wrong_type_rpms(self):
         with self.assertRaises(koji.GenericError) as cm:
             kojihub.import_build(self.src_filename, self.filename)
-        self.assertEqual(f"Invalid type for value '{self.filename}': {type(self.filename)}",
+        self.assertEqual(f"Invalid type for value '{self.filename}': {type(self.filename)}, "
+                         f"expected type <class 'list'>",
                          str(cm.exception))

tests/test_hub/test_maven_build.py

@@ -30,7 +30,8 @@ class TestMaven(unittest.TestCase):
         self.context.opts.get.return_value = True
         with self.assertRaises(koji.GenericError) as cm:
             self.exports.mavenBuild(url, self.target)
-        self.assertEqual(f"Invalid type for value '{url}': {type(url)}", str(cm.exception))
+        self.assertEqual(f"Invalid type for value '{url}': {type(url)}, "
+                         f"expected type <class 'str'>", str(cm.exception))
 
     def test_priority_without_admin(self):
         priority = -10

tests/test_hub/test_repo_delete.py

@@ -10,4 +10,5 @@ class TestRepoDelete(unittest.TestCase):
         repo_id = 'test-repo-id'
         with self.assertRaises(koji.ParameterError) as cm:
             kojihub.repo_delete(repo_id)
-        self.assertEqual(f"Invalid type for value '{repo_id}': {type(repo_id)}", str(cm.exception))
+        self.assertEqual(f"Invalid type for value '{repo_id}': {type(repo_id)}, "
+                         f"expected type <class 'int'>", str(cm.exception))

tests/test_hub/test_repo_init.py

@@ -10,4 +10,5 @@ class TestRepoInit(unittest.TestCase):
         task_id = 'test-task_id'
         with self.assertRaises(koji.ParameterError) as cm:
             kojihub.repo_init('test-tag', task_id)
-        self.assertEqual(f"Invalid type for value '{task_id}': {type(task_id)}", str(cm.exception))
+        self.assertEqual(f"Invalid type for value '{task_id}': {type(task_id)}, "
+                         f"expected type <class 'int'>", str(cm.exception))

tests/test_hub/test_repo_set_state.py

@@ -10,4 +10,5 @@ class TestRepoSetState(unittest.TestCase):
         repo_id = 'test-repo-id'
         with self.assertRaises(koji.ParameterError) as cm:
             kojihub.repo_set_state(repo_id, 'failed')
-        self.assertEqual(f"Invalid type for value '{repo_id}': {type(repo_id)}", str(cm.exception))
+        self.assertEqual(f"Invalid type for value '{repo_id}': {type(repo_id)}, "
+                         f"expected type <class 'int'>", str(cm.exception))

tests/test_hub/test_win_build.py

@@ -39,14 +39,16 @@ class TestWinBuild(unittest.TestCase):
         self.context.opts.get.return_value = True
         with self.assertRaises(koji.GenericError) as cm:
             self.exports.winBuild(vm, self.url, self.target)
-        self.assertEqual(f"Invalid type for value '{vm}': {type(vm)}", str(cm.exception))
+        self.assertEqual(f"Invalid type for value '{vm}': {type(vm)}, "
+                         f"expected type <class 'str'>", str(cm.exception))
 
     def test_url_wrong_type(self):
         url = ['test-url']
         self.context.opts.get.return_value = True
         with self.assertRaises(koji.GenericError) as cm:
             self.exports.winBuild(self.vm, url, self.target)
-        self.assertEqual(f"Invalid type for value '{url}': {type(url)}", str(cm.exception))
+        self.assertEqual(f"Invalid type for value '{url}': {type(url)}, "
+                         f"expected type <class 'str'>", str(cm.exception))
 
     def test_priority_without_admin(self):
         priority = -10

tests/test_hub/test_wrapper_rpm.py

@@ -40,7 +40,8 @@ class TestWrapperRPM(unittest.TestCase):
         self.context.opts.get.return_value = True
         with self.assertRaises(koji.GenericError) as cm:
             self.exports.wrapperRPM(self.build, url, self.target)
-        self.assertEqual(f"Invalid type for value '{url}': {type(url)}", str(cm.exception))
+        self.assertEqual(f"Invalid type for value '{url}': {type(url)}, "
+                         f"expected type <class 'str'>", str(cm.exception))
 
     def test_channel_not_str(self):
         priority = 10

tests/test_plugins/test_runroot_hub.py

@@ -152,22 +152,9 @@ class TestRunrootHub(unittest.TestCase):
     @mock.patch('kojihub.get_tag')
     @mock.patch('kojihub.make_task')
     @mock.patch('runroot_hub.context')
-    def test_non_exist_channel(self, context, make_task, get_tag, get_channel):
+    def test_command_wrong_format(self, context, make_task, get_tag, get_channel):
         context.session.assertPerm = mock.MagicMock()
-        get_channel.side_effect = koji.GenericError
+        command = {'ls'}
-        with self.assertRaises(koji.GenericError):
-            runroot_hub.runroot(tagInfo='some_tag', arch='x86_64', command='ls',
-                                channel='non-exist-channel')
-        make_task.assert_not_called()
-        get_tag.assert_not_called()
-
-    @mock.patch('kojihub.get_channel')
-    @mock.patch('kojihub.get_tag')
-    @mock.patch('kojihub.make_task')
-    @mock.patch('runroot_hub.context')
-    def test_commang_wrong_format(self, context, make_task, get_tag, get_channel):
-        context.session.assertPerm = mock.MagicMock()
-        command = ['ls']
         with self.assertRaises(koji.GenericError) as ex:
             runroot_hub.runroot(tagInfo='some_tag', arch='x86_64', command=command,
                                 channel='non-exist-channel')