Skip to content

chore(ci): audit only the dependencies a PR adds #6337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jbocce merged 6 commits into from
Oct 7, 2026
Merged

chore(ci): audit only the dependencies a PR adds #6337

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
06d1de3
chore(ci): remove NPM token from PR build and stop using npx wait-on@…
jbocce Oct 6, 2026
f72fd88
chore(ci): audit only the dependencies a PR adds, in a GitHub workflow
jbocce Oct 7, 2026
cd4cb49
chore(ci): show audit ignores a PR adds as a neutral check
jbocce Oct 7, 2026
715a6c8
chore(ci): flag new audit ignores with a commit status
jbocce Oct 7, 2026
d26655f
fix(ci): harden the dependency audit per review
jbocce Oct 7, 2026
62bf66a
test(ci): add tests for the dependency audit script
jbocce Oct 7, 2026
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
74 changes: 15 additions & 59 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ jobs:
- run:
name: 'React Compiler coverage budget'
command: pnpm run compiler:coverage:ci
# The PR's copy of the dependency-audit script. The GitHub workflow that
# uses it always runs master's copy, so changes are tested here.
- run:
name: 'Dependency audit script tests'
working_directory: .scripts/dependency-audit
command: npm ci --ignore-scripts --no-audit --no-fund && npm test
# platform/app
- run:
name: 'VIEWER: Combine report output'
Expand Down Expand Up @@ -122,67 +128,14 @@ jobs:
- checkout
- attach_workspace:
at: ~/repo
# SECURITY AUDIT - only when pnpm-lock.yaml has changed
- run:
name: 'Security Audit - High Risk Vulnerabilities'
command: |
git fetch origin master 2>/dev/null || true
BASE_REF=$(git merge-base HEAD origin/master 2>/dev/null)
if [[ -z "$BASE_REF" ]]; then
echo "Could not determine base ref (e.g. shallow clone or no origin/master), skipping security audit."
exit 0
fi
CHANGED_FILES=$(git diff --name-only origin/master...HEAD 2>/dev/null || echo "")
if ! echo "$CHANGED_FILES" | grep -qx 'pnpm-lock.yaml'; then
echo "pnpm-lock.yaml unchanged - skipping security audit."
exit 0
fi
echo "pnpm-lock.yaml changed - running pnpm audit for security vulnerabilities..."
echo "Checking for HIGH-RISK vulnerabilities..."

if pnpm audit --audit-level high; then
echo "No high-risk vulnerabilities found"
echo "Security audit passed!"
else
echo ""
echo "HIGH-RISK VULNERABILITIES DETECTED!"
echo "======================================"
echo ""
echo "To fix these issues:"
echo " 1. Run: pnpm audit"
echo " 2. Review the vulnerability details"
echo " 3. Update affected packages to secure versions"
echo " 4. Test your changes"
echo " 5. Re-run: pnpm audit --audit-level high"
echo ""
echo "Full audit report:"

pnpm audit || true

echo ""
echo "This build cannot proceed until high-risk vulnerabilities are resolved."
exit 1
fi
# The dependency audit is the GitHub "Dependency Audit" workflow
# (.github/workflows/dependency-audit.yml).
- run:
name: Install Dependencies
command: pnpm install
- run:
name: Avoid hosts unknown for github
command: |
rm -rf ~/.ssh
mkdir ~/.ssh/
echo -e "Host github.com\n\tStrictHostKeyChecking no\n" > ~/.ssh/config
git config --global user.email "danny.ri.brown+ohif-bot@gmail.com"
git config --global user.name "ohif-bot"
- run:
name: Authenticate with NPM registry
# Write the auth token to npm's per-user config (~/.npmrc), not the
# repo root. publish-package.mjs chdir's into each package dir to run
# `npm publish`, and npm only reads the project .npmrc from that dir
# plus the user .npmrc from $HOME -- it never sees ~/repo/.npmrc, so a
# root-level token yields ENEEDAUTH. Using ~/.npmrc also avoids
# clobbering the committed workspace-config .npmrc (node-linker=hoisted).
command: echo "//registry.npmjs.org/:_authToken=$NPM_TOKEN" > ~/.npmrc
# No git identity or NPM token here: this job only builds the packages for
# PRs and never pushes or publishes. Keep publish credentials in NPM_PUBLISH
# so third-party build code on PR branches never runs with them on disk.
- run:
name: build half of the packages (to avoid out of memory in circleci)
command: |
Expand Down Expand Up @@ -442,8 +395,11 @@ jobs:
# WebGL. Newer Chromium deprecated that implicit fallback (canvas
# rendering degrades and races with element clicks). ELECTRON_EXTRA_LAUNCH_ARGS
# is the reliable way to pass the opt-in flag to Cypress's Electron browser.
# Wait for the dev server with curl rather than `npx wait-on`: npx would
# fetch wait-on and its dependencies unpinned, outside the lockfile, in a
# job that holds the Cypress record key.
cypress-command: |
npx wait-on@latest http://localhost:3000 && cd platform/app && ELECTRON_EXTRA_LAUNCH_ARGS="--enable-unsafe-swiftshader" npx cypress run --record --parallel
timeout 600 bash -c 'until curl -sf -o /dev/null http://localhost:3000; do sleep 2; done' && cd platform/app && ELECTRON_EXTRA_LAUNCH_ARGS="--enable-unsafe-swiftshader" npx cypress run --record --parallel
start-command: pnpm run test:data && pnpm run test:e2e:serve

DOCKER_MULTIARCH_MANIFEST:
Expand Down
102 changes: 102 additions & 0 deletions .github/workflows/dependency-audit.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
name: Dependency Audit

