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

justinwilaby · 2025-03-22T21:30:45Z

Remove .netrc support in favor of modern encryption and OS-level key stores

This PR modernizes credential storage by removing .netrc file support and implementing secure credential storage using OS-level key stores and encryption.

OS-Specific Keystore Implementation

The CLI now uses the following secure credential storage mechanisms:

macOS: Keychain Access
Windows: Windows Credential Manager
Linux: libsecret (GNOME Keyring/KWallet)
- Requires libsecret library to be installed
- For Debian/Ubuntu: sudo apt-get install libsecret-1-dev
- For Red Hat/Fedora: sudo yum install libsecret-devel
- For Arch Linux: sudo pacman -S libsecret

Breaking Changes

No longer writes or reads token data from .netrc files
- Any ancillary uses of the .netrc file will no longer contain Heroku credentials
- Applications depending on Heroku credentials contained in the .netrc will need to be updated
- The Netrc machines for 'api.heroku.com' and 'git.heroku.com' are explicitly removed - any existing .netrc files containing Heroku credentials will be permanently removed.
Token storage implementation changes
- New dependencies on file encryption and OS keystore. These are OS-level dependencies and are described below.
- Token retrieval now uses retrieveToken() and getConfigContents()
- Applications directly accessing token storage need to be updated to use new methods
- *Applications using code patterns like Authorization: bearer ${this.heroku.auth} can safely continue to do so.

System Requirements

Linux Users:
- Must have libsecret installed and a running keyring daemon (GNOME Keyring or KWallet)
- Without libsecret, the Heroku CLI will fail to store credentials securely and alternative auth methods such as the HEROKU_API_KEY env var must be used
- Desktop environments like GNOME and KDE include the above listed utils by default
macOS Users:
- No additional requirements
- Uses the built-in Keychain Access
Windows Users:
- No additional requirements
- Uses the built-in Windows Credential Manager

Testing Instructions

Build the package:
```
npm run build
npm pack
```
install this by file path
fix above mentioned breaking changes in code and unit tests if applicable
test your CLI

mars · 2025-03-25T17:51:37Z

scripts/cleanup-netrc.cjs

+  return path.join(home, os.platform() === 'win32' ? '_netrc' : '.netrc')
+}
+
+removeUnencryptedNetrcMachines()


I think it's fine to include the code for deleting the old cleartext Netrc entries, but I do not agree that it should be automatically run on the initial release of the new storage mechanism. Let's put the activation/calls to removeUnencryptedNetrcMachines() in a subsequent PR that can be released in a patch version after the major release.

If initial release of the new storage mechanism breaks something for a customer, they need to be able to CLI update back to the previous CLI version to restore functionality.

mars · 2025-03-25T17:52:58Z

src/api-client.ts

+import {removeToken, retrieveToken} from './token-storage'
+
+// intentional side effect
+import '../scripts/cleanup-netrc.cjs'


Same comment as prior, that activation/calls to the clean-up code should be in a separate subsequent CLI release, after the major release implementing the new storage mechanism.

mars · 2025-03-25T18:00:22Z

src/token-storage.ts

