diff --git a/tests/tools/test_base_environment.py b/tests/tools/test_base_environment.py index ba620aac8c..9939a0f747 100644 --- a/tests/tools/test_base_environment.py +++ b/tests/tools/test_base_environment.py @@ -6,6 +6,7 @@ init_session() failure handling, and the CWD marker contract. from unittest.mock import MagicMock +import tools.terminal_tool_sudo as terminal_tool_sudo from tools.environments.base import BaseEnvironment from tools.environments.base_output import _BoundedOutputCollector @@ -23,6 +24,48 @@ class _TestableEnv(BaseEnvironment): pass +def test_prepare_command_uses_selected_environment_for_nopasswd(monkeypatch): + monkeypatch.delenv("SUDO_PASSWORD", raising=False) + monkeypatch.setenv("HERMES_INTERACTIVE", "1") + env = _TestableEnv() + monkeypatch.setattr(env, "_sudo_nopasswd_works", lambda: True) + + def _fail_prompt(*_args, **_kwargs): + raise AssertionError("interactive sudo prompt should not run for NOPASSWD") + + monkeypatch.setattr(terminal_tool_sudo, "_prompt_for_sudo_password", _fail_prompt) + + transformed, sudo_stdin = env._prepare_command("sudo true") + + assert transformed == "sudo true" + assert sudo_stdin is None + + +def test_nopasswd_probe_runs_inside_selected_environment(monkeypatch): + env = _TestableEnv() + proc = object() + run = MagicMock(return_value=proc) + wait = MagicMock(return_value={"returncode": 0}) + monkeypatch.setattr(env, "_run_bash", run) + monkeypatch.setattr(env, "_wait_for_process", wait) + + assert env._sudo_nopasswd_works() is True + run.assert_called_once_with( + "sudo -n true", + login=False, + timeout=3, + stdin_data=None, + ) + wait.assert_called_once_with(proc, timeout=3) + + +def test_nopasswd_probe_fails_closed_on_backend_error(monkeypatch): + env = _TestableEnv() + monkeypatch.setattr(env, "_run_bash", MagicMock(side_effect=RuntimeError("offline"))) + + assert env._sudo_nopasswd_works() is False + + class TestBoundedOutputCollector: def test_large_stream_retains_bounded_head_and_tail(self): collector = _BoundedOutputCollector(1_000) diff --git a/tests/tools/test_terminal_tool.py b/tests/tools/test_terminal_tool.py index 1f36223b29..75fed7e665 100644 --- a/tests/tools/test_terminal_tool.py +++ b/tests/tools/test_terminal_tool.py @@ -78,6 +78,39 @@ def test_explicit_empty_sudo_password_tries_empty_without_prompt(monkeypatch): assert sudo_stdin == "\n" +def test_backend_nopasswd_probe_skips_interactive_prompt(monkeypatch): + monkeypatch.delenv("SUDO_PASSWORD", raising=False) + monkeypatch.setenv("HERMES_INTERACTIVE", "1") + + def _fail_prompt(*_args, **_kwargs): + raise AssertionError("interactive sudo prompt should not run for NOPASSWD") + + monkeypatch.setattr(terminal_tool_sudo, "_prompt_for_sudo_password", _fail_prompt) + + transformed, sudo_stdin = terminal_tool_sudo._transform_sudo_command( + "sudo true", + sudo_nopasswd_check=lambda: True, + ) + + assert transformed == "sudo true" + assert sudo_stdin is None + + +def test_configured_password_does_not_run_backend_nopasswd_probe(monkeypatch): + monkeypatch.setenv("SUDO_PASSWORD", "testpass") + + def _fail_probe(): + raise AssertionError("configured passwords must bypass the NOPASSWD probe") + + transformed, sudo_stdin = terminal_tool_sudo._transform_sudo_command( + "sudo true", + sudo_nopasswd_check=_fail_probe, + ) + + assert transformed == "sudo -S -p '' true" + assert sudo_stdin == "testpass\n" + + def test_validate_workdir_blocks_shell_metacharacters_in_windows_paths(): assert terminal_tool._validate_workdir(r"C:\Users\Alice\project; rm -rf /") assert terminal_tool._validate_workdir(r"C:\Users\Alice\project$(whoami)") diff --git a/tools/environments/base.py b/tools/environments/base.py index 47998cd27b..8c4d15f6dc 100644 --- a/tools/environments/base.py +++ b/tools/environments/base.py @@ -587,9 +587,27 @@ class BaseEnvironment(ABC): pass def _prepare_command(self, command: str) -> tuple[str, str | None]: - """Transform sudo commands if SUDO_PASSWORD is available.""" + """Prepare sudo using this environment's passwordless-sudo probe.""" from tools.terminal_tool_sudo import _transform_sudo_command - return _transform_sudo_command(command) + + return _transform_sudo_command( + command, + sudo_nopasswd_check=self._sudo_nopasswd_works, + ) + + def _sudo_nopasswd_works(self) -> bool: + """Probe passwordless sudo inside this execution environment.""" + try: + proc = self._run_bash( + "sudo -n true", + login=False, + timeout=3, + stdin_data=None, + ) + result = self._wait_for_process(proc, timeout=3) + return result.get("returncode") == 0 + except Exception: + return False # ---- BEGIN PLUGIN-COMPAT (revert-scheduled; see COMPAT_MANIFEST.md) ---- diff --git a/tools/terminal_tool_sudo.py b/tools/terminal_tool_sudo.py index 5ab2be5026..c775c290d6 100644 --- a/tools/terminal_tool_sudo.py +++ b/tools/terminal_tool_sudo.py @@ -13,6 +13,7 @@ import subprocess import sys import threading import time +from typing import Callable from collections.abc import Iterator from utils import env_var_enabled @@ -434,7 +435,10 @@ def _rewrite_compound_background(command: str) -> str: return result -def _transform_sudo_command(command: str | None) -> tuple[str | None, str | None]: +def _transform_sudo_command( + command: str | None, + sudo_nopasswd_check: Callable[[], bool] | None = None, +) -> tuple[str | None, str | None]: """Rewrite command-position ``sudo`` executables to ``sudo -S -p ''`` when a password is available (shared by every execution environment). Returns ``(command, sudo_stdin)``: ``sudo_stdin`` is one password line per sudo invocation that the caller must PREPEND to the process stdin (sudo -S consumes @@ -461,9 +465,16 @@ def _transform_sudo_command(command: str | None) -> tuple[str | None, str | None has_configured_password = _configured_password is not None sudo_password = _configured_password if has_configured_password else _get_cached_sudo_password() - # sudoers NOPASSWD hosts must not be forced through the prompt or the -S pipe (local only). - if not has_configured_password and not sudo_password and _sudo_nopasswd_works(): - return command, None + # sudoers NOPASSWD must not be forced through the prompt or the -S pipe. BaseEnvironment + # supplies a probe scoped to the selected backend; direct callers keep the local-host + # fallback. Re-probed every call so an expired sudo timestamp cannot silently block. + if not has_configured_password and not sudo_password: + nopasswd_check = sudo_nopasswd_check or _sudo_nopasswd_works + try: + if nopasswd_check(): + return command, None + except Exception: + pass # delegate_task children inherit HERMES_INTERACTIVE=1 (and possibly a stale thread-local # callback on a recycled worker) but have no user on the other side — always headless;