PR#2207: frontend: restrict the CoprDir names to <copr>:custom:<suffix> - copr/copr

copr / copr

#2207 frontend: restrict the CoprDir names to <copr>:custom:<suffix>

Merged 2 years ago by praiskup. Opened 2 years ago by praiskup.

Unknown source custom-dirs-only into main

frontend: restrict the CoprDir names to <copr>:custom:<suffix>

Pavel Raiskup • 2 years ago

8ceebf5

frontend/coprs_frontend/coprs/logic/coprs_logic.py

file modified

+12 -1

		`@@ -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:]):`
frostyx commented 2 years ago `if dirname.replace(":", "").isalnum():` might be more readable
frostyx commented 2 years ago 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)`

frontend/coprs_frontend/pagure_events.py

file modified

+2 -1

		`@@ -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))`

frontend/coprs_frontend/tests/test_apiv3/test_coprdirs.py

file modified

+17

		`@@ -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`

praiskup commented 2 years ago

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

zuul commented 2 years ago

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

csdiff-pylint : FAILURE in 1m 13s

rebased onto c8998401d8a22c0b21caba751b5f18567144df7f

2 years ago

zuul commented 2 years ago

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

csdiff-pylint : FAILURE in 1m 41s

frostyx commented on line 20 of frontend/coprs_frontend/coprs/logic/coprs_logic.py 2 years ago

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

frostyx commented on line 20 of frontend/coprs_frontend/coprs/logic/coprs_logic.py 2 years ago

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

frostyx commented 2 years ago

Maybe just addressing the Zuul error but otherwise +1

praiskup commented 2 years ago

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

Edited 2 years ago by praiskup

rebased onto f099194c93f080ac6c57569b3bcd0352ef78f512

2 years ago

praiskup commented 2 years 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).

zuul commented 2 years ago

Build succeeded.