From 7b02ec7892c98540bcff61025cf43039f77b0ed2 Mon Sep 17 00:00:00 2001 From: olivrg Date: Fri, 19 Jun 2026 18:03:33 +0100 Subject: [PATCH 1/2] chore(ci): decouple the supply-chain audit from unrelated PRs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `pnpm audit --audit-level=high` gate kept failing in-flight feature PRs on newly-published advisories for dependencies those PRs never touched (esbuild, vite, undici — three times). The audit's coverage is correct; the per-PR *coupling* was the problem. Keep full-tree coverage (dev + build + transitive — npm supply-chain campaigns target those, not just prod), change only the enforcement: - ci.yml: on a pull_request the audit runs ONLY when the PR changes a dependency manifest or install input — package.json / pnpm-lock.yaml / pnpm-workspace.yaml / .npmrc / .pnpmfile.* / patches/** — detected via inline `git diff` (no new action dependency). Otherwise it emits a notice and the check stays green. Fails CLOSED: unknown event, missing base SHA, diff error, or grep error all run the full audit. push-to-main still audits unconditionally. - security-audit.yml: new daily (+ manual) full-tree audit as the standing-tree backstop; opens/updates a tracking issue on failure so it's actionable. - release.yml already audits the full tree before publishing (unchanged) — the ship-time guarantee. - CONTRIBUTING.md: documents the posture, the triage-by-type rule (a malicious / install-RCE / token-exfil advisory is an incident regardless of dev-vs-prod; benign dev-tool vulns get a time-boxed ignore + tracking issue), the register of current dev-only ignores, and the branch-protection this control depends on. Safety invariant: with --frozen-lockfile, leaving every dependency manifest / install input unchanged means an unchanged installed tree, so a skipped PR audit can introduce nothing unaudited; the standing tree stays covered by main, release, the daily audit, and Dependabot. No pull_request_target; base SHA is passed via env, not interpolated into the script. Implements the convention/decoupling from #75 (enable Dependabot security updates + branch protection separately, as repo settings). #64 stays open: the esbuild/vite ignores remain until vite is upgraded. --- .github/workflows/ci.yml | 47 ++++++++++++++++++++- .github/workflows/security-audit.yml | 63 ++++++++++++++++++++++++++++ CONTRIBUTING.md | 49 ++++++++++++++++++++++ 3 files changed, 158 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/security-audit.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 02f4fe5..cabd601 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -62,8 +62,53 @@ jobs: - name: Docs drift check run: pnpm docs:check:ci + # Supply-chain audit. Full-tree (dev + build + transitive) on every push to + # main and on release (release.yml). On a pull_request it runs ONLY when the + # PR changes a dependency manifest — so an unrelated feature PR is not blocked + # by a newly-published advisory on a dep it never touched. Safe because, with + # `--frozen-lockfile`, an unchanged manifest means an unchanged installed tree + # (nothing new to audit), and that standing tree is still covered by the + # main-push audit, the release audit, the daily Security Audit workflow, and + # Dependabot security updates. Fails CLOSED: any uncertainty runs the audit. - name: Audit dependencies - run: pnpm audit --audit-level=high + shell: bash + env: + EVENT_NAME: ${{ github.event_name }} + BASE_SHA: ${{ github.event.pull_request.base.sha }} + run: | + set -uo pipefail + run_audit() { + echo "Running full-tree dependency audit (pnpm audit --audit-level=high)" + pnpm audit --audit-level=high + } + if [ "$EVENT_NAME" != "pull_request" ]; then + run_audit; exit $? + fi + if [ -z "${BASE_SHA:-}" ]; then + echo "::notice::No PR base SHA available — running full audit (fail-closed)." + run_audit; exit $? + fi + if ! changed="$(git diff --name-only "$BASE_SHA" HEAD 2>/dev/null)"; then + echo "::notice::Could not compute changed files — running full audit (fail-closed)." + run_audit; exit $? + fi + # Any file that can affect what `pnpm install` produces or executes: + # declared/resolved deps (package.json, pnpm-lock.yaml, pnpm-workspace.yaml), + # registry/install config (.npmrc), install hooks (.pnpmfile.cjs/.js/.mjs), + # and applied patches (patches/**). Missing any of these would be a + # fail-open (a PR could change the installed tree while the audit skips). + # Branch on grep's exit code explicitly: 0 = matched (run), 1 = no match + # (skip), anything else = grep error → run (fail-closed). + printf '%s\n' "$changed" | grep -Eq '(^|/)(package\.json|pnpm-lock\.yaml|pnpm-workspace\.yaml|\.npmrc|\.pnpmfile\.(c|m)?js)$|(^|/)patches/' + rc=$? + if [ "$rc" -eq 0 ]; then + echo "Dependency manifest / install input changed in this PR — running full audit." + run_audit; exit $? + elif [ "$rc" -ne 1 ]; then + echo "::notice::Manifest-change detection errored (grep rc=$rc) — running full audit (fail-closed)." + run_audit; exit $? + fi + echo "::notice title=Dependency audit skipped::No dependency manifest or install input changed in this PR (package.json / pnpm-lock.yaml / pnpm-workspace.yaml / .npmrc / .pnpmfile.* / patches/). The full-tree audit is enforced on main, on release, and by the daily Security Audit workflow." - name: Build run: pnpm build diff --git a/.github/workflows/security-audit.yml b/.github/workflows/security-audit.yml new file mode 100644 index 0000000..40663ad --- /dev/null +++ b/.github/workflows/security-audit.yml @@ -0,0 +1,63 @@ +name: Security Audit + +# Standing-tree supply-chain backstop. The per-PR audit (ci.yml) only runs when a +# PR changes a dependency manifest, so this daily full-tree audit catches a +# newly-published advisory on an already-installed dependency during quiet periods +# (no pushes to main). Dependabot security updates is the proactive remediator; +# this is the visible fail-loud signal. Full tree (dev + build + transitive). + +on: + schedule: + - cron: '17 7 * * *' # daily at 07:17 UTC + workflow_dispatch: + +concurrency: + group: security-audit + cancel-in-progress: true + +permissions: + contents: read + issues: write # open/append a tracking issue when the audit fails (see below) + +jobs: + audit: + name: Full-tree dependency audit + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Install pnpm + uses: pnpm/action-setup@v4 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version-file: '.nvmrc' + cache: 'pnpm' + + - name: Install dependencies + run: pnpm install --frozen-lockfile + + - name: Audit dependencies (full tree) + run: pnpm audit --audit-level=high + + # Make a failure actionable rather than a quiet red run: open a tracking + # issue (or comment on the existing open one, to avoid daily duplicates). + - name: Report audit failure + if: failure() + env: + GH_TOKEN: ${{ github.token }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + shell: bash + run: | + set -uo pipefail + title='Scheduled security audit failing' + body="The daily full-tree \`pnpm audit --audit-level=high\` failed. Triage per CONTRIBUTING.md (Dependencies & supply-chain audits). Run: ${RUN_URL}" + existing="$(gh issue list --state open --search "in:title \"${title}\"" --json number --jq '.[0].number // empty' 2>/dev/null || true)" + if [ -n "${existing:-}" ]; then + gh issue comment "$existing" --body "$body" || true + else + gh issue create --title "$title" --body "$body" || true + fi diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a95a999..0b79cda 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -163,6 +163,55 @@ The proxy sits in the critical path of every agent tool call. Performance matter - Audit writes must be **async and non-blocking**: never add latency to the tool call path. - If you're adding a dependency, consider its impact on startup time and memory. +### Dependencies & supply-chain audits + +Helio is a security tool, so the dependency tree is part of the threat model — npm +supply-chain campaigns (e.g. the 2026 TeamPCP wave) compromise **dev, build, and +transitive** packages, not just production ones, with install-time code execution. +Our audit posture reflects that: + +- **Coverage is always the full tree** (dev + build + transitive). We never scope + the audit to production-only — build tooling produces the shipped bundle and runs + in CI with credentials, so it is in scope. +- **`pnpm audit --audit-level=high` is enforced unconditionally on `main`, on every + release (`release.yml`), and daily (`security-audit.yml`).** These are the + guarantees: a flagged dependency can never be merged to `main` unnoticed or + shipped in a release. +- **On a pull request, the audit runs only when the PR changes a dependency + manifest or install input** — `package.json`, `pnpm-lock.yaml`, + `pnpm-workspace.yaml`, `.npmrc`, `.pnpmfile.*` (install hooks), or anything under + `patches/`. With `--frozen-lockfile`, leaving all of these unchanged means an + unchanged installed tree, so an unrelated feature PR is not blocked by a + newly-published advisory on a dependency it never touched. The check still reports + green with a notice explaining the skip. The guard **fails closed** — any + uncertainty (unknown event, missing base SHA, diff/grep error) runs the audit. If + you add a new install-affecting input (e.g. a new pnpm hook mechanism), add it to + the trigger set in `ci.yml`. +- **Dependabot security updates** is enabled so advisories on the standing tree are + auto-PR'd within hours rather than discovered by a CI failure. +- **This gate's integrity depends on branch protection.** `main` must require the + `ci` status check and code-owner review (workflow files are owned via CODEOWNERS), + with admin bypass disabled — otherwise a PR could weaken the workflow itself. Treat + those settings as part of the control, not optional. + +**Handling an advisory — triage by _type_, not just dev-vs-prod:** + +- A **malicious package, install-time RCE, or credential-exfiltration** advisory is + an **incident** regardless of whether the package is dev-only or shipped — remediate + immediately (upgrade, or remove), do not ignore. +- A **benign vulnerability with no exploit path in our usage** (e.g. a dev-server + SSRF or ReDoS in a build tool we only invoke on trusted input) may be **time-boxed + ignored** via `pnpm.auditConfig.ignoreGhsas`, but **only** with a tracking issue to + remove it. Prefer a real upgrade over an ignore. + +Current dev-only ignores (each tracked for removal): + +| GHSA | Package (path) | Why ignored | Tracking | +| --------------------- | ----------------------------------- | --------------------------------------------------------------------------------------------------------------------- | -------- | +| `GHSA-gv7w-rqvm-qjhr` | esbuild (via vite, dashboard build) | dev-server request vuln; not in the shipped bundle | #64 | +| `GHSA-fx2h-pf6j-xcff` | vite (dashboard build) | dev-server only; vite is not run in production | #64 | +| `GHSA-vmh5-mc38-953g` | undici (via `jsdom`, test env) | SOCKS5 ProxyAgent TLS path, not exercised in tests; no patched undici is compatible with `jsdom@29`'s internal layout | #64 | + ## Issue Labels | Label | Meaning | From 9c8b8b1c29d551f52937f23e7c051d7582e74e5f Mon Sep 17 00:00:00 2001 From: olivrg Date: Fri, 19 Jun 2026 18:18:56 +0100 Subject: [PATCH 2/2] fix(ci): keep the audit guard from aborting on the skip path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GitHub runs `shell: bash` with `-e` (errexit), which my `set -uo pipefail` did not override. On the skip path the bare `printf … | grep -Eq` returns 1 (no dependency manifest changed) and errexit aborted the step before `rc=$?` — so any dep-unrelated PR (the exact case this feature is meant to allow) failed CI with exit 1. Add `set +e`; exit codes are already handled explicitly (rc checks + `run_audit; exit $?`), so the audit's own failure still propagates. Verified under `bash -e -o pipefail`: skip path exits 0 with the notice; a manifest / .pnpmfile / patches change runs the full audit. --- .github/workflows/ci.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cabd601..88e2fcc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -76,7 +76,12 @@ jobs: EVENT_NAME: ${{ github.event_name }} BASE_SHA: ${{ github.event.pull_request.base.sha }} run: | + # GitHub runs `shell: bash` with `-e` (errexit); disable it so the + # no-match `grep` (rc=1, the skip case) doesn't abort before we read + # `rc`. Exit codes are handled explicitly below (rc checks + exit $?), + # and the audit's own failure still propagates via `run_audit; exit $?`. set -uo pipefail + set +e run_audit() { echo "Running full-tree dependency audit (pnpm audit --audit-level=high)" pnpm audit --audit-level=high