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.
diff --git a/Lib/multiprocessing/util.py b/Lib/multiprocessing/util.py
index 207a2f7..3e640b9 100644
--- a/Lib/multiprocessing/util.py
+++ b/Lib/multiprocessing/util.py
@@ -429,7 +429,7 @@
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 1016874..b5a45d9 100644
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -733,6 +733,8 @@
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 @@
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 @@
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 @@
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 @@
c2pread, c2pwrite,
errread, errwrite,
restore_signals,
- gid, gids, uid,
+ gid, gids, uid, umask,
start_new_session):
"""Execute program (POSIX version)"""
@@ -1684,7 +1687,8 @@
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 @@
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 6731447..2024435 100644
--- a/Lib/test/test_capi.py
+++ b/Lib/test/test_capi.py
@@ -97,7 +97,7 @@
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 @@
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 @@
# 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 2251640..5cc324b 100644
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -1894,6 +1894,28 @@
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 @@
-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 @@
-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: