From d03677ee42624b971fa051329019978cb6277650 Mon Sep 17 00:00:00 2001 From: MK Date: Thu, 8 Oct 2026 12:34:26 +0800 Subject: [PATCH] fix: isolate bundle rebuilds from the app token --- .github/workflows/rebuild-bundle.yml | 154 +++++++++++-- CONTRIBUTING.md | 2 + src/rebuild-bundle-workflow.test.ts | 333 +++++++++++++++++++++++++++ 3 files changed, 468 insertions(+), 21 deletions(-) create mode 100644 src/rebuild-bundle-workflow.test.ts diff --git a/.github/workflows/rebuild-bundle.yml b/.github/workflows/rebuild-bundle.yml index 8aa305f..ea3aa83 100644 --- a/.github/workflows/rebuild-bundle.yml +++ b/.github/workflows/rebuild-bundle.yml @@ -29,20 +29,15 @@ jobs: runs-on: ubuntu-latest permissions: contents: read + outputs: + artifact-id: ${{ steps.bundles.outputs.artifact-id }} steps: - # A GitHub App token so the follow-up push re-triggers the PR's required - # checks; a push with the default GITHUB_TOKEN does not (loop prevention). - - uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 - id: app-token - with: - client-id: ${{ secrets.APP_ID }} - private-key: ${{ secrets.APP_PRIVATE_KEY }} - + # PR code and dependencies run only on this unprivileged runner. # Actions are pinned to a full commit SHA (org policy). - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: - ref: ${{ github.event.pull_request.head.ref }} - token: ${{ steps.app-token.outputs.token }} + ref: ${{ github.event.pull_request.head.sha }} + persist-credentials: false # pnpm version is read from package.json's packageManager field. - uses: pnpm/action-setup@ea17c68df8912ef543352723c149a84f56e3d413 # v6.1.0 @@ -58,14 +53,131 @@ jobs: - name: Rebuild the action bundle run: pnpm build - - name: Commit and push the rebuilt bundle if it changed - run: | - if git diff --quiet -- dist/; then - echo "Bundle already up to date; nothing to push." - exit 0 - fi - git config user.name "github-actions[bot]" - git config user.email "41898282+github-actions[bot]@users.noreply.github.com" - git add dist/ - git commit -m "chore: rebuild action bundle for dependency update" - git push + - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + id: bundles + with: + name: rebuilt-bundles-${{ github.run_attempt }} + path: | + dist/index.mjs + dist/gitlab/index.mjs + dist/azure/index.mjs + if-no-files-found: error + retention-days: 1 + + commit: + name: commit rebuilt bundle + needs: rebuild + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read + # Use a fresh runner with no checkout, package installs, or cache restore. + # Treat the artifact as data; never execute it or apply a supplied patch. + steps: + - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + # Keep the build's artifact ID when only this failed job is rerun. + artifact-ids: ${{ needs.rebuild.outputs.artifact-id }} + path: ${{ runner.temp }}/rebuilt-bundles + + - name: Validate bundles and prepare commit + id: prepare + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + with: + script: | + const fs = require('node:fs'); + const path = require('node:path'); + const { createHash } = require('node:crypto'); + const expected = context.payload.pull_request; + const repository = `${context.repo.owner}/${context.repo.repo}`; + const { data: pr } = await github.rest.pulls.get({ + ...context.repo, pull_number: expected.number, + }); + if (pr.state !== 'open' || pr.head.sha !== expected.head.sha || + pr.head.ref !== expected.head.ref || pr.head.repo?.full_name !== repository || + !pr.labels.some(label => label.name === 'needs-bundle-rebuild')) { + core.info('The PR changed or no longer qualifies for a bundle rebuild; skip this result.'); + return; + } + + const files = ['index.mjs', 'gitlab/index.mjs', 'azure/index.mjs']; + const directories = ['', 'gitlab', 'azure']; + const root = path.join(process.env.RUNNER_TEMP, 'rebuilt-bundles'); + // Reject unexpected paths, symlinks, and non-regular files before reading data. + for (const directory of directories) { + const absolute = path.join(root, directory); + if (!fs.lstatSync(absolute).isDirectory()) { + throw new Error(`Expected a bundle directory: ${directory}`); + } + for (const entry of fs.readdirSync(absolute, { withFileTypes: true })) { + const relative = directory ? `${directory}/${entry.name}` : entry.name; + if (directories.includes(relative) && entry.isDirectory()) continue; + if (files.includes(relative) && entry.isFile()) continue; + throw new Error(`Unexpected bundle artifact entry: ${relative}`); + } + } + + const { data: commit } = await github.rest.git.getCommit({ + ...context.repo, commit_sha: expected.head.sha, + }); + const { data: tree } = await github.rest.git.getTree({ + ...context.repo, tree_sha: commit.tree.sha, recursive: 'true', + }); + if (tree.truncated) throw new Error('Cannot verify a truncated repository tree.'); + const additions = []; + for (const file of files) { + const absolute = path.join(root, file); + const stat = fs.lstatSync(absolute); + if (!stat.isFile() || stat.size === 0 || stat.size > 10 * 1024 * 1024) { + throw new Error(`Invalid bundle file: ${file}`); + } + const content = fs.readFileSync(absolute); + const destination = `dist/${file}`; + const previous = tree.tree.find(entry => entry.path === destination); + if (previous && (previous.type !== 'blob' || previous.mode !== '100644')) { + throw new Error(`Expected a regular repository file: ${destination}`); + } + const sha = createHash('sha1').update(`blob ${content.length}\0`).update(content).digest('hex'); + if (sha !== previous?.sha) { + additions.push({ path: destination, contents: content.toString('base64') }); + } + } + if (additions.length === 0) { + core.info('Bundle already up to date; nothing to push.'); + return; + } + + fs.writeFileSync(path.join(process.env.RUNNER_TEMP, 'bundle-commit.json'), JSON.stringify({ + branch: { repositoryNameWithOwner: repository, refName: `refs/heads/${expected.head.ref}` }, + expectedHeadOid: expected.head.sha, + message: { headline: 'chore: rebuild action bundle for dependency update' }, + fileChanges: { additions }, + })); + core.setOutput('changed', 'true'); + + # Create the narrowly scoped token only after validation on this runner. + # The App identity ensures the commit triggers the PR's required checks. + - uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 + if: steps.prepare.outputs.changed == 'true' + id: app-token + with: + client-id: ${{ secrets.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} + permission-contents: write + + - name: Commit the rebuilt bundles + if: steps.prepare.outputs.changed == 'true' + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + with: + github-token: ${{ steps.app-token.outputs.token }} + script: | + const fs = require('node:fs'); + const path = require('node:path'); + const input = JSON.parse(fs.readFileSync(path.join(process.env.RUNNER_TEMP, 'bundle-commit.json'), 'utf8')); + // The API atomically checks the head SHA, so a concurrent push or rebase + // cannot receive bundles built from an older revision. + await github.graphql(` + mutation($input: CreateCommitOnBranchInput!) { + createCommitOnBranch(input: $input) { commit { oid } } + } + `, { input }); diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0cd74da..3bdfd12 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -68,6 +68,8 @@ For a manual run, use `workflow_dispatch`. Set `setup_ref` to an exact commit SH Renovate opens a PR when SocketDev publishes a new `sfw-free` release. The custom manager in [`.github/renovate.json`](.github/renovate.json) updates the shared `SFW_VERSION` pin in `src/ci/install-sfw.ts` for all three integrations. +The `needs-bundle-rebuild` label rebuilds bundles for same-repository PRs when the label is added or the branch changes. The build job checks out the event's exact commit without persisted credentials and runs with read-only repository access. A separate job validates the three bundle files, then creates a GitHub App token with `contents: write` to commit changes through the API. It does not check out or execute PR code. It skips unchanged bundles and stale PRs; the commit fails if the branch head changes after validation. + ## Releasing Publish releases as Git tags, not as an npm package. Keep `package.json.version` aligned with the release tag. Consumers pin an exact tag such as `voidzero-dev/setup-vp@v1.21.1` or a commit SHA. Do not move the `v1` major tag, which is frozen at `v1.15.0`. diff --git a/src/rebuild-bundle-workflow.test.ts b/src/rebuild-bundle-workflow.test.ts new file mode 100644 index 0000000..96ad244 --- /dev/null +++ b/src/rebuild-bundle-workflow.test.ts @@ -0,0 +1,333 @@ +import { createHash } from "node:crypto"; +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + symlinkSync, + truncateSync, + writeFileSync, +} from "node:fs"; +import { createRequire } from "node:module"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { runInNewContext } from "node:vm"; +import { afterEach, describe, expect, it, vi } from "vite-plus/test"; +import { parse as parseYaml } from "yaml"; + +interface Step { + id?: string; + uses?: string; + if?: string; + run?: string; + with?: Record; +} + +const workflow = parseYaml( + readFileSync(new URL("../.github/workflows/rebuild-bundle.yml", import.meta.url), "utf8"), +); +const buildSteps = workflow.jobs.rebuild.steps as Step[]; +const commitSteps = workflow.jobs.commit.steps as Step[]; +const prepareStep = commitSteps.find((step) => step.id === "prepare")!; +const publishStep = commitSteps.at(-1)!; +const bundleFiles = ["index.mjs", "gitlab/index.mjs", "azure/index.mjs"]; +const headSha = "a".repeat(40); +const tempDirs: string[] = []; +const require = createRequire(import.meta.url); + +afterEach(() => { + for (const dir of tempDirs.splice(0)) { + rmSync(dir, { recursive: true, force: true }); + } +}); + +function blobSha(content: string): string { + return createHash("sha1") + .update(`blob ${Buffer.byteLength(content)}\0`) + .update(content) + .digest("hex"); +} + +function fixture() { + const dir = mkdtempSync(join(tmpdir(), "setup-vp-rebuild-")); + tempDirs.push(dir); + const root = join(dir, "rebuilt-bundles"); + const inputPath = join(dir, "bundle-commit.json"); + for (const file of bundleFiles) { + mkdirSync(dirname(join(root, file)), { recursive: true }); + writeFileSync(join(root, file), `throw new Error('Do not execute ${file}');\n`); + } + const pr = { + number: 123, + state: "open", + head: { sha: headSha, ref: "renovate/dependencies", repo: { full_name: "upstream/setup-vp" } }, + labels: [{ name: "needs-bundle-rebuild" }], + }; + const context = { + repo: { owner: "upstream", repo: "setup-vp" }, + payload: { pull_request: structuredClone(pr) }, + }; + const tree = { + truncated: false, + tree: bundleFiles.map((file) => ({ + path: `dist/${file}`, + mode: "100644", + type: "blob", + sha: blobSha("old bundle"), + })), + }; + const github = { + rest: { + pulls: { get: vi.fn().mockResolvedValue({ data: pr }) }, + git: { + getCommit: vi.fn().mockResolvedValue({ data: { tree: { sha: "base-tree" } } }), + getTree: vi.fn().mockResolvedValue({ data: tree }), + }, + }, + graphql: vi.fn().mockResolvedValue({ createCommitOnBranch: { commit: { oid: "new-commit" } } }), + }; + const core = { info: vi.fn(), setOutput: vi.fn() }; + function run(step: Step): Promise { + return runInNewContext(`(async () => { ${step.with!.script} })()`, { + require, + github, + context, + core, + process: { env: { RUNNER_TEMP: dir } }, + }); + } + return { + root, + inputPath, + pr, + tree, + context, + github, + core, + prepare: () => run(prepareStep), + publish: () => run(publishStep), + input: () => JSON.parse(readFileSync(inputPath, "utf8")), + }; +} + +describe("bundle rebuild credential isolation", () => { + it("runs PR builds without secrets or persisted credentials at the event SHA", () => { + expect(workflow.on).toEqual({ pull_request: { types: ["labeled", "synchronize"] } }); + expect(workflow.permissions).toEqual({}); + expect(workflow.jobs.rebuild.permissions).toEqual({ contents: "read" }); + expect(workflow.jobs.rebuild.if).toContain("'needs-bundle-rebuild'"); + expect(workflow.jobs.rebuild.if).toContain( + "github.event.pull_request.head.repo.full_name == github.repository", + ); + const checkout = buildSteps.find((step) => step.uses?.startsWith("actions/checkout@"))!; + expect(checkout.with).toEqual({ + ref: "${{ github.event.pull_request.head.sha }}", + "persist-credentials": false, + }); + expect(JSON.stringify(workflow.jobs.rebuild)).not.toMatch( + /secrets\.|app-token|contents.*write/, + ); + const upload = buildSteps.at(-1)!; + expect(upload.id).toBe("bundles"); + expect(workflow.jobs.rebuild.outputs).toEqual({ + "artifact-id": "${{ steps.bundles.outputs.artifact-id }}", + }); + expect(upload.uses).toMatch(/^actions\/upload-artifact@[a-f0-9]{40}$/); + expect(String(upload.with!.path).trim().split("\n").sort()).toEqual( + bundleFiles.map((file) => `dist/${file}`).sort(), + ); + expect(upload.with!["if-no-files-found"]).toBe("error"); + }); + + it("creates a contents-only App token after validation on a fresh runner", () => { + expect(workflow.jobs.commit.needs).toBe("rebuild"); + expect(workflow.jobs.commit.permissions).toEqual({ contents: "read", "pull-requests": "read" }); + expect(commitSteps.map((step) => step.uses?.split("@")[0])).toEqual([ + "actions/download-artifact", + "actions/github-script", + "actions/create-github-app-token", + "actions/github-script", + ]); + expect(commitSteps.every((step) => !step.run && /@[a-f0-9]{40}$/.test(step.uses!))).toBe(true); + expect(commitSteps[0].with).toEqual({ + "artifact-ids": "${{ needs.rebuild.outputs.artifact-id }}", + path: "${{ runner.temp }}/rebuilt-bundles", + }); + expect(buildSteps.at(-1)!.with!.name).toContain("github.run_attempt"); + expect(commitSteps[2].with).toEqual({ + "client-id": "${{ secrets.APP_ID }}", + "private-key": "${{ secrets.APP_PRIVATE_KEY }}", + "permission-contents": "write", + }); + for (const step of commitSteps.slice(2)) { + expect(step.if).toBe("steps.prepare.outputs.changed == 'true'"); + } + expect(publishStep.with!["github-token"]).toBe("${{ steps.app-token.outputs.token }}"); + expect(prepareStep.with!["github-token"]).toBeUndefined(); + }); +}); + +describe("bundle artifact validation and commit", () => { + it("commits only bundle data to the event branch with an atomic head check", async () => { + const f = fixture(); + await f.prepare(); + expect(f.core.setOutput).toHaveBeenCalledWith("changed", "true"); + const input = f.input(); + expect(input).toEqual({ + branch: { + repositoryNameWithOwner: "upstream/setup-vp", + refName: "refs/heads/renovate/dependencies", + }, + expectedHeadOid: headSha, + message: { headline: "chore: rebuild action bundle for dependency update" }, + fileChanges: { + additions: bundleFiles.map((file) => ({ + path: `dist/${file}`, + contents: readFileSync(join(f.root, file)).toString("base64"), + })), + }, + }); + expect(f.github.rest.git.getCommit).toHaveBeenCalledWith({ + ...f.context.repo, + commit_sha: headSha, + }); + expect(f.github.rest.git.getTree).toHaveBeenCalledWith({ + ...f.context.repo, + tree_sha: "base-tree", + recursive: "true", + }); + await f.publish(); + expect(f.github.graphql).toHaveBeenCalledExactlyOnceWith( + expect.stringContaining("createCommitOnBranch(input: $input)"), + { input }, + ); + }); + + it("skips unchanged bundles so the follow-up run cannot create a commit loop", async () => { + const f = fixture(); + for (const entry of f.tree.tree) { + entry.sha = blobSha(readFileSync(join(f.root, entry.path.slice(5)), "utf8")); + } + await f.prepare(); + expect(f.core.info).toHaveBeenCalledWith("Bundle already up to date; nothing to push."); + expect(f.core.setOutput).not.toHaveBeenCalled(); + expect(existsSync(f.inputPath)).toBe(false); + }); + + it("includes only changed or missing files", async () => { + const f = fixture(); + f.tree.tree[0].sha = blobSha(readFileSync(join(f.root, "index.mjs"), "utf8")); + f.tree.tree.pop(); + await f.prepare(); + expect(f.input().fileChanges.additions.map((entry: { path: string }) => entry.path)).toEqual([ + "dist/gitlab/index.mjs", + "dist/azure/index.mjs", + ]); + }); + + it.each(["sha", "branch", "repository", "closed", "label"])( + "skips a PR after its %s changes", + async (change) => { + const f = fixture(); + if (change === "sha") f.pr.head.sha = "b".repeat(40); + if (change === "branch") f.pr.head.ref = "other-branch"; + if (change === "repository") f.pr.head.repo.full_name = "fork/setup-vp"; + if (change === "closed") f.pr.state = "closed"; + if (change === "label") f.pr.labels = []; + await f.prepare(); + expect(f.core.setOutput).not.toHaveBeenCalled(); + expect(f.github.rest.git.getCommit).not.toHaveBeenCalled(); + expect(existsSync(f.inputPath)).toBe(false); + }, + ); + + it.each([".git/config", "package.json", "azure/unexpected.mjs", "gitlab/hooks/pre-commit"])( + "rejects the unexpected artifact path %s", + async (file) => { + const f = fixture(); + mkdirSync(dirname(join(f.root, file)), { recursive: true }); + writeFileSync(join(f.root, file), "untrusted data"); + await expect(f.prepare()).rejects.toThrow("Unexpected bundle artifact entry"); + expect(f.core.setOutput).not.toHaveBeenCalled(); + expect(existsSync(f.inputPath)).toBe(false); + }, + ); + + it.skipIf(process.platform === "win32").each(["index.mjs", "gitlab"])( + "rejects a symlink at %s", + async (file) => { + const f = fixture(); + rmSync(join(f.root, file), { recursive: true }); + symlinkSync(f.inputPath, join(f.root, file)); + await expect(f.prepare()).rejects.toThrow("Unexpected bundle artifact entry"); + expect(f.core.setOutput).not.toHaveBeenCalled(); + }, + ); + + it.each(["missing", "empty", "oversized"])("rejects a %s bundle", async (kind) => { + const f = fixture(); + const bundle = join(f.root, "index.mjs"); + if (kind === "missing") rmSync(bundle); + else truncateSync(bundle, kind === "empty" ? 0 : 10 * 1024 * 1024 + 1); + await expect(f.prepare()).rejects.toThrow(); + expect(f.core.setOutput).not.toHaveBeenCalled(); + expect(existsSync(f.inputPath)).toBe(false); + }); + + it("rejects a truncated Git tree", async () => { + const f = fixture(); + f.tree.truncated = true; + await expect(f.prepare()).rejects.toThrow("truncated repository tree"); + expect(f.core.setOutput).not.toHaveBeenCalled(); + }); + + it.each(["120000", "160000", "040000", "100755"])( + "rejects an existing bundle with Git mode %s", + async (mode) => { + const f = fixture(); + f.tree.tree[0].mode = mode; + await expect(f.prepare()).rejects.toThrow("Expected a regular repository file"); + expect(f.core.setOutput).not.toHaveBeenCalled(); + }, + ); + + it("keeps shell syntax in branch names and bundle contents as data", async () => { + const f = fixture(); + const branch = "renovate/$(echo-injected)"; + f.pr.head.ref = branch; + f.context.payload.pull_request.head.ref = branch; + const content = "`${process.env.APP_TOKEN}`\n$(echo injected)\n\u0000\u00e9"; + writeFileSync(join(f.root, "index.mjs"), content); + await f.prepare(); + expect(f.input().branch.refName).toBe(`refs/heads/${branch}`); + expect(Buffer.from(f.input().fileChanges.additions[0].contents, "base64").toString()).toBe( + content, + ); + await f.publish(); + expect(f.github.graphql).toHaveBeenCalledOnce(); + }); + + it.each(["pull", "commit", "tree"])("fails closed on a %s API error", async (api) => { + const f = fixture(); + const method = + api === "pull" + ? f.github.rest.pulls.get + : api === "commit" + ? f.github.rest.git.getCommit + : f.github.rest.git.getTree; + method.mockRejectedValue(new Error("API unavailable")); + await expect(f.prepare()).rejects.toThrow("API unavailable"); + expect(f.core.setOutput).not.toHaveBeenCalled(); + expect(existsSync(f.inputPath)).toBe(false); + }); + + it("does not retry a rejected commit against a newer branch head", async () => { + const f = fixture(); + await f.prepare(); + f.github.graphql.mockRejectedValue(new Error("expectedHeadOid does not match")); + await expect(f.publish()).rejects.toThrow("expectedHeadOid does not match"); + expect(f.github.graphql).toHaveBeenCalledOnce(); + expect(f.input().expectedHeadOid).toBe(headSha); + }); +});