Skip to content
Snippets Groups Projects
Unverified Commit fa8d3ff6 authored by Sigurd Spieckermann's avatar Sigurd Spieckermann Committed by GitHub
Browse files

feat(git): add support for SSH-based commit signing (#29550)


Co-authored-by: default avatarRhys Arkins <rhys@arkins.net>
parent 4d3f7199
No related branches found
No related tags found
No related merge requests found
...@@ -655,7 +655,9 @@ To learn more about Git hooks, read the [Pro Git 2 book, section on Git Hooks](h ...@@ -655,7 +655,9 @@ To learn more about Git hooks, read the [Pro Git 2 book, section on Git Hooks](h
## gitPrivateKey ## gitPrivateKey
This should be an armored private key, so the type you get from running `gpg --export-secret-keys --armor 92066A17F0D1707B4E96863955FEF5171C45FAE5 > private.key`. This is a private PGP or SSH key for signing Git commits.
For PGP, it should be an armored private key, so the type you get from running `gpg --export-secret-keys --armor 92066A17F0D1707B4E96863955FEF5171C45FAE5 > private.key`.
Replace the newlines with `\n` before adding the resulting single-line value to your bot's config. Replace the newlines with `\n` before adding the resulting single-line value to your bot's config.
<!-- prettier-ignore --> <!-- prettier-ignore -->
...@@ -665,8 +667,8 @@ Replace the newlines with `\n` before adding the resulting single-line value to ...@@ -665,8 +667,8 @@ Replace the newlines with `\n` before adding the resulting single-line value to
It will be loaded _lazily_. It will be loaded _lazily_.
Before the first commit in a repository, Renovate will: Before the first commit in a repository, Renovate will:
1. Run `gpg import` (if you haven't before) 1. Run `gpg import` (if you haven't before) when using PGP
1. Run `git config user.signingkey` and `git config commit.gpgsign true` 1. Run `git config user.signingkey`, `git config commit.gpgsign true` and `git config gpg.format`
The `git` commands are run locally in the cloned repo instead of globally. The `git` commands are run locally in the cloned repo instead of globally.
This reduces the chance of unintended consequences with global Git configs on shared systems. This reduces the chance of unintended consequences with global Git configs on shared systems.
......
import os from 'node:os'; import os from 'node:os';
import fs from 'fs-extra';
import { any, mockDeep } from 'jest-mock-extended'; import { any, mockDeep } from 'jest-mock-extended';
import upath from 'upath'; import upath from 'upath';
import { Fixtures } from '../../../test/fixtures';
import { mockedExtended } from '../../../test/util'; import { mockedExtended } from '../../../test/util';
import * as exec_ from '../exec'; import * as exec_ from '../exec';
import { configSigningKey, writePrivateKey } from './private-key'; import { configSigningKey, writePrivateKey } from './private-key';
...@@ -19,6 +21,10 @@ const exec = mockedExtended(exec_); ...@@ -19,6 +21,10 @@ const exec = mockedExtended(exec_);
describe('util/git/private-key', () => { describe('util/git/private-key', () => {
describe('writePrivateKey()', () => { describe('writePrivateKey()', () => {
beforeEach(() => {
Fixtures.reset();
});
it('returns if no private key', async () => { it('returns if no private key', async () => {
await expect(writePrivateKey()).resolves.not.toThrow(); await expect(writePrivateKey()).resolves.not.toThrow();
await expect(configSigningKey('/tmp/some-repo')).resolves.not.toThrow(); await expect(configSigningKey('/tmp/some-repo')).resolves.not.toThrow();
...@@ -38,7 +44,7 @@ describe('util/git/private-key', () => { ...@@ -38,7 +44,7 @@ describe('util/git/private-key', () => {
await expect(writePrivateKey()).rejects.toThrow(); await expect(writePrivateKey()).rejects.toThrow();
}); });
it('imports the private key', async () => { it('imports the private GPG key', async () => {
const publicKey = 'BADC0FFEE'; const publicKey = 'BADC0FFEE';
const repoDir = '/tmp/some-repo'; const repoDir = '/tmp/some-repo';
exec.exec.calledWith(any()).mockResolvedValue({ stdout: '', stderr: '' }); exec.exec.calledWith(any()).mockResolvedValue({ stdout: '', stderr: '' });
...@@ -60,11 +66,72 @@ describe('util/git/private-key', () => { ...@@ -60,11 +66,72 @@ describe('util/git/private-key', () => {
expect(exec.exec).toHaveBeenCalledWith('git config commit.gpgsign true', { expect(exec.exec).toHaveBeenCalledWith('git config commit.gpgsign true', {
cwd: repoDir, cwd: repoDir,
}); });
expect(exec.exec).toHaveBeenCalledWith('git config gpg.format opengpg', {
cwd: repoDir,
});
}); });
it('does not import the key again', async () => { it('does not import the key again', async () => {
await expect(writePrivateKey()).resolves.not.toThrow(); await expect(writePrivateKey()).resolves.not.toThrow();
await expect(configSigningKey('/tmp/some-repo')).resolves.not.toThrow(); await expect(configSigningKey('/tmp/some-repo')).resolves.not.toThrow();
}); });
it('throws error if the private SSH key has a passphrase', async () => {
const privateKeyFile = upath.join(os.tmpdir() + '/git-private-ssh.key');
exec.exec.calledWith(any()).mockResolvedValue({ stdout: '', stderr: '' });
exec.exec
.calledWith(`ssh-keygen -y -P "" -f ${privateKeyFile}`)
.mockRejectedValueOnce({
stderr: `Load key "${privateKeyFile}": incorrect passphrase supplied to decrypt private key`,
stdout: '',
});
setPrivateKey(`\
-----BEGIN OPENSSH PRIVATE KEY-----
some-private-key with-passphrase
some-private-key with-passphrase
-----END OPENSSH PRIVATE KEY-----`);
await expect(writePrivateKey()).rejects.toThrow();
});
it('imports the private SSH key', async () => {
const privateKey = `\
-----BEGIN OPENSSH PRIVATE KEY-----
some-private-key
some-private-key
-----END OPENSSH PRIVATE KEY-----`;
const privateKeyFile = upath.join(os.tmpdir() + '/git-private-ssh.key');
const publicKeyFile = `${privateKeyFile}.pub`;
const publicKey = 'some-public-key';
const repoDir = '/tmp/some-repo';
exec.exec.calledWith(any()).mockResolvedValue({ stdout: '', stderr: '' });
exec.exec
.calledWith(`ssh-keygen -y -P "" -f ${privateKeyFile}`)
.mockResolvedValue({
stderr: '',
stdout: publicKey,
});
setPrivateKey(privateKey);
await expect(writePrivateKey()).resolves.not.toThrow();
await expect(configSigningKey(repoDir)).resolves.not.toThrow();
expect(exec.exec).toHaveBeenCalledWith(
`git config user.signingkey ${privateKeyFile}`,
{ cwd: repoDir },
);
const privateKeyFileMode = (await fs.stat(privateKeyFile)).mode;
expect((privateKeyFileMode & 0o777).toString(8)).toBe('600');
expect((await fs.readFile(privateKeyFile)).toString()).toEqual(
privateKey,
);
expect((await fs.readFile(publicKeyFile)).toString()).toEqual(publicKey);
expect(exec.exec).toHaveBeenCalledWith('git config commit.gpgsign true', {
cwd: repoDir,
});
expect(exec.exec).toHaveBeenCalledWith('git config gpg.format ssh', {
cwd: repoDir,
});
process.emit('exit', 0);
expect(fs.existsSync(privateKeyFile)).toBeFalse();
expect(fs.existsSync(publicKeyFile)).toBeFalse();
});
}); });
}); });
...@@ -5,14 +5,23 @@ import upath from 'upath'; ...@@ -5,14 +5,23 @@ import upath from 'upath';
import { PLATFORM_GPG_FAILED } from '../../constants/error-messages'; import { PLATFORM_GPG_FAILED } from '../../constants/error-messages';
import { logger } from '../../logger'; import { logger } from '../../logger';
import { exec } from '../exec'; import { exec } from '../exec';
import { newlineRegex } from '../regex'; import type { ExecResult } from '../exec/types';
import { newlineRegex, regEx } from '../regex';
import { addSecretForSanitizing } from '../sanitize'; import { addSecretForSanitizing } from '../sanitize';
type PrivateKeyFormat = 'gpg' | 'ssh';
const sshKeyRegex = regEx(
/-----BEGIN ([A-Z ]+ )?PRIVATE KEY-----.*?-----END ([A-Z]+ )?PRIVATE KEY-----/,
's',
);
let gitPrivateKey: PrivateKey | undefined; let gitPrivateKey: PrivateKey | undefined;
abstract class PrivateKey { abstract class PrivateKey {
protected readonly key: string; protected readonly key: string;
protected keyId: string | undefined; protected keyId: string | undefined;
protected abstract readonly gpgFormat: string;
constructor(key: string) { constructor(key: string) {
this.key = key.trim(); this.key = key.trim();
...@@ -39,12 +48,15 @@ abstract class PrivateKey { ...@@ -39,12 +48,15 @@ abstract class PrivateKey {
// TODO: types (#22198) // TODO: types (#22198)
await exec(`git config user.signingkey ${this.keyId!}`, { cwd }); await exec(`git config user.signingkey ${this.keyId!}`, { cwd });
await exec(`git config commit.gpgsign true`, { cwd }); await exec(`git config commit.gpgsign true`, { cwd });
await exec(`git config gpg.format ${this.gpgFormat}`, { cwd });
} }
protected abstract importKey(): Promise<string | undefined>; protected abstract importKey(): Promise<string | undefined>;
} }
class GPGKey extends PrivateKey { class GPGKey extends PrivateKey {
protected readonly gpgFormat = 'opengpg';
protected async importKey(): Promise<string | undefined> { protected async importKey(): Promise<string | undefined> {
const keyFileName = upath.join(os.tmpdir() + '/git-private-gpg.key'); const keyFileName = upath.join(os.tmpdir() + '/git-private-gpg.key');
await fs.outputFile(keyFileName, this.key); await fs.outputFile(keyFileName, this.key);
...@@ -60,11 +72,63 @@ class GPGKey extends PrivateKey { ...@@ -60,11 +72,63 @@ class GPGKey extends PrivateKey {
} }
} }
class SSHKey extends PrivateKey {
protected readonly gpgFormat = 'ssh';
protected async importKey(): Promise<string | undefined> {
const keyFileName = upath.join(os.tmpdir() + '/git-private-ssh.key');
if (await this.hasPassphrase(keyFileName)) {
throw new Error('SSH key must have an empty passhprase');
}
await fs.outputFile(keyFileName, this.key);
process.on('exit', () => fs.removeSync(keyFileName));
await fs.chmod(keyFileName, 0o600);
// HACK: `git` calls `ssh-keygen -Y sign ...` internally for SSH-based
// commit signing. Technically, only the private key is needed for signing,
// but `ssh-keygen` has an implementation quirk which requires also the
// public key file to exist. Therefore, we derive the public key from the
// private key just to satisfy `ssh-keygen` until the problem has been
// resolved.
// https://github.com/renovatebot/renovate/issues/18197#issuecomment-2152333710
const { stdout } = await exec(`ssh-keygen -y -P "" -f ${keyFileName}`);
const pubFileName = `${keyFileName}.pub`;
await fs.outputFile(pubFileName, stdout);
process.on('exit', () => fs.removeSync(pubFileName));
return keyFileName;
}
private async hasPassphrase(keyFileName: string): Promise<boolean> {
try {
await exec(`ssh-keygen -y -P "" -f ${keyFileName}`);
} catch (err) {
return (err as ExecResult).stderr.includes(
'incorrect passphrase supplied to decrypt private key',
);
}
return false;
}
}
function getPrivateKeyFormat(key: string): PrivateKeyFormat {
return sshKeyRegex.test(key) ? 'ssh' : 'gpg';
}
function createPrivateKey(key: string): PrivateKey {
switch (getPrivateKeyFormat(key)) {
case 'gpg':
logger.debug('gitPrivateKey: GPG key detected');
return new GPGKey(key);
case 'ssh':
logger.debug('gitPrivateKey: SSH key detected');
return new SSHKey(key);
}
}
export function setPrivateKey(key: string | undefined): void { export function setPrivateKey(key: string | undefined): void {
if (!is.nonEmptyStringAndNotWhitespace(key)) { if (!is.nonEmptyStringAndNotWhitespace(key)) {
return; return;
} }
gitPrivateKey = new GPGKey(key); gitPrivateKey = createPrivateKey(key);
} }
export async function writePrivateKey(): Promise<void> { export async function writePrivateKey(): Promise<void> {
......
...@@ -97,10 +97,12 @@ export class Fixtures { ...@@ -97,10 +97,12 @@ export class Fixtures {
vol.reset(); vol.reset();
fsExtraMock.pathExists.mockImplementation(pathExists); fsExtraMock.pathExists.mockImplementation(pathExists);
fsExtraMock.remove.mockImplementation(memfs.promises.rm); fsExtraMock.remove.mockImplementation(memfs.promises.rm);
fsExtraMock.removeSync.mockImplementation(memfs.rmSync);
fsExtraMock.readFile.mockImplementation(readFile); fsExtraMock.readFile.mockImplementation(readFile);
fsExtraMock.writeFile.mockImplementation(memfs.promises.writeFile); fsExtraMock.writeFile.mockImplementation(memfs.promises.writeFile);
fsExtraMock.outputFile.mockImplementation(outputFile); fsExtraMock.outputFile.mockImplementation(outputFile);
fsExtraMock.stat.mockImplementation(stat); fsExtraMock.stat.mockImplementation(stat);
fsExtraMock.chmod.mockImplementation(memfs.promises.chmod);
} }
private static getPathToFixtures(fixturesRoot = '.'): string { private static getPathToFixtures(fixturesRoot = '.'): string {
...@@ -113,10 +115,12 @@ export class Fixtures { ...@@ -113,10 +115,12 @@ export class Fixtures {
const fsExtraMock = { const fsExtraMock = {
pathExists: jest.fn(), pathExists: jest.fn(),
remove: jest.fn(), remove: jest.fn(),
removeSync: jest.fn(),
readFile: jest.fn(), readFile: jest.fn(),
writeFile: jest.fn(), writeFile: jest.fn(),
outputFile: jest.fn(), outputFile: jest.fn(),
stat: jest.fn(), stat: jest.fn(),
chmod: jest.fn(),
}; };
// Temporary solution, when all tests will be rewritten to Fixtures mocks can be moved into __mocks__ folder // Temporary solution, when all tests will be rewritten to Fixtures mocks can be moved into __mocks__ folder
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment