diff options
author | Mike Frysinger <vapier@google.com> | 2020-04-22 19:05:06 -0400 |
---|---|---|
committer | Mike Frysinger <vapier@google.com> | 2020-04-22 19:07:02 -0400 |
commit | 95222193d775362cf1a5820eb57884fdac9fb530 (patch) | |
tree | adb5b22f55eab495162b04d2baf5376726e62358 | |
parent | 2bbd47c729c0158812fad67ff3b9fc583b77ae2e (diff) | |
download | repohooks-95222193d775362cf1a5820eb57884fdac9fb530.tar.gz |
utils: sudo_run: delete
We were using this function in only one error case in one scenario
that shouldn't really happen. Expecting to get access to root in
general & invoking sudo is weird for tests, so don't even try to
support it. If we hit this situation, let's abort and make the
user clean things up.
Bug: None
Test: preupload unittests pass
Change-Id: Ie1ac33b477b46ddacf372b661f160421ec61049b
-rw-r--r-- | rh/utils.py | 66 | ||||
-rwxr-xr-x | rh/utils_unittest.py | 44 |
2 files changed, 1 insertions, 109 deletions
diff --git a/rh/utils.py b/rh/utils.py index 13c7adf..75df2e1 100644 --- a/rh/utils.py +++ b/rh/utils.py @@ -152,57 +152,6 @@ class TerminateCalledProcessError(CalledProcessError): """ -def sudo_run(cmd, user='root', **kwargs): - """Run a command via sudo. - - Client code must use this rather than coming up with their own RunCommand - invocation that jams sudo in- this function is used to enforce certain - rules in our code about sudo usage, and as a potential auditing point. - - Args: - cmd: The command to run. See RunCommand for rules of this argument- - SudoRunCommand purely prefixes it with sudo. - user: The user to run the command as. - kwargs: See RunCommand options, it's a direct pass thru to it. - Note that this supports a 'strict' keyword that defaults to True. - If set to False, it'll suppress strict sudo behavior. - - Returns: - See RunCommand documentation. - - Raises: - This function may immediately raise CalledProcessError if we're operating - in a strict sudo context and the API is being misused. - Barring that, see RunCommand's documentation- it can raise the same things - RunCommand does. - """ - # We don't use this anywhere, so it's easier to not bother supporting it. - assert not isinstance(cmd, string_types), 'shell commands not supported' - assert 'shell' not in kwargs, 'shell=True is not supported' - - sudo_cmd = ['sudo'] - - if user == 'root' and os.geteuid() == 0: - return run(cmd, **kwargs) - - if user != 'root': - sudo_cmd += ['-u', user] - - # Pass these values down into the sudo environment, since sudo will - # just strip them normally. - extra_env = kwargs.pop('extra_env', None) - extra_env = {} if extra_env is None else extra_env.copy() - - sudo_cmd.extend('%s=%s' % (k, v) for k, v in extra_env.items()) - - # Finally, block people from passing options to sudo. - sudo_cmd.append('--') - - sudo_cmd.extend(cmd) - - return run(sudo_cmd, **kwargs) - - def _kill_child_process(proc, int_timeout, kill_timeout, cmd, original_handler, signum, frame): """Used as a signal handler by RunCommand. @@ -277,20 +226,7 @@ class _Popen(subprocess.Popen): try: os.kill(self.pid, signum) except EnvironmentError as e: - if e.errno == errno.EPERM: - # Kill returns either 0 (signal delivered), or 1 (signal wasn't - # delivered). This isn't particularly informative, but we still - # need that info to decide what to do, thus check=False. - ret = sudo_run(['kill', '-%i' % signum, str(self.pid)], - capture_output=True, check=False) - if ret.returncode == 1: - # The kill binary doesn't distinguish between permission - # denied and the pid is missing. Denied can only occur - # under weird grsec/selinux policies. We ignore that - # potential and just assume the pid was already dead and - # try to reap it. - self.poll() - elif e.errno == errno.ESRCH: + if e.errno == errno.ESRCH: # Since we know the process is dead, reap it now. # Normally Popen would throw this error- we suppress it since # frankly that's a misfeature and we're already overriding diff --git a/rh/utils_unittest.py b/rh/utils_unittest.py index 8f50998..bddb0e7 100755 --- a/rh/utils_unittest.py +++ b/rh/utils_unittest.py @@ -33,7 +33,6 @@ del _path # pylint: disable=wrong-import-position import rh import rh.utils -from rh.sixish import mock class TimeDeltaStrTests(unittest.TestCase): @@ -131,49 +130,6 @@ class CalledProcessErrorTests(unittest.TestCase): self.assertNotEqual('', repr(err)) -# We shouldn't require sudo to run unittests :). -@mock.patch.object(rh.utils, 'run') -@mock.patch.object(os, 'geteuid', return_value=1000) -class SudoRunCommandTests(unittest.TestCase): - """Verify behavior of sudo_run helper.""" - - def test_run_as_root_as_root(self, mock_geteuid, mock_run): - """Check behavior when we're already root.""" - mock_geteuid.return_value = 0 - ret = rh.utils.sudo_run(['ls'], user='root') - self.assertIsNotNone(ret) - args, _kwargs = mock_run.call_args - self.assertEqual((['ls'],), args) - - def test_run_as_root_as_nonroot(self, _mock_geteuid, mock_run): - """Check behavior when we're not already root.""" - ret = rh.utils.sudo_run(['ls'], user='root') - self.assertIsNotNone(ret) - args, _kwargs = mock_run.call_args - self.assertEqual((['sudo', '--', 'ls'],), args) - - def test_run_as_nonroot_as_nonroot(self, _mock_geteuid, mock_run): - """Check behavior when we're not already root.""" - ret = rh.utils.sudo_run(['ls'], user='nobody') - self.assertIsNotNone(ret) - args, _kwargs = mock_run.call_args - self.assertEqual((['sudo', '-u', 'nobody', '--', 'ls'],), args) - - def test_env(self, _mock_geteuid, mock_run): - """Check passing through env vars.""" - ret = rh.utils.sudo_run(['ls'], extra_env={'FOO': 'bar'}) - self.assertIsNotNone(ret) - args, _kwargs = mock_run.call_args - self.assertEqual((['sudo', 'FOO=bar', '--', 'ls'],), args) - - def test_shell(self, _mock_geteuid, _mock_run): - """Check attempts to use shell code are rejected.""" - with self.assertRaises(AssertionError): - rh.utils.sudo_run('foo') - with self.assertRaises(AssertionError): - rh.utils.sudo_run(['ls'], shell=True) - - class RunCommandTests(unittest.TestCase): """Verify behavior of run helper.""" |