Hub, plugins and tools inputs validation

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

hub/kojihub.py

@@ -91,6 +91,30 @@ def xform_user_krb(entry):
     return entry
 
 
+def convert_value(value, cast=None, message=None, exc_type=None, none_allowed=False):
+    if exc_type is None:
+        exc_type = koji.ParameterError
+    if value is None:
+        if not none_allowed:
+            raise(exc_type(message or "Invalid type"))
+        else:
+            return value
+    try:
+        return cast(value)
+    except (ValueError, TypeError):
+        raise(exc_type(message or f"Invalid type for value '{value}': {type(value)}"))
+
+
+def check_value_type(value, cast=None, message=None, exc_type=None, none_allowed=False):
+    if none_allowed:
+        if value is None:
+            return value
+    if exc_type is None:
+        exc_type = koji.ParameterError
+    if not isinstance(value, cast):
+        raise (exc_type(message or f"Invalid type for value '{value}': {type(value)}"))
+
+
 class Task(object):
     """A task for the build hosts"""
 
@@ -116,7 +140,7 @@ class Task(object):
         ('task.weight', 'weight'))
 
     def __init__(self, id):
-        self.id = int(id)
+        self.id = convert_value(id, cast=int)
         self.logger = logging.getLogger("koji.hub.Task")
 
     def _split_fields(self, fields=None):
@@ -288,7 +312,7 @@ class Task(object):
     def setWeight(self, weight):
         """Set weight for task"""
         task_id = self.id
-        weight = float(weight)
+        weight = convert_value(weight, cast=float)
         info = self.getInfo(request=True)
         self.runCallbacks('preTaskStateChange', info, 'weight', weight)
         # access checks should be performed by calling function
@@ -299,7 +323,7 @@ class Task(object):
     def setPriority(self, priority, recurse=False):
         """Set priority for task"""
         task_id = self.id
-        priority = int(priority)
+        priority = convert_value(priority, cast=int)
         info = self.getInfo(request=True)
         self.runCallbacks('preTaskStateChange', info, 'priority', priority)
         # access checks should be performed by calling function
@@ -540,6 +564,22 @@ def make_task(method, arglist, **opts):
         priority: the priority of the task
         assign: a host_id to assign the task to
     """
+    check_value_type(method, cast=str)
+    if 'parent' in opts:
+        opts['parent'] = convert_value(opts['parent'], cast=int)
+    if 'label' in opts:
+        check_value_type(opts['label'], cast=str)
+    if 'owner' in opts:
+        if not isinstance(opts['owner'], int):
+            opts['owner'] = get_user(opts['owner'], strict=True)['id']
+    if 'arch' in opts:
+        opts['arch'] = koji.parse_arches(opts['arch'], strict=True, allow_none=True)
+    if 'priority' in opts:
+        opts['priority'] = \
+            convert_value(opts['priority'], cast=int)
+    if 'assign' in opts:
+        if not isinstance(opts['assign'], int):
+            opts['assign'] = get_host(opts['assign'], strict=True)['id']
     if 'parent' in opts:
         # for subtasks, we use some of the parent's options as defaults
         fields = ('state', 'owner', 'channel_id', 'priority', 'arch')
@@ -1389,6 +1429,8 @@ def readTaggedBuilds(tag, event=None, inherit=False, latest=False, package=None,
         'build.state = %(st_complete)i'
     ]
     if package:
+        if not isinstance(package, str):
+            package = lookup_package(package, strict=True)['name']
         clauses.append('package.name = %(package)s')
     if owner:
         clauses.append('users.name = %(owner)s')
@@ -1636,8 +1678,10 @@ def check_tag_access(tag_id, user_id=None):
     """
     if user_id is None:
         user_id = context.session.user_id
-    if user_id is None:
-        raise koji.GenericError("a user_id is required")
+        if not user_id:
+            raise koji.GenericError("a user_id is required")
+    else:
+        user_id = convert_value(user_id, cast=int)
     perms = koji.auth.get_user_perms(user_id)
     override = False
     if 'admin' in perms:
