salt.utils.path.which() on the windows platform can return a non-executable file if a file without an extension already exists (breaks npm module on windows)

[arizvisa]

Description of Issue/Question

Okay...this is getting pretty fucking outrageous. This condition can happen in numerous other places, but I encountered it specifically when installing nodejs via chocolatey and then trying to use the npm modules in salt. The issue is that in order to detect the existence of npm, salt will use salt.utils.path.which() to locate the npm binary.

The salt.utils.path.which() function mixes up the semantics what's considered executable on Windows with what's considered executable on Posix. This mix up occurs when a script with the exact name that is being searched for exists despite the filename not ending with the correct extension. Specifically if there's two scripts..one of which is "blah.cmd" and another which is "blah", salt will return to the path to "blah" instead of "blah.cmd".

This situation results in the script w/o the extension being given priority. i.e. The node.js installer bundles two scripts in your PATH one of which is a bourne-script named npm, and another cmd-script named npm.cmd. Due to the existence of the first one, salt.util.path.which() returns the path to this instead of npm.cmd which is the correct command to run since it has an extension in PATHEXT.

In the node.js example which calls salt.utils.path.which('npm'), it'll log the following error due to npm not being being executable from the windows shell.

2019-02-23 19:21:06,789 [salt.state       :320 ][ERROR   ][1356] State 'npm.installed' was not found in SLS 'develop'
Reason: 'npm' __virtual__ returned False: 'npm' binary not found on system

Setup

On the Windows platform, drop two files named test and test.cmd in the search PATH. If you want to execute something, put some cmd-script that redirects some text into a logfile or something in either.

Steps to Reproduce Issue

After getting two files in your path, simply call salt.utils.path.which() and it will return the path to the test file instead of the one with the correct extension. As mentioned, this is wrong as test cannot be executed from the command shell due to its extension not existing in PATHEXT.

To prove that you can't execute the first script, try passing it to os.system or something.

Versions Report

Salt Version:
           Salt: 2018.3.3
 
Dependency Versions:
           cffi: 1.10.0
       cherrypy: 10.2.1
       dateutil: 2.6.1
      docker-py: Not Installed
          gitdb: 2.0.5
      gitpython: 2.1.3
          ioflo: Not Installed
         Jinja2: 2.10
        libgit2: Not Installed
        libnacl: 1.6.1
       M2Crypto: Not Installed
           Mako: 1.0.7
   msgpack-pure: Not Installed
 msgpack-python: 0.4.8
   mysql-python: Not Installed
      pycparser: 2.17
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.14 (v2.7.14:84471935ed, Sep 16 2017, 20:19:30) [MSC v.1500 32 bit (Intel)]
   python-gnupg: 0.4.1
         PyYAML: 3.12
          PyZMQ: 16.0.3
           RAET: Not Installed
          smmap: 2.0.5
        timelib: 0.2.4
        Tornado: 4.5.1
            ZMQ: 4.1.6
 
System Versions:
           dist:   
         locale: cp1252
        machine: AMD64
        release: 8
         system: Windows
        version: 8 6.2.9200 SP0 Multiprocessor Free

[arizvisa]

Ftr, this is not limited to 2018.3.3 as the related code in salt.utils.path.which has been wrong since 2017-07-24.

[arizvisa]

I take that back, since 2015-02-05.

[arizvisa]

...and PR #51785 fixes this.

[dwoz]

@arizvisa Thanks for reporting this and working on a fix.