# Fails a PR that adds a package version with a high or critical advisory to
# pnpm-lock.yaml. Only versions the PR adds count, compared with the point where
# it branched off its target branch, so advisories already on the target never
# block an unrelated PR. The logic is .scripts/dependency-audit/audit.mjs.
#
# Runs on pull_request_target so the workflow and script always come from the
# default branch: a PR cannot edit its own check. The PR's files are only
# downloaded and read as data (lockfile, pnpm-workspace.yaml); none of its code is
# checked out or run. Keep it that way — never check out the PR head here.
# Changes to this file or the script take effect once they are on master.
#
# No `edited` trigger: a title edit would start a run that cancels the real one
# and is then skipped, and a skipped check counts as passing. After retargeting
# a PR to another branch, push a commit (or close and reopen) to rerun.

on:
pull_request_target:

@wayfarer3130 wayfarer3130 Oct 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two points about the trigger:

  1. This PR removes the CircleCI audit. After the merge, this check blocks a merge only when branch protection lists "Dependency audit" as a required check. Please add the check to the branch protection of master and release/* when this PR merges.
  2. This check audits only the versions that a PR adds. No job audits master now. When an advisory appears for a version that is already on master, no job reports the advisory. I suggest a schedule run of pnpm audit --audit-level high on master that does not block, or an explicit decision to rely on Dependabot alerts.

@jbocce jbocce Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

  1. Yes. I will add "Dependency audit" as a required check on master and release/* once this merges. GitHub only offers a check there after it has run at least once.
  2. A daily check of master is planned as a separate task. It needs to run privately, since this repo's Actions logs and issues are public, so it will live outside this repo. Dependabot alerts don't work as the source for us: GitHub's dependency graph can't read our pnpm 12 lockfile (two YAML documents), so the alerts only reflect package.json declarations, not the installed versions.

types: [opened, synchronize, reopened]

permissions: {}

concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number }}
cancel-in-progress: true

jobs:
audit:
name: Dependency audit
runs-on: ubuntu-latest
timeout-minutes: 10
permissions:
contents: read
statuses: write # the "New dependency audit ignores" status
steps:
- uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
ref: ${{ github.event.repository.default_branch }}
persist-credentials: false
sparse-checkout: .scripts/dependency-audit
- uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
with:
node-version: 24.15.0
- name: Download the PR's lockfile and the target branch's lockfile
env:
GH_TOKEN: ${{ github.token }}
REPO: ${{ github.repository }}
BASE_SHA: ${{ github.event.pull_request.base.sha }}
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
run: |
set -euo pipefail
mkdir -p "$RUNNER_TEMP/audit"
# Compare against where the PR branched off, not the target's tip, so
# packages the target changed since then are not credited to the PR.
merge_base=$(gh api "repos/$REPO/compare/$BASE_SHA...$HEAD_SHA" --jq .merge_base_commit.sha)
echo "Target branch point: $merge_base"
# <path> <ref> <dest>. Only a 404 means "no such file" (dest left
# absent); any other error fails the job rather than skip the audit.
fetch() {
if ! gh api -H 'Accept: application/vnd.github.raw' "repos/$REPO/contents/$1?ref=$2" > "$3" 2> "$3.err"; then
grep -q 'HTTP 404' "$3.err" || { cat "$3.err" >&2; exit 1; }
rm -f "$3"
fi
}
fetch pnpm-lock.yaml "$merge_base" "$RUNNER_TEMP/audit/base-lock.yaml"
fetch pnpm-lock.yaml "$HEAD_SHA" "$RUNNER_TEMP/audit/head-lock.yaml"
fetch pnpm-workspace.yaml "$merge_base" "$RUNNER_TEMP/audit/base-workspace.yaml"
fetch pnpm-workspace.yaml "$HEAD_SHA" "$RUNNER_TEMP/audit/head-workspace.yaml"
- name: Install the audit script's dependencies
working-directory: .scripts/dependency-audit
run: npm ci --ignore-scripts --no-audit --no-fund
# No GH_TOKEN here: the script and its dependency never get the token.
- name: Audit the package versions this PR adds
id: audit
run: |
node .scripts/dependency-audit/audit.mjs \
--base "$RUNNER_TEMP/audit/base-lock.yaml" \
--head "$RUNNER_TEMP/audit/head-lock.yaml" \
--base-workspace "$RUNNER_TEMP/audit/base-workspace.yaml" \
--head-workspace "$RUNNER_TEMP/audit/head-workspace.yaml"
# Ignores the PR adds also get a line of their own in the PR's checks
# list, not only on this run's page. A commit status rather than a check
# run: an API-created check run joins whichever check suite GitHub picks
# for the commit, and when that suite is a skipped one the PR hides it.
# Statuses always show. State is success, so it never blocks a merge.
# Runs even when the audit failed.
- name: Flag new ignores in the PR's checks list
if: ${{ !cancelled() && steps.audit.outputs.new_ignores != '' && steps.audit.outputs.new_ignores != '0' }}
env:
GH_TOKEN: ${{ github.token }}
REPO: ${{ github.repository }}
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
COUNT: ${{ steps.audit.outputs.new_ignores }}
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
run: |
gh api --method POST "repos/$REPO/statuses/$HEAD_SHA" \
-f state=success \
-f context='New dependency audit ignores' \
-f description="⚠️ $COUNT GHSA(s) added to auditConfig.ignoreGhsas: review pnpm-workspace.yaml" \
-f target_url="$RUN_URL" \
--jq '"\(.context): \(.description)"'
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ yalc.lock
.npm
npm-debug.log
package-lock.json
# The dependency-audit PR check installs its own dependency with `npm ci`.
!.scripts/dependency-audit/package-lock.json
yarn-error.log
.DS_Store
.env
Expand Down
Loading
Loading