diff --git a/actions/storage b/actions/storage index a3a11ef1e..508dc6d11 100755 --- a/actions/storage +++ b/actions/storage @@ -333,6 +333,9 @@ def subcommand_usage_info(_): def subcommand_validate_directory(arguments): """Validate a directory""" + if os.geteuid() == 0: + raise RuntimeError('You must not be root to run this command') + directory = arguments.path def part_exists(path): @@ -349,14 +352,15 @@ def subcommand_validate_directory(arguments): if not os.path.exists(directory): # doesn't exist print('ValidationError: 1') + return if not os.path.isdir(directory): # is not a directory print('ValidationError: 2') - if not os.access(directory, os.R_OK): + elif not os.access(directory, os.R_OK): # is not readable print('ValidationError: 3') - if arguments.check_writable or arguments.check_creatable: + elif arguments.check_writable or arguments.check_creatable: if not os.access(directory, os.W_OK): # is not writable print('ValidationError: 4') diff --git a/plinth/modules/deluge/forms.py b/plinth/modules/deluge/forms.py index e85dc8792..a969cb301 100644 --- a/plinth/modules/deluge/forms.py +++ b/plinth/modules/deluge/forms.py @@ -18,5 +18,5 @@ class DelugeForm(DirectorySelectForm): check_creatable=True) super(DelugeForm, self).__init__( title=_('Download directory'), - default='/var/lib/deluged/Downloads/', validator=validator, *args, + default='/var/lib/deluged/Downloads', validator=validator, *args, **kw) diff --git a/plinth/modules/deluge/views.py b/plinth/modules/deluge/views.py index 78576fbae..4547d6ab7 100644 --- a/plinth/modules/deluge/views.py +++ b/plinth/modules/deluge/views.py @@ -4,7 +4,6 @@ Django views for Deluge. """ import json -import os from django.contrib import messages from django.utils.translation import ugettext as _ @@ -25,8 +24,7 @@ class DelugeAppView(views.AppView): status = super().get_initial() configuration = json.loads( actions.superuser_run('deluge', ['get-configuration'])) - status['storage_path'] = os.path.normpath( - configuration['download_location']) + status['storage_path'] = configuration['download_location'] return status def form_valid(self, form): diff --git a/plinth/modules/storage/forms.py b/plinth/modules/storage/forms.py index 89501529f..d871dd3b5 100644 --- a/plinth/modules/storage/forms.py +++ b/plinth/modules/storage/forms.py @@ -103,7 +103,7 @@ class DirectorySelectForm(AppForm): if title: self.fields['storage_dir'].label = title self.validator = validator - self.default = os.path.normpath(default) + self.default = default self.set_form_data() def clean(self): diff --git a/plinth/modules/storage/tests/test_storage.py b/plinth/modules/storage/tests/test_storage.py index 60c01192f..c277ef762 100644 --- a/plinth/modules/storage/tests/test_storage.py +++ b/plinth/modules/storage/tests/test_storage.py @@ -250,45 +250,25 @@ class TestActions: assert output == error @pytest.mark.usefixtures('needs_not_root') - @pytest.mark.parametrize('directory', [{ - 'path': '/missing', - 'error': '1' - }, { - 'path': '/etc/os-release', - 'error': '2' - }, { - 'path': '/root', - 'error': '3' - }, { - 'path': '/', - 'error': '' - }]) - def test_validate_directory(self, directory): + @pytest.mark.parametrize('path,error', [('/missing', '1'), + ('/etc/os-release', '2'), + ('/root', '3'), ('/', ''), + ('/etc/..', '')]) + def test_validate_directory(self, path, error): """Test that directory validation returns expected output.""" - self.assert_validate_directory(directory['path'], directory['error']) + self.assert_validate_directory(path, error) @pytest.mark.usefixtures('needs_not_root') - @pytest.mark.parametrize('directory', [{ - 'path': '/', - 'error': '4' - }, { - 'path': '/tmp', - 'error': '' - }]) - def test_validate_directory_writable(self, directory): + @pytest.mark.parametrize('path,error', [('/', '4'), ('/tmp', '')]) + def test_validate_directory_writable(self, path, error): """Test that directory writable validation returns expected output.""" - self.assert_validate_directory(directory['path'], directory['error'], - check_writable=True) + self.assert_validate_directory(path, error, check_writable=True) @pytest.mark.usefixtures('needs_not_root') - @pytest.mark.parametrize('directory', [{ - 'path': '/var/lib/plinth_storage_test_not_exists', - 'error': '4' - }, { - 'path': '/tmp/plint_storage_test_not_exists', - 'error': '' - }]) - def test_validate_directory_creatable(self, directory): + @pytest.mark.parametrize( + 'path,error', [('/var/lib/plinth_storage_test_not_exists', '4'), + ('/tmp/plint_storage_test_not_exists', ''), + ('/var/../tmp/plint_storage_test_not_exists', '')]) + def test_validate_directory_creatable(self, path, error): """Test that directory creatable validation returns expected output.""" - self.assert_validate_directory(directory['path'], directory['error'], - check_creatable=True) + self.assert_validate_directory(path, error, check_creatable=True) diff --git a/plinth/modules/transmission/forms.py b/plinth/modules/transmission/forms.py index 9eb059eeb..95f251a62 100644 --- a/plinth/modules/transmission/forms.py +++ b/plinth/modules/transmission/forms.py @@ -18,5 +18,5 @@ class TransmissionForm(DirectorySelectForm): username=reserved_usernames[0], check_writable=True) super(TransmissionForm, self).__init__( title=_('Download directory'), - default='/var/lib/transmission-daemon/downloads/', + default='/var/lib/transmission-daemon/downloads', validator=validator, *args, **kw) diff --git a/plinth/modules/transmission/views.py b/plinth/modules/transmission/views.py index b0a0bde59..84319ecfb 100644 --- a/plinth/modules/transmission/views.py +++ b/plinth/modules/transmission/views.py @@ -5,7 +5,6 @@ FreedomBox app for configuring Transmission Server. import json import logging -import os import socket from django.contrib import messages @@ -29,8 +28,7 @@ class TransmissionAppView(views.AppView): configuration = actions.superuser_run('transmission', ['get-configuration']) configuration = json.loads(configuration) - status['storage_path'] = os.path.normpath( - configuration['download-dir']) + status['storage_path'] = configuration['download-dir'] status['hostname'] = socket.gethostname() return status