security: isolate release issue content from shell execution (#307)
This commit is contained in:
1 parent
bd5e66b4e5
commit
7d633e67e8
4 files changed
+111
-23
No files matched your search
+20
-15
@@ -63,7 +63,7 @@ class Client(Protocol):
|
||||
|
||||
|
||||
class Runner(Protocol):
|
||||
def run(self, command: str, cwd: Path, env: dict[str, str] | None = None) -> CommandResult: ...
|
||||
def run(self, command: list[str], cwd: Path, env: dict[str, str] | None = None) -> CommandResult: ...
|
||||
|
||||
|
||||
class StateStore:
|
||||
@@ -113,8 +113,18 @@ def branch_name(agent: str, issue: Issue) -> str:
|
||||
return f"{actor}/{issue.number}-{slug or 'issue'}"
|
||||
|
||||
|
||||
def parse_command(command: str, name: str) -> list[str]:
|
||||
try:
|
||||
arguments = shlex.split(command)
|
||||
except ValueError as exc:
|
||||
raise ValueError(f"invalid {name}: {exc}") from exc
|
||||
if not arguments:
|
||||
raise ValueError(f"invalid {name}: command is empty")
|
||||
return arguments
|
||||
|
||||
|
||||
class ShellRunner:
|
||||
def run(self, command: str, cwd: Path, env: dict[str, str] | None = None) -> CommandResult:
|
||||
def run(self, command: list[str], cwd: Path, env: dict[str, str] | None = None) -> CommandResult:
|
||||
merged_env = os.environ.copy()
|
||||
if env:
|
||||
merged_env.update(env)
|
||||
@@ -122,7 +132,7 @@ class ShellRunner:
|
||||
command,
|
||||
cwd=str(cwd),
|
||||
env=merged_env,
|
||||
shell=True,
|
||||
shell=False,
|
||||
text=True,
|
||||
capture_output=True,
|
||||
)
|
||||
@@ -220,6 +230,9 @@ class ReleaseEngine:
|
||||
"agent": self.config.agent,
|
||||
}
|
||||
|
||||
agent_command = parse_command(self.config.agent_command, "agent command")
|
||||
test_command = parse_command(self.config.test_command, "test command")
|
||||
|
||||
self.client.claim_issue(self.config.repo, issue.number, self.config.agent)
|
||||
verified = self.client.get_issue(self.config.repo, issue.number)
|
||||
if verified.assignee != self.config.agent:
|
||||
@@ -228,19 +241,11 @@ class ReleaseEngine:
|
||||
)
|
||||
self.store.save(RunState(issue.number, branch, "claimed"))
|
||||
|
||||
branch_result = self.runner.run(f"git checkout -B {shlex.quote(branch)}", self.config.repo_path)
|
||||
branch_result = self.runner.run(["git", "checkout", "-B", branch], self.config.repo_path)
|
||||
if branch_result.returncode != 0:
|
||||
raise RuntimeError(f"branch creation failed: {branch_result.stderr.strip()}")
|
||||
|
||||
context = {
|
||||
"issue_number": issue.number,
|
||||
"issue_title": issue.title,
|
||||
"issue_body": issue.body,
|
||||
"repo": self.config.repo,
|
||||
"branch": branch,
|
||||
}
|
||||
command = self.config.agent_command.format(**context)
|
||||
agent_result = self.runner.run(command, self.config.repo_path, {
|
||||
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,
|
||||
@@ -252,14 +257,14 @@ class ReleaseEngine:
|
||||
raise RuntimeError(f"coding agent failed: {agent_result.stderr.strip()}")
|
||||
self.store.save(RunState(issue.number, branch, "agent_complete"))
|
||||
|
||||
test_result = self.runner.run(self.config.test_command, self.config.repo_path)
|
||||
test_result = self.runner.run(test_command, self.config.repo_path)
|
||||
evidence = (test_result.stdout + "\n" + test_result.stderr).strip()[-4000:]
|
||||
if test_result.returncode != 0:
|
||||
self.store.save(RunState(issue.number, branch, "tests_failed", evidence))
|
||||
raise RuntimeError(f"tests failed: {evidence}")
|
||||
self.store.save(RunState(issue.number, branch, "tests_passed", evidence))
|
||||
|
||||
push_result = self.runner.run(f"git push -u origin {shlex.quote(branch)}", self.config.repo_path)
|
||||
push_result = self.runner.run(["git", "push", "-u", "origin", branch], self.config.repo_path)
|
||||
if push_result.returncode != 0:
|
||||
self.store.save(RunState(issue.number, branch, "push_failed", push_result.stderr[-2000:]))
|
||||
raise RuntimeError(f"push failed: {push_result.stderr.strip()}")
|
||||
|
||||
Reference in new issue
Block a user