fix(auto_submit): Pass requestSha to GitHub merge API (#5107)
Defense in depth - prevent autosubmit from trying to land a pushed (but not approved) pr.
This is not a real issue since we have branch protection rules, but it's good to have.
diff --git a/auto_submit/lib/service/validation_service.dart b/auto_submit/lib/service/validation_service.dart
index acd713f..9f2edcd 100644
--- a/auto_submit/lib/service/validation_service.dart
+++ b/auto_submit/lib/service/validation_service.dart
@@ -82,7 +82,20 @@
if (pullRequest.isMergeQueueEnabled) {
return _enqueuePullRequest(slug, pullRequest);
} else {
- return _mergePullRequest(number, commitMessage, slug);
+ if (pullRequest.head?.sha == null) {
+ return (
+ result: false,
+ message:
+ 'Failed to merge ${slug.fullName}/#${pullRequest.number}: invalid head',
+ method: SubmitMethod.merge,
+ );
+ }
+ return _mergePullRequest(
+ number,
+ commitMessage,
+ slug,
+ requestSha: pullRequest.head!.sha!,
+ );
}
}
@@ -122,8 +135,9 @@
Future<MergeResult> _mergePullRequest(
int number,
String commitMessage,
- github.RepositorySlug slug,
- ) async {
+ github.RepositorySlug slug, {
+ required String requestSha,
+ }) async {
try {
github.PullRequestMerge? result;
@@ -134,6 +148,7 @@
slug: slug,
number: number,
mergeMethod: github.MergeMethod.squash,
+ requestSha: requestSha,
);
}, retryIf: (Exception e) => e is RetryableException);
diff --git a/auto_submit/test/service/pull_request_validation_service_test.dart b/auto_submit/test/service/pull_request_validation_service_test.dart
index cfeb94d..0cd1612 100644
--- a/auto_submit/test/service/pull_request_validation_service_test.dart
+++ b/auto_submit/test/service/pull_request_validation_service_test.dart
@@ -288,6 +288,33 @@
);
expect(result.message, contains('Reland "My first PR!"'));
+ expect(githubService.mergePrShaMap[0], pullRequest.head?.sha);
+ });
+
+ test('Fails for invalid sha', () async {
+ final pullRequest = generatePullRequest(
+ prNumber: 1001,
+ repoName: slug.name,
+ title: 'Revert "Revert "My first PR!"',
+ mergeable: true,
+ );
+ pullRequest.head!.sha = null;
+ githubService.pullRequestData = pullRequest;
+ githubService.mergeRequestMock = PullRequestMerge(
+ merged: true,
+ sha: pullRequest.mergeCommitSha,
+ );
+
+ final result = await validationService.submitPullRequest(
+ config: config,
+ pullRequest: pullRequest,
+ );
+
+ expect(
+ result.message,
+ contains('Failed to merge flutter/cocoon/#1001: invalid head'),
+ );
+ expect(githubService.mergePrShaMap[1001], isNull);
});
test(
diff --git a/auto_submit/test/src/service/fake_github_service.dart b/auto_submit/test/src/service/fake_github_service.dart
index cc94df0..7abff92 100644
--- a/auto_submit/test/src/service/fake_github_service.dart
+++ b/auto_submit/test/src/service/fake_github_service.dart
@@ -341,6 +341,9 @@
bool throwExceptionOnMerge = false;
+ /// Recorded commit shas made during [mergePullRequest].
+ Map<int, String?> mergePrShaMap = <int, String?>{};
+
/// If useMergeRequestMockList is true then we will return elements from that
/// list until it is empty.
///
@@ -358,6 +361,7 @@
throw Exception('Exception occurred during merging of pull request.');
}
verifyPullRequestMergeCallMap[number] = slug;
+ mergePrShaMap[number] = requestSha;
if (useMergeRequestMockList) {
return pullRequestMergeMockList.removeAt(0);
} else {