Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Threading with Sudo with Context and Passing in Arguments #195

Closed
nickbabcock opened this issue Aug 15, 2014 · 6 comments
Closed

Threading with Sudo with Context and Passing in Arguments #195

nickbabcock opened this issue Aug 15, 2014 · 6 comments

Comments

@nickbabcock
Copy link

Given a system with a user of 'nick' who is not in the sudoers file and the following code that is ran as root:

import sh
import threading
from time import sleep

def worker(username):
    with sh.sudo('-u', username, _with=True):
        sleep(5)
        print username, sh.id()

t1 = threading.Thread(target=worker, args=('nick',))
t2 = threading.Thread(target=worker, args=('root',))

t1.start()
sleep(1)
t2.start()

The expected output is:

nick uid=502(nick) gid=502(nick) groups=502(nick)
root uid=0(root) gid=0(root) groups=0(root)

The actual output is:

Exception in thread Thread-1:
Traceback (most recent call last):
  File "/usr/lib64/python2.6/threading.py", line 532, in __bootstrap_inner
    self.run()
  File "/usr/lib64/python2.6/threading.py", line 484, in run
    self.__target(*self.__args, **self.__kwargs)
  File "thr.py", line 8, in worker
    print username, sh.id()
  File "/home/nick/threading/lib/python2.6/site-packages/sh.py", line 769, in __call__
    return RunningCommand(cmd, call_args, stdin, stdout, stderr)
  File "/home/nick/threading/lib/python2.6/site-packages/sh.py", line 330, in __init__
    self.wait()
  File "/home/nick/threading/lib/python2.6/site-packages/sh.py", line 334, in wait
    self._handle_exit_code(self.process.wait())
  File "/home/nick/threading/lib/python2.6/site-packages/sh.py", line 348, in _handle_exit_code
    self.process.stderr
ErrorReturnCode_1:

  RAN: '/usr/bin/sudo -u nick /usr/bin/sudo -u nick /usr/bin/sudo -u root /usr/bin/id'

  STDOUT:


  STDERR:
nick is not in the sudoers file.  This incident will be reported.


nick root uid=502(nick) gid=502(nick) groups=502(nick)

EDIT: Looks like it is due to the static variable _prepend_stack being manipulated in the threads before they can properly execute.

amoffat pushed a commit that referenced this issue Dec 30, 2014
@amoffat
Copy link
Owner

amoffat commented Dec 30, 2014

i think the only fix here is a documentation fix about thread safety. the only way to make with context's thread safe would be to introduce a handle on the context object:

import sh

with sh.sudo(_with=True) as h:
    print(h.ls("/root"))

you would need to prefix every command in the block with h....but this already exists in the form of subcommands:

from sh import sudo

print(sudo.ls("/root"))

in the interest of not removing functionality right now that people are using in non-threaded environments, i'm just going to deprecate

@amoffat amoffat closed this as completed Dec 30, 2014
@noisygecko
Copy link

It would be good to remove the examples from the documentation then, if it is deprecated. http://amoffat.github.io/sh/

@vlovich
Copy link

vlovich commented Jul 10, 2015

Why not just use thread-local storage?

http://stackoverflow.com/questions/1408171/thread-local-storage-in-python

Everything stays the same from the API perspective & the with works as intended.

@vlovich
Copy link

vlovich commented Jul 10, 2015

It's may be worthwhile to ensure, if possible, that any new threads inherit the with from the thread they were spawned from. That way:

with sudo:
    t1 = sh.Thread(target=worker, args=('nick',))
    t2 = sh.Thread(target=worker, args=('root',))
    t1.start()
    t2.start()

works as expected & both threads run as sudo. You'd need to override Thread so that you could interject on the new thread & propogate the thread-local variable from the current thread.

@amoffat amoffat reopened this Jul 10, 2015
@AGSaidi
Copy link

AGSaidi commented Feb 24, 2016

Using the sudo.ls() example above, is it possible to pass parameters to sudo?
For example, how would you replace:

    with sudo('-u foo', _with=True):
        ls()

@amoffat
Copy link
Owner

amoffat commented Oct 14, 2016

