TMPDIR is taken over, but weird permissions are used

[rjbs]

Test::Harness::Runner::Job sets up environment variables for the jobs it will run. Among these, it sets TMPDIR to $self->tmp_dir. That's computed from this:

File::Temp::tempdir("XXXXXX", DIR => $self->runner->tmp_dir);

File::Temp::tempdir boils down to calling its internal _gettemp, which will do this:

} elsif ($options{"mkdir"}) {

  # Open the temp directory
  if (mkdir( $path, 0700)) {
    # in case of odd umask
    chmod(0700, $path);

    return undef, $path;

So, the TMPDIR we set is set 0700, but a normal unix TMPDIR is usually 1777. Also, the job's tmp_dir is under the runner's tmp_dir, which is another 0700 directory.

The net result is that processes spawned by tests can't be guaranteed that they can read and write in the TMPDIR.

For a more concrete example: I have a set of tests that runs as root and then forks a subprocess, which switches to a lower-privilege user. The subprocess then wants to create a lockfile using Path::Tiny->tempfile, but:

1. TMPDIR is 0700, owned by root, so the subprocess may not write there
2. a parent of TMPDIR (the runner's tmp_dir) is also 0700, so chmoding TMPDIR before dropping privileges won't help

I'm not sure what I actually suggest here, but I'm going to start by saying that my current thinking is that either the job's tmp_dir is for the job's use and shouldn't take over TMPDIR or if it's meant to create a localized TMPDIR-like space, it should make sure that it's 1777 and that every parent of it have all of the bits in 0555 set.

[exodist]

Ok, I need to work out the permissions issues here. I thought they had been nailed down recently via another group who uses yath and had similar problems.

As for the feature. Yath intentionally gives each job its own temp dir, that is supposed to take over as "the temp dir" so that it can enforce isolation of temp files, and more importantly it can clean up any leftover temp files by deleting the jobs dir. This is an intentional feature that has been in place through 3 rewrites and Is desired. So we have to fix it, not remove it (though we can probably also make it optional if someone really wants to turn it off).

[rjbs]

For the record, I like the feature, but only when it doesn't break my tests. 😄

[exodist]

Improved, but not completely fixed, see PR feedback