gitweb: Validate repository name also in actions script

This prevents writing to an arbitrary directory if running actions script
as a root user.

- Included tests for invalid names

Signed-off-by: Veiko Aasa <veiko17@disroot.org>
[sunil@medhas.org Parametrize the test case]
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Sunil Mohan Adapa <sunil@medhas.org>

actions/gitweb

@@ -27,6 +27,7 @@ import shutil
 import subprocess
 
 from plinth import action_utils
+from plinth.modules.gitweb.forms import validate_repository
 from plinth.modules.gitweb.manifest import GIT_REPO_PATH
 
 
@@ -168,6 +169,8 @@ def _set_access_status(repo, status):
 
 def subcommand_rename_repo(arguments):
     """Rename a repository."""
+    validate_repository(arguments.oldname)
+    validate_repository(arguments.newname)
     oldpath = os.path.join(GIT_REPO_PATH, arguments.oldname + '.git')
     newpath = os.path.join(GIT_REPO_PATH, arguments.newname + '.git')
     os.rename(oldpath, newpath)
@@ -175,21 +178,25 @@ def subcommand_rename_repo(arguments):
 
 def subcommand_set_repo_description(arguments):
     """Set description of the repository."""
+    validate_repository(arguments.name)
     _set_repo_description(arguments.name, arguments.description)
 
 
 def subcommand_set_repo_owner(arguments):
     """Set repository's owner name."""
+    validate_repository(arguments.name)
     _set_repo_owner(arguments.name, arguments.owner)
 
 
 def subcommand_set_repo_access(arguments):
     """Set repository's access status."""
+    validate_repository(arguments.name)
     _set_access_status(arguments.name, arguments.access)
 
 
 def subcommand_repo_info(arguments):
     """Get information about repository."""
+    validate_repository(arguments.name)
     repo_path = os.path.join(GIT_REPO_PATH, arguments.name + '.git')
     if not os.path.exists(repo_path):
         raise RuntimeError('Repository not found')
@@ -203,6 +210,7 @@ def subcommand_repo_info(arguments):
 
 def subcommand_create_repo(arguments):
     """Create a new git repository."""
+    validate_repository(arguments.name)
     os.chdir(GIT_REPO_PATH)
     repo_name = arguments.name + '.git'
     subprocess.check_call(['git', 'init', '--bare', repo_name])
@@ -216,6 +224,7 @@ def subcommand_create_repo(arguments):
 
 def subcommand_delete_repo(arguments):
     """Delete a git repository."""
+    validate_repository(arguments.name)
     repo_path = os.path.join(GIT_REPO_PATH, arguments.name + '.git')
     shutil.rmtree(repo_path)
 

plinth/modules/gitweb/forms.py

@@ -18,6 +18,8 @@
 Django form for configuring Gitweb.
 """
 
+import re
+
 from django import forms
 from django.core.exceptions import ValidationError
 from django.utils.translation import ugettext_lazy as _
@@ -25,13 +27,26 @@ from django.utils.translation import ugettext_lazy as _
 from plinth.modules import gitweb
 
 
+def validate_repository(name):
+    """Validate a Git repository name."""
+
+    if not re.match(r'^[a-zA-Z0-9-._]+$', name):
+        raise ValidationError(_('Invalid repository name.'))
+
+    if name.startswith(('-', '.')):
+        raise ValidationError(_('Invalid repository name.'))
+
+    if name.endswith('.git.git'):
+        raise ValidationError(_('Invalid repository name.'))
+
+
 class EditRepoForm(forms.Form):
     """Form to create and edit a new repository."""
 
-    name = forms.RegexField(
+    name = forms.CharField(
         label=_('Name of the repository'),
         strip=True,
-        regex=r'^[a-zA-Z0-9-._]+$',
+        validators=[validate_repository],
         help_text=_(
             'An alpha-numeric string that uniquely identifies a repository.'),
     )
@@ -62,9 +77,6 @@ class EditRepoForm(forms.Form):
         if name.endswith('.git'):
             name = name[:-4]
 
-        if (not name) or name.startswith(('-', '.')):
-            raise ValidationError(_('Invalid repository name.'))
-
         for repo in gitweb.app.get_repo_list():
             if name == repo['name']:
                 raise ValidationError(

plinth/modules/gitweb/tests/test_gitweb.py

@@ -24,6 +24,7 @@ import pathlib
 from unittest.mock import patch
 
 import pytest
+from django.forms import ValidationError
 
 
 def _action_file():
@@ -91,3 +92,15 @@ def test_actions(call_action):
     call_action(['delete-repo', '--name', repo_renamed])
     with pytest.raises(RuntimeError, match='Repository not found'):
         call_action(['repo-info', '--name', repo_renamed])
+
+
+@pytest.mark.parametrize(
+    'name',
+    ['.Test-repo', 'Test-repo.git.git', '/root/Test-repo', 'Test-repö'])
+def test_action_create_repo_with_invalid_names(call_action, name):
+    """Test that creating repository with invalid names fails."""
+    with pytest.raises(ValidationError):
+        call_action([
+            'create-repo', '--name', name, '--description', '', '--owner', '',
+            '--keep-ownership'
+        ])