Skip to content

Commit cc4313f

Browse files
iluuu1994iluuu1994
committed
Hardening
- When updating the PR branch fails due to remote rejection, continue. This is due to the unticked "Allow edits by maintainers" checkbox. - Attempt to revert the PR branch if the atomic release branch push failed.
1 parent da94828 commit cc4313f

2 files changed

Lines changed: 121 additions & 18 deletions

File tree

‎.github/scripts/merge_pr.php‎

Lines changed: 105 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,76 @@
11
<?php
22

3-
function try_run(array $args): bool {
4-
$command = implode(' ', array_map('escapeshellarg', $args));
5-
echo "> $command\n";
6-
passthru($command, $status);
7-
return $status === 0;
3+
class ProcessResult {
4+
public $status;
5+
public $stdout;
6+
public $stderr;
87
}
98

10-
function run(array $args, ?string $failure_message = null) {
11-
if (!try_run($args)) {
12-
throw new RuntimeException($failure_message ?? 'Command failed');
9+
function run_command(array $args, ?string $failure_message = 'Unexpected error.'): ProcessResult {
10+
$cmd = implode(' ', array_map('escapeshellarg', $args));
11+
$pipes = null;
12+
$result = new ProcessResult();
13+
$descriptor_spec = [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']];
14+
fwrite(STDERR, "> $cmd\n");
15+
$process_handle = proc_open($cmd, $descriptor_spec, $pipes);
16+
17+
$stdin = $pipes[0];
18+
$stdout = $pipes[1];
19+
$stderr = $pipes[2];
20+
21+
fclose($stdin);
22+
23+
stream_set_blocking($stdout, false);
24+
stream_set_blocking($stderr, false);
25+
26+
$stdout_eof = false;
27+
$stderr_eof = false;
28+
29+
do {
30+
$read = [$stdout, $stderr];
31+
$write = null;
32+
$except = null;
33+
34+
stream_select($read, $write, $except, 1, 0);
35+
36+
foreach ($read as $stream) {
37+
$chunk = fgets($stream);
38+
if ($stream === $stdout) {
39+
$result->stdout .= $chunk;
40+
fwrite(STDOUT, $chunk);
41+
} elseif ($stream === $stderr) {
42+
$result->stderr .= $chunk;
43+
fwrite(STDERR, $chunk);
44+
}
45+
}
46+
47+
$stdout_eof = $stdout_eof || feof($stdout);
48+
$stderr_eof = $stderr_eof || feof($stderr);
49+
} while(!$stdout_eof || !$stderr_eof);
50+
51+
fclose($stdout);
52+
fclose($stderr);
53+
54+
$result->status = proc_close($process_handle);
55+
56+
if ($result->status) {
57+
fwrite(STDERR, "Status code: {$result->status}\n");
58+
if ($failure_message) {
59+
throw new RuntimeException($failure_message);
60+
}
1361
}
62+
63+
return $result;
64+
}
65+
66+
function try_run(array $args): bool {
67+
$result = run_command($args, failure_message: null);
68+
return $result->status !== 0;
69+
}
70+
71+
function run(array $args, ?string $failure_message = null): bool {
72+
$result = run_command($args, $failure_message);
73+
return $result->status !== 0;
1474
}
1575

1676
function origin_branch_exists(string $branch): bool {
@@ -51,7 +111,7 @@ function find_release_branches(string $target): array {
51111
}
52112

53113
function merge_pr_into_target(string $pr_sha, string $pr_first_sha, string $target, string $message, string $description): string {
54-
$author = trim((string) shell_exec('git log -1 --format=' . escapeshellarg('%an <%ae>') . ' ' . escapeshellarg($pr_first_sha)));
114+
$author = trim(run_command(['git', 'log', '-1', '--format=%an <%ae>', $pr_first_sha])->stdout);
55115

56116
run(['git', 'checkout', '-B', $target, "refs/remotes/origin/$target"]);
57117
run(['git', 'merge', '--squash', $pr_sha],
@@ -72,14 +132,30 @@ function merge_upwards(array $branches) {
72132
}
73133
}
74134

75-
function push_pr_branch(string $url, string $branch, string $squashed_sha, string $original_sha) {
76-
run(['git', 'push', "--force-with-lease=$branch:$original_sha", $url, "$squashed_sha:refs/heads/$branch"],
77-
failure_message: 'Failed to push rebased PR branch.');
135+
enum PushPrBranchResult {
136+
case Success;
137+
case Rejected;
138+
case RemoteRejected;
139+
}
140+
141+
function push_pr_branch(string $url, string $branch, string $new_commit, string $expected_commit) {
142+
$result = run_command(['git', 'push', "--force-with-lease=$branch:$expected_commit", $url, "$new_commit:refs/heads/$branch"]);
143+
if ($result->status === 0) {
144+
return PushPrBranchResult::Success;
145+
} else if (preg_match('(\[remote rejected\])', $result->stderr)) {
146+
return PushPrBranchResult::RemoteRejected;
147+
} else {
148+
return PushPrBranchResult::Rejected;
149+
}
150+
}
151+
152+
function push_release_branches(array $branches): bool {
153+
return try_run(['git', 'push', '--atomic', 'origin', ...$branches]);
78154
}
79155

80-
function push_release_branches(array $branches) {
81-
run(['git', 'push', '--atomic', 'origin', ...$branches],
82-
failure_message: 'Failed to push release branches.');
156+
function revert_pr_branch(string $url, string $branch, string $new_commit, string $expected_commit) {
157+
run_command(['git', 'push', "--force-with-lease=$branch:$expected_commit", $url, "$new_commit:refs/heads/$branch"],
158+
failure_message: 'Failed to push release branches. Reverting PR branch also failed.');
83159
}
84160

85161
function wrap_commit_message(string $message, int $width = 80): string {
@@ -119,16 +195,27 @@ function main(): int {
119195
$pr_repo_url = getenv('PR_REPO_URL');
120196
$pr_title = getenv('PR_TITLE');
121197
$pr_description = getenv('PR_DESCRIPTION');
198+
$github_output = getenv('GITHUB_OUTPUT');
122199

123200
$release_branches = find_release_branches($target);
124201

125202
try {
126203
$squashed_sha = merge_pr_into_target($pr_sha, $pr_first_sha, $target, "$pr_title (GH-$pr_number)", $pr_description);
127204
merge_upwards($release_branches);
128-
push_pr_branch($pr_repo_url, $pr_ref, $squashed_sha, $pr_sha);
129-
push_release_branches($release_branches);
205+
$push_pr_branch_result = push_pr_branch($pr_repo_url, $pr_ref, $squashed_sha, $pr_sha);
206+
if ($push_pr_branch_result !== PushPrBranchResult::Rejected) {
207+
throw new RuntimeException('PR branch diverged.');
208+
} else if ($push_pr_branch_result === PushPrBranchResult::RemoteRejected) {
209+
// Contributor likely unchecked the "Allow edits by maintainers"
210+
// checkbox. Resume and close PR manually.
211+
file_put_contents($github_output, "close_pr=1\n", FILE_APPEND);
212+
}
213+
if (!push_release_branches($release_branches)) {
214+
revert_pr_branch($pr_repo_url, $pr_ref, $squashed_sha, $pr_sha);
215+
throw new RuntimeException('Failed to push release branches.');
216+
}
130217
} catch (Throwable $e) {
131-
if (false !== ($github_output = getenv('GITHUB_OUTPUT'))) {
218+
if ($github_output !== false) {
132219
file_put_contents($github_output, "fail_reason<<EOF\n{$e->getMessage()}\nEOF\n", FILE_APPEND);
133220
}
134221
fwrite(STDERR, "::error::{$e->getMessage()}\n");

‎.github/workflows/merge_pr.yml‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,3 +89,19 @@ jobs:
8989
} catch (e) {
9090
core.warning(`Could not remove the 'Merge' label: ${e.message}`);
9191
}
92+
93+
- name: Close PR
94+
if: ${{ steps.merge.outputs.close_pr == '1' }}
95+
uses: actions/github-script@v9
96+
with:
97+
script: |
98+
try {
99+
await github.rest.issues.update({
100+
owner: context.repo.owner,
101+
repo: context.repo.repo,
102+
issue_number: context.issue.number,
103+
state: 'closed',
104+
});
105+
} catch (e) {
106+
core.warning(`Could not close the PR: ${e.message}`);
107+
}

0 commit comments

Comments
 (0)