fix(W-18094367): remove support for .netrc files

Author: justinwilaby

.vscode/settings.json

@@ -0,0 +1,6 @@
+{
+    "cSpell.words": [
+        "herokurc",
+        "yubikey"
+    ]
+}

package.json

@@ -12,7 +12,6 @@
     "debug": "^4.4.0",
     "fs-extra": "^9.1.0",
     "heroku-client": "^3.1.0",
-    "netrc-parser": "^3.1.6",
     "open": "^8.4.2",
     "uuid": "^8.3.0",
     "yargs-parser": "^18.1.3",
@@ -22,6 +21,7 @@
     "@heroku-cli/schema": "^1.0.25",
     "@types/ansi-styles": "^3.2.1",
     "@types/chai": "^4.3.16",
+    "@types/debug": "^4.1.12",
     "@types/fs-extra": "^9.0.13",
     "@types/mocha": "^10.0.6",
     "@types/node": "20.14.8",
@@ -55,7 +55,8 @@
     "node": ">= 20"
   },
   "files": [
-    "lib"
+    "lib",
+    "scripts/cleanup-netrc.cjs"
   ],
   "homepage": "https://github.com/heroku/heroku-cli-command",
   "keywords": [
@@ -77,10 +78,12 @@
   },
   "scripts": {
     "build": "rm -rf lib && tsc",
+    "build:dev": "rm -rf lib && tsc --sourceMap",
     "lint": "tsc -p test --noEmit && eslint . --ext .ts",
     "posttest": "yarn run lint",
     "prepublishOnly": "yarn run build",
-    "test": "mocha --forbid-only \"test/**/*.test.ts\""
+    "test": "mocha --forbid-only \"test/**/*.test.ts\"",
+    "postinstall": "node scripts/cleanup-netrc.cjs"
   },
   "types": "./lib/index.d.ts"
 }

scripts/cleanup-netrc.cjs

@@ -0,0 +1,118 @@
+const fs = require('fs')
+const os = require('os')
+const path = require('path')
+/**
+ * List of Heroku machines to remove from the .netrc file
+ * as they are no longer used and pose a security risk.
+ *
+ * Add any additional Heroku machines to this list that you
+ *  want to remove from the .netrc file.
+ */
+const machinesToRemove = ['api.heroku.com', 'git.heroku.com', 'api.staging.herokudev.com']
+/**
+ * Removes the unencrypted Heroku entries from the .netrc file
+ * This is a mitigation for a critical security vulnerability
+ * where unencrypted Heroku API tokens could be leaked
+ * if the .netrc file is compromised. This function removes
+ * any entries related to Heroku from the .netrc file as Heroku
+ * has discontinued it's use.
+ *
+ * BE ADVISED: a defect exists in the original implementation
+ * where orphaned credentials (passwords without machine blocks)
+ * are created when attempting to delete machine entries using the
+ * netrc-parser library.
+ *
+ * This implementation corrects that issue by removing orphaned
+ * credentials as well.
+ *
+ * @returns {void}
+ */
+function removeUnencryptedNetrcMachines() {
+  try {
+    const netrcPath = getNetrcFileLocation()
+
+    if (!fs.existsSync(netrcPath)) {
+      console.log('.netrc file not found, nothing to clean up')
+      return
+    }
+
+    const content = fs.readFileSync(netrcPath, 'utf8')
+    const lines = content.split('\n')
+    const filteredLines = []
+    let skipLines = false
+
+    // Iterate through lines, handling machine blocks and orphaned credentials
+    for (const line of lines) {
+      const trimmedLine = line.trim().toLowerCase()
+
+      // Check if we're starting a Heroku machine block
+      if (trimmedLine.startsWith('machine') &&
+        (machinesToRemove.some(machine => trimmedLine.includes(machine)))) {
+        skipLines = true
+        continue
+      }
+
+      // Check if we're starting a new machine block (non-Heroku)
+      if (trimmedLine.startsWith('machine') && !skipLines) {
+        skipLines = false
+      }
+
+      // Check for orphaned Heroku passwords (HKRU-) and their associated usernames
+      if (/(HRKUSTG-|HKRU-)/.test(line)) {
+        // Remove the previous line if it exists (username)
+        if (filteredLines.length > 0) {
+          filteredLines.pop()
+        }
+
+        continue
+      }
+
+      // Only keep lines if we're not in a Heroku block
+      if (!skipLines) {
+        filteredLines.push(line)
+      }
+    }
+
+    // Remove any trailing empty lines
+    while (filteredLines.length > 0 && !filteredLines[filteredLines.length - 1].trim()) {
+      filteredLines.pop()
+    }
+
+    // Add a newline at the end if we have content
+    const outputContent = filteredLines.length > 0 ?
+      filteredLines.join('\n') + '\n' :
+      ''
+
+    fs.writeFileSync(netrcPath, outputContent)
+  } catch (error) {
+    throw new Error(`Error cleaning up .netrc: ${error.message}`)
+  }
+}
+
+/**
+ * Finds the absolute path to the .netrc file
+ * on disk based on the operating system. This
+ * code was copied directly from `netrc-parser`
+ * and optimized for use here.
+ *
+ * @see [netrc-parser](https://github.com/jdx/node-netrc-parser/blob/master/src/netrc.ts#L177)
+ *
+ * @returns {string} the file path of the .netrc on disk.
+ */
+function getNetrcFileLocation() {
+  let home = ''
+  if (os.platform() === 'win32') {
+    home =
+      process.env.HOME ??
+      (process.env.HOMEDRIVE && process.env.HOMEPATH && path.join(process.env.HOMEDRIVE, process.env.HOMEPATH)) ??
+      process.env.USERPROFILE
+  }
+
+  if (!home) {
+    home = os.homedir() ?? os.tmpdir()
+  }
+
+  return path.join(home, os.platform() === 'win32' ? '_netrc' : '.netrc')
+}
+
+removeUnencryptedNetrcMachines()