@@ -2376,12 +2420,14 @@ def edit_channel(channelInfo, **kw):
         return False
 
     if kw.get('name'):
-        if not isinstance(kw['name'], str):
-            raise koji.GenericError("new channel name must be a string")
         verify_name_internal(kw['name'])
         dup_check = get_channel(kw['name'], strict=False)
         if dup_check:
             raise koji.GenericError("channel %(name)s already exists (id=%(id)i)" % dup_check)
+    if kw.get('description'):
+        check_value_type(kw['description'], cast=str)
+    if kw.get('comment'):
+        check_value_type(kw['comment'], cast=str)
 
     update = UpdateProcessor('channels',
                              values={'channelID': channel['id']},
@@ -2433,8 +2479,7 @@ def add_channel(channel_name, description=None):
     :param str description: description of channel
     """
     context.session.assertPerm('admin')
-    if not isinstance(channel_name, str):
-        raise koji.GenericError("Channel name must be a string")
+    check_value_type(description, cast=str, none_allowed=True)
     verify_name_internal(channel_name)
     dup_check = get_channel(channel_name, strict=False)
     if dup_check:
@@ -2449,6 +2494,7 @@ def add_channel(channel_name, description=None):
 
 def set_channel_enabled(channelname, enabled=True, comment=None):
     context.session.assertPerm('host')
+    check_value_type(comment, cast=str, none_allowed=True)
     channel = get_channel(channelname)
     if not channel:
         raise koji.GenericError('No such channel: %s' % channelname)
@@ -2640,6 +2686,7 @@ def repo_init(tag, task_id=None, with_src=False, with_debuginfo=False, event=Non
     Returns a dictionary containing
         repo_id, event_id
     """
+    task_id = convert_value(task_id, cast=int, none_allowed=True)
     logger = logging.getLogger("koji.hub.repo_init")
     state = koji.REPO_INIT
     tinfo = get_tag(tag, strict=True, event=event)
@@ -2694,9 +2741,9 @@ def repo_init(tag, task_id=None, with_src=False, with_debuginfo=False, event=Non
         'tag_id': tinfo['id'],
         'task_id': task_id,
         'event_id': event_id,
-        'with_src': with_src,
-        'with_separate_src': with_separate_src,
-        'with_debuginfo': with_debuginfo,
+        'with_src': bool(with_src),
+        'with_separate_src': bool(with_separate_src),
+        'with_debuginfo': bool(with_debuginfo),
     }
     with open('%s/repo.json' % repodir, 'wt', encoding='utf-8') as fp:
         json.dump(repo_info, fp, indent=2)
@@ -2823,9 +2870,12 @@ def _write_maven_repo_metadata(destdir, artifacts):
 def dist_repo_init(tag, keys, task_opts):
     """Create a new repo entry in the INIT state, return full repo data"""
     state = koji.REPO_INIT
+    check_value_type(keys, cast=list)
     tinfo = get_tag(tag, strict=True)
     tag_id = tinfo['id']
+    check_value_type(task_opts, cast=dict)
     event = task_opts.get('event')
+    event = convert_value(event, cast=int, none_allowed=True)
     volume = task_opts.get('volume')
     if volume is not None:
         volume = lookup_name('volume', volume, strict=True)['name']
@@ -2853,6 +2903,7 @@ def dist_repo_init(tag, keys, task_opts):
         os.symlink(relpath, basedir)
     # handle comps
     if task_opts.get('comps'):
+        check_value_type(task_opts['comps'], cast=str)
         groupsdir = joinpath(repodir, 'groups')
         koji.ensuredir(groupsdir)
         shutil.copyfile(joinpath(koji.pathinfo.work(),
@@ -2877,6 +2928,7 @@ def dist_repo_init(tag, keys, task_opts):
 
 def repo_set_state(repo_id, state, check=True):
     """Set repo state"""
+    repo_id = convert_value(repo_id, cast=int)
     if check:
         # The repo states are sequential, going backwards makes no sense
         q = """SELECT state FROM repo WHERE id = %(repo_id)s FOR UPDATE"""
@@ -2934,6 +2986,7 @@ def repo_delete(repo_id):
     """Attempt to mark repo deleted, return number of references
 
     If the number of references is nonzero, no change is made"""
+    repo_id = convert_value(repo_id, cast=int)
     # get a row lock on the repo
     q = """SELECT state FROM repo WHERE id = %(repo_id)s FOR UPDATE"""
     _singleValue(q, locals())
@@ -3235,14 +3288,12 @@ def name_or_id_clause(table, info):
     """
     if isinstance(info, dict):
         if 'id' in info:
-            try:
-                info = int(info['id'])
-            except (ValueError, TypeError):
-                raise koji.ParameterError('Invalid name or id value: %r' % info)
+            info = convert_value(info['id'], cast=int,
+                                 message=fr"Invalid name or id value: {info}")
         elif 'name' in info:
             info = info['name']
         else:
-            raise koji.ParameterError('Invalid name or id value: %r' % info)
+            raise koji.ParameterError(fr'Invalid name or id value: {info}')
     if isinstance(info, int):
         clause = f"({table}.id = %({table}_id)s)"
         values = {f"{table}_id": info}
@@ -3250,7 +3301,7 @@ def name_or_id_clause(table, info):
         clause = f"({table}.name = %({table}_name)s)"
         values = {f"{table}_name": info}
     else:
-        raise koji.ParameterError('Invalid name or id value: %r' % info)
+        raise koji.ParameterError(fr"Invalid name or id value: {info}")
 
     return clause, values
 
@@ -3557,6 +3608,10 @@ def _edit_tag(tagInfo, **kwargs):
     if not context.opts.get('EnableMaven') \
             and dslice(kwargs, ['maven_support', 'maven_include_all'], strict=False):
         raise koji.GenericError("Maven support not enabled")
+    if kwargs.get('remove_extra'):
+        check_value_type(kwargs['remove_extra'], cast=list, none_allowed=True)
+    if kwargs.get('block_extra'):
+        check_value_type(kwargs['block_extra'], cast=list, none_allowed=True)
 
     tag = get_tag(tagInfo, strict=True)
     if 'perm' in kwargs and 'perm_id' not in kwargs:
@@ -3882,6 +3937,8 @@ def add_external_repo_to_tag(tag_info, repo_info, priority, merge_mode='koji', a
     if arches is not None:
         arches = koji.parse_arches(arches, strict=True)
 
+    priority = convert_value(priority, cast=int)
+
     tag_repos = get_tag_external_repos(tag_info=tag_id)
     if [tr for tr in tag_repos if tr['external_repo_id'] == repo_id]:
         raise koji.GenericError('tag %s already associated with external repo %s' %
@@ -4442,8 +4499,7 @@ def get_next_release(build_info, incr=1):
     :raises: BuildError if the latest build uses a release value that Koji
              does not know how to increment.
     """
-    if not isinstance(incr, int):
-        raise koji.ParameterError("incr parameter must be an integer")
+    incr = convert_value(incr, cast=int, message='incr parameter must be an integer')
     values = {
         'name': build_info['name'],
         'version': build_info['version'],
@@ -5522,6 +5578,10 @@ def edit_host(hostInfo, **kw):
     if not changes:
         return False
 
+    for change in changes:
+        if change in ['description', 'comment', 'arches']:
+            check_value_type(kw[change], cast=str)
+
     update = UpdateProcessor('host_config', values=host, clauses=['host_id = %(id)i'])
     update.make_revoke()
     update.execute()
@@ -5675,7 +5735,7 @@ def query_buildroots(hostID=None, tagID=None, state=None, rpmID=None, archiveID=
 
 def get_buildroot(buildrootID, strict=False):
     """Return information about a buildroot.  buildrootID must be an int ID."""
-
+    buildrootID = convert_value(buildrootID, cast=int)
     result = query_buildroots(buildrootID=buildrootID)
     if len(result) == 0:
         if strict:
@@ -5741,6 +5801,7 @@ def list_channels(hostID=None, event=None, enabled=None):
 
 
 def new_package(name, strict=True):
+    verify_name_internal(name)
     c = context.cnx.cursor()
     # TODO - table lock?
     # check for existing
@@ -6171,6 +6232,10 @@ def import_build(srpm, rpms, brmap=None, task_id=None, build_id=None, logs=None)
     """
     if brmap is None:
         brmap = {}
+    else:
+        check_value_type(brmap, cast=dict)
+    check_value_type(srpm, cast=str)
+    check_value_type(rpms, cast=list)
     koji.plugin.run_callbacks('preImport', type='build', srpm=srpm, rpms=rpms, brmap=brmap,
                               task_id=task_id, build_id=build_id, build=None, logs=logs)
     uploadpath = koji.pathinfo.work()
@@ -6536,7 +6601,6 @@ class CG_Importer(object):
 
     def get_metadata(self, metadata, directory):
         """Get the metadata from the args"""
-
         if isinstance(metadata, dict):
             self.metadata = metadata
             try:
@@ -6548,8 +6612,7 @@ class CG_Importer(object):
         if metadata is None:
             # default to looking for uploaded file
             metadata = 'metadata.json'
-        if not isinstance(metadata, str):
-            raise koji.GenericError("Invalid type for metadata value: %r" % type(metadata))
+        check_value_type(metadata, cast=str)
         if metadata.endswith('.json'):
             # handle uploaded metadata
             workdir = koji.pathinfo.work()
@@ -7324,8 +7387,7 @@ def get_archive_type(filename=None, type_name=None, type_id=None, strict=False):
     elif type_name:
         return _get_archive_type_by_name(type_name, strict)
     elif filename:
-        # we handle that below
-        pass
+        check_value_type(filename, cast=str)
     else:
         raise koji.GenericError('one of filename, type_name, or type_id must be specified')
 
@@ -7365,6 +7427,8 @@ def add_archive_type(name, description, extensions):
     """
     context.session.assertPerm('admin')
     verify_name_internal(name)
+    check_value_type(description, cast=str)
+    check_value_type(extensions, cast=str)
     data = {'name': name,
             'description': description,
             'extensions': extensions,
@@ -8774,6 +8838,9 @@ def tag_notification(is_successful, tag_id, from_id, build_id, user_id, ignore_s
                      failure_msg=''):
     if context.opts.get('DisableNotifications'):
         return
+    if user_id:
+        if not isinstance(user_id, int):
+            user_id = get_user(user_id, strict=True)
     if is_successful:
         state = koji.BUILD_STATES['COMPLETE']
     else:
@@ -10320,6 +10387,7 @@ def policy_data_from_task_args(method, arglist):
             # newRepo has a 'src' parameter that means something else
             break
         if k in params:
+            check_value_type(params[k], cast=str)
             policy_data['source'] = params.get(k)
             break
     # parameters that indicate build target
@@ -10482,8 +10550,10 @@ class RootExports(object):
         context.session.assertPerm('host')
         if options is None:
             args = []
-        else:
+        elif isinstance(options, dict):
             args = [options]
+        else:
+            raise koji.ParameterError('Invalid type of options: %s' % type(options))
         return make_task('restartHosts', args, priority=priority)
 
     def build(self, src, target, opts=None, priority=None, channel=None):
@@ -10499,6 +10569,7 @@ class RootExports(object):
         if not opts:
             opts = {}
         taskOpts = {}
+        check_value_type(src, cast=str)
         if priority:
             if priority < 0:
                 if not context.session.hasPerm('admin'):
@@ -10522,6 +10593,7 @@ class RootExports(object):
         Returns a list of all the dependent task ids
         """
         context.session.assertLogin()
+        check_value_type(srcs, cast=list)
         if not opts:
             opts = {}
         taskOpts = {}
@@ -10550,6 +10622,7 @@ class RootExports(object):
         context.session.assertLogin()
         if not context.opts.get('EnableMaven'):
             raise koji.GenericError("Maven support not enabled")
+        check_value_type(url, cast=str)
         if not opts:
             opts = {}
         taskOpts = {}
@@ -10586,13 +10659,13 @@ class RootExports(object):
         if not opts:
             opts = {}
 
+        check_value_type(url, cast=str)
+
         build = self.getBuild(build, strict=True)
         if list_rpms(build['id']) and not (opts.get('scratch') or opts.get('create_build')):
             raise koji.PreBuildError('wrapper rpms for %s have already been built' %
                                      koji.buildLabel(build))
-        build_target = self.getBuildTarget(target)
-        if not build_target:
-            raise koji.PreBuildError('no such build target: %s' % target)
+        build_target = self.getBuildTarget(target, strict=True)
         build_tag = self.getTag(build_target['build_tag'], strict=True)
         repo_info = self.getRepo(build_tag['id'])
         if not repo_info:
@@ -10602,7 +10675,8 @@ class RootExports(object):
         taskOpts = {}
         if priority:
             taskOpts['priority'] = koji.PRIO_DEFAULT + priority
-        taskOpts['channel'] = channel
+        if channel:
+            taskOpts['channel'] = channel
 
         return make_task('wrapperRPM', [url, build_target, build, None, opts], **taskOpts)
 
@@ -10621,6 +10695,7 @@ class RootExports(object):
         context.session.assertLogin()
         if not context.opts.get('EnableMaven'):
             raise koji.GenericError("Maven support not enabled")
+        check_value_type(builds, cast=list)
         taskOpts = {}
         if priority:
             if priority < 0:
@@ -10650,7 +10725,9 @@ class RootExports(object):
         context.session.assertLogin()
         if not context.opts.get('EnableWin'):
             raise koji.GenericError("Windows support not enabled")
-        targ_info = self.getBuildTarget(target)
+        check_value_type(vm, cast=str)
+        check_value_type(url, cast=str)
+        targ_info = get_build_target(target, strict=True)
         policy_data = {'vm_name': vm,
                        'tag': targ_info['dest_tag']}
         assert_policy('vm', policy_data)
@@ -10676,6 +10753,8 @@ class RootExports(object):
 
         if img_type not in ('livecd', 'appliance', 'livemedia'):
             raise koji.GenericError('Unrecognized image type: %s' % img_type)
+        for i in [name, ksfile, version]:
+            check_value_type(i, cast=str)
 
         context.session.assertPerm(img_type)
 
@@ -10723,6 +10802,8 @@ class RootExports(object):
         """
         Create an image using a kickstart file and group package list.
         """
+        for i in [name, inst_tree, version]:
+            check_value_type(i, cast=str)
         context.session.assertPerm('image')
         taskOpts = {'channel': 'image'}
         if priority:
@@ -10986,6 +11067,9 @@ class RootExports(object):
     def downloadTaskOutput(self, taskID, fileName, offset=0, size=-1, volume=None):
         """Download the file with the given name, generated by the task with the
         given ID."""
+        size = convert_value(size, cast=int)
+        if volume:
+            volume = self.getVolume(volume, strict=True)['name']
         if '..' in fileName:
             raise koji.GenericError('Invalid file name: %s' % fileName)
         filePath = '%s/%s/%s' % (koji.pathinfo.work(volume),
@@ -12493,6 +12577,7 @@ class RootExports(object):
             verify_name_internal(permission)
         if description is not None and not create:
             raise koji.GenericError('Description should be specified only with create.')
+        check_value_type(description, cast=str, none_allowed=True)
         user_id = get_user(userinfo, strict=True)['id']
         perm = lookup_perm(permission, strict=(not create), create=create)
         perm_id = perm['id']
@@ -12526,6 +12611,7 @@ class RootExports(object):
     def editPermission(self, permission, description):
         """Edit a permission description"""
         context.session.assertPerm('admin')
+        check_value_type(description, cast=str)
         perm = lookup_perm(permission, strict=True)
         perm_id = perm['id']
         update = UpdateProcessor('permissions', clauses=['id=%(perm_id)i'],
@@ -12550,22 +12636,18 @@ class RootExports(object):
             raise koji.GenericError('user already exists: %s' % username)
         if krb_principal and get_user_by_krb_principal(krb_principal):
             raise koji.GenericError(
-                'user with this Kerberos principal already exists: %s'
-                % krb_principal)
-
-        return context.session.createUser(username, status=status,
-                                          krb_principal=krb_principal)
+                'user with this Kerberos principal already exists: %s' % krb_principal)
+        status = convert_value(status, cast=int, none_allowed=True)
+        return context.session.createUser(username, status=status, krb_principal=krb_principal)
 
     def addUserKrbPrincipal(self, user, krb_principal):
         """Add a Kerberos principal for user"""
         context.session.assertPerm('admin')
         userinfo = get_user(user, strict=True)
-        if not krb_principal:
-            raise koji.GenericError('krb_principal must be specified')
+        verify_name_user(krb=krb_principal)
         if get_user_by_krb_principal(krb_principal):
             raise koji.GenericError(
-                'user with this Kerberos principal already exists: %s'
-                % krb_principal)
+                'user with this Kerberos principal already exists: %s' % krb_principal)
         return context.session.setKrbPrincipal(userinfo['name'], krb_principal)
 
     def removeUserKrbPrincipal(self, user, krb_principal):
@@ -13036,6 +13118,7 @@ class RootExports(object):
         # builder user can already exist, if host tried to log in before adding into db
         userinfo = {'name': hostname}
         if krb_principal:
+            verify_name_user(krb=krb_principal)
             userinfo['krb_principal'] = krb_principal
         user = get_user(userInfo=userinfo)
         if user:
@@ -13757,6 +13840,7 @@ class BuildRoot(object):
         self.is_standard = True
 
     def new(self, host, repo, arch, task_id=None, ctype='chroot'):
+        arch = koji.parse_arches(arch, strict=True, allow_none=True)
         state = koji.BR_STATES['INIT']
         br_id = _singleValue("SELECT nextval('buildroot_id_seq')", strict=True)
         insert = InsertProcessor('buildroot', data={'id': br_id})
@@ -14016,7 +14100,7 @@ class Host(object):
 
     def taskSetWait(self, parent, tasks):
         """Mark task waiting and subtasks awaited"""
-
+        check_value_type(tasks, cast=list, none_allowed=True)
         # mark parent as waiting
         update = UpdateProcessor('task', clauses=['id=%(parent)s'], values=locals())
         update.set(waiting=True)
@@ -14134,6 +14218,7 @@ class Host(object):
 
     def updateHost(self, task_load, ready):
         host_data = get_host(self.id)
+        task_load = convert_value(task_load, cast=float)
         if task_load != host_data['task_load'] or ready != host_data['ready']:
             c = context.cnx.cursor()
             id = self.id
@@ -14416,6 +14501,7 @@ class HostExports(object):
 
     def moveImageBuildToScratch(self, task_id, results):
         """move a completed image scratch build into place"""
+        check_value_type(results, cast=dict)
         host = Host()
         host.verify()
         task = Task(task_id)
@@ -14861,6 +14947,8 @@ class HostExports(object):
         pkg_id = build['package_id']
         tag_id = get_tag(tag, strict=True)['id']
         user_id = task.getOwner()
+        if not isinstance(fromtag, str):
+            tag = get_tag(tag, strict=True)['name']
         policy_data = {'tag': tag, 'build': build, 'fromtag': fromtag}
         policy_data['user_id'] = user_id
         if fromtag is None:
@@ -15156,6 +15244,7 @@ class HostExports(object):
         host = Host()
         host.verify()
         rinfo = repo_info(repo_id, strict=True)
+        check_value_type(data, cast=dict)
         koji.plugin.run_callbacks('preRepoDone', repo=rinfo, data=data, expire=expire)
         if rinfo['state'] != koji.REPO_INIT:
             raise koji.GenericError("Repo %(id)s not in INIT state (got %(state)s)" % rinfo)