From c1f4c60bc23fd4e4a6fa64d22c17f1d348e16247 Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Thu, 26 Sep 2024 14:12:18 -0700 Subject: [PATCH] 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: https://git.lix.systems/lix-project/lix/issues/533 Change-Id: I8e11ddbece2b12749fda13efe0b587a71b00bfe5 --- src/legacy/nix-build.cc | 79 +++++++++++--------- tests/functional/meson.build | 1 + tests/functional/nix-shell/regression-533.sh | 18 +++++ 3 files changed, 63 insertions(+), 35 deletions(-) create mode 100644 tests/functional/nix-shell/regression-533.sh diff --git a/src/legacy/nix-build.cc b/src/legacy/nix-build.cc index eb9b6cd8d..1107bc443 100644 --- a/src/legacy/nix-build.cc +++ b/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 + - (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%" - // always clear PATH. - // when nix-shell is run impure, we rehydrate it with the `p=$PATH` above - "unset PATH;" - "dontAddDisableDepTrack=1;\n" - + structuredAttrsRC + - "\n[ -e $stdenv/setup ] && source $stdenv/setup; " - "%3%" - "PATH=%4%:\"$PATH\"; " - "SHELL=%5%; " - "BASH=%5%; " - "set +e; " - 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" - : 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%" - "shopt -s execfail;" - "%7%", - 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")) + "; ") : ""), - envCommand); + R"(_nix_shell_clean_tmpdir() { command rm -rf %1%; }; )" + "%2%" + "%3%" + // always clear PATH. + // when nix-shell is run impure, we rehydrate it with the `p=$PATH` above + "unset PATH;" + "dontAddDisableDepTrack=1;\n", + 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; " + "%1%" + "PATH=%2%:\"$PATH\"; " + "SHELL=%3%; " + "BASH=%3%; " + "set +e; " + R"s([ -n "$PS1" -a -z "$NIX_SHELL_PRESERVE_PROMPT" ] && )s" + "%4%" + "if [ \"$(type -t runHook)\" = function ]; then runHook shellHook; fi; " + "unset NIX_ENFORCE_PURITY; " + "shopt -u nullglob; " + "unset TZ; %5%" + "shopt -s execfail;" + "%6%", + (pure ? "" : "PATH=$PATH:$p; unset p; "), + shellEscape(dirOf(*shell)), + shellEscape(*shell), + (getuid() == 0 ? R"s(PS1='\n\[\033[1;31m\][nix-shell:\w]\$\[\033[0m\] '; )s" + : 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); diff --git a/tests/functional/meson.build b/tests/functional/meson.build index 3b610bcdc..8315a4652 100644 --- a/tests/functional/meson.build +++ b/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', diff --git a/tests/functional/nix-shell/regression-533.sh b/tests/functional/nix-shell/regression-533.sh new file mode 100644 index 000000000..422f809fb --- /dev/null +++ b/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