Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Review routing for CI-critical paths. Requires an approval from one of the
# listed owners when a PR touches a matching path (enforced via branch
# protection's "require review from Code Owners" on main).
#
# Notes on how GitHub evaluates this file:
# - gitignore-style patterns; the LAST matching pattern wins.
# - Owners must have write access or the rule silently does not bind.
# - Any single listed owner's approval satisfies the rule; an author's own
# approval never counts, so keep at least two owners per rule.
# - Only the copy of this file on the PR's base branch is enforced.
#
# No default (catch-all) owner: paths not listed here keep the normal review
# flow.

/.github/ @jbocce @sedghi @wayfarer3130
/tools/ci/ @jbocce @sedghi @wayfarer3130
358 changes: 358 additions & 0 deletions .github/workflows/bench.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,358 @@
name: Bench

# CodSpeed simulation bench (the blocking perf regression gate), split out of
# pr-checks.yml so the workflow that talks to the self-hosted bench runner is
# small and changes rarely. It reuses the dist artifacts built by the
# "PR checks" run for the same commit (the `wait` step below), so nothing is
# compiled twice.
#
# Two jobs:
# gate — hosted VM, metadata only (never checks out or executes PR code).
# Decides whether the bench should run and what to bench, then waits
# for the PR checks build artifacts.
# codspeed-bench — the bench itself on the self-hosted box (see the comments
# on the job).
#
# PRs that modify CI-defining files (workflows, tools/ci/, root manifests) are
# only benched pre-merge when the author has write access; for other authors
# the bench is deferred until the change is reviewed and merged, and the merge
# push benches it on main. Package-only PRs are always benched.

on:
pull_request:
push:
# main only — each merge seeds a fresh CodSpeed baseline. Same reasoning
# as pr-checks.yml (see the comment there).
branches:
- main
# Lets CodSpeed trigger a backtest run from the dashboard. The artifact
# wait below resolves the dist artifacts from the latest PR checks run for
# the same commit (every main commit has one, from its push run).
workflow_dispatch:

# Cancel in-flight benches when a new push lands on the same PR / branch.
# Killing a bench mid-run is safe for the shared-box mutex: flock releases
# with the process (see tools/ci/with-nashua-lock.sh).
concurrency:
group: bench-${{ github.head_ref || github.ref }}
cancel-in-progress: true

permissions:
contents: read

jobs:
gate:
runs-on: ubuntu-latest
# Generous because this job WAITS for the PR checks wasm builds to finish
# (poll deadline 90 min below) before handing over to the bench.
timeout-minutes: 100
permissions:
contents: read
actions: read # poll the PR checks run / jobs
pull-requests: read # list the PR's changed files
outputs:
proceed: ${{ steps.decide.outputs.proceed }}
bench: ${{ steps.decide.outputs.bench }}
ready: ${{ steps.wait.outputs.ready }}
run_id: ${{ steps.wait.outputs.run_id }}
env:
GH_TOKEN: ${{ github.token }}
steps:
- id: decide
name: Decide bench scope
# Metadata only, deliberately no checkout. Computes the bench scope
# from the PR's changed-file list (API) the same way detect-changes
# in pr-checks.yml does from the git diff — keep the two path lists
# in sync. Baseline runs (push to main / dispatch) bench everything.
env:
EVENT_NAME: ${{ github.event_name }}
PR_NUMBER: ${{ github.event.pull_request.number }}
# Relationship of the PR author to this repo, per GitHub
# (OWNER / MEMBER / COLLABORATOR / CONTRIBUTOR / ...).
AUTHOR_ASSOCIATION: ${{ github.event.pull_request.author_association }}
run: |
set -euo pipefail
ALL=(charls libjpeg-turbo-8bit libjpeg-turbo-12bit openjpeg openjphjs little-endian big-endian dicom-codec)
ALL_JSON=$(printf '%s\n' "${ALL[@]}" | jq -R . | jq -s -c .)

if [ "$EVENT_NAME" != "pull_request" ]; then
echo "Baseline run ($EVENT_NAME): benching all packages"
echo "proceed=true" >> "$GITHUB_OUTPUT"
echo "bench=$ALL_JSON" >> "$GITHUB_OUTPUT"
exit 0
fi

