Purity checking should also accept $TMP, $TMPDIR and $TEMP, $TEMPDIR

[twhitehead]

Motivation for this change

@FRidh @jtojnar @matthewbauer @vcunat @pbogdan have to submit a new pull request for an update to #93560 as it was accepted (and hence closed) but later reverted.

The details again are, the badPath routine used by the various compiler paths to check purity is hard coded to reject any paths that lie outside of

• $NIX_STORE,
• $NIX_BUILD_TOP, and
• /tmp.

In build environments, however, /tmp is frequently overriden by settings the $TMP, $TMPDIR, and $TEMP environment variables, so I submitted a pull request #93560 to make the system use $TMP, if set, instead of /tmp.

This broke packages that were hardcoded to use /tmp (e.g., mktemp /tmp/FOOXXXX). In retrospect, the patch was also somewhat lacking as it should technically should also have accept $TMPDIR and $TEMP in addition to $TMP. This is an updated pull request that accepts all of

• $NIX_STORE,
• $NIX_BUILD_TOP,
• /tmp,
• $TMP,
• $TMPDIR, and
• $TEMP.

Being strictly more accepting than the previous one, this one should not causes any failures.

Things done

I have verified the above GNU helloworld (see above) compiles correctly in the above example once this change is made. This is actually quite extensive as change the badPath routine does result in a fairly significant rebuild of dependencies.

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Determined the impact on package closure size (by running nix path-info -S before and after)
• Ensured that relevant documentation is up to date
• Fits CONTRIBUTING.md.

[vcunat]

A quick thought: if we go for such a long positive list, wouldn't it be easier to just reject /usr/** and /lib/**? EDIT: perhaps it would be less clean.

[twhitehead]

I agree with the more clean. If you have a specific list of acceptable paths, an exact whitelist seems like a better filter than an approximate blacklist to me.

It is unfortunate that a temporary location is a bit messy, but $TMP, $TMPDIR, and $TEMP are all the ones set by nix-shell and the likes. Actually, when I went to double checked this, I noted that the correct list is $TMP, $TMPDIR and $TEMP, $TEMPDIR. I'll update the patch

[nix-shell:~]$ env | egrep 'T[^=]*MP[^=]*='
...
TMP=/run/user/1000
TMPDIR=/run/user/1000
TEMPDIR=/run/user/1000
TEMP=/run/user/1000

In the original patch I just did $TMP as I assumed it was representative of the lot of them, but I figured making it exact was probably better to avoid any possibility of a corner case at some point. I can switch it back to just $TMP if people want. The point of this patch was to include /tmp regardless of the setting of any environment variables as it seems not all programs/build scripts honour them.

[vcunat]

Seems good to me. The conflict is due to the previous version of this getting reverted.

I believe this can be merged to staging without further testing. This new version "breaks" only a subset of badPath invocations in comparison to the previous version which turned out not to be that bad (was in master and release-20.09 for some time).