src/api-client.ts

@@ -1,7 +1,6 @@
 import {Interfaces} from '@oclif/core'
 import {CLIError, warn} from '@oclif/core/lib/errors'
 import {HTTP, HTTPError, HTTPRequestOptions} from '@heroku/http-call'
-import Netrc from 'netrc-parser'
 import * as url from 'url'
 
 import deps from './deps'
@@ -11,7 +10,11 @@ import {RequestId, requestIdHeader} from './request-id'
 import {vars} from './vars'
 import {ParticleboardClient, IDelinquencyInfo, IDelinquencyConfig} from './particleboard-client'
 
-const debug = require('debug')
+import debug from 'debug'
+import {removeToken, retrieveToken} from './token-storage'
+
+// intentional side effect
+import '../scripts/cleanup-netrc.cjs'
 
 export namespace APIClient {
   export interface Options extends HTTPRequestOptions {
@@ -192,7 +195,7 @@ export class APIClient {
         }
 
         if (!Object.keys(opts.headers).some(h => h.toLowerCase() === 'authorization')) {
-          opts.headers.authorization = `Bearer ${self.auth}`
+          opts.headers.authorization = `Bearer ${await self.auth}`
         }
 
         this.configDelinquency(url, opts)
@@ -204,7 +207,7 @@ export class APIClient {
           const particleboardClient: ParticleboardClient = self.particleboard
 
           if (delinquencyConfig.fetch_delinquency && !delinquencyConfig.warning_shown) {
-            self._particleboard.auth = self.auth
+            self._particleboard.auth = await self.auth
             const settledResponses = await Promise.allSettled([
               super.request<T>(url, opts),
               particleboardClient.get<IDelinquencyInfo>(delinquencyConfig.fetch_url as string),
@@ -240,7 +243,7 @@ export class APIClient {
 
               if (!self.authPromise) self.authPromise = self.login()
               await self.authPromise
-              opts.headers.authorization = `Bearer ${self.auth}`
+              opts.headers.authorization = `Bearer ${await self.auth}`
               return this.request<T>(url, opts, retries)
             }
 
@@ -273,9 +276,13 @@ export class APIClient {
     if (!this._auth) {
       if (process.env.HEROKU_API_TOKEN && !process.env.HEROKU_API_KEY) deps.cli.warn('HEROKU_API_TOKEN is set but you probably meant HEROKU_API_KEY')
       this._auth = process.env.HEROKU_API_KEY
-      if (!this._auth) {
-        deps.netrc.loadSync()
-        this._auth = deps.netrc.machines[vars.apiHost] && deps.netrc.machines[vars.apiHost].password
+    }
+
+    if (!this._auth) {
+      try {
+        this._auth = retrieveToken()
+      } catch {
+        // noop
       }
     }
 
@@ -346,9 +353,7 @@ export class APIClient {
       if (error instanceof CLIError) warn(error)
     }
 
-    delete Netrc.machines['api.heroku.com']
-    delete Netrc.machines['git.heroku.com']
-    await Netrc.save()
+    removeToken()
   }
 
   get defaults(): typeof HTTP.defaults {

src/deps.ts

@@ -1,7 +1,10 @@
-// remote
+// This file isn't necessary and should be removed.
+// I reorganized the code to make it easier to understand
+// but it is entirely unnecessary to have a file like this.
+// I'm leaving it here for now, but it should be removed
+// in the future.
 import oclif = require('@oclif/core')
 import HTTP = require('@heroku/http-call')
-import netrc = require('netrc-parser')
 
 import apiClient = require('./api-client')
 import particleboardClient = require('./particleboard-client')
@@ -14,49 +17,15 @@ import yubikey = require('./yubikey')
 const {ux} = oclif
 
 export const deps = {
-  // remote
-  get cli(): typeof ux {
-    return fetch('@oclif/core').ux
-  },
-  get HTTP(): typeof HTTP {
-    return fetch('@heroku/http-call')
-  },
-  get netrc(): typeof netrc.default {
-    return fetch('netrc-parser').default
-  },
-
-  // local
-  get Mutex(): typeof mutex.Mutex {
-    return fetch('./mutex').Mutex
-  },
-  get yubikey(): typeof yubikey.yubikey {
-    return fetch('./yubikey').yubikey
-  },
-  get APIClient(): typeof apiClient.APIClient {
-    return fetch('./api-client').APIClient
-  },
-  get ParticleboardClient(): typeof particleboardClient.ParticleboardClient {
-    return fetch('./particleboard-client').ParticleboardClient
-  },
-  get file(): typeof file {
-    return fetch('./file')
-  },
-  get flags(): typeof flags {
-    return fetch('./flags')
-  },
-  get Git(): typeof git.Git {
-    return fetch('./git').Git
-  },
-}
-
-const cache: any = {}
-
-function fetch(s: string) {
-  if (!cache[s]) {
-    cache[s] = require(s)
-  }
-
-  return cache[s]
+  cli: ux,
+  HTTP,
+  Mutex: mutex.Mutex,
+  yubikey: yubikey.yubikey,
+  APIClient: apiClient.APIClient,
+  ParticleboardClient: particleboardClient.ParticleboardClient,
+  file,
+  flags,
+  Git: git.Git,
 }
 
 export default deps