fix(ng-dev): scrub GitHub tokens from build worker child process environment

ng-dev/pr/common/checkout-pr.ts

@@ -132,19 +132,16 @@ export async function checkOutPullRequestLocally(
     resetGitState: (): boolean => {
       return git.checkout(previousBranchOrRevision, true);
     },
-    pushToUpstreamCommand: `git push ${upstreamUrlToPush(pr.headRef.repository.url)} HEAD:${escapedHeadRefName} ${escapedForceWithLeaseFlag}`,
+    pushToUpstreamCommand: `git push ${upstreamUrlToPush(pr.headRef.repository.url, git)} HEAD:${escapedHeadRefName} ${escapedForceWithLeaseFlag}`,
     resetGitStateCommand: `git rebase --abort && git reset --hard && git checkout ${previousBranchOrRevision}`,
     pullRequest: pr,
   };
 }
 
-function upstreamUrlToPush(repoUrl: string): string {
+export function upstreamUrlToPush(repoUrl: string, git: AuthenticatedGitClient): string {
   const pushToUpstreamUrl = new URL(repoUrl);
-  if (process.env['GITHUB_TOKEN']) {
-    pushToUpstreamUrl.password = '$GITHUB_TOKEN';
-    pushToUpstreamUrl.username = 'x-access-token';
-  } else if (process.env['TOKEN']) {
-    pushToUpstreamUrl.password = '$TOKEN';
+  if (git.tokenEnvironmentVariable) {
+    pushToUpstreamUrl.password = `$${git.tokenEnvironmentVariable}`;
     pushToUpstreamUrl.username = 'x-access-token';
   }
 

ng-dev/pr/common/test/common.spec.ts

@@ -27,6 +27,7 @@ import {assertValidPullRequest} from '../validation/validate-pull-request.js';
 import {PullRequestConfig, PullRequestValidationConfig} from '../../config/index.js';
 import {PullRequestTarget} from '../targeting/target-label.js';
 import {AuthenticatedGitClient} from '../../../utils/git/authenticated-git-client.js';
+import {upstreamUrlToPush} from '../checkout-pr.js';
 import {
   cleanTestTmpDir,
   installVirtualGitClientSpies,
@@ -484,6 +485,44 @@ describe('pull request validation', () => {
       expect(results.length).toBe(0);
     });
   });
+
+  describe('upstreamUrlToPush', () => {
+    const repoUrl = 'https://github.com/angular/angular.git';
+
+    it('should use $GITHUB_TOKEN when git.tokenEnvironmentVariable is GITHUB_TOKEN', () => {
+      const mockGit = {tokenEnvironmentVariable: 'GITHUB_TOKEN'} as AuthenticatedGitClient;
+      expect(upstreamUrlToPush(repoUrl, mockGit)).toBe(
+        'https://x-access-token:$GITHUB_TOKEN@github.com/angular/angular.git',
+      );
+    });
+
+    it('should use $TOKEN when git.tokenEnvironmentVariable is TOKEN', () => {
+      const mockGit = {tokenEnvironmentVariable: 'TOKEN'} as AuthenticatedGitClient;
+      expect(upstreamUrlToPush(repoUrl, mockGit)).toBe(
+        'https://x-access-token:$TOKEN@github.com/angular/angular.git',
+      );
+    });
+
+    it('should return unauthenticated URL when git.tokenEnvironmentVariable is null', () => {
+      const mockGit = {tokenEnvironmentVariable: null} as AuthenticatedGitClient;
+      expect(upstreamUrlToPush(repoUrl, mockGit)).toBe('https://github.com/angular/angular.git');
+    });
+
+    it('should ignore process.env when determining push url credentials', () => {
+      const originalGithubToken = process.env['GITHUB_TOKEN'];
+      try {
+        process.env['GITHUB_TOKEN'] = 'super-secret';
+        const mockGit = {tokenEnvironmentVariable: null} as AuthenticatedGitClient;
+        expect(upstreamUrlToPush(repoUrl, mockGit)).toBe('https://github.com/angular/angular.git');
+      } finally {
+        if (originalGithubToken !== undefined) {
+          process.env['GITHUB_TOKEN'] = originalGithubToken;
+        } else {
+          delete process.env['GITHUB_TOKEN'];
+        }
+      }
+    });
+  });
 });
 
 function createIsolatedValidationConfig(

ng-dev/release/build/build-worker.ts

@@ -28,6 +28,12 @@ async function main() {
     throw Error('This script needs to be invoked as a NodeJS worker.');
   }
 
+  // Scrub tokens as defense-in-depth before loading config or executing buildPackages.
+  delete process.env['GITHUB_TOKEN'];
+  delete process.env['TOKEN'];
+  delete process.env['GH_TOKEN'];
+  delete process.env['SNAPSHOT_BUILDS_GITHUB_TOKEN'];
+
   const config = await getConfig();
   assertValidReleaseConfig(config);
 

ng-dev/release/build/build.spec.ts

@@ -6,6 +6,8 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import childProcess, {ChildProcess as NodeChildProcess} from 'child_process';
+import {EventEmitter} from 'events';
 import {BuildWorker} from './index.js';
 import {setConfig} from '../../utils/config.js';
 import {BuiltPackage, NpmPackage, ReleaseConfig} from '../config/index.js';
@@ -82,3 +84,67 @@ describe('ng-dev release build', () => {
     expect(process.exit).toHaveBeenCalledWith(1);
   });
 });
