From 6a44da050f0f2a9a382101c2457387de1d9ddc2f Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Thu, 8 Oct 2026 15:35:54 -0300 Subject: [PATCH 1/2] feat(repo-settings): release environment for the ccf-release-bot secrets RELEASE_BOT_PRIVATE_KEY was an org secret visible to the workflows repo and every mock, so any workflow on any branch of those repos (or a same-repo PR's) could read the key of an app installed on every org repo. The in-workflow "Check the token scope" steps only bound workflows that run them. REPO_ADMIN_PRIVATE_KEY had the same exposure in the workflows repo, guarded only by steps inside repo-settings.yml. - repo-settings manages an environment `release` on every manifest repo: custom deployment policies (the default branch and the v*.*.* / *-v*.*.* release tags, which only the bot creates; other policies are deleted) and the secrets RELEASE_BOT_APP_ID and RELEASE_BOT_PRIVATE_KEY, written when missing (or all with --rotate-secrets) after the policies are in place, encrypted with the environment's public key (nacl sealed box). A secret is never written to an environment whose policy steps failed. - Every job that mints a ccf-release-bot token names the environment: release-please, cut-prerelease and release-finished (the caller's), and this repo's train, renovate, ccf-bump sync/merge, attention digest and vuln summary (main only). The workflow_call secrets are optional, so callers keep `secrets: inherit` and the environment secret wins. - repo-settings.yml runs in a `repo-admin` environment (main only, required reviewers; set up by hand) holding the ccf-repo-admin key and the release-bot values it copies, which only an apply receives. Its token adds Actions read and Environments read/write, so ccf-repo-admin can keep fixed permissions instead of being raised to write for each apply. - internal/ciworkflows: every job that reads an app private key names the environment that holds it. - README: the environments, the app's permissions and the cut-over steps. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/attention-digest.yml | 3 + .github/workflows/ccf-bump-merge.yml | 3 + .github/workflows/ccf-bump-sync.yml | 3 + .github/workflows/cut-prerelease.yml | 11 +- .github/workflows/release-action.yml | 8 +- .github/workflows/release-finished.yml | 11 +- .github/workflows/release-go-image.yml | 8 +- .github/workflows/release-go-lib.yml | 8 +- .github/workflows/release-go-plugin.yml | 8 +- .github/workflows/release-helm.yml | 8 +- .github/workflows/release-please.yml | 11 +- .github/workflows/release-policies.yml | 8 +- .github/workflows/release-ui.yml | 8 +- .github/workflows/renovate.yml | 3 + .github/workflows/repo-settings.yml | 25 ++- .github/workflows/train.yml | 4 + .github/workflows/vuln-summary.yml | 3 + README.md | 84 +++++++--- cmd/repo-settings/main.go | 22 ++- cmd/repo-settings/main_test.go | 30 +++- docs/ccf-bump.md | 2 +- docs/renovate.md | 2 +- docs/train.md | 2 +- docs/vuln-summary.md | 2 +- go.mod | 2 +- internal/ciworkflows/environment_test.go | 63 ++++++++ internal/reposettings/desired.go | 45 ++++++ internal/reposettings/environment_test.go | 147 ++++++++++++++++++ internal/reposettings/github.go | 123 ++++++++++++++- internal/reposettings/github_test.go | 98 ++++++++++++ internal/reposettings/plan.go | 177 +++++++++++++++++++++- internal/reposettings/plan_test.go | 98 +++++++++++- 32 files changed, 941 insertions(+), 89 deletions(-) create mode 100644 internal/ciworkflows/environment_test.go create mode 100644 internal/reposettings/environment_test.go diff --git a/.github/workflows/attention-digest.yml b/.github/workflows/attention-digest.yml index 99502e4..15205a9 100644 --- a/.github/workflows/attention-digest.yml +++ b/.github/workflows/attention-digest.yml @@ -31,6 +31,9 @@ jobs: digest: name: digest runs-on: ubuntu-latest + # This repo's release environment holds the ccf-release-bot secrets; only main may use it + # (repo-settings), so a workflow on another branch can't read them. + environment: release timeout-minutes: 15 env: # A scheduled run has no inputs: keep this fallback equal to the input's default. diff --git a/.github/workflows/ccf-bump-merge.yml b/.github/workflows/ccf-bump-merge.yml index 4061cb4..6b288ba 100644 --- a/.github/workflows/ccf-bump-merge.yml +++ b/.github/workflows/ccf-bump-merge.yml @@ -31,6 +31,9 @@ jobs: merge: name: merge runs-on: ubuntu-latest + # This repo's release environment holds the ccf-release-bot secrets; only main may use it + # (repo-settings), so a workflow on another branch can't read them. + environment: release timeout-minutes: 15 env: # A scheduled run has no inputs: keep this fallback equal to the input's default. diff --git a/.github/workflows/ccf-bump-sync.yml b/.github/workflows/ccf-bump-sync.yml index 40cf310..1c8d6b8 100644 --- a/.github/workflows/ccf-bump-sync.yml +++ b/.github/workflows/ccf-bump-sync.yml @@ -31,6 +31,9 @@ jobs: sync: name: sync runs-on: ubuntu-latest + # This repo's release environment holds the ccf-release-bot secrets; only main may use it + # (repo-settings), so a workflow on another branch can't read them. + environment: release # --batch 10 waits an hour after every 10 PRs. timeout-minutes: 300 steps: diff --git a/.github/workflows/cut-prerelease.yml b/.github/workflows/cut-prerelease.yml index d76d028..7ae3cfd 100644 --- a/.github/workflows/cut-prerelease.yml +++ b/.github/workflows/cut-prerelease.yml @@ -17,11 +17,11 @@ on: default: v secrets: RELEASE_BOT_APP_ID: - description: "ccf-release-bot App ID" - required: true + description: "ccf-release-bot App ID (the caller\'s `release` environment secret is used first)" + required: false RELEASE_BOT_PRIVATE_KEY: - description: "ccf-release-bot private key" - required: true + description: "ccf-release-bot private key (the caller\'s `release` environment secret is used first)" + required: false outputs: tag: description: "The release-candidate tag created" @@ -33,6 +33,9 @@ jobs: cut: name: cut runs-on: ubuntu-latest + # The caller's release environment holds the ccf-release-bot secrets; only the default branch + # and release tags may use it (repo-settings), so a workflow on another branch can't read them. + environment: release timeout-minutes: 10 permissions: contents: read diff --git a/.github/workflows/release-action.yml b/.github/workflows/release-action.yml index 0023f34..16be2cd 100644 --- a/.github/workflows/release-action.yml +++ b/.github/workflows/release-action.yml @@ -13,11 +13,11 @@ on: default: v secrets: RELEASE_BOT_APP_ID: - description: "ccf-release-bot App ID" - required: true + description: "ccf-release-bot App ID (the caller\'s `release` environment secret is used first)" + required: false RELEASE_BOT_PRIVATE_KEY: - description: "ccf-release-bot private key" - required: true + description: "ccf-release-bot private key (the caller\'s `release` environment secret is used first)" + required: false permissions: {} diff --git a/.github/workflows/release-finished.yml b/.github/workflows/release-finished.yml index 4cf385c..ea492d2 100644 --- a/.github/workflows/release-finished.yml +++ b/.github/workflows/release-finished.yml @@ -17,11 +17,11 @@ on: default: '[{}]' secrets: RELEASE_BOT_APP_ID: - description: "ccf-release-bot App ID" - required: true + description: "ccf-release-bot App ID (the caller\'s `release` environment secret is used first)" + required: false RELEASE_BOT_PRIVATE_KEY: - description: "ccf-release-bot private key" - required: true + description: "ccf-release-bot private key (the caller\'s `release` environment secret is used first)" + required: false permissions: {} @@ -30,6 +30,9 @@ jobs: name: dispatch if: always() # also when the run is cancelled runs-on: ubuntu-latest + # The caller's release environment holds the ccf-release-bot secrets; only the default branch + # and release tags may use it (repo-settings), so a workflow on another branch can't read them. + environment: release timeout-minutes: 10 permissions: {} env: diff --git a/.github/workflows/release-go-image.yml b/.github/workflows/release-go-image.yml index a3313de..e1b2775 100644 --- a/.github/workflows/release-go-image.yml +++ b/.github/workflows/release-go-image.yml @@ -18,11 +18,11 @@ on: default: v secrets: RELEASE_BOT_APP_ID: - description: "ccf-release-bot App ID" - required: true + description: "ccf-release-bot App ID (the caller\'s `release` environment secret is used first)" + required: false RELEASE_BOT_PRIVATE_KEY: - description: "ccf-release-bot private key" - required: true + description: "ccf-release-bot private key (the caller\'s `release` environment secret is used first)" + required: false permissions: {} diff --git a/.github/workflows/release-go-lib.yml b/.github/workflows/release-go-lib.yml index 5090a98..12e1e69 100644 --- a/.github/workflows/release-go-lib.yml +++ b/.github/workflows/release-go-lib.yml @@ -10,11 +10,11 @@ on: workflow_call: secrets: RELEASE_BOT_APP_ID: - description: "ccf-release-bot App ID" - required: true + description: "ccf-release-bot App ID (the caller\'s `release` environment secret is used first)" + required: false RELEASE_BOT_PRIVATE_KEY: - description: "ccf-release-bot private key" - required: true + description: "ccf-release-bot private key (the caller\'s `release` environment secret is used first)" + required: false permissions: {} diff --git a/.github/workflows/release-go-plugin.yml b/.github/workflows/release-go-plugin.yml index 1d5c8f5..5d0d84d 100644 --- a/.github/workflows/release-go-plugin.yml +++ b/.github/workflows/release-go-plugin.yml @@ -20,11 +20,11 @@ on: default: "2" secrets: RELEASE_BOT_APP_ID: - description: "ccf-release-bot App ID" - required: true + description: "ccf-release-bot App ID (the caller\'s `release` environment secret is used first)" + required: false RELEASE_BOT_PRIVATE_KEY: - description: "ccf-release-bot private key" - required: true + description: "ccf-release-bot private key (the caller\'s `release` environment secret is used first)" + required: false permissions: {} diff --git a/.github/workflows/release-helm.yml b/.github/workflows/release-helm.yml index ba88ebb..1e13a93 100644 --- a/.github/workflows/release-helm.yml +++ b/.github/workflows/release-helm.yml @@ -18,11 +18,11 @@ on: default: oci://ghcr.io/compliance-framework/helm-charts secrets: RELEASE_BOT_APP_ID: - description: "ccf-release-bot App ID" - required: true + description: "ccf-release-bot App ID (the caller\'s `release` environment secret is used first)" + required: false RELEASE_BOT_PRIVATE_KEY: - description: "ccf-release-bot private key" - required: true + description: "ccf-release-bot private key (the caller\'s `release` environment secret is used first)" + required: false permissions: {} diff --git a/.github/workflows/release-please.yml b/.github/workflows/release-please.yml index 7653f1b..41c9bfd 100644 --- a/.github/workflows/release-please.yml +++ b/.github/workflows/release-please.yml @@ -7,11 +7,11 @@ on: workflow_call: secrets: RELEASE_BOT_APP_ID: - description: "ccf-release-bot App ID" - required: true + description: "ccf-release-bot App ID (the caller\'s `release` environment secret is used first)" + required: false RELEASE_BOT_PRIVATE_KEY: - description: "ccf-release-bot private key" - required: true + description: "ccf-release-bot private key (the caller\'s `release` environment secret is used first)" + required: false outputs: releases-created: description: "true when this run created at least one release" @@ -26,6 +26,9 @@ jobs: release-please: name: release-please runs-on: ubuntu-latest + # The caller's release environment holds the ccf-release-bot secrets; only the default branch + # and release tags may use it (repo-settings), so a workflow on another branch can't read them. + environment: release timeout-minutes: 10 permissions: contents: read # check out release-please-config.json for the defaults check diff --git a/.github/workflows/release-policies.yml b/.github/workflows/release-policies.yml index bf34df6..819ca27 100644 --- a/.github/workflows/release-policies.yml +++ b/.github/workflows/release-policies.yml @@ -22,11 +22,11 @@ on: default: v secrets: RELEASE_BOT_APP_ID: - description: "ccf-release-bot App ID" - required: true + description: "ccf-release-bot App ID (the caller\'s `release` environment secret is used first)" + required: false RELEASE_BOT_PRIVATE_KEY: - description: "ccf-release-bot private key" - required: true + description: "ccf-release-bot private key (the caller\'s `release` environment secret is used first)" + required: false permissions: {} diff --git a/.github/workflows/release-ui.yml b/.github/workflows/release-ui.yml index 147f2a5..893ce19 100644 --- a/.github/workflows/release-ui.yml +++ b/.github/workflows/release-ui.yml @@ -16,11 +16,11 @@ on: default: v secrets: RELEASE_BOT_APP_ID: - description: "ccf-release-bot App ID" - required: true + description: "ccf-release-bot App ID (the caller\'s `release` environment secret is used first)" + required: false RELEASE_BOT_PRIVATE_KEY: - description: "ccf-release-bot private key" - required: true + description: "ccf-release-bot private key (the caller\'s `release` environment secret is used first)" + required: false permissions: {} diff --git a/.github/workflows/renovate.yml b/.github/workflows/renovate.yml index f839a40..eea8fe1 100644 --- a/.github/workflows/renovate.yml +++ b/.github/workflows/renovate.yml @@ -41,6 +41,9 @@ jobs: renovate: name: renovate runs-on: ubuntu-latest + # This repo's release environment holds the ccf-release-bot secrets; only main may use it + # (repo-settings), so a workflow on another branch can't read them. + environment: release timeout-minutes: 60 env: # A scheduled run has no inputs: keep these fallbacks equal to the dispatch defaults. diff --git a/.github/workflows/repo-settings.yml b/.github/workflows/repo-settings.yml index 06da591..6af885d 100644 --- a/.github/workflows/repo-settings.yml +++ b/.github/workflows/repo-settings.yml @@ -1,8 +1,11 @@ name: repo-settings -# Brings the manifest repos' merge settings, security settings, Actions settings and rulesets to the -# desired state (internal/reposettings). Prints a per-repo diff; writes only with apply: true, which a human runs -# after granting ccf-repo-admin Administration write (README: "Repo settings"). +# Brings the manifest repos' merge settings, security settings, Actions settings, rulesets and +# release environments (with the ccf-release-bot secrets) to the desired state +# (internal/reposettings). Prints a per-repo diff; writes only with apply: true, from main. The +# job runs in this repo's repo-admin environment: its secrets (ccf-repo-admin's key and the +# ccf-release-bot secrets it copies) are released only to a run on main a reviewer approves +# (README: "Repo settings"). on: workflow_dispatch: @@ -28,6 +31,10 @@ on: description: Status check context the ccf-required ruleset requires type: string default: ci / required + rotate-secrets: + description: Rewrite every release environment secret, not only missing ones (after a key rotation) + type: boolean + default: false permissions: contents: read @@ -40,6 +47,10 @@ jobs: sync: name: sync runs-on: ubuntu-latest + # Holds REPO_ADMIN_APP_ID/REPO_ADMIN_PRIVATE_KEY and the RELEASE_BOT_APP_ID/RELEASE_BOT_PRIVATE_KEY + # values copied into each repo's release environment. Main only, with required reviewers: + # ccf-repo-admin keeps Administration write, so its key must not reach any other run. + environment: repo-admin timeout-minutes: 15 steps: - name: Only apply from main @@ -74,6 +85,8 @@ jobs: owner: compliance-framework repositories: ${{ steps.select.outputs.repos }} permission-administration: ${{ inputs.apply && 'write' || 'read' }} + permission-actions: read # environments and their deployment policies + permission-environments: ${{ inputs.apply && 'write' || 'read' }} # environment secrets - name: Sync repo settings env: GH_TOKEN: ${{ steps.token.outputs.token }} @@ -82,6 +95,10 @@ jobs: APPLY: ${{ inputs.apply }} BYPASS_APP_ID: ${{ inputs.release-bot-app-id || vars.RELEASE_BOT_APP_ID }} REQUIRED_CHECK: ${{ inputs.required-check }} + ROTATE_SECRETS: ${{ inputs.rotate-secrets }} + # The values written to the release environments; a dry run doesn't get them. + RELEASE_BOT_APP_ID: ${{ inputs.apply && secrets.RELEASE_BOT_APP_ID || '' }} + RELEASE_BOT_PRIVATE_KEY: ${{ inputs.apply && secrets.RELEASE_BOT_PRIVATE_KEY || '' }} run: | go run ./cmd/repo-settings sync --manifest "$MANIFEST" --repos "$REPOS" --apply="$APPLY" \ - --bypass-app-id "$BYPASS_APP_ID" --required-check "$REQUIRED_CHECK" + --bypass-app-id "$BYPASS_APP_ID" --required-check "$REQUIRED_CHECK" --rotate-secrets="$ROTATE_SECRETS" diff --git a/.github/workflows/train.yml b/.github/workflows/train.yml index 3874bb9..f4eab11 100644 --- a/.github/workflows/train.yml +++ b/.github/workflows/train.yml @@ -48,6 +48,10 @@ jobs: startsWith(github.event.comment.body, '/') && github.event.comment.user.type != 'Bot') runs-on: ubuntu-latest + # This repo's release environment holds the ccf-release-bot secrets; only main may use it + # (repo-settings), so a workflow on another branch can't read them. Comments, schedules and + # repository_dispatch run on main. + environment: release timeout-minutes: 60 # One run at a time, at the job level so that skipped runs (other comments) don't queue. # GitHub keeps one pending run and cancels older pending ones; every run reads the state and diff --git a/.github/workflows/vuln-summary.yml b/.github/workflows/vuln-summary.yml index d6c35dc..507d2a9 100644 --- a/.github/workflows/vuln-summary.yml +++ b/.github/workflows/vuln-summary.yml @@ -26,6 +26,9 @@ jobs: summary: name: summary runs-on: ubuntu-latest + # This repo's release environment holds the ccf-release-bot secrets; only main may use it + # (repo-settings), so a workflow on another branch can't read them. + environment: release timeout-minutes: 10 env: # A scheduled run has no inputs: keep this fallback equal to the input's default. diff --git a/README.md b/README.md index be55072..aa2637f 100644 --- a/README.md +++ b/README.md @@ -308,8 +308,11 @@ The bot is installed on every org repo, so the job refuses to mint a token when is empty or `github.repository` isn't `compliance-framework/`: a token minted with an owner and no repositories would reach the whole org. -Secrets: `RELEASE_BOT_APP_ID` and `RELEASE_BOT_PRIVATE_KEY`, via `secrets: inherit`. Outputs: -`releases-created` and `paths-released` (JSON list of package paths), from the action. +Secrets: `RELEASE_BOT_APP_ID` and `RELEASE_BOT_PRIVATE_KEY`, from the caller's `release` +environment (the job names it; [Repo settings](#repo-settings) creates it, limited to the default +branch and release tags, and copies the secrets in). Callers keep `secrets: inherit`: the +`workflow_call` secrets are optional, and an environment secret wins over an inherited one. +Outputs: `releases-created` and `paths-released` (JSON list of package paths), from the action. **Shared defaults.** `release-please/defaults.json` holds the settings every repo uses: `bump-minor-pre-major: true` (breaking changes bump the minor before 1.0), @@ -495,8 +498,8 @@ A repo calls its kind's release workflow when a release is published: by release candidate, both as ccf-release-bot so the event starts workflows. Each one decides its tags from the release's tag name (`github.event.release.tag_name`), never from the release's `prerelease` flag (rules in `internal/release`, `ReleaseTags`), and ends with -[`release-finished.yml`](#release-finishedyml). They need `secrets: inherit` for the -release-bot secrets. +[`release-finished.yml`](#release-finishedyml). Its `dispatch` job reads the release-bot secrets +from the caller's `release` environment (release tags may use it); callers keep `secrets: inherit`. | Workflow | Kind | Publishes | | --- | --- | --- | @@ -656,7 +659,7 @@ The tail of every release workflow, called as a job with `needs` on every other Inputs: `needs` (the caller's `toJSON(needs)`) and `images` (the packages to prune, as for `publish-image.yml`; only `name` is read). Secrets: `RELEASE_BOT_APP_ID` and -`RELEASE_BOT_PRIVATE_KEY`. Permissions: `packages: write`. +`RELEASE_BOT_PRIVATE_KEY`, from the caller's `release` environment (the `dispatch` job). Permissions: `packages: write`. ## Releasing workflows @@ -708,6 +711,14 @@ desired state, defined in `internal/reposettings`: without a prerelease suffix): only `ccf-release-bot` creates, moves or deletes them. Release workflows run from the tag, so nobody else can start one on an unreviewed commit. Floating major tags (`v1`) don't match. +- **Environment `release`**: only the default branch and the tags `v*.*.*` and `*-v*.*.*` may use + it (custom deployment branch and tag policies; any other is deleted), and it holds the secrets + `RELEASE_BOT_APP_ID` and `RELEASE_BOT_PRIVATE_KEY`. Every job that mints a ccf-release-bot token + names it (`release-please.yml`, `cut-prerelease.yml`, `release-finished.yml`, and this repo's + train, Renovate, ccf-bump, attention digest and vulnerability summary), so a workflow pushed to + any other branch, or a PR's, is refused before it starts and never sees the key. Missing + secrets are written (encrypted with the environment's public key); present ones are left alone + unless `rotate-secrets` is on. Its other protection rules (reviewers, wait timer) aren't managed. Every repo in the manifest gets these settings, the `workflows` repo (`release: false`) too: every caller's CI and releases run its code. Rulesets are matched by name; other rulesets are left alone. @@ -725,11 +736,24 @@ Inputs: | `apply` | `false` | `false` prints the diff and writes nothing; `true` writes it (from `main` only). | | `release-bot-app-id` | org variable `RELEASE_BOT_APP_ID` | `ccf-release-bot`'s app ID, the `ccf-review` bypass actor. Set the org variable (Settings, Secrets and variables, Actions, Variables) to the app's ID from its settings page, or pass the input; the run fails if both are empty. | | `required-check` | `ci / required` | The status check context `ccf-required` requires. | - -Secrets: `REPO_ADMIN_APP_ID` and `REPO_ADMIN_PRIVATE_KEY` (`ccf-repo-admin`). The app is installed -on every org repo, so the job lists the selected repos first and mints a token scoped to exactly -those (`repositories:`, with Administration `read`, or `write` when applying). It refuses an empty -selection, and the tool checks the token reaches no other repo before reading anything. +| `rotate-secrets` | `false` | Rewrite every `release` environment secret, not only missing ones: after rotating the ccf-release-bot key. | + +**The `repo-admin` environment.** The job runs in this repo's `repo-admin` environment, which holds +`REPO_ADMIN_APP_ID` and `REPO_ADMIN_PRIVATE_KEY` (`ccf-repo-admin`) and the ccf-release-bot +`RELEASE_BOT_APP_ID` and `RELEASE_BOT_PRIVATE_KEY` it copies into each repo's `release` +environment (only on apply; a dry run doesn't get them). Configure it by hand once (it is the one +thing this tool doesn't manage): deployment branches `main` only, required reviewers (the org +admins), and "Prevent self-review" on. Then every run, dry runs included, waits for an admin's +approval, and no other branch or workflow can read the keys. + +**ccf-repo-admin's permissions** stay set, so a run (and, later, onboarding a new repo) works with +no permission change: repository **Administration: Read and write** (settings, rulesets, +environments and their policies, Actions settings), **Environments: Read and write** (environment +secrets), **Actions: Read** (reading environments and policies) and Metadata: Read. The app is +installed on every org repo, so the job lists the selected repos first and mints a token scoped to +exactly those (`repositories:`), with only read access for a dry run (Administration and +Environments `write` when applying). It refuses an empty selection, and the tool checks the token +reaches no other repo before reading anything. Output, per repo, is one line per differing setting, `key: current -> desired`, then a total: @@ -775,16 +799,36 @@ the end if any step failed. How to run it: -1. **Dry run** (anyone, any time): Actions, `repo-settings`, Run workflow, with `apply` off. Or - `gh workflow run repo-settings.yml -f manifest=repos.mock.yaml`. `ccf-repo-admin` has - Administration **read** only, so a dry run can't change anything. Review the diff. -2. **Apply** (a human, never an agent): in the `ccf-repo-admin` app settings, temporarily grant - **Administration: Read and write** and accept the new permission on the org installation. Run - the workflow from `main` with the same inputs and `apply` on. Run it again with `apply` on: - every repo should be `up to date` and nothing is written (the sync is idempotent). A dry run - can't show this, since its token is always read-only: a repo that is otherwise in sync shows - `0 change(s), 10 unknown` (7 merge settings, 3 ruleset bypass lists). Then set Administration back to **read**. - Do the mocks first. +1. **Dry run**: Actions, `repo-settings`, Run workflow, with `apply` off. Or + `gh workflow run repo-settings.yml -f manifest=repos.mock.yaml`. An admin approves the + `repo-admin` deployment; the token is read-only, so a dry run can't change anything. Review + the diff. +2. **Apply** (a human, never an agent): run it from `main` with the same inputs and `apply` on, + and approve the deployment. Run it again with `apply` on: every repo should be `up to date` and + nothing is written (the sync is idempotent). A dry run can't show this, since its token is + read-only: a repo that is otherwise in sync shows `0 change(s), 10 unknown` (7 merge settings, + 3 ruleset bypass lists). Do the mocks first. + +**Moving the ccf-release-bot secrets into the environments** (once; the mocks first): + +1. Grant `ccf-repo-admin` the permissions above (app settings, Permissions; then accept them on + the org installation) and create the `repo-admin` environment with its four secrets. Generate a + new ccf-release-bot private key for it (app settings, Private keys): the old one was readable + by every workflow of every repo the org secret reached. +2. Merge this change; the jobs that mint a token now name the `release` environment. Until a + repo's environment exists, GitHub creates an empty one on first use and the job still gets + the org secret, so nothing breaks in between. +3. Apply `repo-settings` (`repos.mock.yaml`, then `repos.yaml`): each repo gets its `release` + environment, its policies, then the secrets. Check a release on a mock (release-please on + `main`, the release workflow on its tag). +4. Remove the repos from the org secrets `RELEASE_BOT_APP_ID`/`RELEASE_BOT_PRIVATE_KEY` (or + delete them) and the org secrets `REPO_ADMIN_*`, then delete the old private keys from both + apps. + +Callers need nothing new: `secrets: inherit` stays (it passes `SLACK_BOT_TOKEN`, which stays an +org secret since `notify-failure.yml` runs on PRs), and the reusable workflows read the +release-bot secrets from the environment. The `workflows` repo's own scheduled tools only run +from `main` now; a dispatch from another branch is refused. The workflow calls `go run ./cmd/repo-settings sync`. Flags it doesn't expose: `--owner` (default `compliance-framework`), `--required-check-app-id` (default `15368`, GitHub Actions; `0` diff --git a/cmd/repo-settings/main.go b/cmd/repo-settings/main.go index 1a587ef..010ebc6 100644 --- a/cmd/repo-settings/main.go +++ b/cmd/repo-settings/main.go @@ -1,6 +1,6 @@ -// Command repo-settings brings the merge settings, security settings, Actions settings and rulesets -// of the manifest's repos to the desired state in internal/reposettings, for the repo-settings -// workflow: +// Command repo-settings brings the merge settings, security settings, Actions settings, rulesets and +// environments of the manifest's repos to the desired state in internal/reposettings, for the +// repo-settings workflow: // // repo-settings list [--manifest repos.yaml] [--repos a,b] // repo-settings sync [--manifest repos.yaml] [--repos a,b] --bypass-app-id ID [--apply] [flags] @@ -8,7 +8,8 @@ // list prints the selected repo names, comma-separated, to scope the token. sync prints each repo's // diff of current vs desired settings, and writes the changes only with --apply. Both select every // repo in the manifest (the workflows repo too, whatever its release flag) unless --repos names a -// subset. sync reads the token from GH_TOKEN. +// subset. sync reads the token from GH_TOKEN, and the values of the environment secrets it writes +// (reposettings.ReleaseBotSecrets) from the variables of the same names. package main import ( @@ -56,6 +57,7 @@ func run(ctx context.Context, args []string, getenv func(string) string, stdout checkApp := fs.Int64("required-check-app-id", reposettings.GitHubActionsAppID, "app that must report the required check (0: any)") bypass := fs.String("bypass-app-id", "", "ccf-release-bot app ID, the ccf-review bypass actor (sync)") scope := fs.Bool("check-token-scope", true, "fail if the token reaches repos outside the selection (needs an installation token)") + rotate := fs.Bool("rotate-secrets", false, "rewrite every environment secret, not only missing ones (after a key rotation)") if err := fs.Parse(args[1:]); err != nil { return err } @@ -79,15 +81,23 @@ func run(ctx context.Context, args []string, getenv func(string) string, stdout if err != nil { return fmt.Errorf("--bypass-app-id %q: want the ccf-release-bot app's numeric ID", *bypass) } - desired, err := reposettings.DesiredState(reposettings.Config{RequiredCheck: *check, RequiredCheckAppID: *checkApp, BypassAppID: bypassID}) + desired, err := reposettings.DesiredState(reposettings.Config{RequiredCheck: *check, RequiredCheckAppID: *checkApp, BypassAppID: bypassID, RotateSecrets: *rotate}) if err != nil { return err } if getenv("GH_TOKEN") == "" { return errors.New("GH_TOKEN is not set") } + secrets := map[string]string{} + for _, e := range desired.Environments { + for _, s := range e.Secrets { + if v := getenv(s); v != "" { + secrets[s] = v + } + } + } return reposettings.Run(ctx, client(getenv), reposettings.Options{ - Owner: *owner, Repos: selected, Desired: desired, Apply: *apply, CheckTokenScope: *scope, + Owner: *owner, Repos: selected, Desired: desired, Apply: *apply, CheckTokenScope: *scope, Secrets: secrets, }, stdout) } diff --git a/cmd/repo-settings/main_test.go b/cmd/repo-settings/main_test.go index 992c75a..d4a6b72 100644 --- a/cmd/repo-settings/main_test.go +++ b/cmd/repo-settings/main_test.go @@ -3,6 +3,8 @@ package main import ( "bytes" "context" + "crypto/rand" + "encoding/base64" "io" "net/http" "net/http/httptest" @@ -10,6 +12,8 @@ import ( "strings" "testing" + "golang.org/x/crypto/nacl/box" + "github.com/compliance-framework/workflows/internal/manifest" "github.com/compliance-framework/workflows/internal/reposettings" ) @@ -80,8 +84,12 @@ func TestRunSyncErrors(t *testing.T) { } // TestRunSyncFailedStepFailsRun: a refused write fails the run (main exits 1) after the other -// steps ran. +// steps ran. The release environment's secrets come from the variables of the same names. func TestRunSyncFailedStepFailsRun(t *testing.T) { + pub, _, err := box.GenerateKey(rand.Reader) + if err != nil { + t.Fatal(err) + } var writes []string srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { const repo = "/repos/compliance-framework/mock-api" @@ -96,7 +104,11 @@ func TestRunSyncFailedStepFailsRun(t *testing.T) { } switch r.URL.Path { case repo: - _, _ = io.WriteString(w, `{"allow_merge_commit":true}`) + _, _ = io.WriteString(w, `{"allow_merge_commit":true,"default_branch":"main"}`) + case repo + "/environments/release": + http.NotFound(w, r) + case repo + "/environments/release/secrets/public-key": + _, _ = io.WriteString(w, `{"key_id":"k","key":"`+base64.StdEncoding.EncodeToString(pub[:])+`"}`) case repo + "/automated-security-fixes": _, _ = io.WriteString(w, `{"enabled":false,"paused":false}`) case repo + "/rulesets": @@ -115,16 +127,20 @@ func TestRunSyncFailedStepFailsRun(t *testing.T) { } args := []string{"sync", "--manifest", "../../repos.mock.yaml", "--repos", "mock-api", "--bypass-app-id", "42", "--check-token-scope=false", "--apply"} var out bytes.Buffer - err := run(context.Background(), args, func(string) string { return "t" }, &out, client) + err = run(context.Background(), args, func(string) string { return "t" }, &out, client) if err == nil || !strings.Contains(err.Error(), "repo merge settings") { t.Fatalf("err = %v, want the failed merge settings step", err) } - if want := []string{"PATCH /repos/compliance-framework/mock-api", "POST /repos/compliance-framework/mock-api/rulesets", "POST /repos/compliance-framework/mock-api/rulesets", - "POST /repos/compliance-framework/mock-api/rulesets", "PUT /repos/compliance-framework/mock-api/actions/permissions/workflow", - "PUT /repos/compliance-framework/mock-api/actions/permissions/fork-pr-contributor-approval"}; !slices.Equal(writes, want) { + const repo = "/repos/compliance-framework/mock-api" + if want := []string{"PATCH " + repo, "POST " + repo + "/rulesets", "POST " + repo + "/rulesets", "POST " + repo + "/rulesets", + "PUT " + repo + "/actions/permissions/workflow", "PUT " + repo + "/actions/permissions/fork-pr-contributor-approval", + "PUT " + repo + "/environments/release", "POST " + repo + "/environments/release/deployment-branch-policies", + "POST " + repo + "/environments/release/deployment-branch-policies", "POST " + repo + "/environments/release/deployment-branch-policies", + "PUT " + repo + "/environments/release/secrets/RELEASE_BOT_APP_ID", "PUT " + repo + "/environments/release/secrets/RELEASE_BOT_PRIVATE_KEY", + }; !slices.Equal(writes, want) { t.Errorf("writes = %v, want %v", writes, want) } - if !strings.Contains(out.String(), "partly applied: 1 of 6 step(s) failed") { + if !strings.Contains(out.String(), "partly applied: 1 of 12 step(s) failed") { t.Errorf("output:\n%s", out.String()) } } diff --git a/docs/ccf-bump.md b/docs/ccf-bump.md index 6aff139..3a6c818 100644 --- a/docs/ccf-bump.md +++ b/docs/ccf-bump.md @@ -151,7 +151,7 @@ them first is harmless. (default `true`). Scheduled runs read `repos.mock.yaml` until go-live (W1-S4-T03), like renovate, train and attention-digest (the fallback equals the input's default), and are dry runs until the repo variable `CCF_BUMP_SYNC_LIVE` is `true`. It lists the repos, then mints a `ccf-release-bot` token scoped to -exactly those (`RELEASE_BOT_APP_ID`/`RELEASE_BOT_PRIVATE_KEY`): read-only for dry runs; contents, +exactly those (`RELEASE_BOT_APP_ID`/`RELEASE_BOT_PRIVATE_KEY`, this repo's `release` environment secrets; main only): read-only for dry runs; contents, pull requests and workflows write otherwise (bumps edit workflow files), and checks read. Commits are authored by the bot. A live run then runs `ccf-bump merge --wait 20m` over the same repos. diff --git a/docs/renovate.md b/docs/renovate.md index 388eea0..022d4db 100644 --- a/docs/renovate.md +++ b/docs/renovate.md @@ -102,7 +102,7 @@ repo variable `RENOVATE_LIVE` is `"true"`, and `full` otherwise. So by default a daily `full` dry run on the mocks, and setting `RENOVATE_LIVE` makes it live on the mocks, the same switch as `CCF_BUMP_SYNC_LIVE` and `TRAIN_LIVE`. A dispatch always follows its `dry_run` input. A live scheduled run is also what merges Renovate's green PRs (see above), so leave it on once -Renovate is live. Secrets: `RELEASE_BOT_APP_ID` and `RELEASE_BOT_PRIVATE_KEY`. Variable: +Renovate is live. Secrets: `RELEASE_BOT_APP_ID` and `RELEASE_BOT_PRIVATE_KEY`, from this repo's `release` environment (main only). Variable: `RENOVATE_LIVE`. ### Commit statuses diff --git a/docs/train.md b/docs/train.md index 88daf58..efca99e 100644 --- a/docs/train.md +++ b/docs/train.md @@ -136,7 +136,7 @@ A train started while another is open only moves the open one on: one train at a ### Tokens - `GITHUB_TOKEN` with `issues: write`: the tracking issue. -- ccf-release-bot (`RELEASE_BOT_APP_ID`/`RELEASE_BOT_PRIVATE_KEY`), scoped to exactly the train's +- ccf-release-bot (`RELEASE_BOT_APP_ID`/`RELEASE_BOT_PRIVATE_KEY`, this repo's `release` environment secrets; main only), scoped to exactly the train's repos (`train select` lists them; an open train's repos come from its state and must be in its manifest with `release: true`). Read-only for a dry run; otherwise contents, pull requests, issues (chart issues) and actions (`/retry`) write, workflows write (bumps edit workflow files), diff --git a/docs/vuln-summary.md b/docs/vuln-summary.md index e6fa5a3..375ef74 100644 --- a/docs/vuln-summary.md +++ b/docs/vuln-summary.md @@ -31,7 +31,7 @@ Severity is the advisory's (`critical`, `high`, `medium`, `low`); anything else | Name | Kind | What | | --- | --- | --- | -| `RELEASE_BOT_APP_ID`, `RELEASE_BOT_PRIVATE_KEY` | secrets | `ccf-release-bot`, to read the alerts. | +| `RELEASE_BOT_APP_ID`, `RELEASE_BOT_PRIVATE_KEY` | `release` environment secrets (main only) | `ccf-release-bot`, to read the alerts. | | `SLACK_BOT_TOKEN` | secret | Bot token with `chat:write`. Without it the run prints the summary and posts nothing. | | `SLACK_CHANNEL_VULNS` | variable | Channel ID to post to. Empty means `SLACK_CHANNEL_CI_FAILURES`. | diff --git a/go.mod b/go.mod index 908980f..51febe0 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/hashicorp/go-hclog v1.6.3 github.com/hashicorp/go-plugin v1.7.0 github.com/open-policy-agent/opa v1.14.1 + golang.org/x/crypto v0.52.0 golang.org/x/mod v0.41.0 google.golang.org/grpc v1.79.3 ) @@ -78,7 +79,6 @@ require ( go.uber.org/multierr v1.11.0 // indirect go.uber.org/zap v1.27.1 // indirect go.yaml.in/yaml/v2 v2.4.4 // indirect - golang.org/x/crypto v0.52.0 // indirect golang.org/x/net v0.55.0 // indirect golang.org/x/sync v0.21.0 // indirect golang.org/x/sys v0.45.0 // indirect diff --git a/internal/ciworkflows/environment_test.go b/internal/ciworkflows/environment_test.go new file mode 100644 index 0000000..a3a6e53 --- /dev/null +++ b/internal/ciworkflows/environment_test.go @@ -0,0 +1,63 @@ +package ciworkflows_test + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "go.yaml.in/yaml/v3" +) + +// TestAppKeysOnlyInEnvironments: every job that reads an app private key names the environment +// that holds it (repo-settings limits each to the default branch, and release tags for release), +// so a workflow on another branch never gets the key from an org or repo secret by mistake. +func TestAppKeysOnlyInEnvironments(t *testing.T) { + files, err := filepath.Glob(filepath.Join("..", "..", ".github", "workflows", "*.yml")) + if err != nil { + t.Fatal(err) + } + want := map[string]string{"RELEASE_BOT_PRIVATE_KEY": "release", "REPO_ADMIN_PRIVATE_KEY": "repo-admin"} + found := 0 + for _, file := range files { + var wf struct { + Jobs map[string]yaml.Node `yaml:"jobs"` + } + b, err := os.ReadFile(file) + if err != nil { + t.Fatal(err) + } + if err := yaml.Unmarshal(b, &wf); err != nil { + t.Fatal(err) + } + for name, node := range wf.Jobs { + var job struct { + Environment string `yaml:"environment"` + Uses string `yaml:"uses"` + Steps []struct { + Uses string `yaml:"uses"` + With map[string]string `yaml:"with"` + } `yaml:"steps"` + } + if err := node.Decode(&job); err != nil { + t.Fatalf("%s: job %s: %v", file, name, err) + } + for _, s := range job.Steps { + if !strings.HasPrefix(s.Uses, "actions/create-github-app-token@") { + continue + } + for secret, env := range want { + if strings.Contains(s.With["private-key"], "secrets."+secret) { + found++ + if job.Environment != env { + t.Errorf("%s: job %s reads %s outside the %s environment (environment %q)", filepath.Base(file), name, secret, env, job.Environment) + } + } + } + } + } + } + if found < 10 { + t.Errorf("found %d token mints, want every workflow that mints one checked", found) + } +} diff --git a/internal/reposettings/desired.go b/internal/reposettings/desired.go index 84dc543..9e41422 100644 --- a/internal/reposettings/desired.go +++ b/internal/reposettings/desired.go @@ -26,6 +26,19 @@ var ReleaseTagPatterns = []string{"v*.*.*", "*-v*.*.*"} // contributor's fork PR run, not only a first-time contributor's. const ForkPRApprovalAll = "all_external_contributors" +// ReleaseEnvironment holds the secrets that mint ccf-release-bot tokens. The jobs that mint one +// (release-please, cut-prerelease, release-finished and this repo's scheduled tools) name it, so +// only code on the default branch or a release tag (ccf-release-tags: created by the bot only) +// can read them; a workflow on any other branch or PR is refused before it starts. +const ReleaseEnvironment = "release" + +// ReleaseBotSecrets are the release environment's secrets. Their values are the repo-settings +// job's (the workflows repo's repo-admin environment), passed to Run in Options.Secrets. +var ReleaseBotSecrets = []string{"RELEASE_BOT_APP_ID", "RELEASE_BOT_PRIVATE_KEY"} + +// DefaultBranchPattern in Environment.Branches stands for the repo's default branch. +const DefaultBranchPattern = "~DEFAULT_BRANCH" + // DefaultRequiredCheck is the check context the `required` job of a caller's `ci` job reports. const DefaultRequiredCheck = "ci / required" @@ -120,6 +133,8 @@ type Config struct { RequiredCheckAppID int64 // BypassAppID is the ccf-release-bot app's ID, the only actor that bypasses ccf-review. BypassAppID int64 + // RotateSecrets rewrites every environment secret (Environment.RotateSecrets). + RotateSecrets bool } // WorkflowPermissions are a repo's GITHUB_TOKEN defaults (GET/PUT @@ -140,6 +155,28 @@ type ActionsSettings struct { ForkPRApproval string } +// Environment is a deployment environment this tool manages: which refs may use it and which +// secrets it holds. Its other protection rules (reviewers, wait timer) are not managed. +type Environment struct { + Name string + // Branches and Tags are the only refs whose jobs may use it (fnmatch patterns, as GitHub's + // deployment branch and tag policies); DefaultBranchPattern is the repo's default branch. + Branches, Tags []string + // Secrets must exist in it; missing ones are written from Options.Secrets. + Secrets []string + // RotateSecrets writes every secret, not only the missing ones (after a key rotation). + RotateSecrets bool +} + +// DeploymentPolicy is a deployment branch or tag policy of an environment. +type DeploymentPolicy struct { + ID int64 `json:"id,omitempty"` + Name string `json:"name"` + Type string `json:"type"` // "branch" or "tag" +} + +func (p DeploymentPolicy) String() string { return p.Type + ":" + p.Name } + // Desired is the state every repo should have. type Desired struct { Merge MergeSettings @@ -147,6 +184,7 @@ type Desired struct { SecurityUpdates bool Actions ActionsSettings Rulesets []Ruleset + Environments []Environment } // DesiredState builds the desired state from c. @@ -207,6 +245,13 @@ func DesiredState(c Config) (Desired, error) { Rules: []Rule{{Type: "creation"}, {Type: "update"}, {Type: "deletion"}}, }, }, + Environments: []Environment{{ + Name: ReleaseEnvironment, + Branches: []string{DefaultBranchPattern}, + Tags: slices.Clone(ReleaseTagPatterns), + Secrets: slices.Clone(ReleaseBotSecrets), + RotateSecrets: c.RotateSecrets, + }}, }, nil } diff --git a/internal/reposettings/environment_test.go b/internal/reposettings/environment_test.go new file mode 100644 index 0000000..4cfe0b4 --- /dev/null +++ b/internal/reposettings/environment_test.go @@ -0,0 +1,147 @@ +package reposettings + +import ( + "bytes" + "context" + "errors" + "slices" + "strings" + "testing" +) + +// secretValues are the release environment's secrets as the repo-settings job passes them. +var secretValues = map[string]string{"RELEASE_BOT_APP_ID": "123", "RELEASE_BOT_PRIVATE_KEY": "-----BEGIN KEY-----s3cr3t"} + +func TestEnvironmentCreated(t *testing.T) { + f := newFake("a") + opts := Options{Owner: "o", Repos: []string{"a"}, Desired: desiredAll(t, false), Secrets: secretValues} + var out bytes.Buffer + if err := Run(context.Background(), f, opts, &out); err != nil { + t.Fatal(err) + } + for _, want := range []string{ + " environment[release]: missing -> create", + " environment[release].policy[branch:main]: (unset) -> add", + " environment[release].policy[tag:v*.*.*]: (unset) -> add", + " environment[release].policy[tag:*-v*.*.*]: (unset) -> add", + " environment[release].secret.RELEASE_BOT_APP_ID: missing -> set", + " environment[release].secret.RELEASE_BOT_PRIVATE_KEY: missing -> set", + } { + if !strings.Contains(out.String(), want) { + t.Errorf("output lacks %q:\n%s", want, out.String()) + } + } + + opts.Apply = true + f.writes = nil + out.Reset() + if err := Run(context.Background(), f, opts, &out); err != nil { + t.Fatal(err) + } + var env []string + for _, w := range f.writes { + if strings.Contains(w, " env ") || strings.Contains(w, "policy") || strings.HasPrefix(w, "secret ") { + env = append(env, w) + } + } + // The policies are in place before any secret is written. + want := []string{"put env release o/a", "add policy release branch:main o/a", "add policy release tag:v*.*.* o/a", + "add policy release tag:*-v*.*.* o/a", "secret release RELEASE_BOT_APP_ID o/a", "secret release RELEASE_BOT_PRIVATE_KEY o/a"} + if !slices.Equal(env, want) { + t.Errorf("environment writes = %v, want %v", env, want) + } + if got := f.repos["o/a"].envs["release"].secrets; got["RELEASE_BOT_PRIVATE_KEY"] != secretValues["RELEASE_BOT_PRIVATE_KEY"] || got["RELEASE_BOT_APP_ID"] != "123" { + t.Errorf("secrets = %v", got) + } + if strings.Contains(out.String(), "s3cr3t") { + t.Errorf("a secret value is in the output:\n%s", out.String()) + } + + // Idempotent: the secrets exist, so nothing is written (their values can't be read back). + f.writes = nil + out.Reset() + if err := Run(context.Background(), f, opts, &out); err != nil { + t.Fatal(err) + } + if len(f.writes) > 0 || !strings.Contains(out.String(), "o/a: up to date") { + t.Errorf("second apply: writes = %v, output:\n%s", f.writes, out.String()) + } +} + +// TestEnvironmentTightened: an environment every branch may use is switched to the desired +// policies, a stray policy is deleted, and a present secret is left as it is. +func TestEnvironmentTightened(t *testing.T) { + f := inSync(t) + r := f.repos["o/a"] + r.envs = map[string]*fakeEnv{"release": {secrets: map[string]string{"RELEASE_BOT_APP_ID": "old", "OTHER": "x"}}} + opts := Options{Owner: "o", Repos: []string{"a"}, Desired: desiredAll(t, false), Secrets: secretValues, Apply: true} + var out bytes.Buffer + if err := Run(context.Background(), f, opts, &out); err != nil { + t.Fatal(err) + } + if !strings.Contains(out.String(), " environment[release].deployment_branch_policy: all branches -> custom") { + t.Errorf("output:\n%s", out.String()) + } + e := r.envs["release"] + if !e.custom || len(e.policies) != 3 || e.secrets["RELEASE_BOT_APP_ID"] != "old" || e.secrets["RELEASE_BOT_PRIVATE_KEY"] == "" || e.secrets["OTHER"] != "x" { + t.Errorf("environment = %+v", e) + } + + e.policies = append(e.policies, DeploymentPolicy{ID: 900, Name: "*", Type: "branch"}) + f.writes = nil + out.Reset() + if err := Run(context.Background(), f, opts, &out); err != nil { + t.Fatal(err) + } + if !slices.Equal(f.writes, []string{"delete policy release 900 o/a"}) || !strings.Contains(out.String(), "environment[release].policy[branch:*]: present -> delete") { + t.Errorf("writes = %v, output:\n%s", f.writes, out.String()) + } +} + +func TestEnvironmentRotateSecrets(t *testing.T) { + f := inSync(t) + f.repos["o/a"].envs = map[string]*fakeEnv{"release": {custom: true, secrets: map[string]string{"RELEASE_BOT_APP_ID": "old", "RELEASE_BOT_PRIVATE_KEY": "old"}}} + opts := Options{Owner: "o", Repos: []string{"a"}, Desired: desiredAll(t, true), Secrets: secretValues, Apply: true} + for _, p := range []string{"main", "v*.*.*", "*-v*.*.*"} { + typ := map[bool]string{true: "branch", false: "tag"}[p == "main"] + f.repos["o/a"].envs["release"].policies = append(f.repos["o/a"].envs["release"].policies, DeploymentPolicy{ID: int64(len(p)), Name: p, Type: typ}) + } + var out bytes.Buffer + if err := Run(context.Background(), f, opts, &out); err != nil { + t.Fatal(err) + } + if want := []string{"secret release RELEASE_BOT_APP_ID o/a", "secret release RELEASE_BOT_PRIVATE_KEY o/a"}; !slices.Equal(f.writes, want) { + t.Errorf("writes = %v, want %v", f.writes, want) + } + if !strings.Contains(out.String(), "environment[release].secret.RELEASE_BOT_PRIVATE_KEY: present -> rotate") || + f.repos["o/a"].envs["release"].secrets["RELEASE_BOT_APP_ID"] != "123" { + t.Errorf("output:\n%s", out.String()) + } +} + +// TestEnvironmentSecretsWaitForPolicies: a secret is never written to an environment whose +// policies failed, nor without a value. +func TestEnvironmentSecretsWaitForPolicies(t *testing.T) { + f := inSync(t) + f.fail = map[string]error{"add policy release tag:v*.*.* o/a": errors.New("422 boom")} + opts := Options{Owner: "o", Repos: []string{"a"}, Desired: desiredAll(t, false), Secrets: secretValues, Apply: true} + var out bytes.Buffer + err := Run(context.Background(), f, opts, &out) + if err == nil || !strings.Contains(out.String(), "environment[release].secret.RELEASE_BOT_PRIVATE_KEY failed: skipped: the environment's deployment policies are not in place") { + t.Fatalf("err = %v, output:\n%s", err, out.String()) + } + if len(f.repos["o/a"].envs["release"].secrets) > 0 { + t.Errorf("secrets written: %v", f.repos["o/a"].envs["release"].secrets) + } + + f.fail = nil + opts.Secrets = map[string]string{"RELEASE_BOT_APP_ID": "123"} + out.Reset() + err = Run(context.Background(), f, opts, &out) + if err == nil || !strings.Contains(out.String(), "environment[release].secret.RELEASE_BOT_PRIVATE_KEY failed: no value for RELEASE_BOT_PRIVATE_KEY") { + t.Fatalf("err = %v, output:\n%s", err, out.String()) + } + if s := f.repos["o/a"].envs["release"].secrets; s["RELEASE_BOT_APP_ID"] != "123" || s["RELEASE_BOT_PRIVATE_KEY"] != "" { + t.Errorf("secrets = %v", s) + } +} diff --git a/internal/reposettings/github.go b/internal/reposettings/github.go index 10e615a..af16eff 100644 --- a/internal/reposettings/github.go +++ b/internal/reposettings/github.go @@ -3,6 +3,8 @@ package reposettings import ( "bytes" "context" + "crypto/rand" + "encoding/base64" "encoding/json" "errors" "fmt" @@ -12,10 +14,13 @@ import ( "slices" "strings" "time" + + "golang.org/x/crypto/nacl/box" ) -// GitHub reads and writes repo settings with the GitHub REST API. Reads need Administration read; -// writes need Administration write. +// GitHub reads and writes repo settings with the GitHub REST API. Reads need Administration read, +// Actions read (environments and their policies) and Environments read (their secrets' names); +// writes need Administration write and Environments write (secrets). type GitHub struct { BaseURL string // GITHUB_API_URL Token string @@ -215,6 +220,120 @@ func (g *GitHub) SetForkPRApproval(ctx context.Context, repo, policy string) err return g.do(ctx, http.MethodPut, repoPath(repo, "/actions/permissions/fork-pr-contributor-approval"), map[string]string{"approval_policy": policy}, nil) } +// DefaultBranch reads the repo's default branch. +func (g *GitHub) DefaultBranch(ctx context.Context, repo string) (string, error) { + var r struct { + DefaultBranch string `json:"default_branch"` + } + if err := g.do(ctx, http.MethodGet, repoPath(repo, ""), nil, &r); err != nil { + return "", err + } + if r.DefaultBranch == "" { + return "", fmt.Errorf("%s: no default branch in the response", repo) + } + return r.DefaultBranch, nil +} + +// envPath is /repos/{owner}/{name}/environments/{env}{suffix}. +func envPath(repo, env, suffix string) string { + return repoPath(repo, "/environments/"+url.PathEscape(env)+suffix) +} + +// permissionHint names the app permissions an environment call needs when GitHub refuses it. +func permissionHint(err error) error { + if isForbidden(err) { + return fmt.Errorf("%w (the token needs Actions read and Environments read, or write to apply: README, Repo settings)", err) + } + return err +} + +// Environment reads the named environment; a 404 means the repo has none by that name. +func (g *GitHub) Environment(ctx context.Context, repo, name string) (*CurrentEnvironment, error) { + var e CurrentEnvironment + err := g.do(ctx, http.MethodGet, envPath(repo, name, ""), nil, &e) + if isNotFound(err) { + return nil, nil + } + if err != nil { + return nil, permissionHint(err) + } + return &e, nil +} + +func (g *GitHub) DeploymentPolicies(ctx context.Context, repo, env string) ([]DeploymentPolicy, error) { + var r struct { + TotalCount int `json:"total_count"` + BranchPolicies []DeploymentPolicy `json:"branch_policies"` + } + if err := g.do(ctx, http.MethodGet, envPath(repo, env, fmt.Sprintf("/deployment-branch-policies?per_page=%d", perPage)), nil, &r); err != nil { + return nil, permissionHint(err) + } + if r.TotalCount > len(r.BranchPolicies) { + return nil, fmt.Errorf("%s: environment %s has %d deployment policies, more than one page", repo, env, r.TotalCount) + } + return r.BranchPolicies, nil +} + +func (g *GitHub) EnvironmentSecrets(ctx context.Context, repo, env string) ([]string, error) { + var r struct { + TotalCount int `json:"total_count"` + Secrets []struct { + Name string `json:"name"` + } `json:"secrets"` + } + if err := g.do(ctx, http.MethodGet, envPath(repo, env, fmt.Sprintf("/secrets?per_page=%d", perPage)), nil, &r); err != nil { + return nil, permissionHint(err) + } + if r.TotalCount > len(r.Secrets) { + return nil, fmt.Errorf("%s: environment %s has %d secrets, more than one page", repo, env, r.TotalCount) + } + names := make([]string, len(r.Secrets)) + for i, s := range r.Secrets { + names[i] = s.Name + } + return names, nil +} + +// PutEnvironment creates the environment or switches it to custom branch and tag policies. Only +// the deployment branch policy is sent. +func (g *GitHub) PutEnvironment(ctx context.Context, repo, name string) error { + body := map[string]any{"deployment_branch_policy": map[string]bool{"protected_branches": false, "custom_branch_policies": true}} + return permissionHint(g.do(ctx, http.MethodPut, envPath(repo, name, ""), body, nil)) +} + +func (g *GitHub) AddDeploymentPolicy(ctx context.Context, repo, env string, p DeploymentPolicy) error { + body := DeploymentPolicy{Name: p.Name, Type: p.Type} + return permissionHint(g.do(ctx, http.MethodPost, envPath(repo, env, "/deployment-branch-policies"), body, nil)) +} + +func (g *GitHub) DeleteDeploymentPolicy(ctx context.Context, repo, env string, id int64) error { + return permissionHint(g.do(ctx, http.MethodDelete, envPath(repo, env, fmt.Sprintf("/deployment-branch-policies/%d", id)), nil, nil)) +} + +// SetEnvironmentSecret encrypts value with the environment's public key (a libsodium sealed box, +// as GitHub requires) and writes it. Neither the value nor the ciphertext is ever in an error. +func (g *GitHub) SetEnvironmentSecret(ctx context.Context, repo, env, name, value string) error { + var key struct { + KeyID string `json:"key_id"` + Key string `json:"key"` + } + if err := g.do(ctx, http.MethodGet, envPath(repo, env, "/secrets/public-key"), nil, &key); err != nil { + return permissionHint(err) + } + raw, err := base64.StdEncoding.DecodeString(key.Key) + if err != nil || len(raw) != 32 || key.KeyID == "" { + return fmt.Errorf("%s: environment %s: unexpected public key (id %q, %d bytes)", repo, env, key.KeyID, len(raw)) + } + var pub [32]byte + copy(pub[:], raw) + sealed, err := box.SealAnonymous(nil, []byte(value), &pub, rand.Reader) + if err != nil { + return fmt.Errorf("%s: environment %s: encrypting %s: %w", repo, env, name, err) + } + body := map[string]string{"encrypted_value": base64.StdEncoding.EncodeToString(sealed), "key_id": key.KeyID} + return permissionHint(g.do(ctx, http.MethodPut, envPath(repo, env, "/secrets/"+url.PathEscape(name)), body, nil)) +} + func onOff(enabled bool) string { if enabled { return http.MethodPut diff --git a/internal/reposettings/github_test.go b/internal/reposettings/github_test.go index 4ea18b2..0ee5beb 100644 --- a/internal/reposettings/github_test.go +++ b/internal/reposettings/github_test.go @@ -3,6 +3,8 @@ package reposettings import ( "bytes" "context" + "crypto/rand" + "encoding/base64" "encoding/json" "io" "net/http" @@ -12,6 +14,8 @@ import ( "strings" "sync" "testing" + + "golang.org/x/crypto/nacl/box" ) // defaultRepoJSON is GET /repos/o/a for a token that can read the merge settings. @@ -326,3 +330,97 @@ func TestGitHubRulesetBypassAbsentVsEmpty(t *testing.T) { }) } } + +func TestGitHubEnvironment(t *testing.T) { + pub, priv, err := box.GenerateKey(rand.Reader) + if err != nil { + t.Fatal(err) + } + const env = "/repos/o/a/environments/release" + var mu sync.Mutex + writes := map[string]string{} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + key := r.Method + " " + r.URL.Path + if r.Method != http.MethodGet { + mu.Lock() + writes[key] = string(body) + mu.Unlock() + w.WriteHeader(http.StatusCreated) + return + } + switch key { + case "GET /repos/o/a": + _, _ = io.WriteString(w, `{"default_branch":"trunk"}`) + case "GET " + env: + _, _ = io.WriteString(w, `{"name":"release","deployment_branch_policy":{"protected_branches":false,"custom_branch_policies":true}}`) + case "GET " + env + "/deployment-branch-policies": + _, _ = io.WriteString(w, `{"total_count":2,"branch_policies":[{"id":1,"name":"trunk","type":"branch"},{"id":2,"name":"v*.*.*","type":"tag"}]}`) + case "GET " + env + "/secrets": + _, _ = io.WriteString(w, `{"total_count":1,"secrets":[{"name":"RELEASE_BOT_APP_ID","created_at":"2026-10-08T00:00:00Z"}]}`) + case "GET " + env + "/secrets/public-key": + _, _ = io.WriteString(w, `{"key_id":"k1","key":"`+base64.StdEncoding.EncodeToString(pub[:])+`"}`) + case "GET /repos/o/a/environments/denied": + http.Error(w, `{"message":"Resource not accessible by integration"}`, http.StatusForbidden) + default: + http.NotFound(w, r) + } + })) + defer srv.Close() + gh := &GitHub{BaseURL: srv.URL, Token: "tok"} + ctx := context.Background() + + if b, err := gh.DefaultBranch(ctx, "o/a"); err != nil || b != "trunk" { + t.Errorf("DefaultBranch = %q, %v", b, err) + } + e, err := gh.Environment(ctx, "o/a", "release") + if err != nil || e == nil || !e.Custom() { + t.Errorf("Environment = %+v, %v", e, err) + } + if e, err := gh.Environment(ctx, "o/a", "missing"); err != nil || e != nil { + t.Errorf("Environment(missing) = %+v, %v", e, err) + } + if _, err := gh.Environment(ctx, "o/a", "denied"); err == nil || !strings.Contains(err.Error(), "Environments read") { + t.Errorf("Environment(denied) err = %v, want the permission hint", err) + } + if p, err := gh.DeploymentPolicies(ctx, "o/a", "release"); err != nil || len(p) != 2 || p[1].String() != "tag:v*.*.*" || p[1].ID != 2 { + t.Errorf("DeploymentPolicies = %+v, %v", p, err) + } + if s, err := gh.EnvironmentSecrets(ctx, "o/a", "release"); err != nil || !slices.Equal(s, []string{"RELEASE_BOT_APP_ID"}) { + t.Errorf("EnvironmentSecrets = %v, %v", s, err) + } + + for _, err := range []error{ + gh.PutEnvironment(ctx, "o/a", "release"), + gh.AddDeploymentPolicy(ctx, "o/a", "release", DeploymentPolicy{ID: 9, Name: "*-v*.*.*", Type: "tag"}), + gh.DeleteDeploymentPolicy(ctx, "o/a", "release", 2), + gh.SetEnvironmentSecret(ctx, "o/a", "release", "RELEASE_BOT_PRIVATE_KEY", "s3cr3t"), + } { + if err != nil { + t.Fatal(err) + } + } + for k, want := range map[string]string{ + "PUT " + env: `{"deployment_branch_policy":{"custom_branch_policies":true,"protected_branches":false}}`, + "POST " + env + "/deployment-branch-policies": `{"name":"*-v*.*.*","type":"tag"}`, + "DELETE " + env + "/deployment-branch-policies/2": ``, + } { + if writes[k] != want { + t.Errorf("%s body = %q, want %q", k, writes[k], want) + } + } + var sent struct { + EncryptedValue string `json:"encrypted_value"` + KeyID string `json:"key_id"` + } + if err := json.Unmarshal([]byte(writes["PUT "+env+"/secrets/RELEASE_BOT_PRIVATE_KEY"]), &sent); err != nil { + t.Fatal(err) + } + sealed, _ := base64.StdEncoding.DecodeString(sent.EncryptedValue) + if plain, ok := box.OpenAnonymous(nil, sealed, pub, priv); !ok || string(plain) != "s3cr3t" || sent.KeyID != "k1" { + t.Errorf("secret = %q (decrypted %v), key %q", plain, ok, sent.KeyID) + } + if strings.Contains(writes["PUT "+env+"/secrets/RELEASE_BOT_PRIVATE_KEY"], "s3cr3t") { + t.Error("the secret was sent in clear") + } +} diff --git a/internal/reposettings/plan.go b/internal/reposettings/plan.go index 50c91c1..cf1c05d 100644 --- a/internal/reposettings/plan.go +++ b/internal/reposettings/plan.go @@ -1,6 +1,7 @@ package reposettings import ( + "cmp" "context" "encoding/json" "errors" @@ -30,6 +31,40 @@ type Reader interface { // ForkPRApproval returns the fork PR contributor approval policy, or "" if the repo has none // (a private repo). ForkPRApproval(ctx context.Context, repo string) (string, error) + DefaultBranch(ctx context.Context, repo string) (string, error) + // Environment returns the named environment, or nil if the repo has none by that name. + Environment(ctx context.Context, repo, name string) (*CurrentEnvironment, error) + // DeploymentPolicies lists an environment's branch and tag policies; it has some only with + // custom policies (CurrentEnvironment.Custom). + DeploymentPolicies(ctx context.Context, repo, env string) ([]DeploymentPolicy, error) + // EnvironmentSecrets lists the names of an environment's secrets (never their values). + EnvironmentSecrets(ctx context.Context, repo, env string) ([]string, error) +} + +// CurrentEnvironment is an environment as GET /repos/{owner}/{repo}/environments/{name} returns +// it. A nil DeploymentBranchPolicy lets every branch use it. +type CurrentEnvironment struct { + DeploymentBranchPolicy *struct { + ProtectedBranches bool `json:"protected_branches"` + CustomBranchPolicies bool `json:"custom_branch_policies"` + } `json:"deployment_branch_policy"` +} + +// Custom reports whether only the environment's branch and tag policies may use it. +func (e *CurrentEnvironment) Custom() bool { + return e.DeploymentBranchPolicy != nil && e.DeploymentBranchPolicy.CustomBranchPolicies +} + +// policyMode describes the environment's deployment branch policy. +func (e *CurrentEnvironment) policyMode() string { + switch { + case e.DeploymentBranchPolicy == nil: + return "all branches" + case e.DeploymentBranchPolicy.ProtectedBranches: + return "protected branches" + default: + return "custom" + } } // SecurityConfiguration is the part of an org code security configuration this tool checks. Each @@ -51,6 +86,12 @@ type Writer interface { UpdateRuleset(ctx context.Context, repo string, id int64, r Ruleset) error SetWorkflowPermissions(ctx context.Context, repo string, p WorkflowPermissions) error SetForkPRApproval(ctx context.Context, repo, policy string) error + // PutEnvironment creates the environment, or switches it, to custom branch and tag policies + // (none yet: no ref may use it until AddDeploymentPolicy). + PutEnvironment(ctx context.Context, repo, name string) error + AddDeploymentPolicy(ctx context.Context, repo, env string, p DeploymentPolicy) error + DeleteDeploymentPolicy(ctx context.Context, repo, env string, id int64) error + SetEnvironmentSecret(ctx context.Context, repo, env, name, value string) error } // Client reads and writes repo settings. @@ -101,6 +142,17 @@ type Plan struct { workflowPermissions *WorkflowPermissions forkPRApproval *string rulesets []rulesetWrite + environments []environmentWrite +} + +// environmentWrite is what Apply does to one environment, in this order: put it (create, or +// switch to custom policies), delete and add policies, then write the secrets, so a secret never +// lands in an environment that more refs may use than the desired ones. +type environmentWrite struct { + name string + put bool + delete, add []DeploymentPolicy + secrets []string } type rulesetWrite struct { @@ -163,6 +215,9 @@ func PlanRepo(ctx context.Context, r Reader, repo string, d Desired) (*Plan, err if err := p.planActions(ctx, r, d.Actions); err != nil { return nil, err } + if err := p.planEnvironments(ctx, r, d.Environments); err != nil { + return nil, err + } current, err := r.Rulesets(ctx, repo) if err != nil { @@ -230,6 +285,87 @@ func (p *Plan) planActions(ctx context.Context, r Reader, want ActionsSettings) return nil } +// planEnvironments compares the environments with want. +func (p *Plan) planEnvironments(ctx context.Context, r Reader, want []Environment) error { + if len(want) == 0 { + return nil + } + branch, err := r.DefaultBranch(ctx, p.Repo) + if err != nil { + return err + } + for _, e := range want { + prefix := "environment[" + e.Name + "]" + w := environmentWrite{name: e.Name} + var policies []DeploymentPolicy + for _, b := range e.Branches { + if b == DefaultBranchPattern { + b = branch + } + policies = append(policies, DeploymentPolicy{Name: b, Type: "branch"}) + } + for _, t := range e.Tags { + policies = append(policies, DeploymentPolicy{Name: t, Type: "tag"}) + } + + cur, err := r.Environment(ctx, p.Repo, e.Name) + if err != nil { + return err + } + var current []DeploymentPolicy + var secrets []string + switch { + case cur == nil: + p.Changes = append(p.Changes, Change{Key: prefix, Current: "missing", Desired: "create"}) + w.put = true + case !cur.Custom(): + p.Changes = append(p.Changes, Change{Key: prefix + ".deployment_branch_policy", Current: cur.policyMode(), Desired: "custom"}) + w.put = true + fallthrough + default: + if cur.Custom() { + if current, err = r.DeploymentPolicies(ctx, p.Repo, e.Name); err != nil { + return err + } + } + if secrets, err = r.EnvironmentSecrets(ctx, p.Repo, e.Name); err != nil { + return err + } + } + + for _, c := range current { + if c.Type == "" { + c.Type = "branch" // policies made before tag policies existed have no type + } + if !slices.ContainsFunc(policies, func(d DeploymentPolicy) bool { return d.Name == c.Name && d.Type == c.Type }) { + p.Changes = append(p.Changes, Change{Key: prefix + ".policy[" + c.String() + "]", Current: "present", Desired: "delete"}) + w.delete = append(w.delete, c) + } + } + for _, d := range policies { + if !slices.ContainsFunc(current, func(c DeploymentPolicy) bool { return c.Name == d.Name && cmp.Or(c.Type, "branch") == d.Type }) { + p.Changes = append(p.Changes, Change{Key: prefix + ".policy[" + d.String() + "]", Current: unset, Desired: "add"}) + w.add = append(w.add, d) + } + } + for _, s := range e.Secrets { + switch { + case !slices.Contains(secrets, s): + p.Changes = append(p.Changes, Change{Key: prefix + ".secret." + s, Current: "missing", Desired: "set"}) + case e.RotateSecrets: + p.Changes = append(p.Changes, Change{Key: prefix + ".secret." + s, Current: "present", Desired: "rotate"}) + default: + continue + } + w.secrets = append(w.secrets, s) + } + if w.put || len(w.delete) > 0 || len(w.add) > 0 || len(w.secrets) > 0 { + p.environments = append(p.environments, w) + } + } + return nil +} + // stateName names a bool security setting as GitHub does. var stateName = map[bool]string{true: "enabled", false: "disabled"} @@ -283,9 +419,11 @@ type StepResult struct { } // Apply writes the changes in p, one step per setting: the merge settings, each ruleset, the -// Actions settings, then the security settings, which an org configuration can refuse. Steps are independent: a failed step -// doesn't stop the others. Apply returns every step's result, in order. -func Apply(ctx context.Context, w Writer, p *Plan) []StepResult { +// Actions settings, each environment, then the security settings, which an org configuration can +// refuse. Steps are independent: a failed step doesn't stop the others, except that an +// environment's secrets are skipped if its policies failed. secrets holds the secrets' values; +// they are never printed. Apply returns every step's result, in order. +func Apply(ctx context.Context, w Writer, p *Plan, secrets map[string]string) []StepResult { var results []StepResult step := func(name string, err error) { results = append(results, StepResult{Step: name, Err: err}) } if p.merge != nil { @@ -305,6 +443,34 @@ func Apply(ctx context.Context, w Writer, p *Plan) []StepResult { if p.forkPRApproval != nil { step("actions.fork_pr_approval", w.SetForkPRApproval(ctx, p.Repo, *p.forkPRApproval)) } + for _, e := range p.environments { + prefix := "environment[" + e.name + "]" + ok := true + policyStep := func(name string, err error) { + step(name, err) + ok = ok && err == nil + } + if e.put { + policyStep(prefix, w.PutEnvironment(ctx, p.Repo, e.name)) + } + for _, d := range e.delete { + policyStep(prefix+".policy["+d.String()+"] delete", w.DeleteDeploymentPolicy(ctx, p.Repo, e.name, d.ID)) + } + for _, d := range e.add { + policyStep(prefix+".policy["+d.String()+"]", w.AddDeploymentPolicy(ctx, p.Repo, e.name, d)) + } + for _, s := range e.secrets { + name := prefix + ".secret." + s + switch v := secrets[s]; { + case !ok: + step(name, errors.New("skipped: the environment's deployment policies are not in place")) + case v == "": + step(name, fmt.Errorf("no value for %s: set it in the job's environment (README: Repo settings)", s)) + default: + step(name, w.SetEnvironmentSecret(ctx, p.Repo, e.name, s, v)) + } + } + } if p.vulnerabilityAlerts != nil { step("security.vulnerability_alerts", w.SetVulnerabilityAlerts(ctx, p.Repo, *p.vulnerabilityAlerts)) } @@ -324,6 +490,9 @@ type Options struct { Apply bool // CheckTokenScope fails before reading any repo if the token reaches a repo outside Repos. CheckTokenScope bool + // Secrets are the environment secrets' values, by name; Apply writes the missing ones (all of + // them with Environment.RotateSecrets). A dry run needs none. + Secrets map[string]string } // Run prints each repo's diff to out and, with o.Apply, writes the changes. A failure on one repo, @@ -383,7 +552,7 @@ func Run(ctx context.Context, gh Client, o Options, out io.Writer) error { continue } var failed []error - results := Apply(ctx, gh, p) + results := Apply(ctx, gh, p, o.Secrets) for _, r := range results { if r.Err != nil { fmt.Fprintf(out, "%s: %s failed: %v\n", repo, r.Step, r.Err) diff --git a/internal/reposettings/plan_test.go b/internal/reposettings/plan_test.go index 5af6294..4455bd2 100644 --- a/internal/reposettings/plan_test.go +++ b/internal/reposettings/plan_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "fmt" + "maps" "slices" "strings" "testing" @@ -27,6 +28,14 @@ type fakeRepo struct { perms WorkflowPermissions // forkPolicy is the fork PR approval policy ("": none, as for a private repo). forkPolicy string + envs map[string]*fakeEnv +} + +// fakeEnv is an environment: custom is false for one every branch may use. +type fakeEnv struct { + custom bool + policies []DeploymentPolicy + secrets map[string]string // name -> value } // fakeGitHub keeps repo state in memory, records every call and applies writes to the state. Rulesets @@ -150,6 +159,83 @@ func (f *fakeGitHub) SetForkPRApproval(_ context.Context, repo, policy string) e return err } +func (f *fakeGitHub) DefaultBranch(_ context.Context, repo string) (string, error) { + _, err := f.repo("default branch", repo, false) + return "main", err +} + +func (f *fakeGitHub) Environment(_ context.Context, repo, name string) (*CurrentEnvironment, error) { + r, err := f.repo("environment "+name, repo, false) + if err != nil || r.envs[name] == nil { + return nil, err + } + var e CurrentEnvironment + if r.envs[name].custom { + _ = json.Unmarshal([]byte(`{"deployment_branch_policy":{"protected_branches":false,"custom_branch_policies":true}}`), &e) + } + return &e, nil +} + +func (f *fakeGitHub) DeploymentPolicies(_ context.Context, repo, env string) ([]DeploymentPolicy, error) { + r, err := f.repo("policies "+env, repo, false) + if err != nil { + return nil, err + } + if e := r.envs[env]; e == nil || !e.custom { + return nil, fmt.Errorf("%s: 404, no custom policies", env) // as GitHub answers + } + return slices.Clone(r.envs[env].policies), nil +} + +func (f *fakeGitHub) EnvironmentSecrets(_ context.Context, repo, env string) ([]string, error) { + r, err := f.repo("secrets "+env, repo, false) + if err != nil || r.envs[env] == nil { + return nil, err + } + return slices.Sorted(maps.Keys(r.envs[env].secrets)), nil +} + +func (f *fakeGitHub) PutEnvironment(_ context.Context, repo, name string) error { + r, err := f.repo("put env "+name, repo, true) + if err != nil { + return err + } + if r.envs == nil { + r.envs = map[string]*fakeEnv{} + } + if r.envs[name] == nil { + r.envs[name] = &fakeEnv{secrets: map[string]string{}} + } + r.envs[name].custom = true + return nil +} + +func (f *fakeGitHub) AddDeploymentPolicy(_ context.Context, repo, env string, p DeploymentPolicy) error { + r, err := f.repo("add policy "+env+" "+p.String(), repo, true) + if err == nil { + f.nextID++ + p.ID = f.nextID + r.envs[env].policies = append(r.envs[env].policies, p) + } + return err +} + +func (f *fakeGitHub) DeleteDeploymentPolicy(_ context.Context, repo, env string, id int64) error { + r, err := f.repo(fmt.Sprintf("delete policy %s %d", env, id), repo, true) + if err == nil { + r.envs[env].policies = slices.DeleteFunc(r.envs[env].policies, func(p DeploymentPolicy) bool { return p.ID == id }) + } + return err +} + +func (f *fakeGitHub) SetEnvironmentSecret(_ context.Context, repo, env, name, value string) error { + r, err := f.repo("secret "+env+" "+name, repo, true) + if err == nil { + r.envs[env].secrets[name] = value + } + return err +} + func (f *fakeGitHub) UpdateMergeSettings(_ context.Context, repo string, s MergeSettings) error { r, err := f.repo("PATCH repo", repo, true) if err == nil { @@ -232,9 +318,19 @@ func newFake(names ...string) *fakeGitHub { return f } +// desired is the desired state without its environments, which TestEnvironment* cover (with +// secret values). func desired(t *testing.T) Desired { t.Helper() - d, err := DesiredState(Config{RequiredCheck: DefaultRequiredCheck, RequiredCheckAppID: GitHubActionsAppID, BypassAppID: 42}) + d := desiredAll(t, false) + d.Environments = nil + return d +} + +// desiredAll is the full desired state, environments included. +func desiredAll(t *testing.T, rotate bool) Desired { + t.Helper() + d, err := DesiredState(Config{RequiredCheck: DefaultRequiredCheck, RequiredCheckAppID: GitHubActionsAppID, BypassAppID: 42, RotateSecrets: rotate}) if err != nil { t.Fatal(err) } From 7ded49c636195ae57dcd73481faad999e504b5fb Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Thu, 8 Oct 2026 18:25:06 -0300 Subject: [PATCH 2/2] fix(ci): pass ci / required when release-checks is skipped A called workflow whose jobs all skip reports skipped, not success, so the required job failed on every PR that isn't a release-please PR (where release-checks skips). Skipped is accepted for release-checks only; any other result but success still fails. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 6 ++++-- internal/ciworkflows/steps_test.go | 23 ++++++++++++++++++++--- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d283f97..d789a94 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -69,7 +69,8 @@ jobs: # The ccf-required ruleset (repo-settings) requires the check `ci / required`, which a caller's # `ci` job gets from the kind workflows' `required` job. This repo calls no kind workflow, so the - # job carries that name itself. The same check as the kind workflows: anything but success fails. + # job carries that name itself. Anything but success fails, except release-checks being skipped: + # it only runs on release-please PRs, and a called workflow whose jobs all skip reports skipped. required: name: ci / required needs: [go, actionlint, release-checks] @@ -82,7 +83,8 @@ jobs: env: NEEDS: ${{ toJSON(needs) }} run: | - failed="$(jq -r 'to_entries[] | select(.value.result != "success") | "\(.key): \(.value.result)"' <<< "$NEEDS")" + failed="$(jq -r 'to_entries[] | select(.value.result != "success") + | select(.key != "release-checks" or .value.result != "skipped") | "\(.key): \(.value.result)"' <<< "$NEEDS")" if [ -n "$failed" ]; then echo "::error::required jobs did not succeed:" echo "$failed" diff --git a/internal/ciworkflows/steps_test.go b/internal/ciworkflows/steps_test.go index ffb444a..29b6447 100644 --- a/internal/ciworkflows/steps_test.go +++ b/internal/ciworkflows/steps_test.go @@ -477,7 +477,8 @@ func TestGoreleaserCheck(t *testing.T) { } // TestSelfRequired: this repo's own ci.yml reports `ci / required` (what ccf-required requires) -// with the kind workflows' check, after every other job. +// after every other job: anything but success fails, except release-checks skipped (every PR but +// a release-please one, and every push). func TestSelfRequired(t *testing.T) { var wf struct { Jobs map[string]struct { @@ -498,7 +499,23 @@ func TestSelfRequired(t *testing.T) { if got := slices.Sorted(slices.Values(req.Needs)); req.Name != "ci / required" || !slices.Equal(got, others) || req.If != "${{ always() }}" { t.Errorf("ci.yml required = %+v, want name ci / required, needs %v, if always()", req, others) } - if script(t, "ci.yml", "required", "Check the results") != script(t, goPlugin, "required", "Check the results") { - t.Error("ci.yml's required check differs from the kind workflows'") + need(t, "jq") + src := script(t, "ci.yml", "required", "Check the results") + for _, tc := range []struct { + name, needs, want string + failed bool + }{ + {"release PR", `{"go":{"result":"success"},"actionlint":{"result":"success"},"release-checks":{"result":"success"}}`, "All required jobs succeeded.", false}, + {"other PR", `{"go":{"result":"success"},"actionlint":{"result":"success"},"release-checks":{"result":"skipped"}}`, "All required jobs succeeded.", false}, + {"release-checks failed", `{"go":{"result":"success"},"actionlint":{"result":"success"},"release-checks":{"result":"failure"}}`, "release-checks: failure", true}, + {"go skipped", `{"go":{"result":"skipped"},"actionlint":{"result":"success"},"release-checks":{"result":"skipped"}}`, "go: skipped", true}, + {"cancelled", `{"go":{"result":"success"},"actionlint":{"result":"cancelled"},"release-checks":{"result":"skipped"}}`, "actionlint: cancelled", true}, + } { + t.Run(tc.name, func(t *testing.T) { + r := run(t, t.TempDir(), src, "NEEDS="+tc.needs) + if r.failed != tc.failed || !strings.Contains(r.out, tc.want) { + t.Fatalf("failed=%v, want %v; output:\n%s", r.failed, tc.failed, r.out) + } + }) } }