From 351ce1414a352a72c8a0f5cf382248c6e5eb963b Mon Sep 17 00:00:00 2001 From: RTUnreal Date: Tue, 25 Mar 2025 22:40:02 +0100 Subject: [PATCH 1/4] clan_cli: fix support for non-root deployment user --- pkgs/clan-cli/clan_cli/machines/update.py | 2 ++ pkgs/clan-cli/clan_cli/ssh/upload.py | 42 +++++++---------------- 2 files changed, 14 insertions(+), 30 deletions(-) diff --git a/pkgs/clan-cli/clan_cli/machines/update.py b/pkgs/clan-cli/clan_cli/machines/update.py index e9c9a0304..3300df038 100644 --- a/pkgs/clan-cli/clan_cli/machines/update.py +++ b/pkgs/clan-cli/clan_cli/machines/update.py @@ -185,6 +185,7 @@ def deploy_machines(machines: list[Machine]) -> None: test_cmd, RunOpts(msg_color=MsgColor(stderr=AnsiColor.DEFAULT)), extra_env=env, + become_root=True, ) # retry nixos-rebuild switch if the first attempt failed @@ -193,6 +194,7 @@ def deploy_machines(machines: list[Machine]) -> None: switch_cmd, RunOpts(msg_color=MsgColor(stderr=AnsiColor.DEFAULT)), extra_env=env, + become_root=True, ) with AsyncRuntime() as runtime: diff --git a/pkgs/clan-cli/clan_cli/ssh/upload.py b/pkgs/clan-cli/clan_cli/ssh/upload.py index 207ec49b2..370c72bcf 100644 --- a/pkgs/clan-cli/clan_cli/ssh/upload.py +++ b/pkgs/clan-cli/clan_cli/ssh/upload.py @@ -55,44 +55,26 @@ def upload( with local_src.open("rb") as f: tar.addfile(tarinfo, f) + priviledge_escalation = [] + if host.user != "root": + priviledge_escalation = ["sudo", "--"] + if local_src.is_dir(): cmd = [ *host.ssh_cmd(), - "rm", - "-r", - str(remote_dest), - ";", - "mkdir", - "-m", - f"{dir_mode:o}", - "-p", - str(remote_dest), - "&&", - "tar", - "-C", - str(remote_dest), - "-xzf", - "-", + "--", + *priviledge_escalation, + "bash", "-c", "exec \"$@\"", "--", + f"rm -r {remote_dest!s} ; mkdir -m {dir_mode:o} -p {str(remote_dest)} && tar -C {str(remote_dest)} -xzf -", ] else: # For single file, extract to parent directory and ensure correct name cmd = [ *host.ssh_cmd(), - "rm", - "-f", - str(remote_dest), - ";", - "mkdir", - "-m", - f"{dir_mode:o}", - "-p", - str(remote_dest.parent), - "&&", - "tar", - "-C", - str(remote_dest.parent), - "-xzf", - "-", + "--", + *priviledge_escalation, + "bash", "-c", "exec \"$@\"", "--", + f"rm -f {str(remote_dest)} ; mkdir -m {dir_mode:o} -p {str(remote_dest.parent)} && tar -C {str(remote_dest.parent)} -xzf -", ] # TODO accept `input` to be an IO object instead of bytes so that we don't have to read the tarfile into memory. From d1a79653fe2e9caa13aa479c1a27dc9e13cd98c8 Mon Sep 17 00:00:00 2001 From: Qubasa Date: Wed, 26 Mar 2025 18:35:20 +0100 Subject: [PATCH 2/4] checks/installation-without-system: modify to install through normal user instead of root --- .../flake-module.nix | 13 ++++-- pkgs/clan-cli/clan_cli/machines/hardware.py | 5 +++ pkgs/clan-cli/clan_cli/ssh/upload.py | 40 +++++++++++++++++-- 3 files changed, 50 insertions(+), 8 deletions(-) diff --git a/checks/installation-without-system/flake-module.nix b/checks/installation-without-system/flake-module.nix index e47e4283a..ac6e8d7e6 100644 --- a/checks/installation-without-system/flake-module.nix +++ b/checks/installation-without-system/flake-module.nix @@ -165,7 +165,6 @@ (modulesPath + "/../tests/common/auto-format-root-device.nix") ]; services.openssh.enable = true; - users.users.root.openssh.authorizedKeys.keyFiles = [ ../lib/ssh/pubkey ]; system.nixos.variant_id = "installer"; environment.systemPackages = [ pkgs.nixos-facter ]; virtualisation.emptyDiskImages = [ 512 ]; @@ -184,6 +183,12 @@ "flakes" ]; }; + users.users.nonrootuser = { + isNormalUser = true; + openssh.authorizedKeys.keyFiles = [ ../lib/ssh/pubkey ]; + extraGroups = [ "wheel" ]; + }; + security.sudo.wheelNeedsPassword = false; system.extraDependencies = dependencies; }; nodes.client = { @@ -211,14 +216,14 @@ installer.start() client.succeed("${pkgs.coreutils}/bin/install -Dm 600 ${../lib/ssh/privkey} /root/.ssh/id_ed25519") - client.wait_until_succeeds("timeout 2 ssh -o StrictHostKeyChecking=accept-new -v root@installer hostname") + client.wait_until_succeeds("timeout 2 ssh -o StrictHostKeyChecking=accept-new -v nonrootuser@installer hostname") client.succeed("cp -r ${../..} test-flake && chmod -R +w test-flake") client.fail("test -f test-flake/machines/test-install-machine-without-system/hardware-configuration.nix") client.fail("test -f test-flake/machines/test-install-machine-without-system/facter.json") - client.succeed("clan machines update-hardware-config --flake test-flake test-install-machine-without-system root@installer >&2") + client.succeed("clan machines update-hardware-config --flake test-flake test-install-machine-without-system nonrootuser@installer >&2") client.succeed("test -f test-flake/machines/test-install-machine-without-system/facter.json") client.succeed("rm test-flake/machines/test-install-machine-without-system/facter.json") - client.succeed("clan machines install --debug --flake test-flake --yes test-install-machine-without-system --target-host root@installer --update-hardware-config nixos-facter >&2") + client.succeed("clan machines install --debug --flake test-flake --yes test-install-machine-without-system --target-host nonrootuser@installer --update-hardware-config nixos-facter >&2") try: installer.shutdown() except BrokenPipeError: diff --git a/pkgs/clan-cli/clan_cli/machines/hardware.py b/pkgs/clan-cli/clan_cli/machines/hardware.py index 6074e889e..6835cc725 100644 --- a/pkgs/clan-cli/clan_cli/machines/hardware.py +++ b/pkgs/clan-cli/clan_cli/machines/hardware.py @@ -135,6 +135,11 @@ def generate_machine_hardware_info(opts: HardwareGenerateOptions) -> HardwareCon ] host = machine.target_host + + # HACK: to make non-root user work + if host.user != "root": + config_command.insert(0, "sudo") + cmd = nix_shell( [ "nixpkgs#openssh", diff --git a/pkgs/clan-cli/clan_cli/ssh/upload.py b/pkgs/clan-cli/clan_cli/ssh/upload.py index 370c72bcf..f7ed5e74e 100644 --- a/pkgs/clan-cli/clan_cli/ssh/upload.py +++ b/pkgs/clan-cli/clan_cli/ssh/upload.py @@ -64,8 +64,24 @@ def upload( *host.ssh_cmd(), "--", *priviledge_escalation, - "bash", "-c", "exec \"$@\"", "--", - f"rm -r {remote_dest!s} ; mkdir -m {dir_mode:o} -p {str(remote_dest)} && tar -C {str(remote_dest)} -xzf -", + "bash", + "-c", + 'exec "$@"', + "--", + "rm", + "-r", + str(remote_dest), + "mkdir", + "-m", + f"{dir_mode:o}", + "-p", + str(remote_dest), + "&&", + "tar", + "-C", + str(remote_dest), + "-xzf", + "-", ] else: # For single file, extract to parent directory and ensure correct name @@ -73,8 +89,24 @@ def upload( *host.ssh_cmd(), "--", *priviledge_escalation, - "bash", "-c", "exec \"$@\"", "--", - f"rm -f {str(remote_dest)} ; mkdir -m {dir_mode:o} -p {str(remote_dest.parent)} && tar -C {str(remote_dest.parent)} -xzf -", + "bash", + "-c", + 'exec "$@"', + "--", + "rm", + "-r", + str(remote_dest), + "mkdir", + "-m", + f"{dir_mode:o}", + "-p", + str(remote_dest.parent), + "&&", + "tar", + "-C", + str(remote_dest.parent), + "-xzf", + "-", ] # TODO accept `input` to be an IO object instead of bytes so that we don't have to read the tarfile into memory. From 064edf61ef5bd5a628ec54cfd824d2da93ba1da6 Mon Sep 17 00:00:00 2001 From: Qubasa Date: Thu, 27 Mar 2025 17:04:35 +0100 Subject: [PATCH 3/4] test_secrets_upload: Don't prepend sudo inside test; Improve secret upload test --- pkgs/clan-cli/clan_cli/ssh/upload.py | 73 ++++++---------------- pkgs/clan-cli/default.nix | 2 + pkgs/clan-cli/tests/test_secrets_upload.py | 15 ++++- 3 files changed, 33 insertions(+), 57 deletions(-) diff --git a/pkgs/clan-cli/clan_cli/ssh/upload.py b/pkgs/clan-cli/clan_cli/ssh/upload.py index f7ed5e74e..ec22df73f 100644 --- a/pkgs/clan-cli/clan_cli/ssh/upload.py +++ b/pkgs/clan-cli/clan_cli/ssh/upload.py @@ -1,21 +1,28 @@ import tarfile from pathlib import Path +from shlex import quote from tempfile import TemporaryDirectory from clan_cli.cmd import Log, RunOpts from clan_cli.cmd import run as run_local +from clan_cli.errors import ClanError from clan_cli.ssh.host import Host def upload( host: Host, - local_src: Path, # must be a directory + local_src: Path, remote_dest: Path, # must be a directory file_user: str = "root", file_group: str = "root", dir_mode: int = 0o700, file_mode: int = 0o400, ) -> None: + # Check if the remote destination is at least 3 directories deep + if len(remote_dest.parts) < 3: + msg = f"The remote destination must be at least 3 directories deep. Got: {remote_dest}. Reason: The directory will be deleted with 'rm -rf'." + raise ClanError(msg) + # Create the tarball from the temporary directory with TemporaryDirectory(prefix="facts-upload-") as tardir: tar_path = Path(tardir) / "upload.tar.gz" @@ -55,64 +62,22 @@ def upload( with local_src.open("rb") as f: tar.addfile(tarinfo, f) - priviledge_escalation = [] - if host.user != "root": - priviledge_escalation = ["sudo", "--"] + sudo = "" + if host.user != "root" and os.environ.get("IN_PYTEST") is None: + sudo = "sudo -- " - if local_src.is_dir(): - cmd = [ - *host.ssh_cmd(), - "--", - *priviledge_escalation, - "bash", - "-c", - 'exec "$@"', - "--", - "rm", - "-r", - str(remote_dest), - "mkdir", - "-m", - f"{dir_mode:o}", - "-p", - str(remote_dest), - "&&", - "tar", - "-C", - str(remote_dest), - "-xzf", - "-", - ] - else: - # For single file, extract to parent directory and ensure correct name - cmd = [ - *host.ssh_cmd(), - "--", - *priviledge_escalation, - "bash", - "-c", - 'exec "$@"', - "--", - "rm", - "-r", - str(remote_dest), - "mkdir", - "-m", - f"{dir_mode:o}", - "-p", - str(remote_dest.parent), - "&&", - "tar", - "-C", - str(remote_dest.parent), - "-xzf", - "-", - ] + cmd = "rm -rf $0 && mkdir -m $1 -p $0 && tar -C $0 -xzf -" # TODO accept `input` to be an IO object instead of bytes so that we don't have to read the tarfile into memory. with tar_path.open("rb") as f: run_local( - cmd, + [ + *host.ssh_cmd(), + "--", + f"{sudo}bash -c {quote(cmd)}", + str(remote_dest), + f"{dir_mode:o}", + ], RunOpts( input=f.read(), log=Log.BOTH, diff --git a/pkgs/clan-cli/default.nix b/pkgs/clan-cli/default.nix index a3214cb2d..101f872aa 100644 --- a/pkgs/clan-cli/default.nix +++ b/pkgs/clan-cli/default.nix @@ -147,6 +147,7 @@ pythonRuntime.pkgs.buildPythonApplication { cd ./src export NIX_STATE_DIR=$TMPDIR/nix IN_NIX_SANDBOX=1 PYTHONWARNINGS=error + export IN_PYTEST=1 # required to prevent concurrent 'nix flake lock' operations export CLAN_TEST_STORE=$TMPDIR/store @@ -198,6 +199,7 @@ pythonRuntime.pkgs.buildPythonApplication { export NIX_STATE_DIR=$TMPDIR/nix export IN_NIX_SANDBOX=1 export PYTHONWARNINGS=error + export IN_PYTEST=1 export CLAN_TEST_STORE=$TMPDIR/store # required to prevent concurrent 'nix flake lock' operations export LOCK_NIX=$TMPDIR/nix_lock diff --git a/pkgs/clan-cli/tests/test_secrets_upload.py b/pkgs/clan-cli/tests/test_secrets_upload.py index 2556fb863..8b3ed7930 100644 --- a/pkgs/clan-cli/tests/test_secrets_upload.py +++ b/pkgs/clan-cli/tests/test_secrets_upload.py @@ -26,6 +26,17 @@ def test_secrets_upload( monkeypatch.chdir(str(flake.path)) monkeypatch.setenv("SOPS_AGE_KEY", age_keys[0].privkey) + sops_dir = flake.path / "facts" + + # the flake defines this path as the location where the sops key should be installed + sops_key = sops_dir / "key.txt" + sops_key2 = sops_dir / "key2.txt" + + # Create old state, which should be cleaned up + sops_dir.mkdir() + sops_key.write_text("OLD STATE") + sops_key2.write_text("OLD STATE2") + cli.run( [ "secrets", @@ -56,8 +67,6 @@ def test_secrets_upload( cli.run(["facts", "upload", "--flake", str(flake_path), "vm1"]) - # the flake defines this path as the location where the sops key should be installed - sops_key = flake.path / "facts" / "key.txt" - assert sops_key.exists() assert sops_key.read_text() == age_keys[0].privkey + assert not sops_key2.exists() From de740cf686ff119c79388f0f7873d5b2c97bb5ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Fri, 28 Mar 2025 17:46:44 +0100 Subject: [PATCH 4/4] tests: add fake_sudo to sshd fixture This allows to use the same code for both testing and real-world. --- pkgs/clan-cli/clan_cli/ssh/upload.py | 2 +- pkgs/clan-cli/default.nix | 2 -- pkgs/clan-cli/tests/sshd.py | 19 +++++++++++++------ 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/pkgs/clan-cli/clan_cli/ssh/upload.py b/pkgs/clan-cli/clan_cli/ssh/upload.py index ec22df73f..8fd798677 100644 --- a/pkgs/clan-cli/clan_cli/ssh/upload.py +++ b/pkgs/clan-cli/clan_cli/ssh/upload.py @@ -63,7 +63,7 @@ def upload( tar.addfile(tarinfo, f) sudo = "" - if host.user != "root" and os.environ.get("IN_PYTEST") is None: + if host.user != "root": sudo = "sudo -- " cmd = "rm -rf $0 && mkdir -m $1 -p $0 && tar -C $0 -xzf -" diff --git a/pkgs/clan-cli/default.nix b/pkgs/clan-cli/default.nix index 101f872aa..a3214cb2d 100644 --- a/pkgs/clan-cli/default.nix +++ b/pkgs/clan-cli/default.nix @@ -147,7 +147,6 @@ pythonRuntime.pkgs.buildPythonApplication { cd ./src export NIX_STATE_DIR=$TMPDIR/nix IN_NIX_SANDBOX=1 PYTHONWARNINGS=error - export IN_PYTEST=1 # required to prevent concurrent 'nix flake lock' operations export CLAN_TEST_STORE=$TMPDIR/store @@ -199,7 +198,6 @@ pythonRuntime.pkgs.buildPythonApplication { export NIX_STATE_DIR=$TMPDIR/nix export IN_NIX_SANDBOX=1 export PYTHONWARNINGS=error - export IN_PYTEST=1 export CLAN_TEST_STORE=$TMPDIR/store # required to prevent concurrent 'nix flake lock' operations export LOCK_NIX=$TMPDIR/nix_lock diff --git a/pkgs/clan-cli/tests/sshd.py b/pkgs/clan-cli/tests/sshd.py index 82c9c1251..9245dd96b 100644 --- a/pkgs/clan-cli/tests/sshd.py +++ b/pkgs/clan-cli/tests/sshd.py @@ -57,7 +57,10 @@ def sshd_config(test_root: Path) -> Iterator[SshdConfig]: ) config = tmpdir / "sshd_config" config.write_text(content) - login_shell = tmpdir / "shell" + bin_path = tmpdir / "bin" + login_shell = bin_path / "shell" + fake_sudo = bin_path / "sudo" + login_shell.parent.mkdir(parents=True) bash = shutil.which("bash") path = os.environ["PATH"] @@ -65,19 +68,23 @@ def sshd_config(test_root: Path) -> Iterator[SshdConfig]: login_shell.write_text( f"""#!{bash} +set -x if [[ -f /etc/profile ]]; then source /etc/profile fi -if [[ -n "$REALPATH" ]]; then - export PATH="$REALPATH:${path}" -else - export PATH="${path}" -fi +export PATH="{bin_path}:{path}" exec {bash} -l "${{@}}" """ ) login_shell.chmod(0o755) + fake_sudo.write_text( + f"""#!{bash} +exec "${{@}}" + """ + ) + fake_sudo.chmod(0o755) + lib_path = None extension = ".so"