Skip to content

Use checked subprocess calls in PR scheduler #75

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
seonghobae merged 1 commit into from
Jun 26, 2026
Merged

Use checked subprocess calls in PR scheduler #75

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions scripts/ci/pr_review_merge_scheduler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,13 +301,21 @@ def run(args: Sequence[str], *, stdin: str | None = None) -> str:
if isinstance(args, str) or not all(isinstance(arg, str) for arg in args):
raise TypeError("run() requires a sequence of argv strings; shell command strings are not allowed")
argv = list(args)
process = subprocess.run(argv, input=stdin, capture_output=True, text=True, shell=False)
if process.returncode != 0:
try:
process = subprocess.run(
argv,
input=stdin,
capture_output=True,
text=True,
shell=False,
check=True,
)
except subprocess.CalledProcessError as exc:
scrubbed_args = scrub_sensitive_data(' '.join(argv))
scrubbed_stderr = scrub_sensitive_data(process.stderr or "")
scrubbed_stderr = scrub_sensitive_data(exc.stderr or "")
raise RuntimeError(
f"Command failed ({process.returncode}): {scrubbed_args}\n{scrubbed_stderr}"
)
f"Command failed ({exc.returncode}): {scrubbed_args}\n{scrubbed_stderr}"
) from exc
return process.stdout


Expand Down
1 change: 1 addition & 0 deletions scripts/ci/test_strix_quick_gate.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -820,6 +820,7 @@ assert_pr_review_merge_scheduler_uses_github_actions_bot_token() {
assert_file_contains "$scheduler_file" "update-branch" "scheduler calls the GitHub update-branch API for outdated approved PRs"
assert_file_contains "$scheduler_file" "expected_head_sha={head}" "scheduler guards branch updates with the current PR head SHA"
assert_file_contains "$scheduler_file" "shell=False" "scheduler subprocess wrapper forbids shell command execution"
assert_file_contains "$scheduler_file" "check=True" "scheduler subprocess wrapper raises on failed commands"
assert_file_contains "$REPO_ROOT/tests/test_pr_review_merge_scheduler.py" "test_run_passes_shell_metacharacters_as_plain_arguments" "scheduler tests prove branch-like shell metacharacters stay argv data"
assert_file_contains "$scheduler_file" "dispatch_strix_evidence" "scheduler dispatches same-head Strix evidence before OpenCode review"
assert_file_contains "$scheduler_file" "--security-workflow" "scheduler allows the canonical Strix workflow name to be configured"
Expand Down