Skip to content

Commit c025357

Browse files
josephperrottatscott
authored andcommitted
feat(dev-infra): Add oauth scope check to ensure necessary permissions for merge tooling (angular#37421)
Adds an assertion that the provided TOKEN has OAuth scope permissions for `repo` as this is required for all merge attempts. On failure, provides detailed error message with remediation steps for the user. PR Close angular#37421
1 parent 57411c8 commit c025357

File tree

4 files changed

+80
-5
lines changed

4 files changed

+80
-5
lines changed

dev-infra/pr/merge/failures.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,9 @@ export class PullRequestFailure {
7171
return new this(`Pull request could not be found upstream.`);
7272
}
7373

74-
static insufficientPermissionsToMerge() {
75-
return new this(
76-
`Insufficient Github API permissions to merge pull request. Please ` +
77-
`ensure that your auth token has write access.`);
74+
static insufficientPermissionsToMerge(
75+
message = `Insufficient Github API permissions to merge pull request. Please ensure that ` +
76+
`your auth token has write access.`) {
77+
return new this(message);
7878
}
7979
}

dev-infra/pr/merge/git.ts

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,15 @@
99
import * as Octokit from '@octokit/rest';
1010
import {spawnSync, SpawnSyncOptions, SpawnSyncReturns} from 'child_process';
1111

12-
import {info} from '../../utils/console';
12+
import {info, yellow} from '../../utils/console';
1313

1414
import {MergeConfigWithRemote} from './config';
1515

16+
/** Github response type extended to include the `x-oauth-scopes` headers presence. */
17+
type RateLimitResponseWithOAuthScopeHeader = Octokit.Response<Octokit.RateLimitGetResponse>&{
18+
headers: {'x-oauth-scopes': string};
19+
};
20+
1621
/** Error for failed Github API requests. */
1722
export class GithubApiRequestError extends Error {
1823
constructor(public status: number, message: string) {
@@ -43,6 +48,8 @@ export class GitClient {
4348
/** Instance of the authenticated Github octokit API. */
4449
api: Octokit;
4550

51+
/** The OAuth scopes available for the provided Github token. */
52+
private _oauthScopes = Promise.resolve<string[]>([]);
4653
/** Regular expression that matches the provided Github token. */
4754
private _tokenRegex = new RegExp(this._githubToken, 'g');
4855

@@ -117,4 +124,55 @@ export class GitClient {
117124
omitGithubTokenFromMessage(value: string): string {
118125
return value.replace(this._tokenRegex, '<TOKEN>');
119126
}
127+
128+
/**
129+
* Assert the GitClient instance is using a token with permissions for the all of the
130+
* provided OAuth scopes.
131+
*/
132+
async hasOauthScopes(...requestedScopes: string[]): Promise<true|{error: string}> {
133+
const missingScopes: string[] = [];
134+
const scopes = await this.getAuthScopes();
135+
requestedScopes.forEach(scope => {
136+
if (!scopes.includes(scope)) {
137+
missingScopes.push(scope);
138+
}
139+
});
140+
// If no missing scopes are found, return true to indicate all OAuth Scopes are available.
141+
if (missingScopes.length === 0) {
142+
return true;
143+
}
144+
145+
/**
146+
* Preconstructed error message to log to the user, providing missing scopes and
147+
* remediation instructions.
148+
**/
149+
const error =
150+
`The provided <TOKEN> does not have required permissions due to missing scope(s): ` +
151+
`${yellow(missingScopes.join(', '))}\n\n` +
152+
`Update the token in use at:\n` +
153+
` https://github.com/settings/tokens\n\n` +
154+
`Alternatively, a new token can be created at: https://github.com/settings/tokens/new\n`;
155+
156+
return {error};
157+
}
158+
159+
160+
/**
161+
* Retrieves the OAuth scopes for the loaded Github token, returning the already retrived
162+
* list of OAuth scopes if available.
163+
**/
164+
private getAuthScopes() {
165+
// If the OAuth scopes have already been loaded, return the Promise containing them.
166+
if (this._oauthScopes) {
167+
return this._oauthScopes;
168+
}
169+
// OAuth scopes are loaded via the /rate_limit endpoint to prevent
170+
// usage of a request against that rate_limit for this lookup.
171+
this._oauthScopes = this.api.rateLimit.get().then(_response => {
172+
const response = _response as RateLimitResponseWithOAuthScopeHeader;
173+
const scopes: string = response.headers['x-oauth-scopes'] || '';
174+
return scopes.split(',').map(scope => scope.trim());
175+
});
176+
return this._oauthScopes;
177+
}
120178
}

dev-infra/pr/merge/index.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,10 @@ export async function mergePullRequest(
110110
red('An unknown Git error has been thrown. Please check the output ' +
111111
'above for details.'));
112112
return false;
113+
case MergeStatus.GITHUB_ERROR:
114+
error(red('An error related to interacting with Github has been discovered.'));
115+
error(failure!.message);
116+
return false;
113117
case MergeStatus.FAILED:
114118
error(yellow(`Could not merge the specified pull request.`));
115119
error(red(failure!.message));

dev-infra/pr/merge/task.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,16 @@ import {isPullRequest, loadAndValidatePullRequest,} from './pull-request';
1313
import {GithubApiMergeStrategy} from './strategies/api-merge';
1414
import {AutosquashMergeStrategy} from './strategies/autosquash-merge';
1515

16+
/** Github OAuth scopes required for the merge task. */
17+
const REQUIRED_SCOPES = ['repo'];
18+
1619
/** Describes the status of a pull request merge. */
1720
export const enum MergeStatus {
1821
UNKNOWN_GIT_ERROR,
1922
DIRTY_WORKING_DIR,
2023
SUCCESS,
2124
FAILED,
25+
GITHUB_ERROR,
2226
}
2327

2428
/** Result of a pull request merge. */
@@ -48,6 +52,15 @@ export class PullRequestMergeTask {
4852
* @param force Whether non-critical pull request failures should be ignored.
4953
*/
5054
async merge(prNumber: number, force = false): Promise<MergeResult> {
55+
// Assert the authenticated GitClient has access on the required scopes.
56+
const hasOauthScopes = await this.git.hasOauthScopes(...REQUIRED_SCOPES);
57+
if (hasOauthScopes !== true) {
58+
return {
59+
status: MergeStatus.GITHUB_ERROR,
60+
failure: PullRequestFailure.insufficientPermissionsToMerge(hasOauthScopes.error)
61+
};
62+
}
63+
5164
if (this.git.hasUncommittedChanges()) {
5265
return {status: MergeStatus.DIRTY_WORKING_DIR};
5366
}

0 commit comments

Comments
 (0)