fix(connectors.local): fragile sudo handling resulting in faillocks and premature errors.

src/pyinfra/connectors/util.py

@@ -1,18 +1,18 @@
 from __future__ import annotations
 
+from collections.abc import Callable, Iterable
 from dataclasses import dataclass
 from getpass import getpass
 from queue import Queue
-from gevent.subprocess import PIPE, Popen
 from typing import TYPE_CHECKING
-from collections.abc import Callable, Iterable
 
 import gevent
+from gevent.subprocess import PIPE, Popen
 
 from pyinfra import logger
-from pyinfra.api.output import echo, format_text
 from pyinfra.api import MaskString, QuoteString, StringCommand
 from pyinfra.api.exceptions import PyinfraError
+from pyinfra.api.output import echo, format_text
 from pyinfra.api.util import memoize
 
 if TYPE_CHECKING:
@@ -29,6 +29,11 @@
 temp=$(mktemp "${{TMPDIR:={0}}}/pyinfra-sudo-askpass-XXXXXXXXXXXX")
 cat >"$temp"<<'__EOF__'
 #!/bin/sh
+if [ -f "$0.${{PPID}}.called" ]; then
+    echo "sudo: 1 incorrect password attempt" >&2
+    exit 1
+fi
+touch "$0.${{PPID}}.called"
 printf '%s\n' "${1}"
 __EOF__
 chmod 755 "$temp"
@@ -199,19 +204,34 @@ def execute_command_with_sudo_retry(
 ) -> tuple[int, CommandOutput]:
     return_code, output = execute_command()
 
+    attempts = 0
     # If we failed look for a sudo password prompt line and re-submit using the sudo password. Look
     # at all lines here in case anything else gets printed, eg in:
     # https://github.com/pyinfra-dev/pyinfra/issues/1292
-    if return_code != 0 and output and output.combined_lines:
+    while return_code != 0 and output and output.combined_lines:
+        requires_password = False
         for line in reversed(output.combined_lines):
-            if line.line.strip() == "sudo: a password is required":
-                # If we need a password, ask the user for it and attach to the host
-                # internal connector data for use when executing future commands.
-                sudo_password = getpass(f"{host.print_prefix}sudo password: ")
-                host.connector_data["prompted_sudo_password"] = sudo_password
-                return_code, output = execute_command()
+            line_stripped = line.line.strip()
+
+            if line_stripped.startswith("sudo:") and "incorrect password attempt" in line_stripped:
                 break
 
+            if line_stripped == "sudo: a password is required":
+                if attempts != 0:
+                    break
+                requires_password = True
+                break
+
+        if requires_password:
+            # If we need a password, ask the user for it and attach to the host
+            # internal connector data for use when executing future commands.
+            sudo_password = getpass(f"{host.print_prefix}sudo password: ")
+            host.connector_data["prompted_sudo_password"] = sudo_password
+            return_code, output = execute_command()
+            attempts += 1
+        else:
+            break
+
     return return_code, output
 
 
@@ -237,15 +257,23 @@ def remove_any_sudo_askpass_file(host) -> None:
     sudo_askpass_path = host.connector_data.get("sudo_askpass_path")
     if sudo_askpass_path:
         try:
-            host.run_shell_command(StringCommand("rm", "-f", QuoteString(sudo_askpass_path)))
+            wildcard_path = StringCommand(
+                QuoteString(sudo_askpass_path), ".*.called", _separator=""
+            )
+            host.run_shell_command(
+                StringCommand("rm", "-f", QuoteString(sudo_askpass_path), wildcard_path)
+            )
         except Exception as e:
             logger.debug("Could not remove sudo askpass file %s: %s", sudo_askpass_path, e)
         host.connector_data["sudo_askpass_path"] = None
 
     su_askpass_path = host.connector_data.get("su_askpass_path")
     if su_askpass_path:
         try:
-            host.run_shell_command(StringCommand("rm", "-f", QuoteString(su_askpass_path)))
+            wildcard_path = StringCommand(QuoteString(su_askpass_path), ".*.called", _separator="")
+            host.run_shell_command(
+                StringCommand("rm", "-f", QuoteString(su_askpass_path), wildcard_path)
+            )
         except Exception as e:
             logger.debug("Could not remove su askpass file %s: %s", su_askpass_path, e)
         host.connector_data["su_askpass_path"] = None
