nix-shell: stop using dynamic format strings!!

This was always a terrible idea independently of whether it crashes.
Stop doing it!

This commit was verified by running nix-shell on a trivial derivation
with --debug --verbose to get the vomit-level output of the shell rc
file and then diffing it before/after this change. I have reasonable
confidence it did not regress anything, though this code is genuinely
really hard to follow (which is a second reason that I split it into two
fmt calls).

Fixes: lix-project/lix#533
Change-Id: I8e11ddbece2b12749fda13efe0b587a71b00bfe5

src/legacy/nix-build.cc

@@ -490,42 +490,51 @@ static void main_nix_build(int argc, char * * argv)
            environment variables and shell functions.  Also don't
            lose the current $PATH directories. */
         auto rcfile = (Path) tmpDir + "/rc";
+        auto tz = getEnv("TZ");
         std::string rc = fmt(
-                R"(_nix_shell_clean_tmpdir() { command rm -rf %1%; }; )"s +
+            R"(_nix_shell_clean_tmpdir() { command rm -rf %1%; }; )"
-                (keepTmp ?
-                    "trap _nix_shell_clean_tmpdir EXIT; "
-                    "exitHooks+=(_nix_shell_clean_tmpdir); "
-                    "failureHooks+=(_nix_shell_clean_tmpdir); ":
-                    "_nix_shell_clean_tmpdir; ") +
-                (pure ? "" : "[ -n \"$PS1\" ] && [ -e ~/.bashrc ] && source ~/.bashrc;") +
             "%2%"
+            "%3%"
             // always clear PATH.
             // when nix-shell is run impure, we rehydrate it with the `p=$PATH` above
             "unset PATH;"
-                "dontAddDisableDepTrack=1;\n"
+            "dontAddDisableDepTrack=1;\n",
-                + structuredAttrsRC +
+            shellEscape(tmpDir),
+            (keepTmp
+                ? "trap _nix_shell_clean_tmpdir EXIT; "
+                  "exitHooks+=(_nix_shell_clean_tmpdir); "
+                  "failureHooks+=(_nix_shell_clean_tmpdir); "
+                : "_nix_shell_clean_tmpdir; "),
+            (pure
+                ? ""
+                : "[ -n \"$PS1\" ] && [ -e ~/.bashrc ] && source ~/.bashrc; p=$PATH; ")
+        );
+        rc += structuredAttrsRC;
+        rc += fmt(
             "\n[ -e $stdenv/setup ] && source $stdenv/setup; "
-                "%3%"
+            "%1%"
-                "PATH=%4%:\"$PATH\"; "
+            "PATH=%2%:\"$PATH\"; "
-                "SHELL=%5%; "
+            "SHELL=%3%; "
-                "BASH=%5%; "
+            "BASH=%3%; "
             "set +e; "
-                R"s([ -n "$PS1" -a -z "$NIX_SHELL_PRESERVE_PROMPT" ] && )s" +
+            R"s([ -n "$PS1" -a -z "$NIX_SHELL_PRESERVE_PROMPT" ] && )s"
-                (getuid() == 0 ? R"s(PS1='\n\[\033[1;31m\][nix-shell:\w]\$\[\033[0m\] '; )s"
+            "%4%"
-                               : R"s(PS1='\n\[\033[1;32m\][nix-shell:\w]\$\[\033[0m\] '; )s") +
             "if [ \"$(type -t runHook)\" = function ]; then runHook shellHook; fi; "
             "unset NIX_ENFORCE_PURITY; "
             "shopt -u nullglob; "
-                "unset TZ; %6%"
+            "unset TZ; %5%"
             "shopt -s execfail;"
-                "%7%",
+            "%6%",
-                shellEscape(tmpDir),
-                (pure ? "" : "p=$PATH; "),
             (pure ? "" : "PATH=$PATH:$p; unset p; "),
             shellEscape(dirOf(*shell)),
             shellEscape(*shell),
-                (getenv("TZ") ? (std::string("export TZ=") + shellEscape(getenv("TZ")) + "; ") : ""),
+            (getuid() == 0 ? R"s(PS1='\n\[\033[1;31m\][nix-shell:\w]\$\[\033[0m\] '; )s"
-                envCommand);
+                           : R"s(PS1='\n\[\033[1;32m\][nix-shell:\w]\$\[\033[0m\] '; )s"),
+            (tz.has_value()
+                ? (std::string("export TZ=") + shellEscape(*tz) + "; ")
+                : ""),
+            envCommand
+        );
         vomit("Sourcing nix-shell with file %s and contents:\n%s", rcfile, rc);
         writeFile(rcfile, rc);
 

tests/functional/meson.build

@@ -115,6 +115,7 @@ functional_tests_scripts = [
   'check.sh',
   'nix-shell/basic.sh',
   'nix-shell/structured-attrs.sh',
+  'nix-shell/regression-533.sh',
   'check-refs.sh',
   'build-remote-input-addressed.sh',
   'secure-drv-outputs.sh',

tests/functional/nix-shell/regression-533.sh

@@ -0,0 +1,18 @@
+source ../common.sh
+
+clearStore
+
+evil=$(cat <<-'EOF'
+builtins.derivation {
+  name = "evil-kbity";
+  system = "x86_64-darwin";
+  builder = "/bin/sh";
+  args = [ "-c" "> $out" ];
+  __structuredAttrs = true;
+  env.oops = "lol %s";
+}
+EOF
+)
+
+# This should not crash
+nix-shell --expr "$evil" --run 'echo yay' | grepQuiet yay