Merge pull request #391 from github/fixup/commit-safety-checks

Clean up / improve: `commit-safety-checks.js`

Author: GrantBirki

__tests__/functions/commit-safety-checks.test.js

@@ -1,4 +1,7 @@
-import {commitSafetyChecks} from '../../src/functions/commit-safety-checks'
+import {
+  commitSafetyChecks,
+  isTimestampOlder
+} from '../../src/functions/commit-safety-checks'
 import {COLORS} from '../../src/functions/colors'
 import * as core from '@actions/core'
 
@@ -164,3 +167,89 @@ test('raises an error if the date format is invalid', async () => {
     'Invalid date format. Please ensure the dates are valid UTC timestamps.'
   )
 })
+
+test('throws if context.payload.comment.created_at is missing', async () => {
+  const brokenContext = {payload: {comment: {}}}
+  await expect(commitSafetyChecks(brokenContext, data)).rejects.toThrow(
+    'Missing context.payload.comment.created_at'
+  )
+})
+
+test('throws if commit.author.date is missing', async () => {
+  const brokenData = JSON.parse(JSON.stringify(data))
+  delete brokenData.commit.author.date
+  await expect(commitSafetyChecks(context, brokenData)).rejects.toThrow(
+    'Missing commit.author.date'
+  )
+})
+
+test('rejects a deployment if commit.verification.verified_at is null and commit_verification is true', async () => {
+  data.inputs.commit_verification = true
+  data.commit.verification = {
+    verified: true,
+    reason: 'valid',
+    signature: 'SOME_SIGNATURE',
+    payload: 'SOME_PAYLOAD',
+    verified_at: null
+  }
+
+  await expect(commitSafetyChecks(context, data)).resolves.toEqual({
+    message: `### ⚠️ Cannot proceed with deployment\n\n- commit: \`${sha}\`\n- verification failed reason: \`valid\`\n\n> The commit signature is not valid as there is no valid \`verified_at\` date. Please ensure the commit has been properly signed and try again.`,
+    status: false,
+    isVerified: true
+  })
+})
+
+test('rejects a deployment if commit.verification.verified_at is missing and commit_verification is true', async () => {
+  data.inputs.commit_verification = true
+  data.commit.verification = {
+    verified: true,
+    reason: 'valid',
+    signature: 'SOME_SIGNATURE',
+    payload: 'SOME_PAYLOAD'
+  }
+
+  await expect(commitSafetyChecks(context, data)).resolves.toEqual({
+    message: `### ⚠️ Cannot proceed with deployment\n\n- commit: \`${sha}\`\n- verification failed reason: \`valid\`\n\n> The commit signature is not valid as there is no valid \`verified_at\` date. Please ensure the commit has been properly signed and try again.`,
+    status: false,
+    isVerified: true
+  })
+})
+
+test('isTimestampOlder throws if timestampA is missing', () => {
+  expect(() => isTimestampOlder(undefined, '2024-10-15T11:00:00Z')).toThrow(
+    'One or both timestamps are missing or empty.'
+  )
+})
+
+test('isTimestampOlder throws if timestampB is missing', () => {
+  expect(() => isTimestampOlder('2024-10-15T11:00:00Z', undefined)).toThrow(
+    'One or both timestamps are missing or empty.'
+  )
+})
+
+test('isTimestampOlder throws if both timestamps are invalid', () => {
+  expect(() => isTimestampOlder('bad', 'bad')).toThrow(/Invalid date format/)
+})
+
+test('isTimestampOlder covers else branch (not older)', async () => {
+  const context = {payload: {comment: {created_at: '2024-10-15T12:00:00Z'}}}
+  const data = {
+    sha: 'abc123',
+    commit: {
+      author: {date: '2024-10-15T11:00:00Z'},
+      verification: {
+        verified: false,
+        reason: 'unsigned',
+        signature: null,
+        payload: null,
+        verified_at: null
+      }
+    },
+    inputs: {commit_verification: false}
+  }
+  await commitSafetyChecks(context, data)
+  expect(debugMock).toHaveBeenCalledWith(
+    '2024-10-15T12:00:00Z is not older than 2024-10-15T11:00:00Z'
+  )
+})