files=$(gh api --paginate "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files?per_page=100" --jq '.[].filename')
count=$(wc -l <<<"$files")

# CI-defining paths: the workflows themselves, the scripts they exec
# on the bench runner, and the root manifests that steer install/
# bench orchestration there.
ci_touched=false
# Toolchain paths force a full bench sweep — same list as
# detect-changes in pr-checks.yml.
toolchain_touched=false
changed=()
while IFS= read -r f; do
[ -n "$f" ] || continue
case "$f" in
.github/*|tools/ci/*|package.json|yarn.lock|vitest.workspace.mjs|babel.config.json|lerna.json)
ci_touched=true ;;
esac
case "$f" in
.github/workflows/*|package.json|yarn.lock|vitest.workspace.mjs|babel.config.json|lerna.json|tools/ci/*|tools/dist-size/*|tools/browser-smoke/*|tools/fixture-verification/*)
toolchain_touched=true ;;
packages/*)
pkg=${f#packages/}; pkg=${pkg%%/*}
for known in "${ALL[@]}"; do
[ "$pkg" = "$known" ] && changed+=("$pkg") && break
done ;;
esac
done <<<"$files"
# The list endpoint caps at 3000 files; past that we cannot see
# every path, so treat the PR as CI-touching (full-sweep / defer).
if [ "$count" -ge 3000 ]; then ci_touched=true; toolchain_touched=true; fi

writer=false
case "$AUTHOR_ASSOCIATION" in
OWNER|MEMBER|COLLABORATOR) writer=true ;;
esac

if [ "$ci_touched" = true ] && [ "$writer" = false ]; then
echo "::notice::Bench deferred: this PR changes CI-defining files and the author does not have write access. It will be benched on main once the change is reviewed and merged."
echo "proceed=false" >> "$GITHUB_OUTPUT"
echo "bench=[]" >> "$GITHUB_OUTPUT"
exit 0
fi

if [ "$toolchain_touched" = true ]; then
echo "Toolchain change: benching all packages"
echo "proceed=true" >> "$GITHUB_OUTPUT"
echo "bench=$ALL_JSON" >> "$GITHUB_OUTPUT"
elif [ ${#changed[@]} -gt 0 ]; then
bench_json=$(printf '%s\n' "${changed[@]}" | sort -u | jq -R . | jq -s -c .)
echo "Benching changed packages: $bench_json"
echo "proceed=true" >> "$GITHUB_OUTPUT"
echo "bench=$bench_json" >> "$GITHUB_OUTPUT"
else
echo "No package changes to bench."
echo "proceed=false" >> "$GITHUB_OUTPUT"
echo "bench=[]" >> "$GITHUB_OUTPUT"
fi
- id: wait
name: Wait for PR checks build artifacts
if: steps.decide.outputs.proceed == 'true'
# The dists are built by the "PR checks" run for this same commit
# (bench.yml cannot `needs:` across workflow files). Poll that run
# until every build job has finished, then hand its run id to the
# bench job for the cross-run artifact download.
env:
HEAD_SHA: ${{ github.event.pull_request.head.sha || github.sha }}
run: |
set -euo pipefail
deadline=$((SECONDS + 5400)) # 90 min: wasm builds are the slow part
run_id=""
ready=false
while :; do
if [ -z "$run_id" ]; then
run_id=$(gh api "repos/$GITHUB_REPOSITORY/actions/workflows/pr-checks.yml/runs?head_sha=$HEAD_SHA&per_page=1" --jq '.workflow_runs[0].id // empty')
[ -n "$run_id" ] && echo "PR checks run: $run_id"
fi
if [ -n "$run_id" ]; then
jobs=$(gh api --paginate "repos/$GITHUB_REPOSITORY/actions/runs/$run_id/jobs?per_page=100" --jq '[.jobs[] | select(.name | startswith("build (")) | {status, conclusion}]' | jq -s -c 'add // []')
total=$(jq 'length' <<<"$jobs")
completed=$(jq '[.[] | select(.status == "completed")] | length' <<<"$jobs")
failed=$(jq '[.[] | select(.status == "completed" and .conclusion != "success")] | length' <<<"$jobs")
if [ "$total" -gt 0 ] && [ "$completed" -eq "$total" ]; then
if [ "$failed" -gt 0 ]; then
echo "::notice::Bench skipped: $failed build job(s) in the PR checks run did not succeed."
else
ready=true
fi
break
fi
# A completed run with no build jobs means detect-changes found
# nothing to build (e.g. docs-only) — nothing to bench either.
status=$(gh api "repos/$GITHUB_REPOSITORY/actions/runs/$run_id" --jq '.status')
if [ "$status" = "completed" ] && [ "$total" -eq 0 ]; then
echo "::notice::Bench skipped: the PR checks run built nothing."
break
fi
echo "Waiting on build jobs: $completed/$total completed"
else
echo "Waiting for the PR checks run to appear for $HEAD_SHA"
fi
if [ $SECONDS -ge $deadline ]; then
echo "::error::Timed out waiting for PR checks build artifacts"
exit 1
fi
sleep 30
done
echo "ready=$ready" >> "$GITHUB_OUTPUT"
echo "run_id=$run_id" >> "$GITHUB_OUTPUT"

codspeed-bench:
needs: gate
if: needs.gate.outputs.proceed == 'true' && needs.gate.outputs.ready == 'true'
# Runs on a self-hosted runner dedicated to this repo, on fixed hardware
# (label `codspeed-bench`). GitHub's shared runners randomly assign different
# physical CPUs (Intel Xeon 8370C vs AMD EPYC 7763); simulation-mode
# instruction counts are derived from the runner CPU's cache model, so a
# baseline (main) and a PR landing on different CPUs produce spurious
# "Different runtime environments detected" deltas. One fixed box keeps
# every run on identical hardware, so Simulation is stable run-to-run.
# That box ("nashua") is SHARED with the cornerstone3D and OHIF Playwright
# runners — three runner processes on one machine, one per repo, each free
# to start a job whenever its repo has work. GitHub's `concurrency:` cannot
# coordinate across repos, so a filesystem flock mutex
# (tools/ci/with-nashua-lock.sh) keeps only one heavy job running at a time;
# it wraps the bench command in the "Run CodSpeed benchmarks" step below.
# See docs/ci/self-hosted-runner.md for what the box must provide: CodSpeed's
# own patched valgrind (NOT the distro valgrind package), libc6-dbg, flock and
# a fixed CPU model — node and yarn are provisioned per-job below. That doc
# also covers how the shared mutex works and the cutover steps.
# IMPORTANT: moving the bench between workflow files (or runners) is a
# baseline re-seed event: one main run must complete here before PR
# comparisons are meaningful again.
runs-on: [self-hosted, codspeed-bench, nashua]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Declare the custom runner labels for actionlint.

actionlint fails on the codspeed-bench and nashua labels because they are not in its known label set. Add an actionlint.yaml config so CI lint stays green.

🧹 Proposed config file
# .github/actionlint.yaml
self-hosted-runner:
  labels:
    - codspeed-bench
    - nashua
🧰 Tools
🪛 actionlint (1.7.12)

[error] 217-217: label "codspeed-bench" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2025-vs2026", "windows-2022", "windows-11-arm", "ubuntu-slim", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xlarge", "macos-latest-large", "macos-26-intel", "macos-26-xlarge", "macos-26-large", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xlarge", "macos-14-large", "macos-14", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)


[error] 217-217: label "nashua" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2025-vs2026", "windows-2022", "windows-11-arm", "ubuntu-slim", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xlarge", "macos-latest-large", "macos-26-intel", "macos-26-xlarge", "macos-26-large", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xlarge", "macos-14-large", "macos-14", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/bench.yml at line 217, Add a .github/actionlint.yaml
configuration declaring codspeed-bench and nashua under
self-hosted-runner.labels, so actionlint recognizes the custom labels used by
the runs-on declaration in the workflow.

Source: Linters/SAST tools

permissions:
contents: read
actions: read # cross-run artifact download from the PR checks run
pull-requests: write # CodSpeed action posts a sticky PR comment
id-token: write # OIDC token used by CodSpeedHQ/action for auth
# Bounded because the bench command can queue behind a cornerstone3D or
# OHIF Playwright run on the shared box: up to NASHUA_LOCK_WAIT (90 min) of
# waiting plus the valgrind fan-out itself. Well under GitHub's 360 min
# default, so a wedged lock surfaces in hours rather than most of a day.
timeout-minutes: 180
steps:
- uses: actions/checkout@v4
Comment on lines +217 to +229

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift

Restrict fork pull requests before this job runs on the shared self-hosted runner.

The repository is public and this workflow triggers on pull_request. A fork PR that touches only packages/* sets proceed=true, so actions/checkout fetches fork code and later steps execute it through yarn install --frozen-lockfile and yarn lerna run bench. The AUTHOR_ASSOCIATION gate in decide does not prevent this; it only defers PRs that touch CI-defining files. Also, author_association is not an authorization signal: MEMBER means organization membership, not write access on this repository.

Consequences on a persistent runner shared with cornerstonejs/cornerstone3D and OHIF/Viewers:

  • Exfiltration of the job GITHUB_TOKEN, which holds pull-requests: write, and of the OIDC token from id-token: write.
  • Poisoning of the persistent workspace and of the node_modules cache restored at line 274.
  • Access to co-tenant runner registration credentials if the three runners share one OS user.

Apply defense in depth:

  1. Gate the job on same-repository PRs, or require manual approval through a protected environment: with required reviewers.
  2. Set persist-credentials: false on the checkout so the job token does not stay in .git/config on the persistent workspace (zizmor artipacked).
  3. Run each runner under its own OS user and prefer ephemeral runner registration, as also requested in the earlier review.
🔒 Proposed gating and checkout hardening
     runs-on: [self-hosted, codspeed-bench, nashua]
+    # Fork PRs must not execute on the persistent shared box. The environment
+    # requires a maintainer approval before the job starts.
+    environment: bench-self-hosted
     permissions:
       contents: read
       actions: read         # cross-run artifact download from the PR checks run
       pull-requests: write  # CodSpeed action posts a sticky PR comment
       id-token: write       # OIDC token used by CodSpeedHQ/action for auth
     timeout-minutes: 180
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v4
+        with:
+          persist-credentials: false

If approval-on-every-PR is too costly, replace environment: with a fork check and let fork PRs bench on main after merge, which matches the existing deferral model:

    if: >-
      needs.gate.outputs.proceed == 'true' && needs.gate.outputs.ready == 'true'
      && (github.event_name != 'pull_request'
          || github.event.pull_request.head.repo.full_name == github.repository)
🧰 Tools
🪛 actionlint (1.7.12)

[error] 217-217: label "codspeed-bench" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2025-vs2026", "windows-2022", "windows-11-arm", "ubuntu-slim", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xlarge", "macos-latest-large", "macos-26-intel", "macos-26-xlarge", "macos-26-large", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xlarge", "macos-14-large", "macos-14", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)


[error] 217-217: label "nashua" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2025-vs2026", "windows-2022", "windows-11-arm", "ubuntu-slim", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xlarge", "macos-latest-large", "macos-26-intel", "macos-26-xlarge", "macos-26-large", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xlarge", "macos-14-large", "macos-14", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)

🪛 zizmor (1.29.0)

[warning] 229-229: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/bench.yml around lines 217 - 229, Harden the benchmark job
gated by needs.gate.outputs.proceed and needs.gate.outputs.ready so fork pull
requests cannot execute on the shared self-hosted runner; require the pull
request head repository to match github.repository, or use a protected
environment with required reviewers. Update actions/checkout@v4 to set
persist-credentials: false, and preserve the existing same-repository and
non-pull-request execution paths.

Source: Linters/SAST tools

- uses: actions/setup-node@v4
with:
# EXACT version, not the '22' range every other job uses. For a range,
# setup-node takes any satisfying version already in the runner's tool
# cache without checking the network — and on a self-hosted box that
# cache persists, so the bench would silently freeze on whichever 22.x
# landed there first and jump whenever the box is rebuilt. A V8 patch
# bump shifts instruction counts the same way a different CPU does.
# 22.23.1 is what the current main baseline was measured on; changing
# it is a deliberate re-seed event (see docs/ci/self-hosted-runner.md).
node-version: '22.23.1'
# nashua has no yarn: setup-node ships node + npm only, GitHub's hosted
# images preinstall yarn 1, and the other two repos on this box use pnpm.
# Corepack is bundled with node 22 and fetches over Node's own https, so it
# works where `npm i -g yarn` is unreliable here — the runner's bundled node
# has a corrupted npm ("Cannot find module '../lib/cli.js'"), which is why
# OHIF's workflow also went the Corepack route on this box. Pinned to the
# same yarn the build job uses rather than Corepack's bundled default, and
# activated AFTER setup-node so the shim lands in that node's bin dir.
# NOTE: node 25 unbundles corepack — revisit this step before any such bump.
- name: Provide yarn 1 via Corepack
run: |
corepack enable yarn
corepack prepare yarn@1.22.22 --activate
yarn --version
- name: Download all built dists
uses: actions/download-artifact@v4
with:
pattern: dist-*
path: tmp/
# Cross-run download: the artifacts live on the PR checks run the
# gate waited for, not on this run.
run-id: ${{ needs.gate.outputs.run_id }}
github-token: ${{ github.token }}
- name: Replay dists into packages/<pkg>/dist
run: |
set -e
for d in tmp/dist-*; do
[ -d "$d" ] || continue
pkg=$(basename "$d" | sed 's/^dist-//')
mkdir -p "packages/$pkg/dist"
shopt -s dotglob nullglob
cp -r "$d"/* "packages/$pkg/dist/" 2>/dev/null || true
done
- name: Restore node_modules cache
id: modules-cache
uses: actions/cache@v4
with:
path: |
node_modules
packages/*/node_modules
key: modules-node22-${{ runner.os }}-${{ runner.arch }}-${{ hashFiles('yarn.lock') }}
- name: Install dependencies
if: steps.modules-cache.outputs.cache-hit != 'true'
run: yarn install --frozen-lockfile
- name: Log CPU info
# GitHub standard runners are randomly assigned different physical
# CPUs (e.g. Intel Xeon 8370C vs AMD EPYC 7763) with different cache
# sizes and ISA extensions; glibc dispatches different code paths on
# each, so even simulation-mode instruction counts shift between
# them. CodSpeed flags such comparisons as "different runtime
# environments". Logging the CPU makes those warnings diagnosable
# at a glance (recommendation from
# https://codspeed.io/blog/unrelated-benchmark-regression).
run: lscpu | grep -E "Model name|Cache|Flags" | head -5 || true
- name: Compute bench scope
# Translate the changed-package directory names into lerna --scope
# flags so PRs only bench what they touched. Baseline runs (main /
# workflow_dispatch) get the full list from the gate, which makes
# this a no-op filter there.
id: scope
env:
BENCH: ${{ needs.gate.outputs.bench }}
run: |
flags=""
for pkg in $(echo "$BENCH" | jq -r '.[]'); do
name=$(node -p "require('./packages/$pkg/package.json').name")
flags="$flags --scope $name"
done
echo "Bench scope flags:$flags"
echo "flags=$flags" >> "$GITHUB_OUTPUT"
- name: Run CodSpeed benchmarks
# CodSpeedHQ/action@v4 sets up CPU simulation (Cachegrind-based
# instruction counting on a modeled CPU + cache hierarchy), runs
# the inner command under valgrind, uploads to codspeed.io, and
# posts/updates a sticky PR comment with the per-bench deltas.
# Authenticates via GitHub OIDC (id-token: write above) so no
# CODSPEED_TOKEN secret is needed.
#
# mode: simulation — the blocking regression gate:
# - deterministic: <1% run-to-run drift (verified across 3
# runs of identical source)
# - no metered CI minutes (self-hosted runners are not billed;
# they cost shared time on the nashua box instead, which is
# what the flock mutex below rations)
# - regression-detection signal is strong even though the
# headline numbers are MODELED instruction-time, not real
# wall-clock (JS-loop benches inflate 30-100x vs production
# V8 due to no JIT under Cachegrind; wasm decode kernels
# inflate ~5-15x; pure native ~1x)
# The codspeed-walltime job in pr-checks.yml complements this with
# real wall-clock measurements on CodSpeed macro runners.
# See BENCHMARKING.md at the repo root for the full measurement
# model, how to read the cold/warm bench split, and what the
# CodSpeed dashboard warnings mean.
# Pinned to the commit SHA of v4.18.2 (not the floating @v4, nor the
# tag — tags can be moved) so the instrumentation environment stays
# byte-identical between the main baseline and PR runs. Bump by
# resolving the new tag: gh api repos/CodSpeedHQ/action/git/ref/tags/<tag>
# (dereference the annotated tag to its commit).
# The inner command is wrapped in tools/ci/with-nashua-lock.sh, the
# cross-repo flock mutex for this shared box (see that script's header).
# Taking the lock INSIDE the action's `run` — rather than in an earlier
# step — is deliberate on both counts: flock lives and dies with a
# process, so it cannot be held across separate workflow steps, and this
# way only the CPU-saturating bench fan-out is serialized, leaving the
# action's own setup and result upload free to overlap with a Playwright
# run on one of the other two repos.
# Note: vitest 3's hard-coded 60s worker-RPC timer counts real
# seconds while valgrind slows the process ~60x, so large suites
# structurally trip "Timeout calling onTaskUpdate" AFTER their
# benches complete. The vitest configs set
# dangerouslyIgnoreUnhandledErrors when CODSPEED_RUNNER_MODE is
# "simulation" to keep that exit-code noise from failing the job
# (config-level because yarn 1 mangles `--`-forwarded CLI flags).
uses: CodSpeedHQ/action@4e969336ab9acd4f6f8d025fdd793292b0835df0 # v4.18.2
with:
mode: simulation
run: bash tools/ci/with-nashua-lock.sh yarn lerna run bench --parallel --stream ${{ steps.scope.outputs.flags }}
Comment on lines +295 to +358

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Validate the package name before it reaches GITHUB_OUTPUT and the run: string.

