diff --git a/.github/workflows/README.md b/.github/workflows/README.md new file mode 100644 index 00000000..1110c69d --- /dev/null +++ b/.github/workflows/README.md @@ -0,0 +1,175 @@ +# GitHub Actions Workflows + +This directory contains the GitHub Actions workflows for the NOFX project. + +## šŸ“š Documentation Index + +- **[README.md](./README.md)** - This file, overview of all workflows +- **[PERMISSIONS.md](./PERMISSIONS.md)** - Detailed permission analysis and security model +- **[TRIGGERS.md](./TRIGGERS.md)** - Comparison of event triggers (pull_request vs pull_request_target vs workflow_run) +- **[FORK_PR_FLOW.md](./FORK_PR_FLOW.md)** - Complete analysis of what happens when a fork PR is submitted +- **[FLOW_DIAGRAM.md](./FLOW_DIAGRAM.md)** - Visual flow diagrams and quick reference + +## šŸš€ Quick Start + +**Want to understand how fork PRs work?** → Read [FLOW_DIAGRAM.md](./FLOW_DIAGRAM.md) + +**Need security details?** → Read [PERMISSIONS.md](./PERMISSIONS.md) + +**Confused about triggers?** → Read [TRIGGERS.md](./TRIGGERS.md) + +## PR Check Workflows + +We use a **two-workflow pattern** to safely handle PR checks from both internal and fork PRs: + +### 1. `pr-checks-run.yml` - Execute Checks + +**Trigger:** On pull request (opened, synchronize, reopened) + +**Permissions:** Read-only + +**Purpose:** Executes all PR checks with read-only permissions, making it safe for fork PRs. + +**What it does:** +- āœ… Checks PR title format (Conventional Commits) +- āœ… Calculates PR size +- āœ… Runs backend checks (Go formatting, vet, tests) +- āœ… Runs frontend checks (linting, type checking, build) +- āœ… Saves all results as artifacts + +**Security:** Safe for fork PRs because it only has read permissions and cannot access secrets or modify the repository. + +### 2. `pr-checks-comment.yml` - Post Results + +**Trigger:** When `pr-checks-run.yml` completes (workflow_run) + +**Permissions:** Write (pull-requests, issues) + +**Purpose:** Posts check results as PR comments, running in the main repository context. + +**What it does:** +- āœ… Downloads artifacts from `pr-checks-run.yml` +- āœ… Reads check results +- āœ… Posts a comprehensive comment to the PR + +**Security:** Safe because: +- Runs in the main repository context (not fork context) +- Has write permissions but doesn't execute untrusted code +- Only reads pre-generated results from artifacts + +### 3. `pr-checks.yml` - Strict Checks + +**Trigger:** On pull request + +**Permissions:** Read + conditional write + +**Purpose:** Runs mandatory checks that must pass before PR can be merged. + +**What it does:** +- āœ… Validates PR title (blocks merge if invalid) +- āœ… Auto-labels PR based on size and files changed (non-fork only) +- āœ… Runs backend tests (Go) +- āœ… Runs frontend tests (React/TypeScript) +- āœ… Security scanning (Trivy, Gitleaks) + +**Security:** +- Fork PRs: Only runs read-only operations (tests, security scans) +- Non-fork PRs: Can add labels and comments +- Uses `continue-on-error` for operations that may fail on forks + +## Why Two Workflows for PR Checks? + +### The Problem + +When a PR comes from a forked repository: +- GitHub restricts `GITHUB_TOKEN` permissions for security +- Fork PRs cannot write comments, add labels, or access secrets +- This prevents malicious contributors from: + - Stealing repository secrets + - Modifying workflow files to execute malicious code + - Spamming issues/PRs with automated comments + +### The Solution + +**Two-Workflow Pattern:** + +``` +Fork PR Submitted + ↓ +[pr-checks-run.yml] + - Runs with read-only permissions + - Executes all checks safely + - Saves results to artifacts + ↓ +[pr-checks-comment.yml] + - Triggered by workflow_run + - Runs in main repo context (has write permissions) + - Downloads artifacts + - Posts comment with results +``` + +This approach: +- āœ… Allows fork PRs to run checks +- āœ… Safely posts results as comments +- āœ… Prevents security vulnerabilities +- āœ… Follows GitHub's best practices + +### Can workflow_run Comment on Fork PRs? + +**Yes! āœ… The permissions are sufficient.** + +**Key Understanding:** +- `workflow_run` executes in the **base repository** context +- Fork PRs exist in the **base repository** (not in the fork) +- The base repository's `GITHUB_TOKEN` has write permissions +- Therefore, `workflow_run` can comment on fork PRs + +**Security:** +- Fork PR code runs in isolated environment (read-only) +- Comment workflow doesn't execute fork code +- Only reads pre-generated artifact data + +**For detailed permission analysis, see:** [PERMISSIONS.md](./PERMISSIONS.md) + +## Workflow Comparison + +| Workflow | Fork PRs | Write Access | Blocks Merge | Purpose | +|----------|----------|--------------|--------------|---------| +| `pr-checks-run.yml` | āœ… Yes | āŒ No | āŒ No | Advisory checks | +| `pr-checks-comment.yml` | āœ… Yes | āœ… Yes* | āŒ No | Post results | +| `pr-checks.yml` | āœ… Yes | āš ļø Partial | āœ… Yes | Mandatory checks | + +\* Write access only in main repo context, not available to fork PR code + +## File History + +- `pr-checks-advisory.yml.old` - Old advisory workflow that failed on fork PRs (deprecated) +- Now replaced by the two-workflow pattern (`pr-checks-run.yml` + `pr-checks-comment.yml`) + +## Testing the Workflows + +### Test with a Fork PR + +1. Fork the repository +2. Make changes in your fork +3. Create a PR to the main repository +4. Observe: + - `pr-checks-run.yml` runs successfully with read-only access + - `pr-checks-comment.yml` posts results as a comment + - `pr-checks.yml` runs tests but skips labeling + +### Test with a Branch PR + +1. Create a branch in the main repository +2. Make changes +3. Create a PR +4. Observe: + - All workflows run with full permissions + - Labels are added automatically + - Comments are posted + +## References + +- [GitHub Actions: Keeping your GitHub Actions and workflows secure Part 1](https://securitylab.github.com/research/github-actions-preventing-pwn-requests/) +- [Safely posting comments from untrusted workflows](https://securitylab.github.com/research/github-actions-building-blocks/) +- [GitHub Actions: workflow_run trigger](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#workflow_run) diff --git a/.github/workflows/pr-checks-advisory.yml b/.github/workflows/pr-checks-advisory.yml.old similarity index 100% rename from .github/workflows/pr-checks-advisory.yml rename to .github/workflows/pr-checks-advisory.yml.old diff --git a/.github/workflows/pr-checks-comment.yml b/.github/workflows/pr-checks-comment.yml new file mode 100644 index 00000000..f0806026 --- /dev/null +++ b/.github/workflows/pr-checks-comment.yml @@ -0,0 +1,239 @@ +name: PR Checks - Comment + +# This workflow posts PR check results as comments +# Runs in the main repo context with write permissions (SAFE) +# Triggered after pr-checks-run.yml completes + +on: + workflow_run: + workflows: ["PR Checks - Run"] + types: [completed] + +# Write permissions - SAFE because runs in main repo context +# This token has write access to the base repository +# Fork PRs exist in the base repo, so we can comment on them +permissions: + pull-requests: write + issues: write + actions: read # Needed to download artifacts + +jobs: + comment: + name: Post Check Results + runs-on: ubuntu-latest + # Only run if the workflow was triggered by a pull_request event + if: github.event.workflow_run.event == 'pull_request' + steps: + - name: Download artifacts + uses: actions/download-artifact@v4 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + run-id: ${{ github.event.workflow_run.id }} + path: artifacts + + - name: List downloaded artifacts + run: | + echo "=== Checking downloaded artifacts ===" + ls -la artifacts/ || echo "No artifacts directory" + find artifacts/ -type f || echo "No files found" + + - name: Read PR info results + id: pr-info + continue-on-error: true + run: | + if [ -f artifacts/pr-info-results/pr-info.json ]; then + echo "=== PR Info JSON ===" + cat artifacts/pr-info-results/pr-info.json + echo "pr_number=$(jq -r '.pr_number' artifacts/pr-info-results/pr-info.json)" >> $GITHUB_OUTPUT + echo "title_status=$(jq -r '.title_status' artifacts/pr-info-results/pr-info.json)" >> $GITHUB_OUTPUT + echo "title_message=$(jq -r '.title_message' artifacts/pr-info-results/pr-info.json)" >> $GITHUB_OUTPUT + echo "size=$(jq -r '.size' artifacts/pr-info-results/pr-info.json)" >> $GITHUB_OUTPUT + echo "lines=$(jq -r '.lines' artifacts/pr-info-results/pr-info.json)" >> $GITHUB_OUTPUT + echo "size_suggestion=$(jq -r '.size_suggestion' artifacts/pr-info-results/pr-info.json)" >> $GITHUB_OUTPUT + else + echo "pr_number=0" >> $GITHUB_OUTPUT + echo "āš ļø PR info artifact not found" + fi + + - name: Read backend results + id: backend + continue-on-error: true + run: | + if [ -f artifacts/backend-results/backend-results.json ]; then + echo "=== Backend Results JSON ===" + cat artifacts/backend-results/backend-results.json + echo "fmt_status=$(jq -r '.fmt_status' artifacts/backend-results/backend-results.json)" >> $GITHUB_OUTPUT + echo "vet_status=$(jq -r '.vet_status' artifacts/backend-results/backend-results.json)" >> $GITHUB_OUTPUT + echo "test_status=$(jq -r '.test_status' artifacts/backend-results/backend-results.json)" >> $GITHUB_OUTPUT + + # Read output files + if [ -f artifacts/backend-results/fmt-files.txt ]; then + echo "fmt_files<> $GITHUB_OUTPUT + cat artifacts/backend-results/fmt-files.txt >> $GITHUB_OUTPUT + echo "EOF" >> $GITHUB_OUTPUT + fi + if [ -f artifacts/backend-results/vet-output-short.txt ]; then + echo "vet_output<> $GITHUB_OUTPUT + cat artifacts/backend-results/vet-output-short.txt >> $GITHUB_OUTPUT + echo "EOF" >> $GITHUB_OUTPUT + fi + if [ -f artifacts/backend-results/test-output-short.txt ]; then + echo "test_output<> $GITHUB_OUTPUT + cat artifacts/backend-results/test-output-short.txt >> $GITHUB_OUTPUT + echo "EOF" >> $GITHUB_OUTPUT + fi + else + echo "āš ļø Backend results artifact not found" + fi + + - name: Read frontend results + id: frontend + continue-on-error: true + run: | + if [ -f artifacts/frontend-results/frontend-results.json ]; then + echo "=== Frontend Results JSON ===" + cat artifacts/frontend-results/frontend-results.json + echo "lint_status=$(jq -r '.lint_status' artifacts/frontend-results/frontend-results.json)" >> $GITHUB_OUTPUT + echo "typecheck_status=$(jq -r '.typecheck_status' artifacts/frontend-results/frontend-results.json)" >> $GITHUB_OUTPUT + echo "build_status=$(jq -r '.build_status' artifacts/frontend-results/frontend-results.json)" >> $GITHUB_OUTPUT + + # Read output files + if [ -f artifacts/frontend-results/lint-output-short.txt ]; then + echo "lint_output<> $GITHUB_OUTPUT + cat artifacts/frontend-results/lint-output-short.txt >> $GITHUB_OUTPUT + echo "EOF" >> $GITHUB_OUTPUT + fi + if [ -f artifacts/frontend-results/typecheck-output-short.txt ]; then + echo "typecheck_output<> $GITHUB_OUTPUT + cat artifacts/frontend-results/typecheck-output-short.txt >> $GITHUB_OUTPUT + echo "EOF" >> $GITHUB_OUTPUT + fi + if [ -f artifacts/frontend-results/build-output-short.txt ]; then + echo "build_output<> $GITHUB_OUTPUT + cat artifacts/frontend-results/build-output-short.txt >> $GITHUB_OUTPUT + echo "EOF" >> $GITHUB_OUTPUT + fi + else + echo "āš ļø Frontend results artifact not found" + fi + + - name: Post combined comment + if: steps.pr-info.outputs.pr_number != '0' + uses: actions/github-script@v7 + with: + script: | + const prNumber = ${{ steps.pr-info.outputs.pr_number }}; + + // PR Info section + const titleStatus = '${{ steps.pr-info.outputs.title_status }}' || 'āš ļø Unknown'; + const prSize = '${{ steps.pr-info.outputs.size }}' || 'āš ļø Unknown'; + const prLines = '${{ steps.pr-info.outputs.lines }}' || '0'; + const sizeSuggestion = '${{ steps.pr-info.outputs.size_suggestion }}' || ''; + + let comment = '## šŸ¤– PR Checks Results\n\n'; + comment += 'Thank you for your contribution! Here are the automated check results:\n\n'; + + // PR Info + comment += '### šŸ“‹ PR Information\n\n'; + comment += '**Title Format:** ' + titleStatus + '\n'; + comment += '**PR Size:** ' + prSize + ' (' + prLines + ' lines changed)\n'; + if (sizeSuggestion) { + comment += '\nšŸ’” **Suggestion:** ' + sizeSuggestion + '\n'; + } + + // Backend checks + const fmtStatus = '${{ steps.backend.outputs.fmt_status }}'; + const vetStatus = '${{ steps.backend.outputs.vet_status }}'; + const testStatus = '${{ steps.backend.outputs.test_status }}'; + + if (fmtStatus || vetStatus || testStatus) { + comment += '\n### šŸ”§ Backend Checks\n\n'; + + if (fmtStatus) { + comment += '**Go Formatting:** ' + fmtStatus + '\n'; + const fmtFiles = `${{ steps.backend.outputs.fmt_files }}`; + if (fmtFiles && fmtFiles.trim()) { + comment += '
Files needing formatting\n\n```\n' + fmtFiles + '\n```\n
\n\n'; + } + } + + if (vetStatus) { + comment += '**Go Vet:** ' + vetStatus + '\n'; + const vetOutput = `${{ steps.backend.outputs.vet_output }}`; + if (vetOutput && vetOutput.trim()) { + comment += '
Issues found\n\n```\n' + vetOutput.substring(0, 1000) + '\n```\n
\n\n'; + } + } + + if (testStatus) { + comment += '**Tests:** ' + testStatus + '\n'; + const testOutput = `${{ steps.backend.outputs.test_output }}`; + if (testOutput && testOutput.trim()) { + comment += '
Test output\n\n```\n' + testOutput.substring(0, 1000) + '\n```\n
\n\n'; + } + } + + comment += '\n**Fix locally:**\n'; + comment += '```bash\n'; + comment += 'go fmt ./... # Format code\n'; + comment += 'go vet ./... # Check for issues\n'; + comment += 'go test ./... # Run tests\n'; + comment += '```\n'; + } + + // Frontend checks + const lintStatus = '${{ steps.frontend.outputs.lint_status }}'; + const typecheckStatus = '${{ steps.frontend.outputs.typecheck_status }}'; + const buildStatus = '${{ steps.frontend.outputs.build_status }}'; + + if (lintStatus || typecheckStatus || buildStatus) { + comment += '\n### āš›ļø Frontend Checks\n\n'; + + if (lintStatus) { + comment += '**Linting:** ' + lintStatus + '\n'; + const lintOutput = `${{ steps.frontend.outputs.lint_output }}`; + if (lintOutput && lintOutput.trim()) { + comment += '
Issues found\n\n```\n' + lintOutput.substring(0, 500) + '\n```\n
\n\n'; + } + } + + if (typecheckStatus) { + comment += '**Type Checking:** ' + typecheckStatus + '\n'; + const typecheckOutput = `${{ steps.frontend.outputs.typecheck_output }}`; + if (typecheckOutput && typecheckOutput.trim()) { + comment += '
Type errors\n\n```\n' + typecheckOutput.substring(0, 500) + '\n```\n
\n\n'; + } + } + + if (buildStatus) { + comment += '**Build:** ' + buildStatus + '\n'; + const buildOutput = `${{ steps.frontend.outputs.build_output }}`; + if (buildOutput && buildOutput.trim()) { + comment += '
Build output\n\n```\n' + buildOutput.substring(0, 500) + '\n```\n
\n\n'; + } + } + + comment += '\n**Fix locally:**\n'; + comment += '```bash\n'; + comment += 'cd web\n'; + comment += 'npm run lint -- --fix # Fix linting\n'; + comment += 'npm run type-check # Check types\n'; + comment += 'npm run build # Test build\n'; + comment += '```\n'; + } + + comment += '\n---\n\n'; + comment += '### šŸ“– Resources\n\n'; + comment += '- [Contributing Guidelines](https://github.com/tinkle-community/nofx/blob/dev/CONTRIBUTING.md)\n'; + comment += '- [Migration Guide](https://github.com/tinkle-community/nofx/blob/dev/docs/community/MIGRATION_ANNOUNCEMENT.md)\n\n'; + comment += '**Questions?** Feel free to ask in the comments! šŸ™\n\n'; + comment += '---\n\n'; + comment += '*These checks are advisory and won\'t block your PR from being merged. This comment is automatically generated from [pr-checks-run.yml](https://github.com/tinkle-community/nofx/blob/dev/.github/workflows/pr-checks-run.yml).*'; + + // Post comment + await github.rest.issues.createComment({ + issue_number: prNumber, + owner: context.repo.owner, + repo: context.repo.repo, + body: comment + }); diff --git a/.github/workflows/pr-checks-run.yml b/.github/workflows/pr-checks-run.yml new file mode 100644 index 00000000..eacee17b --- /dev/null +++ b/.github/workflows/pr-checks-run.yml @@ -0,0 +1,238 @@ +name: PR Checks - Run + +# This workflow runs all PR checks with read-only permissions +# Safe for fork PRs - results are saved as artifacts +# Companion workflow (pr-checks-comment.yml) will post comments + +on: + pull_request: + types: [opened, synchronize, reopened] + branches: [main, dev] + +# Read-only permissions - safe for fork PRs +permissions: + contents: read + +jobs: + pr-info: + name: PR Information + runs-on: ubuntu-latest + steps: + - name: Check PR title format + id: check-title + run: | + PR_TITLE="${{ github.event.pull_request.title }}" + + # Check if title follows conventional commits + if echo "$PR_TITLE" | grep -qE "^(feat|fix|docs|style|refactor|perf|test|chore|ci|security)(\(.+\))?: .+"; then + echo "status=āœ… Good" >> $GITHUB_OUTPUT + echo "message=PR title follows Conventional Commits format" >> $GITHUB_OUTPUT + else + echo "status=āš ļø Suggestion" >> $GITHUB_OUTPUT + echo "message=Consider using Conventional Commits format: type(scope): description" >> $GITHUB_OUTPUT + fi + + - name: Calculate PR size and save results + id: pr-size + run: | + ADDITIONS=${{ github.event.pull_request.additions }} + DELETIONS=${{ github.event.pull_request.deletions }} + TOTAL=$((ADDITIONS + DELETIONS)) + + if [ $TOTAL -lt 100 ]; then + SIZE="🟢 Small" + SUGGESTION="" + elif [ $TOTAL -lt 500 ]; then + SIZE="🟔 Medium" + SUGGESTION="" + else + SIZE="šŸ”“ Large" + SUGGESTION="Consider breaking this into smaller PRs for easier review" + fi + + # Save all results to JSON file + cat > pr-info.json </dev/null || echo "") + if [ -n "$UNFORMATTED" ]; then + echo "status=āš ļø Needs formatting" >> $GITHUB_OUTPUT + echo "$UNFORMATTED" | head -10 > fmt-files.txt + else + echo "status=āœ… Good" >> $GITHUB_OUTPUT + echo "" > fmt-files.txt + fi + + - name: Run go vet + id: go-vet + continue-on-error: true + run: | + if go vet ./... 2>&1 | tee vet-output.txt; then + echo "status=āœ… Good" >> $GITHUB_OUTPUT + else + echo "status=āš ļø Issues found" >> $GITHUB_OUTPUT + cat vet-output.txt | head -20 > vet-output-short.txt + fi + + - name: Run tests + id: go-test + continue-on-error: true + run: | + if go test ./... -v 2>&1 | tee test-output.txt; then + echo "status=āœ… Passed" >> $GITHUB_OUTPUT + else + echo "status=āš ļø Failed" >> $GITHUB_OUTPUT + cat test-output.txt | tail -30 > test-output-short.txt + fi + + - name: Save backend results + if: always() + run: | + cat > backend-results.json <> $GITHUB_OUTPUT + else + echo "exists=false" >> $GITHUB_OUTPUT + fi + + - name: Install dependencies + if: steps.check-web.outputs.exists == 'true' + working-directory: ./web + continue-on-error: true + run: npm ci + + - name: Run linter + if: steps.check-web.outputs.exists == 'true' + id: lint + working-directory: ./web + continue-on-error: true + run: | + if npm run lint 2>&1 | tee lint-output.txt; then + echo "status=āœ… Good" >> $GITHUB_OUTPUT + else + echo "status=āš ļø Issues found" >> $GITHUB_OUTPUT + cat lint-output.txt | head -20 > lint-output-short.txt + fi + + - name: Type check + if: steps.check-web.outputs.exists == 'true' + id: typecheck + working-directory: ./web + continue-on-error: true + run: | + if npm run type-check 2>&1 | tee typecheck-output.txt; then + echo "status=āœ… Good" >> $GITHUB_OUTPUT + else + echo "status=āš ļø Issues found" >> $GITHUB_OUTPUT + cat typecheck-output.txt | head -20 > typecheck-output-short.txt + fi + + - name: Build + if: steps.check-web.outputs.exists == 'true' + id: build + working-directory: ./web + continue-on-error: true + run: | + if npm run build 2>&1 | tee build-output.txt; then + echo "status=āœ… Success" >> $GITHUB_OUTPUT + else + echo "status=āš ļø Failed" >> $GITHUB_OUTPUT + cat build-output.txt | tail -20 > build-output-short.txt + fi + + - name: Save frontend results + if: always() && steps.check-web.outputs.exists == 'true' + working-directory: ./web + run: | + cat > frontend-results.json <= 1000) { - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: pr.number, - body: comment - }); + // Add comment for large PRs + if (total >= 1000) { + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: pr.number, + body: comment + }); + } + } catch (error) { + console.log('Failed to add label/comment (expected for fork PRs):', error.message); + } + } else { + console.log('Fork PR detected - skipping label/comment (will be handled by pr-checks-comment.yml)'); } # Backend tests @@ -186,6 +201,8 @@ jobs: auto-label: name: Auto Label PR runs-on: ubuntu-latest + # Only run for non-fork PRs (fork PRs don't have write permission) + if: github.event.pull_request.head.repo.full_name == github.repository permissions: contents: read pull-requests: write