@@ -0,0 +1,313 @@
+/**
+ * Functions for securely storing bearer tokens using OS-level utilities.


I see zero test coverage for these new functions.

mars · 2025-03-25T18:01:45Z

src/file-encryption.ts

+}
+
+/**
+ * Reads the contents of the config file and decrypts it using the provided bearer token


Not seeing test coverage of these functions either.

mble-sfdc · 2025-04-11T15:48:18Z

src/file-encryption.ts

+  return crypto.pbkdf2Sync(
+    bearerToken,
+    salt,
+    100000,


sec: number of iterations is too low - as per SFSS-074, this needs to be greater than or equal to 210,000 for HMAC-SHA512.

Great feedback! I'll make that change.

I think NIST really goes for 600K but that's pretty overkill for SHA512.

tt

I think using the credential storage makes sense but there is a minimal change in security posture: A local user who can read the .netrc file will also be able to execute heroku auth:token.

tt · 2025-04-14T14:28:50Z

src/login.ts

-        }
-      })
+    try {
+      await writeConfigContents(config, entry.password)


This seems like something we should consider simplifying.

If I'm understanding it correctly, we're storing the username and token in a file and then encrypting the file with the token. The token is then stored in the operating system's credential storage.

Couldn't we store the username (which I think is mostly unnecessary) next to the token and avoiding having to maintain the encryption logic?

I think this is a great idea!

I originally had to provide affordances to automated processes that might need to read credentials directly from a file. It's not clear that the intent was to make this transient by looking at the source code alone. When this is rolled out, I'm hoping to include guidance for customers in this area while supporting a minimally disruptive solution until a full migration can take place.

How will such processes read credentials from the file if it's encrypted?

tt · 2025-04-14T14:34:00Z

src/file-encryption.ts

+
+export type ClientConfig = {
+  'api.heroku.com': ConfigEntry,
+  'git.heroku.com': ConfigEntry,


Applications directly accessing token storage need to be updated to use new methods

As far as I know, we don't implement a custom credential helper so Git relies on cURL reading this entry from the .netrc file.

justinwilaby · 2025-04-14T16:42:59Z

I think using the credential storage makes sense but there is a minimal change in security posture: A local user who can read the .netrc file will also be able to execute heroku auth:token.

@tt - Exactly. This was an area of consideration for me as well. I understand the best practice to be:

Generate a token and display it once and only once
Regular Auth flows never display the token
If a token is not securely saved by the user, it is lost and the user must generate a new one.

Thoughts?

tt · 2025-04-23T09:38:43Z

Generate a token and display it once and only once

Regular Auth flows never display the token

If a token is not securely saved by the user, it is lost and the user must generate a new one.

Thoughts?

I think we should definitely implement these but it still doesn't change the fact that any user who can invoke the CLI can exfiltrate a token.

We could make heroku auth:token require explicit consent similar to when you sign in but that wouldn't prevent you from getting the token that the CLI itself is using. As long as you can run a plug-in, this.heroku.auth could be read and exposed. Even if we block that by, say, only exposing a way to issue API requests, a plug-in could communicate with the credential store directly because it's running inside the CLI process.

I'm not saying this because we shouldn't more in the direction of adopting better platform primitives. I only want us to recognize that we're making it more complicated to receive the plaintext token but not impossible.

mars · 2025-04-23T17:59:33Z

Do the "secure credential storage mechanisms" implemented by this change (Keychain Access, Windows Credential Manager, libsecret) prompt the user for a local password or biometric authentication, before saving or reading a credential?

justinwilaby · 2025-05-05T14:26:14Z

Do the "secure credential storage mechanisms" implemented by this change (Keychain Access, Windows Credential Manager, libsecret) prompt the user for a local password or biometric authentication, before saving or reading a credential?

Not in any of the tests I performed. AFAIK, these tools are intended to be used in this way and shouldn't need explicit input from the user.

justinwilaby requested a review from a team as a code owner March 22, 2025 21:30

justinwilaby force-pushed the jw/fix/W-18094367 branch 9 times, most recently from 91211ec to 90604a4 Compare March 24, 2025 18:03

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

61729c5

justinwilaby force-pushed the jw/fix/W-18094367 branch from 90604a4 to 61729c5 Compare March 24, 2025 18:19

mars requested changes Mar 25, 2025

View reviewed changes

mble-sfdc reviewed Apr 11, 2025

View reviewed changes

tt reviewed Apr 14, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

justinwilaby commented Mar 22, 2025 •

edited

Loading

mars Mar 25, 2025

mars Mar 25, 2025

mars Mar 25, 2025

mars Mar 25, 2025

mble-sfdc Apr 11, 2025

justinwilaby Apr 11, 2025

mble-sfdc Apr 11, 2025

tt left a comment

tt Apr 14, 2025

justinwilaby Apr 14, 2025

tt Apr 23, 2025

tt Apr 14, 2025

justinwilaby commented Apr 14, 2025 •

edited

Loading

tt commented Apr 23, 2025

mars commented Apr 23, 2025

justinwilaby commented May 5, 2025

		@@ -0,0 +1,313 @@
		/**
		* Functions for securely storing bearer tokens using OS-level utilities.

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

Are you sure you want to change the base?

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

Conversation

justinwilaby commented Mar 22, 2025 • edited Loading

Remove .netrc support in favor of modern encryption and OS-level key stores

OS-Specific Keystore Implementation

Breaking Changes

System Requirements

Testing Instructions

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justinwilaby commented Apr 14, 2025 • edited Loading

tt commented Apr 23, 2025

mars commented Apr 23, 2025

justinwilaby commented May 5, 2025

justinwilaby commented Mar 22, 2025 •

edited

Loading

justinwilaby commented Apr 14, 2025 •

edited

Loading