fix(ng-dev): merge tool accidentally performing unauthenticated Github requests

ng-dev/pr/check-target-branches/check-target-branches.ts

@@ -7,16 +7,10 @@
  */
 
 import {assertValidGithubConfig, getConfig, GithubConfig} from '../../utils/config';
-import {error, info, red} from '../../utils/console';
+import {info} from '../../utils/console';
 import {GitClient} from '../../utils/git/git-client';
 import {assertValidMergeConfig, MergeConfig} from '../merge/config';
-import {
-  getBranchesFromTargetLabel,
-  getMatchingTargetLabelForPullRequest,
-  getTargetBranchesForPullRequest,
-  InvalidTargetLabelError,
-  TargetLabel,
-} from '../merge/target-label';
+import {getTargetBranchesForPullRequest} from '../merge/target-label';
 
 async function getTargetBranchesForPr(
   prNumber: number,
@@ -40,7 +34,7 @@ async function getTargetBranchesForPr(
 
   // Note: We do not pass a list of commits here because we did not fetch this information
   // and the commits are only used for validation (which we can skip here).
-  return getTargetBranchesForPullRequest(config, labels, githubTargetBranch, []);
+  return getTargetBranchesForPullRequest(git.github, config, labels, githubTargetBranch, []);
 }
 
 export async function printTargetBranchesForPr(prNumber: number) {

ng-dev/pr/merge/defaults/integration.spec.ts

@@ -18,7 +18,6 @@ import {
   getMatchingTargetLabelForPullRequest,
   TargetLabel,
 } from '../target-label';
-import * as labelDefaults from './labels';
 
 import {fakeGithubPaginationResponse} from '../../../utils/testing/github-interception';
 import {getTargetLabelsForActiveReleaseTrains} from './labels';

ng-dev/pr/merge/defaults/labels.ts

@@ -23,6 +23,7 @@ import {
 } from '../target-label';
 
 import {assertActiveLtsBranch} from './lts-branch';
+import {GithubClient} from '../../../utils/git/github';
 
 /**
  * Gets a list of target labels which should be considered by the merge
@@ -32,14 +33,13 @@ import {assertActiveLtsBranch} from './lts-branch';
  * specifies versioning, branching and releasing for the Angular organization:
  * https://docs.google.com/document/d/197kVillDwx-RZtSVOBtPb4BBIAw0E9RT3q3v6DZkykU
  *
- * @param api Instance of an authenticated Github client.
- *   for the release train branches.
+ * @param api Instance of a Github client. Used to query for the release train branches.
  * @param config Configuration for the Github remote and release packages. Used to fetch
  *   NPM version data when LTS version branches are validated.
  */
 export async function getTargetLabelsForActiveReleaseTrains(
-  api = GitClient.get().github,
-  config = getConfig() as Partial<{github: GithubConfig; release: ReleaseConfig}>,
+  api: GithubClient,
+  config: Partial<{github: GithubConfig; release: ReleaseConfig}>,
 ): Promise<TargetLabel[]> {
   assertValidReleaseConfig(config);
   assertValidGithubConfig(config);

ng-dev/pr/merge/pull-request.ts

@@ -67,6 +67,7 @@ export async function loadAndValidatePullRequest(
   const githubTargetBranch = prData.baseRefName;
 
   const targetBranches = await getTargetBranchesForPullRequest(
+    git.github,
     {github: git.config.github, merge: config},
     labels,
     githubTargetBranch,

ng-dev/pr/merge/target-label.ts

@@ -12,6 +12,7 @@ import {GithubConfig} from '../../utils/config';
 import {Commit} from '../../commit-message/parse';
 import {assertChangesAllowForTargetLabel} from './validations';
 import {PullRequestFailure} from './failures';
+import {GithubClient} from '../../utils/git/github';
 
 /**
  * Enum capturing available target label names in the Angular organization. A target
@@ -95,6 +96,7 @@ export async function getMatchingTargetLabelForPullRequest(
 
 /** Get the branches the pull request should be merged into. */
 export async function getTargetBranchesForPullRequest(
+  api: GithubClient,
   config: {merge: MergeConfig; github: GithubConfig},
   labelsOnPullRequest: string[],
   githubTargetBranch: string,
@@ -109,7 +111,7 @@ export async function getTargetBranchesForPullRequest(
   // can lazily compute branches for a target label and throw. e.g. if an invalid target
   // label is applied, we want to exit the script gracefully with an error message.
   try {
-    const targetLabels = await getTargetLabelsForActiveReleaseTrains();
+    const targetLabels = await getTargetLabelsForActiveReleaseTrains(api, config);
     const matchingLabel = await getMatchingTargetLabelForPullRequest(
       config.merge,
       labelsOnPullRequest,

ng-dev/release/notes/changelog.spec.ts

@@ -15,23 +15,25 @@ describe('Changelog', () => {
       /* useSandboxGitClient */ false,
     );
     spyOn(GitClient, 'get').and.returnValue(gitClient);
-    changelog = Changelog.getChangelogFilePaths();
+    changelog = Changelog.getChangelogFilePaths(gitClient);
   });
 
   it('throws an error if it cannot find the anchor containing the version for an entry', () => {
-    expect(() => Changelog.prependEntryToChangelogFile('does not have version <a> tag')).toThrow();
+    expect(() =>
+      Changelog.prependEntryToChangelogFile(gitClient, 'does not have version <a> tag'),
+    ).toThrow();
   });
 
   it('throws an error if it cannot determine the version for an entry', () => {
     expect(() =>
-      Changelog.prependEntryToChangelogFile(createChangelogEntry('NotSemVer')),
+      Changelog.prependEntryToChangelogFile(gitClient, createChangelogEntry('NotSemVer')),
     ).toThrow();
   });
 
   it('concatenates the changelog entries into the changelog file with the split marker between', () => {
-    Changelog.prependEntryToChangelogFile(createChangelogEntry('1.0.0'));
-    Changelog.prependEntryToChangelogFile(createChangelogEntry('2.0.0'));
-    Changelog.prependEntryToChangelogFile(createChangelogEntry('3.0.0'));
+    Changelog.prependEntryToChangelogFile(gitClient, createChangelogEntry('1.0.0'));
+    Changelog.prependEntryToChangelogFile(gitClient, createChangelogEntry('2.0.0'));
+    Changelog.prependEntryToChangelogFile(gitClient, createChangelogEntry('3.0.0'));
 
     expect(readFileAsString(changelog.filePath)).toBe(
       dedent`
@@ -47,7 +49,7 @@ describe('Changelog', () => {
     `.trim(),
     );
 
-    Changelog.moveEntriesPriorToVersionToArchive(new SemVer('3.0.0'));
+    Changelog.moveEntriesPriorToVersionToArchive(gitClient, new SemVer('3.0.0'));
 
     expect(readFileAsString(changelog.archiveFilePath)).toBe(
       dedent`
@@ -66,19 +68,19 @@ describe('Changelog', () => {
     it('creates a new changelog file if one does not exist.', () => {
       expect(existsSync(changelog.filePath)).toBe(false);
 
-      Changelog.prependEntryToChangelogFile(createChangelogEntry('0.0.0'));
+      Changelog.prependEntryToChangelogFile(gitClient, createChangelogEntry('0.0.0'));
       expect(existsSync(changelog.filePath)).toBe(true);
     });
 
     it('should not include a split marker when only one changelog entry is in the changelog.', () => {
-      Changelog.prependEntryToChangelogFile(createChangelogEntry('0.0.0'));
+      Changelog.prependEntryToChangelogFile(gitClient, createChangelogEntry('0.0.0'));
 
       expect(readFileAsString(changelog.filePath)).not.toContain(splitMarker);
     });
 
     it('separates multiple changelog entries using a standard split marker', () => {
       for (let i = 0; i < 2; i++) {
-        Changelog.prependEntryToChangelogFile(createChangelogEntry(`0.0.${i}`));
+        Changelog.prependEntryToChangelogFile(gitClient, createChangelogEntry(`0.0.${i}`));
       }
 
       expect(readFileAsString(changelog.filePath)).toContain(splitMarker);
@@ -87,31 +89,31 @@ describe('Changelog', () => {
 
   describe('adds entries to the changelog archive', () => {
     it('only updates or creates the changelog archive if necessary', () => {
-      Changelog.prependEntryToChangelogFile(createChangelogEntry('1.0.0'));
+      Changelog.prependEntryToChangelogFile(gitClient, createChangelogEntry('1.0.0'));
       expect(existsSync(changelog.archiveFilePath)).toBe(false);
 
-      Changelog.moveEntriesPriorToVersionToArchive(new SemVer('1.0.0'));
+      Changelog.moveEntriesPriorToVersionToArchive(gitClient, new SemVer('1.0.0'));
       expect(existsSync(changelog.archiveFilePath)).toBe(false);
 
-      Changelog.moveEntriesPriorToVersionToArchive(new SemVer('2.0.0'));
+      Changelog.moveEntriesPriorToVersionToArchive(gitClient, new SemVer('2.0.0'));
       expect(existsSync(changelog.archiveFilePath)).toBe(true);
     });
 
     it('from the primary changelog older than a provided version', () => {
       Changelog.prependEntryToChangelogFile(
-        createChangelogEntry('1.0.0', 'This is version 1'),
         gitClient,
+        createChangelogEntry('1.0.0', 'This is version 1'),
       );
       Changelog.prependEntryToChangelogFile(
-        createChangelogEntry('2.0.0', 'This is version 2'),
         gitClient,
+        createChangelogEntry('2.0.0', 'This is version 2'),
       );
       Changelog.prependEntryToChangelogFile(
-        createChangelogEntry('3.0.0', 'This is version 3'),
         gitClient,
+        createChangelogEntry('3.0.0', 'This is version 3'),
       );
 
-      Changelog.moveEntriesPriorToVersionToArchive(new SemVer('3.0.0'));
+      Changelog.moveEntriesPriorToVersionToArchive(gitClient, new SemVer('3.0.0'));
       expect(readFileAsString(changelog.archiveFilePath)).toContain('version 1');
       expect(readFileAsString(changelog.archiveFilePath)).toContain('version 2');
       expect(readFileAsString(changelog.archiveFilePath)).not.toContain('version 3');

ng-dev/release/notes/changelog.ts

@@ -38,7 +38,7 @@ interface ChangelogEntry {
 
 export class Changelog {
   /** Prepend a changelog entry to the current changelog file. */
-  static prependEntryToChangelogFile(entry: string, git = GitClient.get()) {
+  static prependEntryToChangelogFile(git: GitClient, entry: string) {
     const changelog = new this(git);
     changelog.prependEntryToChangelogFile(entry);
   }
@@ -52,14 +52,14 @@ export class Changelog {
    * other changelog entries.  This allows for example, moving all changelog entries out of the
    * main changelog when a version moves out of support.
    */
-  static moveEntriesPriorToVersionToArchive(version: semver.SemVer, git = GitClient.get()) {
+  static moveEntriesPriorToVersionToArchive(git: GitClient, version: semver.SemVer) {
     const changelog = new this(git);
     changelog.moveEntriesPriorToVersionToArchive(version);
   }
 
   // TODO(josephperrott): Remove this after it is unused.
   /** Retrieve the file paths for the changelog files. */
-  static getChangelogFilePaths(git = GitClient.get()) {
+  static getChangelogFilePaths(git: GitClient) {
     return new this(git);
   }
 

ng-dev/release/notes/cli.ts

@@ -11,6 +11,7 @@ import {Arguments, Argv, CommandModule} from 'yargs';
 import {info} from '../../utils/console';
 
 import {ReleaseNotes} from './release-notes';
+import {GitClient} from '../../utils/git/git-client';
 
 /** Command line options for building a release. */
 export interface Options {
@@ -54,8 +55,10 @@ function builder(argv: Argv): Argv<Options> {
 
 /** Yargs command handler for generating release notes. */
 async function handler({releaseVersion, from, to, prependToChangelog, type}: Arguments<Options>) {
+  /** Git client to use for generating the release notes. */
+  const git = GitClient.get();
   /** The ReleaseNotes instance to generate release notes. */
-  const releaseNotes = await ReleaseNotes.forRange(releaseVersion, from, to);
+  const releaseNotes = await ReleaseNotes.forRange(git, releaseVersion, from, to);
 
   if (prependToChangelog) {
     await releaseNotes.prependEntryToChangelogFile();

ng-dev/release/notes/release-notes.ts

@@ -27,8 +27,7 @@ export const changelogPath = 'CHANGELOG.md';
 
 /** Release note generation. */
 export class ReleaseNotes {
-  static async forRange(version: semver.SemVer, baseRef: string, headRef: string) {
-    const git = GitClient.get();
+  static async forRange(git: GitClient, version: semver.SemVer, baseRef: string, headRef: string) {
     const commits = getCommitsForRangeWithDeduping(git, baseRef, headRef);
     return new ReleaseNotes(version, commits, git);
   }
@@ -67,7 +66,7 @@ export class ReleaseNotes {
    * provided by the GitClient.
    */
   async prependEntryToChangelogFile() {
-    Changelog.prependEntryToChangelogFile(await this.getChangelogEntry(), this.git);
+    Changelog.prependEntryToChangelogFile(this.git, await this.getChangelogEntry());
 
     // TODO(josephperrott): Remove file formatting calls.
     //   Upon reaching a standardized formatting for markdown files, rather than calling a formatter

ng-dev/release/publish/actions.ts

@@ -417,7 +417,12 @@ export abstract class ReleaseAction {
     ]);
 
     // Build release notes for commits from `<releaseNotesCompareTag>..HEAD`.
-    const releaseNotes = await ReleaseNotes.forRange(newVersion, releaseNotesCompareTag, 'HEAD');
+    const releaseNotes = await ReleaseNotes.forRange(
+      this.git,
+      newVersion,
+      releaseNotesCompareTag,
+      'HEAD',
+    );
 
     await this.updateProjectVersion(newVersion);
     await this.prependReleaseNotesToChangelog(releaseNotes);

ng-dev/release/publish/cli.ts

@@ -10,11 +10,11 @@ import {Argv, CommandModule} from 'yargs';
 
 import {assertValidGithubConfig, getConfig} from '../../utils/config';
 import {error, green, info, red, yellow} from '../../utils/console';
-import {GitClient} from '../../utils/git/git-client';
 import {addGithubTokenOption} from '../../utils/git/github-yargs';
 import {assertValidReleaseConfig} from '../config/index';
 
 import {CompletionState, ReleaseTool} from './index';
+import {AuthenticatedGitClient} from '../../utils/git/authenticated-git-client';
 
 /** Command line options for publishing a release. */
 export interface ReleasePublishOptions {
@@ -28,11 +28,11 @@ function builder(argv: Argv): Argv<ReleasePublishOptions> {
 
 /** Yargs command handler for staging a release. */
 async function handler() {
-  const git = GitClient.get();
+  const git = AuthenticatedGitClient.get();
   const config = getConfig();
   assertValidReleaseConfig(config);
   assertValidGithubConfig(config);
-  const task = new ReleaseTool(config.release, config.github, git.baseDir);
+  const task = new ReleaseTool(git, config.release, config.github, git.baseDir);
   const result = await task.run();
 
   switch (result) {

ng-dev/release/publish/index.ts

@@ -27,12 +27,11 @@ export enum CompletionState {
 }
 
 export class ReleaseTool {
-  /** The singleton instance of the authenticated git client. */
-  private _git = AuthenticatedGitClient.get();
   /** The previous git commit to return back to after the release tool runs. */
   private previousGitBranchOrRevision = this._git.getCurrentBranchOrRevision();
 
   constructor(
+    protected _git: AuthenticatedGitClient,
     protected _config: ReleaseConfig,
     protected _github: GithubConfig,
     protected _projectRoot: string,

ng-dev/release/publish/test/common.spec.ts

@@ -272,13 +272,18 @@ class TestAction extends ReleaseAction {
     distTag: NpmDistTag,
     releaseNotesCompareTag = 'HEAD',
   ) {
-    const releaseNotes = await ReleaseNotes.forRange(version, releaseNotesCompareTag, 'HEAD');
+    const releaseNotes = await ReleaseNotes.forRange(
+      this.git,
+      version,
+      releaseNotesCompareTag,
+      'HEAD',
+    );
     debugger;
     await this.buildAndPublish(releaseNotes, publishBranch, distTag);
   }
 
   async testCherryPickWithPullRequest(version: semver.SemVer, branch: string) {
-    const releaseNotes = await ReleaseNotes.forRange(version, '', '');
+    const releaseNotes = await ReleaseNotes.forRange(this.git, version, '', '');
     await this.cherryPickChangelogIntoNextBranch(releaseNotes, branch);
   }
 }