mirror of
https://github.com/freedombox/FreedomBox.git
synced 2026-01-21 07:55:00 +00:00
actions: Drop unused superuser_run and related methods
Tests: - All tests in patch series have been done with this patch applied - Unit tests pass Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org> Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
This commit is contained in:
Sunil Mohan Adapa
James Valleroy
parent
0bda4843a7
commit
5c5fc9eb61
20
conftest.py
20
conftest.py
@ -148,26 +148,6 @@ def fixture_actions_module(request):
|
||||
return module
|
||||
|
||||
|
||||
@pytest.fixture(name='call_action')
|
||||
def fixture_call_action(request, capsys, actions_module):
|
||||
"""Run actions with custom root path."""
|
||||
|
||||
actions_name = getattr(request.module, 'actions_name')
|
||||
|
||||
def _call_action(*args, **_kwargs):
|
||||
if isinstance(args[0], list):
|
||||
argv = [actions_name] + args[0] # Command line style usage
|
||||
else:
|
||||
argv = [args[0]] + args[1] # superuser_run style usage
|
||||
|
||||
with patch('argparse._sys.argv', argv):
|
||||
actions_module.main()
|
||||
captured = capsys.readouterr()
|
||||
return captured.out
|
||||
|
||||
return _call_action
|
||||
|
||||
|
||||
@pytest.fixture(name='mock_privileged')
|
||||
def fixture_mock_privileged(request):
|
||||
"""Mock the privileged decorator to nullify its effects."""
|
||||
|
||||
@ -13,4 +13,4 @@ else. These actions are also directly usable by a skilled administrator.
|
||||
The following documentation for the ``actions`` module.
|
||||
|
||||
.. automodule:: plinth.actions
|
||||
:members: run, superuser_run, run_as_user, _run, privileged
|
||||
:members: privileged
|
||||
|
||||
@ -1,79 +1,5 @@
|
||||
# SPDX-License-Identifier: AGPL-3.0-or-later
|
||||
"""Run specified actions.
|
||||
|
||||
Actions run commands with this contract (version 1.1):
|
||||
|
||||
1. (promise) Super-user actions run as root. Normal actions do not.
|
||||
|
||||
2. (promise) The actions directory can't be changed at run time.
|
||||
|
||||
This guarantees that we can only select from the correct set of actions.
|
||||
|
||||
3. (restriction) Only specifically allowed actions can run.
|
||||
|
||||
A. Scripts in a directory above the actions directory can't be run.
|
||||
|
||||
Arguments (and options) can't coerce the system to run actions in
|
||||
directories above the actions directory.
|
||||
|
||||
Arguments that fail this validation will raise a ValueError.
|
||||
|
||||
B. Scripts in a directory beneath the actions directory can't be run.
|
||||
|
||||
Arguments (and options) can't coerce the system to run actions in
|
||||
sub-directories of the actions directory.
|
||||
|
||||
(An important side-effect of this is that the system will not try to
|
||||
follow symlinks to other action directories.)
|
||||
|
||||
Arguments that fail this validation will raise a ValueError.
|
||||
|
||||
C. Only one action can be called at a time.
|
||||
|
||||
This prevents us from appending multiple (unexpected) actions to
|
||||
the call. Any special shell characters in the action name will
|
||||
simply be treated as the action itself when trying to search for
|
||||
an action. The action will then not be found.
|
||||
|
||||
$ action="echo '$options'; echo 'oops'"
|
||||
$ options="hi"
|
||||
$ $action # oops, the file system is gone!
|
||||
|
||||
Arguments that fail this validation will raise a ValueError.
|
||||
|
||||
D. Options can't be used to run other actions:
|
||||
|
||||
$ action="echo '$options'"
|
||||
$ options="hi'; rm -rf /;'"
|
||||
$ $action # oops, the file system is gone!
|
||||
|
||||
Any option that tries to include special shell characters will
|
||||
simply be treated as an option with special characters and will
|
||||
not be interpreted by the shell.
|
||||
|
||||
Any call wishing to include special shell characters in options
|
||||
list does not need to escape them. They are taken care of. The
|
||||
option string is passed to the action exactly as it is passed in.
|
||||
|
||||
E. Actions must exist in the actions directory.
|
||||
|
||||
4. (promise) Options are passed as arguments to the action.
|
||||
|
||||
Options can be provided as a list or as a tuple.
|
||||
|
||||
5. (promise) Output is returned from the command. In case of an
|
||||
error, ActionError is raised with action, output and error strings
|
||||
as arguments.
|
||||
|
||||
6. (limitation) Providing the process with input is not possible.
|
||||
|
||||
Don't expect to give the process additional input after it's started. Any
|
||||
interaction with the spawned process must be carried out through some other
|
||||
method (maybe the process opens a socket, or something).
|
||||
|
||||
7. Option
|
||||
|
||||
"""
|
||||
"""Framework to run specified actions with elevated privileges."""
|
||||
|
||||
import functools
|
||||
import importlib
|
||||
@ -81,168 +7,14 @@ import inspect
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
import re
|
||||
import shlex
|
||||
import subprocess
|
||||
import threading
|
||||
|
||||
from plinth import cfg
|
||||
from plinth.errors import ActionError
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def run(action, options=None, input=None, run_in_background=False):
|
||||
"""Safely run a specific action as the current user.
|
||||
|
||||
See actions._run for more information.
|
||||
"""
|
||||
return _run(action, options, input, run_in_background, False)
|
||||
|
||||
|
||||
def superuser_run(action, options=None, input=None, run_in_background=False,
|
||||
log_error=True):
|
||||
"""Safely run a specific action as root.
|
||||
|
||||
See actions._run for more information.
|
||||
"""
|
||||
return _run(action, options, input, run_in_background, True,
|
||||
log_error=log_error)
|
||||
|
||||
|
||||
def run_as_user(action, options=None, input=None, run_in_background=False,
|
||||
become_user=None):
|
||||
"""Run a command as a different user.
|
||||
|
||||
If become_user is None, run as current user.
|
||||
"""
|
||||
return _run(action, options, input, run_in_background, False, become_user)
|
||||
|
||||
|
||||
def _run(action, options=None, input=None, run_in_background=False,
|
||||
run_as_root=False, become_user=None, log_error=True):
|
||||
"""Safely run a specific action as a normal user or root.
|
||||
|
||||
Actions are pulled from the actions directory.
|
||||
|
||||
- options are added to the action command.
|
||||
|
||||
- input: data (as bytes) that will be sent to the action command's stdin.
|
||||
|
||||
- run_in_background: run asynchronously or wait for the command to
|
||||
complete.
|
||||
|
||||
- run_as_root: execute the command through sudo.
|
||||
|
||||
"""
|
||||
if options is None:
|
||||
options = []
|
||||
|
||||
# Contract 3A and 3B: don't call anything outside of the actions directory.
|
||||
if os.sep in action:
|
||||
raise ValueError('Action cannot contain: ' + os.sep)
|
||||
|
||||
cmd = os.path.join(cfg.actions_dir, action)
|
||||
if not os.path.realpath(cmd).startswith(cfg.actions_dir):
|
||||
raise ValueError('Action has to be in directory %s' % cfg.actions_dir)
|
||||
|
||||
# Contract 3C: interpret shell escape sequences as literal file names.
|
||||
# Contract 3E: fail if the action doesn't exist or exists elsewhere.
|
||||
if not os.access(cmd, os.F_OK):
|
||||
raise ValueError('Action must exist in action directory.')
|
||||
|
||||
cmd = [cmd]
|
||||
|
||||
# Contract: 3C, 3D: don't allow shell special characters in
|
||||
# options be interpreted by the shell. When using
|
||||
# subprocess.Popen with list invocation and not shell invocation,
|
||||
# escaping is unnecessary as each argument is passed directly to
|
||||
# the command and not parsed by a shell.
|
||||
if options:
|
||||
if not isinstance(options, (list, tuple)):
|
||||
raise ValueError('Options must be list or tuple.')
|
||||
|
||||
cmd += list(options) # No escaping necessary
|
||||
|
||||
# Contract 1: commands can run via sudo.
|
||||
sudo_call = []
|
||||
if run_as_root:
|
||||
sudo_call = ['sudo', '-n']
|
||||
elif become_user:
|
||||
sudo_call = ['sudo', '-n', '-u', become_user]
|
||||
|
||||
if cfg.develop and sudo_call:
|
||||
# Passing 'env' does not work with sudo, so append the PYTHONPATH
|
||||
# as part of the command
|
||||
sudo_call += ['PYTHONPATH=%s' % cfg.file_root]
|
||||
|
||||
if sudo_call:
|
||||
cmd = sudo_call + cmd
|
||||
|
||||
_log_command(cmd)
|
||||
|
||||
# Contract 3C: don't interpret shell escape sequences.
|
||||
# Contract 5 (and 6-ish).
|
||||
kwargs = {
|
||||
'stdin': subprocess.PIPE,
|
||||
'stdout': subprocess.PIPE,
|
||||
'stderr': subprocess.PIPE,
|
||||
'shell': False,
|
||||
}
|
||||
if cfg.develop:
|
||||
# In development mode pass on local pythonpath to access Plinth
|
||||
kwargs['env'] = {'PYTHONPATH': cfg.file_root}
|
||||
|
||||
proc = subprocess.Popen(cmd, **kwargs)
|
||||
|
||||
if not run_in_background:
|
||||
output, error = proc.communicate(input=input)
|
||||
output, error = output.decode(), error.decode()
|
||||
if proc.returncode != 0:
|
||||
if log_error:
|
||||
logger.error('Error executing command - %s, %s, %s', cmd,
|
||||
output, error)
|
||||
raise ActionError(action, output, error)
|
||||
|
||||
return output
|
||||
|
||||
return proc
|
||||
|
||||
|
||||
def _log_command(cmd):
|
||||
"""Log a command with special pretty formatting to catch the eye."""
|
||||
cmd = list(cmd) # Make a copy of the command not to affect the original
|
||||
|
||||
prompt = '$'
|
||||
user = ''
|
||||
if cmd and cmd[0] == 'sudo':
|
||||
cmd = cmd[1:]
|
||||
prompt = '#'
|
||||
|
||||
# Drop -n argument to sudo
|
||||
if cmd and cmd[0] == '-n':
|
||||
cmd = cmd[1:]
|
||||
|
||||
# Capture username separately
|
||||
if len(cmd) > 1 and cmd[0] == '-u':
|
||||
prompt = '$'
|
||||
user = cmd[1]
|
||||
cmd = cmd[2:]
|
||||
|
||||
# Drop environmental variables set via sudo
|
||||
while cmd and re.match(r'.*=.*', cmd[0]):
|
||||
cmd = cmd[1:]
|
||||
|
||||
# Strip the command's prefix
|
||||
if cmd:
|
||||
cmd[0] = cmd[0].split('/')[-1]
|
||||
|
||||
# Shell escape and join command arguments
|
||||
cmd = ' '.join([shlex.quote(part) for part in cmd])
|
||||
|
||||
logger.info('%s%s %s', user, prompt, cmd)
|
||||
|
||||
|
||||
def privileged(func):
|
||||
"""Mark a method as allowed to be run as privileged method.
|
||||
|
||||
|
||||
@ -8,10 +8,6 @@ class PlinthError(Exception):
|
||||
"""Base class for all FreedomBox specific errors."""
|
||||
|
||||
|
||||
class ActionError(PlinthError):
|
||||
"""Use this error for exceptions when executing an action."""
|
||||
|
||||
|
||||
class PackageNotInstalledError(PlinthError):
|
||||
"""Could not complete module setup due to missing package."""
|
||||
|
||||
|
||||
@ -9,17 +9,13 @@ description of the expectations.
|
||||
|
||||
import json
|
||||
import os
|
||||
import pathlib
|
||||
import shutil
|
||||
import subprocess
|
||||
import tempfile
|
||||
from unittest.mock import Mock, call, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from plinth import cfg
|
||||
from plinth.actions import _log_command as log_command
|
||||
from plinth.actions import privileged, run, superuser_run
|
||||
from plinth.actions import privileged
|
||||
|
||||
|
||||
@pytest.fixture(name='popen')
|
||||
@ -44,160 +40,6 @@ def fixture_popen():
|
||||
yield popen
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def actions_test_setup(load_cfg):
|
||||
"""Setup a temporary directory for testing actions.
|
||||
|
||||
Copy system commands ``echo`` and ``id`` into actions directory during
|
||||
testing.
|
||||
|
||||
"""
|
||||
with tempfile.TemporaryDirectory() as tmp_directory:
|
||||
old_actions_dir = cfg.actions_dir
|
||||
cfg.actions_dir = str(tmp_directory)
|
||||
|
||||
actions_dir = pathlib.Path(__file__).parent / '../../actions'
|
||||
shutil.copy(str(actions_dir / 'test_path'), str(tmp_directory))
|
||||
shutil.copy('/bin/echo', str(tmp_directory))
|
||||
shutil.copy('/usr/bin/id', str(tmp_directory))
|
||||
|
||||
yield
|
||||
cfg.actions_dir = old_actions_dir
|
||||
|
||||
|
||||
def notest_run_as_root():
|
||||
"""1. Privileged actions run as root. """
|
||||
assert superuser_run('id', ['-ur'])[0].strip() == '0' # user 0 is root
|
||||
|
||||
|
||||
def test_breakout_actions_dir():
|
||||
"""2. The actions directory can't be changed at run time.
|
||||
|
||||
Can't currently be tested, as the actions directory is hardcoded.
|
||||
"""
|
||||
|
||||
|
||||
def test_breakout_up():
|
||||
"""3A. Users can't call actions above the actions directory.
|
||||
|
||||
Tests both a relative and a literal path.
|
||||
"""
|
||||
for action in ('../echo', '/bin/echo'):
|
||||
with pytest.raises(ValueError):
|
||||
run(action, ['hi'])
|
||||
|
||||
|
||||
def test_breakout_down():
|
||||
"""3B. Users can't call actions beneath the actions directory."""
|
||||
action = 'directory/echo'
|
||||
with pytest.raises(ValueError):
|
||||
superuser_run(action)
|
||||
|
||||
|
||||
def test_breakout_actions():
|
||||
"""3C. Actions can't be used to run other actions.
|
||||
|
||||
If multiple actions are specified, bail out.
|
||||
"""
|
||||
# Counting is safer than actual badness.
|
||||
actions = ('echo ""; echo $((1+1))', 'echo "" && echo $((1+1))',
|
||||
'echo "" || echo $((1+1))')
|
||||
options = ('good', '')
|
||||
|
||||
for action in actions:
|
||||
for option in options:
|
||||
with pytest.raises(ValueError):
|
||||
run(action, [option])
|
||||
|
||||
|
||||
def test_breakout_option_string():
|
||||
"""3D. Option strings can't be used to run other actions.
|
||||
|
||||
Verify that shell control characters aren't interpreted.
|
||||
"""
|
||||
options = ('; echo hello', '&& echo hello', '|| echo hello',
|
||||
'& echo hello', r'\; echo hello', '| echo hello',
|
||||
r':;!&\/$%@`"~#*(){}[]|+=')
|
||||
for option in options:
|
||||
output = run('echo', [option])
|
||||
output = output.rstrip('\n')
|
||||
assert option == output
|
||||
|
||||
|
||||
def test_breakout_option_list():
|
||||
"""3D. Option lists can't be used to run other actions.
|
||||
|
||||
Verify that shell control characters aren't interpreted in
|
||||
option lists.
|
||||
"""
|
||||
option_lists = (
|
||||
(';', 'echo', 'hello'),
|
||||
('&&', 'echo', 'hello'),
|
||||
('||', 'echo', 'hello'),
|
||||
('&', 'echo', 'hello'),
|
||||
(r'\;', 'echo'
|
||||
'hello'),
|
||||
('|', 'echo', 'hello'),
|
||||
('', 'echo', '', 'hello'), # Empty option argument
|
||||
tuple(r':;!&\/$%@`"~#*(){}[]|+='))
|
||||
for options in option_lists:
|
||||
output = run('echo', options)
|
||||
output = output.rstrip('\n')
|
||||
expected_output = ' '.join(options)
|
||||
assert output == expected_output
|
||||
|
||||
|
||||
def test_multiple_options_and_output():
|
||||
"""4. Multiple options can be provided as a list or as a tuple.
|
||||
|
||||
5. Output is returned from the command.
|
||||
"""
|
||||
options = '1 2 3 4 5 6 7 8 9'
|
||||
|
||||
output = run('echo', options.split())
|
||||
output = output.rstrip('\n')
|
||||
assert options == output
|
||||
|
||||
output = run('echo', tuple(options.split()))
|
||||
output = output.rstrip('\n')
|
||||
assert options == output
|
||||
|
||||
|
||||
@pytest.mark.usefixtures('develop_mode', 'needs_root')
|
||||
def test_action_path(monkeypatch):
|
||||
"""Test that in development mode, python action scripts get the
|
||||
correct PYTHONPATH"""
|
||||
monkeypatch.setitem(os.environ, 'PYTHONPATH', '')
|
||||
plinth_path = run('test_path').strip()
|
||||
su_plinth_path = superuser_run('test_path').strip()
|
||||
assert plinth_path.startswith(cfg.file_root)
|
||||
assert plinth_path == su_plinth_path
|
||||
|
||||
|
||||
@patch('plinth.actions.logger.info')
|
||||
@pytest.mark.parametrize('cmd,message', [
|
||||
[['ls'], '$ ls'],
|
||||
[['/bin/ls'], '$ ls'],
|
||||
[['ls', 'a', 'b', 'c'], '$ ls a b c'],
|
||||
[['ls', 'a b c'], "$ ls 'a b c'"],
|
||||
[['ls', 'a;'], "$ ls 'a;'"],
|
||||
[['sudo', 'ls'], '# ls'],
|
||||
[['sudo', '-n', 'ls'], '# ls'],
|
||||
[['sudo', '-u', 'tester', 'ls'], 'tester$ ls'],
|
||||
[['sudo', 'key1=value1', 'key2=value2', 'ls'], '# ls'],
|
||||
[[
|
||||
'sudo', '-n', 'PYTHONPATH=/vagrant', '/vagrant/actions/ejabberd',
|
||||
'add-domain', '--domainname', 'freedombox.local'
|
||||
], '# ejabberd add-domain --domainname freedombox.local'],
|
||||
])
|
||||
def test_log_command(logger, cmd, message):
|
||||
"""Test log messages for various action calls."""
|
||||
log_command(cmd)
|
||||
logger.assert_called_once()
|
||||
args = logger.call_args[0]
|
||||
assert message == args[0] % args[1:]
|
||||
|
||||
|
||||
def test_privileged_properties():
|
||||
"""Test that privileged decorator sets proper properties on the method."""
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user