From e9742e202f6048ad2b7eb2f7a62e68de67ae6e05 Mon Sep 17 00:00:00 2001 From: johnh4098 Date: Sun, 8 Mar 2026 17:46:11 +0330 Subject: [PATCH] fix(security): pipe sudo password via stdin instead of shell cmdline --- tools/environments/base.py | 12 +++++- tools/environments/daytona.py | 15 +++++++- tools/environments/docker.py | 18 ++++++--- tools/environments/local.py | 19 +++++++-- tools/environments/modal.py | 15 +++++++- tools/environments/singularity.py | 16 ++++++-- tools/environments/ssh.py | 18 ++++++--- tools/terminal_tool.py | 64 ++++++++++++++++++++----------- 8 files changed, 132 insertions(+), 45 deletions(-) diff --git a/tools/environments/base.py b/tools/environments/base.py index 50bf3b2ad..295c84daa 100644 --- a/tools/environments/base.py +++ b/tools/environments/base.py @@ -59,8 +59,16 @@ class BaseEnvironment(ABC): # Shared helpers (eliminate duplication across backends) # ------------------------------------------------------------------ - def _prepare_command(self, command: str) -> str: - """Transform sudo commands if SUDO_PASSWORD is available.""" + def _prepare_command(self, command: str) -> tuple[str, str | None]: + """Transform sudo commands if SUDO_PASSWORD is available. + + Returns: + (transformed_command, sudo_stdin) — see _transform_sudo_command + for the full contract. Callers that drive a subprocess directly + should prepend sudo_stdin (when not None) to any stdin_data they + pass to Popen. Callers that embed stdin via heredoc (modal, + daytona) handle sudo_stdin in their own execute() method. + """ from tools.terminal_tool import _transform_sudo_command return _transform_sudo_command(command) diff --git a/tools/environments/daytona.py b/tools/environments/daytona.py index 11efa7c08..5c2204e60 100644 --- a/tools/environments/daytona.py +++ b/tools/environments/daytona.py @@ -181,7 +181,20 @@ class DaytonaEnvironment(BaseEnvironment): marker = f"HERMES_EOF_{uuid.uuid4().hex[:8]}" command = f"{command} << '{marker}'\n{stdin_data}\n{marker}" - exec_command = self._prepare_command(command) + exec_command, sudo_stdin = self._prepare_command(command) + + # Daytona sandboxes execute commands via the Daytona SDK and cannot + # pipe subprocess stdin directly the way a local Popen can. When a + # sudo password is present, use a shell-level pipe from printf so that + # the password feeds sudo -S without appearing as an echo argument + # embedded in the shell string. The password is still visible in the + # remote sandbox's command line, but it is not exposed on the user's + # local machine — which is the primary threat being mitigated. + if sudo_stdin is not None: + import shlex + exec_command = ( + f"printf '%s\\n' {shlex.quote(sudo_stdin.rstrip())} | {exec_command}" + ) effective_cwd = cwd or self.cwd or None effective_timeout = timeout or self.timeout diff --git a/tools/environments/docker.py b/tools/environments/docker.py index faf01b2a2..6b380b31a 100644 --- a/tools/environments/docker.py +++ b/tools/environments/docker.py @@ -193,10 +193,18 @@ class DockerEnvironment(BaseEnvironment): def execute(self, command: str, cwd: str = "", *, timeout: int | None = None, stdin_data: str | None = None) -> dict: - exec_command = self._prepare_command(command) + exec_command, sudo_stdin = self._prepare_command(command) work_dir = cwd or self.cwd effective_timeout = timeout or self.timeout + # Merge sudo password (if any) with caller-supplied stdin_data. + if sudo_stdin is not None and stdin_data is not None: + effective_stdin = sudo_stdin + stdin_data + elif sudo_stdin is not None: + effective_stdin = sudo_stdin + else: + effective_stdin = stdin_data + # docker exec -w doesn't expand ~, so prepend a cd into the command if work_dir == "~" or work_dir.startswith("~/"): exec_command = f"cd {work_dir} && {exec_command}" @@ -204,7 +212,7 @@ class DockerEnvironment(BaseEnvironment): assert self._inner.container_id, "Container not started" cmd = [self._inner.config.executable, "exec"] - if stdin_data is not None: + if effective_stdin is not None: cmd.append("-i") cmd.extend(["-w", work_dir]) for key in self._inner.config.forward_env: @@ -219,12 +227,12 @@ class DockerEnvironment(BaseEnvironment): proc = subprocess.Popen( cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, - stdin=subprocess.PIPE if stdin_data else subprocess.DEVNULL, + stdin=subprocess.PIPE if effective_stdin else subprocess.DEVNULL, text=True, ) - if stdin_data: + if effective_stdin: try: - proc.stdin.write(stdin_data) + proc.stdin.write(effective_stdin) proc.stdin.close() except Exception: pass diff --git a/tools/environments/local.py b/tools/environments/local.py index e1df97b4c..828de8181 100644 --- a/tools/environments/local.py +++ b/tools/environments/local.py @@ -161,7 +161,18 @@ class LocalEnvironment(BaseEnvironment): work_dir = cwd or self.cwd or os.getcwd() effective_timeout = timeout or self.timeout - exec_command = self._prepare_command(command) + exec_command, sudo_stdin = self._prepare_command(command) + + # Merge the sudo password (if any) with caller-supplied stdin_data. + # sudo -S reads exactly one line (the password) then passes the rest + # of stdin to the child, so prepending is safe even when stdin_data + # is also present. + if sudo_stdin is not None and stdin_data is not None: + effective_stdin = sudo_stdin + stdin_data + elif sudo_stdin is not None: + effective_stdin = sudo_stdin + else: + effective_stdin = stdin_data try: # The fence wrapper uses bash syntax (semicolons, $?, printf). @@ -195,14 +206,14 @@ class LocalEnvironment(BaseEnvironment): errors="replace", stdout=subprocess.PIPE, stderr=subprocess.STDOUT, - stdin=subprocess.PIPE if stdin_data is not None else subprocess.DEVNULL, + stdin=subprocess.PIPE if effective_stdin is not None else subprocess.DEVNULL, preexec_fn=None if _IS_WINDOWS else os.setsid, ) - if stdin_data is not None: + if effective_stdin is not None: def _write_stdin(): try: - proc.stdin.write(stdin_data) + proc.stdin.write(effective_stdin) proc.stdin.close() except (BrokenPipeError, OSError): pass diff --git a/tools/environments/modal.py b/tools/environments/modal.py index d9732529a..dbdd0a7c9 100644 --- a/tools/environments/modal.py +++ b/tools/environments/modal.py @@ -106,7 +106,20 @@ class ModalEnvironment(BaseEnvironment): marker = f"HERMES_EOF_{uuid.uuid4().hex[:8]}" command = f"{command} << '{marker}'\n{stdin_data}\n{marker}" - exec_command = self._prepare_command(command) + exec_command, sudo_stdin = self._prepare_command(command) + + # Modal sandboxes execute commands via the Modal SDK and cannot pipe + # subprocess stdin directly the way a local Popen can. When a sudo + # password is present, use a shell-level pipe from printf so that the + # password feeds sudo -S without appearing as an echo argument embedded + # in the shell string. The password is still visible in the remote + # sandbox's command line, but it is not exposed on the user's local + # machine — which is the primary threat being mitigated. + if sudo_stdin is not None: + import shlex + exec_command = ( + f"printf '%s\\n' {shlex.quote(sudo_stdin.rstrip())} | {exec_command}" + ) # Run in a background thread so we can poll for interrupts result_holder = {"value": None, "error": None} diff --git a/tools/environments/singularity.py b/tools/environments/singularity.py index c5d10e9db..0be1c38f0 100644 --- a/tools/environments/singularity.py +++ b/tools/environments/singularity.py @@ -228,7 +228,15 @@ class SingularityEnvironment(BaseEnvironment): effective_timeout = timeout or self.timeout work_dir = cwd or self.cwd - exec_command = self._prepare_command(command) + exec_command, sudo_stdin = self._prepare_command(command) + + # Merge sudo password (if any) with caller-supplied stdin_data. + if sudo_stdin is not None and stdin_data is not None: + effective_stdin = sudo_stdin + stdin_data + elif sudo_stdin is not None: + effective_stdin = sudo_stdin + else: + effective_stdin = stdin_data # apptainer exec --pwd doesn't expand ~, so prepend a cd into the command if work_dir == "~" or work_dir.startswith("~/"): @@ -245,12 +253,12 @@ class SingularityEnvironment(BaseEnvironment): proc = subprocess.Popen( cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, - stdin=subprocess.PIPE if stdin_data else subprocess.DEVNULL, + stdin=subprocess.PIPE if effective_stdin else subprocess.DEVNULL, text=True, ) - if stdin_data: + if effective_stdin: try: - proc.stdin.write(stdin_data) + proc.stdin.write(effective_stdin) proc.stdin.close() except Exception: pass diff --git a/tools/environments/ssh.py b/tools/environments/ssh.py index 02acce244..83cc335b1 100644 --- a/tools/environments/ssh.py +++ b/tools/environments/ssh.py @@ -69,15 +69,23 @@ class SSHEnvironment(BaseEnvironment): timeout: int | None = None, stdin_data: str | None = None) -> dict: work_dir = cwd or self.cwd - exec_command = self._prepare_command(command) + exec_command, sudo_stdin = self._prepare_command(command) wrapped = f'cd {work_dir} && {exec_command}' effective_timeout = timeout or self.timeout + # Merge sudo password (if any) with caller-supplied stdin_data. + if sudo_stdin is not None and stdin_data is not None: + effective_stdin = sudo_stdin + stdin_data + elif sudo_stdin is not None: + effective_stdin = sudo_stdin + else: + effective_stdin = stdin_data + cmd = self._build_ssh_command() cmd.extend(["bash", "-c", wrapped]) try: - kwargs = self._build_run_kwargs(timeout, stdin_data) + kwargs = self._build_run_kwargs(timeout, effective_stdin) # Remove timeout from kwargs -- we handle it in the poll loop kwargs.pop("timeout", None) @@ -87,13 +95,13 @@ class SSHEnvironment(BaseEnvironment): cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, - stdin=subprocess.PIPE if stdin_data else subprocess.DEVNULL, + stdin=subprocess.PIPE if effective_stdin else subprocess.DEVNULL, text=True, ) - if stdin_data: + if effective_stdin: try: - proc.stdin.write(stdin_data) + proc.stdin.write(effective_stdin) proc.stdin.close() except Exception: pass diff --git a/tools/terminal_tool.py b/tools/terminal_tool.py index 7e196466a..24c2428e3 100644 --- a/tools/terminal_tool.py +++ b/tools/terminal_tool.py @@ -291,32 +291,50 @@ def _prompt_for_sudo_password(timeout_seconds: int = 45) -> str: del os.environ["HERMES_SPINNER_PAUSE"] -def _transform_sudo_command(command: str) -> str: +def _transform_sudo_command(command: str) -> tuple[str, str | None]: """ Transform sudo commands to use -S flag if SUDO_PASSWORD is available. - + This is a shared helper used by all execution environments to provide consistent sudo handling across local, SSH, and container environments. - - If SUDO_PASSWORD is set (via env, config, or interactive prompt): - 'sudo apt install curl' -> password piped via sudo -S - + + Returns: + (transformed_command, sudo_stdin) where: + - transformed_command has every bare ``sudo`` replaced with + ``sudo -S -p ''`` so sudo reads its password from stdin. + - sudo_stdin is the password string with a trailing newline that the + caller must prepend to the process's stdin stream. sudo -S reads + exactly one line (the password) and passes the rest of stdin to the + child command, so prepending is safe even when the caller also has + its own stdin_data to pipe. + - If no password is available, sudo_stdin is None and the command is + returned unchanged so it fails gracefully with + "sudo: a password is required". + + Callers that drive a subprocess directly (local, ssh, docker, singularity) + should prepend sudo_stdin to their stdin_data and pass the merged bytes to + Popen's stdin pipe. + + Callers that cannot pipe subprocess stdin (modal, daytona) must embed the + password in the command string themselves; see their execute() methods for + how they handle the non-None sudo_stdin case. + If SUDO_PASSWORD is not set and in interactive mode (HERMES_INTERACTIVE=1): Prompts user for password with 45s timeout, caches for session. - + If SUDO_PASSWORD is not set and NOT interactive: Command runs as-is (fails gracefully with "sudo: a password is required"). """ global _cached_sudo_password import re - + # Check if command even contains sudo if not re.search(r'\bsudo\b', command): - return command # No sudo in command, return as-is - + return command, None # No sudo in command, nothing to do + # Try to get password from: env var -> session cache -> interactive prompt sudo_password = os.getenv("SUDO_PASSWORD", "") or _cached_sudo_password - + if not sudo_password: # No password configured - check if we're in interactive mode if os.getenv("HERMES_INTERACTIVE"): @@ -324,21 +342,21 @@ def _transform_sudo_command(command: str) -> str: sudo_password = _prompt_for_sudo_password(timeout_seconds=45) if sudo_password: _cached_sudo_password = sudo_password # Cache for session - + if not sudo_password: - return command # No password, let it fail gracefully - + return command, None # No password, let it fail gracefully + def replace_sudo(match): - # Replace 'sudo' with password-piped version - # The -S flag makes sudo read password from stdin - # The -p '' suppresses the password prompt - # Use shlex.quote() to prevent shell injection via password content - import shlex - return f"echo {shlex.quote(sudo_password)} | sudo -S -p ''" - + # Replace bare 'sudo' with 'sudo -S -p ""'. + # The password is returned as sudo_stdin and must be written to the + # process's stdin pipe by the caller — it never appears in any + # command-line argument or shell string. + return "sudo -S -p ''" + # Match 'sudo' at word boundaries (not 'visudo' or 'sudoers') - # This handles: sudo, sudo -flag, etc. - return re.sub(r'\bsudo\b', replace_sudo, command) + transformed = re.sub(r'\bsudo\b', replace_sudo, command) + # Trailing newline is required: sudo -S reads one line for the password. + return transformed, sudo_password + "\n" # Environment classes now live in tools/environments/