mirror of
https://github.com/coleam00/Archon.git
synced 2025-12-24 02:39:17 -05:00
Simplify fork PR review to single workflow with pull_request_target
- Remove complex two-stage workflow approach - Use pull_request_target with security safeguards - Add first-time contributor check and approval requirement - Never checkout PR code - only analyze diff - Mirror full review format from main claude-review workflow - Manual trigger via @claude-review-fork for maintainers
This commit is contained in:
Rasmus Widing
150
.github/workflows/README-EXTERNAL-PRS.md
vendored
150
.github/workflows/README-EXTERNAL-PRS.md
vendored
@@ -1,150 +0,0 @@
|
||||
# Claude Review for External PRs (Forked Repositories)
|
||||
|
||||
## Overview
|
||||
|
||||
This document explains the two-stage workflow system for running Claude Code reviews on pull requests from forked repositories. This approach solves the GitHub Actions security limitation where forked PRs cannot access repository secrets.
|
||||
|
||||
## The Problem
|
||||
|
||||
- **Direct PRs**: Contributors with write access push branches directly to the main repository. Workflows have full access to secrets. ✅
|
||||
- **Forked PRs**: External contributors fork the repo and submit PRs. GitHub blocks secret access for security. ❌
|
||||
|
||||
## The Solution: Two-Stage Workflow
|
||||
|
||||
### Stage 1: PR Information Collection (`claude-review-ext-stage1.yml`)
|
||||
|
||||
- **Trigger**: Runs on `pull_request` events or when someone comments `@claude-review-ext`
|
||||
- **Permissions**: Read-only, no secrets
|
||||
- **Purpose**: Safely collect PR information from untrusted code
|
||||
- **Output**: Uploads PR details as an artifact
|
||||
|
||||
### Stage 2: Secure Review (`claude-review-ext.yml`)
|
||||
|
||||
- **Trigger**: Runs after Stage 1 completes via `workflow_run`
|
||||
- **Permissions**: Has access to repository secrets
|
||||
- **Purpose**: Downloads PR info and runs Claude review in secure context
|
||||
- **Security**: Never checks out untrusted PR code directly
|
||||
|
||||
## How to Use
|
||||
|
||||
### For External Contributors
|
||||
|
||||
1. Fork the repository
|
||||
2. Create your feature branch
|
||||
3. Submit a pull request
|
||||
4. Wait for automatic review or ask a maintainer to comment `@claude-review-ext`
|
||||
|
||||
### For Maintainers
|
||||
|
||||
To trigger a review on an external PR:
|
||||
```
|
||||
@claude-review-ext
|
||||
```
|
||||
|
||||
Authorized users:
|
||||
- @Wirasm
|
||||
- @coleam00
|
||||
- @sean-eskerium
|
||||
|
||||
## Security Model
|
||||
|
||||
### Stage 1 (Untrusted Environment)
|
||||
```yaml
|
||||
permissions:
|
||||
contents: read # Read-only
|
||||
pull-requests: read # Read-only
|
||||
# No secrets available
|
||||
```
|
||||
|
||||
### Stage 2 (Trusted Environment)
|
||||
```yaml
|
||||
permissions:
|
||||
contents: read # Read base repo
|
||||
pull-requests: write # Post comments
|
||||
issues: write # Post comments
|
||||
id-token: write # OIDC auth
|
||||
# Secrets ARE available
|
||||
```
|
||||
|
||||
### Key Security Features
|
||||
|
||||
1. **No Direct Code Execution**: Stage 2 never runs PR code directly
|
||||
2. **Artifact-Based Communication**: PR information passed via artifacts, not code execution
|
||||
3. **Authorization Check**: Only authorized users can trigger reviews via comments
|
||||
4. **Base Branch Checkout**: Stage 2 checks out main branch, not PR branch
|
||||
5. **Diff Analysis**: Reviews changes via git diff, not by running PR code
|
||||
|
||||
## Workflow Diagram
|
||||
|
||||
```
|
||||
┌──────────────────┐
|
||||
│ External Fork PR │
|
||||
└────────┬─────────┘
|
||||
│
|
||||
v
|
||||
┌────────────────────────┐
|
||||
│ Stage 1: Collect Info │
|
||||
│ (No Secrets) │
|
||||
│ - Checkout PR code │
|
||||
│ - Gather PR details │
|
||||
│ - Upload as artifact │
|
||||
└────────┬───────────────┘
|
||||
│
|
||||
v
|
||||
┌────────────────────────┐
|
||||
│ Stage 2: Claude Review │
|
||||
│ (Has Secrets) │
|
||||
│ - Download artifact │
|
||||
│ - Checkout base branch │
|
||||
│ - Analyze diff │
|
||||
│ - Post review │
|
||||
└────────────────────────┘
|
||||
```
|
||||
|
||||
## Comparison with Standard Workflows
|
||||
|
||||
| Feature | Standard (`@claude-review`) | External (`@claude-review-ext`) |
|
||||
|---------|----------------------------|--------------------------------|
|
||||
| Trigger | `@claude-review` | `@claude-review-ext` |
|
||||
| Works on direct branches | ✅ | ✅ |
|
||||
| Works on forked PRs | ❌ | ✅ |
|
||||
| Workflow files | 1 | 2 |
|
||||
| Security model | Direct execution | Two-stage isolation |
|
||||
| Setup complexity | Simple | Moderate |
|
||||
|
||||
## Troubleshooting
|
||||
|
||||
### Review Not Triggering
|
||||
|
||||
1. Check if you used the correct trigger: `@claude-review-ext` (not `@claude-review`)
|
||||
2. Verify you're an authorized user
|
||||
3. Check if Stage 1 workflow completed successfully
|
||||
|
||||
### Stage 1 Fails
|
||||
|
||||
- Usually indicates a problem with the PR itself
|
||||
- Check workflow logs for details
|
||||
|
||||
### Stage 2 Fails
|
||||
|
||||
- Often indicates an issue with Claude API or token
|
||||
- Check if `CLAUDE_CODE_OAUTH_TOKEN` secret is configured
|
||||
- Verify artifact was created in Stage 1
|
||||
|
||||
## Limitations
|
||||
|
||||
1. **Slight Delay**: Two-stage process takes longer than direct review
|
||||
2. **Artifact Size**: Large PRs might exceed artifact limits
|
||||
3. **Manual Trigger**: Automatic reviews on PR open require maintainer action for forks
|
||||
|
||||
## Future Improvements
|
||||
|
||||
- GitHub App integration for better fork support
|
||||
- Automatic security scanning before review
|
||||
- Caching for faster reviews on PR updates
|
||||
|
||||
## References
|
||||
|
||||
- [GitHub Actions Security Best Practices](https://docs.github.com/en/actions/security-guides)
|
||||
- [Preventing PWN Requests](https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/)
|
||||
- [Claude Code GitHub Actions](https://docs.anthropic.com/en/docs/claude-code/github-actions)
|
||||
281
.github/workflows/claude-review-ext-stage1.yml
vendored
281
.github/workflows/claude-review-ext-stage1.yml
vendored
@@ -1,281 +0,0 @@
|
||||
name: Claude Review External - Stage 1 (PR Info Collection)
|
||||
|
||||
# This workflow runs on pull requests from forks
|
||||
# It collects PR information and saves it as an artifact for Stage 2
|
||||
# No secrets are available or needed in this stage
|
||||
|
||||
on:
|
||||
pull_request:
|
||||
types: [opened, synchronize, reopened]
|
||||
issue_comment:
|
||||
types: [created]
|
||||
pull_request_review_comment:
|
||||
types: [created]
|
||||
|
||||
jobs:
|
||||
collect-pr-info:
|
||||
# Only trigger on specific conditions
|
||||
# For PRs: always collect info
|
||||
# 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(fromJSON('["Wirasm", "coleam00", "sean-eskerium"]'), github.event.comment.user.login)
|
||||
)
|
||||
|
||||
runs-on: ubuntu-latest
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
pull-requests: write # Need write to post comments
|
||||
|
||||
steps:
|
||||
- name: Checkout PR code
|
||||
uses: actions/checkout@v4
|
||||
with:
|
||||
# For PRs, this automatically checks out the merge commit
|
||||
fetch-depth: 0
|
||||
|
||||
- name: Collect PR Information
|
||||
id: pr-info
|
||||
uses: actions/github-script@v7
|
||||
with:
|
||||
script: |
|
||||
const fs = require('fs');
|
||||
|
||||
let prNumber, prTitle, prBody, prAuthor, baseBranch, headBranch, headSha;
|
||||
let triggerPhrase = '';
|
||||
let commentBody = '';
|
||||
let commentId = null;
|
||||
let commentAuthor = '';
|
||||
|
||||
if (context.eventName === 'pull_request') {
|
||||
prNumber = context.payload.pull_request.number;
|
||||
prTitle = context.payload.pull_request.title;
|
||||
prBody = context.payload.pull_request.body || '';
|
||||
prAuthor = context.payload.pull_request.user.login;
|
||||
baseBranch = context.payload.pull_request.base.ref;
|
||||
headBranch = context.payload.pull_request.head.ref;
|
||||
headSha = context.payload.pull_request.head.sha;
|
||||
} else if (context.eventName === 'issue_comment') {
|
||||
// For issue comments on PRs
|
||||
const issue = context.payload.issue;
|
||||
if (!issue.pull_request) {
|
||||
console.log('Not a PR comment, skipping');
|
||||
return;
|
||||
}
|
||||
|
||||
prNumber = issue.number;
|
||||
triggerPhrase = '@claude-review-ext';
|
||||
commentBody = context.payload.comment.body;
|
||||
commentId = context.payload.comment.id;
|
||||
commentAuthor = context.payload.comment.user.login;
|
||||
|
||||
// Fetch full PR details
|
||||
const { data: pr } = await github.rest.pulls.get({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
pull_number: prNumber
|
||||
});
|
||||
|
||||
prTitle = pr.title;
|
||||
prBody = pr.body || '';
|
||||
prAuthor = pr.user.login;
|
||||
baseBranch = pr.base.ref;
|
||||
headBranch = pr.head.ref;
|
||||
headSha = pr.head.sha;
|
||||
} else if (context.eventName === 'pull_request_review_comment') {
|
||||
prNumber = context.payload.pull_request.number;
|
||||
prTitle = context.payload.pull_request.title;
|
||||
prBody = context.payload.pull_request.body || '';
|
||||
prAuthor = context.payload.pull_request.user.login;
|
||||
baseBranch = context.payload.pull_request.base.ref;
|
||||
headBranch = context.payload.pull_request.head.ref;
|
||||
headSha = context.payload.pull_request.head.sha;
|
||||
triggerPhrase = '@claude-review-ext';
|
||||
commentBody = context.payload.comment.body;
|
||||
commentId = context.payload.comment.id;
|
||||
commentAuthor = context.payload.comment.user.login;
|
||||
}
|
||||
|
||||
// Get list of changed files
|
||||
const { data: files } = await github.rest.pulls.listFiles({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
pull_number: prNumber
|
||||
});
|
||||
|
||||
const changedFiles = files.map(f => ({
|
||||
filename: f.filename,
|
||||
status: f.status,
|
||||
additions: f.additions,
|
||||
deletions: f.deletions,
|
||||
changes: f.changes,
|
||||
patch: f.patch
|
||||
}));
|
||||
|
||||
// Get diff
|
||||
const { data: diff } = await github.rest.pulls.get({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
pull_number: prNumber,
|
||||
mediaType: {
|
||||
format: 'diff'
|
||||
}
|
||||
});
|
||||
|
||||
const prInfo = {
|
||||
prNumber,
|
||||
prTitle,
|
||||
prBody,
|
||||
prAuthor,
|
||||
baseBranch,
|
||||
headBranch,
|
||||
headSha,
|
||||
triggerPhrase,
|
||||
commentBody,
|
||||
commentId,
|
||||
commentAuthor,
|
||||
changedFiles,
|
||||
diff,
|
||||
eventName: context.eventName,
|
||||
repository: `${context.repo.owner}/${context.repo.repo}`,
|
||||
triggeredAt: new Date().toISOString()
|
||||
};
|
||||
|
||||
// Save to file
|
||||
fs.writeFileSync('pr-info.json', JSON.stringify(prInfo, null, 2));
|
||||
|
||||
console.log(`Collected info for PR #${prNumber} by ${prAuthor}`);
|
||||
console.log(`Changed files: ${changedFiles.length}`);
|
||||
console.log(`Event: ${context.eventName}`);
|
||||
if (triggerPhrase) {
|
||||
console.log(`Triggered by: ${commentAuthor} with phrase: ${triggerPhrase}`);
|
||||
}
|
||||
|
||||
// Set outputs for job summary
|
||||
core.setOutput('pr_number', prNumber);
|
||||
core.setOutput('pr_author', prAuthor);
|
||||
core.setOutput('changed_files_count', changedFiles.length);
|
||||
|
||||
- name: Run Basic Validation
|
||||
id: validation
|
||||
run: |
|
||||
echo "Running basic validation checks..."
|
||||
|
||||
# Check if this is from a fork
|
||||
IS_FORK="${{ github.event.pull_request.head.repo.fork }}"
|
||||
echo "Is from fork: $IS_FORK"
|
||||
|
||||
# Count files changed
|
||||
CHANGED_FILES=$(jq '.changedFiles | length' pr-info.json)
|
||||
echo "Files changed: $CHANGED_FILES"
|
||||
|
||||
# Check file types
|
||||
echo "File types changed:"
|
||||
jq -r '.changedFiles[].filename' pr-info.json | sed 's/.*\.//' | sort | uniq -c
|
||||
|
||||
# Check for potential issues
|
||||
LARGE_DIFF=false
|
||||
if [ "$CHANGED_FILES" -gt 100 ]; then
|
||||
LARGE_DIFF=true
|
||||
echo "⚠️ Large PR detected: $CHANGED_FILES files changed"
|
||||
fi
|
||||
|
||||
echo "is_fork=$IS_FORK" >> $GITHUB_OUTPUT
|
||||
echo "large_diff=$LARGE_DIFF" >> $GITHUB_OUTPUT
|
||||
|
||||
- name: Upload PR Information
|
||||
uses: actions/upload-artifact@v4
|
||||
with:
|
||||
name: pr-info-${{ github.event.pull_request.number || github.event.issue.number }}
|
||||
path: pr-info.json
|
||||
retention-days: 1
|
||||
|
||||
- name: Post Status Comment
|
||||
if: github.event_name == 'issue_comment' || github.event_name == 'pull_request_review_comment'
|
||||
uses: actions/github-script@v7
|
||||
continue-on-error: true # Don't fail the workflow if comment posting fails
|
||||
with:
|
||||
script: |
|
||||
const prNumber = ${{ steps.pr-info.outputs.pr_number }};
|
||||
const isFork = '${{ steps.validation.outputs.is_fork }}' === 'true';
|
||||
const filesCount = ${{ steps.pr-info.outputs.changed_files_count }};
|
||||
|
||||
let statusMessage = `🔄 **Claude Review Stage 1 Complete**\n\n`;
|
||||
statusMessage += `- PR: #${prNumber}\n`;
|
||||
statusMessage += `- Author: @${{ steps.pr-info.outputs.pr_author }}\n`;
|
||||
statusMessage += `- Files Changed: ${filesCount}\n`;
|
||||
statusMessage += `- Fork Status: ${isFork ? '🔱 External Fork' : '✅ Direct Branch'}\n\n`;
|
||||
statusMessage += `📋 Stage 2 (Claude Review) will run automatically after this workflow completes.\n`;
|
||||
statusMessage += `This two-stage process ensures secure handling of forked PRs.`;
|
||||
|
||||
try {
|
||||
// Post comment on the PR
|
||||
await github.rest.issues.createComment({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
issue_number: prNumber,
|
||||
body: statusMessage
|
||||
});
|
||||
console.log(`Successfully posted status comment on PR #${prNumber}`);
|
||||
} catch (error) {
|
||||
console.log(`Warning: Could not post comment on PR #${prNumber}: ${error.message}`);
|
||||
console.log('This is expected for some permission scenarios. Stage 2 will still run.');
|
||||
}
|
||||
|
||||
- name: Job Summary
|
||||
run: |
|
||||
echo "## Claude Review Stage 1 Summary" >> $GITHUB_STEP_SUMMARY
|
||||
echo "" >> $GITHUB_STEP_SUMMARY
|
||||
echo "- **PR Number**: #${{ steps.pr-info.outputs.pr_number }}" >> $GITHUB_STEP_SUMMARY
|
||||
echo "- **Author**: @${{ steps.pr-info.outputs.pr_author }}" >> $GITHUB_STEP_SUMMARY
|
||||
echo "- **Files Changed**: ${{ steps.pr-info.outputs.changed_files_count }}" >> $GITHUB_STEP_SUMMARY
|
||||
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
|
||||
|
||||
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
|
||||
});
|
||||
}
|
||||
314
.github/workflows/claude-review-ext.yml
vendored
314
.github/workflows/claude-review-ext.yml
vendored
@@ -1,314 +0,0 @@
|
||||
name: Claude Review External - Stage 2 (Secure Review)
|
||||
|
||||
# This workflow runs after Stage 1 completes
|
||||
# It has access to secrets and performs the actual Claude review
|
||||
# Security: Runs in the context of the base repository, not the fork
|
||||
|
||||
on:
|
||||
workflow_run:
|
||||
workflows: ["Claude Review External - Stage 1 (PR Info Collection)"]
|
||||
types: [completed]
|
||||
|
||||
jobs:
|
||||
claude-review:
|
||||
# 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
|
||||
|
||||
permissions:
|
||||
contents: read # Read repository contents
|
||||
pull-requests: write # Comment on PRs
|
||||
issues: write # Comment on issues
|
||||
actions: read # Read CI results
|
||||
id-token: write # Required for OIDC authentication
|
||||
|
||||
steps:
|
||||
- name: Checkout Repository (Base Branch)
|
||||
uses: actions/checkout@v4
|
||||
with:
|
||||
# SECURITY: Checkout base branch, not PR code
|
||||
# We'll checkout the default branch first, then switch to the PR's base branch after downloading PR info
|
||||
fetch-depth: 0
|
||||
|
||||
- name: Download PR Info
|
||||
uses: actions/github-script@v7
|
||||
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');
|
||||
fs.writeFileSync('pr-info.zip', Buffer.from(download.data));
|
||||
|
||||
// Extract the artifact using execSync instead of exec to avoid declaration issues
|
||||
const childProcess = require('child_process');
|
||||
try {
|
||||
childProcess.execSync('unzip -o pr-info.zip');
|
||||
console.log('Artifact extracted successfully');
|
||||
} catch (error) {
|
||||
core.setFailed(`Failed to extract artifact: ${error.message}`);
|
||||
return;
|
||||
}
|
||||
|
||||
// Read and parse PR info
|
||||
const prInfo = JSON.parse(fs.readFileSync('pr-info.json', 'utf8'));
|
||||
|
||||
// Save key information as environment variables
|
||||
core.exportVariable('PR_NUMBER', prInfo.prNumber);
|
||||
core.exportVariable('PR_TITLE', prInfo.prTitle);
|
||||
core.exportVariable('PR_AUTHOR', prInfo.prAuthor);
|
||||
core.exportVariable('HEAD_SHA', prInfo.headSha);
|
||||
core.exportVariable('PR_BASE_BRANCH', prInfo.baseBranch);
|
||||
|
||||
console.log(`Loaded PR #${prInfo.prNumber} information`);
|
||||
console.log(`Base branch: ${prInfo.baseBranch}`);
|
||||
|
||||
- name: Switch to PR Base Branch
|
||||
run: |
|
||||
# Switch to the PR's actual target base branch
|
||||
git checkout ${{ env.PR_BASE_BRANCH }}
|
||||
echo "Switched to base branch: ${{ env.PR_BASE_BRANCH }}"
|
||||
|
||||
- name: Fetch PR Branch for Analysis
|
||||
run: |
|
||||
# Fetch the PR branch to analyze (but don't checkout)
|
||||
git fetch origin pull/${{ env.PR_NUMBER }}/head:pr-branch
|
||||
|
||||
# Create a safe diff for analysis against the PR's target base branch
|
||||
git diff origin/${{ env.PR_BASE_BRANCH }}...pr-branch > pr-diff.patch
|
||||
|
||||
echo "Fetched PR branch for analysis (not checked out for security)"
|
||||
|
||||
- name: Post Starting Comment
|
||||
uses: actions/github-script@v7
|
||||
with:
|
||||
script: |
|
||||
await github.rest.issues.createComment({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
issue_number: ${{ env.PR_NUMBER }},
|
||||
body: `🤖 **Claude Review (External PR) Starting...**\n\nAnalyzing PR changes in a secure environment. This may take a few minutes.`
|
||||
});
|
||||
|
||||
- name: Create Event Context for Claude
|
||||
run: |
|
||||
# Create a fake event.json that simulates an issue_comment event
|
||||
cat > /tmp/event.json << EOF
|
||||
{
|
||||
"action": "created",
|
||||
"issue": {
|
||||
"number": ${{ env.PR_NUMBER }},
|
||||
"title": "${{ env.PR_TITLE }}",
|
||||
"user": {
|
||||
"login": "${{ env.PR_AUTHOR }}"
|
||||
},
|
||||
"pull_request": {
|
||||
"url": "https://api.github.com/repos/${{ github.repository }}/pulls/${{ env.PR_NUMBER }}"
|
||||
}
|
||||
},
|
||||
"comment": {
|
||||
"body": "@claude-review-ext",
|
||||
"user": {
|
||||
"login": "github-actions"
|
||||
}
|
||||
},
|
||||
"repository": {
|
||||
"name": "$(echo ${{ github.repository }} | cut -d'/' -f2)",
|
||||
"owner": {
|
||||
"login": "$(echo ${{ github.repository }} | cut -d'/' -f1)"
|
||||
}
|
||||
}
|
||||
}
|
||||
EOF
|
||||
echo "Created issue_comment event context for PR #${{ env.PR_NUMBER }}"
|
||||
|
||||
- name: Run Claude Code Review
|
||||
id: claude
|
||||
uses: anthropics/claude-code-action@beta
|
||||
timeout-minutes: 15
|
||||
env:
|
||||
# Set environment variables to provide PR context
|
||||
GITHUB_EVENT_NAME: 'issue_comment'
|
||||
GITHUB_EVENT_PATH: '/tmp/event.json'
|
||||
with:
|
||||
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
|
||||
|
||||
# Explicitly provide GitHub token since OIDC fails in workflow_run context
|
||||
github_token: ${{ secrets.GITHUB_TOKEN }}
|
||||
|
||||
# Use the external review trigger phrase
|
||||
trigger_phrase: "@claude-review-ext"
|
||||
|
||||
# Direct prompt to review the diff file
|
||||
direct_prompt: |
|
||||
Please review the code changes in the pr-diff.patch file and post your review as a comment on PR #${{ env.PR_NUMBER }}.
|
||||
|
||||
The diff file contains all changes from the pull request. Analyze it thoroughly.
|
||||
|
||||
# Review-specific instructions (same as claude-review.yml)
|
||||
custom_instructions: |
|
||||
You are performing a CODE REVIEW for an EXTERNAL PULL REQUEST #${{ env.PR_NUMBER }}
|
||||
This is a secure two-stage review process for PRs from forks.
|
||||
|
||||
## Security Context
|
||||
You are running in a secure environment with access to repository secrets.
|
||||
The PR code has been fetched but NOT checked out for security reasons.
|
||||
Review the changes in pr-diff.patch file using the Read tool.
|
||||
|
||||
## Important
|
||||
Start by reading the pr-diff.patch file to see all the changes.
|
||||
Post your review as a comment on issue #${{ env.PR_NUMBER }}.
|
||||
|
||||
## Your Role
|
||||
You are reviewing code for Archon V2 Alpha, a local-first AI knowledge management system in early alpha stage.
|
||||
|
||||
## Architecture Context
|
||||
- Frontend: React + TypeScript + Vite (port 3737)
|
||||
- Backend: FastAPI + Socket.IO + Python (port 8181)
|
||||
- MCP Service: MCP protocol server (port 8051)
|
||||
- Agents Service: PydanticAI agents (port 8052)
|
||||
- Database: Supabase (PostgreSQL + pgvector)
|
||||
|
||||
## Review Process
|
||||
1. **Understand Changes**
|
||||
- Review the pr-diff.patch file to see all changes
|
||||
- Check what files were changed and understand the context
|
||||
- Analyze the impact across all services (frontend, backend, MCP, agents)
|
||||
- Consider interactions between components
|
||||
|
||||
## Review Focus Areas
|
||||
|
||||
### 1. Code Quality - Backend (Python)
|
||||
- Type hints on all functions and classes
|
||||
- Pydantic v2 models for data validation (ConfigDict not class Config, model_dump() not dict())
|
||||
- No print() statements (use logging instead)
|
||||
- Proper error handling with detailed error messages
|
||||
- Following PEP 8
|
||||
- Google style docstrings where appropriate
|
||||
|
||||
### 2. Code Quality - Frontend (React/TypeScript)
|
||||
- Proper TypeScript types (avoid 'any')
|
||||
- React hooks used correctly
|
||||
- Component composition and reusability
|
||||
- Proper error boundaries
|
||||
- Following existing component patterns
|
||||
|
||||
### 3. Structure & Architecture
|
||||
- Each feature self-contained with its own models, service, and tools
|
||||
- Shared components only for things used by multiple features
|
||||
- Proper separation of concerns across services
|
||||
- API endpoints follow RESTful conventions
|
||||
|
||||
### 4. Testing
|
||||
- Unit tests co-located with code in tests/ folders
|
||||
- Edge cases covered
|
||||
- Mocking external dependencies
|
||||
- Frontend: Vitest tests for components
|
||||
- Backend: Pytest tests for services
|
||||
|
||||
### 5. Alpha Project Principles (from CLAUDE.md)
|
||||
- No backwards compatibility needed - can break things
|
||||
- Fail fast with detailed errors (not graceful failures)
|
||||
- Remove dead code immediately
|
||||
- Focus on functionality over production patterns
|
||||
|
||||
### 6. Security Considerations for External PRs
|
||||
- Check for any attempts to access secrets or environment variables
|
||||
- Look for suspicious network requests or data exfiltration
|
||||
- Verify no hardcoded credentials or API keys
|
||||
- Check for SQL injection vulnerabilities
|
||||
- Review any file system operations
|
||||
|
||||
## Required Output Format
|
||||
|
||||
## Summary
|
||||
[2-3 sentence overview of what the changes do and their impact]
|
||||
|
||||
## Security Review (External PR)
|
||||
⚠️ This is an external PR from a fork. Additional security checks performed:
|
||||
- [ ] No unauthorized access to secrets/env vars
|
||||
- [ ] No suspicious network requests
|
||||
- [ ] No hardcoded credentials
|
||||
- [ ] No SQL injection risks
|
||||
- [ ] Safe file system operations
|
||||
[List any security concerns found or state "No security issues found"]
|
||||
|
||||
## Issues Found
|
||||
Total: [X critical, Y important, Z minor]
|
||||
|
||||
### 🔴 Critical (Must Fix)
|
||||
[Issues that will break functionality or cause data loss]
|
||||
- **[Issue Title]** - `path/to/file.py:123`
|
||||
Problem: [What's wrong]
|
||||
Fix: [Specific solution]
|
||||
|
||||
### 🟡 Important (Should Fix)
|
||||
[Issues that impact user experience or code maintainability]
|
||||
- **[Issue Title]** - `path/to/file.tsx:45`
|
||||
Problem: [What's wrong]
|
||||
Fix: [Specific solution]
|
||||
|
||||
### 🟢 Minor (Consider)
|
||||
[Nice-to-have improvements]
|
||||
- **[Suggestion]** - `path/to/file.py:67`
|
||||
[Brief description and why it would help]
|
||||
|
||||
## Performance Considerations
|
||||
- Database query efficiency (no N+1 queries)
|
||||
- Frontend bundle size impacts
|
||||
- Async/await usage in Python
|
||||
- React re-render optimization
|
||||
[List any performance issues or state "No performance concerns"]
|
||||
|
||||
## Good Practices Observed
|
||||
- [Highlight what was done well]
|
||||
- [Patterns that should be replicated]
|
||||
|
||||
## Test Coverage
|
||||
**Current Coverage:** [Estimate based on what you see]
|
||||
**Missing Tests:**
|
||||
|
||||
1. **[Component/Function Name]**
|
||||
- What to test: [Specific functionality]
|
||||
- Why important: [Impact if it fails]
|
||||
- Suggested test: [One sentence description]
|
||||
|
||||
## Recommendations
|
||||
|
||||
**Merge Decision:**
|
||||
- [ ] Ready to merge as-is
|
||||
- [ ] Requires fixes before merging
|
||||
- [ ] Needs security review by maintainer
|
||||
|
||||
**Priority Actions:**
|
||||
1. [Most important fix needed, if any]
|
||||
2. [Second priority, if applicable]
|
||||
|
||||
**External PR Note:**
|
||||
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*
|
||||
323
.github/workflows/claude-review-fork.yml
vendored
Normal file
323
.github/workflows/claude-review-fork.yml
vendored
Normal file
@@ -0,0 +1,323 @@
|
||||
name: Claude Review for Forked PRs
|
||||
|
||||
# This workflow handles Claude reviews for PRs from forks
|
||||
# Uses pull_request_target for security with first-time contributor checks
|
||||
|
||||
on:
|
||||
pull_request_target:
|
||||
types: [opened, synchronize, reopened]
|
||||
issue_comment:
|
||||
types: [created]
|
||||
|
||||
jobs:
|
||||
claude-review:
|
||||
# Trigger on:
|
||||
# 1. Any PR from a fork (automatic review)
|
||||
# 2. Comment with @claude-review-fork from authorized users
|
||||
if: |
|
||||
(
|
||||
github.event_name == 'pull_request_target' &&
|
||||
github.event.pull_request.head.repo.fork == true
|
||||
) ||
|
||||
(
|
||||
github.event_name == 'issue_comment' &&
|
||||
github.event.issue.pull_request &&
|
||||
contains(github.event.comment.body, '@claude-review-fork') &&
|
||||
contains(fromJSON('["Wirasm", "coleam00", "sean-eskerium"]'), github.event.comment.user.login)
|
||||
)
|
||||
|
||||
runs-on: ubuntu-latest
|
||||
|
||||
permissions:
|
||||
contents: read # Read repository contents
|
||||
pull-requests: write # Comment on PRs
|
||||
issues: write # Comment on issues
|
||||
actions: read # Read CI results
|
||||
id-token: write # Required for OIDC authentication
|
||||
|
||||
steps:
|
||||
# SECURITY: Check if this is a first-time contributor
|
||||
- name: Check First-Time Contributor
|
||||
id: first-time
|
||||
uses: actions/github-script@v7
|
||||
with:
|
||||
script: |
|
||||
const pr = context.payload.pull_request ||
|
||||
(context.payload.issue?.pull_request ?
|
||||
await github.rest.pulls.get({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
pull_number: context.payload.issue.number
|
||||
}).then(r => r.data) : null);
|
||||
|
||||
if (!pr) {
|
||||
core.setFailed('Could not find pull request');
|
||||
return;
|
||||
}
|
||||
|
||||
// Check if author has previous merged PRs
|
||||
const author = pr.user.login;
|
||||
const { data: prs } = await github.rest.pulls.list({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
state: 'closed',
|
||||
per_page: 1
|
||||
});
|
||||
|
||||
const previousPRs = prs.filter(p =>
|
||||
p.user.login === author &&
|
||||
p.merged_at !== null
|
||||
);
|
||||
|
||||
const isFirstTime = previousPRs.length === 0;
|
||||
|
||||
if (isFirstTime) {
|
||||
// Post warning comment for first-time contributors
|
||||
await github.rest.issues.createComment({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
issue_number: pr.number,
|
||||
body: `⚠️ **First-Time Contributor Notice**\n\n` +
|
||||
`Hi @${author}! This appears to be your first contribution to this repository.\n\n` +
|
||||
`For security reasons, automated Claude review requires manual approval for first-time contributors.\n` +
|
||||
`A maintainer will review your PR and may trigger Claude review if appropriate.\n\n` +
|
||||
`Maintainers can trigger review with: \`@claude-review-fork\``
|
||||
});
|
||||
|
||||
// Skip the rest of the workflow for first-time contributors
|
||||
core.setOutput('is_first_time', 'true');
|
||||
console.log(`First-time contributor detected: ${author}`);
|
||||
} else {
|
||||
core.setOutput('is_first_time', 'false');
|
||||
console.log(`Returning contributor: ${author}`);
|
||||
}
|
||||
|
||||
# Skip review for first-time contributors unless manually triggered
|
||||
- name: Check if Should Continue
|
||||
id: should-continue
|
||||
run: |
|
||||
if [[ "${{ steps.first-time.outputs.is_first_time }}" == "true" ]] && [[ "${{ github.event_name }}" == "pull_request_target" ]]; then
|
||||
echo "Skipping automatic review for first-time contributor"
|
||||
echo "should_continue=false" >> $GITHUB_OUTPUT
|
||||
else
|
||||
echo "Proceeding with review"
|
||||
echo "should_continue=true" >> $GITHUB_OUTPUT
|
||||
fi
|
||||
|
||||
# SECURITY: Checkout base branch, NOT the PR branch
|
||||
# This prevents executing untrusted code from forks
|
||||
- name: Checkout Base Branch
|
||||
if: steps.should-continue.outputs.should_continue == 'true'
|
||||
uses: actions/checkout@v4
|
||||
with:
|
||||
# Explicitly checkout the base branch, not the PR head
|
||||
ref: ${{ github.event.pull_request.base.ref || github.event.repository.default_branch }}
|
||||
fetch-depth: 0
|
||||
|
||||
# Fetch PR diff for analysis (without checking out PR code)
|
||||
- name: Fetch PR Diff
|
||||
if: steps.should-continue.outputs.should_continue == 'true'
|
||||
id: pr-info
|
||||
run: |
|
||||
# Get PR number
|
||||
if [[ "${{ github.event_name }}" == "pull_request_target" ]]; then
|
||||
PR_NUMBER="${{ github.event.pull_request.number }}"
|
||||
else
|
||||
PR_NUMBER="${{ github.event.issue.number }}"
|
||||
fi
|
||||
|
||||
echo "PR_NUMBER=$PR_NUMBER" >> $GITHUB_ENV
|
||||
|
||||
# Fetch the PR branch without checking it out
|
||||
git fetch origin pull/$PR_NUMBER/head:pr-branch
|
||||
|
||||
# Create diff file for Claude to analyze
|
||||
git diff ${{ github.event.pull_request.base.ref || github.event.repository.default_branch }}...pr-branch > pr-diff.patch
|
||||
|
||||
echo "Created diff for PR #$PR_NUMBER"
|
||||
echo "Diff size: $(wc -l < pr-diff.patch) lines"
|
||||
|
||||
- name: Run Claude Code Review
|
||||
if: steps.should-continue.outputs.should_continue == 'true'
|
||||
id: claude
|
||||
uses: anthropics/claude-code-action@beta
|
||||
timeout-minutes: 15
|
||||
with:
|
||||
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
|
||||
|
||||
# Trigger phrase for manual reviews
|
||||
trigger_phrase: "@claude-review-fork"
|
||||
|
||||
# Custom instructions for fork review
|
||||
custom_instructions: |
|
||||
You are performing a CODE REVIEW ONLY for a PULL REQUEST FROM A FORK. You cannot make any changes to files.
|
||||
|
||||
## CRITICAL: Fork Security Review
|
||||
This PR is from an external fork. First, check the pr-diff.patch file for any security concerns:
|
||||
- Attempts to access secrets or environment variables
|
||||
- Suspicious network requests or data exfiltration
|
||||
- Hardcoded credentials or API keys
|
||||
- SQL injection or command injection attempts
|
||||
- Malicious file system operations
|
||||
- Changes to GitHub Actions workflows
|
||||
|
||||
## Your Role
|
||||
You are reviewing code for Archon V2 Alpha, a local-first AI knowledge management system in early alpha stage.
|
||||
|
||||
## Architecture Context
|
||||
- Frontend: React + TypeScript + Vite (port 3737)
|
||||
- Backend: FastAPI + Socket.IO + Python (port 8181)
|
||||
- MCP Service: MCP protocol server (port 8051)
|
||||
- Agents Service: PydanticAI agents (port 8052)
|
||||
- Database: Supabase (PostgreSQL + pgvector)
|
||||
|
||||
## Review Process
|
||||
1. **Understand Changes**
|
||||
- Read the pr-diff.patch file to see all changes
|
||||
- Check what files were changed and understand the context
|
||||
- Analyze the impact across all services (frontend, backend, MCP, agents)
|
||||
- Consider interactions between components
|
||||
|
||||
## Review Focus Areas
|
||||
|
||||
### 1. Code Quality - Backend (Python)
|
||||
- Type hints on all functions and classes
|
||||
- Pydantic v2 models for data validation (ConfigDict not class Config, model_dump() not dict())
|
||||
- No print() statements (use logging instead)
|
||||
- Proper error handling with detailed error messages
|
||||
- Following PEP 8
|
||||
- Google style docstrings where appropriate
|
||||
|
||||
### 2. Code Quality - Frontend (React/TypeScript)
|
||||
- Proper TypeScript types (avoid 'any')
|
||||
- React hooks used correctly
|
||||
- Component composition and reusability
|
||||
- Proper error boundaries
|
||||
- Following existing component patterns
|
||||
|
||||
### 3. Structure & Architecture
|
||||
- Each feature self-contained with its own models, service, and tools
|
||||
- Shared components only for things used by multiple features
|
||||
- Proper separation of concerns across services
|
||||
- API endpoints follow RESTful conventions
|
||||
|
||||
### 4. Testing
|
||||
- Unit tests co-located with code in tests/ folders
|
||||
- Edge cases covered
|
||||
- Mocking external dependencies
|
||||
- Frontend: Vitest tests for components
|
||||
- Backend: Pytest tests for services
|
||||
|
||||
### 5. Alpha Project Principles (from CLAUDE.md)
|
||||
- No backwards compatibility needed - can break things
|
||||
- Fail fast with detailed errors (not graceful failures)
|
||||
- Remove dead code immediately
|
||||
- Focus on functionality over production patterns
|
||||
|
||||
## Required Output Format
|
||||
|
||||
## Summary
|
||||
[2-3 sentence overview of what the changes do and their impact]
|
||||
|
||||
## Security Review (Fork PR)
|
||||
⚠️ This is an external PR from a fork. Enhanced security checks performed:
|
||||
- [ ] No unauthorized access to secrets/env vars
|
||||
- [ ] No suspicious network requests
|
||||
- [ ] No hardcoded credentials
|
||||
- [ ] No SQL injection risks
|
||||
- [ ] Safe file system operations
|
||||
- [ ] No malicious workflow changes
|
||||
[List any security concerns found or state "No security issues found"]
|
||||
|
||||
## Previous Review Comments
|
||||
- [If this is a follow-up review, summarize unaddressed comments]
|
||||
- [If first review, state: "First review - no previous comments"]
|
||||
|
||||
## Issues Found
|
||||
Total: [X critical, Y important, Z minor]
|
||||
|
||||
### 🔴 Critical (Must Fix)
|
||||
[Issues that will break functionality or cause data loss]
|
||||
- **[Issue Title]** - `path/to/file.py:123`
|
||||
Problem: [What's wrong]
|
||||
Fix: [Specific solution]
|
||||
|
||||
### 🟡 Important (Should Fix)
|
||||
[Issues that impact user experience or code maintainability]
|
||||
- **[Issue Title]** - `path/to/file.tsx:45`
|
||||
Problem: [What's wrong]
|
||||
Fix: [Specific solution]
|
||||
|
||||
### 🟢 Minor (Consider)
|
||||
[Nice-to-have improvements]
|
||||
- **[Suggestion]** - `path/to/file.py:67`
|
||||
[Brief description and why it would help]
|
||||
|
||||
## Performance Considerations
|
||||
- Database query efficiency (no N+1 queries)
|
||||
- Frontend bundle size impacts
|
||||
- Async/await usage in Python
|
||||
- React re-render optimization
|
||||
[List any performance issues or state "No performance concerns"]
|
||||
|
||||
## Good Practices Observed
|
||||
- [Highlight what was done well]
|
||||
- [Patterns that should be replicated]
|
||||
|
||||
## Questionable Practices
|
||||
- [Design decisions that might need reconsideration]
|
||||
- [Architectural concerns for discussion]
|
||||
|
||||
## Test Coverage
|
||||
**Current Coverage:** [Estimate based on what you see]
|
||||
**Missing Tests:**
|
||||
|
||||
1. **[Component/Function Name]**
|
||||
- What to test: [Specific functionality]
|
||||
- Why important: [Impact if it fails]
|
||||
- Suggested test: [One sentence description]
|
||||
|
||||
## Recommendations
|
||||
|
||||
**Merge Decision:**
|
||||
- [ ] Ready to merge as-is
|
||||
- [ ] Requires fixes before merging
|
||||
- [ ] Needs additional security review
|
||||
|
||||
**Priority Actions:**
|
||||
1. [Most important fix needed, if any]
|
||||
2. [Second priority, if applicable]
|
||||
|
||||
**Fork PR Note:**
|
||||
This PR is from an external fork. Maintainers should manually verify critical changes before merging.
|
||||
|
||||
---
|
||||
*Review based on Archon V2 Alpha guidelines with enhanced security checks for fork PRs*
|
||||
|
||||
unauthorized-comment:
|
||||
# Post message for unauthorized users trying to trigger review
|
||||
if: |
|
||||
github.event_name == 'issue_comment' &&
|
||||
github.event.issue.pull_request &&
|
||||
contains(github.event.comment.body, '@claude-review-fork') &&
|
||||
!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: |
|
||||
await github.rest.issues.createComment({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
issue_number: context.issue.number,
|
||||
body: `❌ @${context.actor} - You are not authorized to trigger Claude reviews.\n\n` +
|
||||
`Only maintainers can trigger Claude: @Wirasm, @coleam00, @sean-eskerium\n\n` +
|
||||
`Please wait for a maintainer to review your PR.`
|
||||
});
|
||||
Reference in New Issue
Block a user