TMPDIR is taken over, but weird permissions are used

Open
#185 3 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
35/100
Issue type
Bug
Clarity
Needs clarification
Activity status
Stale
Tech stack
perl

Research direction

Start with Test::Harness::Runner::Job's tmp_dir setup and the code that assigns TMPDIR for spawned jobs. Review how File::Temp creates the runner and job directories, then check the Path::Tiny->tempfile scenario under a lower-privilege subprocess; done requires an agreed, tested directory-permission design, but the issue names no target file or test.

Written by the indexing model from the issue text.

Description

bug

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.

Dominant language
Perl
Stars
28
Forks
30
PR merge metrics
No merged PRs in 30d

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from Test-More/Test2-Harness

All issues in Test-More/Test2-Harness

Similar issues

More Perl issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.