fix(dev-infra): merge script should not always require full repo permissions (#37718)
We recently added OAuth scope checking to the dev-infra Git client and started leveraging it for the merge script. We set the `repo` scope as required for running the merge script. We can loosen this requirement as in the Angular org where the script is consumed, only pull requests on public repositories are merged through the script. This should help with reducing the risk with compromised tokens as no access had to be granted on `repo:invite`, `repo_deployment` etc. PR Close #37718
This commit is contained in:
parent
dbc2364d16
commit
3ee666580a
|
@ -16,9 +16,6 @@ import {isPullRequest, loadAndValidatePullRequest,} from './pull-request';
|
||||||
import {GithubApiMergeStrategy} from './strategies/api-merge';
|
import {GithubApiMergeStrategy} from './strategies/api-merge';
|
||||||
import {AutosquashMergeStrategy} from './strategies/autosquash-merge';
|
import {AutosquashMergeStrategy} from './strategies/autosquash-merge';
|
||||||
|
|
||||||
/** Github OAuth scopes required for the merge task. */
|
|
||||||
const REQUIRED_SCOPES = ['repo'];
|
|
||||||
|
|
||||||
/** Describes the status of a pull request merge. */
|
/** Describes the status of a pull request merge. */
|
||||||
export const enum MergeStatus {
|
export const enum MergeStatus {
|
||||||
UNKNOWN_GIT_ERROR,
|
UNKNOWN_GIT_ERROR,
|
||||||
|
@ -56,8 +53,19 @@ export class PullRequestMergeTask {
|
||||||
* @param force Whether non-critical pull request failures should be ignored.
|
* @param force Whether non-critical pull request failures should be ignored.
|
||||||
*/
|
*/
|
||||||
async merge(prNumber: number, force = false): Promise<MergeResult> {
|
async merge(prNumber: number, force = false): Promise<MergeResult> {
|
||||||
// Assert the authenticated GitClient has access on the required scopes.
|
// Check whether the given Github token has sufficient permissions for writing
|
||||||
const hasOauthScopes = await this.git.hasOauthScopes(...REQUIRED_SCOPES);
|
// to the configured repository. If the repository is not private, only the
|
||||||
|
// reduced `public_repo` OAuth scope is sufficient for performing merges.
|
||||||
|
const hasOauthScopes = await this.git.hasOauthScopes((scopes, missing) => {
|
||||||
|
if (!scopes.includes('repo')) {
|
||||||
|
if (this.config.remote.private) {
|
||||||
|
missing.push('repo');
|
||||||
|
} else if (!scopes.includes('public_repo')) {
|
||||||
|
missing.push('public_repo');
|
||||||
|
}
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
if (hasOauthScopes !== true) {
|
if (hasOauthScopes !== true) {
|
||||||
return {
|
return {
|
||||||
status: MergeStatus.GITHUB_ERROR,
|
status: MergeStatus.GITHUB_ERROR,
|
||||||
|
|
|
@ -21,6 +21,8 @@ export interface GitClientConfig {
|
||||||
name: string;
|
name: string;
|
||||||
/** If SSH protocol should be used for git interactions. */
|
/** If SSH protocol should be used for git interactions. */
|
||||||
useSsh?: boolean;
|
useSsh?: boolean;
|
||||||
|
/** Whether the specified repository is private. */
|
||||||
|
private?: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
|
@ -21,6 +21,9 @@ type RateLimitResponseWithOAuthScopeHeader = Octokit.Response<Octokit.RateLimitG
|
||||||
headers: {'x-oauth-scopes': string};
|
headers: {'x-oauth-scopes': string};
|
||||||
};
|
};
|
||||||
|
|
||||||
|
/** Describes a function that can be used to test for given Github OAuth scopes. */
|
||||||
|
export type OAuthScopeTestFunction = (scopes: string[], missing: string[]) => void;
|
||||||
|
|
||||||
/** Error for failed Git commands. */
|
/** Error for failed Git commands. */
|
||||||
export class GitCommandError extends Error {
|
export class GitCommandError extends Error {
|
||||||
constructor(client: GitClient, public args: string[]) {
|
constructor(client: GitClient, public args: string[]) {
|
||||||
|
@ -155,14 +158,11 @@ export class GitClient {
|
||||||
* Assert the GitClient instance is using a token with permissions for the all of the
|
* Assert the GitClient instance is using a token with permissions for the all of the
|
||||||
* provided OAuth scopes.
|
* provided OAuth scopes.
|
||||||
*/
|
*/
|
||||||
async hasOauthScopes(...requestedScopes: string[]): Promise<true|{error: string}> {
|
async hasOauthScopes(testFn: OAuthScopeTestFunction): Promise<true|{error: string}> {
|
||||||
const missingScopes: string[] = [];
|
|
||||||
const scopes = await this.getAuthScopesForToken();
|
const scopes = await this.getAuthScopesForToken();
|
||||||
requestedScopes.forEach(scope => {
|
const missingScopes: string[] = [];
|
||||||
if (!scopes.includes(scope)) {
|
// Test Github OAuth scopes and collect missing ones.
|
||||||
missingScopes.push(scope);
|
testFn(scopes, missingScopes);
|
||||||
}
|
|
||||||
});
|
|
||||||
// If no missing scopes are found, return true to indicate all OAuth Scopes are available.
|
// If no missing scopes are found, return true to indicate all OAuth Scopes are available.
|
||||||
if (missingScopes.length === 0) {
|
if (missingScopes.length === 0) {
|
||||||
return true;
|
return true;
|
||||||
|
|
Loading…
Reference in New Issue