diff --git a/reviewer/noema_reviewer/github_io.py b/reviewer/noema_reviewer/github_io.py index e695193f7..d3910833b 100644 --- a/reviewer/noema_reviewer/github_io.py +++ b/reviewer/noema_reviewer/github_io.py @@ -59,11 +59,36 @@ ) +def _github_cli_environment() -> dict[str, str]: + """Build the least-authority environment for reviewer GitHub CLI calls.""" + safe_env = { + "GH_HOST": "github.com", + "NO_COLOR": "1", + } + path = os.environ.get("PATH") + if path: + safe_env["PATH"] = path + token = os.environ.get("GH_TOKEN") + if token: + safe_env["GH_TOKEN"] = token + return safe_env + + +def _redact_delegated_github_token(text: str, child_env: dict[str, str]) -> str: + """Remove the exact delegated GitHub token before an error can be retained.""" + token = child_env.get("GH_TOKEN", "") + if not token: + return text + return text.replace(token, "[REDACTED]") + + def default_runner(args: Sequence[str], stdin: str | None = None) -> str: - """Run a ``gh`` command without a shell and return stdout.""" + """Run a ``gh`` command with explicit least-authority environment.""" + child_env = _github_cli_environment() completed = subprocess.run( list(args), input=stdin, + env=child_env, text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, @@ -71,7 +96,8 @@ def default_runner(args: Sequence[str], stdin: str | None = None) -> str: shell=False, ) if completed.returncode != 0: - raise RuntimeError(f"Command failed ({completed.returncode}): {args[0]}\n{completed.stderr.strip()}") + detail = _redact_delegated_github_token(completed.stderr.strip(), child_env) + raise RuntimeError(f"Command failed ({completed.returncode}): {args[0]}\n{detail}") return completed.stdout diff --git a/reviewer/tests/test_github_io_environment_boundary.py b/reviewer/tests/test_github_io_environment_boundary.py new file mode 100644 index 000000000..4eaaa83f5 --- /dev/null +++ b/reviewer/tests/test_github_io_environment_boundary.py @@ -0,0 +1,86 @@ +"""Regression coverage for reviewer GitHub CLI subprocess authority.""" + +from __future__ import annotations + +from types import SimpleNamespace + +import pytest + +from noema_reviewer.github_io import default_runner + + +def test_default_runner_passes_only_reviewed_github_cli_environment(monkeypatch) -> None: + """A hostile parent must not become ambient authority for the ``gh`` child.""" + observed: dict[str, object] = {} + + def fake_run(args, **kwargs): + """Capture the subprocess contract without executing GitHub CLI.""" + observed.update(kwargs) + return SimpleNamespace(returncode=0, stdout="ok", stderr="") + + monkeypatch.setenv("PATH", "/reviewed/bin") + monkeypatch.setenv("GH_TOKEN", "delegated-github-token") + monkeypatch.setenv("GITHUB_TOKEN", "ambient-github-token") + monkeypatch.setenv("NVIDIA_NIM_API_KEY", "model-secret") + monkeypatch.setenv("NOEMA_LLM_API_KEY", "reviewer-secret") + monkeypatch.setenv("NOEMA_REVIEWER_APP_PRIVATE_KEY", "app-private-key") + monkeypatch.setenv("HTTPS_PROXY", "http://proxy.invalid") + monkeypatch.setenv("HOME", "/hostile/home") + monkeypatch.setenv("NODE_OPTIONS", "--require=/hostile/preload.cjs") + monkeypatch.setattr("noema_reviewer.github_io.subprocess.run", fake_run) + + assert default_runner(["gh", "api", "user"], None) == "ok" + assert observed["env"] == { + "PATH": "/reviewed/bin", + "GH_TOKEN": "delegated-github-token", + "GH_HOST": "github.com", + "NO_COLOR": "1", + } + assert observed["shell"] is False + + +def test_default_runner_keeps_only_pinned_defaults_without_path_or_token(monkeypatch) -> None: + """Missing optional launch/token authority must not widen the child environment.""" + observed: dict[str, object] = {} + + def fake_run(args, **kwargs): + """Capture the subprocess contract without executing GitHub CLI.""" + observed.update(kwargs) + return SimpleNamespace(returncode=0, stdout="ok", stderr="") + + monkeypatch.delenv("PATH", raising=False) + monkeypatch.delenv("GH_TOKEN", raising=False) + monkeypatch.setenv("GITHUB_TOKEN", "ambient-github-token") + monkeypatch.setenv("HOME", "/hostile/home") + monkeypatch.setattr("noema_reviewer.github_io.subprocess.run", fake_run) + + assert default_runner(["gh", "api", "user"], None) == "ok" + assert observed["env"] == { + "GH_HOST": "github.com", + "NO_COLOR": "1", + } + assert observed["shell"] is False + + +def test_default_runner_redacts_delegated_token_from_failure_diagnostics(monkeypatch) -> None: + """A hostile ``gh`` failure cannot copy the delegated token into retained errors.""" + token = "delegated-github-token" + + def fake_run(args, **kwargs): + """Echo the delegated credential as a hostile child executable might.""" + return SimpleNamespace( + returncode=1, + stdout="", + stderr=f"authentication failed for {token}; retry token={token}", + ) + + monkeypatch.setenv("GH_TOKEN", token) + monkeypatch.setattr("noema_reviewer.github_io.subprocess.run", fake_run) + + with pytest.raises(RuntimeError) as raised: + default_runner(["gh", "api", "user"], None) + + detail = str(raised.value) + assert token not in detail + assert "authentication failed" in detail + assert "retry token=[REDACTED]" in detail