From 4de06391058724814756a8c74ae36c279cf34006 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 20 Sep 2016 15:39:08 +0200 Subject: [PATCH] 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. --- src/libutil/util.cc | 15 +++++++++ src/libutil/util.hh | 5 +++ src/nix-build/nix-build.cc | 64 +++++++++++++++++++++----------------- 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 getEnv() +{ + std::map 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 #include #include + #include #include #include +#include #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 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 skippedEnv{"HOME", "USER", "LOGNAME", "DISPLAY", "PATH", "TERM", "IN_NIX_SHELL", "TZ", "PAGER", "NIX_BUILD_SHELL"}; - std::vector 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 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"); }