From 7342b2e6dea6a0872ff2ed1d5ee31ebc5f3af793 Mon Sep 17 00:00:00 2001 From: timmy Date: Sat, 8 Aug 2026 19:34:43 +0000 Subject: [PATCH] security: isolate release subprocess credentials (#333) --- README.md | 11 +++++ docs/release-engine-spec.md | 5 +++ src/release_engine.py | 83 ++++++++++++++++++++++++++++-------- tests/test_release_engine.py | 74 ++++++++++++++++++++++++++++++++ 4 files changed, 156 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 0b1d3ba..9c1e730 100644 --- a/README.md +++ b/README.md @@ -273,6 +273,7 @@ python3 -m src.release_engine \ --agent-timeout 1800 \ --test-timeout 900 \ --termination-grace 5 \ + --agent-env AGENT_CONFIG_HOME \ --test-command 'python3 -m pytest tests/ -q' ``` @@ -289,6 +290,16 @@ text. Shell substitutions, redirects, and pipelines are intentionally not interpreted. Put any trusted shell workflow in a reviewed wrapper script and configure that script as `RELEASE_AGENT_COMMAND` instead. +Every child process receives an explicit least-privilege environment rather +than inheriting the worker's credentials. Coding agents receive a minimal +runtime environment plus the five `RELEASE_*` context values above. Tests and +Git receive only the minimal runtime environment, so `GITEA_TOKEN` and +unrelated parent secrets remain in the release-engine process. If an agent +runtime needs another non-secret setting, allow it explicitly with a repeated +`--agent-env NAME` option or the comma-separated `RELEASE_AGENT_ENV` variable. +`GITEA_TOKEN`, reserved `RELEASE_*` names, and malformed names are rejected +before the issue is claimed. Secret-bearing values must not be allowlisted. + Agent and test deadlines are independent. On expiry, the engine terminates the command's entire process group, escalates from `SIGTERM` to `SIGKILL` after the configured grace period, and records `agent_timed_out` or `tests_timed_out` diff --git a/docs/release-engine-spec.md b/docs/release-engine-spec.md index d9b0c29..3015f30 100644 --- a/docs/release-engine-spec.md +++ b/docs/release-engine-spec.md @@ -15,6 +15,7 @@ Convert one eligible Gitea issue into a tested, traceable pull request without d - Coding-agent command, parsed into an argument vector. - Test command, parsed into an argument vector. - Positive, independent coding-agent and test deadlines plus a termination grace period. +- Optional explicit allowlist of non-secret parent environment variables needed by the coding-agent runtime. - Local repository path and durable state-file path. ## State machine @@ -54,6 +55,8 @@ repeating or advancing delivery. - Branch format: `/-`. - Coding and test commands are parsed with POSIX argument quoting and executed directly without a shell. - Issue number, title, body, repo, and branch are supplied to the coding agent only through `RELEASE_ISSUE_NUMBER`, `RELEASE_ISSUE_TITLE`, `RELEASE_ISSUE_BODY`, `RELEASE_REPO`, and `RELEASE_BRANCH` environment variables. Gitea-controlled content is never interpolated into executable syntax. +- Child processes never inherit the complete parent environment. Coding receives a minimal runtime environment, explicitly allowlisted non-secret agent variables, and the five issue-context values. Tests and Git receive only the minimal runtime environment; `GITEA_TOKEN` remains parent-process-only. +- Agent variables are allowlisted with repeated `--agent-env NAME` options or comma-separated `RELEASE_AGENT_ENV`. `GITEA_TOKEN`, `RELEASE_*`, and malformed names fail validation before claim. - Shell operators, substitutions, and pipelines are not interpreted. Operators that are intentionally required must live in a separately reviewed wrapper script configured as the command. - Empty or malformed commands abort before the issue claim. - Non-zero coding-agent exit blocks tests and PR creation. @@ -96,3 +99,5 @@ repeating or advancing delivery. 12. Restarts from `agent_complete`, `tests_passed`, and `pushed` skip only the completed stages and open exactly one linked PR. 13. Dirty or identity-mismatched checkpoints stop before push or PR creation. +14. Parent credentials are absent from coding-agent, test, and Git subprocesses while explicit agent variables and issue context reach only the coding stage. +15. Reserved or malformed agent environment allowlist entries abort before claim. diff --git a/src/release_engine.py b/src/release_engine.py index 0d4d2b5..3f0b4f1 100644 --- a/src/release_engine.py +++ b/src/release_engine.py @@ -15,6 +15,10 @@ from urllib import error, request PRIORITY = {"P0": 0, "P1": 1, "P2": 2} +RUNTIME_ENV_NAMES = ( + "HOME", "LANG", "LC_ALL", "LC_CTYPE", "LOGNAME", "SHELL", + "SSH_AUTH_SOCK", "TMPDIR", "USER", +) @dataclass(frozen=True) @@ -47,6 +51,7 @@ class EngineConfig: agent_timeout_seconds: float = 1800.0 test_timeout_seconds: float = 900.0 termination_grace_seconds: float = 5.0 + agent_env_vars: tuple[str, ...] = () @dataclass(frozen=True) @@ -138,6 +143,30 @@ def parse_command(command: str, name: str) -> list[str]: return arguments +def subprocess_environment( + allowed_names: Iterable[str] = (), + overrides: dict[str, str] | None = None, +) -> dict[str, str]: + """Build an explicit child environment without inheriting parent secrets.""" + environment = {"PATH": os.environ.get("PATH", os.defpath)} + for name in (*RUNTIME_ENV_NAMES, *allowed_names): + if name in os.environ: + environment[name] = os.environ[name] + if overrides: + environment.update(overrides) + return environment + + +def validate_agent_env_vars(names: Iterable[str]) -> None: + for name in names: + if ( + re.fullmatch(r"[A-Za-z_][A-Za-z0-9_]*", name) is None + or name == "GITEA_TOKEN" + or name.startswith("RELEASE_") + ): + raise ValueError(f"invalid agent environment variable: {name}") + + class ShellRunner: def run( self, @@ -147,13 +176,10 @@ class ShellRunner: timeout_seconds: float | None = None, termination_grace_seconds: float = 5.0, ) -> CommandResult: - merged_env = os.environ.copy() - if env: - merged_env.update(env) process = subprocess.Popen( command, cwd=str(cwd), - env=merged_env, + env=dict(env or {}), shell=False, text=True, stdout=subprocess.PIPE, @@ -249,6 +275,7 @@ class ReleaseEngine: for name, value in deadlines.items(): if value <= 0: raise ValueError(f"{name} must be positive") + validate_agent_env_vars(self.config.agent_env_vars) issue = select_issue(self.client.list_open_issues(self.config.repo), self.config.agent, issue_number) branch = branch_name(self.config.agent, issue) @@ -276,6 +303,17 @@ class ReleaseEngine: agent_command = parse_command(self.config.agent_command, "agent command") test_command = parse_command(self.config.test_command, "test command") + runtime_env = subprocess_environment() + agent_env = subprocess_environment( + self.config.agent_env_vars, + { + "RELEASE_ISSUE_NUMBER": str(issue.number), + "RELEASE_ISSUE_TITLE": issue.title, + "RELEASE_ISSUE_BODY": issue.body, + "RELEASE_REPO": self.config.repo, + "RELEASE_BRANCH": branch, + }, + ) checkpoint = self.store.load() resumable_statuses = {"agent_complete", "tests_passed", "pushed"} if checkpoint is not None and checkpoint.status in resumable_statuses: @@ -287,14 +325,18 @@ class ReleaseEngine: or not checkpoint.head_sha ): raise RuntimeError("release checkpoint does not match this run") - branch_result = self.runner.run(["git", "checkout", branch], self.config.repo_path) + branch_result = self.runner.run( + ["git", "checkout", branch], self.config.repo_path, runtime_env + ) if branch_result.returncode != 0: raise RuntimeError(f"checkpoint branch checkout failed: {branch_result.stderr.strip()}") - head_result = self.runner.run(["git", "rev-parse", "HEAD"], self.config.repo_path) + head_result = self.runner.run( + ["git", "rev-parse", "HEAD"], self.config.repo_path, runtime_env + ) if head_result.returncode != 0 or head_result.stdout.strip() != checkpoint.head_sha: raise RuntimeError("release checkpoint commit does not match the local branch") status_result = self.runner.run( - ["git", "status", "--porcelain"], self.config.repo_path + ["git", "status", "--porcelain"], self.config.repo_path, runtime_env ) if status_result.returncode != 0 or status_result.stdout.strip(): raise RuntimeError("release checkpoint worktree is not clean") @@ -311,20 +353,16 @@ class ReleaseEngine: ) self.store.save(RunState(issue.number, branch, "claimed")) - branch_result = self.runner.run(["git", "checkout", "-B", branch], self.config.repo_path) + branch_result = self.runner.run( + ["git", "checkout", "-B", branch], self.config.repo_path, runtime_env + ) if branch_result.returncode != 0: raise RuntimeError(f"branch creation failed: {branch_result.stderr.strip()}") agent_result = self.runner.run( agent_command, self.config.repo_path, - { - "RELEASE_ISSUE_NUMBER": str(issue.number), - "RELEASE_ISSUE_TITLE": issue.title, - "RELEASE_ISSUE_BODY": issue.body, - "RELEASE_REPO": self.config.repo, - "RELEASE_BRANCH": branch, - }, + agent_env, timeout_seconds=self.config.agent_timeout_seconds, termination_grace_seconds=self.config.termination_grace_seconds, ) @@ -335,7 +373,9 @@ class ReleaseEngine: if agent_result.returncode != 0: self.store.save(RunState(issue.number, branch, "agent_failed", agent_result.stderr[-2000:])) raise RuntimeError(f"coding agent failed: {agent_result.stderr.strip()}") - head_result = self.runner.run(["git", "rev-parse", "HEAD"], self.config.repo_path) + head_result = self.runner.run( + ["git", "rev-parse", "HEAD"], self.config.repo_path, runtime_env + ) head_sha = head_result.stdout.strip() if head_result.returncode != 0 or not head_sha: raise RuntimeError("could not identify the release checkpoint commit") @@ -348,6 +388,7 @@ class ReleaseEngine: test_result = self.runner.run( test_command, self.config.repo_path, + runtime_env, timeout_seconds=self.config.test_timeout_seconds, termination_grace_seconds=self.config.termination_grace_seconds, ) @@ -373,13 +414,14 @@ class ReleaseEngine: remote_result = self.runner.run( ["git", "ls-remote", "--heads", "origin", branch], self.config.repo_path, + runtime_env, ) expected_remote = f"{checkpoint.head_sha}\trefs/heads/{branch}" if remote_result.returncode != 0 or remote_result.stdout.strip() != expected_remote: raise RuntimeError("release checkpoint commit does not match the remote branch") else: push_result = self.runner.run( - ["git", "push", "-u", "origin", branch], self.config.repo_path + ["git", "push", "-u", "origin", branch], self.config.repo_path, runtime_env ) if push_result.returncode != 0: self.store.save(RunState( @@ -442,6 +484,12 @@ def _parse_args() -> argparse.Namespace: parser.add_argument("--agent-timeout", type=float, default=float(os.getenv("RELEASE_AGENT_TIMEOUT", "1800"))) parser.add_argument("--test-timeout", type=float, default=float(os.getenv("RELEASE_TEST_TIMEOUT", "900"))) parser.add_argument("--termination-grace", type=float, default=float(os.getenv("RELEASE_TERMINATION_GRACE", "5"))) + parser.add_argument( + "--agent-env", + action="append", + default=[name for name in os.getenv("RELEASE_AGENT_ENV", "").split(",") if name], + help="parent environment variable to pass to the coding agent; repeat as needed", + ) return parser.parse_args() @@ -462,6 +510,7 @@ def main() -> int: agent_timeout_seconds=args.agent_timeout, test_timeout_seconds=args.test_timeout, termination_grace_seconds=args.termination_grace, + agent_env_vars=tuple(args.agent_env), ) result = ReleaseEngine(config, GiteaClient(args.api, token), ShellRunner()).execute( issue_number=args.issue, diff --git a/tests/test_release_engine.py b/tests/test_release_engine.py index e4620f9..d3419f4 100644 --- a/tests/test_release_engine.py +++ b/tests/test_release_engine.py @@ -1,4 +1,5 @@ import json +import os import sys import threading import time @@ -44,6 +45,29 @@ def test_shell_runner_treats_metacharacters_as_literal_arguments(tmp_path): assert not injected.exists() +def test_shell_runner_uses_only_the_explicit_environment(tmp_path, monkeypatch): + output = tmp_path / "environment.json" + monkeypatch.setenv("GITEA_TOKEN", "write-capable-secret") + monkeypatch.setenv("UNRELATED_SECRET", "also-secret") + + result = ShellRunner().run( + [ + sys.executable, + "-c", + "import json, os, sys; json.dump(dict(os.environ), open(sys.argv[1], 'w'))", + str(output), + ], + tmp_path, + {"PATH": os.environ["PATH"], "EXPECTED_VALUE": "available"}, + ) + + environment = json.loads(output.read_text()) + assert result.returncode == 0 + assert environment["EXPECTED_VALUE"] == "available" + assert "GITEA_TOKEN" not in environment + assert "UNRELATED_SECRET" not in environment + + def test_shell_runner_times_out_and_terminates_spawned_children(tmp_path): marker = tmp_path / "child-survived.txt" child = ( @@ -215,6 +239,18 @@ def test_non_positive_command_deadlines_fail_before_claim(tmp_path, field, value assert client.claimed == [] +@pytest.mark.parametrize("name", ["GITEA_TOKEN", "RELEASE_REPO", "bad-name", "1START"]) +def test_invalid_agent_environment_allowlist_fails_before_claim(tmp_path, name): + issue = Issue(14, "Engine", "body", "open", [], None) + client = FakeClient([issue]) + cfg = replace(config(tmp_path), agent_env_vars=(name,)) + + with pytest.raises(ValueError, match="invalid agent environment variable"): + ReleaseEngine(cfg, client, FakeRunner([])).execute(issue_number=14) + + assert client.claimed == [] + + def test_command_deadlines_are_configurable_from_cli(monkeypatch): monkeypatch.setattr(sys, "argv", [ "release-engine", @@ -222,6 +258,8 @@ def test_command_deadlines_are_configurable_from_cli(monkeypatch): "--agent-timeout", "120", "--test-timeout", "45.5", "--termination-grace", "2", + "--agent-env", "CODEX_HOME", + "--agent-env", "HTTPS_PROXY", ]) args = _parse_args() @@ -229,6 +267,7 @@ def test_command_deadlines_are_configurable_from_cli(monkeypatch): assert args.agent_timeout == 120 assert args.test_timeout == 45.5 assert args.termination_grace == 2 + assert args.agent_env == ["CODEX_HOME", "HTTPS_PROXY"] def test_agent_failure_blocks_tests_and_pr(tmp_path): @@ -306,6 +345,41 @@ def test_issue_content_reaches_agent_only_through_environment(tmp_path): assert runner.environments[1]["RELEASE_ISSUE_BODY"] == body +def test_release_stages_receive_least_privilege_environments(tmp_path, monkeypatch): + monkeypatch.setenv("GITEA_TOKEN", "write-capable-secret") + monkeypatch.setenv("UNRELATED_SECRET", "also-secret") + monkeypatch.setenv("CODEX_HOME", "/approved/codex") + issue = Issue(14, "Engine MVP", "sensitive body", "open", [], None) + client = FakeClient([issue]) + runner = FakeRunner([ + CommandResult(0, "", ""), + CommandResult(0, "implemented", ""), + CommandResult(0, "abc123\n", ""), + CommandResult(0, "8 passed", ""), + CommandResult(0, "", ""), + ]) + cfg = replace(config(tmp_path), agent_env_vars=("CODEX_HOME",)) + + ReleaseEngine(cfg, client, runner).execute(issue_number=14) + + for environment in runner.environments: + assert environment is not None + assert "PATH" in environment + assert "GITEA_TOKEN" not in environment + assert "UNRELATED_SECRET" not in environment + agent_environment = runner.environments[1] + assert agent_environment["CODEX_HOME"] == "/approved/codex" + assert agent_environment["RELEASE_ISSUE_NUMBER"] == "14" + assert agent_environment["RELEASE_ISSUE_TITLE"] == issue.title + assert agent_environment["RELEASE_ISSUE_BODY"] == issue.body + assert agent_environment["RELEASE_REPO"] == cfg.repo + assert agent_environment["RELEASE_BRANCH"] == "timmy/14-engine-mvp" + assert not any( + name.startswith("RELEASE_") + for name in runner.environments[3] + ) + + def test_failed_tests_block_push_and_pr(tmp_path): issue = Issue(14, "Engine", "body", "open", [], None) client = FakeClient([issue])