#2207 frontend: restrict the CoprDir names to <copr>:custom:<suffix>
Merged 2 months ago by praiskup. Opened 3 months ago by praiskup.
copr/ praiskup/copr custom-dirs-only  into  main

@@ -567,7 +567,7 @@ 

          return coprdir

  

      @classmethod

-     def get_or_create(cls, copr, dirname):

+     def get_or_create(cls, copr, dirname, trusted_caller=False):

          """

          Create a CoprDir on-demand, e.g. before pull-request builds is

          submitted.  We don't create the "main" CoprDirs here (those are created
@@ -583,6 +583,17 @@ 

                  copr.name,

              ))

  

+         if not trusted_caller:

+             if not dirname.startswith(copr.name+":custom:"):

+                 raise exceptions.BadRequest(

+                     f"Please use directory format {copr.name}:custom:<SUFFIX_OF_CHOICE>"

+                 )

+ 

+         if not all(x.isalnum() for x in dirname.split(":")[1:]):

if dirname.replace(":", "").isalnum(): might be more readable

Ah, scratch that.
It won't find any problems with dirnames ending with :

+             raise exceptions.BadRequest(

+                 f"Wrong directory '{dirname}' specified.  Directory name can "

+                 "consist of alpha-numeric strings separated by colons.")

+ 

          copr_dir = models.CoprDir(name=dirname, copr=copr, main=False)

          ActionsLogic.send_createrepo(copr, dirnames=[dirname])

          db.session.add(copr_dir)

@@ -259,7 +259,8 @@ 

  

                  if event_info.object_type == 'pull-request':

                      dirname = pkg.copr.name + ':pr:' + str(event_info.object_id)

-                     copr_dir = CoprDirsLogic.get_or_create(pkg.copr, dirname)

+                     copr_dir = CoprDirsLogic.get_or_create(pkg.copr, dirname,

+                                                            trusted_caller=True)

                      update_callback = 'pagure_flag_pull_request'

                      scm_object_url = os.path.join(base_url, event_info.project_url_path,

                                                    'c', str(event_info.end_commit))

@@ -81,3 +81,20 @@ 

          assert latest_build["project_dirname"] == "test"

          latest_build = packages[1]["builds"]["latest"]

          assert latest_build["project_dirname"] == "test:custom:subdir"

+ 

+ 

+     @TransactionDecorator("u1")

+     @pytest.mark.usefixtures("f_users", "f_users_api", "f_coprs", "f_builds",

+                              "f_mock_chroots", "f_other_distgit", "f_db")

+     def test_custom_dir_validation(self):

+         self.web_ui.new_project("test", ["fedora-rawhide-i386"])

+         self.web_ui.create_distgit_package("test", "copr-cli")

+         # succeeds

+         assert self.api3.rebuild_package("test:custom:subdir", "copr-cli").status_code == 200

+         assert self.api3.rebuild_package("test:custom:123", "copr-cli").status_code  == 200

+         assert self.api3.rebuild_package("test:custom:žluťoučký", "copr-cli").status_code  == 200

+         assert self.api3.rebuild_package("test:custom:", "copr-cli").status_code == 400

+         assert self.api3.rebuild_package("test:custom:.", "copr-cli").status_code == 400

+         assert self.api3.rebuild_package("test:custom:@", "copr-cli").status_code == 400

+         # This can only be created by pagure-events.py code for now.

+         assert self.api3.rebuild_package("test:pr:13", "copr-cli").status_code == 400

Non-"custom" directories are forbidden for now, pull-request directories
can be crated by pagure-events only, and parts (between colons) can only
be alphanumeric, non-empty strings.

Relates: #2175

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

rebased onto c8998401d8a22c0b21caba751b5f18567144df7f

3 months ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

if dirname.replace(":", "").isalnum(): might be more readable

Ah, scratch that.
It won't find any problems with dirnames ending with :

Maybe just addressing the Zuul error but otherwise +1

I can't find an obvious solution to the pylint problem?

rebased onto f099194c93f080ac6c57569b3bcd0352ef78f512

3 months ago

This is pylint-friendly, but I don't really think it is better (at least we have to evaluate the whole list even to just get the negative result to raise the exception).

Build succeeded.

rebased onto 81a4718d5167e8c08416663361dfc3f3930aa0ae

3 months ago

rebased onto 8ceebf5

3 months ago

Build succeeded.

1 new commit added

  • zuul: use newer Fedora VM
3 months ago

Build succeeded.

1 new commit added

  • frontend: restrict the CoprDir names to <copr>:custom:<suffix>
3 months ago

Build succeeded.

Weird, pylint seems to like this variant now?

Strange ... I don't quite understand why it had a problem with it in the first place.
Anyway, this is +1

Pull-Request has been merged by praiskup

2 months ago