Mercurial > kallithea
changeset 7296:caa482f8fb5f
repos: only allow api repo creation in existing groups
Fix problem with '../something' paths being allowed; '..' will always exist and
can't be created.
This also introduce a small API change: Repository groups must now exist before
repositories can be created. This makes the API more explicit and simpler.
This issue was found and reported by
Kacper Szurek
https://security.szurek.pl/
| author | Mads Kiilerich <mads@kiilerich.com> |
|---|---|
| date | Tue, 29 May 2018 12:25:41 +0200 |
| parents | d314edb04d11 |
| children | 552170092d06 |
| files | docs/api/api.rst kallithea/controllers/api/api.py kallithea/tests/api/api_base.py |
| diffstat | 3 files changed, 43 insertions(+), 23 deletions(-) [+] |
line wrap: on
line diff
--- a/docs/api/api.rst Tue May 29 12:25:40 2018 +0200 +++ b/docs/api/api.rst Tue May 29 12:25:41 2018 +0200 @@ -796,10 +796,12 @@ create_repo ^^^^^^^^^^^ -Create a repository. If the repository name contains "/", all needed repository -groups will be created. For example "foo/bar/baz" will create repository groups -"foo", "bar" (with "foo" as parent), and create "baz" repository with -"bar" as group. +Create a repository. If the repository name contains "/", the repository will be +created in the repository group indicated by that path. Any such repository +groups need to exist before calling this method, or the call will fail. +For example "foo/bar/baz" will create a repository "baz" inside the repository +group "bar" which itself is in a repository group "foo", but both "foo" and +"bar" already need to exist before calling this method. This command can only be executed using the api_key of a user with admin rights, or that of a regular user with create repository permission. Regular users cannot specify owner parameter.
--- a/kallithea/controllers/api/api.py Tue May 29 12:25:40 2018 +0200 +++ b/kallithea/controllers/api/api.py Tue May 29 12:25:41 2018 +0200 @@ -1350,9 +1350,9 @@ enable_downloads=Optional(False), copy_permissions=Optional(False)): """ - Creates a repository. If repository name contains "/", all needed repository - groups will be created. For example "foo/bar/baz" will create groups - "foo", "bar" (with "foo" as parent), and create "baz" repository with + Creates a repository. The repository name contains the full path, but the + parent repository group must exist. For example "foo/bar/baz" require the groups + "foo" and "bar" (with "foo" as parent), and create "baz" repository with "bar" as group. This command can be executed only using api_key belonging to user with admin rights or regular user that have create repository permission. Regular users cannot specify owner parameter @@ -1432,11 +1432,15 @@ copy_permissions = Optional.extract(copy_permissions) try: - repo_name_cleaned = repo_name.split('/')[-1] - # create structure of groups and return the last group - repo_group = map_groups(repo_name) + repo_name_parts = repo_name.split('/') + repo_group = None + if len(repo_name_parts) > 1: + group_name = '/'.join(repo_name_parts[:-1]) + repo_group = RepoGroup.get_by_group_name(group_name) + if repo_group is None: + raise JSONRPCError("repo group `%s` not found" % group_name) data = dict( - repo_name=repo_name_cleaned, + repo_name=repo_name_parts[-1], repo_name_full=repo_name, repo_type=repo_type, repo_description=description, @@ -1608,14 +1612,18 @@ owner = get_user_or_error(owner) try: - # create structure of groups and return the last group - group = map_groups(fork_name) - fork_base_name = fork_name.rsplit('/', 1)[-1] + fork_name_parts = fork_name.split('/') + repo_group = None + if len(fork_name_parts) > 1: + group_name = '/'.join(fork_name_parts[:-1]) + repo_group = RepoGroup.get_by_group_name(group_name) + if repo_group is None: + raise JSONRPCError("repo group `%s` not found" % group_name) form_data = dict( - repo_name=fork_base_name, + repo_name=fork_name_parts[-1], repo_name_full=fork_name, - repo_group=group, + repo_group=repo_group, repo_type=repo.repo_type, description=Optional.extract(description), private=Optional.extract(private),
--- a/kallithea/tests/api/api_base.py Tue May 29 12:25:40 2018 +0200 +++ b/kallithea/tests/api/api_base.py Tue May 29 12:25:41 2018 +0200 @@ -1065,6 +1065,20 @@ repo_group_name = u'my_gr' repo_name = u'%s/api-repo' % repo_group_name + # repo creation can no longer also create repo group + id_, params = _build_data(self.apikey, 'create_repo', + repo_name=repo_name, + owner=TEST_USER_ADMIN_LOGIN, + repo_type=self.REPO_TYPE,) + response = api_call(self, params) + expected = u'repo group `%s` not found' % repo_group_name + self._compare_error(id_, expected, given=response.body) + assert RepoModel().get_by_repo_name(repo_name) is None + + # create group before creating repo + rg = fixture.create_repo_group(repo_group_name) + Session().commit() + id_, params = _build_data(self.apikey, 'create_repo', repo_name=repo_name, owner=TEST_USER_ADMIN_LOGIN, @@ -1191,7 +1205,7 @@ self._compare_error(id_, expected, given=response.body) def test_api_create_repo_dot_dot(self): - # FIXME: it should only be possible to create repositories in existing repo groups - and '..' can't be used + # it is only possible to create repositories in existing repo groups - and '..' can't be used group_name = '%s/..' % TEST_REPO_GROUP repo_name = '%s/%s' % (group_name, 'could-be-outside') id_, params = _build_data(self.apikey, 'create_repo', @@ -1199,12 +1213,8 @@ owner=TEST_USER_ADMIN_LOGIN, repo_type=self.REPO_TYPE,) response = api_call(self, params) - expected = { - u'msg': u"Created new repository `%s`" % repo_name, - u'success': True, - u'task': None, - } - self._compare_ok(id_, expected, given=response.body) + expected = u'repo group `%s` not found' % group_name + self._compare_error(id_, expected, given=response.body) fixture.destroy_repo(repo_name) @mock.patch.object(RepoModel, 'create', crash)