From 786cbd496a73ec99a07c894c2393922e1edf11d4 Mon Sep 17 00:00:00 2001 From: Evan Hu Date: Tue, 5 May 2026 01:18:46 +0900 Subject: [PATCH] chore(ci): harden refresh-cache workflow per PR re-review CRITICAL #1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first iteration moved signing from the Cloudflare worker into this repo's CI to fix the worker-as-sign-oracle defect. The re-review pointed out that this just relocated the trust problem: anyone who lands a commit on main gets the resulting bytes signed automatically. Mitigations: * CODEOWNERS — sign-script, build scripts, the workflow itself, and the auto-generated index/sig files are owned by the registry owner. Combined with branch protection requiring CODEOWNERS review, a PR touching the signing infrastructure or the artefacts it produces cannot land without explicit owner sign-off. Plugin contributions under plugins// are covered by the standard PR-review rules but don't trip CODEOWNERS unless they touch the signing path. * SHA-pinned actions — actions/checkout@v4 and actions/setup-node@v4 replaced with full-SHA refs (v4.2.2 / v4.1.0). Blocks action-supply-chain swaps (a malicious mutable tag move on a popular action would otherwise execute in the same job that has REGISTRY_PRIVATE_KEY in scope). * Post-sign self-verify — a new step verifies plugins-index.json.sig against the committed pubkey (not a secret) BEFORE the .sig hits main. Catches a buggy sign-script run, an env-leak that produces zero-bytes output, or a half-applied edit. * REGISTRY_PRIVATE_KEY env scope — explicitly noted in workflow comment that the secret is on the sign step ONLY (where it was already), not the job. Prevents future contributors lifting it to job-level out of convenience. * In-workflow security model docstring — enumerates the residual threat model (compromised maintainer, malicious PR, sign-step bug, leaked refresh token) and what each defense addresses. Trust root is now: GitHub branch protection on main + CODEOWNERS on sign infrastructure + SHA-pinned actions + post-sign verification. A maintainer with push rights can still ship malicious bytes through a code-review bypass — that residual is the same as for any signed package registry and falls outside what CI alone can mitigate. Branch protection on `main` MUST be configured by an org admin to match the assumptions in CODEOWNERS: - require pull request reviews (at least 1) - require review from CODEOWNERS - dismiss stale approvals on new commits - restrict who can push directly to main (org admins only) --- .github/CODEOWNERS | 31 ++++++++++++ .github/workflows/refresh-cache.yml | 74 +++++++++++++++++++++++++++-- 2 files changed, 101 insertions(+), 4 deletions(-) create mode 100644 .github/CODEOWNERS diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 0000000..b5af694 --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1,31 @@ +# CODEOWNERS for librefang-registry +# +# A push to main can trigger the registry-worker forced-refresh and have +# whatever's in plugins-index.json signed by the registry's Ed25519 key +# (REGISTRY_PRIVATE_KEY in this repo's GitHub Actions store). Branch +# protection alone doesn't constrain WHICH files a maintainer can land — +# CODEOWNERS does. +# +# Files listed here REQUIRE explicit approval from the listed owners +# before a PR can land. The signing infrastructure (workflow + script) +# and the artefacts it produces (committed indexes + signature) carry +# the highest sensitivity. Plugin contributions under plugins// +# are owned by the plugin author but still go through PR review. +# +# Branch protection on `main` MUST be configured to: +# - require pull request reviews (at least 1) +# - require review from CODEOWNERS +# - dismiss stale approvals on new commits +# - restrict who can push directly to main (org admins only) + +# ---- Signing infrastructure (highest sensitivity) ---- +/scripts/sign-plugins-index.mjs @suzukaze-haduki +/scripts/build-plugins-index.mjs @suzukaze-haduki +/scripts/build-registry-index.mjs @suzukaze-haduki +/.github/workflows/ @suzukaze-haduki +/.github/CODEOWNERS @suzukaze-haduki + +# ---- Auto-generated artefacts (must not be hand-edited) ---- +/plugins-index.json @suzukaze-haduki +/plugins-index.json.sig @suzukaze-haduki +/registry-index.json @suzukaze-haduki diff --git a/.github/workflows/refresh-cache.yml b/.github/workflows/refresh-cache.yml index 8a88822..ee2355f 100644 --- a/.github/workflows/refresh-cache.yml +++ b/.github/workflows/refresh-cache.yml @@ -5,11 +5,38 @@ name: Refresh registry-worker cache # plugins-index.json — daemon-shaped flat plugins array (signed) # registry-index.json — dict-shaped dashboard payload (unsigned) # then poke the worker's forced-refresh endpoint so it pulls both -# (2 subrequests total, regardless of registry size — important under -# Workers Free's 50-subrequest budget) and re-signs / stores them. +# (3 subrequests total, regardless of registry size — fits Workers +# Free's 50-subrequest budget) and stores them. # # Without this, dashboard + daemon would have to wait for the next # 02:00 UTC cron tick to see content changes (up to ~24h delay). +# +# === SECURITY MODEL === +# +# REGISTRY_PRIVATE_KEY (Ed25519 PKCS#8) is the trust root for every +# `librefang plugin install ` worldwide. Threats and mitigations: +# +# 1. Compromised maintainer pushes a malicious plugins-index.json +# directly to main. Mitigation: GitHub branch protection on `main` +# requires PR review (admin-configured); .github/CODEOWNERS forces +# sign-script and committed-artefact changes through the registry +# owner. +# +# 2. Malicious PR adds a step that exfiltrates the secret. Mitigation: +# .github/CODEOWNERS owns this file; can't be modified without +# registry-owner approval. Action versions are SHA-pinned to block +# action-supply-chain attacks (transitive `uses:` swap). +# +# 3. Sign step writes garbage. Mitigation: post-sign verify step +# validates the signature against the corresponding pubkey before +# committing — a malformed sign-script run is caught here, not +# after publish. +# +# 4. Worker secret REGISTRY_REFRESH_TOKEN leaked. Mitigation: worker +# no longer holds signing material (PR #4600), so a leaked token +# lets an attacker trigger refreshes against existing committed +# bytes — they can't substitute attacker-supplied bytes for the +# worker to sign. on: push: @@ -25,6 +52,7 @@ on: - 'mcp/**' - 'scripts/build-plugins-index.mjs' - 'scripts/build-registry-index.mjs' + - 'scripts/sign-plugins-index.mjs' workflow_dispatch: permissions: @@ -34,9 +62,11 @@ jobs: refresh: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + # SHA-pinned to block action-supply-chain swaps. Update with care + # (read the diff between current and target SHA upstream). + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - - uses: actions/setup-node@v4 + - uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0 with: node-version: '20' @@ -45,11 +75,47 @@ jobs: node scripts/build-plugins-index.mjs node scripts/build-registry-index.mjs + # REGISTRY_PRIVATE_KEY is scoped to ONLY this step's env so prior + # build steps and later trigger steps cannot read it. Keep this + # narrow; do NOT lift the env to the job level. - name: Sign plugins-index.json env: REGISTRY_PRIVATE_KEY: ${{ secrets.REGISTRY_PRIVATE_KEY }} run: node scripts/sign-plugins-index.mjs + # Defense in depth: verify the signature locally against the + # public key (committed in this repo as REGISTRY_PUBLIC_KEY env) + # before the .sig hits main. Catches a buggy or tampered + # sign-script run; closes PR re-review on the in-repo signing + # path. No secret material here. + - name: Verify signature against committed pubkey + env: + REGISTRY_PUBLIC_KEY: ClGa0Ucap8NdrKAy1rw9Tt6A9I8eg4zJ53+xIuKMuq0= + run: | + node -e ' + const c = require("crypto"), fs = require("fs"); + const idx = fs.readFileSync("plugins-index.json"); + const sig = Buffer.from( + fs.readFileSync("plugins-index.json.sig", "utf8").trim(), + "base64", + ); + const pub = Buffer.from(process.env.REGISTRY_PUBLIC_KEY, "base64"); + if (pub.length !== 32) { + console.error("REGISTRY_PUBLIC_KEY is not a raw 32-byte Ed25519 pubkey"); + process.exit(1); + } + const spki = Buffer.concat([ + Buffer.from("302a300506032b6570032100", "hex"), + pub, + ]); + const k = c.createPublicKey({ key: spki, format: "der", type: "spki" }); + if (!c.verify(null, idx, k, sig)) { + console.error("plugins-index.json.sig does NOT verify against the committed pubkey"); + process.exit(1); + } + console.log("Signature verifies OK against committed pubkey."); + ' + - name: Commit regenerated indexes if changed run: | git config user.name "github-actions[bot]"