name comes from packages/<pkg>/package.json in the checked-out PR tree, so a fork PR controls its value. Two problems follow:

  • A name that contains a newline injects extra key-value pairs into $GITHUB_OUTPUT.
  • ${{ steps.scope.outputs.flags }} is expanded into the action run: command at line 358, so a name that contains shell metacharacters runs commands on the self-hosted runner.

Reject names that are not plain scoped package names, and pass the flags through the environment instead of template interpolation.

🔒 Proposed validation and env-based passing
         run: |
+          set -euo pipefail
           flags=""
           for pkg in $(echo "$BENCH" | jq -r '.[]'); do
             name=$(node -p "require('./packages/$pkg/package.json').name")
+            case "$name" in
+              @*/*|[a-z0-9._-]*) ;;
+              *) echo "::error::Unexpected package name for $pkg"; exit 1 ;;
+            esac
+            if printf '%s' "$name" | grep -qP '[^A-Za-z0-9@/._-]'; then
+              echo "::error::Rejected package name for $pkg"; exit 1
+            fi
             flags="$flags --scope $name"
           done
           echo "Bench scope flags:$flags"
           echo "flags=$flags" >> "$GITHUB_OUTPUT"
         uses: CodSpeedHQ/action@4e969336ab9acd4f6f8d025fdd793292b0835df0 # v4.18.2
+        env:
+          SCOPE_FLAGS: ${{ steps.scope.outputs.flags }}
         with:
           mode: simulation
-          run: bash tools/ci/with-nashua-lock.sh yarn lerna run bench --parallel --stream ${{ steps.scope.outputs.flags }}
+          run: bash tools/ci/with-nashua-lock.sh yarn lerna run bench --parallel --stream $SCOPE_FLAGS

The walltime job in .github/workflows/pr-checks.yml uses the same pattern, so apply the same validation there.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/bench.yml around lines 295 - 358, Validate each package
name in the scope-computation step before appending it to flags, accepting only
plain scoped package names and rejecting invalid values such as newlines or
shell metacharacters. Pass the resulting scope flags to the CodSpeed action
through an environment variable rather than interpolating
steps.scope.outputs.flags into the run command. Apply the equivalent validation
and environment-based flag passing to the matching walltime benchmark flow.

Loading
Loading