+
+describe('BuildWorker.invokeBuild', () => {
+  let originalEnv: NodeJS.ProcessEnv;
+
+  beforeEach(() => {
+    originalEnv = {...process.env};
+  });
+
+  afterEach(() => {
+    process.env = originalEnv;
+  });
+
+  it('should scrub GITHUB_TOKEN, TOKEN, GH_TOKEN, and SNAPSHOT_BUILDS_GITHUB_TOKEN from child environment', async () => {
+    process.env['GITHUB_TOKEN'] = 'secret-github-token';
+    process.env['TOKEN'] = 'secret-token';
+    process.env['GH_TOKEN'] = 'secret-gh-token';
+    process.env['SNAPSHOT_BUILDS_GITHUB_TOKEN'] = 'secret-snapshot-token';
+    process.env['CUSTOM_VAR'] = 'preserved-var';
+
+    let capturedOptions: childProcess.ForkOptions | undefined;
+    const mockChildProcess = new EventEmitter() as NodeChildProcess;
+
+    spyOn(childProcess, 'fork').and.callFake((_modulePath: any, options?: any) => {
+      capturedOptions = options;
+      process.nextTick(() => {
+        mockChildProcess.emit('exit', 0);
+      });
+      return mockChildProcess;
+    });
+
+    const result = await BuildWorker.invokeBuild();
+
+    expect(result).toBeNull();
+    expect(capturedOptions).toBeDefined();
+    expect(capturedOptions?.env).toBeDefined();
+
+    const env = capturedOptions!.env!;
+    expect(env['GITHUB_TOKEN']).toBeUndefined();
+    expect(env['TOKEN']).toBeUndefined();
+    expect(env['GH_TOKEN']).toBeUndefined();
+    expect(env['SNAPSHOT_BUILDS_GITHUB_TOKEN']).toBeUndefined();
+
+    expect(env['CUSTOM_VAR']).toBe('preserved-var');
+    if (process.env['PATH']) {
+      expect(env['PATH']).toBe(process.env['PATH']);
+    }
+  });
+
+  it('should resolve with built packages received from worker process', async () => {
+    const mockPackages: BuiltPackage[] = [{name: '@angular/pkg1', outputPath: 'dist/pkg1'}];
+    const mockChildProcess = new EventEmitter() as NodeChildProcess;
+
+    spyOn(childProcess, 'fork').and.callFake(() => {
+      process.nextTick(() => {
+        mockChildProcess.emit('message', mockPackages);
+        mockChildProcess.emit('exit', 0);
+      });
+      return mockChildProcess;
+    });
+
+    const result = await BuildWorker.invokeBuild();
+    expect(result).toEqual(mockPackages);
+  });
+});

ng-dev/release/build/index.ts

@@ -8,7 +8,7 @@
 
 import {dirname, join} from 'path';
 import {fileURLToPath} from 'url';
