From 49ddccef0db2ad24d63112a17d4760ee3ace95e5 Mon Sep 17 00:00:00 2001 From: Katy Bowman Date: Thu, 25 Jun 2026 16:04:26 -0400 Subject: [PATCH 1/7] feat: make password optional in AccountsWrapper.add() --- src/lib/accounts/accounts.ts | 11 +++++++---- test/unit/lib/accounts/accounts.unit.test.ts | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/lib/accounts/accounts.ts b/src/lib/accounts/accounts.ts index d51dced93d..8d716ba4ae 100644 --- a/src/lib/accounts/accounts.ts +++ b/src/lib/accounts/accounts.ts @@ -16,7 +16,7 @@ export interface AccountEntry { } export interface IAccountsWrapper { - add(name: string, username: string, password: string): void + add(name: string, username: string, password?: string): void current(heroku: APIClient): Promise currentNetrc(): Promise getStorageConfig(): ReturnType @@ -29,11 +29,14 @@ export interface IAccountsWrapper { export class AccountsWrapper implements IAccountsWrapper { private netrc: any - add(name: string, username: string, password: string): void { + add(name: string, username: string, password?: string): void { fs.mkdirSync(this.accountsDir(), {recursive: true}) - // eslint-disable-next-line perfectionist/sort-objects - this.writeAccountFile(name, {username, password}) + const content: Record = {username} + if (password !== undefined) { + content.password = password + } + this.writeAccountFile(name, content) } async current(heroku: APIClient): Promise { diff --git a/test/unit/lib/accounts/accounts.unit.test.ts b/test/unit/lib/accounts/accounts.unit.test.ts index f9f2a4388f..ef32c6552e 100644 --- a/test/unit/lib/accounts/accounts.unit.test.ts +++ b/test/unit/lib/accounts/accounts.unit.test.ts @@ -119,6 +119,22 @@ describe('accounts', function () { expect(() => AccountsModule.add('test-user', 'username123', 'password123')).to.throw() }) + + it('should write only username when password is undefined (keychain-mode)', function () { + AccountsModule.add('test-user', 'username123') + + expect(writeFileSyncStub.calledOnce).to.be.true + expect(writeFileSyncStub.firstCall.args[1]).to.equal('username: username123\n') + expect(writeFileSyncStub.firstCall.args[2]).to.equal('utf8') + }) + + it('should write username and password when password is provided (netrc-mode)', function () { + AccountsModule.add('test-user', 'username123', 'password123') + + expect(writeFileSyncStub.calledOnce).to.be.true + expect(writeFileSyncStub.firstCall.args[1]).to.equal('username: username123\npassword: password123\n') + expect(writeFileSyncStub.firstCall.args[2]).to.equal('utf8') + }) }) describe('set()', function () { From f9a1778d060965f911b54ae919eaa6830a18ad52 Mon Sep 17 00:00:00 2001 From: Katy Bowman Date: Thu, 25 Jun 2026 16:08:02 -0400 Subject: [PATCH 2/7] style: fix linting - add blank line before statement --- src/lib/accounts/accounts.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib/accounts/accounts.ts b/src/lib/accounts/accounts.ts index 8d716ba4ae..5e72ed5ab8 100644 --- a/src/lib/accounts/accounts.ts +++ b/src/lib/accounts/accounts.ts @@ -36,6 +36,7 @@ export class AccountsWrapper implements IAccountsWrapper { if (password !== undefined) { content.password = password } + this.writeAccountFile(name, content) } From 2faeb4e5866672295b8a9f2c2d5235e166013377 Mon Sep 17 00:00:00 2001 From: Katy Bowman Date: Thu, 25 Jun 2026 16:23:06 -0400 Subject: [PATCH 3/7] feat: conditionally pass token in accounts:add based on storage mode --- src/commands/accounts/add.ts | 11 +++++- test/unit/commands/accounts/add.unit.test.ts | 41 ++++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/commands/accounts/add.ts b/src/commands/accounts/add.ts index e9aadc0b99..b5bb542d36 100644 --- a/src/commands/accounts/add.ts +++ b/src/commands/accounts/add.ts @@ -30,7 +30,14 @@ export default class Add extends Command { } const token = this.heroku.auth! - - AccountsModule.add(name, email, token) + const config = AccountsModule.getStorageConfig() + + if (config.credentialStore) { + // Keychain-mode: don't save token to cache file + AccountsModule.add(name, email) + } else { + // Netrc-mode: save token to cache file + AccountsModule.add(name, email, token) + } } } diff --git a/test/unit/commands/accounts/add.unit.test.ts b/test/unit/commands/accounts/add.unit.test.ts index ead9eef57f..bc76dc1f0a 100644 --- a/test/unit/commands/accounts/add.unit.test.ts +++ b/test/unit/commands/accounts/add.unit.test.ts @@ -31,10 +31,51 @@ describe('accounts:add', function () { getAuth: async () => ({account: 'testEmail', token: 'testHerokuAPIKey'}), }) + const getStorageConfigStub = stub(AccountsModule, 'getStorageConfig') + getStorageConfigStub.returns({credentialStore: null, useNetrc: true}) + + api.get('/account') + .reply(200, {email: 'testEmail'}) + + await runCommand(Cmd, ['testAccountName']) + expect(addStub.calledOnce).to.equal(true) + expect(addStub.args[0][0]).to.equal('testAccountName') + expect(addStub.args[0][1]).to.equal('testEmail') + expect(addStub.args[0][2]).to.equal('testHerokuAPIKey') + }) + + it('should not pass token to add() when credentialStore is active (keychain-mode)', async function () { + stubCredentialManager({ + getAuth: async () => ({account: 'testEmail', token: 'testHerokuAPIKey'}), + }) + + const getStorageConfigStub = stub(AccountsModule, 'getStorageConfig') + getStorageConfigStub.returns({credentialStore: 'keychain' as any, useNetrc: false}) + api.get('/account') .reply(200, {email: 'testEmail'}) await runCommand(Cmd, ['testAccountName']) + + expect(addStub.calledOnce).to.equal(true) + expect(addStub.args[0][0]).to.equal('testAccountName') + expect(addStub.args[0][1]).to.equal('testEmail') + expect(addStub.args[0][2]).to.be.undefined + }) + + it('should pass token to add() when credentialStore is not active (netrc-mode)', async function () { + stubCredentialManager({ + getAuth: async () => ({account: 'testEmail', token: 'testHerokuAPIKey'}), + }) + + const getStorageConfigStub = stub(AccountsModule, 'getStorageConfig') + getStorageConfigStub.returns({credentialStore: null, useNetrc: true}) + + api.get('/account') + .reply(200, {email: 'testEmail'}) + + await runCommand(Cmd, ['testAccountName']) + expect(addStub.calledOnce).to.equal(true) expect(addStub.args[0][0]).to.equal('testAccountName') expect(addStub.args[0][1]).to.equal('testEmail') From dccd13ec114984e7117789f994235caaa6fe30a0 Mon Sep 17 00:00:00 2001 From: Katy Bowman Date: Thu, 25 Jun 2026 16:57:29 -0400 Subject: [PATCH 4/7] fix: restore original test to verify default behavior --- test/unit/commands/accounts/add.unit.test.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/test/unit/commands/accounts/add.unit.test.ts b/test/unit/commands/accounts/add.unit.test.ts index bc76dc1f0a..6dc7d1503d 100644 --- a/test/unit/commands/accounts/add.unit.test.ts +++ b/test/unit/commands/accounts/add.unit.test.ts @@ -26,14 +26,11 @@ describe('accounts:add', function () { }) describe('when the user is logged in', function () { - it('should call the accounts.add function with the account name, user email, and auth token', async function () { + it('should call the accounts.add function with the account name and user email', async function () { stubCredentialManager({ getAuth: async () => ({account: 'testEmail', token: 'testHerokuAPIKey'}), }) - const getStorageConfigStub = stub(AccountsModule, 'getStorageConfig') - getStorageConfigStub.returns({credentialStore: null, useNetrc: true}) - api.get('/account') .reply(200, {email: 'testEmail'}) @@ -41,7 +38,6 @@ describe('accounts:add', function () { expect(addStub.calledOnce).to.equal(true) expect(addStub.args[0][0]).to.equal('testAccountName') expect(addStub.args[0][1]).to.equal('testEmail') - expect(addStub.args[0][2]).to.equal('testHerokuAPIKey') }) it('should not pass token to add() when credentialStore is active (keychain-mode)', async function () { From ab46b2962aa3bd82fbb743317a5a8f4d5b9d3600 Mon Sep 17 00:00:00 2001 From: Katy Bowman Date: Fri, 26 Jun 2026 08:56:50 -0400 Subject: [PATCH 5/7] feat: skip netrc updates in set() when keychain-mode is active --- src/lib/accounts/accounts.ts | 8 ++++++-- test/unit/lib/accounts/accounts.unit.test.ts | 8 ++++---- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/lib/accounts/accounts.ts b/src/lib/accounts/accounts.ts index 5e72ed5ab8..c2301333a1 100644 --- a/src/lib/accounts/accounts.ts +++ b/src/lib/accounts/accounts.ts @@ -96,12 +96,16 @@ export class AccountsWrapper implements IAccountsWrapper { if (account.name) { if (config.credentialStore) { + // Keychain mode: only update login state, skip netrc const email = this.getAliasEmail(account.name) - if (email) { - await this.writeLoginState(dataDir, email) + if (!email) { + throw new Error(`We can't find the alias file for ${account.name}.`) } + await this.writeLoginState(dataDir, email) + return } + // Netrc mode: update both login state and netrc files const netrcInstance = await this.initNetrc() let current try { diff --git a/test/unit/lib/accounts/accounts.unit.test.ts b/test/unit/lib/accounts/accounts.unit.test.ts index ef32c6552e..dd6e33a6c5 100644 --- a/test/unit/lib/accounts/accounts.unit.test.ts +++ b/test/unit/lib/accounts/accounts.unit.test.ts @@ -161,7 +161,7 @@ describe('accounts', function () { setNetrc(null as unknown as typeof fakeNetrc) }) - it('calls writeLoginState with email from alias file and writes to netrc', async function () { + it('calls writeLoginState with email from alias file but does not write to netrc', async function () { const account = {name: 'production', username: 'prod@example.com'} existsSyncStub.withArgs('/user/home/.config/heroku').returns(false) existsSyncStub.withArgs(match(/production$/)).returns(true) @@ -173,9 +173,9 @@ describe('accounts', function () { expect(writeLoginStateStub.calledOnce).to.be.true expect(writeLoginStateStub.firstCall.args[0]).to.equal('/data/heroku') expect(writeLoginStateStub.firstCall.args[1]).to.equal('prod@example.com') - expect(fakeNetrc.machines['api.heroku.com']).to.deep.equal({login: 'prod@example.com', password: 'secret'}) - expect(fakeNetrc.machines['git.heroku.com']).to.deep.equal({login: 'prod@example.com', password: 'secret'}) - expect(fakeNetrc.save.calledOnce).to.be.true + expect(fakeNetrc.machines['api.heroku.com']).to.be.undefined + expect(fakeNetrc.machines['git.heroku.com']).to.be.undefined + expect(fakeNetrc.save.called).to.be.false }) it('throws error when alias file does not exist', async function () { From 2a548cb5b8777d8a2155d055c225855ad16617fd Mon Sep 17 00:00:00 2001 From: Katy Bowman Date: Fri, 26 Jun 2026 09:00:30 -0400 Subject: [PATCH 6/7] style: fix linting - add blank line before statement --- src/lib/accounts/accounts.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib/accounts/accounts.ts b/src/lib/accounts/accounts.ts index c2301333a1..f122cd4d21 100644 --- a/src/lib/accounts/accounts.ts +++ b/src/lib/accounts/accounts.ts @@ -101,6 +101,7 @@ export class AccountsWrapper implements IAccountsWrapper { if (!email) { throw new Error(`We can't find the alias file for ${account.name}.`) } + await this.writeLoginState(dataDir, email) return } From 61a8832fe45aa0350f10f1a6f226c7fd982e9f02 Mon Sep 17 00:00:00 2001 From: Katy Bowman Date: Fri, 26 Jun 2026 12:44:57 -0400 Subject: [PATCH 7/7] refactor: refactor accounts.add() to use ternary --- src/lib/accounts/accounts.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/lib/accounts/accounts.ts b/src/lib/accounts/accounts.ts index f122cd4d21..a3d487f4ae 100644 --- a/src/lib/accounts/accounts.ts +++ b/src/lib/accounts/accounts.ts @@ -32,10 +32,9 @@ export class AccountsWrapper implements IAccountsWrapper { add(name: string, username: string, password?: string): void { fs.mkdirSync(this.accountsDir(), {recursive: true}) - const content: Record = {username} - if (password !== undefined) { - content.password = password - } + const content: Record = password === undefined + ? {username} + : {username, password} // eslint-disable-line perfectionist/sort-objects this.writeAccountFile(name, content) }