@@ -388,7 +416,7 @@ def make_unix_command(
     command_bits: list[str | StringCommand | QuoteString] = []
 
     if _doas:
-        command_bits.extend(["doas", "-n"])
+        command_bits.extend(["env", "LC_ALL=C", "doas", "-n"])
 
         if _doas_user:
             command_bits.extend(["-u", QuoteString(_doas_user)])
@@ -403,12 +431,15 @@ def make_unix_command(
         command_bits.extend(
             [
                 "env",
+                "LC_ALL=C",
                 StringCommand("SUDO_ASKPASS=", QuoteString(_sudo_askpass_path), _separator=""),
                 MaskString(
                     f"{SUDO_ASKPASS_ENV_VAR}={StringCommand(QuoteString(_sudo_password)).get_raw_value()}"
                 ),
             ],
         )
+    elif _sudo:
+        command_bits.extend(["env", "LC_ALL=C"])
 
     if _sudo:
         command_bits.extend(["sudo", "-H"])
@@ -432,13 +463,16 @@ def make_unix_command(
             command_bits.extend(
                 [
                     "env",
+                    "LC_ALL=C",
                     MaskString(
                         f"{SU_ASKPASS_ENV_VAR}={StringCommand(QuoteString(_su_password)).get_raw_value()}"
                     ),
                     QuoteString(_su_askpass_path),
                     "|",
                 ],
             )
+        elif not _sudo and not _doas:
+            command_bits.extend(["env", "LC_ALL=C"])
 
         command_bits.append("su")
 

tests/test_connectors/test_ssh.py

@@ -7,8 +7,8 @@
 from pyinfra.api import Config, Host, MaskString, State, StringCommand
 from pyinfra.api.connect import connect_all
 from pyinfra.api.exceptions import ConnectError, PyinfraError
-from pyinfra.context import ctx_state
 from pyinfra.connectors import ssh
+from pyinfra.context import ctx_state
 
 from ..util import make_inventory
 
@@ -522,11 +522,13 @@ def test_run_shell_command_sudo_password_automatic_prompt(
         status, output = out
         assert status is True
 
-        fake_ssh.exec_command.assert_any_call(("sudo -H -n sh -c 'echo Šablony'"), get_pty=False)
+        fake_ssh.exec_command.assert_any_call(
+            ("env LC_ALL=C sudo -H -n sh -c 'echo Šablony'"), get_pty=False
+        )
 
         fake_ssh.exec_command.assert_called_with(
             (
-                "env SUDO_ASKPASS=/tmp/pyinfra-sudo-askpass-XXXXXXXXXXXX "
+                "env LC_ALL=C SUDO_ASKPASS=/tmp/pyinfra-sudo-askpass-XXXXXXXXXXXX "
                 "PYINFRA_SUDO_PASSWORD=password "
                 "sudo -H -A -k sh -c 'echo Šablony'"
             ),
@@ -585,11 +587,13 @@ def test_run_shell_command_sudo_password_automatic_prompt_with_special_chars_in_
         status, output = out
         assert status is True
 
-        fake_ssh.exec_command.assert_any_call(("sudo -H -n sh -c 'echo Šablony'"), get_pty=False)
+        fake_ssh.exec_command.assert_any_call(
+            ("env LC_ALL=C sudo -H -n sh -c 'echo Šablony'"), get_pty=False
+        )
 
         fake_ssh.exec_command.assert_called_with(
             (
-                "env SUDO_ASKPASS=/tmp/pyinfra-sudo-askpass-XXXXXXXXXXXX "
+                "env LC_ALL=C SUDO_ASKPASS=/tmp/pyinfra-sudo-askpass-XXXXXXXXXXXX "
                 """PYINFRA_SUDO_PASSWORD='p@ss'"'"'word'"'"';' """
                 "sudo -H -A -k sh -c 'echo Šablony'"
             ),
@@ -631,7 +635,7 @@ def test_run_shell_command_retry_for_sudo_password(
         assert out[0] is True
         assert fake_getpass.called
         fake_ssh.exec_command.assert_called_with(
-            "env SUDO_ASKPASS=/tmp/pyinfra-sudo-askpass-XXXXXXXXXXXX "
+            "env LC_ALL=C SUDO_ASKPASS=/tmp/pyinfra-sudo-askpass-XXXXXXXXXXXX "
             "PYINFRA_SUDO_PASSWORD=PASSWORD sudo -H -A -k sh -c 'echo hi'",
             get_pty=False,
         )
@@ -704,7 +708,7 @@ def test_put_file_sudo(self, fake_sftp_client, fake_ssh_client):
                 ),
                 mock.call(
                     (
-                        "sudo -H -n -u ubuntu sh -c 'cp /tmp/pyinfra-de01e82cb691e8a31369da3c7c8f17341c44ac24 '\"'\"'not another file'\"'\"''"  # noqa: E501
+                        "env LC_ALL=C sudo -H -n -u ubuntu sh -c 'cp /tmp/pyinfra-de01e82cb691e8a31369da3c7c8f17341c44ac24 '\"'\"'not another file'\"'\"''"  # noqa: E501
                     ),
                     get_pty=False,
                 ),
@@ -761,7 +765,7 @@ def test_put_file_doas(self, fake_sftp_client, fake_ssh_client):
                 ),
                 mock.call(
                     (
-                        "doas -n -u ubuntu sh -c 'cp /tmp/pyinfra-de01e82cb691e8a31369da3c7c8f17341c44ac24 '\"'\"'not another file'\"'\"''"  # noqa: E501
+                        "env LC_ALL=C doas -n -u ubuntu sh -c 'cp /tmp/pyinfra-de01e82cb691e8a31369da3c7c8f17341c44ac24 '\"'\"'not another file'\"'\"''"  # noqa: E501
                     ),
                     get_pty=False,
                 ),
@@ -855,7 +859,7 @@ def test_put_file_su_user_fail_copy(self, fake_sftp_client, fake_ssh_client):
 
         fake_ssh_client().exec_command.assert_any_call(
             (
-                "su centos -c 'sh -c '\"'\"'cp "
+                "env LC_ALL=C su centos -c 'sh -c '\"'\"'cp "
                 "/tmp/pyinfra-43db9984686317089fefcf2e38de527e4cb44487 "
                 "not-another-file'\"'\"''"
             ),
@@ -967,14 +971,14 @@ def test_get_file_sudo(self, fake_sftp_client, fake_ssh_client):
             [
                 mock.call(
                     (
-                        "sudo -H -n -u ubuntu sh -c 'cp not-a-file "
+                        "env LC_ALL=C sudo -H -n -u ubuntu sh -c 'cp not-a-file "
                         "/tmp/pyinfra-e9c0d3c8ffca943daa0e75511b0a09c84b59c508 && chmod +r /tmp/pyinfra-e9c0d3c8ffca943daa0e75511b0a09c84b59c508'"  # noqa
                     ),
                     get_pty=False,
                 ),
                 mock.call(
                     (
-                        "sudo -H -n -u ubuntu sh -c 'rm -f "
+                        "env LC_ALL=C sudo -H -n -u ubuntu sh -c 'rm -f "
                         "/tmp/pyinfra-e9c0d3c8ffca943daa0e75511b0a09c84b59c508'"
                     ),
                     get_pty=False,
@@ -1018,7 +1022,7 @@ def test_get_file_sudo_copy_fail(self, fake_ssh_client):
             [
                 mock.call(
                     (
-                        "sudo -H -n -u ubuntu sh -c 'cp not-a-file "
+                        "env LC_ALL=C sudo -H -n -u ubuntu sh -c 'cp not-a-file "
                         "/tmp/pyinfra-e9c0d3c8ffca943daa0e75511b0a09c84b59c508 && chmod +r /tmp/pyinfra-e9c0d3c8ffca943daa0e75511b0a09c84b59c508'"  # noqa
                     ),
                     get_pty=False,
@@ -1059,14 +1063,14 @@ def test_get_file_sudo_remove_fail(self, fake_sftp_client, fake_ssh_client):
             [
                 mock.call(
                     (
-                        "sudo -H -n -u ubuntu sh -c 'cp not-a-file "
+                        "env LC_ALL=C sudo -H -n -u ubuntu sh -c 'cp not-a-file "
                         "/tmp/pyinfra-e9c0d3c8ffca943daa0e75511b0a09c84b59c508 && chmod +r /tmp/pyinfra-e9c0d3c8ffca943daa0e75511b0a09c84b59c508'"  # noqa
                     ),
                     get_pty=False,
                 ),
                 mock.call(
                     (
-                        "sudo -H -n -u ubuntu sh -c 'rm -f "
+                        "env LC_ALL=C sudo -H -n -u ubuntu sh -c 'rm -f "
                         "/tmp/pyinfra-e9c0d3c8ffca943daa0e75511b0a09c84b59c508'"
                     ),
                     get_pty=False,
@@ -1112,15 +1116,15 @@ def test_get_file_su_user(self, fake_sftp_client, fake_ssh_client):
             [
                 mock.call(
                     (
-                        "su centos -c 'sh -c '\"'\"'cp not-a-file "
+                        "env LC_ALL=C su centos -c 'sh -c '\"'\"'cp not-a-file "
                         "/tmp/pyinfra-e9c0d3c8ffca943daa0e75511b0a09c84b59c508 && chmod +r "
                         "/tmp/pyinfra-e9c0d3c8ffca943daa0e75511b0a09c84b59c508'\"'\"''"
                     ),
                     get_pty=False,
                 ),
                 mock.call(
                     (
-                        "su centos -c 'sh -c '\"'\"'rm -f "
+                        "env LC_ALL=C su centos -c 'sh -c '\"'\"'rm -f "
                         "/tmp/pyinfra-e9c0d3c8ffca943daa0e75511b0a09c84b59c508'\"'\"''"
                     ),
                     get_pty=False,