Merge pull request 'Fix deploying with sudo + password' (#3470) from target-host into main

Reviewed-on: https://git.clan.lol/clan/clan-core/pulls/3470

pkgs/clan-cli/clan_cli/custom_logger.py

@@ -2,6 +2,7 @@ import inspect
 import logging
 import os
 import sys
+import termios
 from pathlib import Path
 from typing import Any
 
@@ -74,7 +75,16 @@ class PrefixFormatter(logging.Formatter):
         if self.trace_prints:
             format_str += f"\nSource: {filepath}:%(lineno)d::%(funcName)s\n"
 
-        return logging.Formatter(format_str).format(record)
+        line = logging.Formatter(format_str).format(record)
+        try:
+            # Programs like sudo can set the terminal into raw mode.
+            # This means newlines are no longer translated to include a carriage return to the end.
+            # https://unix.stackexchange.com/questions/151916/why-is-this-binary-file-transferred-over-ssh-t-being-changed/151963#15196
+            if not termios.tcgetattr(sys.stdout.fileno())[3] & termios.ECHO:
+                line += "\r"
+        except Exception:  # not a tty or mocked sys.stdout
+            pass
+        return line
 
     def hostname_colorcode(self, hostname: str) -> tuple[int, int, int]:
         colorcodes = RgbColor.list_values()

pkgs/clan-cli/clan_cli/machines/update.py

@@ -9,7 +9,7 @@ import sys
 from clan_lib.api import API
 
 from clan_cli.async_run import AsyncContext, AsyncOpts, AsyncRuntime, is_async_cancelled
-from clan_cli.cmd import MsgColor, RunOpts, run
+from clan_cli.cmd import Log, MsgColor, RunOpts, run
 from clan_cli.colors import AnsiColor
 from clan_cli.completions import (
     add_dynamic_completer,
@@ -158,6 +158,7 @@ def deploy_machines(machines: list[Machine]) -> None:
         ]
 
         become_root = machine.deploy_as_root
+        needs_tty_for_sudo = become_root or machine._class_ == "darwin"
 
         if machine._class_ == "nixos":
             nix_options += [
@@ -173,6 +174,7 @@ def deploy_machines(machines: list[Machine]) -> None:
 
                 if target_host.user != "root":
                     nix_options += ["--use-remote-sudo"]
+            needs_tty_for_sudo = become_root
 
         switch_cmd = [f"{machine._class_}-rebuild", "switch", *nix_options]
         test_cmd = [f"{machine._class_}-rebuild", "test", *nix_options]
@@ -180,7 +182,10 @@ def deploy_machines(machines: list[Machine]) -> None:
         env = host.nix_ssh_env(None)
         ret = host.run(
             switch_cmd,
-            RunOpts(check=False, msg_color=MsgColor(stderr=AnsiColor.DEFAULT)),
+            RunOpts(
+                check=False, msg_color=MsgColor(stderr=AnsiColor.DEFAULT), log=Log.BOTH
+            ),
+            tty=needs_tty_for_sudo,
             extra_env=env,
             become_root=become_root,
         )

pkgs/clan-cli/clan_cli/ssh/upload.py

@@ -2,13 +2,84 @@ import tarfile
 from pathlib import Path
 from shlex import quote
 from tempfile import TemporaryDirectory
+from typing import IO
 
 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 unpack_archive_as_root(
+    host: Host, f: IO[bytes], local_src: Path, remote_dest: Path, dir_mode: int = 0o700
+) -> None:
+    if local_src.is_dir():
+        cmd = 'rm -rf "$0" && mkdir -m "$1" -p "$0" && tar -C "$0" -xzf -'
+    elif local_src.is_file():
+        cmd = 'rm -f "$0" && tar -C "$(dirname "$0")" -xzf -'
+    else:
+        msg = f"Unsupported source file type: {local_src}"
+        raise ClanError(msg)
+
+    host.run(
+        [
+            "sudo",
+            "-p",
+            f"Enter sudo password for {quote(host.host)}: ",
+            "--",
+            "bash",
+            "-c",
+            cmd,
+            str(remote_dest),
+            f"{dir_mode:o}",
+        ],
+        RunOpts(
+            input=f,
+            log=Log.BOTH,
+        ),
+    )
+
+
+def unpack_archive_as_user(
+    host: Host, f: IO[bytes], local_src: Path, remote_dest: Path, dir_mode: int = 0o700
+) -> None:
+    archive = host.run(
+        ["bash", "-c", "f=$(mktemp); echo $f; cat > $f"],
+        RunOpts(
+            input=f,
+            log=Log.BOTH,
+        ),
+    ).stdout.strip()
+
+    if local_src.is_dir():
+        cmd = 'trap "rm -f $0" EXIT; rm -rf "$1" && mkdir -m "$2" -p "$1" && tar -C "$1" -xzf "$0"'
+    elif local_src.is_file():
+        cmd = 'trap "rm -f $0" EXIT; rm -f "$1" && tar -C "$(dirname "$1")" -xzf "$0"'
+    else:
+        msg = f"Unsupported source type: {local_src}"
+        raise ClanError(msg)
+
+    # We also need some sort of locks in case we have multiple prompts
+    host.run(
+        [
+            "sudo",
+            "-p",
+            f"Enter sudo password for {host.host}:\n",
+            "--",
+            "bash",
+            "-c",
+            cmd,
+            archive,
+            str(remote_dest),
+            f"{dir_mode:o}",
+        ],
+        tty=True,
+        opts=RunOpts(
+            log=Log.BOTH,
+            prefix="",
+        ),
+    )
+
+
 def upload(
     host: Host,
     local_src: Path,
@@ -89,33 +160,22 @@ def upload(
                 with local_src.open("rb") as f:
                     tar.addfile(tarinfo, f)
 
-        sudo = ""
-        if host.user != "root":
-            sudo = "sudo -- "
-
-        cmd = None
-        if local_src.is_dir():
-            cmd = 'rm -rf "$0" && mkdir -m "$1" -p "$0" && tar -C "$0" -xzf -'
-        elif local_src.is_file():
-            cmd = 'rm -f "$0" && tar -C "$(dirname "$0")" -xzf -'
-        else:
-            msg = f"Unsupported source type: {local_src}"
-            raise ClanError(msg)
-
         # 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(
+            if host.user == "root":
-                [
+                unpack_archive_as_root(
-                    *host.ssh_cmd(),
+                    host,
-                    "--",
+                    f,
-                    f"{sudo}bash -c {quote(cmd)}",
+                    local_src,
-                    str(remote_dest),
+                    remote_dest,
-                    f"{dir_mode:o}",
+                    dir_mode=dir_mode,
-                ],
+                )
-                RunOpts(
+            else:
-                    input=f.read(),
+                # For sudo we need to split the upload into two steps
-                    log=Log.BOTH,
+                unpack_archive_as_user(
-                    prefix=host.command_prefix,
+                    host,
-                    needs_user_terminal=True,
+                    f,
-                ),
+                    local_src,
-            )
+                    remote_dest,
+                    dir_mode=dir_mode,
+                )

pkgs/clan-cli/clan_cli/tests/sshd.py

@@ -80,6 +80,16 @@ exec {bash} -l "${{@}}"
 
         fake_sudo.write_text(
             f"""#!{bash}
+# skip over every sudo option
+for arg in "${{@}}"; do
+    if [[ "$arg" == "-p" ]]; then
+        shift
+        shift
+        continue
+    fi
+    break
+done
+
 exec "${{@}}"
         """
         )

pkgs/clan-cli/clan_cli/tests/test_secrets_cli.py

@@ -982,7 +982,10 @@ def test_secrets_key_generate_gpg(
                 ]
             )
         assert "age private key" not in output.out
-        assert re.match(r"PGP key.+is already set", output.err) is not None
+
+        assert re.match(r"PGP key.+is already set", output.err), (
+            f"expected /PGP key.+is already set/ =~ {output.err}"
+        )
 
         with capture_output as output:
             cli.run(