Fix security vulnerability in workflows (#1804)
* Fix a security hole in #1787 found by Arun Murugesan: "The workflow .github/workflows/eslint-check.yml contained a critical "pwn request" vulnerability that allows any GitHub user to execute arbitrary code with access to repository secrets by opening a pull request." See https://github.com/preactjs/compressed-size-action/issues/54 for why that action shouldn't be used with pull_request_target This change in this PR drops compressed-size-action in favour of executing the steps ourselves in two workflows, one which produces the size artifact, and the other which reads the artifact has the permissions to write the message back to the original PR (which is in a third party repo) * The annotate action also needed pull-requests: write permission (fixes failing run 'ESLint Annotation') * ci(bundle-size): extract bundle size scripts and simplify workflow - Add `.github/scripts/measure-bundle-sizes.js` and `render-bundle-size-comment.js` to replace inline node scripts embedded in workflow YAML, improving readability and reusability - Refactor `eslint-check.yml` to use the new script files and fix checkout steps to handle both PR and non-PR triggers correctly - Refactor `pr-checks-privileged.yml` to replace the large `github-script` block with `render-bundle-size-comment.js` and the `marocchino/sticky-pull-request-comment` action; remove the now-unnecessary `pr_number.txt` artifact by reading the PR number directly from the workflow_run event - Pin `ataylorme/eslint-annotate-action` to a specific commit SHA - Add `actions: read` permission where needed for artifact downloads * ci: add fork PR support and harden workflow - Look up PR number via API when workflow_run.pull_requests is empty (GitHub leaves it empty for fork PRs), falling back gracefully - Use head SHA instead of branch name for PR checkout to avoid TOCTOU - Fix formatSignedSize to produce +0 instead of -0 for zero values - Gate comment steps on successful PR number lookup Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Eoghan Murray <eoghan@getthere.ie> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
52
.github/scripts/measure-bundle-sizes.js
vendored
Normal file
52
.github/scripts/measure-bundle-sizes.js
vendored
Normal file
@@ -0,0 +1,52 @@
|
|||||||
|
const fs = require('fs');
|
||||||
|
const path = require('path');
|
||||||
|
|
||||||
|
const outputPath = process.argv[2];
|
||||||
|
|
||||||
|
if (!outputPath) {
|
||||||
|
console.error(
|
||||||
|
'Usage: node .github/scripts/measure-bundle-sizes.js <output-path>',
|
||||||
|
);
|
||||||
|
process.exit(1);
|
||||||
|
}
|
||||||
|
|
||||||
|
const rootDir = process.cwd();
|
||||||
|
const sizes = {};
|
||||||
|
|
||||||
|
function normalizePath(filePath) {
|
||||||
|
return path.relative(rootDir, filePath).replace(/\\/g, '/');
|
||||||
|
}
|
||||||
|
|
||||||
|
function shouldTrack(filePath) {
|
||||||
|
const normalizedPath = normalizePath(filePath);
|
||||||
|
|
||||||
|
if (normalizedPath.startsWith('packages/packer/dist/')) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
return /(^|\/)dist\/[^/]+\.(js|cjs|mjs|css)$/.test(normalizedPath);
|
||||||
|
}
|
||||||
|
|
||||||
|
function walk(dirPath) {
|
||||||
|
for (const entry of fs.readdirSync(dirPath, { withFileTypes: true })) {
|
||||||
|
if (entry.name === 'node_modules') {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
const entryPath = path.join(dirPath, entry.name);
|
||||||
|
|
||||||
|
if (entry.isDirectory()) {
|
||||||
|
walk(entryPath);
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!shouldTrack(entryPath)) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
sizes[normalizePath(entryPath)] = fs.statSync(entryPath).size;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
walk(rootDir);
|
||||||
|
fs.writeFileSync(outputPath, `${JSON.stringify(sizes, null, 2)}\n`);
|
||||||
155
.github/scripts/render-bundle-size-comment.js
vendored
Normal file
155
.github/scripts/render-bundle-size-comment.js
vendored
Normal file
@@ -0,0 +1,155 @@
|
|||||||
|
const fs = require('fs');
|
||||||
|
|
||||||
|
const prPath = process.argv[2];
|
||||||
|
const basePath = process.argv[3];
|
||||||
|
|
||||||
|
if (!prPath || !basePath) {
|
||||||
|
console.error(
|
||||||
|
'Usage: node .github/scripts/render-bundle-size-comment.js <pr-sizes.json> <base-sizes.json>',
|
||||||
|
);
|
||||||
|
process.exit(1);
|
||||||
|
}
|
||||||
|
|
||||||
|
function readJson(filePath) {
|
||||||
|
return JSON.parse(fs.readFileSync(filePath, 'utf8'));
|
||||||
|
}
|
||||||
|
|
||||||
|
function formatSize(bytes) {
|
||||||
|
if (bytes == null) {
|
||||||
|
return '-';
|
||||||
|
}
|
||||||
|
|
||||||
|
if (Math.abs(bytes) < 1024) {
|
||||||
|
return `${bytes} B`;
|
||||||
|
}
|
||||||
|
|
||||||
|
return `${(bytes / 1024).toFixed(2)} kB`;
|
||||||
|
}
|
||||||
|
|
||||||
|
function formatSignedSize(bytes) {
|
||||||
|
const absoluteBytes = Math.abs(bytes);
|
||||||
|
|
||||||
|
if (absoluteBytes < 1024) {
|
||||||
|
return `${bytes >= 0 ? '+' : '-'}${absoluteBytes} B`;
|
||||||
|
}
|
||||||
|
|
||||||
|
return `${bytes >= 0 ? '+' : '-'}${(absoluteBytes / 1024).toFixed(2)} kB`;
|
||||||
|
}
|
||||||
|
|
||||||
|
function formatDiff(diff, baseValue) {
|
||||||
|
if (diff === 0) {
|
||||||
|
return '-';
|
||||||
|
}
|
||||||
|
|
||||||
|
const percentage =
|
||||||
|
baseValue > 0
|
||||||
|
? ` (${diff > 0 ? '+' : ''}${((diff / baseValue) * 100).toFixed(2)}%)`
|
||||||
|
: '';
|
||||||
|
|
||||||
|
return `${formatSignedSize(diff)}${percentage}`;
|
||||||
|
}
|
||||||
|
|
||||||
|
function getPackageName(filePath) {
|
||||||
|
const match = filePath.match(/^packages\/([^/]+)\//);
|
||||||
|
return match ? match[1] : '(root)';
|
||||||
|
}
|
||||||
|
|
||||||
|
function getFileLabel(filePath, packageName) {
|
||||||
|
const packagePrefix = `packages/${packageName}/dist/`;
|
||||||
|
|
||||||
|
if (packageName !== '(root)' && filePath.startsWith(packagePrefix)) {
|
||||||
|
return filePath.slice(packagePrefix.length);
|
||||||
|
}
|
||||||
|
|
||||||
|
return filePath;
|
||||||
|
}
|
||||||
|
|
||||||
|
const prSizes = readJson(prPath);
|
||||||
|
const baseSizes = readJson(basePath);
|
||||||
|
|
||||||
|
const allFiles = [
|
||||||
|
...new Set([...Object.keys(prSizes), ...Object.keys(baseSizes)]),
|
||||||
|
].sort();
|
||||||
|
const changedFiles = allFiles.filter(
|
||||||
|
(filePath) => prSizes[filePath] !== baseSizes[filePath],
|
||||||
|
);
|
||||||
|
|
||||||
|
if (changedFiles.length === 0) {
|
||||||
|
process.stdout.write('## Bundle Size Changes\n\nNo bundle size changes.\n');
|
||||||
|
process.exit(0);
|
||||||
|
}
|
||||||
|
|
||||||
|
const totalPrSize = allFiles.reduce(
|
||||||
|
(sum, filePath) => sum + (prSizes[filePath] ?? 0),
|
||||||
|
0,
|
||||||
|
);
|
||||||
|
const totalBaseSize = allFiles.reduce(
|
||||||
|
(sum, filePath) => sum + (baseSizes[filePath] ?? 0),
|
||||||
|
0,
|
||||||
|
);
|
||||||
|
const totalDiff = totalPrSize - totalBaseSize;
|
||||||
|
|
||||||
|
const filesByPackage = new Map();
|
||||||
|
|
||||||
|
for (const filePath of changedFiles) {
|
||||||
|
const packageName = getPackageName(filePath);
|
||||||
|
const files = filesByPackage.get(packageName) ?? [];
|
||||||
|
files.push(filePath);
|
||||||
|
filesByPackage.set(packageName, files);
|
||||||
|
}
|
||||||
|
|
||||||
|
const sections = [...filesByPackage.entries()]
|
||||||
|
.sort(([left], [right]) => left.localeCompare(right))
|
||||||
|
.map(([packageName, files]) => {
|
||||||
|
const packagePrSize = files.reduce(
|
||||||
|
(sum, filePath) => sum + (prSizes[filePath] ?? 0),
|
||||||
|
0,
|
||||||
|
);
|
||||||
|
const packageBaseSize = files.reduce(
|
||||||
|
(sum, filePath) => sum + (baseSizes[filePath] ?? 0),
|
||||||
|
0,
|
||||||
|
);
|
||||||
|
const packageDiff = packagePrSize - packageBaseSize;
|
||||||
|
|
||||||
|
const rows = files
|
||||||
|
.map((filePath) => {
|
||||||
|
const fileDiff = (prSizes[filePath] ?? 0) - (baseSizes[filePath] ?? 0);
|
||||||
|
return `| \`${getFileLabel(filePath, packageName)}\` | ${formatSize(
|
||||||
|
baseSizes[filePath],
|
||||||
|
)} | ${formatSize(prSizes[filePath])} | ${formatDiff(
|
||||||
|
fileDiff,
|
||||||
|
baseSizes[filePath] ?? 0,
|
||||||
|
)} |`;
|
||||||
|
})
|
||||||
|
.join('\n');
|
||||||
|
|
||||||
|
return [
|
||||||
|
'<details>',
|
||||||
|
`<summary>\`${packageName}\` - ${formatSize(
|
||||||
|
packageBaseSize,
|
||||||
|
)} -> ${formatSize(packagePrSize)} (${formatDiff(
|
||||||
|
packageDiff,
|
||||||
|
packageBaseSize,
|
||||||
|
)})</summary>`,
|
||||||
|
'',
|
||||||
|
'| File | Base | PR | Diff |',
|
||||||
|
'|------|------|----|------|',
|
||||||
|
rows,
|
||||||
|
'',
|
||||||
|
'</details>',
|
||||||
|
].join('\n');
|
||||||
|
});
|
||||||
|
|
||||||
|
const body = [
|
||||||
|
'## Bundle Size Changes',
|
||||||
|
'',
|
||||||
|
`**Size change:** ${formatDiff(
|
||||||
|
totalDiff,
|
||||||
|
totalBaseSize,
|
||||||
|
)} | **Total size:** ${formatSize(totalPrSize)}`,
|
||||||
|
'',
|
||||||
|
sections.join('\n\n'),
|
||||||
|
'',
|
||||||
|
].join('\n');
|
||||||
|
|
||||||
|
process.stdout.write(body);
|
||||||
88
.github/workflows/eslint-check.yml
vendored
88
.github/workflows/eslint-check.yml
vendored
@@ -2,7 +2,7 @@ name: ESLint Check
|
|||||||
|
|
||||||
on:
|
on:
|
||||||
push:
|
push:
|
||||||
pull_request_target:
|
pull_request:
|
||||||
|
|
||||||
jobs:
|
jobs:
|
||||||
eslint_check_upload:
|
eslint_check_upload:
|
||||||
@@ -12,10 +12,15 @@ jobs:
|
|||||||
name: ESLint Check and Report Upload
|
name: ESLint Check and Report Upload
|
||||||
|
|
||||||
steps:
|
steps:
|
||||||
- uses: actions/checkout@v4
|
- name: Checkout pull request head
|
||||||
|
if: github.event_name == 'pull_request'
|
||||||
|
uses: actions/checkout@v4
|
||||||
with:
|
with:
|
||||||
repository: ${{ github.event.pull_request.head.repo.full_name }}
|
repository: ${{ github.event.pull_request.head.repo.full_name }}
|
||||||
ref: ${{ github.head_ref }}
|
ref: ${{ github.event.pull_request.head.sha }}
|
||||||
|
- name: Checkout current branch
|
||||||
|
if: github.event_name != 'pull_request'
|
||||||
|
uses: actions/checkout@v4
|
||||||
- name: Setup Node
|
- name: Setup Node
|
||||||
uses: actions/setup-node@v3
|
uses: actions/setup-node@v3
|
||||||
with:
|
with:
|
||||||
@@ -38,49 +43,60 @@ jobs:
|
|||||||
with:
|
with:
|
||||||
name: eslint_report.json
|
name: eslint_report.json
|
||||||
path: eslint_report.json
|
path: eslint_report.json
|
||||||
|
- name: Measure PR bundle sizes
|
||||||
|
if: github.event_name == 'pull_request'
|
||||||
|
run: node .github/scripts/measure-bundle-sizes.js pr-sizes.json
|
||||||
|
- name: Upload PR bundle sizes
|
||||||
|
if: github.event_name == 'pull_request'
|
||||||
|
uses: actions/upload-artifact@v4
|
||||||
|
with:
|
||||||
|
name: pr-sizes
|
||||||
|
path: pr-sizes.json
|
||||||
|
|
||||||
annotation:
|
bundle_size_build:
|
||||||
# Skip the annotation action in push events
|
# Only runs on PRs. Reuses the PR build from eslint_check_upload (via the
|
||||||
if: github.event_name == 'pull_request_target'
|
# pr-sizes artifact) and only builds the base branch itself. The privileged
|
||||||
permissions:
|
# bundle-size-comment workflow then posts the PR comment without ever
|
||||||
checks: write
|
# executing fork code.
|
||||||
|
if: github.event_name == 'pull_request'
|
||||||
needs: eslint_check_upload
|
needs: eslint_check_upload
|
||||||
runs-on: ubuntu-latest
|
runs-on: ubuntu-latest
|
||||||
name: ESLint Annotation
|
|
||||||
steps:
|
|
||||||
- uses: actions/download-artifact@v4
|
|
||||||
with:
|
|
||||||
name: eslint_report.json
|
|
||||||
- name: Annotate Code Linting Results
|
|
||||||
uses: ataylorme/eslint-annotate-action@v2
|
|
||||||
with:
|
|
||||||
repo-token: '${{ secrets.GITHUB_TOKEN }}'
|
|
||||||
report-json: 'eslint_report.json'
|
|
||||||
|
|
||||||
bundle_size:
|
|
||||||
# Only runs on PRs (needs a base branch to compare against)
|
|
||||||
if: github.event_name == 'pull_request_target'
|
|
||||||
runs-on: ubuntu-latest
|
|
||||||
permissions:
|
permissions:
|
||||||
|
actions: read
|
||||||
contents: read
|
contents: read
|
||||||
pull-requests: write
|
name: Build Base for Bundle Size Comparison
|
||||||
name: Check Bundle Sizes
|
|
||||||
steps:
|
steps:
|
||||||
|
- name: Checkout workflow ref
|
||||||
|
uses: actions/checkout@v4
|
||||||
|
- name: Prepare bundle size helper
|
||||||
|
run: |
|
||||||
|
cp .github/scripts/measure-bundle-sizes.js /tmp/measure-bundle-sizes.js
|
||||||
|
# --- Base branch ---
|
||||||
- uses: actions/checkout@v4
|
- uses: actions/checkout@v4
|
||||||
with:
|
with:
|
||||||
repository: ${{ github.event.pull_request.head.repo.full_name }}
|
ref: ${{ github.base_ref }}
|
||||||
ref: ${{ github.head_ref }}
|
- name: Download PR bundle sizes
|
||||||
- name: Setup Node
|
uses: actions/download-artifact@v4
|
||||||
uses: actions/setup-node@v3
|
with:
|
||||||
|
name: pr-sizes
|
||||||
|
- uses: actions/setup-node@v3
|
||||||
with:
|
with:
|
||||||
node-version: lts/*
|
node-version: lts/*
|
||||||
cache: 'yarn'
|
cache: 'yarn'
|
||||||
- name: Check bundle sizes
|
- name: Install base dependencies
|
||||||
uses: preactjs/compressed-size-action@v2
|
run: yarn install --frozen-lockfile
|
||||||
with:
|
|
||||||
install-script: 'yarn install --frozen-lockfile'
|
|
||||||
build-script: 'build:all'
|
|
||||||
compression: 'none'
|
|
||||||
pattern: '**/dist/*.{js,cjs,mjs,css}'
|
|
||||||
env:
|
env:
|
||||||
PUPPETEER_SKIP_DOWNLOAD: true
|
PUPPETEER_SKIP_DOWNLOAD: true
|
||||||
|
- name: Build base branch
|
||||||
|
run: NODE_OPTIONS='--max-old-space-size=4096' yarn build:all
|
||||||
|
env:
|
||||||
|
PUPPETEER_SKIP_DOWNLOAD: true
|
||||||
|
- name: Measure base bundle sizes
|
||||||
|
run: node /tmp/measure-bundle-sizes.js base-sizes.json
|
||||||
|
|
||||||
|
- uses: actions/upload-artifact@v4
|
||||||
|
with:
|
||||||
|
name: bundle-size-data
|
||||||
|
path: |
|
||||||
|
pr-sizes.json
|
||||||
|
base-sizes.json
|
||||||
|
|||||||
85
.github/workflows/pr-checks-privileged.yml
vendored
Normal file
85
.github/workflows/pr-checks-privileged.yml
vendored
Normal file
@@ -0,0 +1,85 @@
|
|||||||
|
name: PR Checks (privileged)
|
||||||
|
|
||||||
|
# Runs in the base-repo context (privileged) after eslint-check.yml completes.
|
||||||
|
# Downloads pre-built artifacts and posts PR comments/annotations.
|
||||||
|
# Never checks out or executes fork code.
|
||||||
|
on:
|
||||||
|
workflow_run:
|
||||||
|
workflows: ['ESLint Check']
|
||||||
|
types: [completed]
|
||||||
|
|
||||||
|
jobs:
|
||||||
|
comment:
|
||||||
|
if: >
|
||||||
|
github.event.workflow_run.event == 'pull_request' &&
|
||||||
|
github.event.workflow_run.conclusion == 'success'
|
||||||
|
runs-on: ubuntu-latest
|
||||||
|
permissions:
|
||||||
|
actions: read
|
||||||
|
contents: read
|
||||||
|
pull-requests: write
|
||||||
|
steps:
|
||||||
|
- name: Checkout trusted workflow helpers
|
||||||
|
uses: actions/checkout@v4
|
||||||
|
with:
|
||||||
|
ref: ${{ github.event.repository.default_branch }}
|
||||||
|
- uses: actions/download-artifact@v4
|
||||||
|
with:
|
||||||
|
name: bundle-size-data
|
||||||
|
github-token: ${{ secrets.GITHUB_TOKEN }}
|
||||||
|
run-id: ${{ github.event.workflow_run.id }}
|
||||||
|
- name: Find PR number
|
||||||
|
id: find-pr
|
||||||
|
uses: actions/github-script@v7
|
||||||
|
with:
|
||||||
|
script: |
|
||||||
|
const run = context.payload.workflow_run;
|
||||||
|
if (run.pull_requests && run.pull_requests.length > 0) {
|
||||||
|
return run.pull_requests[0].number;
|
||||||
|
}
|
||||||
|
// Fallback for fork PRs (pull_requests is empty for forks)
|
||||||
|
const { data: prs } = await github.rest.pulls.list({
|
||||||
|
owner: context.repo.owner,
|
||||||
|
repo: context.repo.repo,
|
||||||
|
head: `${run.head_repository.full_name}:${run.head_branch}`,
|
||||||
|
state: 'open',
|
||||||
|
});
|
||||||
|
if (prs.length === 0) {
|
||||||
|
core.setFailed('Could not determine PR number');
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
return prs[0].number;
|
||||||
|
result-encoding: string
|
||||||
|
|
||||||
|
- name: Render bundle size comment
|
||||||
|
if: steps.find-pr.outputs.result
|
||||||
|
run: |
|
||||||
|
node .github/scripts/render-bundle-size-comment.js pr-sizes.json base-sizes.json > bundle-size-comment.md
|
||||||
|
|
||||||
|
- name: Post bundle size comment
|
||||||
|
if: steps.find-pr.outputs.result
|
||||||
|
uses: marocchino/sticky-pull-request-comment@773744901bac0e8cbb5a0dc842800d45e9b2b405
|
||||||
|
with:
|
||||||
|
header: bundle-size
|
||||||
|
path: bundle-size-comment.md
|
||||||
|
number: ${{ steps.find-pr.outputs.result }}
|
||||||
|
|
||||||
|
annotate:
|
||||||
|
if: >
|
||||||
|
github.event.workflow_run.event == 'pull_request' &&
|
||||||
|
github.event.workflow_run.conclusion == 'success'
|
||||||
|
runs-on: ubuntu-latest
|
||||||
|
permissions:
|
||||||
|
actions: read
|
||||||
|
checks: write
|
||||||
|
steps:
|
||||||
|
- uses: actions/download-artifact@v4
|
||||||
|
with:
|
||||||
|
name: eslint_report.json
|
||||||
|
github-token: ${{ secrets.GITHUB_TOKEN }}
|
||||||
|
run-id: ${{ github.event.workflow_run.id }}
|
||||||
|
- name: Annotate Code Linting Results
|
||||||
|
uses: ataylorme/eslint-annotate-action@5f4dc2e3af8d3c21b727edb597e5503510b1dc9c
|
||||||
|
with:
|
||||||
|
repo-token: '${{ secrets.GITHUB_TOKEN }}'
|
||||||
|
report-json: 'eslint_report.json'
|
||||||
Reference in New Issue
Block a user