diff --git a/.github/workflows/pr-report.yml b/.github/workflows/pr-report.yml new file mode 100644 index 00000000..c5d07683 --- /dev/null +++ b/.github/workflows/pr-report.yml @@ -0,0 +1,198 @@ +name: PR Report + +# Companion to test.yml. A `pull_request` run from a fork gets a read-only +# GITHUB_TOKEN no matter what its `permissions:` block asks for, so labelling and +# commenting from inside that run always fails with +# "Resource not accessible by integration". `workflow_run` runs in the base-repo +# context with a real write token, so the reporting lives here instead. +on: + workflow_run: + workflows: ['Test and Build'] + types: [completed] + +permissions: + issues: write + pull-requests: write + actions: read + +# One report in flight per head branch. Without this, two pushes in quick +# succession race: both runs list the comments, both see no existing report, and +# both create one. The newest run always has the freshest results, so superseding +# an in-flight one loses nothing. +concurrency: + group: pr-report-${{ github.event.workflow_run.head_repository.full_name }}-${{ github.event.workflow_run.head_branch }} + cancel-in-progress: true + +jobs: + report: + name: Label and comment + if: github.event.workflow_run.event == 'pull_request' + runs-on: ubuntu-latest + steps: + # This job holds a write token, so it must never check out or execute the + # PR's code. It only reads text out of the artifact and quotes it. + - name: Download report data + id: download + continue-on-error: true + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4 + with: + name: pr-report + path: pr-report + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ secrets.GITHUB_TOKEN }} + + - name: Label and comment + if: steps.download.outcome == 'success' + uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7 + with: + script: | + const fs = require('fs'); + const { owner, repo } = context.repo; + const run = context.payload.workflow_run; + + // Which PR to write to is decided from the event payload alone, never + // from the artifact. A `pull_request` run executes the fork's own copy + // of test.yml, so every byte of that artifact is attacker-controlled: a + // PR that wrote someone else's number into it could drive labels and + // bot comments onto any issue in this repo. + const findPullRequest = async () => { + // Populated only when head and base are the same repo. + if (run.pull_requests?.length) return run.pull_requests[0].number; + const prs = await github.paginate(github.rest.pulls.list, { + owner, + repo, + state: 'open', + head: `${run.head_repository.owner.login}:${run.head_branch}`, + per_page: 100, + }); + // Prefer the exact commit; a newer push leaves this run's sha behind, + // and the branch match is still the right PR. + return (prs.find((pr) => pr.head.sha === run.head_sha) ?? prs[0])?.number; + }; + + const prNumber = await findPullRequest(); + if (!prNumber) { + core.info( + `No open PR for ${run.head_repository.full_name}:${run.head_branch}; nothing to report.`, + ); + return; + } + + const read = (name) => { + try { + return fs.readFileSync(`pr-report/${name}`, 'utf8'); + } catch { + return ''; + } + }; + + // Advisory only. A fork controls its own copy of test.yml and could + // claim anything here; the authoritative pass/fail signal is the + // required check's own conclusion, which this cannot influence. + const testsFailed = read('tests_failed').trim() === 'true'; + const buildFailed = read('build_failed').trim() === 'true'; + + // Artifact contents come from a fork's build. Fence-breaking is the + // only thing quoting it can do, but neuter it anyway. + const fence = (text, limit) => { + let out = text.replace(/`/g, 'ˋ'); + if (out.length > limit) out = `…(truncated)\n${out.slice(-limit)}`; + return out.trim() || '(no output)'; + }; + + const LABELS = { + 'test case failing': { color: 'd93f0b', description: 'Automated tests are failing on this PR' }, + 'build failed': { color: 'b60205', description: 'The production build is failing on this PR' }, + }; + + const setLabel = async (name, shouldHave) => { + if (shouldHave) { + try { + await github.rest.issues.getLabel({ owner, repo, name }); + } catch { + await github.rest.issues.createLabel({ owner, repo, name, ...LABELS[name] }); + } + await github.rest.issues.addLabels({ + owner, repo, issue_number: prNumber, labels: [name], + }); + } else { + try { + await github.rest.issues.removeLabel({ + owner, repo, issue_number: prNumber, name, + }); + } catch { + // Label was not applied; nothing to remove. + } + } + }; + + await setLabel('test case failing', testsFailed); + await setLabel('build failed', buildFailed); + + const sections = []; + + if (testsFailed) { + sections.push({ + heading: '⚠️ **Tests failed!**', + summary: 'Test Output', + text: read('test_output.log'), + }); + } + + if (buildFailed) { + sections.push({ + heading: '❌ **Build failed!**', + summary: 'Build Output', + text: read('build_output.log'), + }); + } else { + const metrics = read('build_output.log') + .split('\n') + .filter((line) => line.includes('dist/assets/')) + .join('\n'); + sections.push({ + heading: '✅ **Build successful!**', + summary: 'Build Size Metrics', + text: metrics || 'Metrics not found', + }); + } + + // GitHub rejects comment bodies over 65536 characters. Budget is shared + // across sections rather than applied per section, so tests and build + // both failing cannot add up past the cap. + const BODY_LIMIT = 60000; + const OVERHEAD = 600; + const perSection = Math.floor((BODY_LIMIT - OVERHEAD) / sections.length); + + // One comment per PR, edited in place, so a PR with many pushes does + // not accumulate a wall of near-identical reports. + const MARKER = ''; + const runUrl = `${context.serverUrl}/${owner}/${repo}/actions/runs/${run.id}`; + const body = [ + MARKER, + ...sections.map( + (s) => + `${s.heading}\n\n
${s.summary}\n\n\`\`\`\n` + + `${fence(s.text, perSection)}\n\`\`\`\n
`, + ), + `[Workflow run](${runUrl}) · commit ${run.head_sha.slice(0, 7)}`, + ].join('\n\n'); + + const comments = await github.paginate(github.rest.issues.listComments, { + owner, repo, issue_number: prNumber, per_page: 100, + }); + // Must be our own comment: matching the marker anywhere in any comment + // would let a PR author plant it and have their comment overwritten. + const existing = comments.find( + (c) => c.user?.login === 'github-actions[bot]' && c.body?.startsWith(MARKER), + ); + + if (existing) { + await github.rest.issues.updateComment({ + owner, repo, comment_id: existing.id, body, + }); + } else { + await github.rest.issues.createComment({ + owner, repo, issue_number: prNumber, body, + }); + } diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a69fd4ca..6659f65e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -10,25 +10,26 @@ on: paths: - 'frontend/**' +# Read-only on purpose. This workflow runs untrusted fork code, so it must not +# hold a token that can write to the repo. Labels and the build-size comment are +# posted by pr-report.yml, which runs after this one in the base-repo context. permissions: - issues: write - pull-requests: write contents: read jobs: test-and-build: runs-on: ubuntu-latest - + defaults: run: working-directory: ./frontend - + steps: - name: Checkout repository - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - name: Setup Node.js - uses: actions/setup-node@v4 + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: 20 cache: 'npm' @@ -48,56 +49,6 @@ jobs: npm run test:ci > test_output.log 2>&1 || echo "TESTS_FAILED=true" >> $GITHUB_ENV cat test_output.log - - name: Handle Test Failure - if: env.TESTS_FAILED == 'true' && github.event_name == 'pull_request' - uses: actions/github-script@v7 - with: - script: | - const fs = require('fs'); - let testOutput = ''; - try { - testOutput = fs.readFileSync('./frontend/test_output.log', 'utf8'); - if (testOutput.length > 60000) { - testOutput = testOutput.substring(testOutput.length - 60000); - } - } catch (e) { - testOutput = 'Could not read test output.'; - } - - await github.rest.issues.addLabels({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - labels: ['test case failing'] - }); - - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - body: `⚠️ **Tests failed!**\n\n
Test Output\n\n\`\`\`\n${testOutput}\n\`\`\`\n
` - }); - - - name: Fail if tests failed - if: env.TESTS_FAILED == 'true' - run: exit 1 - - - name: Remove test failure label (if passed) - if: env.TESTS_FAILED != 'true' && github.event_name == 'pull_request' - uses: actions/github-script@v7 - with: - script: | - try { - await github.rest.issues.removeLabel({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - name: 'test case failing' - }); - } catch (e) { - // Ignore if label doesn't exist - } - - name: Run build id: build continue-on-error: true @@ -105,40 +56,29 @@ jobs: npm run build > build_output.log 2>&1 || echo "BUILD_FAILED=true" >> $GITHUB_ENV cat build_output.log - - name: Handle Build Failure - if: env.BUILD_FAILED == 'true' && github.event_name == 'pull_request' - uses: actions/github-script@v7 - with: - script: | - await github.rest.issues.addLabels({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - labels: ['build failed'] - }); + # Results only. This job runs the fork's own copy of this file, so nothing + # written here is trustworthy; pr-report.yml resolves the target PR from the + # workflow_run payload rather than from anything in this artifact. + - name: Collect report data + if: always() && github.event_name == 'pull_request' + run: | + mkdir -p pr-report + echo "${TESTS_FAILED:-false}" > pr-report/tests_failed + echo "${BUILD_FAILED:-false}" > pr-report/build_failed + cp test_output.log pr-report/ 2>/dev/null || true + cp build_output.log pr-report/ 2>/dev/null || true - - name: Report Build Size - if: env.BUILD_FAILED != 'true' && github.event_name == 'pull_request' - uses: actions/github-script@v7 + - name: Upload report data + if: always() && github.event_name == 'pull_request' + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 with: - script: | - const fs = require('fs'); - let buildOutput = ''; - try { - buildOutput = fs.readFileSync('./frontend/build_output.log', 'utf8'); - const lines = buildOutput.split('\n'); - const metrics = lines.filter(line => line.includes('dist/assets/')).join('\n'); - buildOutput = metrics || 'Metrics not found'; - } catch (e) { - buildOutput = 'Could not read build output.'; - } - - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - body: `✅ **Build successful!**\n\n
Build Size Metrics\n\n\`\`\`\n${buildOutput}\n\`\`\`\n
` - }); + name: pr-report + path: frontend/pr-report + retention-days: 1 + + - name: Fail if tests failed + if: env.TESTS_FAILED == 'true' + run: exit 1 - name: Fail if build failed if: env.BUILD_FAILED == 'true'