Replace commit comments with a single runtime PR comment

Avoid the added complexity of matching existing commit comments and just track a single PR comment for the run of the workflow. A new PR comment is created on every run and will be polled for eligible reactions. For now stale PR comments are not removed. Change-type: minor Signed-off-by: Kyle Harding <[email protected]>
product-os · Oct 16, 2024 · 0b897fc · 0b897fc
1 parent 0d49d25
commit 0b897fc
Show file tree

Hide file tree

Showing 10 changed files with 265 additions and 466 deletions.
diff --git a/README.md b/README.md
@@ -6,42 +6,29 @@ where you need manual approval before proceeding with certain actions.
 
 ## How It Works
 
-1. When triggered in a PR workflow, the action creates a commit comment on the
-   tip of HEAD.
-2. The comment requests that a repository maintainer react with the specified
-   approval reaction (default: 👍) to approve the workflow.
-3. The action then enters a loop, waiting for a reaction from someone with write
-   access to the repository.
-4. If the required reaction is not found, it will continue looping until the
+1. When triggered in a pull request workflow, the action creates new a comment
+   on the pull request. Reviews are completed by reacting with emoji 👍 or 👎 on
+   the comment to approve or reject respectively.
+2. The action then enters a loop, waiting for a reaction from an eligible
+   reviewer. Reviewers must have at least `write` access to the repository to
+   have their reactions considered as eligible. Read more about collaborator
+   permissions
+   [here](https://docs.github.com/en/rest/collaborators/collaborators#get-repository-permissions-for-a-user)
+3. If the required reaction is not found, it will continue looping until the
    step times out.
-5. If a denial reaction is received from someone with write access, the action
-   will exit with an error.
 
 ### Additional details
 
-- Reviews are completed by reacting with emoji 👍 or 👎 on the generated commit
-  comment.
-- If the review is rejected, the action will throw an error and exit the
-  workflow.
-- If the review is approved, the action will log the approver name and continue
-  the workflow.
-- Users must have at least `write` access to the repository to have their
-  reactions considered as eligible. Read
-  [this](https://docs.github.com/en/rest/collaborators/collaborators?apiVersion=2022-11-28#get-repository-permissions-for-a-user)
-  to see how permissions are mapped.
 - The user associated with the token running the action is excluded from
   eligible reviewers. It is advised to use the actions `GITHUB_TOKEN` secret or
   App Installation tokens.
-- By default, authors of commits on the PR are excluded from eligible reviewers,
-  but this can be toggled via an input.
-- The commit comment requiring review is always associated with the latest SHA
-  that triggered the PR workflow. This is done to prevent Actions Time Of Check
-  to Time Of Use (TOCTOU) attacks. Read more
+- By default, authors of commits on the pull request are excluded from eligible
+  reviewers, but this can be toggled via an input.
+- The comment requiring review is always associated with the current run of the
+  workflow. Reacting to previous comments has no effect. This is done to prevent
+  Actions Time Of Check to Time Of Use (TOCTOU) attacks. Read more
   [here](https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/)
   and [here](https://github.com/AdnaneKhan/ActionsTOCTOU/blob/main/README.md).
-- A helper PR comment is created for convenience, always pointing to the current
-  static commit comment requiring review. This PR comment is purely for
-  convenience and is not part of the chain of trust.
 
 ## Usage
 

diff --git a/__tests__/approval.test.js b/__tests__/approval.test.js
@@ -20,10 +20,10 @@ describe('ApprovalProcess', () => {
       getRefSha: jest.fn(),
       getAuthenticatedUser: jest.fn(),
       getPullRequestRepository: jest.fn(),
-      findCommitComment: jest.fn(),
-      createCommitComment: jest.fn(),
-      deleteStalePullRequestComments: jest.fn(),
-      createPullRequestComment: jest.fn()
+      findIssueComment: jest.fn(),
+      createIssueComment: jest.fn(),
+      deleteStaleIssueComments: jest.fn(),
+      getWorkflowRunUrl: jest.fn()
     }
 
     mockReactionManager = {
@@ -52,35 +52,28 @@ describe('ApprovalProcess', () => {
 
   describe('run', () => {
     beforeEach(() => {
-      mockGitHubClient.getPullRequestHeadSha.mockReturnValue('test-sha')
-      mockGitHubClient.getPullRequestMergeRef.mockReturnValue('pull/1/merge')
-      mockGitHubClient.getRefSha.mockResolvedValue('test-sha')
+      mockGitHubClient.getWorkflowRunUrl.mockReturnValue('http://test-url.com')
       mockGitHubClient.getAuthenticatedUser.mockResolvedValue({
         id: 'test-user-id'
       })
       mockGitHubClient.getPullRequestRepository.mockReturnValue(null)
-      mockGitHubClient.findCommitComment.mockResolvedValue(null)
-      mockGitHubClient.createCommitComment.mockResolvedValue({
+      mockGitHubClient.createIssueComment.mockResolvedValue({
         id: 'test-comment-id',
         html_url: 'http://test-url.com'
       })
       approvalProcess.waitForApproval = jest.fn()
     })
 
-    test('creates a new comment when no existing comment is found', async () => {
+    test('creates a new issue comment', async () => {
       await approvalProcess.run()
 
       const commentBody = [
         mockConfig.commentHeader,
+        'Workflow run: http://test-url.com',
         mockConfig.commentFooter
       ].join('\n\n')
-      expect(mockGitHubClient.findCommitComment).toHaveBeenCalledWith(
-        'test-sha',
-        'test-user-id',
-        commentBody
-      )
-      expect(mockGitHubClient.createCommitComment).toHaveBeenCalledWith(
-        'test-sha',
+      expect(mockGitHubClient.getWorkflowRunUrl).toHaveBeenCalled()
+      expect(mockGitHubClient.createIssueComment).toHaveBeenCalledWith(
         commentBody
       )
       expect(core.setOutput).toHaveBeenCalledWith(
@@ -92,33 +85,9 @@ describe('ApprovalProcess', () => {
         'test-user-id',
         mockConfig.waitReaction
       )
-      expect(
-        mockGitHubClient.deleteStalePullRequestComments
-      ).toHaveBeenCalledWith(mockConfig.commentHeader)
-      expect(mockGitHubClient.createPullRequestComment).toHaveBeenCalledWith(
-        expect.stringContaining(mockConfig.commentHeader)
-      )
-    })
-
-    test('uses existing comment when found', async () => {
-      const existingComment = {
-        id: 'existing-comment-id',
-        html_url: 'http://existing-url.com'
-      }
-      mockGitHubClient.findCommitComment.mockResolvedValue(existingComment)
-
-      await approvalProcess.run()
-
-      expect(mockGitHubClient.createCommitComment).not.toHaveBeenCalled()
-      expect(core.setOutput).toHaveBeenCalledWith(
-        'comment-id',
-        'existing-comment-id'
-      )
-      expect(mockReactionManager.setReaction).toHaveBeenCalledWith(
-        'existing-comment-id',
-        'test-user-id',
-        mockConfig.waitReaction
-      )
+      // expect(mockGitHubClient.deleteStaleIssueComments).toHaveBeenCalledWith(
+      //   mockConfig.commentHeader
+      // )
     })
 
     test('creates success reaction when approval is successful', async () => {