Skip to content
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.

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

fix(dev-infra): merge script should not always require full repo permissions #37718

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 13 additions & 5 deletions dev-infra/pr/merge/task.ts
Expand Up @@ -16,9 +16,6 @@ import {isPullRequest, loadAndValidatePullRequest,} from './pull-request';
import {GithubApiMergeStrategy} from './strategies/api-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. */
export const enum MergeStatus {
UNKNOWN_GIT_ERROR,
Expand Down Expand Up @@ -56,8 +53,19 @@ export class PullRequestMergeTask {
* @param force Whether non-critical pull request failures should be ignored.
*/
async merge(prNumber: number, force = false): Promise<MergeResult> {
// Assert the authenticated GitClient has access on the required scopes.
const hasOauthScopes = await this.git.hasOauthScopes(...REQUIRED_SCOPES);
// Check whether the given Github token has sufficient permissions for writing
// 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) {
return {
status: MergeStatus.GITHUB_ERROR,
Expand Down
2 changes: 2 additions & 0 deletions dev-infra/utils/config.ts
Expand Up @@ -21,6 +21,8 @@ export interface GitClientConfig {
name: string;
/** If SSH protocol should be used for git interactions. */
useSsh?: boolean;
/** Whether the specified repository is private. */
private?: boolean;
josephperrott marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down
14 changes: 7 additions & 7 deletions dev-infra/utils/git/index.ts
Expand Up @@ -21,6 +21,9 @@ type RateLimitResponseWithOAuthScopeHeader = Octokit.Response<Octokit.RateLimitG
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. */
export class GitCommandError extends Error {
constructor(client: GitClient, public args: string[]) {
Expand Down Expand Up @@ -148,14 +151,11 @@ export class GitClient {
* Assert the GitClient instance is using a token with permissions for the all of the
* provided OAuth scopes.
*/
async hasOauthScopes(...requestedScopes: string[]): Promise<true|{error: string}> {
const missingScopes: string[] = [];
async hasOauthScopes(testFn: OAuthScopeTestFunction): Promise<true|{error: string}> {
const scopes = await this.getAuthScopesForToken();
requestedScopes.forEach(scope => {
if (!scopes.includes(scope)) {
missingScopes.push(scope);
}
});
const missingScopes: string[] = [];
// Test Github OAuth scopes and collect missing ones.
testFn(scopes, missingScopes);
// If no missing scopes are found, return true to indicate all OAuth Scopes are available.
if (missingScopes.length === 0) {
return true;
Expand Down