From e46c5f7b077fd6edeab7020d80231b66b17e436e Mon Sep 17 00:00:00 2001 From: Rasmus Widing Date: Tue, 19 Aug 2025 10:34:50 +0300 Subject: [PATCH] Simplify authorization for external PR workflows - Move authorization check to Stage 1 job condition - Remove complex authorization job from Stage 2 - Fix duplicate exec declaration error - Add unauthorized user message handling in Stage 1 - Trust Stage 1's authorization in Stage 2 --- .../workflows/claude-review-ext-stage1.yml | 49 ++++++- .github/workflows/claude-review-ext.yml | 134 +----------------- 2 files changed, 50 insertions(+), 133 deletions(-) diff --git a/.github/workflows/claude-review-ext-stage1.yml b/.github/workflows/claude-review-ext-stage1.yml index 146c33ae..8ddf19fb 100644 --- a/.github/workflows/claude-review-ext-stage1.yml +++ b/.github/workflows/claude-review-ext-stage1.yml @@ -16,12 +16,13 @@ jobs: collect-pr-info: # Only trigger on specific conditions # For PRs: always collect info - # For comments: only when @claude-review-ext is mentioned + # For comments: only when @claude-review-ext is mentioned by authorized users if: | github.event_name == 'pull_request' || ( (github.event_name == 'issue_comment' || github.event_name == 'pull_request_review_comment') && - contains(github.event.comment.body, '@claude-review-ext') + contains(github.event.comment.body, '@claude-review-ext') && + contains(fromJSON('["Wirasm", "coleam00", "sean-eskerium"]'), github.event.comment.user.login) ) runs-on: ubuntu-latest @@ -235,4 +236,46 @@ jobs: echo "- **From Fork**: ${{ steps.validation.outputs.is_fork }}" >> $GITHUB_STEP_SUMMARY echo "- **Large PR**: ${{ steps.validation.outputs.large_diff }}" >> $GITHUB_STEP_SUMMARY echo "" >> $GITHUB_STEP_SUMMARY - echo "✅ PR information collected and uploaded as artifact for Stage 2 processing." >> $GITHUB_STEP_SUMMARY \ No newline at end of file + echo "✅ PR information collected and uploaded as artifact for Stage 2 processing." >> $GITHUB_STEP_SUMMARY + + unauthorized-message: + # Post message for unauthorized users + if: | + ( + github.event_name == 'issue_comment' || + github.event_name == 'pull_request_review_comment' + ) && + contains(github.event.comment.body, '@claude-review-ext') && + !contains(fromJSON('["Wirasm", "coleam00", "sean-eskerium"]'), github.event.comment.user.login) + + runs-on: ubuntu-latest + + permissions: + issues: write + pull-requests: write + + steps: + - name: Post unauthorized message + uses: actions/github-script@v7 + with: + script: | + const comment = { + owner: context.repo.owner, + repo: context.repo.repo, + body: `❌ @${context.actor} - You are not authorized to trigger Claude reviews on external PRs.\n\nOnly the following maintainers can trigger Claude: @Wirasm, @coleam00, @sean-eskerium\n\nPlease ask a maintainer for review.` + }; + + if (context.eventName === 'issue_comment') { + await github.rest.issues.createComment({ + ...comment, + issue_number: context.issue.number + }); + } else if (context.eventName === 'pull_request_review_comment') { + await github.rest.pulls.createReplyForReviewComment({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.pull_request.number, + comment_id: context.payload.comment.id, + body: comment.body + }); + } \ No newline at end of file diff --git a/.github/workflows/claude-review-ext.yml b/.github/workflows/claude-review-ext.yml index 3d19bd16..06eaf5bb 100644 --- a/.github/workflows/claude-review-ext.yml +++ b/.github/workflows/claude-review-ext.yml @@ -10,107 +10,10 @@ on: types: [completed] jobs: - check-authorization: - # Only run if Stage 1 completed successfully - if: github.event.workflow_run.conclusion == 'success' - - runs-on: ubuntu-latest - - outputs: - authorized: ${{ steps.check.outputs.authorized }} - pr_number: ${{ steps.check.outputs.pr_number }} - comment_author: ${{ steps.check.outputs.comment_author }} - trigger_phrase: ${{ steps.check.outputs.trigger_phrase }} - - permissions: - actions: read - pull-requests: read - - steps: - - name: Download PR Info Artifact - uses: actions/github-script@v7 - id: download-artifact - with: - script: | - // Get artifacts from the workflow run - const artifacts = await github.rest.actions.listWorkflowRunArtifacts({ - owner: context.repo.owner, - repo: context.repo.repo, - run_id: ${{ github.event.workflow_run.id }} - }); - - const matchArtifact = artifacts.data.artifacts.find(artifact => - artifact.name.startsWith('pr-info-') - ); - - if (!matchArtifact) { - core.setFailed('No PR info artifact found'); - return; - } - - // Download the artifact - const download = await github.rest.actions.downloadArtifact({ - owner: context.repo.owner, - repo: context.repo.repo, - artifact_id: matchArtifact.id, - archive_format: 'zip' - }); - - const fs = require('fs'); - const path = require('path'); - fs.writeFileSync('pr-info.zip', Buffer.from(download.data)); - - // Extract the artifact - const { exec } = require('child_process'); - await new Promise((resolve, reject) => { - exec('unzip -o pr-info.zip', (error, stdout, stderr) => { - if (error) reject(error); - else resolve(stdout); - }); - }); - - console.log('Artifact downloaded and extracted'); - - - name: Check Authorization - id: check - run: | - # Read PR info - PR_INFO=$(cat pr-info.json) - PR_NUMBER=$(echo "$PR_INFO" | jq -r '.prNumber') - COMMENT_AUTHOR=$(echo "$PR_INFO" | jq -r '.commentAuthor // .prAuthor') - TRIGGER_PHRASE=$(echo "$PR_INFO" | jq -r '.triggerPhrase // ""') - EVENT_NAME=$(echo "$PR_INFO" | jq -r '.eventName') - - echo "PR Number: $PR_NUMBER" - echo "Comment Author: $COMMENT_AUTHOR" - echo "Trigger Phrase: $TRIGGER_PHRASE" - echo "Event: $EVENT_NAME" - - # Check if this was triggered by a comment with the right phrase - AUTHORIZED="false" - if [[ "$EVENT_NAME" == "pull_request" ]]; then - # Automatic review for all PRs - AUTHORIZED="true" - echo "Authorized: Automatic review for PR event" - elif [[ "$TRIGGER_PHRASE" == "@claude-review-ext" ]]; then - # Check if user is authorized - AUTHORIZED_USERS='["Wirasm", "coleam00", "sean-eskerium"]' - if echo "$AUTHORIZED_USERS" | jq -e --arg user "$COMMENT_AUTHOR" 'contains([$user])' > /dev/null; then - AUTHORIZED="true" - echo "Authorized: $COMMENT_AUTHOR is in authorized users list" - else - echo "Not authorized: $COMMENT_AUTHOR is not in authorized users list" - fi - fi - - echo "authorized=$AUTHORIZED" >> $GITHUB_OUTPUT - echo "pr_number=$PR_NUMBER" >> $GITHUB_OUTPUT - echo "comment_author=$COMMENT_AUTHOR" >> $GITHUB_OUTPUT - echo "trigger_phrase=$TRIGGER_PHRASE" >> $GITHUB_OUTPUT - claude-review: - needs: check-authorization - if: needs.check-authorization.outputs.authorized == 'true' + # Only run if Stage 1 completed successfully + # Stage 1 already handles authorization, so we trust it + if: github.event.workflow_run.conclusion == 'success' runs-on: ubuntu-latest @@ -361,33 +264,4 @@ jobs: This PR is from a fork. Maintainers should manually verify the changes before merging. --- - *Review based on Archon V2 Alpha guidelines and secure external PR process* - - unauthorized-message: - needs: check-authorization - if: needs.check-authorization.outputs.authorized == 'false' && needs.check-authorization.outputs.trigger_phrase == '@claude-review-ext' - - runs-on: ubuntu-latest - - permissions: - issues: write - pull-requests: write - - steps: - - name: Post Unauthorized Message - uses: actions/github-script@v7 - with: - script: | - const prNumber = '${{ needs.check-authorization.outputs.pr_number }}'; - const commentAuthor = '${{ needs.check-authorization.outputs.comment_author }}'; - - const comment = { - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: parseInt(prNumber), - body: `❌ @${commentAuthor} - You are not authorized to trigger Claude reviews.\n\n` + - `Only the following maintainers can trigger Claude: @Wirasm, @coleam00, @sean-eskerium\n\n` + - `Please ask a maintainer for review.` - }; - - await github.rest.issues.createComment(comment); \ No newline at end of file + *Review based on Archon V2 Alpha guidelines and secure external PR process* \ No newline at end of file