dist/index.js

@@ -9,19 +9,26 @@ export async function commitSafetyChecks(context, data) {
   const commit = data.commit
   const inputs = data.inputs
   const sha = data.sha
+  const comment_created_at = context?.payload?.comment?.created_at
+  const commit_created_at = commit?.author?.date // fetch the timestamp that the commit was authored (format: "2024-10-21T19:10:24Z" - String)
+  const verified_at = commit?.verification?.verified_at
+  core.debug(`comment_created_at: ${comment_created_at}`)
+  core.debug(`commit_created_at: ${commit_created_at}`)
+  core.debug(`verified_at: ${verified_at}`)
+
+  // Defensive: Ensure required fields exist
+  if (!comment_created_at) {
+    throw new Error('Missing context.payload.comment.created_at')
+  }
+  if (!commit_created_at) {
+    throw new Error('Missing commit.author.date')
+  }
 
   const isVerified = commit?.verification?.verified === true ? true : false
   core.debug(`isVerified: ${isVerified}`)
   core.setOutput('commit_verified', isVerified)
   core.saveState('commit_verified', isVerified)
 
-  const comment_created_at = context.payload.comment.created_at
-  core.debug(`comment_created_at: ${comment_created_at}`)
-
-  // fetch the timestamp that the commit was authored (format: "2024-10-21T19:10:24Z" - String)
-  const commit_created_at = commit.author.date
-  core.debug(`commit_created_at: ${commit_created_at}`)
-
   // check to ensure that the commit was authored before the comment was created
   if (isTimestampOlder(comment_created_at, commit_created_at)) {
     return {
@@ -52,11 +59,20 @@ export async function commitSafetyChecks(context, data) {
     }
   }
 
+  // if commit_verification is enabled and the verified_at timestamp is not present, throw an error
+  if (inputs.commit_verification === true && !verified_at) {
+    return {
+      message: `### ⚠️ Cannot proceed with deployment\n\n- commit: \`${sha}\`\n- verification failed reason: \`${commit?.verification?.reason}\`\n\n> The commit signature is not valid as there is no valid \`verified_at\` date. Please ensure the commit has been properly signed and try again.`,
+      status: false,
+      isVerified: isVerified
+    }
+  }
+
   // check to ensure that the commit signature was authored before the comment was created
   // even if the commit signature is valid, we still want to reject it if it was authored after the comment was created
   if (
     inputs.commit_verification === true &&
-    isTimestampOlder(comment_created_at, commit?.verification?.verified_at)
+    isTimestampOlder(comment_created_at, verified_at)
   ) {
     return {
       message: `### ⚠️ Cannot proceed with deployment\n\nThe latest commit is not safe for deployment. The commit signature was verified after the trigger comment was created. Please try again if you recently pushed a new commit.`,
@@ -77,15 +93,19 @@ export async function commitSafetyChecks(context, data) {
 // :param timestampA: The first timestamp to compare (String - format: "2024-10-21T19:10:24Z")
 // :param timestampB: The second timestamp to compare (String - format: "2024-10-21T19:10:24Z")
 // :returns: true if timestampA is older than timestampB, false otherwise
-function isTimestampOlder(timestampA, timestampB) {
+export function isTimestampOlder(timestampA, timestampB) {
+  // Defensive: handle null/undefined/empty
+  if (!timestampA || !timestampB) {
+    throw new Error('One or both timestamps are missing or empty.')
+  }
   // Parse the date strings into Date objects
   const timestampADate = new Date(timestampA)
   const timestampBDate = new Date(timestampB)
 
   // Check if the parsed dates are valid
   if (isNaN(timestampADate) || isNaN(timestampBDate)) {
     throw new Error(
-      'Invalid date format. Please ensure the dates are valid UTC timestamps.'
+      `Invalid date format. Please ensure the dates are valid UTC timestamps. Received: '${timestampA}', '${timestampB}'`
     )
   }