Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 36 additions & 11 deletions src/github/create-commit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export interface CreateCommitOptions {
* @param {string} refHead the base of the new commit(s)
* @param {string} treeSha the tree SHA that this commit will point to
* @param {string} message the message of the new commit
* @param {CreateCommitOptions} options additional options like author or commit signer
* @returns {Promise<string>} the new commit SHA
* @see https://docs.github.com/en/rest/git/commits?apiVersion=2022-11-28#create-a-commit
*/
Expand All @@ -44,15 +45,37 @@ export async function createCommit(
options: CreateCommitOptions = {}
): Promise<string> {
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;
let author: Required<UserData> | undefined = undefined;
let committer: Required<UserData> | undefined = undefined;

const commitDate = new Date();
// Attach author/commit date.
if (options.author) {
author = {
...options.author,
date: options.author.date ?? commitDate,
};
}
if (options.committer) {
committer = {
...options.committer,
date: options.committer.date ?? commitDate,
};
}

if (options.signer) {
signature = await options.signer.generateSignature({
message,
tree: treeSha,
parents: [refHead],
author,
committer,
});
} else {
signature = undefined;
}

const {
data: {sha, url},
} = await octokit.git.createCommit({
Expand All @@ -62,8 +85,10 @@ export async function createCommit(
tree: treeSha,
parents: [refHead],
signature,
author: options.author,
committer: options.committer,
author: author ? {...author, date: author.date.toISOString()} : author,
committer: committer
? {...committer, date: committer.date.toISOString()}
: committer,
});
logger.info(`Successfully created commit. See commit at ${url}`);
return sha;
Expand Down
7 changes: 6 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,12 @@ 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 {
Expand Down
8 changes: 7 additions & 1 deletion src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ export interface Logger {
export interface UserData {
name: string;
email: string;
date?: Date;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I follow why date is part of the UserData as opposed to the commit

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put this into the UserData as GitHub does it like this:
https://docs.github.com/en/rest/commits/commits?apiVersion=2022-11-28

"commit": {
  "url": "https://api.github.com/repos/octocat/Hello-World/git/commits/6dcb09b5b57875f334f61aebed695e2e4193db5e",
  "author": {
    "name": "Monalisa Octocat",
    "email": "support@github.com",
    "date": "2011-04-14T16:00:49Z"
  },
  "committer": {
    "name": "Monalisa Octocat",
    "email": "support@github.com",
    "date": "2011-04-14T16:00:49Z"
  },
  "message": "Fix all the bugs",
  "tree": {
    "url": "https://api.github.com/repos/octocat/Hello-World/tree/6dcb09b5b57875f334f61aebed695e2e4193db5e",
    "sha": "6dcb09b5b57875f334f61aebed695e2e4193db5e"
  },
  "comment_count": 0,
  "verification": {
    "verified": false,
    "reason": "unsigned",
    "signature": null,
    "payload": null,
    "verified_at": null
  }
},

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also if you run this command here:

gh api \
  -H "Accept: application/vnd.github+json" \
  -H "X-GitHub-Api-Version: 2022-11-28" \
  /repos/googleapis/code-suggester/commits/f470462d6aa6ce356a4b8151780c3131d9dc3431
Full output:

You will get:

{
  "sha": "f470462d6aa6ce356a4b8151780c3131d9dc3431",
  "node_id": "C_kwDOEF_fiNoAKGY0NzA0NjJkNmFhNmNlMzU2YTRiODE1MTc4MGMzMTMxZDlkYzM0MzE",
  "commit": {
    "author": {
      "name": "Dennis Kugelmann",
      "email": "dennis.kugelmann@simpleclub.com",
      "date": "2024-01-30T16:33:12Z"
    },
    "committer": {
      "name": "Dennis Kugelmann",
      "email": "dennis.kugelmann@simpleclub.com",
      "date": "2024-01-30T16:33:12Z"
    },
    "message": "Create/Pass commit date to commit for signing",
    "tree": {
      "sha": "c1cf81371f7f287d14ca2d8d4091db8339e0682a",
      "url": "https://api.github.com/repos/googleapis/code-suggester/git/trees/c1cf81371f7f287d14ca2d8d4091db8339e0682a"
    },
    "url": "https://api.github.com/repos/googleapis/code-suggester/git/commits/f470462d6aa6ce356a4b8151780c3131d9dc3431",
    "comment_count": 0,
    "verification": {
      "verified": true,
      "reason": "valid",
      "signature": "-----BEGIN PGP SIGNATURE-----\n\niQIzBAABCAAdFiEE7H0SA8TDxy7sibehbDlA5QJOreMFAmW5JMgACgkQbDlA5QJO\nreN2LQ/+LUEXtKj2mIxhXJUE6QBtQ62VSrVDm8LassE7fdz8QZfSzDb561L0hcfi\nV6r8aUlq1wTJSJGSTIEZBERO3FQKCtXrML+z0VDnxc34lf9h75lXFGfvvWoWntAE\n6XFTGVCHPx+MVmkexZkz2kk26XLqdD1h+iLwkjIeMP0gSEzRwJVzMlYlPvQFoKTW\n/RewhmN8e+q4Y06gPzbQAVxcMZn7R8NrhxaChM9FS2q7S8yeGCgCiR20uLpMkh0Y\nFWaVdLh8oYAVBn/lrf6CsiW44uiROhzvrhhPRm0fKaXDnyW5fFsElUZPH9/602n5\nPe+vJVtZXfnKhLIT+JyeulQRMakq2vUaqEqOJW/8cfz6of6zTU251Ca0pQ4wXkha\nxLbDlDEh7Mt+KOGHBAi3OqSJo3lKqKS9f+ZvMRPdTc36HUt6DDYZc0lZ3QWkupoa\n7tMGaShiTLEALm2BNFlUqIobm67mGgcm3EuGEohQ8+ZSTyzZLl3V0HDSoM9rNtZI\nVLVfcR6RbDyRjlL6SIdMkXoM1Fk83OYICE5QOiUlDW4teBBjYwb/UjxcUBEQ9zEY\nMI6CULh8k0hNDThttx6DlejtN/3aprbk44w2JyE3txoAmyifMOwtoS5U3zYXJtwF\nBJGCVKvZfDcXBlb76K9GChIAo68tltSH73nChHmot16deT0/dOU=\n=J/h5\n-----END PGP SIGNATURE-----",
      "payload": "tree c1cf81371f7f287d14ca2d8d4091db8339e0682a\nparent f091b69577a0ad2d14eb90888485f28909aba480\nauthor Dennis Kugelmann <dennis.kugelmann@simpleclub.com> 1706632392 +0000\ncommitter Dennis Kugelmann <dennis.kugelmann@simpleclub.com> 1706632392 +0000\n\nCreate/Pass commit date to commit for signing\n",
      "verified_at": "2024-11-14T03:15:56Z"
    }
  },
  "url": "https://api.github.com/repos/googleapis/code-suggester/commits/f470462d6aa6ce356a4b8151780c3131d9dc3431",
  "html_url": "https://github.com/googleapis/code-suggester/commit/f470462d6aa6ce356a4b8151780c3131d9dc3431",
  "comments_url": "https://api.github.com/repos/googleapis/code-suggester/commits/f470462d6aa6ce356a4b8151780c3131d9dc3431/comments",
  "author": {
    "login": "IchordeDionysos",
    "id": 10195482,
    "node_id": "MDQ6VXNlcjEwMTk1NDgy",
    "avatar_url": "https://avatars.githubusercontent.com/u/10195482?v=4",
    "gravatar_id": "",
    "url": "https://api.github.com/users/IchordeDionysos",
    "html_url": "https://github.com/IchordeDionysos",
    "followers_url": "https://api.github.com/users/IchordeDionysos/followers",
    "following_url": "https://api.github.com/users/IchordeDionysos/following{/other_user}",
    "gists_url": "https://api.github.com/users/IchordeDionysos/gists{/gist_id}",
    "starred_url": "https://api.github.com/users/IchordeDionysos/starred{/owner}{/repo}",
    "subscriptions_url": "https://api.github.com/users/IchordeDionysos/subscriptions",
    "organizations_url": "https://api.github.com/users/IchordeDionysos/orgs",
    "repos_url": "https://api.github.com/users/IchordeDionysos/repos",
    "events_url": "https://api.github.com/users/IchordeDionysos/events{/privacy}",
    "received_events_url": "https://api.github.com/users/IchordeDionysos/received_events",
    "type": "User",
    "user_view_type": "public",
    "site_admin": false
  },
  "committer": {
    "login": "IchordeDionysos",
    "id": 10195482,
    "node_id": "MDQ6VXNlcjEwMTk1NDgy",
    "avatar_url": "https://avatars.githubusercontent.com/u/10195482?v=4",
    "gravatar_id": "",
    "url": "https://api.github.com/users/IchordeDionysos",
    "html_url": "https://github.com/IchordeDionysos",
    "followers_url": "https://api.github.com/users/IchordeDionysos/followers",
    "following_url": "https://api.github.com/users/IchordeDionysos/following{/other_user}",
    "gists_url": "https://api.github.com/users/IchordeDionysos/gists{/gist_id}",
    "starred_url": "https://api.github.com/users/IchordeDionysos/starred{/owner}{/repo}",
    "subscriptions_url": "https://api.github.com/users/IchordeDionysos/subscriptions",
    "organizations_url": "https://api.github.com/users/IchordeDionysos/orgs",
    "repos_url": "https://api.github.com/users/IchordeDionysos/repos",
    "events_url": "https://api.github.com/users/IchordeDionysos/events{/privacy}",
    "received_events_url": "https://api.github.com/users/IchordeDionysos/received_events",
    "type": "User",
    "user_view_type": "public",
    "site_admin": false
  },
  "parents": [
    {
      "sha": "f091b69577a0ad2d14eb90888485f28909aba480",
      "url": "https://api.github.com/repos/googleapis/code-suggester/commits/f091b69577a0ad2d14eb90888485f28909aba480",
      "html_url": "https://github.com/googleapis/code-suggester/commit/f091b69577a0ad2d14eb90888485f28909aba480"
    }
  ],
  "stats": {
    "total": 40,
    "additions": 29,
    "deletions": 11
  },
  "files": [
    {
      "sha": "21553db0912990ffd7e9d8092b88aa4e76ac08d7",
      "filename": "src/github/create-commit.ts",
      "status": "modified",
      "additions": 28,
      "deletions": 11,
      "changes": 39,
      "blob_url": "https://github.com/googleapis/code-suggester/blob/f470462d6aa6ce356a4b8151780c3131d9dc3431/src%2Fgithub%2Fcreate-commit.ts",
      "raw_url": "https://github.com/googleapis/code-suggester/raw/f470462d6aa6ce356a4b8151780c3131d9dc3431/src%2Fgithub%2Fcreate-commit.ts",
      "contents_url": "https://api.github.com/repos/googleapis/code-suggester/contents/src%2Fgithub%2Fcreate-commit.ts?ref=f470462d6aa6ce356a4b8151780c3131d9dc3431",
      "patch": "@@ -44,15 +44,28 @@ export async function createCommit(\n   options: CreateCommitOptions = {}\n ): Promise<string> {\n   try {\n-    const signature = options.signer\n-      ? await options.signer.generateSignature({\n-          message,\n-          tree: treeSha,\n-          parents: [refHead],\n-          author: options.author,\n-          committer: options.committer,\n-        })\n-      : undefined;\n+    let signature: string | undefined;\n+    if (options.signer) {\n+      const commitDate = new Date();\n+      // Attach author/commit date.\n+      if (options.author && !options.author?.date) {\n+        options.author.date = commitDate;\n+      }\n+      if (options.committer && !options.committer?.date) {\n+        options.committer.date = commitDate;\n+      }\n+\n+      signature = await options.signer.generateSignature({\n+        message,\n+        tree: treeSha,\n+        parents: [refHead],\n+        author: options.author,\n+        committer: options.committer,\n+      });\n+    } else {\n+      signature = undefined;\n+    }\n+\n     const {\n       data: {sha, url},\n     } = await octokit.git.createCommit({\n@@ -62,8 +75,12 @@ export async function createCommit(\n       tree: treeSha,\n       parents: [refHead],\n       signature,\n-      author: options.author,\n-      committer: options.committer,\n+      author: options.author\n+        ? {...options.author, date: options.author.date?.toISOString()}\n+        : undefined,\n+      committer: options.committer\n+        ? {...options.committer, date: options.committer.date?.toISOString()}\n+        : undefined,\n     });\n     logger.info(`Successfully created commit. See commit at ${url}`);\n     return sha;"
    },
    {
      "sha": "046c05a6385b466399a43e640bc3baecaa26f7f9",
      "filename": "src/types.ts",
      "status": "modified",
      "additions": 1,
      "deletions": 0,
      "changes": 1,
      "blob_url": "https://github.com/googleapis/code-suggester/blob/f470462d6aa6ce356a4b8151780c3131d9dc3431/src%2Ftypes.ts",
      "raw_url": "https://github.com/googleapis/code-suggester/raw/f470462d6aa6ce356a4b8151780c3131d9dc3431/src%2Ftypes.ts",
      "contents_url": "https://api.github.com/repos/googleapis/code-suggester/contents/src%2Ftypes.ts?ref=f470462d6aa6ce356a4b8151780c3131d9dc3431",
      "patch": "@@ -206,6 +206,7 @@ export interface Logger {\n export interface UserData {\n   name: string;\n   email: string;\n+  date?: Date;\n }\n \n export interface CommitData {"
    }
  ]
}

In the verification you will find:

"verification": {
  "verified": true,
  "reason": "valid",
  "signature": "-----BEGIN PGP SIGNATURE-----\n\niQIzBAABCAAdFiEE7H0SA8TDxy7sibehbDlA5QJOreMFAmW5JMgACgkQbDlA5QJO\nreN2LQ/+LUEXtKj2mIxhXJUE6QBtQ62VSrVDm8LassE7fdz8QZfSzDb561L0hcfi\nV6r8aUlq1wTJSJGSTIEZBERO3FQKCtXrML+z0VDnxc34lf9h75lXFGfvvWoWntAE\n6XFTGVCHPx+MVmkexZkz2kk26XLqdD1h+iLwkjIeMP0gSEzRwJVzMlYlPvQFoKTW\n/RewhmN8e+q4Y06gPzbQAVxcMZn7R8NrhxaChM9FS2q7S8yeGCgCiR20uLpMkh0Y\nFWaVdLh8oYAVBn/lrf6CsiW44uiROhzvrhhPRm0fKaXDnyW5fFsElUZPH9/602n5\nPe+vJVtZXfnKhLIT+JyeulQRMakq2vUaqEqOJW/8cfz6of6zTU251Ca0pQ4wXkha\nxLbDlDEh7Mt+KOGHBAi3OqSJo3lKqKS9f+ZvMRPdTc36HUt6DDYZc0lZ3QWkupoa\n7tMGaShiTLEALm2BNFlUqIobm67mGgcm3EuGEohQ8+ZSTyzZLl3V0HDSoM9rNtZI\nVLVfcR6RbDyRjlL6SIdMkXoM1Fk83OYICE5QOiUlDW4teBBjYwb/UjxcUBEQ9zEY\nMI6CULh8k0hNDThttx6DlejtN/3aprbk44w2JyE3txoAmyifMOwtoS5U3zYXJtwF\nBJGCVKvZfDcXBlb76K9GChIAo68tltSH73nChHmot16deT0/dOU=\n=J/h5\n-----END PGP SIGNATURE-----",
  "payload": "tree c1cf81371f7f287d14ca2d8d4091db8339e0682a\nparent f091b69577a0ad2d14eb90888485f28909aba480\nauthor Dennis Kugelmann <dennis.kugelmann@simpleclub.com> 1706632392 +0000\ncommitter Dennis Kugelmann <dennis.kugelmann@simpleclub.com> 1706632392 +0000\n\nCreate/Pass commit date to commit for signing\n",
  "verified_at": "2024-11-14T03:15:56Z"
}

The payload contains a date for both the committer and the author:

tree c1cf81371f7f287d14ca2d8d4091db8339e0682a
parent f091b69577a0ad2d14eb90888485f28909aba480
author Dennis Kugelmann <dennis.kugelmann@simpleclub.com> 1706632392 +0000
committer Dennis Kugelmann <dennis.kugelmann@simpleclub.com> 1706632392 +0000

Create/Pass commit date to commit for signing

I'm unsure whether the committer and the author date will be the same?
Probably for the purpose of this repository, which is all about creating changes and committing them immediately, it probably is.

In the end I do use the same data for committer and author if not passed by the user.

}

export interface CommitData {
Expand All @@ -216,6 +217,11 @@ export interface CommitData {
committer?: UserData;
}

export interface CommitDataWithRequiredDate extends CommitData {
author?: Required<UserData>;
committer?: Required<UserData>;
}

export interface CommitSigner {
generateSignature(commit: CommitData): Promise<string>;
generateSignature(commit: CommitDataWithRequiredDate): Promise<string>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a breaking change which makes additional inputs required. Does this actually need to be a required field?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might have a thinking mistake:

The reason why I was making the date required is because all places where the generateSignature function is being called are from this library.
Consumers of the library would only listen to generateSignature function being called.

So they simply get a new never null field in the function parameters.

I'm happy to change it make that field optional.

}
45 changes: 44 additions & 1 deletion test/commit-and-push.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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', () => {
Expand Down
Loading