feat: add warning messages when credential manager keychain ops fail

package.json

@@ -14,6 +14,7 @@
     "execa": "^9.6.1",
     "inquirer": "^8.2.6",
     "open": "^11.0.0",
+    "tsheredoc": "^1.0.1",
     "yargs-parser": "^20.2.9",
     "yargs-unparser": "^2.0.0"
   },

src/credential-manager-core/index.ts

@@ -1,4 +1,6 @@
+import {ux} from '@oclif/core'
 import debug from 'debug'
+import tsheredoc from 'tsheredoc'
 
 import {LinuxHandler} from './credential-handlers/linux-handler.js'
 import {MacOSHandler} from './credential-handlers/macos-handler.js'
@@ -10,6 +12,7 @@ import {CredentialStore, getStorageConfig} from './lib/credential-storage-select
 import {NetrcAuthEntry} from './lib/types.js'
 
 const credDebug = debug('heroku-credential-manager')
+const heredoc = tsheredoc.default
 
 const SERVICE_NAME = 'heroku-cli'
 
@@ -33,6 +36,13 @@ export async function saveAuth(account: string, token: string, hosts: string[],
     } catch (error) {
       const {message} = error as Error
       credDebug(message)
+      if (process.env.HEROKU_KEYCHAIN_WARNINGS !== 'off') {
+        ux.warn(heredoc(`
+          We can’t save the Heroku token to ${service}.
+          We'll save the token to the .netrc file instead.
+          To turn off this warning, set HEROKU_KEYCHAIN_WARNINGS to "off".`))
+      }
+
       await reportCredentialStoreError(error, {
         credentialStore: config.credentialStore,
         operation: 'saveAuth',
@@ -81,6 +91,13 @@ export async function getAuth(account: string | undefined, host: string, service
     } catch (error) {
       const {message} = error as Error
       credDebug(message)
+      if (process.env.HEROKU_KEYCHAIN_WARNINGS !== 'off') {
+        ux.warn(heredoc(`
+          We can’t retrieve the Heroku token from ${service}.
+          We'll try to retrieve the token from the .netrc file instead.
+          To turn off this warning, set HEROKU_KEYCHAIN_WARNINGS to "off".`))
+      }
+
       await reportCredentialStoreError(error, {
         credentialStore: config.credentialStore,
         operation: 'getAuth',
@@ -132,6 +149,13 @@ export async function removeAuth(account: string | undefined, hosts: string[], s
     } catch (error) {
       const {message} = error as Error
       credDebug(message)
+      if (process.env.HEROKU_KEYCHAIN_WARNINGS !== 'off') {
+        ux.warn(heredoc(`
+          We can’t remove the Heroku token from ${service}.
+          We'll remove the token from the .netrc file instead.
+          To turn off this warning, set HEROKU_KEYCHAIN_WARNINGS to "off".`))
+      }
+
       await reportCredentialStoreError(error, {
         credentialStore: config.credentialStore,
         operation: 'removeAuth',

test/credential-manager/acceptance/index.acceptance.test.ts

@@ -1,5 +1,6 @@
 import {expect, use} from 'chai'
 import chaiAsPromised from 'chai-as-promised'
+import {stderr} from 'stdout-stderr'
 
 import {getAuth, removeAuth, saveAuth} from '../../../src/credential-manager-core/index.js'
 
@@ -8,9 +9,11 @@ import {
   cleanupCredentialStore,
   cleanupDefaultNetrc,
   listCredentialStoreAccounts,
-  snapshotDefaultNetrc,
+  setupFakeCredentialStore,
   skipUnlessAcceptance,
+  snapshotDefaultNetrc,
 } from '../helpers/acceptance-utils.js'
+import { unwrap } from '../../helpers/unwrap.js'
 
 use(chaiAsPromised)
 
@@ -125,12 +128,16 @@ describe('credential-manager acceptance', function () {
     })
 
     it('removes an entry', async function () {
+      // suppressing the keychain warning message since it is not relevant to this test
+      process.env.HEROKU_KEYCHAIN_WARNINGS = 'off'
       await saveAuth(CREDENTIAL.account, CREDENTIAL.token, [], CREDENTIAL.service)
       await removeAuth(CREDENTIAL.account, [], CREDENTIAL.service)
 
       await expect(
 				getAuth(CREDENTIAL.account, 'missing.host.example.com', CREDENTIAL.service),
 			).to.be.rejectedWith(/No auth found|No credentials found/)
+
+      delete process.env.HEROKU_KEYCHAIN_WARNINGS
     })
 
     it('updates entry when account already has credentials', async function () {
@@ -192,6 +199,31 @@ describe('credential-manager acceptance', function () {
       .to.be.rejectedWith(/No auth found|No credentials found/)
     })
 
+    it('saves to netrc when credential store fails', async function () {
+      // Set up fake credential store command for the current platform
+      const fakeSetup = setupFakeCredentialStore()
+
+      if (!fakeSetup) {
+        // Skip if credential store not available on this platform
+        this.skip()
+      }
+
+      try {
+        stderr.start()
+        await saveAuth(CREDENTIAL.account, CREDENTIAL.token, CREDENTIAL.hosts, CREDENTIAL.service)
+        stderr.stop()
+
+        expect(unwrap(stderr.output)).to.contain('We can’t save the Heroku token to heroku-cli-acceptance-test.')
+        expect(unwrap(stderr.output)).to.contain('We\'ll save the token to the .netrc file instead.')
+        expect(unwrap(stderr.output)).to.contain('To turn off this warning, set HEROKU_KEYCHAIN_WARNINGS to "off".')
+
+        const netrcToken = await getAuth('missing-account@example.com', CREDENTIAL.hosts[0], CREDENTIAL.service)
+        expect(netrcToken).to.equal(CREDENTIAL.token)
+      } finally {
+        fakeSetup.cleanup()
+      }
+    })
+
     it('retrieves via netrc when credential store fails', async function () {
       await saveAuth(CREDENTIAL.account, CREDENTIAL.token, CREDENTIAL.hosts, CREDENTIAL.service)
 

test/credential-manager/helpers/acceptance-utils.test.ts

@@ -7,15 +7,19 @@ import {MacOSHandler, Netrc} from '../../../src/credential-manager-core/index.js
 import {
   ALTERNATE_HOST_NAME,
   ALTERNATE_SERVICE_NAME,
-  HOST_NAME,
-  SERVICE_NAME,
   cleanupCredentialStore,
   cleanupDefaultNetrc,
   getAllAcceptanceHosts,
   getAllAcceptanceServices,
+  HOST_NAME,
+  isPowerShellAvailable,
+  isSecretToolAvailable,
+  isSecurityAvailable,
   listCredentialStoreAccounts,
-  snapshotDefaultNetrc,
+  SERVICE_NAME,
+  setupFakeCredentialStore,
   skipUnlessAcceptance,
+  snapshotDefaultNetrc,
 } from './acceptance-utils.js'
 
 describe('acceptance utils', function () {
@@ -233,4 +237,46 @@ describe('acceptance utils', function () {
       expect(rmSyncStub.calledOnceWithExactly(netrcPath, {force: true})).to.equal(true)
     })
   })
+
+  // Integration-style tests that don't require stubbing ES modules
+  describe('isSecretToolAvailable', function () {
+    it('returns a boolean', function () {
+      const result = isSecretToolAvailable()
+      expect(typeof result).to.equal('boolean')
+    })
+  })
+
+  describe('isSecurityAvailable', function () {
+    it('returns a boolean', function () {
+      const result = isSecurityAvailable()
+      expect(typeof result).to.equal('boolean')
+    })
+  })
+
+  describe('isPowerShellAvailable', function () {
+    it('returns a boolean', function () {
+      const result = isPowerShellAvailable()
+      expect(typeof result).to.equal('boolean')
+    })
+  })
+
+  describe('setupFakeCredentialStore', function () {
+    it('returns undefined or a setup object with cleanup', function () {
+      const setup = setupFakeCredentialStore()
+
+      if (setup) {
+        expect(setup).to.have.property('cleanup')
+        expect(setup).to.have.property('originalPath')
+        expect(setup).to.have.property('tmpDir')
+        expect(typeof setup.cleanup).to.equal('function')
+        expect(typeof setup.originalPath).to.equal('string')
+        expect(typeof setup.tmpDir).to.equal('string')
+
+        // Cleanup after test
+        setup.cleanup()
+      } else {
+        expect(setup).to.equal(undefined)
+      }
+    })
+  })
 })