diff --git a/src/libstore/ssh.cc b/src/libstore/ssh.cc index e5d623adf..b8c5f4d97 100644 --- a/src/libstore/ssh.cc +++ b/src/libstore/ssh.cc @@ -3,6 +3,7 @@ #include "current-process.hh" #include "environment-variables.hh" #include "util.hh" +#include "exec.hh" namespace nix { @@ -44,6 +45,10 @@ void SSHMaster::addCommonSSHOpts(Strings & args) if (compress) args.push_back("-C"); + // We use this to make ssh signal back to us that the connection is established. + // It really does run locally; see createSSHEnv which sets up SHELL to make + // it launch more reliably. The local command runs synchronously, so presumably + // the remote session won't be garbled if the local command is slow. args.push_back("-oPermitLocalCommand=yes"); args.push_back("-oLocalCommand=echo started"); } @@ -56,6 +61,27 @@ bool SSHMaster::isMasterRunning() { return res.first == 0; } +Strings createSSHEnv() +{ + // Copy the environment and set SHELL=/bin/sh + std::map env = getEnv(); + + // SSH will invoke the "user" shell for -oLocalCommand, but that means + // $SHELL. To keep things simple and avoid potential issues with other + // shells, we set it to /bin/sh. + // Technically, we don't need that, and we could reinvoke ourselves to print + // "started". Self-reinvocation is tricky with library consumers, but mostly + // solved; refer to the development history of nixExePath in libstore/globals.cc. + env.insert_or_assign("SHELL", "/bin/sh"); + + Strings r; + for (auto & [k, v] : env) { + r.push_back(k + "=" + v); + } + + return r; +} + std::unique_ptr SSHMaster::startCommand( Strings && command, Strings && extraSshArgs) { @@ -104,8 +130,8 @@ std::unique_ptr SSHMaster::startCommand( } args.splice(args.end(), std::move(command)); - - execvp(args.begin()->c_str(), stringsToCharPtrs(args).data()); + auto env = createSSHEnv(); + nix::execvpe(args.begin()->c_str(), stringsToCharPtrs(args).data(), stringsToCharPtrs(env).data()); // could not exec ssh/bash throw SysError("unable to execute '%s'", args.front()); @@ -172,7 +198,8 @@ Path SSHMaster::startMaster() if (verbosity >= lvlChatty) args.push_back("-v"); addCommonSSHOpts(args); - execvp(args.begin()->c_str(), stringsToCharPtrs(args).data()); + auto env = createSSHEnv(); + nix::execvpe(args.begin()->c_str(), stringsToCharPtrs(args).data(), stringsToCharPtrs(env).data()); throw SysError("unable to execute '%s'", args.front()); }, options); diff --git a/tests/nixos/remote-builds.nix b/tests/nixos/remote-builds.nix index 8ddf6ad02..ab159eaad 100644 --- a/tests/nixos/remote-builds.nix +++ b/tests/nixos/remote-builds.nix @@ -81,6 +81,17 @@ in virtualisation.additionalPaths = [ config.system.build.extraUtils ]; nix.settings.substituters = lib.mkForce [ ]; programs.ssh.extraConfig = "ConnectTimeout 30"; + environment.systemPackages = [ + # `bad-shell` is used to make sure Nix works an environment with a misbehaving shell. + # + # More realistically, a bad shell would still run the command ("echo started") + # but considering that our solution is to avoid this shell (set via $SHELL), we + # don't need to bother with a more functional mock shell. + (pkgs.writeScriptBin "bad-shell" '' + #!${pkgs.runtimeShell} + echo "Hello, I am a broken shell" + '') + ]; }; }; @@ -114,9 +125,13 @@ in 'echo hello world on $(hostname)' >&2 """) + # Check that SSH uses SHELL for LocalCommand, as expected, and check that + # our test setup here is working. The next test will use this bad SHELL. + client.succeed(f"SHELL=$(which bad-shell) ssh -oLocalCommand='true' -oPermitLocalCommand=yes {builder1.name} 'echo hello world' | grep -F 'Hello, I am a broken shell'") + # Perform a build and check that it was performed on the builder. out = client.succeed( - "nix-build ${expr nodes.client 1} 2> build-output", + "SHELL=$(which bad-shell) nix-build ${expr nodes.client 1} 2> build-output", "grep -q Hello build-output" ) builder1.succeed(f"test -e {out}")