From bb924b63e64cbe1bb12a2371fa0562801090a24f Mon Sep 17 00:00:00 2001
From: Paul Gschwendtner <paulgschwendtner@gmail.com>
Date: Fri, 5 Jun 2020 23:32:42 +0200
Subject: [PATCH] fix(dev-infra): incorrect token sanitization when no token is
 specified (#37489)

We recently moved over the git client from the merge script to the
common dev-infra utils. This made specifying a token optional, but
it looks like the logic for sanitizing messages doesn't account
for that, and we currently add `<TOKEN>` between every message
character. e.g.

```
Executing: git <TOKEN>g<TOKEN>i<TOKEN>t<TOKEN>
<TOKEN>s<TOKEN>t<TOKEN>a<TOKEN>t<TOKEN>u<TOKEN>s<TOKEN>
```

PR Close #37489
---
 dev-infra/utils/git.ts     | 23 +++++++++++++++++++----
 dev-infra/utils/shelljs.ts |  9 ++++++---
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/dev-infra/utils/git.ts b/dev-infra/utils/git.ts
index 1330ac158c..d234c1d570 100644
--- a/dev-infra/utils/git.ts
+++ b/dev-infra/utils/git.ts
@@ -61,11 +61,21 @@ export class GitClient {
   private _projectRoot = getRepoBaseDir();
   /** The OAuth scopes available for the provided Github token. */
   private _oauthScopes: Promise<string[]>|null = null;
-  /** Regular expression that matches the provided Github token. */
-  private _tokenRegex = new RegExp(this._githubToken, 'g');
+  /**
+   * Regular expression that matches the provided Github token. Used for
+   * sanitizing the token from Git child process output.
+   */
+  private _githubTokenRegex: RegExp|null = null;
 
   constructor(
-      private _githubToken = '', private _config: Pick<NgDevConfig, 'github'> = getConfig()) {
+      private _githubToken?: string, private _config: Pick<NgDevConfig, 'github'> = getConfig()) {
+    // If a token has been specified (and is not empty), pass it to the Octokit API and
+    // also create a regular expression that can be used for sanitizing Git command output
+    // so that it does not print the token accidentally.
+    if (_githubToken != null) {
+      this._githubTokenRegex = new RegExp(_githubToken, 'g');
+    }
+
     this.api = new Octokit({auth: _githubToken});
     this.api.hook.error('request', error => {
       // Wrap API errors in a known error class. This allows us to
@@ -137,7 +147,12 @@ export class GitClient {
 
   /** Sanitizes a given message by omitting the provided Github token if present. */
   omitGithubTokenFromMessage(value: string): string {
-    return value.replace(this._tokenRegex, '<TOKEN>');
+    // If no token has been defined (i.e. no token regex), we just return the
+    // value as is. There is no secret value that needs to be omitted.
+    if (this._githubTokenRegex === null) {
+      return value;
+    }
+    return value.replace(this._githubTokenRegex, '<TOKEN>');
   }
 
   /**
diff --git a/dev-infra/utils/shelljs.ts b/dev-infra/utils/shelljs.ts
index 831dea9291..0040b4135b 100644
--- a/dev-infra/utils/shelljs.ts
+++ b/dev-infra/utils/shelljs.ts
@@ -8,7 +8,10 @@
 
 import {exec as _exec, ExecOptions, ShellString} from 'shelljs';
 
-/* Run an exec command as silent. */
-export function exec(cmd: string, opts?: ExecOptions&{async?: false}): ShellString {
-  return _exec(cmd, {silent: true, ...opts});
+/**
+ * Runs an given command as child process. By default, child process
+ * output will not be printed.
+ */
+export function exec(cmd: string, opts?: Omit<ExecOptions, 'async'>): ShellString {
+  return _exec(cmd, {silent: true, ...opts, async: false});
 }