chore: run make validate in a clean allowlist environment (#77)

AGENTS.md

@@ -86,7 +86,7 @@ All pull requests must comply with [`.agents/skills/custom-codereview-guide.md`]
 
 - `LookupSecret` normalizes hostless URLs against `OH_INTERNAL_SERVER_URL` (set by `openhands-agent-server.__main__` from the bound host/port, rewriting wildcard binds to loopback) and otherwise falls back to `http://127.0.0.1:8000`, so relative secret URLs can safely target the current agent-server instance.
 - read_only ACP session-mode confirmation (`_apply_acp_session_mode`) waits at most `_ACP_SESSION_MODE_CONFIRM_TIMEOUT` (5s) on a per-session `asyncio.Event` in `_OpenHandsACPBridge.wait_for_current_mode` when `set_mode` returned with no observed mode and the advertised current mode differs; a different observed mode or a timeout still raises `ACPSessionModeError` (fail-closed, no provider/CLI version conditions). `_verify_read_only_mode_after_config` is deliberately unchanged (sync, generation-based, post-config), so the wait applies only to the initial `session/set_mode` confirmation.
-- `.aidw/validation.json` (schema `aidw.repository-validation.v1`, only command `make validate`) is the canonical validation the ai-dev runtime VERIFY step trusts from the baseline SHA; changing it in a candidate yields `HUMAN_REQUIRED`, so it must land on main first. `make validate` unsets `CLAUDE_CODE_OAUTH_TOKEN`/`CLAUDE_AUTH_CODE` (the workflow environment exports them and they make `TestACPEnvConflictSuppression` fail) and the agent-server variables `OPENHANDS_CODEX_AUTH_SOURCE`, `OPENHANDS_AGENT_SERVER_CONFIG_PATH`, `OH_PERSISTENCE_DIR`, `OH_SECRETS_DIR`, `CLOUD_AGENT_INJECTED_SECRET_NAMES`, `OH_INTERNAL_SERVER_URL` first (Builders run it in an agent-server shell; otherwise tests read host Codex auth and write into the real `secrets.json`), via `VALIDATE_UNSET_ENV` in the Makefile; `tests/conftest.py` `pytest_configure` strips the agent-server ones for direct `uv run pytest` too (tests that need them must `monkeypatch.setenv`), then runs `uv sync --dev --frozen`, then the `tests/sdk`, `tests/agent_server`, `tests/workspace`, `tests/cross` and `tests/tools` suites as separate pytest invocations (one combined run is order-dependent under xdist), `pyright`, `pre-commit run --all-files`, `git diff --check` and `git diff --exit-code`. `tests/examples` and `tests/integration` are deliberately excluded because they need a live LLM (not part of the `tests.yml` unit jobs either); the DNS-dependent `test_openai_chat_completions_gateway_over_real_server` is deselected. A cold-worktree run took 16m11s (limit 31 min). The `.github/workflows/validate.yml` job named exactly `validate` runs `make validate` on `pull_request`; the merge coordinator matches the Actions job name, so do not rename it or give it a display `name:`. `make validate` must not modify tracked files, `uv.lock`, or the ACP version pins.
+- `.aidw/validation.json` (schema `aidw.repository-validation.v1`, only command `make validate`) is the canonical validation the ai-dev runtime VERIFY step trusts from the baseline SHA; changing it in a candidate yields `HUMAN_REQUIRED`, so it must land on main first. `make validate` runs its steps (`make validate-steps`) through `scripts/validate-env.sh`, i.e. under `env -i` with an allowlist only (`PATH`, `LANG`, `LC_*`, `CI=true`, `UV_CACHE_DIR`/`UV_PYTHON_INSTALL_DIR`/`PRE_COMMIT_HOME` defaulting to the host's cache locations, and a throwaway `HOME`/`TMPDIR` created with `mktemp -d` and removed afterwards), so no inherited variable (`OPENHANDS_*`, `OH_*`, `CLAUDE_*`, `OPENAI_*`, `CODEX_*`, `CURSOR_*`, `GH_TOKEN`/`GITHUB_*`, `CLOUD_AGENT_*`, or any future name) reaches the suites (Builders run it in an agent-server shell; otherwise tests read host Codex auth, write into the real `secrets.json` and see `OPENHANDS_SESSION_API_KEY`); there is no denylist (`VALIDATE_UNSET_ENV` and the `tests/conftest.py` env strip were removed), so direct `uv run pytest` in a polluted shell is not isolated — use `make validate`; tests that need a variable `monkeypatch.setenv` it. `validate-steps` runs `uv sync --dev --frozen`, then the `tests/sdk`, `tests/agent_server`, `tests/workspace`, `tests/cross` and `tests/tools` suites as separate pytest invocations (one combined run is order-dependent under xdist), `pyright`, `pre-commit run --all-files`, `git diff --check` and `git diff --exit-code`. `tests/examples` and `tests/integration` are deliberately excluded because they need a live LLM (not part of the `tests.yml` unit jobs either); the DNS-dependent `test_openai_chat_completions_gateway_over_real_server` is deselected. A cold-worktree run took 16m11s (limit 31 min). The `.github/workflows/validate.yml` job named exactly `validate` runs `make validate` on `pull_request`; the merge coordinator matches the Actions job name, so do not rename it or give it a display `name:`. `make validate` must not modify tracked files, `uv.lock`, or the ACP version pins. Acceptance decision: direct-pytest isolation is intentionally not claimed or kept (the single allowlist lives in `scripts/validate-env.sh`); `TERM`, `SHELL`, `USER` and `VIRTUAL_ENV` are not passed because no suite needs them.
 
 
 

Makefile

@@ -57,18 +57,16 @@ pre-commit:
 	@$(ECHO) "$(GREEN)Pre-commit run successfully.$(RESET)"
 
 # Canonical validation for the ai-dev runtime VERIFY step and the `validate` CI job.
-# The OAuth variables make TestACPEnvConflictSuppression fail, and the
-# agent-server variables point tests at the host's real persistence, secrets,
-# Codex auth and config. All of them are unset first; tests/conftest.py strips
-# the agent-server ones again for direct pytest runs.
+# `validate` re-runs `validate-steps` through scripts/validate-env.sh, which starts
+# it under `env -i` with an allowlisted environment and a throwaway HOME/TMPDIR,
+# so no inherited credential or agent-server variable reaches the suites.
 # Scope: tests/sdk, tools, workspace, agent_server and cross, as in tests.yml.
 # tests/examples and tests/integration need a live LLM and are excluded.
-VALIDATE_UNSET_ENV := CLAUDE_CODE_OAUTH_TOKEN CLAUDE_AUTH_CODE \
-	OPENHANDS_CODEX_AUTH_SOURCE OPENHANDS_AGENT_SERVER_CONFIG_PATH \
-	OH_PERSISTENCE_DIR OH_SECRETS_DIR CLOUD_AGENT_INJECTED_SECRET_NAMES \
-	OH_INTERNAL_SERVER_URL
 validate:
-	@unset $(VALIDATE_UNSET_ENV); \
+	@sh scripts/validate-env.sh make --no-print-directory validate-steps
+
+.PHONY: validate-steps
+validate-steps:
 	uv sync --dev --frozen && \
 	CI=true uv run python -m pytest -n auto -p no:cacheprovider tests/sdk && \
 	CI=true uv run python -m pytest -n auto -p no:cacheprovider tests/agent_server && \

scripts/validate-env.sh

@@ -0,0 +1,27 @@
+#!/bin/sh
+# Runs "$@" in a clean environment (env -i) holding only the allowlist below
+# and throwaway HOME/TMPDIR directories. Nothing else is inherited.
+set -eu
+
+cache_home="${XDG_CACHE_HOME:-$HOME/.cache}"
+data_home="${XDG_DATA_HOME:-$HOME/.local/share}"
+sandbox="$(mktemp -d "${TMPDIR:-/tmp}/oh-validate.XXXXXX")"
+trap 'rm -rf "$sandbox"' EXIT
+mkdir "$sandbox/home" "$sandbox/tmp"
+
+# LANG and LC_* are the only allowlisted names matched by prefix.
+locale_vars="$(env | sed -n 's/^\(LC_[A-Za-z_]*=.*\)$/\1/p' | tr '\n' ' ')"
+
+# $locale_vars is deliberately unquoted: one word per NAME=value pair.
+# shellcheck disable=SC2086
+env -i \
+    PATH="$PATH" \
+    LANG="${LANG:-C.UTF-8}" \
+    $locale_vars \
+    HOME="$sandbox/home" \
+    TMPDIR="$sandbox/tmp" \
+    UV_CACHE_DIR="${UV_CACHE_DIR:-$cache_home/uv}" \
+    UV_PYTHON_INSTALL_DIR="${UV_PYTHON_INSTALL_DIR:-$data_home/uv/python}" \
+    PRE_COMMIT_HOME="${PRE_COMMIT_HOME:-$cache_home/pre-commit}" \
+    CI=true \
+    "$@"

tests/conftest.py

@@ -1,6 +1,5 @@
 """Common test fixtures and utilities."""
 
-import os
 import uuid
 from pathlib import Path
 from unittest.mock import MagicMock
@@ -23,29 +22,6 @@
 )
 
 
-# Inherited from an agent-server shell, these point tests at the host's real
-# persistence, secrets, Codex auth and server config.
-AGENT_SERVER_ENV_VARS = (
-    "OPENHANDS_CODEX_AUTH_SOURCE",
-    "OPENHANDS_AGENT_SERVER_CONFIG_PATH",
-    "OH_PERSISTENCE_DIR",
-    "OH_SECRETS_DIR",
-    "CLOUD_AGENT_INJECTED_SECRET_NAMES",
-    "OH_INTERNAL_SERVER_URL",
-)
-
-
-def strip_agent_server_env() -> None:
-    for name in AGENT_SERVER_ENV_VARS:
-        os.environ.pop(name, None)
-
-
-def pytest_configure(config: pytest.Config) -> None:
-    # Before collection and before xdist spawns workers, so no fixture can
-    # capture a host value and restore it on teardown.
-    strip_agent_server_env()
-
-
 def pytest_addoption(parser: pytest.Parser) -> None:
     group = parser.getgroup("examples")
     group.addoption(

tests/sdk/test_env_isolation.py

@@ -1,69 +1,84 @@
-"""The test suite must never inherit the agent-server's host-side env."""
+"""`make validate` must run in an allowlisted environment, not inherit the parent's."""
 
 import os
 import subprocess
-import sys
 from pathlib import Path
 
-from tests.conftest import AGENT_SERVER_ENV_VARS, REPO_ROOT, strip_agent_server_env
+from tests.conftest import REPO_ROOT
 
 
-ISOLATED_ENV_VARS = (
-    "CLAUDE_CODE_OAUTH_TOKEN",
-    "CLAUDE_AUTH_CODE",
-    *AGENT_SERVER_ENV_VARS,
-)
+SENTINEL = "dummy-sentinel-do-not-leak"
+POLLUTED_ENV = {
+    "OPENHANDS_SESSION_API_KEY": SENTINEL,
+    "OPENHANDS_CODEX_AUTH_SOURCE": SENTINEL,
+    "OH_PERSISTENCE_DIR": SENTINEL,
+    "OH_SECRETS_DIR": SENTINEL,
+    "CLAUDE_CODE_OAUTH_TOKEN": SENTINEL,
+    "OPENAI_API_KEY": SENTINEL,
+    "CODEX_HOME": SENTINEL,
+    "CURSOR_API_KEY": SENTINEL,
+    "GH_TOKEN": SENTINEL,
+    "GITHUB_TOKEN": SENTINEL,
+    "CLOUD_AGENT_INJECTED_SECRET_NAMES": SENTINEL,
+    "SOME_UNLISTED_SECRET": SENTINEL,
+}
+ALLOWED_NAMES = {
+    "PATH",
+    "LANG",
+    "HOME",
+    "TMPDIR",
+    "UV_CACHE_DIR",
+    "UV_PYTHON_INSTALL_DIR",
+    "PRE_COMMIT_HOME",
+    "CI",
+}
 
 
-def test_strip_agent_server_env_removes_inherited_values(monkeypatch):
-    for name in AGENT_SERVER_ENV_VARS:
-        monkeypatch.setenv(name, "/dummy")
-    strip_agent_server_env()
-    assert [n for n in AGENT_SERVER_ENV_VARS if n in os.environ] == []
-
-
-def test_pytest_run_strips_inherited_env(tmp_path: Path):
-    probe = tmp_path / "test_probe.py"
-    probe.write_text(
-        "import os\n"
-        f"NAMES = {AGENT_SERVER_ENV_VARS!r}\n"
-        "def test_probe():\n"
-        "    assert [n for n in NAMES if n in os.environ] == []\n"
-    )
-    env = {**os.environ, **{name: "/dummy" for name in AGENT_SERVER_ENV_VARS}}
-    result = subprocess.run(
-        [
-            sys.executable,
-            "-m",
-            "pytest",
-            "-q",
-            "-p",
-            "no:cacheprovider",
-            "-p",
-            "tests.conftest",
-            "--rootdir",
-            str(tmp_path),
-            str(probe),
-        ],
+def run_in_validate_env(
+    tmp_path: Path, *command: str
+) -> subprocess.CompletedProcess[str]:
+    env = {
+        **os.environ,
+        **POLLUTED_ENV,
+        "LC_ALL": "C.UTF-8",
+        "TMPDIR": str(tmp_path),
+        "HOME": str(tmp_path / "real-home"),
+    }
+    return subprocess.run(
+        ["sh", "scripts/validate-env.sh", *command],
         cwd=REPO_ROOT,
         env=env,
         capture_output=True,
         text=True,
+        check=True,
     )
-    assert result.returncode == 0, result.stdout + result.stderr
 
 
-def test_make_validate_unsets_isolated_env():
+def test_validate_env_contains_only_allowlisted_names(tmp_path: Path):
+    result = run_in_validate_env(tmp_path, "env")
+    names = {line.split("=", 1)[0] for line in result.stdout.splitlines()}
+    locale_names = {name for name in names if name.startswith("LC_")}
+    assert names <= ALLOWED_NAMES | locale_names
+    assert {"PATH", "HOME", "TMPDIR", "CI", "LC_ALL"} <= names
+    assert SENTINEL not in result.stdout + result.stderr
+
+
+def test_validate_env_uses_throwaway_home_and_tmpdir(tmp_path: Path):
+    result = run_in_validate_env(tmp_path, "sh", "-c", 'echo "$HOME" "$TMPDIR"')
+    home, tmpdir = (Path(p) for p in result.stdout.split())
+    assert home != tmp_path / "real-home"
+    assert home.parent == tmpdir.parent
+    assert home.parent.parent == tmp_path
+    assert not home.parent.exists()
+
+
+def test_make_validate_runs_steps_in_validate_env():
     result = subprocess.run(
         ["make", "-n", "validate"],
         cwd=REPO_ROOT,
         capture_output=True,
         text=True,
         check=True,
     )
-    unset_line = next(
-        line for line in result.stdout.splitlines() if line.startswith("unset ")
-    )
-    assert set(unset_line.removeprefix("unset ").split(";")[0].split()) == set(
-        ISOLATED_ENV_VARS
-    )
+    assert result.stdout.startswith("sh scripts/validate-env.sh ")
+    assert "validate-steps" in result.stdout