Added thread safety using @vlovich 's threadlocal idea. This is in the release-1.2 branch and will go out with it when it ships

0-wiz-0 added a commit to NetBSD/pkgsrc-wip that referenced this issue Dec 12, 2016
*   added `_out` and `_out_bufsize` validator [#346](amoffat/sh#346)
*   bugfix for internal stdout thread running when it shouldn't [#346](amoffat/sh#346)

*   regression bugfix on timeout [#344](amoffat/sh#344)
*   regression bugfix on `_ok_code=None`

*   further improvements on cpu usage

*   regression in cpu usage [#339](amoffat/sh#339)

*   fd leak regression and fix for flawed fd leak detection test [#337](amoffat/sh#337)

*   support for `io.StringIO` in python2

*   added support for using raw file descriptors for `_in`, `_out`, and `_err`
*   removed `.close()`ing `_out` handler if FIFO detected

*   composed commands no longer propagate `_bg`
*   better support for using `sys.stdin` and `sys.stdout` for `_in` and `_out`
*   bugfix where `which()` would not stop searching at the first valid executable found in PATH
*   added `_long_prefix` for programs whose long arguments start with something other than `--` [#278](amoffat/sh#278)
*   added `_log_msg` for advanced configuration of log message [#311](amoffat/sh#311)
*   added `sh.contrib.sudo`
*   added `_arg_preprocess` for advanced command wrapping
*   alter callable `_in` arguments to signify completion with falsy chunk
*   bugfix where pipes passed into `_out` or `_err` were not flushed on process end [#252](amoffat/sh#252)
*   deprecated `with sh.args(**kwargs)` in favor of `sh2 = sh(**kwargs)`
*   made `sh.pushd` thread safe
*   added `.kill_group()` and `.signal_group()` methods for better process control [#237](amoffat/sh#237)
*   added `new_session` special keyword argument for controlling spawned process session [#266](amoffat/sh#266)
*   bugfix better handling for EINTR on system calls [#292](amoffat/sh#292)
*   bugfix where with-contexts were not threadsafe [#247](amoffat/sh#195)
*   `_uid` new special keyword param for specifying the user id of the process [#133](amoffat/sh#133)
*   bugfix where exceptions were swallowed by processes that weren't waited on [#309](amoffat/sh#309)
*   bugfix where processes that dupd their stdout/stderr to a long running child process would cause sh to hang [#310](amoffat/sh#310)
*   improved logging output [#323](amoffat/sh#323)
*   bugfix for python3+ where binary data was passed into a process's stdin [#325](amoffat/sh#325)
*   Introduced execution contexts which allow baking of common special keyword arguments into all commands [#269](amoffat/sh#269)
*   `Command` and `which` now can take an optional `paths` parameter which specifies the search paths [#226](amoffat/sh#226)
*   `_preexec_fn` option for executing a function after the child process forks but before it execs [#260](amoffat/sh#260)
*   `_fg` reintroduced, with limited functionality.  hurrah! [#92](amoffat/sh#92)
*   bugfix where a command would block if passed a fd for stdin that wasn't yet ready to read [#253](amoffat/sh#253)
*   `_long_sep` can now take `None` which splits the long form arguments into individual arguments [#258](amoffat/sh#258)
*   making `_piped` perform "direct" piping by default (linking fds together).  this fixes memory problems [#270](amoffat/sh#270)
*   bugfix where calling `next()` on an iterable process that has raised `StopIteration`, hangs [#273](amoffat/sh#273)
*   `sh.cd` called with no arguments no changes into the user's home directory, like native `cd` [#275](amoffat/sh#275)
*   `sh.glob` removed entirely.  the rationale is correctness over hand-holding. [#279](amoffat/sh#279)
*   added `_truncate_exc`, defaulting to `True`, which tells our exceptions to truncate output.
*   bugfix for exceptions whose messages contained unicode
*   `_done` callback no longer assumes you want your command put in the background.
*   `_done` callback is now called asynchronously in a separate thread.
*   `_done` callback is called regardless of exception, which is necessary in order to release held resources, for example a process pool
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants