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

This reverts commit 8a849eb90f, reversing
changes made to 3b5c22ebcf.

pkgs/clan-cli/clan_cli/custom_logger.py

@@ -2,7 +2,6 @@ import inspect
 import logging
 import os
 import sys
-import termios
 from pathlib import Path
 from typing import Any
 
@@ -75,16 +74,7 @@ class PrefixFormatter(logging.Formatter):
         if self.trace_prints:
             format_str += f"\nSource: {filepath}:%(lineno)d::%(funcName)s\n"
 
-        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
+        return logging.Formatter(format_str).format(record)
 
     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 Log, MsgColor, RunOpts, run
+from clan_cli.cmd import MsgColor, RunOpts, run
 from clan_cli.colors import AnsiColor
 from clan_cli.completions import (
     add_dynamic_completer,
@@ -158,7 +158,6 @@ 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 += [
@@ -174,7 +173,6 @@ 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]
@@ -182,10 +180,7 @@ 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), log=Log.BOTH
-            ),
-            tty=needs_tty_for_sudo,
+            RunOpts(check=False, msg_color=MsgColor(stderr=AnsiColor.DEFAULT)),
             extra_env=env,
             become_root=become_root,
         )

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

@@ -5,6 +5,8 @@ import os
 import shlex
 import socket
 import subprocess
+import errno
+import stat
 from dataclasses import dataclass, field
 from pathlib import Path
 from shlex import quote
@@ -38,27 +40,35 @@ class Host:
     ssh_options: dict[str, str] = field(default_factory=dict)
     tor_socks: bool = False
 
+    def setup_control_master(self) -> None:
+        home = Path.home()
+        if not home.exists():
+            return
+        control_path = home / ".ssh"
+        try:
+            if not stat.S_ISDIR(control_path.stat().st_mode):
+                return
+        except OSError as e:
+            if e.errno == errno.ENOENT:
+                try:
+                    control_path.mkdir(exist_ok=True)
+                except OSError:
+                    return
+            else:
+                return
+
+        self.ssh_options["ControlMaster"] = "auto"
+        # Can we make this a temporary directory?
+        self.ssh_options["ControlPath"] = str(control_path / "clan-%h-%p-%r")
+        # We use a short ttl because we want to mainly re-use the connection during the cli run
+        self.ssh_options["ControlPersist"] = "1m"
+
     def __post_init__(self) -> None:
         if not self.command_prefix:
             self.command_prefix = self.host
         if not self.user:
             self.user = "root"
-        home = Path.home()
-        if home.exists() and os.access(home, os.W_OK):
-            control_path = home / ".ssh"
-            if not control_path.exists():
-                try:
-                    control_path.mkdir(exist_ok=True)
-                except OSError:
-                    pass
-                else:
-                    self.ssh_options["ControlMaster"] = "auto"
-                    # Can we make this a temporary directory?
-                    self.ssh_options["ControlPath"] = str(
-                        control_path / "clan-%h-%p-%r"
-                    )
-                    # We use a short ttl because we want to mainly re-use the connection during the cli run
-                    self.ssh_options["ControlPersist"] = "1m"
+        self.setup_control_master()
 
     def __str__(self) -> str:
         return self.target

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

@@ -2,84 +2,13 @@ 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,
@@ -160,22 +89,33 @@ 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:
-            if host.user == "root":
-                unpack_archive_as_root(
-                    host,
-                    f,
-                    local_src,
-                    remote_dest,
-                    dir_mode=dir_mode,
-                )
-            else:
-                # For sudo we need to split the upload into two steps
-                unpack_archive_as_user(
-                    host,
-                    f,
-                    local_src,
-                    remote_dest,
-                    dir_mode=dir_mode,
-                )
+            run_local(
+                [
+                    *host.ssh_cmd(),
+                    "--",
+                    f"{sudo}bash -c {quote(cmd)}",
+                    str(remote_dest),
+                    f"{dir_mode:o}",
+                ],
+                RunOpts(
+                    input=f.read(),
+                    log=Log.BOTH,
+                    prefix=host.command_prefix,
+                    needs_user_terminal=True,
+                ),
+            )

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

@@ -80,16 +80,6 @@ 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,10 +982,7 @@ 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), (
-            f"expected /PGP key.+is already set/ =~ {output.err}"
-        )
+        assert re.match(r"PGP key.+is already set", output.err) is not None
 
         with capture_output as output:
             cli.run(