security: isolate release subprocess credentials (#333)
All checks were successful
CI / lint (pull_request) Successful in 31s
CI / build-frontend (pull_request) Successful in 5s

This commit is contained in:
timmy 2026-08-08 19:34:43 +00:00
parent 671c88d287
commit 7342b2e6de
4 changed files with 156 additions and 17 deletions

View File

@ -273,6 +273,7 @@ python3 -m src.release_engine \
--agent-timeout 1800 \ --agent-timeout 1800 \
--test-timeout 900 \ --test-timeout 900 \
--termination-grace 5 \ --termination-grace 5 \
--agent-env AGENT_CONFIG_HOME \
--test-command 'python3 -m pytest tests/ -q' --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 interpreted. Put any trusted shell workflow in a reviewed wrapper script and
configure that script as `RELEASE_AGENT_COMMAND` instead. 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 Agent and test deadlines are independent. On expiry, the engine terminates the
command's entire process group, escalates from `SIGTERM` to `SIGKILL` after 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` configured grace period, and records `agent_timed_out` or `tests_timed_out`

View File

@ -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. - Coding-agent command, parsed into an argument vector.
- Test 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. - 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. - Local repository path and durable state-file path.
## State machine ## State machine
@ -54,6 +55,8 @@ repeating or advancing delivery.
- Branch format: `<agent>/<issue>-<slug>`. - Branch format: `<agent>/<issue>-<slug>`.
- Coding and test commands are parsed with POSIX argument quoting and executed directly without a shell. - 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. - 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. - 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. - Empty or malformed commands abort before the issue claim.
- Non-zero coding-agent exit blocks tests and PR creation. - 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 12. Restarts from `agent_complete`, `tests_passed`, and `pushed` skip only the
completed stages and open exactly one linked PR. completed stages and open exactly one linked PR.
13. Dirty or identity-mismatched checkpoints stop before push or PR creation. 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.

View File

@ -15,6 +15,10 @@ from urllib import error, request
PRIORITY = {"P0": 0, "P1": 1, "P2": 2} 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) @dataclass(frozen=True)
@ -47,6 +51,7 @@ class EngineConfig:
agent_timeout_seconds: float = 1800.0 agent_timeout_seconds: float = 1800.0
test_timeout_seconds: float = 900.0 test_timeout_seconds: float = 900.0
termination_grace_seconds: float = 5.0 termination_grace_seconds: float = 5.0
agent_env_vars: tuple[str, ...] = ()
@dataclass(frozen=True) @dataclass(frozen=True)
@ -138,6 +143,30 @@ def parse_command(command: str, name: str) -> list[str]:
return arguments 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: class ShellRunner:
def run( def run(
self, self,
@ -147,13 +176,10 @@ class ShellRunner:
timeout_seconds: float | None = None, timeout_seconds: float | None = None,
termination_grace_seconds: float = 5.0, termination_grace_seconds: float = 5.0,
) -> CommandResult: ) -> CommandResult:
merged_env = os.environ.copy()
if env:
merged_env.update(env)
process = subprocess.Popen( process = subprocess.Popen(
command, command,
cwd=str(cwd), cwd=str(cwd),
env=merged_env, env=dict(env or {}),
shell=False, shell=False,
text=True, text=True,
stdout=subprocess.PIPE, stdout=subprocess.PIPE,
@ -249,6 +275,7 @@ class ReleaseEngine:
for name, value in deadlines.items(): for name, value in deadlines.items():
if value <= 0: if value <= 0:
raise ValueError(f"{name} must be positive") 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) issue = select_issue(self.client.list_open_issues(self.config.repo), self.config.agent, issue_number)
branch = branch_name(self.config.agent, issue) branch = branch_name(self.config.agent, issue)
@ -276,6 +303,17 @@ class ReleaseEngine:
agent_command = parse_command(self.config.agent_command, "agent command") agent_command = parse_command(self.config.agent_command, "agent command")
test_command = parse_command(self.config.test_command, "test 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() checkpoint = self.store.load()
resumable_statuses = {"agent_complete", "tests_passed", "pushed"} resumable_statuses = {"agent_complete", "tests_passed", "pushed"}
if checkpoint is not None and checkpoint.status in resumable_statuses: if checkpoint is not None and checkpoint.status in resumable_statuses:
@ -287,14 +325,18 @@ class ReleaseEngine:
or not checkpoint.head_sha or not checkpoint.head_sha
): ):
raise RuntimeError("release checkpoint does not match this run") 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: if branch_result.returncode != 0:
raise RuntimeError(f"checkpoint branch checkout failed: {branch_result.stderr.strip()}") 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: if head_result.returncode != 0 or head_result.stdout.strip() != checkpoint.head_sha:
raise RuntimeError("release checkpoint commit does not match the local branch") raise RuntimeError("release checkpoint commit does not match the local branch")
status_result = self.runner.run( 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(): if status_result.returncode != 0 or status_result.stdout.strip():
raise RuntimeError("release checkpoint worktree is not clean") raise RuntimeError("release checkpoint worktree is not clean")
@ -311,20 +353,16 @@ class ReleaseEngine:
) )
self.store.save(RunState(issue.number, branch, "claimed")) 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: if branch_result.returncode != 0:
raise RuntimeError(f"branch creation failed: {branch_result.stderr.strip()}") raise RuntimeError(f"branch creation failed: {branch_result.stderr.strip()}")
agent_result = self.runner.run( agent_result = self.runner.run(
agent_command, agent_command,
self.config.repo_path, self.config.repo_path,
{ agent_env,
"RELEASE_ISSUE_NUMBER": str(issue.number),
"RELEASE_ISSUE_TITLE": issue.title,
"RELEASE_ISSUE_BODY": issue.body,
"RELEASE_REPO": self.config.repo,
"RELEASE_BRANCH": branch,
},
timeout_seconds=self.config.agent_timeout_seconds, timeout_seconds=self.config.agent_timeout_seconds,
termination_grace_seconds=self.config.termination_grace_seconds, termination_grace_seconds=self.config.termination_grace_seconds,
) )
@ -335,7 +373,9 @@ class ReleaseEngine:
if agent_result.returncode != 0: if agent_result.returncode != 0:
self.store.save(RunState(issue.number, branch, "agent_failed", agent_result.stderr[-2000:])) self.store.save(RunState(issue.number, branch, "agent_failed", agent_result.stderr[-2000:]))
raise RuntimeError(f"coding agent failed: {agent_result.stderr.strip()}") 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() head_sha = head_result.stdout.strip()
if head_result.returncode != 0 or not head_sha: if head_result.returncode != 0 or not head_sha:
raise RuntimeError("could not identify the release checkpoint commit") raise RuntimeError("could not identify the release checkpoint commit")
@ -348,6 +388,7 @@ class ReleaseEngine:
test_result = self.runner.run( test_result = self.runner.run(
test_command, test_command,
self.config.repo_path, self.config.repo_path,
runtime_env,
timeout_seconds=self.config.test_timeout_seconds, timeout_seconds=self.config.test_timeout_seconds,
termination_grace_seconds=self.config.termination_grace_seconds, termination_grace_seconds=self.config.termination_grace_seconds,
) )
@ -373,13 +414,14 @@ class ReleaseEngine:
remote_result = self.runner.run( remote_result = self.runner.run(
["git", "ls-remote", "--heads", "origin", branch], ["git", "ls-remote", "--heads", "origin", branch],
self.config.repo_path, self.config.repo_path,
runtime_env,
) )
expected_remote = f"{checkpoint.head_sha}\trefs/heads/{branch}" expected_remote = f"{checkpoint.head_sha}\trefs/heads/{branch}"
if remote_result.returncode != 0 or remote_result.stdout.strip() != expected_remote: if remote_result.returncode != 0 or remote_result.stdout.strip() != expected_remote:
raise RuntimeError("release checkpoint commit does not match the remote branch") raise RuntimeError("release checkpoint commit does not match the remote branch")
else: else:
push_result = self.runner.run( 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: if push_result.returncode != 0:
self.store.save(RunState( 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("--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("--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("--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() return parser.parse_args()
@ -462,6 +510,7 @@ def main() -> int:
agent_timeout_seconds=args.agent_timeout, agent_timeout_seconds=args.agent_timeout,
test_timeout_seconds=args.test_timeout, test_timeout_seconds=args.test_timeout,
termination_grace_seconds=args.termination_grace, termination_grace_seconds=args.termination_grace,
agent_env_vars=tuple(args.agent_env),
) )
result = ReleaseEngine(config, GiteaClient(args.api, token), ShellRunner()).execute( result = ReleaseEngine(config, GiteaClient(args.api, token), ShellRunner()).execute(
issue_number=args.issue, issue_number=args.issue,

View File

@ -1,4 +1,5 @@
import json import json
import os
import sys import sys
import threading import threading
import time import time
@ -44,6 +45,29 @@ def test_shell_runner_treats_metacharacters_as_literal_arguments(tmp_path):
assert not injected.exists() 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): def test_shell_runner_times_out_and_terminates_spawned_children(tmp_path):
marker = tmp_path / "child-survived.txt" marker = tmp_path / "child-survived.txt"
child = ( child = (
@ -215,6 +239,18 @@ def test_non_positive_command_deadlines_fail_before_claim(tmp_path, field, value
assert client.claimed == [] 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): def test_command_deadlines_are_configurable_from_cli(monkeypatch):
monkeypatch.setattr(sys, "argv", [ monkeypatch.setattr(sys, "argv", [
"release-engine", "release-engine",
@ -222,6 +258,8 @@ def test_command_deadlines_are_configurable_from_cli(monkeypatch):
"--agent-timeout", "120", "--agent-timeout", "120",
"--test-timeout", "45.5", "--test-timeout", "45.5",
"--termination-grace", "2", "--termination-grace", "2",
"--agent-env", "CODEX_HOME",
"--agent-env", "HTTPS_PROXY",
]) ])
args = _parse_args() args = _parse_args()
@ -229,6 +267,7 @@ def test_command_deadlines_are_configurable_from_cli(monkeypatch):
assert args.agent_timeout == 120 assert args.agent_timeout == 120
assert args.test_timeout == 45.5 assert args.test_timeout == 45.5
assert args.termination_grace == 2 assert args.termination_grace == 2
assert args.agent_env == ["CODEX_HOME", "HTTPS_PROXY"]
def test_agent_failure_blocks_tests_and_pr(tmp_path): 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 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): def test_failed_tests_block_push_and_pr(tmp_path):
issue = Issue(14, "Engine", "body", "open", [], None) issue = Issue(14, "Engine", "body", "open", [], None)
client = FakeClient([issue]) client = FakeClient([issue])