diff --git a/src/coder_eval/agents/codex_agent.py b/src/coder_eval/agents/codex_agent.py index 9e05637d..fda16e39 100644 --- a/src/coder_eval/agents/codex_agent.py +++ b/src/coder_eval/agents/codex_agent.py @@ -5,7 +5,9 @@ import json import logging import os +import shlex import shutil +import tempfile import time from collections.abc import Callable from datetime import datetime @@ -215,6 +217,11 @@ def _message_uncached_input(m: AssistantMessage) -> int: # supported"), so this is a fixed constant, not an operator knob. _CODEX_WIRE_API = "responses" +# Login-shell profile files generated into the per-task HOME (see +# _setup_login_shell_home). ``.bash_profile`` is what ``bash -l`` reads; +# ``.profile`` covers ``sh``/``dash`` login shells. +_LOGIN_PROFILE_NAMES = (".bash_profile", ".profile") + def _get_item_root(notification: Any) -> Any: """Extract the typed item root from a Codex SDK notification. @@ -649,6 +656,7 @@ def __init__( self.thread: Any = None self.working_directory: Path | None = None self._env_path_prepend: list[str] = [] + self._login_shell_home: Path | None = None # _state / _iteration / _iteration_was_incremented / pending_turn lifecycle # bookkeeping lives on the Agent base class (shared defaults + helpers). self._log = PrefixedAdapter(logger, {"prefix": instance_name}) @@ -677,6 +685,7 @@ async def start( """ self.working_directory = Path(working_directory) self._env_path_prepend = list(env_path_prepend or []) + self._setup_login_shell_home() self._state = AgentState.WORKING try: @@ -872,6 +881,7 @@ async def stop(self) -> None: self._close_client() self.thread = None self._active_turn_handle = None + self._cleanup_login_shell_home() self._mark_stopped() async def kill(self) -> None: @@ -887,6 +897,7 @@ def kill_sync(self) -> None: """ self._interrupt_active_turn() self._close_client() + self._cleanup_login_shell_home() def _interrupt_active_turn(self) -> None: """Interrupt the in-flight Codex turn, if any (best-effort, idempotent).""" @@ -1071,8 +1082,111 @@ def _build_codex_env(self) -> dict[str, str] | None: path_key = next((k for k in os.environ if k.upper() == "PATH"), "PATH") env[path_key] = os.pathsep.join([*self._env_path_prepend, os.environ.get(path_key, "")]) self._log.debug(f"PATH prepend: {os.pathsep.join(self._env_path_prepend)}") + if self._login_shell_home is not None: + # Point login shells at the generated profile dir (see + # _setup_login_shell_home) while pinning codex state (auth, rollout + # sessions) to its real location — _codex_home() reads the same + # resolution for sub-agent rollout recovery, so both sides agree. + env["HOME"] = str(self._login_shell_home) + env["CODEX_HOME"] = str(self._codex_home()) return env if env else None + @staticmethod + def _login_shell_profiles_supported() -> bool: + """Whether to generate login-shell profile shims (POSIX shells only).""" + return os.name == "posix" + + def _setup_login_shell_home(self) -> None: + """Create a per-task HOME whose profiles restore the mock PATH prepend. + + Codex issues every shell command as ``bash -lc`` (bash/sh only — if it + ever shells through zsh or fish, their profile files need the same + treatment here). A login shell re-sources ``/etc/profile``, which + unconditionally RESETS PATH — silently dropping the mock-CLI prepend + passed via the app-server environment, so bare commands resolve to the + REAL CLIs (real-tenant contamination). ``~/.bash_profile`` is sourced + AFTER ``/etc/profile``, so a generated per-task HOME (wired up in + ``_build_codex_env``; codex state stays in CODEX_HOME) gets the last + word and re-prepends the mock dirs. Per-task rather than the user's + real dotfiles so parallel tasks with different mocks cannot collide. + No-op without mock dirs or on non-POSIX hosts (Windows codex does not + shell through bash). + + Bash uses the env HOME only to PICK the profile file; the generated + profile's first act is to export the ORIGINAL home back, so the + sourced user profile and the command body see the real ``$HOME`` + (git config, tool caches, ``$HOME``-relative sourcing keep working). + Known residual gap: a NESTED login shell inside a command re-reads the + real profiles and loses the prepend again. + """ + self._cleanup_login_shell_home() + if not (self._env_path_prepend and self._login_shell_profiles_supported()): + return + original_home = os.environ.get("HOME", "") + # The profile only ever executes under a POSIX shell, so the PATH + # separator is ':' regardless of the host building it. + quoted_prepend = shlex.quote(":".join(self._env_path_prepend)) + export_line = f'export PATH={quoted_prepend}:"$PATH"' + # Track the dir BEFORE writing so a failed write can't orphan it — + # the except below (and any later cleanup) always sees it. + home = Path(tempfile.mkdtemp(prefix="coder-eval-codex-home-")) + self._login_shell_home = home + try: + for name in _LOGIN_PROFILE_NAMES: + content = self._login_profile_content(name, original_home, export_line) + # newline="\n": the profile must stay LF-only no matter which host + # builds it, or bash sees literal \r at end of line. + (home / name).write_text(content, encoding="utf-8", newline="\n") + except Exception: + self._cleanup_login_shell_home() + raise + self._log.debug(f"Login-shell mock-PATH home: {home}") + + @staticmethod + def _login_profile_content(profile_name: str, original_home: str, export_line: str) -> str: + """One generated profile file: restore the ORIGINAL ``$HOME``, source + that home's own profile (so image/user setup isn't lost), then + re-prepend the mock dirs. + + The env HOME pointing at the generated dir exists ONLY so bash selects + this file; exporting the original home back on the first line keeps + every ``$HOME`` consumer (git, npm, the sourced profile's own + ``$HOME/.bashrc`` references) on the real home. + + ``.bash_profile`` mimics bash's first-found chain over the original + home; the ``.profile`` twin (read by ``sh``/``dash`` login shells) + sources only ``.profile`` — the bash-specific files may contain + bashisms a POSIX shell would choke on. + """ + lines = [ + "# Generated by coder_eval (CodexAgent): /etc/profile resets PATH in", + "# login shells, dropping the mock-CLI prepend — this restores it.", + ] + if original_home: + orig = shlex.quote(original_home) + lines.append(f"export HOME={orig}") + if profile_name == ".bash_profile": + lines += [ + f"if [ -r {orig}/.bash_profile ]; then . {orig}/.bash_profile", + f"elif [ -r {orig}/.bash_login ]; then . {orig}/.bash_login", + f"elif [ -r {orig}/.profile ]; then . {orig}/.profile", + "fi", + ] + else: + lines += [ + f"if [ -r {orig}/.profile ]; then . {orig}/.profile", + "fi", + ] + lines.append(export_line) + return "\n".join(lines) + "\n" + + def _cleanup_login_shell_home(self) -> None: + """Remove the generated per-task HOME (best-effort, idempotent).""" + home, self._login_shell_home = self._login_shell_home, None + if home is None: + return + shutil.rmtree(home, ignore_errors=True) + def _build_thread_options(self) -> dict[str, Any]: """Build thread_start options from agent config. diff --git a/tests/test_codex_agent.py b/tests/test_codex_agent.py index 68794f33..a0dd0cb3 100644 --- a/tests/test_codex_agent.py +++ b/tests/test_codex_agent.py @@ -498,7 +498,13 @@ def test_get_state_returns_current_state(): # without a live SDK. These mirror the notification shapes the real stream emits. # --------------------------------------------------------------------------- +import os # noqa: E402 +import shlex # noqa: E402 +import shutil # noqa: E402 +import subprocess # noqa: E402 +import tempfile # noqa: E402 import time # noqa: E402 +from pathlib import Path # noqa: E402 from types import SimpleNamespace # noqa: E402 from openai_codex.generated.v2_all import Turn, TurnCompletedNotification # noqa: E402 @@ -1563,3 +1569,295 @@ async def test_create_agent_returns_codex_agent(self, tmp_path): orch = Orchestrator(task=task, run_dir=tmp_path / "run", variant_id="t") agent = await orch._create_agent() assert isinstance(agent, CodexAgent) + + +class TestLoginShellMockPathHome: + """Codex issues every shell command as ``bash -lc``. The login shell + re-sources ``/etc/profile``, which unconditionally RESETS PATH — silently + dropping the mock-CLI prepend passed via the app-server env, so bare + commands resolve to the REAL CLIs (real-tenant contamination). The agent + therefore generates a per-task HOME whose ``.bash_profile``/``.profile`` + run AFTER /etc/profile and restore the prepend; ``_build_codex_env`` points + HOME at it and pins CODEX_HOME so codex state stays put.""" + + @staticmethod + def _force_posix(monkeypatch, supported: bool = True): + monkeypatch.setattr(CodexAgent, "_login_shell_profiles_supported", staticmethod(lambda: supported)) + + @staticmethod + def _agent_with_prepend(prepend): + agent = CodexAgent(parse_agent_config(type=AgentKind.CODEX)) + agent._env_path_prepend = list(prepend) + return agent + + def test_setup_writes_profiles_with_mock_prepend(self, monkeypatch, tmp_path): + self._force_posix(monkeypatch) + monkeypatch.setenv("HOME", str(tmp_path / "orig-home")) + agent = self._agent_with_prepend(["/sandbox/mocks", "/sandbox/bins"]) + + agent._setup_login_shell_home() + try: + home = agent._login_shell_home + assert home is not None and home.is_dir() + for name in (".bash_profile", ".profile"): + content = (home / name).read_text(encoding="utf-8") + # The export must prepend the mock dirs (POSIX ':' joined) ahead + # of whatever /etc/profile left in PATH. + assert 'export PATH=/sandbox/mocks:/sandbox/bins:"$PATH"' in content + finally: + agent._cleanup_login_shell_home() + + def test_bash_profile_sources_original_home_first_found_chain(self, monkeypatch, tmp_path): + """The generated .bash_profile mimics bash's first-found chain over the + ORIGINAL home (so image/user setup isn't lost); the .profile twin (read + by sh/dash login shells) sources only .profile — the bash-specific + files may contain bashisms.""" + self._force_posix(monkeypatch) + orig = tmp_path / "orig-home" + monkeypatch.setenv("HOME", str(orig)) + agent = self._agent_with_prepend(["/sandbox/mocks"]) + + agent._setup_login_shell_home() + try: + home = agent._login_shell_home + assert home is not None + qorig = shlex.quote(str(orig)) + bash_profile = (home / ".bash_profile").read_text(encoding="utf-8") + for name in (".bash_profile", ".bash_login", ".profile"): + assert f". {qorig}/{name}" in bash_profile + profile = (home / ".profile").read_text(encoding="utf-8") + assert f". {qorig}/.profile" in profile + assert f". {qorig}/.bash_profile" not in profile + finally: + agent._cleanup_login_shell_home() + + def test_mock_dirs_with_shell_metacharacters_are_quoted(self, monkeypatch, tmp_path): + self._force_posix(monkeypatch) + monkeypatch.setenv("HOME", str(tmp_path / "orig-home")) + weird = "/sandbox/mo cks/$(evil)" + agent = self._agent_with_prepend([weird]) + + agent._setup_login_shell_home() + try: + home = agent._login_shell_home + assert home is not None + content = (home / ".bash_profile").read_text(encoding="utf-8") + assert f'export PATH={shlex.quote(weird)}:"$PATH"' in content + finally: + agent._cleanup_login_shell_home() + + def test_profile_restores_original_home_before_sourcing(self, monkeypatch, tmp_path): + """The generated profile's FIRST act is exporting the original HOME + back — the temp HOME exists only so bash picks this file; everything + sourced after (and the command body) must see the real $HOME.""" + self._force_posix(monkeypatch) + orig = tmp_path / "orig-home" + monkeypatch.setenv("HOME", str(orig)) + agent = self._agent_with_prepend(["/sandbox/mocks"]) + + agent._setup_login_shell_home() + try: + home = agent._login_shell_home + assert home is not None + for name in (".bash_profile", ".profile"): + content = (home / name).read_text(encoding="utf-8") + export_home = content.index(f"export HOME={shlex.quote(str(orig))}") + assert export_home < content.index(". ") # before any sourcing + assert export_home < content.index("export PATH=") + finally: + agent._cleanup_login_shell_home() + + def test_profile_without_original_home_only_prepends(self, monkeypatch): + """HOME unset in the harness env: no restore, no sourcing — just the + mock prepend.""" + content = CodexAgent._login_profile_content(".bash_profile", "", 'export PATH=/m:"$PATH"') + assert "export HOME" not in content + assert ". " not in content + assert 'export PATH=/m:"$PATH"' in content + + def test_generated_profiles_are_lf_only(self, monkeypatch, tmp_path): + """A profile built on ANY host must stay LF-only — bash chokes on \\r.""" + self._force_posix(monkeypatch) + monkeypatch.setenv("HOME", str(tmp_path / "orig-home")) + agent = self._agent_with_prepend(["/sandbox/mocks"]) + + agent._setup_login_shell_home() + try: + home = agent._login_shell_home + assert home is not None + for name in (".bash_profile", ".profile"): + raw = (home / name).read_bytes() + assert b"\r" not in raw + finally: + agent._cleanup_login_shell_home() + + def test_no_login_home_without_mock_dirs(self, monkeypatch): + self._force_posix(monkeypatch) + agent = self._agent_with_prepend([]) + agent._setup_login_shell_home() + assert agent._login_shell_home is None + + def test_no_login_home_on_unsupported_platform(self, monkeypatch): + self._force_posix(monkeypatch, supported=False) + agent = self._agent_with_prepend(["/sandbox/mocks"]) + agent._setup_login_shell_home() + assert agent._login_shell_home is None + + def test_build_codex_env_sets_home_and_pins_codex_home(self, monkeypatch, tmp_path): + monkeypatch.delenv("CODEX_API_KEY", raising=False) + monkeypatch.setenv("PATH", "/parent/bin") + agent = self._agent_with_prepend(["/sandbox/mocks"]) + agent._login_shell_home = tmp_path / "login-home" + + env = agent._build_codex_env() + assert env is not None + assert env["HOME"] == str(tmp_path / "login-home") + # Codex state (auth, rollout sessions) must NOT move with HOME — the + # harness reads the same _codex_home() for sub-agent rollout recovery. + assert env["CODEX_HOME"] == str(agent._codex_home()) + + def test_build_codex_env_without_login_home_leaves_home_alone(self, monkeypatch): + monkeypatch.setenv("CODEX_API_KEY", "k") + agent = CodexAgent(parse_agent_config(type=AgentKind.CODEX)) + env = agent._build_codex_env() + assert env is not None + assert "HOME" not in env + assert "CODEX_HOME" not in env + + def test_setup_is_rerunnable_and_cleanup_removes_dir(self, monkeypatch, tmp_path): + self._force_posix(monkeypatch) + monkeypatch.setenv("HOME", str(tmp_path / "orig-home")) + agent = self._agent_with_prepend(["/sandbox/mocks"]) + + agent._setup_login_shell_home() + first = agent._login_shell_home + assert first is not None + agent._setup_login_shell_home() # retried start() must not leak the old dir + second = agent._login_shell_home + assert second is not None + assert not first.exists() + + agent._cleanup_login_shell_home() + assert agent._login_shell_home is None + assert not second.exists() + + def test_failed_profile_write_rolls_back_temp_home(self, monkeypatch, tmp_path): + """A write failure must not orphan the mkdtemp dir — it is tracked + before writing and removed on the way out.""" + self._force_posix(monkeypatch) + monkeypatch.setenv("HOME", str(tmp_path / "orig-home")) + agent = self._agent_with_prepend(["/sandbox/mocks"]) + + created: list = [] + real_mkdtemp = tempfile.mkdtemp + + def tracking_mkdtemp(*args, **kwargs): + d = real_mkdtemp(*args, **kwargs) + created.append(d) + return d + + monkeypatch.setattr("coder_eval.agents.codex_agent.tempfile.mkdtemp", tracking_mkdtemp) + monkeypatch.setattr( + CodexAgent, + "_login_profile_content", + staticmethod(lambda *_a, **_kw: (_ for _ in ()).throw(OSError("disk full"))), + ) + + with pytest.raises(OSError, match="disk full"): + agent._setup_login_shell_home() + + assert agent._login_shell_home is None + assert created and not Path(created[0]).exists() + + def test_kill_sync_cleans_login_home(self, monkeypatch, tmp_path): + """The watchdog's terminal kill path must not leak the temp HOME.""" + self._force_posix(monkeypatch) + monkeypatch.setenv("HOME", str(tmp_path / "orig-home")) + agent = self._agent_with_prepend(["/sandbox/mocks"]) + agent.codex_client = SimpleNamespace(close=lambda: None) + + agent._setup_login_shell_home() + home = agent._login_shell_home + assert home is not None + + agent.kill_sync() + + assert agent._login_shell_home is None + assert not home.exists() + + async def test_start_creates_and_stop_cleans_login_home(self, monkeypatch, tmp_path): + """start() must compose the pieces: generated HOME + pinned CODEX_HOME + + mock-first PATH must all land in the CodexConfig env handed to the + SDK (not just exist on the agent), and stop() must clean up.""" + import openai_codex + + self._force_posix(monkeypatch) + monkeypatch.setenv("HOME", str(tmp_path / "orig-home")) + monkeypatch.delenv("CODEX_API_KEY", raising=False) + captured: dict = {} + + def fake_codex(**kwargs): + captured.update(kwargs) + return SimpleNamespace(close=lambda: None) + + monkeypatch.setattr(openai_codex, "Codex", fake_codex) + + agent = CodexAgent(parse_agent_config(type=AgentKind.CODEX)) + await agent.start(str(tmp_path), env_path_prepend=["/sandbox/mocks"]) + + home = agent._login_shell_home + assert home is not None and (home / ".bash_profile").is_file() + config = captured.get("config") + assert config is not None and config.env is not None + assert config.env["HOME"] == str(home) + assert config.env["CODEX_HOME"] == str(agent._codex_home()) + path_key = next(k for k in config.env if k.upper() == "PATH") + assert config.env[path_key].startswith("/sandbox/mocks") + + await agent.stop() + assert agent._login_shell_home is None + assert not home.exists() + + @pytest.mark.skipif( + os.name != "posix" or not shutil.which("bash"), + reason="requires a POSIX bash to exercise a real login shell", + ) + def test_login_shell_restores_mock_prepend_end_to_end(self, monkeypatch, tmp_path): + """Real ``bash -lc`` with the generated HOME, against a CONTROLLED + original home (hermetic — the developer/CI dotfiles play no part): + + - the original .bash_profile resets PATH (worst case) and sources + ``$HOME/.bashrc`` — which must resolve to the ORIGINAL home; + - the mock dir still comes out FIRST on PATH; + - the command body sees the original ``$HOME``. + """ + self._force_posix(monkeypatch) + orig = tmp_path / "orig-home" + orig.mkdir() + (orig / ".bash_profile").write_text( + 'export PATH="/usr/local/bin:/usr/bin:/bin"\n[ -r "$HOME/.bashrc" ] && . "$HOME/.bashrc"\n', + encoding="utf-8", + newline="\n", + ) + (orig / ".bashrc").write_text("export BASHRC_SOURCED=1\n", encoding="utf-8", newline="\n") + monkeypatch.setenv("HOME", str(orig)) + agent = self._agent_with_prepend([str(tmp_path / "mocks")]) + (tmp_path / "mocks").mkdir() + + agent._setup_login_shell_home() + try: + home = agent._login_shell_home + assert home is not None + result = subprocess.run( + ["bash", "-lc", 'echo "$PATH|$HOME|${BASHRC_SOURCED:-0}"'], + env={"HOME": str(home), "PATH": "/usr/bin:/bin"}, + capture_output=True, + text=True, + check=True, + ) + path_value, home_value, bashrc_sourced = result.stdout.strip().split("|") + assert path_value.split(":")[0] == str(tmp_path / "mocks") + assert home_value == str(orig) + assert bashrc_sourced == "1" + finally: + agent._cleanup_login_shell_home()