salt-tests-tmpdir directory name fixed

[boltronics]

Description of Issue/Question

I created a virtualenv, attempted to run some tests and then hit the dreaded file name length issue. I then deleted this virtualenv and created it again under a different directory with a shorter path name.

Running tests would always fail like so:

 * Starting salt-master ... Traceback (most recent call last):
  File "/tmp/salt-tests-tmpdir/scripts/cli_master.py", line 10, in <module>
    import salt.utils.platform
ImportError: No module named platform
Traceback (most recent call last):
  File "/tmp/salt-tests-tmpdir/scripts/cli_master.py", line 10, in <module>
    import salt.utils.platform
ImportError: No module named platform
Traceback (most recent call last):
  File "/tmp/salt-tests-tmpdir/scripts/cli_master.py", line 10, in <module>
    import salt.utils.platform
ImportError: No module named platform
Traceback (most recent call last):
  File "/tmp/salt-tests-tmpdir/scripts/cli_master.py", line 10, in <module>
    import salt.utils.platform
ImportError: No module named platform
 * Starting salt-master ... FAILED!

The reason being was the file /tmp/salt-tests-tmpdir/scripts/cli_master.py which had the #!/path/to/python using the original virtual env which no longer existed.

The work-around was to run rm -rf /tmp/salt-tests-tmpdir/ but this directory shouldn't have been left around (it looks like it was left around due to the aforementioned file length issue not allowing it to get cleaned up properly), and it definitely shouldn't have been reused and blindly trusted to be correct.

This is likely a security risk. If malicious user X creates the /tmp/salt-tests-tmpdir/scripts/cli_master.py file that does something nasty, it'll execute as the next user who runs the Salt test suite, which is particularly dangerous where shared systems are used. It will also cause conflicts if multiple users are experimenting with Salt. These situations are common in university labs or companies with shared development hosts. The temporary directory should be unique and not guessable if it's going to live in a shared temporary directory.

Setup

Installed using the official instructions:
https://docs.saltstack.com/en/latest/topics/development/hacking.html#running-unit-and-integration-tests

I elected to use this pip command. Not sure if it matters.

GENERATE_SALT_SYSPATHS=1 pip install --global-option='--salt-root-dir=/path/to/your/virtualenv/' \
    -e ./salt   # the path to the salt git clone from above

This was tested against the latest in the develop branch.

Steps to Reproduce Issue

Not applicable.

Versions Report

```$ salt --versions-report
Salt Version:
Salt: 2018.11.0-384-g499fccd

Dependency Versions:
cffi: Not Installed
cherrypy: Not Installed
dateutil: Not Installed
docker-py: Not Installed
gitdb: Not Installed
gitpython: Not Installed
Jinja2: 2.10
libgit2: Not Installed
M2Crypto: Not Installed
Mako: Not Installed
msgpack-pure: Not Installed
msgpack-python: 0.5.6
mysql-python: Not Installed
pycparser: Not Installed
pycrypto: 2.6.1
pycryptodome: Not Installed
pygit2: Not Installed
Python: 2.7.13 (default, Sep 26 2018, 18:42:22)
python-gnupg: Not Installed
PyYAML: 3.13
PyZMQ: 17.0.0
smmap: Not Installed
timelib: Not Installed
Tornado: 4.5.3
ZMQ: 4.1.6

System Versions:
dist: debian 9.6
locale: UTF-8
machine: x86_64
release: 4.17.0-0.bpo.3-amd64
system: Linux
version: debian 9.6

```

[dwoz]

@boltronics Thanks for pointing this out. A few things come to mind:

We definitely should be cleaning up the tests temporary directory. I don't think this is the only case where tests create directories and/or files without cleaning them up. We have some work to do in this area.

As far as a security risk goes. Generally we run the test suite as root on brand new boxes and the boxes go away after the test suite runs. We do this because most of the modules and states being tested need root access to make changes to the host system.

I agree making the test dir random and making sure it is cleaned up even in the event of a failure/exception in the test suite is a step in the right direction. We might also want to document this part and what should be expected when running as a normal user more.

@saltstack/team-core I am curious what others' thoughts are?

[boltronics]

@dwoz That makes sense and I figured it might be the case since Salt Stack has dedicated CI servers so don't run into such problems. Instead, I feel this is an issue for external contributors that don't have dedicated test hardware/environments, don't need to run the entire test suite, and don't even need (and probably don't have privileges) to run tests as root, instead using a setup like that detailed in the _Installing Salt for Development_ instructions.

[dwoz]

@boltronics Long term I would like to see these issues fixes as much as possible. At the very least we could make it so the test directory is unique per test run.

For now you might look into barnacle which you can use to tests salt in docker images. Salt also has test-kitchen configurations which can be run locally (using docker) or on aws. The documentation for running test-kitchen to test salt can be found here

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[boltronics]

I think someone should be looking into this.

[stale[bot]]

Thank you for updating this issue. It is no longer marked as stale.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[boltronics]

I still think someone should be looking into this. Otherwise, we should be making it crystal clear to users and developers that Salt's tests (by default) are likely to present security risks to the user the tests are executed as.

[stale[bot]]

Thank you for updating this issue. It is no longer marked as stale.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[stale[bot]]

Thank you for updating this issue. It is no longer marked as stale.

[s0undt3ch]

Each test run will get it's own temporary directory after https://github.com/saltstack/salt/pull/56460 is merged

[s0undt3ch]

I might have to move this fix to a separate PR because there's a few things in the test suite which, sadly, rely on that hardcoded path.