From f470462d6aa6ce356a4b8151780c3131d9dc3431 Mon Sep 17 00:00:00 2001 From: Dennis Kugelmann Date: Tue, 30 Jan 2024 16:33:12 +0000 Subject: [PATCH 1/4] Create/Pass commit date to commit for signing --- src/github/create-commit.ts | 39 ++++++++++++++++++++++++++----------- src/types.ts | 1 + 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/github/create-commit.ts b/src/github/create-commit.ts index acb01b1f..21553db0 100644 --- a/src/github/create-commit.ts +++ b/src/github/create-commit.ts @@ -44,15 +44,28 @@ export async function createCommit( options: CreateCommitOptions = {} ): Promise { try { - const signature = options.signer - ? await options.signer.generateSignature({ - message, - tree: treeSha, - parents: [refHead], - author: options.author, - committer: options.committer, - }) - : undefined; + let signature: string | undefined; + if (options.signer) { + const commitDate = new Date(); + // Attach author/commit date. + if (options.author && !options.author?.date) { + options.author.date = commitDate; + } + if (options.committer && !options.committer?.date) { + options.committer.date = commitDate; + } + + signature = await options.signer.generateSignature({ + message, + tree: treeSha, + parents: [refHead], + author: options.author, + committer: options.committer, + }); + } else { + signature = undefined; + } + const { data: {sha, url}, } = await octokit.git.createCommit({ @@ -62,8 +75,12 @@ export async function createCommit( tree: treeSha, parents: [refHead], signature, - author: options.author, - committer: options.committer, + author: options.author + ? {...options.author, date: options.author.date?.toISOString()} + : undefined, + committer: options.committer + ? {...options.committer, date: options.committer.date?.toISOString()} + : undefined, }); logger.info(`Successfully created commit. See commit at ${url}`); return sha; diff --git a/src/types.ts b/src/types.ts index 46c8e35c..046c05a6 100644 --- a/src/types.ts +++ b/src/types.ts @@ -206,6 +206,7 @@ export interface Logger { export interface UserData { name: string; email: string; + date?: Date; } export interface CommitData { From c42090c37814ad77a329882030033c78c8c63fc1 Mon Sep 17 00:00:00 2001 From: Dennis Kugelmann Date: Tue, 30 Jan 2024 18:21:44 +0000 Subject: [PATCH 2/4] Write tests --- test/commit-and-push.ts | 45 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/test/commit-and-push.ts b/test/commit-and-push.ts index 4099c3fe..0471aff6 100644 --- a/test/commit-and-push.ts +++ b/test/commit-and-push.ts @@ -15,7 +15,7 @@ /* eslint-disable node/no-unsupported-features/node-builtins */ import * as assert from 'assert'; -import {describe, it, before, afterEach} from 'mocha'; +import {describe, it, before, afterEach, beforeEach} from 'mocha'; import {octokit, setup} from './util'; import * as sinon from 'sinon'; import {GetResponseTypeFromEndpointMethod} from '@octokit/types'; @@ -31,6 +31,7 @@ import { } from '../src/types'; import {createCommit} from '../src/github/create-commit'; import {CommitError} from '../src/errors'; +import {SinonFakeTimers} from 'sinon'; type GetCommitResponse = GetResponseTypeFromEndpointMethod< typeof octokit.git.getCommit @@ -190,8 +191,13 @@ describe('Push', () => { describe('Commit', () => { const sandbox = sinon.createSandbox(); + let clock: SinonFakeTimers; + beforeEach(() => { + clock = sinon.useFakeTimers(); + }); afterEach(() => { sandbox.restore(); + clock.restore(); }); const origin: RepoDomain = { owner: 'Foo', @@ -225,6 +231,43 @@ describe('Commit', () => { parents: [head], }); }); + it('Uses current date when date in author/committer is undefined and commit signing is enabled', async () => { + // setup + const createCommitResponseData = await import( + './fixtures/create-commit-response.json' + ); + const createCommitResponse = { + headers: {}, + status: 201, + url: 'http://fake-url.com', + data: createCommitResponseData, + } as unknown as CreateCommitResponse; + const stubCreateCommit = sandbox + .stub(octokit.git, 'createCommit') + .resolves(createCommitResponse); + // tests + const sha = await createCommit(octokit, origin, head, treeSha, message, { + author: { + name: 'Fake Author', + email: 'fake.email@example.com', + }, + signer: new FakeCommitSigner('fake-signature'), + }); + assert.strictEqual(sha, createCommitResponse.data.sha); + sandbox.assert.calledOnceWithMatch(stubCreateCommit, { + owner: origin.owner, + repo: origin.repo, + message, + tree: treeSha, + parents: [head], + author: { + name: 'Fake Author', + email: 'fake.email@example.com', + date: new Date(clock.now).toISOString(), + }, + signature: 'fake-signature', + }); + }); }); describe('Update branch reference', () => { From 085fe71c52f5e57783d97f448948ae06b7bdc504 Mon Sep 17 00:00:00 2001 From: Dennis Kugelmann Date: Tue, 30 Jan 2024 18:49:01 +0000 Subject: [PATCH 3/4] Improve types to make date always required in signer --- src/github/create-commit.ts | 19 +++++++++++++------ src/types.ts | 7 ++++++- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/github/create-commit.ts b/src/github/create-commit.ts index 21553db0..f2e7a91c 100644 --- a/src/github/create-commit.ts +++ b/src/github/create-commit.ts @@ -47,20 +47,27 @@ export async function createCommit( let signature: string | undefined; if (options.signer) { const commitDate = new Date(); + let author, committer: Required | undefined = undefined; // Attach author/commit date. - if (options.author && !options.author?.date) { - options.author.date = commitDate; + if (options.author) { + author = { + ...options.author, + date: options.author.date ?? commitDate, + }; } - if (options.committer && !options.committer?.date) { - options.committer.date = commitDate; + if (options.committer) { + committer = { + ...options.committer, + date: options.committer.date ?? commitDate, + } } signature = await options.signer.generateSignature({ message, tree: treeSha, parents: [refHead], - author: options.author, - committer: options.committer, + author, + committer, }); } else { signature = undefined; diff --git a/src/types.ts b/src/types.ts index 046c05a6..87836025 100644 --- a/src/types.ts +++ b/src/types.ts @@ -217,6 +217,11 @@ export interface CommitData { committer?: UserData; } +export interface CommitDataWithRequiredDate extends CommitData { + author?: Required; + committer?: Required; +} + export interface CommitSigner { - generateSignature(commit: CommitData): Promise; + generateSignature(commit: CommitDataWithRequiredDate): Promise; } From 5481c5e486e1e96a8597b93d610a74381406338f Mon Sep 17 00:00:00 2001 From: Dennis Kugelmann Date: Tue, 30 Jan 2024 18:56:14 +0000 Subject: [PATCH 4/4] Expose CommitDataWithRequiredDate --- src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index 9c5894f3..52edfcf2 100644 --- a/src/index.ts +++ b/src/index.ts @@ -22,7 +22,7 @@ import { FileDiffContent, CreateReviewCommentUserOptions, } from './types'; -export {Changes, CommitData, CommitSigner} from './types'; +export {Changes, CommitData, CommitDataWithRequiredDate, CommitSigner} from './types'; import {Octokit} from '@octokit/rest'; import {logger, setupLogger} from './logger'; import {