diff --git a/.github/workflows/ci-poller.yml b/.github/workflows/ci-poller.yml index 487e9da..54e1f4b 100644 --- a/.github/workflows/ci-poller.yml +++ b/.github/workflows/ci-poller.yml @@ -96,10 +96,13 @@ jobs: ( number=$(echo "$issue" | jq -r '.number') title=$(echo "$issue" | jq -r '.title') + expected_security_labels=$(echo "$issue" | jq -c '[.labels[].name | select(. == "accepted" or . == "ci-pending" or . == "ci-ready" or . == "ci-failed" or . == "dry-run")] | sort') body_file=$(mktemp) resolver_output_file="" updated_body_file="" - trap 'for file in "${body_file:-}" "${resolver_output_file:-}" "${updated_body_file:-}"; do [[ -z "$file" ]] || rm -f "$file"; done' EXIT + live_issue_file="" + live_body_file="" + trap 'for file in "${body_file:-}" "${resolver_output_file:-}" "${updated_body_file:-}" "${live_issue_file:-}" "${live_body_file:-}"; do [[ -z "$file" ]] || rm -f "$file"; done' EXIT printf '%s' "$issue" | jq -jr '.body' > "$body_file" if ! publish_input=$(PUBLISH_ISSUE_BODY_FILE="$body_file" PUBLISH_TITLE="$title" \ @@ -108,6 +111,7 @@ jobs: exit 0 fi repo=$(echo "$publish_input" | jq -r '.repo') + path=$(echo "$publish_input" | jq -r '.path') version=$(echo "$publish_input" | jq -r '.version') issue_sha=$(echo "$publish_input" | jq -r '.revision') @@ -160,6 +164,7 @@ jobs: exit 0 fi gh issue edit "$number" -R "$GITHUB_REPOSITORY" --body-file "$updated_body_file" + cp "$updated_body_file" "$body_file" fi echo "Checking CI for ${repo}@${version} commit ${sha:0:8} (issue #${number})..." @@ -215,6 +220,56 @@ jobs: if [[ "$status_ok" == "true" \ && "$pending_checks" == "0" && "$unsuccessful_checks" == "0" ]]; then + # Verify live approval: the current accepted-label actor must + # still have target access. + if ! APPROVAL_ISSUE_NUMBER="$number" \ + APPROVAL_ISSUE_REPOSITORY="$GITHUB_REPOSITORY" \ + APPROVAL_TOKEN="$GH_TOKEN" \ + REQUIRE_AUTHORIZED="true" \ + TARGET_REPOSITORY="${repo#getsentry/}" \ + TARGET_REPOSITORY_PATH="$path" \ + TARGET_REPOSITORY_TOKEN="$RELEASE_TOKEN" \ + node .__publish__/src/publish/authorize-approval.js; then + echo " Approval is no longer authorized; revoking approval." + gh issue edit "$number" -R "$GITHUB_REPOSITORY" \ + --remove-label "accepted" \ + --remove-label "ci-pending" + gh issue comment "$number" -R "$GITHUB_REPOSITORY" \ + --body "Approval is no longer valid for the target repository. Re-add the **accepted** label after resolving access." + exit 0 + fi + + # Verify live approval and request state immediately before the + # transition so no network lookup separates this fence from ci-ready. + live_issue_file=$(mktemp) + live_body_file=$(mktemp) + if ! gh issue view "$number" -R "$GITHUB_REPOSITORY" \ + --json title,body,labels > "$live_issue_file"; then + echo "::warning::Could not retrieve live issue #${number}; revoking approval." + gh issue edit "$number" -R "$GITHUB_REPOSITORY" \ + --remove-label "accepted" \ + --remove-label "ci-pending" + exit 0 + fi + live_title=$(jq -r '.title' "$live_issue_file") + jq -jr '.body' "$live_issue_file" > "$live_body_file" + has_accepted=$(jq '[.labels[].name == "accepted"] | any' "$live_issue_file") + has_ci_pending=$(jq '[.labels[].name == "ci-pending"] | any' "$live_issue_file") + live_security_labels=$(jq -c '[.labels[].name | select(. == "accepted" or . == "ci-pending" or . == "ci-ready" or . == "ci-failed" or . == "dry-run")] | sort' "$live_issue_file") + if [[ "$live_title" != "$title" \ + || "$has_accepted" != "true" \ + || "$has_ci_pending" != "true" \ + || "$live_security_labels" != "$expected_security_labels" ]] \ + || ! cmp -s "$body_file" "$live_body_file"; then + echo " Publish request changed after approval; revoking approval." + gh issue edit "$number" -R "$GITHUB_REPOSITORY" \ + --remove-label "accepted" \ + --remove-label "ci-pending" + gh issue comment "$number" -R "$GITHUB_REPOSITORY" \ + --body "The publish request changed after approval. Review it and re-add the **accepted** label to retry." + exit 0 + fi + echo " CI passed! Adding ci-ready label." gh issue edit "$number" -R "$GITHUB_REPOSITORY" \ --remove-label "ci-pending" \ diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 2635ea6..269044e 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -32,6 +32,68 @@ jobs: && github.event.issue.state == 'open' && startsWith(github.event.issue.title, 'publish: ') steps: + - name: Get publish code + uses: actions/checkout@v7 + with: + persist-credentials: false + + - name: Setup Node + uses: actions/setup-node@v6 + with: + node-version: 24 + cache: yarn + + - name: Install yarn dependencies + run: yarn install --frozen-lockfile + + - name: Parse target repository + id: target + continue-on-error: true + run: node src/publish/inputs.js + + - name: Get target repository token + id: target-token + if: steps.target.outcome == 'success' + continue-on-error: true + uses: actions/create-github-app-token@v3 + with: + client-id: ${{ vars.SENTRY_RELEASE_BOT_CLIENT_ID }} + private-key: ${{ secrets.SENTRY_RELEASE_BOT_PRIVATE_KEY }} + owner: getsentry + repositories: ${{ fromJSON(steps.target.outputs.result).repo }} + + - name: Authorize approval + id: authorization + if: steps.target.outcome == 'success' + continue-on-error: true + env: + APPROVAL_ACTOR: ${{ github.actor }} + TARGET_REPOSITORY: ${{ fromJSON(steps.target.outputs.result).repo }} + TARGET_REPOSITORY_PATH: ${{ fromJSON(steps.target.outputs.result).path }} + TARGET_REPOSITORY_TOKEN: ${{ steps.target-token.outputs.token }} + run: node src/publish/authorize-approval.js + + - name: Reject invalid approval + if: >- + always() + && ( + steps.target.outcome != 'success' + || steps.authorization.outcome != 'success' + || steps.authorization.outputs.authorized != 'true' + ) + env: + GH_TOKEN: ${{ github.token }} + run: | + gh issue edit "${{ github.event.issue.number }}" \ + -R "$GITHUB_REPOSITORY" \ + --remove-label "accepted" \ + --remove-label "ci-pending" \ + --remove-label "ci-ready" + gh issue comment "${{ github.event.issue.number }}" \ + -R "$GITHUB_REPOSITORY" \ + --body "Approval rejected. The approver needs write access to the target repository." + exit 1 + - name: Get auth token id: token uses: actions/create-github-app-token@v3 @@ -48,6 +110,7 @@ jobs: # Label ops are idempotent: --add/--remove don't fail if already # added/removed. - name: Mark ci-pending + id: mark-pending env: GH_TOKEN: ${{ steps.token.outputs.token }} run: | @@ -57,6 +120,42 @@ jobs: --remove-label "ci-ready" \ --add-label "ci-pending" + - name: Revoke stranded approval + if: >- + always() + && steps.authorization.outputs.authorized == 'true' + && ( + steps.token.outcome != 'success' + || steps.mark-pending.outcome != 'success' + ) + env: + GH_TOKEN: ${{ github.token }} + run: | + for attempt in 1 2 3; do + gh issue edit "${{ github.event.issue.number }}" \ + -R "$GITHUB_REPOSITORY" \ + --remove-label "accepted" || true + gh issue edit "${{ github.event.issue.number }}" \ + -R "$GITHUB_REPOSITORY" \ + --remove-label "ci-pending" || true + gh issue edit "${{ github.event.issue.number }}" \ + -R "$GITHUB_REPOSITORY" \ + --remove-label "ci-ready" || true + if labels=$(gh issue view "${{ github.event.issue.number }}" \ + -R "$GITHUB_REPOSITORY" \ + --json labels \ + --jq '.labels[].name'); then + if ! grep -Fxq "accepted" <<< "$labels" \ + && ! grep -Fxq "ci-pending" <<< "$labels" \ + && ! grep -Fxq "ci-ready" <<< "$labels"; then + exit 0 + fi + fi + echo "::warning::Approval cleanup attempt ${attempt} failed." + done + echo "::error::Could not revoke the stranded approval." + exit 1 + - name: Comment on issue env: GH_TOKEN: ${{ github.token }} @@ -135,6 +234,26 @@ jobs: id: inputs run: node .__publish__/src/publish/inputs.js + - name: Get approval target token + id: approval-target-token + uses: actions/create-github-app-token@v3 + with: + client-id: ${{ vars.SENTRY_RELEASE_BOT_CLIENT_ID }} + private-key: ${{ secrets.SENTRY_RELEASE_BOT_PRIVATE_KEY }} + owner: getsentry + repositories: ${{ fromJSON(steps.inputs.outputs.result).repo }} + + - name: Verify approved target + env: + APPROVAL_ISSUE_NUMBER: ${{ github.event.issue.number }} + APPROVAL_ISSUE_REPOSITORY: ${{ github.repository }} + APPROVAL_TOKEN: ${{ github.token }} + REQUIRE_AUTHORIZED: "true" + TARGET_REPOSITORY: ${{ fromJSON(steps.inputs.outputs.result).repo }} + TARGET_REPOSITORY_PATH: ${{ fromJSON(steps.inputs.outputs.result).path }} + TARGET_REPOSITORY_TOKEN: ${{ steps.approval-target-token.outputs.token }} + run: node .__publish__/src/publish/authorize-approval.js + - name: Resolve CI-approved release revision id: release-revision env: diff --git a/README.md b/README.md index 4cbc9b9..6b73661 100644 --- a/README.md +++ b/README.md @@ -28,14 +28,14 @@ flowchart TD 1. We do not want employees to publish through their own accounts 1. We do not want employees to have access to the global credentials 1. We do not want employees to build and publish releases from their machines -1. We want releases to require formal approvals from a limited set of release managers +1. We want releases to require formal approval from someone with write access to the target repository 1. We want all the above to not discourage from any engineer initiating a release ## Usage 1. Go to your repo and trigger the workflow (example: https://github.com/getsentry/sentry/actions/manual?workflow=.github%2Fworkflows%2Frelease.yml) 1. Once the workflow finishes, see the publishing request in this repo (example: #40) -1. Add the [**`accepted`**](https://github.com/getsentry/publish/labels/accepted) label to initiate publishing. Since this action requires elevated permissions, you may need to ask your team lead or manager +1. Add the [**`accepted`**](https://github.com/getsentry/publish/labels/accepted) label to initiate publishing. Approvers need write access to the target repository 1. Observe the issue for information about the triggered run 1. The issue will automatically be closed when publishing succeeds diff --git a/src/modules/__tests__/approval-authorizer.js b/src/modules/__tests__/approval-authorizer.js new file mode 100644 index 0000000..0a1aa40 --- /dev/null +++ b/src/modules/__tests__/approval-authorizer.js @@ -0,0 +1,186 @@ +import { describe, expect, test, vi } from "vitest"; + +const { + AUTO_APPROVAL_LABELER, + authorizeApproval, +} = require("../approval-authorizer.js"); + +describe("authorizeApproval", () => { + test.each([ + ["write", "write"], + ["write", "maintain"], + ["admin", "admin"], + ])("allows a user with %s/%s access", async (permission, roleName) => { + const getPermission = vi.fn().mockResolvedValue({ + permission, + role_name: roleName, + }); + + await expect( + authorizeApproval({ + actor: "approver", + repository: "sentry-javascript", + getPermission, + }) + ).resolves.toEqual({ + authorized: true, + repository: "getsentry/sentry-javascript", + }); + }); + + test("allows a target repository writer to approve their own request", async () => { + const getPermission = vi.fn().mockResolvedValue({ + permission: "write", + role_name: "write", + }); + + await expect( + authorizeApproval({ + actor: "publisher", + repository: "sentry-javascript", + getPermission, + }) + ).resolves.toEqual({ + authorized: true, + repository: "getsentry/sentry-javascript", + }); + }); + + test("rejects users without write access", async () => { + const getPermission = vi.fn().mockResolvedValue({ + permission: "read", + role_name: "read", + }); + + await expect( + authorizeApproval({ + actor: "reader", + repository: "sentry-javascript", + getPermission, + }) + ).resolves.toEqual({ + authorized: false, + repository: "getsentry/sentry-javascript", + }); + }); + + test("fails closed on a contradictory role name", async () => { + const getPermission = vi.fn().mockResolvedValue({ + permission: "read", + role_name: "admin", + }); + + await expect( + authorizeApproval({ + actor: "reader", + repository: "sentry-javascript", + getPermission, + }) + ).resolves.toEqual({ + authorized: false, + repository: "getsentry/sentry-javascript", + }); + }); + + test("preserves the existing automated approval path", async () => { + const getPermission = vi.fn(); + + await expect( + authorizeApproval({ + actor: AUTO_APPROVAL_LABELER, + publishPath: ".", + repository: "relay", + getPermission, + }) + ).resolves.toEqual({ + authorized: true, + repository: "getsentry/relay", + }); + expect(getPermission).not.toHaveBeenCalled(); + }); + + test.each([ + ["sentry-javascript", "."], + ["objectstore", "."], + ["objectstore", "./other"], + ])( + "rejects automated approval outside the allowlist: %s%s", + async (repository, publishPath) => { + const getPermission = vi.fn(); + + await expect( + authorizeApproval({ + actor: AUTO_APPROVAL_LABELER, + publishPath, + repository, + getPermission, + }) + ).resolves.toEqual({ + authorized: false, + repository: `getsentry/${repository}`, + }); + expect(getPermission).not.toHaveBeenCalled(); + } + ); + + test("preserves an allowlisted automated monorepo path", async () => { + const getPermission = vi.fn(); + + await expect( + authorizeApproval({ + actor: AUTO_APPROVAL_LABELER, + publishPath: "./clients", + repository: "objectstore", + getPermission, + }) + ).resolves.toEqual({ + authorized: true, + repository: "getsentry/objectstore", + }); + expect(getPermission).not.toHaveBeenCalled(); + }); + + test.each(["github-actions[bot]", "unlisted-app[bot]"])( + "rejects direct approval by %s", + async (actor) => { + const getPermission = vi.fn(); + + await expect( + authorizeApproval({ + actor, + repository: "sentry-javascript", + getPermission, + }) + ).resolves.toEqual({ + authorized: false, + repository: "getsentry/sentry-javascript", + }); + expect(getPermission).not.toHaveBeenCalled(); + } + ); + + test("rejects an invalid target repository", async () => { + const getPermission = vi.fn(); + + await expect( + authorizeApproval({ + actor: "approver", + repository: "../private", + getPermission, + }) + ).resolves.toEqual({ authorized: false, repository: null }); + expect(getPermission).not.toHaveBeenCalled(); + }); + + test("propagates permission lookup failures", async () => { + const getPermission = vi.fn().mockRejectedValue(new Error("unavailable")); + + await expect( + authorizeApproval({ + actor: "approver", + repository: "sentry-javascript", + getPermission, + }) + ).rejects.toThrow("unavailable"); + }); +}); diff --git a/src/modules/__tests__/ci-poller-input.js b/src/modules/__tests__/ci-poller-input.js index 140ccef..ea39b88 100644 --- a/src/modules/__tests__/ci-poller-input.js +++ b/src/modules/__tests__/ci-poller-input.js @@ -21,6 +21,7 @@ test("parses root and full-path publish titles", () => { title: "publish: getsentry/toolkit/cli@1.2.3", }) ).toEqual({ + path: "./cli", repo: "getsentry/toolkit", revision: REVISION, version: "1.2.3", @@ -32,6 +33,7 @@ test("parses root and full-path publish titles", () => { title: "publish: toolkit/packages/cli/v2@1.2.3", }) ).toEqual({ + path: "./packages/cli/v2", repo: "getsentry/toolkit", revision: REVISION, version: "1.2.3", @@ -54,7 +56,7 @@ test("uses the shared resolver in the CI poller", () => { test.each(["-toolkit", ".", "..", "__proto__"])( "rejects unsafe repository identity before CI API calls: %s", - repo => { + (repo) => { expect(() => getCiPollerInput({ issueBody, @@ -66,7 +68,7 @@ test.each(["-toolkit", ".", "..", "__proto__"])( test.each(["--config", "1.2"])( "rejects invalid release version before CI API calls: %s", - version => { + (version) => { expect(() => getCiPollerInput({ issueBody, @@ -78,7 +80,7 @@ test.each(["--config", "1.2"])( test.each(["publish: getsentry/@1.2.3", "publish: getsentry/toolkit$@1.2.3"])( "rejects malformed repository identity before CI API calls: %s", - title => { + (title) => { expect(() => getCiPollerInput({ issueBody, title })).toThrow( "Invalid publish issue title" ); @@ -90,7 +92,7 @@ test.each([ "publish: getsentry/toolkit/./other@1.2.3", "publish: getsentry/toolkit/__proto__/other@1.2.3", "publish: getsentry/toolkit/--config@1.2.3", -])("rejects unsafe publish path before CI API calls: %s", title => { +])("rejects unsafe publish path before CI API calls: %s", (title) => { expect(() => getCiPollerInput({ issueBody, title })).toThrow( "Invalid publish issue path" ); diff --git a/src/modules/__tests__/ci-poller-workflow.js b/src/modules/__tests__/ci-poller-workflow.js index c859f5a..f869018 100644 --- a/src/modules/__tests__/ci-poller-workflow.js +++ b/src/modules/__tests__/ci-poller-workflow.js @@ -36,7 +36,15 @@ function writeExecutable(path, content) { writeFileSync(path, content, { mode: 0o755 }); } -function runPoller({ initialExit = 0, updateOutput = "", issueBody = "" } = {}) { +function runPoller({ + authorizationExit = 0, + initialExit = 0, + updateOutput = "", + issueBody = "", + liveIssueBody = issueBody, + initialLabels = ["accepted", "ci-pending"], + liveLabels = initialLabels, +} = {}) { const directory = mkdtempSync(join(tmpdir(), "ci-poller-workflow-test-")); temporaryDirectories.push(directory); const binDirectory = join(directory, "bin"); @@ -51,7 +59,9 @@ function runPoller({ initialExit = 0, updateOutput = "", issueBody = "" } = {}) join(binDirectory, "node"), `#!/usr/bin/env bash set -eu -if [[ -n "\${PUBLISH_REVISION:-}" ]]; then +if [[ "\${REQUIRE_AUTHORIZED:-}" == "true" ]]; then + exit "\${AUTHORIZATION_EXIT:-0}" +elif [[ -n "\${PUBLISH_REVISION:-}" ]]; then printf '%s' "\${NODE_UPDATE_OUTPUT:-}" elif [[ "\${NODE_INITIAL_EXIT:-0}" != "0" ]]; then exit "\${NODE_INITIAL_EXIT}" @@ -83,6 +93,9 @@ case "$*" in "issue list "*) printf '%s' "$GH_ISSUES" ;; + "issue view "*) + printf '%s' "$GH_LIVE_ISSUE" + ;; *"/check-suites"*) printf '%s' "release/1.2.3" ;; @@ -110,36 +123,51 @@ esac const issue = JSON.stringify([ { body: issueBody, + labels: initialLabels.map((name) => ({ name })), number: 1, title: "publish: getsentry/toolkit@1.2.3", }, ]); + const liveIssue = JSON.stringify({ + body: liveIssueBody, + labels: liveLabels.map((name) => ({ name })), + title: "publish: getsentry/toolkit@1.2.3", + }); const initialOutput = JSON.stringify({ + path: ".", repo: "getsentry/toolkit", revision: INITIAL_REVISION, version: "1.2.3", }); - const result = spawnSync("bash", ["-e", "-o", "pipefail", "-c", getPollerScript()], { - encoding: "utf8", - env: { - ...process.env, - CAPTURED_BODY: capturedBody, - GH_LOG: logFile, - GH_ISSUES: issue, - GITHUB_REPOSITORY: "getsentry/publish", - NODE_INITIAL_EXIT: String(initialExit), - NODE_INITIAL_OUTPUT: initialOutput, - NODE_UPDATE_OUTPUT: updateOutput, - PATH: `${binDirectory}:${process.env.PATH}`, - TEMPORARY_FILES: temporaryFiles, - }, - }); + const result = spawnSync( + "bash", + ["-e", "-o", "pipefail", "-c", getPollerScript()], + { + encoding: "utf8", + env: { + ...process.env, + AUTHORIZATION_EXIT: String(authorizationExit), + CAPTURED_BODY: capturedBody, + GH_LOG: logFile, + GH_ISSUES: issue, + GH_LIVE_ISSUE: liveIssue, + GITHUB_REPOSITORY: "getsentry/publish", + NODE_INITIAL_EXIT: String(initialExit), + NODE_INITIAL_OUTPUT: initialOutput, + NODE_UPDATE_OUTPUT: updateOutput, + PATH: `${binDirectory}:${process.env.PATH}`, + TEMPORARY_FILES: temporaryFiles, + }, + } + ); return { capturedBody, log: readFileSync(logFile, "utf8"), result, - temporaryFiles: readdirSync(temporaryFiles).filter(file => file !== "counter"), + temporaryFiles: readdirSync(temporaryFiles).filter( + (file) => file !== "counter" + ), }; } @@ -176,13 +204,16 @@ test("skips an issue without editing it when the rewrite response body is empty" test.each([ ["malformed JSON", "not JSON"], ["a non-object JSON value", "[]"], -])("skips an issue without editing it when the rewrite response is %s", (_name, updateOutput) => { - const poller = runPoller({ updateOutput }); +])( + "skips an issue without editing it when the rewrite response is %s", + (_name, updateOutput) => { + const poller = runPoller({ updateOutput }); - expect(poller.result.status, poller.result.stderr).toBe(0); - expect(poller.log).not.toContain("--body-file"); - expect(poller.temporaryFiles).toEqual([]); -}); + expect(poller.result.status, poller.result.stderr).toBe(0); + expect(poller.log).not.toContain("--body-file"); + expect(poller.temporaryFiles).toEqual([]); + } +); test("preserves all rewrite body bytes outside the revision", () => { const body = "canonical issue body\n\n"; @@ -195,3 +226,49 @@ test("preserves all rewrite body bytes outside the revision", () => { expect(readFileSync(poller.capturedBody, "utf8")).toBe(body); expect(poller.temporaryFiles).toEqual([]); }); + +test("revokes approval when the request changes before ci-ready", () => { + const body = "canonical issue body\n"; + const poller = runPoller({ + issueBody: body, + liveIssueBody: "changed issue body\n", + updateOutput: JSON.stringify({ issueBody: body }), + }); + + expect(poller.result.status, poller.result.stderr).toBe(0); + expect(poller.log).toContain( + "--remove-label accepted --remove-label ci-pending" + ); + expect(poller.log).not.toContain("--add-label ci-ready"); +}); + +test("revokes approval when the approver loses target access", () => { + const body = "canonical issue body\n"; + const poller = runPoller({ + authorizationExit: 1, + issueBody: body, + updateOutput: JSON.stringify({ issueBody: body }), + }); + + expect(poller.result.status, poller.result.stderr).toBe(0); + expect(poller.log).toContain( + "--remove-label accepted --remove-label ci-pending" + ); + expect(poller.log).not.toContain("--add-label ci-ready"); +}); + +test("revokes approval when dry-run changes before ci-ready", () => { + const body = "canonical issue body\n"; + const poller = runPoller({ + initialLabels: ["accepted", "ci-pending", "dry-run"], + issueBody: body, + liveLabels: ["accepted", "ci-pending"], + updateOutput: JSON.stringify({ issueBody: body }), + }); + + expect(poller.result.status, poller.result.stderr).toBe(0); + expect(poller.log).toContain( + "--remove-label accepted --remove-label ci-pending" + ); + expect(poller.log).not.toContain("--add-label ci-ready"); +}); diff --git a/src/modules/__tests__/publish-workflow.js b/src/modules/__tests__/publish-workflow.js index 76a8826..4cfcc16 100644 --- a/src/modules/__tests__/publish-workflow.js +++ b/src/modules/__tests__/publish-workflow.js @@ -62,6 +62,7 @@ afterEach(() => { test("uses the CI-approved revision for checkout, location resolution, and publishing", () => { const workflow = getWorkflow(); + const publishJob = workflow.slice(workflow.indexOf(" publish:")); const revision = workflow.indexOf( "name: Resolve CI-approved release revision" ); @@ -80,7 +81,7 @@ test("uses the CI-approved revision for checkout, location resolution, and publi expect(workflow).toContain( "ref: ${{ steps.release-revision.outputs.revision }}" ); - expect(workflow.match(/actions\/checkout@v7/g)).toHaveLength(2); + expect(publishJob.match(/actions\/checkout@v7/g)).toHaveLength(2); expect(workflow).toContain( "node .__publish__/src/publish/discover-location.js" ); @@ -101,10 +102,18 @@ test("publishes only on a fresh CI-ready label event", () => { const workflow = getWorkflow(); expect(workflow).toContain("github.event.label.name == 'ci-ready'"); - expect(workflow).toContain("contains(github.event.issue.labels.*.name, 'accepted')"); - expect(workflow).toContain("contains(github.event.issue.labels.*.name, 'ci-ready')"); - expect(workflow).toContain("!contains(github.event.issue.labels.*.name, 'ci-pending')"); - expect(workflow).toContain("!contains(github.event.issue.labels.*.name, 'ci-failed')"); + expect(workflow).toContain( + "contains(github.event.issue.labels.*.name, 'accepted')" + ); + expect(workflow).toContain( + "contains(github.event.issue.labels.*.name, 'ci-ready')" + ); + expect(workflow).toContain( + "!contains(github.event.issue.labels.*.name, 'ci-pending')" + ); + expect(workflow).toContain( + "!contains(github.event.issue.labels.*.name, 'ci-failed')" + ); }); test("uses Craft's legacy root state filename", () => { diff --git a/src/modules/approval-authorizer.js b/src/modules/approval-authorizer.js new file mode 100644 index 0000000..4844c4e --- /dev/null +++ b/src/modules/approval-authorizer.js @@ -0,0 +1,60 @@ +const fs = require("fs"); +const path = require("path"); + +const { isPublishPath } = require("./publish-location"); +const { isPublishRepository } = require("./publish-issue-validation"); + +const AUTO_APPROVAL_LABELER = "sentry-internal-app[bot]"; +const AUTO_APPROVAL_TARGETS = new Set( + fs + .readFileSync( + path.resolve(__dirname, "../../auto-approve-repos.txt"), + "utf8" + ) + .split(/\r?\n/) + .filter(Boolean) +); + +async function authorizeApproval({ + actor, + repository, + publishPath = ".", + getPermission, +}) { + if (!isPublishRepository(repository)) { + return { authorized: false, repository: null }; + } + + const fullRepository = `getsentry/${repository}`; + if (!isPublishPath(publishPath)) { + return { authorized: false, repository: fullRepository }; + } + if (actor === AUTO_APPROVAL_LABELER) { + const pathSuffix = publishPath === "." ? "" : publishPath.slice(1); + return { + authorized: AUTO_APPROVAL_TARGETS.has(`${fullRepository}${pathSuffix}`), + repository: fullRepository, + }; + } + + if (typeof actor !== "string" || /\[bot\]$/i.test(actor)) { + return { authorized: false, repository: fullRepository }; + } + + const permission = await getPermission({ + owner: "getsentry", + repository, + username: actor, + }); + const hasWriteAccess = + permission.permission === "write" || permission.permission === "admin"; + return { + authorized: hasWriteAccess, + repository: fullRepository, + }; +} + +module.exports = { + AUTO_APPROVAL_LABELER, + authorizeApproval, +}; diff --git a/src/modules/ci-poller-input.js b/src/modules/ci-poller-input.js index ca08a72..8fa3d2b 100644 --- a/src/modules/ci-poller-input.js +++ b/src/modules/ci-poller-input.js @@ -33,6 +33,7 @@ function getCiPollerInput({ title, issueBody, revision }) { ...(revision ? { issueBody: updateReleaseRevision({ issueBody, repo, revision }) } : {}), + path, repo: `getsentry/${repo}`, revision: currentRevision, version, diff --git a/src/publish/__tests__/approval-workflow.js b/src/publish/__tests__/approval-workflow.js new file mode 100644 index 0000000..5bcf913 --- /dev/null +++ b/src/publish/__tests__/approval-workflow.js @@ -0,0 +1,136 @@ +import { readFileSync } from "fs"; +import { describe, expect, test } from "vitest"; + +const workflow = readFileSync(".github/workflows/publish.yml", "utf8"); + +describe("publish approval workflow", () => { + test("authorizes the approver against the target repository first", () => { + const waitingForCi = workflow.slice( + workflow.indexOf(" waiting-for-ci:"), + workflow.indexOf(" publish:") + ); + const parseTarget = waitingForCi.indexOf("name: Parse target repository"); + const targetToken = waitingForCi.indexOf( + "name: Get target repository token" + ); + const authorize = waitingForCi.indexOf("name: Authorize approval"); + const markPending = waitingForCi.indexOf("name: Mark ci-pending"); + + expect(parseTarget).toBeGreaterThan(-1); + expect(targetToken).toBeGreaterThan(parseTarget); + expect(authorize).toBeGreaterThan(targetToken); + expect(markPending).toBeGreaterThan(authorize); + expect(waitingForCi.slice(targetToken, authorize)).toContain( + "repositories: ${{ fromJSON(steps.target.outputs.result).repo }}" + ); + expect(waitingForCi.slice(authorize, markPending)).toContain( + "node src/publish/authorize-approval.js" + ); + expect(waitingForCi.slice(authorize, markPending)).toContain( + "TARGET_REPOSITORY_TOKEN: ${{ steps.target-token.outputs.token }}" + ); + expect(waitingForCi.slice(authorize, markPending)).toContain( + "TARGET_REPOSITORY_PATH: ${{ fromJSON(steps.target.outputs.result).path }}" + ); + }); + + test("revokes an invalid approval before changing CI state", () => { + const waitingForCi = workflow.slice( + workflow.indexOf(" waiting-for-ci:"), + workflow.indexOf(" publish:") + ); + const authorize = waitingForCi.indexOf("name: Authorize approval"); + const reject = waitingForCi.indexOf("name: Reject invalid approval"); + const markPending = waitingForCi.indexOf("name: Mark ci-pending"); + + expect(reject).toBeGreaterThan(authorize); + expect(reject).toBeLessThan(markPending); + expect(waitingForCi.slice(reject, markPending)).toContain( + "steps.authorization.outputs.authorized != 'true'" + ); + expect(waitingForCi.slice(reject, markPending)).toContain( + '--remove-label "accepted"' + ); + }); + + test("revokes approval when setup or ci-pending fails", () => { + const waitingForCi = workflow.slice( + workflow.indexOf(" waiting-for-ci:"), + workflow.indexOf(" publish:") + ); + const markPending = waitingForCi.indexOf("name: Mark ci-pending"); + const cleanup = waitingForCi.indexOf("name: Revoke stranded approval"); + + expect(cleanup).toBeGreaterThan(markPending); + expect(waitingForCi.slice(cleanup)).toContain("always()"); + expect(waitingForCi.slice(cleanup)).toContain( + "steps.authorization.outputs.authorized == 'true'" + ); + expect(waitingForCi.slice(cleanup)).toContain('--remove-label "accepted"'); + expect(waitingForCi.slice(cleanup)).toContain( + '--remove-label "ci-pending"' + ); + expect(waitingForCi.slice(cleanup)).toContain('--remove-label "ci-ready"'); + expect(waitingForCi.slice(cleanup)).toContain("for attempt in 1 2 3; do"); + expect(waitingForCi.slice(cleanup)).toContain( + 'gh issue view "${{ github.event.issue.number }}"' + ); + }); + + test("clears pending authorization on every rejected approval", () => { + const waitingForCi = workflow.slice( + workflow.indexOf(" waiting-for-ci:"), + workflow.indexOf(" publish:") + ); + const reject = waitingForCi.slice( + waitingForCi.indexOf("name: Reject invalid approval"), + waitingForCi.indexOf("name: Get auth token") + ); + + expect(reject).toContain("always()"); + expect(reject).toContain("steps.target.outcome != 'success'"); + expect(reject).toContain('--remove-label "accepted"'); + expect(reject).toContain('--remove-label "ci-pending"'); + }); + + test("revalidates the live request and approver before ci-ready", () => { + const poller = readFileSync(".github/workflows/ci-poller.yml", "utf8"); + const ciPassed = poller.slice( + poller.indexOf('if [[ "$status_ok" == "true"'), + poller.indexOf("elif [[", poller.indexOf('if [[ "$status_ok" == "true"')) + ); + + expect(ciPassed).toContain("# Verify live approval"); + expect(ciPassed).toContain('REQUIRE_AUTHORIZED="true"'); + expect(ciPassed).toContain('TARGET_REPOSITORY_PATH="$path"'); + expect(ciPassed).toContain( + "node .__publish__/src/publish/authorize-approval.js" + ); + expect( + ciPassed.indexOf("node .__publish__/src/publish/authorize-approval.js") + ).toBeLessThan(ciPassed.indexOf('gh issue view "$number"')); + expect(ciPassed).toContain("expected_security_labels"); + expect(ciPassed).toContain("live_security_labels"); + expect(ciPassed.indexOf("Verify live approval")).toBeLessThan( + ciPassed.indexOf('--add-label "ci-ready"') + ); + }); + + test("re-authorizes the ci-ready event target before target checkout", () => { + const publishJob = workflow.slice(workflow.indexOf(" publish:")); + const verification = publishJob.indexOf("name: Verify approved target"); + const checkout = publishJob.indexOf("name: Check out target repo"); + + expect(verification).toBeGreaterThan(0); + expect(verification).toBeLessThan(checkout); + expect(publishJob.slice(verification, checkout)).toContain( + 'REQUIRE_AUTHORIZED: "true"' + ); + expect(publishJob.slice(verification, checkout)).toContain( + "node .__publish__/src/publish/authorize-approval.js" + ); + expect(publishJob.slice(verification, checkout)).toContain( + "TARGET_REPOSITORY_PATH: ${{ fromJSON(steps.inputs.outputs.result).path }}" + ); + }); +}); diff --git a/src/publish/__tests__/authorize-approval.js b/src/publish/__tests__/authorize-approval.js new file mode 100644 index 0000000..4b09a26 --- /dev/null +++ b/src/publish/__tests__/authorize-approval.js @@ -0,0 +1,120 @@ +import { afterEach, describe, expect, test, vi } from "vitest"; +import fs from "fs"; + +const { + currentAcceptedActor, + getPermission, + main, +} = require("../authorize-approval.js"); + +const originalEnvironment = { ...process.env }; + +afterEach(() => { + process.env = { ...originalEnvironment }; + vi.restoreAllMocks(); +}); + +describe("authorize approval entry point", () => { + test("uses the actor from the current accepted-label transition", () => { + expect( + currentAcceptedActor([ + { + actor: { login: "first-approver" }, + event: "labeled", + label: { name: "accepted" }, + }, + { + actor: { login: "automation" }, + event: "unlabeled", + label: { name: "accepted" }, + }, + { + actor: { login: "current-approver" }, + event: "labeled", + label: { name: "accepted" }, + }, + ]) + ).toBe("current-approver"); + }); + + test("rejects an accepted label that was removed", () => { + expect( + currentAcceptedActor([ + { + actor: { login: "approver" }, + event: "labeled", + label: { name: "accepted" }, + }, + { + actor: { login: "automation" }, + event: "unlabeled", + label: { name: "accepted" }, + }, + ]) + ).toBeNull(); + }); + + test("uses the target-scoped token and authorizes a writer", async () => { + process.env.GITHUB_OUTPUT = "/tmp/github-output"; + process.env.APPROVAL_ACTOR = "approver"; + process.env.TARGET_REPOSITORY = "sentry-javascript"; + process.env.TARGET_REPOSITORY_PATH = "."; + process.env.TARGET_REPOSITORY_TOKEN = "target-token"; + global.fetch = vi.fn().mockResolvedValue({ + ok: true, + json: vi.fn().mockResolvedValue({ + permission: "write", + role_name: "write", + }), + }); + const appendFileSync = vi + .spyOn(fs, "appendFileSync") + .mockImplementation(() => {}); + + await main(); + + expect(global.fetch).toHaveBeenCalledWith( + "https://api.github.com/repos/getsentry/sentry-javascript/collaborators/approver/permission", + expect.objectContaining({ + headers: expect.objectContaining({ + Authorization: "Bearer target-token", + }), + }) + ); + expect(appendFileSync).toHaveBeenCalledWith( + "/tmp/github-output", + "authorized=true\n" + ); + }); + + test("fails closed when the target token is missing", async () => { + delete process.env.TARGET_REPOSITORY_TOKEN; + global.fetch = vi.fn(); + + await expect( + getPermission({ + owner: "getsentry", + repository: "sentry-javascript", + username: "approver", + }) + ).rejects.toThrow( + 'No "TARGET_REPOSITORY_TOKEN" environment variable found' + ); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + test("does not expose permission details when GitHub rejects the lookup", async () => { + process.env.TARGET_REPOSITORY_TOKEN = "target-token"; + global.fetch = vi.fn().mockResolvedValue({ ok: false, status: 403 }); + + await expect( + getPermission({ + owner: "getsentry", + repository: "sentry-javascript", + username: "approver", + }) + ).rejects.toThrow( + "Could not verify approval permission: GitHub returned 403" + ); + }); +}); diff --git a/src/publish/authorize-approval.js b/src/publish/authorize-approval.js new file mode 100644 index 0000000..92105e8 --- /dev/null +++ b/src/publish/authorize-approval.js @@ -0,0 +1,141 @@ +const fs = require("fs"); + +const { authorizeApproval } = require("../modules/approval-authorizer"); +const { isPublishRepository } = require("../modules/publish-issue-validation"); + +async function getGitHubJson(url, token, description) { + const response = await fetch(url, { + headers: { + Accept: "application/vnd.github+json", + Authorization: `Bearer ${token}`, + "X-GitHub-Api-Version": "2026-03-10", + }, + }); + + if (!response.ok) { + throw new Error( + `Could not ${description}: GitHub returned ${response.status}` + ); + } + + return response.json(); +} + +async function getPermission({ owner, repository, username }) { + if (!process.env.TARGET_REPOSITORY_TOKEN) { + throw new Error('No "TARGET_REPOSITORY_TOKEN" environment variable found'); + } + + return getGitHubJson( + `https://api.github.com/repos/${encodeURIComponent( + owner + )}/${encodeURIComponent(repository)}/collaborators/${encodeURIComponent( + username + )}/permission`, + process.env.TARGET_REPOSITORY_TOKEN, + "verify approval permission" + ); +} + +async function getAllIssueEvents({ repository, issueNumber, token, page = 1 }) { + const [owner, name, extra] = repository.split("/"); + if ( + owner !== "getsentry" || + !isPublishRepository(name) || + extra || + !/^[1-9][0-9]*$/.test(issueNumber) + ) { + throw new Error("Invalid approval issue input"); + } + + const events = await getGitHubJson( + `https://api.github.com/repos/${encodeURIComponent( + owner + )}/${encodeURIComponent( + name + )}/issues/${issueNumber}/events?per_page=100&page=${page}`, + token, + "retrieve approval events" + ); + if (!Array.isArray(events)) { + throw new Error("Invalid approval events response"); + } + if (events.length < 100) { + return events; + } + + return events.concat( + await getAllIssueEvents({ repository, issueNumber, token, page: page + 1 }) + ); +} + +function currentAcceptedActor(events) { + return events.reduce((actor, event) => { + if (event?.label?.name !== "accepted") { + return actor; + } + if (event.event === "unlabeled") { + return null; + } + if (event.event === "labeled") { + return event.actor?.login || null; + } + return actor; + }, null); +} + +async function getCurrentAcceptedActor() { + if (!process.env.APPROVAL_TOKEN) { + throw new Error('No "APPROVAL_TOKEN" environment variable found'); + } + + const events = await getAllIssueEvents({ + repository: process.env.APPROVAL_ISSUE_REPOSITORY || "", + issueNumber: process.env.APPROVAL_ISSUE_NUMBER || "", + token: process.env.APPROVAL_TOKEN, + }); + return currentAcceptedActor(events); +} + +async function main() { + const requireAuthorized = process.env.REQUIRE_AUTHORIZED === "true"; + if (!process.env.GITHUB_OUTPUT && !requireAuthorized) { + throw new Error('No "GITHUB_OUTPUT" environment variable found'); + } + + const actor = process.env.APPROVAL_ACTOR || (await getCurrentAcceptedActor()); + const publishPath = process.env.TARGET_REPOSITORY_PATH || ""; + if (!publishPath) { + throw new Error("Invalid approval authorization input"); + } + const { authorized } = await authorizeApproval({ + actor, + publishPath, + repository: process.env.TARGET_REPOSITORY, + getPermission, + }); + if (process.env.GITHUB_OUTPUT) { + fs.appendFileSync( + process.env.GITHUB_OUTPUT, + `authorized=${authorized ? "true" : "false"}\n` + ); + } + if (requireAuthorized && !authorized) { + throw new Error("Approval is not authorized for the target repository"); + } +} + +if (require.main === module) { + main().catch((error) => { + console.error(error); + process.exitCode = 1; + }); +} + +module.exports = { + currentAcceptedActor, + getAllIssueEvents, + getCurrentAcceptedActor, + getPermission, + main, +};