diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/process.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/process.py index 75b4e2b96823..ac83d92039f7 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/process.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/process.py @@ -41,15 +41,6 @@ type Args = Sequence[Arg] type EnvValue = str | Literal[_Env.PRESERVE_ENV] -@dataclass(frozen=True) -class _RawShellArg: - value: str - - @override - def __str__(self) -> str: - return self.value - - @dataclass(frozen=True) class Remote: host: str @@ -144,11 +135,13 @@ def run_wrapper( resolved_env = _resolve_env_local(normalized_env) if remote: - # Apply env for the *remote command* (not for ssh itself) - remote_run_args: list[Arg | _RawShellArg] = [] - remote_run_args.extend(run_args) - if normalized_env: - remote_run_args = _prefix_env_cmd_remote(run_args, normalized_env) + remote_run_args: list[Arg] = [ + "/bin/sh", + "-c", + _remote_shell_script(normalized_env), + "sh", + *run_args, + ] if sudo: sudo_args = shlex.split(os.getenv("NIX_SUDOOPTS", "")) @@ -275,30 +268,29 @@ def _prefix_env_cmd(cmd: Sequence[Arg], resolved_env: dict[str, str]) -> list[Ar return ["env", "-i", *assigns, *cmd] -def _prefix_env_cmd_remote( - cmd: Args, - env: dict[str, EnvValue], -) -> list[Arg | _RawShellArg]: +def _remote_shell_script(env: Mapping[str, EnvValue]) -> str: """ - Prefix remote commands with env assignments. Preserve markers are expanded - by the remote shell at execution time. + Build the POSIX shell wrapper used for remote execution over SSH. + + SSH sends the remote command as a shell-interpreted command line, so we + need a wrapper to establish a clean environment before `exec`-ing the real + command. This wrapper is always run under `/bin/sh -c` so preserved + variables like `${PATH-}` do not depend on the remote user's login shell. """ - assigns: list[str | _RawShellArg] = [] + shell_assigns: list[str] = [] for k, v in env.items(): if v is PRESERVE_ENV: - assigns.append(_RawShellArg(f"{k}=${{{k}-}}")) + shell_assigns.append(f'{k}="${{{k}-}}"') else: - assigns.append(f"{k}={v}") - return ["env", "-i", *assigns, *cmd] + shell_assigns.append(f"{k}={shlex.quote(v)}") + return f'exec env -i {" ".join(shell_assigns)} "$@"' -def _quote_remote_arg(arg: Arg | _RawShellArg) -> str: - if isinstance(arg, _RawShellArg): - return str(arg) +def _quote_remote_arg(arg: Arg) -> str: return shlex.quote(str(arg)) -def _sanitize_env_run_args(run_args: list[Arg] | list[Arg | _RawShellArg]) -> list[Arg]: +def _sanitize_env_run_args(run_args: list[Arg]) -> list[Arg]: """ Sanitize long or sensitive environment variables from logs. """ diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_main.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_main.py index 83004b18b8a8..7f51c09d7497 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_main.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_main.py @@ -617,9 +617,10 @@ def test_execute_nix_switch_build_target_host( *nr.process.SSH_DEFAULT_OPTS, "user@build-host", "--", - "env", - "-i", - "PATH=${PATH-}", + "/bin/sh", + "-c", + """'exec env -i PATH="${PATH-}" "$@"'""", + "sh", "mktemp", "-d", "-t", @@ -635,9 +636,10 @@ def test_execute_nix_switch_build_target_host( *nr.process.SSH_DEFAULT_OPTS, "user@build-host", "--", - "env", - "-i", - "PATH=${PATH-}", + "/bin/sh", + "-c", + """'exec env -i PATH="${PATH-}" "$@"'""", + "sh", "nix-store", "--realise", str(config_path), @@ -654,9 +656,10 @@ def test_execute_nix_switch_build_target_host( *nr.process.SSH_DEFAULT_OPTS, "user@build-host", "--", - "env", - "-i", - "PATH=${PATH-}", + "/bin/sh", + "-c", + """'exec env -i PATH="${PATH-}" "$@"'""", + "sh", "readlink", "-f", "/tmp/tmpdir/config", @@ -671,9 +674,10 @@ def test_execute_nix_switch_build_target_host( *nr.process.SSH_DEFAULT_OPTS, "user@build-host", "--", - "env", - "-i", - "PATH=${PATH-}", + "/bin/sh", + "-c", + """'exec env -i PATH="${PATH-}" "$@"'""", + "sh", "rm", "-rf", "/tmp/tmpdir", @@ -703,9 +707,10 @@ def test_execute_nix_switch_build_target_host( "user@target-host", "--", "sudo", - "env", - "-i", - "PATH=${PATH-}", + "/bin/sh", + "-c", + """'exec env -i PATH="${PATH-}" "$@"'""", + "sh", "nix-env", "-p", "/nix/var/nix/profiles/system", @@ -721,9 +726,10 @@ def test_execute_nix_switch_build_target_host( *nr.process.SSH_DEFAULT_OPTS, "user@target-host", "--", - "env", - "-i", - "PATH=${PATH-}", + "/bin/sh", + "-c", + """'exec env -i PATH="${PATH-}" "$@"'""", + "sh", "test", "-d", "/run/systemd/system", @@ -738,12 +744,10 @@ def test_execute_nix_switch_build_target_host( "user@target-host", "--", "sudo", - "env", - "-i", - "PATH=${PATH-}", - "LOCALE_ARCHIVE=${LOCALE_ARCHIVE-}", - "NIXOS_NO_CHECK=${NIXOS_NO_CHECK-}", - "NIXOS_INSTALL_BOOTLOADER=0", + "/bin/sh", + "-c", + """'exec env -i PATH="${PATH-}" LOCALE_ARCHIVE="${LOCALE_ARCHIVE-}" NIXOS_NO_CHECK="${NIXOS_NO_CHECK-}" NIXOS_INSTALL_BOOTLOADER=0 "$@"'""", + "sh", *nr.nix.SWITCH_TO_CONFIGURATION_CMD_PREFIX, str(config_path / "bin/switch-to-configuration"), "switch", @@ -820,9 +824,10 @@ def test_execute_nix_switch_flake_target_host( "user@localhost", "--", "sudo", - "env", - "-i", - "PATH=${PATH-}", + "/bin/sh", + "-c", + """'exec env -i PATH="${PATH-}" "$@"'""", + "sh", "nix-env", "-p", "/nix/var/nix/profiles/system", @@ -838,9 +843,10 @@ def test_execute_nix_switch_flake_target_host( *nr.process.SSH_DEFAULT_OPTS, "user@localhost", "--", - "env", - "-i", - "PATH=${PATH-}", + "/bin/sh", + "-c", + """'exec env -i PATH="${PATH-}" "$@"'""", + "sh", "test", "-d", "/run/systemd/system", @@ -855,12 +861,10 @@ def test_execute_nix_switch_flake_target_host( "user@localhost", "--", "sudo", - "env", - "-i", - "PATH=${PATH-}", - "LOCALE_ARCHIVE=${LOCALE_ARCHIVE-}", - "NIXOS_NO_CHECK=${NIXOS_NO_CHECK-}", - "NIXOS_INSTALL_BOOTLOADER=0", + "/bin/sh", + "-c", + """'exec env -i PATH="${PATH-}" LOCALE_ARCHIVE="${LOCALE_ARCHIVE-}" NIXOS_NO_CHECK="${NIXOS_NO_CHECK-}" NIXOS_INSTALL_BOOTLOADER=0 "$@"'""", + "sh", *nr.nix.SWITCH_TO_CONFIGURATION_CMD_PREFIX, str(config_path / "bin/switch-to-configuration"), "switch", @@ -936,9 +940,10 @@ def test_execute_nix_switch_flake_build_host( *nr.process.SSH_DEFAULT_OPTS, "user@localhost", "--", - "env", - "-i", - "PATH=${PATH-}", + "/bin/sh", + "-c", + """'exec env -i PATH="${PATH-}" "$@"'""", + "sh", "nix", "--extra-experimental-features", "'nix-command flakes'", @@ -1148,9 +1153,10 @@ def test_execute_build_dry_run_build_and_target_remote( *nr.process.SSH_DEFAULT_OPTS, "user@build-host", "--", - "env", - "-i", - "PATH=${PATH-}", + "/bin/sh", + "-c", + """'exec env -i PATH="${PATH-}" "$@"'""", + "sh", "nix", "--extra-experimental-features", "'nix-command flakes'", @@ -1383,9 +1389,10 @@ def test_execute_switch_store_path_target_host( *nr.process.SSH_DEFAULT_OPTS, "user@remote-host", "--", - "env", - "-i", - "PATH=${PATH-}", + "/bin/sh", + "-c", + """'exec env -i PATH="${PATH-}" "$@"'""", + "sh", "test", "-f", str(config_path / "nixos-version"), @@ -1400,9 +1407,10 @@ def test_execute_switch_store_path_target_host( "user@remote-host", "--", "sudo", - "env", - "-i", - "PATH=${PATH-}", + "/bin/sh", + "-c", + """'exec env -i PATH="${PATH-}" "$@"'""", + "sh", "nix-env", "-p", "/nix/var/nix/profiles/system", @@ -1418,9 +1426,10 @@ def test_execute_switch_store_path_target_host( *nr.process.SSH_DEFAULT_OPTS, "user@remote-host", "--", - "env", - "-i", - "PATH=${PATH-}", + "/bin/sh", + "-c", + """'exec env -i PATH="${PATH-}" "$@"'""", + "sh", "test", "-d", "/run/systemd/system", @@ -1435,12 +1444,10 @@ def test_execute_switch_store_path_target_host( "user@remote-host", "--", "sudo", - "env", - "-i", - "PATH=${PATH-}", - "LOCALE_ARCHIVE=${LOCALE_ARCHIVE-}", - "NIXOS_NO_CHECK=${NIXOS_NO_CHECK-}", - "NIXOS_INSTALL_BOOTLOADER=0", + "/bin/sh", + "-c", + """'exec env -i PATH="${PATH-}" LOCALE_ARCHIVE="${LOCALE_ARCHIVE-}" NIXOS_NO_CHECK="${NIXOS_NO_CHECK-}" NIXOS_INSTALL_BOOTLOADER=0 "$@"'""", + "sh", *nr.nix.SWITCH_TO_CONFIGURATION_CMD_PREFIX, str(config_path / "bin/switch-to-configuration"), "switch", diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_process.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_process.py index 858850f569a8..a3c31d65c69d 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_process.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_process.py @@ -7,6 +7,26 @@ import nixos_rebuild.models as m import nixos_rebuild.process as p +def test_remote_shell_script() -> None: + assert p._remote_shell_script({"PATH": p.PRESERVE_ENV}) == ( + '''exec env -i PATH="${PATH-}" "$@"''' + ) + assert p._remote_shell_script( + { + "PATH": p.PRESERVE_ENV, + "LOCALE_ARCHIVE": p.PRESERVE_ENV, + "NIXOS_NO_CHECK": p.PRESERVE_ENV, + "NIXOS_INSTALL_BOOTLOADER": "0", + } + ) == ( + """exec env -i PATH="${PATH-}" LOCALE_ARCHIVE="${LOCALE_ARCHIVE-}" """ + '''NIXOS_NO_CHECK="${NIXOS_NO_CHECK-}" NIXOS_INSTALL_BOOTLOADER=0 "$@"''' + ) + assert p._remote_shell_script({"PATH": p.PRESERVE_ENV, "FOO": "some value"}) == ( + '''exec env -i PATH="${PATH-}" FOO='some value' "$@"''' + ) + + @patch.dict(p.os.environ, {"PATH": "/path/to/bin"}, clear=True) @patch("subprocess.run", autospec=True) def test_run(mock_run: Any) -> None: @@ -57,9 +77,10 @@ def test_run(mock_run: Any) -> None: *p.SSH_DEFAULT_OPTS, "user@localhost", "--", - "env", - "-i", - "PATH=${PATH-}", + "/bin/sh", + "-c", + """'exec env -i PATH="${PATH-}" "$@"'""", + "sh", "test", "--with", "'some flags'", @@ -89,10 +110,10 @@ def test_run(mock_run: Any) -> None: "sudo", "--prompt=", "--stdin", - "env", - "-i", - "PATH=${PATH-}", - "FOO=bar", + "/bin/sh", + "-c", + """'exec env -i PATH="${PATH-}" FOO=bar "$@"'""", + "sh", "test", "--with", "flags", @@ -221,9 +242,10 @@ def test_custom_sudo_args(mock_run: Any) -> None: "--custom", "foo", "--args", - "env", - "-i", - "PATH=${PATH-}", + "/bin/sh", + "-c", + """'exec env -i PATH="${PATH-}" "$@"'""", + "sh", "test", ], check=False,