nix-shell: Fix $PATH handling in the impure case

We were passing "p=$PATH" rather than "p=$PATH;", resulting in some
invalid shell code.

Also, construct a separate environment for the child rather than
overwriting the parent's.

3 files changed, 55 insertions, 29 deletions

diff --git a/src/libutil/util.cc b/src/libutil/util.cc
index 1750e0373..c95a713fa 100644
--- a/src/libutil/util.cc
+++ b/src/libutil/util.cc
@@ -59,6 +59,21 @@ string getEnv(const string & key, const string & def)
 }
 
 
+std::map<std::string, std::string> getEnv()
+{
+    std::map<std::string, std::string> env;
+    for (size_t i = 0; environ[i]; ++i) {
+        auto s = environ[i];
+        auto eq = strchr(s, '=');
+        if (!eq)
+            // invalid env, just keep going
+            continue;
+        env.emplace(std::string(s, eq), std::string(eq + 1));
+    }
+    return env;
+}
+
+
 Path absPath(Path path, Path dir)
 {
     if (path[0] != '/') {
diff --git a/src/libutil/util.hh b/src/libutil/util.hh
index 182a38fb3..221165780 100644
--- a/src/libutil/util.hh
+++ b/src/libutil/util.hh
@@ -8,9 +8,11 @@
 #include <dirent.h>
 #include <unistd.h>
 #include <signal.h>
+
 #include <functional>
 #include <limits>
 #include <cstdio>
+#include <map>
 
 #ifndef HAVE_STRUCT_DIRENT_D_TYPE
 #define DT_UNKNOWN 0
@@ -25,6 +27,9 @@ namespace nix {
 /* Return an environment variable. */
 string getEnv(const string & key, const string & def = "");
 
+/* Get the entire environment. */
+std::map<std::string, std::string> getEnv();
+
 /* Return an absolutized path, resolving paths relative to the
    specified directory, or the current directory otherwise.  The path
    is also canonicalised. */
diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc
index a31f7c96c..eb5719e47 100755
--- a/src/nix-build/nix-build.cc
+++ b/src/nix-build/nix-build.cc
@@ -16,8 +16,6 @@
 
 using namespace nix;
 
-extern char ** environ;
-
 /* Recreate the effect of the perl shellwords function, breaking up a
  * string into arguments like a shell word, including escapes
  */
@@ -375,32 +373,26 @@ int main(int argc, char ** argv)
                 runProgram(settings.nixBinDir + "/nix-store", false, nixStoreArgs);
 
                 // Set the environment.
+                auto env = getEnv();
+
                 auto tmp = getEnv("TMPDIR", getEnv("XDG_RUNTIME_DIR", "/tmp"));
+
                 if (pure) {
-                    std::vector<string> skippedEnv{"HOME", "USER", "LOGNAME", "DISPLAY", "PATH", "TERM", "IN_NIX_SHELL", "TZ", "PAGER", "NIX_BUILD_SHELL"};
-                    std::vector<string> removed;
-                    for (auto i = size_t{0}; environ[i]; ++i) {
-                        auto eq = strchr(environ[i], '=');
-                        if (!eq)
-                            // invalid env, just keep going
-                            continue;
-                        std::string name(environ[i], eq);
-                        if (find(skippedEnv.begin(), skippedEnv.end(), name) == skippedEnv.end())
-                            removed.emplace_back(std::move(name));
-                    }
-                    for (const auto & name : removed)
-                        unsetenv(name.c_str());
+                    std::set<string> keepVars{"HOME", "USER", "LOGNAME", "DISPLAY", "PATH", "TERM", "IN_NIX_SHELL", "TZ", "PAGER", "NIX_BUILD_SHELL"};
+                    decltype(env) newEnv;
+                    for (auto & i : env)
+                        if (keepVars.count(i.first))
+                            newEnv.emplace(i);
+                    env = newEnv;
                     // NixOS hack: prevent /etc/bashrc from sourcing /etc/profile.
-                    setenv("__ETC_PROFILE_SOURCED", "1", 1);
+                    env["__ETC_PROFILE_SOURCED"] = "1";
                 }
-                setenv("NIX_BUILD_TOP", tmp.c_str(), 1);
-                setenv("TMPDIR", tmp.c_str(), 1);
-                setenv("TEMPDIR", tmp.c_str(), 1);
-                setenv("TMP", tmp.c_str(), 1);
-                setenv("TEMP", tmp.c_str(), 1);
-                setenv("NIX_STORE", store->storeDir.c_str(), 1);
-                for (const auto & env : drv.env)
-                    setenv(env.first.c_str(), env.second.c_str(), 1);
+
+                env["NIX_BUILD_TOP"] = env["TMPDIR"] = env["TEMPDIR"] = env["TMP"] = env["TEMP"] = tmp;
+                env["NIX_STORE"] = store->storeDir;
+
+                for (auto & var : drv.env)
+                    env.emplace(var);
 
                 restoreAffinity();
 
@@ -424,11 +416,25 @@ int main(int argc, char ** argv)
                         "shopt -u nullglob; "
                         "unset TZ; %4%"
                         "%5%"
-                        ) % (Path) tmpDir % (pure ? "" : "p=$PATH") % (pure ? "" : "PATH=$PATH:$p; unset p; ") % (getenv("TZ") ? (string("export TZ='") + getenv("TZ") + "'; ") : "") % envCommand).str());
-                if (interactive)
-                    execlp(getEnv("NIX_BUILD_SHELL", "bash").c_str(), "bash", "--rcfile", rcfile.c_str(), NULL);
-                else
-                    execlp(getEnv("NIX_BUILD_SHELL", "bash").c_str(), "bash", rcfile.c_str(), NULL);
+                        )
+                        % (Path) tmpDir
+                        % (pure ? "" : "p=$PATH; ")
+                        % (pure ? "" : "PATH=$PATH:$p; unset p; ")
+                        % (getenv("TZ") ? (string("export TZ='") + getenv("TZ") + "'; ") : "")
+                        % envCommand).str());
+
+                Strings envStrs;
+                for (auto & i : env)
+                    envStrs.push_back(i.first + "=" + i.second);
+
+                auto args = interactive
+                    ? Strings{"bash", "--rcfile", rcfile}
+                    : Strings{"bash", rcfile};
+
+                execvpe(getEnv("NIX_BUILD_SHELL", "bash").c_str(),
+                        stringsToCharPtrs(args).data(),
+                        stringsToCharPtrs(envStrs).data());
+
                 throw SysError("executing shell");
             }