-import {fork} from 'child_process';
+import childProcess from 'child_process';
 import {BuiltPackage} from '../config/index.js';
 
 export abstract class BuildWorker {
@@ -21,11 +21,18 @@ export abstract class BuildWorker {
    */
   static async invokeBuild(): Promise<BuiltPackage[] | null> {
     return new Promise((resolve) => {
-      const buildProcess = fork(getBuildWorkerScriptPath(), {
+      const env: NodeJS.ProcessEnv = {...process.env};
+      delete env['GITHUB_TOKEN'];
+      delete env['TOKEN'];
+      delete env['GH_TOKEN'];
+      delete env['SNAPSHOT_BUILDS_GITHUB_TOKEN'];
+
+      const buildProcess = childProcess.fork(getBuildWorkerScriptPath(), {
         // The stdio option is set to redirect any "stdout" output directly to the "stderr" file
         // descriptor. An additional "ipc" file descriptor is created to support communication with
         // the build process. https://nodejs.org/api/child_process.html#child_process_options_stdio.
         stdio: ['inherit', 2, 2, 'ipc'],
+        env,
       });
       let builtPackages: BuiltPackage[] | null = null;
 

ng-dev/utils/git/authenticated-git-client.ts

@@ -21,6 +21,9 @@ import {
 /** Describes a function that can be used to test for given Github OAuth scopes. */
 export type OAuthScopeTestFunction = (scopes: string[], missing: string[]) => void;
 
+/** The possible environment variables containing a GitHub token. */
+export type TokenEnvironmentVariable = 'GITHUB_TOKEN' | 'TOKEN';
+
 /** The possible types of users which could be used for authentication. */
 type UserType = 'bot' | 'user';
 
@@ -49,6 +52,7 @@ export class AuthenticatedGitClient extends GitClient {
     readonly userType: UserType,
     config: {github: GithubConfig},
     baseDir?: string,
+    readonly tokenEnvironmentVariable: TokenEnvironmentVariable | null = null,
   ) {
     super(config, baseDir);
   }
@@ -154,6 +158,8 @@ export class AuthenticatedGitClient extends GitClient {
 
   /** The previously configured access token. */
   private static _token: string | null = null;
+  /** The previously configured token environment variable name. */
+  private static _tokenEnvironmentVariable: TokenEnvironmentVariable | null = null;
   /** The singleton instance of the `AuthenticatedGitClient`. */
   private static _authenticatedInstance: Promise<AuthenticatedGitClient> | null = null;
   /** The previously configured user type. */
@@ -175,20 +181,31 @@ export class AuthenticatedGitClient extends GitClient {
       AuthenticatedGitClient._authenticatedInstance = (async (
         token: string,
         userType: UserType,
+        tokenEnvironmentVariable: TokenEnvironmentVariable | null,
       ) => {
         return new AuthenticatedGitClient(
           token,
           userType,
           await getConfig([assertValidGithubConfig]),
+          undefined,
+          tokenEnvironmentVariable,
         );
-      })(AuthenticatedGitClient._token, AuthenticatedGitClient._userType);
+      })(
+        AuthenticatedGitClient._token,
+        AuthenticatedGitClient._userType,
+        AuthenticatedGitClient._tokenEnvironmentVariable,
+      );
     }
 
     return AuthenticatedGitClient._authenticatedInstance;
   }
 
   /** Configures an authenticated git client. */
-  static configure(token: string, userType: UserType = 'user'): void {
+  static configure(
+    token: string,
+    userType: UserType = 'user',
+    tokenEnvironmentVariable: TokenEnvironmentVariable | null = null,
+  ): void {
     if (AuthenticatedGitClient._token) {
       throw Error(
         'Unable to configure `AuthenticatedGitClient` as it has been configured already.',
@@ -197,5 +214,6 @@ export class AuthenticatedGitClient extends GitClient {
 
     AuthenticatedGitClient._token = token;
     AuthenticatedGitClient._userType = userType;
+    AuthenticatedGitClient._tokenEnvironmentVariable = tokenEnvironmentVariable;
   }
 }

ng-dev/utils/git/github-yargs.ts

@@ -9,7 +9,7 @@
 import {Log} from '../logging.js';
 
 import {Argv} from 'yargs';
-import {AuthenticatedGitClient} from './authenticated-git-client.js';
+import {AuthenticatedGitClient, TokenEnvironmentVariable} from './authenticated-git-client.js';
 import {GITHUB_TOKEN_GENERATE_URL} from './github-urls.js';
 
 /** Sets up the `github-token` command option for the given Yargs instance. */
@@ -42,13 +42,25 @@ export function addGithubTokenOption<T>(argv: Argv<T>) {
  * that the token should only be accessed from the AuthenticatedGitClient itself.
  */
 export function configureGitClientWithTokenOrFromEnvironment(token: string | undefined): void {
-  const githubToken = token || (process.env['GITHUB_TOKEN'] ?? process.env['TOKEN']);
+  let tokenEnvVar: TokenEnvironmentVariable | null = null;
+  if (process.env['GITHUB_TOKEN']) {
+    tokenEnvVar = 'GITHUB_TOKEN';
+  } else if (process.env['TOKEN']) {
+    tokenEnvVar = 'TOKEN';
+  }
+
+  const githubToken = token || (tokenEnvVar ? process.env[tokenEnvVar] : undefined);
   if (!githubToken) {
     Log.error('No Github token set. Please set the `GITHUB_TOKEN` environment variable.');
     Log.error('Alternatively, pass the `--github-token` command line flag.');
     Log.warn(`You can generate a token here: ${GITHUB_TOKEN_GENERATE_URL}`);
     throw Error('Unable to determine the Github token.');
   }
 
-  AuthenticatedGitClient.configure(githubToken);
+  const sourceEnvVar = token ? null : tokenEnvVar;
+
+  AuthenticatedGitClient.configure(githubToken, 'user', sourceEnvVar);
+
+  delete process.env['GITHUB_TOKEN'];
+  delete process.env['TOKEN'];
 }

ng-dev/utils/test/github-yargs.spec.ts

@@ -0,0 +1,85 @@
+/**
+ * @license
+ * Copyright Google LLC
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import {AuthenticatedGitClient} from '../git/authenticated-git-client.js';
+import {configureGitClientWithTokenOrFromEnvironment} from '../git/github-yargs.js';
+
+describe('github-yargs', () => {
+  let originalEnv: NodeJS.ProcessEnv;
+
+  beforeEach(() => {
+    originalEnv = {...process.env};
+    spyOn(AuthenticatedGitClient, 'configure');
+  });
+
+  afterEach(() => {
+    process.env = originalEnv;
+  });
+
+  it('should configure git client with GITHUB_TOKEN from environment and delete it', () => {
+    process.env['GITHUB_TOKEN'] = 'test-github-token';
+    delete process.env['TOKEN'];
+
+    configureGitClientWithTokenOrFromEnvironment('');
+
+    expect(AuthenticatedGitClient.configure).toHaveBeenCalledWith(
+      'test-github-token',
+      'user',
+      'GITHUB_TOKEN',
+    );
+    expect(process.env['GITHUB_TOKEN']).toBeUndefined();
+    expect(process.env['TOKEN']).toBeUndefined();
+  });
+
+  it('should configure git client with TOKEN from environment and delete it', () => {
+    delete process.env['GITHUB_TOKEN'];
+    process.env['TOKEN'] = 'test-token';
+
+    configureGitClientWithTokenOrFromEnvironment('');
+
+    expect(AuthenticatedGitClient.configure).toHaveBeenCalledWith('test-token', 'user', 'TOKEN');
+    expect(process.env['GITHUB_TOKEN']).toBeUndefined();
+    expect(process.env['TOKEN']).toBeUndefined();
+  });
+
+  it('should prefer GITHUB_TOKEN over TOKEN in environment', () => {
+    process.env['GITHUB_TOKEN'] = 'test-github-token';
+    process.env['TOKEN'] = 'test-token';
+
+    configureGitClientWithTokenOrFromEnvironment('');
+
+    expect(AuthenticatedGitClient.configure).toHaveBeenCalledWith(
+      'test-github-token',
+      'user',
+      'GITHUB_TOKEN',
+    );
+    expect(process.env['GITHUB_TOKEN']).toBeUndefined();
+    expect(process.env['TOKEN']).toBeUndefined();
+  });
+
+  it('should configure git client with CLI token, pass null for tokenEnvironmentVariable, and scrub ambient tokens', () => {
+    process.env['GITHUB_TOKEN'] = 'ambient-github-token';
+    process.env['TOKEN'] = 'ambient-token';
+
+    configureGitClientWithTokenOrFromEnvironment('cli-token');
+
+    expect(AuthenticatedGitClient.configure).toHaveBeenCalledWith('cli-token', 'user', null);
+    expect(process.env['GITHUB_TOKEN']).toBeUndefined();
+    expect(process.env['TOKEN']).toBeUndefined();
+  });
+
+  it('should throw an error if no token is available', () => {
+    delete process.env['GITHUB_TOKEN'];
+    delete process.env['TOKEN'];
+
+    expect(() => configureGitClientWithTokenOrFromEnvironment('')).toThrowError(
+      'Unable to determine the Github token.',
+    );
+    expect(AuthenticatedGitClient.configure).not.toHaveBeenCalled();
+  });
+});