diff --git a/package.json b/package.json index a96fead4fc..e66143c6e3 100644 --- a/package.json +++ b/package.json @@ -183,7 +183,8 @@ "init": [ "./dist/hooks/init/version", "./dist/hooks/init/terms-of-service", - "./dist/hooks/init/setup-otel-telemetry" + "./dist/hooks/init/setup-otel-telemetry", + "./dist/hooks/init/configure-git-credential-manager" ], "postrun": [ "./dist/hooks/postrun/send-otel-telemetry" diff --git a/src/hooks/init/configure-git-credential-manager.ts b/src/hooks/init/configure-git-credential-manager.ts new file mode 100644 index 0000000000..7ffbb44995 --- /dev/null +++ b/src/hooks/init/configure-git-credential-manager.ts @@ -0,0 +1,41 @@ +import {Hook} from '@oclif/core/hooks' + +import Git from '../../lib/git/git.js' + +const hook: Hook.Init = async function () { + // Skip for --version / --help / completion-style invocations to avoid a needless git subprocess. + // Version-style invocations include `--version` plus any `oclif.additionalVersionFlags` + // (e.g. `-v`, `version`) declared in package.json — mirror version.ts, which reads the + // same config. We hard-include `-v`/`version` as a defensive fallback so this skip can + // never silently regress if config is somehow unavailable at runtime. + // Help-style invocations are skipped for the same reason: they're fast informational + // commands that shouldn't touch git. They include `--help` plus any + // `oclif.additionalHelpFlags` (e.g. `-h`) declared in package.json, and we hard-include + // `--help`/`-h`/`help` as the same kind of defensive fallback. + // Completion invocations are matched by the `autocomplete` prefix, since shell completion + // actually runs `autocomplete:options` (not the bare `autocomplete` command). + // NOTE: temporary hook until v12. + const arg = process.argv[2] + const additionalVersionFlags = this.config?.pjson?.oclif?.additionalVersionFlags ?? [] + const additionalHelpFlags = this.config?.pjson?.oclif?.additionalHelpFlags ?? [] + const skip = new Set([ + '--help', + '--version', + '-h', + '-v', + 'help', + 'version', + ...additionalHelpFlags, + ...additionalVersionFlags, + ]) + if (!arg || skip.has(arg) || arg.startsWith('autocomplete')) return + + const git = new Git() + try { + await git.configureCredentialHelper() + } catch { + // ignore — best-effort; never block a command on git config + } +} + +export default hook diff --git a/src/lib/git/git.ts b/src/lib/git/git.ts index ab40f3c189..bc8ea35b22 100644 --- a/src/lib/git/git.ts +++ b/src/lib/git/git.ts @@ -16,12 +16,17 @@ export default class Git { */ async configureCredentialHelper() { const {httpGitHost} = vars - await this.exec([ - 'config', - '--global', - `credential.https://${httpGitHost}.helper`, - '!heroku git:credentials', - ]) + const key = `credential.https://${httpGitHost}.helper` + const helper = '!heroku git:credentials' + // Read the existing value first so we can skip the write when it is already + // set to our helper. `git config` rewrites the global config file (new inode) + // even when the value is byte-identical, and this runs before nearly every + // command via the init hook, so the no-op write churns the user's gitconfig. + // When the key is absent the `--get` exits non-zero and `exec` throws, so we + // treat that as an empty string. + const existing = await this.exec(['config', '--global', '--get', key]).catch(() => '') + if (existing === helper) return + await this.exec(['config', '--global', key, helper]) } createRemote(remote: string, url: string) { diff --git a/test/unit/hooks/configure-git-credential-manager.unit.test.ts b/test/unit/hooks/configure-git-credential-manager.unit.test.ts new file mode 100644 index 0000000000..e14583da3a --- /dev/null +++ b/test/unit/hooks/configure-git-credential-manager.unit.test.ts @@ -0,0 +1,213 @@ +import {expect} from 'chai' +import {readFileSync} from 'node:fs' +import {join} from 'node:path' +import {fileURLToPath} from 'node:url' +import * as sinon from 'sinon' + +import configureGitCredentialManagerHook from '../../../src/hooks/init/configure-git-credential-manager.js' +import Git from '../../../src/lib/git/git.js' + +const callHook = async (id: string) => { + // The hook reads `this.config.pjson.oclif.additionalVersionFlags` and + // `.additionalHelpFlags`, so the bound `this` context must carry a real config to + // exercise the production code path (this.config IS set when oclif invokes the hook). + // The `--sentinel-help` flag below appears ONLY in additionalHelpFlags (not the + // hardcoded set), so a test using it proves the dynamic merge spread actually works. + const config = {pjson: {oclif: {additionalHelpFlags: ['-h', '--sentinel-help'], additionalVersionFlags: ['-v', 'version']}}} + const context = {config} + const options = { + argv: [], + config: config as any, + context: {} as any, + id, + } + + await configureGitCredentialManagerHook.call(context as any, options) +} + +describe('configure-git-credential-manager hook', function () { + let originalArgv: string[] + let configureStub: sinon.SinonStub + + beforeEach(function () { + // Save original argv since the hook reads process.argv[2] + originalArgv = process.argv + + // Stub on the prototype so the real git subprocess never runs + configureStub = sinon.stub(Git.prototype, 'configureCredentialHelper').resolves() + }) + + afterEach(function () { + process.argv = originalArgv + sinon.restore() + }) + + describe('when a normal command is invoked', function () { + beforeEach(function () { + process.argv = ['node', 'heroku', 'apps'] + }) + + it('configures the git credential helper exactly once', async function () { + await callHook('apps') + + expect(configureStub.calledOnce).to.be.true + }) + }) + + describe('when configuring the credential helper fails', function () { + beforeEach(function () { + process.argv = ['node', 'heroku', 'apps'] + configureStub.rejects(new Error('Git not found')) + }) + + it('does not throw', async function () { + try { + await callHook('apps') + } catch (error: any) { + expect.fail(`hook should not throw, but threw: ${error.message}`) + } + + expect(configureStub.calledOnce).to.be.true + }) + }) + + describe('when command is --version', function () { + beforeEach(function () { + process.argv = ['node', 'heroku', '--version'] + }) + + it('does not configure the git credential helper', async function () { + await callHook('--version') + + expect(configureStub.called).to.be.false + }) + }) + + describe('when command is the version command', function () { + beforeEach(function () { + process.argv = ['node', 'heroku', 'version'] + }) + + it('does not configure the git credential helper', async function () { + await callHook('version') + + expect(configureStub.called).to.be.false + }) + }) + + describe('when command is -v', function () { + beforeEach(function () { + process.argv = ['node', 'heroku', '-v'] + }) + + it('does not configure the git credential helper', async function () { + await callHook('-v') + + expect(configureStub.called).to.be.false + }) + }) + + describe('when command is --help', function () { + beforeEach(function () { + process.argv = ['node', 'heroku', '--help'] + }) + + it('does not configure the git credential helper', async function () { + await callHook('--help') + + expect(configureStub.called).to.be.false + }) + }) + + describe('when command is the help command', function () { + beforeEach(function () { + process.argv = ['node', 'heroku', 'help'] + }) + + it('does not configure the git credential helper', async function () { + await callHook('help') + + expect(configureStub.called).to.be.false + }) + }) + + describe('when command is -h', function () { + beforeEach(function () { + process.argv = ['node', 'heroku', '-h'] + }) + + it('does not configure the git credential helper', async function () { + await callHook('-h') + + expect(configureStub.called).to.be.false + }) + }) + + describe('when command is a flag present only in additionalHelpFlags', function () { + beforeEach(function () { + process.argv = ['node', 'heroku', '--sentinel-help'] + }) + + it('does not configure the git credential helper', async function () { + await callHook('--sentinel-help') + + expect(configureStub.called).to.be.false + }) + }) + + describe('when command is autocomplete', function () { + beforeEach(function () { + process.argv = ['node', 'heroku', 'autocomplete'] + }) + + it('does not configure the git credential helper', async function () { + await callHook('autocomplete') + + expect(configureStub.called).to.be.false + }) + }) + + describe('when command is autocomplete:options (the real completion path)', function () { + beforeEach(function () { + process.argv = ['node', 'heroku', 'autocomplete:options', 'apps'] + }) + + it('does not configure the git credential helper', async function () { + await callHook('autocomplete:options') + + expect(configureStub.called).to.be.false + }) + }) + + describe('when no command argument is provided', function () { + beforeEach(function () { + process.argv = ['node', 'heroku'] + }) + + it('does not configure the git credential helper', async function () { + await callHook('') + + expect(configureStub.called).to.be.false + }) + }) + + // Registration regression guard: the unit tests above call the hook function + // directly, so they pass even if the hook is removed from package.json. This + // pins the oclif `init` hook list so an accidental deletion or path typo — + // including a botched removal when this temporary hook is taken out at v12 — + // fails a test instead of silently disabling the feature. + describe('oclif init hook registration', function () { + it('registers the expected init hooks in package.json', function () { + const root = fileURLToPath(new URL('.', import.meta.url)) + const pjsonPath = join(root, '..', '..', '..', 'package.json') + const pjson = JSON.parse(readFileSync(pjsonPath, 'utf8')) + + expect(pjson.oclif.hooks.init).to.deep.equal([ + './dist/hooks/init/version', + './dist/hooks/init/terms-of-service', + './dist/hooks/init/setup-otel-telemetry', + './dist/hooks/init/configure-git-credential-manager', + ]) + }) + }) +}) diff --git a/test/unit/lib/git/git.unit.test.ts b/test/unit/lib/git/git.unit.test.ts index bc34119a70..265fc9f440 100644 --- a/test/unit/lib/git/git.unit.test.ts +++ b/test/unit/lib/git/git.unit.test.ts @@ -119,15 +119,45 @@ describe('git', function () { expect(git.url('foo')).to.equal('https://git.heroku.com/foo.git') }) - it('configures git credential helper globally for the Heroku Git host', async function () { + it('configures git credential helper globally when the key is absent', async function () { + // `git config --global --get` exits non-zero when the key is absent, so + // `exec` throws and the helper treats it as an empty existing value. + const err: any = new Error('not found') + err.code = 1 + execFileStub.onFirstCall().rejects(err) execFileStub.resolves({stderr: '', stdout: ''}) await git.configureCredentialHelper() + expect(execFileStub.calledTwice).to.be.true + expect(execFileStub.firstCall.args).to.deep.equal(['git', ['config', '--global', '--get', 'credential.https://git.heroku.com.helper']]) + const [cmd, args] = execFileStub.secondCall.args + expect(cmd).to.equal('git') + expect(args).to.deep.equal(['config', '--global', 'credential.https://git.heroku.com.helper', '!heroku git:credentials']) + }) + + it('overwrites the git credential helper when it is set to a different value', async function () { + execFileStub.onFirstCall().resolves({stderr: '', stdout: '!some-other-helper'}) + execFileStub.resolves({stderr: '', stdout: ''}) + + await git.configureCredentialHelper() + + expect(execFileStub.calledTwice).to.be.true + expect(execFileStub.firstCall.args).to.deep.equal(['git', ['config', '--global', '--get', 'credential.https://git.heroku.com.helper']]) + const [cmd, args] = execFileStub.secondCall.args + expect(cmd).to.equal('git') + expect(args).to.deep.equal(['config', '--global', 'credential.https://git.heroku.com.helper', '!heroku git:credentials']) + }) + + it('skips the write when the git credential helper is already configured', async function () { + execFileStub.resolves({stderr: '', stdout: '!heroku git:credentials'}) + + await git.configureCredentialHelper() + expect(execFileStub.calledOnce).to.be.true const [cmd, args] = execFileStub.firstCall.args expect(cmd).to.equal('git') - expect(args).to.deep.equal(['config', '--global', 'credential.https://git.heroku.com.helper', '!heroku git:credentials']) + expect(args).to.deep.equal(['config', '--global', '--get', 'credential.https://git.heroku.com.helper']) }) it('removes git credential helper from global config', async function () {