Closes issue #104 -- cannot execute sudo(8)

Previously, misinterpreted that os.access(file, X_OK)
always returns True on Solaris.  Yes, but only for
the uid of 0. Python issue #13706 closed "not a bug"
reads to "just use os.stat()", so we went to great
lengths to do so quite exhaustively.

But this is wrong -- *only* when root, should we check
the file modes -- os.access of X_OK works perfectly
fine for non-root users.

And, we should only check if any of the executable bits
are set.  Alas, it is true, you may execute that which
you may not read -- because as root, you can always read
it anyway.

Verified similar solution in NetBSD test.c (/bin/test),
OpenBSD ksh for its built-in test, and what FreeBSD/Darwin
for their implementation of which.c.

2 files changed, 106 insertions, 47 deletions

diff --git a/pexpect/__init__.py b/pexpect/__init__.py
index 4a34f15..57d8d91 100644
--- a/pexpect/__init__.py
+++ b/pexpect/__init__.py
@@ -2007,46 +2007,30 @@ class searcher_re(object):
 
 
 def is_executable_file(path):
-    """Checks that path is an executable regular file (or a symlink to a file).
-    
-    This is roughly ``os.path isfile(path) and os.access(path, os.X_OK)``, but
-    on some platforms :func:`os.access` gives us the wrong answer, so this
-    checks permission bits directly.
+    """Checks that path is an executable regular file, or a symlink to one.
+
+    This is roughly ``os.path isfile(path) and os.access(path, os.X_OK)``,
+    except for root users, which are permitted to execute a file only if
+    any of the execute bits are set.
     """
     # follow symlinks,
     fpath = os.path.realpath(path)
 
-    # return False for non-files (directories, fifo, etc.)
     if not os.path.isfile(fpath):
+        # non-files (directories, fifo, etc.)
         return False
 
-    # On Solaris, etc., "If the process has appropriate privileges, an
-    # implementation may indicate success for X_OK even if none of the
-    # execute file permission bits are set."
-    #
-    # For this reason, it is necessary to explicitly check st_mode
-
-    # get file mode using os.stat, and check if `other',
-    # that is anybody, may read and execute.
     mode = os.stat(fpath).st_mode
-    if mode & stat.S_IROTH and mode & stat.S_IXOTH:
-        return True
-
-    # get current user's group ids, and check if `group',
-    # when matching ours, may read and execute.
-    user_gids = os.getgroups() + [os.getgid()]
-    if (os.stat(fpath).st_gid in user_gids and
-            mode & stat.S_IRGRP and mode & stat.S_IXGRP):
-        return True
-
-    # finally, if file owner matches our effective userid,
-    # check if `user', may read and execute.
-    user_gids = os.getgroups() + [os.getgid()]
-    if (os.stat(fpath).st_uid == os.geteuid() and
-            mode & stat.S_IRUSR and mode & stat.S_IXUSR):
-        return True
-
-    return False
+
+    if os.getuid() == 0:
+        # when root, any permission bit of any section
+        # is fine, even if we do not own the file.
+        return bool(mode & (stat.S_IXUSR |
+                            stat.S_IXGRP |
+                            stat.S_IXOTH))
+
+    return os.access(fpath, os.X_OK)
+
 
 def which(filename):
     '''This takes a given filename; tries to find it in the environment path;
diff --git a/tests/test_which.py b/tests/test_which.py
index 83575fb..ed9493d 100644
--- a/tests/test_which.py
+++ b/tests/test_which.py
@@ -1,9 +1,14 @@
+import subprocess
 import tempfile
+import shutil
+import errno
 import os
 
 import pexpect
 from . import PexpectTestCase
 
+import pytest
+
 
 class TestCaseWhich(PexpectTestCase.PexpectTestCase):
     " Tests for pexpect.which(). "
@@ -162,27 +167,97 @@ class TestCaseWhich(PexpectTestCase.PexpectTestCase):
         try:
             # setup
             os.environ['PATH'] = bin_dir
-            with open(bin_path, 'w') as fp:
-                fp.write('#!/bin/sh\necho hello, world\n')
-            for should_match, mode in ((False, 0o000),
-                                       (True,  0o005),
-                                       (True,  0o050),
-                                       (True,  0o500),
-                                       (False, 0o004),
-                                       (False, 0o040),
-                                       (False, 0o400)):
+
+            # an interpreted script requires the ability to read,
+            # whereas a binary program requires only to be executable.
+            #
+            # to gain access to a binary program, we make a copy of
+            # the existing system program echo(1).
+            bin_echo = None
+            for pth in ('/bin/echo', '/usr/bin/echo'):
+                if os.path.exists(pth):
+                    bin_echo = pth
+                    break
+            bin_which = None
+            for pth in ('/bin/which', '/usr/bin/which'):
+                if os.path.exists(pth):
+                    bin_which = pth
+                    break
+            if not bin_echo or not bin_which:
+                pytest.skip('needs `echo` and `which` binaries')
+            shutil.copy(bin_echo, bin_path)
+
+            for should_match, mode in (
+                (False, 0o000),   # ----------, no
+                (False, 0o001),   # ---------x, no
+                (False, 0o010),   # ------x---, no
+                (True,  0o100),   # ---x------, yes
+                (False, 0o002),   # --------w-, no
+                (False, 0o020),   # -----w----, no
+                (False, 0o200),   # --w-------, no
+                (False, 0o003),   # --------wx, no
+                (False, 0o030),   # -----wx---, no
+                (True,  0o300),   # --wx------, yes
+                (False, 0o004),   # -------r--, no
+                (False, 0o040),   # ----r-----, no
+                (False, 0o400),   # -r--------, no
+                (False, 0o005),   # -------r-x, no
+                (False, 0o050),   # ----r-x---, no
+                (True,  0o500),   # -r-x------, yes
+                (False, 0o006),   # -------rw-, no
+                (False, 0o060),   # ----rw----, no
+                (False, 0o600),   # -rw-------, no
+                (False, 0o007),   # -------rwx, no
+                (False, 0o070),   # ----rwx---, no
+                (True,  0o700),   # -rwx------, yes
+                (False, 0o4001),  # ---S-----x, no
+                (False, 0o4010),  # ---S--x---, no
+                (True,  0o4100),  # ---s------, yes
+                (False, 0o4003),  # ---S----wx, no
+                (False, 0o4030),  # ---S-wx---, no
+                (True,  0o4300),  # --ws------, yes
+                (False, 0o2001),  # ------S--x, no
+                (False, 0o2010),  # ------s---, no
+                (True,  0o2100),  # ---x--S---, yes
+
+            ):
+                mode_str = '{0:0>4o}'.format(mode)
+
+                # given file mode,
                 os.chmod(bin_path, mode)
 
-                if not should_match:
-                    # should not be found because it is not executable
-                    assert pexpect.which(fname) is None
-                else:
-                    # should match full path
-                    assert pexpect.which(fname) == bin_path
+                # exercise whether we may execute
+                can_execute = True
+                try:
+                    subprocess.Popen(fname).wait() == 0
+                except OSError as err:
+                    if err.errno != errno.EACCES:
+                        raise
+                    # permission denied
+                    can_execute = False
+
+                assert should_match == can_execute, (
+                    should_match, can_execute, mode_str)
+
+                # exercise whether which(1) would match
+                proc = subprocess.Popen((bin_which, fname),
+                                        env={'PATH': bin_dir},
+                                        stdout=subprocess.PIPE)
+                bin_which_match = bool(not proc.wait())
+                assert should_match == bin_which_match, (
+                    should_match, bin_which_match, mode_str)
+
+                # finally, exercise pexpect's which(1) matches
+                # the same.
+                pexpect_match = bool(pexpect.which(fname))
+
+                assert should_match == pexpect_match == bin_which_match, (
+                    should_match, pexpect_match, bin_which_match, mode_str)
 
         finally:
             # restore,
             os.environ['PATH'] = save_path
+
             # destroy scratch files and folders,
             if os.path.exists(bin_path):
                 os.unlink(bin_path)