From f3751efb5c8b53b37efbbf75d9422c1d11c01646 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Sat, 12 Oct 2019 13:24:56 -0700 Subject: bpo-38417: Add umask support to subprocess (GH-16726) On POSIX systems, allow the umask to be set in the child process before we exec. --- Doc/library/subprocess.rst | 12 +++++++--- Lib/multiprocessing/util.py | 2 +- Lib/subprocess.py | 14 +++++++----- Lib/test/test_capi.py | 6 ++--- Lib/test/test_subprocess.py | 26 ++++++++++++++++++++-- .../2019-10-12-00-13-47.bpo-38417.W7x_aS.rst | 2 ++ Modules/_posixsubprocess.c | 14 +++++++----- 7 files changed, 57 insertions(+), 19 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2019-10-12-00-13-47.bpo-38417.W7x_aS.rst diff --git a/Doc/library/subprocess.rst b/Doc/library/subprocess.rst index 1a98bb33d0..19290bfc34 100644 --- a/Doc/library/subprocess.rst +++ b/Doc/library/subprocess.rst @@ -339,9 +339,9 @@ functions. stderr=None, preexec_fn=None, close_fds=True, shell=False, \ cwd=None, env=None, universal_newlines=None, \ startupinfo=None, creationflags=0, restore_signals=True, \ - start_new_session=False, pass_fds=(), *, group=None, \ - extra_groups=None, user=None, encoding=None, errors=None, \ - text=None) + start_new_session=False, pass_fds=(), \*, group=None, \ + extra_groups=None, user=None, umask=-1, \ + encoding=None, errors=None, text=None) Execute a child program in a new process. On POSIX, the class uses :meth:`os.execvp`-like behavior to execute the child program. On Windows, @@ -572,6 +572,12 @@ functions. .. availability:: POSIX .. versionadded:: 3.9 + If *umask* is not negative, the umask() system call will be made in the + child process prior to the execution of the subprocess. + + .. availability:: POSIX + .. versionadded:: 3.9 + If *env* is not ``None``, it must be a mapping that defines the environment variables for the new process; these are used instead of the default behavior of inheriting the current process' environment. diff --git a/Lib/multiprocessing/util.py b/Lib/multiprocessing/util.py index 207a2f70b2..3e640b944e 100644 --- a/Lib/multiprocessing/util.py +++ b/Lib/multiprocessing/util.py @@ -429,7 +429,7 @@ def spawnv_passfds(path, args, passfds): return _posixsubprocess.fork_exec( args, [os.fsencode(path)], True, passfds, None, None, -1, -1, -1, -1, -1, -1, errpipe_read, errpipe_write, - False, False, None, None, None, None) + False, False, None, None, None, -1, None) finally: os.close(errpipe_read) os.close(errpipe_write) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 1016874c4a..b5a45d9fea 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -733,6 +733,8 @@ class Popen(object): user (POSIX only) + umask (POSIX only) + pass_fds (POSIX only) encoding and errors: Text mode encoding and error handling to use for @@ -750,7 +752,7 @@ class Popen(object): startupinfo=None, creationflags=0, restore_signals=True, start_new_session=False, pass_fds=(), *, user=None, group=None, extra_groups=None, - encoding=None, errors=None, text=None): + encoding=None, errors=None, text=None, umask=-1): """Create new Popen instance.""" _cleanup() # Held while anything is calling waitpid before returncode has been @@ -945,7 +947,7 @@ class Popen(object): c2pread, c2pwrite, errread, errwrite, restore_signals, - gid, gids, uid, + gid, gids, uid, umask, start_new_session) except: # Cleanup if the child failed starting. @@ -1318,6 +1320,7 @@ class Popen(object): errread, errwrite, unused_restore_signals, unused_gid, unused_gids, unused_uid, + unused_umask, unused_start_new_session): """Execute program (MS Windows version)""" @@ -1645,7 +1648,7 @@ class Popen(object): c2pread, c2pwrite, errread, errwrite, restore_signals, - gid, gids, uid, + gid, gids, uid, umask, start_new_session): """Execute program (POSIX version)""" @@ -1684,7 +1687,8 @@ class Popen(object): and not start_new_session and gid is None and gids is None - and uid is None): + and uid is None + and umask < 0): self._posix_spawn(args, executable, env, restore_signals, p2cread, p2cwrite, c2pread, c2pwrite, @@ -1738,7 +1742,7 @@ class Popen(object): errread, errwrite, errpipe_read, errpipe_write, restore_signals, start_new_session, - gid, gids, uid, + gid, gids, uid, umask, preexec_fn) self._child_created = True finally: diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index 6731447af4..2024435109 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -97,7 +97,7 @@ class CAPITest(unittest.TestCase): def __len__(self): return 1 self.assertRaises(TypeError, _posixsubprocess.fork_exec, - 1,Z(),3,(1, 2),5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20) + 1,Z(),3,(1, 2),5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21) # Issue #15736: overflow in _PySequence_BytesToCharpArray() class Z(object): def __len__(self): @@ -105,7 +105,7 @@ class CAPITest(unittest.TestCase): def __getitem__(self, i): return b'x' self.assertRaises(MemoryError, _posixsubprocess.fork_exec, - 1,Z(),3,(1, 2),5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20) + 1,Z(),3,(1, 2),5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21) @unittest.skipUnless(_posixsubprocess, '_posixsubprocess required for this test.') def test_subprocess_fork_exec(self): @@ -115,7 +115,7 @@ class CAPITest(unittest.TestCase): # Issue #15738: crash in subprocess_fork_exec() self.assertRaises(TypeError, _posixsubprocess.fork_exec, - Z(),[b'1'],3,(1, 2),5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20) + Z(),[b'1'],3,(1, 2),5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21) @unittest.skipIf(MISSING_C_DOCSTRINGS, "Signature information for builtins requires docstrings") diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 22516402da..5cc324b878 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1894,6 +1894,28 @@ class POSIXProcessTestCase(BaseTestCase): with self.assertRaises(ValueError): subprocess.check_call([sys.executable, "-c", "pass"], extra_groups=[]) + @unittest.skipIf(mswindows or not hasattr(os, 'umask'), + 'POSIX umask() is not available.') + def test_umask(self): + tmpdir = None + try: + tmpdir = tempfile.mkdtemp() + name = os.path.join(tmpdir, "beans") + # We set an unusual umask in the child so as a unique mode + # for us to test the child's touched file for. + subprocess.check_call( + [sys.executable, "-c", f"open({name!r}, 'w')"], # touch + umask=0o053) + # Ignore execute permissions entirely in our test, + # filesystems could be mounted to ignore or force that. + st_mode = os.stat(name).st_mode & 0o666 + expected_mode = 0o624 + self.assertEqual(expected_mode, st_mode, + msg=f'{oct(expected_mode)} != {oct(st_mode)}') + finally: + if tmpdir is not None: + shutil.rmtree(tmpdir) + def test_run_abort(self): # returncode handles signal termination with support.SuppressCrashReport(): @@ -2950,7 +2972,7 @@ class POSIXProcessTestCase(BaseTestCase): -1, -1, -1, -1, 1, 2, 3, 4, True, True, - False, [], 0, + False, [], 0, -1, func) # Attempt to prevent # "TypeError: fork_exec() takes exactly N arguments (M given)" @@ -2999,7 +3021,7 @@ class POSIXProcessTestCase(BaseTestCase): -1, -1, -1, -1, 1, 2, 3, 4, True, True, - None, None, None, + None, None, None, -1, None) self.assertIn('fds_to_keep', str(c.exception)) finally: diff --git a/Misc/NEWS.d/next/Library/2019-10-12-00-13-47.bpo-38417.W7x_aS.rst b/Misc/NEWS.d/next/Library/2019-10-12-00-13-47.bpo-38417.W7x_aS.rst new file mode 100644 index 0000000000..c2356ddbf1 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-10-12-00-13-47.bpo-38417.W7x_aS.rst @@ -0,0 +1,2 @@ +Added support for setting the umask in the child process to the subprocess +module on POSIX systems. diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index 66db93e450..80bb44dce9 100644 --- a/Modules/_posixsubprocess.c +++ b/Modules/_posixsubprocess.c @@ -8,7 +8,7 @@ #ifdef HAVE_SYS_TYPES_H #include #endif -#if defined(HAVE_SYS_STAT_H) && defined(__FreeBSD__) +#if defined(HAVE_SYS_STAT_H) #include #endif #ifdef HAVE_SYS_SYSCALL_H @@ -428,7 +428,7 @@ child_exec(char *const exec_array[], int call_setsid, int call_setgid, gid_t gid, int call_setgroups, size_t groups_size, const gid_t *groups, - int call_setuid, uid_t uid, + int call_setuid, uid_t uid, int child_umask, PyObject *py_fds_to_keep, PyObject *preexec_fn, PyObject *preexec_fn_args_tuple) @@ -498,6 +498,9 @@ child_exec(char *const exec_array[], if (cwd) POSIX_CALL(chdir(cwd)); + if (child_umask >= 0) + umask(child_umask); /* umask() always succeeds. */ + if (restore_signals) _Py_RestoreSignals(); @@ -609,6 +612,7 @@ subprocess_fork_exec(PyObject* self, PyObject *args) int call_setgid = 0, call_setgroups = 0, call_setuid = 0; uid_t uid; gid_t gid, *groups = NULL; + int child_umask; PyObject *cwd_obj, *cwd_obj2; const char *cwd; pid_t pid; @@ -619,14 +623,14 @@ subprocess_fork_exec(PyObject* self, PyObject *args) int saved_errno = 0; if (!PyArg_ParseTuple( - args, "OOpO!OOiiiiiiiiiiOOOO:fork_exec", + args, "OOpO!OOiiiiiiiiiiOOOiO:fork_exec", &process_args, &executable_list, &close_fds, &PyTuple_Type, &py_fds_to_keep, &cwd_obj, &env_list, &p2cread, &p2cwrite, &c2pread, &c2pwrite, &errread, &errwrite, &errpipe_read, &errpipe_write, &restore_signals, &call_setsid, - &gid_object, &groups_list, &uid_object, + &gid_object, &groups_list, &uid_object, &child_umask, &preexec_fn)) return NULL; @@ -843,7 +847,7 @@ subprocess_fork_exec(PyObject* self, PyObject *args) errread, errwrite, errpipe_read, errpipe_write, close_fds, restore_signals, call_setsid, call_setgid, gid, call_setgroups, num_groups, groups, - call_setuid, uid, + call_setuid, uid, child_umask, py_fds_to_keep, preexec_fn, preexec_fn_args_tuple); _exit(255); return NULL; /* Dead code to avoid a potential compiler warning. */ -- cgit v1.2.1